feat: refactor certStore and KMP to support multi-tenancy [multi-tena…

…ncy PR 10] (ratify-project#1423)

Signed-off-by: akashsinghal <[email protected]>

charts/ratify/templates/inline-key-management-provider.yaml

@@ -2,7 +2,7 @@
 ---
 {{- if .Values.notationCert }} 
 apiVersion: config.ratify.deislabs.io/v1beta1
-kind: NamespacedKeyManagementProvider
+kind: KeyManagementProvider
 metadata:
   name: {{$fullname}}-notation-inline-cert
   annotations:
@@ -17,7 +17,7 @@ spec:
 ---
 {{-  range $i, $cert := .Values.notationCerts }}
 apiVersion: config.ratify.deislabs.io/v1beta1
-kind: NamespacedKeyManagementProvider
+kind: KeyManagementProvider
 metadata:
   name: {{$fullname}}-notation-inline-cert-{{$i}}
   annotations:
@@ -32,7 +32,7 @@ spec:
 ---
 {{-  range $i, $key := .Values.cosignKeys }}
 apiVersion: config.ratify.deislabs.io/v1beta1
-kind: NamespacedKeyManagementProvider
+kind: KeyManagementProvider
 metadata:
   name: {{$fullname}}-cosign-inline-key-{{$i}}
   annotations:

charts/ratify/templates/verifier.yaml

@@ -60,10 +60,10 @@ spec:
           {{- end }}
         keys:
           {{- range $i, $key := .Values.cosignKeys }}
-          - provider: {{ $.Release.Namespace }}/{{$fullname}}-cosign-inline-key-{{$i}}
+          - provider: {{$fullname}}-cosign-inline-key-{{$i}}
           {{- end }}
           {{- if and .Values.azurekeyvault.enabled (gt (len .Values.azurekeyvault.keys) 0) }}
-          - provider: {{ $.Release.Namespace }}/kmprovider-akv
+          - provider: kmprovider-akv
           {{- end }}
     {{- else }}
     key: {{ .Values.cosign.key | quote }}

pkg/controllers/certificatestore_controller.go → pkg/controllers/namespaceresource/certificatestore_controller.go

@@ -11,7 +11,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package controllers
+package namespaceresource
 
 import (
 	"context"
@@ -23,6 +23,7 @@ import (
 	"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
+	"github.com/deislabs/ratify/pkg/controllers"
 	"github.com/deislabs/ratify/pkg/utils"
 
 	"github.com/sirupsen/logrus"
@@ -63,8 +64,7 @@ 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)
-			// TODO: pass the actual namespace once multi-tenancy is supported.
-			NamespacedCertStores.DeleteStore(constants.EmptyNamespace, resource)
+			controllers.NamespacedCertStores.DeleteStore(resource)
 		} else {
 			logger.Error(err, "unable to fetch certificate store")
 		}
@@ -95,8 +95,7 @@ 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)
 	}
 
-	// TODO: pass the actual namespace once multi-tenancy is supported.
-	NamespacedCertStores.AddStore(constants.EmptyNamespace, resource, certificates)
+	controllers.NamespacedCertStores.AddStore(resource, certificates)
 	isFetchSuccessful = true
 	emptyErrorString := ""
 	writeCertStoreStatus(ctx, r, certStore, logger, isFetchSuccessful, emptyErrorString, lastFetchedTime, certAttributes)
@@ -148,8 +147,8 @@ func writeCertStoreStatus(ctx context.Context, r *CertificateStoreReconciler, ce
 func updateErrorStatus(certStore *configv1beta1.CertificateStore, errorString string, operationTime *metav1.Time) {
 	// truncate brief error string to maxBriefErrLength
 	briefErr := errorString
-	if len(errorString) > maxBriefErrLength {
-		briefErr = fmt.Sprintf("%s...", errorString[:maxBriefErrLength])
+	if len(errorString) > constants.MaxBriefErrLength {
+		briefErr = fmt.Sprintf("%s...", errorString[:constants.MaxBriefErrLength])
 	}
 	certStore.Status.IsSuccess = false
 	certStore.Status.Error = errorString

pkg/controllers/certificatestore_controller_test.go → pkg/controllers/namespaceresource/certificatestore_controller_test.go

@@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package controllers
+package namespaceresource
 
 import (
 	"fmt"

pkg/controllers/verifier_controller.go

@@ -19,13 +19,10 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"os"
 
 	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"
 	vc "github.com/deislabs/ratify/pkg/verifier/config"
 	vf "github.com/deislabs/ratify/pkg/verifier/factory"
 	"github.com/deislabs/ratify/pkg/verifier/types"
@@ -76,13 +73,7 @@ func (r *VerifierReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	namespace, err := getCertStoreNamespace(req.Namespace)
-	if err != nil {
-		verifierLogger.Error(err, "unable to get default namespace for certstore specified in verifier crd")
-		return ctrl.Result{}, err
-	}
-
-	if err = verifierAddOrReplace(verifier.Spec, resource, namespace); err != nil {
+	if err := verifierAddOrReplace(verifier.Spec, resource, constants.EmptyNamespace); err != nil {
 		verifierLogger.Error(err, "unable to create verifier from verifier crd")
 		writeVerifierStatus(ctx, r, &verifier, verifierLogger, false, err.Error())
 		return ctrl.Result{}, err
@@ -152,23 +143,6 @@ func (r *VerifierReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-// Historically certStore defined in trust policy only contains name which means the CertStore cannot be uniquely identified
-// If verifierNamespace is not empty, this method returns the default cert store namespace else returns the ratify deployed namespace
-func getCertStoreNamespace(verifierNamespace string) (string, error) {
-	// first, check if we can use the verifier namespace as the cert store namespace
-	if verifierNamespace != "" {
-		return verifierNamespace, nil
-	}
-
-	// next, return the ratify deployed namespace
-	ns, found := os.LookupEnv(utils.RatifyNamespaceEnvVar)
-	if !found {
-		return "", re.ErrorCodeEnvNotSet.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("environment variable %s not set", utils.RatifyNamespaceEnvVar))
-	}
-
-	return ns, nil
-}
-
 func writeVerifierStatus(ctx context.Context, r client.StatusClient, verifier *configv1beta1.Verifier, logger *logrus.Entry, isSuccess bool, errorString string) {
 	if isSuccess {
 		verifier.Status.IsSuccess = true

pkg/controllers/verifier_controller_test.go

@@ -212,31 +212,6 @@ func TestWriteVerifierStatus(t *testing.T) {
 	}
 }
 
-func TestGetCertStoreNamespace(t *testing.T) {
-	// error scenario, everything is empty, expect error
-	_, err := getCertStoreNamespace("")
-	if err.Error() == "environment variable" {
-		t.Fatalf("env not set should trigger an error")
-	}
-
-	ratifyDeployedNamespace := "sample"
-	os.Setenv(utils.RatifyNamespaceEnvVar, ratifyDeployedNamespace)
-	defer os.Unsetenv(utils.RatifyNamespaceEnvVar)
-
-	// scenario1, when default namespace is provided, then we should expect default
-	verifierNamespace := "verifierNamespace"
-	ns, _ := getCertStoreNamespace(verifierNamespace)
-	if ns != verifierNamespace {
-		t.Fatalf("default namespace expected")
-	}
-
-	// scenario2, default is empty, should return ratify installed namespace
-	ns, _ = getCertStoreNamespace("")
-	if ns != ratifyDeployedNamespace {
-		t.Fatalf("default namespace expected")
-	}
-}
-
 func resetVerifierMap() {
 	NamespacedVerifiers = verifiers.NewActiveVerifiers()
 }

pkg/customresources/certificatestores/api.go

@@ -13,19 +13,19 @@ limitations under the License.
 
 package certificatestores
 
-import "crypto/x509"
+import (
+	"context"
+	"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
+	// GetCertsFromStore returns certificates from the given certificate store.
+	GetCertsFromStore(ctx context.Context, storeName string) ([]*x509.Certificate, error)
 
-	// AddStore adds the given certificate under the given scope.
-	AddStore(scope, storeName string, cert []*x509.Certificate)
+	// AddStore adds the given certificate.
+	AddStore(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
+	DeleteStore(storeName string)
 }