same certificateStore name in different namespace

[fseldow]

What happened in your environment?

crd certificateStore is namespaced resource, which means it could exist multiple certificateStore with the same name. However, in verifier crd, the verificationCertStores/certs does not support to input namespace.

My repro:

1. curl -L https://raw.githubusercontent.com/deislabs/ratify/main/helmfile.yaml | helmfile sync -f -
2. duplicate the inline certificate store from gatekeeper-system namespace to default namespace
3. modify the certificate store under gatekeeper-system to an incorrect cert
4. run kubectl run demo --image=ghcr.io/deislabs/ratify/notary-image:signed it will reports failed. error: signature is not produced by a trusted signer
5. delete the certificate store under gatekeeper-system, rerun the pod creation, it will still failed. report error: error while loading the trust store, unable to fetch certificates for namedStore: certs
6. Restart ratify pod, the cerification will succeeded

What did you expect to happen?

No response

What version of Kubernetes are you running?

1.26.4

What version of Ratify are you running?

v1.0.0-rc.7

Anything else you would like to add?

No response

Are you willing to submit PRs to contribute to this bug fix?

• Yes, I am willing to implement it.

[susanshi]

Immediate action item: Update doc to state namespace is not supported

[susanshi]

Currently ratify CRD controller will process CR regardless of namespace. In th certStore example, if CR "myAkv" exist in both namespaces, Verifier would consume which ever is defined last.

Options:

1. Should Ratify only process CR in the ratify installed namespace? ( We haven't currently define our multi tenant scenario , is it a possibility in the future that different namespaces shares the ratify instance )

2. Currently certStores are references only through the name, should we do "namespace/name", is that a common practice?

spec:
  name: notation
  artifactTypes: application/vnd.cncf.notary.signature
  parameters:
    verificationCertStores:  # certificates for validating signatures
      certs: # name of the trustStore
        - certstore-akv # name of the certificate store CRD to include in this trustStore
        - certstore-akv1 

[fseldow]

for 1, what if i have one verifier cr that wants cluster wide shared?
for 2, i cannot say namespace/name is common practice, most are object of {name: xxx, namespace: xxx}, however, i think this change need to change the structure of the certificate store crd.

[susanshi]

For backward compatibility we have 3 options when certStore namespace is not provided in the verifier CR.

Idea 1: look in the ratify installed namespace

If customer uses the ratify charts as basis for their installation step, their custom verifier is likely to be in the ratify installed namespace. Ratify maybe installed in the gatekeeper namespace, but it makes sense for customer to install verifier and certStore in a different namespace. ( Future multi tenant scenario)

Idea 2: look in the default namespace

If customer verifier was installed in a separate namespace to Ratify, it is our best guess it is in the non default namespace. It is difficult for us to guess which namespace CRs are installed in, the customer will have to update the CR to include namespace of the certSTore. ( this would be considered a breaking change RC8 -> GA ? )

Note:
adding namespace to ratify charts also requires customer to use custom chart, since they are not likely to actually want CRs being in the default namespace out of the box.

Installing ratify CR in a different namespace to ratify may also be a confusing experience.

Idea 3: Look for certStore in the verifier namespace.
This option is most backward compat if we can assume existing ratify customers have custom verifier and certStore installed in the same Namespace.

I personally prefer Idea 3, but still need to confirm the feasibility. let me know what you think @binbin-li , @fseldow , @yizha1

[binbin-li]

@susanshi thanks for the investigation. I think it makes sense to assume verifier and certStore are defined in the same namespace. Does that mean in Ratify controllers we would need to keep a mapping from namesapce to verifier/certStore?

[fseldow]

about idea 3 i have oen question. Currenlty, verifier is not one namespaced resource. Will we make the verifier change first?

[susanshi]

thanks for the discussion @binbin-li and @fseldow. Verifier doesn't have include namespace in the ymal spec, however the CR it self will need to be applied to a specific k8 namespace, is that correct?

We will need to add namespace information to Verifier and pipe that through to trust store, so that at the time of GetCertficiate, we have the information needed to find the certStore.

 type ReferenceVerifier interface {
    // Name returns the name of the verifier
    Name() string

	Namespace ?? //only applicable for k8 clusters
	
    // CanVerify returns if the verifier can verify the given reference
    CanVerify(ctx context.Context, referenceDescriptor ocispecs.ReferenceDescriptor) bool
    // Verify verifies the given reference of a subject and returns the result of verification
    Verify(ctx context.Context,
        subjectReference common.Reference,
        referenceDescriptor ocispecs.ReferenceDescriptor,
        referrerStore referrerstore.ReferrerStore) (VerifierResult, error)
    GetNestedReferences() []string
}


type trustStore struct {
    certPaths  []string
    certStores map[string][]string
}


func getVerifierService(conf *NotationPluginVerifierConfig, pluginDirectory string) (notation.Verifier, error) {
    store := &trustStore{
        certPaths:  conf.VerificationCerts,
        certStores: conf.VerificationCertStores,
    }
    return notationVerifier.New(&conf.TrustPolicyDoc, store, NewRatifyPluginManager(pluginDirectory))
}

Timeline considerations:
Given this change impacts Both certStore and verfiers interfaces, this requires more time to bake. I prefer taking this change post ratify 1.0 . Our goal is to make this backwards and we will need to thoroughly validate both k8 and cli code path.