From 84cb90c76d9f07683277779d18db54c6a53b91b1 Mon Sep 17 00:00:00 2001 From: Binbin Li Date: Mon, 1 Apr 2024 06:03:14 +0000 Subject: [PATCH 1/4] refactor: refactor verifiers to support namespaced Signed-off-by: Binbin Li --- pkg/customresources/verifiers/api.go | 32 ++++++ pkg/customresources/verifiers/verifiers.go | 75 +++++++++++++ .../verifiers/verifiers_test.go | 105 ++++++++++++++++++ pkg/executor/core/executor.go | 3 +- 4 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 pkg/customresources/verifiers/api.go create mode 100644 pkg/customresources/verifiers/verifiers.go create mode 100644 pkg/customresources/verifiers/verifiers_test.go diff --git a/pkg/customresources/verifiers/api.go b/pkg/customresources/verifiers/api.go new file mode 100644 index 000000000..b1e492c38 --- /dev/null +++ b/pkg/customresources/verifiers/api.go @@ -0,0 +1,32 @@ +/* +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 verifiers + +import ( + vr "github.com/deislabs/ratify/pkg/verifier" +) + +type Verifiers interface { + GetVerifiers(scope string) []vr.ReferenceVerifier + + AddVerifier(scope, verifierName string, verifier vr.ReferenceVerifier) + + DeleteVerifier(scope, verifierName string) + + IsEmpty() bool + + GetVerifierCount() int +} diff --git a/pkg/customresources/verifiers/verifiers.go b/pkg/customresources/verifiers/verifiers.go new file mode 100644 index 000000000..d0ef99d6a --- /dev/null +++ b/pkg/customresources/verifiers/verifiers.go @@ -0,0 +1,75 @@ +/* +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 verifiers + +import ( + "github.com/deislabs/ratify/internal/constants" + vr "github.com/deislabs/ratify/pkg/verifier" +) + +type ActiveVerifiers struct { + NamespacedVerifiers map[string]map[string]vr.ReferenceVerifier +} + +func NewActiveVerifiers() ActiveVerifiers { + return ActiveVerifiers{ + NamespacedVerifiers: make(map[string]map[string]vr.ReferenceVerifier), + } +} + +// GetVerifiers implements the Verifiers 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. +func (v *ActiveVerifiers) GetVerifiers(scope string) []vr.ReferenceVerifier { + verifiers := []vr.ReferenceVerifier{} + for _, verifier := range v.NamespacedVerifiers[scope] { + verifiers = append(verifiers, verifier) + } + + if len(verifiers) == 0 && scope != constants.EmptyNamespace { + for _, verifier := range v.NamespacedVerifiers[constants.EmptyNamespace] { + verifiers = append(verifiers, verifier) + } + } + return verifiers +} + +func (v *ActiveVerifiers) AddVerifier(scope, verifierName string, verifier vr.ReferenceVerifier) { + if _, ok := v.NamespacedVerifiers[scope]; !ok { + v.NamespacedVerifiers[scope] = make(map[string]vr.ReferenceVerifier) + } + v.NamespacedVerifiers[scope][verifierName] = verifier +} + +func (v *ActiveVerifiers) DeleteVerifier(scope, verifierName string) { + if verifiers, ok := v.NamespacedVerifiers[scope]; ok { + delete(verifiers, verifierName) + if len(verifiers) == 0 { + delete(v.NamespacedVerifiers, scope) + } + } +} + +func (v *ActiveVerifiers) IsEmpty() bool { + return len(v.NamespacedVerifiers) == 0 +} + +func (v *ActiveVerifiers) GetVerifierCount() int { + count := 0 + for _, verifiers := range v.NamespacedVerifiers { + count += len(verifiers) + } + return count +} diff --git a/pkg/customresources/verifiers/verifiers_test.go b/pkg/customresources/verifiers/verifiers_test.go new file mode 100644 index 000000000..24d44c912 --- /dev/null +++ b/pkg/customresources/verifiers/verifiers_test.go @@ -0,0 +1,105 @@ +/* +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 verifiers + +import ( + "context" + "testing" + + "github.com/deislabs/ratify/internal/constants" + "github.com/deislabs/ratify/pkg/common" + "github.com/deislabs/ratify/pkg/ocispecs" + "github.com/deislabs/ratify/pkg/referrerstore" + "github.com/deislabs/ratify/pkg/verifier" +) + +type mockVerifier struct { + name string +} + +func (v mockVerifier) Name() string { + return v.name +} + +func (v mockVerifier) Type() string { + return "mockType" +} + +func (v mockVerifier) CanVerify(_ context.Context, _ ocispecs.ReferenceDescriptor) bool { + return true +} + +func (v mockVerifier) Verify(_ context.Context, _ common.Reference, _ ocispecs.ReferenceDescriptor, _ referrerstore.ReferrerStore) (verifier.VerifierResult, error) { + return verifier.VerifierResult{}, nil +} + +func (v mockVerifier) GetNestedReferences() []string { + return nil +} + +const ( + namespace1 = constants.EmptyNamespace + namespace2 = "namespace2" + name1 = "name1" + name2 = "name2" +) + +var ( + verifier1 = mockVerifier{name: name1} + verifier2 = mockVerifier{name: name2} +) + +func TestVerifiersOperations(t *testing.T) { + verifiers := NewActiveVerifiers() + verifiers.AddVerifier(namespace1, verifier1.Name(), verifier1) + verifiers.AddVerifier(namespace1, verifier2.Name(), verifier2) + verifiers.AddVerifier(namespace2, verifier1.Name(), verifier1) + verifiers.AddVerifier(namespace2, verifier2.Name(), verifier2) + + if verifiers.IsEmpty() { + t.Error("Expected verifiers to not be empty") + } + + if verifiers.GetVerifierCount() != 4 { + t.Errorf("Expected 4 verifiers, got %d", verifiers.GetVerifierCount()) + } + + if len(verifiers.GetVerifiers(namespace1)) != 2 { + t.Errorf("Expected 2 verifiers, got %d", len(verifiers.GetVerifiers(namespace1))) + } + + if len(verifiers.GetVerifiers(namespace2)) != 2 { + t.Errorf("Expected 2 verifiers, got %d", len(verifiers.GetVerifiers(namespace2))) + } + + verifiers.DeleteVerifier(namespace2, verifier1.Name()) + verifiers.DeleteVerifier(namespace2, verifier2.Name()) + + if len(verifiers.GetVerifiers(namespace2)) != 2 { + t.Errorf("Expected 2 verifiers, got %d", len(verifiers.GetVerifiers(namespace2))) + } + + verifiers.DeleteVerifier(namespace1, verifier1.Name()) + verifiers.DeleteVerifier(namespace1, verifier2.Name()) + + if !verifiers.IsEmpty() { + t.Error("Expected verifiers to be empty") + } + + if verifiers.GetVerifierCount() != 0 { + t.Errorf("Expected 0 verifiers, got %d", verifiers.GetVerifierCount()) + } +} diff --git a/pkg/executor/core/executor.go b/pkg/executor/core/executor.go index 74a071d26..f1dea1cd4 100644 --- a/pkg/executor/core/executor.go +++ b/pkg/executor/core/executor.go @@ -37,6 +37,7 @@ import ( vr "github.com/deislabs/ratify/pkg/verifier" vt "github.com/deislabs/ratify/pkg/verifier/types" "golang.org/x/sync/errgroup" + ) const ( @@ -53,7 +54,7 @@ var logOpt = logger.Option{ type Executor struct { ReferrerStores []referrerstore.ReferrerStore PolicyEnforcer policyprovider.PolicyProvider - Verifiers []vr.ReferenceVerifier + Verifiers verifiers.Verifiers Config *config.ExecutorConfig } From e716f5427d202c78c4e86a36f6755402199eff2a Mon Sep 17 00:00:00 2001 From: Binbin Li Date: Mon, 1 Apr 2024 14:28:01 +0000 Subject: [PATCH 2/4] feat: add verifiers interface to wrap up operations on namespaced verifiers --- config/config.go | 6 +- httpserver/handlers.go | 2 +- httpserver/server_test.go | 94 +++---- pkg/controllers/resource_map.go | 18 ++ pkg/controllers/verifier_controller.go | 17 +- pkg/controllers/verifier_controller_test.go | 32 +-- pkg/customresources/verifiers/api.go | 6 + pkg/customresources/verifiers/verifiers.go | 33 ++- .../verifiers/verifiers_test.go | 8 +- pkg/executor/core/executor.go | 11 +- pkg/executor/core/executor_test.go | 255 +++++++----------- pkg/executor/core/testtypes.go | 6 +- pkg/manager/manager.go | 11 +- pkg/verifier/factory/factory.go | 7 +- pkg/verifier/factory/factory_test.go | 20 +- 15 files changed, 233 insertions(+), 293 deletions(-) create mode 100644 pkg/controllers/resource_map.go diff --git a/config/config.go b/config/config.go index 901a0eb74..e21ae119f 100644 --- a/config/config.go +++ b/config/config.go @@ -26,6 +26,7 @@ import ( "github.com/deislabs/ratify/internal/constants" "github.com/deislabs/ratify/internal/logger" + "github.com/deislabs/ratify/pkg/customresources/verifiers" exConfig "github.com/deislabs/ratify/pkg/executor/config" "github.com/deislabs/ratify/pkg/homedir" "github.com/deislabs/ratify/pkg/policyprovider" @@ -34,7 +35,6 @@ import ( "github.com/deislabs/ratify/pkg/referrerstore" rsConfig "github.com/deislabs/ratify/pkg/referrerstore/config" sf "github.com/deislabs/ratify/pkg/referrerstore/factory" - "github.com/deislabs/ratify/pkg/verifier" vfConfig "github.com/deislabs/ratify/pkg/verifier/config" vf "github.com/deislabs/ratify/pkg/verifier/factory" "github.com/pkg/errors" @@ -84,7 +84,7 @@ func getHomeDir() string { } // Returns created referer store, verifier, policyprovider objects from config -func CreateFromConfig(cf Config) ([]referrerstore.ReferrerStore, []verifier.ReferenceVerifier, policyprovider.PolicyProvider, error) { +func CreateFromConfig(cf Config) ([]referrerstore.ReferrerStore, verifiers.Verifiers, policyprovider.PolicyProvider, error) { stores, err := sf.CreateStoresFromConfig(cf.StoresConfig, GetDefaultPluginPath()) if err != nil { @@ -99,7 +99,7 @@ func CreateFromConfig(cf Config) ([]referrerstore.ReferrerStore, []verifier.Refe return nil, nil, nil, errors.Wrap(err, "failed to load verifiers from config") } - logrus.Infof("verifiers successfully created. number of verifiers %d", len(verifiers)) + logrus.Infof("verifiers successfully created. number of verifiers %d", verifiers.GetVerifierCount()) policyEnforcer, err := pf.CreatePolicyProviderFromConfig(cf.PoliciesConfig) diff --git a/httpserver/handlers.go b/httpserver/handlers.go index 57c999bad..d96f64e67 100644 --- a/httpserver/handlers.go +++ b/httpserver/handlers.go @@ -256,7 +256,7 @@ func (server *Server) validateComponents(handlerComponents string) error { if server.GetExecutor().PolicyEnforcer == nil { return errors.ErrorCodeConfigInvalid.WithComponentType(errors.PolicyProvider).WithDetail("policy provider config must be specified") } - if len(server.GetExecutor().Verifiers) == 0 { + if server.GetExecutor().Verifiers.IsEmpty() { return errors.ErrorCodeConfigInvalid.WithComponentType(errors.Verifier).WithDetail("verifiers config should have at least one verifier") } } diff --git a/httpserver/server_test.go b/httpserver/server_test.go index 9a4587287..e86bff205 100644 --- a/httpserver/server_test.go +++ b/httpserver/server_test.go @@ -35,6 +35,8 @@ import ( config "github.com/deislabs/ratify/pkg/policyprovider/configpolicy" "github.com/sirupsen/logrus" + vrs "github.com/deislabs/ratify/pkg/customresources/verifiers" + "github.com/deislabs/ratify/pkg/policyprovider" "github.com/deislabs/ratify/pkg/policyprovider/types" "github.com/deislabs/ratify/pkg/referrerstore" "github.com/deislabs/ratify/pkg/referrerstore/mocks" @@ -47,12 +49,7 @@ const testArtifactType string = "test-type1" const testImageNameTagged string = "localhost:5000/net-monitor:v1" func testGetExecutor() *core.Executor { - return &core.Executor{ - Verifiers: []verifier.ReferenceVerifier{}, - ReferrerStores: []referrerstore.ReferrerStore{}, - PolicyEnforcer: nil, - Config: nil, - } + return newTestExecutor(nil, nil, nil, nil) } func TestNewServer_Expected(t *testing.T) { @@ -132,11 +129,7 @@ func TestServer_Timeout_Failed(t *testing.T) { }, } - ex := &core.Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{ver}, - } + ex := newTestExecutor(configPolicy, store, ver, nil) getExecutor := func() *core.Executor { return ex @@ -199,15 +192,7 @@ func TestServer_MultipleSubjects_Success(t *testing.T) { }, } - ex := &core.Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{ver}, - Config: &exconfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }, - } + ex := newTestExecutor(configPolicy, store, ver, nil) getExecutor := func() *core.Executor { return ex @@ -274,11 +259,7 @@ func TestServer_Mutation_Success(t *testing.T) { }, } - ex := &core.Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{ver}, - } + ex := newTestExecutor(configPolicy, store, ver, nil) getExecutor := func() *core.Executor { return ex @@ -350,11 +331,7 @@ func TestServer_Mutation_ReferrerStoreConfigInvalid_Failure(t *testing.T) { }, } - ex := &core.Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{}, - Verifiers: []verifier.ReferenceVerifier{ver}, - } + ex := newTestExecutor(configPolicy, nil, ver, nil) getExecutor := func() *core.Executor { return ex @@ -429,15 +406,10 @@ func TestServer_MultipleRequestsForSameSubject_Success(t *testing.T) { } verifyTimeout := 5000 - ex := &core.Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{ver}, - Config: &exconfig.ExecutorConfig{ - VerificationRequestTimeout: &verifyTimeout, - MutationRequestTimeout: nil, - }, - } + ex := newTestExecutor(configPolicy, store, ver, &exconfig.ExecutorConfig{ + VerificationRequestTimeout: &verifyTimeout, + MutationRequestTimeout: nil, + }) getExecutor := func() *core.Executor { return ex @@ -481,15 +453,10 @@ func TestServer_Verify_ParseReference_Failure(t *testing.T) { responseRecorder := httptest.NewRecorder() - ex := &core.Executor{ - PolicyEnforcer: config.PolicyEnforcer{}, - ReferrerStores: []referrerstore.ReferrerStore{&mocks.TestStore{}}, - Verifiers: []verifier.ReferenceVerifier{&core.TestVerifier{}}, - Config: &exconfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }, - } + ex := newTestExecutor(config.PolicyEnforcer{}, &mocks.TestStore{}, &core.TestVerifier{}, &exconfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }) getExecutor := func() *core.Executor { return ex @@ -558,11 +525,7 @@ func TestServer_Verify_PolicyEnforcerConfigInvalid_Failure(t *testing.T) { }, } - ex := &core.Executor{ - PolicyEnforcer: nil, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{ver}, - } + ex := newTestExecutor(nil, store, ver, nil) getExecutor := func() *core.Executor { return ex @@ -627,11 +590,7 @@ func TestServer_Verify_VerifierConfigInvalid_Failure(t *testing.T) { }, } - ex := &core.Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{}, - } + ex := newTestExecutor(configPolicy, store, nil, nil) getExecutor := func() *core.Executor { return ex @@ -716,3 +675,22 @@ func TestServer_serverGracefulShutdown(t *testing.T) { // wait some time to see shutdown logs time.Sleep(5 * time.Second) } + +func newTestExecutor(policyEnforcer policyprovider.PolicyProvider, store referrerstore.ReferrerStore, verifier verifier.ReferenceVerifier, config *exconfig.ExecutorConfig) *core.Executor { + activeStores := make([]referrerstore.ReferrerStore, 0) + if store != nil { + activeStores = append(activeStores, store) + } + + activeVerifiers := vrs.NewActiveVerifiers() + if verifier != nil { + activeVerifiers.AddVerifier("", verifier.Name(), verifier) + } + + return &core.Executor{ + PolicyEnforcer: policyEnforcer, + ReferrerStores: activeStores, + Verifiers: activeVerifiers, + Config: config, + } +} diff --git a/pkg/controllers/resource_map.go b/pkg/controllers/resource_map.go new file mode 100644 index 000000000..bc2ea14d1 --- /dev/null +++ b/pkg/controllers/resource_map.go @@ -0,0 +1,18 @@ +/* +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 controllers + +import "github.com/deislabs/ratify/pkg/customresources/verifiers" + +var VerifierMap = verifiers.NewActiveVerifiers() diff --git a/pkg/controllers/verifier_controller.go b/pkg/controllers/verifier_controller.go index 31cb83a5b..dc39fd119 100644 --- a/pkg/controllers/verifier_controller.go +++ b/pkg/controllers/verifier_controller.go @@ -24,8 +24,8 @@ import ( configv1beta1 "github.com/deislabs/ratify/api/v1beta1" "github.com/deislabs/ratify/config" re "github.com/deislabs/ratify/errors" + "github.com/deislabs/ratify/internal/constants" "github.com/deislabs/ratify/pkg/utils" - vr "github.com/deislabs/ratify/pkg/verifier" vc "github.com/deislabs/ratify/pkg/verifier/config" vf "github.com/deislabs/ratify/pkg/verifier/factory" "github.com/deislabs/ratify/pkg/verifier/types" @@ -43,11 +43,6 @@ type VerifierReconciler struct { Scheme *runtime.Scheme } -var ( - // a map to track of active verifiers - VerifierMap = map[string]vr.ReferenceVerifier{} -) - //+kubebuilder:rbac:groups=config.ratify.deislabs.io,resources=verifiers,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=config.ratify.deislabs.io,resources=verifiers/status,verbs=get;update;patch //+kubebuilder:rbac:groups=config.ratify.deislabs.io,resources=verifiers/finalizers,verbs=update @@ -72,7 +67,8 @@ func (r *VerifierReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c if err := r.Get(ctx, req.NamespacedName, &verifier); err != nil { if apierrors.IsNotFound(err) { verifierLogger.Infof("delete event detected, removing verifier %v", resource) - verifierRemove(resource) + // TODO: pass the actual namespace once multi-tenancy is supported. + VerifierMap.DeleteVerifier(constants.EmptyNamespace, resource) } else { verifierLogger.Error(err, "unable to fetch verifier") } @@ -122,17 +118,12 @@ func verifierAddOrReplace(spec configv1beta1.VerifierSpec, objectName string, na logrus.Error(err, "unable to create verifier from verifier config") return err } - VerifierMap[objectName] = referenceVerifier + VerifierMap.AddVerifier(constants.EmptyNamespace, objectName, referenceVerifier) logrus.Infof("verifier '%v' added to verifier map", referenceVerifier.Name()) return nil } -// remove verifier from map -func verifierRemove(objectName string) { - delete(VerifierMap, objectName) -} - // returns a verifier reference from spec func specToVerifierConfig(verifierSpec configv1beta1.VerifierSpec, verifierName string) (vc.VerifierConfig, error) { verifierConfig := vc.VerifierConfig{} diff --git a/pkg/controllers/verifier_controller_test.go b/pkg/controllers/verifier_controller_test.go index 6ecad4020..48ceea6ec 100644 --- a/pkg/controllers/verifier_controller_test.go +++ b/pkg/controllers/verifier_controller_test.go @@ -23,8 +23,8 @@ import ( configv1beta1 "github.com/deislabs/ratify/api/v1beta1" "github.com/deislabs/ratify/internal/constants" + "github.com/deislabs/ratify/pkg/customresources/verifiers" "github.com/deislabs/ratify/pkg/utils" - vr "github.com/deislabs/ratify/pkg/verifier" "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -34,7 +34,7 @@ const licenseChecker = "licensechecker" func TestMain(m *testing.M) { // make sure to reset verifierMap before each test run - VerifierMap = map[string]vr.ReferenceVerifier{} + VerifierMap = verifiers.NewActiveVerifiers() code := m.Run() os.Exit(code) } @@ -56,15 +56,15 @@ func TestVerifierAdd_EmptyParameter(t *testing.T) { if err := verifierAddOrReplace(testVerifierSpec, sampleName, constants.EmptyNamespace); err != nil { t.Fatalf("verifierAddOrReplace() expected no error, actual %v", err) } - if len(VerifierMap) != 1 { - t.Fatalf("Verifier map expected size 1, actual %v", len(VerifierMap)) + if VerifierMap.GetVerifierCount() != 1 { + t.Fatalf("Verifier map expected size 1, actual %v", VerifierMap.GetVerifierCount()) } } func TestVerifierAdd_WithParameters(t *testing.T) { resetVerifierMap() - if len(VerifierMap) != 0 { - t.Fatalf("Verifier map expected size 0, actual %v", len(VerifierMap)) + if VerifierMap.GetVerifierCount() != 0 { + t.Fatalf("Verifier map expected size 0, actual %v", VerifierMap.GetVerifierCount()) } dirPath, err := utils.CreatePlugin(licenseChecker) @@ -78,8 +78,8 @@ func TestVerifierAdd_WithParameters(t *testing.T) { if err := verifierAddOrReplace(testVerifierSpec, "testObject", constants.EmptyNamespace); err != nil { t.Fatalf("verifierAddOrReplace() expected no error, actual %v", err) } - if len(VerifierMap) != 1 { - t.Fatalf("Verifier map expected size 1, actual %v", len(VerifierMap)) + if VerifierMap.GetVerifierCount() != 1 { + t.Fatalf("Verifier map expected size 1, actual %v", VerifierMap.GetVerifierCount()) } } @@ -109,8 +109,8 @@ func TestVerifier_UpdateAndDelete(t *testing.T) { if err := verifierAddOrReplace(testVerifierSpec, licenseChecker, constants.EmptyNamespace); err != nil { t.Fatalf("verifierAddOrReplace() expected no error, actual %v", err) } - if len(VerifierMap) != 1 { - t.Fatalf("Verifier map expected size 1, actual %v", len(VerifierMap)) + if VerifierMap.GetVerifierCount() != 1 { + t.Fatalf("Verifier map expected size 1, actual %v", VerifierMap.GetVerifierCount()) } // modify the verifier @@ -121,14 +121,14 @@ func TestVerifier_UpdateAndDelete(t *testing.T) { } // validate no verifier has been added - if len(VerifierMap) != 1 { - t.Fatalf("Verifier map should be 1 after replacement, actual %v", len(VerifierMap)) + if VerifierMap.GetVerifierCount() != 1 { + t.Fatalf("Verifier map should be 1 after replacement, actual %v", VerifierMap.GetVerifierCount()) } - verifierRemove(licenseChecker) + VerifierMap.DeleteVerifier(constants.EmptyNamespace, licenseChecker) - if len(VerifierMap) != 0 { - t.Fatalf("Verifier map should be 0 after deletion, actual %v", len(VerifierMap)) + if VerifierMap.GetVerifierCount() != 0 { + t.Fatalf("Verifier map should be 0 after deletion, actual %v", VerifierMap.GetVerifierCount()) } } @@ -206,7 +206,7 @@ func TestGetCertStoreNamespace(t *testing.T) { } func resetVerifierMap() { - VerifierMap = map[string]vr.ReferenceVerifier{} + VerifierMap = verifiers.NewActiveVerifiers() } func getLicenseCheckerFromParam(parametersString, pluginPath string) configv1beta1.VerifierSpec { diff --git a/pkg/customresources/verifiers/api.go b/pkg/customresources/verifiers/api.go index b1e492c38..04d6a3e0e 100644 --- a/pkg/customresources/verifiers/api.go +++ b/pkg/customresources/verifiers/api.go @@ -19,14 +19,20 @@ import ( vr "github.com/deislabs/ratify/pkg/verifier" ) +// Verifiers is an interface that defines the methods for managing verifiers across different scopes. type Verifiers interface { + // GetVerifiers returns verifiers under the given scope. GetVerifiers(scope string) []vr.ReferenceVerifier + // AddVerifier adds a verifier to the given scope. AddVerifier(scope, verifierName string, verifier vr.ReferenceVerifier) + // DeleteVerifier deletes a verifier from the given scope. DeleteVerifier(scope, verifierName string) + // IsEmpty returns true if verifiers are empty. IsEmpty() bool + // GetVerifierCount returns the number of verifiers in all scopes. GetVerifierCount() int } diff --git a/pkg/customresources/verifiers/verifiers.go b/pkg/customresources/verifiers/verifiers.go index d0ef99d6a..d48aadbfa 100644 --- a/pkg/customresources/verifiers/verifiers.go +++ b/pkg/customresources/verifiers/verifiers.go @@ -16,30 +16,38 @@ limitations under the License. package verifiers import ( - "github.com/deislabs/ratify/internal/constants" vr "github.com/deislabs/ratify/pkg/verifier" ) +// ActiveVerifiers implements Verifiers interface. type ActiveVerifiers struct { + // TODO: Implement concurrent safety using sync.Map + // The structure of the map is as follows: + // The first level maps from scope to verifiers + // The second level maps from verifier name to verifier + // Example: + // { + // "namespace1": { + // "verifier1": verifier1, + // "verifier2": verifier2 + // } + // } NamespacedVerifiers map[string]map[string]vr.ReferenceVerifier } -func NewActiveVerifiers() ActiveVerifiers { - return ActiveVerifiers{ +func NewActiveVerifiers() Verifiers { + return &ActiveVerifiers{ NamespacedVerifiers: make(map[string]map[string]vr.ReferenceVerifier), } } // GetVerifiers implements the Verifiers 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. -func (v *ActiveVerifiers) GetVerifiers(scope string) []vr.ReferenceVerifier { +// 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 _, verifier := range v.NamespacedVerifiers[scope] { - verifiers = append(verifiers, verifier) - } - - if len(verifiers) == 0 && scope != constants.EmptyNamespace { - for _, verifier := range v.NamespacedVerifiers[constants.EmptyNamespace] { + for _, namespacedVerifiers := range v.NamespacedVerifiers { + for _, verifier := range namespacedVerifiers { verifiers = append(verifiers, verifier) } } @@ -56,14 +64,11 @@ func (v *ActiveVerifiers) AddVerifier(scope, verifierName string, verifier vr.Re func (v *ActiveVerifiers) DeleteVerifier(scope, verifierName string) { if verifiers, ok := v.NamespacedVerifiers[scope]; ok { delete(verifiers, verifierName) - if len(verifiers) == 0 { - delete(v.NamespacedVerifiers, scope) - } } } func (v *ActiveVerifiers) IsEmpty() bool { - return len(v.NamespacedVerifiers) == 0 + return v.GetVerifierCount() == 0 } func (v *ActiveVerifiers) GetVerifierCount() int { diff --git a/pkg/customresources/verifiers/verifiers_test.go b/pkg/customresources/verifiers/verifiers_test.go index 24d44c912..652a9cfcf 100644 --- a/pkg/customresources/verifiers/verifiers_test.go +++ b/pkg/customresources/verifiers/verifiers_test.go @@ -77,12 +77,12 @@ func TestVerifiersOperations(t *testing.T) { t.Errorf("Expected 4 verifiers, got %d", verifiers.GetVerifierCount()) } - if len(verifiers.GetVerifiers(namespace1)) != 2 { - t.Errorf("Expected 2 verifiers, got %d", len(verifiers.GetVerifiers(namespace1))) + if len(verifiers.GetVerifiers(namespace1)) != 4 { + t.Errorf("Expected 4 verifiers, got %d", len(verifiers.GetVerifiers(namespace1))) } - if len(verifiers.GetVerifiers(namespace2)) != 2 { - t.Errorf("Expected 2 verifiers, got %d", len(verifiers.GetVerifiers(namespace2))) + if len(verifiers.GetVerifiers(namespace2)) != 4 { + t.Errorf("Expected 4 verifiers, got %d", len(verifiers.GetVerifiers(namespace2))) } verifiers.DeleteVerifier(namespace2, verifier1.Name()) diff --git a/pkg/executor/core/executor.go b/pkg/executor/core/executor.go index f1dea1cd4..61ff05612 100644 --- a/pkg/executor/core/executor.go +++ b/pkg/executor/core/executor.go @@ -22,8 +22,10 @@ import ( "time" "github.com/deislabs/ratify/errors" + ctxUtils "github.com/deislabs/ratify/internal/context" "github.com/deislabs/ratify/internal/logger" "github.com/deislabs/ratify/pkg/common" + "github.com/deislabs/ratify/pkg/customresources/verifiers" e "github.com/deislabs/ratify/pkg/executor" "github.com/deislabs/ratify/pkg/executor/config" "github.com/deislabs/ratify/pkg/executor/types" @@ -37,7 +39,6 @@ import ( vr "github.com/deislabs/ratify/pkg/verifier" vt "github.com/deislabs/ratify/pkg/verifier/types" "golang.org/x/sync/errgroup" - ) const ( @@ -54,8 +55,8 @@ var logOpt = logger.Option{ type Executor struct { ReferrerStores []referrerstore.ReferrerStore PolicyEnforcer policyprovider.PolicyProvider - Verifiers verifiers.Verifiers - Config *config.ExecutorConfig + verifiers.Verifiers + Config *config.ExecutorConfig } // TODO Logging within executor @@ -169,7 +170,7 @@ func (executor Executor) verifyReferenceForJSONPolicy(ctx context.Context, subje var verifyResults []interface{} var isSuccess = true - for _, verifier := range executor.Verifiers { + for _, verifier := range executor.GetVerifiers(ctxUtils.GetNamespace(ctx)) { if verifier.CanVerify(ctx, referenceDesc) { verifierStartTime := time.Now() verifyResult, err := verifier.Verify(ctx, subjectRef, referenceDesc, referrerStore) @@ -214,7 +215,7 @@ func (executor Executor) verifyReferenceForRegoPolicy(ctx context.Context, subje return executor.addNestedReports(errCtx, referenceDesc, subjectRef, &nestedReport) }) - for _, verifier := range executor.Verifiers { + for _, verifier := range executor.GetVerifiers(ctxUtils.GetNamespace(ctx)) { if !verifier.CanVerify(ctx, referenceDesc) { continue } diff --git a/pkg/executor/core/executor_test.go b/pkg/executor/core/executor_test.go index 36edaff76..31dcfb980 100644 --- a/pkg/executor/core/executor_test.go +++ b/pkg/executor/core/executor_test.go @@ -24,6 +24,7 @@ import ( ratifyerrors "github.com/deislabs/ratify/errors" "github.com/deislabs/ratify/pkg/common" + vrs "github.com/deislabs/ratify/pkg/customresources/verifiers" e "github.com/deislabs/ratify/pkg/executor" exConfig "github.com/deislabs/ratify/pkg/executor/config" "github.com/deislabs/ratify/pkg/executor/types" @@ -157,7 +158,7 @@ func (v *mockVerifier) GetNestedReferences() []string { } func TestVerifySubjectInternal_ResolveSubjectDescriptor_Failed(t *testing.T) { - executor := Executor{} + executor := newEmptyExecutor() verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -179,10 +180,7 @@ func TestVerifySubjectInternal_ResolveSubjectDescriptor_Success(t *testing.T) { }, } - executor := Executor{ - ReferrerStores: []referrerstore.ReferrerStore{store}, - PolicyEnforcer: &mockPolicyProvider{}, - } + executor := newTestExecutor(&mockPolicyProvider{}, store, nil, nil) verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -196,19 +194,17 @@ func TestVerifySubjectInternal_ResolveSubjectDescriptor_Success(t *testing.T) { func TestVerifySubjectInternal_Verify_NoReferrers(t *testing.T) { testDigest := digest.FromString("test") configPolicy := policyConfig.PolicyEnforcer{} - ex := &Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{&mocks.TestStore{ + ex := newTestExecutor(configPolicy, + &mocks.TestStore{ ResolveMap: map[string]digest.Digest{ "v1": testDigest, }, - }}, - Verifiers: []verifier.ReferenceVerifier{&TestVerifier{}}, - Config: &exConfig.ExecutorConfig{ + }, + []verifier.ReferenceVerifier{&TestVerifier{}}, + &exConfig.ExecutorConfig{ VerificationRequestTimeout: nil, MutationRequestTimeout: nil, - }, - } + }) verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -245,15 +241,10 @@ func TestVerifySubjectInternal_CanVerify_ExpectedResults(t *testing.T) { }, } - ex := &Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{ver}, - Config: &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }, - } + ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{ver}, &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }) verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -301,15 +292,10 @@ func TestVerifySubjectInternal_VerifyFailures_ExpectedResults(t *testing.T) { }, } - ex := &Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{ver}, - Config: &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }, - } + ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{ver}, &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }) verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -353,15 +339,10 @@ func TestVerifySubjectInternal_VerifySuccess_ExpectedResults(t *testing.T) { }, } - ex := &Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{ver}, - Config: &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }, - } + ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{ver}, &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }) verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -413,15 +394,10 @@ func TestVerifySubjectInternalWithDecision_MultipleArtifacts_ExpectedResults(t * }, } - ex := &Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{ver}, - Config: &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }, - } + ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{ver}, &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }) verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -464,6 +440,7 @@ func TestVerifySubjectInternal_NestedReferences_Expected(t *testing.T) { return true }, nestedReferences: []string{"string-content-does-not-matter"}, + name: "sbom-verifier", } signatureVerifier := &TestVerifier{ @@ -475,15 +452,7 @@ func TestVerifySubjectInternal_NestedReferences_Expected(t *testing.T) { }, } - ex := &Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{sbomVerifier, signatureVerifier}, - Config: &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }, - } + ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{sbomVerifier, signatureVerifier}, &exConfig.ExecutorConfig{}) verifyParameters := e.VerifyParameters{ Subject: mocks.TestSubjectWithDigest, @@ -541,6 +510,7 @@ func TestVerifySubjectInternal_NoNestedReferences_Expected(t *testing.T) { VerifyResult: func(artifactType string) bool { return true }, + name: "sbom-verifier", } signatureVer := &TestVerifier{ @@ -552,15 +522,7 @@ func TestVerifySubjectInternal_NoNestedReferences_Expected(t *testing.T) { }, } - ex := &Executor{ - PolicyEnforcer: configPolicy, - ReferrerStores: []referrerstore.ReferrerStore{store}, - Verifiers: []verifier.ReferenceVerifier{sbomVer, signatureVer}, - Config: &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }, - } + ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{sbomVer, signatureVer}, &exConfig.ExecutorConfig{}) verifyParameters := e.VerifyParameters{ Subject: mocks.TestSubjectWithDigest, @@ -603,39 +565,21 @@ func TestGetVerifyRequestTimeout_ExpectedResults(t *testing.T) { expectedTimeout int }{ { - setTimeout: -1, - ex: Executor{ - PolicyEnforcer: policyConfig.PolicyEnforcer{}, - ReferrerStores: []referrerstore.ReferrerStore{}, - Verifiers: []verifier.ReferenceVerifier{}, - Config: nil, - }, + setTimeout: -1, + ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, nil), expectedTimeout: 2900, }, { - setTimeout: -1, - ex: Executor{ - PolicyEnforcer: policyConfig.PolicyEnforcer{}, - ReferrerStores: []referrerstore.ReferrerStore{}, - Verifiers: []verifier.ReferenceVerifier{}, - Config: &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }, - }, + setTimeout: -1, + ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, &exConfig.ExecutorConfig{}), expectedTimeout: 2900, }, { setTimeout: 5000, - ex: Executor{ - PolicyEnforcer: policyConfig.PolicyEnforcer{}, - ReferrerStores: []referrerstore.ReferrerStore{}, - Verifiers: []verifier.ReferenceVerifier{}, - Config: &exConfig.ExecutorConfig{ - VerificationRequestTimeout: new(int), - MutationRequestTimeout: nil, - }, - }, + ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, &exConfig.ExecutorConfig{ + VerificationRequestTimeout: new(int), + MutationRequestTimeout: nil, + }), expectedTimeout: 5000, }, } @@ -660,39 +604,21 @@ func TestGetMutationRequestTimeout_ExpectedResults(t *testing.T) { expectedTimeout int }{ { - setTimeout: -1, - ex: Executor{ - PolicyEnforcer: policyConfig.PolicyEnforcer{}, - ReferrerStores: []referrerstore.ReferrerStore{}, - Verifiers: []verifier.ReferenceVerifier{}, - Config: nil, - }, + setTimeout: -1, + ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, nil), expectedTimeout: 950, }, { - setTimeout: -1, - ex: Executor{ - PolicyEnforcer: policyConfig.PolicyEnforcer{}, - ReferrerStores: []referrerstore.ReferrerStore{}, - Verifiers: []verifier.ReferenceVerifier{}, - Config: &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }, - }, + setTimeout: -1, + ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, &exConfig.ExecutorConfig{}), expectedTimeout: 950, }, { setTimeout: 2400, - ex: Executor{ - PolicyEnforcer: policyConfig.PolicyEnforcer{}, - ReferrerStores: []referrerstore.ReferrerStore{}, - Verifiers: []verifier.ReferenceVerifier{}, - Config: &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: new(int), - }, - }, + ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: new(int), + }), expectedTimeout: 2400, }, } @@ -714,7 +640,7 @@ func TestVerifySubject(t *testing.T) { name string params e.VerifyParameters expectedResult types.VerifyResult - stores []referrerstore.ReferrerStore + store referrerstore.ReferrerStore policyEnforcer policyprovider.PolicyProvider verifiers []verifier.ReferenceVerifier referrers []ocispecs.ReferenceDescriptor @@ -733,10 +659,8 @@ func TestVerifySubject(t *testing.T) { params: e.VerifyParameters{ Subject: subject1, }, - stores: []referrerstore.ReferrerStore{ - &mockStore{ - referrers: nil, - }, + store: &mockStore{ + referrers: nil, }, policyEnforcer: &mockPolicyProvider{ policyType: pt.RegoPolicy, @@ -749,10 +673,8 @@ func TestVerifySubject(t *testing.T) { params: e.VerifyParameters{ Subject: subject1, }, - stores: []referrerstore.ReferrerStore{ - &mockStore{ - referrers: nil, - }, + store: &mockStore{ + referrers: nil, }, policyEnforcer: &mockPolicyProvider{ policyType: pt.RegoPolicy, @@ -765,15 +687,13 @@ func TestVerifySubject(t *testing.T) { params: e.VerifyParameters{ Subject: subject1, }, - stores: []referrerstore.ReferrerStore{ - &mockStore{ - referrers: map[string][]ocispecs.ReferenceDescriptor{ - subjectDigest: { - { - ArtifactType: artifactType, - Descriptor: oci.Descriptor{ - Digest: signatureDigest, - }, + store: &mockStore{ + referrers: map[string][]ocispecs.ReferenceDescriptor{ + subjectDigest: { + { + ArtifactType: artifactType, + Descriptor: oci.Descriptor{ + Digest: signatureDigest, }, }, }, @@ -798,15 +718,13 @@ func TestVerifySubject(t *testing.T) { params: e.VerifyParameters{ Subject: subject1, }, - stores: []referrerstore.ReferrerStore{ - &mockStore{ - referrers: map[string][]ocispecs.ReferenceDescriptor{ - subjectDigest: { - { - ArtifactType: artifactType, - Descriptor: oci.Descriptor{ - Digest: signatureDigest, - }, + store: &mockStore{ + referrers: map[string][]ocispecs.ReferenceDescriptor{ + subjectDigest: { + { + ArtifactType: artifactType, + Descriptor: oci.Descriptor{ + Digest: signatureDigest, }, }, }, @@ -832,15 +750,13 @@ func TestVerifySubject(t *testing.T) { params: e.VerifyParameters{ Subject: subject1, }, - stores: []referrerstore.ReferrerStore{ - &mockStore{ - referrers: map[string][]ocispecs.ReferenceDescriptor{ - subjectDigest: { - { - ArtifactType: artifactType, - Descriptor: oci.Descriptor{ - Digest: signatureDigest, - }, + store: &mockStore{ + referrers: map[string][]ocispecs.ReferenceDescriptor{ + subjectDigest: { + { + ArtifactType: artifactType, + Descriptor: oci.Descriptor{ + Digest: signatureDigest, }, }, }, @@ -867,7 +783,7 @@ func TestVerifySubject(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - ex := &Executor{tc.stores, tc.policyEnforcer, tc.verifiers, nil} + ex := newTestExecutor(tc.policyEnforcer, tc.store, tc.verifiers, nil) result, err := ex.VerifySubject(context.Background(), tc.params) if (err != nil) != tc.expectErr { @@ -879,3 +795,30 @@ func TestVerifySubject(t *testing.T) { }) } } + +func newTestExecutor(policyEnforcer policyprovider.PolicyProvider, store referrerstore.ReferrerStore, verifiers []verifier.ReferenceVerifier, config *exConfig.ExecutorConfig) *Executor { + activeStores := make([]referrerstore.ReferrerStore, 0) + if store != nil { + activeStores = append(activeStores, store) + } + + activeVerifiers := vrs.NewActiveVerifiers() + for _, verifier := range verifiers { + activeVerifiers.AddVerifier("", verifier.Name(), verifier) + } + + return &Executor{ + PolicyEnforcer: policyEnforcer, + ReferrerStores: activeStores, + Verifiers: activeVerifiers, + Config: config, + } +} + +func newEmptyExecutor() Executor { + verifiers := vrs.NewActiveVerifiers() + + return Executor{ + Verifiers: verifiers, + } +} diff --git a/pkg/executor/core/testtypes.go b/pkg/executor/core/testtypes.go index fd28a410d..d16409ef8 100644 --- a/pkg/executor/core/testtypes.go +++ b/pkg/executor/core/testtypes.go @@ -28,10 +28,14 @@ type TestVerifier struct { CanVerifyFunc func(artifactType string) bool VerifyResult func(artifactType string) bool nestedReferences []string + name string } func (s *TestVerifier) Name() string { - return "verifier-testVerifier" + if len(s.name) == 0 { + return "verifier-testVerifier" + } + return s.name } func (s *TestVerifier) Type() string { diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 36cd2cd09..38679443f 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -52,7 +52,6 @@ import ( "github.com/deislabs/ratify/pkg/controllers" ef "github.com/deislabs/ratify/pkg/executor/core" "github.com/deislabs/ratify/pkg/referrerstore" - vr "github.com/deislabs/ratify/pkg/verifier" //+kubebuilder:scaffold:imports ) @@ -85,17 +84,9 @@ func StartServer(httpServerAddress, configFilePath, certDirectory, caCertFile st // initialize server server, err := httpserver.NewServer(context.Background(), httpServerAddress, func() *ef.Executor { - var activeVerifiers []vr.ReferenceVerifier var activeStores []referrerstore.ReferrerStore var activePolicyEnforcer policyprovider.PolicyProvider - // check if there are active verifiers from crd controller - if len(controllers.VerifierMap) > 0 { - for _, value := range controllers.VerifierMap { - activeVerifiers = append(activeVerifiers, value) - } - } - // check if there are active stores from crd controller if len(controllers.StoreMap) > 0 { for _, value := range controllers.StoreMap { @@ -109,7 +100,7 @@ func StartServer(httpServerAddress, configFilePath, certDirectory, caCertFile st // return executor with latest configuration executor := ef.Executor{ - Verifiers: activeVerifiers, + Verifiers: controllers.VerifierMap, ReferrerStores: activeStores, PolicyEnforcer: activePolicyEnforcer, Config: &cf.ExecutorConfig, diff --git a/pkg/verifier/factory/factory.go b/pkg/verifier/factory/factory.go index 43e16be3c..53dc5defa 100644 --- a/pkg/verifier/factory/factory.go +++ b/pkg/verifier/factory/factory.go @@ -23,6 +23,7 @@ import ( re "github.com/deislabs/ratify/errors" pluginCommon "github.com/deislabs/ratify/pkg/common/plugin" + "github.com/deislabs/ratify/pkg/customresources/verifiers" "github.com/deislabs/ratify/pkg/featureflag" "github.com/deislabs/ratify/pkg/verifier" "github.com/deislabs/ratify/pkg/verifier/config" @@ -102,7 +103,7 @@ func CreateVerifierFromConfig(verifierConfig config.VerifierConfig, configVersio // TODO pointer to avoid copy // returns an array of verifiers from VerifiersConfig -func CreateVerifiersFromConfig(verifiersConfig config.VerifiersConfig, defaultPluginPath string, namespace string) ([]verifier.ReferenceVerifier, error) { +func CreateVerifiersFromConfig(verifiersConfig config.VerifiersConfig, defaultPluginPath string, namespace string) (verifiers.Verifiers, error) { if verifiersConfig.Version == "" { verifiersConfig.Version = types.SpecVersion } @@ -116,7 +117,7 @@ func CreateVerifiersFromConfig(verifiersConfig config.VerifiersConfig, defaultPl return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail("verifiers config should have at least one verifier") } - verifiers := make([]verifier.ReferenceVerifier, 0) + verifiers := verifiers.NewActiveVerifiers() if len(verifiersConfig.PluginBinDirs) == 0 { verifiersConfig.PluginBinDirs = []string{defaultPluginPath} @@ -129,7 +130,7 @@ func CreateVerifiersFromConfig(verifiersConfig config.VerifiersConfig, defaultPl if err != nil { return nil, re.ErrorCodePluginInitFailure.WithComponentType(re.Verifier).WithError(err) } - verifiers = append(verifiers, verifier) + verifiers.AddVerifier(namespace, verifier.Name(), verifier) } return verifiers, nil diff --git a/pkg/verifier/factory/factory_test.go b/pkg/verifier/factory/factory_test.go index b8ee940b3..85100d0a7 100644 --- a/pkg/verifier/factory/factory_test.go +++ b/pkg/verifier/factory/factory_test.go @@ -82,19 +82,21 @@ func TestCreateVerifiersFromConfig_BuiltInVerifiers_ReturnsExpected(t *testing.T t.Fatalf("create verifiers failed with err %v", err) } - if len(verifiers) != 1 { - t.Fatalf("expected to have %d verifiers, actual count %d", 1, len(verifiers)) + if verifiers.GetVerifierCount() != 1 { + t.Fatalf("expected to have %d verifiers, actual count %d", 1, verifiers.GetVerifierCount()) } - if nameResult := verifiers[0].Name(); nameResult != "test-verifier-0" { + verifiers.GetVerifiers(constants.EmptyNamespace)[0].Name() + + if nameResult := verifiers.GetVerifiers(constants.EmptyNamespace)[0].Name(); nameResult != "test-verifier-0" { t.Fatalf("expected to create test-verifier-0 for test case but got %v", nameResult) } - if _, ok := verifiers[0].(*plugin.VerifierPlugin); ok { + if _, ok := verifiers.GetVerifiers(constants.EmptyNamespace)[0].(*plugin.VerifierPlugin); ok { t.Fatalf("type assertion failed expected a built in verifier") } - if verifierTest, ok := verifiers[0].(*TestVerifier); !ok { + if verifierTest, ok := verifiers.GetVerifiers(constants.EmptyNamespace)[0].(*TestVerifier); !ok { t.Fatalf("type assertion failed expected a test verifier") } else { if verifierTest.verifierDirectory != "test/dir" { @@ -124,15 +126,15 @@ func TestCreateVerifiersFromConfig_PluginVerifiers_ReturnsExpected(t *testing.T) t.Fatalf("create verifiers failed with err %v", err) } - if len(verifiers) != 1 { - t.Fatalf("expected to have %d verifiers, actual count %d", 1, len(verifiers)) + if verifiers.GetVerifierCount() != 1 { + t.Fatalf("expected to have %d verifiers, actual count %d", 1, verifiers.GetVerifierCount()) } - if verifiers[0].Name() != "plugin-verifier-0" { + if verifiers.GetVerifiers(constants.EmptyNamespace)[0].Name() != "plugin-verifier-0" { t.Fatalf("expected to create plugin-verifier-0") } - if _, ok := verifiers[0].(*plugin.VerifierPlugin); !ok { + if _, ok := verifiers.GetVerifiers(constants.EmptyNamespace)[0].(*plugin.VerifierPlugin); !ok { t.Fatalf("type assertion failed expected a plugin in verifier") } } From 20611997b287fb1996f0d953d4e3731af2c52c90 Mon Sep 17 00:00:00 2001 From: Binbin Li Date: Tue, 2 Apr 2024 11:33:15 +0000 Subject: [PATCH 3/4] feat: add context to getExecutor method --- config/config.go | 6 +- config/configManager.go | 11 +- httpserver/handlers.go | 39 ++-- httpserver/server.go | 4 +- httpserver/server_test.go | 128 ++++++++------ pkg/executor/core/executor.go | 10 +- pkg/executor/core/executor_test.go | 255 ++++++++++++++++----------- pkg/executor/core/testtypes.go | 6 +- pkg/manager/manager.go | 7 +- pkg/verifier/factory/factory.go | 7 +- pkg/verifier/factory/factory_test.go | 20 +-- 11 files changed, 285 insertions(+), 208 deletions(-) diff --git a/config/config.go b/config/config.go index e21ae119f..901a0eb74 100644 --- a/config/config.go +++ b/config/config.go @@ -26,7 +26,6 @@ import ( "github.com/deislabs/ratify/internal/constants" "github.com/deislabs/ratify/internal/logger" - "github.com/deislabs/ratify/pkg/customresources/verifiers" exConfig "github.com/deislabs/ratify/pkg/executor/config" "github.com/deislabs/ratify/pkg/homedir" "github.com/deislabs/ratify/pkg/policyprovider" @@ -35,6 +34,7 @@ import ( "github.com/deislabs/ratify/pkg/referrerstore" rsConfig "github.com/deislabs/ratify/pkg/referrerstore/config" sf "github.com/deislabs/ratify/pkg/referrerstore/factory" + "github.com/deislabs/ratify/pkg/verifier" vfConfig "github.com/deislabs/ratify/pkg/verifier/config" vf "github.com/deislabs/ratify/pkg/verifier/factory" "github.com/pkg/errors" @@ -84,7 +84,7 @@ func getHomeDir() string { } // Returns created referer store, verifier, policyprovider objects from config -func CreateFromConfig(cf Config) ([]referrerstore.ReferrerStore, verifiers.Verifiers, policyprovider.PolicyProvider, error) { +func CreateFromConfig(cf Config) ([]referrerstore.ReferrerStore, []verifier.ReferenceVerifier, policyprovider.PolicyProvider, error) { stores, err := sf.CreateStoresFromConfig(cf.StoresConfig, GetDefaultPluginPath()) if err != nil { @@ -99,7 +99,7 @@ func CreateFromConfig(cf Config) ([]referrerstore.ReferrerStore, verifiers.Verif return nil, nil, nil, errors.Wrap(err, "failed to load verifiers from config") } - logrus.Infof("verifiers successfully created. number of verifiers %d", verifiers.GetVerifierCount()) + logrus.Infof("verifiers successfully created. number of verifiers %d", len(verifiers)) policyEnforcer, err := pf.CreatePolicyProviderFromConfig(cf.PoliciesConfig) diff --git a/config/configManager.go b/config/configManager.go index e43f253d3..a507d3228 100644 --- a/config/configManager.go +++ b/config/configManager.go @@ -16,6 +16,7 @@ limitations under the License. package config import ( + "context" "os" "time" @@ -25,7 +26,7 @@ import ( "github.com/sirupsen/logrus" ) -type GetExecutor func() *ef.Executor +type GetExecutor func(context.Context) *ef.Executor var ( configHash string @@ -38,7 +39,7 @@ func GetExecutorAndWatchForUpdate(configFilePath string) (GetExecutor, error) { cf, err := Load(configFilePath) if err != nil { - return func() *ef.Executor { return &ef.Executor{} }, err + return func(context.Context) *ef.Executor { return &ef.Executor{} }, err } configHash = cf.fileHash @@ -46,7 +47,7 @@ func GetExecutorAndWatchForUpdate(configFilePath string) (GetExecutor, error) { stores, verifiers, policyEnforcer, err := CreateFromConfig(cf) if err != nil { - return func() *ef.Executor { return &ef.Executor{} }, err + return func(context.Context) *ef.Executor { return &ef.Executor{} }, err } executor = ef.Executor{ @@ -59,12 +60,12 @@ func GetExecutorAndWatchForUpdate(configFilePath string) (GetExecutor, error) { err = watchForConfigurationChange(configFilePath) if err != nil { - return func() *ef.Executor { return &ef.Executor{} }, err + return func(context.Context) *ef.Executor { return &ef.Executor{} }, err } logrus.Info("configuration successfully loaded.") - return func() *ef.Executor { return &executor }, nil + return func(context.Context) *ef.Executor { return &executor }, nil } func reloadExecutor(configFilePath string) { diff --git a/httpserver/handlers.go b/httpserver/handlers.go index d96f64e67..46267bf91 100644 --- a/httpserver/handlers.go +++ b/httpserver/handlers.go @@ -83,11 +83,6 @@ func (server *Server) verify(ctx context.Context, w http.ResponseWriter, r *http results = append(results, returnItem) mu.Unlock() }() - if err := server.validateComponents(verifyComponents); err != nil { - logger.GetLogger(ctx, server.LogOption).Error(err) - returnItem.Error = err.Error() - return - } requestKey, err := pkgUtils.ParseRequestKey(key) if err != nil { returnItem.Error = err.Error() @@ -100,6 +95,12 @@ func (server *Server) verify(ctx context.Context, w http.ResponseWriter, r *http } ctx = ctxUtils.SetContextWithNamespace(ctx, requestKey.Namespace) + if err := server.validateComponents(ctx, verifyComponents); err != nil { + logger.GetLogger(ctx, server.LogOption).Error(err) + returnItem.Error = err.Error() + return + } + if subjectReference.Digest.String() == "" { logger.GetLogger(ctx, server.LogOption).Warn("Digest should be used instead of tagged reference. The resolved digest may not point to the same signed artifact, since tags are mutable.") } @@ -129,7 +130,7 @@ func (server *Server) verify(ctx context.Context, w http.ResponseWriter, r *http verifyParameters := executor.VerifyParameters{ Subject: resolvedSubjectReference, } - if result, err = server.GetExecutor().VerifySubject(ctx, verifyParameters); err != nil { + if result, err = server.GetExecutor(ctx).VerifySubject(ctx, verifyParameters); err != nil { returnItem.Error = errors.ErrorCodeExecutorFailure.WithError(err).WithComponentType(errors.Executor).Error() return } @@ -145,7 +146,7 @@ func (server *Server) verify(ctx context.Context, w http.ResponseWriter, r *http logger.GetLogger(ctx, server.LogOption).Infof("verify result for subject %s: %s", resolvedSubjectReference, string(res)) } } - returnItem.Value = fromVerifyResult(result, server.GetExecutor().PolicyEnforcer.GetPolicyType(ctx)) + returnItem.Value = fromVerifyResult(result, server.GetExecutor(ctx).PolicyEnforcer.GetPolicyType(ctx)) logger.GetLogger(ctx, server.LogOption).Debugf("verification: execution time for image %s: %dms", resolvedSubjectReference, time.Since(routineStartTime).Milliseconds()) }(utils.SanitizeString(key), ctx) } @@ -192,11 +193,6 @@ func (server *Server) mutate(ctx context.Context, w http.ResponseWriter, r *http results = append(results, returnItem) mu.Unlock() }() - if err := server.validateComponents(mutateComponents); err != nil { - logger.GetLogger(ctx, server.LogOption).Error(err) - returnItem.Error = err.Error() - return - } requestKey, err := pkgUtils.ParseRequestKey(image) if err != nil { returnItem.Error = err.Error() @@ -209,11 +205,18 @@ func (server *Server) mutate(ctx context.Context, w http.ResponseWriter, r *http returnItem.Error = err.Error() return } + ctx = ctxUtils.SetContextWithNamespace(ctx, requestKey.Namespace) + if err := server.validateComponents(ctx, mutateComponents); err != nil { + logger.GetLogger(ctx, server.LogOption).Error(err) + returnItem.Error = err.Error() + return + } + if parsedReference.Digest == "" { var selectedStore referrerstore.ReferrerStore - for _, store := range server.GetExecutor().ReferrerStores { + for _, store := range server.GetExecutor(ctx).ReferrerStores { if store.Name() == server.MutationStoreName { selectedStore = store break @@ -243,20 +246,20 @@ func (server *Server) mutate(ctx context.Context, w http.ResponseWriter, r *http return sendResponse(&results, "", w, http.StatusOK, true) } -func (server *Server) validateComponents(handlerComponents string) error { +func (server *Server) validateComponents(ctx context.Context, handlerComponents string) error { if handlerComponents == mutateComponents { - if len(server.GetExecutor().ReferrerStores) == 0 { + if len(server.GetExecutor(ctx).ReferrerStores) == 0 { return errors.ErrorCodeConfigInvalid.WithComponentType(errors.ReferrerStore).WithDetail("referrer store config should have at least one store") } } if handlerComponents == verifyComponents { - if len(server.GetExecutor().ReferrerStores) == 0 { + if len(server.GetExecutor(ctx).ReferrerStores) == 0 { return errors.ErrorCodeConfigInvalid.WithComponentType(errors.ReferrerStore).WithDetail("referrer store config should have at least one store") } - if server.GetExecutor().PolicyEnforcer == nil { + if server.GetExecutor(ctx).PolicyEnforcer == nil { return errors.ErrorCodeConfigInvalid.WithComponentType(errors.PolicyProvider).WithDetail("policy provider config must be specified") } - if server.GetExecutor().Verifiers.IsEmpty() { + if len(server.GetExecutor(ctx).Verifiers) == 0 { return errors.ErrorCodeConfigInvalid.WithComponentType(errors.Verifier).WithDetail("verifiers config should have at least one verifier") } } diff --git a/httpserver/server.go b/httpserver/server.go index c52daf19d..7e37c0e2b 100644 --- a/httpserver/server.go +++ b/httpserver/server.go @@ -178,13 +178,13 @@ func (server *Server) registerHandlers() error { if err != nil { return err } - server.register(http.MethodPost, verifyPath, processTimeout(server.verify, server.GetExecutor().GetVerifyRequestTimeout(), false)) + server.register(http.MethodPost, verifyPath, processTimeout(server.verify, server.GetExecutor(context.Background()).GetVerifyRequestTimeout(), false)) mutatePath, err := url.JoinPath(ServerRootURL, "mutate") if err != nil { return err } - server.register(http.MethodPost, mutatePath, processTimeout(server.mutate, server.GetExecutor().GetMutationRequestTimeout(), true)) + server.register(http.MethodPost, mutatePath, processTimeout(server.mutate, server.GetExecutor(context.Background()).GetMutationRequestTimeout(), true)) return nil } diff --git a/httpserver/server_test.go b/httpserver/server_test.go index e86bff205..cb04b73d2 100644 --- a/httpserver/server_test.go +++ b/httpserver/server_test.go @@ -35,8 +35,6 @@ import ( config "github.com/deislabs/ratify/pkg/policyprovider/configpolicy" "github.com/sirupsen/logrus" - vrs "github.com/deislabs/ratify/pkg/customresources/verifiers" - "github.com/deislabs/ratify/pkg/policyprovider" "github.com/deislabs/ratify/pkg/policyprovider/types" "github.com/deislabs/ratify/pkg/referrerstore" "github.com/deislabs/ratify/pkg/referrerstore/mocks" @@ -48,8 +46,13 @@ import ( const testArtifactType string = "test-type1" const testImageNameTagged string = "localhost:5000/net-monitor:v1" -func testGetExecutor() *core.Executor { - return newTestExecutor(nil, nil, nil, nil) +func testGetExecutor(context.Context) *core.Executor { + return &core.Executor{ + Verifiers: []verifier.ReferenceVerifier{}, + ReferrerStores: []referrerstore.ReferrerStore{}, + PolicyEnforcer: nil, + Config: nil, + } } func TestNewServer_Expected(t *testing.T) { @@ -129,9 +132,13 @@ func TestServer_Timeout_Failed(t *testing.T) { }, } - ex := newTestExecutor(configPolicy, store, ver, nil) + ex := &core.Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{ver}, + } - getExecutor := func() *core.Executor { + getExecutor := func(context.Context) *core.Executor { return ex } @@ -144,7 +151,7 @@ func TestServer_Timeout_Failed(t *testing.T) { handler := contextHandler{ context: server.Context, - handler: processTimeout(server.verify, server.GetExecutor().GetVerifyRequestTimeout(), false), + handler: processTimeout(server.verify, server.GetExecutor(nil).GetVerifyRequestTimeout(), false), } handler.ServeHTTP(responseRecorder, request) @@ -192,9 +199,17 @@ func TestServer_MultipleSubjects_Success(t *testing.T) { }, } - ex := newTestExecutor(configPolicy, store, ver, nil) + ex := &core.Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{ver}, + Config: &exconfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }, + } - getExecutor := func() *core.Executor { + getExecutor := func(context.Context) *core.Executor { return ex } @@ -207,7 +222,7 @@ func TestServer_MultipleSubjects_Success(t *testing.T) { handler := contextHandler{ context: server.Context, - handler: processTimeout(server.verify, server.GetExecutor().GetVerifyRequestTimeout(), false), + handler: processTimeout(server.verify, server.GetExecutor(nil).GetVerifyRequestTimeout(), false), } handler.ServeHTTP(responseRecorder, request) @@ -259,9 +274,13 @@ func TestServer_Mutation_Success(t *testing.T) { }, } - ex := newTestExecutor(configPolicy, store, ver, nil) + ex := &core.Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{ver}, + } - getExecutor := func() *core.Executor { + getExecutor := func(context.Context) *core.Executor { return ex } @@ -275,7 +294,7 @@ func TestServer_Mutation_Success(t *testing.T) { handler := contextHandler{ context: server.Context, - handler: processTimeout(server.mutate, server.GetExecutor().GetMutationRequestTimeout(), true), + handler: processTimeout(server.mutate, server.GetExecutor(nil).GetMutationRequestTimeout(), true), } handler.ServeHTTP(responseRecorder, request) @@ -331,9 +350,13 @@ func TestServer_Mutation_ReferrerStoreConfigInvalid_Failure(t *testing.T) { }, } - ex := newTestExecutor(configPolicy, nil, ver, nil) + ex := &core.Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{}, + Verifiers: []verifier.ReferenceVerifier{ver}, + } - getExecutor := func() *core.Executor { + getExecutor := func(context.Context) *core.Executor { return ex } @@ -347,7 +370,7 @@ func TestServer_Mutation_ReferrerStoreConfigInvalid_Failure(t *testing.T) { handler := contextHandler{ context: server.Context, - handler: processTimeout(server.mutate, server.GetExecutor().GetMutationRequestTimeout(), true), + handler: processTimeout(server.mutate, server.GetExecutor(nil).GetMutationRequestTimeout(), true), } handler.ServeHTTP(responseRecorder, request) @@ -406,12 +429,17 @@ func TestServer_MultipleRequestsForSameSubject_Success(t *testing.T) { } verifyTimeout := 5000 - ex := newTestExecutor(configPolicy, store, ver, &exconfig.ExecutorConfig{ - VerificationRequestTimeout: &verifyTimeout, - MutationRequestTimeout: nil, - }) + ex := &core.Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{ver}, + Config: &exconfig.ExecutorConfig{ + VerificationRequestTimeout: &verifyTimeout, + MutationRequestTimeout: nil, + }, + } - getExecutor := func() *core.Executor { + getExecutor := func(context.Context) *core.Executor { return ex } @@ -424,7 +452,7 @@ func TestServer_MultipleRequestsForSameSubject_Success(t *testing.T) { handler := contextHandler{ context: server.Context, - handler: processTimeout(server.verify, server.GetExecutor().GetVerifyRequestTimeout(), false), + handler: processTimeout(server.verify, server.GetExecutor(nil).GetVerifyRequestTimeout(), false), } handler.ServeHTTP(responseRecorder, request) @@ -453,12 +481,17 @@ func TestServer_Verify_ParseReference_Failure(t *testing.T) { responseRecorder := httptest.NewRecorder() - ex := newTestExecutor(config.PolicyEnforcer{}, &mocks.TestStore{}, &core.TestVerifier{}, &exconfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }) + ex := &core.Executor{ + PolicyEnforcer: config.PolicyEnforcer{}, + ReferrerStores: []referrerstore.ReferrerStore{&mocks.TestStore{}}, + Verifiers: []verifier.ReferenceVerifier{&core.TestVerifier{}}, + Config: &exconfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }, + } - getExecutor := func() *core.Executor { + getExecutor := func(context.Context) *core.Executor { return ex } @@ -471,7 +504,7 @@ func TestServer_Verify_ParseReference_Failure(t *testing.T) { handler := contextHandler{ context: server.Context, - handler: processTimeout(server.verify, server.GetExecutor().GetVerifyRequestTimeout(), false), + handler: processTimeout(server.verify, server.GetExecutor(nil).GetVerifyRequestTimeout(), false), } handler.ServeHTTP(responseRecorder, request) @@ -525,9 +558,13 @@ func TestServer_Verify_PolicyEnforcerConfigInvalid_Failure(t *testing.T) { }, } - ex := newTestExecutor(nil, store, ver, nil) + ex := &core.Executor{ + PolicyEnforcer: nil, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{ver}, + } - getExecutor := func() *core.Executor { + getExecutor := func(context.Context) *core.Executor { return ex } @@ -541,7 +578,7 @@ func TestServer_Verify_PolicyEnforcerConfigInvalid_Failure(t *testing.T) { handler := contextHandler{ context: server.Context, - handler: processTimeout(server.verify, server.GetExecutor().GetVerifyRequestTimeout(), false), + handler: processTimeout(server.verify, server.GetExecutor(nil).GetVerifyRequestTimeout(), false), } handler.ServeHTTP(responseRecorder, request) @@ -590,9 +627,13 @@ func TestServer_Verify_VerifierConfigInvalid_Failure(t *testing.T) { }, } - ex := newTestExecutor(configPolicy, store, nil, nil) + ex := &core.Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{}, + } - getExecutor := func() *core.Executor { + getExecutor := func(context.Context) *core.Executor { return ex } @@ -606,7 +647,7 @@ func TestServer_Verify_VerifierConfigInvalid_Failure(t *testing.T) { handler := contextHandler{ context: server.Context, - handler: processTimeout(server.verify, server.GetExecutor().GetVerifyRequestTimeout(), false), + handler: processTimeout(server.verify, server.GetExecutor(nil).GetVerifyRequestTimeout(), false), } handler.ServeHTTP(responseRecorder, request) @@ -675,22 +716,3 @@ func TestServer_serverGracefulShutdown(t *testing.T) { // wait some time to see shutdown logs time.Sleep(5 * time.Second) } - -func newTestExecutor(policyEnforcer policyprovider.PolicyProvider, store referrerstore.ReferrerStore, verifier verifier.ReferenceVerifier, config *exconfig.ExecutorConfig) *core.Executor { - activeStores := make([]referrerstore.ReferrerStore, 0) - if store != nil { - activeStores = append(activeStores, store) - } - - activeVerifiers := vrs.NewActiveVerifiers() - if verifier != nil { - activeVerifiers.AddVerifier("", verifier.Name(), verifier) - } - - return &core.Executor{ - PolicyEnforcer: policyEnforcer, - ReferrerStores: activeStores, - Verifiers: activeVerifiers, - Config: config, - } -} diff --git a/pkg/executor/core/executor.go b/pkg/executor/core/executor.go index 61ff05612..74a071d26 100644 --- a/pkg/executor/core/executor.go +++ b/pkg/executor/core/executor.go @@ -22,10 +22,8 @@ import ( "time" "github.com/deislabs/ratify/errors" - ctxUtils "github.com/deislabs/ratify/internal/context" "github.com/deislabs/ratify/internal/logger" "github.com/deislabs/ratify/pkg/common" - "github.com/deislabs/ratify/pkg/customresources/verifiers" e "github.com/deislabs/ratify/pkg/executor" "github.com/deislabs/ratify/pkg/executor/config" "github.com/deislabs/ratify/pkg/executor/types" @@ -55,8 +53,8 @@ var logOpt = logger.Option{ type Executor struct { ReferrerStores []referrerstore.ReferrerStore PolicyEnforcer policyprovider.PolicyProvider - verifiers.Verifiers - Config *config.ExecutorConfig + Verifiers []vr.ReferenceVerifier + Config *config.ExecutorConfig } // TODO Logging within executor @@ -170,7 +168,7 @@ func (executor Executor) verifyReferenceForJSONPolicy(ctx context.Context, subje var verifyResults []interface{} var isSuccess = true - for _, verifier := range executor.GetVerifiers(ctxUtils.GetNamespace(ctx)) { + for _, verifier := range executor.Verifiers { if verifier.CanVerify(ctx, referenceDesc) { verifierStartTime := time.Now() verifyResult, err := verifier.Verify(ctx, subjectRef, referenceDesc, referrerStore) @@ -215,7 +213,7 @@ func (executor Executor) verifyReferenceForRegoPolicy(ctx context.Context, subje return executor.addNestedReports(errCtx, referenceDesc, subjectRef, &nestedReport) }) - for _, verifier := range executor.GetVerifiers(ctxUtils.GetNamespace(ctx)) { + for _, verifier := range executor.Verifiers { if !verifier.CanVerify(ctx, referenceDesc) { continue } diff --git a/pkg/executor/core/executor_test.go b/pkg/executor/core/executor_test.go index 31dcfb980..36edaff76 100644 --- a/pkg/executor/core/executor_test.go +++ b/pkg/executor/core/executor_test.go @@ -24,7 +24,6 @@ import ( ratifyerrors "github.com/deislabs/ratify/errors" "github.com/deislabs/ratify/pkg/common" - vrs "github.com/deislabs/ratify/pkg/customresources/verifiers" e "github.com/deislabs/ratify/pkg/executor" exConfig "github.com/deislabs/ratify/pkg/executor/config" "github.com/deislabs/ratify/pkg/executor/types" @@ -158,7 +157,7 @@ func (v *mockVerifier) GetNestedReferences() []string { } func TestVerifySubjectInternal_ResolveSubjectDescriptor_Failed(t *testing.T) { - executor := newEmptyExecutor() + executor := Executor{} verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -180,7 +179,10 @@ func TestVerifySubjectInternal_ResolveSubjectDescriptor_Success(t *testing.T) { }, } - executor := newTestExecutor(&mockPolicyProvider{}, store, nil, nil) + executor := Executor{ + ReferrerStores: []referrerstore.ReferrerStore{store}, + PolicyEnforcer: &mockPolicyProvider{}, + } verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -194,17 +196,19 @@ func TestVerifySubjectInternal_ResolveSubjectDescriptor_Success(t *testing.T) { func TestVerifySubjectInternal_Verify_NoReferrers(t *testing.T) { testDigest := digest.FromString("test") configPolicy := policyConfig.PolicyEnforcer{} - ex := newTestExecutor(configPolicy, - &mocks.TestStore{ + ex := &Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{&mocks.TestStore{ ResolveMap: map[string]digest.Digest{ "v1": testDigest, }, - }, - []verifier.ReferenceVerifier{&TestVerifier{}}, - &exConfig.ExecutorConfig{ + }}, + Verifiers: []verifier.ReferenceVerifier{&TestVerifier{}}, + Config: &exConfig.ExecutorConfig{ VerificationRequestTimeout: nil, MutationRequestTimeout: nil, - }) + }, + } verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -241,10 +245,15 @@ func TestVerifySubjectInternal_CanVerify_ExpectedResults(t *testing.T) { }, } - ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{ver}, &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }) + ex := &Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{ver}, + Config: &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }, + } verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -292,10 +301,15 @@ func TestVerifySubjectInternal_VerifyFailures_ExpectedResults(t *testing.T) { }, } - ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{ver}, &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }) + ex := &Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{ver}, + Config: &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }, + } verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -339,10 +353,15 @@ func TestVerifySubjectInternal_VerifySuccess_ExpectedResults(t *testing.T) { }, } - ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{ver}, &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }) + ex := &Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{ver}, + Config: &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }, + } verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -394,10 +413,15 @@ func TestVerifySubjectInternalWithDecision_MultipleArtifacts_ExpectedResults(t * }, } - ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{ver}, &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: nil, - }) + ex := &Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{ver}, + Config: &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }, + } verifyParameters := e.VerifyParameters{ Subject: "localhost:5000/net-monitor:v1", @@ -440,7 +464,6 @@ func TestVerifySubjectInternal_NestedReferences_Expected(t *testing.T) { return true }, nestedReferences: []string{"string-content-does-not-matter"}, - name: "sbom-verifier", } signatureVerifier := &TestVerifier{ @@ -452,7 +475,15 @@ func TestVerifySubjectInternal_NestedReferences_Expected(t *testing.T) { }, } - ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{sbomVerifier, signatureVerifier}, &exConfig.ExecutorConfig{}) + ex := &Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{sbomVerifier, signatureVerifier}, + Config: &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }, + } verifyParameters := e.VerifyParameters{ Subject: mocks.TestSubjectWithDigest, @@ -510,7 +541,6 @@ func TestVerifySubjectInternal_NoNestedReferences_Expected(t *testing.T) { VerifyResult: func(artifactType string) bool { return true }, - name: "sbom-verifier", } signatureVer := &TestVerifier{ @@ -522,7 +552,15 @@ func TestVerifySubjectInternal_NoNestedReferences_Expected(t *testing.T) { }, } - ex := newTestExecutor(configPolicy, store, []verifier.ReferenceVerifier{sbomVer, signatureVer}, &exConfig.ExecutorConfig{}) + ex := &Executor{ + PolicyEnforcer: configPolicy, + ReferrerStores: []referrerstore.ReferrerStore{store}, + Verifiers: []verifier.ReferenceVerifier{sbomVer, signatureVer}, + Config: &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }, + } verifyParameters := e.VerifyParameters{ Subject: mocks.TestSubjectWithDigest, @@ -565,21 +603,39 @@ func TestGetVerifyRequestTimeout_ExpectedResults(t *testing.T) { expectedTimeout int }{ { - setTimeout: -1, - ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, nil), + setTimeout: -1, + ex: Executor{ + PolicyEnforcer: policyConfig.PolicyEnforcer{}, + ReferrerStores: []referrerstore.ReferrerStore{}, + Verifiers: []verifier.ReferenceVerifier{}, + Config: nil, + }, expectedTimeout: 2900, }, { - setTimeout: -1, - ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, &exConfig.ExecutorConfig{}), + setTimeout: -1, + ex: Executor{ + PolicyEnforcer: policyConfig.PolicyEnforcer{}, + ReferrerStores: []referrerstore.ReferrerStore{}, + Verifiers: []verifier.ReferenceVerifier{}, + Config: &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }, + }, expectedTimeout: 2900, }, { setTimeout: 5000, - ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, &exConfig.ExecutorConfig{ - VerificationRequestTimeout: new(int), - MutationRequestTimeout: nil, - }), + ex: Executor{ + PolicyEnforcer: policyConfig.PolicyEnforcer{}, + ReferrerStores: []referrerstore.ReferrerStore{}, + Verifiers: []verifier.ReferenceVerifier{}, + Config: &exConfig.ExecutorConfig{ + VerificationRequestTimeout: new(int), + MutationRequestTimeout: nil, + }, + }, expectedTimeout: 5000, }, } @@ -604,21 +660,39 @@ func TestGetMutationRequestTimeout_ExpectedResults(t *testing.T) { expectedTimeout int }{ { - setTimeout: -1, - ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, nil), + setTimeout: -1, + ex: Executor{ + PolicyEnforcer: policyConfig.PolicyEnforcer{}, + ReferrerStores: []referrerstore.ReferrerStore{}, + Verifiers: []verifier.ReferenceVerifier{}, + Config: nil, + }, expectedTimeout: 950, }, { - setTimeout: -1, - ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, &exConfig.ExecutorConfig{}), + setTimeout: -1, + ex: Executor{ + PolicyEnforcer: policyConfig.PolicyEnforcer{}, + ReferrerStores: []referrerstore.ReferrerStore{}, + Verifiers: []verifier.ReferenceVerifier{}, + Config: &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: nil, + }, + }, expectedTimeout: 950, }, { setTimeout: 2400, - ex: *newTestExecutor(policyConfig.PolicyEnforcer{}, nil, nil, &exConfig.ExecutorConfig{ - VerificationRequestTimeout: nil, - MutationRequestTimeout: new(int), - }), + ex: Executor{ + PolicyEnforcer: policyConfig.PolicyEnforcer{}, + ReferrerStores: []referrerstore.ReferrerStore{}, + Verifiers: []verifier.ReferenceVerifier{}, + Config: &exConfig.ExecutorConfig{ + VerificationRequestTimeout: nil, + MutationRequestTimeout: new(int), + }, + }, expectedTimeout: 2400, }, } @@ -640,7 +714,7 @@ func TestVerifySubject(t *testing.T) { name string params e.VerifyParameters expectedResult types.VerifyResult - store referrerstore.ReferrerStore + stores []referrerstore.ReferrerStore policyEnforcer policyprovider.PolicyProvider verifiers []verifier.ReferenceVerifier referrers []ocispecs.ReferenceDescriptor @@ -659,8 +733,10 @@ func TestVerifySubject(t *testing.T) { params: e.VerifyParameters{ Subject: subject1, }, - store: &mockStore{ - referrers: nil, + stores: []referrerstore.ReferrerStore{ + &mockStore{ + referrers: nil, + }, }, policyEnforcer: &mockPolicyProvider{ policyType: pt.RegoPolicy, @@ -673,8 +749,10 @@ func TestVerifySubject(t *testing.T) { params: e.VerifyParameters{ Subject: subject1, }, - store: &mockStore{ - referrers: nil, + stores: []referrerstore.ReferrerStore{ + &mockStore{ + referrers: nil, + }, }, policyEnforcer: &mockPolicyProvider{ policyType: pt.RegoPolicy, @@ -687,13 +765,15 @@ func TestVerifySubject(t *testing.T) { params: e.VerifyParameters{ Subject: subject1, }, - store: &mockStore{ - referrers: map[string][]ocispecs.ReferenceDescriptor{ - subjectDigest: { - { - ArtifactType: artifactType, - Descriptor: oci.Descriptor{ - Digest: signatureDigest, + stores: []referrerstore.ReferrerStore{ + &mockStore{ + referrers: map[string][]ocispecs.ReferenceDescriptor{ + subjectDigest: { + { + ArtifactType: artifactType, + Descriptor: oci.Descriptor{ + Digest: signatureDigest, + }, }, }, }, @@ -718,13 +798,15 @@ func TestVerifySubject(t *testing.T) { params: e.VerifyParameters{ Subject: subject1, }, - store: &mockStore{ - referrers: map[string][]ocispecs.ReferenceDescriptor{ - subjectDigest: { - { - ArtifactType: artifactType, - Descriptor: oci.Descriptor{ - Digest: signatureDigest, + stores: []referrerstore.ReferrerStore{ + &mockStore{ + referrers: map[string][]ocispecs.ReferenceDescriptor{ + subjectDigest: { + { + ArtifactType: artifactType, + Descriptor: oci.Descriptor{ + Digest: signatureDigest, + }, }, }, }, @@ -750,13 +832,15 @@ func TestVerifySubject(t *testing.T) { params: e.VerifyParameters{ Subject: subject1, }, - store: &mockStore{ - referrers: map[string][]ocispecs.ReferenceDescriptor{ - subjectDigest: { - { - ArtifactType: artifactType, - Descriptor: oci.Descriptor{ - Digest: signatureDigest, + stores: []referrerstore.ReferrerStore{ + &mockStore{ + referrers: map[string][]ocispecs.ReferenceDescriptor{ + subjectDigest: { + { + ArtifactType: artifactType, + Descriptor: oci.Descriptor{ + Digest: signatureDigest, + }, }, }, }, @@ -783,7 +867,7 @@ func TestVerifySubject(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - ex := newTestExecutor(tc.policyEnforcer, tc.store, tc.verifiers, nil) + ex := &Executor{tc.stores, tc.policyEnforcer, tc.verifiers, nil} result, err := ex.VerifySubject(context.Background(), tc.params) if (err != nil) != tc.expectErr { @@ -795,30 +879,3 @@ func TestVerifySubject(t *testing.T) { }) } } - -func newTestExecutor(policyEnforcer policyprovider.PolicyProvider, store referrerstore.ReferrerStore, verifiers []verifier.ReferenceVerifier, config *exConfig.ExecutorConfig) *Executor { - activeStores := make([]referrerstore.ReferrerStore, 0) - if store != nil { - activeStores = append(activeStores, store) - } - - activeVerifiers := vrs.NewActiveVerifiers() - for _, verifier := range verifiers { - activeVerifiers.AddVerifier("", verifier.Name(), verifier) - } - - return &Executor{ - PolicyEnforcer: policyEnforcer, - ReferrerStores: activeStores, - Verifiers: activeVerifiers, - Config: config, - } -} - -func newEmptyExecutor() Executor { - verifiers := vrs.NewActiveVerifiers() - - return Executor{ - Verifiers: verifiers, - } -} diff --git a/pkg/executor/core/testtypes.go b/pkg/executor/core/testtypes.go index d16409ef8..fd28a410d 100644 --- a/pkg/executor/core/testtypes.go +++ b/pkg/executor/core/testtypes.go @@ -28,14 +28,10 @@ type TestVerifier struct { CanVerifyFunc func(artifactType string) bool VerifyResult func(artifactType string) bool nestedReferences []string - name string } func (s *TestVerifier) Name() string { - if len(s.name) == 0 { - return "verifier-testVerifier" - } - return s.name + return "verifier-testVerifier" } func (s *TestVerifier) Type() string { diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 38679443f..230e17f5f 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -49,6 +49,7 @@ import ( configv1alpha1 "github.com/deislabs/ratify/api/v1alpha1" configv1beta1 "github.com/deislabs/ratify/api/v1beta1" + ctxUtils "github.com/deislabs/ratify/internal/context" "github.com/deislabs/ratify/pkg/controllers" ef "github.com/deislabs/ratify/pkg/executor/core" "github.com/deislabs/ratify/pkg/referrerstore" @@ -83,10 +84,12 @@ func StartServer(httpServerAddress, configFilePath, certDirectory, caCertFile st } // initialize server - server, err := httpserver.NewServer(context.Background(), httpServerAddress, func() *ef.Executor { + 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)) + // check if there are active stores from crd controller if len(controllers.StoreMap) > 0 { for _, value := range controllers.StoreMap { @@ -100,7 +103,7 @@ func StartServer(httpServerAddress, configFilePath, certDirectory, caCertFile st // return executor with latest configuration executor := ef.Executor{ - Verifiers: controllers.VerifierMap, + Verifiers: activeVerifiers, ReferrerStores: activeStores, PolicyEnforcer: activePolicyEnforcer, Config: &cf.ExecutorConfig, diff --git a/pkg/verifier/factory/factory.go b/pkg/verifier/factory/factory.go index 53dc5defa..43e16be3c 100644 --- a/pkg/verifier/factory/factory.go +++ b/pkg/verifier/factory/factory.go @@ -23,7 +23,6 @@ import ( re "github.com/deislabs/ratify/errors" pluginCommon "github.com/deislabs/ratify/pkg/common/plugin" - "github.com/deislabs/ratify/pkg/customresources/verifiers" "github.com/deislabs/ratify/pkg/featureflag" "github.com/deislabs/ratify/pkg/verifier" "github.com/deislabs/ratify/pkg/verifier/config" @@ -103,7 +102,7 @@ func CreateVerifierFromConfig(verifierConfig config.VerifierConfig, configVersio // TODO pointer to avoid copy // returns an array of verifiers from VerifiersConfig -func CreateVerifiersFromConfig(verifiersConfig config.VerifiersConfig, defaultPluginPath string, namespace string) (verifiers.Verifiers, error) { +func CreateVerifiersFromConfig(verifiersConfig config.VerifiersConfig, defaultPluginPath string, namespace string) ([]verifier.ReferenceVerifier, error) { if verifiersConfig.Version == "" { verifiersConfig.Version = types.SpecVersion } @@ -117,7 +116,7 @@ func CreateVerifiersFromConfig(verifiersConfig config.VerifiersConfig, defaultPl return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail("verifiers config should have at least one verifier") } - verifiers := verifiers.NewActiveVerifiers() + verifiers := make([]verifier.ReferenceVerifier, 0) if len(verifiersConfig.PluginBinDirs) == 0 { verifiersConfig.PluginBinDirs = []string{defaultPluginPath} @@ -130,7 +129,7 @@ func CreateVerifiersFromConfig(verifiersConfig config.VerifiersConfig, defaultPl if err != nil { return nil, re.ErrorCodePluginInitFailure.WithComponentType(re.Verifier).WithError(err) } - verifiers.AddVerifier(namespace, verifier.Name(), verifier) + verifiers = append(verifiers, verifier) } return verifiers, nil diff --git a/pkg/verifier/factory/factory_test.go b/pkg/verifier/factory/factory_test.go index 85100d0a7..b8ee940b3 100644 --- a/pkg/verifier/factory/factory_test.go +++ b/pkg/verifier/factory/factory_test.go @@ -82,21 +82,19 @@ func TestCreateVerifiersFromConfig_BuiltInVerifiers_ReturnsExpected(t *testing.T t.Fatalf("create verifiers failed with err %v", err) } - if verifiers.GetVerifierCount() != 1 { - t.Fatalf("expected to have %d verifiers, actual count %d", 1, verifiers.GetVerifierCount()) + if len(verifiers) != 1 { + t.Fatalf("expected to have %d verifiers, actual count %d", 1, len(verifiers)) } - verifiers.GetVerifiers(constants.EmptyNamespace)[0].Name() - - if nameResult := verifiers.GetVerifiers(constants.EmptyNamespace)[0].Name(); nameResult != "test-verifier-0" { + if nameResult := verifiers[0].Name(); nameResult != "test-verifier-0" { t.Fatalf("expected to create test-verifier-0 for test case but got %v", nameResult) } - if _, ok := verifiers.GetVerifiers(constants.EmptyNamespace)[0].(*plugin.VerifierPlugin); ok { + if _, ok := verifiers[0].(*plugin.VerifierPlugin); ok { t.Fatalf("type assertion failed expected a built in verifier") } - if verifierTest, ok := verifiers.GetVerifiers(constants.EmptyNamespace)[0].(*TestVerifier); !ok { + if verifierTest, ok := verifiers[0].(*TestVerifier); !ok { t.Fatalf("type assertion failed expected a test verifier") } else { if verifierTest.verifierDirectory != "test/dir" { @@ -126,15 +124,15 @@ func TestCreateVerifiersFromConfig_PluginVerifiers_ReturnsExpected(t *testing.T) t.Fatalf("create verifiers failed with err %v", err) } - if verifiers.GetVerifierCount() != 1 { - t.Fatalf("expected to have %d verifiers, actual count %d", 1, verifiers.GetVerifierCount()) + if len(verifiers) != 1 { + t.Fatalf("expected to have %d verifiers, actual count %d", 1, len(verifiers)) } - if verifiers.GetVerifiers(constants.EmptyNamespace)[0].Name() != "plugin-verifier-0" { + if verifiers[0].Name() != "plugin-verifier-0" { t.Fatalf("expected to create plugin-verifier-0") } - if _, ok := verifiers.GetVerifiers(constants.EmptyNamespace)[0].(*plugin.VerifierPlugin); !ok { + if _, ok := verifiers[0].(*plugin.VerifierPlugin); !ok { t.Fatalf("type assertion failed expected a plugin in verifier") } } From 810c93ed395076e9cd3babd2a1c925b99cf4de9e Mon Sep 17 00:00:00 2001 From: Binbin Li Date: Tue, 9 Apr 2024 03:10:27 +0000 Subject: [PATCH 4/4] chore: address comments --- httpserver/server.go | 4 ++-- pkg/controllers/verifier_controller.go | 1 + pkg/customresources/verifiers/api.go | 4 ++-- pkg/customresources/verifiers/verifiers.go | 21 +++++++++++---------- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/httpserver/server.go b/httpserver/server.go index 7e37c0e2b..bca7db43a 100644 --- a/httpserver/server.go +++ b/httpserver/server.go @@ -178,13 +178,13 @@ func (server *Server) registerHandlers() error { if err != nil { return err } - server.register(http.MethodPost, verifyPath, processTimeout(server.verify, server.GetExecutor(context.Background()).GetVerifyRequestTimeout(), false)) + server.register(http.MethodPost, verifyPath, processTimeout(server.verify, server.GetExecutor(server.Context).GetVerifyRequestTimeout(), false)) mutatePath, err := url.JoinPath(ServerRootURL, "mutate") if err != nil { return err } - server.register(http.MethodPost, mutatePath, processTimeout(server.mutate, server.GetExecutor(context.Background()).GetMutationRequestTimeout(), true)) + server.register(http.MethodPost, mutatePath, processTimeout(server.mutate, server.GetExecutor(server.Context).GetMutationRequestTimeout(), true)) return nil } diff --git a/pkg/controllers/verifier_controller.go b/pkg/controllers/verifier_controller.go index dc39fd119..1cd8bb726 100644 --- a/pkg/controllers/verifier_controller.go +++ b/pkg/controllers/verifier_controller.go @@ -118,6 +118,7 @@ func verifierAddOrReplace(spec configv1beta1.VerifierSpec, objectName string, na logrus.Error(err, "unable to create verifier from verifier config") return err } + // TODO: pass the actual namespace once multi-tenancy is supported. VerifierMap.AddVerifier(constants.EmptyNamespace, objectName, referenceVerifier) logrus.Infof("verifier '%v' added to verifier map", referenceVerifier.Name()) diff --git a/pkg/customresources/verifiers/api.go b/pkg/customresources/verifiers/api.go index 04d6a3e0e..7d35e7be5 100644 --- a/pkg/customresources/verifiers/api.go +++ b/pkg/customresources/verifiers/api.go @@ -19,8 +19,8 @@ import ( vr "github.com/deislabs/ratify/pkg/verifier" ) -// Verifiers is an interface that defines the methods for managing verifiers across different scopes. -type Verifiers interface { +// VerifierManager is an interface that defines the methods for managing verifiers across different scopes. +type VerifierManager interface { // GetVerifiers returns verifiers under the given scope. GetVerifiers(scope string) []vr.ReferenceVerifier diff --git a/pkg/customresources/verifiers/verifiers.go b/pkg/customresources/verifiers/verifiers.go index d48aadbfa..5825353ce 100644 --- a/pkg/customresources/verifiers/verifiers.go +++ b/pkg/customresources/verifiers/verifiers.go @@ -19,7 +19,7 @@ import ( vr "github.com/deislabs/ratify/pkg/verifier" ) -// ActiveVerifiers implements Verifiers interface. +// ActiveVerifiers implements VerifierManger interface. type ActiveVerifiers struct { // TODO: Implement concurrent safety using sync.Map // The structure of the map is as follows: @@ -32,12 +32,13 @@ type ActiveVerifiers struct { // "verifier2": verifier2 // } // } - NamespacedVerifiers map[string]map[string]vr.ReferenceVerifier + // 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. + ScopedVerifiers map[string]map[string]vr.ReferenceVerifier } -func NewActiveVerifiers() Verifiers { +func NewActiveVerifiers() VerifierManager { return &ActiveVerifiers{ - NamespacedVerifiers: make(map[string]map[string]vr.ReferenceVerifier), + ScopedVerifiers: make(map[string]map[string]vr.ReferenceVerifier), } } @@ -46,7 +47,7 @@ func NewActiveVerifiers() 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.NamespacedVerifiers { + for _, namespacedVerifiers := range v.ScopedVerifiers { for _, verifier := range namespacedVerifiers { verifiers = append(verifiers, verifier) } @@ -55,14 +56,14 @@ func (v *ActiveVerifiers) GetVerifiers(_ string) []vr.ReferenceVerifier { } func (v *ActiveVerifiers) AddVerifier(scope, verifierName string, verifier vr.ReferenceVerifier) { - if _, ok := v.NamespacedVerifiers[scope]; !ok { - v.NamespacedVerifiers[scope] = make(map[string]vr.ReferenceVerifier) + if _, ok := v.ScopedVerifiers[scope]; !ok { + v.ScopedVerifiers[scope] = make(map[string]vr.ReferenceVerifier) } - v.NamespacedVerifiers[scope][verifierName] = verifier + v.ScopedVerifiers[scope][verifierName] = verifier } func (v *ActiveVerifiers) DeleteVerifier(scope, verifierName string) { - if verifiers, ok := v.NamespacedVerifiers[scope]; ok { + if verifiers, ok := v.ScopedVerifiers[scope]; ok { delete(verifiers, verifierName) } } @@ -73,7 +74,7 @@ func (v *ActiveVerifiers) IsEmpty() bool { func (v *ActiveVerifiers) GetVerifierCount() int { count := 0 - for _, verifiers := range v.NamespacedVerifiers { + for _, verifiers := range v.ScopedVerifiers { count += len(verifiers) } return count