From fd157b5a35a65ffce6892d05dafae0f67e69fc97 Mon Sep 17 00:00:00 2001 From: Torsten Walter Date: Mon, 7 Sep 2020 21:08:24 +0200 Subject: [PATCH 01/25] prepare testdata Signed-off-by: Torsten Walter --- .../output/template-name-template.txt | 30 ++++++++++++++- cmd/helm/testdata/output/template-set.txt | 30 ++++++++++++++- .../output/template-show-only-glob.txt | 3 +- .../testdata/output/template-values-files.txt | 30 ++++++++++++++- .../output/template-with-api-version.txt | 30 ++++++++++++++- .../testdata/output/template-with-crds.txt | 37 +++++++++++++++++-- cmd/helm/testdata/output/template.txt | 30 ++++++++++++++- .../testcharts/subchart/crds/crdA.yaml | 7 ++-- .../subchart/templates/subdir/role.yaml | 3 +- .../subchart/templates/tests/test-config.yaml | 6 +++ .../templates/tests/test-nothing.yaml | 17 +++++++++ 11 files changed, 209 insertions(+), 14 deletions(-) create mode 100644 cmd/helm/testdata/testcharts/subchart/templates/tests/test-config.yaml create mode 100644 cmd/helm/testdata/testcharts/subchart/templates/tests/test-nothing.yaml diff --git a/cmd/helm/testdata/output/template-name-template.txt b/cmd/helm/testdata/output/template-name-template.txt index 84a9e565c..741630922 100644 --- a/cmd/helm/testdata/output/template-name-template.txt +++ b/cmd/helm/testdata/output/template-name-template.txt @@ -5,13 +5,22 @@ kind: ServiceAccount metadata: name: subchart-sa --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "foobar-YWJj-baz-testconfig" +data: + message: Hello World +--- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: name: subchart-role rules: -- resources: ["*"] +- apiGroups: [""] + resources: ["pods"] verbs: ["get","list","watch"] --- # Source: subchart/templates/subdir/rolebinding.yaml @@ -82,3 +91,22 @@ spec: name: nginx selector: app.kubernetes.io/name: subchart +--- +# Source: subchart/templates/tests/test-nothing.yaml +apiVersion: v1 +kind: Pod +metadata: + name: "foobar-YWJj-baz-test" + annotations: + "helm.sh/hook": test +spec: + containers: + - name: test + image: "alpine:latest" + envFrom: + - configMapRef: + name: "foobar-YWJj-baz-testconfig" + command: + - echo + - "$message" + restartPolicy: Never diff --git a/cmd/helm/testdata/output/template-set.txt b/cmd/helm/testdata/output/template-set.txt index 1cb97723e..42a08c391 100644 --- a/cmd/helm/testdata/output/template-set.txt +++ b/cmd/helm/testdata/output/template-set.txt @@ -5,13 +5,22 @@ kind: ServiceAccount metadata: name: subchart-sa --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "RELEASE-NAME-testconfig" +data: + message: Hello World +--- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: name: subchart-role rules: -- resources: ["*"] +- apiGroups: [""] + resources: ["pods"] verbs: ["get","list","watch"] --- # Source: subchart/templates/subdir/rolebinding.yaml @@ -82,3 +91,22 @@ spec: name: apache selector: app.kubernetes.io/name: subchart +--- +# Source: subchart/templates/tests/test-nothing.yaml +apiVersion: v1 +kind: Pod +metadata: + name: "RELEASE-NAME-test" + annotations: + "helm.sh/hook": test +spec: + containers: + - name: test + image: "alpine:latest" + envFrom: + - configMapRef: + name: "RELEASE-NAME-testconfig" + command: + - echo + - "$message" + restartPolicy: Never diff --git a/cmd/helm/testdata/output/template-show-only-glob.txt b/cmd/helm/testdata/output/template-show-only-glob.txt index cc651f596..b2d2b1c2d 100644 --- a/cmd/helm/testdata/output/template-show-only-glob.txt +++ b/cmd/helm/testdata/output/template-show-only-glob.txt @@ -5,7 +5,8 @@ kind: Role metadata: name: subchart-role rules: -- resources: ["*"] +- apiGroups: [""] + resources: ["pods"] verbs: ["get","list","watch"] --- # Source: subchart/templates/subdir/rolebinding.yaml diff --git a/cmd/helm/testdata/output/template-values-files.txt b/cmd/helm/testdata/output/template-values-files.txt index 1cb97723e..42a08c391 100644 --- a/cmd/helm/testdata/output/template-values-files.txt +++ b/cmd/helm/testdata/output/template-values-files.txt @@ -5,13 +5,22 @@ kind: ServiceAccount metadata: name: subchart-sa --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "RELEASE-NAME-testconfig" +data: + message: Hello World +--- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: name: subchart-role rules: -- resources: ["*"] +- apiGroups: [""] + resources: ["pods"] verbs: ["get","list","watch"] --- # Source: subchart/templates/subdir/rolebinding.yaml @@ -82,3 +91,22 @@ spec: name: apache selector: app.kubernetes.io/name: subchart +--- +# Source: subchart/templates/tests/test-nothing.yaml +apiVersion: v1 +kind: Pod +metadata: + name: "RELEASE-NAME-test" + annotations: + "helm.sh/hook": test +spec: + containers: + - name: test + image: "alpine:latest" + envFrom: + - configMapRef: + name: "RELEASE-NAME-testconfig" + command: + - echo + - "$message" + restartPolicy: Never diff --git a/cmd/helm/testdata/output/template-with-api-version.txt b/cmd/helm/testdata/output/template-with-api-version.txt index ea4b5c96b..da77c51c0 100644 --- a/cmd/helm/testdata/output/template-with-api-version.txt +++ b/cmd/helm/testdata/output/template-with-api-version.txt @@ -5,13 +5,22 @@ kind: ServiceAccount metadata: name: subchart-sa --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "RELEASE-NAME-testconfig" +data: + message: Hello World +--- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: name: subchart-role rules: -- resources: ["*"] +- apiGroups: [""] + resources: ["pods"] verbs: ["get","list","watch"] --- # Source: subchart/templates/subdir/rolebinding.yaml @@ -83,3 +92,22 @@ spec: name: nginx selector: app.kubernetes.io/name: subchart +--- +# Source: subchart/templates/tests/test-nothing.yaml +apiVersion: v1 +kind: Pod +metadata: + name: "RELEASE-NAME-test" + annotations: + "helm.sh/hook": test +spec: + containers: + - name: test + image: "alpine:latest" + envFrom: + - configMapRef: + name: "RELEASE-NAME-testconfig" + command: + - echo + - "$message" + restartPolicy: Never diff --git a/cmd/helm/testdata/output/template-with-crds.txt b/cmd/helm/testdata/output/template-with-crds.txt index fa2a79bac..57e770176 100644 --- a/cmd/helm/testdata/output/template-with-crds.txt +++ b/cmd/helm/testdata/output/template-with-crds.txt @@ -3,13 +3,14 @@ apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: - name: testCRDs + name: testcrds.testcrdgroups.example.com spec: - group: testCRDGroups + group: testcrdgroups.example.com + version: v1alpha1 names: kind: TestCRD listKind: TestCRDList - plural: TestCRDs + plural: testcrds shortNames: - tc singular: authconfig @@ -21,13 +22,22 @@ kind: ServiceAccount metadata: name: subchart-sa --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "RELEASE-NAME-testconfig" +data: + message: Hello World +--- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: name: subchart-role rules: -- resources: ["*"] +- apiGroups: [""] + resources: ["pods"] verbs: ["get","list","watch"] --- # Source: subchart/templates/subdir/rolebinding.yaml @@ -99,3 +109,22 @@ spec: name: nginx selector: app.kubernetes.io/name: subchart +--- +# Source: subchart/templates/tests/test-nothing.yaml +apiVersion: v1 +kind: Pod +metadata: + name: "RELEASE-NAME-test" + annotations: + "helm.sh/hook": test +spec: + containers: + - name: test + image: "alpine:latest" + envFrom: + - configMapRef: + name: "RELEASE-NAME-testconfig" + command: + - echo + - "$message" + restartPolicy: Never diff --git a/cmd/helm/testdata/output/template.txt b/cmd/helm/testdata/output/template.txt index 9195f98b7..b2c65a4e1 100644 --- a/cmd/helm/testdata/output/template.txt +++ b/cmd/helm/testdata/output/template.txt @@ -5,13 +5,22 @@ kind: ServiceAccount metadata: name: subchart-sa --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "RELEASE-NAME-testconfig" +data: + message: Hello World +--- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: name: subchart-role rules: -- resources: ["*"] +- apiGroups: [""] + resources: ["pods"] verbs: ["get","list","watch"] --- # Source: subchart/templates/subdir/rolebinding.yaml @@ -82,3 +91,22 @@ spec: name: nginx selector: app.kubernetes.io/name: subchart +--- +# Source: subchart/templates/tests/test-nothing.yaml +apiVersion: v1 +kind: Pod +metadata: + name: "RELEASE-NAME-test" + annotations: + "helm.sh/hook": test +spec: + containers: + - name: test + image: "alpine:latest" + envFrom: + - configMapRef: + name: "RELEASE-NAME-testconfig" + command: + - echo + - "$message" + restartPolicy: Never diff --git a/cmd/helm/testdata/testcharts/subchart/crds/crdA.yaml b/cmd/helm/testdata/testcharts/subchart/crds/crdA.yaml index fca77fd4b..ad770b632 100644 --- a/cmd/helm/testdata/testcharts/subchart/crds/crdA.yaml +++ b/cmd/helm/testdata/testcharts/subchart/crds/crdA.yaml @@ -1,13 +1,14 @@ apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: - name: testCRDs + name: testcrds.testcrdgroups.example.com spec: - group: testCRDGroups + group: testcrdgroups.example.com + version: v1alpha1 names: kind: TestCRD listKind: TestCRDList - plural: TestCRDs + plural: testcrds shortNames: - tc singular: authconfig diff --git a/cmd/helm/testdata/testcharts/subchart/templates/subdir/role.yaml b/cmd/helm/testdata/testcharts/subchart/templates/subdir/role.yaml index 91b954e5f..31cff9200 100644 --- a/cmd/helm/testdata/testcharts/subchart/templates/subdir/role.yaml +++ b/cmd/helm/testdata/testcharts/subchart/templates/subdir/role.yaml @@ -3,5 +3,6 @@ kind: Role metadata: name: {{ .Chart.Name }}-role rules: -- resources: ["*"] +- apiGroups: [""] + resources: ["pods"] verbs: ["get","list","watch"] diff --git a/cmd/helm/testdata/testcharts/subchart/templates/tests/test-config.yaml b/cmd/helm/testdata/testcharts/subchart/templates/tests/test-config.yaml new file mode 100644 index 000000000..de639e03b --- /dev/null +++ b/cmd/helm/testdata/testcharts/subchart/templates/tests/test-config.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: "{{ .Release.Name }}-testconfig" +data: + message: Hello World diff --git a/cmd/helm/testdata/testcharts/subchart/templates/tests/test-nothing.yaml b/cmd/helm/testdata/testcharts/subchart/templates/tests/test-nothing.yaml new file mode 100644 index 000000000..0fe6dbbf3 --- /dev/null +++ b/cmd/helm/testdata/testcharts/subchart/templates/tests/test-nothing.yaml @@ -0,0 +1,17 @@ +apiVersion: v1 +kind: Pod +metadata: + name: "{{ .Release.Name }}-test" + annotations: + "helm.sh/hook": test +spec: + containers: + - name: test + image: "alpine:latest" + envFrom: + - configMapRef: + name: "{{ .Release.Name }}-testconfig" + command: + - echo + - "$message" + restartPolicy: Never From 3d66daeb55d947c4d30d542dfb7459afc21a3c10 Mon Sep 17 00:00:00 2001 From: Torsten Walter Date: Tue, 8 Sep 2020 11:16:16 +0200 Subject: [PATCH 02/25] fix(helm): allow skipping manifests in tests directories When helm template is called with `--skip-tests` no manifests in tests directories are rendered. No matter if they have a `"helm.sh/hook": test` annotation or not. This helps to avoid rendering manifests which are only used for test execution. Closes #8691 Signed-off-by: Torsten Walter --- cmd/helm/template.go | 3 + cmd/helm/template_test.go | 5 + .../testdata/output/template-no-tests.txt | 105 ++++++++++++++++++ pkg/action/action.go | 17 ++- pkg/action/install.go | 3 +- pkg/action/upgrade.go | 2 +- 6 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 cmd/helm/testdata/output/template-no-tests.txt diff --git a/cmd/helm/template.go b/cmd/helm/template.go index 6123d29d4..e504bc774 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -47,6 +47,7 @@ faked locally. Additionally, none of the server-side testing of chart validity func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { var validate bool var includeCrds bool + var skipTests bool client := action.NewInstall(cfg) valueOpts := &values.Options{} var extraAPIs []string @@ -67,6 +68,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.ClientOnly = !validate client.APIVersions = chartutil.VersionSet(extraAPIs) client.IncludeCRDs = includeCrds + client.SkipTests = skipTests rel, err := runInstall(args, client, valueOpts, out) if err != nil && !settings.Debug { @@ -163,6 +165,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.StringVar(&client.OutputDir, "output-dir", "", "writes the executed templates to files in output-dir instead of stdout") f.BoolVar(&validate, "validate", false, "validate your manifests against the Kubernetes cluster you are currently pointing at. This is the same validation performed on an install") f.BoolVar(&includeCrds, "include-crds", false, "include CRDs in the templated output") + f.BoolVar(&skipTests, "skip-tests", false, "skip tests and manifests in tests directories from templated output") f.BoolVar(&client.IsUpgrade, "is-upgrade", false, "set .Release.IsUpgrade instead of .Release.IsInstall") f.StringArrayVarP(&extraAPIs, "api-versions", "a", []string{}, "Kubernetes api versions used for Capabilities.APIVersions") f.BoolVar(&client.UseReleaseName, "release-name", false, "use release name in the output-dir path.") diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go index 6f7ca939d..dd30b3836 100644 --- a/cmd/helm/template_test.go +++ b/cmd/helm/template_test.go @@ -121,6 +121,11 @@ func TestTemplateCmd(t *testing.T) { wantError: true, golden: "output/template-with-invalid-yaml-debug.txt", }, + { + name: "template with skip-tests", + cmd: fmt.Sprintf(`template '%s' --skip-tests`, chartPath), + golden: "output/template-no-tests.txt", + }, } runTestCmd(t, tests) } diff --git a/cmd/helm/testdata/output/template-no-tests.txt b/cmd/helm/testdata/output/template-no-tests.txt new file mode 100644 index 000000000..de537c214 --- /dev/null +++ b/cmd/helm/testdata/output/template-no-tests.txt @@ -0,0 +1,105 @@ +--- +# Source: subchart/templates/subdir/serviceaccount.yaml +apiVersion: v1 +kind: ServiceAccount +metadata: + name: subchart-sa +--- +# Source: subchart/templates/subdir/role.yaml +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: subchart-role +rules: +- apiGroups: [""] + resources: ["pods"] + verbs: ["get","list","watch"] +--- +# Source: subchart/templates/subdir/rolebinding.yaml +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: subchart-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: subchart-role +subjects: +- kind: ServiceAccount + name: subchart-sa + namespace: default +--- +# Source: subchart/charts/subcharta/templates/service.yaml +apiVersion: v1 +kind: Service +metadata: + name: subcharta + labels: + helm.sh/chart: "subcharta-0.1.0" +spec: + type: ClusterIP + ports: + - port: 80 + targetPort: 80 + protocol: TCP + name: apache + selector: + app.kubernetes.io/name: subcharta +--- +# Source: subchart/charts/subchartb/templates/service.yaml +apiVersion: v1 +kind: Service +metadata: + name: subchartb + labels: + helm.sh/chart: "subchartb-0.1.0" +spec: + type: ClusterIP + ports: + - port: 80 + targetPort: 80 + protocol: TCP + name: nginx + selector: + app.kubernetes.io/name: subchartb +--- +# Source: subchart/templates/service.yaml +apiVersion: v1 +kind: Service +metadata: + name: subchart + labels: + helm.sh/chart: "subchart-0.1.0" + app.kubernetes.io/instance: "RELEASE-NAME" + kube-version/major: "1" + kube-version/minor: "18" + kube-version/version: "v1.18.0" + kube-api-version/test: v1 +spec: + type: ClusterIP + ports: + - port: 80 + targetPort: 80 + protocol: TCP + name: nginx + selector: + app.kubernetes.io/name: subchart +--- +# Source: subchart/templates/tests/test-nothing.yaml +apiVersion: v1 +kind: Pod +metadata: + name: "RELEASE-NAME-test" + annotations: + "helm.sh/hook": test +spec: + containers: + - name: test + image: "alpine:latest" + envFrom: + - configMapRef: + name: "RELEASE-NAME-testconfig" + command: + - echo + - "$message" + restartPolicy: Never diff --git a/pkg/action/action.go b/pkg/action/action.go index 071db709b..2fc452b7f 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -101,7 +101,7 @@ type Configuration struct { // TODO: This function is badly in need of a refactor. // TODO: As part of the refactor the duplicate code in cmd/helm/template.go should be removed // This code has to do with writing files to disk. -func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) { +func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, skipTests bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) { hs := []*release.Hook{} b := bytes.NewBuffer(nil) @@ -194,7 +194,7 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values } } - for _, m := range manifests { + for _, m := range filterManifests(manifests, skipTests) { if outputDir == "" { fmt.Fprintf(b, "---\n# Source: %s\n%s\n", m.Name, m.Content) } else { @@ -224,6 +224,19 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values return hs, b, notes, nil } +func filterManifests(manifests []releaseutil.Manifest, skipTests bool) []releaseutil.Manifest { + if skipTests { + var manifestsWithoutTests []releaseutil.Manifest + for _, m := range manifests { + if !strings.Contains(m.Name, "tests/") { + manifestsWithoutTests = append(manifestsWithoutTests, m) + } + } + return manifestsWithoutTests + } + return manifests +} + // RESTClientGetter gets the rest client type RESTClientGetter interface { ToRESTConfig() (*rest.Config, error) diff --git a/pkg/action/install.go b/pkg/action/install.go index 00fb208b0..f065e818c 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -91,6 +91,7 @@ type Install struct { SubNotes bool DisableOpenAPIValidation bool IncludeCRDs bool + SkipTests bool // APIVersions allows a manual set of supported API Versions to be passed // (for things like templating). These are ignored if ClientOnly is false APIVersions chartutil.VersionSet @@ -236,7 +237,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. rel := i.createRelease(chrt, vals) var manifestDoc *bytes.Buffer - rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, i.DryRun) + rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.SkipTests, i.PostRenderer, i.DryRun) // Even for errors, attach this if available if manifestDoc != nil { rel.Manifest = manifestDoc.String() diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index b707e7e69..f4110f6af 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -223,7 +223,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin return nil, nil, err } - hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, u.DryRun) + hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, false, u.PostRenderer, u.DryRun) if err != nil { return nil, nil, err } From 9ea10ef04a19796507d09334228471b0b60d3842 Mon Sep 17 00:00:00 2001 From: Torsten Walter Date: Wed, 14 Oct 2020 16:25:42 +0200 Subject: [PATCH 03/25] Skip tests when running helm template Signed-off-by: Torsten Walter --- cmd/helm/template.go | 17 +++++++++++++++-- cmd/helm/template_test.go | 4 ++-- .../output/template-name-template.txt | 18 ++++++++++-------- cmd/helm/testdata/output/template-set.txt | 18 ++++++++++-------- ...e-no-tests.txt => template-skip-tests.txt} | 19 ------------------- .../testdata/output/template-values-files.txt | 18 ++++++++++-------- .../output/template-with-api-version.txt | 18 ++++++++++-------- .../testdata/output/template-with-crds.txt | 18 ++++++++++-------- cmd/helm/testdata/output/template.txt | 18 ++++++++++-------- .../subchart/templates/tests/test-config.yaml | 2 ++ pkg/action/action.go | 17 ++--------------- pkg/action/install.go | 3 +-- pkg/action/upgrade.go | 2 +- 13 files changed, 83 insertions(+), 89 deletions(-) rename cmd/helm/testdata/output/{template-no-tests.txt => template-skip-tests.txt} (82%) diff --git a/cmd/helm/template.go b/cmd/helm/template.go index e504bc774..d760fb87b 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -27,6 +27,8 @@ import ( "sort" "strings" + "helm.sh/helm/v3/pkg/release" + "github.com/spf13/cobra" "helm.sh/helm/v3/cmd/helm/require" @@ -68,7 +70,6 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.ClientOnly = !validate client.APIVersions = chartutil.VersionSet(extraAPIs) client.IncludeCRDs = includeCrds - client.SkipTests = skipTests rel, err := runInstall(args, client, valueOpts, out) if err != nil && !settings.Debug { @@ -86,6 +87,9 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if !client.DisableHooks { fileWritten := make(map[string]bool) for _, m := range rel.Hooks { + if skipTests && isTestHook(m) { + continue + } if client.OutputDir == "" { fmt.Fprintf(&manifests, "---\n# Source: %s\n%s\n", m.Path, m.Manifest) } else { @@ -165,7 +169,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.StringVar(&client.OutputDir, "output-dir", "", "writes the executed templates to files in output-dir instead of stdout") f.BoolVar(&validate, "validate", false, "validate your manifests against the Kubernetes cluster you are currently pointing at. This is the same validation performed on an install") f.BoolVar(&includeCrds, "include-crds", false, "include CRDs in the templated output") - f.BoolVar(&skipTests, "skip-tests", false, "skip tests and manifests in tests directories from templated output") + f.BoolVar(&skipTests, "skip-tests", false, "skip tests from templated output") f.BoolVar(&client.IsUpgrade, "is-upgrade", false, "set .Release.IsUpgrade instead of .Release.IsInstall") f.StringArrayVarP(&extraAPIs, "api-versions", "a", []string{}, "Kubernetes api versions used for Capabilities.APIVersions") f.BoolVar(&client.UseReleaseName, "release-name", false, "use release name in the output-dir path.") @@ -174,6 +178,15 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return cmd } +func isTestHook(h *release.Hook) bool { + for _, e := range h.Events { + if e == release.HookTest { + return true + } + } + return false +} + // The following functions (writeToFile, createOrOpenFile, and ensureDirectoryForFile) // are coppied from the actions package. This is part of a change to correct a // bug introduced by #8156. As part of the todo to refactor renderResources diff --git a/cmd/helm/template_test.go b/cmd/helm/template_test.go index dd30b3836..9e6a0c434 100644 --- a/cmd/helm/template_test.go +++ b/cmd/helm/template_test.go @@ -122,9 +122,9 @@ func TestTemplateCmd(t *testing.T) { golden: "output/template-with-invalid-yaml-debug.txt", }, { - name: "template with skip-tests", + name: "template skip-tests", cmd: fmt.Sprintf(`template '%s' --skip-tests`, chartPath), - golden: "output/template-no-tests.txt", + golden: "output/template-skip-tests.txt", }, } runTestCmd(t, tests) diff --git a/cmd/helm/testdata/output/template-name-template.txt b/cmd/helm/testdata/output/template-name-template.txt index 741630922..5e4478937 100644 --- a/cmd/helm/testdata/output/template-name-template.txt +++ b/cmd/helm/testdata/output/template-name-template.txt @@ -5,14 +5,6 @@ kind: ServiceAccount metadata: name: subchart-sa --- -# Source: subchart/templates/tests/test-config.yaml -apiVersion: v1 -kind: ConfigMap -metadata: - name: "foobar-YWJj-baz-testconfig" -data: - message: Hello World ---- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -92,6 +84,16 @@ spec: selector: app.kubernetes.io/name: subchart --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "foobar-YWJj-baz-testconfig" + annotations: + "helm.sh/hook": test +data: + message: Hello World +--- # Source: subchart/templates/tests/test-nothing.yaml apiVersion: v1 kind: Pod diff --git a/cmd/helm/testdata/output/template-set.txt b/cmd/helm/testdata/output/template-set.txt index 42a08c391..0db9a9b74 100644 --- a/cmd/helm/testdata/output/template-set.txt +++ b/cmd/helm/testdata/output/template-set.txt @@ -5,14 +5,6 @@ kind: ServiceAccount metadata: name: subchart-sa --- -# Source: subchart/templates/tests/test-config.yaml -apiVersion: v1 -kind: ConfigMap -metadata: - name: "RELEASE-NAME-testconfig" -data: - message: Hello World ---- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -92,6 +84,16 @@ spec: selector: app.kubernetes.io/name: subchart --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "RELEASE-NAME-testconfig" + annotations: + "helm.sh/hook": test +data: + message: Hello World +--- # Source: subchart/templates/tests/test-nothing.yaml apiVersion: v1 kind: Pod diff --git a/cmd/helm/testdata/output/template-no-tests.txt b/cmd/helm/testdata/output/template-skip-tests.txt similarity index 82% rename from cmd/helm/testdata/output/template-no-tests.txt rename to cmd/helm/testdata/output/template-skip-tests.txt index de537c214..16808ede3 100644 --- a/cmd/helm/testdata/output/template-no-tests.txt +++ b/cmd/helm/testdata/output/template-skip-tests.txt @@ -84,22 +84,3 @@ spec: name: nginx selector: app.kubernetes.io/name: subchart ---- -# Source: subchart/templates/tests/test-nothing.yaml -apiVersion: v1 -kind: Pod -metadata: - name: "RELEASE-NAME-test" - annotations: - "helm.sh/hook": test -spec: - containers: - - name: test - image: "alpine:latest" - envFrom: - - configMapRef: - name: "RELEASE-NAME-testconfig" - command: - - echo - - "$message" - restartPolicy: Never diff --git a/cmd/helm/testdata/output/template-values-files.txt b/cmd/helm/testdata/output/template-values-files.txt index 42a08c391..0db9a9b74 100644 --- a/cmd/helm/testdata/output/template-values-files.txt +++ b/cmd/helm/testdata/output/template-values-files.txt @@ -5,14 +5,6 @@ kind: ServiceAccount metadata: name: subchart-sa --- -# Source: subchart/templates/tests/test-config.yaml -apiVersion: v1 -kind: ConfigMap -metadata: - name: "RELEASE-NAME-testconfig" -data: - message: Hello World ---- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -92,6 +84,16 @@ spec: selector: app.kubernetes.io/name: subchart --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "RELEASE-NAME-testconfig" + annotations: + "helm.sh/hook": test +data: + message: Hello World +--- # Source: subchart/templates/tests/test-nothing.yaml apiVersion: v1 kind: Pod diff --git a/cmd/helm/testdata/output/template-with-api-version.txt b/cmd/helm/testdata/output/template-with-api-version.txt index da77c51c0..3e488f0d2 100644 --- a/cmd/helm/testdata/output/template-with-api-version.txt +++ b/cmd/helm/testdata/output/template-with-api-version.txt @@ -5,14 +5,6 @@ kind: ServiceAccount metadata: name: subchart-sa --- -# Source: subchart/templates/tests/test-config.yaml -apiVersion: v1 -kind: ConfigMap -metadata: - name: "RELEASE-NAME-testconfig" -data: - message: Hello World ---- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -93,6 +85,16 @@ spec: selector: app.kubernetes.io/name: subchart --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "RELEASE-NAME-testconfig" + annotations: + "helm.sh/hook": test +data: + message: Hello World +--- # Source: subchart/templates/tests/test-nothing.yaml apiVersion: v1 kind: Pod diff --git a/cmd/helm/testdata/output/template-with-crds.txt b/cmd/helm/testdata/output/template-with-crds.txt index 57e770176..4bd5d2e29 100644 --- a/cmd/helm/testdata/output/template-with-crds.txt +++ b/cmd/helm/testdata/output/template-with-crds.txt @@ -22,14 +22,6 @@ kind: ServiceAccount metadata: name: subchart-sa --- -# Source: subchart/templates/tests/test-config.yaml -apiVersion: v1 -kind: ConfigMap -metadata: - name: "RELEASE-NAME-testconfig" -data: - message: Hello World ---- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -110,6 +102,16 @@ spec: selector: app.kubernetes.io/name: subchart --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "RELEASE-NAME-testconfig" + annotations: + "helm.sh/hook": test +data: + message: Hello World +--- # Source: subchart/templates/tests/test-nothing.yaml apiVersion: v1 kind: Pod diff --git a/cmd/helm/testdata/output/template.txt b/cmd/helm/testdata/output/template.txt index b2c65a4e1..a81934b20 100644 --- a/cmd/helm/testdata/output/template.txt +++ b/cmd/helm/testdata/output/template.txt @@ -5,14 +5,6 @@ kind: ServiceAccount metadata: name: subchart-sa --- -# Source: subchart/templates/tests/test-config.yaml -apiVersion: v1 -kind: ConfigMap -metadata: - name: "RELEASE-NAME-testconfig" -data: - message: Hello World ---- # Source: subchart/templates/subdir/role.yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -92,6 +84,16 @@ spec: selector: app.kubernetes.io/name: subchart --- +# Source: subchart/templates/tests/test-config.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: "RELEASE-NAME-testconfig" + annotations: + "helm.sh/hook": test +data: + message: Hello World +--- # Source: subchart/templates/tests/test-nothing.yaml apiVersion: v1 kind: Pod diff --git a/cmd/helm/testdata/testcharts/subchart/templates/tests/test-config.yaml b/cmd/helm/testdata/testcharts/subchart/templates/tests/test-config.yaml index de639e03b..0aa3eea29 100644 --- a/cmd/helm/testdata/testcharts/subchart/templates/tests/test-config.yaml +++ b/cmd/helm/testdata/testcharts/subchart/templates/tests/test-config.yaml @@ -2,5 +2,7 @@ apiVersion: v1 kind: ConfigMap metadata: name: "{{ .Release.Name }}-testconfig" + annotations: + "helm.sh/hook": test data: message: Hello World diff --git a/pkg/action/action.go b/pkg/action/action.go index 2fc452b7f..071db709b 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -101,7 +101,7 @@ type Configuration struct { // TODO: This function is badly in need of a refactor. // TODO: As part of the refactor the duplicate code in cmd/helm/template.go should be removed // This code has to do with writing files to disk. -func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, skipTests bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) { +func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) { hs := []*release.Hook{} b := bytes.NewBuffer(nil) @@ -194,7 +194,7 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values } } - for _, m := range filterManifests(manifests, skipTests) { + for _, m := range manifests { if outputDir == "" { fmt.Fprintf(b, "---\n# Source: %s\n%s\n", m.Name, m.Content) } else { @@ -224,19 +224,6 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values return hs, b, notes, nil } -func filterManifests(manifests []releaseutil.Manifest, skipTests bool) []releaseutil.Manifest { - if skipTests { - var manifestsWithoutTests []releaseutil.Manifest - for _, m := range manifests { - if !strings.Contains(m.Name, "tests/") { - manifestsWithoutTests = append(manifestsWithoutTests, m) - } - } - return manifestsWithoutTests - } - return manifests -} - // RESTClientGetter gets the rest client type RESTClientGetter interface { ToRESTConfig() (*rest.Config, error) diff --git a/pkg/action/install.go b/pkg/action/install.go index f065e818c..00fb208b0 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -91,7 +91,6 @@ type Install struct { SubNotes bool DisableOpenAPIValidation bool IncludeCRDs bool - SkipTests bool // APIVersions allows a manual set of supported API Versions to be passed // (for things like templating). These are ignored if ClientOnly is false APIVersions chartutil.VersionSet @@ -237,7 +236,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. rel := i.createRelease(chrt, vals) var manifestDoc *bytes.Buffer - rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.SkipTests, i.PostRenderer, i.DryRun) + rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, i.DryRun) // Even for errors, attach this if available if manifestDoc != nil { rel.Manifest = manifestDoc.String() diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index f4110f6af..b707e7e69 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -223,7 +223,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin return nil, nil, err } - hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, false, u.PostRenderer, u.DryRun) + hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, u.DryRun) if err != nil { return nil, nil, err } From 2a7a98ae5acc943f798ca78fcb1de44b6252a0d4 Mon Sep 17 00:00:00 2001 From: Chris Wells Date: Sun, 19 Jul 2020 14:26:48 -0400 Subject: [PATCH 04/25] feat: Allow helm test to run a subset of tests Signed-off-by: Chris Wells --- cmd/helm/release_testing.go | 12 ++++++ .../helm/repository/test-name-index.yaml | 2 +- pkg/action/release_testing.go | 39 ++++++++++++++++++- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/cmd/helm/release_testing.go b/cmd/helm/release_testing.go index e4e09ef3b..fbf0dd112 100644 --- a/cmd/helm/release_testing.go +++ b/cmd/helm/release_testing.go @@ -19,6 +19,8 @@ package main import ( "fmt" "io" + "regexp" + "strings" "time" "github.com/spf13/cobra" @@ -39,6 +41,7 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command client := action.NewReleaseTesting(cfg) var outfmt = output.Table var outputLogs bool + var filter []string cmd := &cobra.Command{ Use: "test [RELEASE]", @@ -53,6 +56,14 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command }, RunE: func(cmd *cobra.Command, args []string) error { client.Namespace = settings.Namespace() + notName := regexp.MustCompile(`^!\s?name=`) + for _, f := range filter { + if strings.HasPrefix(f, "name=") { + client.Filters["name"] = append(client.Filters["name"], strings.TrimPrefix(f, "name=")) + } else if notName.MatchString(f) { + client.Filters["!name"] = append(client.Filters["!name"], notName.ReplaceAllLiteralString(f, "")) + } + } rel, runErr := client.Run(args[0]) // We only return an error if we weren't even able to get the // release, otherwise we keep going so we can print status and logs @@ -80,6 +91,7 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command f := cmd.Flags() f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)") f.BoolVar(&outputLogs, "logs", false, "dump the logs from test pods (this runs after all tests are complete, but before any cleanup)") + f.StringSliceVar(&filter, "filter", []string{}, "specify tests by attribute (currently \"name\") using attribute=value syntax or '!attribute=value' to exclude a test (can specify multiple or separate values with commas: name=test1,name=test2)") return cmd } diff --git a/cmd/helm/testdata/helmhome/helm/repository/test-name-index.yaml b/cmd/helm/testdata/helmhome/helm/repository/test-name-index.yaml index 895e79d39..d5ab620ad 100644 --- a/cmd/helm/testdata/helmhome/helm/repository/test-name-index.yaml +++ b/cmd/helm/testdata/helmhome/helm/repository/test-name-index.yaml @@ -1,3 +1,3 @@ apiVersion: v1 entries: {} -generated: "2020-06-23T10:01:59.2530763-07:00" +generated: "2020-09-09T19:50:50.198347916-04:00" diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go index 2f6f5cfce..ecaeaf59f 100644 --- a/pkg/action/release_testing.go +++ b/pkg/action/release_testing.go @@ -37,12 +37,14 @@ type ReleaseTesting struct { Timeout time.Duration // Used for fetching logs from test pods Namespace string + Filters map[string][]string } // NewReleaseTesting creates a new ReleaseTesting object with the given configuration. func NewReleaseTesting(cfg *Configuration) *ReleaseTesting { return &ReleaseTesting{ - cfg: cfg, + cfg: cfg, + Filters: map[string][]string{}, } } @@ -62,11 +64,37 @@ func (r *ReleaseTesting) Run(name string) (*release.Release, error) { return rel, err } + skippedHooks := []*release.Hook{} + executingHooks := []*release.Hook{} + if len(r.Filters["!name"]) != 0 { + for _, h := range rel.Hooks { + if contains(r.Filters["!name"], h.Name) { + skippedHooks = append(skippedHooks, h) + } else { + executingHooks = append(executingHooks, h) + } + } + rel.Hooks = executingHooks + } + if len(r.Filters["name"]) != 0 { + executingHooks = nil + for _, h := range rel.Hooks { + if contains(r.Filters["name"], h.Name) { + executingHooks = append(executingHooks, h) + } else { + skippedHooks = append(skippedHooks, h) + } + } + rel.Hooks = executingHooks + } + if err := r.cfg.execHook(rel, release.HookTest, r.Timeout); err != nil { + rel.Hooks = append(skippedHooks, rel.Hooks...) r.cfg.Releases.Update(rel) return rel, err } + rel.Hooks = append(skippedHooks, rel.Hooks...) return rel, r.cfg.Releases.Update(rel) } @@ -99,3 +127,12 @@ func (r *ReleaseTesting) GetPodLogs(out io.Writer, rel *release.Release) error { } return nil } + +func contains(arr []string, value string) bool { + for _, item := range arr { + if item == value { + return true + } + } + return false +} From 5825112a8b385ca48e67c4782ba77656c6f4fba5 Mon Sep 17 00:00:00 2001 From: zh168654 Date: Mon, 29 Jun 2020 01:53:43 +0800 Subject: [PATCH 05/25] helm upgrade with --wait support jobs in manifest to be completed Signed-off-by: zh168654 --- pkg/kube/wait.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pkg/kube/wait.go b/pkg/kube/wait.go index c3beb232d..3381b7881 100644 --- a/pkg/kube/wait.go +++ b/pkg/kube/wait.go @@ -67,6 +67,11 @@ func (w *waiter) waitForResources(created ResourceList) error { if err != nil || !w.isPodReady(pod) { return false, err } + case *batchv1.Job: + job, err := w.c.BatchV1().Jobs(v.Namespace).Get(context.Background(), v.Name, metav1.GetOptions{}) + if err != nil || !w.jobReady(job) { + return false, err + } case *appsv1.Deployment, *appsv1beta1.Deployment, *appsv1beta2.Deployment, *extensionsv1beta1.Deployment: currentDeployment, err := w.c.AppsV1().Deployments(v.Namespace).Get(context.Background(), v.Name, metav1.GetOptions{}) if err != nil { @@ -182,6 +187,18 @@ func (w *waiter) isPodReady(pod *corev1.Pod) bool { return false } +func (w *waiter) jobReady(job *batchv1.Job) bool { + if job.Status.Failed >= *job.Spec.BackoffLimit { + w.log("Job is failed: %s/%s", job.GetNamespace(), job.GetName()) + return false + } + if job.Status.Succeeded < *job.Spec.Completions { + w.log("Job is not completed: %s/%s", job.GetNamespace(), job.GetName()) + return false + } + return true +} + func (w *waiter) serviceReady(s *corev1.Service) bool { // ExternalName Services are external to cluster so helm shouldn't be checking to see if they're 'ready' (i.e. have an IP Set) if s.Spec.Type == corev1.ServiceTypeExternalName { From 8d498d58e7b383b6c50d43cc5a55eae4955d354e Mon Sep 17 00:00:00 2001 From: zh168654 Date: Thu, 9 Jul 2020 01:10:24 +0800 Subject: [PATCH 06/25] add test cases Signed-off-by: zh168654 --- pkg/kube/wait_test.go | 514 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 514 insertions(+) create mode 100644 pkg/kube/wait_test.go diff --git a/pkg/kube/wait_test.go b/pkg/kube/wait_test.go new file mode 100644 index 000000000..e84924f2e --- /dev/null +++ b/pkg/kube/wait_test.go @@ -0,0 +1,514 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "context" + appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/fake" + "testing" +) + +const defaultNamespace = metav1.NamespaceDefault + +func Test_waiter_deploymentReady(t *testing.T) { + type args struct { + rs *appsv1.ReplicaSet + dep *appsv1.Deployment + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "deployment is ready", + args: args{ + rs: newReplicaSet("foo", 1, 1), + dep: newDeployment("foo", 1, 1, 0), + }, + want: true, + }, + { + name: "deployment is not ready", + args: args{ + rs: newReplicaSet("foo", 0, 0), + dep: newDeployment("foo", 1, 1, 0), + }, + want: false, + }, + { + name: "deployment is ready when maxUnavailable is set", + args: args{ + rs: newReplicaSet("foo", 2, 1), + dep: newDeployment("foo", 2, 1, 1), + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := &waiter{ + c: fake.NewSimpleClientset(), + log: nopLogger, + } + if got := w.deploymentReady(tt.args.rs, tt.args.dep); got != tt.want { + t.Errorf("deploymentReady() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_waiter_daemonSetReady(t *testing.T) { + type args struct { + ds *appsv1.DaemonSet + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "daemonset is ready", + args: args{ + ds: newDaemonSet("foo", 0, 1, 1, 1), + }, + want: true, + }, + { + name: "daemonset is not ready", + args: args{ + ds: newDaemonSet("foo", 0, 0, 1, 1), + }, + want: false, + }, + { + name: "daemonset pods have not been scheduled successfully", + args: args{ + ds: newDaemonSet("foo", 0, 0, 1, 0), + }, + want: false, + }, + { + name: "daemonset is ready when maxUnavailable is set", + args: args{ + ds: newDaemonSet("foo", 1, 1, 2, 2), + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := &waiter{ + c: fake.NewSimpleClientset(), + log: nopLogger, + } + if got := w.daemonSetReady(tt.args.ds); got != tt.want { + t.Errorf("daemonSetReady() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_waiter_statefulSetReady(t *testing.T) { + type args struct { + sts *appsv1.StatefulSet + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "statefulset is ready", + args: args{ + sts: newStatefulSet("foo", 1, 0, 1, 1), + }, + want: true, + }, + { + name: "statefulset is not ready", + args: args{ + sts: newStatefulSet("foo", 1, 0, 0, 1), + }, + want: false, + }, + { + name: "statefulset is ready when partition is specified", + args: args{ + sts: newStatefulSet("foo", 2, 1, 2, 1), + }, + want: true, + }, + { + name: "statefulset is not ready when partition is set", + args: args{ + sts: newStatefulSet("foo", 1, 1, 1, 1), + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := &waiter{ + c: fake.NewSimpleClientset(), + log: nopLogger, + } + if got := w.statefulSetReady(tt.args.sts); got != tt.want { + t.Errorf("statefulSetReady() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_waiter_podsReadyForObject(t *testing.T) { + type args struct { + namespace string + obj runtime.Object + } + tests := []struct { + name string + args args + existPods []corev1.Pod + want bool + wantErr bool + }{ + { + name: "pods ready for a replicaset", + args: args{ + namespace: defaultNamespace, + obj: newReplicaSet("foo", 1, 1), + }, + existPods: []corev1.Pod{ + *newPodWithCondition("foo", corev1.ConditionTrue), + }, + want: true, + wantErr: false, + }, + { + name: "pods not ready for a replicaset", + args: args{ + namespace: defaultNamespace, + obj: newReplicaSet("foo", 1, 1), + }, + existPods: []corev1.Pod{ + *newPodWithCondition("foo", corev1.ConditionFalse), + }, + want: false, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := &waiter{ + c: fake.NewSimpleClientset(), + log: nopLogger, + } + for _, pod := range tt.existPods { + if _, err := w.c.CoreV1().Pods(defaultNamespace).Create(context.TODO(), &pod, metav1.CreateOptions{}); err != nil { + t.Errorf("Failed to create Pod error: %v", err) + return + } + } + got, err := w.podsReadyForObject(tt.args.namespace, tt.args.obj) + if (err != nil) != tt.wantErr { + t.Errorf("podsReadyForObject() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("podsReadyForObject() got = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_waiter_jobReady(t *testing.T) { + type args struct { + job *batchv1.Job + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "job is completed", + args: args{job: newJob("foo", 1, 1, 1, 0)}, + want: true, + }, + { + name: "job is incomplete", + args: args{job: newJob("foo", 1, 1, 0, 0)}, + want: false, + }, + { + name: "job is failed", + args: args{job: newJob("foo", 1, 1, 0, 1)}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := &waiter{ + c: fake.NewSimpleClientset(), + log: nopLogger, + } + if got := w.jobReady(tt.args.job); got != tt.want { + t.Errorf("jobReady() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_waiter_volumeReady(t *testing.T) { + type args struct { + v *corev1.PersistentVolumeClaim + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "pvc is bound", + args: args{ + v: newPersistentVolumeClaim("foo", corev1.ClaimBound), + }, + want: true, + }, + { + name: "pvc is not ready", + args: args{ + v: newPersistentVolumeClaim("foo", corev1.ClaimPending), + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := &waiter{ + c: fake.NewSimpleClientset(), + log: nopLogger, + } + if got := w.volumeReady(tt.args.v); got != tt.want { + t.Errorf("volumeReady() = %v, want %v", got, tt.want) + } + }) + } +} + +func newDaemonSet(name string, maxUnavailable, numberReady, desiredNumberScheduled, updatedNumberScheduled int) *appsv1.DaemonSet { + return &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: defaultNamespace, + }, + Spec: appsv1.DaemonSetSpec{ + UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ + Type: appsv1.RollingUpdateDaemonSetStrategyType, + RollingUpdate: &appsv1.RollingUpdateDaemonSet{ + MaxUnavailable: func() *intstr.IntOrString { i := intstr.FromInt(maxUnavailable); return &i }(), + }, + }, + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"name": name}}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{"name": name}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx", + }, + }, + }, + }, + }, + Status: appsv1.DaemonSetStatus{ + DesiredNumberScheduled: int32(desiredNumberScheduled), + NumberReady: int32(numberReady), + UpdatedNumberScheduled: int32(updatedNumberScheduled), + }, + } +} + +func newStatefulSet(name string, replicas, partition, readyReplicas, updatedReplicas int) *appsv1.StatefulSet { + return &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: defaultNamespace, + }, + Spec: appsv1.StatefulSetSpec{ + UpdateStrategy: appsv1.StatefulSetUpdateStrategy{ + Type: appsv1.RollingUpdateStatefulSetStrategyType, + RollingUpdate: &appsv1.RollingUpdateStatefulSetStrategy{ + Partition: intToInt32(partition), + }, + }, + Replicas: intToInt32(replicas), + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"name": name}}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{"name": name}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx", + }, + }, + }, + }, + }, + Status: appsv1.StatefulSetStatus{ + UpdatedReplicas: int32(updatedReplicas), + ReadyReplicas: int32(readyReplicas), + }, + } +} + +func newDeployment(name string, replicas, maxSurge, maxUnavailable int) *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: defaultNamespace, + }, + Spec: appsv1.DeploymentSpec{ + Strategy: appsv1.DeploymentStrategy{ + Type: appsv1.RollingUpdateDeploymentStrategyType, + RollingUpdate: &appsv1.RollingUpdateDeployment{ + MaxUnavailable: func() *intstr.IntOrString { i := intstr.FromInt(maxUnavailable); return &i }(), + MaxSurge: func() *intstr.IntOrString { i := intstr.FromInt(maxSurge); return &i }(), + }, + }, + Replicas: intToInt32(replicas), + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"name": name}}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{"name": name}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx", + }, + }, + }, + }, + }, + } +} + +func newReplicaSet(name string, replicas int, readyReplicas int) *appsv1.ReplicaSet { + d := newDeployment(name, replicas, 0, 0) + return &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: defaultNamespace, + Labels: d.Spec.Selector.MatchLabels, + OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(d, d.GroupVersionKind())}, + }, + Spec: appsv1.ReplicaSetSpec{ + Selector: d.Spec.Selector, + Replicas: intToInt32(replicas), + Template: d.Spec.Template, + }, + Status: appsv1.ReplicaSetStatus{ + ReadyReplicas: int32(readyReplicas), + }, + } +} + +func newPodWithCondition(name string, podReadyCondition corev1.ConditionStatus) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: defaultNamespace, + Labels: map[string]string{"name": name}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx", + }, + }, + }, + Status: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: podReadyCondition, + }, + }, + }, + } +} + +func newPersistentVolumeClaim(name string, phase corev1.PersistentVolumeClaimPhase) *corev1.PersistentVolumeClaim { + return &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: defaultNamespace, + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: phase, + }, + } +} + +func newJob(name string, backoffLimit, completions, succeeded, failed int) *batchv1.Job { + return &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: defaultNamespace, + }, + Spec: batchv1.JobSpec{ + BackoffLimit: intToInt32(backoffLimit), + Completions: intToInt32(completions), + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{"name": name}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx", + }, + }, + }, + }, + }, + Status: batchv1.JobStatus{ + Succeeded: int32(succeeded), + Failed: int32(failed), + }, + } +} + +func intToInt32(i int) *int32 { + i32 := int32(i) + return &i32 +} From c96dc48f21adbc79e410fafc63f6f6daa221c424 Mon Sep 17 00:00:00 2001 From: zhangye15 Date: Thu, 9 Jul 2020 01:44:18 +0800 Subject: [PATCH 07/25] fix test-style error Signed-off-by: zhangye15 --- pkg/kube/wait_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kube/wait_test.go b/pkg/kube/wait_test.go index e84924f2e..c6ce494c3 100644 --- a/pkg/kube/wait_test.go +++ b/pkg/kube/wait_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package kube +package kube // import "helm.sh/helm/v3/pkg/kube" import ( "context" From bd03e1b5c70cffd13e740f40ef1c0e8c3a49e092 Mon Sep 17 00:00:00 2001 From: zhangye15 Date: Thu, 9 Jul 2020 01:52:24 +0800 Subject: [PATCH 08/25] fix style conformance Signed-off-by: zhangye15 --- pkg/kube/wait_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/kube/wait_test.go b/pkg/kube/wait_test.go index c6ce494c3..3f7b86710 100644 --- a/pkg/kube/wait_test.go +++ b/pkg/kube/wait_test.go @@ -18,6 +18,8 @@ package kube // import "helm.sh/helm/v3/pkg/kube" import ( "context" + "testing" + appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" @@ -25,7 +27,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/fake" - "testing" ) const defaultNamespace = metav1.NamespaceDefault From 957d2a2bf978b06cb148b9429b8dd9258c24b887 Mon Sep 17 00:00:00 2001 From: zh168654 Date: Tue, 3 Nov 2020 19:48:29 +0800 Subject: [PATCH 09/25] add wait-for-jobs flag Signed-off-by: zh168654 --- cmd/helm/install.go | 1 + cmd/helm/install_test.go | 6 +++++ cmd/helm/rollback.go | 1 + cmd/helm/rollback_test.go | 5 ++++ .../output/install-with-wait-for-jobs.txt | 6 +++++ .../output/rollback-wait-for-jobs.txt | 1 + .../output/upgrade-with-wait-for-jobs.txt | 7 ++++++ cmd/helm/upgrade.go | 2 ++ cmd/helm/upgrade_test.go | 6 +++++ pkg/action/install.go | 6 ++--- pkg/action/install_test.go | 17 ++++++++++++++ pkg/action/rollback.go | 3 ++- pkg/action/upgrade.go | 5 +++- pkg/action/upgrade_test.go | 23 +++++++++++++++++++ pkg/kube/client.go | 4 ++-- pkg/kube/fake/fake.go | 4 ++-- pkg/kube/fake/printer.go | 2 +- pkg/kube/interface.go | 2 +- pkg/kube/wait.go | 14 ++++++----- 19 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 cmd/helm/testdata/output/install-with-wait-for-jobs.txt create mode 100644 cmd/helm/testdata/output/rollback-wait-for-jobs.txt create mode 100644 cmd/helm/testdata/output/upgrade-with-wait-for-jobs.txt diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 7edd98091..fac2131c1 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -140,6 +140,7 @@ func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Instal f.BoolVar(&client.Replace, "replace", false, "re-use the given name, only if that name is a deleted release which remains in the history. This is unsafe in production") f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)") f.BoolVar(&client.Wait, "wait", false, "if set, will wait until all Pods, PVCs, Services, and minimum number of Pods of a Deployment, StatefulSet, or ReplicaSet are in a ready state before marking the release as successful. It will wait for as long as --timeout") + f.BoolVar(&client.WaitForJobs, "wait-for-jobs", false, "if set and --wait enabled, will wait until all Jobs have been completed before marking the release as successful. It will wait for as long as --timeout") f.BoolVarP(&client.GenerateName, "generate-name", "g", false, "generate the name (and omit the NAME parameter)") f.StringVar(&client.NameTemplate, "name-template", "", "specify template used to name the release") f.StringVar(&client.Description, "description", "", "add a custom description") diff --git a/cmd/helm/install_test.go b/cmd/helm/install_test.go index 6892fcd86..0fae79534 100644 --- a/cmd/helm/install_test.go +++ b/cmd/helm/install_test.go @@ -85,6 +85,12 @@ func TestInstall(t *testing.T) { cmd: "install apollo testdata/testcharts/empty --wait", golden: "output/install-with-wait.txt", }, + // Install, with wait-for-jobs + { + name: "install with wait-for-jobs", + cmd: "install apollo testdata/testcharts/empty --wait --wait-for-jobs", + golden: "output/install-with-wait-for-jobs.txt", + }, // Install, using the name-template { name: "install with name-template", diff --git a/cmd/helm/rollback.go b/cmd/helm/rollback.go index 2cd6fa2cb..9699b9c05 100644 --- a/cmd/helm/rollback.go +++ b/cmd/helm/rollback.go @@ -82,6 +82,7 @@ func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&client.DisableHooks, "no-hooks", false, "prevent hooks from running during rollback") f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)") f.BoolVar(&client.Wait, "wait", false, "if set, will wait until all Pods, PVCs, Services, and minimum number of Pods of a Deployment, StatefulSet, or ReplicaSet are in a ready state before marking the release as successful. It will wait for as long as --timeout") + f.BoolVar(&client.WaitForJobs, "wait-for-jobs", false, "if set and --wait enabled, will wait until all Jobs have been completed before marking the release as successful. It will wait for as long as --timeout") f.BoolVar(&client.CleanupOnFail, "cleanup-on-fail", false, "allow deletion of new resources created in this rollback when rollback fails") f.IntVar(&client.MaxHistory, "history-max", settings.MaxHistory, "limit the maximum number of revisions saved per release. Use 0 for no limit") diff --git a/cmd/helm/rollback_test.go b/cmd/helm/rollback_test.go index b39378f92..9ca921557 100644 --- a/cmd/helm/rollback_test.go +++ b/cmd/helm/rollback_test.go @@ -54,6 +54,11 @@ func TestRollbackCmd(t *testing.T) { cmd: "rollback funny-honey 1 --wait", golden: "output/rollback-wait.txt", rels: rels, + }, { + name: "rollback a release with wait-for-jobs", + cmd: "rollback funny-honey 1 --wait --wait-for-jobs", + golden: "output/rollback-wait-for-jobs.txt", + rels: rels, }, { name: "rollback a release without revision", cmd: "rollback funny-honey", diff --git a/cmd/helm/testdata/output/install-with-wait-for-jobs.txt b/cmd/helm/testdata/output/install-with-wait-for-jobs.txt new file mode 100644 index 000000000..7ce22d4ec --- /dev/null +++ b/cmd/helm/testdata/output/install-with-wait-for-jobs.txt @@ -0,0 +1,6 @@ +NAME: apollo +LAST DEPLOYED: Fri Sep 2 22:04:05 1977 +NAMESPACE: default +STATUS: deployed +REVISION: 1 +TEST SUITE: None diff --git a/cmd/helm/testdata/output/rollback-wait-for-jobs.txt b/cmd/helm/testdata/output/rollback-wait-for-jobs.txt new file mode 100644 index 000000000..ae3c6f1c4 --- /dev/null +++ b/cmd/helm/testdata/output/rollback-wait-for-jobs.txt @@ -0,0 +1 @@ +Rollback was a success! Happy Helming! diff --git a/cmd/helm/testdata/output/upgrade-with-wait-for-jobs.txt b/cmd/helm/testdata/output/upgrade-with-wait-for-jobs.txt new file mode 100644 index 000000000..500d07a11 --- /dev/null +++ b/cmd/helm/testdata/output/upgrade-with-wait-for-jobs.txt @@ -0,0 +1,7 @@ +Release "crazy-bunny" has been upgraded. Happy Helming! +NAME: crazy-bunny +LAST DEPLOYED: Fri Sep 2 22:04:05 1977 +NAMESPACE: default +STATUS: deployed +REVISION: 3 +TEST SUITE: None diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 12d797545..c2e92fb36 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -103,6 +103,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { instClient.SkipCRDs = client.SkipCRDs instClient.Timeout = client.Timeout instClient.Wait = client.Wait + instClient.WaitForJobs = client.WaitForJobs instClient.Devel = client.Devel instClient.Namespace = client.Namespace instClient.Atomic = client.Atomic @@ -179,6 +180,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&client.ResetValues, "reset-values", false, "when upgrading, reset the values to the ones built into the chart") f.BoolVar(&client.ReuseValues, "reuse-values", false, "when upgrading, reuse the last release's values and merge in any overrides from the command line via --set and -f. If '--reset-values' is specified, this is ignored") f.BoolVar(&client.Wait, "wait", false, "if set, will wait until all Pods, PVCs, Services, and minimum number of Pods of a Deployment, StatefulSet, or ReplicaSet are in a ready state before marking the release as successful. It will wait for as long as --timeout") + f.BoolVar(&client.WaitForJobs, "wait-for-jobs", false, "if set and --wait enabled, will wait until all Jobs have been completed before marking the release as successful. It will wait for as long as --timeout") f.BoolVar(&client.Atomic, "atomic", false, "if set, upgrade process rolls back changes made in case of failed upgrade. The --wait flag will be set automatically if --atomic is used") f.IntVar(&client.MaxHistory, "history-max", settings.MaxHistory, "limit the maximum number of revisions saved per release. Use 0 for no limit") f.BoolVar(&client.CleanupOnFail, "cleanup-on-fail", false, "allow deletion of new resources created in this upgrade when upgrade fails") diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go index 6fe79ebce..e952a5933 100644 --- a/cmd/helm/upgrade_test.go +++ b/cmd/helm/upgrade_test.go @@ -131,6 +131,12 @@ func TestUpgradeCmd(t *testing.T) { golden: "output/upgrade-with-wait.txt", rels: []*release.Release{relMock("crazy-bunny", 2, ch2)}, }, + { + name: "upgrade a release with wait-for-jobs", + cmd: fmt.Sprintf("upgrade crazy-bunny --wait --wait-for-jobs '%s'", chartPath), + golden: "output/upgrade-with-wait-for-jobs.txt", + rels: []*release.Release{relMock("crazy-bunny", 2, ch2)}, + }, { name: "upgrade a release with missing dependencies", cmd: fmt.Sprintf("upgrade bonkers-bunny %s", missingDepsPath), diff --git a/pkg/action/install.go b/pkg/action/install.go index caeefca68..6ef754a45 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -77,6 +77,7 @@ type Install struct { DisableHooks bool Replace bool Wait bool + WaitForJobs bool Devel bool DependencyUpdate bool Timeout time.Duration @@ -156,7 +157,7 @@ func (i *Install) installCRDs(crds []chart.CRD) error { discoveryClient.Invalidate() // Give time for the CRD to be recognized. - if err := i.cfg.KubeClient.Wait(totalItems, 60*time.Second); err != nil { + if err := i.cfg.KubeClient.Wait(totalItems, 60*time.Second, false); err != nil { return err } @@ -345,10 +346,9 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. } if i.Wait { - if err := i.cfg.KubeClient.Wait(resources, i.Timeout); err != nil { + if err := i.cfg.KubeClient.Wait(resources, i.Timeout, i.WaitForJobs); err != nil { return i.failRelease(rel, err) } - } if !i.DisableHooks { diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 6c4012cfd..466b15c51 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -362,6 +362,23 @@ func TestInstallRelease_Wait(t *testing.T) { is.Equal(res.Info.Status, release.StatusFailed) } +func TestInstallRelease_WaitForJobs(t *testing.T) { + is := assert.New(t) + instAction := installAction(t) + instAction.ReleaseName = "come-fail-away" + failer := instAction.cfg.KubeClient.(*kubefake.FailingKubeClient) + failer.WaitError = fmt.Errorf("I timed out") + instAction.cfg.KubeClient = failer + instAction.Wait = true + instAction.WaitForJobs = true + vals := map[string]interface{}{} + + res, err := instAction.Run(buildChart(), vals) + is.Error(err) + is.Contains(res.Info.Description, "I timed out") + is.Equal(res.Info.Status, release.StatusFailed) +} + func TestInstallRelease_Atomic(t *testing.T) { is := assert.New(t) diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index 542acefae..5c3fabaee 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -38,6 +38,7 @@ type Rollback struct { Version int Timeout time.Duration Wait bool + WaitForJobs bool DisableHooks bool DryRun bool Recreate bool // will (if true) recreate pods after a rollback. @@ -199,7 +200,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas } if r.Wait { - if err := r.cfg.KubeClient.Wait(target, r.Timeout); err != nil { + if err := r.cfg.KubeClient.Wait(target, r.Timeout, r.WaitForJobs); err != nil { targetRelease.SetStatus(release.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) r.cfg.recordRelease(currentRelease) r.cfg.recordRelease(targetRelease) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index c439af79d..db74e1ece 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -64,6 +64,8 @@ type Upgrade struct { Timeout time.Duration // Wait determines whether the wait operation should be performed after the upgrade is requested. Wait bool + // WaitForJobs determines whether the wait operation for the Jobs should be performed after the upgrade is requested. + WaitForJobs bool // DisableHooks disables hook processing if set to true. DisableHooks bool // DryRun controls whether the operation is prepared, but not executed. @@ -329,7 +331,7 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea } if u.Wait { - if err := u.cfg.KubeClient.Wait(target, u.Timeout); err != nil { + if err := u.cfg.KubeClient.Wait(target, u.Timeout, u.WaitForJobs); err != nil { u.cfg.recordRelease(originalRelease) return u.failRelease(upgradedRelease, results.Created, err) } @@ -400,6 +402,7 @@ func (u *Upgrade) failRelease(rel *release.Release, created kube.ResourceList, e rollin := NewRollback(u.cfg) rollin.Version = filteredHistory[0].Version rollin.Wait = true + rollin.WaitForJobs = u.WaitForJobs rollin.DisableHooks = u.DisableHooks rollin.Recreate = u.Recreate rollin.Force = u.Force diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index f16de6479..5cca7ca1a 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -60,6 +60,29 @@ func TestUpgradeRelease_Wait(t *testing.T) { is.Equal(res.Info.Status, release.StatusFailed) } +func TestUpgradeRelease_WaitForJobs(t *testing.T) { + is := assert.New(t) + req := require.New(t) + + upAction := upgradeAction(t) + rel := releaseStub() + rel.Name = "come-fail-away" + rel.Info.Status = release.StatusDeployed + upAction.cfg.Releases.Create(rel) + + failer := upAction.cfg.KubeClient.(*kubefake.FailingKubeClient) + failer.WaitError = fmt.Errorf("I timed out") + upAction.cfg.KubeClient = failer + upAction.Wait = true + upAction.WaitForJobs = true + vals := map[string]interface{}{} + + res, err := upAction.Run(rel.Name, buildChart(), vals) + req.Error(err) + is.Contains(res.Info.Description, "I timed out") + is.Equal(res.Info.Status, release.StatusFailed) +} + func TestUpgradeRelease_CleanupOnFail(t *testing.T) { is := assert.New(t) req := require.New(t) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 6fd3336c9..d1681a534 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -127,7 +127,7 @@ func (c *Client) Create(resources ResourceList) (*Result, error) { } // Wait up to the given timeout for the specified resources to be ready -func (c *Client) Wait(resources ResourceList, timeout time.Duration) error { +func (c *Client) Wait(resources ResourceList, timeout time.Duration, waitForJobsEnabled bool) error { cs, err := c.getKubeClient() if err != nil { return err @@ -137,7 +137,7 @@ func (c *Client) Wait(resources ResourceList, timeout time.Duration) error { log: c.Log, timeout: timeout, } - return w.waitForResources(resources) + return w.waitForResources(resources, waitForJobsEnabled) } func (c *Client) namespace() string { diff --git a/pkg/kube/fake/fake.go b/pkg/kube/fake/fake.go index b3f7a393b..55b887ab3 100644 --- a/pkg/kube/fake/fake.go +++ b/pkg/kube/fake/fake.go @@ -51,11 +51,11 @@ func (f *FailingKubeClient) Create(resources kube.ResourceList) (*kube.Result, e } // Wait returns the configured error if set or prints -func (f *FailingKubeClient) Wait(resources kube.ResourceList, d time.Duration) error { +func (f *FailingKubeClient) Wait(resources kube.ResourceList, d time.Duration, waitForJobsEnabled bool) error { if f.WaitError != nil { return f.WaitError } - return f.PrintingKubeClient.Wait(resources, d) + return f.PrintingKubeClient.Wait(resources, d, waitForJobsEnabled) } // Delete returns the configured error if set or prints diff --git a/pkg/kube/fake/printer.go b/pkg/kube/fake/printer.go index 58b389ab5..b5f869c71 100644 --- a/pkg/kube/fake/printer.go +++ b/pkg/kube/fake/printer.go @@ -47,7 +47,7 @@ func (p *PrintingKubeClient) Create(resources kube.ResourceList) (*kube.Result, return &kube.Result{Created: resources}, nil } -func (p *PrintingKubeClient) Wait(resources kube.ResourceList, _ time.Duration) error { +func (p *PrintingKubeClient) Wait(resources kube.ResourceList, _ time.Duration, _ bool) error { _, err := io.Copy(p.Out, bufferize(resources)) return err } diff --git a/pkg/kube/interface.go b/pkg/kube/interface.go index 4bf61211e..d89abed34 100644 --- a/pkg/kube/interface.go +++ b/pkg/kube/interface.go @@ -30,7 +30,7 @@ type Interface interface { // Create creates one or more resources. Create(resources ResourceList) (*Result, error) - Wait(resources ResourceList, timeout time.Duration) error + Wait(resources ResourceList, timeout time.Duration, waitForJobsEnabled bool) error // Delete destroys one or more resources. Delete(resources ResourceList) (*Result, []error) diff --git a/pkg/kube/wait.go b/pkg/kube/wait.go index 3381b7881..40f7b7a6e 100644 --- a/pkg/kube/wait.go +++ b/pkg/kube/wait.go @@ -47,9 +47,9 @@ type waiter struct { log func(string, ...interface{}) } -// waitForResources polls to get the current status of all pods, PVCs, and Services -// until all are ready or a timeout is reached -func (w *waiter) waitForResources(created ResourceList) error { +// waitForResources polls to get the current status of all pods, PVCs, Services and +// Jobs(optional) until all are ready or a timeout is reached +func (w *waiter) waitForResources(created ResourceList, waitForJobsEnabled bool) error { w.log("beginning wait for %d resources with timeout of %v", len(created), w.timeout) return wait.Poll(2*time.Second, w.timeout, func() (bool, error) { @@ -68,9 +68,11 @@ func (w *waiter) waitForResources(created ResourceList) error { return false, err } case *batchv1.Job: - job, err := w.c.BatchV1().Jobs(v.Namespace).Get(context.Background(), v.Name, metav1.GetOptions{}) - if err != nil || !w.jobReady(job) { - return false, err + if waitForJobsEnabled { + job, err := w.c.BatchV1().Jobs(v.Namespace).Get(context.Background(), v.Name, metav1.GetOptions{}) + if err != nil || !w.jobReady(job) { + return false, err + } } case *appsv1.Deployment, *appsv1beta1.Deployment, *appsv1beta2.Deployment, *extensionsv1beta1.Deployment: currentDeployment, err := w.c.AppsV1().Deployments(v.Namespace).Get(context.Background(), v.Name, metav1.GetOptions{}) From bfc575dec2f6ed5ce897c38d0d89b0fe936606c0 Mon Sep 17 00:00:00 2001 From: zh168654 Date: Thu, 5 Nov 2020 17:13:15 +0800 Subject: [PATCH 10/25] add waitwithjobs instead of changing wait api Signed-off-by: zh168654 --- pkg/action/install.go | 12 +++++++++--- pkg/action/rollback.go | 19 ++++++++++++++----- pkg/action/upgrade.go | 13 ++++++++++--- pkg/kube/client.go | 18 ++++++++++++++++-- pkg/kube/fake/fake.go | 12 ++++++++++-- pkg/kube/fake/printer.go | 7 ++++++- pkg/kube/interface.go | 4 +++- 7 files changed, 68 insertions(+), 17 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index 6ef754a45..4de0b64e6 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -157,7 +157,7 @@ func (i *Install) installCRDs(crds []chart.CRD) error { discoveryClient.Invalidate() // Give time for the CRD to be recognized. - if err := i.cfg.KubeClient.Wait(totalItems, 60*time.Second, false); err != nil { + if err := i.cfg.KubeClient.Wait(totalItems, 60*time.Second); err != nil { return err } @@ -346,8 +346,14 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. } if i.Wait { - if err := i.cfg.KubeClient.Wait(resources, i.Timeout, i.WaitForJobs); err != nil { - return i.failRelease(rel, err) + if i.WaitForJobs { + if err := i.cfg.KubeClient.WaitWithJobs(resources, i.Timeout); err != nil { + return i.failRelease(rel, err) + } + } else { + if err := i.cfg.KubeClient.Wait(resources, i.Timeout); err != nil { + return i.failRelease(rel, err) + } } } diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index 5c3fabaee..f3f958f3d 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -200,11 +200,20 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas } if r.Wait { - if err := r.cfg.KubeClient.Wait(target, r.Timeout, r.WaitForJobs); err != nil { - targetRelease.SetStatus(release.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) - r.cfg.recordRelease(currentRelease) - r.cfg.recordRelease(targetRelease) - return targetRelease, errors.Wrapf(err, "release %s failed", targetRelease.Name) + if r.WaitForJobs { + if err := r.cfg.KubeClient.WaitWithJobs(target, r.Timeout); err != nil { + targetRelease.SetStatus(release.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) + r.cfg.recordRelease(currentRelease) + r.cfg.recordRelease(targetRelease) + return targetRelease, errors.Wrapf(err, "release %s failed", targetRelease.Name) + } + } else { + if err := r.cfg.KubeClient.Wait(target, r.Timeout); err != nil { + targetRelease.SetStatus(release.StatusFailed, fmt.Sprintf("Release %q failed: %s", targetRelease.Name, err.Error())) + r.cfg.recordRelease(currentRelease) + r.cfg.recordRelease(targetRelease) + return targetRelease, errors.Wrapf(err, "release %s failed", targetRelease.Name) + } } } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index db74e1ece..b0f294cae 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -331,9 +331,16 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea } if u.Wait { - if err := u.cfg.KubeClient.Wait(target, u.Timeout, u.WaitForJobs); err != nil { - u.cfg.recordRelease(originalRelease) - return u.failRelease(upgradedRelease, results.Created, err) + if u.WaitForJobs { + if err := u.cfg.KubeClient.WaitWithJobs(target, u.Timeout); err != nil { + u.cfg.recordRelease(originalRelease) + return u.failRelease(upgradedRelease, results.Created, err) + } + } else { + if err := u.cfg.KubeClient.Wait(target, u.Timeout); err != nil { + u.cfg.recordRelease(originalRelease) + return u.failRelease(upgradedRelease, results.Created, err) + } } } diff --git a/pkg/kube/client.go b/pkg/kube/client.go index d1681a534..afa0cb5b3 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -127,7 +127,7 @@ func (c *Client) Create(resources ResourceList) (*Result, error) { } // Wait up to the given timeout for the specified resources to be ready -func (c *Client) Wait(resources ResourceList, timeout time.Duration, waitForJobsEnabled bool) error { +func (c *Client) Wait(resources ResourceList, timeout time.Duration) error { cs, err := c.getKubeClient() if err != nil { return err @@ -137,7 +137,21 @@ func (c *Client) Wait(resources ResourceList, timeout time.Duration, waitForJobs log: c.Log, timeout: timeout, } - return w.waitForResources(resources, waitForJobsEnabled) + return w.waitForResources(resources, false) +} + +// WaitWithJobs wait up to the given timeout for the specified resources to be ready, including jobs. +func (c *Client) WaitWithJobs(resources ResourceList, timeout time.Duration) error { + cs, err := c.getKubeClient() + if err != nil { + return err + } + w := waiter{ + c: cs, + log: c.Log, + timeout: timeout, + } + return w.waitForResources(resources, true) } func (c *Client) namespace() string { diff --git a/pkg/kube/fake/fake.go b/pkg/kube/fake/fake.go index 55b887ab3..ff800864c 100644 --- a/pkg/kube/fake/fake.go +++ b/pkg/kube/fake/fake.go @@ -51,11 +51,19 @@ func (f *FailingKubeClient) Create(resources kube.ResourceList) (*kube.Result, e } // Wait returns the configured error if set or prints -func (f *FailingKubeClient) Wait(resources kube.ResourceList, d time.Duration, waitForJobsEnabled bool) error { +func (f *FailingKubeClient) Wait(resources kube.ResourceList, d time.Duration) error { if f.WaitError != nil { return f.WaitError } - return f.PrintingKubeClient.Wait(resources, d, waitForJobsEnabled) + return f.PrintingKubeClient.Wait(resources, d) +} + +// WaitWithJobs returns the configured error if set or prints +func (f *FailingKubeClient) WaitWithJobs(resources kube.ResourceList, d time.Duration) error { + if f.WaitError != nil { + return f.WaitError + } + return f.PrintingKubeClient.Wait(resources, d) } // Delete returns the configured error if set or prints diff --git a/pkg/kube/fake/printer.go b/pkg/kube/fake/printer.go index b5f869c71..e8bd1845b 100644 --- a/pkg/kube/fake/printer.go +++ b/pkg/kube/fake/printer.go @@ -47,7 +47,12 @@ func (p *PrintingKubeClient) Create(resources kube.ResourceList) (*kube.Result, return &kube.Result{Created: resources}, nil } -func (p *PrintingKubeClient) Wait(resources kube.ResourceList, _ time.Duration, _ bool) error { +func (p *PrintingKubeClient) Wait(resources kube.ResourceList, _ time.Duration) error { + _, err := io.Copy(p.Out, bufferize(resources)) + return err +} + +func (p *PrintingKubeClient) WaitWithJobs(resources kube.ResourceList, _ time.Duration) error { _, err := io.Copy(p.Out, bufferize(resources)) return err } diff --git a/pkg/kube/interface.go b/pkg/kube/interface.go index d89abed34..545985996 100644 --- a/pkg/kube/interface.go +++ b/pkg/kube/interface.go @@ -30,7 +30,9 @@ type Interface interface { // Create creates one or more resources. Create(resources ResourceList) (*Result, error) - Wait(resources ResourceList, timeout time.Duration, waitForJobsEnabled bool) error + Wait(resources ResourceList, timeout time.Duration) error + + WaitWithJobs(resources ResourceList, timeout time.Duration) error // Delete destroys one or more resources. Delete(resources ResourceList) (*Result, []error) From 3ad08f3ea9c09d16ddf6519d65f3f6f2ceee2c37 Mon Sep 17 00:00:00 2001 From: Peter Engelbert Date: Thu, 1 Oct 2020 16:37:44 -0500 Subject: [PATCH 11/25] Implement `helm pull` for OCI registries * Implement `helm dep update` for oci dependencies * New unit tests * Remove `helm chart pull` command * New `helm pull` does not depend on registry cache Signed-off-by: Peter Engelbert --- cmd/helm/dependency.go | 4 +- cmd/helm/dependency_update.go | 3 +- cmd/helm/dependency_update_test.go | 46 +++++ cmd/helm/pull.go | 13 +- cmd/helm/pull_test.go | 58 +++++- cmd/helm/root.go | 23 ++- .../testcharts/oci-dependent-chart-0.1.0.tgz | Bin 0 -> 3599 bytes internal/experimental/registry/client.go | 61 +++++- internal/experimental/registry/client_test.go | 6 +- internal/resolver/resolver.go | 39 +++- pkg/action/chart_pull.go | 2 +- pkg/action/pull.go | 17 +- pkg/downloader/chart_downloader.go | 6 + pkg/downloader/manager.go | 60 +++++- pkg/getter/getter.go | 29 ++- pkg/getter/getter_test.go | 2 +- pkg/getter/ocigetter.go | 69 +++++++ pkg/repo/repotest/server.go | 192 +++++++++++++++++- 18 files changed, 585 insertions(+), 45 deletions(-) create mode 100644 cmd/helm/testdata/testcharts/oci-dependent-chart-0.1.0.tgz create mode 100644 pkg/getter/ocigetter.go diff --git a/cmd/helm/dependency.go b/cmd/helm/dependency.go index 2cc4c5045..3de3ef014 100644 --- a/cmd/helm/dependency.go +++ b/cmd/helm/dependency.go @@ -82,7 +82,7 @@ the contents of a chart. This will produce an error if the chart cannot be loaded. ` -func newDependencyCmd(out io.Writer) *cobra.Command { +func newDependencyCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { cmd := &cobra.Command{ Use: "dependency update|build|list", Aliases: []string{"dep", "dependencies"}, @@ -92,7 +92,7 @@ func newDependencyCmd(out io.Writer) *cobra.Command { } cmd.AddCommand(newDependencyListCmd(out)) - cmd.AddCommand(newDependencyUpdateCmd(out)) + cmd.AddCommand(newDependencyUpdateCmd(cfg, out)) cmd.AddCommand(newDependencyBuildCmd(out)) return cmd diff --git a/cmd/helm/dependency_update.go b/cmd/helm/dependency_update.go index 9855afb92..ad0188f17 100644 --- a/cmd/helm/dependency_update.go +++ b/cmd/helm/dependency_update.go @@ -43,7 +43,7 @@ in the Chart.yaml file, but (b) at the wrong version. ` // newDependencyUpdateCmd creates a new dependency update command. -func newDependencyUpdateCmd(out io.Writer) *cobra.Command { +func newDependencyUpdateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client := action.NewDependency() cmd := &cobra.Command{ @@ -63,6 +63,7 @@ func newDependencyUpdateCmd(out io.Writer) *cobra.Command { Keyring: client.Keyring, SkipUpdate: client.SkipRefresh, Getters: getter.All(settings), + RegistryClient: cfg.RegistryClient, RepositoryConfig: settings.RepositoryConfig, RepositoryCache: settings.RepositoryCache, Debug: settings.Debug, diff --git a/cmd/helm/dependency_update_test.go b/cmd/helm/dependency_update_test.go index bf27c7b6c..ce93e5c41 100644 --- a/cmd/helm/dependency_update_test.go +++ b/cmd/helm/dependency_update_test.go @@ -40,6 +40,23 @@ func TestDependencyUpdateCmd(t *testing.T) { defer srv.Stop() t.Logf("Listening on directory %s", srv.Root()) + ociSrv, err := repotest.NewOCIServer(t, srv.Root()) + if err != nil { + t.Fatal(err) + } + + ociChartName := "oci-depending-chart" + c := createTestingMetadataForOCI(ociChartName, ociSrv.RegistryURL) + if err := chartutil.SaveDir(c, ociSrv.Dir); err != nil { + t.Fatal(err) + } + ociSrv.Run(t, repotest.WithDependingChart(c)) + + err = os.Setenv("HELM_EXPERIMENTAL_OCI", "1") + if err != nil { + t.Fatal("failed to set environment variable enabling OCI support") + } + if err := srv.LinkIndices(); err != nil { t.Fatal(err) } @@ -115,6 +132,22 @@ func TestDependencyUpdateCmd(t *testing.T) { if _, err := os.Stat(unexpected); err == nil { t.Fatalf("Unexpected %q", unexpected) } + + // test for OCI charts + cmd := fmt.Sprintf("dependency update '%s' --repository-config %s --repository-cache %s --registry-config %s/config.json", + dir(ociChartName), + dir("repositories.yaml"), + dir(), + dir()) + _, out, err = executeActionCommand(cmd) + if err != nil { + t.Logf("Output: %s", out) + t.Fatal(err) + } + expect = dir(ociChartName, "charts/oci-dependent-chart") + if _, err := os.Stat(expect); err != nil { + t.Fatal(err) + } } func TestDependencyUpdateCmd_DontDeleteOldChartsOnError(t *testing.T) { @@ -193,6 +226,19 @@ func createTestingMetadata(name, baseURL string) *chart.Chart { } } +func createTestingMetadataForOCI(name, registryURL string) *chart.Chart { + return &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV2, + Name: name, + Version: "1.2.3", + Dependencies: []*chart.Dependency{ + {Name: "oci-dependent-chart", Version: "0.1.0", Repository: fmt.Sprintf("oci://%s/u/ocitestuser", registryURL)}, + }, + }, + } +} + // createTestingChart creates a basic chart that depends on reqtest-0.1.0 // // The baseURL can be used to point to a particular repository server. diff --git a/cmd/helm/pull.go b/cmd/helm/pull.go index 3f62bf0c7..ded0609e5 100644 --- a/cmd/helm/pull.go +++ b/cmd/helm/pull.go @@ -20,6 +20,7 @@ import ( "fmt" "io" "log" + "strings" "github.com/spf13/cobra" @@ -42,8 +43,8 @@ file, and MUST pass the verification process. Failure in any part of this will result in an error, and the chart will not be saved locally. ` -func newPullCmd(out io.Writer) *cobra.Command { - client := action.NewPull() +func newPullCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { + client := action.NewPull(cfg) cmd := &cobra.Command{ Use: "pull [chart URL | repo/chartname] [...]", @@ -64,6 +65,14 @@ func newPullCmd(out io.Writer) *cobra.Command { client.Version = ">0.0.0-0" } + if strings.HasPrefix(args[0], "oci://") { + if !FeatureGateOCI.IsEnabled() { + return FeatureGateOCI.Error() + } + + client.OCI = true + } + for i := 0; i < len(args); i++ { output, err := client.Run(args[i]) if err != nil { diff --git a/cmd/helm/pull_test.go b/cmd/helm/pull_test.go index 1d439e873..51cdfdfa4 100644 --- a/cmd/helm/pull_test.go +++ b/cmd/helm/pull_test.go @@ -32,6 +32,13 @@ func TestPullCmd(t *testing.T) { } defer srv.Stop() + os.Setenv("HELM_EXPERIMENTAL_OCI", "1") + ociSrv, err := repotest.NewOCIServer(t, srv.Root()) + if err != nil { + t.Fatal(err) + } + ociSrv.Run(t) + if err := srv.LinkIndices(); err != nil { t.Fatal(err) } @@ -139,23 +146,70 @@ func TestPullCmd(t *testing.T) { failExpect: "Failed to fetch chart version", wantError: true, }, + { + name: "Fetch OCI Chart", + args: fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart --version 0.1.0", ociSrv.RegistryURL), + expectFile: "./oci-dependent-chart-0.1.0.tgz", + }, + { + name: "Fetch OCI Chart with untar", + args: fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart --version 0.1.0 --untar", ociSrv.RegistryURL), + expectFile: "./oci-dependent-chart", + expectDir: true, + }, + { + name: "Fetch OCI Chart with untar and untardir", + args: fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart --version 0.1.0 --untar --untardir ocitest2", ociSrv.RegistryURL), + expectFile: "./ocitest2", + expectDir: true, + }, + { + name: "OCI Fetch untar when dir with same name existed", + args: fmt.Sprintf("oci-test-chart oci://%s/u/ocitestuser/oci-dependent-chart --version 0.1.0 --untar --untardir ocitest2 --untar --untardir ocitest2", ociSrv.RegistryURL), + wantError: true, + wantErrorMsg: fmt.Sprintf("failed to untar: a file or directory with the name %s already exists", filepath.Join(srv.Root(), "ocitest2")), + }, + { + name: "Fail fetching non-existent OCI chart", + args: fmt.Sprintf("oci://%s/u/ocitestuser/nosuchthing --version 0.1.0", ociSrv.RegistryURL), + failExpect: "Failed to fetch", + wantError: true, + }, + { + name: "Fail fetching OCI chart without version specified", + args: fmt.Sprintf("oci://%s/u/ocitestuser/nosuchthing", ociSrv.RegistryURL), + wantErrorMsg: "Error: --version flag is explicitly required for OCI registries", + wantError: true, + }, + { + name: "Fail fetching OCI chart without version specified", + args: fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart:0.1.0", ociSrv.RegistryURL), + wantErrorMsg: "Error: --version flag is explicitly required for OCI registries", + wantError: true, + }, + { + name: "Fail fetching OCI chart without version specified", + args: fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart:0.1.0 --version 0.1.0", ociSrv.RegistryURL), + wantError: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { outdir := srv.Root() - cmd := fmt.Sprintf("fetch %s -d '%s' --repository-config %s --repository-cache %s ", + cmd := fmt.Sprintf("fetch %s -d '%s' --repository-config %s --repository-cache %s --registry-config %s", tt.args, outdir, filepath.Join(outdir, "repositories.yaml"), outdir, + filepath.Join(outdir, "config.json"), ) // Create file or Dir before helm pull --untar, see: https://github.com/helm/helm/issues/7182 if tt.existFile != "" { file := filepath.Join(outdir, tt.existFile) _, err := os.Create(file) if err != nil { - t.Fatal("err") + t.Fatal(err) } } if tt.existDir != "" { diff --git a/cmd/helm/root.go b/cmd/helm/root.go index f2be0b5a9..8025a9ddf 100644 --- a/cmd/helm/root.go +++ b/cmd/helm/root.go @@ -153,12 +153,22 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string flags.ParseErrorsWhitelist.UnknownFlags = true flags.Parse(args) + registryClient, err := registry.NewClient( + registry.ClientOptDebug(settings.Debug), + registry.ClientOptWriter(out), + registry.ClientOptCredentialsFile(settings.RegistryConfig), + ) + if err != nil { + return nil, err + } + actionConfig.RegistryClient = registryClient + // Add subcommands cmd.AddCommand( // chart commands newCreateCmd(out), - newDependencyCmd(out), - newPullCmd(out), + newDependencyCmd(actionConfig, out), + newPullCmd(actionConfig, out), newShowCmd(out), newLintCmd(out), newPackageCmd(out), @@ -188,15 +198,6 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string ) // Add *experimental* subcommands - registryClient, err := registry.NewClient( - registry.ClientOptDebug(settings.Debug), - registry.ClientOptWriter(out), - registry.ClientOptCredentialsFile(settings.RegistryConfig), - ) - if err != nil { - return nil, err - } - actionConfig.RegistryClient = registryClient cmd.AddCommand( newRegistryCmd(actionConfig, out), newChartCmd(actionConfig, out), diff --git a/cmd/helm/testdata/testcharts/oci-dependent-chart-0.1.0.tgz b/cmd/helm/testdata/testcharts/oci-dependent-chart-0.1.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..7b4cbeccc1c06dd27ff6376bcc6607ad03fa3839 GIT binary patch literal 3599 zcmV+q4)F0GiwG0|00000|0w_~VMtOiV@ORlOnEsqVl!4SWK%V1T2nbTPgYhoO;>Dc zVQyr3R8em|NM&qo0PH+#Z`(Mw{j6Vcu9HO{x0Yq+(Lz8kkel|-2HU1a(&=I_7!s#+V?Fk?U?WB~tf}&8NR0 zGxp?OMi2zS;r_n;8w5e~Z!icBpY-=$?H%+7d;R|4NzgwCg259A?uY#Pkt$8(li<#@ zn!^1>21)2=l!^)-!hGP7Bq_f3{r;gJcrmI-(nQ;PNAP!KGqCFf#zMkB(h*9I8kNV% z3`1yHP@Y~S7y?NWMk8VndGnk|;P?H&`_WqX&mC>{KPN0jb$yPooQzL}9!gZmwFj^RALl3~jSbx?g2e-xOyF`V6hfQ{O5J1U!%Bze zdtiV==yWn3hACs`7)jJBgkOKN4lXBQ!Nw_LD>prba!w;WiXtnLj^LxSXP% zq67jN91sTxYAR9|8+}C*iN@H2>?#B;Q?!VVI2YRbP^(-L$L5cbr-9A`ASG-F%WM1o zrzwJA8N|5lErTpo=v&y6F>s+lp$5X^j2Ejunc_Ni6s8W+2TwH{eP69S}2XPik@Z4kJPE)2B3NSXN59^e#VN`pP}HQ+%Zd) zMtL$ci&aP+!t22ED4$4FmMP@MG~y7(geh#DCPV3>2&_I8z3hEIVnnIZ8dd={Q(Y2S zH5;Zo9>7#6Z7CZCm@lDQ`(3;uvxK3~L`0Z<4v-K%b*mjfv;1nuysE4zoZ308A?RwR zG7Xo+b{xKLD=kl=5~+-^T$ukvNL5N0TY$t!%x1`AyZp2OWvypXSa9*SU6)z_Lo!Fu z#y=6`pCZ9kL`CY@il)LWapT#{%1jkX-#IhFlmN=j%2ucht2}alrB-ILL2y7mr&914 za;@N!>M1a)HOm%6&dN(rX*4zDKTuB1r1-{w79xdMz$M`|Nh+_U+)1mQ*$BqOCFK)~ zol$N;Nc?-M?DLr+z%fFlh+Mq1@=KfvD?LJ#O16NJBvv=`}H@^BjYjU zR4?q~A03k)a6!Pel**aWbTL?+`1Yy^N1PI@(K-*AZ zU;#>K-v%IecdB)=TpB{V&3833FlxC36DF>&!(MN>bfhd^xV~O4!7s5pFt2&Z6dL@I z;F?gbpmQo>915vB1-n^<_f2?r`0&aEb~yd`_T87^$FomwcApu4A87^?)X?pkJVzxY zLNH96Av58{KP~QFUqe^G?@DkZ?VHZhH*vu?SkC4JQ+5aG$hnFu}U@0ESE|XohqNiv6&LI_p&e79m_y86?PKCUT-&JBit?2 zcEb$37bN~)!b~KV>t+D}jC%^1Atu$?UdF8KDoQvRX1fwHCzP25 z>u0%-9lwYaBEsR4=xFLvWGkmhm@5|X^wu-3<`Hx+Z#j=o%XvJ1clP<@xMeCW;%zv% z9ck!x^FRQd<+8I}W+wWK@))lmpMO?SZf>6tJhv9;TQhV9*ST(46{S$2VY@NeR_+^3 zwaB!$u+`zmw_i?=KD=wFE)nszM!=v$lEklP+vdeLo77sW^yMlB%9S>%d()(BGni5D^O-8D z^C|BSUipC^`2Fy8@H)u$o31KQ2|#Awl5@Tl$A*jUSjbBv;|lwRMz~v7N;L@l=Fd^-V5*DRbn^NjoMIF_b`3wS$~%M=xV_XzJiu1kKl06UeeilIv&$2=yW@cbj`Dz9=jS8DJYnj zwSYfT!swV{Y1RWc2ilrfWcq{{HAOs7&?Db-M^_$ z1D7MZ{ZebctS!LG%o@wd+F52+?d)bt#X&JLLbl}$+s|@(n_^Wp?yj@qHGBn$2{Xa4 z^WPoleCgcYw8U&CxZ_rB$E|~P>`sx*d^PRaC*j6iGZWNztgMGsS?%V1M$WIStV|6@ zMH$~bTTQcyexpesR$loEf22ZVLoKUVW`vUo#@*2(tYsqSSrNTWt$E?;R*G#(+-+sY zmD9IRKU+}b>1Vi-S*=_B%|OxKMA%l;YPaO{Cht}U^Rr$=BiuaOc6;;uCcL~VdBwlv z6gt_(%9f)Q!HN(0(b0Z&tk3^qOqf{Akl*Pz$j1C{Kd9$_gP=dyd(8hHVpPw6kR(yP zdEdLBJPzS)G521+-3*H!-t9^W9%SCnb)l?juX~#nj{YlfYM;Qq!DAAxqWZ{%F08Q9 zbf{J#Z4Wu?EL7?X)s}O^LIHB$>v)|J9Zla?nBRi^)p%8kap;KTCYBMY=-1)5ZKj;v zdb6~se$^U>MG4Eit;H{7#%gq5=daadHG|cJyQ6po88=fU#+pFQ?t010O5c5JxmqM! z4K14&Zd*9=t0?Q8=T`R0X>0Ve+xafbDJ8mYX=l}>uXtxIFE{X`Ze*rEsAF~~FrZU0 z8GFkwPh`OLb-40alL>rn?hk6IS*J;5SUJzJg0DR6QRmN=oa*xC3Y%)3)E-pyJIveh z_2-EcT10|{@acG1dB*4*IVv@jVuW>ShRRRaP`PWKhtO-@wPYKNNhC4-j@a{_@1{jj zw$j^|!;0-zo2eyOt;Kp5n_Z!ocV;VYFYndfE1gP9xP+?qiNB57kB;w~#`^qknvi=q z2Dl;r>mMElEB)X8{$u|45Tmipx*?CMWr-z7Z$22ICV9sAcOmJY#@Vu=h>tQbl>ct& zBY@@WlRZ3ghGU1dEfF^NSCV1t?!HZm+R>Lvw5w{p0+YQ|Jsxw@OC?P95^6~!tFSdQ zy#AO??$5f}$+ojBRCx6mQ#wYcl_*JzJC4B793`1xg~*M9Y{OB_mv5AA&FP?T6wsY_ zx~q&E`FfZN!g~LoAGf$$4A|iR2f<#`Y*`EssCr(Z6k*(C#J%+BfU34~Aq*)tS zsSDhYvF@)pnmj<>uqxcCtL$WOfjK+4brWoU>bQ|XXKz&wfw|GOZnIjFufEmT-v7!QSRem$C;eUHe{k48Sc(6^WBva@ zM(y{X`9?15Xa4(b++3dCa#FB7@K>n~cC+Hxnv;dq6n@yOq_q_WYgNX2tKd$B^Zg&> zShuqOtgrj6ZeWxBAMCC4|6UzD)_)#kY}ICOrpQf4k%51MzgW|5`EGf0L&kedpZ|h6 z(t7*XsI>d+MuKyUj8<;#1~$h3y~gi<27|%iasU4yqt&d3B5twe&h3JKx3O$G_h{2A zCfdo5pG}3h=!9Ts7g2G-hN=}ry zc{_cq`GJGQEYYk)oi{^IagOr)yOB2g_l+{?gbRuHLSu9MZ|468gU9~Q!;Bpm60K3X ze<$E39WJNHVU$u9Q$B%&L>FX&s`wWWDot#b4Qh&v!GwvCJ10Z=&U+=I5s5C+#QqZt<-PFLW#YZ?N94kL z;p>^X7Lrc97ys|=z-J<k>YoZwCB?pNzZ{x@w?~GRU~5U{biDX$MHBG V$A9GbR{#J2|NqD05>x Date: Mon, 5 Oct 2020 12:20:04 -0500 Subject: [PATCH 12/25] Clean up imports and add doc comments Additionally, revert `NewPull()` to its existing signature. Signed-off-by: Peter Engelbert --- cmd/helm/pull.go | 2 +- go.mod | 1 + internal/experimental/registry/client.go | 12 ++++--- internal/experimental/registry/client_test.go | 20 ++--------- internal/resolver/resolver.go | 6 ++-- pkg/action/pull.go | 24 +++++++++++-- pkg/repo/repotest/server.go | 35 +++++-------------- 7 files changed, 44 insertions(+), 56 deletions(-) diff --git a/cmd/helm/pull.go b/cmd/helm/pull.go index ded0609e5..b094db6c3 100644 --- a/cmd/helm/pull.go +++ b/cmd/helm/pull.go @@ -44,7 +44,7 @@ result in an error, and the chart will not be saved locally. ` func newPullCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { - client := action.NewPull(cfg) + client := action.NewPullWithOpts(action.WithConfig(cfg)) cmd := &cobra.Command{ Use: "pull [chart URL | repo/chartname] [...]", diff --git a/go.mod b/go.mod index 971f4949d..8b182f6d3 100644 --- a/go.mod +++ b/go.mod @@ -27,6 +27,7 @@ require ( github.com/mitchellh/copystructure v1.0.0 github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.0.1 + github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2 github.com/pkg/errors v0.9.1 github.com/rubenv/sql-migrate v0.0.0-20200616145509-8d140a17f351 github.com/sirupsen/logrus v1.7.0 diff --git a/internal/experimental/registry/client.go b/internal/experimental/registry/client.go index 55b34d68f..c889ee913 100644 --- a/internal/experimental/registry/client.go +++ b/internal/experimental/registry/client.go @@ -25,16 +25,15 @@ import ( "net/http" "sort" - "helm.sh/helm/v3/pkg/chart" - "helm.sh/helm/v3/pkg/helmpath" - - "github.com/deislabs/oras/pkg/content" - auth "github.com/deislabs/oras/pkg/auth/docker" + "github.com/deislabs/oras/pkg/content" "github.com/deislabs/oras/pkg/oras" "github.com/gosuri/uitable" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" + + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/helmpath" ) const ( @@ -197,6 +196,9 @@ func (c *Client) PullChart(ref *Reference) (*bytes.Buffer, error) { return buf, nil } +// PullChartToCache pulls a chart from an OCI Registry to the Registry Cache. +// This function is needed for `helm chart pull`, which is experimental and will be deprecated soon. +// Likewise, the Registry cache will soon be deprecated as will this function. func (c *Client) PullChartToCache(ref *Reference) error { if ref.Tag == "" { return errors.New("tag explicitly required") diff --git a/internal/experimental/registry/client_test.go b/internal/experimental/registry/client_test.go index 0d5d508d5..a9936ba13 100644 --- a/internal/experimental/registry/client_test.go +++ b/internal/experimental/registry/client_test.go @@ -22,7 +22,6 @@ import ( "fmt" "io" "io/ioutil" - "net" "net/http" "net/http/httptest" "net/url" @@ -33,12 +32,12 @@ import ( "time" "github.com/containerd/containerd/errdefs" - auth "github.com/deislabs/oras/pkg/auth/docker" "github.com/docker/distribution/configuration" "github.com/docker/distribution/registry" _ "github.com/docker/distribution/registry/auth/htpasswd" _ "github.com/docker/distribution/registry/storage/driver/inmemory" + "github.com/phayes/freeport" "github.com/stretchr/testify/suite" "golang.org/x/crypto/bcrypt" @@ -107,7 +106,7 @@ func (suite *RegistryClientTestSuite) SetupSuite() { // Registry config config := &configuration.Configuration{} - port, err := getFreePort() + port, err := freeport.GetFreePort() suite.Nil(err, "no error finding free port for test registry") suite.DockerRegistryHost = fmt.Sprintf("localhost:%d", port) config.HTTP.Addr = fmt.Sprintf(":%d", port) @@ -254,21 +253,6 @@ func TestRegistryClientTestSuite(t *testing.T) { suite.Run(t, new(RegistryClientTestSuite)) } -// borrowed from https://github.com/phayes/freeport -func getFreePort() (int, error) { - addr, err := net.ResolveTCPAddr("tcp", "localhost:0") - if err != nil { - return 0, err - } - - l, err := net.ListenTCP("tcp", addr) - if err != nil { - return 0, err - } - defer l.Close() - return l.Addr().(*net.TCPAddr).Port, nil -} - func initCompromisedRegistryTestServer() string { s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.Contains(r.URL.Path, "manifests") { diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 6692942a1..de0634093 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -23,15 +23,15 @@ import ( "strings" "time" + "github.com/Masterminds/semver/v3" + "github.com/pkg/errors" + "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/gates" "helm.sh/helm/v3/pkg/helmpath" "helm.sh/helm/v3/pkg/provenance" "helm.sh/helm/v3/pkg/repo" - - "github.com/Masterminds/semver/v3" - "github.com/pkg/errors" ) const FeatureGateOCI = gates.Gate("HELM_EXPERIMENTAL_OCI") diff --git a/pkg/action/pull.go b/pkg/action/pull.go index 258685441..a7f69c4bc 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -49,9 +49,27 @@ type Pull struct { cfg *Configuration } -// NewPull creates a new Pull object with the given configuration. -func NewPull(cfg *Configuration) *Pull { - return &Pull{cfg: cfg} +type PullOpt func(*Pull) + +func WithConfig(cfg *Configuration) PullOpt { + return func(p *Pull) { + p.cfg = cfg + } +} + +// NewPull creates a new Pull object. +func NewPull() *Pull { + return NewPullWithOpts() +} + +// NewPull creates a new pull, with configuration options. +func NewPullWithOpts(opts ...PullOpt) *Pull { + p := &Pull{} + for _, fn := range opts { + fn(p) + } + + return p } // Run executes 'helm pull' against the given release. diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go index 7dc60e948..94b5ce7f9 100644 --- a/pkg/repo/repotest/server.go +++ b/pkg/repo/repotest/server.go @@ -19,7 +19,6 @@ import ( "context" "fmt" "io/ioutil" - "net" "net/http" "net/http/httptest" "os" @@ -27,23 +26,21 @@ import ( "testing" "time" - "helm.sh/helm/v3/internal/tlsutil" - "helm.sh/helm/v3/pkg/chart" - "helm.sh/helm/v3/pkg/chart/loader" - "helm.sh/helm/v3/pkg/chartutil" - "helm.sh/helm/v3/pkg/repo" - - "sigs.k8s.io/yaml" - auth "github.com/deislabs/oras/pkg/auth/docker" "github.com/docker/distribution/configuration" "github.com/docker/distribution/registry" _ "github.com/docker/distribution/registry/auth/htpasswd" // used for docker test registry _ "github.com/docker/distribution/registry/storage/driver/inmemory" // used for docker test registry + "github.com/phayes/freeport" + "golang.org/x/crypto/bcrypt" + "sigs.k8s.io/yaml" ociRegistry "helm.sh/helm/v3/internal/experimental/registry" - - "golang.org/x/crypto/bcrypt" + "helm.sh/helm/v3/internal/tlsutil" + "helm.sh/helm/v3/pkg/chart" + "helm.sh/helm/v3/pkg/chart/loader" + "helm.sh/helm/v3/pkg/chartutil" + "helm.sh/helm/v3/pkg/repo" ) // NewTempServerWithCleanup creates a server inside of a temp dir. @@ -96,7 +93,7 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) { // Registry config config := &configuration.Configuration{} - port, err := getFreePort() + port, err := freeport.GetFreePort() if err != nil { t.Fatalf("error finding free port for test registry") } @@ -404,17 +401,3 @@ func setTestingRepository(url, fname string) error { }) return r.WriteFile(fname, 0644) } - -func getFreePort() (int, error) { - addr, err := net.ResolveTCPAddr("tcp", "localhost:0") - if err != nil { - return 0, err - } - - l, err := net.ListenTCP("tcp", addr) - if err != nil { - return 0, err - } - defer l.Close() - return l.Addr().(*net.TCPAddr).Port, nil -} From f666fceb30a1033f3309a4e47bedb6193791619e Mon Sep 17 00:00:00 2001 From: Peter Engelbert Date: Thu, 8 Oct 2020 10:26:55 -0500 Subject: [PATCH 13/25] Remove OCI boolean from struct Signed-off-by: Peter Engelbert --- cmd/helm/pull.go | 2 -- pkg/action/pull.go | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/cmd/helm/pull.go b/cmd/helm/pull.go index b094db6c3..7711320f1 100644 --- a/cmd/helm/pull.go +++ b/cmd/helm/pull.go @@ -69,8 +69,6 @@ func newPullCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if !FeatureGateOCI.IsEnabled() { return FeatureGateOCI.Error() } - - client.OCI = true } for i := 0; i < len(args); i++ { diff --git a/pkg/action/pull.go b/pkg/action/pull.go index a7f69c4bc..acd39bf05 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -43,7 +43,6 @@ type Pull struct { Devel bool Untar bool VerifyLater bool - OCI bool UntarDir string DestDir string cfg *Configuration @@ -90,7 +89,7 @@ func (p *Pull) Run(chartRef string) (string, error) { RepositoryCache: p.Settings.RepositoryCache, } - if p.OCI { + if strings.HasPrefix(chartRef, "oci://") { if p.Version == "" { return out.String(), errors.Errorf("--version flag is explicitly required for OCI registries") } From bed1a42a398b30a63a279d68cc7319ceb4618ec3 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Sat, 12 Dec 2020 21:54:11 -0500 Subject: [PATCH 14/25] fix(pkg/chartutil): Remove warning for nils Nil tables should not be reported as non-tables. Signed-off-by: Marc Khouzam --- pkg/chartutil/coalesce.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/chartutil/coalesce.go b/pkg/chartutil/coalesce.go index 1d3d45e99..e086d8b6e 100644 --- a/pkg/chartutil/coalesce.go +++ b/pkg/chartutil/coalesce.go @@ -157,7 +157,11 @@ func coalesceValues(c *chart.Chart, v map[string]interface{}) { // if v[key] is a table, merge nv's val table into v[key]. src, ok := val.(map[string]interface{}) if !ok { - log.Printf("warning: skipped value for %s: Not a table.", key) + // If the original value is nil, there is nothing to coalesce, so we don't print + // the warning but simply continue + if val != nil { + log.Printf("warning: skipped value for %s: Not a table.", key) + } continue } // Because v has higher precedence than nv, dest values override src @@ -195,7 +199,7 @@ func CoalesceTables(dst, src map[string]interface{}) map[string]interface{} { } else { log.Printf("warning: cannot overwrite table with non table for %s (%v)", key, val) } - } else if istable(dv) { + } else if istable(dv) && val != nil { log.Printf("warning: destination for %s is a table. Ignoring non-table value %v", key, val) } } From c495e88250a74f5e55e8c5eabfe8ae52ce248121 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Thu, 17 Dec 2020 14:17:04 -0500 Subject: [PATCH 15/25] Replace Helm Hub with Artifact Hub (#8626) * Replace Helm Hub with Artifact Hub Signed-off-by: Scott Rigby * Update link to new doc entry for Monocular compatible search API Signed-off-by: Scott Rigby * Add struct for Artifact Hub data, and return correct URL for both artifact hub instances and backwards compatibility for Monocular search API Signed-off-by: Scott Rigby * Keep default endpoint hub.helm.sh, so the helm org controls the domain. At least until artifacthub moves to CNCF incubation Signed-off-by: Scott Rigby --- README.md | 2 +- cmd/helm/search.go | 4 ++-- cmd/helm/search_hub.go | 39 +++++++++++++++++++++++++----------- internal/monocular/doc.go | 7 ++++--- internal/monocular/search.go | 6 ++++++ 5 files changed, 40 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index f294a8a61..7d2958f5e 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ Helm is a tool for managing Charts. Charts are packages of pre-configured Kubern Use Helm to: -- Find and use [popular software packaged as Helm Charts](https://hub.helm.sh) to run in Kubernetes +- Find and use [popular software packaged as Helm Charts](https://artifacthub.io/packages/search?kind=0) to run in Kubernetes - Share your own applications as Helm Charts - Create reproducible builds of your Kubernetes applications - Intelligently manage your Kubernetes manifest files diff --git a/cmd/helm/search.go b/cmd/helm/search.go index 240d5e7c7..6c62d5d2e 100644 --- a/cmd/helm/search.go +++ b/cmd/helm/search.go @@ -24,8 +24,8 @@ import ( const searchDesc = ` Search provides the ability to search for Helm charts in the various places -they can be stored including the Helm Hub and repositories you have added. Use -search subcommands to search different locations for charts. +they can be stored including the Artifact Hub and repositories you have added. +Use search subcommands to search different locations for charts. ` func newSearchCmd(out io.Writer) *cobra.Command { diff --git a/cmd/helm/search_hub.go b/cmd/helm/search_hub.go index 89139ec16..82b555788 100644 --- a/cmd/helm/search_hub.go +++ b/cmd/helm/search_hub.go @@ -30,15 +30,23 @@ import ( ) const searchHubDesc = ` -Search the Helm Hub or an instance of Monocular for Helm charts. - -The Helm Hub provides a centralized search for publicly available distributed -charts. It is maintained by the Helm project. It can be visited at -https://hub.helm.sh - -Monocular is a web-based application that enables the search and discovery of -charts from multiple Helm Chart repositories. It is the codebase that powers the -Helm Hub. You can find it at https://github.com/helm/monocular +Search for Helm charts in the Artifact Hub or your own hub instance. + +Artifact Hub is a web-based application that enables finding, installing, and +publishing packages and configurations for CNCF projects, including publicly +available distributed charts Helm charts. It is a Cloud Native Computing +Foundation sandbox project. You can browse the hub at https://artifacthub.io/ + +The [KEYWORD] argument accepts either a keyword string, or quoted string of rich +query options. For rich query options documentation, see +https://artifacthub.github.io/hub/api/?urls.primaryName=Monocular%20compatible%20search%20API#/Monocular/get_api_chartsvc_v1_charts_search + +Previous versions of Helm used an instance of Monocular as the default +'endpoint', so for backwards compatibility Artifact Hub is compatible with the +Monocular search API. Similarly, when setting the 'endpoint' flag, the specified +endpoint must also be implement a Monocular compatible search API endpoint. +Note that when specifying a Monocular instance as the 'endpoint', rich queries +are not supported. For API details, see https://github.com/helm/monocular ` type searchHubOptions struct { @@ -51,8 +59,8 @@ func newSearchHubCmd(out io.Writer) *cobra.Command { o := &searchHubOptions{} cmd := &cobra.Command{ - Use: "hub [keyword]", - Short: "search for charts in the Helm Hub or an instance of Monocular", + Use: "hub [KEYWORD]", + Short: "search for charts in the Artifact Hub or your own hub instance", Long: searchHubDesc, RunE: func(cmd *cobra.Command, args []string) error { return o.run(out, args) @@ -60,7 +68,7 @@ func newSearchHubCmd(out io.Writer) *cobra.Command { } f := cmd.Flags() - f.StringVar(&o.searchEndpoint, "endpoint", "https://hub.helm.sh", "monocular instance to query for charts") + f.StringVar(&o.searchEndpoint, "endpoint", "https://hub.helm.sh", "Hub instance to query for charts") f.UintVar(&o.maxColWidth, "max-col-width", 50, "maximum column width for output table") bindOutputFlag(cmd, &o.outputFormat) @@ -98,7 +106,14 @@ type hubSearchWriter struct { func newHubSearchWriter(results []monocular.SearchResult, endpoint string, columnWidth uint) *hubSearchWriter { var elements []hubChartElement for _, r := range results { + // Backwards compatibility for Monocular url := endpoint + "/charts/" + r.ID + + // Check for artifactHub compatibility + if r.ArtifactHub.PackageURL != "" { + url = r.ArtifactHub.PackageURL + } + elements = append(elements, hubChartElement{url, r.Relationships.LatestChartVersion.Data.Version, r.Relationships.LatestChartVersion.Data.AppVersion, r.Attributes.Description}) } return &hubSearchWriter{elements, columnWidth} diff --git a/internal/monocular/doc.go b/internal/monocular/doc.go index 485cfdd45..5d402d35f 100644 --- a/internal/monocular/doc.go +++ b/internal/monocular/doc.go @@ -14,8 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package monocular contains the logic for interacting with monocular instances -// like the Helm Hub. +// Package monocular contains the logic for interacting with a Monocular +// compatible search API endpoint. For example, as implemented by the Artifact +// Hub. // -// This is a library for interacting with monocular +// This is a library for interacting with a monocular compatible search API package monocular diff --git a/internal/monocular/search.go b/internal/monocular/search.go index 10e1f2136..3082ff361 100644 --- a/internal/monocular/search.go +++ b/internal/monocular/search.go @@ -40,12 +40,18 @@ const SearchPath = "api/chartsvc/v1/charts/search" // SearchResult represents an individual chart result type SearchResult struct { ID string `json:"id"` + ArtifactHub ArtifactHub `json:"artifactHub"` Type string `json:"type"` Attributes Chart `json:"attributes"` Links Links `json:"links"` Relationships Relationships `json:"relationships"` } +// ArtifactHub represents data specific to Artifact Hub instances +type ArtifactHub struct { + PackageURL string `json:"packageUrl"` +} + // Chart is the attributes for the chart type Chart struct { Name string `json:"name"` From a202fb0c0b73a1093609251476e0d8a1b76b3b8f Mon Sep 17 00:00:00 2001 From: Dinu Mathai Date: Fri, 18 Dec 2020 02:19:16 +0530 Subject: [PATCH 16/25] Fixed bug - The flags --cert-file/--key-file where ignored when --insecure-skip-tls-verify flag is set (#9070) * fix: Fixed bug - The flags --cert-file/--key-file where ignored when --insecure-skip-tls-verify flag is set Signed-off-by: Dinu Mathai * fix: Added unit test Signed-off-by: Dinu Mathai --- pkg/getter/httpgetter.go | 9 ++++-- pkg/getter/httpgetter_test.go | 51 ++++++++++++++++++++++++++++++++++ pkg/getter/testdata/ca.crt | 25 +++++++++++++++++ pkg/getter/testdata/client.crt | 21 ++++++++++++++ pkg/getter/testdata/client.key | 27 ++++++++++++++++++ 5 files changed, 130 insertions(+), 3 deletions(-) create mode 100644 pkg/getter/testdata/ca.crt create mode 100644 pkg/getter/testdata/client.crt create mode 100644 pkg/getter/testdata/client.key diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index c100b2cc0..bd60629ae 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -111,10 +111,13 @@ func (g *HTTPGetter) httpClient() (*http.Client, error) { } if g.opts.insecureSkipVerifyTLS { - transport.TLSClientConfig = &tls.Config{ - InsecureSkipVerify: true, + if transport.TLSClientConfig == nil { + transport.TLSClientConfig = &tls.Config{ + InsecureSkipVerify: true, + } + } else { + transport.TLSClientConfig.InsecureSkipVerify = true } - } client := &http.Client{ diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index 90578f7b7..3aab22abe 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -294,3 +294,54 @@ func TestHTTPGetterTarDownload(t *testing.T) { t.Fatalf("Expected response with MIME type %s, but got %s", expectedMimeType, mimeType) } } + +func TestHttpClientInsecureSkipVerify(t *testing.T) { + g := HTTPGetter{} + g.opts.url = "https://localhost" + verifyInsecureSkipVerify(t, g, "Blank HTTPGetter", false) + + g = HTTPGetter{} + g.opts.url = "https://localhost" + g.opts.caFile = "testdata/ca.crt" + verifyInsecureSkipVerify(t, g, "HTTPGetter with ca file", false) + + g = HTTPGetter{} + g.opts.url = "https://localhost" + g.opts.insecureSkipVerifyTLS = true + verifyInsecureSkipVerify(t, g, "HTTPGetter with skip cert verification only", true) + + g = HTTPGetter{} + g.opts.url = "https://localhost" + g.opts.certFile = "testdata/client.crt" + g.opts.keyFile = "testdata/client.key" + g.opts.insecureSkipVerifyTLS = true + transport := verifyInsecureSkipVerify(t, g, "HTTPGetter with 2 way ssl", true) + if len(transport.TLSClientConfig.Certificates) <= 0 { + t.Fatal("transport.TLSClientConfig.Certificates is not present") + } + if transport.TLSClientConfig.ServerName == "" { + t.Fatal("TLSClientConfig.ServerName is blank") + } +} + +func verifyInsecureSkipVerify(t *testing.T, g HTTPGetter, caseName string, expectedValue bool) *http.Transport { + returnVal, err := g.httpClient() + + if err != nil { + t.Fatal(err) + } + + if returnVal == nil { + t.Fatalf("Expected non nil value for http client") + } + transport := (returnVal.Transport).(*http.Transport) + gotValue := false + if transport.TLSClientConfig != nil { + gotValue = transport.TLSClientConfig.InsecureSkipVerify + } + if gotValue != expectedValue { + t.Fatalf("Case Name = %s\nInsecureSkipVerify did not come as expected. Expected = %t; Got = %v", + caseName, expectedValue, gotValue) + } + return transport +} diff --git a/pkg/getter/testdata/ca.crt b/pkg/getter/testdata/ca.crt new file mode 100644 index 000000000..c17820085 --- /dev/null +++ b/pkg/getter/testdata/ca.crt @@ -0,0 +1,25 @@ +-----BEGIN CERTIFICATE----- +MIIEJDCCAwygAwIBAgIUcGE5xyj7IH7sZLntsHKxZHCd3awwDQYJKoZIhvcNAQEL +BQAwYTELMAkGA1UEBhMCSU4xDzANBgNVBAgMBktlcmFsYTEOMAwGA1UEBwwFS29j +aGkxGDAWBgNVBAoMD2NoYXJ0bXVzZXVtLmNvbTEXMBUGA1UEAwwOY2hhcnRtdXNl +dW1fY2EwIBcNMjAxMjA0MDkxMjU4WhgPMjI5NDA5MTkwOTEyNThaMGExCzAJBgNV +BAYTAklOMQ8wDQYDVQQIDAZLZXJhbGExDjAMBgNVBAcMBUtvY2hpMRgwFgYDVQQK +DA9jaGFydG11c2V1bS5jb20xFzAVBgNVBAMMDmNoYXJ0bXVzZXVtX2NhMIIBIjAN +BgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAqQJi/BRWzaXlkDP48kUAWgaLtD0Y +72E30WBZDAw3S+BaYulRk1LWK1QM+ALiZQb1a6YgNvuERyywOv45pZaC2xtP6Bju ++59kwBrEtNCTNa2cSqs0hSw6NCDe+K8lpFKlTdh4c5sAkiDkMBr1R6uu7o4HvfO0 +iGMZ9VUdrbf4psZIyPVRdt/sAkAKqbjQfxr6VUmMktrZNND+mwPgrhS2kPL4P+JS +zpxgpkuSUvg5DvJuypmCI0fDr6GwshqXM1ONHE0HT8MEVy1xZj9rVHt7sgQhjBX1 +PsFySZrq1lSz8R864c1l+tCGlk9+1ldQjc9tBzdvCjJB+nYfTTpBUk/VKwIDAQAB +o4HRMIHOMB0GA1UdDgQWBBSv1IMZGHWsZVqJkJoPDzVLMcUivjCBngYDVR0jBIGW +MIGTgBSv1IMZGHWsZVqJkJoPDzVLMcUivqFlpGMwYTELMAkGA1UEBhMCSU4xDzAN +BgNVBAgMBktlcmFsYTEOMAwGA1UEBwwFS29jaGkxGDAWBgNVBAoMD2NoYXJ0bXVz +ZXVtLmNvbTEXMBUGA1UEAwwOY2hhcnRtdXNldW1fY2GCFHBhOcco+yB+7GS57bBy +sWRwnd2sMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAI6Fg9F8cjB9 +2jJn1vZPpynSFs7XPlUBVh0YXBt+o6g7+nKInwFBPzPEQ7ZZotz3GIe4I7wYiQAn +c6TU2nnqK+9TLbJIyv6NOfikLgwrTy+dAW8wrOiu+IIzA8Gdy8z8m3B7v9RUYVhx +zoNoqCEvOIzCZKDH68PZDJrDVSuvPPK33Ywj3zxYeDNXU87BKGER0vjeVG4oTAcQ +hKJURh4IRy/eW9NWiFqvNgst7k5MldOgLIOUBh1faaxlWkjuGpfdr/EBAAr491S5 +IPFU7TopsrgANnxldSzVbcgfo2nt0A976T3xZQHy3xpk1rIt55xVzT0W55NRAc7v ++9NTUOB10so= +-----END CERTIFICATE----- diff --git a/pkg/getter/testdata/client.crt b/pkg/getter/testdata/client.crt new file mode 100644 index 000000000..f005f401d --- /dev/null +++ b/pkg/getter/testdata/client.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDejCCAmKgAwIBAgIUfSn63/ldeo1prOaxXV8I0Id6HTEwDQYJKoZIhvcNAQEL +BQAwYTELMAkGA1UEBhMCSU4xDzANBgNVBAgMBktlcmFsYTEOMAwGA1UEBwwFS29j +aGkxGDAWBgNVBAoMD2NoYXJ0bXVzZXVtLmNvbTEXMBUGA1UEAwwOY2hhcnRtdXNl +dW1fY2EwIBcNMjAxMjA0MDkxMzIwWhgPMjI5NDA5MTkwOTEzMjBaMFwxCzAJBgNV +BAYTAklOMQ8wDQYDVQQIDAZLZXJhbGExDjAMBgNVBAcMBUtvY2hpMRgwFgYDVQQK +DA9jaGFydG11c2V1bS5jb20xEjAQBgNVBAMMCTEyNy4wLjAuMTCCASIwDQYJKoZI +hvcNAQEBBQADggEPADCCAQoCggEBAKeCbADaK+7yrM9rQszF54334mGoSXbXY6Ca +7FKdkgmKCjeeqZ+lr+i+6WQ+O+Tn0dhlyHier42IqUw5Rzzegvl7QrhiChd8C6sW +pEqDK7Z1U+cv9gIabYd+qWDwFw67xiMNQfdZxwI/AgPzixlfsMw3ZNKM3Q0Vxtdz +EEYdEDDNgZ34Cj+KXCPpYDi2i5hZnha4wzIfbL3+z2o7sPBBLBrrsOtPdVVkxysN +HM4h7wp7w7QyOosndFvcTaX7yRA1ka0BoulCt2wdVc2ZBRPiPMySi893VCQ8zeHP +QMFDL3rGmKVLbP1to2dgf9ZgckMEwE8chm2D8Ls87F9tsK9fVlUCAwEAAaMtMCsw +EwYDVR0lBAwwCgYIKwYBBQUHAwIwFAYDVR0RBA0wC4IJMTI3LjAuMC4xMA0GCSqG +SIb3DQEBCwUAA4IBAQCi7z5U9J5DkM6eYzyyH/8p32Azrunw+ZpwtxbKq3xEkpcX +0XtbyTG2szegKF0eLr9NizgEN8M1nvaMO1zuxFMB6tCWO/MyNWH/0T4xvFnnVzJ4 +OKlGSvyIuMW3wofxCLRG4Cpw750iWpJ0GwjTOu2ep5tbnEMC5Ueg55WqCAE/yDrd +nL1wZSGXy1bj5H6q8EM/4/yrzK80QkfdpbDR0NGkDO2mmAKL8d57NuASWljieyV3 +Ty5C8xXw5jF2JIESvT74by8ufozUOPKmgRqySgEPgAkNm0s5a05KAi5Cpyxgdylm +CEvjni1LYGhJp9wXucF9ehKSdsw4qn9T5ire8YfI +-----END CERTIFICATE----- diff --git a/pkg/getter/testdata/client.key b/pkg/getter/testdata/client.key new file mode 100644 index 000000000..4f676ba42 --- /dev/null +++ b/pkg/getter/testdata/client.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAp4JsANor7vKsz2tCzMXnjffiYahJdtdjoJrsUp2SCYoKN56p +n6Wv6L7pZD475OfR2GXIeJ6vjYipTDlHPN6C+XtCuGIKF3wLqxakSoMrtnVT5y/2 +Ahpth36pYPAXDrvGIw1B91nHAj8CA/OLGV+wzDdk0ozdDRXG13MQRh0QMM2BnfgK +P4pcI+lgOLaLmFmeFrjDMh9svf7Pajuw8EEsGuuw6091VWTHKw0cziHvCnvDtDI6 +iyd0W9xNpfvJEDWRrQGi6UK3bB1VzZkFE+I8zJKLz3dUJDzN4c9AwUMvesaYpUts +/W2jZ2B/1mByQwTATxyGbYPwuzzsX22wr19WVQIDAQABAoIBABw7qUSDgUAm+uWC +6KFnAd4115wqJye2qf4Z3pcWI9UjxREW1vQnkvyhoOjabHHqeL4GecGKzYAHdrF4 +Pf+OaXjvQ5GcRKMsrzLJACvm6+k24UtoFAjKt4dM2/OQw/IhyAWEaIfuQ9KnGAne +dKV0MXJaK84pG+DmuLr7k9SddWskElEyxK2j0tvdyI5byRfjf5schac9M4i5ZAYV +pT+PuXZQh8L8GEY2koE+uEMpXGOstD7yUxyV8zHFyBC7FVDkqF4S8IWY+RXQtVd6 +l8B8dRLjKSLBKDB+neStepcwNUyCDYiqyqsKfN7eVHDd0arPm6LeTuAsHKBw2OoN +YdAmUUkCgYEA0vb9mxsMgr0ORTZ14vWghz9K12oKPk9ajYuLTQVn8GQazp0XTIi5 +Mil2I78Qj87ApzGqOyHbkEgpg0C9/mheYLOYNHC4of83kNF+pHbDi1TckwxIaIK0 +rZLb3Az3zZQ2rAWZ2IgSyoeVO9RxYK/RuvPFp+UBeucuXiYoI0YlEXcCgYEAy0Sk +LTiYuLsnk21RIFK01iq4Y+4112K1NGTKu8Wm6wPaPsnLznP6339cEkbiSgbRgERE +jgOfa/CiSw5CVT9dWZuQ3OoQ83pMRb7IB0TobPmhBS/HQZ8Ocbfb6PnxQ3o1Bx7I +QuIpZFxzuTP80p1p2DMDxEl+r/DCvT/wgBKX6ZMCgYAdw1bYMSK8tytyPFK5aGnz +asyGQ6GaVNuzqIJIpYCae6UEjUkiNQ/bsdnHBUey4jpv3CPmH8q4OlYQ/GtRnyvh +fLT2gQirYjRWrBev4EmKOLi9zjfQ9s/CxTtbekDjsgtcjZW85MWx6Rr2y+wK9gMi +2w2BuF9TFZaHFd8Hyvej1QKBgAoFbU6pbqYU3AOhrRE54p54ZrTOhqsCu8pEedY+ +DVeizfyweDLKdwDTx5dDFV7u7R80vmh99zscFvQ6VLzdLd4AFGk/xOwsCFyb5kKt +fAP7Xpvh2iH7FHw4w0e+Is3f1YNvWhIqEj5XbIEh9gHwLsqw4SupL+y+ousvnszB +nemvAoGBAJa7bYG8MMCFJ4OFAmkpgQzHSzq7dzOR6O4GKsQQhiZ/0nRK5l3sLcDO +9viuRfhRepJGbcQ/Hw0AVIRWU01y4mejbuxfUE/FgWBoBBvpbot2zfuJgeFAIvkY +iFsZwuxPQUFobTu2hj6gh0gOKj/LpNXHkZGbZ2zTXmK3GDYlf6bR +-----END RSA PRIVATE KEY----- From beda5e1e2be460543c32e2267982d6a7333be483 Mon Sep 17 00:00:00 2001 From: Peter Engelbert Date: Fri, 18 Dec 2020 16:29:15 -0600 Subject: [PATCH 17/25] Address error on deletion of old dependencies Signed-off-by: Peter Engelbert --- cmd/helm/dependency_update_test.go | 2 +- pkg/downloader/manager.go | 16 ++-------------- pkg/getter/getter_test.go | 2 +- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/cmd/helm/dependency_update_test.go b/cmd/helm/dependency_update_test.go index ce93e5c41..896018735 100644 --- a/cmd/helm/dependency_update_test.go +++ b/cmd/helm/dependency_update_test.go @@ -144,7 +144,7 @@ func TestDependencyUpdateCmd(t *testing.T) { t.Logf("Output: %s", out) t.Fatal(err) } - expect = dir(ociChartName, "charts/oci-dependent-chart") + expect = dir(ociChartName, "charts/oci-dependent-chart-0.1.0.tgz") if _, err := os.Stat(expect); err != nil { t.Fatal(err) } diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index f2945fdb6..38ed04279 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -335,7 +335,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { }, } - untar, version := false, "" + version := "" if strings.HasPrefix(churl, "oci://") { if !resolver.FeatureGateOCI.IsEnabled() { return errors.Wrapf(resolver.FeatureGateOCI.Error(), @@ -346,29 +346,17 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { if err != nil { return errors.Wrapf(err, "could not parse OCI reference") } - untar = true dl.Options = append(dl.Options, getter.WithRegistryClient(m.RegistryClient), getter.WithTagName(version)) } - destFile, _, err := dl.DownloadTo(churl, version, destPath) + _, _, err = dl.DownloadTo(churl, version, destPath) if err != nil { saveError = errors.Wrapf(err, "could not download %s", churl) break } - if untar { - err = chartutil.ExpandFile(destPath, destFile) - if err != nil { - return errors.Wrapf(err, "could not open %s to untar", destFile) - } - err = os.RemoveAll(destFile) - if err != nil { - return errors.Wrapf(err, "chart was downloaded and untarred, but was unable to remove the tarball: %s", destFile) - } - } - churls[churl] = struct{}{} } diff --git a/pkg/getter/getter_test.go b/pkg/getter/getter_test.go index 95d309c16..ab14784ab 100644 --- a/pkg/getter/getter_test.go +++ b/pkg/getter/getter_test.go @@ -58,7 +58,7 @@ func TestAll(t *testing.T) { all := All(env) if len(all) != 4 { - t.Errorf("expected 3 providers (default plus two plugins), got %d", len(all)) + t.Errorf("expected 4 providers (default plus three plugins), got %d", len(all)) } if _, err := all.ByScheme("test2"); err != nil { From 1da2212a9de64671edb17f727b937215cc252309 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Wed, 16 Dec 2020 21:44:43 -0800 Subject: [PATCH 18/25] Add explanatory comments to action.List and action.History While the comments may seem to state the obvious to someone with helm CLI experience, an SDK-first user may find these comments helpful. Signed-off-by: Daniel Lipovetsky --- pkg/action/history.go | 3 +++ pkg/action/list.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/pkg/action/history.go b/pkg/action/history.go index f4043609c..0430aaf7a 100644 --- a/pkg/action/history.go +++ b/pkg/action/history.go @@ -26,6 +26,9 @@ import ( // History is the action for checking the release's ledger. // // It provides the implementation of 'helm history'. +// It returns all the revisions for a specific release. +// To list up to one revision of every release in one specific, or in all, +// namespaces, see the List action. type History struct { cfg *Configuration diff --git a/pkg/action/list.go b/pkg/action/list.go index ebbc56b01..c9e6e364a 100644 --- a/pkg/action/list.go +++ b/pkg/action/list.go @@ -98,6 +98,9 @@ const ( // List is the action for listing releases. // // It provides, for example, the implementation of 'helm list'. +// It returns no more than one revision of every release in one specific, or in +// all, namespaces. +// To list all the revisions of a specific release, see the History action. type List struct { cfg *Configuration From b880fc5c0fa9dcdfdd8b3ca43be6b463937ec280 Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Tue, 5 Jan 2021 15:46:24 -0500 Subject: [PATCH 19/25] Bumping kubernetes to 1.20.1 Signed-off-by: Matt Farina --- go.mod | 12 ++++++------ go.sum | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index b636d700d..074d9dc0b 100644 --- a/go.mod +++ b/go.mod @@ -35,13 +35,13 @@ require ( github.com/stretchr/testify v1.6.1 github.com/xeipuuv/gojsonschema v1.2.0 golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0 - k8s.io/api v0.20.0 - k8s.io/apiextensions-apiserver v0.20.0 - k8s.io/apimachinery v0.20.0 - k8s.io/cli-runtime v0.20.0 - k8s.io/client-go v0.20.0 + k8s.io/api v0.20.1 + k8s.io/apiextensions-apiserver v0.20.1 + k8s.io/apimachinery v0.20.1 + k8s.io/cli-runtime v0.20.1 + k8s.io/client-go v0.20.1 k8s.io/klog/v2 v2.4.0 - k8s.io/kubectl v0.20.0 + k8s.io/kubectl v0.20.1 sigs.k8s.io/yaml v1.2.0 ) diff --git a/go.sum b/go.sum index 4186e039c..2a8668d08 100644 --- a/go.sum +++ b/go.sum @@ -1177,19 +1177,34 @@ honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= k8s.io/api v0.20.0 h1:WwrYoZNM1W1aQEbyl8HNG+oWGzLpZQBlcerS9BQw9yI= k8s.io/api v0.20.0/go.mod h1:HyLC5l5eoS/ygQYl1BXBgFzWNlkHiAuyNAbevIn+FKg= +k8s.io/api v0.20.1 h1:ud1c3W3YNzGd6ABJlbFfKXBKXO+1KdGfcgGGNgFR03E= +k8s.io/api v0.20.1/go.mod h1:KqwcCVogGxQY3nBlRpwt+wpAMF/KjaCc7RpywacvqUo= k8s.io/apiextensions-apiserver v0.20.0 h1:HmeP9mLET/HlIQ5gjP+1c20tgJrlshY5nUyIand3AVg= k8s.io/apiextensions-apiserver v0.20.0/go.mod h1:ZH+C33L2Bh1LY1+HphoRmN1IQVLTShVcTojivK3N9xg= +k8s.io/apiextensions-apiserver v0.20.1 h1:ZrXQeslal+6zKM/HjDXLzThlz/vPSxrfK3OqL8txgVQ= +k8s.io/apiextensions-apiserver v0.20.1/go.mod h1:ntnrZV+6a3dB504qwC5PN/Yg9PBiDNt1EVqbW2kORVk= k8s.io/apimachinery v0.20.0 h1:jjzbTJRXk0unNS71L7h3lxGDH/2HPxMPaQY+MjECKL8= k8s.io/apimachinery v0.20.0/go.mod h1:WlLqWAHZGg07AeltaI0MV5uk1Omp8xaN0JGLY6gkRpU= +k8s.io/apimachinery v0.20.1 h1:LAhz8pKbgR8tUwn7boK+b2HZdt7MiTu2mkYtFMUjTRQ= +k8s.io/apimachinery v0.20.1/go.mod h1:WlLqWAHZGg07AeltaI0MV5uk1Omp8xaN0JGLY6gkRpU= k8s.io/apiserver v0.20.0/go.mod h1:6gRIWiOkvGvQt12WTYmsiYoUyYW0FXSiMdNl4m+sxY8= +k8s.io/apiserver v0.20.1/go.mod h1:ro5QHeQkgMS7ZGpvf4tSMx6bBOgPfE+f52KwvXfScaU= k8s.io/cli-runtime v0.20.0 h1:UfTR9vGUWshJpwuekl7MqRmWumNs5tvqPj20qnmOns8= k8s.io/cli-runtime v0.20.0/go.mod h1:C5tewU1SC1t09D7pmkk83FT4lMAw+bvMDuRxA7f0t2s= +k8s.io/cli-runtime v0.20.1 h1:fJhRQ9EfTpJpCqSFOAqnYLuu5aAM7yyORWZ26qW1jJc= +k8s.io/cli-runtime v0.20.1/go.mod h1:6wkMM16ZXTi7Ow3JLYPe10bS+XBnIkL6V9dmEz0mbuY= k8s.io/client-go v0.20.0 h1:Xlax8PKbZsjX4gFvNtt4F5MoJ1V5prDvCuoq9B7iax0= k8s.io/client-go v0.20.0/go.mod h1:4KWh/g+Ocd8KkCwKF8vUNnmqgv+EVnQDK4MBF4oB5tY= +k8s.io/client-go v0.20.1 h1:Qquik0xNFbK9aUG92pxHYsyfea5/RPO9o9bSywNor+M= +k8s.io/client-go v0.20.1/go.mod h1:/zcHdt1TeWSd5HoUe6elJmHSQ6uLLgp4bIJHVEuy+/Y= k8s.io/code-generator v0.20.0/go.mod h1:UsqdF+VX4PU2g46NC2JRs4gc+IfrctnwHb76RNbWHJg= +k8s.io/code-generator v0.20.1/go.mod h1:UsqdF+VX4PU2g46NC2JRs4gc+IfrctnwHb76RNbWHJg= k8s.io/component-base v0.20.0 h1:BXGL8iitIQD+0NgW49UsM7MraNUUGDU3FBmrfUAtmVQ= k8s.io/component-base v0.20.0/go.mod h1:wKPj+RHnAr8LW2EIBIK7AxOHPde4gme2lzXwVSoRXeA= +k8s.io/component-base v0.20.1 h1:6OQaHr205NSl24t5wOF2IhdrlxZTWEZwuGlLvBgaeIg= +k8s.io/component-base v0.20.1/go.mod h1:guxkoJnNoh8LNrbtiQOlyp2Y2XFCZQmrcg2n/DeYNLk= k8s.io/component-helpers v0.20.0/go.mod h1:nx6NOtfSfGOxnSZsDJxpGbnsVuUA1UXpwDvZIrtigNk= +k8s.io/component-helpers v0.20.1/go.mod h1:Q8trCj1zyLNdeur6pD2QvsF8d/nWVfK71YjN5+qVXy4= k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/gengo v0.0.0-20201113003025-83324d819ded/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= @@ -1200,8 +1215,11 @@ k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd h1:sOHNzJIkytDF6qadMNKhhD k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd/go.mod h1:WOJ3KddDSol4tAGcJo0Tvi+dK12EcqSLqcWsryKMpfM= k8s.io/kubectl v0.20.0 h1:q6HH6jILYi2lkzFqBhs63M4bKLxYlM0HpFJ///MgARA= k8s.io/kubectl v0.20.0/go.mod h1:8x5GzQkgikz7M2eFGGuu6yOfrenwnw5g4RXOUgbjR1M= +k8s.io/kubectl v0.20.1 h1:7h1vSrL/B3hLrhlCJhbTADElPKDbx+oVUt3+QDSXxBo= +k8s.io/kubectl v0.20.1/go.mod h1:2bE0JLYTRDVKDiTREFsjLAx4R2GvUtL/mGYFXfFFMzY= k8s.io/kubernetes v1.13.0/go.mod h1:ocZa8+6APFNC2tX1DZASIbocyYT5jHzqFVsY5aoB7Jk= k8s.io/metrics v0.20.0/go.mod h1:9yiRhfr8K8sjdj2EthQQE9WvpYDvsXIV3CjN4Ruq4Jw= +k8s.io/metrics v0.20.1/go.mod h1:JhpBE/fad3yRGsgEpiZz5FQQM5wJ18OTLkD7Tv40c0s= k8s.io/utils v0.0.0-20201110183641-67b214c5f920 h1:CbnUZsM497iRC5QMVkHwyl8s2tB3g7yaSHkYPkpgelw= k8s.io/utils v0.0.0-20201110183641-67b214c5f920/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= From a58209dfa41d291c49dcb42b123b336c782356f3 Mon Sep 17 00:00:00 2001 From: Adam Reese Date: Tue, 5 Jan 2021 13:40:06 -0800 Subject: [PATCH 20/25] fix(Makefile): rebuild the binary if go.mod has changed Rebuild the binary when go.mod or go.sum has changed Signed-off-by: Adam Reese --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 931fe973d..571241cbe 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,9 @@ TESTS := . TESTFLAGS := LDFLAGS := -w -s GOFLAGS := -SRC := $(shell find . -type f -name '*.go' -print) + +# Rebuild the buinary if any of these files change +SRC := $(shell find . -type f -name '*.go' -print) go.mod go.sum # Required for globs to work correctly SHELL = /usr/bin/env bash From fdcd22ef589d781090e704a8c366e89be4dbc1e4 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Tue, 5 Jan 2021 15:05:33 -0800 Subject: [PATCH 21/25] Reduce linting severity for users of out-of-date kubernetes (#8608) * Reduce linting severity for users of out-of-date kubernetes Fixes #8596 Signed-off-by: Joe Julian * add more verbose deprecation info Signed-off-by: Joe Julian * use new upstream deprecations Signed-off-by: Joe Julian * do not error for custom resources Signed-off-by: Joe Julian * Define deprecation version in lint rules by LDFLAG Signed-off-by: Joe Julian * make comment clearer Signed-off-by: Joe Julian * Extend the k8s version discovery and constants to chartutil Signed-off-by: Joe Julian * remove awk dependency Signed-off-by: Joe Julian * align k8s version constant names between capabilities.go and deprecations.go Signed-off-by: Joe Julian * show the error if the unexpected happens Signed-off-by: Joe Julian * bump k8sVersionMinor and golden chart templates for k8s 1.20 Signed-off-by: Joe Julian * bump for tests to match 1.20.1 Signed-off-by: Joe Julian --- Makefile | 10 ++ .../output/template-name-template.txt | 4 +- cmd/helm/testdata/output/template-set.txt | 4 +- .../output/template-show-only-multiple.txt | 4 +- .../output/template-show-only-one.txt | 4 +- .../testdata/output/template-skip-tests.txt | 4 +- .../testdata/output/template-values-files.txt | 4 +- .../output/template-with-api-version.txt | 4 +- .../testdata/output/template-with-crds.txt | 4 +- cmd/helm/testdata/output/template.txt | 4 +- pkg/chartutil/capabilities.go | 14 ++- pkg/chartutil/capabilities_test.go | 16 +-- pkg/lint/rules/deprecations.go | 100 +++++++++--------- pkg/lint/rules/deprecations_test.go | 5 +- pkg/lint/rules/template.go | 7 +- 15 files changed, 106 insertions(+), 82 deletions(-) diff --git a/Makefile b/Makefile index 931fe973d..91fb8914d 100644 --- a/Makefile +++ b/Makefile @@ -55,6 +55,16 @@ LDFLAGS += -X helm.sh/helm/v3/internal/version.gitCommit=${GIT_COMMIT} LDFLAGS += -X helm.sh/helm/v3/internal/version.gitTreeState=${GIT_DIRTY} LDFLAGS += $(EXT_LDFLAGS) +# Define constants based on the client-go version +K8S_MODULES_VER=$(subst ., ,$(subst v,,$(shell go list -f '{{.Version}}' -m k8s.io/client-go))) +K8S_MODULES_MAJOR_VER=$(shell echo $$(($(firstword $(K8S_MODULES_VER)) + 1))) +K8S_MODULES_MINOR_VER=$(word 2,$(K8S_MODULES_VER)) + +LDFLAGS += -X helm.sh/helm/v3/pkg/lint/rules.k8sVersionMajor=$(K8S_MODULES_MAJOR_VER) +LDFLAGS += -X helm.sh/helm/v3/pkg/lint/rules.k8sVersionMinor=$(K8S_MODULES_MINOR_VER) +LDFLAGS += -X helm.sh/helm/v3/pkg/chartutil.k8sVersionMajor=$(K8S_MODULES_MAJOR_VER) +LDFLAGS += -X helm.sh/helm/v3/pkg/chartutil.k8sVersionMinor=$(K8S_MODULES_MINOR_VER) + .PHONY: all all: build diff --git a/cmd/helm/testdata/output/template-name-template.txt b/cmd/helm/testdata/output/template-name-template.txt index 5e4478937..b9e7cbbe4 100644 --- a/cmd/helm/testdata/output/template-name-template.txt +++ b/cmd/helm/testdata/output/template-name-template.txt @@ -72,8 +72,8 @@ metadata: helm.sh/chart: "subchart-0.1.0" app.kubernetes.io/instance: "foobar-YWJj-baz" kube-version/major: "1" - kube-version/minor: "18" - kube-version/version: "v1.18.0" + kube-version/minor: "20" + kube-version/version: "v1.20.0" spec: type: ClusterIP ports: diff --git a/cmd/helm/testdata/output/template-set.txt b/cmd/helm/testdata/output/template-set.txt index 0db9a9b74..177d8e58c 100644 --- a/cmd/helm/testdata/output/template-set.txt +++ b/cmd/helm/testdata/output/template-set.txt @@ -72,8 +72,8 @@ metadata: helm.sh/chart: "subchart-0.1.0" app.kubernetes.io/instance: "RELEASE-NAME" kube-version/major: "1" - kube-version/minor: "18" - kube-version/version: "v1.18.0" + kube-version/minor: "20" + kube-version/version: "v1.20.0" spec: type: ClusterIP ports: diff --git a/cmd/helm/testdata/output/template-show-only-multiple.txt b/cmd/helm/testdata/output/template-show-only-multiple.txt index 1c4b1f29e..20b6bebed 100644 --- a/cmd/helm/testdata/output/template-show-only-multiple.txt +++ b/cmd/helm/testdata/output/template-show-only-multiple.txt @@ -8,8 +8,8 @@ metadata: helm.sh/chart: "subchart-0.1.0" app.kubernetes.io/instance: "RELEASE-NAME" kube-version/major: "1" - kube-version/minor: "18" - kube-version/version: "v1.18.0" + kube-version/minor: "20" + kube-version/version: "v1.20.0" kube-api-version/test: v1 spec: type: ClusterIP diff --git a/cmd/helm/testdata/output/template-show-only-one.txt b/cmd/helm/testdata/output/template-show-only-one.txt index 7b1443ea8..f3aedb55d 100644 --- a/cmd/helm/testdata/output/template-show-only-one.txt +++ b/cmd/helm/testdata/output/template-show-only-one.txt @@ -8,8 +8,8 @@ metadata: helm.sh/chart: "subchart-0.1.0" app.kubernetes.io/instance: "RELEASE-NAME" kube-version/major: "1" - kube-version/minor: "18" - kube-version/version: "v1.18.0" + kube-version/minor: "20" + kube-version/version: "v1.20.0" kube-api-version/test: v1 spec: type: ClusterIP diff --git a/cmd/helm/testdata/output/template-skip-tests.txt b/cmd/helm/testdata/output/template-skip-tests.txt index 16808ede3..6e657e50b 100644 --- a/cmd/helm/testdata/output/template-skip-tests.txt +++ b/cmd/helm/testdata/output/template-skip-tests.txt @@ -72,8 +72,8 @@ metadata: helm.sh/chart: "subchart-0.1.0" app.kubernetes.io/instance: "RELEASE-NAME" kube-version/major: "1" - kube-version/minor: "18" - kube-version/version: "v1.18.0" + kube-version/minor: "20" + kube-version/version: "v1.20.0" kube-api-version/test: v1 spec: type: ClusterIP diff --git a/cmd/helm/testdata/output/template-values-files.txt b/cmd/helm/testdata/output/template-values-files.txt index 0db9a9b74..177d8e58c 100644 --- a/cmd/helm/testdata/output/template-values-files.txt +++ b/cmd/helm/testdata/output/template-values-files.txt @@ -72,8 +72,8 @@ metadata: helm.sh/chart: "subchart-0.1.0" app.kubernetes.io/instance: "RELEASE-NAME" kube-version/major: "1" - kube-version/minor: "18" - kube-version/version: "v1.18.0" + kube-version/minor: "20" + kube-version/version: "v1.20.0" spec: type: ClusterIP ports: diff --git a/cmd/helm/testdata/output/template-with-api-version.txt b/cmd/helm/testdata/output/template-with-api-version.txt index 3e488f0d2..4b2d4ee84 100644 --- a/cmd/helm/testdata/output/template-with-api-version.txt +++ b/cmd/helm/testdata/output/template-with-api-version.txt @@ -72,8 +72,8 @@ metadata: helm.sh/chart: "subchart-0.1.0" app.kubernetes.io/instance: "RELEASE-NAME" kube-version/major: "1" - kube-version/minor: "18" - kube-version/version: "v1.18.0" + kube-version/minor: "20" + kube-version/version: "v1.20.0" kube-api-version/test: v1 spec: type: ClusterIP diff --git a/cmd/helm/testdata/output/template-with-crds.txt b/cmd/helm/testdata/output/template-with-crds.txt index 4bd5d2e29..fe8e24520 100644 --- a/cmd/helm/testdata/output/template-with-crds.txt +++ b/cmd/helm/testdata/output/template-with-crds.txt @@ -89,8 +89,8 @@ metadata: helm.sh/chart: "subchart-0.1.0" app.kubernetes.io/instance: "RELEASE-NAME" kube-version/major: "1" - kube-version/minor: "18" - kube-version/version: "v1.18.0" + kube-version/minor: "20" + kube-version/version: "v1.20.0" kube-api-version/test: v1 spec: type: ClusterIP diff --git a/cmd/helm/testdata/output/template.txt b/cmd/helm/testdata/output/template.txt index a81934b20..4146a0749 100644 --- a/cmd/helm/testdata/output/template.txt +++ b/cmd/helm/testdata/output/template.txt @@ -72,8 +72,8 @@ metadata: helm.sh/chart: "subchart-0.1.0" app.kubernetes.io/instance: "RELEASE-NAME" kube-version/major: "1" - kube-version/minor: "18" - kube-version/version: "v1.18.0" + kube-version/minor: "20" + kube-version/version: "v1.20.0" spec: type: ClusterIP ports: diff --git a/pkg/chartutil/capabilities.go b/pkg/chartutil/capabilities.go index adfe2363d..c002e33f2 100644 --- a/pkg/chartutil/capabilities.go +++ b/pkg/chartutil/capabilities.go @@ -16,6 +16,9 @@ limitations under the License. package chartutil import ( + "fmt" + "strconv" + "k8s.io/client-go/kubernetes/scheme" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -24,6 +27,11 @@ import ( helmversion "helm.sh/helm/v3/internal/version" ) +const ( + k8sVersionMajor = 1 + k8sVersionMinor = 20 +) + var ( // DefaultVersionSet is the default version set, which includes only Core V1 ("v1"). DefaultVersionSet = allKnownVersions() @@ -31,9 +39,9 @@ var ( // DefaultCapabilities is the default set of capabilities. DefaultCapabilities = &Capabilities{ KubeVersion: KubeVersion{ - Version: "v1.18.0", - Major: "1", - Minor: "18", + Version: fmt.Sprintf("v%d.%d.0", k8sVersionMajor, k8sVersionMinor), + Major: strconv.Itoa(k8sVersionMajor), + Minor: strconv.Itoa(k8sVersionMinor), }, APIVersions: DefaultVersionSet, HelmVersion: helmversion.Get(), diff --git a/pkg/chartutil/capabilities_test.go b/pkg/chartutil/capabilities_test.go index 4ba2f847f..04a6d36bb 100644 --- a/pkg/chartutil/capabilities_test.go +++ b/pkg/chartutil/capabilities_test.go @@ -42,20 +42,20 @@ func TestDefaultVersionSet(t *testing.T) { func TestDefaultCapabilities(t *testing.T) { kv := DefaultCapabilities.KubeVersion - if kv.String() != "v1.18.0" { - t.Errorf("Expected default KubeVersion.String() to be v1.18.0, got %q", kv.String()) + if kv.String() != "v1.20.0" { + t.Errorf("Expected default KubeVersion.String() to be v1.20.0, got %q", kv.String()) } - if kv.Version != "v1.18.0" { - t.Errorf("Expected default KubeVersion.Version to be v1.18.0, got %q", kv.Version) + if kv.Version != "v1.20.0" { + t.Errorf("Expected default KubeVersion.Version to be v1.20.0, got %q", kv.Version) } - if kv.GitVersion() != "v1.18.0" { - t.Errorf("Expected default KubeVersion.GitVersion() to be v1.18.0, got %q", kv.Version) + if kv.GitVersion() != "v1.20.0" { + t.Errorf("Expected default KubeVersion.GitVersion() to be v1.20.0, got %q", kv.Version) } if kv.Major != "1" { t.Errorf("Expected default KubeVersion.Major to be 1, got %q", kv.Major) } - if kv.Minor != "18" { - t.Errorf("Expected default KubeVersion.Minor to be 18, got %q", kv.Minor) + if kv.Minor != "20" { + t.Errorf("Expected default KubeVersion.Minor to be 20, got %q", kv.Minor) } } diff --git a/pkg/lint/rules/deprecations.go b/pkg/lint/rules/deprecations.go index 88921408d..384c17973 100644 --- a/pkg/lint/rules/deprecations.go +++ b/pkg/lint/rules/deprecations.go @@ -16,65 +16,69 @@ limitations under the License. package rules // import "helm.sh/helm/v3/pkg/lint/rules" -import "fmt" +import ( + "fmt" -// deprecatedAPIs lists APIs that are deprecated (left) with suggested alternatives (right). -// -// An empty rvalue indicates that the API is completely deprecated. -var deprecatedAPIs = map[string]string{ - "extensions/v1beta1 Deployment": "apps/v1 Deployment", - "extensions/v1beta1 DaemonSet": "apps/v1 DaemonSet", - "extensions/v1beta1 ReplicaSet": "apps/v1 ReplicaSet", - "extensions/v1beta1 PodSecurityPolicy": "policy/v1beta1 PodSecurityPolicy", - "extensions/v1beta1 NetworkPolicy": "networking.k8s.io/v1beta1 NetworkPolicy", - "extensions/v1beta1 Ingress": "networking.k8s.io/v1beta1 Ingress", - "apps/v1beta1 Deployment": "apps/v1 Deployment", - "apps/v1beta1 StatefulSet": "apps/v1 StatefulSet", - "apps/v1beta1 ReplicaSet": "apps/v1 ReplicaSet", - "apps/v1beta2 Deployment": "apps/v1 Deployment", - "apps/v1beta2 StatefulSet": "apps/v1 StatefulSet", - "apps/v1beta2 DaemonSet": "apps/v1 DaemonSet", - "apps/v1beta2 ReplicaSet": "apps/v1 ReplicaSet", - "apiextensions.k8s.io/v1beta1 CustomResourceDefinition": "apiextensions.k8s.io/v1 CustomResourceDefinition", - "rbac.authorization.k8s.io/v1alpha1 ClusterRole": "rbac.authorization.k8s.io/v1 ClusterRole", - "rbac.authorization.k8s.io/v1alpha1 ClusterRoleList": "rbac.authorization.k8s.io/v1 ClusterRoleList", - "rbac.authorization.k8s.io/v1alpha1 ClusterRoleBinding": "rbac.authorization.k8s.io/v1 ClusterRoleBinding", - "rbac.authorization.k8s.io/v1alpha1 ClusterRoleBindingList": "rbac.authorization.k8s.io/v1 ClusterRoleBindingList", - "rbac.authorization.k8s.io/v1alpha1 Role": "rbac.authorization.k8s.io/v1 Role", - "rbac.authorization.k8s.io/v1alpha1 RoleList": "rbac.authorization.k8s.io/v1 RoleList", - "rbac.authorization.k8s.io/v1alpha1 RoleBinding": "rbac.authorization.k8s.io/v1 RoleBinding", - "rbac.authorization.k8s.io/v1alpha1 RoleBindingList": "rbac.authorization.k8s.io/v1 RoleBindingList", - "rbac.authorization.k8s.io/v1beta1 ClusterRole": "rbac.authorization.k8s.io/v1 ClusterRole", - "rbac.authorization.k8s.io/v1beta1 ClusterRoleList": "rbac.authorization.k8s.io/v1 ClusterRoleList", - "rbac.authorization.k8s.io/v1beta1 ClusterRoleBinding": "rbac.authorization.k8s.io/v1 ClusterRoleBinding", - "rbac.authorization.k8s.io/v1beta1 ClusterRoleBindingList": "rbac.authorization.k8s.io/v1 ClusterRoleBindingList", - "rbac.authorization.k8s.io/v1beta1 Role": "rbac.authorization.k8s.io/v1 Role", - "rbac.authorization.k8s.io/v1beta1 RoleList": "rbac.authorization.k8s.io/v1 RoleList", - "rbac.authorization.k8s.io/v1beta1 RoleBinding": "rbac.authorization.k8s.io/v1 RoleBinding", - "rbac.authorization.k8s.io/v1beta1 RoleBindingList": "rbac.authorization.k8s.io/v1 RoleBindingList", -} + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/endpoints/deprecation" + kscheme "k8s.io/client-go/kubernetes/scheme" +) + +const ( + // This should be set in the Makefile based on the version of client-go being imported. + // These constants will be overwritten with LDFLAGS + k8sVersionMajor = 1 + k8sVersionMinor = 20 +) // deprecatedAPIError indicates than an API is deprecated in Kubernetes type deprecatedAPIError struct { - Deprecated string - Alternative string + Deprecated string + Message string } func (e deprecatedAPIError) Error() string { - msg := fmt.Sprintf("the kind %q is deprecated", e.Deprecated) - if e.Alternative != "" { - msg += fmt.Sprintf(" in favor of %q", e.Alternative) - } + msg := e.Message return msg } func validateNoDeprecations(resource *K8sYamlStruct) error { - gvk := fmt.Sprintf("%s %s", resource.APIVersion, resource.Kind) - if alt, ok := deprecatedAPIs[gvk]; ok { - return deprecatedAPIError{ - Deprecated: gvk, - Alternative: alt, + // if `resource` does not have an APIVersion or Kind, we cannot test it for deprecation + if resource.APIVersion == "" { + return nil + } + if resource.Kind == "" { + return nil + } + + runtimeObject, err := resourceToRuntimeObject(resource) + if err != nil { + // do not error for non-kubernetes resources + if runtime.IsNotRegisteredError(err) { + return nil } + return err + } + if !deprecation.IsDeprecated(runtimeObject, k8sVersionMajor, k8sVersionMinor) { + return nil + } + gvk := fmt.Sprintf("%s %s", resource.APIVersion, resource.Kind) + return deprecatedAPIError{ + Deprecated: gvk, + Message: deprecation.WarningMessage(runtimeObject), + } +} + +func resourceToRuntimeObject(resource *K8sYamlStruct) (runtime.Object, error) { + scheme := runtime.NewScheme() + kscheme.AddToScheme(scheme) + + gvk := schema.FromAPIVersionAndKind(resource.APIVersion, resource.Kind) + out, err := scheme.New(gvk) + if err != nil { + return nil, err } - return nil + out.GetObjectKind().SetGroupVersionKind(gvk) + return out, nil } diff --git a/pkg/lint/rules/deprecations_test.go b/pkg/lint/rules/deprecations_test.go index 1e8d34702..96e072d14 100644 --- a/pkg/lint/rules/deprecations_test.go +++ b/pkg/lint/rules/deprecations_test.go @@ -27,10 +27,9 @@ func TestValidateNoDeprecations(t *testing.T) { if err == nil { t.Fatal("Expected deprecated extension to be flagged") } - depErr := err.(deprecatedAPIError) - if depErr.Alternative != "apps/v1 Deployment" { - t.Errorf("Expected %q to be replaced by %q", depErr.Deprecated, depErr.Alternative) + if depErr.Message == "" { + t.Fatalf("Expected error message to be non-blank: %v", err) } if err := validateNoDeprecations(&K8sYamlStruct{ diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index 0bb9f8671..10121c108 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -137,8 +137,11 @@ func Templates(linter *support.Linter, values map[string]interface{}, namespace linter.RunLinterRule(support.ErrorSev, fpath, validateYamlContent(err)) if yamlStruct != nil { - linter.RunLinterRule(support.ErrorSev, fpath, validateMetadataName(yamlStruct)) - linter.RunLinterRule(support.ErrorSev, fpath, validateNoDeprecations(yamlStruct)) + // NOTE: set to warnings to allow users to support out-of-date kubernetes + // Refs https://github.com/helm/helm/issues/8596 + linter.RunLinterRule(support.WarningSev, fpath, validateMetadataName(yamlStruct)) + linter.RunLinterRule(support.WarningSev, fpath, validateNoDeprecations(yamlStruct)) + linter.RunLinterRule(support.ErrorSev, fpath, validateMatchSelector(yamlStruct, renderedContent)) } } From da4c40c5421773de1a28624c50e2cbebfb5fc11f Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Tue, 5 Jan 2021 21:15:41 -0500 Subject: [PATCH 22/25] Adding apiserver to mod/sum This is a follow-up to #8608. k8s.io/apiserver was added but not added to the go.mod file. Go handled the situation cleanly but left a dirty git tree. Signed-off-by: Matt Farina --- go.mod | 1 + go.sum | 1 + 2 files changed, 2 insertions(+) diff --git a/go.mod b/go.mod index 997713a67..94f836670 100644 --- a/go.mod +++ b/go.mod @@ -39,6 +39,7 @@ require ( k8s.io/api v0.20.1 k8s.io/apiextensions-apiserver v0.20.1 k8s.io/apimachinery v0.20.1 + k8s.io/apiserver v0.20.1 k8s.io/cli-runtime v0.20.1 k8s.io/client-go v0.20.1 k8s.io/klog/v2 v2.4.0 diff --git a/go.sum b/go.sum index 2a8668d08..ed04c2d2e 100644 --- a/go.sum +++ b/go.sum @@ -1188,6 +1188,7 @@ k8s.io/apimachinery v0.20.0/go.mod h1:WlLqWAHZGg07AeltaI0MV5uk1Omp8xaN0JGLY6gkRp k8s.io/apimachinery v0.20.1 h1:LAhz8pKbgR8tUwn7boK+b2HZdt7MiTu2mkYtFMUjTRQ= k8s.io/apimachinery v0.20.1/go.mod h1:WlLqWAHZGg07AeltaI0MV5uk1Omp8xaN0JGLY6gkRpU= k8s.io/apiserver v0.20.0/go.mod h1:6gRIWiOkvGvQt12WTYmsiYoUyYW0FXSiMdNl4m+sxY8= +k8s.io/apiserver v0.20.1 h1:yEqdkxlnQbxi/3e74cp0X16h140fpvPrNnNRAJBDuBk= k8s.io/apiserver v0.20.1/go.mod h1:ro5QHeQkgMS7ZGpvf4tSMx6bBOgPfE+f52KwvXfScaU= k8s.io/cli-runtime v0.20.0 h1:UfTR9vGUWshJpwuekl7MqRmWumNs5tvqPj20qnmOns8= k8s.io/cli-runtime v0.20.0/go.mod h1:C5tewU1SC1t09D7pmkk83FT4lMAw+bvMDuRxA7f0t2s= From 8082f6db45d60663ee1540e36b067ae2cc75459e Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Tue, 5 Jan 2021 21:35:54 -0500 Subject: [PATCH 23/25] bump version to Signed-off-by: Matt Farina (cherry picked from commit f546ebb1aca7c45a09a71886b720b6e11d45e9d8) --- cmd/helm/testdata/output/version-client-shorthand.txt | 2 +- cmd/helm/testdata/output/version-client.txt | 2 +- cmd/helm/testdata/output/version-short.txt | 2 +- cmd/helm/testdata/output/version-template.txt | 2 +- cmd/helm/testdata/output/version.txt | 2 +- internal/version/version.go | 2 +- pkg/chartutil/capabilities_test.go | 4 ++-- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/helm/testdata/output/version-client-shorthand.txt b/cmd/helm/testdata/output/version-client-shorthand.txt index e37819483..9dc0a8cfa 100644 --- a/cmd/helm/testdata/output/version-client-shorthand.txt +++ b/cmd/helm/testdata/output/version-client-shorthand.txt @@ -1 +1 @@ -version.BuildInfo{Version:"v3.4", GitCommit:"", GitTreeState:"", GoVersion:""} +version.BuildInfo{Version:"v3.5", GitCommit:"", GitTreeState:"", GoVersion:""} diff --git a/cmd/helm/testdata/output/version-client.txt b/cmd/helm/testdata/output/version-client.txt index e37819483..9dc0a8cfa 100644 --- a/cmd/helm/testdata/output/version-client.txt +++ b/cmd/helm/testdata/output/version-client.txt @@ -1 +1 @@ -version.BuildInfo{Version:"v3.4", GitCommit:"", GitTreeState:"", GoVersion:""} +version.BuildInfo{Version:"v3.5", GitCommit:"", GitTreeState:"", GoVersion:""} diff --git a/cmd/helm/testdata/output/version-short.txt b/cmd/helm/testdata/output/version-short.txt index 794508350..3c81e0c56 100644 --- a/cmd/helm/testdata/output/version-short.txt +++ b/cmd/helm/testdata/output/version-short.txt @@ -1 +1 @@ -v3.4 +v3.5 diff --git a/cmd/helm/testdata/output/version-template.txt b/cmd/helm/testdata/output/version-template.txt index eefb1dfcb..68945e7a4 100644 --- a/cmd/helm/testdata/output/version-template.txt +++ b/cmd/helm/testdata/output/version-template.txt @@ -1 +1 @@ -Version: v3.4 \ No newline at end of file +Version: v3.5 \ No newline at end of file diff --git a/cmd/helm/testdata/output/version.txt b/cmd/helm/testdata/output/version.txt index e37819483..9dc0a8cfa 100644 --- a/cmd/helm/testdata/output/version.txt +++ b/cmd/helm/testdata/output/version.txt @@ -1 +1 @@ -version.BuildInfo{Version:"v3.4", GitCommit:"", GitTreeState:"", GoVersion:""} +version.BuildInfo{Version:"v3.5", GitCommit:"", GitTreeState:"", GoVersion:""} diff --git a/internal/version/version.go b/internal/version/version.go index 73c433f57..15822e914 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -29,7 +29,7 @@ var ( // // Increment major number for new feature additions and behavioral changes. // Increment minor number for bug fixes and performance enhancements. - version = "v3.4" + version = "v3.5" // metadata is extra build time data metadata = "" diff --git a/pkg/chartutil/capabilities_test.go b/pkg/chartutil/capabilities_test.go index 04a6d36bb..7134abfc5 100644 --- a/pkg/chartutil/capabilities_test.go +++ b/pkg/chartutil/capabilities_test.go @@ -62,7 +62,7 @@ func TestDefaultCapabilities(t *testing.T) { func TestDefaultCapabilitiesHelmVersion(t *testing.T) { hv := DefaultCapabilities.HelmVersion - if hv.Version != "v3.4" { - t.Errorf("Expected default HelmVersion to be v3.4, got %q", hv.Version) + if hv.Version != "v3.5" { + t.Errorf("Expected default HelmVersion to be v3.5, got %q", hv.Version) } } From fee2257e3493e9d06ca6caa4be7ef7660842cbdb Mon Sep 17 00:00:00 2001 From: Guangwen Feng Date: Wed, 6 Jan 2021 14:19:37 +0800 Subject: [PATCH 24/25] Fix typo in comment Signed-off-by: Guangwen Feng --- pkg/action/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/action/pull.go b/pkg/action/pull.go index acd39bf05..04faa3b6b 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -61,7 +61,7 @@ func NewPull() *Pull { return NewPullWithOpts() } -// NewPull creates a new pull, with configuration options. +// NewPullWithOpts creates a new pull, with configuration options. func NewPullWithOpts(opts ...PullOpt) *Pull { p := &Pull{} for _, fn := range opts { From 1135392b482f26f244c3c69f51511a1d82590eb7 Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Wed, 6 Jan 2021 09:55:10 -0500 Subject: [PATCH 25/25] Fix dep build with OCI based charts The recent addition of oci:// to specify dependencies in the Chart.yaml dependencies and with helm pull missed handling for the dependency build command. This command was failing to handle OCI. This change adds support for the dep build command following the same pattern used to add oci:// functionality. Signed-off-by: Matt Farina --- cmd/helm/dependency.go | 2 +- cmd/helm/dependency_build.go | 3 ++- cmd/helm/dependency_build_test.go | 38 +++++++++++++++++++++++++++++++ pkg/downloader/manager.go | 7 +++--- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/cmd/helm/dependency.go b/cmd/helm/dependency.go index 3de3ef014..6bb82e217 100644 --- a/cmd/helm/dependency.go +++ b/cmd/helm/dependency.go @@ -93,7 +93,7 @@ func newDependencyCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { cmd.AddCommand(newDependencyListCmd(out)) cmd.AddCommand(newDependencyUpdateCmd(cfg, out)) - cmd.AddCommand(newDependencyBuildCmd(out)) + cmd.AddCommand(newDependencyBuildCmd(cfg, out)) return cmd } diff --git a/cmd/helm/dependency_build.go b/cmd/helm/dependency_build.go index a0b63f038..1ee46d3d2 100644 --- a/cmd/helm/dependency_build.go +++ b/cmd/helm/dependency_build.go @@ -41,7 +41,7 @@ If no lock file is found, 'helm dependency build' will mirror the behavior of 'helm dependency update'. ` -func newDependencyBuildCmd(out io.Writer) *cobra.Command { +func newDependencyBuildCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client := action.NewDependency() cmd := &cobra.Command{ @@ -60,6 +60,7 @@ func newDependencyBuildCmd(out io.Writer) *cobra.Command { Keyring: client.Keyring, SkipUpdate: client.SkipRefresh, Getters: getter.All(settings), + RegistryClient: cfg.RegistryClient, RepositoryConfig: settings.RepositoryConfig, RepositoryCache: settings.RepositoryCache, Debug: settings.Debug, diff --git a/cmd/helm/dependency_build_test.go b/cmd/helm/dependency_build_test.go index 8e5f24af7..33198a9dd 100644 --- a/cmd/helm/dependency_build_test.go +++ b/cmd/helm/dependency_build_test.go @@ -22,6 +22,7 @@ import ( "strings" "testing" + "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/provenance" "helm.sh/helm/v3/pkg/repo" "helm.sh/helm/v3/pkg/repo/repotest" @@ -37,6 +38,27 @@ func TestDependencyBuildCmd(t *testing.T) { rootDir := srv.Root() srv.LinkIndices() + ociSrv, err := repotest.NewOCIServer(t, srv.Root()) + if err != nil { + t.Fatal(err) + } + + ociChartName := "oci-depending-chart" + c := createTestingMetadataForOCI(ociChartName, ociSrv.RegistryURL) + if err := chartutil.SaveDir(c, ociSrv.Dir); err != nil { + t.Fatal(err) + } + ociSrv.Run(t, repotest.WithDependingChart(c)) + + err = os.Setenv("HELM_EXPERIMENTAL_OCI", "1") + if err != nil { + t.Fatal("failed to set environment variable enabling OCI support") + } + + dir := func(p ...string) string { + return filepath.Join(append([]string{srv.Root()}, p...)...) + } + chartname := "depbuild" createTestingChart(t, rootDir, chartname, srv.URL()) repoFile := filepath.Join(rootDir, "repositories.yaml") @@ -112,6 +134,22 @@ func TestDependencyBuildCmd(t *testing.T) { if strings.Contains(out, `update from the "test" chart repository`) { t.Errorf("Repo did get updated\n%s", out) } + + // OCI dependencies + cmd = fmt.Sprintf("dependency build '%s' --repository-config %s --repository-cache %s --registry-config %s/config.json", + dir(ociChartName), + dir("repositories.yaml"), + dir(), + dir()) + _, out, err = executeActionCommand(cmd) + if err != nil { + t.Logf("Output: %s", out) + t.Fatal(err) + } + expect = dir(ociChartName, "charts/oci-dependent-chart-0.1.0.tgz") + if _, err := os.Stat(expect); err != nil { + t.Fatal(err) + } } func TestDependencyBuildCmdWithHelmV2Hash(t *testing.T) { diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 38ed04279..d2d3e9f31 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -453,8 +453,8 @@ func (m *Manager) hasAllRepos(deps []*chart.Dependency) error { missing := []string{} Loop: for _, dd := range deps { - // If repo is from local path, continue - if strings.HasPrefix(dd.Repository, "file://") { + // If repo is from local path or OCI, continue + if strings.HasPrefix(dd.Repository, "file://") || strings.HasPrefix(dd.Repository, "oci://") { continue } @@ -555,7 +555,8 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, missing := []string{} for _, dd := range deps { // Don't map the repository, we don't need to download chart from charts directory - if dd.Repository == "" { + // When OCI is used there is no Helm repository + if dd.Repository == "" || strings.HasPrefix(dd.Repository, "oci://") { continue } // if dep chart is from local path, verify the path is valid