From f3f2d132af5ea7b2a6baa204ea309ad5de5e9527 Mon Sep 17 00:00:00 2001 From: Binbin Li Date: Thu, 11 Apr 2024 07:45:31 +0000 Subject: [PATCH] feat: add certStoreManager interface to wrap operations on namespaced certStores --- .../certificatestore_controller.go | 18 ++-- pkg/controllers/resource_map.go | 4 + pkg/controllers/utils/cert_store.go | 28 ++++++ pkg/controllers/utils/cert_store_test.go | 35 ++++++++ pkg/customresources/certificatestores/api.go | 31 +++++++ .../certificatestores/certificatestores.go | 85 +++++++++++++++++++ .../certificatestores_test.go | 55 ++++++++++++ pkg/verifier/notation/notation.go | 1 + pkg/verifier/notation/truststore.go | 4 +- 9 files changed, 246 insertions(+), 15 deletions(-) create mode 100644 pkg/controllers/utils/cert_store.go create mode 100644 pkg/controllers/utils/cert_store_test.go create mode 100644 pkg/customresources/certificatestores/api.go create mode 100644 pkg/customresources/certificatestores/certificatestores.go create mode 100644 pkg/customresources/certificatestores/certificatestores_test.go diff --git a/pkg/controllers/certificatestore_controller.go b/pkg/controllers/certificatestore_controller.go index 8615b3b79c..2aecad5a3d 100644 --- a/pkg/controllers/certificatestore_controller.go +++ b/pkg/controllers/certificatestore_controller.go @@ -15,11 +15,11 @@ package controllers import ( "context" - "crypto/x509" "encoding/json" "fmt" configv1beta1 "github.com/deislabs/ratify/api/v1beta1" + "github.com/deislabs/ratify/internal/constants" "github.com/deislabs/ratify/pkg/certificateprovider" _ "github.com/deislabs/ratify/pkg/certificateprovider/azurekeyvault" // register azure keyvault certificate provider _ "github.com/deislabs/ratify/pkg/certificateprovider/inline" // register inline certificate provider @@ -40,11 +40,6 @@ type CertificateStoreReconciler struct { Scheme *runtime.Scheme } -var ( - // a map between CertificateStore name to array of x509 certificates - certificatesMap = map[string][]*x509.Certificate{} -) - //+kubebuilder:rbac:groups=config.ratify.deislabs.io,resources=certificatestores,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=config.ratify.deislabs.io,resources=certificatestores/status,verbs=get;update;patch //+kubebuilder:rbac:groups=config.ratify.deislabs.io,resources=certificatestores/finalizers,verbs=update @@ -68,7 +63,8 @@ func (r *CertificateStoreReconciler) Reconcile(ctx context.Context, req ctrl.Req if err := r.Get(ctx, req.NamespacedName, &certStore); err != nil { if apierrors.IsNotFound(err) { logger.Infof("deletion detected, removing certificate store %v", resource) - delete(certificatesMap, resource) + // TODO: pass the actual namespace once multi-tenancy is supported. + CertificatesMap.DeleteStore(constants.EmptyNamespace, resource) } else { logger.Error(err, "unable to fetch certificate store") } @@ -99,7 +95,8 @@ func (r *CertificateStoreReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, fmt.Errorf("Error fetching certificates in store %v with %v provider, error: %w", resource, certStore.Spec.Provider, err) } - certificatesMap[resource] = certificates + // TODO: pass the actual namespace once multi-tenancy is supported. + CertificatesMap.AddStore(constants.EmptyNamespace, resource, certificates) isFetchSuccessful = true emptyErrorString := "" writeCertStoreStatus(ctx, r, certStore, logger, isFetchSuccessful, emptyErrorString, lastFetchedTime, certAttributes) @@ -110,11 +107,6 @@ func (r *CertificateStoreReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, nil } -// returns the internal certificate map -func GetCertificatesMap() map[string][]*x509.Certificate { - return certificatesMap -} - // SetupWithManager sets up the controller with the Manager. func (r *CertificateStoreReconciler) SetupWithManager(mgr ctrl.Manager) error { pred := predicate.GenerationChangedPredicate{} diff --git a/pkg/controllers/resource_map.go b/pkg/controllers/resource_map.go index 5352a14626..c47a686874 100644 --- a/pkg/controllers/resource_map.go +++ b/pkg/controllers/resource_map.go @@ -14,6 +14,7 @@ limitations under the License. package controllers import ( + cs "github.com/deislabs/ratify/pkg/customresources/certificatestores" "github.com/deislabs/ratify/pkg/customresources/policies" rs "github.com/deislabs/ratify/pkg/customresources/referrerstores" "github.com/deislabs/ratify/pkg/customresources/verifiers" @@ -28,4 +29,7 @@ var ( // a map to track active stores StoreMap = rs.NewActiveStores() + + // a map between CertificateStore name to array of x509 certificates + CertificatesMap = cs.NewActiveCertStores() ) diff --git a/pkg/controllers/utils/cert_store.go b/pkg/controllers/utils/cert_store.go new file mode 100644 index 0000000000..0c643a2375 --- /dev/null +++ b/pkg/controllers/utils/cert_store.go @@ -0,0 +1,28 @@ +/* +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 utils + +import ( + "context" + "crypto/x509" + + ctxUtils "github.com/deislabs/ratify/internal/context" + "github.com/deislabs/ratify/pkg/controllers" +) + +// returns the internal certificate map +// TODO: returns certificates from both cluster-wide and given namespace as namespaced verifier could access both. +func GetCertificatesMap(ctx context.Context) map[string][]*x509.Certificate { + return controllers.CertificatesMap.GetCertStores(ctxUtils.GetNamespace(ctx)) +} diff --git a/pkg/controllers/utils/cert_store_test.go b/pkg/controllers/utils/cert_store_test.go new file mode 100644 index 0000000000..a7aa5b2276 --- /dev/null +++ b/pkg/controllers/utils/cert_store_test.go @@ -0,0 +1,35 @@ +/* +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 utils + +import ( + "context" + "crypto/x509" + "testing" + + "github.com/deislabs/ratify/pkg/controllers" + + ctxUtils "github.com/deislabs/ratify/internal/context" + cs "github.com/deislabs/ratify/pkg/customresources/certificatestores" +) + +func TestGetCertificatesMap(t *testing.T) { + controllers.CertificatesMap = cs.NewActiveCertStores() + controllers.CertificatesMap.AddStore("default", "default/certStore", []*x509.Certificate{}) + ctx := ctxUtils.SetContextWithNamespace(context.Background(), "default") + + if certs := GetCertificatesMap(ctx); len(certs) != 1 { + t.Fatalf("Expected 1 certificate store, got %d", len(certs)) + } +} diff --git a/pkg/customresources/certificatestores/api.go b/pkg/customresources/certificatestores/api.go new file mode 100644 index 0000000000..e6cfebafd0 --- /dev/null +++ b/pkg/customresources/certificatestores/api.go @@ -0,0 +1,31 @@ +/* +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 certificatestores + +import "crypto/x509" + +// CertStoreManager is an interface that defines the methods for managing certificate stores across different scopes. +type CertStoreManager interface { + // GetCertStores returns certificates for the given scope. + GetCertStores(scope string) map[string][]*x509.Certificate + + // AddStore adds the given certificate under the given scope. + AddStore(scope, storeName string, cert []*x509.Certificate) + + // DeleteStore deletes the certificate from the given scope. + DeleteStore(scope, storeName string) + + // IsEmpty returns true if there are no certificates. + IsEmpty() bool +} diff --git a/pkg/customresources/certificatestores/certificatestores.go b/pkg/customresources/certificatestores/certificatestores.go new file mode 100644 index 0000000000..100448a6ff --- /dev/null +++ b/pkg/customresources/certificatestores/certificatestores.go @@ -0,0 +1,85 @@ +/* +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 certificatestores + +import ( + "crypto/x509" + + "github.com/deislabs/ratify/internal/constants" +) + +// ActiveCertStores implements the CertStoreManager interface +type ActiveCertStores struct { + // TODO: Implement concurrent safety using sync.Map + // The structure of the map is as follows: + // The first level maps from scope to certificate stores. + // The second level maps from certificate store name to certificates. + // The certificate store name is prefixed with the namespace. + // Example: + // { + // "namespace1": { + // "namespace1/store1": []*x509.Certificate, + // "namespace1/store2": []*x509.Certificate + // }, + // "namespace2": { + // "namespace2/store1": []*x509.Certificate, + // "namespace2/store2": []*x509.Certificate + // } + // } + // Note: Scope is utilized for organizing and isolating cert stores. In a Kubernetes (K8s) environment, the scope can be either a namespace or an empty string ("") for cluster-wide cert stores. + ScopedCertStores map[string]map[string][]*x509.Certificate +} + +func NewActiveCertStores() CertStoreManager { + return &ActiveCertStores{ + ScopedCertStores: make(map[string]map[string][]*x509.Certificate), + } +} + +// GetCertStores fulfills the CertStoreManager interface. +// It returns a list of cert stores for the given scope. If no cert stores are found for the given scope, it returns cluster-wide cert stores. +// TODO: Current implementation always fetches cluster-wide cert stores. Will support actual namespaced certStores in future. +func (c *ActiveCertStores) GetCertStores(_ string) map[string][]*x509.Certificate { + return c.ScopedCertStores[constants.EmptyNamespace] +} + +// AddStore fulfills the CertStoreManager interface. +// It adds the given certificate under the given scope. +// TODO: Current implementation always adds the given certificate to cluster-wide cert store. Will support actual namespaced certStores in future. +func (c *ActiveCertStores) AddStore(_, storeName string, certs []*x509.Certificate) { + scope := constants.EmptyNamespace + if c.ScopedCertStores[scope] == nil { + c.ScopedCertStores[scope] = make(map[string][]*x509.Certificate) + } + c.ScopedCertStores[scope][storeName] = certs +} + +// DeleteStore fulfills the CertStoreManager interface. +// It deletes the certificate from the given scope. +// TODO: Current implementation always deletes the cluster-wide cert store. Will support actual namespaced certStores in future. +func (c *ActiveCertStores) DeleteStore(_, storeName string) { + if store, ok := c.ScopedCertStores[constants.EmptyNamespace]; ok { + delete(store, storeName) + } +} + +// IsEmpty fulfills the CertStoreManager interface. +// It returns true if there are no certificates. +func (c *ActiveCertStores) IsEmpty() bool { + count := 0 + for _, certStores := range c.ScopedCertStores { + count += len(certStores) + } + return count == 0 +} diff --git a/pkg/customresources/certificatestores/certificatestores_test.go b/pkg/customresources/certificatestores/certificatestores_test.go new file mode 100644 index 0000000000..935dc515bb --- /dev/null +++ b/pkg/customresources/certificatestores/certificatestores_test.go @@ -0,0 +1,55 @@ +/* +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 certificatestores + +import ( + "crypto/x509" + "testing" +) + +const ( + namespace1 = "namespace1" + namespace2 = "namespace2" + name1 = "name1" + name2 = "name2" +) + +func TestCertStoresOperations(t *testing.T) { + activeCertStores := NewActiveCertStores() + + if !activeCertStores.IsEmpty() { + t.Errorf("Expected activeCertStores to be empty") + } + + certStore1 := []*x509.Certificate{} + certStore2 := []*x509.Certificate{} + + activeCertStores.AddStore(namespace1, name1, certStore1) + activeCertStores.AddStore(namespace2, name2, certStore2) + + if activeCertStores.IsEmpty() { + t.Errorf("Expected activeCertStores to not be empty") + } + + if len(activeCertStores.GetCertStores(namespace1)) != 2 { + t.Errorf("Expected activeCertStores to have 2 cert store") + } + + activeCertStores.DeleteStore(namespace1, name1) + activeCertStores.DeleteStore(namespace2, name2) + + if !activeCertStores.IsEmpty() { + t.Errorf("Expected activeCertStores to be empty") + } +} diff --git a/pkg/verifier/notation/notation.go b/pkg/verifier/notation/notation.go index 7ac4b6f9ac..678f35f56d 100644 --- a/pkg/verifier/notation/notation.go +++ b/pkg/verifier/notation/notation.go @@ -221,6 +221,7 @@ func (v *notationPluginVerifier) GetNestedReferences() []string { // append namespace to certStore so they are uniquely identifiable func prependNamespaceToCertStore(verificationCertStore map[string][]string, namespace string) (map[string][]string, error) { + // TODO: once we support multi-tenancy, empty namespace would be reserved for cluster scope. if namespace == "" { return nil, re.ErrorCodeEnvNotSet.WithComponentType(re.Verifier).WithDetail("failure to parse VerificationCertStores, namespace for VerificationCertStores must be provided") } diff --git a/pkg/verifier/notation/truststore.go b/pkg/verifier/notation/truststore.go index cf7373cef6..89a7fa9686 100644 --- a/pkg/verifier/notation/truststore.go +++ b/pkg/verifier/notation/truststore.go @@ -22,7 +22,7 @@ import ( "fmt" "github.com/deislabs/ratify/internal/logger" - "github.com/deislabs/ratify/pkg/controllers" + cutils "github.com/deislabs/ratify/pkg/controllers/utils" "github.com/deislabs/ratify/pkg/keymanagementprovider" "github.com/deislabs/ratify/pkg/utils" "github.com/notaryproject/notation-go/verifier/truststore" @@ -42,7 +42,7 @@ type trustStore struct { // will be loaded for each signature verification. // And this API must follow the Notation Trust Store spec: https://github.com/notaryproject/notaryproject/blob/main/specs/trust-store-trust-policy.md#trust-store func (s trustStore) GetCertificates(ctx context.Context, _ truststore.Type, namedStore string) ([]*x509.Certificate, error) { - certs, err := s.getCertificatesInternal(ctx, namedStore, controllers.GetCertificatesMap()) + certs, err := s.getCertificatesInternal(ctx, namedStore, cutils.GetCertificatesMap(ctx)) if err != nil { return nil, err }