Add IPv6 and dual-stack networking support to GCP machine provider#2795
Open
barbacbd wants to merge 1 commit intoopenshift:masterfrom
Open
Add IPv6 and dual-stack networking support to GCP machine provider#2795barbacbd wants to merge 1 commit intoopenshift:masterfrom
barbacbd wants to merge 1 commit intoopenshift:masterfrom
Conversation
Adds support for IPv6 and dual-stack network configurations in GCP machine provider specifications: - New StackType field (IPv4Only, DualStack) with IPv4Only as default - New IPv6Address field for static IPv6 address assignment - New IPv6AccessType field (External, Internal) to control IPv6 internet access - Comprehensive integration test suite for IPv6 network interface configurations Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Note: This is a temporary commit. Migration from MAPI to CAPI is in place for compute nodes. If it turns out that these changes are still required after the migration, then this can be changed.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
Contributor
Author
|
/hold |
Contributor
|
Hello @barbacbd! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
do-not-merge/hold
📝 WalkthroughWalkthroughThis pull request adds IPv6 network interface support for GCP machines. It introduces two new public string enum types ( ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented 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 |
Review Summary by QodoAdd IPv6 and dual-stack networking support to GCP machine provider
WalkthroughsDescription• Add IPv6 and dual-stack network stack support to GCP • Introduce StackType field with IPv4Only and DualStack options • Add IPv6Address field for static IPv6 address assignment • Add IPv6AccessType field controlling IPv6 internet access • Comprehensive integration test suite for IPv6 configurations Diagramflowchart LR
A["GCPNetworkInterface"] -->|adds| B["StackType field<br/>IPv4Only/DualStack"]
A -->|adds| C["IPv6Address field<br/>static IPv6 assignment"]
A -->|adds| D["IPv6AccessType field<br/>External/Internal"]
B -->|validated by| E["Kubebuilder enums"]
C -->|optional| F["Auto-assign if DualStack"]
D -->|optional| G["Defaults to External"]
H["Test Suite"] -->|validates| B
H -->|validates| C
H -->|validates| D
File Changes1. machine/v1beta1/types_gcpprovider.go
|
Code Review by Qodo
1. New fields lack FeatureGate
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+292
to
+312
| // stackType determines the IP stack configuration for the network interface. | ||
| // This field defaults to IPv4Only. Valid values are "IPv4Only" and "DualStack". | ||
| // | ||
| // +kubebuilder:default:="IPv4Only" | ||
| // +optional | ||
| // +default="IPv4Only" | ||
| StackType StackType `json:"stackType,omitempty"` | ||
|
|
||
| // ipv6Address is an IPv6 internal network address for this network interface. | ||
| // To use a static internal IP address, it must be unused and in the same region as the instance's zone. | ||
| // If not specified and stackType is "DualStack", Google Cloud can automatically assign an internal IPv6 address. | ||
| // +optional | ||
| IPv6Address string `json:"ipv6Address,omitempty"` | ||
|
|
||
| // ipv6AccessType indicates whether the IPv6 endpoint can be accessed from the Internet. | ||
| // Valid values are "External" or "Internal". Only valid when stackType is "DualStack". | ||
| // | ||
| // +kubebuilder:default:="External" | ||
| // +optional | ||
| // +default="External" | ||
| IPv6AccessType IPv6AccessType `json:"ipv6AccessType,omitempty"` |
There was a problem hiding this comment.
1. New fields lack featuregate 📘 Rule violation ⚙ Maintainability
The new stackType, ipv6Address, and ipv6AccessType fields are added to the stable machine.openshift.io/v1beta1 API without any +openshift:enable:FeatureGate=... marker to gate rollout. This can introduce behavior changes without a controlled enablement mechanism across cluster profiles.
Agent Prompt
## Issue description
New fields were added to `machine.openshift.io/v1beta1` without feature-gating, which violates the requirement to gate stable-API field additions.
## Issue Context
Fields added: `stackType`, `ipv6Address`, `ipv6AccessType`. They currently have kubebuilder defaults/optionality but no `+openshift:enable:FeatureGate=...` marker.
## Fix Focus Areas
- machine/v1beta1/types_gcpprovider.go[292-312]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+306
to
+312
| // ipv6AccessType indicates whether the IPv6 endpoint can be accessed from the Internet. | ||
| // Valid values are "External" or "Internal". Only valid when stackType is "DualStack". | ||
| // | ||
| // +kubebuilder:default:="External" | ||
| // +optional | ||
| // +default="External" | ||
| IPv6AccessType IPv6AccessType `json:"ipv6AccessType,omitempty"` |
There was a problem hiding this comment.
2. ipv6accesstype constraint unenforced 📘 Rule violation ≡ Correctness
The API comment states ipv6AccessType is "Only valid when stackType is \"DualStack\"", but there is no XValidation/CEL rule enforcing this relationship. This allows invalid combinations (e.g., stackType: IPv4Only with ipv6AccessType set) to pass schema validation.
Agent Prompt
## Issue description
`ipv6AccessType` is documented as only valid when `stackType` is `DualStack`, but this is not enforced by schema validation.
## Issue Context
Without cross-field validation, invalid specs may be accepted and fail later at runtime.
## Fix Focus Areas
- machine/v1beta1/types_gcpprovider.go[306-312]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+214
to
+312
| - name: Should reject invalid stackType value | ||
| initial: | | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: Machine | ||
| spec: | ||
| providerSpec: | ||
| value: | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: GCPMachineProviderSpec | ||
| machineType: n1-standard-4 | ||
| region: us-central1 | ||
| zone: us-central1-a | ||
| networkInterfaces: | ||
| - network: test-network | ||
| subnetwork: test-subnet | ||
| stackType: InvalidStackType | ||
| expectedError: "spec.providerSpec.value.networkInterfaces[0].stackType: Unsupported value: \"InvalidStackType\": supported values: \"DualStack\", \"IPv4Only\"" | ||
| - name: Should reject invalid ipv6AccessType value | ||
| initial: | | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: Machine | ||
| spec: | ||
| providerSpec: | ||
| value: | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: GCPMachineProviderSpec | ||
| machineType: n1-standard-4 | ||
| region: us-central1 | ||
| zone: us-central1-a | ||
| networkInterfaces: | ||
| - network: test-network | ||
| subnetwork: test-subnet | ||
| stackType: DualStack | ||
| ipv6AccessType: InvalidAccessType | ||
| expectedError: "spec.providerSpec.value.networkInterfaces[0].ipv6AccessType: Unsupported value: \"InvalidAccessType\": supported values: \"External\", \"Internal\"" | ||
| - name: Should be able to create a GCP machine with IPv4Only and IPv6 fields omitted | ||
| initial: | | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: Machine | ||
| spec: | ||
| providerSpec: | ||
| value: | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: GCPMachineProviderSpec | ||
| machineType: n1-standard-4 | ||
| region: us-central1 | ||
| zone: us-central1-a | ||
| networkInterfaces: | ||
| - network: test-network | ||
| subnetwork: test-subnet | ||
| stackType: IPv4Only | ||
| expected: | | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: Machine | ||
| spec: | ||
| authoritativeAPI: MachineAPI | ||
| providerSpec: | ||
| value: | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: GCPMachineProviderSpec | ||
| machineType: n1-standard-4 | ||
| region: us-central1 | ||
| zone: us-central1-a | ||
| networkInterfaces: | ||
| - network: test-network | ||
| subnetwork: test-subnet | ||
| stackType: IPv4Only | ||
| - name: Should default stackType to IPv4Only when not specified | ||
| initial: | | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: Machine | ||
| spec: | ||
| providerSpec: | ||
| value: | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: GCPMachineProviderSpec | ||
| machineType: n1-standard-4 | ||
| region: us-central1 | ||
| zone: us-central1-a | ||
| networkInterfaces: | ||
| - network: test-network | ||
| subnetwork: test-subnet | ||
| expected: | | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: Machine | ||
| spec: | ||
| authoritativeAPI: MachineAPI | ||
| providerSpec: | ||
| value: | ||
| apiVersion: machine.openshift.io/v1beta1 | ||
| kind: GCPMachineProviderSpec | ||
| machineType: n1-standard-4 | ||
| region: us-central1 | ||
| zone: us-central1-a | ||
| networkInterfaces: | ||
| - network: test-network | ||
| subnetwork: test-subnet | ||
| stackType: IPv4Only | ||
| onUpdate: |
There was a problem hiding this comment.
3. Providerspec validation tests invalid 🐞 Bug ≡ Correctness
IPv6NetworkInterface.yaml expects the apiserver to reject invalid stackType/ipv6AccessType and to default stackType inside spec.providerSpec.value, but providerSpec.value is preserved-unknown RawExtension so nested validation/defaulting is not applied. These new test cases will fail because the API server will accept invalid nested fields and will not inject stackType defaults.
Agent Prompt
## Issue description
The new CRD integration tests assume server-side defaulting and enum validation for fields inside `spec.providerSpec.value` (e.g., `stackType`, `ipv6AccessType`). But `providerSpec.value` is a preserved-unknown `RawExtension`, so the apiserver will not apply nested schema/defaults/enums.
## Issue Context
These tests run against envtest’s real apiserver and therefore only reflect CRD schema behavior. With `x-kubernetes-preserve-unknown-fields: true`, invalid nested values will be accepted and missing nested fields will not be defaulted.
## Fix Focus Areas
- machine/v1beta1/tests/machines.machine.openshift.io/IPv6NetworkInterface.yaml[214-312]
### What to change
- Remove or rewrite the test cases that expect:
- rejection of invalid `stackType` / `ipv6AccessType`
- defaulting of `stackType` when omitted
- Replace them with tests that assert **pass-through preservation** (i.e., what is sent is what is stored), or (if true validation/defaulting is required) move those assertions to where validation actually exists (e.g., a webhook/controller in the owning component) rather than CRD schema tests.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
machine/v1beta1/tests/machines.machine.openshift.io/IPv6NetworkInterface.yaml (1)
249-280: Consider adding edge case tests for IPv6 fields with IPv4Only stack type.The test suite covers the main scenarios well, but could benefit from additional edge case tests:
- Setting
ipv6AddresswhenstackType: IPv4Only- to verify the field is ignored or rejected- Transitioning from
DualStackwithipv6AddresstoIPv4Only- to verify whether IPv6 fields are clearedThese tests would document the expected behavior when users misconfigure the IPv6 fields.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@machine/v1beta1/tests/machines.machine.openshift.io/IPv6NetworkInterface.yaml` around lines 249 - 280, Add two edge-case tests to the IPv6NetworkInterface suite: (1) a test creating a Machine with spec.providerSpec.value.kind == GCPMachineProviderSpec where networkInterfaces[].stackType == IPv4Only but networkInterfaces[].ipv6Address is set, asserting the API either rejects the field or ignores it (validate returned Machine.networkInterfaces has no ipv6Address); (2) a transition test that starts with a Machine having stackType == DualStack and a populated ipv6Address, then updates the spec to stackType == IPv4Only and assert the controller clears or rejects the ipv6Address after reconciliation; reference GCPMachineProviderSpec, networkInterfaces, stackType, and ipv6Address when adding these test cases.machine/v1beta1/types_gcpprovider.go (1)
306-312: Constraint is documented but not schema-enforced; consider alternative approaches.The documentation correctly states that
ipv6AccessTypeis "only valid whenstackTypeisDualStack", but there's no schema-level validation to prevent users from settingipv6AccessTypewhenstackTypeisIPv4Only.However, CEL-based validation (
x-kubernetes-validations) is not used in this codebase, which relies on kubebuilder annotation-based validation (Enum, Pattern, MaxLength, etc.). Sinceipv6AccessTypeis optional with a default, consider whether:
- The current approach of documenting the constraint and letting the field be set (with GCP API rejecting invalid combinations) is intentional and acceptable, or
- A stronger constraint is needed, in which case alternative approaches (such as dedicated validation middleware or improving field documentation) might be more aligned with the codebase patterns than CEL validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@machine/v1beta1/types_gcpprovider.go` around lines 306 - 312, The struct allows IPv6AccessType to be set regardless of stackType, but the constraint "only valid when stackType is DualStack" isn't enforced; either explicitly accept this and strengthen the field doc comment or implement a schema-level check by adding validation logic in the provider's webhook/validation methods (e.g., the ValidateCreate/ValidateUpdate/ValidateDelete methods for the GCP provider type) to reject requests where IPv6AccessType is non-empty/default and StackType != "DualStack"; locate the IPv6AccessType and StackType fields in types_gcpprovider.go and add the conditional check that returns a validation error (with a clear message) when the combination is invalid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@machine/v1beta1/tests/machines.machine.openshift.io/IPv6NetworkInterface.yaml`:
- Around line 249-280: Add two edge-case tests to the IPv6NetworkInterface
suite: (1) a test creating a Machine with spec.providerSpec.value.kind ==
GCPMachineProviderSpec where networkInterfaces[].stackType == IPv4Only but
networkInterfaces[].ipv6Address is set, asserting the API either rejects the
field or ignores it (validate returned Machine.networkInterfaces has no
ipv6Address); (2) a transition test that starts with a Machine having stackType
== DualStack and a populated ipv6Address, then updates the spec to stackType ==
IPv4Only and assert the controller clears or rejects the ipv6Address after
reconciliation; reference GCPMachineProviderSpec, networkInterfaces, stackType,
and ipv6Address when adding these test cases.
In `@machine/v1beta1/types_gcpprovider.go`:
- Around line 306-312: The struct allows IPv6AccessType to be set regardless of
stackType, but the constraint "only valid when stackType is DualStack" isn't
enforced; either explicitly accept this and strengthen the field doc comment or
implement a schema-level check by adding validation logic in the provider's
webhook/validation methods (e.g., the
ValidateCreate/ValidateUpdate/ValidateDelete methods for the GCP provider type)
to reject requests where IPv6AccessType is non-empty/default and StackType !=
"DualStack"; locate the IPv6AccessType and StackType fields in
types_gcpprovider.go and add the conditional check that returns a validation
error (with a clear message) when the combination is invalid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 1b11bd16-868c-4fbe-aafa-922ee055c365
⛔ Files ignored due to path filters (1)
openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**
📒 Files selected for processing (3)
machine/v1beta1/tests/machines.machine.openshift.io/IPv6NetworkInterface.yamlmachine/v1beta1/types_gcpprovider.gomachine/v1beta1/zz_generated.swagger_doc_generated.go
Contributor
|
@barbacbd: The following tests 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. |
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
2 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.
Adds support for IPv6 and dual-stack network configurations in GCP machine provider specifications:
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
Note: This is a temporary commit. Migration from MAPI to CAPI is in place for compute nodes. If it turns out that these changes are still required after the migration, then this can be changed.