Skip to content

[release-4.20] OCPBUGS-88347: ensure org match in validation#4291

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:release-4.20from
jrvaldes:cherry-pick-4225-to-release-4.20
Jul 10, 2026
Merged

[release-4.20] OCPBUGS-88347: ensure org match in validation#4291
openshift-merge-bot[bot] merged 1 commit into
openshift:release-4.20from
jrvaldes:cherry-pick-4225-to-release-4.20

Conversation

@jrvaldes

@jrvaldes jrvaldes commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This is a manual cherry-pick of commit 88d72cc from PR #4253, adapted for release-4.20. That branch does not yet contain the pkg/csr/validation package refactor, so the equivalent fix is applied directly to validateKubeletServingCSR in pkg/csr/csr.go.

Summary by CodeRabbit

  • Bug Fixes
    • Tightened certificate request validation so the organization must match exactly, preventing requests with extra, missing, or incorrect organizations from being accepted.
    • Added coverage for valid and invalid organization combinations to confirm the stricter validation behavior.

This commit requires an exact single-org match using reflect.DeepEqual,
mirroring the kubelet client path in isNodeClientCert().

This is a cherry-pick of commit 88d72cc
from PR openshift#4253, adapted for release-4.20. That branch does not yet
contain the pkg/csr/validation package refactor, so the equivalent fix
is applied directly to validateKubeletServingCSR in pkg/csr/csr.go.
@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 Jul 9, 2026
@openshift-ci

openshift-ci Bot commented Jul 9, 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

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Walkthrough

Kubelet-serving CSR validation now requires an exact match between the CSR's subject organization list and the expected single-element organization slice, using reflect.DeepEqual instead of membership checking. Corresponding tests were added covering valid and various invalid organization scenarios.

Changes

CSR Organization Validation

Layer / File(s) Summary
Exact organization match check
pkg/csr/csr.go
Subject organization validation now uses reflect.DeepEqual to require the CSR's organization list to exactly equal []string{nodeGroup}, rejecting lists with extra, missing, or mismatched entries.
Validation tests
pkg/csr/csr_test.go
Adds imports for crypto/x509, crypto/x509/pkix, and Kubernetes certificate/meta types; introduces newTestKubeletServingCSR helper and TestValidateKubeletServingCSROrganizationMatch, which checks acceptance for the exact organization and rejection (with expected error message) for extra, missing, wrong, nil, or empty organization lists.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
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 Added test names are static t.Run strings; no Ginkgo titles or dynamic values appear in the changed tests.
Test Structure And Quality ✅ Passed The new table-driven unit test covers one behavior (org matching), uses no cluster resources or waits, and matches the package’s existing test style.
Microshift Test Compatibility ✅ Passed Only plain Go unit tests were added in pkg/csr; no Ginkgo e2e tests, MicroShift guards, or unsupported OpenShift APIs were introduced.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The PR adds only Go unit tests in pkg/csr, with no Ginkgo e2e tests or SNO-multi-node assumptions; no Ginkgo/SNO markers were found.
Topology-Aware Scheduling Compatibility ✅ Passed Only CSR validation/tests changed; no manifests, controllers, affinity, replicas, or node selectors were added.
Ote Binary Stdout Contract ✅ Passed Changed files only adjust CSR validation and add unit tests; no main/init/TestMain/BeforeSuite code or stdout logging was added.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed Only unit tests in pkg/csr were added; no new Ginkgo e2e tests or external-network dependencies were introduced.
No-Weak-Crypto ✅ Passed No MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, no custom crypto, and reflect.DeepEqual is only used for CSR org slice validation, not secrets.
Container-Privileges ✅ Passed The PR only changes pkg/csr Go validation/tests; no container/K8s manifests were modified, and no privileged settings appear in the touched files.
No-Sensitive-Data-In-Logs ✅ Passed No new logging was introduced; the patch only changes CSR org validation and adds tests, with no sensitive values emitted to logs.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: tightening validation to require an exact organization match.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@jrvaldes jrvaldes changed the title ensure org match in validation [release-4.20] ensure org match in validation Jul 9, 2026
@jrvaldes

jrvaldes commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@openshift-ci

openshift-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jrvaldes

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

The pull request process is described 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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 9, 2026
@jrvaldes

jrvaldes commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/test ?

@jrvaldes

jrvaldes commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/test vsphere-e2e-operator

@jrvaldes

jrvaldes commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/test gcp-e2e-operator

@mansikulkarni96

Copy link
Copy Markdown
Member

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 9, 2026
@jrvaldes jrvaldes marked this pull request as ready for review July 9, 2026 17:11
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 9, 2026
@openshift-ci openshift-ci Bot requested review from mansikulkarni96 and sebsoto July 9, 2026 17:12
@jrvaldes jrvaldes changed the title [release-4.20] ensure org match in validation [release-4.20] OCPBUGS-88347: ensure org match in validation Jul 9, 2026
@openshift-ci-robot openshift-ci-robot added jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jul 9, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@jrvaldes: This pull request references Jira Issue OCPBUGS-88347, which is invalid:

  • expected the vulnerability to target the "4.20.z" version, but no target version was set
  • release note text must be set and not match the template OR release note type must be set to "Release Note Not Required". For more information you can reference the OpenShift Bug Process.
  • expected dependent Jira Issue OCPBUGS-88349 to target a version in 4.21.0, 4.21.z, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

This is a manual cherry-pick of commit 88d72cc from PR #4253, adapted for release-4.20. That branch does not yet contain the pkg/csr/validation package refactor, so the equivalent fix is applied directly to validateKubeletServingCSR in pkg/csr/csr.go.

Summary by CodeRabbit

  • Bug Fixes
  • Tightened certificate request validation so the organization must match exactly, preventing requests with extra, missing, or incorrect organizations from being accepted.
  • Added coverage for valid and invalid organization combinations to confirm the stricter validation behavior.

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.

@jrvaldes

jrvaldes commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/jira refresh

@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jul 9, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@jrvaldes: This pull request references Jira Issue OCPBUGS-88347, which is valid.

7 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.20.z) matches configured target version for branch (4.20.z)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
  • release note text is set and does not match the template
  • dependent bug Jira Issue OCPBUGS-88349 is in the state Verified, which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA))
  • dependent Jira Issue OCPBUGS-88349 targets the "4.21.z" version, which is one of the valid target versions: 4.21.0, 4.21.z
  • bug has dependents
Details

In response to this:

/jira refresh

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.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 87d7e9b and 2 for PR HEAD 29517ea in total

@jrvaldes

jrvaldes commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/override ci/prow/vsphere-disconnected-e2e-operator

@openshift-ci

openshift-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@jrvaldes: Overrode contexts on behalf of jrvaldes: ci/prow/vsphere-disconnected-e2e-operator

Details

In response to this:

/override ci/prow/vsphere-disconnected-e2e-operator

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.

@jrvaldes

Copy link
Copy Markdown
Contributor Author

/retest

@jrvaldes

jrvaldes commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

/override ci/prow/azure-e2e-operator ci/prow/azure-e2e-upgrade

missing 2019 template in azure CI

@jrvaldes

Copy link
Copy Markdown
Contributor Author

/override ci/prow/nutanix-e2e-operator

cluster install failed, there is CI coverage across other jobs, no specific change for this platform was introduced.

@openshift-ci

openshift-ci Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

@jrvaldes: Overrode contexts on behalf of jrvaldes: ci/prow/azure-e2e-operator, ci/prow/azure-e2e-upgrade

Details

In response to this:

/override ci/prow/azure-e2e-operator ci/prow/azure-e2e-upgrade

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 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

@jrvaldes: Overrode contexts on behalf of jrvaldes: ci/prow/nutanix-e2e-operator

Details

In response to this:

/override ci/prow/nutanix-e2e-operator

cluster install failed, there is CI coverage across other jobs, no specific change for this platform was introduced.

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 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

@jrvaldes: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/azure-e2e-upgrade 29517ea link true /test azure-e2e-upgrade
ci/prow/azure-e2e-operator 29517ea link true /test azure-e2e-operator
ci/prow/nutanix-e2e-operator 29517ea link true /test nutanix-e2e-operator

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-merge-bot openshift-merge-bot Bot merged commit 3b63ac9 into openshift:release-4.20 Jul 10, 2026
19 checks passed
@openshift-ci-robot

Copy link
Copy Markdown

@jrvaldes: Jira Issue OCPBUGS-88347: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-88347 has been moved to the MODIFIED state.

Details

In response to this:

This is a manual cherry-pick of commit 88d72cc from PR #4253, adapted for release-4.20. That branch does not yet contain the pkg/csr/validation package refactor, so the equivalent fix is applied directly to validateKubeletServingCSR in pkg/csr/csr.go.

Summary by CodeRabbit

  • Bug Fixes
  • Tightened certificate request validation so the organization must match exactly, preventing requests with extra, missing, or incorrect organizations from being accepted.
  • Added coverage for valid and invalid organization combinations to confirm the stricter validation behavior.

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.

@jrvaldes

Copy link
Copy Markdown
Contributor Author

/cherry-pick release-4.19
/cherry-pick release-4.18

@openshift-cherrypick-robot

Copy link
Copy Markdown

@jrvaldes: new pull request created: #4304

Details

In response to this:

/cherry-pick release-4.19
/cherry-pick release-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.

@openshift-cherrypick-robot

Copy link
Copy Markdown

@jrvaldes: new pull request created: #4305

Details

In response to this:

/cherry-pick release-4.19
/cherry-pick release-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

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants