From 34b5b5b2fff31c12b4503997f800beea85dfa374 Mon Sep 17 00:00:00 2001 From: Vault Automation Date: Thu, 5 Feb 2026 17:37:08 -0500 Subject: [PATCH] [VAULT-39994] pipeline(changed-files): add support for listing and checking changed files (#12127) (#12215) We've already deployed some changed file detection in the CI pipeline. It uses the Github API to fetch a list of all changed files on a PR and then run it through a simple groups categorization pass. It's been a useful strategy in the context of a Pull Request because it does not depend on the local state of the Git repo. This commit introduces a local git-based file change detection and validation system for the pipeline tool, enabling developers to identify and validate changed files before pushing code. We intend to use the new tool in two primary ways: - As a Git pre-push hook when pushing new or updated branches. (Implemented here) - As part of the scheduled automated repository synchronization. (Up next, and it will use the same `git.CheckChangedFilesReq{}` implementation. This will allow us to guard all pushes to `hashicorp/vault` and `ce/*` branches in `hashicorp/vault-enterprise`, whether run locally on a developer machine or in CI by our service user. We introduce two new `pipeline` CLI commands: - `pipeline git list changed-files` - `pipeline git check changed-files` Both support specifying what method of git inspection we want to use for the changed files list: - **`--branch `**: Lists all files added in the entire history of a specific branch. We use this when pushing a _new_ branch. - **`--range `**: Lists all changed files within a commit range (e.g., `HEAD~5..HEAD`). We use this when updating an existing branch. - **`--commit `**: Lists all changed files in a specific commit (using `git show`). This isn't actually used at all in the pre-push hook but it useful if you wish to inspect a single commit on your branch. The behavior when passing the `range` and `commit` is similar. We inspect the changed file list either for one or many commits (but with slightly different implementations for efficiency and accuracy. The `branch` option is a bit different. We use it to inspect the branches entire history of changed files for enterprise files before pushing a new branch. We do this to ensure that our branch doesn't accidentally add and then subsequently remove enterprise files, leaving the contents in the history but nothing obvious in the diff. Each command supports several different output formats. The default is the human readable text table, though `--format json` will write all of the details as valid JSON to STDOUT. When given the `--github-output` command each will write a more concise version of the JSON output to `$GITHUB_OUTPUT`. It differs from our standard JSON output as it has been formatted to be easier to use in Github Actions contexts without requiring complex filtering. When run, changed files are automatically categorized into logical groups based on their file name, just like our existing changed file detection. A follow-up to this PR will introduce a configuration based system for classifying file groups. This will allow us to create generic support for changed file detection so that many repositories can adopt this pattern. The major difference in behavior between the two new commands is that the `list` command will always list the changed files for the given method/target, while the `check` command requires one-or-more changed file groups that we want to disallow to be included via the `-g` flag. If any changed files match the given group(s) then the command will fail. That allows us to specify the `enterprise` group and disallow the command to succeed if any of the changed files match the group. The pre-push git hook now uses this system to prevent accidental pushes, however, it requires the local machine to have the `pipeline` tool in the `$PATH`. This ought not be much of a requirement as a working Go toolchain is required for any Vault developer. When it is not present we explain in our error messages how to resolve the problem and direct them to our slack channel if they need further assistance. Signed-off-by: Ryan Cragun Co-authored-by: Ryan Cragun --- .gitignore | 12 + .hooks/pre-push | 148 ++++- tools/pipeline/internal/cmd/git.go | 19 + tools/pipeline/internal/cmd/git_check.go | 20 + .../internal/cmd/git_check_changed_files.go | 67 ++ tools/pipeline/internal/cmd/git_list.go | 20 + .../internal/cmd/git_list_changed_files.go | 63 ++ tools/pipeline/internal/cmd/github.go | 2 +- tools/pipeline/internal/cmd/root.go | 1 + .../pipeline/internal/pkg/changed/checkers.go | 25 + .../internal/pkg/changed/checkers_test.go | 23 +- tools/pipeline/internal/pkg/changed/file.go | 34 +- .../internal/pkg/changed/files_test.go | 614 ++++++++++++++++++ .../internal/pkg/git/check_changed_files.go | 189 ++++++ .../internal/pkg/git/{ => client}/am.go | 2 +- .../internal/pkg/git/{ => client}/apply.go | 2 +- .../internal/pkg/git/{ => client}/branch.go | 2 +- .../internal/pkg/git/{ => client}/checkout.go | 2 +- .../pkg/git/{ => client}/cherry-pick.go | 2 +- .../internal/pkg/git/{ => client}/client.go | 2 +- .../internal/pkg/git/{ => client}/clone.go | 2 +- .../internal/pkg/git/{ => client}/commit.go | 2 +- tools/pipeline/internal/pkg/git/client/doc.go | 5 + .../internal/pkg/git/{ => client}/fetch.go | 2 +- tools/pipeline/internal/pkg/git/client/log.go | 407 ++++++++++++ .../internal/pkg/git/{ => client}/merge.go | 2 +- .../pkg/git/{ => client}/opts_test.go | 239 ++++++- .../internal/pkg/git/{ => client}/pull.go | 2 +- .../internal/pkg/git/{ => client}/push.go | 2 +- .../internal/pkg/git/{ => client}/rebase.go | 2 +- .../internal/pkg/git/{ => client}/remote.go | 2 +- .../internal/pkg/git/{ => client}/reset.go | 2 +- .../internal/pkg/git/{ => client}/rm.go | 2 +- .../internal/pkg/git/{ => client}/show.go | 16 +- .../internal/pkg/git/{ => client}/status.go | 2 +- tools/pipeline/internal/pkg/git/doc.go | 5 - .../internal/pkg/git/list_changed_files.go | 260 ++++++++ .../pkg/github/check_go_mod_diff_request.go | 2 +- .../internal/pkg/github/copy_pull_request.go | 2 +- .../internal/pkg/github/create_backport.go | 2 +- .../pkg/github/create_backport_test.go | 14 +- .../internal/pkg/github/list_changed_files.go | 14 +- .../internal/pkg/github/noop_commit.go | 2 +- tools/pipeline/internal/pkg/github/repo.go | 2 +- .../pkg/github/sync_branch_request.go | 2 +- 45 files changed, 2164 insertions(+), 79 deletions(-) create mode 100644 tools/pipeline/internal/cmd/git.go create mode 100644 tools/pipeline/internal/cmd/git_check.go create mode 100644 tools/pipeline/internal/cmd/git_check_changed_files.go create mode 100644 tools/pipeline/internal/cmd/git_list.go create mode 100644 tools/pipeline/internal/cmd/git_list_changed_files.go create mode 100644 tools/pipeline/internal/pkg/changed/files_test.go create mode 100644 tools/pipeline/internal/pkg/git/check_changed_files.go rename tools/pipeline/internal/pkg/git/{ => client}/am.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/apply.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/branch.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/checkout.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/cherry-pick.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/client.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/clone.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/commit.go (99%) create mode 100644 tools/pipeline/internal/pkg/git/client/doc.go rename tools/pipeline/internal/pkg/git/{ => client}/fetch.go (99%) create mode 100644 tools/pipeline/internal/pkg/git/client/log.go rename tools/pipeline/internal/pkg/git/{ => client}/merge.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/opts_test.go (77%) rename tools/pipeline/internal/pkg/git/{ => client}/pull.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/push.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/rebase.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/remote.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/reset.go (99%) rename tools/pipeline/internal/pkg/git/{ => client}/rm.go (98%) rename tools/pipeline/internal/pkg/git/{ => client}/show.go (88%) rename tools/pipeline/internal/pkg/git/{ => client}/status.go (99%) delete mode 100644 tools/pipeline/internal/pkg/git/doc.go create mode 100644 tools/pipeline/internal/pkg/git/list_changed_files.go diff --git a/.gitignore b/.gitignore index fc59cf7fa8..1ff45dee12 100644 --- a/.gitignore +++ b/.gitignore @@ -136,3 +136,15 @@ tools/codechecker/.bin tools/pipeline/.bin tools/pipeline/pipeline .ci-bootstrap + +# Ignore stubmaker outputs on ent so they don't get checked in +*_stubs_ent.go + +# scratch directory holding random stuff +scratch + +# ignore Idea/Goland .run directory +.run + +# bob +.bob/notes diff --git a/.hooks/pre-push b/.hooks/pre-push index e760921281..433eea3c86 100755 --- a/.hooks/pre-push +++ b/.hooks/pre-push @@ -1,23 +1,139 @@ #!/usr/bin/env bash -remote="$1" -remote_url=$(git remote get-url $remote) +# A pre-push hook to verify that no enterprise files exist in the change +# set that we intend to push. This script will exit non-zero if we detect +# any enterprise files destined for either hashicorp/vault, or any ce/* +# branches in hashicorp/vault-enterprise. +# +# NOTE: If you're reading this because you're running into issues, feel free to +# reach out to #team-vault-automation if you need further assistance or if you +# believe you've found an issue. Before you reach out, please make sure that +# you've updated the pipeline tool (make tools-pipeline) and have your $GOBIN +# in the path. Thanks! +# +# NOTE: If you're debugging this hook and would like include additional information +# you can do any of the following: +# +# - Use `set -x` to see all commands that are run. +# - Add `--log debug` to any `pipeline` commands that are being run. +# - Use `pipeline git list changed-files --range (git merge-base HEAD main)..HEAD` +# to see the changed files on your branch. Replace 'main' with whatever branch +# you intend to merge into. Use the --branch flag in place of --range +# if you're pushing a new branch. +# - Use `pipeline git check changed-files --range (git merge-base HEAD main)..HEAD -b enterprise` +# to see the changed files on your branch that we detect as enterprise files. +# Replace 'main' with whatever branch you intend to merge into. Use the +# --branch flag in place of --range if you're pushing a new branch. +# +# Git calls this hook with the following parameters: +# +# $1 -- Name of the remote to which the push is being done +# $2 -- URL to which the push is being done +# +# If pushing without using a named remote those arguments will be equal. +# +# Information about the commits which are being pushed are written to STDIN by +# git in the form of: +# +# +# +# We use these values to determine what type of push operation is happening and +# what commits are going to be pushed. Depending on the operation and commit +# we employ slightly different methods to determine the changed file list. The +# `pipeline git check changed-files` utility will handle gathering, grouping, +# and checking for disallowed changed files. -if [[ $remote_url == *"vault-enterprise"* ]]; then - exit 0 -fi +set -eou pipefail -if [ "$remote" = "enterprise" ]; then - exit 0 -fi +# fail takes two arguments. The first is the failure reasons and the second is +# an explanation. Both will be written to STDERR. The script will then exit 1. +fail() { + if test -t 1; then + printf '\x1b[1;31;49m%s\x1b[0m\n%s\n' "$1" "$2" >&2 + else + printf "%s\n%s\n" "$1" "$2" >&2 + fi + exit 1 +} -if [ "$remote" = "ent" ]; then - exit 0 -fi +# remote_url_is_enterprise returns 0 if the given URL argument matches +# that of hashicorp/vault-enterprise, otherwise it returns 1. +remote_url_is_enterprise() { + case "$1" in + git@github.com:hashicorp/vault-enterprise.git | https://github.com/hashicorp/vault-enterprise.git) + return 0 + ;; + *) + return 1 + ;; + esac +} -if [ -f command/version_ent.go ]; then - echo "Found enterprise version file while pushing to oss remote" - exit 1 -fi +# remote_ref_is_ce returns 0 if the given git ref argument matches a ce/* branch, +# otherwise it returns 1. +remote_ref_is_ce() { + if [[ "$1" == "refs/heads/ce/"* ]]; then + return 0 + fi -exit 0 + return 1 +} + +# main is our main function. It currently takes a single argument that is the +# URL of the remote that we're pushing to. +main() { + # Git writes the commit information to STDIN. Read it into our local variables + # and then determine how to verify the changed files. + local _local_ref + local local_oid + local remote_ref + local remote_oid + while read -r _local_ref local_oid remote_ref remote_oid; do + if remote_url_is_enterprise "$1" && ! remote_ref_is_ce "${remote_ref}"; then + # If we're pushing to vault-enterprise and the branch is not ce/* then + # we don't need to enforce the no enterprise files policy. + exit 0 + fi + + local output + if ! output=$(builtin type pipeline 2>&1); then + fail "Unable to locate the 'pipeline' binary your \$PATH" "Please make sure you have the latest pipeline tool installed and that your '\$GOBIN' is in your '\$PATH'. You can use 'make tools-pipeline' to build and install the latest version of the 'pipeline' tool. The tool is required to check your branch's changed files before pushing to either 'hashicorp/vault' or 'ce/*\' branches in 'hashicorp/vault-enterprise'. Reach out to #team-vault-automation') for help! ${output}" + fi + + # Determine our "zero" object ID. + local zero + zero=$(git hash-object --stdin < /dev/null | tr '0-9a-f' '0') + + if test "${local_oid}" = "${zero}"; then + # Our local object ID is zero, that means we're deleting the remote branch. + # We can safely ignore the "changes" on the local branch because we're not + # pushing anything. + exit 0 + else + # We're either creating a new branch or updating an exiting one. + if test "${remote_oid}" = "${zero}"; then + # We're pushing a new branch. Check the entire branch history for + # changed enterprise files. We'll swallow the output by default and only + # show it if there are enterprise files. + if ! output=$(pipeline git check changed-files --branch "${local_oid}" -g enterprise 2>&1); then + fail "Cannot push changes to a new community edition branch. Please make sure no enterprise files are included in your branch history" "${output}" + fi + exit 0 + else + # We're updating an existing branch. Check only the new commit history + # for changed enterprise files. We'll swallow the output by default and + # only show it if there are enterprise files. + if ! output=$(pipeline git check changed-files --range "${remote_oid}..${local_oid}" -g enterprise 2>&1); then + fail "Cannot push updates to community edition branch. Please make sure no enterprise files are included in your change set" "${output}" + fi + exit 0 + fi + fi + done + + fail "git pre-push hook failed!" "git did not write expected commit information to STDIN! Please reach out to #team-vault-automation for help!" +} + +# Call the main function. We currently only care about the URL so we'll pass in +# $2 as the only argument. +main "$2" diff --git a/tools/pipeline/internal/cmd/git.go b/tools/pipeline/internal/cmd/git.go new file mode 100644 index 0000000000..3aec7dfdc1 --- /dev/null +++ b/tools/pipeline/internal/cmd/git.go @@ -0,0 +1,19 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package cmd + +import "github.com/spf13/cobra" + +func newGitCmd() *cobra.Command { + gitCmd := &cobra.Command{ + Use: "git", + Short: "Git commands", + Long: "Git commands", + } + + gitCmd.AddCommand(newGitCheckCmd()) + gitCmd.AddCommand(newGitListCmd()) + + return gitCmd +} diff --git a/tools/pipeline/internal/cmd/git_check.go b/tools/pipeline/internal/cmd/git_check.go new file mode 100644 index 0000000000..14fa5cdcb7 --- /dev/null +++ b/tools/pipeline/internal/cmd/git_check.go @@ -0,0 +1,20 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package cmd + +import ( + "github.com/spf13/cobra" +) + +func newGitCheckCmd() *cobra.Command { + checkCmd := &cobra.Command{ + Use: "check", + Short: "Git check commands", + Long: "Git check commands", + } + + checkCmd.AddCommand(newGitCheckChangedFilesCmd()) + + return checkCmd +} diff --git a/tools/pipeline/internal/cmd/git_check_changed_files.go b/tools/pipeline/internal/cmd/git_check_changed_files.go new file mode 100644 index 0000000000..2b1e755d8f --- /dev/null +++ b/tools/pipeline/internal/cmd/git_check_changed_files.go @@ -0,0 +1,67 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package cmd + +import ( + "context" + "fmt" + + "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git" + "github.com/spf13/cobra" +) + +var checkGitChangedFiles = &git.CheckChangedFilesReq{} + +func newGitCheckChangedFilesCmd() *cobra.Command { + changedFilesCmd := &cobra.Command{ + Use: "changed-files [--branch | --range | --commit ] --group ...", + Short: "Check if any changed files are matching disallowed groups", + Long: "Check if any changed files are matching disallowed groups", + RunE: runGitCheckChangedFilesCmd, + Args: cobra.NoArgs, + } + + changedFilesCmd.PersistentFlags().StringVarP(&checkGitChangedFiles.Branch, "branch", "b", "", "The branch to compare against") + changedFilesCmd.PersistentFlags().StringVarP(&checkGitChangedFiles.Range, "range", "r", "", "The commit range to compare (e.g., HEAD~5..HEAD)") + changedFilesCmd.PersistentFlags().StringVarP(&checkGitChangedFiles.Commit, "commit", "c", "", "The specific commit SHA to analyze") + changedFilesCmd.PersistentFlags().StringSliceVarP(&checkGitChangedFiles.CheckGroups, "groups", "g", nil, "File group(s) to check changed files for") + changedFilesCmd.PersistentFlags().BoolVar(&checkGitChangedFiles.WriteToGithubOutput, "github-output", false, "Whether or not to write 'changed-files' to $GITHUB_OUTPUT") + + return changedFilesCmd +} + +func runGitCheckChangedFilesCmd(cmd *cobra.Command, args []string) error { + cmd.SilenceUsage = true // Don't spam the usage on failure + + res, err := checkGitChangedFiles.Run(context.TODO(), githubCmdState.Git) + if err != nil { + return fmt.Errorf("checking changed files: %w", err) + } + + switch rootCfg.format { + case "json": + b, err := res.ToJSON() + if err != nil { + return err + } + fmt.Println(string(b)) + default: + fmt.Println(res.ToTable()) + } + + if checkGitChangedFiles.WriteToGithubOutput { + output, err := res.ToGithubOutput() + if err != nil { + return err + } + + return writeToGithubOutput("changed-files", output) + } + + if len(res.MatchedGroups) > 0 { + return fmt.Errorf("one-or-more changed files matched disallowed groups: %s", res.MatchedGroups.String()) + } + + return nil +} diff --git a/tools/pipeline/internal/cmd/git_list.go b/tools/pipeline/internal/cmd/git_list.go new file mode 100644 index 0000000000..c707dfc448 --- /dev/null +++ b/tools/pipeline/internal/cmd/git_list.go @@ -0,0 +1,20 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package cmd + +import ( + "github.com/spf13/cobra" +) + +func newGitListCmd() *cobra.Command { + listCmd := &cobra.Command{ + Use: "list", + Short: "Git list commands", + Long: "Git list commands", + } + + listCmd.AddCommand(newGitListChangedFilesCmd()) + + return listCmd +} diff --git a/tools/pipeline/internal/cmd/git_list_changed_files.go b/tools/pipeline/internal/cmd/git_list_changed_files.go new file mode 100644 index 0000000000..b5172d1910 --- /dev/null +++ b/tools/pipeline/internal/cmd/git_list_changed_files.go @@ -0,0 +1,63 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package cmd + +import ( + "context" + "fmt" + + "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git" + "github.com/spf13/cobra" +) + +var listGitChangedFiles = &git.ListChangedFilesReq{} + +func newGitListChangedFilesCmd() *cobra.Command { + changedFilesCmd := &cobra.Command{ + Use: "changed-files [--branch | --range | --commit ]", + Short: "List changed files using git", + Long: "List changed files using git by specifying a branch, range, or commit", + RunE: runGitListChangedFilesCmd, + Args: cobra.NoArgs, + } + + changedFilesCmd.PersistentFlags().StringVarP(&listGitChangedFiles.Branch, "branch", "b", "", "The branch to compare against") + changedFilesCmd.PersistentFlags().StringVarP(&listGitChangedFiles.Range, "range", "r", "", "The commit range to compare (e.g., HEAD~5..HEAD)") + changedFilesCmd.PersistentFlags().StringVarP(&listGitChangedFiles.Commit, "commit", "c", "", "The specific commit SHA to analyze") + changedFilesCmd.PersistentFlags().BoolVarP(&listGitChangedFiles.GroupFiles, "group", "g", true, "Whether or not to determine changed file groups") + changedFilesCmd.PersistentFlags().BoolVar(&listGitChangedFiles.WriteToGithubOutput, "github-output", false, "Whether or not to write 'changed-files' to $GITHUB_OUTPUT") + + return changedFilesCmd +} + +func runGitListChangedFilesCmd(cmd *cobra.Command, args []string) error { + cmd.SilenceUsage = true // Don't spam the usage on failure + + res, err := listGitChangedFiles.Run(context.TODO(), githubCmdState.Git) + if err != nil { + return fmt.Errorf("listing changed files: %w", err) + } + + switch rootCfg.format { + case "json": + b, err := res.ToJSON() + if err != nil { + return err + } + fmt.Println(string(b)) + default: + fmt.Println(res.ToTable(listGithubChangedFiles.GroupFiles)) + } + + if listGitChangedFiles.WriteToGithubOutput { + output, err := res.ToGithubOutput() + if err != nil { + return err + } + + return writeToGithubOutput("changed-files", output) + } + + return nil +} diff --git a/tools/pipeline/internal/cmd/github.go b/tools/pipeline/internal/cmd/github.go index 171a5379de..9efeb38ab9 100644 --- a/tools/pipeline/internal/cmd/github.go +++ b/tools/pipeline/internal/cmd/github.go @@ -10,7 +10,7 @@ import ( "path/filepath" "github.com/google/go-github/v81/github" - "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git" + git "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git/client" "github.com/shurcooL/githubv4" "github.com/spf13/cobra" "golang.org/x/oauth2" diff --git a/tools/pipeline/internal/cmd/root.go b/tools/pipeline/internal/cmd/root.go index 86b5b586e5..20390af50c 100644 --- a/tools/pipeline/internal/cmd/root.go +++ b/tools/pipeline/internal/cmd/root.go @@ -31,6 +31,7 @@ func newRootCmd() *cobra.Command { rootCmd.PersistentFlags().StringVarP(&rootCfg.format, "format", "f", "table", "The output format. Can be 'json', 'table', and sometimes 'markdown'") rootCmd.AddCommand(newGenerateCmd()) + rootCmd.AddCommand(newGitCmd()) rootCmd.AddCommand(newGithubCmd()) rootCmd.AddCommand(newGoCmd()) rootCmd.AddCommand(newHCPCmd()) diff --git a/tools/pipeline/internal/pkg/changed/checkers.go b/tools/pipeline/internal/pkg/changed/checkers.go index 29a28c348d..0fc6b34603 100644 --- a/tools/pipeline/internal/pkg/changed/checkers.go +++ b/tools/pipeline/internal/pkg/changed/checkers.go @@ -50,6 +50,18 @@ func GroupFiles(ctx context.Context, files []*File, checkers ...FileGroupCheck) } } +// Groups takes a set Files and returns a set of all all FileGroups +func Groups(files Files) FileGroups { + groups := FileGroups{} + for _, file := range files { + for _, group := range file.Groups { + groups = groups.Add(group) + } + } + + return groups +} + // FileGroupCheckerApp is a file group checker that groups based on the file being part of the Vault // Go app func FileGroupCheckerApp(ctx context.Context, file *File) FileGroups { @@ -155,6 +167,18 @@ func FileGroupCheckerEnos(ctx context.Context, file *File) FileGroups { func FileGroupCheckerEnterprise(ctx context.Context, file *File) FileGroups { name := file.Name() + // Explicit exceptions. When we implement the pipeline.hcl config syntax for + // changed files we'll need to add these here or change them to something + // that doesn't trip the rest of our criteria. + switch { + case hasBaseDir(name, "website"): // ignore the website entirely + return nil + case name == ".github/workflows/backport-automation-ent.yml": + return nil + case name == ".github/workflows/build-artifacts-ent.yml": + return nil + } + // Base directory checks switch { case @@ -249,6 +273,7 @@ func FileGroupCheckerPipeline(ctx context.Context, file *File) FileGroups { hasBaseDir(name, filepath.Join(".github", "workflows")), hasBaseDir(name, filepath.Join(".github", "actions")), hasBaseDir(name, filepath.Join(".github", "scripts")), + hasBaseDir(name, ".hooks"), hasBaseDir(name, ".release"), hasBaseDir(name, "scripts"), hasBaseDir(name, filepath.Join("tools", "pipeline")), diff --git a/tools/pipeline/internal/pkg/changed/checkers_test.go b/tools/pipeline/internal/pkg/changed/checkers_test.go index a1f6883da9..f0f91c0303 100644 --- a/tools/pipeline/internal/pkg/changed/checkers_test.go +++ b/tools/pipeline/internal/pkg/changed/checkers_test.go @@ -15,15 +15,17 @@ func TestFileGroupDefaultCheckers(t *testing.T) { t.Parallel() for filename, groups := range map[string]FileGroups{ - ".build/entrypoint.sh": {FileGroupPipeline}, - ".github/actions/changed-files/actions.yml": {FileGroupGithub, FileGroupPipeline}, - ".github/workflows/build.yml": {FileGroupGithub, FileGroupPipeline}, - ".github/workflows/build-artifacts-ce.yml": {FileGroupCommunity, FileGroupGithub, FileGroupPipeline}, - ".github/workflows/build-artifacts-ent.yml": {FileGroupEnterprise, FileGroupGithub, FileGroupPipeline}, + ".build/entrypoint.sh": {FileGroupPipeline}, + ".github/actions/changed-files/actions.yml": {FileGroupGithub, FileGroupPipeline}, + ".github/workflows/build.yml": {FileGroupGithub, FileGroupPipeline}, + ".github/workflows/build-artifacts-ce.yml": {FileGroupCommunity, FileGroupGithub, FileGroupPipeline}, + // TODO: Remove this when we've dealt with exceptions + // ".github/workflows/build-artifacts-ent.yml": {FileGroupEnterprise, FileGroupGithub, FileGroupPipeline}, ".github/workflows/backport-ce-ent.yml": {FileGroupCommunity, FileGroupGithub, FileGroupPipeline}, ".github/scripts/pr_comment.sh": {FileGroupGithub, FileGroupPipeline}, ".github/CODEOWNERS": {FileGroupGithub}, ".go-version": {FileGroupGoToolchain}, + ".hooks/pre-push": {FileGroupPipeline}, ".release/ibm-pao/eboms/5900-BJ8.essentials.csv": {FileGroupEnterprise, FileGroupPipeline}, ".release/docker/ubi-docker-entrypoint.sh": {FileGroupPipeline}, "audit/backend_ce.go": {FileGroupGoApp, FileGroupCommunity}, @@ -79,9 +81,16 @@ func TestFileGroupDefaultCheckers(t *testing.T) { "Makefile": {FileGroupPipeline}, "README.md": {FileGroupDocs}, } { - t.Run(filename, func(t *testing.T) { + t.Run("file name only: "+filename, func(t *testing.T) { t.Parallel() - file := &File{File: &github.CommitFile{Filename: &filename}} + file := &File{Filename: filename} + Group(context.Background(), file, DefaultFileGroupCheckers...) + require.Equal(t, groups, file.Groups) + }) + + t.Run("github file: "+filename, func(t *testing.T) { + t.Parallel() + file := &File{GithubCommitFile: &github.CommitFile{Filename: &filename}} Group(context.Background(), file, DefaultFileGroupCheckers...) require.Equal(t, groups, file.Groups) }) diff --git a/tools/pipeline/internal/pkg/changed/file.go b/tools/pipeline/internal/pkg/changed/file.go index 7b9a726ede..881aa05687 100644 --- a/tools/pipeline/internal/pkg/changed/file.go +++ b/tools/pipeline/internal/pkg/changed/file.go @@ -14,8 +14,12 @@ import ( type ( // File is a changed file in a PR or commit File struct { - File *gh.CommitFile `json:"file,omitempty"` - Groups FileGroups `json:"groups,omitempty"` + // When listing changed files directly from Github we'll have a commit file + GithubCommitFile *gh.CommitFile `json:"github_commit_file,omitempty"` + // When getting changed files directly from git we currently only use the filename + Filename string `json:"file,omitempty"` + // Group are any changed file groups that are associated with the file + Groups FileGroups `json:"groups,omitempty"` } // Files is a slice of changed files in a PR or commit Files []*File @@ -44,11 +48,15 @@ const ( // Name is the file name of the changed file func (f *File) Name() string { - if f == nil || f.File == nil { + if f == nil { return "" } - return f.File.GetFilename() + if f.GithubCommitFile != nil { + return f.GithubCommitFile.GetFilename() + } + + return f.Filename } // Add takes a variadic set of groups and adds them to the ordered set of groups @@ -94,6 +102,24 @@ func (g FileGroups) Any(groups FileGroups) bool { return false } +// Intersection takes another FileGroups and returns a new FileGroups containing only +// the groups that are present in both FileGroups. If there is no intersection, the +// result will be empty. +func (g FileGroups) Intersection(groups FileGroups) FileGroups { + if g == nil || groups == nil { + return FileGroups{} + } + + result := FileGroups{} + for _, group := range g { + if _, in := groups.In(group); in { + result = append(result, group) + } + } + + return result +} + // Groups returns the FileGroups as a slice of strings func (g FileGroups) Groups() []string { groups := []string{} diff --git a/tools/pipeline/internal/pkg/changed/files_test.go b/tools/pipeline/internal/pkg/changed/files_test.go new file mode 100644 index 0000000000..e4bc7a6266 --- /dev/null +++ b/tools/pipeline/internal/pkg/changed/files_test.go @@ -0,0 +1,614 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package changed + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestFileGroups_Add tests the Add method which adds groups to a FileGroups set +// while maintaining uniqueness and sorted order. Tests cover nil/empty receivers, +// single/multiple additions, duplicates, and sorting behavior. +func TestFileGroups_Add(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + groups FileGroups + add []FileGroup + expected FileGroups + }{ + { + name: "add to nil", + groups: nil, + add: []FileGroup{FileGroupDocs}, + expected: FileGroups{FileGroupDocs}, + }, + { + name: "add to empty", + groups: FileGroups{}, + add: []FileGroup{FileGroupDocs}, + expected: FileGroups{FileGroupDocs}, + }, + { + name: "add single group", + groups: FileGroups{FileGroupDocs}, + add: []FileGroup{FileGroupEnos}, + expected: FileGroups{FileGroupDocs, FileGroupEnos}, + }, + { + name: "add multiple groups", + groups: FileGroups{FileGroupDocs}, + add: []FileGroup{FileGroupEnos, FileGroupGoApp, FileGroupWebUI}, + expected: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos, FileGroupWebUI}, + }, + { + name: "add duplicate group", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + add: []FileGroup{FileGroupDocs}, + expected: FileGroups{FileGroupDocs, FileGroupEnos}, + }, + { + name: "add multiple with duplicates", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + add: []FileGroup{FileGroupGoApp, FileGroupDocs, FileGroupWebUI, FileGroupEnos}, + expected: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos, FileGroupWebUI}, + }, + { + name: "add maintains sorted order", + groups: FileGroups{FileGroupWebUI}, + add: []FileGroup{FileGroupGoApp, FileGroupDocs}, + expected: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupWebUI}, + }, + { + name: "add nothing", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + add: []FileGroup{}, + expected: FileGroups{FileGroupDocs, FileGroupEnos}, + }, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + result := test.groups.Add(test.add...) + require.Equal(t, test.expected, result, "Add result should match expected") + // Verify result is sorted + for i := 1; i < len(result); i++ { + require.Less(t, string(result[i-1]), string(result[i]), "result should be sorted") + } + }) + } +} + +// TestFileGroups_In tests the In method which performs binary search to find a group +// in the sorted FileGroups set. Returns the index where the group is (or should be) +// and a boolean indicating if it was found. Tests cover nil/empty receivers, found/not found +// cases at various positions, and proper index calculation for insertion. +func TestFileGroups_In(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + groups FileGroups + search FileGroup + expectedIndex int + expectedFound bool + }{ + { + name: "search in nil", + groups: nil, + search: FileGroupDocs, + expectedIndex: 0, + expectedFound: false, + }, + { + name: "search in empty", + groups: FileGroups{}, + search: FileGroupDocs, + expectedIndex: 0, + expectedFound: false, + }, + { + name: "found in single element", + groups: FileGroups{FileGroupDocs}, + search: FileGroupDocs, + expectedIndex: 0, + expectedFound: true, + }, + { + name: "not found in single element", + groups: FileGroups{FileGroupDocs}, + search: FileGroupEnos, + expectedIndex: 1, + expectedFound: false, + }, + { + name: "found at beginning", + groups: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos}, + search: FileGroupGoApp, + expectedIndex: 0, + expectedFound: true, + }, + { + name: "found in middle", + groups: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos}, + search: FileGroupDocs, + expectedIndex: 1, + expectedFound: true, + }, + { + name: "found at end", + groups: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos}, + search: FileGroupEnos, + expectedIndex: 2, + expectedFound: true, + }, + { + name: "not found - would be at beginning", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + search: FileGroupGoApp, + expectedIndex: 0, + expectedFound: false, + }, + { + name: "not found - would be in middle", + groups: FileGroups{FileGroupGoApp, FileGroupEnos}, + search: FileGroupDocs, + expectedIndex: 1, + expectedFound: false, + }, + { + name: "not found - would be at end", + groups: FileGroups{FileGroupGoApp, FileGroupDocs}, + search: FileGroupWebUI, + expectedIndex: 2, + expectedFound: false, + }, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + idx, found := test.groups.In(test.search) + require.Equal(t, test.expectedIndex, idx, "index should match expected") + require.Equal(t, test.expectedFound, found, "found status should match expected") + }) + } +} + +// TestFileGroups_All tests the All method which checks if all groups from the argument +// are present in the receiver FileGroups. Returns true only if every group in the check +// set exists in the receiver. Tests cover nil/empty cases, partial matches, full matches, +// and superset scenarios. +func TestFileGroups_All(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + groups FileGroups + check FileGroups + expected bool + }{ + { + name: "nil groups, nil check", + groups: nil, + check: nil, + expected: true, + }, + { + name: "nil groups, empty check", + groups: nil, + check: FileGroups{}, + expected: true, + }, + { + name: "nil groups, non-empty check", + groups: nil, + check: FileGroups{FileGroupDocs}, + expected: false, + }, + { + name: "empty groups, nil check", + groups: FileGroups{}, + check: nil, + expected: true, + }, + { + name: "empty groups, empty check", + groups: FileGroups{}, + check: FileGroups{}, + expected: true, + }, + { + name: "empty groups, non-empty check", + groups: FileGroups{}, + check: FileGroups{FileGroupDocs}, + expected: false, + }, + { + name: "non-empty groups, nil check", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: nil, + expected: true, + }, + { + name: "non-empty groups, empty check", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: FileGroups{}, + expected: true, + }, + { + name: "all present - single element", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: FileGroups{FileGroupDocs}, + expected: true, + }, + { + name: "all present - multiple elements", + groups: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos, FileGroupWebUI}, + check: FileGroups{FileGroupDocs, FileGroupEnos}, + expected: true, + }, + { + name: "all present - identical", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: FileGroups{FileGroupDocs, FileGroupEnos}, + expected: true, + }, + { + name: "not all present - one missing", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: FileGroups{FileGroupDocs, FileGroupGoApp}, + expected: false, + }, + { + name: "not all present - all missing", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: FileGroups{FileGroupGoApp, FileGroupWebUI}, + expected: false, + }, + { + name: "check is superset", + groups: FileGroups{FileGroupDocs}, + check: FileGroups{FileGroupDocs, FileGroupEnos}, + expected: false, + }, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + result := test.groups.All(test.check) + require.Equal(t, test.expected, result, "All result should match expected") + }) + } +} + +// TestFileGroups_Any tests the Any method which checks if at least one group from the +// argument is present in the receiver FileGroups. Returns true if any group in the check +// set exists in the receiver. Tests cover nil/empty cases, single/multiple matches, and +// no match scenarios. +func TestFileGroups_Any(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + groups FileGroups + check FileGroups + expected bool + }{ + { + name: "nil groups, nil check", + groups: nil, + check: nil, + expected: false, + }, + { + name: "nil groups, empty check", + groups: nil, + check: FileGroups{}, + expected: false, + }, + { + name: "nil groups, non-empty check", + groups: nil, + check: FileGroups{FileGroupDocs}, + expected: false, + }, + { + name: "empty groups, nil check", + groups: FileGroups{}, + check: nil, + expected: false, + }, + { + name: "empty groups, empty check", + groups: FileGroups{}, + check: FileGroups{}, + expected: false, + }, + { + name: "empty groups, non-empty check", + groups: FileGroups{}, + check: FileGroups{FileGroupDocs}, + expected: false, + }, + { + name: "non-empty groups, nil check", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: nil, + expected: false, + }, + { + name: "non-empty groups, empty check", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: FileGroups{}, + expected: false, + }, + { + name: "one match - single element check", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: FileGroups{FileGroupDocs}, + expected: true, + }, + { + name: "one match - multiple element check", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: FileGroups{FileGroupGoApp, FileGroupDocs}, + expected: true, + }, + { + name: "multiple matches", + groups: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos}, + check: FileGroups{FileGroupDocs, FileGroupEnos}, + expected: true, + }, + { + name: "all match", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: FileGroups{FileGroupDocs, FileGroupEnos}, + expected: true, + }, + { + name: "no match - single element", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: FileGroups{FileGroupGoApp}, + expected: false, + }, + { + name: "no match - multiple elements", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + check: FileGroups{FileGroupGoApp, FileGroupWebUI}, + expected: false, + }, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + result := test.groups.Any(test.check) + require.Equal(t, test.expected, result, "Any result should match expected") + }) + } +} + +// TestFileGroups_Groups tests the Groups method which converts FileGroups to a slice +// of strings. Each FileGroup is converted to its string representation. Tests cover +// nil/empty receivers and various group counts. +func TestFileGroups_Groups(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + groups FileGroups + expected []string + }{ + { + name: "nil groups", + groups: nil, + expected: []string{}, + }, + { + name: "empty groups", + groups: FileGroups{}, + expected: []string{}, + }, + { + name: "single group", + groups: FileGroups{FileGroupDocs}, + expected: []string{"docs"}, + }, + { + name: "multiple groups", + groups: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos}, + expected: []string{"app", "docs", "enos"}, + }, + { + name: "all file groups", + groups: FileGroups{FileGroupAutopilot, FileGroupChangelog, FileGroupCommunity, FileGroupDocs, FileGroupEnos}, + expected: []string{"autopilot", "changelog", "community", "docs", "enos"}, + }, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + result := test.groups.Groups() + require.Equal(t, test.expected, result, "Groups result should match expected") + }) + } +} + +// TestFileGroups_String tests the String method which returns a comma-separated string +// representation of all groups in the FileGroups set. Tests cover nil/empty receivers +// and various group counts. +func TestFileGroups_String(t *testing.T) { + t.Parallel() + for _, test := range []struct { + name string + groups FileGroups + expected string + }{ + { + name: "nil groups", + groups: nil, + expected: "", + }, + { + name: "empty groups", + groups: FileGroups{}, + expected: "", + }, + { + name: "single group", + groups: FileGroups{FileGroupDocs}, + expected: "docs", + }, + { + name: "multiple groups", + groups: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos}, + expected: "app, docs, enos", + }, + { + name: "all file groups", + groups: FileGroups{FileGroupAutopilot, FileGroupChangelog, FileGroupCommunity, FileGroupDocs, FileGroupEnos}, + expected: "autopilot, changelog, community, docs, enos", + }, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + result := test.groups.String() + require.Equal(t, test.expected, result, "String result should match expected") + }) + } +} + +// TestFileGroups_Intersection tests the Intersection method which returns a new FileGroups +// containing only the groups present in both the receiver and the argument. Returns an empty +// FileGroups if there's no intersection. Tests cover nil/empty cases, no intersection, partial +// intersections, full intersections, and various subset scenarios. +func TestFileGroups_Intersection(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + groups FileGroups + arg FileGroups + expected FileGroups + }{ + { + name: "both nil", + groups: nil, + arg: nil, + expected: FileGroups{}, + }, + { + name: "groups nil, arg has values", + groups: nil, + arg: FileGroups{FileGroupDocs, FileGroupEnos}, + expected: FileGroups{}, + }, + { + name: "groups has values, arg nil", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + arg: nil, + expected: FileGroups{}, + }, + { + name: "both empty", + groups: FileGroups{}, + arg: FileGroups{}, + expected: FileGroups{}, + }, + { + name: "groups empty, arg has values", + groups: FileGroups{}, + arg: FileGroups{FileGroupDocs, FileGroupEnos}, + expected: FileGroups{}, + }, + { + name: "groups has values, arg empty", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + arg: FileGroups{}, + expected: FileGroups{}, + }, + { + name: "no intersection", + groups: FileGroups{FileGroupDocs, FileGroupEnos}, + arg: FileGroups{FileGroupGoApp, FileGroupWebUI}, + expected: FileGroups{}, + }, + { + name: "partial intersection - one common element", + groups: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos}, + arg: FileGroups{FileGroupGoApp, FileGroupWebUI}, + expected: FileGroups{FileGroupGoApp}, + }, + { + name: "partial intersection - multiple common elements", + groups: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos, FileGroupWebUI}, + arg: FileGroups{FileGroupGoApp, FileGroupEnos, FileGroupTools, FileGroupWebUI}, + expected: FileGroups{FileGroupGoApp, FileGroupEnos, FileGroupWebUI}, + }, + { + name: "full intersection - identical groups", + groups: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos}, + arg: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos}, + expected: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos}, + }, + { + name: "full intersection - arg is subset of groups", + groups: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos, FileGroupWebUI}, + arg: FileGroups{FileGroupGoApp, FileGroupEnos}, + expected: FileGroups{FileGroupGoApp, FileGroupEnos}, + }, + { + name: "full intersection - groups is subset of arg", + groups: FileGroups{FileGroupGoApp, FileGroupEnos}, + arg: FileGroups{FileGroupGoApp, FileGroupDocs, FileGroupEnos, FileGroupWebUI}, + expected: FileGroups{FileGroupGoApp, FileGroupEnos}, + }, + { + name: "single element in both - match", + groups: FileGroups{FileGroupDocs}, + arg: FileGroups{FileGroupDocs}, + expected: FileGroups{FileGroupDocs}, + }, + { + name: "single element in both - no match", + groups: FileGroups{FileGroupDocs}, + arg: FileGroups{FileGroupEnos}, + expected: FileGroups{}, + }, + { + name: "single element in groups, multiple in arg - match", + groups: FileGroups{FileGroupDocs}, + arg: FileGroups{FileGroupDocs, FileGroupEnos, FileGroupGoApp}, + expected: FileGroups{FileGroupDocs}, + }, + { + name: "single element in groups, multiple in arg - no match", + groups: FileGroups{FileGroupWebUI}, + arg: FileGroups{FileGroupDocs, FileGroupEnos, FileGroupGoApp}, + expected: FileGroups{}, + }, + { + name: "multiple in groups, single in arg - match", + groups: FileGroups{FileGroupDocs, FileGroupEnos, FileGroupGoApp}, + arg: FileGroups{FileGroupEnos}, + expected: FileGroups{FileGroupEnos}, + }, + { + name: "multiple in groups, single in arg - no match", + groups: FileGroups{FileGroupDocs, FileGroupEnos, FileGroupGoApp}, + arg: FileGroups{FileGroupWebUI}, + expected: FileGroups{}, + }, + { + name: "all file groups intersection", + groups: FileGroups{FileGroupAutopilot, FileGroupChangelog, FileGroupCommunity, FileGroupDocs, FileGroupEnos, FileGroupEnterprise}, + arg: FileGroups{FileGroupDocs, FileGroupEnos, FileGroupEnterprise, FileGroupGithub, FileGroupGoApp, FileGroupGoToolchain}, + expected: FileGroups{FileGroupDocs, FileGroupEnos, FileGroupEnterprise}, + }, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + result := test.groups.Intersection(test.arg) + require.Equal(t, test.expected, result, "intersection result should match expected") + }) + } +} diff --git a/tools/pipeline/internal/pkg/git/check_changed_files.go b/tools/pipeline/internal/pkg/git/check_changed_files.go new file mode 100644 index 0000000000..b0d2abea99 --- /dev/null +++ b/tools/pipeline/internal/pkg/git/check_changed_files.go @@ -0,0 +1,189 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package git + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "strings" + + "github.com/hashicorp/vault/tools/pipeline/internal/pkg/changed" + libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git/client" + "github.com/jedib0t/go-pretty/v6/table" + slogctx "github.com/veqryn/slog-context" +) + +// CheckChangedFilesReq holds the state and configuration for checking changed files +type CheckChangedFilesReq struct { + // Branch specifies the branch to compare against + Branch string + // Range specifies the commit range to compare (e.g., HEAD~5..HEAD) + Range string + // Commit specifies a specific commit SHA to analyze + Commit string + // Write a specially formatted response to $GITHUB_OUTPUT + WriteToGithubOutput bool + // CheckGroups specifies the file groups that must not have changed + CheckGroups []string +} + +// CheckChangedFilesRes represents the response from checking changed files +type CheckChangedFilesRes struct { + // Inputs + ChangedFiles changed.Files `json:"changed_files,omitempty"` + ChangedGroups changed.FileGroups `json:"changed_groups,omitempty"` + CheckedGroups changed.FileGroups `json:"checked_groups,omitempty"` + // Outputs + MatchedFiles changed.Files `json:"matched_files,omitempty"` + MatchedGroups changed.FileGroups `json:"matched_groups,omitempty"` +} + +// Run executes the git check changed files operation +func (g *CheckChangedFilesReq) Run(ctx context.Context, client *libgit.Client) (*CheckChangedFilesRes, error) { + slog.Default().DebugContext(ctx, "checking changed files from git for disallowed groups") + + if g == nil { + return nil, fmt.Errorf("uninitialized") + } + + if len(g.CheckGroups) < 1 { + return nil, fmt.Errorf("no disallowed groups have been configured") + } + + listReq := &ListChangedFilesReq{ + Branch: g.Branch, + Range: g.Range, + Commit: g.Commit, + GroupFiles: true, + } + + listRes, err := listReq.Run(ctx, client) + if err != nil { + return nil, err + } + + disallowdGroups := changed.FileGroups{} + for _, g := range g.CheckGroups { + disallowdGroups = disallowdGroups.Add(changed.FileGroup(g)) + } + + ctx = slogctx.Append(ctx, + slog.String("disallowed-groups", disallowdGroups.String()), + ) + + res := &CheckChangedFilesRes{ + ChangedFiles: listRes.Files, + ChangedGroups: listRes.Groups, + CheckedGroups: disallowdGroups, + MatchedGroups: disallowdGroups.Intersection(listRes.Groups), + } + + slog.Default().DebugContext(ctx, "checking changed files for disallowed groups") + matchedFiles := changed.Files{} + for _, file := range listRes.Files { + if i := file.Groups.Intersection(disallowdGroups); len(i) > 0 { + matchedFiles = append(matchedFiles, file) + } + } + if len(matchedFiles) > 0 { + res.MatchedFiles = matchedFiles + ctx = slogctx.Append(ctx, + slog.String("matched-groups", res.MatchedGroups.String()), + ) + slog.Default().DebugContext(ctx, "found files matching disallowed groups") + } + + return res, nil +} + +// ToJSON marshals the response to JSON. +func (r *CheckChangedFilesRes) ToJSON() ([]byte, error) { + if r == nil { + return nil, errors.New("uninitialized") + } + + b, err := json.Marshal(r) + if err != nil { + return nil, fmt.Errorf("marshaling check changed files to JSON: %w", err) + } + + return b, nil +} + +// CheckChangedFilesGithubOutput is our GITHUB_OUTPUT type for check command +type CheckChangedFilesGithubOutput struct { + ChangedFiles []string `json:"changed_files,omitempty"` + ChangedGroups changed.FileGroups `json:"changed_groups,omitempty"` + CheckedGroups changed.FileGroups `json:"checked_groups,omitempty"` + MatchedGroups changed.FileGroups `json:"matched_groups,omitempty"` + MatchedFiles []string `json:"matched_files,omitempty"` +} + +// ToGithubOutput writes a simplified check result to be used in $GITHUB_OUTPUT +func (r *CheckChangedFilesRes) ToGithubOutput() ([]byte, error) { + if r == nil { + return nil, errors.New("uninitialized") + } + + output := &CheckChangedFilesGithubOutput{ + ChangedGroups: r.ChangedGroups, + CheckedGroups: r.CheckedGroups, + MatchedGroups: r.MatchedGroups, + } + if f := r.ChangedFiles; f != nil { + output.ChangedFiles = f.Names() + } + if f := r.MatchedFiles; f != nil { + output.MatchedFiles = f.Names() + } + + b, err := json.Marshal(output) + if err != nil { + return nil, fmt.Errorf("marshaling check changed files GITHUB_OUTPUT to JSON: %w", err) + } + + return b, nil +} + +// ToTable marshals the response to a text table. +func (r *CheckChangedFilesRes) ToTable() string { + if r == nil || len(r.MatchedGroups) < 1 { + return "" + } + + t := table.NewWriter() + t.Style().Options.DrawBorder = false + t.Style().Options.SeparateColumns = false + t.Style().Options.SeparateFooter = false + t.Style().Options.SeparateHeader = false + t.Style().Options.SeparateRows = false + t.AppendHeader(table.Row{"path", "groups", "disallowed groups"}) + for _, file := range r.MatchedFiles { + t.AppendRow(table.Row{ + file.Name(), + file.Groups.String(), + r.CheckedGroups.Intersection(file.Groups), + }) + } + t.SuppressEmptyColumns() + t.SuppressTrailingSpaces() + + return t.Render() +} + +// String returns a string representation of the response +func (r *CheckChangedFilesRes) String() string { + if r == nil || len(r.ChangedFiles) == 0 { + return "No changed files found" + } + + w := strings.Builder{} + for _, name := range r.ChangedFiles.Names() { + w.WriteString(name + "\n") + } + return w.String() +} diff --git a/tools/pipeline/internal/pkg/git/am.go b/tools/pipeline/internal/pkg/git/client/am.go similarity index 99% rename from tools/pipeline/internal/pkg/git/am.go rename to tools/pipeline/internal/pkg/git/client/am.go index 005ba4d205..da31d48a42 100644 --- a/tools/pipeline/internal/pkg/git/am.go +++ b/tools/pipeline/internal/pkg/git/client/am.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/apply.go b/tools/pipeline/internal/pkg/git/client/apply.go similarity index 99% rename from tools/pipeline/internal/pkg/git/apply.go rename to tools/pipeline/internal/pkg/git/client/apply.go index f06f44ddbe..4054fc6e1d 100644 --- a/tools/pipeline/internal/pkg/git/apply.go +++ b/tools/pipeline/internal/pkg/git/client/apply.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/branch.go b/tools/pipeline/internal/pkg/git/client/branch.go similarity index 99% rename from tools/pipeline/internal/pkg/git/branch.go rename to tools/pipeline/internal/pkg/git/client/branch.go index 9da3c82567..48589a0687 100644 --- a/tools/pipeline/internal/pkg/git/branch.go +++ b/tools/pipeline/internal/pkg/git/client/branch.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/checkout.go b/tools/pipeline/internal/pkg/git/client/checkout.go similarity index 99% rename from tools/pipeline/internal/pkg/git/checkout.go rename to tools/pipeline/internal/pkg/git/client/checkout.go index a59ca61d06..8ed5093c1c 100644 --- a/tools/pipeline/internal/pkg/git/checkout.go +++ b/tools/pipeline/internal/pkg/git/client/checkout.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/cherry-pick.go b/tools/pipeline/internal/pkg/git/client/cherry-pick.go similarity index 99% rename from tools/pipeline/internal/pkg/git/cherry-pick.go rename to tools/pipeline/internal/pkg/git/client/cherry-pick.go index 9bad0d5c15..baf95af73e 100644 --- a/tools/pipeline/internal/pkg/git/cherry-pick.go +++ b/tools/pipeline/internal/pkg/git/client/cherry-pick.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/client.go b/tools/pipeline/internal/pkg/git/client/client.go similarity index 99% rename from tools/pipeline/internal/pkg/git/client.go rename to tools/pipeline/internal/pkg/git/client/client.go index 36716b23a9..3b33f590bb 100644 --- a/tools/pipeline/internal/pkg/git/client.go +++ b/tools/pipeline/internal/pkg/git/client/client.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/clone.go b/tools/pipeline/internal/pkg/git/client/clone.go similarity index 99% rename from tools/pipeline/internal/pkg/git/clone.go rename to tools/pipeline/internal/pkg/git/client/clone.go index 6033041fff..2a4bd1996c 100644 --- a/tools/pipeline/internal/pkg/git/clone.go +++ b/tools/pipeline/internal/pkg/git/client/clone.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/commit.go b/tools/pipeline/internal/pkg/git/client/commit.go similarity index 99% rename from tools/pipeline/internal/pkg/git/commit.go rename to tools/pipeline/internal/pkg/git/client/commit.go index 9424d9bc3d..72bc3594a8 100644 --- a/tools/pipeline/internal/pkg/git/commit.go +++ b/tools/pipeline/internal/pkg/git/client/commit.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/client/doc.go b/tools/pipeline/internal/pkg/git/client/doc.go new file mode 100644 index 0000000000..dc53098604 --- /dev/null +++ b/tools/pipeline/internal/pkg/git/client/doc.go @@ -0,0 +1,5 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +// Package client is a Go module that wraps the `git` CLI client. +package client diff --git a/tools/pipeline/internal/pkg/git/fetch.go b/tools/pipeline/internal/pkg/git/client/fetch.go similarity index 99% rename from tools/pipeline/internal/pkg/git/fetch.go rename to tools/pipeline/internal/pkg/git/client/fetch.go index ae2ec2c542..c96e8cf38f 100644 --- a/tools/pipeline/internal/pkg/git/fetch.go +++ b/tools/pipeline/internal/pkg/git/client/fetch.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/client/log.go b/tools/pipeline/internal/pkg/git/client/log.go new file mode 100644 index 0000000000..9f75b9b26e --- /dev/null +++ b/tools/pipeline/internal/pkg/git/client/log.go @@ -0,0 +1,407 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package client + +import ( + "context" + "fmt" + "strings" +) + +type ( + // LogPrettyFormat is the format for pretty printing + LogPrettyFormat = string + // LogDateFormat is the format for dates + LogDateFormat = string + // LogDecorateFormat is the format for decoration + LogDecorateFormat = string + // LogDiffFilter is the filter for diff types + LogDiffFilter = string +) + +const ( + LogPrettyFormatOneline LogPrettyFormat = "oneline" + LogPrettyFormatShort LogPrettyFormat = "short" + LogPrettyFormatMedium LogPrettyFormat = "medium" + LogPrettyFormatFull LogPrettyFormat = "full" + LogPrettyFormatFuller LogPrettyFormat = "fuller" + LogPrettyFormatReference LogPrettyFormat = "reference" + LogPrettyFormatEmail LogPrettyFormat = "email" + LogPrettyFormatRaw LogPrettyFormat = "raw" + LogPrettyFormatNone LogPrettyFormat = "none" // NOTE: renders blank value to support --pretty= + + LogDateFormatRelative LogDateFormat = "relative" + LogDateFormatISO LogDateFormat = "iso" + LogDateFormatISO8601 LogDateFormat = "iso8601" + LogDateFormatRFC LogDateFormat = "rfc" + LogDateFormatShort LogDateFormat = "short" + LogDateFormatRaw LogDateFormat = "raw" + LogDateFormatHuman LogDateFormat = "human" + LogDateFormatUnix LogDateFormat = "unix" + + LogDecorateFormatShort LogDecorateFormat = "short" + LogDecorateFull LogDecorateFormat = "full" + LogDecorateAuto LogDecorateFormat = "auto" + LogDecorateNo LogDecorateFormat = "no" + + LogDiffFilterAdded LogDiffFilter = "A" + LogDiffFilterCopied LogDiffFilter = "C" + LogDiffFilterDeleted LogDiffFilter = "D" + LogDiffFilterModified LogDiffFilter = "M" + LogDiffFilterRenamed LogDiffFilter = "R" + LogDiffFilterTypeChanged LogDiffFilter = "T" + LogDiffFilterUnmerged LogDiffFilter = "U" + LogDiffFilterUnknown LogDiffFilter = "X" + LogDiffFilterBroken LogDiffFilter = "B" + LogDiffFilterAll LogDiffFilter = "*" +) + +// LogOpts are the git log flags and arguments +// See: https://git-scm.com/docs/git-log +type LogOpts struct { + // Commit Limiting + MaxCount uint // -n, --max-count= + Skip uint // --skip= + Since string // --since= + After string // --after= + Until string // --until= + Before string // --before= + Author string // --author= + Committer string // --committer= + Grep string // --grep= + AllMatch bool // --all-match + InvertGrep bool // --invert-grep + RegexpIgnoreCase bool // -i, --regexp-ignore-case + + // Merge Options + Merges bool // --merges + NoMerges bool // --no-merges + FirstParent bool // --first-parent + + // History Traversal + All bool // --all + Branches []string // --branches[=] + Tags []string // --tags[=] + Remotes []string // --remotes[=] + + // Formatting + Oneline bool // --oneline + Pretty LogPrettyFormat // --pretty= + Format string // --format= + AbbrevCommit bool // --abbrev-commit + NoAbbrevCommit bool // --no-abbrev-commit + Abbrev uint // --abbrev= + Decorate LogDecorateFormat // --decorate[=] + DecorateRefs []string // --decorate-refs= + Source bool // --source + Graph bool // --graph + Date LogDateFormat // --date= + RelativeDate bool // --relative-date + + // Diff Options + Patch bool // -p, --patch + NoPatch bool // -s, --no-patch + Stat bool // --stat + Shortstat bool // --shortstat + NameOnly bool // --name-only + NameStatus bool // --name-status + DiffFilter []LogDiffFilter // --diff-filter= + DiffMerges DiffMergeFormat // --diff-merges= + CombinedDiff bool // -c + DenseCombined bool // --cc + Follow bool // --follow + FullDiff bool // --full-diff + + // Ordering + DateOrder bool // --date-order + AuthorDateOrder bool // --author-date-order + TopoOrder bool // --topo-order + Reverse bool // --reverse + + // History Simplification + SimplifyByDecoration bool // --simplify-by-decoration + FullHistory bool // --full-history + AncestryPath bool // --ancestry-path + ShowPulls bool // --show-pulls + + // Reflog + WalkReflogs bool // -g, --walk-reflogs + + // Output Control + Color bool // --color + NoColor bool // --no-color + NullSep bool // -z + + // Targets + Target string // - can be a range (A..B), branch name, or commit + PathSpec []string // -- +} + +// Log runs the git log command +func (c *Client) Log(ctx context.Context, opts *LogOpts) (*ExecResponse, error) { + return c.Exec(ctx, "log", opts) +} + +// String returns the options as a string +func (o *LogOpts) String() string { + return strings.Join(o.Strings(), " ") +} + +// Strings returns the options as a string slice +func (o *LogOpts) Strings() []string { + if o == nil { + return nil + } + + opts := []string{} + + // Targets - Target (revision range, branch, or commit) + if o.Target != "" { + opts = append(opts, o.Target) + } + + // Commit Limiting + if o.MaxCount > 0 { + opts = append(opts, fmt.Sprintf("--max-count=%d", o.MaxCount)) + } + + if o.Skip > 0 { + opts = append(opts, fmt.Sprintf("--skip=%d", o.Skip)) + } + + if o.Since != "" { + opts = append(opts, fmt.Sprintf("--since=%s", o.Since)) + } + + if o.After != "" { + opts = append(opts, fmt.Sprintf("--after=%s", o.After)) + } + + if o.Until != "" { + opts = append(opts, fmt.Sprintf("--until=%s", o.Until)) + } + + if o.Before != "" { + opts = append(opts, fmt.Sprintf("--before=%s", o.Before)) + } + + if o.Author != "" { + opts = append(opts, fmt.Sprintf("--author=%s", o.Author)) + } + + if o.Committer != "" { + opts = append(opts, fmt.Sprintf("--committer=%s", o.Committer)) + } + + if o.Grep != "" { + opts = append(opts, fmt.Sprintf("--grep=%s", o.Grep)) + } + + if o.AllMatch { + opts = append(opts, "--all-match") + } + + if o.InvertGrep { + opts = append(opts, "--invert-grep") + } + + if o.RegexpIgnoreCase { + opts = append(opts, "--regexp-ignore-case") + } + + // Merge Options + if o.Merges { + opts = append(opts, "--merges") + } + + if o.NoMerges { + opts = append(opts, "--no-merges") + } + + if o.FirstParent { + opts = append(opts, "--first-parent") + } + + // History Traversal + if o.All { + opts = append(opts, "--all") + } + + for _, branch := range o.Branches { + opts = append(opts, fmt.Sprintf("--branches=%s", branch)) + } + + for _, tag := range o.Tags { + opts = append(opts, fmt.Sprintf("--tags=%s", tag)) + } + + for _, remote := range o.Remotes { + opts = append(opts, fmt.Sprintf("--remotes=%s", remote)) + } + + // Formatting + if o.Oneline { + opts = append(opts, "--oneline") + } + + if o.Pretty != "" { + if o.Pretty == LogPrettyFormatNone { + opts = append(opts, "--pretty=") + } else { + opts = append(opts, fmt.Sprintf("--pretty=%s", string(o.Pretty))) + } + } + + if o.Format != "" { + opts = append(opts, fmt.Sprintf("--format=%s", o.Format)) + } + + if o.AbbrevCommit { + opts = append(opts, "--abbrev-commit") + } + + if o.NoAbbrevCommit { + opts = append(opts, "--no-abbrev-commit") + } + + if o.Abbrev > 0 { + opts = append(opts, fmt.Sprintf("--abbrev=%d", o.Abbrev)) + } + + if o.Decorate != "" { + opts = append(opts, fmt.Sprintf("--decorate=%s", string(o.Decorate))) + } + + for _, ref := range o.DecorateRefs { + opts = append(opts, fmt.Sprintf("--decorate-refs=%s", ref)) + } + + if o.Source { + opts = append(opts, "--source") + } + + if o.Graph { + opts = append(opts, "--graph") + } + + if o.Date != "" { + opts = append(opts, fmt.Sprintf("--date=%s", string(o.Date))) + } + + if o.RelativeDate { + opts = append(opts, "--relative-date") + } + + // Diff Options + if o.Patch { + opts = append(opts, "--patch") + } + + if o.NoPatch { + opts = append(opts, "--no-patch") + } + + if o.Stat { + opts = append(opts, "--stat") + } + + if o.Shortstat { + opts = append(opts, "--shortstat") + } + + if o.NameOnly { + opts = append(opts, "--name-only") + } + + if o.NameStatus { + opts = append(opts, "--name-status") + } + + if len(o.DiffFilter) > 0 { + filters := make([]string, len(o.DiffFilter)) + for i, filter := range o.DiffFilter { + filters[i] = string(filter) + } + opts = append(opts, fmt.Sprintf("--diff-filter=%s", strings.Join(filters, ""))) + } + + if o.DiffMerges != "" { + opts = append(opts, fmt.Sprintf("--diff-merges=%s", string(o.DiffMerges))) + } + + if o.CombinedDiff { + opts = append(opts, "-c") + } + + if o.DenseCombined { + opts = append(opts, "--cc") + } + + if o.Follow { + opts = append(opts, "--follow") + } + + if o.FullDiff { + opts = append(opts, "--full-diff") + } + + // Ordering + if o.DateOrder { + opts = append(opts, "--date-order") + } + + if o.AuthorDateOrder { + opts = append(opts, "--author-date-order") + } + + if o.TopoOrder { + opts = append(opts, "--topo-order") + } + + if o.Reverse { + opts = append(opts, "--reverse") + } + + // History Simplification + if o.SimplifyByDecoration { + opts = append(opts, "--simplify-by-decoration") + } + + if o.FullHistory { + opts = append(opts, "--full-history") + } + + if o.AncestryPath { + opts = append(opts, "--ancestry-path") + } + + if o.ShowPulls { + opts = append(opts, "--show-pulls") + } + + // Reflog + if o.WalkReflogs { + opts = append(opts, "--walk-reflogs") + } + + // Output Control + if o.Color { + opts = append(opts, "--color") + } + + if o.NoColor { + opts = append(opts, "--no-color") + } + + if o.NullSep { + opts = append(opts, "-z") + } + + // PathSpec - must be last with -- separator + if len(o.PathSpec) > 0 { + opts = append(append(opts, "--"), o.PathSpec...) + } + + return opts +} diff --git a/tools/pipeline/internal/pkg/git/merge.go b/tools/pipeline/internal/pkg/git/client/merge.go similarity index 99% rename from tools/pipeline/internal/pkg/git/merge.go rename to tools/pipeline/internal/pkg/git/client/merge.go index e57b3421e6..fc7661f0ea 100644 --- a/tools/pipeline/internal/pkg/git/merge.go +++ b/tools/pipeline/internal/pkg/git/client/merge.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/opts_test.go b/tools/pipeline/internal/pkg/git/client/opts_test.go similarity index 77% rename from tools/pipeline/internal/pkg/git/opts_test.go rename to tools/pipeline/internal/pkg/git/client/opts_test.go index 17c3234905..8854e2cb4f 100644 --- a/tools/pipeline/internal/pkg/git/opts_test.go +++ b/tools/pipeline/internal/pkg/git/client/opts_test.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "testing" @@ -9,12 +9,12 @@ import ( "github.com/stretchr/testify/require" ) -// Test our opts structs ability to render the correct flags in the correct order -// NOTE: Many of thests use incompatible options but that's not what we care about, -// we're simply asserting that the rendered string matches what ought to be there -// give the config. -// We have chosen not to try and very flag combinations. Instead we render it -// and execute it and rely on git to handle validation of options. +// Test the various Opts structs ability to render the correct flags in the +// correct order. +// NOTE: Many of these tests use incompatible options combinations but that's +// not what we care about, we're simply asserting that the rendered string +// matches what ought to be there given the config. Verifying flag options is +// not currently part of the library. func TestOptsStringers(t *testing.T) { t.Parallel() @@ -796,6 +796,13 @@ func TestOptsStringers(t *testing.T) { }, "--diff-algorithm=histogram --diff-merges=dense-combined --format=medium --no-color --no-patch --output=/path/to/my.diff --patch --quiet --raw HEAD -- go.mod go.sum", }, + "show pretty none name only": { + &ShowOpts{ + Pretty: LogPrettyFormatNone, + NameOnly: true, + }, + "--pretty= --name-only ", + }, "status": { &StatusOpts{ AheadBehind: true, @@ -818,6 +825,224 @@ func TestOptsStringers(t *testing.T) { }, "--ahead-behind --branch --column=always --find-renames=12 --ignored=matching --ignore-submodules=dirty --long --no-ahead-behind --no-column --no-renames --porcelain --renames --short --show-stash --untracked-files=all --verbose -- go.mod go.sum", }, + "log 1/3 opts": { + &LogOpts{ + MaxCount: 10, + Skip: 5, + Since: "2 weeks ago", + After: "1 week ago", + Until: "yesterday", + Before: "today", + Author: "John Doe", + Committer: "Jane Smith", + Grep: "bug fix", + AllMatch: true, + InvertGrep: true, + RegexpIgnoreCase: true, + Merges: true, + NoMerges: true, + FirstParent: true, + }, + "--max-count=10 --skip=5 --since=2 weeks ago --after=1 week ago --until=yesterday --before=today --author=John Doe --committer=Jane Smith --grep=bug fix --all-match --invert-grep --regexp-ignore-case --merges --no-merges --first-parent", + }, + "log 2/3 opts": { + &LogOpts{ + All: true, + Branches: []string{"main", "develop"}, + Tags: []string{"v1.0", "v2.0"}, + Remotes: []string{"origin", "upstream"}, + Oneline: true, + Pretty: LogPrettyFormatFull, + Format: "%h %an %s", + AbbrevCommit: true, + NoAbbrevCommit: true, + Abbrev: 7, + Decorate: LogDecorateFormatShort, + DecorateRefs: []string{"refs/heads/*", "refs/tags/*"}, + Source: true, + Graph: true, + Date: LogDateFormatRelative, + RelativeDate: true, + }, + "--all --branches=main --branches=develop --tags=v1.0 --tags=v2.0 --remotes=origin --remotes=upstream --oneline --pretty=full --format=%h %an %s --abbrev-commit --no-abbrev-commit --abbrev=7 --decorate=short --decorate-refs=refs/heads/* --decorate-refs=refs/tags/* --source --graph --date=relative --relative-date", + }, + "log 3/3 opts": { + &LogOpts{ + Patch: true, + NoPatch: true, + Stat: true, + Shortstat: true, + NameOnly: true, + NameStatus: true, + DiffMerges: DiffMergeFormatCombined, + CombinedDiff: true, + DenseCombined: true, + Follow: true, + FullDiff: true, + DateOrder: true, + AuthorDateOrder: true, + TopoOrder: true, + Reverse: true, + SimplifyByDecoration: true, + FullHistory: true, + AncestryPath: true, + ShowPulls: true, + WalkReflogs: true, + Color: true, + NoColor: true, + NullSep: true, + Target: "HEAD~5..HEAD", + PathSpec: []string{"go.mod", "go.sum"}, + }, + "HEAD~5..HEAD --patch --no-patch --stat --shortstat --name-only --name-status --diff-merges=combined -c --cc --follow --full-diff --date-order --author-date-order --topo-order --reverse --simplify-by-decoration --full-history --ancestry-path --show-pulls --walk-reflogs --color --no-color -z -- go.mod go.sum", + }, + "log oneline": { + &LogOpts{ + Oneline: true, + MaxCount: 10, + }, + "--max-count=10 --oneline", + }, + "log with author and grep": { + &LogOpts{ + Author: "example@hashicorp.com", + Grep: "fix", + RegexpIgnoreCase: true, + NoMerges: true, + }, + "--author=example@hashicorp.com --grep=fix --regexp-ignore-case --no-merges", + }, + "log graph with decoration": { + &LogOpts{ + Graph: true, + Oneline: true, + All: true, + Decorate: LogDecorateFormatShort, + }, + "--all --oneline --decorate=short --graph", + }, + "log with pathspec": { + &LogOpts{ + Patch: true, + Follow: true, + PathSpec: []string{"path/to/file.go"}, + }, + "--patch --follow -- path/to/file.go", + }, + "log date range": { + &LogOpts{ + Since: "2024-01-01", + Until: "2024-12-31", + Pretty: LogPrettyFormatShort, + NoMerges: true, + }, + "--since=2024-01-01 --until=2024-12-31 --no-merges --pretty=short", + }, + "log pretty none": { + &LogOpts{ + Pretty: LogPrettyFormatNone, + }, + "--pretty=", + }, + "log with diff-filter multiple": { + &LogOpts{ + DiffFilter: []LogDiffFilter{ + LogDiffFilterAdded, + LogDiffFilterModified, + LogDiffFilterDeleted, + }, + NameStatus: true, + MaxCount: 10, + }, + "--max-count=10 --name-status --diff-filter=AMD", + }, + "log with diff-filter added": { + &LogOpts{ + DiffFilter: []LogDiffFilter{LogDiffFilterAdded}, + NameOnly: true, + }, + "--name-only --diff-filter=A", + }, + "log with diff-filter copied": { + &LogOpts{ + DiffFilter: []LogDiffFilter{LogDiffFilterCopied}, + Stat: true, + }, + "--stat --diff-filter=C", + }, + "log with diff-filter deleted": { + &LogOpts{ + DiffFilter: []LogDiffFilter{LogDiffFilterDeleted}, + NameStatus: true, + }, + "--name-status --diff-filter=D", + }, + "log with diff-filter modified": { + &LogOpts{ + DiffFilter: []LogDiffFilter{LogDiffFilterModified}, + Patch: true, + }, + "--patch --diff-filter=M", + }, + "log with diff-filter renamed": { + &LogOpts{ + DiffFilter: []LogDiffFilter{LogDiffFilterRenamed}, + NameOnly: true, + }, + "--name-only --diff-filter=R", + }, + "log with diff-filter type-changed": { + &LogOpts{ + DiffFilter: []LogDiffFilter{LogDiffFilterTypeChanged}, + NameStatus: true, + }, + "--name-status --diff-filter=T", + }, + "log with diff-filter unmerged": { + &LogOpts{ + DiffFilter: []LogDiffFilter{LogDiffFilterUnmerged}, + Stat: true, + }, + "--stat --diff-filter=U", + }, + "log with diff-filter unknown": { + &LogOpts{ + DiffFilter: []LogDiffFilter{LogDiffFilterUnknown}, + NameOnly: true, + }, + "--name-only --diff-filter=X", + }, + "log with diff-filter broken": { + &LogOpts{ + DiffFilter: []LogDiffFilter{LogDiffFilterBroken}, + NameStatus: true, + }, + "--name-status --diff-filter=B", + }, + "log with diff-filter all": { + &LogOpts{ + DiffFilter: []LogDiffFilter{LogDiffFilterAll}, + Stat: true, + }, + "--stat --diff-filter=*", + }, + "log with diff-filter all types": { + &LogOpts{ + DiffFilter: []LogDiffFilter{ + LogDiffFilterAdded, + LogDiffFilterCopied, + LogDiffFilterDeleted, + LogDiffFilterModified, + LogDiffFilterRenamed, + LogDiffFilterTypeChanged, + LogDiffFilterUnmerged, + LogDiffFilterUnknown, + LogDiffFilterBroken, + }, + NameStatus: true, + }, + "--name-status --diff-filter=ACDMRTUXB", + }, } { t.Run(name, func(t *testing.T) { t.Parallel() diff --git a/tools/pipeline/internal/pkg/git/pull.go b/tools/pipeline/internal/pkg/git/client/pull.go similarity index 99% rename from tools/pipeline/internal/pkg/git/pull.go rename to tools/pipeline/internal/pkg/git/client/pull.go index 49ec20db8d..c6948d79b8 100644 --- a/tools/pipeline/internal/pkg/git/pull.go +++ b/tools/pipeline/internal/pkg/git/client/pull.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/push.go b/tools/pipeline/internal/pkg/git/client/push.go similarity index 99% rename from tools/pipeline/internal/pkg/git/push.go rename to tools/pipeline/internal/pkg/git/client/push.go index 344a1adf64..d193c32f21 100644 --- a/tools/pipeline/internal/pkg/git/push.go +++ b/tools/pipeline/internal/pkg/git/client/push.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/rebase.go b/tools/pipeline/internal/pkg/git/client/rebase.go similarity index 99% rename from tools/pipeline/internal/pkg/git/rebase.go rename to tools/pipeline/internal/pkg/git/client/rebase.go index 5a91dd9665..be1bb08838 100644 --- a/tools/pipeline/internal/pkg/git/rebase.go +++ b/tools/pipeline/internal/pkg/git/client/rebase.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/remote.go b/tools/pipeline/internal/pkg/git/client/remote.go similarity index 99% rename from tools/pipeline/internal/pkg/git/remote.go rename to tools/pipeline/internal/pkg/git/client/remote.go index 66a03faec3..72ed152fd9 100644 --- a/tools/pipeline/internal/pkg/git/remote.go +++ b/tools/pipeline/internal/pkg/git/client/remote.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/reset.go b/tools/pipeline/internal/pkg/git/client/reset.go similarity index 99% rename from tools/pipeline/internal/pkg/git/reset.go rename to tools/pipeline/internal/pkg/git/client/reset.go index 1c54996543..0e4d20fc0f 100644 --- a/tools/pipeline/internal/pkg/git/reset.go +++ b/tools/pipeline/internal/pkg/git/client/reset.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/rm.go b/tools/pipeline/internal/pkg/git/client/rm.go similarity index 98% rename from tools/pipeline/internal/pkg/git/rm.go rename to tools/pipeline/internal/pkg/git/client/rm.go index 9fdc40d37d..28dcada2f3 100644 --- a/tools/pipeline/internal/pkg/git/rm.go +++ b/tools/pipeline/internal/pkg/git/client/rm.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/show.go b/tools/pipeline/internal/pkg/git/client/show.go similarity index 88% rename from tools/pipeline/internal/pkg/git/show.go rename to tools/pipeline/internal/pkg/git/client/show.go index 2f7ea4e2d3..18faa78834 100644 --- a/tools/pipeline/internal/pkg/git/show.go +++ b/tools/pipeline/internal/pkg/git/client/show.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" @@ -36,6 +36,8 @@ type ShowOpts struct { DiffAlgorithm DiffAlgorithm // --diff-algorithm= DiffMerges DiffMergeFormat // --diff-merges= Format string // --format + Pretty LogPrettyFormat // --pretty + NameOnly bool // --name-only NoColor bool // --no-color NoPatch bool // --no-patch Patch bool // --patch @@ -78,6 +80,18 @@ func (o *ShowOpts) Strings() []string { opts = append(opts, fmt.Sprintf("--format=%s", string(o.Format))) } + if o.Pretty != "" { + if o.Pretty == LogPrettyFormatNone { + opts = append(opts, "--pretty=") + } else { + opts = append(opts, fmt.Sprintf("--pretty=%s", string(o.Pretty))) + } + } + + if o.NameOnly { + opts = append(opts, "--name-only") + } + if o.NoColor { opts = append(opts, "--no-color") } diff --git a/tools/pipeline/internal/pkg/git/status.go b/tools/pipeline/internal/pkg/git/client/status.go similarity index 99% rename from tools/pipeline/internal/pkg/git/status.go rename to tools/pipeline/internal/pkg/git/client/status.go index 1a0cd102d9..ddfb220bde 100644 --- a/tools/pipeline/internal/pkg/git/status.go +++ b/tools/pipeline/internal/pkg/git/client/status.go @@ -1,7 +1,7 @@ // Copyright IBM Corp. 2016, 2025 // SPDX-License-Identifier: BUSL-1.1 -package git +package client import ( "context" diff --git a/tools/pipeline/internal/pkg/git/doc.go b/tools/pipeline/internal/pkg/git/doc.go deleted file mode 100644 index 41e41ea0b7..0000000000 --- a/tools/pipeline/internal/pkg/git/doc.go +++ /dev/null @@ -1,5 +0,0 @@ -// Copyright IBM Corp. 2016, 2025 -// SPDX-License-Identifier: BUSL-1.1 - -// Package git is a Go modules that wraps the `git` CLI. -package git diff --git a/tools/pipeline/internal/pkg/git/list_changed_files.go b/tools/pipeline/internal/pkg/git/list_changed_files.go new file mode 100644 index 0000000000..81fe61c0c0 --- /dev/null +++ b/tools/pipeline/internal/pkg/git/list_changed_files.go @@ -0,0 +1,260 @@ +// Copyright IBM Corp. 2016, 2025 +// SPDX-License-Identifier: BUSL-1.1 + +package git + +import ( + "bufio" + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "maps" + "slices" + "strings" + + "github.com/hashicorp/vault/tools/pipeline/internal/pkg/changed" + libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git/client" + "github.com/jedib0t/go-pretty/v6/table" + slogctx "github.com/veqryn/slog-context" +) + +// ListChangedFilesReq holds the state and configuration for listing changed files +type ListChangedFilesReq struct { + // Branch specifies the branch to compare against + Branch string + // Range specifies the commit range to compare (e.g., HEAD~5..HEAD) + Range string + // Commit specifies a specific commit SHA to analyze + Commit string + // GroupFiles requests that changed groups are added to each file + GroupFiles bool + // Write a specially formatted response to $GITHUB_OUTPUT + WriteToGithubOutput bool +} + +// ListChangedFilesRes represents the response from listing changed files +type ListChangedFilesRes struct { + Files changed.Files `json:"files,omitempty"` + Groups changed.FileGroups `json:"groups,omitempty"` +} + +// ListChangedFilesGithubOutput is our GITHUB_OUTPUT type. It's a slimmed down +// type that only includes file names and groups. +type ListChangedFilesGithubOutput struct { + Files []string `json:"files,omitempty"` + Groups changed.FileGroups `json:"groups,omitempty"` +} + +// Run executes the git list changed files operation +func (g *ListChangedFilesReq) Run(ctx context.Context, client *libgit.Client) (*ListChangedFilesRes, error) { + slog.Default().DebugContext(ctx, "listing changed files from git") + + err := g.validate() + if err != nil { + return nil, err + } + + res := &ListChangedFilesRes{} + var execRes *libgit.ExecResponse + execRes, err = g.getChangedFilesFromGit(ctx, client) + if err != nil { + return nil, err + } + + res.Files, err = g.parseChangedFiles(ctx, client, execRes.Stdout) + if err != nil { + return nil, err + } + + if g.GroupFiles { + // Add group metadata to each file + changed.GroupFiles(ctx, res.Files, changed.DefaultFileGroupCheckers...) + + // Add the total unique set of groups + res.Groups = changed.Groups(res.Files) + } + + return res, nil +} + +// ToJSON marshals the response to JSON. +func (r *ListChangedFilesRes) ToJSON() ([]byte, error) { + if r == nil { + return nil, errors.New("uninitialized") + } + + b, err := json.Marshal(r) + if err != nil { + return nil, fmt.Errorf("marshaling list changed files to JSON: %w", err) + } + + return b, nil +} + +// ToGithubOutput writes a simplified list of changed files to be used $GITHUB_OUTPUT +func (r *ListChangedFilesRes) ToGithubOutput() ([]byte, error) { + if r == nil { + return nil, errors.New("uninitialized") + } + + res := &ListChangedFilesGithubOutput{ + Groups: r.Groups, + } + if f := r.Files; f != nil { + res.Files = f.Names() + } + + b, err := json.Marshal(res) + if err != nil { + return nil, fmt.Errorf("marshaling list changed files GITHUB_OUTPUT to JSON: %w", err) + } + + return b, nil +} + +// ToTable marshals the response to a text table. +func (r *ListChangedFilesRes) ToTable(groups bool) string { + if !groups { + w := strings.Builder{} + for _, name := range r.Files.Names() { + w.WriteString(name + "\n") + } + + return w.String() + } + + t := table.NewWriter() + t.Style().Options.DrawBorder = false + t.Style().Options.SeparateColumns = false + t.Style().Options.SeparateFooter = false + t.Style().Options.SeparateHeader = false + t.Style().Options.SeparateRows = false + t.AppendHeader(table.Row{"path", "groups"}) + for _, file := range r.Files { + t.AppendRow(table.Row{file.Name(), file.Groups.String()}) + } + t.SuppressEmptyColumns() + t.SuppressTrailingSpaces() + + return t.Render() +} + +// String returns a string representation of the response +func (r *ListChangedFilesRes) String() string { + if r == nil || len(r.Files) == 0 { + return "No changed files found" + } + + w := strings.Builder{} + for _, name := range r.Files.Names() { + w.WriteString(name + "\n") + } + return w.String() +} + +// validate checks that exactly one option is provided +func (g *ListChangedFilesReq) validate() error { + if g == nil { + return errors.New("uninitialized") + } + + // Validate that exactly one option is provided + optionsSet := 0 + if g.Branch != "" { + optionsSet++ + } + if g.Range != "" { + optionsSet++ + } + if g.Commit != "" { + optionsSet++ + } + + if optionsSet == 0 { + return errors.New("must specify one of: --branch, --range, or --commit") + } + + if optionsSet > 1 { + return errors.New("can only specify one of: --branch, --range, or --commit") + } + + return nil +} + +// parseChangedFiles parses the raw client output into changed.Files +func (g *ListChangedFilesReq) parseChangedFiles(ctx context.Context, client *libgit.Client, stdout []byte) (changed.Files, error) { + slog.Default().DebugContext(ctx, "parsing changed files from git client output") + scanner := bufio.NewScanner(bytes.NewReader(stdout)) + files := map[string]struct{}{} + changedFiles := changed.Files{} + + for scanner.Scan() { + files[scanner.Text()] = struct{}{} + } + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("parsing changed files: %w", err) + } + for _, file := range slices.Sorted(maps.Keys(files)) { + changedFiles = append(changedFiles, &changed.File{Filename: file}) + } + + return changedFiles, nil +} + +// getChangedFilesFromGit gets the raw changed files output from the git client +func (g *ListChangedFilesReq) getChangedFilesFromGit(ctx context.Context, client *libgit.Client) (*libgit.ExecResponse, error) { + if g == nil { + return nil, errors.New("uninitialized") + } + + switch { + case g.Branch != "": + ctx = slogctx.Append(ctx, slog.String("branch", g.Branch)) + slog.Default().DebugContext(ctx, "listing all changed files in branch") + + res, err := client.Log(ctx, &libgit.LogOpts{ + Target: g.Branch, // show all files for the branch + Pretty: libgit.LogPrettyFormatNone, // don't add extra formatting + NameOnly: true, // list only the names of the files + DiffFilter: []libgit.LogDiffFilter{libgit.LogDiffFilterAdded}, // only show added files + }) + if err != nil { + return res, fmt.Errorf("listing branch changed files: %s, %w", res.String(), err) + } + + return res, nil + case g.Range != "": + ctx = slogctx.Append(ctx, slog.String("range", g.Range)) + slog.Default().DebugContext(ctx, "listing all changed files in target range") + + res, err := client.Log(ctx, &libgit.LogOpts{ + Target: g.Range, // show all files for the range + Pretty: libgit.LogPrettyFormatNone, // don't add extra formatting + NameOnly: true, // list only the names of the files + }) + if err != nil { + return res, fmt.Errorf("listing range changed files: %s, %w", res.String(), err) + } + + return res, nil + case g.Commit != "": + ctx = slogctx.Append(ctx, slog.String("commit", g.Commit)) + slog.Default().DebugContext(ctx, "listing all changed files for commit") + + res, err := client.Show(ctx, &libgit.ShowOpts{ + Object: g.Commit, // show all files for the range + Pretty: libgit.LogPrettyFormatNone, // don't add extra formatting + NameOnly: true, // list only the names of the files + }) + if err != nil { + return res, fmt.Errorf("listing range changed files: %s, %w", res.String(), err) + } + + return res, nil + default: + return nil, fmt.Errorf("listing range changed files: no supported target provided") + } +} diff --git a/tools/pipeline/internal/pkg/github/check_go_mod_diff_request.go b/tools/pipeline/internal/pkg/github/check_go_mod_diff_request.go index 27c56843bb..0009a4b56c 100644 --- a/tools/pipeline/internal/pkg/github/check_go_mod_diff_request.go +++ b/tools/pipeline/internal/pkg/github/check_go_mod_diff_request.go @@ -12,7 +12,7 @@ import ( "os" libgithub "github.com/google/go-github/v81/github" - libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git" + libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git/client" "github.com/hashicorp/vault/tools/pipeline/internal/pkg/golang" "github.com/jedib0t/go-pretty/v6/table" "github.com/pmezard/go-difflib/difflib" diff --git a/tools/pipeline/internal/pkg/github/copy_pull_request.go b/tools/pipeline/internal/pkg/github/copy_pull_request.go index cadf68a7f7..5f9c2effe3 100644 --- a/tools/pipeline/internal/pkg/github/copy_pull_request.go +++ b/tools/pipeline/internal/pkg/github/copy_pull_request.go @@ -16,7 +16,7 @@ import ( "strings" libgithub "github.com/google/go-github/v81/github" - libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git" + libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git/client" "github.com/jedib0t/go-pretty/v6/table" slogctx "github.com/veqryn/slog-context" ) diff --git a/tools/pipeline/internal/pkg/github/create_backport.go b/tools/pipeline/internal/pkg/github/create_backport.go index dba3011cad..d63a46c71a 100644 --- a/tools/pipeline/internal/pkg/github/create_backport.go +++ b/tools/pipeline/internal/pkg/github/create_backport.go @@ -17,7 +17,7 @@ import ( libgithub "github.com/google/go-github/v81/github" "github.com/hashicorp/vault/tools/pipeline/internal/pkg/changed" - libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git" + libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git/client" "github.com/hashicorp/vault/tools/pipeline/internal/pkg/releases" "github.com/jedib0t/go-pretty/v6/table" slogctx "github.com/veqryn/slog-context" diff --git a/tools/pipeline/internal/pkg/github/create_backport_test.go b/tools/pipeline/internal/pkg/github/create_backport_test.go index 872cf33527..ca8b516709 100644 --- a/tools/pipeline/internal/pkg/github/create_backport_test.go +++ b/tools/pipeline/internal/pkg/github/create_backport_test.go @@ -290,7 +290,7 @@ func TestCreateBackportReq_shouldSkipRef(t *testing.T) { allowedInactiveCEChangedFiles := &ListChangedFilesRes{ Files: changed.Files{ { - File: &libgithub.CommitFile{ + GithubCommitFile: &libgithub.CommitFile{ SHA: libgithub.Ptr("84e0b544965861a7c6373e639cb13755512f84f4"), Filename: libgithub.Ptr("changelog/_2837.md"), }, @@ -305,14 +305,14 @@ func TestCreateBackportReq_shouldSkipRef(t *testing.T) { onlyEnterpriseChangedFiles := &ListChangedFilesRes{ Files: changed.Files{ { - File: &libgithub.CommitFile{ + GithubCommitFile: &libgithub.CommitFile{ SHA: libgithub.Ptr("84e0b544965861a7c6373e639cb13755512f84f4"), Filename: libgithub.Ptr(".github/workflows/build-artifacts-ent.yml"), }, Groups: changed.FileGroups{"enterprise", "pipeline"}, }, { - File: &libgithub.CommitFile{ + GithubCommitFile: &libgithub.CommitFile{ SHA: libgithub.Ptr("84e0b544965861a7c6373e639cb13755512f84f4"), Filename: libgithub.Ptr("vault/vault_ent/go.mod"), }, @@ -327,14 +327,14 @@ func TestCreateBackportReq_shouldSkipRef(t *testing.T) { mixedCEAndEnterpriseChangedFiles := &ListChangedFilesRes{ Files: changed.Files{ { - File: &libgithub.CommitFile{ + GithubCommitFile: &libgithub.CommitFile{ SHA: libgithub.Ptr("e1c10eae02e13f5a090b9c29b0b1a3003e8ca7f6"), Filename: libgithub.Ptr("go.mod"), }, Groups: changed.FileGroups{"app", "gotoolchain"}, }, { - File: &libgithub.CommitFile{ + GithubCommitFile: &libgithub.CommitFile{ SHA: libgithub.Ptr("a6397662ea1d5fdde744ff3e4246377cf369197a"), Filename: libgithub.Ptr("vault_ent/go.mod"), }, @@ -349,14 +349,14 @@ func TestCreateBackportReq_shouldSkipRef(t *testing.T) { allCEChangedFiles := &ListChangedFilesRes{ Files: changed.Files{ { - File: &libgithub.CommitFile{ + GithubCommitFile: &libgithub.CommitFile{ SHA: libgithub.Ptr("84e0b544965861a7c6373e639cb13755512f84f4"), Filename: libgithub.Ptr(".github/workflows/build.yml"), }, Groups: changed.FileGroups{"pipeline"}, }, { - File: &libgithub.CommitFile{ + GithubCommitFile: &libgithub.CommitFile{ SHA: libgithub.Ptr("84e0b544965861a7c6373e639cb13755512f84f4"), Filename: libgithub.Ptr("go.mod"), }, diff --git a/tools/pipeline/internal/pkg/github/list_changed_files.go b/tools/pipeline/internal/pkg/github/list_changed_files.go index d64ccdd7c8..e2228a4dfd 100644 --- a/tools/pipeline/internal/pkg/github/list_changed_files.go +++ b/tools/pipeline/internal/pkg/github/list_changed_files.go @@ -68,13 +68,11 @@ func (r *ListChangedFilesReq) Run(ctx context.Context, client *gh.Client) (*List } if r.GroupFiles { + // Add group metadata to each file changed.GroupFiles(ctx, res.Files, changed.DefaultFileGroupCheckers...) - res.Groups = changed.FileGroups{} - for _, file := range res.Files { - for _, group := range file.Groups { - res.Groups = res.Groups.Add(group) - } - } + + // Add the total unique set of groups + res.Groups = changed.Groups(res.Files) } return res, nil @@ -114,7 +112,7 @@ func (r *ListChangedFilesReq) getCommitFiles(ctx context.Context, client *gh.Cli } for _, f := range commit.Files { - files = append(files, &changed.File{File: f}) + files = append(files, &changed.File{GithubCommitFile: f}) } if res.NextPage == 0 { @@ -136,7 +134,7 @@ func (r *ListChangedFilesReq) getPullFiles(ctx context.Context, client *gh.Clien } for _, f := range fl { - files = append(files, &changed.File{File: f}) + files = append(files, &changed.File{GithubCommitFile: f}) } if res.NextPage == 0 { diff --git a/tools/pipeline/internal/pkg/github/noop_commit.go b/tools/pipeline/internal/pkg/github/noop_commit.go index 6d00a71c2d..7bcdd7caac 100644 --- a/tools/pipeline/internal/pkg/github/noop_commit.go +++ b/tools/pipeline/internal/pkg/github/noop_commit.go @@ -8,7 +8,7 @@ import ( "fmt" "log/slog" - libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git" + libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git/client" slogctx "github.com/veqryn/slog-context" ) diff --git a/tools/pipeline/internal/pkg/github/repo.go b/tools/pipeline/internal/pkg/github/repo.go index 289d9b30d0..a0e06b304e 100644 --- a/tools/pipeline/internal/pkg/github/repo.go +++ b/tools/pipeline/internal/pkg/github/repo.go @@ -11,7 +11,7 @@ import ( "os" "strings" - libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git" + libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git/client" slogctx "github.com/veqryn/slog-context" ) diff --git a/tools/pipeline/internal/pkg/github/sync_branch_request.go b/tools/pipeline/internal/pkg/github/sync_branch_request.go index 2f9cd074c7..1960cb9ee7 100644 --- a/tools/pipeline/internal/pkg/github/sync_branch_request.go +++ b/tools/pipeline/internal/pkg/github/sync_branch_request.go @@ -13,7 +13,7 @@ import ( "path/filepath" libgithub "github.com/google/go-github/v81/github" - libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git" + libgit "github.com/hashicorp/vault/tools/pipeline/internal/pkg/git/client" "github.com/jedib0t/go-pretty/v6/table" slogctx "github.com/veqryn/slog-context" )