fix: set default certstore namespace in notation verifier to uniquely identify certificate store resource

.github/workflows/build-pr.yml

@@ -203,4 +203,4 @@ jobs:
 
       - name: clean up
         run: |
-          make e2e-cleanup AZURE_SUBSCRIPTION_ID=${{ env.AZURE_SUBSCRIPTION_ID }}
+          make e2e-cleanup AZURE_SUBSCRIPTION_ID=${{ env.AZURE_SUBSCRIPTION_ID }}

Makefile

@@ -644,4 +644,4 @@ $(CONTROLLER_GEN): $(LOCALBIN)
 .PHONY: conversion-gen
 conversion-gen: $(CONVERSION_GEN) ## Download conversion-gen locally if necessary.
 $(CONVERSION_GEN): $(LOCALBIN)
-	test -s $(LOCALBIN)/conversion-gen || GOBIN=$(LOCALBIN) go install k8s.io/code-generator/cmd/conversion-gen@$(CONVERSION_TOOLS_VERSION)
+	test -s $(LOCALBIN)/conversion-gen || GOBIN=$(LOCALBIN) go install k8s.io/code-generator/cmd/conversion-gen@$(CONVERSION_TOOLS_VERSION)

charts/ratify/templates/configmap.yaml

@@ -72,14 +72,7 @@ data:
             {
                 "name":"notation",
                 "artifactTypes" : "application/vnd.cncf.notary.signature",
-                "verificationCertStores": {
-                  "certs":[
-                    {{- if .Values.akvCertConfig.enabled }}
-                    "certstore-akv"
-                    {{- else }}
-                    "{{ include "ratify.fullname" . }}-notation-inline-cert"
-                    {{- end }}
-                   ]
+                "verificationCertStores": {                 
                 },
                 "trustPolicyDoc": {
                   "version": "1.0",

cmd/ratify/cmd/verify.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	"github.com/deislabs/ratify/config"
+	"github.com/deislabs/ratify/internal/constants"
 	"github.com/deislabs/ratify/internal/logger"
 	e "github.com/deislabs/ratify/pkg/executor"
 	ef "github.com/deislabs/ratify/pkg/executor/core"
@@ -99,7 +100,7 @@ func verify(opts verifyCmdOptions) error {
 		return err
 	}
 
-	verifiers, err := vf.CreateVerifiersFromConfig(cf.VerifiersConfig, config.GetDefaultPluginPath())
+	verifiers, err := vf.CreateVerifiersFromConfig(cf.VerifiersConfig, config.GetDefaultPluginPath(), constants.EmptyNamespace)
 
 	if err != nil {
 		return err

config/config.go

@@ -24,6 +24,7 @@
 	"path/filepath"
 	"sync"
 
+	"github.com/deislabs/ratify/internal/constants"
 	"github.com/deislabs/ratify/internal/logger"
 	exConfig "github.com/deislabs/ratify/pkg/executor/config"
 	"github.com/deislabs/ratify/pkg/homedir"
@@ -91,7 +92,8 @@
 	}
 	logrus.Infof("stores successfully created. number of stores %d", len(stores))
 
-	verifiers, err := vf.CreateVerifiersFromConfig(cf.VerifiersConfig, GetDefaultPluginPath())
+	// in k8 , verifiers CR are deployed to specific namespace, namespace is not applicable in config file scenario
+	verifiers, err := vf.CreateVerifiersFromConfig(cf.VerifiersConfig, GetDefaultPluginPath(), constants.EmptyNamespace)
 
 	if err != nil {
 		return nil, nil, nil, errors.Wrap(err, "failed to load verifiers from config")

config/samples/config_v1beta1_verifier_notation_specificnscertstore.yaml

@@ -0,0 +1,23 @@
+apiVersion: config.ratify.deislabs.io/v1beta1
+kind: Verifier
+metadata:
+  name: verifier-notation
+spec:
+  name: notation
+  artifactTypes: application/vnd.cncf.notary.signature
+  parameters:
+     verificationCertStores:
+       certs:
+         - default/ratify-notation-inline-cert
+     trustPolicyDoc:
+      version: "1.0"
+      trustPolicies:
+        - name: default
+          registryScopes:
+            - "*"
+          signatureVerification:
+            level: strict
+          trustStores:
+            - ca:certs
+          trustedIdentities:
+            - "*"

internal/constants/constants.go

@@ -17,3 +17,4 @@ limitations under the License.
 package constants
 
 const RatifyPolicy = "ratify-policy"
+const EmptyNamespace = ""

pkg/common/oras/authprovider/k8secret_authprovider.go

@@ -25,6 +25,8 @@
 	"time"
 
 	re "github.com/deislabs/ratify/errors"
+	"github.com/deislabs/ratify/pkg/utils"
+
 	"github.com/docker/cli/cli/config"
 	core "k8s.io/api/core/v1"
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -51,7 +53,6 @@
 }
 
 const defaultName = "default"
-const ratifyNamespaceEnv = "RATIFY_NAMESPACE"
 const secretTimeout = time.Hour * 12
 
 // init calls Register for our k8Secrets provider
@@ -87,9 +88,9 @@
 	}
 
 	// get name of namespace ratify is running in
-	namespace := os.Getenv(ratifyNamespaceEnv)
+	namespace := os.Getenv(utils.RatifyNamespaceEnvVar)
 	if namespace == "" {
-		return nil, re.ErrorCodeEnvNotSet.WithComponentType(re.AuthProvider).WithDetail(fmt.Sprintf("environment variable %s not set", ratifyNamespaceEnv))
+		return nil, re.ErrorCodeEnvNotSet.WithComponentType(re.AuthProvider).WithDetail(fmt.Sprintf("environment variable %s not set", utils.RatifyNamespaceEnvVar))
 	}
 
 	return &k8SecretAuthProvider{

pkg/controllers/certificatestore_controller.go

@@ -65,14 +65,14 @@
 func (r *CertificateStoreReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	logger := logrus.WithContext(ctx)
 
-	var resource = req.Name
+	var resource = req.NamespacedName.String()
 	var certStore configv1beta1.CertificateStore
 
 	logger.Infof("reconciling certificate store '%v'", resource)
 
 	if err := r.Get(ctx, req.NamespacedName, &certStore); err != nil {
 		if apierrors.IsNotFound(err) {
-			logger.Infof("deletion detected, removing certificate store %v", req.Name)
+			logger.Infof("deletion detected, removing certificate store %v", resource)
 			delete(certificatesMap, resource)
 		} else {
 			logger.Error(err, "unable to fetch certificate store")

pkg/controllers/verifier_controller.go

@@ -19,13 +19,17 @@
 	"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/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"
+
 	"github.com/sirupsen/logrus"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -62,6 +66,7 @@
 
 	var verifier configv1beta1.Verifier
 	var resource = req.Name
+
 	verifierLogger.Infof("reconciling verifier '%v'", resource)
 
 	if err := r.Get(ctx, req.NamespacedName, &verifier); err != nil {
@@ -75,7 +80,13 @@
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	if err := verifierAddOrReplace(verifier.Spec, resource); err != nil {
+	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 {
 		verifierLogger.Error(err, "unable to create verifier from verifier crd")
 		return ctrl.Result{}, err
 	}
@@ -85,7 +96,7 @@
 }
 
 // creates a verifier reference from CRD spec and add store to map
-func verifierAddOrReplace(spec configv1beta1.VerifierSpec, objectName string) error {
+func verifierAddOrReplace(spec configv1beta1.VerifierSpec, objectName string, namespace string) error {
 	verifierConfig, err := specToVerifierConfig(spec)
 
 	if err != nil {
@@ -100,7 +111,7 @@
 		spec.Address = config.GetDefaultPluginPath()
 		logrus.Infof("Address was empty, setting to default path: %v", spec.Address)
 	}
-	verifierReference, err := vf.CreateVerifierFromConfig(verifierConfig, verifierConfigVersion, []string{spec.Address})
+	verifierReference, err := vf.CreateVerifierFromConfig(verifierConfig, verifierConfigVersion, []string{spec.Address}, namespace)
 
 	if err != nil || verifierReference == nil {
 		logrus.Error(err, "unable to create verifier from verifier config")
@@ -143,3 +154,20 @@
 		For(&configv1beta1.Verifier{}).
 		Complete(r)
 }
+
+// Historically certStore defined in trust policy only contains name which means the CertStore cannot be uniquely identified
+// If verifierNamesapce is not empty, this method returns the default cert store namespace else returns the ratify deployed namespace
+func getCertStoreNamespace(verifierNamesapce string) (string, error) {
+	// first, check if we can use the verifier namespace
+	if verifierNamesapce != "" {
+		return verifierNamesapce, 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
+}

pkg/controllers/verifier_controller_test.go

@@ -20,6 +20,8 @@ import (
 	"testing"
 
 	configv1beta1 "github.com/deislabs/ratify/api/v1beta1"
+	"github.com/deislabs/ratify/internal/constants"
+	"github.com/deislabs/ratify/pkg/utils"
 	vr "github.com/deislabs/ratify/pkg/verifier"
 	"k8s.io/apimachinery/pkg/runtime"
 )
@@ -39,7 +41,7 @@ func TestVerifierAdd_EmptyParameter(t *testing.T) {
 	}
 	var resource = "notation"
 
-	if err := verifierAddOrReplace(testVerifierSpec, resource); err != nil {
+	if err := verifierAddOrReplace(testVerifierSpec, resource, constants.EmptyNamespace); err != nil {
 		t.Fatalf("verifierAddOrReplace() expected no error, actual %v", err)
 	}
 	if len(VerifierMap) != 1 {
@@ -55,7 +57,7 @@ func TestVerifierAdd_WithParameters(t *testing.T) {
 
 	var testVerifierSpec = getDefaultLicenseCheckerSpec()
 
-	if err := verifierAddOrReplace(testVerifierSpec, "testObject"); err != nil {
+	if err := verifierAddOrReplace(testVerifierSpec, "testObject", constants.EmptyNamespace); err != nil {
 		t.Fatalf("verifierAddOrReplace() expected no error, actual %v", err)
 	}
 	if len(VerifierMap) != 1 {
@@ -70,7 +72,7 @@ func TestVerifier_UpdateAndDelete(t *testing.T) {
 	var testVerifierSpec = getDefaultLicenseCheckerSpec()
 
 	// add a verifier
-	if err := verifierAddOrReplace(testVerifierSpec, resource); err != nil {
+	if err := verifierAddOrReplace(testVerifierSpec, resource, constants.EmptyNamespace); err != nil {
 		t.Fatalf("verifierAddOrReplace() expected no error, actual %v", err)
 	}
 	if len(VerifierMap) != 1 {
@@ -80,7 +82,7 @@ func TestVerifier_UpdateAndDelete(t *testing.T) {
 	// modify the verifier
 	var parametersString = "{\"allowedLicenses\":[\"MIT\",\"GNU\"]}"
 	testVerifierSpec = getLicenseCheckerFromParam(parametersString)
-	if err := verifierAddOrReplace(testVerifierSpec, resource); err != nil {
+	if err := verifierAddOrReplace(testVerifierSpec, resource, constants.EmptyNamespace); err != nil {
 		t.Fatalf("verifierAddOrReplace() expected no error, actual %v", err)
 	}
 
@@ -96,6 +98,31 @@ func TestVerifier_UpdateAndDelete(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() {
 	VerifierMap = map[string]vr.ReferenceVerifier{}
 }

pkg/utils/utils.go

@@ -27,6 +27,8 @@ import (
 	"github.com/opencontainers/go-digest"
 )
 
+const RatifyNamespaceEnvVar = "RATIFY_NAMESPACE"
+
 // ParseDigest parses the given string and returns a validated Digest object.
 func ParseDigest(digestStr string) (digest.Digest, error) {
 	digest, err := digest.Parse(digestStr)

pkg/verifier/factory/factory.go

@@ -34,7 +34,7 @@ import (
 var builtInVerifiers = make(map[string]VerifierFactory)
 
 type VerifierFactory interface {
-	Create(version string, verifierConfig config.VerifierConfig, pluginDirectory string) (verifier.ReferenceVerifier, error)
+	Create(version string, verifierConfig config.VerifierConfig, pluginDirectory string, namespace string) (verifier.ReferenceVerifier, error)
 }
 
 func Register(name string, factory VerifierFactory) {
@@ -50,7 +50,8 @@ func Register(name string, factory VerifierFactory) {
 }
 
 // returns a single verifier from a verifierConfig
-func CreateVerifierFromConfig(verifierConfig config.VerifierConfig, configVersion string, pluginBinDir []string) (verifier.ReferenceVerifier, error) {
+// namespace is only applicable in k8 environment, namespace is appended to the certstore of the truststore so it is uniquely identifiable in a cluster env
+func CreateVerifierFromConfig(verifierConfig config.VerifierConfig, configVersion string, pluginBinDir []string, namespace string) (verifier.ReferenceVerifier, error) {
 	verifierName, ok := verifierConfig[types.Name]
 	if !ok {
 		return nil, re.ErrorCodeConfigInvalid.WithComponentType(re.Verifier).WithDetail(fmt.Sprintf("failed to find verifier name in the verifier config with key %s", "name"))
@@ -82,14 +83,14 @@ func CreateVerifierFromConfig(verifierConfig config.VerifierConfig, configVersio
 
 	verifierFactory, ok := builtInVerifiers[verifierNameStr]
 	if ok {
-		return verifierFactory.Create(configVersion, verifierConfig, pluginBinDir[0])
+		return verifierFactory.Create(configVersion, verifierConfig, pluginBinDir[0], namespace)
 	}
 	return plugin.NewVerifier(configVersion, verifierConfig, pluginBinDir)
 }
 
 // TODO pointer to avoid copy
 // returns an array of verifiers from VerifiersConfig
-func CreateVerifiersFromConfig(verifiersConfig config.VerifiersConfig, defaultPluginPath string) ([]verifier.ReferenceVerifier, error) {
+func CreateVerifiersFromConfig(verifiersConfig config.VerifiersConfig, defaultPluginPath string, namespace string) ([]verifier.ReferenceVerifier, error) {
 	if verifiersConfig.Version == "" {
 		verifiersConfig.Version = types.SpecVersion
 	}
@@ -112,7 +113,7 @@ func CreateVerifiersFromConfig(verifiersConfig config.VerifiersConfig, defaultPl
 
 	// TODO: do we need to append defaultPlugin path?
 	for _, verifierConfig := range verifiersConfig.Verifiers {
-		verifier, err := CreateVerifierFromConfig(verifierConfig, verifiersConfig.Version, verifiersConfig.PluginBinDirs)
+		verifier, err := CreateVerifierFromConfig(verifierConfig, verifiersConfig.Version, verifiersConfig.PluginBinDirs, namespace)
 		if err != nil {
 			return nil, re.ErrorCodePluginInitFailure.WithComponentType(re.Verifier).WithError(err)
 		}

pkg/verifier/factory/factory_test.go

@@ -19,6 +19,7 @@ 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"
@@ -52,7 +53,7 @@ func (s *TestVerifier) GetNestedReferences() []string {
 	return []string{}
 }
 
-func (f *TestVerifierFactory) Create(_ string, _ config.VerifierConfig, pluginDirectory string) (verifier.ReferenceVerifier, error) {
+func (f *TestVerifierFactory) Create(_ string, _ config.VerifierConfig, pluginDirectory string, _ string) (verifier.ReferenceVerifier, error) {
 	return &TestVerifier{verifierDirectory: pluginDirectory}, nil
 }
 
@@ -68,7 +69,7 @@ func TestCreateVerifiersFromConfig_BuiltInVerifiers_ReturnsExpected(t *testing.T
 		Verifiers: []config.VerifierConfig{verifierConfig},
 	}
 
-	verifiers, err := CreateVerifiersFromConfig(verifiersConfig, "test/dir")
+	verifiers, err := CreateVerifiersFromConfig(verifiersConfig, "test/dir", constants.EmptyNamespace)
 
 	if err != nil {
 		t.Fatalf("create verifiers failed with err %v", err)
@@ -103,7 +104,7 @@ func TestCreateVerifiersFromConfig_PluginVerifiers_ReturnsExpected(t *testing.T)
 		Verifiers: []config.VerifierConfig{verifierConfig},
 	}
 
-	verifiers, err := CreateVerifiersFromConfig(verifiersConfig, "")
+	verifiers, err := CreateVerifiersFromConfig(verifiersConfig, "", "")
 
 	if err != nil {
 		t.Fatalf("create verifiers failed with err %v", err)