Repository: GoogleContainerTools/skaffold
Pull request #8117 (2022-11-18 17:20:02)

fix: new condition to create hydrate-dir only if a kpt renderer or deployer
Fixes: #8041

**Description**
This PR adds an extra condition before creating the `hydration-dir`, so it is created only when kpt is used (as renderer, deployer, or both).

codecov[bot]: # [Codecov](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools) Report
> Merging [#8117](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools) (9826e75) into [main](https://codecov.io/gh/GoogleContainerTools/skaffold/commit/290280e8c6c19231e9ed5b5c027f2c01d22f75be?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools) (290280e) will **decrease** coverage by `4.22%`.
> The diff coverage is `53.26%`.

```diff
@@            Coverage Diff             @@
##             main    #8117      +/-   ##
==========================================
- Coverage   70.48%   66.25%   -4.23%     
==========================================
  Files         515      599      +84     
  Lines       23150    29340    +6190     
==========================================
+ Hits        16317    19439    +3122     
- Misses       5776     8450    +2674     
- Partials     1057     1451     +394     
```


| [Impacted Files](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools) | Coverage Δ | |
|---|---|---|
| [cmd/skaffold/app/cmd/completion.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvY29tcGxldGlvbi5nbw==) | `13.04% <0.00%> (-1.25%)` | :arrow_down: |
| [cmd/skaffold/app/cmd/credits/export.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvY3JlZGl0cy9leHBvcnQuZ28=) | `0.00% <0.00%> (ø)` | |
| [cmd/skaffold/app/cmd/deploy.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvZGVwbG95Lmdv) | `40.90% <0.00%> (-12.94%)` | :arrow_down: |
| [cmd/skaffold/app/cmd/test.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvdGVzdC5nbw==) | `44.44% <0.00%> (ø)` | |
| [cmd/skaffold/app/exitcode.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9leGl0Y29kZS5nbw==) | `100.00% <ø> (+6.66%)` | :arrow_up: |
| [cmd/skaffold/skaffold.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL3NrYWZmb2xkLmdv) | `0.00% <0.00%> (ø)` | |
| [cmd/skaffold/app/cmd/inspect\_tests.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvaW5zcGVjdF90ZXN0cy5nbw==) | `62.50% <14.28%> (-1.14%)` | :arrow_down: |
| [cmd/skaffold/app/cmd/render.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvcmVuZGVyLmdv) | `35.48% <18.18%> (-5.90%)` | :arrow_down: |
| [cmd/skaffold/app/cmd/lsp.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvbHNwLmdv) | `28.12% <28.12%> (ø)` | |
| [cmd/skaffold/app/cmd/run.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvcnVuLmdv) | `64.28% <33.33%> (-9.63%)` | :arrow_down: |
| ... and [391 more](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/8117/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools) | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools)


Patch:
diff --git a/.github/workflows/integration-linux.yml b/.github/workflows/integration-linux.yml
index 93750149c..b3825e6af 100644
--- a/.github/workflows/integration-linux.yml
+++ b/.github/workflows/integration-linux.yml
@@ -19,6 +19,7 @@ jobs:
         kustomize_version: [3.5.4]
         ko_version: [0.4.0]
         kompose_version: [1.21.0]
+        kpt_version: [1.0.0-beta.13]
         gcloud_sdk_version: [410.0.0]
         container_structure_tests_version: [1.8.0]
         integration_test_partitions: [0, 1, 2, 3]
@@ -78,6 +79,12 @@ jobs:
         wget -O kompose https://github.com/kubernetes/kompose/releases/download/v${{ matrix.kompose_version }}/kompose-linux-amd64 && chmod +x kompose
         sudo mv kompose /usr/local/bin/
 
+    - name: Install Kpt
+      if: ${{ env.NON_DOCS_FILES_CHANGED != 0 }}
+      run: |
+        wget -O kpt https://github.com/GoogleContainerTools/kpt/releases/download/v${{ matrix.kpt_version }}/kpt_linux_amd64 && chmod +x kpt
+        sudo mv kpt /usr/local/bin/
+
     - name: Install GCloud
       if: ${{ env.NON_DOCS_FILES_CHANGED != 0 }}
       run: |
diff --git a/pkg/skaffold/deploy/util/util.go b/pkg/skaffold/deploy/util/util.go
index d55ee9db6..b69636154 100644
--- a/pkg/skaffold/deploy/util/util.go
+++ b/pkg/skaffold/deploy/util/util.go
@@ -18,7 +18,6 @@ package util
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -38,7 +37,6 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/output/log"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util/stringset"
 )
 
@@ -106,10 +104,15 @@ func ConsolidateNamespaces(original, new []string) []string {
 }
 
 // GetHydrationDir points to the directory where the manifest rendering happens. By default, it is set to "<WORKDIR>/.kpt-pipeline".
-func GetHydrationDir(opts config.SkaffoldOptions, workingDir string, promptIfNeeded bool) (string, error) {
+func GetHydrationDir(opts config.SkaffoldOptions, workingDir string, promptIfNeeded bool, isKptRendererOrDeployerUsed bool) (string, error) {
 	var hydratedDir string
 	var err error
 
+	if !isKptRendererOrDeployerUsed {
+		log.Entry(context.TODO()).Info("no kpt renderer or deployer found, skipping hydrated-dir creation")
+		return "", nil
+	}
+
 	if opts.HydrationDir == constants.DefaultHydrationDir {
 		hydratedDir = filepath.Join(workingDir, constants.DefaultHydrationDir)
 		promptIfNeeded = false
@@ -164,37 +167,6 @@ func GroupVersionResource(disco discovery.DiscoveryInterface, gvk schema.GroupVe
 	return false, schema.GroupVersionResource{}, fmt.Errorf("could not find resource for %s", gvk.String())
 }
 
-func GetManifestsFromHydrationDir(ctx context.Context, opts config.SkaffoldOptions) (manifest.ManifestList, error) {
-	workDir, err := util.RealWorkDir()
-	if err != nil {
-		return nil, fmt.Errorf("getting working directory: %w", err)
-	}
-	hydrationDir, err := GetHydrationDir(config.SkaffoldOptions{HydrationDir: opts.HydrationDir, AssumeYes: opts.AssumeYes},
-		workDir,
-		true)
-	if err != nil {
-		return nil, fmt.Errorf("getting render output path: %w", err)
-	}
-	// TODO(aaron-prindle) verify that using manifests.yaml here will work in all cases
-	hydratedManifestPath := filepath.Join(hydrationDir, "manifests.yaml")
-	if _, err := os.Stat(hydratedManifestPath); errors.Is(err, os.ErrNotExist) {
-		log.Entry(ctx).Warn(fmt.Errorf("unable to find manifests, attempted location %s: %w", hydratedManifestPath, err))
-		return nil, nil
-	}
-
-	f, err := os.Open(hydratedManifestPath)
-	if err != nil {
-		return nil, fmt.Errorf("opening hydrated manifest at %s: %w", hydratedManifestPath, err)
-	}
-	defer f.Close()
-
-	manifests, err := manifest.Load(f)
-	if err != nil {
-		return nil, fmt.Errorf("parsing manifests file into manifest list object: %w", err)
-	}
-	return manifests, nil
-}
-
 func GetManifestsFromHydratedManifests(ctx context.Context, hydratedManifests []string) (manifest.ManifestList, error) {
 	var manifests manifest.ManifestList
 	for _, path := range hydratedManifests {
diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go
index 274c50820..1d4d77a50 100644
--- a/pkg/skaffold/runner/new.go
+++ b/pkg/skaffold/runner/new.go
@@ -74,7 +74,7 @@ func NewForConfig(ctx context.Context, runCtx *runcontext.RunContext) (*Skaffold
 
 	var deployer deploy.Deployer
 
-	hydrationDir, err := util.GetHydrationDir(runCtx.Opts, runCtx.WorkingDir, true)
+	hydrationDir, err := util.GetHydrationDir(runCtx.Opts, runCtx.WorkingDir, true, isKptRendererOrDeployerUsed(runCtx.Pipelines))
 
 	if err != nil {
 		return nil, fmt.Errorf("getting render output path: %w", err)
@@ -253,3 +253,21 @@ func getTester(ctx context.Context, cfg test.Config, isLocalImage func(imageName
 
 	return tester, nil
 }
+
+func isKptRendererOrDeployerUsed(pipelines runcontext.Pipelines) bool {
+	for _, configName := range pipelines.AllOrderedConfigNames() {
+		pipeline := pipelines.GetForConfigName(configName)
+		renderConfig := pipeline.Render
+		deployConfig := pipeline.Deploy
+
+		if renderConfig.Validate != nil || renderConfig.Transform != nil || renderConfig.Kpt != nil {
+			return true
+		}
+
+		if deployConfig.KptDeploy != nil {
+			return true
+		}
+	}
+
+	return false
+}


Test patch:
diff --git a/integration/render_test.go b/integration/render_test.go
index 4549f277c..1394bfa07 100644
--- a/integration/render_test.go
+++ b/integration/render_test.go
@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"path/filepath"
 	"regexp"
 	"testing"
 
@@ -705,3 +706,134 @@ spec:
 		})
 	}
 }
+
+func TestRenderHydrationDirCreation(t *testing.T) {
+	MarkIntegrationTest(t, CanRunWithoutGcp)
+	const hydrationDir = "hydration-dir"
+
+	tests := []struct {
+		description              string
+		shouldCreateHydrationDir bool
+		config                   string
+		manifest                 string
+	}{
+		{
+			description:              "project with kpt renderer should create hydration dir",
+			shouldCreateHydrationDir: true,
+			config: `
+apiVersion: skaffold/v4beta1
+kind: Config
+
+build:
+  artifacts: []
+manifests:
+  kpt: []
+`,
+			manifest: `
+apiVersion: v1
+kind: Pod
+metadata:
+  name: getting-started
+spec:
+  containers:
+    - name: getting-started
+      image: skaffold-example`,
+		},
+		{
+			description:              "project with kpt deployer should create hydration dir",
+			shouldCreateHydrationDir: true,
+			config: `
+apiVersion: skaffold/v4beta1
+kind: Config
+
+build:
+  artifacts: []
+manifests:
+  kpt: []
+deploy:
+  kpt: {}
+`,
+			manifest: `
+apiVersion: v1
+kind: Pod
+metadata:
+  name: getting-started
+spec:
+  containers:
+    - name: getting-started
+      image: skaffold-example`,
+		},
+		{
+			description:              "project with rawYaml and transform should create hydration dir (uses kpt renderer)",
+			shouldCreateHydrationDir: true,
+			config: `
+apiVersion: skaffold/v4beta1
+kind: Config
+
+build:
+  artifacts: []
+manifests:
+  rawYaml:
+    - k8s/k8s-pod.yaml
+  transform:
+    - name: set-annotations
+      configMap:
+        - "author:fake-author"
+`,
+			manifest: `
+apiVersion: v1
+kind: Pod
+metadata:
+  name: getting-started
+spec:
+  containers:
+    - name: getting-started
+      image: skaffold-example`,
+		},
+		{
+			description:              "project without kpt should not create hydration dir",
+			shouldCreateHydrationDir: false,
+			config: `
+apiVersion: skaffold/v4beta1
+kind: Config
+
+build:
+  artifacts: []
+manifests:
+  rawYaml:
+    - k8s/k8s-pod.yaml
+`,
+			manifest: `
+apiVersion: v1
+kind: Pod
+metadata:
+  name: getting-started
+spec:
+  containers:
+  - name: getting-started
+    image: skaffold-example`,
+		},
+	}
+
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			tmpDir := t.NewTempDir()
+			tmpDir.Mkdir("k8s")
+			tmpDir.Write("skaffold.yaml", test.config)
+			tmpDir.Write("k8s/k8s-pod.yaml", test.manifest)
+			tmpDir.Chdir()
+
+			args := []string{"--hydration-dir", hydrationDir}
+
+			skaffold.Render(args...).RunOrFail(t.T)
+
+			_, err := os.Stat(filepath.Join(tmpDir.Root(), hydrationDir))
+
+			if test.shouldCreateHydrationDir {
+				t.CheckFalse(os.IsNotExist(err))
+			} else {
+				t.CheckTrue(os.IsNotExist(err))
+			}
+		})
+	}
+}
diff --git a/pkg/skaffold/deploy/util/util_test.go b/pkg/skaffold/deploy/util/util_test.go
index 191a66214..f9573b768 100644
--- a/pkg/skaffold/deploy/util/util_test.go
+++ b/pkg/skaffold/deploy/util/util_test.go
@@ -112,7 +112,7 @@ func TestGetHydrationDir_Default(t *testing.T) {
 		tmpDir.Chdir()
 		actual, err := GetHydrationDir(
 			config.SkaffoldOptions{HydrationDir: constants.DefaultHydrationDir, AssumeYes: true},
-			tmpDir.Root(), false)
+			tmpDir.Root(), false, true)
 		t.CheckNoError(err)
 		t.CheckDeepEqual(filepath.Join(tmpDir.Root(), ".kpt-pipeline"), actual)
 	})
@@ -124,7 +124,7 @@ func TestGetHydrationDir_CustomHydrationDir(t *testing.T) {
 		tmpDir.Chdir()
 		expected := filepath.Join(tmpDir.Root(), "test-hydration")
 		actual, err := GetHydrationDir(
-			config.SkaffoldOptions{HydrationDir: expected, AssumeYes: true}, "", false)
+			config.SkaffoldOptions{HydrationDir: expected, AssumeYes: true}, "", false, true)
 		t.CheckNoError(err)
 		t.CheckDeepEqual(expected, actual)
 		_, err = os.Stat(actual)