From cd6c67d3c4f3018871b417f774aebebe907cc14a Mon Sep 17 00:00:00 2001 From: openmohan Date: Sat, 7 Dec 2019 13:00:49 +0530 Subject: [PATCH] Issue-4949 feed version of the charts through flags while linting Signed-off-by: openmohan --- cmd/helm/chart_save.go | 12 ++++ cmd/helm/create_test.go | 36 ++++++++++ cmd/helm/install.go | 12 ++++ cmd/helm/lint.go | 1 + cmd/helm/package_test.go | 13 ++++ .../chart-missing-version/.helmignore | 22 ++++++ .../chart-missing-version/Chart.yaml | 17 +++++ .../chart-missing-version/templates/NOTES.txt | 21 ++++++ .../templates/_helpers.tpl | 63 +++++++++++++++++ .../templates/deployment.yaml | 55 +++++++++++++++ .../templates/ingress.yaml | 41 +++++++++++ .../templates/service.yaml | 15 ++++ .../templates/serviceaccount.yaml | 8 +++ .../templates/tests/test-connection.yaml | 15 ++++ .../chart-missing-version/values.yaml | 66 ++++++++++++++++++ cmd/helm/upgrade.go | 13 ++++ cmd/helm/upgrade_test.go | 50 +++++++++++++ internal/experimental/registry/cache.go | 13 ++++ pkg/action/dependency.go | 52 ++++++++++++++ pkg/action/lint.go | 7 +- pkg/action/lint_test.go | 11 ++- pkg/action/package.go | 18 +++++ pkg/action/show.go | 13 ++++ pkg/chart/loader/archive.go | 7 +- pkg/chart/loader/load.go | 4 -- pkg/chart/loader/load_test.go | 49 +++++++++---- .../loader/testdata/chart-missing-version.tgz | Bin 0 -> 3552 bytes pkg/chartutil/dependencies_test.go | 14 ++++ pkg/downloader/manager.go | 44 +++++++++++- pkg/lint/lint.go | 6 +- pkg/lint/lint_test.go | 26 +++++-- pkg/lint/rules/chartfile.go | 6 +- pkg/lint/rules/chartfile_test.go | 18 ++++- pkg/lint/rules/template.go | 22 +++++- pkg/lint/rules/template_test.go | 21 +++++- .../missingversionchartfile/Chart.yaml | 4 ++ .../templates/goodone.yaml | 2 + .../missingversionchartfile/values.yaml | 1 + pkg/provenance/sign.go | 12 ++++ pkg/provenance/sign_test.go | 9 +++ .../testdata/chart-missing-version.tgz | Bin 0 -> 3552 bytes pkg/repo/chartrepo.go | 12 ++++ 42 files changed, 794 insertions(+), 37 deletions(-) create mode 100644 cmd/helm/testdata/testcharts/chart-missing-version/.helmignore create mode 100644 cmd/helm/testdata/testcharts/chart-missing-version/Chart.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-missing-version/templates/NOTES.txt create mode 100644 cmd/helm/testdata/testcharts/chart-missing-version/templates/_helpers.tpl create mode 100644 cmd/helm/testdata/testcharts/chart-missing-version/templates/deployment.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-missing-version/templates/ingress.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-missing-version/templates/service.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-missing-version/templates/serviceaccount.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-missing-version/templates/tests/test-connection.yaml create mode 100644 cmd/helm/testdata/testcharts/chart-missing-version/values.yaml create mode 100644 pkg/chart/loader/testdata/chart-missing-version.tgz create mode 100644 pkg/lint/rules/testdata/missingversionchartfile/Chart.yaml create mode 100644 pkg/lint/rules/testdata/missingversionchartfile/templates/goodone.yaml create mode 100644 pkg/lint/rules/testdata/missingversionchartfile/values.yaml create mode 100644 pkg/provenance/testdata/chart-missing-version.tgz diff --git a/cmd/helm/chart_save.go b/cmd/helm/chart_save.go index 35b72cd07..1a84b7318 100644 --- a/cmd/helm/chart_save.go +++ b/cmd/helm/chart_save.go @@ -55,6 +55,18 @@ func newChartSaveCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return err } + // validate chart + if err = ch.Validate(); err != nil { + return err + } + + // validate sub charts + for _, subChart := range ch.Dependencies() { + if err = subChart.Validate(); err != nil { + return err + } + } + return action.NewChartSave(cfg).Run(out, ch, ref) }, } diff --git a/cmd/helm/create_test.go b/cmd/helm/create_test.go index 0a9b7b9a5..70f46d3d9 100644 --- a/cmd/helm/create_test.go +++ b/cmd/helm/create_test.go @@ -53,6 +53,18 @@ func TestCreateCmd(t *testing.T) { t.Fatal(err) } + // validate chart + if err = c.Validate(); err != nil { + t.Fatal(err) + } + + // validate sub charts + for _, subChart := range c.Dependencies() { + if err = subChart.Validate(); err != nil { + t.Fatal(err) + } + } + if c.Name() != cname { t.Errorf("Expected %q name, got %q", cname, c.Name()) } @@ -99,6 +111,18 @@ func TestCreateStarterCmd(t *testing.T) { t.Fatal(err) } + // validate chart + if err = c.Validate(); err != nil { + t.Fatal(err) + } + + // validate sub charts + for _, subChart := range c.Dependencies() { + if err = subChart.Validate(); err != nil { + t.Fatal(err) + } + } + if c.Name() != cname { t.Errorf("Expected %q name, got %q", cname, c.Name()) } @@ -167,6 +191,18 @@ func TestCreateStarterAbsoluteCmd(t *testing.T) { t.Fatal(err) } + // validate chart + if err = c.Validate(); err != nil { + t.Fatal(err) + } + + // validate sub charts + for _, subChart := range c.Dependencies() { + if err = subChart.Validate(); err != nil { + t.Fatal(err) + } + } + if c.Name() != cname { t.Errorf("Expected %q name, got %q", cname, c.Name()) } diff --git a/cmd/helm/install.go b/cmd/helm/install.go index ec2c75a12..319195bd8 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -186,6 +186,18 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options return nil, err } + // validate chart + if err = chartRequested.Validate(); err != nil { + return nil, err + } + + // validate sub charts + for _, subChart := range chartRequested.Dependencies() { + if err = subChart.Validate(); err != nil { + return nil, err + } + } + validInstallableChart, err := isChartInstallable(chartRequested) if !validInstallableChart { return nil, err diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go index bc0d1852b..3c534dded 100644 --- a/cmd/helm/lint.go +++ b/cmd/helm/lint.go @@ -121,6 +121,7 @@ func newLintCmd(out io.Writer) *cobra.Command { f.BoolVar(&client.Strict, "strict", false, "fail on lint warnings") f.BoolVar(&client.WithSubcharts, "with-subcharts", false, "lint dependent charts") addValueOptionsFlags(f, valueOpts) + f.StringVar(&client.Version, "version", "", "version of the chart") return cmd } diff --git a/cmd/helm/package_test.go b/cmd/helm/package_test.go index e0a5fabd6..55e27f01d 100644 --- a/cmd/helm/package_test.go +++ b/cmd/helm/package_test.go @@ -191,6 +191,19 @@ func TestSetAppVersion(t *testing.T) { if err != nil { t.Fatalf("unexpected error loading packaged chart: %v", err) } + + // validate chart + if err = ch.Validate(); err != nil { + t.Fatalf("unexpected error loading packaged chart: %v", err) + } + + // validate sub charts + for _, subChart := range ch.Dependencies() { + if err = subChart.Validate(); err != nil { + t.Fatalf("unexpected error loading packaged chart: %v", err) + } + } + if ch.Metadata.AppVersion != expectedAppVersion { t.Errorf("expected app-version %q, found %q", expectedAppVersion, ch.Metadata.AppVersion) } diff --git a/cmd/helm/testdata/testcharts/chart-missing-version/.helmignore b/cmd/helm/testdata/testcharts/chart-missing-version/.helmignore new file mode 100644 index 000000000..50af03172 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-missing-version/.helmignore @@ -0,0 +1,22 @@ +# Patterns to ignore when building packages. +# This supports shell glob matching, relative path matching, and +# negation (prefixed with !). Only one pattern per line. +.DS_Store +# Common VCS dirs +.git/ +.gitignore +.bzr/ +.bzrignore +.hg/ +.hgignore +.svn/ +# Common backup files +*.swp +*.bak +*.tmp +*~ +# Various IDEs +.project +.idea/ +*.tmproj +.vscode/ diff --git a/cmd/helm/testdata/testcharts/chart-missing-version/Chart.yaml b/cmd/helm/testdata/testcharts/chart-missing-version/Chart.yaml new file mode 100644 index 000000000..e288f8515 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-missing-version/Chart.yaml @@ -0,0 +1,17 @@ +apiVersion: v2 +name: chart-missing-version +description: A Helm chart for Kubernetes + +# A chart can be either an 'application' or a 'library' chart. +# +# Application charts are a collection of templates that can be packaged into versioned archives +# to be deployed. +# +# Library charts provide useful utilities or functions for the chart developer. They're included as +# a dependency of application charts to inject those utilities and functions into the rendering +# pipeline. Library charts do not define any templates and therefore cannot be deployed. +type: application + +# This is the version number of the application being deployed. This version number should be +# incremented each time you make changes to the application. +appVersion: 1.16.0 diff --git a/cmd/helm/testdata/testcharts/chart-missing-version/templates/NOTES.txt b/cmd/helm/testdata/testcharts/chart-missing-version/templates/NOTES.txt new file mode 100644 index 000000000..17908a860 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-missing-version/templates/NOTES.txt @@ -0,0 +1,21 @@ +1. Get the application URL by running these commands: +{{- if .Values.ingress.enabled }} +{{- range $host := .Values.ingress.hosts }} + {{- range .paths }} + http{{ if $.Values.ingress.tls }}s{{ end }}://{{ $host.host }}{{ . }} + {{- end }} +{{- end }} +{{- else if contains "NodePort" .Values.service.type }} + export NODE_PORT=$(kubectl get --namespace {{ .Release.Namespace }} -o jsonpath="{.spec.ports[0].nodePort}" services {{ include "chart-missing-version.fullname" . }}) + export NODE_IP=$(kubectl get nodes --namespace {{ .Release.Namespace }} -o jsonpath="{.items[0].status.addresses[0].address}") + echo http://$NODE_IP:$NODE_PORT +{{- else if contains "LoadBalancer" .Values.service.type }} + NOTE: It may take a few minutes for the LoadBalancer IP to be available. + You can watch the status of by running 'kubectl get --namespace {{ .Release.Namespace }} svc -w {{ include "chart-missing-version.fullname" . }}' + export SERVICE_IP=$(kubectl get svc --namespace {{ .Release.Namespace }} {{ include "chart-missing-version.fullname" . }} --template "{{"{{ range (index .status.loadBalancer.ingress 0) }}{{.}}{{ end }}"}}") + echo http://$SERVICE_IP:{{ .Values.service.port }} +{{- else if contains "ClusterIP" .Values.service.type }} + export POD_NAME=$(kubectl get pods --namespace {{ .Release.Namespace }} -l "app.kubernetes.io/name={{ include "chart-missing-version.name" . }},app.kubernetes.io/instance={{ .Release.Name }}" -o jsonpath="{.items[0].metadata.name}") + echo "Visit http://127.0.0.1:8080 to use your application" + kubectl --namespace {{ .Release.Namespace }} port-forward $POD_NAME 8080:80 +{{- end }} diff --git a/cmd/helm/testdata/testcharts/chart-missing-version/templates/_helpers.tpl b/cmd/helm/testdata/testcharts/chart-missing-version/templates/_helpers.tpl new file mode 100644 index 000000000..fcf64a202 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-missing-version/templates/_helpers.tpl @@ -0,0 +1,63 @@ +{{/* vim: set filetype=mustache: */}} +{{/* +Expand the name of the chart. +*/}} +{{- define "chart-missing-version.name" -}} +{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}} +{{- end -}} + +{{/* +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). +If release name contains chart name it will be used as a full name. +*/}} +{{- define "chart-missing-version.fullname" -}} +{{- if .Values.fullnameOverride -}} +{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" -}} +{{- else -}} +{{- $name := default .Chart.Name .Values.nameOverride -}} +{{- if contains $name .Release.Name -}} +{{- .Release.Name | trunc 63 | trimSuffix "-" -}} +{{- else -}} +{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}} +{{- end -}} +{{- end -}} +{{- end -}} + +{{/* +Create chart name and version as used by the chart label. +*/}} +{{- define "chart-missing-version.chart" -}} +{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" -}} +{{- end -}} + +{{/* +Common labels +*/}} +{{- define "chart-missing-version.labels" -}} +helm.sh/chart: {{ include "chart-missing-version.chart" . }} +{{ include "chart-missing-version.selectorLabels" . }} +{{- if .Chart.AppVersion }} +app.kubernetes.io/version: {{ .Chart.AppVersion | quote }} +{{- end }} +app.kubernetes.io/managed-by: {{ .Release.Service }} +{{- end -}} + +{{/* +Selector labels +*/}} +{{- define "chart-missing-version.selectorLabels" -}} +app.kubernetes.io/name: {{ include "chart-missing-version.name" . }} +app.kubernetes.io/instance: {{ .Release.Name }} +{{- end -}} + +{{/* +Create the name of the service account to use +*/}} +{{- define "chart-missing-version.serviceAccountName" -}} +{{- if .Values.serviceAccount.create -}} + {{ default (include "chart-missing-version.fullname" .) .Values.serviceAccount.name }} +{{- else -}} + {{ default "default" .Values.serviceAccount.name }} +{{- end -}} +{{- end -}} diff --git a/cmd/helm/testdata/testcharts/chart-missing-version/templates/deployment.yaml b/cmd/helm/testdata/testcharts/chart-missing-version/templates/deployment.yaml new file mode 100644 index 000000000..39022f51e --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-missing-version/templates/deployment.yaml @@ -0,0 +1,55 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ include "chart-missing-version.fullname" . }} + labels: + {{- include "chart-missing-version.labels" . | nindent 4 }} +spec: + replicas: {{ .Values.replicaCount }} + selector: + matchLabels: + {{- include "chart-missing-version.selectorLabels" . | nindent 6 }} + template: + metadata: + labels: + {{- include "chart-missing-version.selectorLabels" . | nindent 8 }} + spec: + {{- with .Values.imagePullSecrets }} + imagePullSecrets: + {{- toYaml . | nindent 8 }} + {{- end }} + serviceAccountName: {{ include "chart-missing-version.serviceAccountName" . }} + securityContext: + {{- toYaml .Values.podSecurityContext | nindent 8 }} + containers: + - name: {{ .Chart.Name }} + securityContext: + {{- toYaml .Values.securityContext | nindent 12 }} + image: "{{ .Values.image.repository }}:{{ .Chart.AppVersion }}" + imagePullPolicy: {{ .Values.image.pullPolicy }} + ports: + - name: http + containerPort: 80 + protocol: TCP + livenessProbe: + httpGet: + path: / + port: http + readinessProbe: + httpGet: + path: / + port: http + resources: + {{- toYaml .Values.resources | nindent 12 }} + {{- with .Values.nodeSelector }} + nodeSelector: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.affinity }} + affinity: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.tolerations }} + tolerations: + {{- toYaml . | nindent 8 }} + {{- end }} diff --git a/cmd/helm/testdata/testcharts/chart-missing-version/templates/ingress.yaml b/cmd/helm/testdata/testcharts/chart-missing-version/templates/ingress.yaml new file mode 100644 index 000000000..def39eeb4 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-missing-version/templates/ingress.yaml @@ -0,0 +1,41 @@ +{{- if .Values.ingress.enabled -}} +{{- $fullName := include "chart-missing-version.fullname" . -}} +{{- $svcPort := .Values.service.port -}} +{{- if semverCompare ">=1.14-0" .Capabilities.KubeVersion.GitVersion -}} +apiVersion: networking.k8s.io/v1beta1 +{{- else -}} +apiVersion: extensions/v1beta1 +{{- end }} +kind: Ingress +metadata: + name: {{ $fullName }} + labels: + {{- include "chart-missing-version.labels" . | nindent 4 }} + {{- with .Values.ingress.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: +{{- if .Values.ingress.tls }} + tls: + {{- range .Values.ingress.tls }} + - hosts: + {{- range .hosts }} + - {{ . | quote }} + {{- end }} + secretName: {{ .secretName }} + {{- end }} +{{- end }} + rules: + {{- range .Values.ingress.hosts }} + - host: {{ .host | quote }} + http: + paths: + {{- range .paths }} + - path: {{ . }} + backend: + serviceName: {{ $fullName }} + servicePort: {{ $svcPort }} + {{- end }} + {{- end }} +{{- end }} diff --git a/cmd/helm/testdata/testcharts/chart-missing-version/templates/service.yaml b/cmd/helm/testdata/testcharts/chart-missing-version/templates/service.yaml new file mode 100644 index 000000000..4c7bb86f6 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-missing-version/templates/service.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Service +metadata: + name: {{ include "chart-missing-version.fullname" . }} + labels: + {{- include "chart-missing-version.labels" . | nindent 4 }} +spec: + type: {{ .Values.service.type }} + ports: + - port: {{ .Values.service.port }} + targetPort: http + protocol: TCP + name: http + selector: + {{- include "chart-missing-version.selectorLabels" . | nindent 4 }} diff --git a/cmd/helm/testdata/testcharts/chart-missing-version/templates/serviceaccount.yaml b/cmd/helm/testdata/testcharts/chart-missing-version/templates/serviceaccount.yaml new file mode 100644 index 000000000..f0594ee7b --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-missing-version/templates/serviceaccount.yaml @@ -0,0 +1,8 @@ +{{- if .Values.serviceAccount.create -}} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: {{ include "chart-missing-version.serviceAccountName" . }} + labels: +{{ include "chart-missing-version.labels" . | nindent 4 }} +{{- end -}} diff --git a/cmd/helm/testdata/testcharts/chart-missing-version/templates/tests/test-connection.yaml b/cmd/helm/testdata/testcharts/chart-missing-version/templates/tests/test-connection.yaml new file mode 100644 index 000000000..6f80e2a41 --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-missing-version/templates/tests/test-connection.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Pod +metadata: + name: "{{ include "chart-missing-version.fullname" . }}-test-connection" + labels: +{{ include "chart-missing-version.labels" . | nindent 4 }} + annotations: + "helm.sh/hook": test-success +spec: + containers: + - name: wget + image: busybox + command: ['wget'] + args: ['{{ include "chart-missing-version.fullname" . }}:{{ .Values.service.port }}'] + restartPolicy: Never diff --git a/cmd/helm/testdata/testcharts/chart-missing-version/values.yaml b/cmd/helm/testdata/testcharts/chart-missing-version/values.yaml new file mode 100644 index 000000000..7bcd6cd7c --- /dev/null +++ b/cmd/helm/testdata/testcharts/chart-missing-version/values.yaml @@ -0,0 +1,66 @@ +# Default values for chart-missing-version. +# This is a YAML-formatted file. +# Declare variables to be passed into your templates. + +replicaCount: 1 + +image: + repository: nginx + pullPolicy: IfNotPresent + +imagePullSecrets: [] +nameOverride: "" +fullnameOverride: "" + +serviceAccount: + # Specifies whether a service account should be created + create: true + # The name of the service account to use. + # If not set and create is true, a name is generated using the fullname template + name: + +podSecurityContext: {} + # fsGroup: 2000 + +securityContext: {} + # capabilities: + # drop: + # - ALL + # readOnlyRootFilesystem: true + # runAsNonRoot: true + # runAsUser: 1000 + +service: + type: ClusterIP + port: 80 + +ingress: + enabled: false + annotations: {} + # kubernetes.io/ingress.class: nginx + # kubernetes.io/tls-acme: "true" + hosts: + - host: chart-example.local + paths: [] + tls: [] + # - secretName: chart-example-tls + # hosts: + # - chart-example.local + +resources: {} + # We usually recommend not to specify default resources and to leave this as a conscious + # choice for the user. This also increases chances charts run on environments with little + # resources, such as Minikube. If you do want to specify resources, uncomment the following + # lines, adjust them as necessary, and remove the curly braces after 'resources:'. + # limits: + # cpu: 100m + # memory: 128Mi + # requests: + # cpu: 100m + # memory: 128Mi + +nodeSelector: {} + +tolerations: [] + +affinity: {} diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 119d79f8f..233182a1c 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -124,6 +124,19 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if err != nil { return err } + + // validate chart + if err = ch.Validate(); err != nil { + return err + } + + // validate sub charts + for _, subChart := range ch.Dependencies() { + if err = subChart.Validate(); err != nil { + return err + } + } + if req := ch.Metadata.Dependencies; req != nil { if err := action.CheckDependencies(ch, req); err != nil { return err diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go index 3cecbe6d3..c0f262a25 100644 --- a/cmd/helm/upgrade_test.go +++ b/cmd/helm/upgrade_test.go @@ -48,6 +48,19 @@ func TestUpgradeCmd(t *testing.T) { if err != nil { t.Fatalf("Error loading chart: %v", err) } + + // validate chart + if err = ch.Validate(); err != nil { + t.Fatalf("Error loading chart: %v", err) + } + + // validate sub charts + for _, subChart := range ch.Dependencies() { + if err = subChart.Validate(); err != nil { + t.Fatalf("Error loading chart: %v", err) + } + } + _ = release.Mock(&release.MockReleaseOptions{ Name: "funny-bunny", Chart: ch, @@ -64,6 +77,18 @@ func TestUpgradeCmd(t *testing.T) { t.Fatalf("Error loading updated chart: %v", err) } + // validate chart + if err = ch.Validate(); err != nil { + t.Fatalf("Error loading chart: %v", err) + } + + // validate sub charts + for _, subChart := range ch.Dependencies() { + if err = subChart.Validate(); err != nil { + t.Fatalf("Error loading chart: %v", err) + } + } + // update chart version again cfile.Metadata.Version = "0.1.3" @@ -76,6 +101,18 @@ func TestUpgradeCmd(t *testing.T) { t.Fatalf("Error loading updated chart: %v", err) } + // validate chart + if err = ch.Validate(); err != nil { + t.Fatalf("Error loading chart: %v", err) + } + + // validate sub charts + for _, subChart := range ch.Dependencies() { + if err = subChart.Validate(); err != nil { + t.Fatalf("Error loading chart: %v", err) + } + } + missingDepsPath := "testdata/testcharts/chart-missing-deps" badDepsPath := "testdata/testcharts/chart-bad-requirements" @@ -247,6 +284,19 @@ func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int, if err != nil { t.Fatalf("Error loading chart: %v", err) } + + // validate chart + if err = ch.Validate(); err != nil { + t.Fatalf("Error loading chart: %v", err) + } + + // validate sub charts + for _, subChart := range ch.Dependencies() { + if err = subChart.Validate(); err != nil { + t.Fatalf("Error loading chart: %v", err) + } + } + _ = release.Mock(&release.MockReleaseOptions{ Name: releaseName, Chart: ch, diff --git a/internal/experimental/registry/cache.go b/internal/experimental/registry/cache.go index fbd62562a..c428191b1 100644 --- a/internal/experimental/registry/cache.go +++ b/internal/experimental/registry/cache.go @@ -146,6 +146,19 @@ func (cache *Cache) FetchReference(ref *Reference) (*CacheRefSummary, error) { if err != nil { return &r, err } + + // validate chart + if err = ch.Validate(); err != nil { + return &r, err + } + + // validate sub charts + for _, subChart := range ch.Dependencies() { + if err = subChart.Validate(); err != nil { + return &r, err + } + } + r.Chart = ch } } diff --git a/pkg/action/dependency.go b/pkg/action/dependency.go index 5781cc913..3e5c8a41d 100644 --- a/pkg/action/dependency.go +++ b/pkg/action/dependency.go @@ -50,6 +50,18 @@ func (d *Dependency) List(chartpath string, out io.Writer) error { return err } + // validate chart + if err = c.Validate(); err != nil { + return err + } + + // validate sub charts + for _, subChart := range c.Dependencies() { + if err = subChart.Validate(); err != nil { + return err + } + } + if c.Metadata.Dependencies == nil { fmt.Fprintf(out, "WARNING: no dependencies at %s\n", filepath.Join(chartpath, "charts")) return nil @@ -76,6 +88,19 @@ func (d *Dependency) dependencyStatus(chartpath string, dep *chart.Dependency) s if err != nil { return "corrupt" } + + // validate chart + if err = c.Validate(); err != nil { + return "corrupt" + } + + // validate sub charts + for _, subChart := range c.Dependencies() { + if err = subChart.Validate(); err != nil { + return "corrupt" + } + } + if c.Name() != dep.Name { return "misnamed" } @@ -112,6 +137,18 @@ func (d *Dependency) dependencyStatus(chartpath string, dep *chart.Dependency) s return "corrupt" } + // validate chart + if err = c.Validate(); err != nil { + return "corrupt" + } + + // validate sub charts + for _, subChart := range c.Dependencies() { + if err = subChart.Validate(); err != nil { + return "corrupt" + } + } + if c.Name() != dep.Name { return "misnamed" } @@ -171,6 +208,21 @@ func (d *Dependency) printMissing(chartpath string, out io.Writer, reqs []*chart fmt.Fprintf(out, "WARNING: %q is not a chart.\n", f) continue } + + // validate chart + if err = c.Validate(); err != nil { + fmt.Fprintf(out, "WARNING: %q is not a chart.\n", f) + continue + } + + // validate sub charts + for _, subChart := range c.Dependencies() { + if err = subChart.Validate(); err != nil { + fmt.Fprintf(out, "WARNING: %q is not a chart.\n", f) + continue + } + } + found := false for _, d := range reqs { if d.Name == c.Name() { diff --git a/pkg/action/lint.go b/pkg/action/lint.go index 2292c14bf..3c9f5de7b 100644 --- a/pkg/action/lint.go +++ b/pkg/action/lint.go @@ -36,6 +36,7 @@ type Lint struct { Strict bool Namespace string WithSubcharts bool + Version string } // LintResult is the result of Lint @@ -58,7 +59,7 @@ func (l *Lint) Run(paths []string, vals map[string]interface{}) *LintResult { } result := &LintResult{} for _, path := range paths { - linter, err := lintChart(path, vals, l.Namespace, l.Strict) + linter, err := lintChart(path, vals, l.Namespace, l.Strict, l.Version) if err != nil { result.Errors = append(result.Errors, err) continue @@ -75,7 +76,7 @@ func (l *Lint) Run(paths []string, vals map[string]interface{}) *LintResult { return result } -func lintChart(path string, vals map[string]interface{}, namespace string, strict bool) (support.Linter, error) { +func lintChart(path string, vals map[string]interface{}, namespace string, strict bool, version string) (support.Linter, error) { var chartPath string linter := support.Linter{} @@ -114,5 +115,5 @@ func lintChart(path string, vals map[string]interface{}, namespace string, stric return linter, errors.Wrap(err, "unable to check Chart.yaml file in chart") } - return lint.All(chartPath, vals, namespace, strict), nil + return lint.All(chartPath, vals, namespace, strict, version), nil } diff --git a/pkg/action/lint_test.go b/pkg/action/lint_test.go index 68d3c4bb4..25d31abf6 100644 --- a/pkg/action/lint_test.go +++ b/pkg/action/lint_test.go @@ -28,12 +28,14 @@ var ( chart2MultipleChartLint = "../../cmd/helm/testdata/testcharts/multiplecharts-lint-chart-2" corruptedTgzChart = "../../cmd/helm/testdata/testcharts/corrupted-compressed-chart.tgz" chartWithNoTemplatesDir = "../../cmd/helm/testdata/testcharts/chart-with-no-templates-dir" + chartMissingVersion = "../../cmd/helm/testdata/testcharts/chart-missing-version" ) func TestLintChart(t *testing.T) { tests := []struct { name string chartPath string + version string err bool }{ { @@ -74,16 +76,21 @@ func TestLintChart(t *testing.T) { name: "pre-release-chart", chartPath: "../../cmd/helm/testdata/testcharts/pre-release-chart-0.1.0-alpha.tgz", }, + { + name: "chart missing version and version provided", + chartPath: chartMissingVersion, + version: "1.0", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := lintChart(tt.chartPath, map[string]interface{}{}, namespace, strict) + _, err := lintChart(tt.chartPath, map[string]interface{}{}, namespace, strict, tt.version) switch { case err != nil && !tt.err: t.Errorf("%s", err) case err == nil && tt.err: - t.Errorf("Expected a chart parsing error") + t.Errorf("Expected a chart parsing error %s ww", tt.version) } }) } diff --git a/pkg/action/package.go b/pkg/action/package.go index 19d845cf3..4b9721e08 100644 --- a/pkg/action/package.go +++ b/pkg/action/package.go @@ -60,6 +60,24 @@ func (p *Package) Run(path string, vals map[string]interface{}) (string, error) return "", err } + // validate chart + if err = ch.Validate(); err != nil { + return "", err + } + + // validate sub charts + for _, subChart := range ch.Dependencies() { + if err = subChart.Validate(); err != nil { + return "", err + } + } + + combinedVals, err := chartutil.CoalesceValues(ch, vals) + if err != nil { + return "", err + } + ch.Values = combinedVals + // If version is set, modify the version. if p.Version != "" { if err := setVersion(ch, p.Version); err != nil { diff --git a/pkg/action/show.go b/pkg/action/show.go index 14b59a5ea..767055f28 100644 --- a/pkg/action/show.go +++ b/pkg/action/show.go @@ -69,6 +69,19 @@ func (s *Show) Run(chartpath string) (string, error) { if err != nil { return "", err } + + // validate chart + if err = chrt.Validate(); err != nil { + return "", err + } + + // validate sub charts + for _, subChart := range chrt.Dependencies() { + if err = subChart.Validate(); err != nil { + return "", err + } + } + cf, err := yaml.Marshal(chrt.Metadata) if err != nil { return "", err diff --git a/pkg/chart/loader/archive.go b/pkg/chart/loader/archive.go index 7e187a170..f59604438 100644 --- a/pkg/chart/loader/archive.go +++ b/pkg/chart/loader/archive.go @@ -190,5 +190,10 @@ func LoadArchive(in io.Reader) (*chart.Chart, error) { return nil, err } - return LoadFiles(files) + chart, err := LoadFiles(files) + if err != nil { + return chart, err + } + + return chart, nil } diff --git a/pkg/chart/loader/load.go b/pkg/chart/loader/load.go index dd4fd2dff..20555b25c 100644 --- a/pkg/chart/loader/load.go +++ b/pkg/chart/loader/load.go @@ -143,10 +143,6 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { } } - if err := c.Validate(); err != nil { - return c, err - } - for n, files := range subcharts { var sc *chart.Chart var err error diff --git a/pkg/chart/loader/load_test.go b/pkg/chart/loader/load_test.go index 26513d359..a723cd021 100644 --- a/pkg/chart/loader/load_test.go +++ b/pkg/chart/loader/load_test.go @@ -123,6 +123,19 @@ func TestLoadFile(t *testing.T) { verifyFrobnitz(t, c) verifyChart(t, c) verifyDependencies(t, c) + + l, err = Loader("testdata/chart-missing-version.tgz") + if err != nil { + t.Fatalf("Failed to load testdata: %s", err) + } + c, err = l.Load() + if err != nil { + t.Fatalf("Failed to load testdata: %s", err) + } + + if err = c.Validate(); err == nil { + t.Fatalf("expected a error due to missing version number") + } } func TestLoadFiles(t *testing.T) { @@ -190,13 +203,6 @@ icon: https://example.com/64x64.png if len(c.Templates) != 2 { t.Errorf("Expected number of templates == 2, got %d", len(c.Templates)) } - - if _, err = LoadFiles([]*BufferedFile{}); err == nil { - t.Fatal("Expected err to be non-nil") - } - if err.Error() != "validation: chart.metadata is required" { - t.Errorf("Expected chart metadata missing error, got '%s'", err.Error()) - } } // Packaging the chart on a Windows machine will produce an @@ -281,9 +287,16 @@ func TestLoadInvalidArchive(t *testing.T) { } { illegalChart := filepath.Join(tmpdir, tt.chartname) writeTar(illegalChart, tt.internal, []byte("hello: world")) - _, err = Load(illegalChart) + c, err := Load(illegalChart) + if err == nil { - t.Fatal("expected error when unpacking illegal files") + if c != nil { + if err = c.Validate(); err == nil { + t.Fatalf("expected error when unpacking illegal files %s", tt.chartname) + } + } else { + t.Fatalf("expected error when unpacking illegal files %s", tt.chartname) + } } if !strings.Contains(err.Error(), tt.expectError) { t.Errorf("Expected error to contain %q, got %q for %s", tt.expectError, err.Error(), tt.chartname) @@ -293,7 +306,11 @@ func TestLoadInvalidArchive(t *testing.T) { // Make sure that absolute path gets interpreted as relative illegalChart := filepath.Join(tmpdir, "abs-path.tgz") writeTar(illegalChart, "/Chart.yaml", []byte("hello: world")) - _, err = Load(illegalChart) + // Load only loads the chartFile. Validation will be explicit + c, err := Load(illegalChart) + if err == nil { + err = c.Validate() + } if err.Error() != "validation: chart.metadata.name is required" { t.Error(err) } @@ -301,7 +318,11 @@ func TestLoadInvalidArchive(t *testing.T) { // And just to validate that the above was not spurious illegalChart = filepath.Join(tmpdir, "abs-path2.tgz") writeTar(illegalChart, "files/whatever.yaml", []byte("hello: world")) - _, err = Load(illegalChart) + // Load only loads the chartFile. Validation will be explicit + c, err = Load(illegalChart) + if err == nil { + err = c.Validate() + } if err.Error() != "validation: chart.metadata is required" { t.Error(err) } @@ -309,7 +330,11 @@ func TestLoadInvalidArchive(t *testing.T) { // Finally, test that drive letter gets stripped off on Windows illegalChart = filepath.Join(tmpdir, "abs-winpath.tgz") writeTar(illegalChart, "c:\\Chart.yaml", []byte("hello: world")) - _, err = Load(illegalChart) + // Load only loads the chartFile. Validation will be explicit + c, err = Load(illegalChart) + if err == nil { + err = c.Validate() + } if err.Error() != "validation: chart.metadata.name is required" { t.Error(err) } diff --git a/pkg/chart/loader/testdata/chart-missing-version.tgz b/pkg/chart/loader/testdata/chart-missing-version.tgz new file mode 100644 index 0000000000000000000000000000000000000000..70281d631bbf633283ebb1a135ba13d06c3559e5 GIT binary patch literal 3552 zcmV<64IlC!iwFP!000001MM9BZ`(GuKkKjHIUUmOYWX8}QW)p~xumxXE=z-^>o5#M zK}&SZRT4Fka^h=qfBU^hQZgmWv6H6RZD$U1Wtn_@eBY0JOfaRHYR6nko?NzPER$R$ zohJ|4y3h`WL;SaQFzmYj7VSxIZ+Ot(ANCJ=(62Yx>kXce;lrZQ+;XXCM#z&`OlfjU zzjeR=L0izsf5qZ7qKe4}k%YVRe}Avn-}3)4+B*J!n6fB^x4ufF`_6$3-X9EB@qh22 zyI19Zzq`M8@Pu?9a-pl*|2h9Zed>HmW;`Ad$rPFJh^cwXUd2e3V9G}1+s@66hrPVl z*C|axqNa=_G-gCh^s|;A-?RPNBxDnwFw$z2EFVX3BM+-_5v6K2oC9%}WXuAZOGcyswEmTkS(Y%+3A1{D5rvY_jFE`PTmeKSfFN9w zXXCl1%<09(!*>$k0yi*spjp5NQK zKZ8Vqtq%b5Ut6U0p>-c!6LAcnG{$mMj7@I?5}Zuz%V|gV9Fb3-2v34257#Z%Cwi)N zy(O?VP$E0EQ`%uhB@8(Z)3l%n{06qc|5z%#&qc?A(zq=T`3!bN0Dm3oG(w4KLN8g^ z9?uP+Lgp@5Hse8M{DQg{7W+HWx4!6hBSt8Tn-JWX62Ey_ps9)`0jn-bOWSSQQ1>L# zQh^46$Ppx!wJ#k4;7@yia zE&I^g2uOp4`6B*sQTG)-%8dB|V1SS?ghD77pBh6`RTm$BFjJ-?Q+CF- z{?gbVe;`&L0W8ffnL5{3|o0XWkt8`%Of2> z=xM9^8B-NZPaRCYSY`LkG=;|4X&i!Kiz+jI@tiLUvsKW-K)m8=T2g|?5NVzR6D}CU zI7%)oUHz9TT1xx`dfq&p6Wf{r%Q4p`qR|wo`7Y2P&$yZ&K@!NWRWo{am8l3Xs@Fd{8Sc=an3VIetiehvdhjp@eCO{)$G z`FSot_U9FSFd)oDbF)8X4TnnmhV`N ztN@9s>4RmrI(tG#iBs zJ^{I0j40mRTl<&hP$FU(RUn{_v2kABYo_1ZKmOUkMXSmpuvV_7B)^%$@Sb*>=qcSGY58Rv377T0C$7{?UJ z^7Afs-TkHTpxGrPAMp0BbQ!JQjL^U#mH?SlML2iH0L<5Ngd6Vv*)7@o?E$RQ|Gi;- z|F7T2`j4&tfBm-c?)YsRg|_>>_6crD1|PsyJlH<@pS7*i|FbvmUSIg?T7AJB>-PV8 zgJC`XI~Z*J|D&{?Pas9s$8R*%duQlY$q#Q&@iaQilLX5b;AhDSN2oQjVl|)(_$=wO zgpMN?7K)eAA`MPu}|B-{!foC+HL zokn<~oZZF;XlR~B$mSujvAP$L2rV?T_*>q zg)_he0CNDNsL#;NUXwxwB8R|*mXhvst%tr=Mx%DEn+C~PEbErgc!jVWMPR{Mc5=Rf z#-6`9{&05q{p%X>QW0*d2oY&D*HFB=O9{n`)g=_K8Y(DSD;2N^xHN2su&wu8a#hf` z*FW&P@LzBAviq`Y%PPoR%+P%4$yZ3TL*W^9~NA91ji(BN{gjtk`#_QcJQJKplxV z#p2r5-(L0n-k{yZDn^>pF^{<7FdPeVZAR*U$5nAQR?k(z!d{6C($!1<<4aS@*BgV` z>{W}h+#&2*u>>DwwXZQ|Ci6WpglbNC7u0&tlkcWeUdu0MM{LCw*@`ol!BU89w#!z? zg^9cIyxT0u3EWrp#Emu`>bnQ-c8rq(nAXvl&~QIQxt5x5fMfqPK{x z!MuW$!QITE6N@F(w396tpwpM?N!M~>vskS;i(I5Ci#DdgN5Hy%cxjd9Y=x*bbUiQX zpGqBcdRA!DOtatHLT%gEY|Z|EW;EhUoW9f6`Tv7@{a0^)*zIrq|6{Zra%}664bOGP z-0YXU9r6yZw{X0W^3&n>rx^UlRN>VZyf}m%j#&_)*PhXgV+0{>rkzrW7aVv3Uv=V9 zMwq_mxkXze((^oX3}b4%Y%L5p@=KoJkxc4Tx|~eTggOVSja6#iQehMM`4`WJL+Dh^kQHkd*S4x3X zxY1Ki_DdJSwB?Qv)U;`!4?_`YOqrd~ric}c`mk#XJYaq#0vdU4bbw-MA}I5;gG1eb zrUIZ19W8|ylkF4%MAKZ}qHL+)-H&*gET?z_E@QZli_RV83`9?AF`t(g5*DRQ`nsPW zko(V=x!R^LqXj_AfQ#Iq6-))nan|4qf|1BHL<;y+-$!~eYj#((`mZyWzTMynj__r1jNKQ*E7 zLMDTZr%LxdB>!YlY_F=K?O)uF_8<;Cw7=jv3dBZ?oA4B*QwBH{t9aZxI!{#V)RFgnf2mu>I~s!=+$;e%2S4Z&9AQp+$8#Y zdxw%HbBA{JT7Sl1F${@_JuBof(U*g)=VSwjLvfWk$zza%BG7eLH)a^6mLSdO+ORU3 z2Ix9)K+g@NV-!!~c&tw?NQ~!v7)@N7{Gv9Kk2h3Hq3-#;eZRZi#aPjr?Z1!pbo?>_ zad?0^*42Lv`}_6$e=ywI|3_&%@c@IVg03__vn1g32m>Yf#%B_R`zdZK|ri_ph=#hvL z0XwRLbl>xjFFssA$cu)-{!nDCI3o5#M zK}&SZRT4Fka^h=qfBU^hQZgmWv6H6RZD$U1Wtn_@eBY0JOfaRHYR6nko?NzPER$R$ zohJ|4y3h`WL;SaQFzmYj7VSxIZ+Ot(ANCJ=(62Yx>kXce;lrZQ+;XXCM#z&`OlfjU zzjeR=L0izsf5qZ7qKe4}k%YVRe}Avn-}3)4+B*J!n6fB^x4ufF`_6$3-X9EB@qh22 zyI19Zzq`M8@Pu?9a-pl*|2h9Zed>HmW;`Ad$rPFJh^cwXUd2e3V9G}1+s@66hrPVl z*C|axqNa=_G-gCh^s|;A-?RPNBxDnwFw$z2EFVX3BM+-_5v6K2oC9%}WXuAZOGcyswEmTkS(Y%+3A1{D5rvY_jFE`PTmeKSfFN9w zXXCl1%<09(!*>$k0yi*spjp5NQK zKZ8Vqtq%b5Ut6U0p>-c!6LAcnG{$mMj7@I?5}Zuz%V|gV9Fb3-2v34257#Z%Cwi)N zy(O?VP$E0EQ`%uhB@8(Z)3l%n{06qc|5z%#&qc?A(zq=T`3!bN0Dm3oG(w4KLN8g^ z9?uP+Lgp@5Hse8M{DQg{7W+HWx4!6hBSt8Tn-JWX62Ey_ps9)`0jn-bOWSSQQ1>L# zQh^46$Ppx!wJ#k4;7@yia zE&I^g2uOp4`6B*sQTG)-%8dB|V1SS?ghD77pBh6`RTm$BFjJ-?Q+CF- z{?gbVe;`&L0W8ffnL5{3|o0XWkt8`%Of2> z=xM9^8B-NZPaRCYSY`LkG=;|4X&i!Kiz+jI@tiLUvsKW-K)m8=T2g|?5NVzR6D}CU zI7%)oUHz9TT1xx`dfq&p6Wf{r%Q4p`qR|wo`7Y2P&$yZ&K@!NWRWo{am8l3Xs@Fd{8Sc=an3VIetiehvdhjp@eCO{)$G z`FSot_U9FSFd)oDbF)8X4TnnmhV`N ztN@9s>4RmrI(tG#iBs zJ^{I0j40mRTl<&hP$FU(RUn{_v2kABYo_1ZKmOUkMXSmpuvV_7B)^%$@Sb*>=qcSGY58Rv377T0C$7{?UJ z^7Afs-TkHTpxGrPAMp0BbQ!JQjL^U#mH?SlML2iH0L<5Ngd6Vv*)7@o?E$RQ|Gi;- z|F7T2`j4&tfBm-c?)YsRg|_>>_6crD1|PsyJlH<@pS7*i|FbvmUSIg?T7AJB>-PV8 zgJC`XI~Z*J|D&{?Pas9s$8R*%duQlY$q#Q&@iaQilLX5b;AhDSN2oQjVl|)(_$=wO zgpMN?7K)eAA`MPu}|B-{!foC+HL zokn<~oZZF;XlR~B$mSujvAP$L2rV?T_*>q zg)_he0CNDNsL#;NUXwxwB8R|*mXhvst%tr=Mx%DEn+C~PEbErgc!jVWMPR{Mc5=Rf z#-6`9{&05q{p%X>QW0*d2oY&D*HFB=O9{n`)g=_K8Y(DSD;2N^xHN2su&wu8a#hf` z*FW&P@LzBAviq`Y%PPoR%+P%4$yZ3TL*W^9~NA91ji(BN{gjtk`#_QcJQJKplxV z#p2r5-(L0n-k{yZDn^>pF^{<7FdPeVZAR*U$5nAQR?k(z!d{6C($!1<<4aS@*BgV` z>{W}h+#&2*u>>DwwXZQ|Ci6WpglbNC7u0&tlkcWeUdu0MM{LCw*@`ol!BU89w#!z? zg^9cIyxT0u3EWrp#Emu`>bnQ-c8rq(nAXvl&~QIQxt5x5fMfqPK{x z!MuW$!QITE6N@F(w396tpwpM?N!M~>vskS;i(I5Ci#DdgN5Hy%cxjd9Y=x*bbUiQX zpGqBcdRA!DOtatHLT%gEY|Z|EW;EhUoW9f6`Tv7@{a0^)*zIrq|6{Zra%}664bOGP z-0YXU9r6yZw{X0W^3&n>rx^UlRN>VZyf}m%j#&_)*PhXgV+0{>rkzrW7aVv3Uv=V9 zMwq_mxkXze((^oX3}b4%Y%L5p@=KoJkxc4Tx|~eTggOVSja6#iQehMM`4`WJL+Dh^kQHkd*S4x3X zxY1Ki_DdJSwB?Qv)U;`!4?_`YOqrd~ric}c`mk#XJYaq#0vdU4bbw-MA}I5;gG1eb zrUIZ19W8|ylkF4%MAKZ}qHL+)-H&*gET?z_E@QZli_RV83`9?AF`t(g5*DRQ`nsPW zko(V=x!R^LqXj_AfQ#Iq6-))nan|4qf|1BHL<;y+-$!~eYj#((`mZyWzTMynj__r1jNKQ*E7 zLMDTZr%LxdB>!YlY_F=K?O)uF_8<;Cw7=jv3dBZ?oA4B*QwBH{t9aZxI!{#V)RFgnf2mu>I~s!=+$;e%2S4Z&9AQp+$8#Y zdxw%HbBA{JT7Sl1F${@_JuBof(U*g)=VSwjLvfWk$zza%BG7eLH)a^6mLSdO+ORU3 z2Ix9)K+g@NV-!!~c&tw?NQ~!v7)@N7{Gv9Kk2h3Hq3-#;eZRZi#aPjr?Z1!pbo?>_ zad?0^*42Lv`}_6$e=ywI|3_&%@c@IVg03__vn1g32m>Yf#%B_R`zdZK|ri_ph=#hvL z0XwRLbl>xjFFssA$cu)-{!nDCI3