From c0ea7240bc29546ede0ce420bc1904052d316973 Mon Sep 17 00:00:00 2001 From: Jonathan Innis Date: Wed, 23 Aug 2023 18:03:53 -0700 Subject: [PATCH] Allow `node-restriction.kubernetes.io/` prefix in the label set (#4488) --- go.mod | 2 +- go.sum | 4 +-- .../launchtemplate/launchtemplate.go | 9 ++++++ pkg/providers/launchtemplate/suite_test.go | 22 ++++++++++++++ test/suites/integration/scheduling_test.go | 29 +++++++++++++++++++ test/suites/integration/webhook_test.go | 8 +++++ 6 files changed, 71 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 316613d157dc..8757caaeee15 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/PuerkitoBio/goquery v1.8.1 github.com/avast/retry-go v3.0.0+incompatible github.com/aws/aws-sdk-go v1.44.328 - github.com/aws/karpenter-core v0.30.0-rc.0.0.20230822175121-c71cf73b5a52 + github.com/aws/karpenter-core v0.30.0-rc.0.0.20230823194033-acb9571f6da5 github.com/go-playground/validator/v10 v10.15.1 github.com/imdario/mergo v0.3.16 github.com/mitchellh/hashstructure/v2 v2.0.2 diff --git a/go.sum b/go.sum index 4e38df401706..b704391f1b9d 100644 --- a/go.sum +++ b/go.sum @@ -53,8 +53,8 @@ github.com/avast/retry-go v3.0.0+incompatible h1:4SOWQ7Qs+oroOTQOYnAHqelpCO0biHS github.com/avast/retry-go v3.0.0+incompatible/go.mod h1:XtSnn+n/sHqQIpZ10K1qAevBhOOCWBLXXy3hyiqqBrY= github.com/aws/aws-sdk-go v1.44.328 h1:WBwlf8ym9SDQ/GTIBO9eXyvwappKJyOetWJKl4mT7ZU= github.com/aws/aws-sdk-go v1.44.328/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI= -github.com/aws/karpenter-core v0.30.0-rc.0.0.20230822175121-c71cf73b5a52 h1:vjppWOFi+YWrtNE3KhKka/2YLMet9obGiq/dsikxDJQ= -github.com/aws/karpenter-core v0.30.0-rc.0.0.20230822175121-c71cf73b5a52/go.mod h1:AQl8m8OtgO2N8IlZlzAU6MTrJTJSbe6K4GwdRUNSJVc= +github.com/aws/karpenter-core v0.30.0-rc.0.0.20230823194033-acb9571f6da5 h1:zRYWS4cpDPfE0rV+/hiwJWbR6DrSchJA/gPmdO92Zko= +github.com/aws/karpenter-core v0.30.0-rc.0.0.20230823194033-acb9571f6da5/go.mod h1:AQl8m8OtgO2N8IlZlzAU6MTrJTJSbe6K4GwdRUNSJVc= github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A= github.com/benbjohnson/clock v1.3.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go index 60a252cabf32..5a654045f8bd 100644 --- a/pkg/providers/launchtemplate/launchtemplate.go +++ b/pkg/providers/launchtemplate/launchtemplate.go @@ -20,6 +20,7 @@ import ( "fmt" "math" "net" + "strings" "sync" "time" @@ -29,6 +30,7 @@ import ( "github.com/mitchellh/hashstructure/v2" "github.com/patrickmn/go-cache" "github.com/samber/lo" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "knative.dev/pkg/logging" "knative.dev/pkg/ptr" @@ -139,6 +141,13 @@ func launchTemplateName(options *amifamily.LaunchTemplate) string { } func (p *Provider) createAMIOptions(ctx context.Context, nodeTemplate *v1alpha1.AWSNodeTemplate, labels, tags map[string]string) (*amifamily.Options, error) { + // Remove any labels passed into userData that are prefixed with "node-restriction.kubernetes.io" since the kubelet can't + // register the node with any labels from this domain: https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#noderestriction + for k := range labels { + if strings.HasPrefix(k, v1.LabelNamespaceNodeRestriction) { + delete(labels, k) + } + } instanceProfile, err := p.getInstanceProfile(ctx, nodeTemplate) if err != nil { return nil, err diff --git a/pkg/providers/launchtemplate/suite_test.go b/pkg/providers/launchtemplate/suite_test.go index 43ba2ccd05ec..aca4b6baa692 100644 --- a/pkg/providers/launchtemplate/suite_test.go +++ b/pkg/providers/launchtemplate/suite_test.go @@ -1132,6 +1132,17 @@ var _ = Describe("LaunchTemplates", func() { ExpectScheduled(ctx, env.Client, pod) ExpectLaunchTemplatesCreatedWithUserDataContaining("--cpu-cfs-quota=false") }) + It("should not pass any labels prefixed with the node-restriction.kubernetes.io domain", func() { + provisioner.Spec.Labels = lo.Assign(provisioner.Spec.Labels, map[string]string{ + v1.LabelNamespaceNodeRestriction + "/team": "team-1", + v1.LabelNamespaceNodeRestriction + "/custom-label": "custom-value", + }) + ExpectApplied(ctx, env.Client, provisioner, nodeTemplate) + pod := coretest.UnschedulablePod() + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) + ExpectScheduled(ctx, env.Client, pod) + ExpectLaunchTemplatesCreatedWithUserDataNotContaining(v1.LabelNamespaceNodeRestriction) + }) Context("Bottlerocket", func() { It("should merge in custom user data", func() { ctx = settings.ToContext(ctx, test.Settings(test.SettingOptions{ @@ -1795,6 +1806,17 @@ func ExpectLaunchTemplatesCreatedWithUserDataContaining(substrings ...string) { }) } +func ExpectLaunchTemplatesCreatedWithUserDataNotContaining(substrings ...string) { + Expect(awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.Len()).To(BeNumerically(">=", 1)) + awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.ForEach(func(input *ec2.CreateLaunchTemplateInput) { + userData, err := base64.StdEncoding.DecodeString(*input.LaunchTemplateData.UserData) + Expect(err).To(BeNil()) + for _, substring := range substrings { + Expect(string(userData)).ToNot(ContainSubstring(substring)) + } + }) +} + func ExpectLaunchTemplatesCreatedWithUserData(expected string) { Expect(awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.Len()).To(BeNumerically(">=", 1)) awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.ForEach(func(input *ec2.CreateLaunchTemplateInput) { diff --git a/test/suites/integration/scheduling_test.go b/test/suites/integration/scheduling_test.go index 174beee4ad5b..72dd94c75ff4 100644 --- a/test/suites/integration/scheduling_test.go +++ b/test/suites/integration/scheduling_test.go @@ -266,6 +266,35 @@ var _ = Describe("Scheduling", Ordered, ContinueOnFailure, func() { env.EventuallyExpectHealthyPodCountWithTimeout(time.Minute*15, labels.SelectorFromSet(deployment.Spec.Selector.MatchLabels), int(*deployment.Spec.Replicas)) env.ExpectCreatedNodeCount("==", 1) }) + It("should support the node-restriction.kubernetes.io label domain", func() { + // Assign labels to the provisioner so that it has known values + provisioner.Spec.Requirements = []v1.NodeSelectorRequirement{ + { + Key: v1.LabelNamespaceNodeRestriction + "/team", + Operator: v1.NodeSelectorOpExists, + }, + { + Key: v1.LabelNamespaceNodeRestriction + "/custom-label", + Operator: v1.NodeSelectorOpExists, + }, + } + nodeSelector := map[string]string{ + v1.LabelNamespaceNodeRestriction + "/team": "team-1", + v1.LabelNamespaceNodeRestriction + "/custom-label": "custom-value", + } + selectors.Insert(lo.Keys(nodeSelector)...) // Add node selector keys to selectors used in testing to ensure we test all labels + requirements := lo.MapToSlice(nodeSelector, func(key string, value string) v1.NodeSelectorRequirement { + return v1.NodeSelectorRequirement{Key: key, Operator: v1.NodeSelectorOpIn, Values: []string{value}} + }) + deployment := test.Deployment(test.DeploymentOptions{Replicas: 1, PodOptions: test.PodOptions{ + NodeSelector: nodeSelector, + NodePreferences: requirements, + NodeRequirements: requirements, + }}) + env.ExpectCreated(provisioner, provider, deployment) + env.EventuallyExpectHealthyPodCount(labels.SelectorFromSet(deployment.Spec.Selector.MatchLabels), int(*deployment.Spec.Replicas)) + env.ExpectCreatedNodeCount("==", 1) + }) It("should provision a node for naked pods", func() { pod := test.Pod() diff --git a/test/suites/integration/webhook_test.go b/test/suites/integration/webhook_test.go index 731de3ad43c7..a01e8b42e1a6 100644 --- a/test/suites/integration/webhook_test.go +++ b/test/suites/integration/webhook_test.go @@ -155,6 +155,14 @@ var _ = Describe("Webhooks", func() { }, }))).ToNot(Succeed()) }) + It("should allow a restricted label exception to be used in labels (node-restriction.kubernetes.io/custom-label)", func() { + Expect(env.Client.Create(env, test.Provisioner(test.ProvisionerOptions{ + ProviderRef: &v1alpha5.MachineTemplateRef{Name: "test"}, + Labels: map[string]string{ + v1.LabelNamespaceNodeRestriction + "/custom-label": "custom-value", + }, + }))).To(Succeed()) + }) It("should error when a requirement references a restricted label (karpenter.sh/provisioner-name)", func() { Expect(env.Client.Create(env, test.Provisioner(test.ProvisionerOptions{ ProviderRef: &v1alpha5.MachineTemplateRef{Name: "test"},