feat(helm): remove need for Helm deployer's artifactOverrides (#6949)

* refactor: remove vestiges of parsing release info text from helm deployment (#6913) * refactor(testutil): allow tests to check for envvars that should not be set * feat(helm): remove need for Helm deployer's artifactOverrides * Back out changes to v1 schema * pacify golangci-lint * fix mac test and attempt to fix windows * fix lint Co-authored-by: tejal29 <[email protected]>
GoogleContainerTools · Jan 20, 2022 · 75beccc · 75beccc
1 parent 7c2aae6
commit 75beccc
Show file tree

Hide file tree

Showing 41 changed files with 377 additions and 815 deletions.
diff --git a/cmd/skaffold/app/cmd/filter.go b/cmd/skaffold/app/cmd/filter.go
@@ -32,6 +32,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
 	latestV2 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v2"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
+	pkgutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 
 // for tests
@@ -64,6 +65,16 @@ func runFilter(ctx context.Context, out io.Writer, debuggingFilters bool, buildA
 		if err != nil {
 			return fmt.Errorf("loading manifests: %w", err)
 		}
+
+		manifestList, err = manifestList.SetLabels(pkgutil.EnvSliceToMap(opts.CustomLabels, "="))
+		if err != nil {
+			return err
+		}
+		manifestList, err = manifestList.ReplaceImages(ctx, buildArtifacts)
+		if err != nil {
+			return err
+		}
+
 		if debuggingFilters {
 			// TODO(bdealwis): refactor this code
 			debugHelpersRegistry, err := config.GetDebugHelpersRegistry(opts.GlobalConfig)

diff --git a/cmd/skaffold/app/cmd/flags.go b/cmd/skaffold/app/cmd/flags.go
@@ -230,7 +230,7 @@ var flagRegistry = []Flag{
 		Value:         &opts.CustomLabels,
 		DefValue:      []string{},
 		FlagAddMethod: "StringSliceVar",
-		DefinedOn:     []string{"dev", "run", "debug", "deploy", "render"},
+		DefinedOn:     []string{"dev", "run", "debug", "deploy", "render", "filter"},
 	},
 	{
 		Name:          "toot",

diff --git a/docs/content/en/api/skaffold.swagger.json b/docs/content/en/api/skaffold.swagger.json
diff --git a/docs/content/en/docs/design/config.md b/docs/content/en/docs/design/config.md
@@ -63,8 +63,6 @@ deploy:
       chartPath: helm/project
       valuesFiles:
       - "helm/project/dev-values.yaml"
-      artifactOverrides:
-        image: app
 ```
 
 In this example, the `Dockerfile` for building `app`

diff --git a/docs/content/en/docs/pipeline-stages/deployers/helm.md b/docs/content/en/docs/pipeline-stages/deployers/helm.md
@@ -18,179 +18,9 @@ To use `helm` with Skaffold, the `helm` binary must be installed on your machine
 
 Skaffold supports projects set up to deploy with Helm, but certain aspects of the project need to be configured correctly in order for Skaffold to work properly. This guide should demystify some of the nuance around using Skaffold with Helm to help you get started quickly.
 
-## Image Configuration
-The normal Helm convention for defining image references is through the `values.yaml` file. Often, image information is configured through an `image` stanza in the values file, which might look something like this:
-
-```project_root/values.yaml```
-```yaml
-image:
-  repository: gcr.io/my-project/my-image
-  tag: v1.2.0
-  pullPolicy: IfNotPresent
-```
-
-This image would then be referenced in a templated resource file, maybe like this:
-
-```project_root/templates/deployment.yaml:```
-```yaml
-spec:
-  template:
-    spec:
-      containers:
-        - name: {{ .Chart.Name }}
-          image: {{ .Values.image.repository }}:{{ .Values.image.tag}}
-          imagePullPolicy: {{ .Values.image.pullPolicy }}
-```
-
-**IMPORTANT: To get Skaffold to work with Helm, the `image` key must be configured in the skaffold.yaml.**
-
-Associating the Helm image key allows Skaffold to track the image being built, and then configure Helm to substitute it in the proper resource definitions to be deployed to your cluster. In practice, this looks something like this:
-
-```yaml
-build:
-  artifacts:
-    - image: gcr.io/my-project/my-image # must match in artifactOverrides
-deploy:
-  helm:
-    releases:
-    - name: my-release
-      artifactOverrides:
-        image: gcr.io/my-project/my-image # no tag present!
-      imageStrategy:
-        helm: {}
-```
-
-The `artifactOverrides` binds a Helm value key to a build artifact.  The `imageStrategy` configures the image reference strategy for informing Helm of the image reference to a newly built artifact.
-
-### Image reference strategies
-
-Skaffold supports three _image reference strategies_ for Helm:
-
-1. `fqn`: provides a fully-qualified image reference (default);
-2. `helm`: provides separate repository and tag portions (shown above);
-3. `helm+explicitRegistry`: provides separate registry, repository, and tag portions.
-
-#### `fqn` strategy: single fully-qualified name (default)
-
-With the fully-qualified name strategy, Skaffold configures Helm by setting a key to the fully-tagged image reference.
-
-The `skaffold.yaml` setup:
-```yaml
-build:
-  artifacts:
-    - image: gcr.io/my-project/my-image
-deploy:
-  helm:
-    releases:
-      - name: my-chart
-        chartPath: helm
-        artifactOverrides:
-          imageKey: gcr.io/my-project/my-image
-        imageStrategy:
-          fqn: {}
-```
-
-Note that the `fqn` strategy is the default and the `imageStrategy` can be omitted.
-
-The `values.yaml` (note that Skaffold overrides this value):
-```
-imageKey: gcr.io/other-project/other-image:latest
-```
-
-The chart template:
-```yaml
-spec:
-  containers:
-    - name: {{ .Chart.Name }}
-      image: "{{.Values.imageKey}}"
-```
-
-Skaffold will invoke
-```
-helm install <chart> <chart-path> --set-string imageKey=gcr.io/my-project/my-image:generatedTag@sha256:digest
-```
-
-#### `helm` strategy: split repository and tag
-
-Skaffold can be configured to provide Helm with a separate repository and tag.  The key used in the `artifactOverrides` is used as base portion producing two keys `{key}.repository` and `{key}.tag`.
-
-The `skaffold.yaml` setup:
-```yaml
-build:
-  artifacts:
-    - image: gcr.io/my-project/my-image
-deploy:
-  helm:
-    releases:
-      - name: my-chart
-        chartPath: helm
-        artifactOverrides:
-          imageKey: gcr.io/my-project/my-image
-        imageStrategy:
-          helm: {}
-```
-
-The `values.yaml` (note that Skaffold overrides these values):
-```
-imageKey:
-  repository: gcr.io/other-project/other-image
-  tag: latest
-```
-
-The chart template:
-```yaml
-spec:
-  containers:
-    - name: {{ .Chart.Name }}
-      image: "{{.Values.imageKey.repository}}:{{.Values.imageKey.tag}}"
-```
-
-Skaffold will invoke
-```
-helm install <chart> <chart-path> --set-string imageKey.repository=gcr.io/my-project/my-image,imageKey.tag=generatedTag@sha256:digest
-```
-
-#### `helm`+`explicitRegistry` strategy: split registry, repository, and tag
-
-Skaffold can also be configured to provide Helm with a separate repository and tag.  The key used in the `artifactOverrides` is used as base portion producing three keys: `{key}.registry`, `{key}.repository`, and `{key}.tag`.
-
-The `skaffold.yaml` setup:
-```yaml
-build:
-  artifacts:
-    - image: gcr.io/my-project/my-image
-deploy:
-  helm:
-    releases:
-      - name: my-chart
-        chartPath: helm
-        artifactOverrides:
-          imageKey: gcr.io/my-project/my-image
-        imageStrategy:
-          helm:
-            explicitRegistry: true
-```
-
-The `values.yaml` (note that Skaffold overrides these values):
-```
-imageKey:
-  registry: gcr.io
-  repository: other-project/other-image
-  tag: latest
-```
-
-The chart template:
-```yaml
-spec:
-  containers:
-    - name: {{ .Chart.Name }}
-      image: "{{.Values.imageKey.registry}}/{{.Values.imageKey.repository}}:{{.Values.imageKey.tag}}"
-```
-
-Skaffold will invoke
-```
-helm install <chart> <chart-path> --set-string imageKey.registry=gcr.io,imageKey.repository=my-project/my-image,imageKey.tag=generatedTag@sha256:digest
-```
+{{< alert title="No more `artifactsOverride`" >}}
+Skaffold no longer requires the intricate configuring of `artifactsOverride` and image naming strategies.
+{{< /alert >}}
 
 
 ### Helm Build Dependencies

diff --git a/docs/content/en/docs/references/api/grpc.md b/docs/content/en/docs/references/api/grpc.md
@@ -1018,7 +1018,7 @@ For Cancelled Error code, use range 800 to 850.<br>
 | DEPLOY_CLEANUP_ERR | 1003 | Deploy clean up error |
 | DEPLOY_HELM_APPLY_LABELS | 1004 | Unable to apply helm labels. |
 | DEPLOY_HELM_USER_ERR | 1005 | Deploy error due to user deploy config for helm deployer |
-| DEPLOY_NO_MATCHING_BUILD | 1006 | Helm error when no build result is found of value specified in helm `artifactOverrides` |
+| DEPLOY_NO_MATCHING_BUILD | 1006 | An image was referenced with no matching build result |
 | DEPLOY_HELM_VERSION_ERR | 1007 | Unable to get helm client version |
 | DEPLOY_HELM_MIN_VERSION_ERR | 1008 | Helm version not supported. |
 | DEPLOY_KUBECTL_VERSION_ERR | 1109 | Unable to retrieve kubectl version |
@@ -1109,7 +1109,7 @@ Enum for Suggestion codes
 | CHECK_MINIKUBE_STATUS | 202 | Check minikube status |
 | INSTALL_HELM | 203 | Install helm tool |
 | UPGRADE_HELM | 204 | Upgrade helm tool |
-| FIX_SKAFFOLD_CONFIG_HELM_ARTIFACT_OVERRIDES | 205 | Fix helm `releases.artifactOverrides` config to match with `build.artiofacts` |
+| FIX_SKAFFOLD_CONFIG_HELM_ARTIFACT_OVERRIDES | 205 | Fix helm `releases.artifactOverrides` config to match with `build.artifacts` (no longer used in Skaffold v2) |
 | UPGRADE_HELM32 | 206 | Upgrade helm version to v3.2.0 and higher. |
 | FIX_SKAFFOLD_CONFIG_HELM_CREATE_NAMESPACE | 207 | Set `releases.createNamespace` to false. |
 | INVALID_KPT_MANIFESTS | 208 | check the Kptfile validation. |

diff --git a/docs/content/en/docs/tutorials/ci_cd.md b/docs/content/en/docs/tutorials/ci_cd.md
@@ -89,10 +89,6 @@ profiles:
               - ./deployment/dev/values.yaml
               - ./deployment/dev/secrets.yaml
             skipBuildDependencies: true
-            artifactOverrides:
-              image: asia.gcr.io/my-project/my-image
-            imageStrategy:
-              helm: {}
         flags:
           upgrade:
             - --install

diff --git a/docs/content/en/samples/deployers/helm.yaml b/docs/content/en/samples/deployers/helm.yaml
@@ -3,5 +3,3 @@ deploy:
     releases:
     - name: skaffold-helm
       chartPath: skaffold-helm
-      artifactOverrides:
-        image: gcr.io/k8s-skaffold/skaffold-helm
diff --git a/examples/helm-deployment/charts/values.yaml b/examples/helm-deployment/charts/values.yaml
@@ -2,3 +2,4 @@
 # This is a YAML-formatted file.
 # Declare variables to be passed into your templates.
 replicaCount: 2
+image: skaffold-helm:latest
diff --git a/examples/helm-deployment/skaffold.yaml b/examples/helm-deployment/skaffold.yaml
@@ -8,5 +8,3 @@ deploy:
     releases:
     - name: skaffold-helm
       chartPath: charts
-      artifactOverrides:
-        image: skaffold-helm
diff --git a/integration/examples/helm-deployment-dependencies/README.md b/integration/examples/helm-deployment-dependencies/README.md
@@ -6,8 +6,6 @@ This example follows the [helm](../helm-deployment) example, but with a local ch
 
 The `skipBuildDependencies` option is used to skip the `helm dep build` command. This must be disabled for charts with local dependencies.
 
-The image can be passed to the subchart using the standard Helm format of `subchart-name.value`.
-
 ```yaml
 deploy:
   helm:
@@ -16,9 +14,6 @@ deploy:
       chartPath: skaffold-helm
       namespace: skaffold
       skipBuildDependencies: true # Skip helm dep build
-      artifactOverrides :
-        image: skaffold-helm
-        "subchart.image": skaffold-helm # Set image for subchart
       valuesFiles:
         - helm-values-file.yaml
 ```
diff --git a/integration/examples/helm-deployment-dependencies/skaffold.yaml b/integration/examples/helm-deployment-dependencies/skaffold.yaml
@@ -14,10 +14,6 @@ deploy:
       #valuesFiles:
       #- helm-skaffold-values.yaml
       skipBuildDependencies: true # Skip helm dep build
-      artifactOverrides:
-        image: skaffold-helm
-        skaffold-helm-subchart:
-          image: skaffold-helm
       #recreatePods will pass --recreate-pods to helm upgrade
       #recreatePods: true
       #overrides builds an override values.yaml file to run with the helm deploy

diff --git a/integration/examples/helm-deployment/charts/values.yaml b/integration/examples/helm-deployment/charts/values.yaml
@@ -2,3 +2,4 @@
 # This is a YAML-formatted file.
 # Declare variables to be passed into your templates.
 replicaCount: 2
+image: skaffold-helm:latest
diff --git a/integration/examples/helm-deployment/skaffold.yaml b/integration/examples/helm-deployment/skaffold.yaml
@@ -8,5 +8,3 @@ deploy:
     releases:
     - name: skaffold-helm
       chartPath: charts
-      artifactOverrides:
-        image: skaffold-helm
diff --git a/integration/examples/templated-fields/skaffold.yaml b/integration/examples/templated-fields/skaffold.yaml
@@ -17,8 +17,6 @@ deploy:
     releases:
       - name: skaffold-templated
         chartPath: charts
-        artifactOverrides:
-          image: skaffold-templated
         setValueTemplates:
           imageRepo: "{{.IMAGE_REPO}}"
           imageTag: "{{.IMAGE_TAG}}"
diff --git a/integration/render_test.go b/integration/render_test.go
@@ -274,9 +274,6 @@ func TestHelmRender(t *testing.T) {
 			helmReleases: []latestV2.HelmRelease{{
 				Name:      "gke_loadbalancer",
 				ChartPath: "testdata/gke_loadbalancer/loadbalancer-helm",
-				ArtifactOverrides: map[string]string{
-					"image": "gke-loadbalancer",
-				},
 			}},
 			expectedOut: `---
 # Source: loadbalancer-helm/templates/k8s.yaml
@@ -332,9 +329,6 @@ spec:
 			helmReleases: []latestV2.HelmRelease{{
 				Name:      "skaffold-helm",
 				ChartPath: "testdata/helm/skaffold-helm",
-				ArtifactOverrides: map[string]string{
-					"image": "gcr.io/k8s-skaffold/skaffold-helm",
-				},
 				SetValues: map[string]string{
 					"pullPolicy": "Always",
 				},

diff --git a/integration/testdata/gke_loadbalancer/skaffold.yaml b/integration/testdata/gke_loadbalancer/skaffold.yaml
@@ -10,5 +10,3 @@ deploy:
     # seed test namespace in the release name.
     - name: skaffold-helm-{{.TEST_NS}}
       chartPath: loadbalancer-helm
-      artifactOverrides:
-        image: gke-loadbalancer
diff --git a/pkg/skaffold/deploy/docker/port_test.go b/pkg/skaffold/deploy/docker/port_test.go
@@ -20,12 +20,12 @@ import (
 	"strconv"
 	"testing"
 
-	v2 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v2"
+	"github.com/docker/docker/api/types/container"
 
+	v2 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v2"
 	schemautil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/testutil"
-	"github.com/docker/docker/api/types/container"
 )
 
 func TestAllocatePorts(t *testing.T) {