From d62793d1fda9aeb4ec0d4f6d3163ccf9297fec01 Mon Sep 17 00:00:00 2001 From: Binbin Li Date: Tue, 2 Apr 2024 09:50:48 +0000 Subject: [PATCH] feat: add Policies interface to wrap operations on namespaced policies --- pkg/controllers/policy_controller.go | 32 +------ pkg/controllers/policy_controller_test.go | 61 ------------ pkg/controllers/resource_map.go | 13 ++- pkg/customresources/policies/api.go | 33 +++++++ pkg/customresources/policies/policies.go | 77 +++++++++++++++ pkg/customresources/policies/policies_test.go | 95 +++++++++++++++++++ pkg/customresources/verifiers/verifiers.go | 6 +- pkg/executor/core/executor.go | 3 + pkg/manager/manager.go | 11 +-- 9 files changed, 230 insertions(+), 101 deletions(-) create mode 100644 pkg/customresources/policies/api.go create mode 100644 pkg/customresources/policies/policies.go create mode 100644 pkg/customresources/policies/policies_test.go diff --git a/pkg/controllers/policy_controller.go b/pkg/controllers/policy_controller.go index 5f29a55e1..dfc2eebf9 100644 --- a/pkg/controllers/policy_controller.go +++ b/pkg/controllers/policy_controller.go @@ -38,17 +38,6 @@ type PolicyReconciler struct { Scheme *runtime.Scheme } -type policy struct { - // The name of the policy. - Name string - // The policy enforcer making a decision. - Enforcer policyprovider.PolicyProvider -} - -// ActivePolicy is the active policy generated from CRD. There would be exactly -// one active policy at any given time. -var ActivePolicy policy - //+kubebuilder:rbac:groups=config.ratify.deislabs.io,resources=policies,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=config.ratify.deislabs.io,resources=policies/status,verbs=get;update;patch //+kubebuilder:rbac:groups=config.ratify.deislabs.io,resources=policies/finalizers,verbs=update @@ -66,9 +55,10 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr policyLogger.Infof("Reconciling Policy %s", resource) if err := r.Get(ctx, req.NamespacedName, &policy); err != nil { - if apierrors.IsNotFound(err) && resource == ActivePolicy.Name { + if apierrors.IsNotFound(err) { policyLogger.Infof("delete event detected, removing policy %s", resource) - ActivePolicy.deletePolicy(resource) + // TODO: pass the actual namespace once multi-tenancy is supported. + ActivePolicies.DeletePolicy(constants.EmptyNamespace, resource) } else { policyLogger.Error("failed to get Policy: ", err) } @@ -105,8 +95,8 @@ func policyAddOrReplace(spec configv1beta1.PolicySpec) error { return fmt.Errorf("failed to create policy enforcer: %w", err) } - ActivePolicy.Name = spec.Type - ActivePolicy.Enforcer = policyEnforcer + // TODO: pass the actual namespace once multi-tenancy is supported. + ActivePolicies.AddPolicy(constants.EmptyNamespace, constants.RatifyPolicy, policyEnforcer) return nil } @@ -141,18 +131,6 @@ func rawToPolicyConfig(raw []byte, policyName string) (config.PoliciesConfig, er }, nil } -func (p *policy) deletePolicy(resource string) { - if p.Name == resource { - p.Name = "" - p.Enforcer = nil - } -} - -// IsEmpty returns true if there is no policy set up. -func (p *policy) IsEmpty() bool { - return p.Name == "" && p.Enforcer == nil -} - func writePolicyStatus(ctx context.Context, r client.StatusClient, policy *configv1beta1.Policy, logger *logrus.Entry, isSuccess bool, errString string) { if isSuccess { updatePolicySuccessStatus(policy) diff --git a/pkg/controllers/policy_controller_test.go b/pkg/controllers/policy_controller_test.go index d3b041719..6e81a5764 100644 --- a/pkg/controllers/policy_controller_test.go +++ b/pkg/controllers/policy_controller_test.go @@ -63,67 +63,6 @@ func (c mockStatusClient) Status() client.SubResourceWriter { return writer } -func TestDeletePolicy(t *testing.T) { - testCases := []struct { - name string - policyName string - expectPolicyName string - }{ - { - name: "Delete same name", - policyName: policyName1, - expectPolicyName: "", - }, - { - name: "Delete different name", - policyName: policyName2, - expectPolicyName: policyName1, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - policy := &policy{ - Name: policyName1, - } - policy.deletePolicy(tc.policyName) - if policy.Name != tc.expectPolicyName { - t.Fatalf("Expected policy name to be %s, got %s", tc.expectPolicyName, policy.Name) - } - }) - } -} - -func TestIsEmpty(t *testing.T) { - testCases := []struct { - name string - policy *policy - expect bool - }{ - { - name: "Empty policy", - policy: &policy{}, - expect: true, - }, - { - name: "Non-empty policy", - policy: &policy{ - Name: policyName1, - }, - expect: false, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - isEmpty := tc.policy.IsEmpty() - if isEmpty != tc.expect { - t.Fatalf("Expected to be %t, got %t", tc.expect, isEmpty) - } - }) - } -} - func TestRawToPolicyConfig(t *testing.T) { testCases := []struct { name string diff --git a/pkg/controllers/resource_map.go b/pkg/controllers/resource_map.go index bc2ea14d1..6a496e6a6 100644 --- a/pkg/controllers/resource_map.go +++ b/pkg/controllers/resource_map.go @@ -13,6 +13,15 @@ limitations under the License. package controllers -import "github.com/deislabs/ratify/pkg/customresources/verifiers" +import ( + "github.com/deislabs/ratify/pkg/customresources/policies" + "github.com/deislabs/ratify/pkg/customresources/verifiers" +) -var VerifierMap = verifiers.NewActiveVerifiers() +var ( + VerifierMap = verifiers.NewActiveVerifiers() + + // ActivePolicy is the active policy generated from CRD. There would be exactly + // one active policy belonging to a namespace at any given time. + ActivePolicies = policies.NewActivePolicies() +) diff --git a/pkg/customresources/policies/api.go b/pkg/customresources/policies/api.go new file mode 100644 index 000000000..1973c8cc4 --- /dev/null +++ b/pkg/customresources/policies/api.go @@ -0,0 +1,33 @@ +/* +Copyright The Ratify Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package policies + +import "github.com/deislabs/ratify/pkg/policyprovider" + +// PolicyManager is an interface that defines the methods for managing policies across different scopes. +type PolicyManager interface { + // GetPolicy returns the policy for the given scope. + GetPolicy(scope string) policyprovider.PolicyProvider + + // AddPolicy adds the given policy under the given scope. + AddPolicy(scope, policyName string, policy policyprovider.PolicyProvider) + + // DeletePolicy deletes the policy from the given scope. + DeletePolicy(scope, policyName string) + + // IsEmpty returns true if there are no policies. + IsEmpty() bool +} diff --git a/pkg/customresources/policies/policies.go b/pkg/customresources/policies/policies.go new file mode 100644 index 000000000..d562d659c --- /dev/null +++ b/pkg/customresources/policies/policies.go @@ -0,0 +1,77 @@ +/* +Copyright The Ratify Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package policies + +import ( + "github.com/deislabs/ratify/internal/constants" + "github.com/deislabs/ratify/pkg/policyprovider" +) + +// PolicyWrapper wraps policy provider with its policy name. +type PolicyWrapper struct { + Name string + Policy policyprovider.PolicyProvider +} + +// ActivePolicies implements PolicyManager interface. +type ActivePolicies struct { + // TODO: Implement concurrent safety using sync.Map + // ScopedPolicies is a mapping from scope to a policy. + // Note: Scope is utilized for organizing and isolating verifiers. In a Kubernetes (K8s) environment, the scope can be either a namespace or an empty string ("") for cluster-wide verifiers. + ScopedPolicies map[string]PolicyWrapper +} + +func NewActivePolicies() PolicyManager { + return &ActivePolicies{ + ScopedPolicies: make(map[string]PolicyWrapper), + } +} + +// GetPolicy fulfills the PolicyManager interface. +// It returns the policy for the given scope. If no policy is found for the given scope, it returns cluster-wide policy. +// TODO: Current implementation always fetches the cluster-wide policy. Will implement the logic to fetch the policy for the given scope. +func (p *ActivePolicies) GetPolicy(_ string) policyprovider.PolicyProvider { + policy, ok := p.ScopedPolicies[constants.EmptyNamespace] + if ok { + return policy.Policy + } + return nil +} + +// AddPolicy fulfills the PolicyManager interface. +// It adds the given policy under the given scope. +func (p *ActivePolicies) AddPolicy(scope, policyName string, policy policyprovider.PolicyProvider) { + p.ScopedPolicies[scope] = PolicyWrapper{ + Name: policyName, + Policy: policy, + } +} + +// DeletePolicy fulfills the PolicyManager interface. +// It deletes the policy from the given scope. +func (p *ActivePolicies) DeletePolicy(scope, policyName string) { + if policy, ok := p.ScopedPolicies[scope]; ok { + if policy.Name == policyName { + delete(p.ScopedPolicies, scope) + } + } +} + +// IsEmpty fulfills the PolicyManager interface. +// IsEmpty returns true if there are no policies. +func (p *ActivePolicies) IsEmpty() bool { + return len(p.ScopedPolicies) == 0 +} diff --git a/pkg/customresources/policies/policies_test.go b/pkg/customresources/policies/policies_test.go new file mode 100644 index 000000000..69cd407a1 --- /dev/null +++ b/pkg/customresources/policies/policies_test.go @@ -0,0 +1,95 @@ +/* +Copyright The Ratify Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package policies + +import ( + "context" + "testing" + + "github.com/deislabs/ratify/internal/constants" + "github.com/deislabs/ratify/pkg/common" + "github.com/deislabs/ratify/pkg/executor/types" + "github.com/deislabs/ratify/pkg/ocispecs" +) + +type mockPolicy struct{} + +func (p mockPolicy) VerifyNeeded(_ context.Context, _ common.Reference, _ ocispecs.ReferenceDescriptor) bool { + return true +} + +func (p mockPolicy) ContinueVerifyOnFailure(_ context.Context, _ common.Reference, _ ocispecs.ReferenceDescriptor, _ types.VerifyResult) bool { + return true +} + +func (p mockPolicy) ErrorToVerifyResult(_ context.Context, _ string, _ error) types.VerifyResult { + return types.VerifyResult{} +} + +func (p mockPolicy) OverallVerifyResult(_ context.Context, _ []interface{}) bool { + return true +} + +func (p mockPolicy) GetPolicyType(_ context.Context) string { + return "" +} + +const ( + namespace1 = constants.EmptyNamespace + namespace2 = "namespace2" + name1 = "name1" + name2 = "name2" +) + +var ( + policy1 = mockPolicy{} + policy2 = mockPolicy{} +) + +func TestPoliciesOperations(t *testing.T) { + policies := NewActivePolicies() + + if !policies.IsEmpty() { + t.Errorf("Expected policies to be empty") + } + + policies.AddPolicy(namespace1, name1, policy1) + policies.AddPolicy(namespace2, name1, policy2) + + if policies.IsEmpty() { + t.Errorf("Expected policies to not be empty") + } + + if policies.GetPolicy(namespace1) != policy1 { + t.Errorf("Expected policy1 to be returned") + } + + if policies.GetPolicy(namespace2) != policy2 { + t.Errorf("Expected policy2 to be returned") + } + + policies.DeletePolicy(namespace2, name1) + + if policies.GetPolicy(namespace2) != policy1 { + t.Errorf("Expected policy1 to be returned") + } + + policies.DeletePolicy(namespace1, name1) + + if policies.GetPolicy(namespace1) != nil { + t.Errorf("Expected no policy to be returned") + } +} diff --git a/pkg/customresources/verifiers/verifiers.go b/pkg/customresources/verifiers/verifiers.go index 5825353ce..2e96c85ca 100644 --- a/pkg/customresources/verifiers/verifiers.go +++ b/pkg/customresources/verifiers/verifiers.go @@ -42,13 +42,13 @@ func NewActiveVerifiers() VerifierManager { } } -// GetVerifiers implements the Verifiers interface. +// GetVerifiers implements the VerifierManager interface. // It returns a list of verifiers for the given scope. If no verifiers are found for the given scope, it returns cluster-wide verifiers. // TODO: Current implementation fetches verifiers for all namespaces including cluster-wide ones. Will support actual namespace based verifiers in future. func (v *ActiveVerifiers) GetVerifiers(_ string) []vr.ReferenceVerifier { verifiers := []vr.ReferenceVerifier{} - for _, namespacedVerifiers := range v.ScopedVerifiers { - for _, verifier := range namespacedVerifiers { + for _, scopedVerifiers := range v.ScopedVerifiers { + for _, verifier := range scopedVerifiers { verifiers = append(verifiers, verifier) } } diff --git a/pkg/executor/core/executor.go b/pkg/executor/core/executor.go index 74a071d26..08e89bece 100644 --- a/pkg/executor/core/executor.go +++ b/pkg/executor/core/executor.go @@ -60,6 +60,9 @@ type Executor struct { // TODO Logging within executor // VerifySubject verifies the subject and returns results. func (executor Executor) VerifySubject(ctx context.Context, verifyParameters e.VerifyParameters) (types.VerifyResult, error) { + if executor.PolicyEnforcer == nil { + return types.VerifyResult{}, errors.ErrorCodePolicyProviderNotFound.WithComponentType(errors.Executor) + } result, err := executor.verifySubjectInternal(ctx, verifyParameters) if err != nil { // get the result for the error based on the policy. diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 230e17f5f..bb9dfebec 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -30,7 +30,6 @@ import ( "github.com/deislabs/ratify/config" "github.com/deislabs/ratify/httpserver" "github.com/deislabs/ratify/pkg/featureflag" - "github.com/deislabs/ratify/pkg/policyprovider" _ "github.com/deislabs/ratify/pkg/policyprovider/configpolicy" // register config policy provider _ "github.com/deislabs/ratify/pkg/policyprovider/regopolicy" // register rego policy provider _ "github.com/deislabs/ratify/pkg/referrerstore/oras" // register ORAS referrer store @@ -86,9 +85,9 @@ func StartServer(httpServerAddress, configFilePath, certDirectory, caCertFile st // initialize server server, err := httpserver.NewServer(context.Background(), httpServerAddress, func(ctx context.Context) *ef.Executor { var activeStores []referrerstore.ReferrerStore - var activePolicyEnforcer policyprovider.PolicyProvider - - activeVerifiers := controllers.VerifierMap.GetVerifiers(ctxUtils.GetNamespace(ctx)) + namespace := ctxUtils.GetNamespace(ctx) + activeVerifiers := controllers.VerifierMap.GetVerifiers(namespace) + activePolicyEnforcer := controllers.ActivePolicies.GetPolicy(namespace) // check if there are active stores from crd controller if len(controllers.StoreMap) > 0 { @@ -97,10 +96,6 @@ func StartServer(httpServerAddress, configFilePath, certDirectory, caCertFile st } } - if !controllers.ActivePolicy.IsEmpty() { - activePolicyEnforcer = controllers.ActivePolicy.Enforcer - } - // return executor with latest configuration executor := ef.Executor{ Verifiers: activeVerifiers,