Skip to content

Commit

Permalink
fix!: Fix bug where backslashes couldn't be in global tag keys (#3285)
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathan-innis committed Feb 1, 2023
1 parent 2f880e2 commit bb453ef
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 28 deletions.
11 changes: 7 additions & 4 deletions charts/karpenter/templates/_helpers.tpl
Expand Up @@ -74,18 +74,21 @@ Create the name of the service account to use
{{- end -}}

{{/*
Flatten Values Map using "." syntax
Flatten Settings Map using "." syntax
*/}}
{{- define "flattenMap" -}}
{{- define "flattenSettings" -}}
{{- $map := first . -}}
{{- $label := last . -}}
{{- range $key, $val := $map -}}
{{- $sublabel := $key -}}
{{- if $label -}}
{{- $sublabel = list $label $key | join "." -}}
{{- end -}}
{{- if kindOf $val | eq "map" -}}
{{- list $val $sublabel | include "flattenMap" -}}
{{/* Special-case "tags" since we want this to be a JSON object */}}
{{ if eq $key "tags" }}
{{ $sublabel | quote }}: {{ $val | toJson | quote }}
{{- else if kindOf $val | eq "map" -}}
{{- list $val $sublabel | include "flattenSettings" -}}
{{- else -}}
{{ if not (kindIs "invalid" $val) }}
{{ $sublabel | quote }}: {{ $val | quote }}
Expand Down
2 changes: 1 addition & 1 deletion charts/karpenter/templates/configmap.yaml
Expand Up @@ -10,4 +10,4 @@ metadata:
{{- toYaml . | nindent 4 }}
{{- end }}
data:
{{- list .Values.settings "" | include "flattenMap" | indent 2 }}
{{- list .Values.settings "" | include "flattenSettings" | indent 2 }}
4 changes: 2 additions & 2 deletions charts/karpenter/values.yaml
Expand Up @@ -167,5 +167,5 @@ settings:
# -- interruptionQueueName is currently in ALPHA and is disabled by default. Enabling interruption handling may
# require additional permissions on the controller service account. Additional permissions are outlined in the docs.
interruptionQueueName: ""
# -- The global tags to use on all AWS infrastructure resources (launch templates, instances, SQS queue, etc.)
tags:
# -- The global tags to use on all AWS infrastructure resources (launch templates, instances, etc.) across node templates
tags:
21 changes: 9 additions & 12 deletions pkg/apis/settings/settings.go
Expand Up @@ -16,9 +16,9 @@ package settings

import (
"context"
"encoding/json"
"fmt"
"net/url"
"strings"

"github.com/go-playground/validator/v10"
"go.uber.org/multierr"
Expand Down Expand Up @@ -82,7 +82,7 @@ func (*Settings) Inject(ctx context.Context, cm *v1.ConfigMap) (context.Context,
AsTypedString("aws.nodeNameConvention", &s.NodeNameConvention),
configmap.AsFloat64("aws.vmMemoryOverheadPercent", &s.VMMemoryOverheadPercent),
configmap.AsString("aws.interruptionQueueName", &s.InterruptionQueueName),
AsMap("aws.tags", &s.Tags),
AsStringMap("aws.tags", &s.Tags),
); err != nil {
return ctx, fmt.Errorf("parsing settings, %w", err)
}
Expand Down Expand Up @@ -138,19 +138,16 @@ func AsTypedString[T ~string](key string, target *T) configmap.ParseFunc {
}
}

// AsMap parses any value with the prefix key into a map with suffixes as keys and values as values in the target map.
// e.g. {"aws.tags.tag1":"value1"} gets parsed into the map Tags as {"tag1": "value1"}
func AsMap(key string, target *map[string]string) configmap.ParseFunc {
// AsStringMap parses a value as a JSON map of map[string]string.
func AsStringMap(key string, target *map[string]string) configmap.ParseFunc {
return func(data map[string]string) error {
m := map[string]string{}

// Unwind the values into structured keys
for k, v := range data {
if strings.HasPrefix(k, key+".") {
m[k[len(key+"."):]] = v
if raw, ok := data[key]; ok {
m := map[string]string{}
if err := json.Unmarshal([]byte(raw), &m); err != nil {
return err
}
*target = m
}
*target = m
return nil
}
}
6 changes: 3 additions & 3 deletions pkg/apis/settings/suite_test.go
Expand Up @@ -64,8 +64,7 @@ var _ = Describe("Validation", func() {
"aws.isolatedVPC": "true",
"aws.nodeNameConvention": "resource-name",
"aws.vmMemoryOverheadPercent": "0.1",
"aws.tags.tag1": "value1",
"aws.tags.tag2": "value2",
"aws.tags": `{"tag1": "value1", "tag2": "value2", "example.com/tag": "my-value"}`,
},
}
ctx, err := (&settings.Settings{}).Inject(ctx, cm)
Expand All @@ -77,9 +76,10 @@ var _ = Describe("Validation", func() {
Expect(s.IsolatedVPC).To(BeTrue())
Expect(s.NodeNameConvention).To(Equal(settings.ResourceName))
Expect(s.VMMemoryOverheadPercent).To(Equal(0.1))
Expect(len(s.Tags)).To(Equal(2))
Expect(len(s.Tags)).To(Equal(3))
Expect(s.Tags).To(HaveKeyWithValue("tag1", "value1"))
Expect(s.Tags).To(HaveKeyWithValue("tag2", "value2"))
Expect(s.Tags).To(HaveKeyWithValue("example.com/tag", "my-value"))
})
It("should fail validation with panic when clusterName not included", func() {
cm := &v1.ConfigMap{
Expand Down
4 changes: 3 additions & 1 deletion test/suites/integration/tags_test.go
Expand Up @@ -59,7 +59,7 @@ var _ = Describe("Tags", func() {
})

env.ExpectSettingsOverridden(map[string]string{
"aws.tags.TestTag": "TestVal",
"aws.tags": `{"TestTag": "TestVal", "example.com/tag": "custom-value"}`,
})
provisioner := test.Provisioner(test.ProvisionerOptions{ProviderRef: &v1alpha5.ProviderRef{Name: provider.Name}})
pod := test.Pod()
Expand All @@ -73,6 +73,8 @@ var _ = Describe("Tags", func() {

Expect(instanceTags).To(HaveKeyWithValue("TestTag", "TestVal"))
Expect(volumeTags).To(HaveKeyWithValue("TestTag", "TestVal"))
Expect(instanceTags).To(HaveKeyWithValue("example.com/tag", "custom-value"))
Expect(volumeTags).To(HaveKeyWithValue("example.com/tag", "custom-value"))
})
})

Expand Down
6 changes: 2 additions & 4 deletions website/content/en/preview/concepts/settings.md
Expand Up @@ -66,10 +66,8 @@ data:
# Interruption Handling is currently in ALPHA and is disabled by default. Enabling interruption handling may
# require additional permissions on the controller service account. Additional permissions are outlined in the docs
aws.interruptionQueueName: karpenter-cluster
# Any global tag value can be specified by including the "aws.tags.<tag-key>" prefix
# associated with the value in the key-value tag pair
aws.tags.custom-tag: custom-tag-value
aws.tags.custom-tag2: custom-tag-value
# Global tags are specified by including a JSON object of string to string from tag key to tag value
aws.tags: '{"custom-tag1": "custom-tag-value", "custom-tag2": "custom-tag-value"}'
```

### Feature Gates
Expand Down
2 changes: 1 addition & 1 deletion website/content/en/preview/upgrade-guide.md
Expand Up @@ -93,8 +93,8 @@ By adopting this practice we allow our users who are early adopters to test out

## Upgrading to v0.24.0+
* Settings are no longer updated dynamically while Karpenter is running. If you manually make a change to the `karpenter-global-settings` ConfigMap, you will need to reload the containers by restarting the deployment with `kubectl rollout restart -n karpenter deploy/karpenter`

* Karpenter no longer filters out instance types internally. Previously, `g2` (not supported by the NVIDIA device plugin) and FPGA instance types were filtered. The only way to filter instance types now is to set requirements on your provisioner or pods using well-known node labels described [here]({{<ref "./concepts/scheduling#selecting-nodes" >}}). If you are currently using overly broad requirements that allows all of the `g` instance-category, you will want to tighten the requirement, or add an instance-generation requirement.
* `aws.tags` in `karpenter-global-settings` ConfigMap is now a top-level field and expects the value associated with this key to be a JSON object of string to string. This is change from previous versions where keys were given implicitly by providing the key-value pair `aws.tags.<key>: value` in the ConfigMap.

## Upgrading to v0.22.0+
* Do not upgrade to this version unless you are on Kubernetes >= v1.21. Karpenter no longer supports Kubernetes v1.20, but now supports Kubernetes v1.25. This change is due to the v1 PDB API, which was introduced in K8s v1.20 and subsequent removal of the v1beta1 API in K8s v1.25.
Expand Down

0 comments on commit bb453ef

Please sign in to comment.