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: warn about release name length #3054

Merged
merged 1 commit into from
May 17, 2023
Merged
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
1 change: 1 addition & 0 deletions .changelog/3054.fixed.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fix: warn about release name length
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
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