Skip to content

NO-JIRA: fix(certs): handle IPv6 address normalization in certificate validation#7576

Open
qinqon wants to merge 1 commit intoopenshift:mainfrom
qinqon:kv-ipv6-normalization
Open

NO-JIRA: fix(certs): handle IPv6 address normalization in certificate validation#7576
qinqon wants to merge 1 commit intoopenshift:mainfrom
qinqon:kv-ipv6-normalization

Conversation

@qinqon
Copy link
Contributor

@qinqon qinqon commented Jan 23, 2026

What this PR does / why we need it:

In dual-stack environments, IPv6 addresses can be represented in different formats (e.g., "::1" vs "0:0:0:0:0:0:0:1"). The previous certificate validation used byte comparison via cmp.Diff, which incorrectly identified semantically identical IPv6 addresses as different.

This caused certificate regeneration failures in KubeVirt dual-stack clusters because the addresses stored in x509 certificates use expanded format while the calculated expected addresses use compressed format.

Which issue(s) this PR fixes:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

In dual-stack environments, IPv6 addresses can be represented in different
formats (e.g., "::1" vs "0:0:0:0:0:0:0:1"). The previous certificate
validation used byte comparison via cmp.Diff, which incorrectly identified
semantically identical IPv6 addresses as different.

This caused certificate regeneration failures in KubeVirt dual-stack
clusters because the addresses stored in x509 certificates use expanded
format while the calculated expected addresses use compressed format.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Enrique Llorente <ellorent@redhat.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026

Walkthrough

Adds semantic IP address comparison supporting IPv6 normalization to certificate validation. New compareIPAddresses function normalizes IPv6 addresses, sorts both slices, and compares them semantically. ValidateKeyPair integrates this function for IP validation. Includes comprehensive tests for IPv6 normalization and dual-stack scenarios.

Changes

Cohort / File(s) Changes
IP address comparison logic
support/certs/tls.go
Added compareIPAddresses function for semantic IP comparison with IPv6 normalization and sorting. Integrated into ValidateKeyPair to replace direct positional IP diff. Added sort and strings imports.
IPv6 normalization tests
support/certs/ipv6_fix_test.go
Added TestCompareIPAddresses_IPv6Normalization and TestCompareIPAddresses_DualStackScenario test functions covering IPv6 compression/expansion, mixed-case handling, ordering invariance, count mismatches, and dual-stack scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

✨ Finishing touches
  • 📝 Generate docstrings

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

@openshift-ci openshift-ci bot requested review from enxebre and muraee January 23, 2026 13:15
@openshift-ci openshift-ci bot added area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release and removed do-not-merge/needs-area labels Jan 23, 2026
@jparrill
Copy link
Contributor

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 23, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jparrill, qinqon

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 Jan 23, 2026
@qinqon
Copy link
Contributor Author

qinqon commented Jan 23, 2026

/retest-required

@orenc1
Copy link
Contributor

orenc1 commented Jan 23, 2026

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2026
@qinqon
Copy link
Contributor Author

qinqon commented Jan 24, 2026

/retest-required

@qinqon qinqon changed the title fix(certs): handle IPv6 address normalization in certificate validation NO-JIRA: fix(certs): handle IPv6 address normalization in certificate validation Jan 24, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 24, 2026
@openshift-ci-robot
Copy link

@qinqon: This pull request explicitly references no jira issue.

Details

In response to this:

What this PR does / why we need it:

In dual-stack environments, IPv6 addresses can be represented in different formats (e.g., "::1" vs "0:0:0:0:0:0:0:1"). The previous certificate validation used byte comparison via cmp.Diff, which incorrectly identified semantically identical IPv6 addresses as different.

This caused certificate regeneration failures in KubeVirt dual-stack clusters because the addresses stored in x509 certificates use expanded format while the calculated expected addresses use compressed format.

Which issue(s) this PR fixes:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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.

@qinqon
Copy link
Contributor Author

qinqon commented Jan 24, 2026

/jira refresh

@openshift-ci-robot
Copy link

@qinqon: This pull request explicitly references no jira issue.

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.

@qinqon
Copy link
Contributor Author

qinqon commented Jan 26, 2026

/verify

@qinqon
Copy link
Contributor Author

qinqon commented Jan 26, 2026

/verified

@openshift-ci-robot
Copy link

@qinqon: The /verified command must be used with one of the following actions: by, later, remove, or bypass. See https://docs.ci.openshift.org/docs/architecture/jira/#premerge-verification for more information.

Details

In response to this:

/verified

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.

@orenc1
Copy link
Contributor

orenc1 commented Jan 27, 2026

/verified by @qinqon

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jan 27, 2026
@openshift-ci-robot
Copy link

@orenc1: This PR has been marked as verified by @qinqon.

Details

In response to this:

/verified by @qinqon

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.

@orenc1
Copy link
Contributor

orenc1 commented Jan 27, 2026

/test e2e-aks

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 2633a97 and 2 for PR HEAD 00b1068 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD cf9ea7c and 1 for PR HEAD 00b1068 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD b91b466 and 0 for PR HEAD 00b1068 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 27, 2026

@qinqon: The following test 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/e2e-aks 00b1068 link true /test e2e-aks

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-robot
Copy link

/hold

Revision 00b1068 was retested 3 times: holding

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 27, 2026
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. area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. 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. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants