NO-JIRA: kms rotation controller#905
Draft
tjungblu wants to merge 2 commits into
Draft
Conversation
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
@tjungblu: This pull request explicitly references no jira issue. DetailsIn response to this: 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 openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds two ChangesModule Dependency Redirects
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 12 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Line 137: The replace directive swapping github.com/openshift/library-go to
the fork github.com/tjungblu/library-go v0.0.0-20260601112848-644b24dd829d
changes the trust boundary; add a provenance package and attestations tied to
that specific replacement: document rationale and include a verifiable SBOM
(e.g., CycloneDX or SPDX) for the forked module, provide the git commit hash
(644b24dd829d8acc3f2806deaaf6fe80e9da0c9e) and upstream commit, and attach
Sigstore/cosign signing evidence (signature + certificate) for the forked module
artifact; place these artifacts and a short summary into the repo (e.g., a
PROVENANCE.md or vendor/provenance directory) and update go.mod commit
message/PR description to reference them so reviewers can verify the replacement
of github.com/openshift/library-go with github.com/tjungblu/library-go.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cd7731f6-3681-4134-b172-e94df631c502
⛔ Files ignored due to path filters (19)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/library-go/pkg/operator/certrotation/target.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/controllers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/controllers/encryption_rotation_controller.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/encryptiondata/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/encryptionstatus/convergence.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/encryptionstatus/migration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/encryptionstatus/operator.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/encryptionstatus/rotation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/encryptionstatus/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/pkg/operator/encryption/secrets/secrets.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/test/library/encryption/kms/assets/k8s_mock_kms_plugin_configmap.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/test/library/encryption/kms/assets/k8s_mock_kms_plugin_daemonset.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/test/library/encryption/kms/assets/k8s_mock_kms_plugin_namespace.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/test/library/encryption/kms/assets/k8s_mock_kms_plugin_rolebinding.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/test/library/encryption/kms/assets/k8s_mock_kms_plugin_serviceaccount.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/test/library/encryption/kms/k8s_mock_kms_plugin_deployer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/library-go/test/library/encryption/kms/vault.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (1)
go.mod
|
|
||
| replace github.com/onsi/ginkgo/v2 => github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20251001123353-fd5b1fb35db1 | ||
|
|
||
| replace github.com/openshift/library-go => github.com/tjungblu/library-go v0.0.0-20260601112848-644b24dd829d |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Resolve module metadata from Go proxy (version existence/source timestamp).
curl -s "https://proxy.golang.org/github.com/tjungblu/library-go/@v/v0.0.0-20260601112848-644b24dd829d.info" | jq .
# 2) Check module zip/hash metadata availability (evidence for reproducibility chain).
curl -s "https://proxy.golang.org/github.com/tjungblu/library-go/@v/v0.0.0-20260601112848-644b24dd829d.mod" | sed -n '1,80p'
# 3) Query OSV for known vulns affecting the fork module coordinate.
curl -s https://api.osv.dev/v1/query -H 'content-type: application/json' -d '{
"package": {"name":"github.com/tjungblu/library-go","ecosystem":"Go"},
"version":"v0.0.0-20260601112848-644b24dd829d"
}' | jq .
# 4) Compare with upstream module/version currently required in go.mod.
curl -s https://api.osv.dev/v1/query -H 'content-type: application/json' -d '{
"package": {"name":"github.com/openshift/library-go","ecosystem":"Go"},
"version":"v0.0.0-20260527152424-3ad832f9a5a5"
}' | jq .
# 5) Optional: fetch fork repo metadata (license visibility, default branch, etc.).
curl -s "https://api.github.com/repos/tjungblu/library-go" | jq '{full_name,private,fork,license,default_branch,pushed_at}'Repository: openshift/cluster-authentication-operator
Length of output: 3953
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FORK_MOD="github.com/tjungblu/library-go"
UPSTREAM_MOD="github.com/openshift/library-go"
FORK_VER="v0.0.0-20260601112848-644b24dd829d"
UPSTREAM_VER="v0.0.0-20260527152424-3ad832f9a5a5"
echo "== go proxy .info (fork) =="
curl -s "https://proxy.golang.org/${FORK_MOD}/@v/${FORK_VER}.info" | jq .
echo "== go proxy .info (upstream) =="
curl -s "https://proxy.golang.org/${UPSTREAM_MOD}/@v/${UPSTREAM_VER}.info" | jq .
echo "== GitHub repo license (fork) =="
curl -s "https://api.github.com/repos/tjungblu/library-go" \
| jq '{full_name,license:{key: .license.key, name: .license.name, spdx_id: .license.spdx_id}, fork, private, default_branch}'
echo "== GitHub repo license (upstream) =="
curl -s "https://api.github.com/repos/openshift/library-go" \
| jq '{full_name,license:{key: .license.key, name: .license.name, spdx_id: .license.spdx_id}, fork, private, default_branch}'
echo "== OSV query (fork version) =="
curl -s https://api.osv.dev/v1/query \
-H 'content-type: application/json' \
-d "{\"package\":{\"name\":\"${FORK_MOD}\",\"ecosystem\":\"Go\"},\"version\":\"${FORK_VER}\"}" \
| jq '{vulnerabilities:(.vulns // .vulnerabilities // .))}'
# The jq above might fail if field differs; print whole response if empty
echo "== OSV raw (fork version) =="
curl -s https://api.osv.dev/v1/query \
-H 'content-type: application/json' \
-d "{\"package\":{\"name\":\"${FORK_MOD}\",\"ecosystem\":\"Go\"},\"version\":\"${FORK_VER}\"}" \
| jq .
echo "== OSV raw (upstream version) =="
curl -s https://api.osv.dev/v1/query \
-H 'content-type: application/json' \
-d "{\"package\":{\"name\":\"${UPSTREAM_MOD}\",\"ecosystem\":\"Go\"},\"version\":\"${UPSTREAM_VER}\"}" \
| jq .Repository: openshift/cluster-authentication-operator
Length of output: 1406
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FORK_MOD="github.com/tjungblu/library-go"
UPSTREAM_MOD="github.com/openshift/library-go"
FORK_VER="v0.0.0-20260601112848-644b24dd829d"
UPSTREAM_VER="v0.0.0-20260527152424-3ad832f9a5a5"
echo "== go proxy .info (fork) =="
curl -s "https://proxy.golang.org/${FORK_MOD}/@v/${FORK_VER}.info" | jq '{Version,Time,Origin,GoMod}'
echo "== go proxy .info (upstream) =="
curl -s "https://proxy.golang.org/${UPSTREAM_MOD}/@v/${UPSTREAM_VER}.info" | jq '{Version,Time,Origin,GoMod}'
echo "== GitHub repo license (fork) =="
curl -s "https://api.github.com/repos/tjungblu/library-go" \
| jq '{license:(.license // null | {key:.key,name:.name,spdx_id:.spdx_id}), default_branch, fork, private}'
echo "== GitHub repo license (upstream) =="
curl -s "https://api.github.com/repos/openshift/library-go" \
| jq '{license:(.license // null | {key:.key,name:.name,spdx_id:.spdx_id}), default_branch, fork, private}'
echo "== OSV (fork version) summary =="
curl -s https://api.osv.dev/v1/query \
-H 'content-type: application/json' \
-d "{\"package\":{\"name\":\"${FORK_MOD}\",\"ecosystem\":\"Go\"},\"version\":\"${FORK_VER}\"}" \
| jq '{vuln_count:(.vulns|length), vulns:[.vulns[]?.id, .vulns[]?.details?.summary] }'
echo "== OSV (upstream version) summary =="
curl -s https://api.osv.dev/v1/query \
-H 'content-type: application/json' \
-d "{\"package\":{\"name\":\"${UPSTREAM_MOD}\",\"ecosystem\":\"Go\"},\"version\":\"${UPSTREAM_VER}\"}" \
| jq '{vuln_count:(.vulns|length), vulns:[.vulns[]?.id, .vulns[]?.details?.summary] }'Repository: openshift/cluster-authentication-operator
Length of output: 1213
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FORK_MOD="github.com/tjungblu/library-go"
UPSTREAM_MOD="github.com/openshift/library-go"
FORK_VER="v0.0.0-20260601112848-644b24dd829d"
UPSTREAM_VER="v0.0.0-20260527152424-3ad832f9a5a5"
echo "== go proxy .info (fork) =="
curl -s "https://proxy.golang.org/${FORK_MOD}/@v/${FORK_VER}.info" | jq '{Version,Time,Origin}'
echo "== go proxy .info (upstream) =="
curl -s "https://proxy.golang.org/${UPSTREAM_MOD}/@v/${UPSTREAM_VER}.info" | jq '{Version,Time,Origin}'
echo "== GitHub repo license (fork) =="
curl -s "https://api.github.com/repos/tjungblu/library-go" \
| jq '{license:(.license // null | {key:.key,name:.name,spdx_id:.spdx_id}), default_branch, fork, private}'
echo "== GitHub repo license (upstream) =="
curl -s "https://api.github.com/repos/openshift/library-go" \
| jq '{license:(.license // null | {key:.key,name:.name,spdx_id:.spdx_id}), default_branch, fork, private}'
echo "== OSV (fork version) =="
curl -s https://api.osv.dev/v1/query \
-H 'content-type: application/json' \
-d "{\"package\":{\"name\":\"${FORK_MOD}\",\"ecosystem\":\"Go\"},\"version\":\"${FORK_VER}\"}" \
| jq '{vuln_count:(.vulns|length), vulns:((.vulns // []) | map({id, summary:(.details.summary // .summary // "")}))}'
echo "== OSV (upstream version) =="
curl -s https://api.osv.dev/v1/query \
-H 'content-type: application/json' \
-d "{\"package\":{\"name\":\"${UPSTREAM_MOD}\",\"ecosystem\":\"Go\"},\"version\":\"${UPSTREAM_VER}\"}" \
| jq '{vuln_count:(.vulns|length), vulns:((.vulns // []) | map({id, summary:(.details.summary // .summary // "")}))}'Repository: openshift/cluster-authentication-operator
Length of output: 1163
Add provenance/SBOM/signing governance for the forked module replacement
Redirecting github.com/openshift/library-go to github.com/tjungblu/library-go v0.0.0-20260601112848-644b24dd829d changes the trust boundary—include a clear rationale plus verifiable provenance/SBOM attestation and Sigstore/cosign signing evidence for the resulting artifacts.
- License compatibility: fork and upstream are both Apache-2.0.
- Vulnerabilities: OSV reports 0 known vulns for both the fork version and the upstream version.
- Go proxy origin metadata ties the fork version to commit
644b24dd829d8acc3f2806deaaf6fe80e9da0c9e(and upstream to3ad832f9a5a5302f51fff12cc91f68e74fdbb62b), but that doesn’t replace required SBOM/provenance/signing controls.
go.mod replacement
replace github.com/openshift/library-go => github.com/tjungblu/library-go v0.0.0-20260601112848-644b24dd829d🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` at line 137, The replace directive swapping
github.com/openshift/library-go to the fork github.com/tjungblu/library-go
v0.0.0-20260601112848-644b24dd829d changes the trust boundary; add a provenance
package and attestations tied to that specific replacement: document rationale
and include a verifiable SBOM (e.g., CycloneDX or SPDX) for the forked module,
provide the git commit hash (644b24dd829d8acc3f2806deaaf6fe80e9da0c9e) and
upstream commit, and attach Sigstore/cosign signing evidence (signature +
certificate) for the forked module artifact; place these artifacts and a short
summary into the repo (e.g., a PROVENANCE.md or vendor/provenance directory) and
update go.mod commit message/PR description to reference them so reviewers can
verify the replacement of github.com/openshift/library-go with
github.com/tjungblu/library-go.
ardaguclu
reviewed
Jun 1, 2026
| rotations []encryptionstatus.KMSPluginRotationStatus, | ||
| ) ([]encryptionstatus.KMSPluginRotationStatus, error) { | ||
| // Check KEK convergence across all nodes for this keyID. | ||
| convergedKEKID, converged := encryptionstatus.ConvergedKEKForKeyID(healthReports, keyID) |
Member
There was a problem hiding this comment.
I know that this is draft and WIP. Just dropped a comment if it makes sense;
In order to adopt the invariants of other controllers, if there is active migration or one of the kms plugins are not healthy, we should return just like !converged.
Contributor
Author
There was a problem hiding this comment.
thanks Arda, much appreciated. Can you leave this here? openshift/library-go#2237
This is just mostly slopware to see whether this kinda works.
Member
There was a problem hiding this comment.
I'll extensively review openshift/library-go#2237 tomorrow
Contributor
Author
There was a problem hiding this comment.
it's the same thing you just reviewed ;)
Member
There was a problem hiding this comment.
I didn't know that it is ready for review :), I'll allocate sufficient time for it
ardaguclu
reviewed
Jun 1, 2026
| } | ||
|
|
||
| // EncryptedGroupResources returns group resources from the deployed encryption configuration. | ||
| func (c *Config) EncryptedGroupResources() []schema.GroupResource { |
Member
There was a problem hiding this comment.
Instead of new function, I think it is better to use ToEncryptionState by querying key Secrets.
openshift-ci
Bot
added
the
needs-rebase
tjungblu
force-pushed
the
rotation_controller
branch
from
5da2462 to
799dca9
Compare
openshift-ci
Bot
added
needs-rebase
tjungblu
force-pushed
the
rotation_controller
branch
from
32f811c to
2728167
Compare
openshift-ci
Bot
removed
the
needs-rebase
Contributor
Author
|
/test e2e-aws-operator-encryption-kms |
tjungblu
force-pushed
the
rotation_controller
branch
from
2728167 to
7dc541a
Compare
Contributor
Author
|
/test e2e-aws-operator-encryption-kms |
Contributor
|
@tjungblu: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
tjungblu
force-pushed
the
rotation_controller
branch
from
7dc541a to
907ed99
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit