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

charts/ratify/README.md

@@ -107,7 +107,7 @@ $ helm upgrade -n gatekeeper-system [RELEASE_NAME] ratify/ratify
 | azureWorkloadIdentity.clientId                     | ClientID of AAD application/Managed identity associated with Workload Identity                                                                                                                                                      | ``                             |
 | azureManagedIdentity.clientId                      | ClientID of Managed identity                                                                                                                                                                                                        | ``                             |
 | azureManagedIdentity.tenantId                      | TenantID of Managed Identity resource                                                                                                                                                                                               | ``                             |
-| akvCertConfig.enabled                              | Enables/disables Azure Key Vault certificate store                                                                                                                                                                                  | `false`                        |
+| akvCertConfig.enabled                              | Enables/disables Azure Key Vault certificate store. If you are using a custom chart, certificate store should be referenced through a Verifier CR. References in ConfigMap will not be correctly resolved.                                                                                                                                                                   | `false`                        |
 | akvCertConfig.vaultURI                             | Vault URI for AKV configured                                                                                                                                                                                                        | ``                             |
 | akvCertConfig.cert1Name                            | Exact name of the certificate stored in AKV                                                                                                                                                                                         | ``                             |
 | akvCertConfig.cert1Version                         | Exact version of certificate to use from AKV                                                                                                                                                                                        | ``                             |

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 K8s , 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:
+            - "*"

docs/design/Azure Kubernetes Workload Identity AuthProvider.md

@@ -81,7 +81,7 @@ EOF
 
 ## AzureWIAuthProvider Implementation
 ```
-// AzureK8Conf describes the configuration of Azure K8 Auth Provider
+// AzureK8Conf describes the configuration of Azure K8s Auth Provider
 type AzureWIConf struct {
     Name           string     `json:"name"`
 }

docs/design/K8s Secrets AuthProvider.md

@@ -1,7 +1,7 @@
 # K8s Secrets AuthProvider
 Author: Akash Singhal (@akashsinghal)
 
-Goal: Create a Kubernetes Secret Authentication Provider which will use K8 secrets to resolve registry credentials for an artifact. In the `auth-provider` section of the ORAS plugin configuration, the `k8s-secrets` auth-provider contains a list of `secrets` where each specifies the k8 secret name along with optional `namespace` where the secret resides (the namespace ratify is deployed in will be used as the default value). Along with named secrets being used from the config, the service account linked to the Ratify pod will be queried for associated imagePullSecrets and considered during credential resolution. 
+Goal: Create a Kubernetes Secret Authentication Provider which will use K8s secrets to resolve registry credentials for an artifact. In the `auth-provider` section of the ORAS plugin configuration, the `k8s-secrets` auth-provider contains a list of `secrets` where each specifies the K8s secret name along with optional `namespace` where the secret resides (the namespace ratify is deployed in will be used as the default value). Along with named secrets being used from the config, the service account linked to the Ratify pod will be queried for associated imagePullSecrets and considered during credential resolution. 
 
 The provider will support 2 types of k8s secrets: kubernetes.io/dockercfg, kubernetes.io/dockerconfigjson
 
@@ -111,7 +111,7 @@ type k8SecretAuthProviderConf struct {
 func init() // init calls Register for our k8s-secrets provider
 
 // Create returns a k8AuthProvider instance after parsing auth config and resolving
-// named K8 secrets
+// named K8s secrets
 func (s *k8SecretProviderFactory) Create(authProviderConfig AuthProviderConfig) (AuthProvider, error) {
     // unmarshal the json config for auth provider
 
@@ -130,7 +130,7 @@ func (s *k8SecretProviderFactory) Create(authProviderConfig AuthProviderConfig)
 func (d *k8SecretAuthProvider) Enabled() bool
 
 // Provide finds the secret corresponding to artifact's registryhost,
-// extracts the authentication credentials from k8 secret, and
+// extracts the authentication credentials from K8s secret, and
 // returns AuthConfig
 func (d *k8SecretAuthProvider) Provide(artifact string) (AuthConfig, error) {
     // check provider is properly Enabled

docs/design/Metrics.md

@@ -62,7 +62,7 @@ New Helm values:
 - `metricsType`: string, default: "prometheus"
 - `metricsPort`: int, default: 8888
 
-Corresponding flags for exporter and port will be added to the ratify `serve` command to enable metrics emission in non K8 scenarios. 
+Corresponding flags for exporter and port will be added to the ratify `serve` command to enable metrics emission in non K8s scenarios. 
 
 
 ## Proposed Metrics

docs/discussion/Gatekeeper Timeout Constraint.md

@@ -106,7 +106,7 @@ Now in the simple case of a single image signature verification, Ratify complete
 
 ## Questions
 1. Can we extend the timeout?
-Gatekeeper team has advised that this is not feasible since the 3 second timeout is in place to mitigate a k8 leader election issue that occures with a higher timeout. https://github.com/open-policy-agent/gatekeeper/issues/870
+Gatekeeper team has advised that this is not feasible since the 3 second timeout is in place to mitigate a K8s leader election issue that occures with a higher timeout. https://github.com/open-policy-agent/gatekeeper/issues/870
 3. Can we add a retry? Where would we add the retry?
     - kubectl doesn't seem to have retry abilities built in
     - Helm might have something we can leverage? (/cc: Sajay)

internal/constants/constants.go

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

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
@@ -60,7 +61,7 @@
 }
 
 // Create returns a k8AuthProvider instance after parsing auth config and resolving
-// named K8 secrets
+// named K8s secrets
 func (s *k8SecretProviderFactory) Create(authProviderConfig AuthProviderConfig) (AuthProvider, error) {
 	conf := k8SecretAuthProviderConf{}
 	authProviderConfigBytes, err := json.Marshal(authProviderConfig)
@@ -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{
@@ -109,10 +110,10 @@
 }
 
 // Provide finds secret corresponding to artifact's registry host name, extracts
-// the authentication credentials from k8 secret, and returns AuthConfig
+// the authentication credentials from K8s secret, and returns AuthConfig
 func (d *k8SecretAuthProvider) Provide(ctx context.Context, artifact string) (AuthConfig, error) {
 	if !d.Enabled(ctx) {
-		return AuthConfig{}, fmt.Errorf("k8 auth provider not properly enabled")
+		return AuthConfig{}, fmt.Errorf("K8s auth provider not properly enabled")
 	}
 
 	hostName, err := GetRegistryHostName(artifact)

pkg/common/oras/authprovider/k8secret_authprovider_test.go

@@ -23,7 +23,7 @@ import (
 	core "k8s.io/api/core/v1"
 )
 
-// Checks K8 Docker Json Config Secret is properly extracted and
+// Checks K8s Docker Json Config Secret is properly extracted and
 // credentials returned when Provide is called
 func TestProvide_K8SecretDockerConfigJson_ReturnsExpected(t *testing.T) {
 	var testSecret core.Secret

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)