Skip to content

Commit

Permalink
feat: CEL validation for provider-specific labels/requirements (#274)
Browse files Browse the repository at this point in the history
* chore: use vmname-nic for network interface names

* chore: add CEL validation for provider labels/requirements

* chore: make presubmit

* chore: makefile herlper for deleting CRDs

* Revert "chore: use vmname-nic for network interface names"

This reverts commit 3ebebe1.

* doc: add regex comment as requested

* fix: remove topology.kubernetes.io/zone from allowed labels and requirements
(until the continuous drift it causes is fixed)

* chore: add better error handling
  • Loading branch information
tallaxes authored Apr 23, 2024
1 parent 00e5964 commit a21caab
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 3 deletions.
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ verify: toolchain tidy download ## Verify code. Includes dependencies, linting,
cp $(KARPENTER_CORE_DIR)/pkg/apis/crds/* pkg/apis/crds
yq -i '(.spec.versions[0].additionalPrinterColumns[] | select (.name=="Zone")) .jsonPath=".metadata.labels.karpenter\.azure\.com/zone"' \
pkg/apis/crds/karpenter.sh_nodeclaims.yaml
hack/validation/labels.sh
hack/validation/requirements.sh
hack/validation/common.sh
hack/github/dependabot.sh
$(foreach dir,$(MOD_DIRS),cd $(dir) && golangci-lint run $(newline))
@git diff --quiet ||\
Expand Down
3 changes: 3 additions & 0 deletions Makefile-az.mk
Original file line number Diff line number Diff line change
Expand Up @@ -324,3 +324,6 @@ az-helm-install-snapshot: az-configure-values ## Install Karpenter snapshot rele
--set controller.resources.limits.cpu=1 \
--set controller.resources.limits.memory=1Gi \
--wait

az-rmcrds: ## Delete Karpenter CRDs
kubectl delete crd nodepools.karpenter.sh nodeclaims.karpenter.sh aksnodeclasses.karpenter.azure.com
7 changes: 7 additions & 0 deletions hack/validation/common.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env bash
set -euo pipefail

# remove topology.kubernetes.io/zone from allowed labels and requirements
# until the continuous drift it causes is fixed
sed -e 's|"topology.kubernetes.io/zone", ||g' -i pkg/apis/crds/karpenter.sh_nodeclaims.yaml
sed -e 's|"topology.kubernetes.io/zone", ||g' -i pkg/apis/crds/karpenter.sh_nodepools.yaml
35 changes: 35 additions & 0 deletions hack/validation/labels.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#!/bin/bash
set -euo pipefail

# labels validation for nodepool
# checking for restricted labels while filtering out well known labels

rule=$'self.all(x, x in
[
"karpenter.azure.com/sku-name",
"karpenter.azure.com/sku-family",
"karpenter.azure.com/sku-version",
"karpenter.azure.com/sku-cpu",
"karpenter.azure.com/sku-memory",
"karpenter.azure.com/sku-accelerator",
"karpenter.azure.com/sku-networking-accelerated",
"karpenter.azure.com/sku-storage-premium-capable",
"karpenter.azure.com/sku-storage-ephemeralos-maxsize",
"karpenter.azure.com/sku-encryptionathost-capable",
"karpenter.azure.com/sku-gpu-name",
"karpenter.azure.com/sku-gpu-manufacturer",
"karpenter.azure.com/sku-gpu-count"
]
|| !x.find("^([^/]+)").endsWith("karpenter.azure.com")
)
'
# above regex: everything before the first '/' (any characters except '/' at the beginning of the string)

rule=${rule//\"/\\\"} # escape double quotes
rule=${rule//$'\n'/} # remove newlines
rule=$(echo "$rule" | tr -s ' ') # remove extra spaces

# nodepool
printf -v expr '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.metadata.properties.labels.x-kubernetes-validations +=
[{"message": "label domain \\"karpenter.azure.com\\" is restricted", "rule": "%s"}]' "$rule"
yq eval "${expr}" -i pkg/apis/crds/karpenter.sh_nodepools.yaml
39 changes: 39 additions & 0 deletions hack/validation/requirements.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#!/bin/bash
set -euo pipefail

# requirements validation for nodeclaim and nodepool
# checking for restricted labels while filtering out well known labels

rule=$'self in
[
"karpenter.azure.com/sku-name",
"karpenter.azure.com/sku-family",
"karpenter.azure.com/sku-version",
"karpenter.azure.com/sku-cpu",
"karpenter.azure.com/sku-memory",
"karpenter.azure.com/sku-accelerator",
"karpenter.azure.com/sku-networking-accelerated",
"karpenter.azure.com/sku-storage-premium-capable",
"karpenter.azure.com/sku-storage-ephemeralos-maxsize",
"karpenter.azure.com/sku-encryptionathost-capable",
"karpenter.azure.com/sku-gpu-name",
"karpenter.azure.com/sku-gpu-manufacturer",
"karpenter.azure.com/sku-gpu-count"
]
|| !self.find("^([^/]+)").endsWith("karpenter.azure.com")
'
# above regex: everything before the first '/' (any characters except '/' at the beginning of the string)

rule=${rule//\"/\\\"} # escape double quotes
rule=${rule//$'\n'/} # remove newlines
rule=$(echo "$rule" | tr -s ' ') # remove extra spaces

# nodeclaim
printf -v expr '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.key.x-kubernetes-validations +=
[{"message": "label domain \\"karpenter.azure.com\\" is restricted", "rule": "%s"}]' "$rule"
yq eval "${expr}" -i pkg/apis/crds/karpenter.sh_nodeclaims.yaml

# nodepool
printf -v expr '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.requirements.items.properties.key.x-kubernetes-validations +=
[{"message": "label domain \\"karpenter.azure.com\\" is restricted", "rule": "%s"}]' "$rule"
yq eval "${expr}" -i pkg/apis/crds/karpenter.sh_nodepools.yaml
4 changes: 3 additions & 1 deletion pkg/apis/crds/karpenter.sh_nodeclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -212,13 +212,15 @@ spec:
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(\/))?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$
x-kubernetes-validations:
- message: label domain "kubernetes.io" is restricted
rule: self in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || self.find("^([^/]+)").endsWith("node.kubernetes.io") || self.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !self.find("^([^/]+)").endsWith("kubernetes.io")
rule: self in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || self.find("^([^/]+)").endsWith("node.kubernetes.io") || self.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !self.find("^([^/]+)").endsWith("kubernetes.io")
- message: label domain "k8s.io" is restricted
rule: self.find("^([^/]+)").endsWith("kops.k8s.io") || !self.find("^([^/]+)").endsWith("k8s.io")
- message: label domain "karpenter.sh" is restricted
rule: self in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !self.find("^([^/]+)").endsWith("karpenter.sh")
- message: label "kubernetes.io/hostname" is restricted
rule: self != "kubernetes.io/hostname"
- message: label domain "karpenter.azure.com" is restricted
rule: self in [ "karpenter.azure.com/sku-name", "karpenter.azure.com/sku-family", "karpenter.azure.com/sku-version", "karpenter.azure.com/sku-cpu", "karpenter.azure.com/sku-memory", "karpenter.azure.com/sku-accelerator", "karpenter.azure.com/sku-networking-accelerated", "karpenter.azure.com/sku-storage-premium-capable", "karpenter.azure.com/sku-storage-ephemeralos-maxsize", "karpenter.azure.com/sku-encryptionathost-capable", "karpenter.azure.com/sku-gpu-name", "karpenter.azure.com/sku-gpu-manufacturer", "karpenter.azure.com/sku-gpu-count" ] || !self.find("^([^/]+)").endsWith("karpenter.azure.com")
minValues:
description: |-
This field is ALPHA and can be dropped or replaced at any time
Expand Down
8 changes: 6 additions & 2 deletions pkg/apis/crds/karpenter.sh_nodepools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ spec:
maxProperties: 100
x-kubernetes-validations:
- message: label domain "kubernetes.io" is restricted
rule: self.all(x, x in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/zone", "topology.kubernetes.io/region", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || x.find("^([^/]+)").endsWith("node.kubernetes.io") || x.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !x.find("^([^/]+)").endsWith("kubernetes.io"))
rule: self.all(x, x in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/region", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || x.find("^([^/]+)").endsWith("node.kubernetes.io") || x.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !x.find("^([^/]+)").endsWith("kubernetes.io"))
- message: label domain "k8s.io" is restricted
rule: self.all(x, x.find("^([^/]+)").endsWith("kops.k8s.io") || !x.find("^([^/]+)").endsWith("k8s.io"))
- message: label domain "karpenter.sh" is restricted
Expand All @@ -189,6 +189,8 @@ spec:
rule: self.all(x, x != "karpenter.sh/nodepool")
- message: label "kubernetes.io/hostname" is restricted
rule: self.all(x, x != "kubernetes.io/hostname")
- message: label domain "karpenter.azure.com" is restricted
rule: self.all(x, x in [ "karpenter.azure.com/sku-name", "karpenter.azure.com/sku-family", "karpenter.azure.com/sku-version", "karpenter.azure.com/sku-cpu", "karpenter.azure.com/sku-memory", "karpenter.azure.com/sku-accelerator", "karpenter.azure.com/sku-networking-accelerated", "karpenter.azure.com/sku-storage-premium-capable", "karpenter.azure.com/sku-storage-ephemeralos-maxsize", "karpenter.azure.com/sku-encryptionathost-capable", "karpenter.azure.com/sku-gpu-name", "karpenter.azure.com/sku-gpu-manufacturer", "karpenter.azure.com/sku-gpu-count" ] || !x.find("^([^/]+)").endsWith("karpenter.azure.com"))
type: object
spec:
description: NodeClaimSpec describes the desired state of the NodeClaim
Expand Down Expand Up @@ -336,7 +338,7 @@ spec:
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(\/))?([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$
x-kubernetes-validations:
- message: label domain "kubernetes.io" is restricted
rule: self in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || self.find("^([^/]+)").endsWith("node.kubernetes.io") || self.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !self.find("^([^/]+)").endsWith("kubernetes.io")
rule: self in ["beta.kubernetes.io/instance-type", "failure-domain.beta.kubernetes.io/region", "beta.kubernetes.io/os", "beta.kubernetes.io/arch", "failure-domain.beta.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os", "node.kubernetes.io/windows-build"] || self.find("^([^/]+)").endsWith("node.kubernetes.io") || self.find("^([^/]+)").endsWith("node-restriction.kubernetes.io") || !self.find("^([^/]+)").endsWith("kubernetes.io")
- message: label domain "k8s.io" is restricted
rule: self.find("^([^/]+)").endsWith("kops.k8s.io") || !self.find("^([^/]+)").endsWith("k8s.io")
- message: label domain "karpenter.sh" is restricted
Expand All @@ -345,6 +347,8 @@ spec:
rule: self != "karpenter.sh/nodepool"
- message: label "kubernetes.io/hostname" is restricted
rule: self != "kubernetes.io/hostname"
- message: label domain "karpenter.azure.com" is restricted
rule: self in [ "karpenter.azure.com/sku-name", "karpenter.azure.com/sku-family", "karpenter.azure.com/sku-version", "karpenter.azure.com/sku-cpu", "karpenter.azure.com/sku-memory", "karpenter.azure.com/sku-accelerator", "karpenter.azure.com/sku-networking-accelerated", "karpenter.azure.com/sku-storage-premium-capable", "karpenter.azure.com/sku-storage-ephemeralos-maxsize", "karpenter.azure.com/sku-encryptionathost-capable", "karpenter.azure.com/sku-gpu-name", "karpenter.azure.com/sku-gpu-manufacturer", "karpenter.azure.com/sku-gpu-count" ] || !self.find("^([^/]+)").endsWith("karpenter.azure.com")
minValues:
description: |-
This field is ALPHA and can be dropped or replaced at any time
Expand Down

0 comments on commit a21caab

Please sign in to comment.