Skip to content

Commit

Permalink
feat!: truncate fullname after 22 characters
Browse files Browse the repository at this point in the history
  • Loading branch information
@aboguszewski-sumo
aboguszewski-sumo committed Sep 7, 2023
1 parent 8d8a3b9 commit 51135b6
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 37 deletions.
1 change: 1 addition & 0 deletions .changelog/3248.breaking.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
feat!: truncate fullname after 22 characters
2 changes: 1 addition & 1 deletion deploy/helm/sumologic/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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` |
Expand Down
4 changes: 0 additions & 4 deletions deploy/helm/sumologic/templates/NOTES.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
7 changes: 4 additions & 3 deletions deploy/helm/sumologic/templates/_helpers/_common.tpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 -}}

Expand Down
8 changes: 7 additions & 1 deletion docs/v4-migration-doc.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
- [Running the helm upgrade](#running-the-helm-upgrade)
- [Known issues](#known-issues)
- [Full list of changes](#full-list-of-changes)
<!-- /TOC -->
- [Truncating full name to 22 characters](#truncating-full-name-to-22-characters)
<!-- /TOC -->

Based on feedback from our users, we will be introducing several changes to the Sumo Logic Kubernetes Collection solution.

Expand Down Expand Up @@ -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.
25 changes: 0 additions & 25 deletions tests/helm/common_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions tests/helm/const.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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/
)
Expand Down
2 changes: 2 additions & 0 deletions tests/helm/testdata/everything-enabled.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ sumologic:
logs:
collector:
allowSideBySide: true
otelcloudwatch:
enabled: true

metadata:
logs:
Expand Down
6 changes: 5 additions & 1 deletion tests/integration/internal/strings/strings.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import (
"time"
)

const (
maxReleaseNameLength = 12
)

func NameFromT(t *testing.T) string {
return strings.ReplaceAll(strings.ToLower(t.Name()), "_", "-")
}
Expand All @@ -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]
}

0 comments on commit 51135b6

Please sign in to comment.