From c73052c7d26167178b0fcbe5fbea2240bc300b01 Mon Sep 17 00:00:00 2001 From: vr4manta Date: Wed, 14 Jan 2026 11:27:12 -0500 Subject: [PATCH] Changed AWS dedicated host webhook to allow host info with other tenancies --- pkg/webhooks/machine_webhook.go | 73 ++++++++++++++-------------- pkg/webhooks/machine_webhook_test.go | 8 ++- 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/pkg/webhooks/machine_webhook.go b/pkg/webhooks/machine_webhook.go index 38c4fc867..39359fc03 100644 --- a/pkg/webhooks/machine_webhook.go +++ b/pkg/webhooks/machine_webhook.go @@ -928,47 +928,14 @@ func validateAWS(m *machinev1beta1.Machine, config *admissionConfig) (bool, []st return true, warnings, nil } -// processAWSPlacement analyzes the Placement field in relation to Tenancy and host placement. These are analyzed -// together based based on their relations to one another. +// processAWSPlacement analyzes the Placement field in relation to Tenancy and host placement. func processAWSPlacementTenancy(placement machinev1beta1.Placement) field.ErrorList { var errs field.ErrorList + // Validate tenancy values switch placement.Tenancy { - case "", machinev1beta1.DefaultTenancy, machinev1beta1.DedicatedTenancy: - // Host is not supported for these cases - if placement.Host != nil { - errs = append(errs, field.Forbidden(field.NewPath("spec.placement.host"), "host may only be specified when tenancy is 'host'")) - } - case machinev1beta1.HostTenancy: - if placement.Host != nil { - klog.V(4).Infof("Validating AWS Host Placement") - - if placement.Host.Affinity == nil { - errs = append(errs, field.Required(field.NewPath("spec.placement.host.affinity"), "affinity is required and must be set to either AnyAvailable or DedicatedHost")) - } else { - switch *placement.Host.Affinity { - case machinev1beta1.HostAffinityAnyAvailable: - // DedicatedHost is optional. If it is set, make sure it follows conventions - if placement.Host.DedicatedHost != nil && !awsDedicatedHostNamePattern.MatchString(placement.Host.DedicatedHost.ID) { - errs = append(errs, field.Invalid(field.NewPath("spec.placement.host.dedicatedHost.id"), placement.Host.DedicatedHost.ID, "id must start with 'h-' followed by 8 or 17 lowercase hexadecimal characters (0-9 and a-f)")) - } - case machinev1beta1.HostAffinityDedicatedHost: - // We need to make sure DedicatedHost is set with an ID - if placement.Host.DedicatedHost == nil { - errs = append(errs, field.Required(field.NewPath("spec.placement.host.dedicatedHost"), "dedicatedHost is required when hostAffinity is DedicatedHost, and optional otherwise")) - } else { - // If not set, return required error. If it does not match pattern, return pattern failure message. - if placement.Host.DedicatedHost.ID == "" { - errs = append(errs, field.Required(field.NewPath("spec.placement.host.dedicatedHost.id"), "id is required and must start with 'h-' followed by 8 or 17 lowercase hexadecimal characters (0-9 and a-f)")) - } else if !awsDedicatedHostNamePattern.MatchString(placement.Host.DedicatedHost.ID) { - errs = append(errs, field.Invalid(field.NewPath("spec.placement.host.dedicatedHost.id"), placement.Host.DedicatedHost.ID, "id must start with 'h-' followed by 8 or 17 lowercase hexadecimal characters (0-9 and a-f)")) - } - } - default: - errs = append(errs, field.Invalid(field.NewPath("spec.placement.host.affinity"), placement.Host.Affinity, "hostAffinity must be either AnyAvailable or DedicatedHost")) - } - } - } + case "", machinev1beta1.DefaultTenancy, machinev1beta1.DedicatedTenancy, machinev1beta1.HostTenancy: + // Do nothing, valid values default: errs = append( errs, @@ -980,6 +947,38 @@ func processAWSPlacementTenancy(placement machinev1beta1.Placement) field.ErrorL ) } + // Validate host settings. Upstream CAPI does not enforce host placement requiring tenancy=host, so we are just validating values and allowing MAPI to follow suit. + // Any deviation will result in CAPI2MAPI / MAPI2CAPI converison issues. + if placement.Host != nil { + klog.V(4).Infof("Validating AWS Host Placement") + + if placement.Host.Affinity == nil { + errs = append(errs, field.Required(field.NewPath("spec.placement.host.affinity"), "affinity is required and must be set to either AnyAvailable or DedicatedHost")) + } else { + switch *placement.Host.Affinity { + case machinev1beta1.HostAffinityAnyAvailable: + // DedicatedHost is optional. If it is set, make sure it follows conventions + if placement.Host.DedicatedHost != nil && !awsDedicatedHostNamePattern.MatchString(placement.Host.DedicatedHost.ID) { + errs = append(errs, field.Invalid(field.NewPath("spec.placement.host.dedicatedHost.id"), placement.Host.DedicatedHost.ID, "id must start with 'h-' followed by 8 or 17 lowercase hexadecimal characters (0-9 and a-f)")) + } + case machinev1beta1.HostAffinityDedicatedHost: + // We need to make sure DedicatedHost is set with an ID + if placement.Host.DedicatedHost == nil { + errs = append(errs, field.Required(field.NewPath("spec.placement.host.dedicatedHost"), "dedicatedHost is required when hostAffinity is DedicatedHost, and optional otherwise")) + } else { + // If not set, return required error. If it does not match pattern, return pattern failure message. + if placement.Host.DedicatedHost.ID == "" { + errs = append(errs, field.Required(field.NewPath("spec.placement.host.dedicatedHost.id"), "id is required and must start with 'h-' followed by 8 or 17 lowercase hexadecimal characters (0-9 and a-f)")) + } else if !awsDedicatedHostNamePattern.MatchString(placement.Host.DedicatedHost.ID) { + errs = append(errs, field.Invalid(field.NewPath("spec.placement.host.dedicatedHost.id"), placement.Host.DedicatedHost.ID, "id must start with 'h-' followed by 8 or 17 lowercase hexadecimal characters (0-9 and a-f)")) + } + } + default: + errs = append(errs, field.Invalid(field.NewPath("spec.placement.host.affinity"), placement.Host.Affinity, "hostAffinity must be either AnyAvailable or DedicatedHost")) + } + } + } + return errs } diff --git a/pkg/webhooks/machine_webhook_test.go b/pkg/webhooks/machine_webhook_test.go index 087e90b62..0d08f980b 100644 --- a/pkg/webhooks/machine_webhook_test.go +++ b/pkg/webhooks/machine_webhook_test.go @@ -656,7 +656,9 @@ func TestMachineCreation(t *testing.T) { }, }, }, - expectedError: "admission webhook \"validation.machine.machine.openshift.io\" denied the request: spec.placement.host: Forbidden: host may only be specified when tenancy is 'host'", + // We used to error on the following, but due to CAPI allowing it even though AWS will ignore the host info + // and just honor the tenancy, we are changing our logic to behave the same + //expectedError: "admission webhook \"validation.machine.machine.openshift.io\" denied the request: spec.placement.host: Forbidden: host may only be specified when tenancy is 'host'", }, { name: "configure default tenancy with host placement", @@ -677,7 +679,9 @@ func TestMachineCreation(t *testing.T) { }, }, }, - expectedError: "admission webhook \"validation.machine.machine.openshift.io\" denied the request: spec.placement.host: Forbidden: host may only be specified when tenancy is 'host'", + // We used to error on the following, but due to CAPI allowing it even though AWS will ignore the host info + // and just honor the tenancy, we are changing our logic to behave the same + //expectedError: "admission webhook \"validation.machine.machine.openshift.io\" denied the request: spec.placement.host: Forbidden: host may only be specified when tenancy is 'host'", }, { name: "with VolumeType set to gp3 and Throughput set under minium value",