Skip to content

Conversation

@mburke5678
Copy link
Contributor

@mburke5678 mburke5678 commented Dec 10, 2024

OSDOCS-12627 -- Namepspaced sigstore support added as TP
OSDOCS-12728 -- Cluster-wide sigstore support promoted from DP to TP

Preview:
Nodes -> Manage secure signatures with sigstore -- Minor edits

  • About the Sigstore project - Minor edits
  • About configuring Sigstore support - New module
  • Creating a cluster image policy CR - New module
  • Creating an image policy CR - New module

QE review:

  • QE has approved this change.

@mburke5678 mburke5678 added this to the Planned for 4.18 GA milestone Dec 10, 2024
@openshift-ci openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 10, 2024
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Dec 10, 2024

🤖 Thu Jan 23 15:13:55 - Prow CI generated the docs preview:

https://86087--ocpdocs-pr.netlify.app/openshift-enterprise/latest/nodes/nodes-sigstore-using.html

@openshift-ci openshift-ci bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 17, 2024
@mburke5678 mburke5678 changed the title OCPNODE-2253 Sigstore Support - OpenShift Container Image Validation for namespaced policies OSDOCS-12627/OSDOCS-12728 Sigstore Support Dec 17, 2024
@mburke5678
Copy link
Contributor Author

@QiWang19 PTAL

Copy link
Member

@QiWang19 QiWang19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just reviewed the nodes-sigstore-configure-cluster-policy.adoc for now.

@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 19, 2024
Copy link
Member

@QiWang19 QiWang19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the example output and log according to the PublicKey type imagepolicy example

@openshift-ci openshift-ci bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 28, 2024
@mburke5678
Copy link
Contributor Author

@lyman9966 PTAL

Copy link
Member

@QiWang19 QiWang19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR Looks good to me. I just left a few minor comments.

* You have a Sigstore-supported public key infrastructure (PKI), or provide link:https://docs.sigstore.dev/cosign/[Cosign public and private key pair] for signing operations.
* You have a signing process in place to sign your images.
* You have access to a registry that supports Cosign signatures.
* You enabled the required Technology Preview features for your cluster by editing the `FeatureGate` CR named `cluster`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to explicitly call out the need to mirror Sigstore signatures for OCP release images into your mirror registry before enabling Tech-Preview here? Because if you don't, the openshift ClusterImagePolicy will block your ability to move the CVO Pod to new nodes, or to update to new OCP releases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mirroring Sigstore signatures for OCP release images, is this intended for disconnected users?
I think we should also check if oc-mirror can be mentioned for signature mirroring guidance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without access to the Sigstore signatures, the current openshift ClusterImagePolicy will reject attempts to pull the release image. But with OCPSTRAT-1417 and OCPSTRAT-1869 still undelivered, they can't use oc-mirror for that yet. They'd have to use oc image mirror ... or other lower-level tooling.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under this enabled the required Technology Preview bullet, add the following documentation:

Before enabling TechPreview, mirror Sigstore signatures for OCP release images into your mirror registry if mirrors are configured for OCP release image repositories. Otherwise, the openShift ClusterImagePolicy will block your ability to move the CVO Pod to new nodes or update to new OCP releases.
You can use oc image mirror to mirror the signatures. For example:

oc image mirror source.com/image/repo:sha256-1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef.sig \
mirror.com/image/repo:sha256-1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef.sig

@wking @mburke5678 What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move the CVO Pod to new nodes

Is this a common thing to do? I am unfamiliar with the CVO. But, I see in the docs that during an update, "the current CVO pod stops, and a CVO pod that uses the new version starts". Is this what you are referring to?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wking do you think the above suggestion make sense #86087 (comment)? maybe we should not mention the mirror signature guidance since the oc-mirror is still undelivered

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move the CVO Pod to new nodes

Is this a common thing to do?

ClusterVersion updates will do this, yes. But so will anything that rolls the control plane, e.g. configuring an additional mirror registry or adjusting the Proxy configuration, or otherwise creating a new, drain-inducing MachineConfig targeting the control-plane MachineConfigPool. The MCO will drain the current CVO pod along with everything else from the node being updated, the Deployment controller will replace it with a new CVO Pod, the scheduler will put that CVO pod on a different control-plane Node (because the old one will still be cordoned and draining), and CRI-O will try to pull the new CVO Pod's requested image. Once the control-plane nodes have the tech-preview ClusterImagePolicy in place, CRI-O will reject the CVO/release-image pull unless it can find the Red Hat signature alongside the release image in its configured mirrors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit:

You can use oc image mirror to mirror the signatures. For example...

seems like a reasonable short-term approach to me, but your wording doesn't call out the importance of the OCP release image signature vs. the openshift ClusterImagePolicy, and "sorry, your CVO is dead unless you can get it back over to a Node that had already pulled that release image" seems like a high-stakes thing to surprise disconnected/restricted-network folks with. But maybe we can find words to go along with the oc image mirror ... advice to make the importance of that particular signature clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QiWang19 @wking Do I have this right?

If registry mirrors are configured for the {product-title} release image repositories (quay.io/openshift-release-dev/ocp-release and quay.io/openshift-release-dev/ocp-v4.0-art-dev), before enabling the Technology Preview feature set, you must mirror the sigstore signatures for the {product-title} release images into your mirror registry. Otherwise, the default openshift cluster image policy blocks the ability of the Cluster Version Operator to move the CVO Pod to new nodes, which blocks the node update that results from the feature set change.

@mburke5678 mburke5678 force-pushed the nodes-sigstore branch 3 times, most recently from c19bcba to d0a36c6 Compare January 16, 2025 18:36
@mburke5678 mburke5678 added the peer-review-needed Signifies that the peer review team needs to review this PR label Jan 16, 2025
@xenolinux xenolinux added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Jan 17, 2025
Copy link
Contributor

@xenolinux xenolinux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits; otherwise LGTM

@xenolinux xenolinux added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Jan 17, 2025
@mburke5678
Copy link
Contributor Author

/retest

@openshift-ci
Copy link

openshift-ci bot commented Jan 23, 2025

@mburke5678: all tests passed!

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

@mburke5678 mburke5678 merged commit 6777eb8 into openshift:main Jan 23, 2025
2 checks passed
@mburke5678 mburke5678 deleted the nodes-sigstore branch January 23, 2025 15:32
@mburke5678
Copy link
Contributor Author

/cherrypick enterprise-4.18

@openshift-cherrypick-robot

@mburke5678: new pull request created: #87533

Details

In response to this:

/cherrypick enterprise-4.18

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-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.18 peer-review-done Signifies that the peer review team has reviewed this PR size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants