Repository: GoogleContainerTools/skaffold
Pull request #7426 (2022-05-19 17:11:29)

chore: remove hydrationDir from kubectl renderer

codecov[bot]: # [Codecov](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools) Report
> Merging [#7426](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools) (9e84e56) 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.06%`.
> The diff coverage is `55.29%`.

```diff
@@            Coverage Diff             @@
##             main    #7426      +/-   ##
==========================================
- Coverage   70.48%   66.41%   -4.07%     
==========================================
  Files         515      570      +55     
  Lines       23150    27554    +4404     
==========================================
+ Hits        16317    18301    +1984     
- Misses       5776     7915    +2139     
- Partials     1057     1338     +281     
```


| [Impacted Files](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426?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/credits/export.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426/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/7426/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvZGVwbG95Lmdv) | `50.00% <0.00%> (-3.85%)` | :arrow_down: |
| [cmd/skaffold/app/cmd/test.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426/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/skaffold.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426/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/render.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426/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% <11.11%> (-5.90%)` | :arrow_down: |
| [cmd/skaffold/app/cmd/inspect\_tests.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426/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/lsp.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426/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/fix.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvZml4Lmdv) | `55.93% <38.88%> (-20.54%)` | :arrow_down: |
| [cmd/skaffold/app/cmd/verify.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvdmVyaWZ5Lmdv) | `41.17% <41.17%> (ø)` | |
| [cmd/skaffold/app/cmd/lint.go](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools#diff-Y21kL3NrYWZmb2xkL2FwcC9jbWQvbGludC5nbw==) | `42.85% <42.85%> (ø)` | |
| ... and [299 more](https://codecov.io/gh/GoogleContainerTools/skaffold/pull/7426/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools) | |

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleContainerTools)

tejal29: Thank you, I wanted to get that removed!

Patch:
diff --git a/pkg/skaffold/deploy/kubectl/kubectl.go b/pkg/skaffold/deploy/kubectl/kubectl.go
index 1f148f3dc..3e0a79cbb 100644
--- a/pkg/skaffold/deploy/kubectl/kubectl.go
+++ b/pkg/skaffold/deploy/kubectl/kubectl.go
@@ -80,7 +80,6 @@ type Deployer struct {
 	insecureRegistries map[string]bool
 	labeller           *label.DefaultLabeller
 	skipRender         bool
-	hydrationDir       string
 	namespaces         *[]string
 
 	transformableAllowlist map[apimachinery.GroupKind]latest.ResourceFilter
@@ -89,7 +88,7 @@ type Deployer struct {
 
 // NewDeployer returns a new Deployer for a DeployConfig filled
 // with the needed configuration for `kubectl apply`
-func NewDeployer(cfg Config, labeller *label.DefaultLabeller, d *latest.KubectlDeploy, hydrationDir string) (*Deployer, error) {
+func NewDeployer(cfg Config, labeller *label.DefaultLabeller, d *latest.KubectlDeploy) (*Deployer, error) {
 	defaultNamespace := ""
 	if d.DefaultNamespace != nil {
 		var err error
@@ -132,7 +131,6 @@ func NewDeployer(cfg Config, labeller *label.DefaultLabeller, d *latest.KubectlD
 		// hydratedManifests refers to the DIR in the `skaffold apply DIR`. Used in both v1 and v2.
 		hydratedManifests: cfg.HydratedManifests(),
 		// hydrationDir refers to the path where the hydrated manifests are stored, this is introduced in v2.
-		hydrationDir: hydrationDir,
 
 		transformableAllowlist: transformableAllowlist,
 		transformableDenylist:  transformableDenylist,
diff --git a/pkg/skaffold/runner/deployer.go b/pkg/skaffold/runner/deployer.go
index 7e4c80a4b..8bc903a7c 100644
--- a/pkg/skaffold/runner/deployer.go
+++ b/pkg/skaffold/runner/deployer.go
@@ -115,7 +115,7 @@ func GetDeployer(ctx context.Context, runCtx *runcontext.RunContext, labeller *l
 			}
 		}
 
-		return getDefaultDeployer(runCtx, labeller, hydrationDir)
+		return getDefaultDeployer(runCtx, labeller)
 	}
 
 	var deployers []deploy.Deployer
@@ -158,7 +158,7 @@ func GetDeployer(ctx context.Context, runCtx *runcontext.RunContext, labeller *l
 		}
 
 		if d.KubectlDeploy != nil {
-			deployer, err := kubectl.NewDeployer(dCtx, labeller, d.KubectlDeploy, hydrationDir)
+			deployer, err := kubectl.NewDeployer(dCtx, labeller, d.KubectlDeploy)
 			if err != nil {
 				return nil, err
 			}
@@ -213,7 +213,7 @@ The default deployer will honor a select set of deploy configuration from an exi
 For a multi-config project, we do not currently support resolving conflicts between differing sets of this deploy configuration.
 Therefore, in this function we do implicit validation of the provided configuration, and fail if any conflict cannot be resolved.
 */
-func getDefaultDeployer(runCtx *runcontext.RunContext, labeller *label.DefaultLabeller, hydrationDir string) (deploy.Deployer, error) {
+func getDefaultDeployer(runCtx *runcontext.RunContext, labeller *label.DefaultLabeller) (deploy.Deployer, error) {
 	deployCfgs := runCtx.DeployConfigs()
 
 	var kFlags *latest.KubectlFlags
@@ -280,7 +280,7 @@ func getDefaultDeployer(runCtx *runcontext.RunContext, labeller *label.DefaultLa
 		DefaultNamespace: defaultNamespace,
 	}
 	dCtx := &deployerCtx{runCtx, latest.DeployConfig{StatusCheck: statusCheck, KubeContext: kubeContext, DeployType: latest.DeployType{KubectlDeploy: k}}}
-	defaultDeployer, err := kubectl.NewDeployer(dCtx, labeller, k, hydrationDir)
+	defaultDeployer, err := kubectl.NewDeployer(dCtx, labeller, k)
 	if err != nil {
 		return nil, fmt.Errorf("instantiating default kubectl deployer: %w", err)
 	}


Test patch:
diff --git a/pkg/skaffold/deploy/kubectl/kubectl_test.go b/pkg/skaffold/deploy/kubectl/kubectl_test.go
index acc2da71c..d624e67cc 100644
--- a/pkg/skaffold/deploy/kubectl/kubectl_test.go
+++ b/pkg/skaffold/deploy/kubectl/kubectl_test.go
@@ -28,7 +28,6 @@ import (
 	"time"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/label"
 	deployutil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/util"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
@@ -235,7 +234,7 @@ func TestKubectlV1RenderDeploy(t *testing.T) {
 				},
 				RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{
 					Namespace: skaffoldNamespaceOption}},
-			}, &label.DefaultLabeller{}, &test.kubectl, filepath.Join(tmpDir.Root(), constants.DefaultHydrationDir))
+			}, &label.DefaultLabeller{}, &test.kubectl)
 			t.RequireNoError(err)
 
 			rc := latest.RenderConfig{Generate: test.generate}
@@ -327,7 +326,7 @@ func TestKubectlCleanup(t *testing.T) {
 			k, err := NewDeployer(&kubectlConfig{
 				workingDir: ".",
 				RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{Namespace: TestNamespace}},
-			}, &label.DefaultLabeller{}, &test.kubectl, filepath.Join(tmpDir.Root(), constants.DefaultHydrationDir))
+			}, &label.DefaultLabeller{}, &test.kubectl)
 			t.RequireNoError(err)
 			rc := latest.RenderConfig{Generate: test.generate}
 			mockCfg := &kubectlConfig{
@@ -387,7 +386,7 @@ func TestKubectlDeployerRemoteCleanup(t *testing.T) {
 			k, err := NewDeployer(&kubectlConfig{
 				workingDir: ".",
 				RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{Namespace: TestNamespace}},
-			}, &label.DefaultLabeller{}, &test.kubectl, filepath.Join(tmpDir.Root(), constants.DefaultHydrationDir))
+			}, &label.DefaultLabeller{}, &test.kubectl)
 			t.RequireNoError(err)
 
 			err = k.Cleanup(context.Background(), ioutil.Discard, false, nil)
@@ -416,8 +415,7 @@ func TestKubectlRedeploy(t *testing.T) {
 				Enabled: true,
 				Delay:   0 * time.Millisecond,
 				Max:     10 * time.Second},
-		}, &label.DefaultLabeller{}, &latest.KubectlDeploy{Manifests: []string{tmpDir.Path("deployment-app.yaml"), tmpDir.Path("deployment-web.yaml")}},
-			filepath.Join(tmpDir.Root(), constants.DefaultHydrationDir))
+		}, &label.DefaultLabeller{}, &latest.KubectlDeploy{Manifests: []string{tmpDir.Path("deployment-app.yaml"), tmpDir.Path("deployment-web.yaml")}})
 		t.RequireNoError(err)
 
 		// Deploy both manifests
@@ -485,8 +483,7 @@ func TestKubectlWaitForDeletions(t *testing.T) {
 				Delay:   0 * time.Millisecond,
 				Max:     10 * time.Second,
 			},
-		}, &label.DefaultLabeller{}, &latest.KubectlDeploy{Manifests: []string{tmpDir.Path("deployment-web.yaml")}},
-			filepath.Join(tmpDir.Root(), constants.DefaultHydrationDir))
+		}, &label.DefaultLabeller{}, &latest.KubectlDeploy{Manifests: []string{tmpDir.Path("deployment-web.yaml")}})
 		t.RequireNoError(err)
 
 		var out bytes.Buffer
@@ -524,8 +521,7 @@ func TestKubectlWaitForDeletionsFails(t *testing.T) {
 				Delay:   10 * time.Second,
 				Max:     100 * time.Millisecond,
 			},
-		}, &label.DefaultLabeller{}, &latest.KubectlDeploy{Manifests: []string{tmpDir.Path("deployment-web.yaml")}},
-			filepath.Join(tmpDir.Root(), constants.DefaultHydrationDir))
+		}, &label.DefaultLabeller{}, &latest.KubectlDeploy{Manifests: []string{tmpDir.Path("deployment-web.yaml")}})
 		t.RequireNoError(err)
 
 		m, err := manifest.Load(bytes.NewReader([]byte(DeploymentWebYAMLv1)))
@@ -588,8 +584,7 @@ func TestDependencies(t *testing.T) {
 				Touch("00/b.yaml", "00/a.yaml").
 				Chdir()
 
-			k, err := NewDeployer(&kubectlConfig{}, &label.DefaultLabeller{}, &latest.KubectlDeploy{Manifests: test.manifests},
-				filepath.Join(tmpDir.Root(), constants.DefaultHydrationDir))
+			k, err := NewDeployer(&kubectlConfig{}, &label.DefaultLabeller{}, &latest.KubectlDeploy{Manifests: test.manifests})
 			t.RequireNoError(err)
 
 			dependencies, err := k.Dependencies()
@@ -645,7 +640,7 @@ func TestGCSManifests(t *testing.T) {
 				workingDir: ".",
 				skipRender: test.skipRender,
 				RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{Namespace: TestNamespace}},
-			}, &label.DefaultLabeller{}, &latest.KubectlDeploy{}, filepath.Join("", constants.DefaultHydrationDir))
+			}, &label.DefaultLabeller{}, &latest.KubectlDeploy{})
 			t.RequireNoError(err)
 
 			err = k.Deploy(context.Background(), ioutil.Discard, nil, m)
@@ -682,7 +677,7 @@ func TestHasRunnableHooks(t *testing.T) {
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
-			k, err := NewDeployer(&kubectlConfig{}, &label.DefaultLabeller{}, &test.cfg, "")
+			k, err := NewDeployer(&kubectlConfig{}, &label.DefaultLabeller{}, &test.cfg)
 			t.RequireNoError(err)
 			actual := k.HasRunnableHooks()
 			t.CheckDeepEqual(test.expected, actual)
diff --git a/pkg/skaffold/runner/deployer_test.go b/pkg/skaffold/runner/deployer_test.go
index f5ba47de6..ce5158354 100644
--- a/pkg/skaffold/runner/deployer_test.go
+++ b/pkg/skaffold/runner/deployer_test.go
@@ -95,7 +95,7 @@ func TestGetDeployer(tOuter *testing.T) {
 						Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}),
 					}, &label.DefaultLabeller{}, &latest.KubectlDeploy{
 						Flags: latest.KubectlFlags{},
-					}, "")).(deploy.Deployer),
+					})).(deploy.Deployer),
 				}, false),
 			},
 			{
@@ -135,7 +135,7 @@ func TestGetDeployer(tOuter *testing.T) {
 					Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}),
 				}, &label.DefaultLabeller{}, &latest.KubectlDeploy{
 					Flags: latest.KubectlFlags{},
-				}, "")).(deploy.Deployer),
+				})).(deploy.Deployer),
 			},
 			{
 				description: "apply forces creation of kubectl deployer with helm config",
@@ -152,7 +152,7 @@ func TestGetDeployer(tOuter *testing.T) {
 					Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}),
 				}, &label.DefaultLabeller{}, &latest.KubectlDeploy{
 					Flags: latest.KubectlFlags{},
-				}, "")).(deploy.Deployer),
+				})).(deploy.Deployer),
 			},
 			{
 				description: "multiple deployers",
@@ -229,7 +229,7 @@ func TestGetDeployer(tOuter *testing.T) {
 					Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}),
 				}, &label.DefaultLabeller{}, &latest.KubectlDeploy{
 					Flags: latest.KubectlFlags{},
-				}, "")).(deploy.Deployer),
+				})).(deploy.Deployer),
 			},
 			{
 				description: "apply works with Cloud Run",
@@ -324,7 +324,7 @@ func TestGetDefaultDeployer(tOuter *testing.T) {
 					Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}),
 				}, &label.DefaultLabeller{}, &latest.KubectlDeploy{
 					Flags: latest.KubectlFlags{},
-				}, "")).(*kubectl.Deployer),
+				})).(*kubectl.Deployer),
 			},
 			{
 				name: "one config with kubectl deploy, with flags",
@@ -343,7 +343,7 @@ func TestGetDefaultDeployer(tOuter *testing.T) {
 						Apply:  []string{"--foo"},
 						Global: []string{"--bar"},
 					},
-				}, "")).(*kubectl.Deployer),
+				})).(*kubectl.Deployer),
 			},
 			{
 				name: "two kubectl configs with mismatched flags should fail",
@@ -374,7 +374,7 @@ func TestGetDefaultDeployer(tOuter *testing.T) {
 					Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}),
 				}, &label.DefaultLabeller{}, &latest.KubectlDeploy{
 					Flags: latest.KubectlFlags{},
-				}, "")).(*kubectl.Deployer),
+				})).(*kubectl.Deployer),
 			},
 			{
 				name: "one config with kustomize deploy",
@@ -385,7 +385,7 @@ func TestGetDefaultDeployer(tOuter *testing.T) {
 					Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}),
 				}, &label.DefaultLabeller{}, &latest.KubectlDeploy{
 					Flags: latest.KubectlFlags{},
-				}, "")).(*kubectl.Deployer),
+				})).(*kubectl.Deployer),
 			},
 		}
 
@@ -401,7 +401,7 @@ func TestGetDefaultDeployer(tOuter *testing.T) {
 				}
 				deployer, err := getDefaultDeployer(&runcontext.RunContext{
 					Pipelines: runcontext.NewPipelines(pipelines),
-				}, &label.DefaultLabeller{}, "")
+				}, &label.DefaultLabeller{})
 
 				t.CheckErrorAndFailNow(test.shouldErr, err)