From 78c8bdfe810114a50a649d66adde437dcf47ae70 Mon Sep 17 00:00:00 2001 From: Felix Date: Tue, 25 Aug 2020 21:06:27 -0400 Subject: [PATCH 1/4] adding kustomize hydration into kpt's render --- pkg/skaffold/deploy/kpt.go | 114 ++++++++++++++++---- pkg/skaffold/deploy/kpt_test.go | 182 ++++++++++++++++++++------------ 2 files changed, 204 insertions(+), 92 deletions(-) diff --git a/pkg/skaffold/deploy/kpt.go b/pkg/skaffold/deploy/kpt.go index af5ad55e0fa..6be8fba431c 100644 --- a/pkg/skaffold/deploy/kpt.go +++ b/pkg/skaffold/deploy/kpt.go @@ -17,6 +17,7 @@ limitations under the License. package deploy import ( + "bytes" "context" "errors" "fmt" @@ -36,8 +37,9 @@ import ( ) var ( - kptHydrated = ".kpt-hydrated" inventoryTemplate = "inventory-template.yaml" + kptHydrated = ".kpt-hydrated" + pipeline = ".pipeline" ) // KptDeployer deploys workflows with kpt CLI @@ -47,6 +49,7 @@ type KptDeployer struct { insecureRegistries map[string]bool labels map[string]string globalConfig string + workingDir string } func NewKptDeployer(runCtx *runcontext.RunContext, labels map[string]string) *KptDeployer { @@ -55,6 +58,7 @@ func NewKptDeployer(runCtx *runcontext.RunContext, labels map[string]string) *Kp insecureRegistries: runCtx.GetInsecureRegistries(), labels: labels, globalConfig: runCtx.GlobalConfig(), + workingDir: runCtx.GetWorkingDir(), } } @@ -72,11 +76,19 @@ func (k *KptDeployer) Dependencies() ([]string, error) { configDeps, err := getResources(k.Dir) if err != nil { - return nil, fmt.Errorf("finding dependencies in %s: %w", k.Dir, err) + return nil, fmt.Errorf("finding resources in %s: %w", k.Dir, err) } deps.insert(configDeps...) + // Kpt deployer assumes that the kustomization configuration to build lives directly under k.Dir. + kustomizeDeps, err := dependenciesForKustomization(k.Dir) + if err != nil { + return nil, fmt.Errorf("finding kustomization directly under %s: %w", k.Dir, err) + } + + deps.insert(kustomizeDeps...) + return deps.toList(), nil } @@ -97,6 +109,7 @@ func (k *KptDeployer) Cleanup(ctx context.Context, _ io.Writer) error { return nil } +// Render hydrates manifests using both kustomization and kpt functions. func (k *KptDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, _ bool, filepath string) error { manifests, err := k.renderManifests(ctx, out, builds) if err != nil { @@ -106,12 +119,32 @@ func (k *KptDeployer) Render(ctx context.Context, out io.Writer, builds []build. return outputRenderedManifests(manifests.String(), filepath, out) } +// renderManifests handles a majority of the hydration process for manifests. +// This involves running reading configs from a source directory, kustomize build, kpt pipelines, +// adding image digests, and a run-id label. func (k *KptDeployer) renderManifests(ctx context.Context, _ io.Writer, builds []build.Artifact) (deploy.ManifestList, error) { debugHelpersRegistry, err := config.GetDebugHelpersRegistry(k.globalConfig) if err != nil { return nil, fmt.Errorf("retrieving debug helpers registry: %w", err) } + // .pipeline is a temp dir used to store output between steps of the desired workflow + // This can be removed once kpt can fully support the desired workflow independently. + if err := os.RemoveAll(filepath.Join(pipeline, k.Dir)); err != nil { + return nil, fmt.Errorf("deleting intermediate directory for workflow: %w", err) + } + if err := os.MkdirAll(filepath.Join(pipeline, k.Dir), 0755); err != nil { + return nil, fmt.Errorf("creating directory %s: %w", filepath.Join(pipeline, k.Dir), err) + } + + if err := k.readConfigs(ctx); err != nil { + return nil, fmt.Errorf("reading config manifests: %w", err) + } + + if err := k.kustomizeBuild(ctx); err != nil { + return nil, fmt.Errorf("kustomize build: %w", err) + } + manifests, err := k.kptFnRun(ctx) if err != nil { return nil, fmt.Errorf("running kpt functions: %w", err) @@ -136,30 +169,75 @@ func (k *KptDeployer) renderManifests(ctx context.Context, _ io.Writer, builds [ return manifests.SetLabels(k.labels) } -// kptFnRun does a dry run with the specified kpt functions (fn-path XOR image) against dir. -// If neither fn-path nor image are specified, functions will attempt to be discovered in dir. -// An error is returned when both fn-path and image are specified. +// readConfigs uses `kpt fn source` to read config manifests from k.Dir +// and uses `kpt fn sink` to output those manifests to .pipeline. +func (k *KptDeployer) readConfigs(ctx context.Context) error { + cmd := exec.CommandContext(ctx, "kpt", kptCommandArgs(k.Dir, []string{"fn", "source"}, nil, nil)...) + b, err := util.RunCmdOut(cmd) + if err != nil { + return err + } + + cmd = exec.CommandContext(ctx, "kpt", kptCommandArgs(filepath.Join(pipeline, k.Dir), []string{"fn", "sink"}, nil, nil)...) + cmd.Stdin = bytes.NewBuffer(b) + if _, err := util.RunCmdOut(cmd); err != nil { + return err + } + + return nil +} + +// kustomizeBuild runs `kustomize build` if a kustomization config exists and outputs to .pipeline. +func (k *KptDeployer) kustomizeBuild(ctx context.Context) error { + if _, err := findKustomizationConfig(k.Dir); err != nil { + // No kustomization config was found directly under k.Dir, so there is no need to continue. + return nil + } + + cmd := exec.CommandContext(ctx, "kustomize", buildCommandArgs([]string{"-o", filepath.Join(pipeline, k.Dir)}, k.Dir)...) + if _, err := util.RunCmdOut(cmd); err != nil { + return err + } + + deps, err := dependenciesForKustomization(k.Dir) + if err != nil { + return fmt.Errorf("finding kustomization dependencies: %w", err) + } + + // Kustomize build outputs hydrated configs to .pipeline, so the dry configs must be removed. + for _, v := range deps { + if err := os.RemoveAll(filepath.Join(pipeline, v)); err != nil { + return err + } + } + + return nil +} + +// kptFnRun does a dry run with the specified kpt functions (fn-path XOR image) against .pipeline. +// If neither fn-path nor image are specified, functions will attempt to be discovered in .pipeline. +// An error occurs if both fn-path and image are specified. func (k *KptDeployer) kptFnRun(ctx context.Context) (deploy.ManifestList, error) { var manifests deploy.ManifestList // --dry-run sets the pipeline's output to STDOUT, otherwise output is set to sinkDir. // For now, k.Dir will be treated as sinkDir (and sourceDir). flags := []string{"--dry-run"} - specifiedFnPath := false + count := 0 if len(k.Fn.FnPath) > 0 { flags = append(flags, "--fn-path", k.Fn.FnPath) - specifiedFnPath = true + count++ } if len(k.Fn.Image) > 0 { - if specifiedFnPath { - return nil, errors.New("cannot specify both fn-path and image") - } - flags = append(flags, "--image", k.Fn.Image) + count++ + } + if count > 1 { + return nil, errors.New("cannot specify both fn-path and image") } - cmd := exec.CommandContext(ctx, "kpt", kptCommandArgs(k.Dir, []string{"fn", "run"}, flags, nil)...) + cmd := exec.CommandContext(ctx, "kpt", kptCommandArgs(pipeline, []string{"fn", "run"}, flags, nil)...) out, err := util.RunCmdOut(cmd) if err != nil { return nil, err @@ -224,8 +302,7 @@ func kptCommandArgs(dir string, commands, flags, globalFlags []string) []string return args } -// getResources returns a list of all file names in `root` that end in .yaml or .yml -// and all local kustomization dependencies under root. +// getResources returns a list of all file names in root that end in .yaml or .yml func getResources(root string) ([]string, error) { var files []string @@ -241,14 +318,7 @@ func getResources(root string) ([]string, error) { return fmt.Errorf("matching %s with regex: %w", filepath.Base(path), err) } - if info.IsDir() { - depsForKustomization, err := dependenciesForKustomization(path) - if err != nil { - return err - } - - files = append(files, depsForKustomization...) - } else if isResource { + if !info.IsDir() && isResource { files = append(files, path) } diff --git a/pkg/skaffold/deploy/kpt_test.go b/pkg/skaffold/deploy/kpt_test.go index a634a230c34..4435e849806 100644 --- a/pkg/skaffold/deploy/kpt_test.go +++ b/pkg/skaffold/deploy/kpt_test.go @@ -20,8 +20,10 @@ import ( "bytes" "context" "errors" + "fmt" "io/ioutil" "os" + "path/filepath" "strings" "testing" @@ -241,6 +243,7 @@ func TestKpt_Cleanup(t *testing.T) { } func TestKpt_Render(t *testing.T) { + // The follow are outputs to `kpt fn run` commands. output1 := `apiVersion: v1 kind: Pod metadata: @@ -253,16 +256,6 @@ spec: output2 := `apiVersion: v1 kind: Pod -metadata: - namespace: default -spec: - containers: - - image: gcr.io/project/image2 - name: image2 -` - - output3 := `apiVersion: v1 -kind: Pod metadata: namespace: default spec: @@ -273,7 +266,7 @@ spec: name: image2 ` - output4 := `apiVersion: v1 + output3 := `apiVersion: v1 kind: Pod metadata: namespace: default @@ -293,13 +286,14 @@ spec: ` tests := []struct { - description string - builds []build.Artifact - labels map[string]string - cfg *latest.KptDeploy - commands util.Command - expected string - shouldErr bool + description string + builds []build.Artifact + labels map[string]string + cfg *latest.KptDeploy + commands util.Command + kustomizations map[string]string + expected string + shouldErr bool }{ { description: "no fnPath or image specified", @@ -312,7 +306,10 @@ spec: cfg: &latest.KptDeploy{ Dir: ".", }, - commands: testutil.CmdRunOut("kpt fn run . --dry-run", output1), + commands: testutil. + CmdRunOut("kpt fn source .", ``). + AndRunOut("kpt fn sink .pipeline", ``). + AndRunOut("kpt fn run .pipeline --dry-run", output1), expected: `apiVersion: v1 kind: Pod metadata: @@ -324,21 +321,42 @@ spec: `, }, { - description: "fnPath specified", + description: "fnPath specified, multiple resources, and labels", builds: []build.Artifact{ + { + ImageName: "gcr.io/project/image1", + Tag: "gcr.io/project/image1:tag1", + }, { ImageName: "gcr.io/project/image2", Tag: "gcr.io/project/image2:tag2", }, }, + labels: map[string]string{"user/label": "test"}, cfg: &latest.KptDeploy{ Dir: "test", Fn: latest.KptFn{FnPath: "kpt-func.yaml"}, }, - commands: testutil.CmdRunOut("kpt fn run test --dry-run --fn-path kpt-func.yaml", output2), + commands: testutil. + CmdRunOut("kpt fn source test", ``). + AndRunOut(fmt.Sprintf("kpt fn sink %s", filepath.Join(".pipeline", "test")), ``). + AndRunOut("kpt fn run .pipeline --dry-run --fn-path kpt-func.yaml", output3), expected: `apiVersion: v1 kind: Pod metadata: + labels: + user/label: test + namespace: default +spec: + containers: + - image: gcr.io/project/image1:tag1 + name: image1 +--- +apiVersion: v1 +kind: Pod +metadata: + labels: + user/label: test namespace: default spec: containers: @@ -347,7 +365,7 @@ spec: `, }, { - description: "image specified", + description: "fn image specified, multiple images in resource", builds: []build.Artifact{ { ImageName: "gcr.io/project/image1", @@ -359,10 +377,13 @@ spec: }, }, cfg: &latest.KptDeploy{ - Dir: "test", + Dir: ".", Fn: latest.KptFn{Image: "gcr.io/example.com/my-fn:v1.0.0 -- foo=bar"}, }, - commands: testutil.CmdRunOut("kpt fn run test --dry-run --image gcr.io/example.com/my-fn:v1.0.0 -- foo=bar", output3), + commands: testutil. + CmdRunOut("kpt fn source .", ``). + AndRunOut("kpt fn sink .pipeline", ``). + AndRunOut("kpt fn run .pipeline --dry-run --image gcr.io/example.com/my-fn:v1.0.0 -- foo=bar", output2), expected: `apiVersion: v1 kind: Pod metadata: @@ -376,59 +397,57 @@ spec: `, }, { - description: "multiple resources outputted", + description: "empty output from pipeline", builds: []build.Artifact{ { ImageName: "gcr.io/project/image1", Tag: "gcr.io/project/image1:tag1", }, - { - ImageName: "gcr.io/project/image2", - Tag: "gcr.io/project/image2:tag2", - }, }, + labels: map[string]string{"user/label": "test"}, cfg: &latest.KptDeploy{ - Dir: "test", - Fn: latest.KptFn{FnPath: "kpt-func.yaml"}, + Dir: ".", }, - commands: testutil.CmdRunOut("kpt fn run test --dry-run --fn-path kpt-func.yaml", output4), - expected: `apiVersion: v1 -kind: Pod -metadata: - namespace: default -spec: - containers: - - image: gcr.io/project/image1:tag1 - name: image1 ---- -apiVersion: v1 -kind: Pod -metadata: - namespace: default -spec: - containers: - - image: gcr.io/project/image2:tag2 - name: image2 -`, + commands: testutil. + CmdRunOut("kpt fn source .", ``). + AndRunOut("kpt fn sink .pipeline", ``). + AndRunOut("kpt fn run .pipeline --dry-run", ``), + expected: "\n", + }, + { + description: "both fnPath and image specified", + cfg: &latest.KptDeploy{ + Dir: ".", + Fn: latest.KptFn{ + FnPath: "kpt-func.yaml", + Image: "gcr.io/example.com/my-fn:v1.0.0 -- foo=bar"}, + }, + commands: testutil. + CmdRunOut("kpt fn source .", ``). + AndRunOut("kpt fn sink .pipeline", ``), + shouldErr: true, }, { - description: "user labels", + description: "kustomization render", builds: []build.Artifact{ { ImageName: "gcr.io/project/image1", Tag: "gcr.io/project/image1:tag1", }, }, - labels: map[string]string{"user/label": "test"}, cfg: &latest.KptDeploy{ Dir: ".", }, - commands: testutil.CmdRunOut("kpt fn run . --dry-run", output1), + commands: testutil. + CmdRunOut("kpt fn source .", ``). + AndRunOut("kpt fn sink .pipeline", ``). + AndRunOut("kustomize build -o .pipeline .", ``). + AndRunOut("kpt fn run .pipeline --dry-run", output1), + kustomizations: map[string]string{"kustomization.yaml": `resources: +- foo.yaml`}, expected: `apiVersion: v1 kind: Pod metadata: - labels: - user/label: test namespace: default spec: containers: @@ -437,42 +456,65 @@ spec: `, }, { - description: "empty output from pipeline", + description: "reading configs from sourceDir fails", + cfg: &latest.KptDeploy{ + Dir: ".", + }, + commands: testutil. + CmdRunOutErr("kpt fn source .", ``, errors.New("BUG")). + AndRunOut("kpt fn sink .pipeline", ``). + AndRunOut("kpt fn run .pipeline --dry-run", "invalid pipeline"), + shouldErr: true, + }, + { + description: "outputting configs to sinkDir fails", + cfg: &latest.KptDeploy{ + Dir: ".", + }, + commands: testutil. + CmdRunOut("kpt fn source .", ``). + AndRunOutErr("kpt fn sink .pipeline", ``, errors.New("BUG")). + AndRunOut("kpt fn run .pipeline --dry-run", "invalid pipeline"), + shouldErr: true, + }, + { + description: "kustomize build fails (invalid kustomization config)", builds: []build.Artifact{ { ImageName: "gcr.io/project/image1", Tag: "gcr.io/project/image1:tag1", }, }, - labels: map[string]string{"user/label": "test"}, cfg: &latest.KptDeploy{ Dir: ".", }, - commands: testutil.CmdRunOut("kpt fn run . --dry-run", ``), - expected: "\n", + commands: testutil. + CmdRunOut("kpt fn source .", ``). + AndRunOut("kpt fn sink .pipeline", ``). + AndRunOutErr("kustomize build -o .pipeline .", ``, errors.New("BUG")). + AndRunOut("kpt fn run .pipeline --dry-run", output1), + kustomizations: map[string]string{"kustomization.yaml": `resources: +- foo.yaml`}, + shouldErr: true, }, { description: "kpt fn run fails", cfg: &latest.KptDeploy{ Dir: ".", }, - commands: testutil.CmdRunOutErr("kpt fn run . --dry-run", "invalid pipeline", errors.New("BUG")), - shouldErr: true, - }, - { - description: "both fnPath and image specified", - cfg: &latest.KptDeploy{ - Dir: "test", - Fn: latest.KptFn{ - FnPath: "kpt-func.yaml", - Image: "gcr.io/example.com/my-fn:v1.0.0 -- foo=bar"}, - }, + commands: testutil. + CmdRunOut("kpt fn source .", ``). + AndRunOut("kpt fn sink .pipeline", ``). + AndRunOutErr("kpt fn run .pipeline --dry-run", "invalid pipeline", errors.New("BUG")), shouldErr: true, }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, test.commands) + tmpDir := t.NewTempDir().Chdir() + + tmpDir.WriteFiles(test.kustomizations) k := NewKptDeployer(&runcontext.RunContext{ WorkingDir: ".", From 22b9d9c6a5cc58273e15dc07924f521d140545ad Mon Sep 17 00:00:00 2001 From: Felix Date: Tue, 25 Aug 2020 21:52:29 -0400 Subject: [PATCH 2/4] fix comments --- pkg/skaffold/deploy/kpt.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/skaffold/deploy/kpt.go b/pkg/skaffold/deploy/kpt.go index 6be8fba431c..d2272d450a4 100644 --- a/pkg/skaffold/deploy/kpt.go +++ b/pkg/skaffold/deploy/kpt.go @@ -49,7 +49,6 @@ type KptDeployer struct { insecureRegistries map[string]bool labels map[string]string globalConfig string - workingDir string } func NewKptDeployer(runCtx *runcontext.RunContext, labels map[string]string) *KptDeployer { @@ -58,7 +57,6 @@ func NewKptDeployer(runCtx *runcontext.RunContext, labels map[string]string) *Kp insecureRegistries: runCtx.GetInsecureRegistries(), labels: labels, globalConfig: runCtx.GlobalConfig(), - workingDir: runCtx.GetWorkingDir(), } } @@ -76,7 +74,7 @@ func (k *KptDeployer) Dependencies() ([]string, error) { configDeps, err := getResources(k.Dir) if err != nil { - return nil, fmt.Errorf("finding resources in %s: %w", k.Dir, err) + return nil, fmt.Errorf("finding dependencies in %s: %w", k.Dir, err) } deps.insert(configDeps...) @@ -120,8 +118,8 @@ func (k *KptDeployer) Render(ctx context.Context, out io.Writer, builds []build. } // renderManifests handles a majority of the hydration process for manifests. -// This involves running reading configs from a source directory, kustomize build, kpt pipelines, -// adding image digests, and a run-id label. +// This involves reading configs from a source directory, running kustomize build, running kpt pipelines, +// adding image digests, and adding run-id labels. func (k *KptDeployer) renderManifests(ctx context.Context, _ io.Writer, builds []build.Artifact) (deploy.ManifestList, error) { debugHelpersRegistry, err := config.GetDebugHelpersRegistry(k.globalConfig) if err != nil { @@ -131,10 +129,10 @@ func (k *KptDeployer) renderManifests(ctx context.Context, _ io.Writer, builds [ // .pipeline is a temp dir used to store output between steps of the desired workflow // This can be removed once kpt can fully support the desired workflow independently. if err := os.RemoveAll(filepath.Join(pipeline, k.Dir)); err != nil { - return nil, fmt.Errorf("deleting intermediate directory for workflow: %w", err) + return nil, fmt.Errorf("deleting temporary directory %s: %w", filepath.Join(pipeline, k.Dir), err) } if err := os.MkdirAll(filepath.Join(pipeline, k.Dir), 0755); err != nil { - return nil, fmt.Errorf("creating directory %s: %w", filepath.Join(pipeline, k.Dir), err) + return nil, fmt.Errorf("creating temporary directory %s: %w", filepath.Join(pipeline, k.Dir), err) } if err := k.readConfigs(ctx); err != nil { From 5de979c20079bdfb0b8f8eb94d2b53701a98d22c Mon Sep 17 00:00:00 2001 From: Felix Date: Wed, 26 Aug 2020 02:21:10 -0400 Subject: [PATCH 3/4] improve documentation --- pkg/skaffold/deploy/kpt.go | 4 +++- pkg/skaffold/deploy/kpt_test.go | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/skaffold/deploy/kpt.go b/pkg/skaffold/deploy/kpt.go index d2272d450a4..febc6fc47b7 100644 --- a/pkg/skaffold/deploy/kpt.go +++ b/pkg/skaffold/deploy/kpt.go @@ -131,6 +131,8 @@ func (k *KptDeployer) renderManifests(ctx context.Context, _ io.Writer, builds [ if err := os.RemoveAll(filepath.Join(pipeline, k.Dir)); err != nil { return nil, fmt.Errorf("deleting temporary directory %s: %w", filepath.Join(pipeline, k.Dir), err) } + // 0755 is a permission setting where the owner can read, write, and execute. + // Others can read and execute but not modify the directory. if err := os.MkdirAll(filepath.Join(pipeline, k.Dir), 0755); err != nil { return nil, fmt.Errorf("creating temporary directory %s: %w", filepath.Join(pipeline, k.Dir), err) } @@ -259,7 +261,7 @@ func (k *KptDeployer) getApplyDir(ctx context.Context) (string, error) { } // 0755 is a permission setting where the owner can read, write, and execute. - // Others can read and execute but not modify the file. + // Others can read and execute but not modify the directory. if err := os.MkdirAll(kptHydrated, 0755); err != nil { return "", fmt.Errorf("applyDir was unspecified. creating applyDir: %w", err) } diff --git a/pkg/skaffold/deploy/kpt_test.go b/pkg/skaffold/deploy/kpt_test.go index 4435e849806..eaab1185850 100644 --- a/pkg/skaffold/deploy/kpt_test.go +++ b/pkg/skaffold/deploy/kpt_test.go @@ -215,6 +215,8 @@ func TestKpt_Cleanup(t *testing.T) { t.NewTempDir().Chdir() if test.applyDir == "valid_path" { + // 0755 is a permission setting where the owner can read, write, and execute. + // Others can read and execute but not modify the directory. os.Mkdir(test.applyDir, 0755) } @@ -573,6 +575,8 @@ func TestKpt_GetApplyDir(t *testing.T) { tmpDir := t.NewTempDir().Chdir() if test.applyDir == test.expected { + // 0755 is a permission setting where the owner can read, write, and execute. + // Others can read and execute but not modify the directory. os.Mkdir(test.applyDir, 0755) } From 1525a79683a14c9e36072caec0241ef5db3fdaa3 Mon Sep 17 00:00:00 2001 From: Felix Date: Wed, 26 Aug 2020 14:58:56 -0400 Subject: [PATCH 4/4] nit --- pkg/skaffold/deploy/kpt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/skaffold/deploy/kpt.go b/pkg/skaffold/deploy/kpt.go index febc6fc47b7..b54a8ba399e 100644 --- a/pkg/skaffold/deploy/kpt.go +++ b/pkg/skaffold/deploy/kpt.go @@ -234,7 +234,7 @@ func (k *KptDeployer) kptFnRun(ctx context.Context) (deploy.ManifestList, error) count++ } if count > 1 { - return nil, errors.New("cannot specify both fn-path and image") + return nil, errors.New("only one of `fn-path` or `image` configs can be specified at most") } cmd := exec.CommandContext(ctx, "kpt", kptCommandArgs(pipeline, []string{"fn", "run"}, flags, nil)...)