diff --git a/pkg/app/piped/planpreview/builder.go b/pkg/app/piped/planpreview/builder.go index 859a0bf9a9..f9d5f6d148 100644 --- a/pkg/app/piped/planpreview/builder.go +++ b/pkg/app/piped/planpreview/builder.go @@ -305,7 +305,7 @@ func (b *builder) cloneHeadCommit(ctx context.Context, headBranch, headCommit st } func (b *builder) findTriggerApps(ctx context.Context, repo git.Repo, apps []*model.Application, headCommit string) (triggerApps []*model.Application, failedResults []*model.ApplicationPlanPreviewResult, err error) { - d := trigger.NewDeterminer(repo, headCommit, b.commitGetter, b.logger) + d := trigger.NewDeterminer(repo, headCommit, b.commitGetter, true, b.logger) for _, app := range apps { shouldTrigger, err := d.ShouldTrigger(ctx, app) if err != nil { diff --git a/pkg/app/piped/trigger/determiner.go b/pkg/app/piped/trigger/determiner.go index 34cfbe0ffa..5758f681f8 100644 --- a/pkg/app/piped/trigger/determiner.go +++ b/pkg/app/piped/trigger/determiner.go @@ -37,15 +37,19 @@ type Determiner struct { repo git.Repo targetCommit string commitGetter LastTriggeredCommitGetter - logger *zap.Logger + // Flag `ignoreUserConfig` set to `true` will force check changes and use it to determine + // the application deployment should be triggered or not, regardless of the user's configuration. + ignoreUserConfig bool + logger *zap.Logger } -func NewDeterminer(repo git.Repo, targetCommit string, cg LastTriggeredCommitGetter, logger *zap.Logger) *Determiner { +func NewDeterminer(repo git.Repo, targetCommit string, cg LastTriggeredCommitGetter, ignoreUserConfig bool, logger *zap.Logger) *Determiner { return &Determiner{ - repo: repo, - targetCommit: targetCommit, - commitGetter: cg, - logger: logger.Named("determiner"), + repo: repo, + targetCommit: targetCommit, + commitGetter: cg, + ignoreUserConfig: ignoreUserConfig, + logger: logger.Named("determiner"), } } @@ -57,6 +61,22 @@ func (d *Determiner) ShouldTrigger(ctx context.Context, app *model.Application) zap.String("target-commit", d.targetCommit), ) + // TODO: Add logic to determine trigger or not based on other configuration than onCommit. + return d.shouldTriggerOnCommit(ctx, app, logger) +} + +func (d *Determiner) shouldTriggerOnCommit(ctx context.Context, app *model.Application, logger *zap.Logger) (bool, error) { + deployConfig, err := loadDeploymentConfiguration(d.repo.GetPath(), app) + if err != nil { + return false, err + } + + // Not trigger in case users disable auto trigger deploy on change and the user config is unignorable. + if deployConfig.Trigger.OnCommit.Disabled && !d.ignoreUserConfig { + logger.Info(fmt.Sprintf("auto trigger deployment disabled for application, hash: %s", d.targetCommit)) + return false, nil + } + preCommit, err := d.commitGetter.Get(ctx, app.Id) if err != nil { logger.Error("failed to get last triggered commit", zap.Error(err)) @@ -84,12 +104,25 @@ func (d *Determiner) ShouldTrigger(ctx context.Context, app *model.Application) return false, err } - deployConfig, err := loadDeploymentConfiguration(d.repo.GetPath(), app) - if err != nil { - return false, err + // TODO: Remove deprecated `deployConfig.TriggerPaths` configuration. + checkingPaths := make([]string, 0, len(deployConfig.Trigger.OnCommit.Paths)+len(deployConfig.TriggerPaths)) + // Note: deployConfig.TriggerPaths or deployConfig.Trigger.OnCommit.Paths may contain "" (empty string) + // in case users use one of them without the other, that cause unexpected "" path in the checkingPaths list + // leads to always trigger deployment since "" path matched all other paths. + // The below logic is to remove that "" path from checking path list, will remove after remove the + // deprecated deployConfig.TriggerPaths. + for _, p := range deployConfig.Trigger.OnCommit.Paths { + if p != "" { + checkingPaths = append(checkingPaths, p) + } + } + for _, p := range deployConfig.TriggerPaths { + if p != "" { + checkingPaths = append(checkingPaths, p) + } } - touched, err := isTouchedByChangedFiles(app.GitPath.Path, deployConfig.TriggerPaths, changedFiles) + touched, err := isTouchedByChangedFiles(app.GitPath.Path, checkingPaths, changedFiles) if err != nil { return false, err } diff --git a/pkg/app/piped/trigger/trigger.go b/pkg/app/piped/trigger/trigger.go index 7b3831a75c..1a348640d8 100644 --- a/pkg/app/piped/trigger/trigger.go +++ b/pkg/app/piped/trigger/trigger.go @@ -227,7 +227,7 @@ func (t *Trigger) checkNewCommits(ctx context.Context) error { if err != nil { continue } - d := NewDeterminer(gitRepo, headCommit.Hash, t.commitStore, t.logger) + d := NewDeterminer(gitRepo, headCommit.Hash, t.commitStore, false, t.logger) for _, app := range apps { shouldTrigger, err := d.ShouldTrigger(ctx, app) diff --git a/pkg/config/deployment.go b/pkg/config/deployment.go index 02186ead28..e9659c0435 100644 --- a/pkg/config/deployment.go +++ b/pkg/config/deployment.go @@ -39,7 +39,10 @@ type GenericDeploymentSpec struct { SealedSecrets []SealedSecretMapping `json:"sealedSecrets"` // List of directories or files where their changes will trigger the deployment. // Regular expression can be used. + // Deprecated: use Trigger.Paths instead. TriggerPaths []string `json:"triggerPaths,omitempty"` + // The trigger configuration use to determine trigger logic. + Trigger Trigger `json:"trigger"` // The maximum length of time to execute deployment before giving up. // Default is 6h. Timeout Duration `json:"timeout,omitempty" default:"6h"` @@ -55,6 +58,19 @@ type DeploymentPlanner struct { AlwaysUsePipeline bool `json:"alwaysUsePipeline"` } +type Trigger struct { + // Configuration in case changes on commit are found. + OnCommit OnCommit `json:"onCommit"` +} + +type OnCommit struct { + // Control trigger new deployment on Git change or not. + Disabled bool `json:"disabled,omitempty"` + // List of directories or files where their changes will trigger the deployment. + // Regular expression can be used. + Paths []string `json:"paths,omitempty"` +} + func (s *GenericDeploymentSpec) Validate() error { if s.Pipeline != nil { for _, stage := range s.Pipeline.Stages { diff --git a/pkg/config/deployment_cloudrun_test.go b/pkg/config/deployment_cloudrun_test.go index 14956b5777..23ff9bf0f3 100644 --- a/pkg/config/deployment_cloudrun_test.go +++ b/pkg/config/deployment_cloudrun_test.go @@ -37,6 +37,11 @@ func TestCloudRunDeploymentConfig(t *testing.T) { expectedSpec: &CloudRunDeploymentSpec{ GenericDeploymentSpec: GenericDeploymentSpec{ Timeout: Duration(6 * time.Hour), + Trigger: Trigger{ + OnCommit: OnCommit{ + Disabled: false, + }, + }, }, Input: CloudRunDeploymentInput{ AutoRollback: true, diff --git a/pkg/config/deployment_ecs_test.go b/pkg/config/deployment_ecs_test.go index b0ac78da60..d62a32dafa 100644 --- a/pkg/config/deployment_ecs_test.go +++ b/pkg/config/deployment_ecs_test.go @@ -38,6 +38,11 @@ func TestECSDeploymentConfig(t *testing.T) { expectedSpec: &ECSDeploymentSpec{ GenericDeploymentSpec: GenericDeploymentSpec{ Timeout: Duration(6 * time.Hour), + Trigger: Trigger{ + OnCommit: OnCommit{ + Disabled: false, + }, + }, }, Input: ECSDeploymentInput{ ServiceDefinitionFile: "/path/to/servicedef.yaml", diff --git a/pkg/config/deployment_kubernetes_test.go b/pkg/config/deployment_kubernetes_test.go index eb0e5081fa..e3f681efae 100644 --- a/pkg/config/deployment_kubernetes_test.go +++ b/pkg/config/deployment_kubernetes_test.go @@ -79,6 +79,11 @@ func TestKubernetesDeploymentConfig(t *testing.T) { }, }, Timeout: Duration(6 * time.Hour), + Trigger: Trigger{ + OnCommit: OnCommit{ + Disabled: false, + }, + }, }, Input: KubernetesDeploymentInput{ AutoRollback: true, diff --git a/pkg/config/deployment_lambda_test.go b/pkg/config/deployment_lambda_test.go index de79f56648..3835938cc5 100644 --- a/pkg/config/deployment_lambda_test.go +++ b/pkg/config/deployment_lambda_test.go @@ -37,6 +37,11 @@ func TestLambdaDeploymentConfig(t *testing.T) { expectedSpec: &LambdaDeploymentSpec{ GenericDeploymentSpec: GenericDeploymentSpec{ Timeout: Duration(6 * time.Hour), + Trigger: Trigger{ + OnCommit: OnCommit{ + Disabled: false, + }, + }, }, Input: LambdaDeploymentInput{ FunctionManifestFile: "function.yaml", diff --git a/pkg/config/deployment_terraform_test.go b/pkg/config/deployment_terraform_test.go index f0170a0429..a79ef43209 100644 --- a/pkg/config/deployment_terraform_test.go +++ b/pkg/config/deployment_terraform_test.go @@ -39,6 +39,11 @@ func TestTerraformDeploymentConfig(t *testing.T) { expectedSpec: &TerraformDeploymentSpec{ GenericDeploymentSpec: GenericDeploymentSpec{ Timeout: Duration(6 * time.Hour), + Trigger: Trigger{ + OnCommit: OnCommit{ + Disabled: false, + }, + }, }, Input: TerraformDeploymentInput{}, }, @@ -51,6 +56,11 @@ func TestTerraformDeploymentConfig(t *testing.T) { expectedSpec: &TerraformDeploymentSpec{ GenericDeploymentSpec: GenericDeploymentSpec{ Timeout: Duration(6 * time.Hour), + Trigger: Trigger{ + OnCommit: OnCommit{ + Disabled: false, + }, + }, }, Input: TerraformDeploymentInput{ Workspace: "dev", @@ -73,6 +83,11 @@ func TestTerraformDeploymentConfig(t *testing.T) { }, }, Timeout: Duration(6 * time.Hour), + Trigger: Trigger{ + OnCommit: OnCommit{ + Disabled: false, + }, + }, }, Input: TerraformDeploymentInput{ Workspace: "dev", @@ -108,6 +123,11 @@ func TestTerraformDeploymentConfig(t *testing.T) { }, }, Timeout: Duration(6 * time.Hour), + Trigger: Trigger{ + OnCommit: OnCommit{ + Disabled: false, + }, + }, }, Input: TerraformDeploymentInput{ Workspace: "dev", diff --git a/pkg/config/deployment_test.go b/pkg/config/deployment_test.go index d7a4cb328e..b0ea97872d 100644 --- a/pkg/config/deployment_test.go +++ b/pkg/config/deployment_test.go @@ -16,8 +16,10 @@ package config import ( "testing" + "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/pipe-cd/pipe/pkg/model" ) @@ -251,3 +253,47 @@ func TestValidateMentions(t *testing.T) { }) } } + +func TestGenericTriggerConfiguration(t *testing.T) { + testcases := []struct { + fileName string + expectedKind Kind + expectedAPIVersion string + expectedSpec interface{} + expectedError error + }{ + { + fileName: "testdata/application/generic-trigger.yaml", + expectedKind: KindKubernetesApp, + expectedAPIVersion: "pipecd.dev/v1beta1", + expectedSpec: &KubernetesDeploymentSpec{ + GenericDeploymentSpec: GenericDeploymentSpec{ + Timeout: Duration(6 * time.Hour), + Trigger: Trigger{ + OnCommit: OnCommit{ + Disabled: false, + Paths: []string{ + "deployment.yaml", + }, + }, + }, + }, + Input: KubernetesDeploymentInput{ + AutoRollback: true, + }, + }, + expectedError: nil, + }, + } + for _, tc := range testcases { + t.Run(tc.fileName, func(t *testing.T) { + cfg, err := LoadFromYAML(tc.fileName) + require.Equal(t, tc.expectedError, err) + if err == nil { + assert.Equal(t, tc.expectedKind, cfg.Kind) + assert.Equal(t, tc.expectedAPIVersion, cfg.APIVersion) + assert.Equal(t, tc.expectedSpec, cfg.spec) + } + }) + } +} diff --git a/pkg/config/testdata/application/generic-trigger.yaml b/pkg/config/testdata/application/generic-trigger.yaml new file mode 100644 index 0000000000..2b1aea2f75 --- /dev/null +++ b/pkg/config/testdata/application/generic-trigger.yaml @@ -0,0 +1,7 @@ +apiVersion: pipecd.dev/v1beta1 +kind: KubernetesApp +spec: + trigger: + onCommit: + paths: + - deployment.yaml