CORS-4512: aws: allow privateDNSName and assignPrimaryIPv6 field on CAPI machines in dual-stack clusters

[tthvo]

Description

Remove privateDnsName from the unsupported AWS spec fields admission policy and CAPI-to-MAPI conversion validation. This unblocks dualstack installs that require configuring instance hostname DNS records (AAAA/A) via the CAPA PrivateDNSName field. Additionally, assignPrimaryIPv6 is also handled the same way.

Note: MAPI has no equivalent fields and the infra CR already holds these information via ipFamily, so it is silently dropped during conversion.

See also #593

Summary by CodeRabbit

• Bug Fixes
  • Relaxed admission restrictions by no longer rejecting privateDnsName in AWS machine policy checks.
  • Added admission enforcement to deny requests when ignition proxy or tls settings are present.
• New Features
  • Improved AWS machine conversion to support dual-stack configurations by populating privateDnsName and assignPrimaryIPv6 based on the infrastructure IP family.
• Tests
  • Updated and expanded conversion tests to reflect the new dual-stack and validation behavior.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

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

Details

In response to this:

Description

Remove privateDnsName from the unsupported AWS spec fields admission policy and CAPI-to-MAPI conversion validation. This unblocks dualstack installs that require configuring instance hostname DNS records (AAAA/A) via the CAPA PrivateDNSName field.

Note: MAPI has no equivalent field, so it is silently dropped during conversion.

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.

[coderabbitai]

Walkthrough

Removes the privateDnsName forbidden-field validation from the openshift-cluster-api-unsupported-aws-spec-fields ValidatingAdmissionPolicy in both the source policy file and the generated operator manifests, while adding ignition.proxy and ignition.tls field checks. The CAPI→MAPI conversion stops treating PrivateDNSName as unsupported. The MAPI→CAPI conversion adds logic to populate dual-stack-related fields (PrivateDNSName and AssignPrimaryIPv6) from the infrastructure's IPFamily configuration.

Changes

AWS Dual-Stack Field Support

Layer / File(s)	Summary
Admission policy: remove privateDnsName check and update ignition field validation admission-policies/aws/unsupported-aws-spec-fields.yaml, capi-operator-manifests/aws/manifests.yaml	Deletes the privateDnsName validation entry from both the source admission-policy YAML and the generated manifests. The generated manifests also add forbidden-field checks for ignition.proxy and ignition.tls, each guarded by an ignition existence check.
CAPI→MAPI conversion: stop treating PrivateDNSName as unsupported pkg/conversion/capi2mapi/aws.go, pkg/conversion/capi2mapi/aws_test.go	Removes the validation that rejected spec.PrivateDNSName as unsupported, with comments explaining the field is handled via the infrastructure CR. Removes the corresponding test entry asserting an error for PrivateDNSName.
MAPI→CAPI conversion: populate dual-stack fields from infrastructure pkg/conversion/mapi2capi/aws.go	Adds a call to new helper convertAWSDualStackFieldsToCAPI, which derives and sets PrivateDNSName (for dual-stack modes) and AssignPrimaryIPv6 (for dual-stack IPv6-primary vs IPv4-primary) based on infrastructure.Status.PlatformStatus.AWS.IPFamily.
Test coverage for MAPI→CAPI dual-stack conversion pkg/conversion/mapi2capi/aws_test.go	Adds two test contexts with multiple cases: one verifying PrivateDNSName is set only for DualStackIPv4Primary and DualStackIPv6Primary modes; another verifying AssignPrimaryIPv6 is enabled for IPv6-primary dual-stack, disabled for IPv4-primary dual-stack, and nil otherwise.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 12 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% 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	New tests in mapi2capi/aws_test.go (lines 443-489) lack assertion messages, violating requirement #4. Existing tests in the same file consistently include meaningful messages (e.g., line 552: Expec...	Add meaningful failure messages to all new test assertions: e.g., Expect(err).ToNot(HaveOccurred(), "failed to convert infrastructure") and Expect(capaMachine.Spec.PrivateDNSName).To(BeNil(), "PrivateDNSName should be nil for IPv4-only c...
Microshift Test Compatibility	⚠️ Warning	Two new test contexts added in aws_test.go reference the config.openshift.io/v1 Infrastructure API (unavailable on MicroShift) without protection labels or guards.	Add [apigroup:config.openshift.io] tag to test names, or wrap with [Skipped:MicroShift] label, or guard with exutil.IsMicroShiftCluster() check.

✅ Passed checks (12 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: allowing privateDNSName and assignPrimaryIPv6 fields on CAPI machines in dual-stack AWS clusters, which aligns with the core objective of removing restrictions on these fields.
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 PR are stable and deterministic. No dynamic content (timestamps, generated IDs, pod names, IP addresses, etc.) found in test titles, Contexts, or Entry names.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new e2e tests added in this PR. The only new test contexts are unit tests in pkg/conversion/mapi2capi/aws_test.go that test conversion logic, not e2e scenarios.
Topology-Aware Scheduling Compatibility	✅ Passed	The PR modifies ValidatingAdmissionPolicy rules and field conversion logic between CAPI and MAPI—no pod, deployment, affinity, or topology-aware scheduling constraints are introduced or modified.
Ote Binary Stdout Contract	✅ Passed	No OTE Binary Stdout Contract violations detected. PR adds conversion code with no main(), init(), suite-level setup functions, or direct stdout writes. Test-level code only contains test assertions.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new e2e tests were added in this PR. The PR modifies only unit tests in pkg/conversion/ and pkg/controllers/ subdirectories, plus YAML manifests and implementation files. The custom check for e2...
No-Weak-Crypto	✅ Passed	No weak cryptography (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or non-constant-time secret comparisons detected in any of the changed files.
Container-Privileges	✅ Passed	PR contains no container privilege escalation issues; modified YAML files are admission policies without container specs or privileged settings.
No-Sensitive-Data-In-Logs	✅ Passed	No logging statements were added that expose sensitive data. New code only manipulates DNS/IP configuration fields and infrastructure status without any logging operations.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ 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]

[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 theobarberbany 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:

• OWNERS

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

[tthvo]

/hold

[tthvo]

/hold cancel

[tthvo]

/retitle: CORS-4512: aws: allow privateDnsName field on AWS CAPI machines for dualstack

[openshift-ci-robot]

@tthvo: This pull request references CORS-4512 which is a valid jira issue.

Details

In response to this:

Description

Remove privateDnsName from the unsupported AWS spec fields admission policy and CAPI-to-MAPI conversion validation. This unblocks dualstack installs that require configuring instance hostname DNS records (AAAA/A) via the CAPA PrivateDNSName field.

Note: MAPI has no equivalent field, so it is silently dropped during conversion.

See also #593

Summary by CodeRabbit

• Bug Fixes
• Removed overly restrictive validation for certain AWS configuration fields, allowing broader configuration options.
• Added stricter validation controls for ignition-related parameters in AWS cluster configurations to ensure proper security settings.

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.

[coderabbitai]

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 `@pkg/conversion/mapi2capi/aws.go`:
- Line 287: The convertAWSDualStackFieldsToCAPI helper function can panic when
m.infrastructure is nil because it dereferences status fields without
validation. Add a nil check for the infrastructure parameter at the beginning of
the convertAWSDualStackFieldsToCAPI function to return a validation error
instead of panicking when the parameter is nil. This will ensure the conversion
properly handles cases where infrastructure is not provided and returns
appropriate validation errors rather than crashing.

🪄 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 YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: ee8d3b1b-e0cf-4acf-997e-2d8cf3c34a5f

📥 Commits

Reviewing files that changed from the base of the PR and between 1bbaf6d and 37e5b94.

📒 Files selected for processing (7)

• admission-policies/aws/unsupported-aws-spec-fields.yaml
• capi-operator-manifests/aws/manifests.yaml
• pkg/controllers/machinesync/machine_sync_controller_test.go
• pkg/conversion/capi2mapi/aws.go
• pkg/conversion/capi2mapi/aws_test.go
• pkg/conversion/mapi2capi/aws.go
• pkg/conversion/mapi2capi/aws_test.go

💤 Files with no reviewable changes (4)

• pkg/conversion/capi2mapi/aws_test.go
• admission-policies/aws/unsupported-aws-spec-fields.yaml
• capi-operator-manifests/aws/manifests.yaml
• pkg/controllers/machinesync/machine_sync_controller_test.go

[tthvo]

/test e2e-aws-capi-techpreview
/test e2e-aws-capi-techpreview-post-install
/test e2e-aws-ovn

[tthvo]

/test e2e-aws-capi-techpreview
/test e2e-aws-ovn

[openshift-ci]

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