Skip to content

Commit

Permalink
CEL Validation
Browse files Browse the repository at this point in the history
  • Loading branch information
engedaam committed Oct 23, 2023
1 parent 2529674 commit 63296c7
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 0 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ verify: tidy download ## Verify code. Includes dependencies, linting, formatting
go generate ./...
hack/boilerplate.sh
cp $(KARPENTER_CORE_DIR)/pkg/apis/crds/* pkg/apis/crds
hack/validation/requirements.sh
$(foreach dir,$(MOD_DIRS),cd $(dir) && golangci-lint run $(newline))
@git diff --quiet ||\
{ echo "New file modification detected in the Git working tree. Please check in before commit."; git --no-pager diff --name-only | uniq | awk '{print " - " $$0}'; \
Expand Down
13 changes: 13 additions & 0 deletions hack/validation/requirements.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Requirements Validation

# Adding validation for nodeclaim

## checking for restricted labels while filtering out well known labels
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.key.x-kubernetes-validations += [
{"message": "label domain \"karpenter.k8s.aws\" is restricted", "rule": "self.find(\"^([^/]+)\").endsWith(\"karpenter.k8s.aws\") ? self in [\"karpenter.k8s.aws/instance-encryption-in-transit-supported\", \"karpenter.k8s.aws/instance-category\", \"karpenter.k8s.aws/instance-hypervisor\", \"karpenter.k8s.aws/instance-family\", \"karpenter.k8s.aws/instance-generation\", \"karpenter.k8s.aws/instance-local-nvme\", \"karpenter.k8s.aws/instance-size\", \"karpenter.k8s.aws/instance-cpu\",\"karpenter.k8s.aws/instance-memory\", \"karpenter.k8s.aws/instance-network-bandwidth\", \"karpenter.k8s.aws/instance-pods\", \"karpenter.k8s.aws/instance-gpu-name\", \"karpenter.k8s.aws/instance-gpu-manufacturer\", \"karpenter.k8s.aws/instance-gpu-count\", \"karpenter.k8s.aws/instance-gpu-memory\", \"karpenter.k8s.aws/instance-accelerator-name\", \"karpenter.k8s.aws/instance-accelerator-manufacturer\", \"karpenter.k8s.aws/instance-accelerator-count\"] : true"}]' -i pkg/apis/crds/karpenter.sh_nodeclaims.yaml

# Adding validation for nodepool

## checking for restricted labels while filtering out well known labels
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.requirements.items.properties.key.x-kubernetes-validations += [
{"message": "label domain \"karpenter.k8s.aws\" is restricted", "rule": "self.find(\"^([^/]+)\").endsWith(\"karpenter.k8s.aws\") ? self in [\"karpenter.k8s.aws/instance-encryption-in-transit-supported\", \"karpenter.k8s.aws/instance-category\", \"karpenter.k8s.aws/instance-hypervisor\", \"karpenter.k8s.aws/instance-family\", \"karpenter.k8s.aws/instance-generation\", \"karpenter.k8s.aws/instance-local-nvme\", \"karpenter.k8s.aws/instance-size\", \"karpenter.k8s.aws/instance-cpu\",\"karpenter.k8s.aws/instance-memory\", \"karpenter.k8s.aws/instance-network-bandwidth\", \"karpenter.k8s.aws/instance-pods\", \"karpenter.k8s.aws/instance-gpu-name\", \"karpenter.k8s.aws/instance-gpu-manufacturer\", \"karpenter.k8s.aws/instance-gpu-count\", \"karpenter.k8s.aws/instance-gpu-memory\", \"karpenter.k8s.aws/instance-accelerator-name\", \"karpenter.k8s.aws/instance-accelerator-manufacturer\", \"karpenter.k8s.aws/instance-accelerator-count\"] : true"}]' -i pkg/apis/crds/karpenter.sh_nodepools.yaml
30 changes: 30 additions & 0 deletions pkg/apis/crds/karpenter.sh_nodeclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -182,19 +182,49 @@ spec:
key:
description: The label key that the selector applies to.
type: string
maxLength: 316
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 == "beta.kubernetes.io/instance-type" || self == "failure-domain.beta.kubernetes.io/region"|| self == "beta.kubernetes.io/os" || self == "beta.kubernetes.io/arch" || self == "failure-domain.beta.kubernetes.io/zone" || self.startsWith("node.kubernetes.io/") || self.startsWith("node-restriction.kubernetes.io/") || (self.find("^([^/]+)").endsWith("kubernetes.io") ? self in ["topology.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os"] : true)'
- message: label domain "k8s.io" is restricted
rule: self.startsWith("kops.k8s.io/") || !self.find("^([^/]+)").endsWith("k8s.io")
- message: label domain "karpenter.sh" is restricted
rule: 'self.find("^([^/]+)").endsWith("karpenter.sh") ? self in ["karpenter.sh/nodepool", "karpenter.sh/capacity-type"] : true'
- message: label "kubernetes.io/hostname" is restricted
rule: self != "kubernetes.io/hostname"
- message: label domain "karpenter.k8s.aws" is restricted
rule: 'self.find("^([^/]+)").endsWith("karpenter.k8s.aws") ? self in ["karpenter.k8s.aws/instance-encryption-in-transit-supported", "karpenter.k8s.aws/instance-category", "karpenter.k8s.aws/instance-hypervisor", "karpenter.k8s.aws/instance-family", "karpenter.k8s.aws/instance-generation", "karpenter.k8s.aws/instance-local-nvme", "karpenter.k8s.aws/instance-size", "karpenter.k8s.aws/instance-cpu","karpenter.k8s.aws/instance-memory", "karpenter.k8s.aws/instance-network-bandwidth", "karpenter.k8s.aws/instance-pods", "karpenter.k8s.aws/instance-gpu-name", "karpenter.k8s.aws/instance-gpu-manufacturer", "karpenter.k8s.aws/instance-gpu-count", "karpenter.k8s.aws/instance-gpu-memory", "karpenter.k8s.aws/instance-accelerator-name", "karpenter.k8s.aws/instance-accelerator-manufacturer", "karpenter.k8s.aws/instance-accelerator-count"] : true'
operator:
description: Represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists, DoesNotExist. Gt, and Lt.
type: string
enum:
- In
- NotIn
- Exists
- DoesNotExist
- Gt
- Lt
values:
description: An array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. If the operator is Gt or Lt, the values array must have a single element, which will be interpreted as an integer. This array is replaced during a strategic merge patch.
items:
type: string
type: array
maxLength: 63
pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$
required:
- key
- operator
type: object
maxItems: 20
type: array
x-kubernetes-validations:
- message: requirements with operator 'In' must have a value defined
rule: 'self.all(x, x.operator == ''In'' ? x.values.size() != 0 : true)'
- message: requirements operator 'Gt' or 'Lt' must have a single value
rule: 'self.all(x, (x.operator == ''Gt'' || x.operator == ''Lt'') ? x.values.size() == 1 : true)'
- message: requirements operator 'Gt' or 'Lt' must have a single positive integer value
rule: 'self.all(x, (x.operator == ''Gt'' || x.operator == ''Lt'') ? int(x.values[0]) >= 0 : true)'
resources:
description: Resources models the resource requirements for the NodeClaim to launch
properties:
Expand Down
32 changes: 32 additions & 0 deletions pkg/apis/crds/karpenter.sh_nodepools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -218,19 +218,51 @@ spec:
key:
description: The label key that the selector applies to.
type: string
maxLength: 316
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 == "beta.kubernetes.io/instance-type" || self == "failure-domain.beta.kubernetes.io/region"|| self == "beta.kubernetes.io/os" || self == "beta.kubernetes.io/arch" || self == "failure-domain.beta.kubernetes.io/zone" || self.startsWith("node.kubernetes.io/") || self.startsWith("node-restriction.kubernetes.io/") || (self.find("^([^/]+)").endsWith("kubernetes.io") ? self in ["topology.kubernetes.io/zone", "topology.kubernetes.io/region", "node.kubernetes.io/instance-type", "kubernetes.io/arch", "kubernetes.io/os"] : true)'
- message: label domain "k8s.io" is restricted
rule: self.startsWith("kops.k8s.io/") || !self.find("^([^/]+)").endsWith("k8s.io")
- message: label domain "karpenter.sh" is restricted
rule: 'self.find("^([^/]+)").endsWith("karpenter.sh") ? self in ["karpenter.sh/nodepool", "karpenter.sh/capacity-type"] : true'
- message: label "karpenter.sh/nodepool" is restricted
rule: self != "karpenter.sh/nodepool"
- message: label "kubernetes.io/hostname" is restricted
rule: self != "kubernetes.io/hostname"
- message: label domain "karpenter.k8s.aws" is restricted
rule: 'self.find("^([^/]+)").endsWith("karpenter.k8s.aws") ? self in ["karpenter.k8s.aws/instance-encryption-in-transit-supported", "karpenter.k8s.aws/instance-category", "karpenter.k8s.aws/instance-hypervisor", "karpenter.k8s.aws/instance-family", "karpenter.k8s.aws/instance-generation", "karpenter.k8s.aws/instance-local-nvme", "karpenter.k8s.aws/instance-size", "karpenter.k8s.aws/instance-cpu","karpenter.k8s.aws/instance-memory", "karpenter.k8s.aws/instance-network-bandwidth", "karpenter.k8s.aws/instance-pods", "karpenter.k8s.aws/instance-gpu-name", "karpenter.k8s.aws/instance-gpu-manufacturer", "karpenter.k8s.aws/instance-gpu-count", "karpenter.k8s.aws/instance-gpu-memory", "karpenter.k8s.aws/instance-accelerator-name", "karpenter.k8s.aws/instance-accelerator-manufacturer", "karpenter.k8s.aws/instance-accelerator-count"] : true'
operator:
description: Represents a key's relationship to a set of values. Valid operators are In, NotIn, Exists, DoesNotExist. Gt, and Lt.
type: string
enum:
- In
- NotIn
- Exists
- DoesNotExist
- Gt
- Lt
values:
description: An array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. If the operator is Gt or Lt, the values array must have a single element, which will be interpreted as an integer. This array is replaced during a strategic merge patch.
items:
type: string
type: array
maxLength: 63
pattern: ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$
required:
- key
- operator
type: object
maxItems: 20
type: array
x-kubernetes-validations:
- message: requirements with operator 'In' must have a value defined
rule: 'self.all(x, x.operator == ''In'' ? x.values.size() != 0 : true)'
- message: requirements operator 'Gt' or 'Lt' must have a single value
rule: 'self.all(x, (x.operator == ''Gt'' || x.operator == ''Lt'') ? x.values.size() == 1 : true)'
- message: requirements operator 'Gt' or 'Lt' must have a single positive integer value
rule: 'self.all(x, (x.operator == ''Gt'' || x.operator == ''Lt'') ? int(x.values[0]) >= 0 : true)'
resources:
description: Resources models the resource requirements for the NodeClaim to launch
properties:
Expand Down
60 changes: 60 additions & 0 deletions pkg/apis/v1beta1/nodepool_validation_cel_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
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/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/aws/karpenter-core/pkg/apis/v1beta1"
)

var _ = Describe("CEL/Validation", func() {
var nodePool *v1beta1.NodePool

BeforeEach(func() {
if env.Version.Minor() < 25 {
Skip("CEL Validation is for 1.25>")
}
})
Context("Requirements", func() {
It("should allow restricted domains exceptions", func() {
oldNodepool := nodePool.DeepCopy()
for label := range v1beta1.LabelDomainExceptions {
nodePool.Spec.Template.Spec.Requirements = []v1.NodeSelectorRequirement{
{Key: label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}},
}
Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
Expect(nodePool.RuntimeValidate()).To(Succeed())
Expect(env.Client.Delete(ctx, nodePool)).To(Succeed())
nodePool = oldNodepool.DeepCopy()
}
})
It("should allow well known label exceptions", func() {
orgNodepool := nodePool.DeepCopy()
for label := range v1beta1.WellKnownLabels.Difference(sets.New(v1beta1.NodePoolLabelKey)) {
nodePool.Spec.Template.Spec.Requirements = []v1.NodeSelectorRequirement{
{Key: label, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}},
}
Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
Expect(nodePool.RuntimeValidate()).To(Succeed())
Expect(env.Client.Delete(ctx, nodePool)).To(Succeed())
nodePool = orgNodepool.DeepCopy()
}
})
})
})

0 comments on commit 63296c7

Please sign in to comment.