OTA-917: pkg/customsignaturestore: Implement signatureStores customization

[wking]

The verifier Interface's AddStore has always been:

// AddStore adds additional stores for signature verification.
func (v *releaseVerifier) AddStore(additionalStore store.Store) {
  v.store = &serial.Store{Stores: []store.Store{additionalStore, v.store}}
}

since it landed in 2020. We discussed then whether we were comfortable with "always insert the new addition serially in front of the existing stores", and decided that it was good enough then (although we neglected to commit to that implementation in the Interface's Go docs).

That history sets up my approach in this commit where I add support for ClusterVersion's new spec.signatureStores. The logic is:

1. Try and find the signature in the local ConfigMaps.
   • If we error, e.g. by failing to list ConfigMaps, fail out of the ConfigMap store, which will fail out of the serial store, which will fail verification without further signature traversal.
   • If we find a valid signature, hooray, we're done.
   • If we run out of ConfigMap entries to check without finding a valid signature, fall back to...
2. ClusterVersion's spec.signatureStores.
   • If this property is set to an empty array, fail out. Admins can configure this if they only want to support ConfigMap lookup.
   • If this property is set to a non-empty array, build a parallel store out of its current contents, and search the new store:
     • If we error, check in with the call-back function about whether the error should be fatal, e.g. here. But the verifier callback is very generous about allowing failures, so we'll keep going looking at any later signatures in that sigstore and at the other parallel sigstores.
     • If we find a valid signature, hooray, we're done.
     • If we run out of sigstore entries to check without finding a valid signature, return an error, without wrapping with the callback, because we want to fail out without further signature traversal. Admins who want to check default signature stores as well can configure them explicitly in signatureStores.
     • If this property is unset, fall back to...
3. The default signature stores.
   • Same handling as the parallel store in spec.signatureStores, but with the default signature stores.

We only fall through to the default stores when ConfigMap lookup is exhausted without finding a valid signature and signatureStores is unset, to support admins who would rather not have the cluster-version operator reach out to one or more of the default stores, whether or not they have local stores they would rather use. This allows admins to construct the set of stores they would like to use, while keeping the API surface down at a single property.

Without the AddStore history, it would have been possible to construct simplified stores, like a ConfigMaps store alone in the case where signatureStores is an empty array. We could still pivot to that kind of approach with library-go changes, but for the initial implementation, I'm doing the above dance to work with AddStore as it stands.

[openshift-ci-robot]

@wking: This pull request references OTA-917 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

The verifier Interface's AddStore has always been:

// AddStore adds additional stores for signature verification.
func (v *releaseVerifier) AddStore(additionalStore store.Store) {
 v.store = &serial.Store{Stores: []store.Store{additionalStore, v.store}}
}

since it landed in 2020. We discussed then whether we were comfortable with "always insert the new addition serially in front of the existing stores", and decided that it was good enough then (although we neglected to commit to that implementation in the Interface's Go docs).

That history sets up my approach in this commit where I add support for ClusterVersion's new spec.signatureStores. The logic is:

1. Try and find the signature in the local ConfigMaps.
   • If we error, e.g. by failing to list ConfigMaps, fail out of the ConfigMap store, which will fail out of the serial store, which will fail verification without further signature traversal.
   • If we find a valid signature, hooray, we're done.

• If we run out of ConfigMap entries to check without finding a valid signature, fall back to...

2. ClusterVersion's spec.signatureStores.
   • If this property is set to an empty array, fail out. Admins can configure this if they only want to support ConfigMap lookup.
   • If this property is set to a non-empty array, build a parallel store out of its current contents, and search the new store:
     • If we error, check in with the call-back function about whether the error should be fatal, e.g. here. But the verifier callback is very generous about allowing failures, so we'll keep going looking at any later signatures in that sigstore and at the other parallel sigstores.
     • If we find a valid signature, hooray, we're done.
     • If we run out of sigstore entries to check without finding a valid signature, return an error, without wrapping with the callback, because we want to fail out without further signature traversal. Admins who want to check default signature stores as well can configure them explicitly in signatureStores.
     • If this property is unset, fall back to...
3. The default signature stores.
   • Same handling as the parallel store in spec.signatureStores, but with the default signature stores.

We only fall through to the default stores when ConfigMap lookup is exhausted without finding a valid signature and signatureStores is unset, to support admins who would rather not have the cluster-version operator reach out to one or more of the default stores, whether or not they have local stores they would rather use. This allows admins to construct the set of stores they would like to use, while keeping the API surface down at a single property.

Without the AddStore history, it would have been possible to construct simplified stores, like a ConfigMaps store alone in the case where signatureStores is an empty array. We could still pivot to that kind of approach with library-go changes, but for the initial implementation, I'm doing the above dance to work with AddStore as it stands.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[openshift-ci-robot]

@wking: No Jira issue is referenced in the title of this pull request.
To reference a jira issue, add 'XYZ-NNN:' to the title of this pull request and request another refresh with /jira refresh.

In response to this:

The verifier Interface's AddStore has always been:

// AddStore adds additional stores for signature verification.
func (v *releaseVerifier) AddStore(additionalStore store.Store) {
 v.store = &serial.Store{Stores: []store.Store{additionalStore, v.store}}
}

since it landed in 2020. We discussed then whether we were comfortable with "always insert the new addition serially in front of the existing stores", and decided that it was good enough then (although we neglected to commit to that implementation in the Interface's Go docs).

That history sets up my approach in this commit where I add support for ClusterVersion's new spec.signatureStores. The logic is:

1. Try and find the signature in the local ConfigMaps.
   • If we error, e.g. by failing to list ConfigMaps, fail out of the ConfigMap store, which will fail out of the serial store, which will fail verification without further signature traversal.
   • If we find a valid signature, hooray, we're done.

• If we run out of ConfigMap entries to check without finding a valid signature, fall back to...

2. ClusterVersion's spec.signatureStores.
   • If this property is set to an empty array, fail out. Admins can configure this if they only want to support ConfigMap lookup.
   • If this property is set to a non-empty array, build a parallel store out of its current contents, and search the new store:
     • If we error, check in with the call-back function about whether the error should be fatal, e.g. here. But the verifier callback is very generous about allowing failures, so we'll keep going looking at any later signatures in that sigstore and at the other parallel sigstores.
     • If we find a valid signature, hooray, we're done.
     • If we run out of sigstore entries to check without finding a valid signature, return an error, without wrapping with the callback, because we want to fail out without further signature traversal. Admins who want to check default signature stores as well can configure them explicitly in signatureStores.
     • If this property is unset, fall back to...
3. The default signature stores.
   • Same handling as the parallel store in spec.signatureStores, but with the default signature stores.

We only fall through to the default stores when ConfigMap lookup is exhausted without finding a valid signature and signatureStores is unset, to support admins who would rather not have the cluster-version operator reach out to one or more of the default stores, whether or not they have local stores they would rather use. This allows admins to construct the set of stores they would like to use, while keeping the API surface down at a single property.

Without the AddStore history, it would have been possible to construct simplified stores, like a ConfigMaps store alone in the case where signatureStores is an empty array. We could still pivot to that kind of approach with library-go changes, but for the initial implementation, I'm doing the above dance to work with AddStore as it stands.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[openshift-ci-robot]

@wking: This pull request references OTA-917 which is a valid jira issue.

In response to this:

The verifier Interface's AddStore has always been:

// AddStore adds additional stores for signature verification.
func (v *releaseVerifier) AddStore(additionalStore store.Store) {
 v.store = &serial.Store{Stores: []store.Store{additionalStore, v.store}}
}

since it landed in 2020. We discussed then whether we were comfortable with "always insert the new addition serially in front of the existing stores", and decided that it was good enough then (although we neglected to commit to that implementation in the Interface's Go docs).

That history sets up my approach in this commit where I add support for ClusterVersion's new spec.signatureStores. The logic is:

1. Try and find the signature in the local ConfigMaps.
   • If we error, e.g. by failing to list ConfigMaps, fail out of the ConfigMap store, which will fail out of the serial store, which will fail verification without further signature traversal.
   • If we find a valid signature, hooray, we're done.

• If we run out of ConfigMap entries to check without finding a valid signature, fall back to...

2. ClusterVersion's spec.signatureStores.
   • If this property is set to an empty array, fail out. Admins can configure this if they only want to support ConfigMap lookup.
   • If this property is set to a non-empty array, build a parallel store out of its current contents, and search the new store:
     • If we error, check in with the call-back function about whether the error should be fatal, e.g. here. But the verifier callback is very generous about allowing failures, so we'll keep going looking at any later signatures in that sigstore and at the other parallel sigstores.
     • If we find a valid signature, hooray, we're done.
     • If we run out of sigstore entries to check without finding a valid signature, return an error, without wrapping with the callback, because we want to fail out without further signature traversal. Admins who want to check default signature stores as well can configure them explicitly in signatureStores.
     • If this property is unset, fall back to...
3. The default signature stores.
   • Same handling as the parallel store in spec.signatureStores, but with the default signature stores.

We only fall through to the default stores when ConfigMap lookup is exhausted without finding a valid signature and signatureStores is unset, to support admins who would rather not have the cluster-version operator reach out to one or more of the default stores, whether or not they have local stores they would rather use. This allows admins to construct the set of stores they would like to use, while keeping the API surface down at a single property.

Without the AddStore history, it would have been possible to construct simplified stores, like a ConfigMaps store alone in the case where signatureStores is an empty array. We could still pivot to that kind of approach with library-go changes, but for the initial implementation, I'm doing the above dance to work with AddStore as it stands.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[petr-muller]

/cc

[wking]

ClusterBot testing looks good with launch 4.15,openshift/cluster-version-operator#994 gcp and:

$ oc patch clusterversion version --type json -p '[{"op": "add", "path": "/spec/upstream", "value": "https://raw.githubusercontent.com/wking/cincinnati-graph-data/signatureStores-demo/cincinnati-graph.json"}]'
$ oc adm upgrade channel demo
$ oc adm upgrade --to 4.15.1
$ oc adm upgrade
Cluster version is 4.15.0-0.test-2023-11-17-182139-ci-ln-wbgj66k-latest

ReleaseAccepted=False

  Reason: VerifyPayloadVersion
  Message: Verifying payload failed version="4.15.1" image="quay.io/openshift-release-dev/ocp-release@sha256:beda83fb057e328d6f94f8415382350ca3ddf99bb9094e262184e0f127810ce0" failure=release image version 4.13.0 does not match the expected upstream version 4.15.1

Upstream: https://raw.githubusercontent.com/wking/cincinnati-graph-data/signatureStores-demo/cincinnati-graph.json
Channel: demo

Recommended updates:

  VERSION     IMAGE
  4.15.1      quay.io/openshift-release-dev/ocp-release@sha256:beda83fb057e328d6f94f8415382350ca3ddf99bb9094e262184e0f127810ce0

That isn't initiating the update, because the 4.13.0 signature I stuck in my dummy branch's signature store doesn't match the 4.15.1 version name I stuck in my dummy-branch's graph. But it is getting past the "is there a signature that I trust" stage. And shifting the signature in my dummy branch out of the correct spot, I was getting:

Retrieving payload failed version="4.15.1" image="quay.io/openshift-release-dev/ocp-release@sha256:beda83fb057e328d6f94f8415382350ca3ddf99bb9094e262184e0f127810ce0" failure=The update cannot be verified: unable to verify sha256:beda83fb057e328d6f94f8415382350ca3ddf99bb9094e262184e0f127810ce0 against keyrings: verifier-public-key-redhat

which is my custom store config successfully blocking access to the default stores, which contain a signature for that digest.

[wking]

/payload-job periodic-ci-openshift-release-master-ci-4.15-e2e-aws-sdn-techpreview

[openshift-ci]

@wking: trigger 1 job(s) for the /payload-(job|aggregate) command

• periodic-ci-openshift-release-master-ci-4.15-e2e-aws-sdn-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/00fba6e0-92cc-11ee-87ea-f0be85fbf320-0

[wking]

Azure capacity shortage?

level=error msg=Error: waiting for creation of Linux Virtual Machine: (Name "ci-op-4jfi2pl2-31db5-wlzzz-master-2" / Resource Group "ci-op-4jfi2pl2-31db5-wlzzz-rg"): Code="OSProvisioningTimedOut" Message="OS Provisioning for VM 'ci-op-4jfi2pl2-31db5-wlzzz-master-2' did not finish in the allotted time. The VM may still finish provisioning successfully. Please check provisioning state later. Also, make sure the image has been properly prepared (generalized).\r\n * Instructions for Windows: https://azure.microsoft.com/documentation/articles/virtual-machines-windows-upload-image/ \r\n * Instructions for Linux: https://azure.microsoft.com/documentation/articles/virtual-machines-linux-capture-image/ \r\n * If you are deploying more than 20 Virtual Machines concurrently, consider moving your custom image to shared image gallery. Please refer to https://aka.ms/movetosig for the same."

/retest-required

[LalatenduMohanty]

/hold cancel

[LalatenduMohanty]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LalatenduMohanty, petr-muller, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [LalatenduMohanty,petr-muller,wking]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[LalatenduMohanty]

/retest-required

[wking]

build02 capacity issues.

/retest-required

[LalatenduMohanty]

/test e2e-agnostic-ovn

[LalatenduMohanty]

/test e2e-hypershift

[wking]

so close to branch-cut, I'm adding no-qe to lock in what we have, and we can patch what we're missing post-merge with bugs :)

[wking]

Disruption errors and HyperShift CI account OIDC quotas are both unrelated, because there isn't any CI logic yet to excercise signature retrieval (because we don't sign CI release builds).

/override ci/prow/e2e-agnostic-ovn
/override ci/prow/e2e-hypershift

[openshift-ci]

@wking: Overrode contexts on behalf of wking: ci/prow/e2e-agnostic-ovn, ci/prow/e2e-hypershift

In response to this:

Disruption errors and HyperShift CI account OIDC quotas are both unrelated, because there isn't any CI logic yet to excercise signature retrieval (because we don't sign CI release builds).

/override ci/prow/e2e-agnostic-ovn
/override ci/prow/e2e-hypershift

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[openshift-ci]

@wking: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[openshift-bot]

[ART PR BUILD NOTIFIER]

This PR has been included in build cluster-version-operator-container-v4.15.0-202312080808.p0.gebe0f34.assembly.stream for distgit cluster-version-operator.
All builds following this will include this PR.