Skip to content

WIP: Fake bump#924

Closed
ardaguclu wants to merge 6 commits into
openshift:masterfrom
ardaguclu:fake-bump
Closed

WIP: Fake bump#924
ardaguclu wants to merge 6 commits into
openshift:masterfrom
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
    • Updated dependency override settings in the build configuration.
  • Improvements
    • Enhanced the OAuth API server deployment process to supply additional parameters when injecting the KMS plugin sidecar.
    • Added an OPERATOR_IMAGE environment variable to the deployment, pointing to the cluster authentication operator image.

@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: 1b8035cf-82c3-40c6-81ee-f854211bbed6

📥 Commits

Reviewing files that changed from the base of the PR and between 4109342 and f4b9274.

⛔ 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 (2)
  • go.mod
  • manifests/07_deployment.yaml
✅ Files skipped from review due to trivial changes (1)
  • manifests/07_deployment.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • go.mod

Walkthrough

The PR adds a replace directive in go.mod to override github.com/openshift/library-go with a fork, updates the KMS plugin sidecar integration to pass c.operatorImagePullSpec as an additional argument, and configures the OPERATOR_IMAGE environment variable in the deployment manifest.

Changes

KMS Plugin and Operator Image Integration

Layer / File(s) Summary
library-go fork override and KMS sidecar call update
go.mod, pkg/operator/workload/sync_openshift_oauth_apiserver.go
Converts the single replace for github.com/onsi/ginkgo/v2 into a parenthesized block and adds a new replacement mapping github.com/openshift/library-gogithub.com/ardaguclu/library-go. Updates the kmspluginlifecycle.AddKMSPluginSidecarToPodSpec call to include "authentication-operator" and c.operatorImagePullSpec as additional arguments.
Operator image environment variable
manifests/07_deployment.yaml
Adds OPERATOR_IMAGE environment variable to the deployment manifest pointing to quay.io/openshift/origin-cluster-authentication-operator:v4.0.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 12 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Single Node Openshift (Sno) Test Compatibility ⚠️ Warning PR adds 8 new Ginkgo e2e test files without SNO compatibility labels or guards; tests should be validated on SNO CI jobs. Add [Skipped:SingleReplicaTopology] labels to test names, or run SNO CI validation: periodic-ci-openshift-release-master-ci-4.22-e2e-aws-upgrade-ovn-single-node
Topology-Aware Scheduling Compatibility ⚠️ Warning Deployment uses nodeSelector targeting control-plane nodes and sets replicas based on node count without topology awareness, which breaks on HyperShift and constrains SNO/TNF. Check infrastructure.Status.ControlPlaneTopology before applying master nodeSelector and replica scaling; use WithTopologyAwareReplicasHook and WithControlPlaneNodeSelectorHook from library-go.
Title check ❓ Inconclusive The title 'WIP: Fake bump' is vague and does not convey meaningful information about the actual changes made to the codebase. Replace with a descriptive title that summarizes the main change, such as 'Add KMS plugin sidecar configuration for OpenShift OAuth API server' or 'Update library-go dependency for KMS encryption support'.
✅ Passed checks (12 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 names in the codebase are static and deterministic with no dynamic content like pod names, timestamps, UUIDs, node names, or IP addresses.
Test Structure And Quality ✅ Passed PR contains no Ginkgo test files (verified 43 test files - all use standard Go testing). Custom check for Ginkgo test quality is not applicable.
Microshift Test Compatibility ✅ Passed This PR adds no new Ginkgo e2e tests. It only modifies go.mod, an existing Go function, and a deployment manifest. The MicroShift test compatibility check does not apply when no new tests are being...
Ote Binary Stdout Contract ✅ Passed PR introduces no stdout writes in process-level code. Changes are limited to go.mod dependency update, function parameter updates in workload sync logic (not main/init/suite), and environment varia...
Ipv6 And Disconnected Network Test Compatibility ✅ Passed New Ginkgo e2e tests added use only cluster-internal services (vault-kms namespace) with net.JoinHostPort for correct IPv4/IPv6 URL construction; marked [Serial][Timeout]; no hardcoded IPv4 address...
No-Weak-Crypto ✅ Passed No weak cryptography patterns found. PR contains dependency version bump and function call parameter updates with no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, or non-constant-time secret comparisons.
Container-Privileges ✅ Passed The PR's manifest file contains no flagged privileged settings (privileged: true, hostNetwork, hostPID, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation: true). The runAsUser: 0 configuration has ex...
No-Sensitive-Data-In-Logs ✅ Passed No logging that exposes passwords, tokens, API keys, PII, session IDs, internal hostnames, or customer data detected. Changes add only public container image references and KMS plugin configuration...

✏️ 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 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

@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 kaleemsiddiqu 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 135-137: The go.mod file contains a replace directive on line 137
that redirects the github.com/openshift/library-go import to a personal fork at
github.com/ardaguclu/library-go, which violates supply-chain security controls
and introduces an untrusted dependency source. Remove the entire line containing
the github.com/openshift/library-go replace override (the second line of the
replace block), keeping only the github.com/onsi/ginkgo/v2 replacement. This
personal fork override should only exist in local development environments, not
in the production code.
🪄 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: 7e9213bb-3224-4591-9222-2e347628f523

📥 Commits

Reviewing files that changed from the base of the PR and between 28632f2 and b19410d.

⛔ 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/sync_openshift_oauth_apiserver.go

Comment thread go.mod Outdated
Comment on lines +135 to +137
replace (
github.com/onsi/ginkgo/v2 => github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20251001123353-fd5b1fb35db1
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 "== go.mod replace directives =="
rg -n '^\s*replace|\bgithub\.com/(openshift|ardaguclu)/library-go\b' go.mod

echo
echo "== Imports affected by library-go replace =="
rg -n --type go '"github.com/openshift/library-go/' -C1

Repository: openshift/cluster-authentication-operator

Length of output: 50398


Remove the library-go fork override before merging.

Line 137 redirects all github.com/openshift/library-go imports to a personal fork (github.com/ardaguclu/library-go), introducing a supply-chain trust boundary change in a security-critical authentication operator. This override must be removed before merge—keep it only for local development.

Suggested fix
 replace (
 	github.com/onsi/ginkgo/v2 => github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20251001123353-fd5b1fb35db1
-	github.com/openshift/library-go => github.com/ardaguclu/library-go v0.0.0-20260619072729-555fb535777b
 )

Per coding guidelines, dependency files must meet supply-chain controls: no personal forks in production code, verified provenance, and trusted sources only.

🤖 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 135 - 137, The go.mod file contains a replace directive
on line 137 that redirects the github.com/openshift/library-go import to a
personal fork at github.com/ardaguclu/library-go, which violates supply-chain
security controls and introduces an untrusted dependency source. Remove the
entire line containing the github.com/openshift/library-go replace override (the
second line of the replace block), keeping only the github.com/onsi/ginkgo/v2
replacement. This personal fork override should only exist in local development
environments, not in the production code.

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

@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 closed this Jun 22, 2026
@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.

@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