From fdcd22ef589d781090e704a8c366e89be4dbc1e4 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Tue, 5 Jan 2021 15:05:33 -0800 Subject: [PATCH] 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)) } }