Skip to content

Commit

Permalink
fix: warn about release name length
Browse files Browse the repository at this point in the history
  • Loading branch information
Mikołaj Świątek committed May 17, 2023
1 parent d061f59 commit c6eaaec
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 13 deletions.
4 changes: 4 additions & 0 deletions deploy/helm/sumologic/templates/NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ Thank you for installing {{ .Chart.Name }}.
WARNING: You defined sumologic.clusterName with spaces, which is not supported. Spaces have been replaced with dashes.
{{- end }}

{{- if gt (len .Release.Name) 19 }}
WARNING: Your release name is longer than 19 characters. Some features may not work due length limits on Kubernetes labels. This limit will be enforced explicitly in the next major release.
{{- end }}

A Collector with the name {{ .Values.sumologic.collectorName | default (include "sumologic.clusterNameReplaceSpaceWithDash" . ) | quote }} has been created in your Sumo Logic account.

Check the release status by running:
Expand Down
7 changes: 4 additions & 3 deletions deploy/helm/sumologic/templates/_helpers/_common.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
Expand the name of the chart.
*/}}
{{- define "sumologic.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
{{- $name := default .Chart.Name .Values.nameOverride -}}
{{- $name }}
{{- end -}}

{{/*
Expand All @@ -11,10 +12,10 @@ We truncate at 63 chars because some Kubernetes name fields are limited to this
*/}}
{{- define "sumologic.fullname" -}}
{{- if .Values.fullnameOverride }}
{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
{{- .Values.fullnameOverride }}
{{- else }}
{{- $name := default .Chart.Name .Values.nameOverride -}}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
{{- printf "%s-%s" .Release.Name $name }}
{{- end -}}
{{- end -}}

Expand Down
75 changes: 75 additions & 0 deletions tests/helm/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,81 @@ func TestOtelImageFIPSSuffix(t *testing.T) {
}
}
}
func TestNameAndLabelLength(t *testing.T) {
// object kinds whose names are limited to 63 characters instead of K8s default of 253
// not all of these are strictly required, but it's a good practice to limit them regardless
limitedNameKinds := []string{
"Pod",
"Service",
"Deployment",
"DaemonSet",
"StatefulSet",
}
valuesFilePath := path.Join(testDataDirectory, "everything-enabled.yaml")
releaseName := strings.Repeat("a", maxHelmReleaseNameLength)
renderedYamlString := RenderTemplate(
t,
&helm.Options{
ValuesFiles: []string{valuesFilePath},
SetStrValues: map[string]string{
"sumologic.accessId": "accessId",
"sumologic.accessKey": "accessKey",
},
Logger: logger.Discard, // the log output is noisy and doesn't help much
},
chartDirectory,
releaseName,
[]string{},
true,
"--namespace",
defaultNamespace,
)

// split the rendered Yaml into individual documents and unmarshal them into K8s objects
// we could use the yaml decoder directly, but we'd have to implement our own unmarshaling logic then
renderedObjects := UnmarshalMultipleFromYaml[unstructured.Unstructured](t, renderedYamlString)

for _, renderedObject := range renderedObjects {
name := renderedObject.GetName()
kind := renderedObject.GetKind()
maxNameLength := k8sMaxNameLength
for _, limitedNameKind := range limitedNameKinds {
if kind == limitedNameKind {
maxNameLength = k8sMaxLabelLength
}
}
assert.LessOrEqualf(t, len(name), maxNameLength, "object kind `%s` name `%s` must be no more than %d characters", renderedObject.GetKind(), name, maxNameLength)
labels := renderedObject.GetLabels()
for key, value := range labels {
assert.LessOrEqualf(t, len(value), k8sMaxLabelLength, "value of label %s=%s must be no more than %d characters", key, value, k8sMaxLabelLength)
}
}
}

func TestReleaseNameTooLong(t *testing.T) {
valuesFilePath := path.Join(testDataDirectory, "everything-enabled.yaml")
releaseName := strings.Repeat("a", maxHelmReleaseNameLength+1)
_, err := RenderTemplateE(
t,
&helm.Options{
ValuesFiles: []string{valuesFilePath},
SetStrValues: map[string]string{
"sumologic.accessId": "accessId",
"sumologic.accessKey": "accessKey",
},
Logger: logger.Discard, // the log output is noisy and doesn't help much
},
chartDirectory,
releaseName,
[]string{},
true,
"--namespace",
defaultNamespace,
)
require.NoError(t, err)
// we can't check whether NOTES are rendered correctly due to https://github.com/helm/helm/issues/6901
// TODO: Add an error check here after we start enforcing the limit: https://github.com/SumoLogic/sumologic-kubernetes-collection/issues/3057
}

// check the built-in labels added to all K8s objects created by the chart
func checkBuiltinLabels(t *testing.T, object metav1.Object, chartVersion string) {
Expand Down
24 changes: 14 additions & 10 deletions tests/helm/const.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
package helm

const (
configFileName = "config.sh"
yamlDirectory = "static"
chartDirectory = "../../deploy/helm/sumologic"
chartName = "sumologic"
releaseName = "collection-test"
defaultNamespace = "sumologic"
testDataDirectory = "./testdata"
otelConfigFileName = "config.yaml"
otelImageFIPSSuffix = "-fips"
otelContainerName = "otelcol"
configFileName = "config.sh"
yamlDirectory = "static"
chartDirectory = "../../deploy/helm/sumologic"
chartName = "sumologic"
releaseName = "collection-test"
defaultNamespace = "sumologic"
testDataDirectory = "./testdata"
otelConfigFileName = "config.yaml"
otelImageFIPSSuffix = "-fips"
otelContainerName = "otelcol"
maxHelmReleaseNameLength = 19 // Helm allows up to 53, this is our own limit
k8sMaxNameLength = 253 // see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/
k8sMaxLabelLength = 63 // see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/
)

var subChartNames []string = []string{
Expand All @@ -21,4 +24,5 @@ var subChartNames []string = []string{
"telegraf-operator",
"tailing-sidecar",
"falco",
"opentelemetry-operator",
}
3 changes: 3 additions & 0 deletions tests/helm/testdata/everything-enabled.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,8 @@ falco:
fluent-bit:
enabled: true

opentelemetry-operator:
enabled: true

tailing-sidecar-operator:
enabled: true

0 comments on commit c6eaaec

Please sign in to comment.