Skip to content

WIP: Fake bump#717

Closed
ardaguclu wants to merge 5 commits into
openshift:mainfrom
ardaguclu:fake-bump
Closed

WIP: Fake bump#717
ardaguclu wants to merge 5 commits into
openshift:mainfrom
ardaguclu:fake-bump

Conversation

@ardaguclu

@ardaguclu ardaguclu commented Jun 19, 2026

Copy link
Copy Markdown
Member

In order to test openshift/library-go#2317

Summary by CodeRabbit

  • Chores
    • Reformatted and extended dependency overrides used for testing and library components.
    • Updated the generation of the KMS plugin sidecar configuration to pass additional inputs, improving consistency of sidecar setup in the operator’s workload deployment flow.

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 19, 2026
@openshift-ci

openshift-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ardaguclu

Copy link
Copy Markdown
Member Author

/test e2e-aws-operator-encryption-kms

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8ef9c3ba-aae4-4e8d-96fa-8a3fcead52f4

📥 Commits

Reviewing files that changed from the base of the PR and between c6a1ef3 and 0213b84.

⛔ Files ignored due to path filters (4)
  • go.sum is excluded by !**/*.sum
  • vendor/github.com/openshift/library-go/pkg/operator/encryption/kms/pluginlifecycle/builder.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/operator/encryption/kms/pluginlifecycle/health_reporter.go is excluded by !**/vendor/**, !vendor/**
  • vendor/modules.txt is excluded by !**/vendor/**, !vendor/**
📒 Files selected for processing (1)
  • go.mod
🚧 Files skipped from review as they are similar to previous changes (1)
  • go.mod

Walkthrough

go.mod is updated to redirect github.com/openshift/library-go to a fork at github.com/ardaguclu/library-go. The AddKMSPluginSidecarToPodSpec call in the operator workload sync function is updated to pass operatorImagePullSpec as an additional argument.

Changes

KMS Plugin Sidecar Parameter Updates

Layer / File(s) Summary
Dependency fork and KMS sidecar call propagation
go.mod, pkg/operator/workload/workload_openshiftapiserver_v311_00_sync.go
go.mod gains a replace block pointing github.com/openshift/library-go to github.com/ardaguclu/library-go at a pseudo-version. The AddKMSPluginSidecarToPodSpec invocation in manageOpenShiftAPIServerDeployment_v311_00_to_latest is updated to pass operatorImagePullSpec alongside featureGateAccessor and a fixed operator name string.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 10 | ❌ 5

❌ Failed checks (4 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality ⚠️ Warning Ginkgo tests have quality issues: (1) testGenericNetworkPolicyEnforcement tests multiple unrelated behaviors (allow-all, default-deny, ingress, egress) violating single responsibility; (2) Most ass... Split multi-behavior tests into focused test cases; add descriptive messages to all assertions like o.Expect(err).NotTo(HaveOccurred(), "meaningful context about what failed").
Microshift Test Compatibility ⚠️ Warning Two new Ginkgo tests added use [OCPFeatureGate:KMSEncryption] tag but lack MicroShift protection mechanisms ([Skipped:MicroShift], [apigroup:...], or exutil.IsMicroShiftCluster() checks). Add [apigroup:config.openshift.io] tag to the test names or [Skipped:MicroShift] label to exclude from MicroShift CI, since the tests exercise KMS encryption which may not be tested on MicroShift.
Topology-Aware Scheduling Compatibility ⚠️ Warning The deployment uses nodeSelector targeting node-role.kubernetes.io/master, which will not work on HyperShift where no control-plane nodes exist. Replica count is derived from master node count with... Check infrastructure.Status.ControlPlaneTopology and avoid nodeSelector for control-plane nodes on External (HyperShift) topology. Cap replicas to schedulable nodes and add topology-aware scheduling hooks from library-go/pkg/operator/dep...
Title check ❓ Inconclusive The title 'WIP: Fake bump' is vague and does not clearly describe the actual changes made to the codebase (go.mod restructuring, library-go redirect, and KMS plugin sidecar configuration updates). Replace with a descriptive title that summarizes the main changes, such as 'Update KMS plugin sidecar configuration and redirect library-go dependency' or similar.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed All Ginkgo test titles in the PR are stable and deterministic with no dynamic information. Dynamic values (namespaces) are correctly placed in test bodies, not titles.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests are added in this PR. The changes are limited to go.mod dependency updates and operator workload function call modifications, neither of which introduces new test definition...
Ote Binary Stdout Contract ✅ Passed No OTE Binary Stdout Contract violations found. The PR contains library-go dependency updates and workload function signature changes, with no problematic stdout writes in process-level code (main,...
Ipv6 And Disconnected Network Test Compatibility ✅ Passed New Ginkgo tests added (TestKMSEncryptionOnOff, TestKMSEncryptionProvidersMigration) contain no hardcoded IPv4 addresses, IPv4-only parsing, or external internet connectivity requirements. Tests us...
No-Weak-Crypto ✅ Passed No weak cryptographic algorithms (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB mode), custom crypto implementations, or non-constant-time secret comparisons detected in the PR changes.
Container-Privileges ✅ Passed PR changes only go.mod (dependency replacement) and updates a KMS plugin function call signature. No container privileged settings, hostPID/Network/IPC, SYS_ADMIN capabilities, or allowPrivilegeEsc...
No-Sensitive-Data-In-Logs ✅ Passed No sensitive data logging found: PR only modifies dependency directives and adds safe function arguments (operator name, image spec) without exposing passwords, tokens, API keys, PII, or other sens...

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci

openshift-ci Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign tjungblu for approval. For more information see the Code Review Process.

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

Details Needs approval from an approver in each of these files:

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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`:
- Around line 136-139: The replace directive contains a redirect from
github.com/openshift/library-go to a personal fork
github.com/ardaguclu/library-go, which lacks documented justification and
contradicts the README.md documentation. Either remove this redirect line
entirely from the replace block, or if it is necessary, add explicit written
justification as a comment and obtain documented approval from security and
maintainers before merging to protected branches to mitigate supply chain
security risks.
🪄 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: 7c677642-9c6e-4170-b86e-372f3f1c8b74

📥 Commits

Reviewing files that changed from the base of the PR and between a58b396 and c08f07c.

⛔ Files ignored due to path filters (9)
  • go.sum is excluded by !**/*.sum
  • vendor/github.com/openshift/library-go/pkg/operator/encryption/controllers/key_controller.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/operator/encryption/controllers/kms_preflight_controller.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/operator/encryption/kms/pluginlifecycle/builder.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/operator/encryption/kms/pluginlifecycle/sidecar.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/test/library/encryption/helpers.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/test/library/encryption/kms/vault.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/test/library/encryption/scenarios.go is excluded by !**/vendor/**, !vendor/**
  • vendor/modules.txt is excluded by !**/vendor/**, !vendor/**
📒 Files selected for processing (2)
  • go.mod
  • pkg/operator/workload/workload_openshiftapiserver_v311_00_sync.go

Comment thread go.mod
Comment on lines +136 to +139
replace (
github.com/onsi/ginkgo/v2 => github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20260303184444-1cc650aa0565
github.com/openshift/library-go => github.com/ardaguclu/library-go v0.0.0-20260619072729-555fb535777b
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "library-go replace directives in go.mod:"
rg -n 'github.com/openshift/library-go\s*=>' go.mod || true

echo
echo "Current replace block:"
sed -n '/^replace/,/)/p' go.mod

Repository: openshift/cluster-openshift-apiserver-operator

Length of output: 480


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking context around line 138 in go.mod ==="
sed -n '130,150p' go.mod

echo
echo "=== Searching for any comments explaining the library-go redirect ==="
rg -B3 -A3 'ardaguclu/library-go' go.mod || true

echo
echo "=== Checking if there are any TODO/FIXME comments related to this ==="
rg -i 'todo|fixme|temporary|test-only' go.mod || true

echo
echo "=== Checking git history for this file (last 5 commits) ==="
git log --oneline -5 -- go.mod || echo "Git history not available"

Repository: openshift/cluster-openshift-apiserver-operator

Length of output: 1093


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking if there's any reference to ardaguclu in other files ==="
rg -l 'ardaguclu' --type-list | head -20 || true
rg 'ardaguclu' --type-list 2>/dev/null | head -30 || true

echo
echo "=== Searching for any justification in README or similar ==="
find . -maxdepth 2 -type f \( -name "README*" -o -name "CONTRIBUTING*" \) -exec grep -l 'library-go\|ardaguclu' {} \; || echo "No matches in docs"

echo
echo "=== Checking go.sum for related entries ==="
rg 'ardaguclu' go.sum || echo "No ardaguclu entries in go.sum"

Repository: openshift/cluster-openshift-apiserver-operator

Length of output: 924


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking README.md for library-go or ardaguclu context ==="
grep -B2 -A2 'library-go\|ardaguclu' README.md || echo "No direct mentions in README"

echo
echo "=== Full go.mod replace section with line numbers ==="
sed -n '134,140p' go.mod

Repository: openshift/cluster-openshift-apiserver-operator

Length of output: 825


Remove personal-fork redirect or provide explicit justification before merge.

Line 138 redirects github.com/openshift/library-go to a personal fork (github.com/ardaguclu/library-go). This lacks documented justification, contradicts the README.md (which references the official upstream library-go), and presents a supply chain security risk. Either remove this redirect or provide written justification and approval from security/maintainers before merging to protected branches.

🤖 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` around lines 136 - 139, The replace directive contains a redirect
from github.com/openshift/library-go to a personal fork
github.com/ardaguclu/library-go, which lacks documented justification and
contradicts the README.md documentation. Either remove this redirect line
entirely from the replace block, or if it is necessary, add explicit written
justification as a comment and obtain documented approval from security and
maintainers before merging to protected branches to mitigate supply chain
security risks.

Source: Coding guidelines

@ardaguclu

Copy link
Copy Markdown
Member Author

/test e2e-aws-operator-encryption-kms

@ardaguclu

Copy link
Copy Markdown
Member Author

/test e2e-aws-operator-encryption-kms

@openshift-ci

openshift-ci Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

@ardaguclu: 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.

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 22, 2026
@openshift-ci

openshift-ci Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

PR needs rebase.

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.

@ardaguclu

Copy link
Copy Markdown
Member Author

/close

@openshift-ci

openshift-ci Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

@ardaguclu: Closed this PR.

Details

In response to this:

/close

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.

@openshift-ci openshift-ci Bot closed this Jun 22, 2026
@ardaguclu ardaguclu deleted the fake-bump branch June 22, 2026 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant