-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add command to find a
kustomize
build base
The current approach to finding a build base[1] is a bit too naive. It just uses the names from `git-diff` and excludes deleted files (to avoid answering the question: how do you run a `kustomize build` when you just removed a `kustomize`). This command aims to address this naivety by having explicit actions on different types of changes, e.g. if we delete a `kustomization.yaml` we should walk up the tree until we find another and declare that as the build root. My focus is to get something _simple_ working, and not necessarily perfect, some tradeoffs are noted in the README, though additionally the command expects you to pass the `build-root` (basically, root of the Git repo) rather than computing that itself. But I think it's a sufficient improvement to warrant experimenting with it. Some examples, running it on [2] kubernetes-manifests$ git diff --raw 280d134c02222dd43e9f3230f7378dd838692581~ 280d134c02222dd43e9f3230f7378dd838692581 | ../manifest-checkers/get-kustomize-build-base --build-root "$PWD" | sort --unique dev-aws/sys-mon/thanos-shared And [3] (un-merged, so checked out at `7ae644208385efbeeac7ca7e34cb887f5df80cf7`) kubernetes-manifests$ git diff --raw "$(git merge-base master HEAD)" | ../manifest-checkers/get-kustomize-build-base --build-root "$PWD" | sort --unique dev-merit/dev-enablement dev-merit/kube-system I also tested using a patched version of `kustomize-build-dirs` that just prints the directories it would build, here's the patch: diff --git a/cmd/kustomize-build-dirs/main.go b/cmd/kustomize-build-dirs/main.go index 3d6bd69..880bd22 100644 --- a/cmd/kustomize-build-dirs/main.go +++ b/cmd/kustomize-build-dirs/main.go @@ -74,6 +74,10 @@ func kustomizeBuildDirs(outDir string, doTruncateSecrets bool, filepaths []strin if err != nil { return err } + for _, root := range kustomizationRoots { + fmt.Println(root) + } + return nil kustomizationRoots, err = removeComponentKustomizations(rootDir, kustomizationRoots) if err != nil { //go-cov:skip @@ -127,7 +131,7 @@ func findKustomizationRoots(root string, paths []string) ([]string, error) { } if _, exists := rootsMap[kustomizationRoot]; !exists { - fmt.Printf("Found kustomization build dir: %s\n", kustomizationRoot) + //fmt.Printf("Found kustomization build dir: %s\n", kustomizationRoot) rootsMap[kustomizationRoot] = struct{}{} } } Via a script: #!/usr/bin/env bash set -o errexit -o pipefail -o nounset while read -r commit_sha do our_bases="$(git diff --raw "$commit_sha"^1 "$commit_sha" | ../manifest-checkers/get-kustomize-build-base --build-root "$PWD" | sort --unique)" their_bases="$(git diff --diff-filter=d --name-only "$commit_sha"^1 "$commit_sha" | xargs ../manifest-checkers/kustomize-build-dirs --out-dir /dev/null | sort)" if ! diff <(echo "$our_bases") <(echo "$their_bases") then echo "diff for $commit_sha" fi done < <(git log --format='%H' --max-count 100) It reported two differences, both where a new `kustomization.yaml` was added, i.e. old `kustomize-build dirs` wouldn't have built these, new one would: 1d0 < prod-aws/energy-smart/fabricators diff for cfb45a6726d640c17926fdd3056dd58ad82db0f3 1d0 < dev-merit/partner-commission diff for eb130425af15096fa7984770295199ae745551d8 [1] https://github.com/utilitywarehouse/kubernetes-manifests/blob/d0a3f5eca5a47ae9f0a5cf42d1f6e8a9994fe7bb/.github/workflows/manifest-checks.yaml#L54 [2] utilitywarehouse/kubernetes-manifests#92921 [3] utilitywarehouse/kubernetes-manifests#98196 Ticket: DENA-842
- Loading branch information
1 parent
459f576
commit 0c9ab2b
Showing
4 changed files
with
375 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# `get-kustomize-build-base` | ||
|
||
Command to find the build base to run `kustomize build` provided some changes | ||
from git. Specifically, this command expects to be fed the output of `git diff | ||
--raw` like: | ||
|
||
git diff --raw HEAD^1 HEAD | get-kustomimze-build-base --build-root "$PWD" | ||
|
||
Where `HEAD` is a merge commit or a squashed commit representing a pull request. | ||
The output depends on the type of change for each file in the output. For | ||
modifications it will walk up the directory tree until the nearest | ||
`kustomization.yaml`. For additions or deletions, if the file is not a | ||
`kustomization.yaml` it will do the same as for a modification, otherwise it | ||
will walk up the directory, skipping the current directory, until it finds a | ||
`kustomization.yaml` (i.e. a parent build directory that should exist on both | ||
sides of the change) | ||
|
||
It does **not** (yet): | ||
|
||
- Deduplicate its output, e.g. if two files under a single build directory are | ||
change then that directory will be printed twice | ||
- Handle deleting a build directory and its parent, in this case when | ||
processing the child build directory it will print the parent directory | ||
(which wouldn't exist on the other side of the diff). |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,140 @@ | ||
package main | ||
|
||
import ( | ||
"bufio" | ||
"context" | ||
"errors" | ||
"fmt" | ||
"io" | ||
"io/fs" | ||
"os" | ||
"os/signal" | ||
"path/filepath" | ||
"strings" | ||
"syscall" | ||
|
||
"github.com/urfave/cli/v2" | ||
) | ||
|
||
// see https://git-scm.com/docs/git-diff#_raw_output_format | ||
const ( | ||
changeStatusModification = "M" | ||
changeStatusChangingType = "T" | ||
changeStatusAddition = "A" | ||
changeStatusDeletion = "D" | ||
changeStatusCopy = "C" | ||
changeStatusRenaming = "R" | ||
// not relevant: U and X | ||
|
||
gitDiffStatusColumn = 4 | ||
) | ||
|
||
func main() { | ||
if err := runApp(context.Background(), os.Stdout, os.Stdin, os.Args); err != nil { | ||
fmt.Fprintln(os.Stderr, err) | ||
os.Exit(1) | ||
} | ||
} | ||
|
||
func runApp(ctx context.Context, out io.Writer, in io.Reader, args []string) error { | ||
ctx, cancel := signal.NotifyContext(ctx, syscall.SIGINT) | ||
defer cancel() | ||
|
||
app := buildApp(out, in) | ||
|
||
return app.RunContext(ctx, args) | ||
} | ||
|
||
func buildApp(out io.Writer, in io.Reader) cli.App { | ||
return cli.App{ | ||
Name: "get-kustomize-build-base", | ||
Usage: "Print the list of build bases for kustomize given the output of `git diff --raw` provided via stdin", | ||
Flags: []cli.Flag{ | ||
&cli.StringFlag{ | ||
Name: "build-root", | ||
Required: true, | ||
}, | ||
}, | ||
Action: func(c *cli.Context) error { | ||
return run(c.Context, out, in, c.String("build-root")) | ||
}, | ||
} | ||
} | ||
|
||
func run(_ context.Context, out io.Writer, in io.Reader, buildRoot string) error { | ||
scanner := bufio.NewScanner(in) | ||
for scanner.Scan() { | ||
line := scanner.Text() | ||
parts := strings.Split(line, " ") | ||
// files are listed directly after the status | ||
statusAndFiles := strings.Split(parts[gitDiffStatusColumn], "\t") | ||
status := statusAndFiles[0] | ||
files := statusAndFiles[1:] | ||
kustomizeBase, err := getBaseForChange(buildRoot, status, files) | ||
if err != nil { | ||
return err | ||
} | ||
fmt.Fprintln(out, kustomizeBase) | ||
|
||
} | ||
if err := scanner.Err(); err != nil { | ||
return fmt.Errorf("scanning input: %w", err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func getBaseForChange(buildRoot string, status string, files []string) (string, error) { | ||
switch status { | ||
case changeStatusModification, changeStatusChangingType: | ||
return findKustomizationRoot(buildRoot, files[0]) | ||
case changeStatusAddition, changeStatusDeletion: | ||
return findKustomizationRootForNew(buildRoot, files[0]) | ||
case changeStatusCopy: | ||
dst := files[1] | ||
return findKustomizationRootForNew(buildRoot, dst) | ||
case changeStatusRenaming: | ||
// a rename is just a delete+an add | ||
src := files[0] | ||
deleted, err := findKustomizationRootForNew(buildRoot, src) | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
dst := files[1] | ||
added, err := findKustomizationRootForNew(buildRoot, dst) | ||
if err != nil { | ||
return "", err | ||
} | ||
// bit of a hack to return multiple lines... | ||
return deleted + "\n" + added, nil | ||
|
||
} | ||
return "", nil | ||
} | ||
|
||
func findKustomizationRootForNew(buildRoot string, relativePath string) (string, error) { | ||
var searchRoot string | ||
if filepath.Base(relativePath) == "kustomization.yaml" { | ||
// TODO: what if we've also added/removed this root? | ||
searchRoot = filepath.Dir(relativePath) | ||
} else { | ||
searchRoot = relativePath | ||
} | ||
return findKustomizationRoot(buildRoot, searchRoot) | ||
} | ||
|
||
func findKustomizationRoot(buildRoot string, relativePath string) (string, error) { | ||
for dir := filepath.Dir(relativePath); dir != ".."; dir = filepath.Clean(filepath.Join(dir, "..")) { | ||
_, err := os.Stat(filepath.Join(buildRoot, dir, "kustomization.yaml")) | ||
if err != nil { | ||
if errors.Is(err, fs.ErrNotExist) { | ||
// file not found, continue up the directory tree | ||
continue | ||
} | ||
return "", fmt.Errorf("stating diretcory %s: %w", dir, err) | ||
} | ||
return dir, nil | ||
} | ||
return "", nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,204 @@ | ||
package main | ||
|
||
import ( | ||
"bytes" | ||
"context" | ||
"fmt" | ||
"os" | ||
"path/filepath" | ||
"strings" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
type change struct { | ||
status string | ||
files []string | ||
} | ||
|
||
func Test_KustomizeRootDetection(t *testing.T) { | ||
files := []string{ | ||
"dev-aws/first-namespace/kustomization.yaml", | ||
"dev-aws/first-namespace/first-service/kustomization.yaml", | ||
"dev-aws/first-namespace/first-service/deployment.yaml", | ||
"dev-aws/first-namespace/first-service/config/config.yaml", | ||
"dev-aws/first-namespace/second-service/kustomization.yaml", | ||
"dev-aws/first-namespace/second-service/cronjob.yaml", | ||
|
||
"dev-aws/second-namespace/kustomization.yaml", | ||
"dev-aws/second-namespace/first-service/kustomization.yaml", | ||
"dev-aws/second-namespace/first-service/deployment.yaml", | ||
"dev-aws/second-namespace/first-service/config/nested/config.yaml", | ||
|
||
"prod-aws/first-namespace/kustomization.yaml", | ||
"prod-aws/first-namespace/first-service/kustomization.yaml", | ||
"prod-aws/first-namespace/first-service/deployment.yaml", | ||
"prod-aws/first-namespace/first-service/config/config.yaml", | ||
"prod-aws/first-namespace/second-service/kustomization.yaml", | ||
"prod-aws/first-namespace/second-service/cronjob.yaml", | ||
} | ||
buildRoot := setupFiles(t, files) | ||
|
||
for _, tc := range []struct { | ||
desc string | ||
changes []change | ||
expectedBases []string | ||
}{ | ||
{ | ||
desc: "base of edit of single kustomization is that kustomization", | ||
changes: []change{ | ||
{ | ||
status: changeStatusModification, | ||
files: []string{"dev-aws/first-namespace/first-service/kustomization.yaml"}, | ||
}, | ||
}, | ||
expectedBases: []string{"dev-aws/first-namespace/first-service"}, | ||
}, | ||
{ | ||
desc: "base of edit of single non-kustomization is that nearest kustomization", | ||
changes: []change{ | ||
{ | ||
status: changeStatusModification, | ||
files: []string{"dev-aws/first-namespace/first-service/deployment.yaml"}, | ||
}, | ||
}, | ||
expectedBases: []string{"dev-aws/first-namespace/first-service"}, | ||
}, | ||
{ | ||
desc: "base of adding of single non-kustomization is that nearest kustomization", | ||
changes: []change{ | ||
{ | ||
status: changeStatusAddition, | ||
files: []string{"dev-aws/first-namespace/first-service/deployment.yaml"}, | ||
}, | ||
}, | ||
expectedBases: []string{"dev-aws/first-namespace/first-service"}, | ||
}, | ||
{ | ||
desc: "base of adding a kustomization is the nearest parent kustomization", | ||
changes: []change{ | ||
{ | ||
status: changeStatusAddition, | ||
files: []string{"dev-aws/first-namespace/first-service/kustomization.yaml"}, | ||
}, | ||
}, | ||
expectedBases: []string{"dev-aws/first-namespace"}, | ||
}, | ||
{ | ||
desc: "base of deleting a single non-kustomization is that nearest kustomization", | ||
changes: []change{ | ||
{ | ||
status: changeStatusDeletion, | ||
files: []string{"dev-aws/first-namespace/first-service/deployment.yaml"}, | ||
}, | ||
}, | ||
expectedBases: []string{"dev-aws/first-namespace/first-service"}, | ||
}, | ||
{ | ||
desc: "base of deleting a kustomization is the nearest parent kustomization", | ||
changes: []change{ | ||
{ | ||
status: changeStatusAddition, | ||
files: []string{"dev-aws/first-namespace/first-service/kustomization.yaml"}, | ||
}, | ||
}, | ||
expectedBases: []string{"dev-aws/first-namespace"}, | ||
}, | ||
{ | ||
desc: "base of copying a single non-kustomization is that nearest kustomization", | ||
changes: []change{ | ||
{ | ||
status: changeStatusCopy, | ||
files: []string{ | ||
"dev-aws/first-namespace/first-service/deployment.yaml", | ||
"dev-aws/first-namespace/first-service/deployment2.yaml", | ||
}, | ||
}, | ||
}, | ||
expectedBases: []string{"dev-aws/first-namespace/first-service"}, | ||
}, | ||
{ | ||
desc: "base of copying a kustomization is that nearest parent kustomization", | ||
changes: []change{ | ||
{ | ||
status: changeStatusCopy, | ||
files: []string{ | ||
"dev-aws/first-namespace/first-service/kustomization.yaml", | ||
"dev-aws/second-namespace/first-service/kustomization.yaml", | ||
}, | ||
}, | ||
}, | ||
expectedBases: []string{"dev-aws/second-namespace"}, | ||
}, | ||
{ | ||
desc: "base of renaming file is base of both source and destination", | ||
changes: []change{ | ||
{ | ||
status: changeStatusRenaming, | ||
files: []string{ | ||
"dev-aws/first-namespace/first-service/deployment.yaml", | ||
"dev-aws/first-namespace/first-service/better-named.yaml", | ||
}, | ||
}, | ||
}, | ||
expectedBases: []string{ | ||
"dev-aws/first-namespace/first-service", | ||
"dev-aws/first-namespace/first-service", | ||
}, | ||
}, | ||
} { | ||
t.Run(tc.desc, func(t *testing.T) { | ||
diffTreeOutput := buildDiffTreeOutput(tc.changes) | ||
|
||
out := &bytes.Buffer{} | ||
in := strings.NewReader(diffTreeOutput) | ||
ctx := context.Background() | ||
args := []string{"exec-name", "--build-root", buildRoot} | ||
|
||
err := runApp(ctx, out, in, args) | ||
require.NoError(t, err) | ||
|
||
// trim the trailing newline to avoid a "" when splitting | ||
bases := strings.Split(strings.TrimSuffix(out.String(), "\n"), "\n") | ||
require.Equal(t, tc.expectedBases, bases) | ||
}) | ||
} | ||
} | ||
|
||
func setupFiles(t *testing.T, files []string) string { | ||
t.Helper() | ||
testDir := t.TempDir() | ||
|
||
for _, filename := range files { | ||
normPath := filepath.Join(strings.Split(filename, "/")...) | ||
fullPath := filepath.Join(testDir, normPath) | ||
dir := filepath.Dir(fullPath) | ||
|
||
require.NoError(t, os.MkdirAll(dir, 0o700)) | ||
_, err := os.Create(fullPath) | ||
require.NoError(t, err) | ||
} | ||
|
||
return testDir | ||
} | ||
|
||
func buildDiffTreeOutput(changes []change) string { | ||
// placeholders for parts of the output we aren't interested in | ||
sha := "cb3d1f86f0a5e165ca32e1d9ea8eb748d1ee78d4" | ||
perms := "100644" | ||
|
||
res := "" | ||
for _, change := range changes { | ||
line := fmt.Sprintf( | ||
":%[1]s %[1]s %[2]s %[2]s %[3]s\t%[4]s", | ||
perms, | ||
sha, | ||
change.status, | ||
strings.Join(change.files, "\t"), | ||
) | ||
res += line + "\n" | ||
} | ||
|
||
return res | ||
} |
Oops, something went wrong.