Skip to content

Commit 4b1f0fd

Browse files
authored
[pkg/clusteragent/autoscaling] Filter template labels before applying nodepool (#49938)
### What does this PR do? Filter template metadata before creating the NodePool. Any labels/annotations containing keys with `kubernetes.io`/`k8s.io`/`karpenter.sh` are dropped with a warning log ### Motivation Ensure we don't attempt to create a NodePool with restricted template metadata labels/annotations - this ensures NodePools we create are valid and can be applied ### Describe how you validated your changes Unit tests cover this small change ### Additional Notes Co-authored-by: jennifer.chen <jennifer.chen@datadoghq.com>
1 parent 6d6e5fb commit 4b1f0fd

2 files changed

Lines changed: 121 additions & 10 deletions

File tree

pkg/clusteragent/autoscaling/cluster/model/node_pool_internal.go

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
package model
99

1010
import (
11+
"strings"
12+
1113
// AWS Karpenter provider registers some variables in shared packages
1214
_ "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
1315

@@ -26,6 +28,41 @@ const (
2628
KarpenterNodePoolHashAnnotationKey = "karpenter.sh/nodepool-hash"
2729
)
2830

31+
// templateMetadataBlocklistedDomains lists base domains blocked from template_metadata
32+
// labels and annotations.
33+
var templateMetadataBlocklistedDomains = []string{
34+
"kubernetes.io",
35+
"k8s.io",
36+
"karpenter.sh",
37+
}
38+
39+
// filterTemplateMetadataKeys filters out reserved keys and returns a map that can be applied
40+
// to the nodepool template labels or annotations.
41+
func filterTemplateMetadataKeys(kvs []KeyValue) map[string]string {
42+
out := make(map[string]string, len(kvs))
43+
for _, kv := range kvs {
44+
if isBlocklistedTemplateMetadataKey(kv.Key) {
45+
log.Warnf("Dropping template_metadata key %q", kv.Key)
46+
continue
47+
}
48+
out[kv.Key] = kv.Value
49+
}
50+
return out
51+
}
52+
53+
func isBlocklistedTemplateMetadataKey(key string) bool {
54+
domain := karpenterv1.GetLabelDomain(key)
55+
if domain == "" {
56+
return false
57+
}
58+
for _, blocked := range templateMetadataBlocklistedDomains {
59+
if domain == blocked || strings.HasSuffix(domain, "."+blocked) {
60+
return true
61+
}
62+
}
63+
return false
64+
}
65+
2966
type NodePoolInternal struct {
3067
// targetName is the user-created NodePool the Datadog-managed NodePool is derived from
3168
targetName string
@@ -66,17 +103,9 @@ func buildKarpenterNodePoolFromManifest(kv1 *KarpenterV1NodePool) *karpenterv1.N
66103
// Handle template metadata labels/annotations
67104
spec := *kv1.Spec
68105
if kv1.TemplateMetadata != nil {
69-
templateLabels := make(map[string]string, len(kv1.TemplateMetadata.Labels))
70-
for _, kv := range kv1.TemplateMetadata.Labels {
71-
templateLabels[kv.Key] = kv.Value
72-
}
73-
templateAnnotations := make(map[string]string, len(kv1.TemplateMetadata.Annotations))
74-
for _, kv := range kv1.TemplateMetadata.Annotations {
75-
templateAnnotations[kv.Key] = kv.Value
76-
}
77106
spec.Template.ObjectMeta = karpenterv1.ObjectMeta{
78-
Labels: templateLabels,
79-
Annotations: templateAnnotations,
107+
Labels: filterTemplateMetadataKeys(kv1.TemplateMetadata.Labels),
108+
Annotations: filterTemplateMetadataKeys(kv1.TemplateMetadata.Annotations),
80109
}
81110
}
82111

pkg/clusteragent/autoscaling/cluster/model/node_pool_internal_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,88 @@ func TestBuildKarpenterNodePoolFromManifest_WithTemplateMetadata(t *testing.T) {
144144
assert.Equal(t, map[string]string{"node-ann": "node-ann-val"}, knp.Spec.Template.ObjectMeta.Annotations)
145145
}
146146

147+
func TestBuildKarpenterNodePoolFromManifest_TemplateMetadataBlocklist(t *testing.T) {
148+
kv1 := &KarpenterV1NodePool{
149+
Metadata: Metadata{Name: "test-pool"},
150+
TemplateMetadata: &Metadata{
151+
Labels: Labels{
152+
{Key: "app", Value: "my-app"},
153+
{Key: "karpenter.sh/capacity-type", Value: "spot"},
154+
{Key: "node-role.kubernetes.io/control-plane", Value: ""},
155+
{Key: "k8s.io/foo", Value: "bar"},
156+
},
157+
Annotations: Annotations{
158+
{Key: "team", Value: "infra"},
159+
{Key: "kubernetes.io/created-by", Value: "value"},
160+
{Key: "karpenter.sh/do-not-disrupt", Value: "true"},
161+
},
162+
},
163+
Spec: &karpenterv1.NodePoolSpec{},
164+
}
165+
166+
knp := buildKarpenterNodePoolFromManifest(kv1)
167+
168+
require.NotNil(t, knp)
169+
assert.Equal(t, map[string]string{"app": "my-app"}, knp.Spec.Template.ObjectMeta.Labels)
170+
assert.Equal(t, map[string]string{"team": "infra"}, knp.Spec.Template.ObjectMeta.Annotations)
171+
}
172+
173+
func TestFilterTemplateMetadataKeys(t *testing.T) {
174+
tests := []struct {
175+
name string
176+
input []KeyValue
177+
expected map[string]string
178+
}{
179+
{
180+
name: "empty input",
181+
input: nil,
182+
expected: map[string]string{},
183+
},
184+
{
185+
name: "all allowed",
186+
input: []KeyValue{
187+
{Key: "app", Value: "web"},
188+
{Key: "team", Value: "platform"},
189+
},
190+
expected: map[string]string{"app": "web", "team": "platform"},
191+
},
192+
{
193+
name: "keys that contain blocked strings but are not in blocked domains are allowed",
194+
input: []KeyValue{
195+
{Key: "teamk8s.io/role", Value: "worker"},
196+
{Key: "example.com/karpenter.sh-mode", Value: "fast"},
197+
},
198+
expected: map[string]string{"teamk8s.io/role": "worker", "example.com/karpenter.sh-mode": "fast"},
199+
},
200+
{
201+
name: "reserved labels are dropped",
202+
input: []KeyValue{
203+
{Key: "app", Value: "web"},
204+
{Key: "node-role.kubernetes.io/control-plane", Value: ""},
205+
{Key: "k8s.io/foo", Value: "bar"},
206+
{Key: "karpenter.sh/capacity-type", Value: "spot"},
207+
{Key: "karpenter.sh/injected", Value: "bad"},
208+
},
209+
expected: map[string]string{"app": "web"},
210+
},
211+
{
212+
name: "reserved annotations are dropped",
213+
input: []KeyValue{
214+
{Key: "team", Value: "infra"},
215+
{Key: "karpenter.sh/do-not-disrupt", Value: "true"},
216+
{Key: "kubernetes.io/created-by", Value: "value"},
217+
},
218+
expected: map[string]string{"team": "infra"},
219+
},
220+
}
221+
222+
for _, tt := range tests {
223+
t.Run(tt.name, func(t *testing.T) {
224+
assert.Equal(t, tt.expected, filterTemplateMetadataKeys(tt.input))
225+
})
226+
}
227+
}
228+
147229
func TestBuildKarpenterNodePoolFromManifest_NilSpec(t *testing.T) {
148230
kv1 := &KarpenterV1NodePool{
149231
Metadata: Metadata{Name: "test-pool"},

0 commit comments

Comments
 (0)