msb-public
/
helm
mirror of https://github.com/helm/helm
1
0
Code Issues Projects Releases Wiki Activity

Sanitise label values rendered by the helm create scaffold

Browse Source 
The generated _helpers.tpl builds helm.sh/chart with
`replace "+" "_" | trunc 63 | trimSuffix "-"`. Kubernetes label values
must also not end in "." or "_", so a version whose 63-character cut
lands on either produces a value the API server rejects. The
app.kubernetes.io/version label emits .Chart.AppVersion unmodified and
is invalid for any appVersion over 63 characters or containing "+".

Use `trimAll "-_."` after truncation for helm.sh/chart, and apply the
same replace/trunc/trim chain to app.kubernetes.io/version, in both the
v2 and the internal v3 scaffold. Add a test that renders the generated
chart with such versions and validates the labels with
k8s.io/apimachinery's IsValidLabelValue.

Signed-off-by: KR Ravindra <42912207+KR-Ravindra@users.noreply.github.com>
pull/32634/head
KR Ravindra 3 days ago
parent fa11636b01
commit 17365dcc4a
No known key found for this signature in database
4 changed files with 124 additions and 4 deletions
Show Stats Download Patch File Download Diff File

4
internal/chart/v3/util/create.go
Unescape View File

@ -590,7 +590,7 @@ If release name contains chart name it will be used as a full name.
Create chart name and version as used by the chart label.
*/}}
{{- define "<CHARTNAME>.chart" -}}
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }}
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimAll "-_." }}
{{- end }}
{{/*
@ -600,7 +600,7 @@ Common labels
helm.sh/chart: {{ include "<CHARTNAME>.chart" . }}
{{ include "<CHARTNAME>.selectorLabels" . }}
{{- if .Chart.AppVersion }}
app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
app.kubernetes.io/version: {{ .Chart.AppVersion | replace "+" "_" | trunc 63 | trimAll "-_." | quote }}
{{- end }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- end }}

60
internal/chart/v3/util/create_test.go
Unescape View File

@ -25,9 +25,14 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/util/validation"
"sigs.k8s.io/yaml"
chart "helm.sh/helm/v4/internal/chart/v3"
"helm.sh/helm/v4/internal/chart/v3/loader"
"helm.sh/helm/v4/pkg/chart/common"
"helm.sh/helm/v4/pkg/chart/common/util"
"helm.sh/helm/v4/pkg/engine"
)
func TestCreate(t *testing.T) {
@ -95,6 +100,61 @@ func TestCreateFrom(t *testing.T) {
}
}
func TestCreate_LabelValues(t *testing.T) {
tdir := t.TempDir()
c, err := Create("demo", tdir)
require.NoError(t, err)
mychart, err := loader.LoadDir(c)
require.NoError(t, err)
// "demo-" + version is 64 characters, so `trunc 63` cuts right after the
// character that precedes the final "1".
for name, tc := range map[string]struct {
version string
appVersion string
}{
"truncated chart label ends in dot": {
version: "1.0.0-abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxy.1",
appVersion: "1.16.0",
},
"truncated chart label ends in underscore": {
version: "1.0.0-abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxy+1",
appVersion: "1.16.0",
},
"app version with build metadata longer than 63 characters": {
version: "0.1.0",
appVersion: "v1.2.3+build.123456789012345678901234567890123456789012345678901234567890",
},
} {
t.Run(name, func(t *testing.T) {
mychart.Metadata.Version = tc.version
mychart.Metadata.AppVersion = tc.appVersion
opts := common.ReleaseOptions{Name: "demo", Namespace: "default", Revision: 1, IsInstall: true}
vals, err := util.ToRenderValues(mychart, map[string]any{}, opts, common.DefaultCapabilities)
require.NoError(t, err)
out, err := new(engine.Engine).RenderWithContext(t.Context(), mychart, vals)
require.NoError(t, err)
var sa struct {
Metadata struct {
Labels map[string]string `json:"labels"`
} `json:"metadata"`
}
require.NoError(t, yaml.Unmarshal([]byte(out["demo/templates/serviceaccount.yaml"]), &sa))
for _, key := range []string{"helm.sh/chart", "app.kubernetes.io/version"} {
v, ok := sa.Metadata.Labels[key]
require.True(t, ok, "expected label %q to be set", key)
assert.Empty(t, validation.IsValidLabelValue(v), "label %s=%q", key, v)
}
})
}
}
// TestCreate_Overwrite is a regression test for making sure that files are overwritten.
func TestCreate_Overwrite(t *testing.T) {
tdir := t.TempDir()

4
pkg/chart/v2/util/create.go
Unescape View File

@ -589,7 +589,7 @@ If release name contains chart name it will be used as a full name.
Create chart name and version as used by the chart label.
*/}}
{{- define "<CHARTNAME>.chart" -}}
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }}
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimAll "-_." }}
{{- end }}
{{/*
@ -599,7 +599,7 @@ Common labels
helm.sh/chart: {{ include "<CHARTNAME>.chart" . }}
{{ include "<CHARTNAME>.selectorLabels" . }}
{{- if .Chart.AppVersion }}
app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
app.kubernetes.io/version: {{ .Chart.AppVersion | replace "+" "_" | trunc 63 | trimAll "-_." | quote }}
{{- end }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- end }}

60
pkg/chart/v2/util/create_test.go
Unescape View File

@ -24,9 +24,14 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/util/validation"
"sigs.k8s.io/yaml"
"helm.sh/helm/v4/pkg/chart/common"
"helm.sh/helm/v4/pkg/chart/common/util"
chart "helm.sh/helm/v4/pkg/chart/v2"
"helm.sh/helm/v4/pkg/chart/v2/loader"
"helm.sh/helm/v4/pkg/engine"
)
func TestCreate(t *testing.T) {
@ -92,6 +97,61 @@ func TestCreateFrom(t *testing.T) {
}
}
func TestCreate_LabelValues(t *testing.T) {
tdir := t.TempDir()
c, err := Create("demo", tdir)
require.NoError(t, err)
mychart, err := loader.LoadDir(c)
require.NoError(t, err)
// "demo-" + version is 64 characters, so `trunc 63` cuts right after the
// character that precedes the final "1".
for name, tc := range map[string]struct {
version string
appVersion string
}{
"truncated chart label ends in dot": {
version: "1.0.0-abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxy.1",
appVersion: "1.16.0",
},
"truncated chart label ends in underscore": {
version: "1.0.0-abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxy+1",
appVersion: "1.16.0",
},
"app version with build metadata longer than 63 characters": {
version: "0.1.0",
appVersion: "v1.2.3+build.123456789012345678901234567890123456789012345678901234567890",
},
} {
t.Run(name, func(t *testing.T) {
mychart.Metadata.Version = tc.version
mychart.Metadata.AppVersion = tc.appVersion
opts := common.ReleaseOptions{Name: "demo", Namespace: "default", Revision: 1, IsInstall: true}
vals, err := util.ToRenderValues(mychart, map[string]any{}, opts, common.DefaultCapabilities)
require.NoError(t, err)
out, err := new(engine.Engine).RenderWithContext(t.Context(), mychart, vals)
require.NoError(t, err)
var sa struct {
Metadata struct {
Labels map[string]string `json:"labels"`
} `json:"metadata"`
}
require.NoError(t, yaml.Unmarshal([]byte(out["demo/templates/serviceaccount.yaml"]), &sa))
for _, key := range []string{"helm.sh/chart", "app.kubernetes.io/version"} {
v, ok := sa.Metadata.Labels[key]
require.True(t, ok, "expected label %q to be set", key)
assert.Empty(t, validation.IsValidLabelValue(v), "label %s=%q", key, v)
}
})
}
}
// TestCreate_Overwrite is a regression test for making sure that files are overwritten.
func TestCreate_Overwrite(t *testing.T) {
tdir := t.TempDir()

Write Preview
Loading…
Cancel
Save