From 96065a8324a13d59990ad44856d5cea985df019c Mon Sep 17 00:00:00 2001 From: Cameron McAvoy Date: Mon, 12 Sep 2022 13:26:09 -0500 Subject: [PATCH 1/2] Update AWSMachine webhook validate logic on update to be consistent with validation on create --- api/v1beta2/awsmachine_webhook.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/v1beta2/awsmachine_webhook.go b/api/v1beta2/awsmachine_webhook.go index dd66d12bba..1a622669f3 100644 --- a/api/v1beta2/awsmachine_webhook.go +++ b/api/v1beta2/awsmachine_webhook.go @@ -78,6 +78,7 @@ func (r *AWSMachine) ValidateUpdate(old runtime.Object) error { var allErrs field.ErrorList allErrs = append(allErrs, r.validateCloudInitSecret()...) + allErrs = append(allErrs, r.validateAdditionalSecurityGroups()...) allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...) newAWSMachineSpec := newAWSMachine["spec"].(map[string]interface{}) @@ -203,7 +204,7 @@ func (r *AWSMachine) validateNonRootVolumes() field.ErrorList { var allErrs field.ErrorList for _, volume := range r.Spec.NonRootVolumes { - if VolumeTypesProvisioned.Has(string(r.Spec.RootVolume.Type)) && volume.IOPS == 0 { + if VolumeTypesProvisioned.Has(string(volume.Type)) && volume.IOPS == 0 { allErrs = append(allErrs, field.Required(field.NewPath("spec.nonRootVolumes.iops"), "iops required if type is 'io1' or 'io2'")) } From e23b67d32832ae52b989c7bba1ecd89dc95ad154 Mon Sep 17 00:00:00 2001 From: Cameron McAvoy Date: Mon, 9 Jan 2023 13:47:04 -0600 Subject: [PATCH 2/2] Update AWSMachineTemplate webhook validate logic on create to be consistent with AWSMachine webhook validation on create --- api/v1beta2/awsmachinetemplate_webhook.go | 89 ++++++++++++++++++++--- 1 file changed, 77 insertions(+), 12 deletions(-) diff --git a/api/v1beta2/awsmachinetemplate_webhook.go b/api/v1beta2/awsmachinetemplate_webhook.go index 6fed6a336b..8def3d9868 100644 --- a/api/v1beta2/awsmachinetemplate_webhook.go +++ b/api/v1beta2/awsmachinetemplate_webhook.go @@ -101,6 +101,78 @@ func (r *AWSMachineTemplate) validateNonRootVolumes() field.ErrorList { return allErrs } +func (r *AWSMachineTemplate) validateAdditionalSecurityGroups() field.ErrorList { + var allErrs field.ErrorList + + spec := r.Spec.Template.Spec + + for _, additionalSecurityGroup := range spec.AdditionalSecurityGroups { + if len(additionalSecurityGroup.Filters) > 0 && additionalSecurityGroup.ID != nil { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "additionalSecurityGroups"), "only one of ID or Filters may be specified, specifying both is forbidden")) + } + } + return allErrs +} + +func (r *AWSMachineTemplate) validateCloudInitSecret() field.ErrorList { + var allErrs field.ErrorList + + spec := r.Spec.Template.Spec + if spec.CloudInit.InsecureSkipSecretsManager { + if spec.CloudInit.SecretPrefix != "" { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "cloudInit", "secretPrefix"), "cannot be set if spec.template.spec.cloudInit.insecureSkipSecretsManager is true")) + } + if spec.CloudInit.SecretCount != 0 { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "cloudInit", "secretCount"), "cannot be set if spec.template.spec.cloudInit.insecureSkipSecretsManager is true")) + } + if spec.CloudInit.SecureSecretsBackend != "" { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "cloudInit", "secureSecretsBackend"), "cannot be set if spec.template.spec.cloudInit.insecureSkipSecretsManager is true")) + } + } + + if (spec.CloudInit.SecretPrefix != "") != (spec.CloudInit.SecretCount != 0) { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "cloudInit", "secretCount"), "must be set together with spec.template.spec.CloudInit.SecretPrefix")) + } + + return allErrs +} + +func (r *AWSMachineTemplate) cloudInitConfigured() bool { + spec := r.Spec.Template.Spec + configured := false + + configured = configured || spec.CloudInit.SecretPrefix != "" + configured = configured || spec.CloudInit.SecretCount != 0 + configured = configured || spec.CloudInit.SecureSecretsBackend != "" + configured = configured || spec.CloudInit.InsecureSkipSecretsManager + + return configured +} + +func (r *AWSMachineTemplate) ignitionEnabled() bool { + return r.Spec.Template.Spec.Ignition != nil +} + +func (r *AWSMachineTemplate) validateIgnitionAndCloudInit() field.ErrorList { + var allErrs field.ErrorList + + // Feature gate is not enabled but ignition is enabled then send a forbidden error. + if !feature.Gates.Enabled(feature.BootstrapFormatIgnition) && r.ignitionEnabled() { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "ignition"), + "can be set only if the BootstrapFormatIgnition feature gate is enabled")) + } + + if r.ignitionEnabled() && r.cloudInitConfigured() { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "cloudInit"), + "cannot be set if spec.template.spec.ignition is set")) + } + + return allErrs +} +func (r *AWSMachineTemplate) validateSSHKeyName() field.ErrorList { + return validateSSHKeyName(r.Spec.Template.Spec.SSHKeyName) +} + // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. func (r *AWSMachineTemplateWebhook) ValidateCreate(_ context.Context, raw runtime.Object) error { var allErrs field.ErrorList @@ -123,20 +195,13 @@ func (r *AWSMachineTemplateWebhook) ValidateCreate(_ context.Context, raw runtim allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "providerID"), "cannot be set in templates")) } + allErrs = append(allErrs, obj.validateCloudInitSecret()...) + allErrs = append(allErrs, obj.validateIgnitionAndCloudInit()...) allErrs = append(allErrs, obj.validateRootVolume()...) allErrs = append(allErrs, obj.validateNonRootVolumes()...) - - // Feature gate is not enabled but ignition is enabled then send a forbidden error. - if !feature.Gates.Enabled(feature.BootstrapFormatIgnition) && spec.Ignition != nil { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "ignition"), - "can be set only if the BootstrapFormatIgnition feature gate is enabled")) - } - - cloudInitConfigured := spec.CloudInit.SecureSecretsBackend != "" || spec.CloudInit.InsecureSkipSecretsManager - if cloudInitConfigured && spec.Ignition != nil { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "cloudInit"), - "cannot be set if spec.template.spec.ignition is set")) - } + allErrs = append(allErrs, obj.validateSSHKeyName()...) + allErrs = append(allErrs, obj.validateAdditionalSecurityGroups()...) + allErrs = append(allErrs, obj.Spec.Template.Spec.AdditionalTags.Validate()...) return aggregateObjErrors(obj.GroupVersionKind().GroupKind(), obj.Name, allErrs) }