Skip to content

no-jira: aws: allow privateDnsName field on AWS CAPI machines for dualstack support#592

Open
tthvo wants to merge 1 commit into
openshift:mainfrom
tthvo:capi-dualstack
Open

no-jira: aws: allow privateDnsName field on AWS CAPI machines for dualstack support#592
tthvo wants to merge 1 commit into
openshift:mainfrom
tthvo:capi-dualstack

Conversation

@tthvo

@tthvo tthvo commented Jun 10, 2026

Copy link
Copy Markdown
Member

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

  • New Features

    • AWS machine configurations now support the privateDnsName field.
  • Updates

    • Added validation restrictions for AWS ignition proxy and TLS settings in machine configurations.
    • Updated test coverage for forbidden AWS machine configuration fields.

…pport

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.
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

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 openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 10, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@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

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Walkthrough

This PR removes PrivateDNSName from unsupported AWS spec field validations across admission policies and conversion logic, while adding test coverage for the cloudInit forbidden field in AWSMachine and AWSMachineTemplate resources. The changes synchronize policy definitions, conversion validation, and test assertions across the stack.

Changes

PrivateDNSName removal and cloudInit validation tests

Layer / File(s) Summary
Remove PrivateDNSName unsupported field validation
admission-policies/aws/unsupported-aws-spec-fields.yaml, capi-operator-manifests/aws/manifests.yaml, pkg/conversion/capi2mapi/aws.go, pkg/conversion/capi2mapi/aws_test.go
Removes PrivateDNSName forbidden-field checks from ValidatingAdmissionPolicy definitions and removes the corresponding conversion-time validation and test case for AWSMachineSpec.PrivateDNSName.
Add cloudInit forbidden field validation tests
pkg/controllers/machinesync/machine_sync_controller_test.go
Adds test cases asserting that cloudInit is a forbidden field for both AWSMachine and AWSMachineTemplate, validating the expected ValidatingAdmissionPolicy denial messages.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Test Structure And Quality ⚠️ Warning Tests in machine_sync_controller_test.go (line 2769) have assertions without meaningful failure messages: Expect(err).ToNot(HaveOccurred()) lacks context, violating requirement #4 of the check. Add assertion message to line 2769: Expect(err).ToNot(HaveOccurred(), "should allow creation of AWSMachine without forbidden fields")
✅ Passed checks (14 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing the privateDnsName field restriction from AWS CAPI machines to allow dualstack support.
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 test names are static and deterministic. New cloudInit entries use consistent naming patterns with no dynamic values, fmt.Sprintf, concatenation, or changing identifiers.
Microshift Test Compatibility ✅ Passed PR does not add new Ginkgo e2e tests. Changes are to unit tests (test case entries) in pkg/ directories and YAML configuration files, not to e2e tests in ./e2e/.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests are added in this PR; only existing unit test cases are modified. The SNO Test Compatibility check does not apply.
Topology-Aware Scheduling Compatibility ✅ Passed PR changes admission policies and conversion logic for AWS machine specs, not deployment manifests or pod scheduling constraints. No topology-aware scheduling issues introduced.
Ote Binary Stdout Contract ✅ Passed PR removes privateDnsName validation and adds test cases. No fmt.Print, klog, or stdout writes at process level were introduced. BeforeSuite correctly redirects klog to GinkgoWriter.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR adds test entries to Ginkgo integration tests (not e2e) verifying cloudInit as forbidden field with no IPv4 assumptions or external connectivity.
No-Weak-Crypto ✅ Passed This PR removes privateDnsName from admission policies and conversion validation; it contains no cryptographic code, weak algorithms, custom implementations, or secret comparisons.
Container-Privileges ✅ Passed PR modifies ValidatingAdmissionPolicy manifests and Go source/test files. No privileged containers, hostPID/hostNetwork/hostIPC, SYS_ADMIN, or allowPrivilegeEscalation configurations found.
No-Sensitive-Data-In-Logs ✅ Passed No logging statements found in PR changes. Error messages use generic field validation without exposing sensitive data like passwords, tokens, or API keys.
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 openshift-ci Bot requested review from damdo and mdbooth June 10, 2026 20:48
@openshift-ci

openshift-ci Bot commented Jun 10, 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 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:

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

@tthvo

tthvo commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

/hold

@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 Jun 10, 2026
@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants