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 000000000..70281d631 Binary files /dev/null and b/pkg/chart/loader/testdata/chart-missing-version.tgz differ diff --git a/pkg/chartutil/dependencies_test.go b/pkg/chartutil/dependencies_test.go index ecd632540..9259d52a2 100644 --- a/pkg/chartutil/dependencies_test.go +++ b/pkg/chartutil/dependencies_test.go @@ -31,6 +31,20 @@ func loadChart(t *testing.T, path string) *chart.Chart { if err != nil { t.Fatalf("failed to load testdata: %s", err) } + + // validate chart + if err = c.Validate(); err != nil { + t.Fatalf("failed to load testdata: %s", err) + + } + + // validate sub charts + for _, subChart := range c.Dependencies() { + if err = subChart.Validate(); err != nil { + t.Fatalf("failed to load testdata: %s", err) + } + } + return c } diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index cb139f824..badccfa6f 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -183,7 +183,25 @@ func (m *Manager) loadChartDir() (*chart.Chart, error) { } else if !fi.IsDir() { return nil, errors.New("only unpacked charts can be updated") } - return loader.LoadDir(m.ChartPath) + + chart, err := loader.LoadDir(m.ChartPath) + if err != nil { + return chart, err + } + + // validate chart + if err = chart.Validate(); err != nil { + return chart, err + } + + // validate sub charts + for _, subChart := range chart.Dependencies() { + if err = subChart.Validate(); err != nil { + return chart, err + } + } + + return chart, nil } // resolve takes a list of dependencies and translates them into an exact version to download. @@ -236,6 +254,18 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { return fmt.Errorf("Unable to load chart: %v", err) } + // validate chart + if err = ch.Validate(); err != nil { + return fmt.Errorf("validation on chart failed: %v", err) + } + + // validate sub charts + for _, subChart := range ch.Dependencies() { + if err = subChart.Validate(); err != nil { + return fmt.Errorf("validation on subchart failed: %v", err) + } + } + constraint, err := semver.NewConstraint(dep.Version) if err != nil { return fmt.Errorf("Dependency %s has an invalid version/constraint format: %s", dep.Name, err) @@ -676,6 +706,18 @@ func tarFromLocalDir(chartpath, name, repo, version string) (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 + } + } + constraint, err := semver.NewConstraint(version) if err != nil { return "", errors.Wrapf(err, "dependency %s has an invalid version/constraint format", name) diff --git a/pkg/lint/lint.go b/pkg/lint/lint.go index d47951671..990a39273 100644 --- a/pkg/lint/lint.go +++ b/pkg/lint/lint.go @@ -24,13 +24,13 @@ import ( ) // All runs all of the available linters on the given base directory. -func All(basedir string, values map[string]interface{}, namespace string, strict bool) support.Linter { +func All(basedir string, values map[string]interface{}, namespace string, strict bool, version string) support.Linter { // Using abs path to get directory context chartDir, _ := filepath.Abs(basedir) linter := support.Linter{ChartDir: chartDir} - rules.Chartfile(&linter) + rules.Chartfile(&linter, version) rules.Values(&linter) - rules.Templates(&linter, values, namespace, strict) + rules.Templates(&linter, values, namespace, strict, version) return linter } diff --git a/pkg/lint/lint_test.go b/pkg/lint/lint_test.go index 2a982d088..51bd73b78 100644 --- a/pkg/lint/lint_test.go +++ b/pkg/lint/lint_test.go @@ -29,12 +29,13 @@ const namespace = "testNamespace" const strict = false const badChartDir = "rules/testdata/badchartfile" +const missingVersionOnChart = "rules/testdata/missingversionchartfile" const badValuesFileDir = "rules/testdata/badvaluesfile" const badYamlFileDir = "rules/testdata/albatross" const goodChartDir = "rules/testdata/goodone" func TestBadChart(t *testing.T) { - m := All(badChartDir, values, namespace, strict).Messages + m := All(badChartDir, values, namespace, strict, "").Messages if len(m) != 7 { t.Errorf("Number of errors %v", len(m)) t.Errorf("All didn't fail with expected errors, got %#v", m) @@ -79,7 +80,7 @@ func TestBadChart(t *testing.T) { } func TestInvalidYaml(t *testing.T) { - m := All(badYamlFileDir, values, namespace, strict).Messages + m := All(badYamlFileDir, values, namespace, strict, "").Messages if len(m) != 1 { t.Fatalf("All didn't fail with expected errors, got %#v", m) } @@ -89,7 +90,7 @@ func TestInvalidYaml(t *testing.T) { } func TestBadValues(t *testing.T) { - m := All(badValuesFileDir, values, namespace, strict).Messages + m := All(badValuesFileDir, values, namespace, strict, "").Messages if len(m) < 1 { t.Fatalf("All didn't fail with expected errors, got %#v", m) } @@ -99,7 +100,24 @@ func TestBadValues(t *testing.T) { } func TestGoodChart(t *testing.T) { - m := All(goodChartDir, values, namespace, strict).Messages + m := All(goodChartDir, values, namespace, strict, "").Messages + if len(m) != 0 { + t.Errorf("All failed but shouldn't have: %#v", m) + } +} + +func TestMissingVersionChart(t *testing.T) { + // version not provided in param + m := All(missingVersionOnChart, values, namespace, strict, "").Messages + if len(m) < 1 { + t.Fatalf("All didn't fail with expected errors, got %#v", m) + } + if !strings.Contains(m[0].Err.Error(), "version is required") { + t.Errorf("All didn't have the error for invalid key format: %s", m[0].Err) + } + + // version provided in param + m = All(missingVersionOnChart, values, namespace, strict, "1.0").Messages if len(m) != 0 { t.Errorf("All failed but shouldn't have: %#v", m) } diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 91a64fe13..503d2c370 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -31,7 +31,7 @@ import ( ) // Chartfile runs a set of linter rules related to Chart.yaml file -func Chartfile(linter *support.Linter) { +func Chartfile(linter *support.Linter, version string) { chartFileName := "Chart.yaml" chartPath := filepath.Join(linter.ChartDir, chartFileName) @@ -45,6 +45,10 @@ func Chartfile(linter *support.Linter) { return } + if chartFile.Version == "" && version != "" { + chartFile.Version = version + } + linter.RunLinterRule(support.ErrorSev, chartFileName, validateChartName(chartFile)) // Chart metadata diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go index d032dd12f..6e018fe8e 100644 --- a/pkg/lint/rules/chartfile_test.go +++ b/pkg/lint/rules/chartfile_test.go @@ -30,7 +30,8 @@ import ( ) const ( - badChartDir = "testdata/badchartfile" + badChartDir = "testdata/badchartfile" + chartMissingVersion = "testdata/missingversionchartfile" ) var ( @@ -185,7 +186,7 @@ func TestValidateChartIconURL(t *testing.T) { func TestChartfile(t *testing.T) { linter := support.Linter{ChartDir: badChartDir} - Chartfile(&linter) + Chartfile(&linter, "") msgs := linter.Messages if len(msgs) != 6 { @@ -215,5 +216,18 @@ func TestChartfile(t *testing.T) { if !strings.Contains(msgs[5].Err.Error(), "dependencies are not valid in the Chart file with apiVersion") { t.Errorf("Unexpected message 5: %s", msgs[5].Err) } +} + +func TestChartfileMissingVersion(t *testing.T) { + linter := support.Linter{ChartDir: chartMissingVersion} + Chartfile(&linter, "") + msgs := linter.Messages + if len(msgs) != 1 { + t.Errorf("Expected 1 errors, got %d", len(msgs)) + } + + if !strings.Contains(msgs[0].Err.Error(), "version is required") { + t.Errorf("Unexpected message 0: %s", msgs[0].Err) + } } diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index 5c6cd7336..9499d4de6 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -36,7 +36,7 @@ var ( ) // Templates lints the templates in the Linter. -func Templates(linter *support.Linter, values map[string]interface{}, namespace string, strict bool) { +func Templates(linter *support.Linter, values map[string]interface{}, namespace string, strict bool, version string) { path := "templates/" templatesPath := filepath.Join(linter.ChartDir, path) @@ -56,6 +56,26 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace return } + if chart.Metadata.Version == "" && version != "" { + chart.Metadata.Version = version + } + + // validate chart + err = chart.Validate() + validChart := linter.RunLinterRule(support.ErrorSev, path, err) + if !validChart { + return + } + + // validate sub charts + for _, subChart := range chart.Dependencies() { + err = subChart.Validate() + validChart := linter.RunLinterRule(support.ErrorSev, path, err) + if !validChart { + return + } + } + options := chartutil.ReleaseOptions{ Name: "testRelease", Namespace: namespace, diff --git a/pkg/lint/rules/template_test.go b/pkg/lint/rules/template_test.go index ddb46aba0..e24ea446b 100644 --- a/pkg/lint/rules/template_test.go +++ b/pkg/lint/rules/template_test.go @@ -26,6 +26,7 @@ import ( ) const templateTestBasedir = "./testdata/albatross" +const templateMissingVersionTestBasedir = "./testdata/missingversionchartfile" func TestValidateAllowedExtension(t *testing.T) { var failTest = []string{"/foo", "/test.toml"} @@ -51,7 +52,7 @@ const strict = false func TestTemplateParsing(t *testing.T) { linter := support.Linter{ChartDir: templateTestBasedir} - Templates(&linter, values, namespace, strict) + Templates(&linter, values, namespace, strict, "") res := linter.Messages if len(res) != 1 { @@ -74,7 +75,7 @@ func TestTemplateIntegrationHappyPath(t *testing.T) { defer os.Rename(ignoredTemplatePath, wrongTemplatePath) linter := support.Linter{ChartDir: templateTestBasedir} - Templates(&linter, values, namespace, strict) + Templates(&linter, values, namespace, strict, "") res := linter.Messages if len(res) != 0 { @@ -82,9 +83,23 @@ func TestTemplateIntegrationHappyPath(t *testing.T) { } } +func TestMissingVersionTemplateParsing(t *testing.T) { + linter := support.Linter{ChartDir: templateMissingVersionTestBasedir} + Templates(&linter, values, namespace, strict, "") + res := linter.Messages + + if len(res) != 1 { + t.Fatalf("Expected one error, got %d, %v", len(res), res) + } + + if !strings.Contains(res[0].Err.Error(), "chart.metadata.version is required") { + t.Errorf("Unexpected error: %s", res[0]) + } +} + func TestV3Fail(t *testing.T) { linter := support.Linter{ChartDir: "./testdata/v3-fail"} - Templates(&linter, values, namespace, strict) + Templates(&linter, values, namespace, strict, "") res := linter.Messages if len(res) != 3 { diff --git a/pkg/lint/rules/testdata/missingversionchartfile/Chart.yaml b/pkg/lint/rules/testdata/missingversionchartfile/Chart.yaml new file mode 100644 index 000000000..32d953ce3 --- /dev/null +++ b/pkg/lint/rules/testdata/missingversionchartfile/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +name: goodone +description: good testing chart +icon: http://riverrun.io diff --git a/pkg/lint/rules/testdata/missingversionchartfile/templates/goodone.yaml b/pkg/lint/rules/testdata/missingversionchartfile/templates/goodone.yaml new file mode 100644 index 000000000..0e77f46f2 --- /dev/null +++ b/pkg/lint/rules/testdata/missingversionchartfile/templates/goodone.yaml @@ -0,0 +1,2 @@ +metadata: + name: {{.Values.name | default "foo" | title}} diff --git a/pkg/lint/rules/testdata/missingversionchartfile/values.yaml b/pkg/lint/rules/testdata/missingversionchartfile/values.yaml new file mode 100644 index 000000000..fe9abd983 --- /dev/null +++ b/pkg/lint/rules/testdata/missingversionchartfile/values.yaml @@ -0,0 +1 @@ +name: "goodone here" diff --git a/pkg/provenance/sign.go b/pkg/provenance/sign.go index 5d16779f1..375444bc6 100644 --- a/pkg/provenance/sign.go +++ b/pkg/provenance/sign.go @@ -322,6 +322,18 @@ func messageBlock(chartpath string) (*bytes.Buffer, error) { return b, err } + // validate chart + if err = chart.Validate(); err != nil { + return b, err + } + + // validate sub charts + for _, subChart := range chart.Dependencies() { + if err = subChart.Validate(); err != nil { + return b, err + } + } + // Buffer a hash + checksums YAML file data, err := yaml.Marshal(chart.Metadata) if err != nil { diff --git a/pkg/provenance/sign_test.go b/pkg/provenance/sign_test.go index 1f4d2d232..81193087f 100644 --- a/pkg/provenance/sign_test.go +++ b/pkg/provenance/sign_test.go @@ -46,6 +46,8 @@ const ( testChartfile = "testdata/hashtest-1.2.3.tgz" + testChartfileMissingVersion = "testdata/chart-missing-version.tgz" + // testSigBlock points to a signature generated by an external tool. // This file was generated with GnuPG: // gpg --clearsign -u helm-test --openpgp testdata/msgblock.yaml @@ -85,6 +87,13 @@ func TestMessageBlock(t *testing.T) { } } +func TestMessageBlockMissingVersion(t *testing.T) { + _, err := messageBlock(testChartfileMissingVersion) + if err == nil { + t.Fatal("expected error since missing version number in chart") + } +} + func TestParseMessageBlock(t *testing.T) { md, sc, err := parseMessageBlock([]byte(testMessageBlock)) if err != nil { diff --git a/pkg/provenance/testdata/chart-missing-version.tgz b/pkg/provenance/testdata/chart-missing-version.tgz new file mode 100644 index 000000000..70281d631 Binary files /dev/null and b/pkg/provenance/testdata/chart-missing-version.tgz differ diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index 38b6b8fb0..c224f3cd9 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -177,6 +177,18 @@ func (r *ChartRepository) generateIndex() 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 + } + } + digest, err := provenance.DigestFile(path) if err != nil { return err