diff --git a/admission-policies/aws/unsupported-aws-spec-fields.yaml b/admission-policies/aws/unsupported-aws-spec-fields.yaml index d0aebdd73..21ad129e8 100644 --- a/admission-policies/aws/unsupported-aws-spec-fields.yaml +++ b/admission-policies/aws/unsupported-aws-spec-fields.yaml @@ -43,8 +43,6 @@ spec: messageExpression: "variables.specPath + '.networkInterfaces is a forbidden field'" - expression: "!has(variables.machineSpec.uncompressedUserData)" messageExpression: "variables.specPath + '.uncompressedUserData is a forbidden field'" - - expression: "!has(variables.machineSpec.privateDnsName)" - messageExpression: "variables.specPath + '.privateDnsName is a forbidden field'" - expression: "!has(variables.machineSpec.ignition) || !has(variables.machineSpec.ignition.proxy)" messageExpression: "variables.specPath + '.ignition.proxy is a forbidden field'" - expression: "!has(variables.machineSpec.ignition) || !has(variables.machineSpec.ignition.tls)" diff --git a/capi-operator-manifests/aws/manifests.yaml b/capi-operator-manifests/aws/manifests.yaml index ae89f5ffb..93315f789 100644 --- a/capi-operator-manifests/aws/manifests.yaml +++ b/capi-operator-manifests/aws/manifests.yaml @@ -46,8 +46,6 @@ spec: - expression: '!has(variables.machineSpec.uncompressedUserData)' messageExpression: variables.specPath + '.uncompressedUserData is a forbidden field' - - expression: '!has(variables.machineSpec.privateDnsName)' - messageExpression: variables.specPath + '.privateDnsName is a forbidden field' - expression: '!has(variables.machineSpec.ignition) || !has(variables.machineSpec.ignition.proxy)' messageExpression: variables.specPath + '.ignition.proxy is a forbidden field' - expression: '!has(variables.machineSpec.ignition) || !has(variables.machineSpec.ignition.tls)' diff --git a/pkg/controllers/machinesync/machine_sync_controller_test.go b/pkg/controllers/machinesync/machine_sync_controller_test.go index 5badedc6c..68556d20b 100644 --- a/pkg/controllers/machinesync/machine_sync_controller_test.go +++ b/pkg/controllers/machinesync/machine_sync_controller_test.go @@ -2815,12 +2815,6 @@ var _ = Describe("Unsupported AWS fields validating admission policy", Ordered, }, expectedError: "ValidatingAdmissionPolicy 'openshift-cluster-api-unsupported-aws-spec-fields' with binding 'openshift-cluster-api-unsupported-aws-spec-fields' denied request: spec.cloudInit is a forbidden field", }), - Entry("with a forbidden field (privateDNSName)", testCase{ - modifier: func(m *awsv1.AWSMachine) { - m.Spec.PrivateDNSName = &awsv1.PrivateDNSName{} - }, - expectedError: "ValidatingAdmissionPolicy 'openshift-cluster-api-unsupported-aws-spec-fields' with binding 'openshift-cluster-api-unsupported-aws-spec-fields' denied request: spec.privateDnsName is a forbidden field", - }), Entry("with a forbidden field (ignition.proxy)", testCase{ modifier: func(m *awsv1.AWSMachine) { m.Spec.Ignition = &awsv1.Ignition{Proxy: &awsv1.IgnitionProxy{}} @@ -2946,12 +2940,6 @@ var _ = Describe("Unsupported AWS fields validating admission policy", Ordered, }, expectedError: "ValidatingAdmissionPolicy 'openshift-cluster-api-unsupported-aws-spec-fields' with binding 'openshift-cluster-api-unsupported-aws-spec-fields' denied request: spec.template.spec.cloudInit is a forbidden field", }), - Entry("with a forbidden field (privateDNSName)", testCase{ - modifier: func(mt *awsv1.AWSMachineTemplate) { - mt.Spec.Template.Spec.PrivateDNSName = &awsv1.PrivateDNSName{} - }, - expectedError: "ValidatingAdmissionPolicy 'openshift-cluster-api-unsupported-aws-spec-fields' with binding 'openshift-cluster-api-unsupported-aws-spec-fields' denied request: spec.template.spec.privateDnsName is a forbidden field", - }), Entry("with a forbidden field (ignition.proxy)", testCase{ modifier: func(mt *awsv1.AWSMachineTemplate) { mt.Spec.Template.Spec.Ignition = &awsv1.Ignition{Proxy: &awsv1.IgnitionProxy{}} diff --git a/pkg/conversion/capi2mapi/aws.go b/pkg/conversion/capi2mapi/aws.go index 0a78675b1..a476bf895 100644 --- a/pkg/conversion/capi2mapi/aws.go +++ b/pkg/conversion/capi2mapi/aws.go @@ -788,10 +788,11 @@ func handleUnsupportedAWSMachineFields(fldPath *field.Path, spec awsv1.AWSMachin errs = append(errs, field.Invalid(fldPath.Child("cloudInit"), spec.CloudInit, "cloudInit is not supported")) } - if spec.PrivateDNSName != nil { - // Not required for our use case. - errs = append(errs, field.Invalid(fldPath.Child("privateDNSName"), spec.PrivateDNSName, "privateDNSName is not supported")) - } + // privateDNSName is not checked here because the relevant information is available on the + // infrastructure CR, so nothing additional needs to be added to the MAPI machine spec. + + // assignPrimaryIPv6 is not checked here because the relevant information is available on the + // infrastructure CR, so nothing additional needs to be added to the MAPI machine spec. if spec.Ignition != nil { if spec.Ignition.Proxy != nil { diff --git a/pkg/conversion/capi2mapi/aws_test.go b/pkg/conversion/capi2mapi/aws_test.go index 6a672b1c7..287a40f97 100644 --- a/pkg/conversion/capi2mapi/aws_test.go +++ b/pkg/conversion/capi2mapi/aws_test.go @@ -288,14 +288,6 @@ var _ = Describe("capi2mapi AWS conversion", func() { expectedWarnings: []string{}, }), - Entry("With unsupported PrivateDNSName", awsCAPI2MAPIMachineConversionInput{ - awsClusterBuilder: awsCAPIAWSClusterBase, - awsMachineBuilder: awsCAPIAWSMachineBase.WithPrivateDNSName(&awsv1.PrivateDNSName{}), - machineBuilder: awsCAPIMachineBase, - expectedErrors: []string{"spec.privateDNSName: Invalid value: {}: privateDNSName is not supported"}, - expectedWarnings: []string{}, - }), - Entry("With unsupported Ignition Proxy", awsCAPI2MAPIMachineConversionInput{ awsClusterBuilder: awsCAPIAWSClusterBase, awsMachineBuilder: awsCAPIAWSMachineBase. diff --git a/pkg/conversion/mapi2capi/aws.go b/pkg/conversion/mapi2capi/aws.go index 2a04a3f05..3aa4fea22 100644 --- a/pkg/conversion/mapi2capi/aws.go +++ b/pkg/conversion/mapi2capi/aws.go @@ -284,6 +284,8 @@ func (m *awsMachineAndInfra) toAWSMachine(providerSpec mapiv1beta1.AWSMachinePro spec.CapacityReservationID = &providerSpec.CapacityReservationID } + convertAWSDualStackFieldsToCAPI(m.infrastructure, &spec) + // Unused fields - Below this line are fields not used from the MAPI AWSMachineProviderConfig. // TypeMeta - Only for the purpose of the raw extension, not used for any functionality. @@ -807,3 +809,36 @@ func convertAWSCPUOptionsToCAPI(cpuOptions *mapiv1beta1.CPUOptions) awsv1.CPUOpt return capiCPUOpts } + +// convertAWSDualStackFieldsToCAPI populates dual-stack specific fields on the AWSMachineSpec +// based on the infrastructure CR's IPFamily. These fields are not configurable in MAPI; +// instead, we derive them from the infrastructure CR. +func convertAWSDualStackFieldsToCAPI(infrastructure *configv1.Infrastructure, spec *awsv1.AWSMachineSpec) { + if infrastructure.Status.PlatformStatus == nil || infrastructure.Status.PlatformStatus.AWS == nil { + return + } + + // Dual-stack clusters require "resource-name" hostnames with both A and AAAA records enabled. + // See https://github.com/openshift/machine-api-provider-aws/blob/2e4196b65473ae99bc9bbedc0bc7168ed3da3914/pkg/actuators/machine/instances.go#L833-L849 + switch infrastructure.Status.PlatformStatus.AWS.IPFamily { + case configv1.DualStackIPv4Primary, configv1.DualStackIPv6Primary: + spec.PrivateDNSName = &awsv1.PrivateDNSName{ + EnableResourceNameDNSARecord: ptr.To(true), + EnableResourceNameDNSAAAARecord: ptr.To(true), + HostnameType: ptr.To("resource-name"), + } + case configv1.IPv4: + // No action needed for IPv4-only clusters. + } + + // Dual-stack clusters need explicit assignPrimaryIPv6 configuration to match MAPA behavior. + // See https://github.com/openshift/machine-api-provider-aws/blob/2e4196b65473ae99bc9bbedc0bc7168ed3da3914/pkg/actuators/machine/instances.go#L385-L390 + switch infrastructure.Status.PlatformStatus.AWS.IPFamily { + case configv1.DualStackIPv6Primary: + spec.AssignPrimaryIPv6 = ptr.To(awsv1.PrimaryIPv6AssignmentStateEnabled) + case configv1.DualStackIPv4Primary: + spec.AssignPrimaryIPv6 = ptr.To(awsv1.PrimaryIPv6AssignmentStateDisabled) + case configv1.IPv4: + // No action needed for IPv4-only clusters. + } +} diff --git a/pkg/conversion/mapi2capi/aws_test.go b/pkg/conversion/mapi2capi/aws_test.go index aa93976ef..141e40ca1 100644 --- a/pkg/conversion/mapi2capi/aws_test.go +++ b/pkg/conversion/mapi2capi/aws_test.go @@ -427,6 +427,75 @@ var _ = Describe("mapi2capi AWS conversion", func() { }), ) + Context("DualStack PrivateDNSName Conversion", func() { + DescribeTable("should populate PrivateDNSName based on infrastructure IPFamily", + func(ipFamily configv1.IPFamilyType, expectPrivateDNSName bool) { + dualStackInfra := configbuilder.Infrastructure().AsAWS("sample-cluster-name", "us-east-1"). + WithPlatformStatus(configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + Region: "us-east-1", + IPFamily: ipFamily, + }, + }).Build() + + _, capaObj, warnings, err := FromAWSMachineAndInfra(awsMAPIMachineBase.Build(), dualStackInfra).ToMachineAndInfrastructureMachine() + Expect(err).ToNot(HaveOccurred()) + Expect(warnings).To(BeEmpty()) + + capaMachine, ok := capaObj.(*awsv1.AWSMachine) + Expect(ok).To(BeTrue()) + + if expectPrivateDNSName { + Expect(capaMachine.Spec.PrivateDNSName).To(SatisfyAll( + Not(BeNil()), + HaveField("EnableResourceNameDNSARecord", Equal(ptr.To(true))), + HaveField("EnableResourceNameDNSAAAARecord", Equal(ptr.To(true))), + HaveField("HostnameType", Equal(ptr.To("resource-name"))), + )) + } else { + Expect(capaMachine.Spec.PrivateDNSName).To(BeNil()) + } + }, + Entry("with DualStackIPv4Primary", configv1.DualStackIPv4Primary, true), + Entry("with DualStackIPv6Primary", configv1.DualStackIPv6Primary, true), + Entry("with IPv4", configv1.IPv4, false), + Entry("with empty IPFamily", configv1.IPFamilyType(""), false), + ) + }) + + Context("DualStack AssignPrimaryIPv6 Conversion", func() { + DescribeTable("should populate AssignPrimaryIPv6 based on infrastructure IPFamily", + func(ipFamily configv1.IPFamilyType, expectedState *awsv1.PrimaryIPv6AssignmentState) { + dualStackInfra := configbuilder.Infrastructure().AsAWS("sample-cluster-name", "us-east-1"). + WithPlatformStatus(configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + Region: "us-east-1", + IPFamily: ipFamily, + }, + }).Build() + + _, capaObj, warnings, err := FromAWSMachineAndInfra(awsMAPIMachineBase.Build(), dualStackInfra).ToMachineAndInfrastructureMachine() + Expect(err).ToNot(HaveOccurred()) + Expect(warnings).To(BeEmpty()) + + capaMachine, ok := capaObj.(*awsv1.AWSMachine) + Expect(ok).To(BeTrue()) + + if expectedState != nil { + Expect(capaMachine.Spec.AssignPrimaryIPv6).To(HaveValue(Equal(*expectedState))) + } else { + Expect(capaMachine.Spec.AssignPrimaryIPv6).To(BeNil()) + } + }, + Entry("with DualStackIPv6Primary", configv1.DualStackIPv6Primary, ptr.To(awsv1.PrimaryIPv6AssignmentStateEnabled)), + Entry("with DualStackIPv4Primary", configv1.DualStackIPv4Primary, ptr.To(awsv1.PrimaryIPv6AssignmentStateDisabled)), + Entry("with IPv4", configv1.IPv4, (*awsv1.PrimaryIPv6AssignmentState)(nil)), + Entry("with empty IPFamily", configv1.IPFamilyType(""), (*awsv1.PrimaryIPv6AssignmentState)(nil)), + ) + }) + Context("DedicatedHost Status Conversion", func() { Context("When converting MAPI Machine with DedicatedHost status", func() { It("should populate DedicatedHost status in CAPA Machine", func() {