Skip to content

Commit 54cb2d8

Browse files
matheuscscpmatheuscscp
authored
Merge pull request #1539 from matheuscscp/debug-dup-subcharts
Fix HelmChart local dependency resolution for name-based path
2 parents 8d8e7cc + d941101 commit 54cb2d8

24 files changed

+722
-10
lines changed

‎internal/helm/chart/dependency_manager.go

+3
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,9 @@ func (dm *DependencyManager) resolveRepository(url string) (repo repository.Down
296296
// It does not allow the dependency's path to be outside the scope of
297297
// LocalReference.WorkDir.
298298
func (dm *DependencyManager) secureLocalChartPath(ref LocalReference, dep *helmchart.Dependency) (string, error) {
299+
if dep.Repository == "" {
300+
return securejoin.SecureJoin(ref.WorkDir, filepath.Join(ref.Path, "charts", dep.Name))
301+
}
299302
localUrl, err := url.Parse(dep.Repository)
300303
if err != nil {
301304
return "", fmt.Errorf("failed to parse alleged local chart reference: %w", err)

‎internal/helm/chart/dependency_manager_test.go

+45-10
Original file line numberDiff line numberDiff line change
@@ -290,13 +290,15 @@ func TestDependencyManager_build(t *testing.T) {
290290

291291
func TestDependencyManager_addLocalDependency(t *testing.T) {
292292
tests := []struct {
293-
name string
294-
dep *helmchart.Dependency
295-
wantErr string
296-
wantFunc func(g *WithT, c *helmchart.Chart)
293+
name string
294+
chartName string
295+
dep *helmchart.Dependency
296+
wantErr string
297+
wantFunc func(g *WithT, c *helmchart.Chart)
297298
}{
298299
{
299-
name: "local dependency",
300+
name: "local dependency",
301+
chartName: "helmchartwithdeps",
300302
dep: &helmchart.Dependency{
301303
Name: chartName,
302304
Version: chartVersion,
@@ -307,7 +309,8 @@ func TestDependencyManager_addLocalDependency(t *testing.T) {
307309
},
308310
},
309311
{
310-
name: "version not matching constraint",
312+
name: "version not matching constraint",
313+
chartName: "helmchartwithdeps",
311314
dep: &helmchart.Dependency{
312315
Name: chartName,
313316
Version: "0.2.0",
@@ -316,7 +319,8 @@ func TestDependencyManager_addLocalDependency(t *testing.T) {
316319
wantErr: "can't get a valid version for constraint '0.2.0'",
317320
},
318321
{
319-
name: "invalid local reference",
322+
name: "invalid local reference",
323+
chartName: "helmchartwithdeps",
320324
dep: &helmchart.Dependency{
321325
Name: chartName,
322326
Version: chartVersion,
@@ -325,7 +329,8 @@ func TestDependencyManager_addLocalDependency(t *testing.T) {
325329
wantErr: "no chart found at '/absolutely/invalid'",
326330
},
327331
{
328-
name: "invalid chart archive",
332+
name: "invalid chart archive",
333+
chartName: "helmchartwithdeps",
329334
dep: &helmchart.Dependency{
330335
Name: chartName,
331336
Version: chartVersion,
@@ -334,14 +339,35 @@ func TestDependencyManager_addLocalDependency(t *testing.T) {
334339
wantErr: "failed to load chart from '/empty.tgz'",
335340
},
336341
{
337-
name: "invalid constraint",
342+
name: "invalid constraint",
343+
chartName: "helmchartwithdeps",
338344
dep: &helmchart.Dependency{
339345
Name: chartName,
340346
Version: "invalid",
341347
Repository: "file://../helmchart",
342348
},
343349
wantErr: "invalid version/constraint format 'invalid'",
344350
},
351+
{
352+
name: "no repository",
353+
chartName: "helmchartwithdepsnorepo",
354+
dep: &helmchart.Dependency{
355+
Name: chartName,
356+
Version: chartVersion,
357+
},
358+
wantFunc: func(g *WithT, c *helmchart.Chart) {
359+
g.Expect(c.Dependencies()).To(HaveLen(1))
360+
},
361+
},
362+
{
363+
name: "no repository invalid reference",
364+
chartName: "helmchartwithdepsnorepo",
365+
dep: &helmchart.Dependency{
366+
Name: "nonexistingchart",
367+
Version: chartVersion,
368+
},
369+
wantErr: "no chart found at '/helmchartwithdepsnorepo/charts/nonexistingchart'",
370+
},
345371
}
346372
for _, tt := range tests {
347373
t.Run(tt.name, func(t *testing.T) {
@@ -353,7 +379,7 @@ func TestDependencyManager_addLocalDependency(t *testing.T) {
353379
absWorkDir, err := filepath.Abs("../testdata/charts")
354380
g.Expect(err).ToNot(HaveOccurred())
355381

356-
err = dm.addLocalDependency(LocalReference{WorkDir: absWorkDir, Path: "helmchartwithdeps"},
382+
err = dm.addLocalDependency(LocalReference{WorkDir: absWorkDir, Path: tt.chartName},
357383
&chartWithLock{Chart: chart}, tt.dep)
358384
if tt.wantErr != "" {
359385
g.Expect(err).To(HaveOccurred())
@@ -844,6 +870,15 @@ func TestDependencyManager_secureLocalChartPath(t *testing.T) {
844870
},
845871
wantErr: "not a local chart reference",
846872
},
873+
{
874+
name: "local dependency with empty repository",
875+
dep: &helmchart.Dependency{
876+
Name: "some-subchart",
877+
},
878+
baseDir: "/tmp/workdir",
879+
path: "/chart",
880+
want: "/tmp/workdir/chart/charts/some-subchart",
881+
},
847882
}
848883
for _, tt := range tests {
849884
t.Run(tt.name, func(t *testing.T) {

‎internal/helm/testdata/charts/helmchartwithdepsnorepo/.helmignore

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Patterns to ignore when building packages.
2+
# This supports shell glob matching, relative path matching, and
3+
# negation (prefixed with !). Only one pattern per line.
4+
.DS_Store
5+
# Common VCS dirs
6+
.git/
7+
.gitignore
8+
.bzr/
9+
.bzrignore
10+
.hg/
11+
.hgignore
12+
.svn/
13+
# Common backup files
14+
*.swp
15+
*.bak
16+
*.tmp
17+
*~
18+
# Various IDEs
19+
.project
20+
.idea/
21+
*.tmproj
22+
.vscode/

‎internal/helm/testdata/charts/helmchartwithdepsnorepo/Chart.lock

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
dependencies:
2+
- name: helmchart
3+
repository: file://../helmchart
4+
version: 0.1.0
5+
- name: helmchart
6+
repository: file://../helmchart
7+
version: 0.1.0
8+
- name: grafana
9+
repository: https://grafana.github.io/helm-charts
10+
version: 6.17.4
11+
digest: sha256:1e41c97e27347f433ff0212bf52c344bc82dd435f70129d15e96cd2c8fcc32bb
12+
generated: "2021-11-02T01:25:59.624290788+01:00"

‎internal/helm/testdata/charts/helmchartwithdepsnorepo/Chart.yaml

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
apiVersion: v2
2+
name: helmchartwithdeps
3+
description: A Helm chart for Kubernetes
4+
5+
# A chart can be either an 'application' or a 'library' chart.
6+
#
7+
# Application charts are a collection of templates that can be packaged into versioned archives
8+
# to be deployed.
9+
#
10+
# Library charts provide useful utilities or functions for the chart developer. They're included as
11+
# a dependency of application charts to inject those utilities and functions into the rendering
12+
# pipeline. Library charts do not define any templates and therefore cannot be deployed.
13+
type: application
14+
15+
# This is the chart version. This version number should be incremented each time you make changes
16+
# to the chart and its templates, including the app version.
17+
version: 0.1.0
18+
19+
# This is the version number of the application being deployed. This version number should be
20+
# incremented each time you make changes to the application.
21+
appVersion: 1.16.0
22+
23+
dependencies:
24+
- name: helmchart
25+
version: "0.1.0"
26+
- name: helmchart
27+
alias: aliased
28+
version: "0.1.0"

‎internal/helm/testdata/charts/helmchartwithdepsnorepo/charts/helmchart/.helmignore

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Patterns to ignore when building packages.
2+
# This supports shell glob matching, relative path matching, and
3+
# negation (prefixed with !). Only one pattern per line.
4+
.DS_Store
5+
# Common VCS dirs
6+
.git/
7+
.gitignore
8+
.bzr/
9+
.bzrignore
10+
.hg/
11+
.hgignore
12+
.svn/
13+
# Common backup files
14+
*.swp
15+
*.bak
16+
*.tmp
17+
*~
18+
# Various IDEs
19+
.project
20+
.idea/
21+
*.tmproj
22+
.vscode/

‎internal/helm/testdata/charts/helmchartwithdepsnorepo/charts/helmchart/Chart.yaml

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
apiVersion: v2
2+
name: helmchart
3+
description: A Helm chart for Kubernetes
4+
5+
# A chart can be either an 'application' or a 'library' chart.
6+
#
7+
# Application charts are a collection of templates that can be packaged into versioned archives
8+
# to be deployed.
9+
#
10+
# Library charts provide useful utilities or functions for the chart developer. They're included as
11+
# a dependency of application charts to inject those utilities and functions into the rendering
12+
# pipeline. Library charts do not define any templates and therefore cannot be deployed.
13+
type: application
14+
15+
# This is the chart version. This version number should be incremented each time you make changes
16+
# to the chart and its templates, including the app version.
17+
version: 0.1.0
18+
19+
# This is the version number of the application being deployed. This version number should be
20+
# incremented each time you make changes to the application.
21+
appVersion: 1.16.0

‎internal/helm/testdata/charts/helmchartwithdepsnorepo/charts/helmchart/templates/NOTES.txt

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
1. Get the application URL by running these commands:
2+
{{- if .Values.ingress.enabled }}
3+
{{- range $host := .Values.ingress.hosts }}
4+
{{- range .paths }}
5+
http{{ if $.Values.ingress.tls }}s{{ end }}://{{ $host.host }}{{ . }}
6+
{{- end }}
7+
{{- end }}
8+
{{- else if contains "NodePort" .Values.service.type }}
9+
export NODE_PORT=$(kubectl get --namespace {{ .Release.Namespace }} -o jsonpath="{.spec.ports[0].nodePort}" services {{ include "helmchart.fullname" . }})
10+
export NODE_IP=$(kubectl get nodes --namespace {{ .Release.Namespace }} -o jsonpath="{.items[0].status.addresses[0].address}")
11+
echo http://$NODE_IP:$NODE_PORT
12+
{{- else if contains "LoadBalancer" .Values.service.type }}
13+
NOTE: It may take a few minutes for the LoadBalancer IP to be available.
14+
You can watch the status of by running 'kubectl get --namespace {{ .Release.Namespace }} svc -w {{ include "helmchart.fullname" . }}'
15+
export SERVICE_IP=$(kubectl get svc --namespace {{ .Release.Namespace }} {{ include "helmchart.fullname" . }} --template "{{"{{ range (index .status.loadBalancer.ingress 0) }}{{.}}{{ end }}"}}")
16+
echo http://$SERVICE_IP:{{ .Values.service.port }}
17+
{{- else if contains "ClusterIP" .Values.service.type }}
18+
export POD_NAME=$(kubectl get pods --namespace {{ .Release.Namespace }} -l "app.kubernetes.io/name={{ include "helmchart.name" . }},app.kubernetes.io/instance={{ .Release.Name }}" -o jsonpath="{.items[0].metadata.name}")
19+
echo "Visit http://127.0.0.1:8080 to use your application"
20+
kubectl --namespace {{ .Release.Namespace }} port-forward $POD_NAME 8080:80
21+
{{- end }}

‎internal/helm/testdata/charts/helmchartwithdepsnorepo/charts/helmchart/templates/_helpers.tpl

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
{{/* vim: set filetype=mustache: */}}
2+
{{/*
3+
Expand the name of the chart.
4+
*/}}
5+
{{- define "helmchart.name" -}}
6+
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}}
7+
{{- end -}}
8+
9+
{{/*
10+
Create a default fully qualified app name.
11+
We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
12+
If release name contains chart name it will be used as a full name.
13+
*/}}
14+
{{- define "helmchart.fullname" -}}
15+
{{- if .Values.fullnameOverride -}}
16+
{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" -}}
17+
{{- else -}}
18+
{{- $name := default .Chart.Name .Values.nameOverride -}}
19+
{{- if contains $name .Release.Name -}}
20+
{{- .Release.Name | trunc 63 | trimSuffix "-" -}}
21+
{{- else -}}
22+
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}}
23+
{{- end -}}
24+
{{- end -}}
25+
{{- end -}}
26+
27+
{{/*
28+
Create chart name and version as used by the chart label.
29+
*/}}
30+
{{- define "helmchart.chart" -}}
31+
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" -}}
32+
{{- end -}}
33+
34+
{{/*
35+
Common labels
36+
*/}}
37+
{{- define "helmchart.labels" -}}
38+
helm.sh/chart: {{ include "helmchart.chart" . }}
39+
{{ include "helmchart.selectorLabels" . }}
40+
{{- if .Chart.AppVersion }}
41+
app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
42+
{{- end }}
43+
app.kubernetes.io/managed-by: {{ .Release.Service }}
44+
{{- end -}}
45+
46+
{{/*
47+
Selector labels
48+
*/}}
49+
{{- define "helmchart.selectorLabels" -}}
50+
app.kubernetes.io/name: {{ include "helmchart.name" . }}
51+
app.kubernetes.io/instance: {{ .Release.Name }}
52+
{{- end -}}
53+
54+
{{/*
55+
Create the name of the service account to use
56+
*/}}
57+
{{- define "helmchart.serviceAccountName" -}}
58+
{{- if .Values.serviceAccount.create -}}
59+
{{ default (include "helmchart.fullname" .) .Values.serviceAccount.name }}
60+
{{- else -}}
61+
{{ default "default" .Values.serviceAccount.name }}
62+
{{- end -}}
63+
{{- end -}}

‎internal/helm/testdata/charts/helmchartwithdepsnorepo/charts/helmchart/templates/deployment.yaml

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
apiVersion: apps/v1
2+
kind: Deployment
3+
metadata:
4+
name: {{ include "helmchart.fullname" . }}
5+
labels:
6+
{{- include "helmchart.labels" . | nindent 4 }}
7+
spec:
8+
replicas: {{ .Values.replicaCount }}
9+
selector:
10+
matchLabels:
11+
{{- include "helmchart.selectorLabels" . | nindent 6 }}
12+
template:
13+
metadata:
14+
labels:
15+
{{- include "helmchart.selectorLabels" . | nindent 8 }}
16+
spec:
17+
{{- with .Values.imagePullSecrets }}
18+
imagePullSecrets:
19+
{{- toYaml . | nindent 8 }}
20+
{{- end }}
21+
serviceAccountName: {{ include "helmchart.serviceAccountName" . }}
22+
securityContext:
23+
{{- toYaml .Values.podSecurityContext | nindent 8 }}
24+
containers:
25+
- name: {{ .Chart.Name }}
26+
securityContext:
27+
{{- toYaml .Values.securityContext | nindent 12 }}
28+
image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}"
29+
imagePullPolicy: {{ .Values.image.pullPolicy }}
30+
ports:
31+
- name: http
32+
containerPort: 80
33+
protocol: TCP
34+
livenessProbe:
35+
httpGet:
36+
path: /
37+
port: http
38+
readinessProbe:
39+
httpGet:
40+
path: /
41+
port: http
42+
resources:
43+
{{- toYaml .Values.resources | nindent 12 }}
44+
{{- with .Values.nodeSelector }}
45+
nodeSelector:
46+
{{- toYaml . | nindent 8 }}
47+
{{- end }}
48+
{{- with .Values.affinity }}
49+
affinity:
50+
{{- toYaml . | nindent 8 }}
51+
{{- end }}
52+
{{- with .Values.tolerations }}
53+
tolerations:
54+
{{- toYaml . | nindent 8 }}
55+
{{- end }}

0 commit comments

Comments
 (0)