feat: add PolicyManager interface to wrap operations on namespaced policies [multi-tenancy PR 3]

pkg/controllers/policy_controller.go

@@ -38,17 +38,6 @@
 	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 @@
 	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 @@
 		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 @@
 	}, 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)

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

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()
+)

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
+}

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
+}

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")
+	}
+}

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)
 		}
 	}

pkg/executor/core/executor.go

@@ -60,6 +60,9 @@
 // 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.