-
Notifications
You must be signed in to change notification settings - Fork 147
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Support skipping stages by path pattern or commit message prefix (#4922)
* rename func to distinguish the trigger of skip stage Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Draft: impl check of skipping stage Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Rename to 'prefixes' Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * separate to sub funcs Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Impl Repo.GetCommitFromRev Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Impl skipByPathPattern() Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Add skipOn options to Wait Stage Options Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Add check of skipping stage in Analysis,Wait,WaitApproval Stages Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Fix comments Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * draft: add an empty test file Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * fix comments Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Fix func name Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Rename struct from confusing one Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Refine order of args Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * add test of hasOnlyPathsToSkip Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Refine testcase names Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * rename 'expected' to 'skip' Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Fix testcases Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * separate func Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * add test for skipByCommitMessagePrefixes Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Moved package to new 'skipstage' Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Add SkipOptions to ScriptRunStageOptions Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * rename 'SkipOptions' to 'SkipOn' to match with yaml Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Move skip logic to scheduler Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Add missing reporting Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * fix nits Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Fix check logic Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Add missing error handling Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Move skipstage to controller Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Update docs Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * gen mock by 'make gen/code' Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * nits: fix from 'fromCmd' to 'byCmd' Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * nits: rename func Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * nits: refine configuration-reference Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * fix error logging of zap.Error(err) Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * fix the flow in error handling Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Fix: check skipping before creating executor Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Remove unused func 'GetCommitHashForRev' Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * fix: remove unnecessary formatting Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * refactor: quit separating 'skipstage' package Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * fix: make 'checkSkipStage()' private after refactoring Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Fix: rename test func Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Refactor: merge func 'checkSkipStage' into 'shouldSkipStage' Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Merge funcs Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Add TestSkipByCommitMessagePrefixes with mock Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Refactore: merge funcs and use mock Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> --------- Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>
- Loading branch information
Showing
9 changed files
with
367 additions
and
51 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| // Copyright 2024 The PipeCD Authors. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package controller | ||
|
|
||
| import ( | ||
| "context" | ||
| "strings" | ||
|
|
||
| "github.com/pipe-cd/pipecd/pkg/app/piped/executor" | ||
| "github.com/pipe-cd/pipecd/pkg/config" | ||
| "github.com/pipe-cd/pipecd/pkg/filematcher" | ||
| "github.com/pipe-cd/pipecd/pkg/git" | ||
| "github.com/pipe-cd/pipecd/pkg/model" | ||
| ) | ||
|
|
||
| // checkSkipStage checks whether the stage should be skipped or not. | ||
| func (s *scheduler) shouldSkipStage(ctx context.Context, in executor.Input) (skip bool, err error) { | ||
| stageConfig := in.StageConfig | ||
| var skipOptions config.SkipOptions | ||
| switch stageConfig.Name { | ||
| case model.StageAnalysis: | ||
| skipOptions = stageConfig.AnalysisStageOptions.SkipOn | ||
| case model.StageWait: | ||
| skipOptions = stageConfig.WaitStageOptions.SkipOn | ||
| case model.StageWaitApproval: | ||
| skipOptions = stageConfig.WaitApprovalStageOptions.SkipOn | ||
| case model.StageScriptRun: | ||
| skipOptions = stageConfig.ScriptRunStageOptions.SkipOn | ||
| default: | ||
| return false, nil | ||
| } | ||
|
|
||
| if len(skipOptions.Paths) == 0 && len(skipOptions.CommitMessagePrefixes) == 0 { | ||
| // When no condition is specified. | ||
| return false, nil | ||
| } | ||
|
|
||
| repoCfg := in.Application.GitPath.Repo | ||
| repo, err := in.GitClient.Clone(ctx, repoCfg.Id, repoCfg.Remote, repoCfg.Branch, "") | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| // Check by path pattern | ||
| skip, err = skipByPathPattern(ctx, skipOptions, repo, in.RunningDSP.Revision(), in.TargetDSP.Revision()) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| if skip { | ||
| return true, nil | ||
| } | ||
|
|
||
| // Check by prefix of commit message | ||
| skip, err = skipByCommitMessagePrefixes(ctx, skipOptions, repo, in.TargetDSP.Revision()) | ||
| return skip, err | ||
| } | ||
|
|
||
| // skipByCommitMessagePrefixes returns true if the commit message has ANY one of the specified prefixes. | ||
| func skipByCommitMessagePrefixes(ctx context.Context, opt config.SkipOptions, repo git.Repo, targetRev string) (skip bool, err error) { | ||
| if len(opt.CommitMessagePrefixes) == 0 { | ||
| return false, nil | ||
| } | ||
|
|
||
| commit, err := repo.GetCommitForRev(ctx, targetRev) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| for _, prefix := range opt.CommitMessagePrefixes { | ||
| if strings.HasPrefix(commit.Message, prefix) { | ||
| return true, nil | ||
| } | ||
| } | ||
| return false, nil | ||
| } | ||
|
|
||
| // skipByPathPattern returns true if and only if ALL changed files are included in `opt.Paths`. | ||
| // If ANY changed file does not match all `skipPatterns`, it returns false. | ||
| func skipByPathPattern(ctx context.Context, opt config.SkipOptions, repo git.Repo, runningRev, targetRev string) (skip bool, err error) { | ||
| if len(opt.Paths) == 0 { | ||
| return false, nil | ||
| } | ||
|
|
||
| changedFiles, err := repo.ChangedFiles(ctx, runningRev, targetRev) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| matcher, err := filematcher.NewPatternMatcher(opt.Paths) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| for _, changedFile := range changedFiles { | ||
| if !matcher.Matches(changedFile) { | ||
| return false, nil | ||
| } | ||
| } | ||
|
|
||
| return true, nil | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| // Copyright 2024 The PipeCD Authors. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package controller | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/golang/mock/gomock" | ||
| "github.com/pipe-cd/pipecd/pkg/config" | ||
| "github.com/pipe-cd/pipecd/pkg/git" | ||
| "github.com/pipe-cd/pipecd/pkg/git/gittest" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestSkipByCommitMessagePrefixes(t *testing.T) { | ||
| t.Parallel() | ||
| testcases := []struct { | ||
| name string | ||
| commitMessage string | ||
| prefixes []string | ||
| skip bool | ||
| }{ | ||
| { | ||
| name: "no prefixes", | ||
| commitMessage: "test message", | ||
| prefixes: []string{}, | ||
| skip: false, | ||
| }, | ||
| { | ||
| name: "no commit message", | ||
| commitMessage: "", | ||
| prefixes: []string{"to-skip"}, | ||
| skip: false, | ||
| }, | ||
| { | ||
| name: "prefix matches", | ||
| commitMessage: "to-skip: test message", | ||
| prefixes: []string{"to-skip"}, | ||
| skip: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testcases { | ||
| tc := tc | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| ctrl := gomock.NewController(t) | ||
| defer ctrl.Finish() | ||
| repoMock := gittest.NewMockRepo(ctrl) | ||
| repoMock.EXPECT().GetCommitForRev(gomock.Any(), gomock.Any()).Return(git.Commit{ | ||
| Message: tc.commitMessage, | ||
| }, nil).AnyTimes() | ||
|
|
||
| opt := config.SkipOptions{ | ||
| CommitMessagePrefixes: tc.prefixes, | ||
| } | ||
| skip, err := skipByCommitMessagePrefixes(context.Background(), opt, repoMock, "test-rev") | ||
| assert.Equal(t, tc.skip, skip) | ||
| assert.NoError(t, err) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestSkipByPathPattern(t *testing.T) { | ||
| t.Parallel() | ||
| testcases := []struct { | ||
| name string | ||
| skipPatterns []string | ||
| changedFiles []string | ||
| skip bool | ||
| }{ | ||
| { | ||
| name: "no skip patterns", | ||
| skipPatterns: nil, | ||
| changedFiles: []string{"file1"}, | ||
| skip: false, | ||
| }, | ||
| { | ||
| name: "no changed files", | ||
| skipPatterns: []string{"file1"}, | ||
| changedFiles: nil, | ||
| skip: true, | ||
| }, | ||
| { | ||
| name: "no skip patterns and no changed files", | ||
| skipPatterns: nil, | ||
| changedFiles: nil, | ||
| skip: false, | ||
| }, | ||
| { | ||
| name: "skip pattern matches all changed files", | ||
| skipPatterns: []string{"file1", "file2"}, | ||
| changedFiles: []string{"file1", "file2"}, | ||
| skip: true, | ||
| }, | ||
| { | ||
| name: "skip pattern does not match changed files", | ||
| skipPatterns: []string{"file1", "file2"}, | ||
| changedFiles: []string{"file1", "file3"}, | ||
| skip: false, | ||
| }, | ||
| { | ||
| name: "skip files of a directory", | ||
| skipPatterns: []string{"dir1/*"}, | ||
| changedFiles: []string{"dir1/file1", "dir1/file2"}, | ||
| skip: true, | ||
| }, | ||
| { | ||
| name: "skip files recursively", | ||
| skipPatterns: []string{"dir1/**"}, | ||
| changedFiles: []string{"dir1/file1", "dir1/sub/file2"}, | ||
| skip: true, | ||
| }, | ||
| { | ||
| name: "skip files with the extension recursively", | ||
| skipPatterns: []string{"dir1/**/*.yaml"}, | ||
| changedFiles: []string{"dir1/file1.yaml", "dir1/sub1/file2.yaml", "dir1/sub1/sub2/file3.yaml"}, | ||
| skip: true, | ||
| }, | ||
| { | ||
| name: "skip files not recursively", | ||
| skipPatterns: []string{"*.yaml"}, | ||
| changedFiles: []string{"dir1/file1.yaml"}, | ||
| skip: false, | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testcases { | ||
| tc := tc | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| // We do not use t.Parallel() here due to https://pkg.go.dev/github.com/pipe-cd/pipecd/pkg/filematcher#PatternMatcher.Matches. | ||
| ctrl := gomock.NewController(t) | ||
| defer ctrl.Finish() | ||
| repoMock := gittest.NewMockRepo(ctrl) | ||
| repoMock.EXPECT().ChangedFiles(gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.changedFiles, nil).AnyTimes() | ||
|
|
||
| opt := config.SkipOptions{ | ||
| Paths: tc.skipPatterns, | ||
| } | ||
| actual, err := skipByPathPattern(context.Background(), opt, repoMock, "running-rev", "target-rev") | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, tc.skip, actual) | ||
| }) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.