From 609a937536ce55130a782b4cedd9473db5d610ef Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Thu, 3 Oct 2024 21:34:24 +0200 Subject: [PATCH] refactor(directives): `git-overwrite` -> `git-clear` (#2643) Signed-off-by: Hidde Beydals --- docs/docs/20-quickstart.md | 27 ++-- internal/directives/git_tree_clearer.go | 87 ++++++++++++ ...riter_test.go => git_tree_clearer_test.go} | 55 +++----- internal/directives/git_tree_overwriter.go | 127 ------------------ .../directives/schemas/git-clear-config.json | 14 ++ .../schemas/git-overwrite-config.json | 19 --- internal/directives/zz_config_types.go | 15 +-- .../utils/promotion-steps-example.ts | 15 +-- .../promotion-directives/registry/types.ts | 2 +- .../registry/use-discover-registries.ts | 2 +- 10 files changed, 143 insertions(+), 220 deletions(-) create mode 100644 internal/directives/git_tree_clearer.go rename internal/directives/{git_tree_overwriter_test.go => git_tree_clearer_test.go} (69%) delete mode 100644 internal/directives/git_tree_overwriter.go create mode 100644 internal/directives/schemas/git-clear-config.json delete mode 100644 internal/directives/schemas/git-overwrite-config.json diff --git a/docs/docs/20-quickstart.md b/docs/docs/20-quickstart.md index ae455e054..98ade1935 100644 --- a/docs/docs/20-quickstart.md +++ b/docs/docs/20-quickstart.md @@ -363,6 +363,9 @@ the previous section. - branch: stage/test create: true path: ./out + - uses: git-clear + config: + path: ./out - uses: kustomize-set-image as: update-image config: @@ -372,11 +375,7 @@ the previous section. - uses: kustomize-build config: path: ./main/stages/test - outPath: ./manifests.yaml - - uses: git-overwrite - config: - inPath: ./manifests.yaml - outPath: ./out + outPath: ./out/manifests.yaml - uses: git-commit as: commit config: @@ -420,6 +419,9 @@ the previous section. - branch: stage/uat create: true path: ./out + - uses: git-clear + config: + path: ./out - uses: kustomize-set-image as: update-image config: @@ -429,11 +431,7 @@ the previous section. - uses: kustomize-build config: path: ./main/stages/test - outPath: ./manifests.yaml - - uses: git-overwrite - config: - inPath: ./manifests.yaml - outPath: ./out + outPath: ./out/manifests.yaml - uses: git-commit as: commit config: @@ -477,6 +475,9 @@ the previous section. - branch: stage/prod create: true path: ./out + - uses: git-clear + config: + path: ./out - uses: kustomize-set-image as: update-image config: @@ -486,11 +487,7 @@ the previous section. - uses: kustomize-build config: path: ./main/stages/test - outPath: ./manifests.yaml - - uses: git-overwrite - config: - inPath: ./manifests.yaml - outPath: ./out + outPath: ./out/manifests.yaml - uses: git-commit as: commit config: diff --git a/internal/directives/git_tree_clearer.go b/internal/directives/git_tree_clearer.go new file mode 100644 index 000000000..97aff2eb9 --- /dev/null +++ b/internal/directives/git_tree_clearer.go @@ -0,0 +1,87 @@ +package directives + +import ( + "context" + "fmt" + + securejoin "github.com/cyphar/filepath-securejoin" + "github.com/xeipuuv/gojsonschema" + + kargoapi "github.com/akuity/kargo/api/v1alpha1" + "github.com/akuity/kargo/internal/controller/git" +) + +func init() { + builtins.RegisterPromotionStepRunner(newGitTreeClearer(), nil) +} + +// gitTreeClearer is an implementation of the PromotionStepRunner interface +// that removes the content of a Git working tree. +type gitTreeClearer struct { + schemaLoader gojsonschema.JSONLoader +} + +// newGitTreeClearer returns an implementation of the PromotionStepRunner +// interface that removes the content of a Git working tree. +func newGitTreeClearer() PromotionStepRunner { + r := &gitTreeClearer{} + r.schemaLoader = getConfigSchemaLoader(r.Name()) + return r +} + +// Name implements the PromotionStepRunner interface. +func (g *gitTreeClearer) Name() string { + return "git-clear" +} + +// RunPromotionStep implements the PromotionStepRunner interface. +func (g *gitTreeClearer) RunPromotionStep( + ctx context.Context, + stepCtx *PromotionStepContext, +) (PromotionStepResult, error) { + if err := g.validate(stepCtx.Config); err != nil { + return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, err + } + cfg, err := configToStruct[GitClearConfig](stepCtx.Config) + if err != nil { + return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, + fmt.Errorf("could not convert config into %s config: %w", g.Name(), err) + } + return g.runPromotionStep(ctx, stepCtx, cfg) +} + +// validate validates gitTreeClearer configuration against a JSON schema. +func (g *gitTreeClearer) validate(cfg Config) error { + return validate(g.schemaLoader, gojsonschema.NewGoLoader(cfg), g.Name()) +} + +func (g *gitTreeClearer) runPromotionStep( + _ context.Context, + stepCtx *PromotionStepContext, + cfg GitClearConfig, +) (PromotionStepResult, error) { + p, err := securejoin.SecureJoin(stepCtx.WorkDir, cfg.Path) + if err != nil { + return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, fmt.Errorf( + "error joining path %s with work dir %s: %w", + cfg.Path, stepCtx.WorkDir, err, + ) + } + workTree, err := git.LoadWorkTree(p, nil) + if err != nil { + return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, + fmt.Errorf("error loading working tree from %s: %w", cfg.Path, err) + } + // workTree.Clear() won't remove any files that aren't indexed. This is a bit + // of a hack to ensure that we don't have any untracked files in the working + // tree so that workTree.Clear() will remove everything. + if err = workTree.AddAll(); err != nil { + return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, + fmt.Errorf("error adding all files to working tree at %s: %w", cfg.Path, err) + } + if err = workTree.Clear(); err != nil { + return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, + fmt.Errorf("error clearing working tree at %s: %w", cfg.Path, err) + } + return PromotionStepResult{Status: kargoapi.PromotionPhaseSucceeded}, nil +} diff --git a/internal/directives/git_tree_overwriter_test.go b/internal/directives/git_tree_clearer_test.go similarity index 69% rename from internal/directives/git_tree_overwriter_test.go rename to internal/directives/git_tree_clearer_test.go index 7e1f85605..6b106e86e 100644 --- a/internal/directives/git_tree_overwriter_test.go +++ b/internal/directives/git_tree_clearer_test.go @@ -22,41 +22,25 @@ func Test_gitTreeOverwriter_validate(t *testing.T) { expectedProblems []string }{ { - name: "inPath not specified", + name: "path not specified", config: Config{}, expectedProblems: []string{ - "(root): inPath is required", + "(root): path is required", }, }, { - name: "inPath is empty string", + name: "path is empty string", config: Config{ - "inPath": "", + "path": "", }, expectedProblems: []string{ - "inPath: String length must be greater than or equal to 1", - }, - }, - { - name: "outPath not specified", - config: Config{}, - expectedProblems: []string{ - "(root): outPath is required", - }, - }, - { - name: "outPath is empty string", - config: Config{ - "outPath": "", - }, - expectedProblems: []string{ - "outPath: String length must be greater than or equal to 1", + "path: String length must be greater than or equal to 1", }, }, } - r := newGitTreeOverwriter() - runner, ok := r.(*gitTreeOverwriter) + r := newGitTreeClearer() + runner, ok := r.(*gitTreeClearer) require.True(t, ok) for _, testCase := range testCases { @@ -101,7 +85,9 @@ func Test_gitTreeOverwriter_runPromotionStep(t *testing.T) { }, ) require.NoError(t, err) - defer repo.Close() + t.Cleanup(func() { + _ = repo.Close() + }) // "master" is still the default branch name for a new repository // unless you configure it otherwise. workTreePath := filepath.Join(workDir, "master") @@ -115,16 +101,9 @@ func Test_gitTreeOverwriter_runPromotionStep(t *testing.T) { err = os.WriteFile(filepath.Join(workTree.Dir(), "original.txt"), []byte("foo"), 0600) require.NoError(t, err) - // Write another file to a different directory. This will be the source - // directory for the gitTreeOverwriter. - srcDir := filepath.Join(workDir, "src") - err = os.Mkdir(srcDir, 0700) - require.NoError(t, err) - err = os.WriteFile(filepath.Join(srcDir, "new.txt"), []byte("bar"), 0600) - require.NoError(t, err) - - r := newGitTreeOverwriter() - runner, ok := r.(*gitTreeOverwriter) + // Run the directive + r := newGitTreeClearer() + runner, ok := r.(*gitTreeClearer) require.True(t, ok) res, err := runner.runPromotionStep( @@ -134,9 +113,8 @@ func Test_gitTreeOverwriter_runPromotionStep(t *testing.T) { Stage: "fake-stage", WorkDir: workDir, }, - GitOverwriteConfig{ - InPath: "src", - OutPath: "master", + GitClearConfig{ + Path: "master", }, ) require.NoError(t, err) @@ -146,9 +124,6 @@ func Test_gitTreeOverwriter_runPromotionStep(t *testing.T) { _, err = os.Stat(filepath.Join(workTree.Dir(), "original.txt")) require.Error(t, err) require.True(t, os.IsNotExist(err)) - // Make sure new files are present - _, err = os.Stat(filepath.Join(workTree.Dir(), "new.txt")) - require.NoError(t, err) // Make sure the .git directory is still there _, err = os.Stat(filepath.Join(workTree.Dir(), ".git")) require.NoError(t, err) diff --git a/internal/directives/git_tree_overwriter.go b/internal/directives/git_tree_overwriter.go deleted file mode 100644 index 677c797ef..000000000 --- a/internal/directives/git_tree_overwriter.go +++ /dev/null @@ -1,127 +0,0 @@ -package directives - -import ( - "context" - "fmt" - "os" - "path/filepath" - - securejoin "github.com/cyphar/filepath-securejoin" - "github.com/otiai10/copy" - "github.com/xeipuuv/gojsonschema" - - kargoapi "github.com/akuity/kargo/api/v1alpha1" - "github.com/akuity/kargo/internal/controller/git" - "github.com/akuity/kargo/internal/logging" -) - -func init() { - builtins.RegisterPromotionStepRunner(newGitTreeOverwriter(), nil) -} - -// gitTreeOverwriter is an implementation of the PromotionStepRunner interface -// that overwrites the content of a Git working tree with the content from -// another directory. -type gitTreeOverwriter struct { - schemaLoader gojsonschema.JSONLoader -} - -// newGitTreeOverwriter returns an implementation of the PromotionStepRunner -// interface that overwrites the content of a Git working tree with the content -// from another directory. -func newGitTreeOverwriter() PromotionStepRunner { - r := &gitTreeOverwriter{} - r.schemaLoader = getConfigSchemaLoader(r.Name()) - return r -} - -// Name implements the PromotionStepRunner interface. -func (g *gitTreeOverwriter) Name() string { - return "git-overwrite" -} - -// RunPromotionStep implements the PromotionStepRunner interface. -func (g *gitTreeOverwriter) RunPromotionStep( - ctx context.Context, - stepCtx *PromotionStepContext, -) (PromotionStepResult, error) { - if err := g.validate(stepCtx.Config); err != nil { - return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, err - } - cfg, err := configToStruct[GitOverwriteConfig](stepCtx.Config) - if err != nil { - return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, - fmt.Errorf("could not convert config into %s config: %w", g.Name(), err) - } - return g.runPromotionStep(ctx, stepCtx, cfg) -} - -// validate validates gitTreeOverwriter configuration against a JSON schema. -func (g *gitTreeOverwriter) validate(cfg Config) error { - return validate(g.schemaLoader, gojsonschema.NewGoLoader(cfg), g.Name()) -} - -func (g *gitTreeOverwriter) runPromotionStep( - ctx context.Context, - stepCtx *PromotionStepContext, - cfg GitOverwriteConfig, -) (PromotionStepResult, error) { - inPath, err := securejoin.SecureJoin(stepCtx.WorkDir, cfg.InPath) - if err != nil { - return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, fmt.Errorf( - "error joining path %s with work dir %s: %w", - cfg.InPath, stepCtx.WorkDir, err, - ) - } - outPath, err := securejoin.SecureJoin(stepCtx.WorkDir, cfg.OutPath) - if err != nil { - return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, fmt.Errorf( - "error joining path %s with work dir %s: %w", - cfg.OutPath, stepCtx.WorkDir, err, - ) - } - workTree, err := git.LoadWorkTree(outPath, nil) - if err != nil { - return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, - fmt.Errorf("error loading working tree from %s: %w", cfg.OutPath, err) - } - // workTree.Clear() won't remove any files that aren't indexed. This is a bit - // of a hack to ensure that we don't have any untracked files in the working - // tree so that workTree.Clear() will remove everything. - if err = workTree.AddAll(); err != nil { - return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, - fmt.Errorf("error adding all files to working tree at %s: %w", cfg.OutPath, err) - } - if err = workTree.Clear(); err != nil { - return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, - fmt.Errorf("error clearing working tree at %s: %w", cfg.OutPath, err) - } - inFI, err := os.Stat(inPath) - if err != nil { - return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, - fmt.Errorf("error getting info for path %s: %w", inPath, err) - } - if !inFI.IsDir() { - outPath = filepath.Join(outPath, inFI.Name()) - } - if err = copy.Copy( - inPath, - outPath, - copy.Options{ - Skip: func(_ os.FileInfo, src, _ string) (bool, error) { - return src == filepath.Join(inPath, ".git"), nil - }, - OnSymlink: func(src string) copy.SymlinkAction { - logging.LoggerFromContext(ctx).Trace("ignoring symlink", "src", src) - return copy.Skip - }, - OnError: func(_, _ string, err error) error { - return sanitizePathError(err, stepCtx.WorkDir) - }, - }, - ); err != nil { - return PromotionStepResult{Status: kargoapi.PromotionPhaseErrored}, - fmt.Errorf("failed to copy %q to %q: %w", cfg.InPath, cfg.OutPath, err) - } - return PromotionStepResult{Status: kargoapi.PromotionPhaseSucceeded}, nil -} diff --git a/internal/directives/schemas/git-clear-config.json b/internal/directives/schemas/git-clear-config.json new file mode 100644 index 000000000..d52706ac5 --- /dev/null +++ b/internal/directives/schemas/git-clear-config.json @@ -0,0 +1,14 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "GitClearConfig", + "type": "object", + "additionalProperties": false, + "required": ["path"], + "properties": { + "path": { + "type": "string", + "description": "Path to a working directory of a local repository from which to remove all files, excluding the .git/ directory.", + "minLength": 1 + } + } +} diff --git a/internal/directives/schemas/git-overwrite-config.json b/internal/directives/schemas/git-overwrite-config.json deleted file mode 100644 index df9ed6976..000000000 --- a/internal/directives/schemas/git-overwrite-config.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "$schema": "https://json-schema.org/draft/2020-12/schema", - "title": "GitOverwriteConfig", - "type": "object", - "additionalProperties": false, - "required": ["inPath", "outPath"], - "properties": { - "inPath": { - "type": "string", - "description": "A path to a directory from which to copy all contents, excluding the .git/ directory, if one exists.", - "minLength": 1 - }, - "outPath": { - "type": "string", - "description": "A path to a git working tree which will be cleared of all existing content before receiving a copy of all content specified by inPath.", - "minLength": 1 - } - } -} diff --git a/internal/directives/zz_config_types.go b/internal/directives/zz_config_types.go index 207822ae7..0ede47bd3 100644 --- a/internal/directives/zz_config_types.go +++ b/internal/directives/zz_config_types.go @@ -103,6 +103,12 @@ type CopyConfig struct { OutPath string `json:"outPath"` } +type GitClearConfig struct { + // Path to a working directory of a local repository from which to remove all files, + // excluding the .git/ directory. + Path string `json:"path"` +} + type GitCloneConfig struct { // The commits, branches, or tags to check out from the repository and the paths where they // should be checked out. At least one must be specified. @@ -180,15 +186,6 @@ type GitOpenPRConfig struct { TargetBranch string `json:"targetBranch"` } -type GitOverwriteConfig struct { - // A path to a directory from which to copy all contents, excluding the .git/ directory, if - // one exists. - InPath string `json:"inPath"` - // A path to a git working tree which will be cleared of all existing content before - // receiving a copy of all content specified by inPath. - OutPath string `json:"outPath"` -} - type GitPushConfig struct { // Indicates whether to push to a new remote branch. A value of 'true' is mutually exclusive // with 'targetBranch'. If neither of these is provided, the target branch will be the diff --git a/ui/src/features/project/pipelines/utils/promotion-steps-example.ts b/ui/src/features/project/pipelines/utils/promotion-steps-example.ts index 78db4dc4d..17cdfe1a7 100644 --- a/ui/src/features/project/pipelines/utils/promotion-steps-example.ts +++ b/ui/src/features/project/pipelines/utils/promotion-steps-example.ts @@ -2,12 +2,11 @@ export const promoStepsExample = `- uses: git-clone config: repoURL: https://github.com//kargo-demo-gitops.git checkout: - - fromFreight: true - path: ./main - - branch: 09/stage/test - create: true - path: ./out -- uses: git-overwrite + - fromFreight: true + path: ./main + - branch: 09/stage/test + create: true + path: ./out +- uses: git-clear config: - inPath: ./main - outPath: ./out`; + path: ./out`; diff --git a/ui/src/features/promotion-directives/registry/types.ts b/ui/src/features/promotion-directives/registry/types.ts index 12e7520f6..2371d99f2 100644 --- a/ui/src/features/promotion-directives/registry/types.ts +++ b/ui/src/features/promotion-directives/registry/types.ts @@ -10,7 +10,7 @@ export type PromotionDirectivesRegistry = { // runner is source of truth for all configuration and metadata related to installed runner export type Runner = { // unique identifier such that kargo knows which runner to operate - // example - git-clone, git-overwrite + // example - git-clone, git-clear identifier: string; // UI helper // this accepts font-awesome icon diff --git a/ui/src/features/promotion-directives/registry/use-discover-registries.ts b/ui/src/features/promotion-directives/registry/use-discover-registries.ts index dd1693581..b10c16f2e 100644 --- a/ui/src/features/promotion-directives/registry/use-discover-registries.ts +++ b/ui/src/features/promotion-directives/registry/use-discover-registries.ts @@ -57,7 +57,7 @@ export const useDiscoverPromotionDirectivesRegistries = (): PromotionDirectivesR unstable_icons: [faArrowUp, faCloudUploadAlt] }, { - identifier: 'git-overwrite', + identifier: 'git-clear', unstable_icons: [faRedoAlt, faCodeBranch] }, {