Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Decouple label propagation #1908

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions hack/validation/labels.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
# checking for restricted labels while filtering out well-known labels
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.metadata.properties.labels.maxProperties = 100' -i pkg/apis/crds/karpenter.sh_nodepools.yaml
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.metadata.properties.labels.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\"))"},
{"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", "rule": "self.all(x, x in [\"karpenter.sh/capacity-type\", \"karpenter.sh/nodepool\"] || !x.find(\"^([^/]+)\").endsWith(\"karpenter.sh\"))"},
{"message": "label \"karpenter.sh/nodepool\" is restricted", "rule": "self.all(x, x != \"karpenter.sh/nodepool\")"},
{"message": "label \"kubernetes.io/hostname\" is restricted", "rule": "self.all(x, x != \"kubernetes.io/hostname\")"}]' -i pkg/apis/crds/karpenter.sh_nodepools.yaml
Expand Down
4 changes: 0 additions & 4 deletions hack/validation/requirements.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.req
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.requirements.items.properties.key.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]$"' -i pkg/apis/crds/karpenter.sh_nodeclaims.yaml
## 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 \"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\")"},
{"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\""}]' -i pkg/apis/crds/karpenter.sh_nodeclaims.yaml
## operator enum values
Expand All @@ -21,8 +19,6 @@ yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.tem
yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.requirements.items.properties.key.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]$"' -i pkg/apis/crds/karpenter.sh_nodepools.yaml
## 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 \"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\")"},
{"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 \"karpenter.sh/nodepool\" is restricted", "rule": "self != \"karpenter.sh/nodepool\""},
{"message": "label \"kubernetes.io/hostname\" is restricted", "rule": "self != \"kubernetes.io/hostname\""}]' -i pkg/apis/crds/karpenter.sh_nodepools.yaml
Expand Down
4 changes: 0 additions & 4 deletions kwok/charts/crds/karpenter.sh_nodeclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,6 @@ spec:
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 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")
- 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
Expand Down
8 changes: 0 additions & 8 deletions kwok/charts/crds/karpenter.sh_nodepools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,6 @@ spec:
type: object
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"))
- 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
rule: self.all(x, x in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !x.find("^([^/]+)").endsWith("karpenter.sh"))
- message: label "karpenter.sh/nodepool" is restricted
Expand Down Expand Up @@ -267,10 +263,6 @@ spec:
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 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")
- 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 "karpenter.sh/nodepool" is restricted
Expand Down
4 changes: 0 additions & 4 deletions pkg/apis/crds/karpenter.sh_nodeclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,6 @@ spec:
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 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")
- 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
Expand Down
8 changes: 0 additions & 8 deletions pkg/apis/crds/karpenter.sh_nodepools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,6 @@ spec:
type: object
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"))
- 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
rule: self.all(x, x in ["karpenter.sh/capacity-type", "karpenter.sh/nodepool"] || !x.find("^([^/]+)").endsWith("karpenter.sh"))
- message: label "karpenter.sh/nodepool" is restricted
Expand Down Expand Up @@ -267,10 +263,6 @@ spec:
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 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")
- 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 "karpenter.sh/nodepool" is restricted
Expand Down
77 changes: 53 additions & 24 deletions pkg/apis/v1/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,17 @@ const (
)

var (
// RestrictedLabelDomains are either prohibited by the kubelet or reserved by karpenter
// RestrictedLabelDomains are reserved by karpenter
RestrictedLabelDomains = sets.New(
"kubernetes.io",
"k8s.io",
apis.Group,
)

// LabelDomainExceptions are sub-domains of the RestrictedLabelDomains but allowed because
// they are not used in a context where they may be passed as argument to kubelet.
LabelDomainExceptions = sets.New(
"kops.k8s.io",
v1.LabelNamespaceSuffixNode,
v1.LabelNamespaceNodeRestriction,
K8sLabelDomains = sets.New(
"kubernetes.io",
"k8s.io",
)

// WellKnownLabels are labels that belong to the RestrictedLabelDomains but allowed.
// WellKnownLabels are labels that belong to the RestrictedLabelDomains or K8sLabelDomains but allowed.
// Karpenter is aware of these labels, and they can be used to further narrow down
// the range of the corresponding values by either nodepool or pods.
WellKnownLabels = sets.New(
Expand Down Expand Up @@ -104,38 +99,72 @@ var (
}
)

// IsRestrictedLabel returns an error if the label is restricted.
// IsRestrictedLabel is used for runtime validation of requirements.
// Returns an error if the label is restricted. E.g. using .karpenter.sh suffix.
func IsRestrictedLabel(key string) error {
if WellKnownLabels.Has(key) {
return nil
}
if IsRestrictedNodeLabel(key) {
return fmt.Errorf("label %s is restricted; specify a well known label: %v, or a custom label that does not use a restricted domain: %v", key, sets.List(WellKnownLabels), sets.List(RestrictedLabelDomains))

labelDomain := GetLabelDomain(key)
for restrictedLabelDomain := range RestrictedLabelDomains {
if labelDomain == restrictedLabelDomain || strings.HasSuffix(labelDomain, "."+restrictedLabelDomain) {
return fmt.Errorf("using label %s is not allowed as it might interfere with the internal provisioning logic; specify a well known label: %v, or a custom label that does not use a restricted domain: %v", key, sets.List(WellKnownLabels), sets.List(RestrictedLabelDomains))
}
}

if RestrictedLabels.Has(key) {
return fmt.Errorf("using label %s is not allowed as it might interfere with the internal provisioning logic; specify a well known label: %v, or a custom label that does not use a restricted domain: %v", key, sets.List(WellKnownLabels), sets.List(RestrictedLabelDomains))
}

return nil
}

// IsRestrictedNodeLabel returns true if a node label should not be injected by Karpenter.
// They are either known labels that will be injected by cloud providers,
// or label domain managed by other software (e.g., kops.k8s.io managed by kOps).
func IsRestrictedNodeLabel(key string) bool {
// IsValidLabelToSync returns true if the label key is allowed to be synced to the Node object centrally by Karpenter.
func IsValidToSyncCentrallyLabel(key string) bool {
// TODO(enxebre): consider this to be configurable with runtime flag.
notValidToSyncLabel := WellKnownLabels

return !notValidToSyncLabel.Has(key)
}

// IsKubeletLabel returns true if the label key is one that kubelets are allowed to set on their own Node object.
// This function is similar the one used by the node restriction admission https://github.com/kubernetes/kubernetes/blob/e319c541f144e9bee6160f1dd8671638a9029f4c/staging/src/k8s.io/kubelet/pkg/apis/well_known_labels.go#L67
// but karpenter also restricts the known labels to be passed to kubelet. Only the kubeletLabelNamespaces are allowed.
func IsKubeletLabel(key string) bool {
if WellKnownLabels.Has(key) {
return false
}

if !isKubernetesLabel(key) {
return true
}
labelDomain := GetLabelDomain(key)
for exceptionLabelDomain := range LabelDomainExceptions {
if strings.HasSuffix(labelDomain, exceptionLabelDomain) {
return false

namespace := GetLabelDomain(key)
for allowedNamespace := range kubeletLabelNamespaces {
if namespace == allowedNamespace || strings.HasSuffix(namespace, "."+allowedNamespace) {
return true
}
}
for restrictedLabelDomain := range RestrictedLabelDomains {
if strings.HasSuffix(labelDomain, restrictedLabelDomain) {

return false
}

func isKubernetesLabel(key string) bool {
for k8sDomain := range K8sLabelDomains {
if key == k8sDomain || strings.HasSuffix(key, "."+k8sDomain) {
return true
}
}
return RestrictedLabels.Has(key)

return false
}

var kubeletLabelNamespaces = sets.NewString(
v1.LabelNamespaceSuffixKubelet,
v1.LabelNamespaceSuffixNode,
)

func GetLabelDomain(key string) string {
if parts := strings.SplitN(key, "/", 2); len(parts) == 2 {
return parts[0]
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/v1/nodeclaim_validation_cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,9 @@ var _ = Describe("Validation", func() {
Expect(env.Client.Create(ctx, nodeClaim)).ToNot(Succeed())
}
})
It("should allow restricted domains exceptions", func() {
It("should allow kubernetes domains", func() {
oldNodeClaim := nodeClaim.DeepCopy()
for label := range LabelDomainExceptions {
for label := range K8sLabelDomains {
nodeClaim.Spec.Requirements = []NodeSelectorRequirementWithMinValues{
{NodeSelectorRequirement: v1.NodeSelectorRequirement{Key: label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}},
}
Expand All @@ -152,9 +152,9 @@ var _ = Describe("Validation", func() {
nodeClaim = oldNodeClaim.DeepCopy()
}
})
It("should allow restricted subdomains exceptions", func() {
It("should allow kubernetes subdomains", func() {
oldNodeClaim := nodeClaim.DeepCopy()
for label := range LabelDomainExceptions {
for label := range K8sLabelDomains {
nodeClaim.Spec.Requirements = []NodeSelectorRequirementWithMinValues{
{NodeSelectorRequirement: v1.NodeSelectorRequirement{Key: "subdomain." + label + "/test", Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}},
}
Expand Down
Loading
Loading