From 51135b6d4df02d61e4af6aebe79cfde8eb14ce95 Mon Sep 17 00:00:00 2001 From: Adam Boguszewski Date: Thu, 7 Sep 2023 11:46:16 +0200 Subject: [PATCH] feat!: truncate fullname after 22 characters --- .changelog/3248.breaking.txt | 1 + deploy/helm/sumologic/README.md | 2 +- deploy/helm/sumologic/templates/NOTES.txt | 4 --- .../sumologic/templates/_helpers/_common.tpl | 7 +++--- docs/v4-migration-doc.md | 8 +++++- tests/helm/common_test.go | 25 ------------------- tests/helm/const.go | 4 +-- tests/helm/testdata/everything-enabled.yaml | 2 ++ tests/integration/internal/strings/strings.go | 6 ++++- 9 files changed, 22 insertions(+), 37 deletions(-) create mode 100644 .changelog/3248.breaking.txt diff --git a/.changelog/3248.breaking.txt b/.changelog/3248.breaking.txt new file mode 100644 index 0000000000..44171f1433 --- /dev/null +++ b/.changelog/3248.breaking.txt @@ -0,0 +1 @@ +feat!: truncate fullname after 22 characters \ No newline at end of file diff --git a/deploy/helm/sumologic/README.md b/deploy/helm/sumologic/README.md index 71b53617d4..e89113ef3c 100644 --- a/deploy/helm/sumologic/README.md +++ b/deploy/helm/sumologic/README.md @@ -23,7 +23,7 @@ The following table lists the configurable parameters of the Sumo Logic chart an | Parameter | Description | Default | | ------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | `nameOverride` | Used to override the Chart name. | `Nil` | -| `fullnameOverride` | Used to override the chart's full name. | `Nil` | +| `fullnameOverride` | Used to override the chart's full name. Names longer than 22 characters will be truncated. | `Nil` | | `namespaceOverride` | Used to override the chart's default target namepace. | `Nil` | | `sumologic.setupEnabled` | If enabled, a pre-install hook will create Collector and Sources in Sumo Logic. | `true` | | `sumologic.cleanupEnabled` | If enabled, a pre-delete hook will destroy Kubernetes secret and Sumo Logic Collector. | `false` | diff --git a/deploy/helm/sumologic/templates/NOTES.txt b/deploy/helm/sumologic/templates/NOTES.txt index a700f306c7..fd1037973a 100644 --- a/deploy/helm/sumologic/templates/NOTES.txt +++ b/deploy/helm/sumologic/templates/NOTES.txt @@ -4,10 +4,6 @@ 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: diff --git a/deploy/helm/sumologic/templates/_helpers/_common.tpl b/deploy/helm/sumologic/templates/_helpers/_common.tpl index 76ef205e55..0279404795 100644 --- a/deploy/helm/sumologic/templates/_helpers/_common.tpl +++ b/deploy/helm/sumologic/templates/_helpers/_common.tpl @@ -7,14 +7,15 @@ Expand the name of the chart. {{/* Create a default fully qualified app name. -We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec). +We truncate at 22 chars because some Kubernetes name fields are limited to 63 characters (by the DNS naming spec). +In particular, some statefulsets will have too long names if the name is longer than 22 characters. */}} {{- define "sumologic.fullname" -}} {{- if .Values.fullnameOverride }} -{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }} +{{- .Values.fullnameOverride | trunc 22 | trimSuffix "-" }} {{- else }} {{- $name := default .Chart.Name .Values.nameOverride -}} -{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }} +{{- printf "%s-%s" .Release.Name $name | trunc 22 | trimSuffix "-" }} {{- end -}} {{- end -}} diff --git a/docs/v4-migration-doc.md b/docs/v4-migration-doc.md index b35d22c634..555ea03a0f 100644 --- a/docs/v4-migration-doc.md +++ b/docs/v4-migration-doc.md @@ -11,7 +11,8 @@ - [Running the helm upgrade](#running-the-helm-upgrade) - [Known issues](#known-issues) - [Full list of changes](#full-list-of-changes) - + - [Truncating full name to 22 characters](#truncating-full-name-to-22-characters) + Based on feedback from our users, we will be introducing several changes to the Sumo Logic Kubernetes Collection solution. @@ -67,3 +68,8 @@ require additional action. ## Full list of changes :construction: + +### Truncating full name to 22 characters + +Some Kubernetes objects, for example statefulsets, have a tight (63 characters) limit for their names. Because of that, we truncate the +prefix that is attached to the names. In particular, the value under key `fullnameOverride` will be truncated to 22 characters. diff --git a/tests/helm/common_test.go b/tests/helm/common_test.go index e68d9f4824..4dabd30494 100644 --- a/tests/helm/common_test.go +++ b/tests/helm/common_test.go @@ -172,31 +172,6 @@ func TestNameAndLabelLength(t *testing.T) { } } -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) { labels := object.GetLabels() diff --git a/tests/helm/const.go b/tests/helm/const.go index ff20af642a..5fdb315a1c 100644 --- a/tests/helm/const.go +++ b/tests/helm/const.go @@ -5,14 +5,14 @@ const ( yamlDirectory = "static" chartDirectory = "../../deploy/helm/sumologic" chartName = "sumologic" - releaseName = "collection-test" + releaseName = "col-test" defaultNamespace = "sumologic" defaultK8sVersion = "1.23.0" testDataDirectory = "./testdata" otelConfigFileName = "config.yaml" otelImageFIPSSuffix = "-fips" otelContainerName = "otelcol" - maxHelmReleaseNameLength = 19 // Helm allows up to 53, this is our own limit + maxHelmReleaseNameLength = 22 // Helm allows up to 53, but for a name longer than 22 some statefulset names will be too long 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/ ) diff --git a/tests/helm/testdata/everything-enabled.yaml b/tests/helm/testdata/everything-enabled.yaml index 95dcb3524f..ab70fe2c15 100644 --- a/tests/helm/testdata/everything-enabled.yaml +++ b/tests/helm/testdata/everything-enabled.yaml @@ -2,6 +2,8 @@ sumologic: logs: collector: allowSideBySide: true + otelcloudwatch: + enabled: true metadata: logs: diff --git a/tests/integration/internal/strings/strings.go b/tests/integration/internal/strings/strings.go index 24c2d01ae4..8b08853c42 100644 --- a/tests/integration/internal/strings/strings.go +++ b/tests/integration/internal/strings/strings.go @@ -8,6 +8,10 @@ import ( "time" ) +const ( + maxReleaseNameLength = 12 +) + func NameFromT(t *testing.T) string { return strings.ReplaceAll(strings.ToLower(t.Name()), "_", "-") } @@ -29,5 +33,5 @@ func ValueFileFromT(t *testing.T) string { func ReleaseNameFromT(t *testing.T) string { h := fnv.New32a() h.Write([]byte(t.Name())) - return fmt.Sprintf("rel-%d", h.Sum32()) + return fmt.Sprintf("rel-%d", h.Sum32())[:maxReleaseNameLength] }