diff --git a/Makefile b/Makefile index 5aa0f96ad394..9b58f0cccba2 100644 --- a/Makefile +++ b/Makefile @@ -105,6 +105,7 @@ verify: tidy download ## Verify code. Includes dependencies, linting, formatting hack/validation/kubelet.sh hack/validation/requirements.sh hack/validation/labels.sh + hack/mutation/conversion_webhooks_injection.sh hack/github/dependabot.sh $(foreach dir,$(MOD_DIRS),cd $(dir) && golangci-lint run $(newline)) @git diff --quiet ||\ diff --git a/charts/karpenter/README.md b/charts/karpenter/README.md index f25dc1874b62..ac229664b752 100644 --- a/charts/karpenter/README.md +++ b/charts/karpenter/README.md @@ -107,7 +107,6 @@ cosign verify public.ecr.aws/karpenter/karpenter:0.37.0 \ | terminationGracePeriodSeconds | string | `nil` | Override the default termination grace period for the pod. | | tolerations | list | `[{"key":"CriticalAddonsOnly","operator":"Exists"}]` | Tolerations to allow the pod to be scheduled to nodes with taints. | | topologySpreadConstraints | list | `[{"maxSkew":1,"topologyKey":"topology.kubernetes.io/zone","whenUnsatisfiable":"DoNotSchedule"}]` | Topology spread constraints to increase the controller resilience by distributing pods across the cluster zones. If an explicit label selector is not provided one will be created from the pod selector labels. | -| webhook.enabled | bool | `false` | Whether to enable the webhooks and webhook permissions. | | webhook.metrics.port | int | `8001` | The container port to use for webhook metrics. | | webhook.port | int | `8443` | The container port to use for the webhook. | diff --git a/charts/karpenter/templates/clusterrole-core.yaml b/charts/karpenter/templates/clusterrole-core.yaml index 56aff7894593..400b87a6257d 100644 --- a/charts/karpenter/templates/clusterrole-core.yaml +++ b/charts/karpenter/templates/clusterrole-core.yaml @@ -41,11 +41,9 @@ rules: - apiGroups: ["apps"] resources: ["daemonsets", "deployments", "replicasets", "statefulsets"] verbs: ["list", "watch"] -{{- if .Values.webhook.enabled }} - - apiGroups: ["admissionregistration.k8s.io"] - resources: ["validatingwebhookconfigurations", "mutatingwebhookconfigurations"] - verbs: ["get", "watch", "list"] -{{- end }} + - apiGroups: ["apiextensions.k8s.io"] + resources: ["customresourcedefinitions"] + verbs: ["watch", "list"] - apiGroups: ["policy"] resources: ["poddisruptionbudgets"] verbs: ["get", "list", "watch"] @@ -65,12 +63,9 @@ rules: - apiGroups: [""] resources: ["pods/eviction"] verbs: ["create"] -{{- if .Values.webhook.enabled }} - - apiGroups: ["admissionregistration.k8s.io"] - resources: ["validatingwebhookconfigurations"] + - apiGroups: ["apiextensions.k8s.io"] + resources: ["customresourcedefinitions"] verbs: ["update"] - resourceNames: ["validation.webhook.karpenter.sh", "validation.webhook.config.karpenter.sh"] -{{- end }} {{- with .Values.additionalClusterRoleRules -}} {{ toYaml . | nindent 2 }} - {{- end -}} \ No newline at end of file + {{- end -}} diff --git a/charts/karpenter/templates/clusterrole.yaml b/charts/karpenter/templates/clusterrole.yaml index a4c08562794e..c1ee51e13585 100644 --- a/charts/karpenter/templates/clusterrole.yaml +++ b/charts/karpenter/templates/clusterrole.yaml @@ -36,13 +36,3 @@ rules: - apiGroups: ["karpenter.k8s.aws"] resources: ["ec2nodeclasses", "ec2nodeclasses/status"] verbs: ["patch", "update"] -{{- if .Values.webhook.enabled }} - - apiGroups: ["admissionregistration.k8s.io"] - resources: ["validatingwebhookconfigurations"] - verbs: ["update"] - resourceNames: ["validation.webhook.karpenter.k8s.aws"] - - apiGroups: ["admissionregistration.k8s.io"] - resources: ["mutatingwebhookconfigurations"] - verbs: ["update"] - resourceNames: ["defaulting.webhook.karpenter.k8s.aws"] -{{- end }} diff --git a/charts/karpenter/templates/deployment.yaml b/charts/karpenter/templates/deployment.yaml index d1994b3485b9..c02bc291ae42 100644 --- a/charts/karpenter/templates/deployment.yaml +++ b/charts/karpenter/templates/deployment.yaml @@ -76,14 +76,10 @@ spec: value: "1.19.0-0" - name: KARPENTER_SERVICE value: {{ include "karpenter.fullname" . }} - {{- if .Values.webhook.enabled }} - name: WEBHOOK_PORT value: "{{ .Values.webhook.port }}" - name: WEBHOOK_METRICS_PORT value: "{{ .Values.webhook.metrics.port }}" - - name: DISABLE_WEBHOOK - value: "false" - {{- end }} {{- with .Values.logLevel }} - name: LOG_LEVEL value: "{{ . }}" @@ -159,14 +155,12 @@ spec: - name: http-metrics containerPort: {{ .Values.controller.metrics.port }} protocol: TCP - {{- if .Values.webhook.enabled }} - name: webhook-metrics containerPort: {{ .Values.webhook.metrics.port }} protocol: TCP - name: https-webhook containerPort: {{ .Values.webhook.port }} protocol: TCP - {{- end }} - name: http containerPort: {{ .Values.controller.healthProbe.port }} protocol: TCP diff --git a/charts/karpenter/templates/role.yaml b/charts/karpenter/templates/role.yaml index 3862e5b3f27c..298fec8ef9f1 100644 --- a/charts/karpenter/templates/role.yaml +++ b/charts/karpenter/templates/role.yaml @@ -14,19 +14,15 @@ rules: - apiGroups: ["coordination.k8s.io"] resources: ["leases"] verbs: ["get", "watch"] -{{- if .Values.webhook.enabled }} - apiGroups: [""] resources: ["configmaps", "secrets"] verbs: ["get", "list", "watch"] -{{- end }} # Write -{{- if .Values.webhook.enabled }} - apiGroups: [""] resources: ["secrets"] verbs: ["update"] resourceNames: - "{{ include "karpenter.fullname" . }}-cert" -{{- end }} - apiGroups: ["coordination.k8s.io"] resources: ["leases"] verbs: ["patch", "update"] @@ -75,4 +71,4 @@ rules: # Write - apiGroups: ["coordination.k8s.io"] resources: ["leases"] - verbs: ["delete"] \ No newline at end of file + verbs: ["delete"] diff --git a/charts/karpenter/templates/secret-webhook-cert.yaml b/charts/karpenter/templates/secret-webhook-cert.yaml index b17309f57070..a364f9b71bb5 100644 --- a/charts/karpenter/templates/secret-webhook-cert.yaml +++ b/charts/karpenter/templates/secret-webhook-cert.yaml @@ -1,4 +1,3 @@ -{{- if .Values.webhook.enabled }} apiVersion: v1 kind: Secret metadata: @@ -11,4 +10,3 @@ metadata: {{- toYaml . | nindent 4 }} {{- end }} # data: {} # Injected by karpenter-webhook -{{- end }} diff --git a/charts/karpenter/templates/service.yaml b/charts/karpenter/templates/service.yaml index 2c880b708b2b..49f94171be99 100644 --- a/charts/karpenter/templates/service.yaml +++ b/charts/karpenter/templates/service.yaml @@ -16,7 +16,6 @@ spec: port: {{ .Values.controller.metrics.port }} targetPort: http-metrics protocol: TCP - {{- if .Values.webhook.enabled }} - name: webhook-metrics port: {{ .Values.webhook.metrics.port }} targetPort: webhook-metrics @@ -25,6 +24,5 @@ spec: port: {{ .Values.webhook.port }} targetPort: https-webhook protocol: TCP - {{- end }} selector: {{- include "karpenter.selectorLabels" . | nindent 4 }} diff --git a/charts/karpenter/templates/webhooks-core.yaml b/charts/karpenter/templates/webhooks-core.yaml deleted file mode 100644 index 55297a1d96f6..000000000000 --- a/charts/karpenter/templates/webhooks-core.yaml +++ /dev/null @@ -1,69 +0,0 @@ -{{- if .Values.webhook.enabled }} -apiVersion: admissionregistration.k8s.io/v1 -kind: ValidatingWebhookConfiguration -metadata: - name: validation.webhook.karpenter.sh - labels: - {{- include "karpenter.labels" . | nindent 4 }} - {{- with .Values.additionalAnnotations }} - annotations: - {{- toYaml . | nindent 4 }} - {{- end }} -webhooks: - - name: validation.webhook.karpenter.sh - admissionReviewVersions: ["v1"] - clientConfig: - service: - name: {{ include "karpenter.fullname" . }} - namespace: {{ .Release.Namespace }} - port: {{ .Values.webhook.port }} - failurePolicy: Fail - sideEffects: None - rules: - - apiGroups: - - karpenter.sh - apiVersions: - - v1beta1 - operations: - - CREATE - - UPDATE - resources: - - nodeclaims - - nodeclaims/status - scope: '*' - - apiGroups: - - karpenter.sh - apiVersions: - - v1beta1 - operations: - - CREATE - - UPDATE - resources: - - nodepools - - nodepools/status - scope: '*' ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: ValidatingWebhookConfiguration -metadata: - name: validation.webhook.config.karpenter.sh - labels: - {{- include "karpenter.labels" . | nindent 4 }} - {{- with .Values.additionalAnnotations }} - annotations: - {{- toYaml . | nindent 4 }} - {{- end }} -webhooks: - - name: validation.webhook.config.karpenter.sh - admissionReviewVersions: ["v1"] - clientConfig: - service: - name: {{ include "karpenter.fullname" . }} - namespace: {{ .Release.Namespace }} - port: {{ .Values.webhook.port }} - failurePolicy: Fail - sideEffects: None - objectSelector: - matchLabels: - app.kubernetes.io/part-of: {{ template "karpenter.name" . }} -{{- end }} \ No newline at end of file diff --git a/charts/karpenter/templates/webhooks.yaml b/charts/karpenter/templates/webhooks.yaml deleted file mode 100644 index 481574349b4b..000000000000 --- a/charts/karpenter/templates/webhooks.yaml +++ /dev/null @@ -1,67 +0,0 @@ -{{- if .Values.webhook.enabled }} -apiVersion: admissionregistration.k8s.io/v1 -kind: MutatingWebhookConfiguration -metadata: - name: defaulting.webhook.karpenter.k8s.aws - labels: - {{- include "karpenter.labels" . | nindent 4 }} - {{- with .Values.additionalAnnotations }} - annotations: - {{- toYaml . | nindent 4 }} - {{- end }} -webhooks: - - name: defaulting.webhook.karpenter.k8s.aws - admissionReviewVersions: ["v1"] - clientConfig: - service: - name: {{ include "karpenter.fullname" . }} - namespace: {{ .Release.Namespace }} - port: {{ .Values.webhook.port }} - failurePolicy: Fail - sideEffects: None - rules: - - apiGroups: - - karpenter.k8s.aws - apiVersions: - - v1beta1 - operations: - - CREATE - - UPDATE - resources: - - ec2nodeclasses - - ec2nodeclasses/status - scope: '*' ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: ValidatingWebhookConfiguration -metadata: - name: validation.webhook.karpenter.k8s.aws - labels: - {{- include "karpenter.labels" . | nindent 4 }} - {{- with .Values.additionalAnnotations }} - annotations: - {{- toYaml . | nindent 4 }} - {{- end }} -webhooks: - - name: validation.webhook.karpenter.k8s.aws - admissionReviewVersions: ["v1"] - clientConfig: - service: - name: {{ include "karpenter.fullname" . }} - namespace: {{ .Release.Namespace }} - port: {{ .Values.webhook.port }} - failurePolicy: Fail - sideEffects: None - rules: - - apiGroups: - - karpenter.k8s.aws - apiVersions: - - v1beta1 - operations: - - CREATE - - UPDATE - resources: - - ec2nodeclasses - - ec2nodeclasses/status - scope: '*' -{{- end }} \ No newline at end of file diff --git a/charts/karpenter/values.yaml b/charts/karpenter/values.yaml index f981e6787b48..4945508449f5 100644 --- a/charts/karpenter/values.yaml +++ b/charts/karpenter/values.yaml @@ -27,8 +27,8 @@ serviceMonitor: enabled: false # -- Additional labels for the ServiceMonitor. additionalLabels: {} - # -- Configuration on `http-metrics` endpoint for the ServiceMonitor. - # Not to be used to add additional endpoints. + # -- Configuration on `http-metrics` endpoint for the ServiceMonitor. + # Not to be used to add additional endpoints. # See the Prometheus operator documentation for configurable fields https://github.com/prometheus-operator/prometheus-operator/blob/main/Documentation/api.md#endpoint endpointConfig: {} # -- Number of replicas. @@ -138,8 +138,6 @@ controller: # -- The container port to use for http health probe. port: 8081 webhook: - # -- Whether to enable the webhooks and webhook permissions. - enabled: false # -- The container port to use for the webhook. port: 8443 metrics: diff --git a/go.mod b/go.mod index 4cda3bc733d9..e4cbc0679311 100644 --- a/go.mod +++ b/go.mod @@ -57,7 +57,6 @@ require ( github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.22.4 // indirect github.com/go-task/slim-sprig/v3 v3.0.0 // indirect - github.com/gobuffalo/flect v1.0.2 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.4 // indirect diff --git a/go.sum b/go.sum index b53638d5c635..e8b0a35ae617 100644 --- a/go.sum +++ b/go.sum @@ -130,8 +130,6 @@ github.com/go-openapi/swag v0.22.4/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+ github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= -github.com/gobuffalo/flect v1.0.2 h1:eqjPGSo2WmjgY2XlpGwo2NXgL3RucAKo4k4qQMNA5sA= -github.com/gobuffalo/flect v1.0.2/go.mod h1:A5msMlrHtLqh9umBSnvabjsMrCcCpAyzglnDvkbYKHs= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= diff --git a/hack/mutation/conversion_webhooks_injection.sh b/hack/mutation/conversion_webhooks_injection.sh new file mode 100755 index 000000000000..8c940b0c8d66 --- /dev/null +++ b/hack/mutation/conversion_webhooks_injection.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash + +# Add the conversion stanza to the CRD spec to enable conversion via webhook +yq eval '.spec.conversion = {"strategy": "Webhook", "webhook": {"conversionReviewVersions": ["v1beta1", "v1"], "clientConfig": {"service": {"name": "karpenter", "namespace": "kube-system", "port": 8443}}}}' -i pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +yq eval '.spec.conversion = {"strategy": "Webhook", "webhook": {"conversionReviewVersions": ["v1beta1", "v1"], "clientConfig": {"service": {"name": "karpenter", "namespace": "kube-system", "port": 8443}}}}' -i pkg/apis/crds/karpenter.sh_nodeclaims.yaml +yq eval '.spec.conversion = {"strategy": "Webhook", "webhook": {"conversionReviewVersions": ["v1beta1", "v1"], "clientConfig": {"service": {"name": "karpenter", "namespace": "kube-system", "port": 8443}}}}' -i pkg/apis/crds/karpenter.sh_nodepools.yaml diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index 1de256a30958..95959d19e704 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -1296,3 +1296,14 @@ spec: storage: true subresources: status: {} + conversion: + strategy: Webhook + webhook: + conversionReviewVersions: + - v1beta1 + - v1 + clientConfig: + service: + name: karpenter + namespace: kube-system + port: 8443 diff --git a/pkg/apis/crds/karpenter.sh_nodeclaims.yaml b/pkg/apis/crds/karpenter.sh_nodeclaims.yaml index 53defa8a3c6d..6834866ff541 100644 --- a/pkg/apis/crds/karpenter.sh_nodeclaims.yaml +++ b/pkg/apis/crds/karpenter.sh_nodeclaims.yaml @@ -793,3 +793,14 @@ spec: storage: true subresources: status: {} + conversion: + strategy: Webhook + webhook: + conversionReviewVersions: + - v1beta1 + - v1 + clientConfig: + service: + name: karpenter + namespace: kube-system + port: 8443 diff --git a/pkg/apis/crds/karpenter.sh_nodepools.yaml b/pkg/apis/crds/karpenter.sh_nodepools.yaml index b9f9865a5fff..9ae64056f616 100644 --- a/pkg/apis/crds/karpenter.sh_nodepools.yaml +++ b/pkg/apis/crds/karpenter.sh_nodepools.yaml @@ -1071,3 +1071,14 @@ spec: storage: true subresources: status: {} + conversion: + strategy: Webhook + webhook: + conversionReviewVersions: + - v1beta1 + - v1 + clientConfig: + service: + name: karpenter + namespace: kube-system + port: 8443 diff --git a/pkg/apis/v1/ec2nodeclass_validation.go b/pkg/apis/v1/ec2nodeclass_validation.go deleted file mode 100644 index f0e7dcc6e5cc..000000000000 --- a/pkg/apis/v1/ec2nodeclass_validation.go +++ /dev/null @@ -1,307 +0,0 @@ -/* -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1 - -import ( - "context" - "fmt" - "strings" - - "github.com/aws/aws-sdk-go/service/ec2" - admissionregistrationv1 "k8s.io/api/admissionregistration/v1" - "k8s.io/apimachinery/pkg/api/resource" - "knative.dev/pkg/apis" -) - -const ( - subnetSelectorTermsPath = "subnetSelectorTerms" - securityGroupSelectorTermsPath = "securityGroupSelectorTerms" - amiSelectorTermsPath = "amiSelectorTerms" - amiFamilyPath = "amiFamily" - tagsPath = "tags" - metadataOptionsPath = "metadataOptions" - blockDeviceMappingsPath = "blockDeviceMappings" - rolePath = "role" - instanceProfilePath = "instanceProfile" -) - -var ( - minVolumeSize = *resource.NewScaledQuantity(1, resource.Giga) - maxVolumeSize = *resource.NewScaledQuantity(64, resource.Tera) -) - -func (in *EC2NodeClass) SupportedVerbs() []admissionregistrationv1.OperationType { - return []admissionregistrationv1.OperationType{ - admissionregistrationv1.Create, - admissionregistrationv1.Update, - } -} - -func (in *EC2NodeClass) Validate(ctx context.Context) (errs *apis.FieldError) { - if apis.IsInUpdate(ctx) { - original := apis.GetBaseline(ctx).(*EC2NodeClass) - errs = in.validateImmutableFields(original) - } - return errs.Also( - apis.ValidateObjectMetadata(in).ViaField("metadata"), - in.Spec.validate(ctx).ViaField("spec"), - ) -} - -func (in *EC2NodeClass) validateImmutableFields(original *EC2NodeClass) (errs *apis.FieldError) { - return errs.Also( - in.Spec.validateRoleImmutability(&original.Spec).ViaField("spec"), - ) -} - -func (in *EC2NodeClassSpec) validate(_ context.Context) (errs *apis.FieldError) { - if in.Role != "" && in.InstanceProfile != nil { - errs = errs.Also(apis.ErrMultipleOneOf(rolePath, instanceProfilePath)) - } - if in.Role == "" && in.InstanceProfile == nil { - errs = errs.Also(apis.ErrMissingOneOf(rolePath, instanceProfilePath)) - } - return errs.Also( - in.validateSubnetSelectorTerms().ViaField(subnetSelectorTermsPath), - in.validateSecurityGroupSelectorTerms().ViaField(securityGroupSelectorTermsPath), - in.validateAMISelectorTerms().ViaField(amiSelectorTermsPath), - in.validateMetadataOptions().ViaField(metadataOptionsPath), - in.validateAMIFamily().ViaField(amiFamilyPath), - in.validateBlockDeviceMappings().ViaField(blockDeviceMappingsPath), - in.validateTags().ViaField(tagsPath), - ) -} - -func (in *EC2NodeClassSpec) validateSubnetSelectorTerms() (errs *apis.FieldError) { - if len(in.SubnetSelectorTerms) == 0 { - errs = errs.Also(apis.ErrMissingOneOf()) - } - for i, term := range in.SubnetSelectorTerms { - errs = errs.Also(term.validate()).ViaIndex(i) - } - return errs -} - -func (in *SubnetSelectorTerm) validate() (errs *apis.FieldError) { - errs = errs.Also(validateTags(in.Tags).ViaField("tags")) - if len(in.Tags) == 0 && in.ID == "" { - errs = errs.Also(apis.ErrGeneric("expected at least one, got none", "tags", "id")) - } else if in.ID != "" && len(in.Tags) > 0 { - errs = errs.Also(apis.ErrGeneric(`"id" is mutually exclusive, cannot be set with a combination of other fields in`)) - } - return errs -} - -func (in *EC2NodeClassSpec) validateSecurityGroupSelectorTerms() (errs *apis.FieldError) { - if len(in.SecurityGroupSelectorTerms) == 0 { - errs = errs.Also(apis.ErrMissingOneOf()) - } - for _, term := range in.SecurityGroupSelectorTerms { - errs = errs.Also(term.validate()) - } - return errs -} - -//nolint:gocyclo -func (in *SecurityGroupSelectorTerm) validate() (errs *apis.FieldError) { - errs = errs.Also(validateTags(in.Tags).ViaField("tags")) - if len(in.Tags) == 0 && in.ID == "" && in.Name == "" { - errs = errs.Also(apis.ErrGeneric("expect at least one, got none", "tags", "id", "name")) - } else if in.ID != "" && (len(in.Tags) > 0 || in.Name != "") { - errs = errs.Also(apis.ErrGeneric(`"id" is mutually exclusive, cannot be set with a combination of other fields in`)) - } else if in.Name != "" && (len(in.Tags) > 0 || in.ID != "") { - errs = errs.Also(apis.ErrGeneric(`"name" is mutually exclusive, cannot be set with a combination of other fields in`)) - } - return errs -} - -func (in *EC2NodeClassSpec) validateAMISelectorTerms() (errs *apis.FieldError) { - for _, term := range in.AMISelectorTerms { - errs = errs.Also(term.validate()) - } - return errs -} - -//nolint:gocyclo -func (in *AMISelectorTerm) validate() (errs *apis.FieldError) { - errs = errs.Also(validateTags(in.Tags).ViaField("tags")) - if len(in.Tags) == 0 && in.ID == "" && in.Name == "" { - errs = errs.Also(apis.ErrGeneric("expect at least one, got none", "tags", "id", "name")) - } else if in.ID != "" && (len(in.Tags) > 0 || in.Name != "" || in.Owner != "") { - errs = errs.Also(apis.ErrGeneric(`"id" is mutually exclusive, cannot be set with a combination of other fields in`)) - } - return errs -} - -func validateTags(m map[string]string) (errs *apis.FieldError) { - for k, v := range m { - if k == "" { - errs = errs.Also(apis.ErrInvalidKeyName(`""`, "")) - } - if v == "" { - errs = errs.Also(apis.ErrInvalidValue(`""`, k)) - } - } - return errs -} - -func (in *EC2NodeClassSpec) validateMetadataOptions() (errs *apis.FieldError) { - if in.MetadataOptions == nil { - return nil - } - return errs.Also( - in.validateHTTPEndpoint(), - in.validateHTTPProtocolIpv6(), - in.validateHTTPPutResponseHopLimit(), - in.validateHTTPTokens(), - ) -} - -func (in *EC2NodeClassSpec) validateHTTPEndpoint() *apis.FieldError { - if in.MetadataOptions.HTTPEndpoint == nil { - return nil - } - return in.validateStringEnum(*in.MetadataOptions.HTTPEndpoint, "httpEndpoint", ec2.LaunchTemplateInstanceMetadataEndpointState_Values()) -} - -func (in *EC2NodeClassSpec) validateHTTPProtocolIpv6() *apis.FieldError { - if in.MetadataOptions.HTTPProtocolIPv6 == nil { - return nil - } - return in.validateStringEnum(*in.MetadataOptions.HTTPProtocolIPv6, "httpProtocolIPv6", ec2.LaunchTemplateInstanceMetadataProtocolIpv6_Values()) -} - -func (in *EC2NodeClassSpec) validateHTTPPutResponseHopLimit() *apis.FieldError { - if in.MetadataOptions.HTTPPutResponseHopLimit == nil { - return nil - } - limit := *in.MetadataOptions.HTTPPutResponseHopLimit - if limit < 1 || limit > 64 { - return apis.ErrOutOfBoundsValue(limit, 1, 64, "httpPutResponseHopLimit") - } - return nil -} - -func (in *EC2NodeClassSpec) validateHTTPTokens() *apis.FieldError { - if in.MetadataOptions.HTTPTokens == nil { - return nil - } - return in.validateStringEnum(*in.MetadataOptions.HTTPTokens, "httpTokens", ec2.LaunchTemplateHttpTokensState_Values()) -} - -func (in *EC2NodeClassSpec) validateStringEnum(value, field string, validValues []string) *apis.FieldError { - for _, validValue := range validValues { - if value == validValue { - return nil - } - } - return apis.ErrInvalidValue(fmt.Sprintf("%s not in %v", value, strings.Join(validValues, ", ")), field) -} - -func (in *EC2NodeClassSpec) validateBlockDeviceMappings() (errs *apis.FieldError) { - numRootVolume := 0 - for i, blockDeviceMapping := range in.BlockDeviceMappings { - if err := in.validateBlockDeviceMapping(blockDeviceMapping); err != nil { - errs = errs.Also(err.ViaFieldIndex(blockDeviceMappingsPath, i)) - } - if blockDeviceMapping.RootVolume { - numRootVolume++ - } - } - if numRootVolume > 1 { - errs = errs.Also(apis.ErrMultipleOneOf("more than 1 root volume configured")) - } - return errs -} - -func (in *EC2NodeClassSpec) validateBlockDeviceMapping(blockDeviceMapping *BlockDeviceMapping) (errs *apis.FieldError) { - return errs.Also(in.validateDeviceName(blockDeviceMapping), in.validateEBS(blockDeviceMapping)) -} - -func (in *EC2NodeClassSpec) validateDeviceName(blockDeviceMapping *BlockDeviceMapping) *apis.FieldError { - if blockDeviceMapping.DeviceName == nil { - return apis.ErrMissingField("deviceName") - } - return nil -} - -func (in *EC2NodeClassSpec) validateEBS(blockDeviceMapping *BlockDeviceMapping) (errs *apis.FieldError) { - if blockDeviceMapping.EBS == nil { - return apis.ErrMissingField("ebs") - } - for _, err := range []*apis.FieldError{ - in.validateVolumeType(blockDeviceMapping), - in.validateVolumeSize(blockDeviceMapping), - } { - if err != nil { - errs = errs.Also(err.ViaField("ebs")) - } - } - return errs -} - -func (in *EC2NodeClassSpec) validateVolumeType(blockDeviceMapping *BlockDeviceMapping) *apis.FieldError { - if blockDeviceMapping.EBS.VolumeType != nil { - return in.validateStringEnum(*blockDeviceMapping.EBS.VolumeType, "volumeType", ec2.VolumeType_Values()) - } - return nil -} - -func (in *EC2NodeClassSpec) validateVolumeSize(blockDeviceMapping *BlockDeviceMapping) *apis.FieldError { - // If an EBS mapping is present, one of volumeSize or snapshotID must be present - if blockDeviceMapping.EBS.SnapshotID != nil && blockDeviceMapping.EBS.VolumeSize == nil { - return nil - } else if blockDeviceMapping.EBS.VolumeSize == nil { - return apis.ErrMissingField("volumeSize") - } else if blockDeviceMapping.EBS.VolumeSize.Cmp(minVolumeSize) == -1 || blockDeviceMapping.EBS.VolumeSize.Cmp(maxVolumeSize) == 1 { - return apis.ErrOutOfBoundsValue(blockDeviceMapping.EBS.VolumeSize.String(), minVolumeSize.String(), maxVolumeSize.String(), "volumeSize") - } - return nil -} - -func (in *EC2NodeClassSpec) validateAMIFamily() (errs *apis.FieldError) { - if in.AMIFamily == nil { - return nil - } - if *in.AMIFamily == AMIFamilyCustom && len(in.AMISelectorTerms) == 0 { - errs = errs.Also(apis.ErrMissingField(amiSelectorTermsPath)) - } - return errs -} - -func (in *EC2NodeClassSpec) validateTags() (errs *apis.FieldError) { - for k, v := range in.Tags { - if k == "" { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf( - "the tag with key : '' and value : '%s' is invalid because empty tag keys aren't supported", v), "tags")) - } - for _, pattern := range RestrictedTagPatterns { - if pattern.MatchString(k) { - errs = errs.Also(apis.ErrInvalidKeyName(k, "tags", fmt.Sprintf("tag contains in restricted tag matching %q", pattern.String()))) - } - } - } - return errs -} - -func (in *EC2NodeClassSpec) validateRoleImmutability(originalSpec *EC2NodeClassSpec) *apis.FieldError { - if in.Role != originalSpec.Role { - return &apis.FieldError{ - Message: "Immutable field changed", - Paths: []string{"role"}, - } - } - return nil -} diff --git a/pkg/apis/v1/ec2nodeclass_validation_webhook_test.go b/pkg/apis/v1/ec2nodeclass_validation_webhook_test.go deleted file mode 100644 index 59f388d3dffc..000000000000 --- a/pkg/apis/v1/ec2nodeclass_validation_webhook_test.go +++ /dev/null @@ -1,546 +0,0 @@ -/* -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1_test - -import ( - "github.com/samber/lo" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "knative.dev/pkg/apis" - "sigs.k8s.io/karpenter/pkg/test" - - "github.com/aws/aws-sdk-go/aws" - - v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Webhook/Validation", func() { - var nc *v1.EC2NodeClass - - BeforeEach(func() { - nc = &v1.EC2NodeClass{ - ObjectMeta: test.ObjectMeta(metav1.ObjectMeta{}), - Spec: v1.EC2NodeClassSpec{ - AMIFamily: lo.ToPtr(v1.AMIFamilyAL2023), - Role: "role-1", - SecurityGroupSelectorTerms: []v1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "*": "*", - }, - }, - }, - SubnetSelectorTerms: []v1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "*": "*", - }, - }, - }, - }, - } - }) - It("should succeed if just specifying role", func() { - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed if just specifying instance profile", func() { - nc.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - nc.Spec.Role = "" - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail if specifying both instance profile and role", func() { - nc.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail if not specifying one of instance profile and role", func() { - nc.Spec.Role = "" - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - Context("UserData", func() { - It("should succeed if user data is empty", func() { - Expect(nc.Validate(ctx)).To(Succeed()) - }) - }) - Context("Tags", func() { - It("should succeed when tags are empty", func() { - nc.Spec.Tags = map[string]string{} - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed if tags aren't in restricted tag keys", func() { - nc.Spec.Tags = map[string]string{ - "karpenter.sh/custom-key": "value", - "karpenter.sh/managed": "true", - "kubernetes.io/role/key": "value", - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed by validating that regex is properly escaped", func() { - nc.Spec.Tags = map[string]string{ - "karpenterzsh/nodepool": "value", - } - Expect(nc.Validate(ctx)).To(Succeed()) - nc.Spec.Tags = map[string]string{ - "kubernetesbio/cluster/test": "value", - } - Expect(nc.Validate(ctx)).To(Succeed()) - nc.Spec.Tags = map[string]string{ - "karpenterzsh/managed-by": "test", - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail if tags contain a restricted domain key", func() { - nc.Spec.Tags = map[string]string{ - "karpenter.sh/nodepool": "value", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - nc.Spec.Tags = map[string]string{ - "kubernetes.io/cluster/test": "value", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - nc.Spec.Tags = map[string]string{ - "karpenter.sh/managed-by": "test", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - nc.Spec.Tags = map[string]string{ - v1.LabelNodeClass: "test", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - nc.Spec.Tags = map[string]string{ - "karpenter.sh/nodeclaim": "test", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - }) - }) - Context("SubnetSelectorTerms", func() { - It("should succeed with a valid subnet selector on tags", func() { - nc.Spec.SubnetSelectorTerms = []v1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid subnet selector on id", func() { - nc.Spec.SubnetSelectorTerms = []v1.SubnetSelectorTerm{ - { - ID: "subnet-12345749", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail when subnet selector terms is set to nil", func() { - nc.Spec.SubnetSelectorTerms = nil - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when no subnet selector terms exist", func() { - nc.Spec.SubnetSelectorTerms = []v1.SubnetSelectorTerm{} - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has no values", func() { - nc.Spec.SubnetSelectorTerms = []v1.SubnetSelectorTerm{ - {}, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has no tag map values", func() { - nc.Spec.SubnetSelectorTerms = []v1.SubnetSelectorTerm{ - { - Tags: map[string]string{}, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has a tag map key that is empty", func() { - nc.Spec.SubnetSelectorTerms = []v1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "test": "", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has a tag map value that is empty", func() { - nc.Spec.SubnetSelectorTerms = []v1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when the last subnet selector is invalid", func() { - nc.Spec.SubnetSelectorTerms = []v1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - { - Tags: map[string]string{ - "test2": "testvalue2", - }, - }, - { - Tags: map[string]string{ - "test3": "testvalue3", - }, - }, - { - Tags: map[string]string{ - "": "testvalue4", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with tags", func() { - nc.Spec.SubnetSelectorTerms = []v1.SubnetSelectorTerm{ - { - ID: "subnet-12345749", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - }) - Context("SecurityGroupSelectorTerms", func() { - It("should succeed with a valid security group selector on tags", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid security group selector on id", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{ - { - ID: "sg-12345749", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid security group selector on name", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{ - { - Name: "testname", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail when security group selector terms is set to nil", func() { - nc.Spec.SecurityGroupSelectorTerms = nil - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when no security group selector terms exist", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{} - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has no values", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{ - {}, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has no tag map values", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{}, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has a tag map key that is empty", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "test": "", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has a tag map value that is empty", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when the last security group selector is invalid", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - { - Tags: map[string]string{ - "test2": "testvalue2", - }, - }, - { - Tags: map[string]string{ - "test3": "testvalue3", - }, - }, - { - Tags: map[string]string{ - "": "testvalue4", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with tags", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{ - { - ID: "sg-12345749", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with name", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{ - { - ID: "sg-12345749", - Name: "my-security-group", - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying name with tags", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1.SecurityGroupSelectorTerm{ - { - Name: "my-security-group", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - }) - Context("AMISelectorTerms", func() { - It("should succeed with a valid ami selector on tags", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid ami selector on id", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - ID: "sg-12345749", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid ami selector on name", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - Name: "testname", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid ami selector on name and owner", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - Name: "testname", - Owner: "testowner", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed when an ami selector term has an owner key with tags", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - Owner: "testowner", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail when a ami selector term has no values", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - {}, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a ami selector term has no tag map values", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - Tags: map[string]string{}, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a ami selector term has a tag map key that is empty", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - Tags: map[string]string{ - "test": "", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a ami selector term has a tag map value that is empty", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - Tags: map[string]string{ - "": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when the last ami selector is invalid", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - { - Tags: map[string]string{ - "test2": "testvalue2", - }, - }, - { - Tags: map[string]string{ - "test3": "testvalue3", - }, - }, - { - Tags: map[string]string{ - "": "testvalue4", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with tags", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - ID: "ami-12345749", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with name", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - ID: "ami-12345749", - Name: "my-custom-ami", - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with owner", func() { - nc.Spec.AMISelectorTerms = []v1.AMISelectorTerm{ - { - ID: "ami-12345749", - Owner: "123456789", - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - }) - Context("BlockDeviceMappings", func() { - It("should fail if more than one root volume is specified", func() { - nodeClass := &v1.EC2NodeClass{ - Spec: v1.EC2NodeClassSpec{ - BlockDeviceMappings: []*v1.BlockDeviceMapping{ - { - DeviceName: aws.String("map-device-1"), - EBS: &v1.BlockDevice{ - VolumeSize: resource.NewScaledQuantity(50, resource.Giga), - }, - - RootVolume: true, - }, - { - DeviceName: aws.String("map-device-2"), - EBS: &v1.BlockDevice{ - VolumeSize: resource.NewScaledQuantity(50, resource.Giga), - }, - - RootVolume: true, - }, - }, - }, - } - Expect(nodeClass.Validate(ctx)).To(Not(Succeed())) - }) - }) - Context("Role Immutability", func() { - It("should fail when updating the role", func() { - nc.Spec.Role = "test-role" - Expect(nc.Validate(ctx)).To(Succeed()) - - updateCtx := apis.WithinUpdate(ctx, nc.DeepCopy()) - nc.Spec.Role = "test-role2" - Expect(nc.Validate(updateCtx)).ToNot(Succeed()) - }) - It("should fail to switch between an unmanaged and managed instance profile", func() { - nc.Spec.Role = "" - nc.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - Expect(nc.Validate(ctx)).To(Succeed()) - - updateCtx := apis.WithinUpdate(ctx, nc.DeepCopy()) - nc.Spec.Role = "test-role" - nc.Spec.InstanceProfile = nil - Expect(nc.Validate(updateCtx)).ToNot(Succeed()) - }) - It("should fail to switch between a managed and unmanaged instance profile", func() { - nc.Spec.Role = "test-role" - nc.Spec.InstanceProfile = nil - Expect(nc.Validate(ctx)).To(Succeed()) - - updateCtx := apis.WithinUpdate(ctx, nc.DeepCopy()) - nc.Spec.Role = "" - nc.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - Expect(nc.Validate(updateCtx)).ToNot(Succeed()) - }) - }) -}) diff --git a/pkg/apis/v1beta1/ec2nodeclass_validation.go b/pkg/apis/v1beta1/ec2nodeclass_validation.go deleted file mode 100644 index 4fc18527f00d..000000000000 --- a/pkg/apis/v1beta1/ec2nodeclass_validation.go +++ /dev/null @@ -1,307 +0,0 @@ -/* -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1beta1 - -import ( - "context" - "fmt" - "strings" - - "github.com/aws/aws-sdk-go/service/ec2" - admissionregistrationv1 "k8s.io/api/admissionregistration/v1" - "k8s.io/apimachinery/pkg/api/resource" - "knative.dev/pkg/apis" -) - -const ( - subnetSelectorTermsPath = "subnetSelectorTerms" - securityGroupSelectorTermsPath = "securityGroupSelectorTerms" - amiSelectorTermsPath = "amiSelectorTerms" - amiFamilyPath = "amiFamily" - tagsPath = "tags" - metadataOptionsPath = "metadataOptions" - blockDeviceMappingsPath = "blockDeviceMappings" - rolePath = "role" - instanceProfilePath = "instanceProfile" -) - -var ( - minVolumeSize = *resource.NewScaledQuantity(1, resource.Giga) - maxVolumeSize = *resource.NewScaledQuantity(64, resource.Tera) -) - -func (in *EC2NodeClass) SupportedVerbs() []admissionregistrationv1.OperationType { - return []admissionregistrationv1.OperationType{ - admissionregistrationv1.Create, - admissionregistrationv1.Update, - } -} - -func (in *EC2NodeClass) Validate(ctx context.Context) (errs *apis.FieldError) { - if apis.IsInUpdate(ctx) { - original := apis.GetBaseline(ctx).(*EC2NodeClass) - errs = in.validateImmutableFields(original) - } - return errs.Also( - apis.ValidateObjectMetadata(in).ViaField("metadata"), - in.Spec.validate(ctx).ViaField("spec"), - ) -} - -func (in *EC2NodeClass) validateImmutableFields(original *EC2NodeClass) (errs *apis.FieldError) { - return errs.Also( - in.Spec.validateRoleImmutability(&original.Spec).ViaField("spec"), - ) -} - -func (in *EC2NodeClassSpec) validate(_ context.Context) (errs *apis.FieldError) { - if in.Role != "" && in.InstanceProfile != nil { - errs = errs.Also(apis.ErrMultipleOneOf(rolePath, instanceProfilePath)) - } - if in.Role == "" && in.InstanceProfile == nil { - errs = errs.Also(apis.ErrMissingOneOf(rolePath, instanceProfilePath)) - } - return errs.Also( - in.validateSubnetSelectorTerms().ViaField(subnetSelectorTermsPath), - in.validateSecurityGroupSelectorTerms().ViaField(securityGroupSelectorTermsPath), - in.validateAMISelectorTerms().ViaField(amiSelectorTermsPath), - in.validateMetadataOptions().ViaField(metadataOptionsPath), - in.validateAMIFamily().ViaField(amiFamilyPath), - in.validateBlockDeviceMappings().ViaField(blockDeviceMappingsPath), - in.validateTags().ViaField(tagsPath), - ) -} - -func (in *EC2NodeClassSpec) validateSubnetSelectorTerms() (errs *apis.FieldError) { - if len(in.SubnetSelectorTerms) == 0 { - errs = errs.Also(apis.ErrMissingOneOf()) - } - for i, term := range in.SubnetSelectorTerms { - errs = errs.Also(term.validate()).ViaIndex(i) - } - return errs -} - -func (in *SubnetSelectorTerm) validate() (errs *apis.FieldError) { - errs = errs.Also(validateTags(in.Tags).ViaField("tags")) - if len(in.Tags) == 0 && in.ID == "" { - errs = errs.Also(apis.ErrGeneric("expected at least one, got none", "tags", "id")) - } else if in.ID != "" && len(in.Tags) > 0 { - errs = errs.Also(apis.ErrGeneric(`"id" is mutually exclusive, cannot be set with a combination of other fields in`)) - } - return errs -} - -func (in *EC2NodeClassSpec) validateSecurityGroupSelectorTerms() (errs *apis.FieldError) { - if len(in.SecurityGroupSelectorTerms) == 0 { - errs = errs.Also(apis.ErrMissingOneOf()) - } - for _, term := range in.SecurityGroupSelectorTerms { - errs = errs.Also(term.validate()) - } - return errs -} - -//nolint:gocyclo -func (in *SecurityGroupSelectorTerm) validate() (errs *apis.FieldError) { - errs = errs.Also(validateTags(in.Tags).ViaField("tags")) - if len(in.Tags) == 0 && in.ID == "" && in.Name == "" { - errs = errs.Also(apis.ErrGeneric("expect at least one, got none", "tags", "id", "name")) - } else if in.ID != "" && (len(in.Tags) > 0 || in.Name != "") { - errs = errs.Also(apis.ErrGeneric(`"id" is mutually exclusive, cannot be set with a combination of other fields in`)) - } else if in.Name != "" && (len(in.Tags) > 0 || in.ID != "") { - errs = errs.Also(apis.ErrGeneric(`"name" is mutually exclusive, cannot be set with a combination of other fields in`)) - } - return errs -} - -func (in *EC2NodeClassSpec) validateAMISelectorTerms() (errs *apis.FieldError) { - for _, term := range in.AMISelectorTerms { - errs = errs.Also(term.validate()) - } - return errs -} - -//nolint:gocyclo -func (in *AMISelectorTerm) validate() (errs *apis.FieldError) { - errs = errs.Also(validateTags(in.Tags).ViaField("tags")) - if len(in.Tags) == 0 && in.ID == "" && in.Name == "" { - errs = errs.Also(apis.ErrGeneric("expect at least one, got none", "tags", "id", "name")) - } else if in.ID != "" && (len(in.Tags) > 0 || in.Name != "" || in.Owner != "") { - errs = errs.Also(apis.ErrGeneric(`"id" is mutually exclusive, cannot be set with a combination of other fields in`)) - } - return errs -} - -func validateTags(m map[string]string) (errs *apis.FieldError) { - for k, v := range m { - if k == "" { - errs = errs.Also(apis.ErrInvalidKeyName(`""`, "")) - } - if v == "" { - errs = errs.Also(apis.ErrInvalidValue(`""`, k)) - } - } - return errs -} - -func (in *EC2NodeClassSpec) validateMetadataOptions() (errs *apis.FieldError) { - if in.MetadataOptions == nil { - return nil - } - return errs.Also( - in.validateHTTPEndpoint(), - in.validateHTTPProtocolIpv6(), - in.validateHTTPPutResponseHopLimit(), - in.validateHTTPTokens(), - ) -} - -func (in *EC2NodeClassSpec) validateHTTPEndpoint() *apis.FieldError { - if in.MetadataOptions.HTTPEndpoint == nil { - return nil - } - return in.validateStringEnum(*in.MetadataOptions.HTTPEndpoint, "httpEndpoint", ec2.LaunchTemplateInstanceMetadataEndpointState_Values()) -} - -func (in *EC2NodeClassSpec) validateHTTPProtocolIpv6() *apis.FieldError { - if in.MetadataOptions.HTTPProtocolIPv6 == nil { - return nil - } - return in.validateStringEnum(*in.MetadataOptions.HTTPProtocolIPv6, "httpProtocolIPv6", ec2.LaunchTemplateInstanceMetadataProtocolIpv6_Values()) -} - -func (in *EC2NodeClassSpec) validateHTTPPutResponseHopLimit() *apis.FieldError { - if in.MetadataOptions.HTTPPutResponseHopLimit == nil { - return nil - } - limit := *in.MetadataOptions.HTTPPutResponseHopLimit - if limit < 1 || limit > 64 { - return apis.ErrOutOfBoundsValue(limit, 1, 64, "httpPutResponseHopLimit") - } - return nil -} - -func (in *EC2NodeClassSpec) validateHTTPTokens() *apis.FieldError { - if in.MetadataOptions.HTTPTokens == nil { - return nil - } - return in.validateStringEnum(*in.MetadataOptions.HTTPTokens, "httpTokens", ec2.LaunchTemplateHttpTokensState_Values()) -} - -func (in *EC2NodeClassSpec) validateStringEnum(value, field string, validValues []string) *apis.FieldError { - for _, validValue := range validValues { - if value == validValue { - return nil - } - } - return apis.ErrInvalidValue(fmt.Sprintf("%s not in %v", value, strings.Join(validValues, ", ")), field) -} - -func (in *EC2NodeClassSpec) validateBlockDeviceMappings() (errs *apis.FieldError) { - numRootVolume := 0 - for i, blockDeviceMapping := range in.BlockDeviceMappings { - if err := in.validateBlockDeviceMapping(blockDeviceMapping); err != nil { - errs = errs.Also(err.ViaFieldIndex(blockDeviceMappingsPath, i)) - } - if blockDeviceMapping.RootVolume { - numRootVolume++ - } - } - if numRootVolume > 1 { - errs = errs.Also(apis.ErrMultipleOneOf("more than 1 root volume configured")) - } - return errs -} - -func (in *EC2NodeClassSpec) validateBlockDeviceMapping(blockDeviceMapping *BlockDeviceMapping) (errs *apis.FieldError) { - return errs.Also(in.validateDeviceName(blockDeviceMapping), in.validateEBS(blockDeviceMapping)) -} - -func (in *EC2NodeClassSpec) validateDeviceName(blockDeviceMapping *BlockDeviceMapping) *apis.FieldError { - if blockDeviceMapping.DeviceName == nil { - return apis.ErrMissingField("deviceName") - } - return nil -} - -func (in *EC2NodeClassSpec) validateEBS(blockDeviceMapping *BlockDeviceMapping) (errs *apis.FieldError) { - if blockDeviceMapping.EBS == nil { - return apis.ErrMissingField("ebs") - } - for _, err := range []*apis.FieldError{ - in.validateVolumeType(blockDeviceMapping), - in.validateVolumeSize(blockDeviceMapping), - } { - if err != nil { - errs = errs.Also(err.ViaField("ebs")) - } - } - return errs -} - -func (in *EC2NodeClassSpec) validateVolumeType(blockDeviceMapping *BlockDeviceMapping) *apis.FieldError { - if blockDeviceMapping.EBS.VolumeType != nil { - return in.validateStringEnum(*blockDeviceMapping.EBS.VolumeType, "volumeType", ec2.VolumeType_Values()) - } - return nil -} - -func (in *EC2NodeClassSpec) validateVolumeSize(blockDeviceMapping *BlockDeviceMapping) *apis.FieldError { - // If an EBS mapping is present, one of volumeSize or snapshotID must be present - if blockDeviceMapping.EBS.SnapshotID != nil && blockDeviceMapping.EBS.VolumeSize == nil { - return nil - } else if blockDeviceMapping.EBS.VolumeSize == nil { - return apis.ErrMissingField("volumeSize") - } else if blockDeviceMapping.EBS.VolumeSize.Cmp(minVolumeSize) == -1 || blockDeviceMapping.EBS.VolumeSize.Cmp(maxVolumeSize) == 1 { - return apis.ErrOutOfBoundsValue(blockDeviceMapping.EBS.VolumeSize.String(), minVolumeSize.String(), maxVolumeSize.String(), "volumeSize") - } - return nil -} - -func (in *EC2NodeClassSpec) validateAMIFamily() (errs *apis.FieldError) { - if in.AMIFamily == nil { - return nil - } - if *in.AMIFamily == AMIFamilyCustom && len(in.AMISelectorTerms) == 0 { - errs = errs.Also(apis.ErrMissingField(amiSelectorTermsPath)) - } - return errs -} - -func (in *EC2NodeClassSpec) validateTags() (errs *apis.FieldError) { - for k, v := range in.Tags { - if k == "" { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf( - "the tag with key : '' and value : '%s' is invalid because empty tag keys aren't supported", v), "tags")) - } - for _, pattern := range RestrictedTagPatterns { - if pattern.MatchString(k) { - errs = errs.Also(apis.ErrInvalidKeyName(k, "tags", fmt.Sprintf("tag contains in restricted tag matching %q", pattern.String()))) - } - } - } - return errs -} - -func (in *EC2NodeClassSpec) validateRoleImmutability(originalSpec *EC2NodeClassSpec) *apis.FieldError { - if in.Role != originalSpec.Role { - return &apis.FieldError{ - Message: "Immutable field changed", - Paths: []string{"role"}, - } - } - return nil -} diff --git a/pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go b/pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go deleted file mode 100644 index 18035649e235..000000000000 --- a/pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go +++ /dev/null @@ -1,525 +0,0 @@ -/* -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1beta1_test - -import ( - "github.com/samber/lo" - "k8s.io/apimachinery/pkg/api/resource" - "knative.dev/pkg/apis" - - "github.com/aws/aws-sdk-go/aws" - - "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" - "github.com/aws/karpenter-provider-aws/pkg/test" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Webhook/Validation", func() { - var nc *v1beta1.EC2NodeClass - - BeforeEach(func() { - nc = test.EC2NodeClass() - }) - It("should succeed if just specifying role", func() { - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed if just specifying instance profile", func() { - nc.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - nc.Spec.Role = "" - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail if specifying both instance profile and role", func() { - nc.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail if not specifying one of instance profile and role", func() { - nc.Spec.Role = "" - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - Context("UserData", func() { - It("should succeed if user data is empty", func() { - Expect(nc.Validate(ctx)).To(Succeed()) - }) - }) - Context("Tags", func() { - It("should succeed when tags are empty", func() { - nc.Spec.Tags = map[string]string{} - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed if tags aren't in restricted tag keys", func() { - nc.Spec.Tags = map[string]string{ - "karpenter.sh/custom-key": "value", - "karpenter.sh/managed": "true", - "kubernetes.io/role/key": "value", - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed by validating that regex is properly escaped", func() { - nc.Spec.Tags = map[string]string{ - "karpenterzsh/nodepool": "value", - } - Expect(nc.Validate(ctx)).To(Succeed()) - nc.Spec.Tags = map[string]string{ - "kubernetesbio/cluster/test": "value", - } - Expect(nc.Validate(ctx)).To(Succeed()) - nc.Spec.Tags = map[string]string{ - "karpenterzsh/managed-by": "test", - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail if tags contain a restricted domain key", func() { - nc.Spec.Tags = map[string]string{ - "karpenter.sh/nodepool": "value", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - nc.Spec.Tags = map[string]string{ - "kubernetes.io/cluster/test": "value", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - nc.Spec.Tags = map[string]string{ - "karpenter.sh/managed-by": "test", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - nc.Spec.Tags = map[string]string{ - v1beta1.LabelNodeClass: "test", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - nc.Spec.Tags = map[string]string{ - "karpenter.sh/nodeclaim": "test", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - }) - }) - Context("SubnetSelectorTerms", func() { - It("should succeed with a valid subnet selector on tags", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid subnet selector on id", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - ID: "subnet-12345749", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail when subnet selector terms is set to nil", func() { - nc.Spec.SubnetSelectorTerms = nil - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when no subnet selector terms exist", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{} - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has no values", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - {}, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has no tag map values", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{}, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has a tag map key that is empty", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "test": "", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has a tag map value that is empty", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when the last subnet selector is invalid", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - { - Tags: map[string]string{ - "test2": "testvalue2", - }, - }, - { - Tags: map[string]string{ - "test3": "testvalue3", - }, - }, - { - Tags: map[string]string{ - "": "testvalue4", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with tags", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - ID: "subnet-12345749", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - }) - Context("SecurityGroupSelectorTerms", func() { - It("should succeed with a valid security group selector on tags", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid security group selector on id", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - ID: "sg-12345749", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid security group selector on name", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Name: "testname", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail when security group selector terms is set to nil", func() { - nc.Spec.SecurityGroupSelectorTerms = nil - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when no security group selector terms exist", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{} - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has no values", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - {}, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has no tag map values", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{}, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has a tag map key that is empty", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "test": "", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has a tag map value that is empty", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when the last security group selector is invalid", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - { - Tags: map[string]string{ - "test2": "testvalue2", - }, - }, - { - Tags: map[string]string{ - "test3": "testvalue3", - }, - }, - { - Tags: map[string]string{ - "": "testvalue4", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with tags", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - ID: "sg-12345749", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with name", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - ID: "sg-12345749", - Name: "my-security-group", - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying name with tags", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Name: "my-security-group", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - }) - Context("AMISelectorTerms", func() { - It("should succeed with a valid ami selector on tags", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid ami selector on id", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - ID: "sg-12345749", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid ami selector on name", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Name: "testname", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid ami selector on name and owner", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Name: "testname", - Owner: "testowner", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed when an ami selector term has an owner key with tags", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Owner: "testowner", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail when a ami selector term has no values", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - {}, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a ami selector term has no tag map values", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{}, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a ami selector term has a tag map key that is empty", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{ - "test": "", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a ami selector term has a tag map value that is empty", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{ - "": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when the last ami selector is invalid", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - { - Tags: map[string]string{ - "test2": "testvalue2", - }, - }, - { - Tags: map[string]string{ - "test3": "testvalue3", - }, - }, - { - Tags: map[string]string{ - "": "testvalue4", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with tags", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - ID: "ami-12345749", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with name", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - ID: "ami-12345749", - Name: "my-custom-ami", - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with owner", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - ID: "ami-12345749", - Owner: "123456789", - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - }) - Context("BlockDeviceMappings", func() { - It("should fail if more than one root volume is specified", func() { - nodeClass := test.EC2NodeClass(v1beta1.EC2NodeClass{ - Spec: v1beta1.EC2NodeClassSpec{ - BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{ - { - DeviceName: aws.String("map-device-1"), - EBS: &v1beta1.BlockDevice{ - VolumeSize: resource.NewScaledQuantity(50, resource.Giga), - }, - - RootVolume: true, - }, - { - DeviceName: aws.String("map-device-2"), - EBS: &v1beta1.BlockDevice{ - VolumeSize: resource.NewScaledQuantity(50, resource.Giga), - }, - - RootVolume: true, - }, - }, - }, - }) - Expect(nodeClass.Validate(ctx)).To(Not(Succeed())) - }) - }) - Context("Role Immutability", func() { - It("should fail when updating the role", func() { - nc.Spec.Role = "test-role" - Expect(nc.Validate(ctx)).To(Succeed()) - - updateCtx := apis.WithinUpdate(ctx, nc.DeepCopy()) - nc.Spec.Role = "test-role2" - Expect(nc.Validate(updateCtx)).ToNot(Succeed()) - }) - It("should fail to switch between an unmanaged and managed instance profile", func() { - nc.Spec.Role = "" - nc.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - Expect(nc.Validate(ctx)).To(Succeed()) - - updateCtx := apis.WithinUpdate(ctx, nc.DeepCopy()) - nc.Spec.Role = "test-role" - nc.Spec.InstanceProfile = nil - Expect(nc.Validate(updateCtx)).ToNot(Succeed()) - }) - It("should fail to switch between a managed and unmanaged instance profile", func() { - nc.Spec.Role = "test-role" - nc.Spec.InstanceProfile = nil - Expect(nc.Validate(ctx)).To(Succeed()) - - updateCtx := apis.WithinUpdate(ctx, nc.DeepCopy()) - nc.Spec.Role = "" - nc.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - Expect(nc.Validate(updateCtx)).ToNot(Succeed()) - }) - }) -}) diff --git a/pkg/webhooks/webhooks.go b/pkg/webhooks/webhooks.go index 447a574ab4d5..95b08b2e8581 100644 --- a/pkg/webhooks/webhooks.go +++ b/pkg/webhooks/webhooks.go @@ -21,10 +21,7 @@ import ( "knative.dev/pkg/configmap" "knative.dev/pkg/controller" knativeinjection "knative.dev/pkg/injection" - "knative.dev/pkg/webhook/resourcesemantics" "knative.dev/pkg/webhook/resourcesemantics/conversion" - "knative.dev/pkg/webhook/resourcesemantics/defaulting" - "knative.dev/pkg/webhook/resourcesemantics/validation" "github.com/awslabs/operatorpkg/object" @@ -33,9 +30,6 @@ import ( ) var ( - Resources = map[schema.GroupVersionKind]resourcesemantics.GenericCRD{ - object.GVK(&v1beta1.EC2NodeClass{}): &v1beta1.EC2NodeClass{}, - } ConversionResource = map[schema.GroupKind]conversion.GroupKindConversion{ object.GVK(&v1.EC2NodeClass{}).GroupKind(): { DefinitionName: "ec2nodeclasses.karpenter.k8s.aws", @@ -50,34 +44,13 @@ var ( func NewWebhooks() []knativeinjection.ControllerConstructor { return []knativeinjection.ControllerConstructor{ - NewCRDDefaultingWebhook, - NewCRDValidationWebhook, + NewCRDConversionWebhook, } } -func NewCRDDefaultingWebhook(ctx context.Context, _ configmap.Watcher) *controller.Impl { - return defaulting.NewAdmissionController(ctx, - "defaulting.webhook.karpenter.k8s.aws", - "/default/karpenter.k8s.aws", - Resources, - func(ctx context.Context) context.Context { return ctx }, - true, - ) -} - -func NewCRDValidationWebhook(ctx context.Context, _ configmap.Watcher) *controller.Impl { - return validation.NewAdmissionController(ctx, - "validation.webhook.karpenter.k8s.aws", - "/validate/karpenter.k8s.aws", - Resources, - func(ctx context.Context) context.Context { return ctx }, - true, - ) -} - func NewCRDConversionWebhook(ctx context.Context, _ configmap.Watcher) *controller.Impl { return conversion.NewConversionController(ctx, - "/conversion/karpenter.sh", + "/conversion/karpenter.k8s.aws", ConversionResource, func(ctx context.Context) context.Context { return ctx }, )