From 82fed0b0d8b94c7f4691e7f1573d46e09f263fbc Mon Sep 17 00:00:00 2001 From: emanuelbesliu <32497562+emanuelbesliu@users.noreply.github.com> Date: Tue, 12 May 2026 11:49:18 +0300 Subject: [PATCH 01/23] feat: extend autodiscover.ignore_paths to targeted -d commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, autodiscover.ignore_paths only filtered projects during auto-discovery (plan-all) and apply-all. Targeted commands like 'atlantis plan -d ' and 'atlantis apply -d ' bypassed this filtering entirely, causing issues in multi-instance setups where each Atlantis instance manages a different directory subtree. This change adds ignore_paths checking to buildProjectPlanCommand and buildProjectCommand (the targeted command handlers). When a user runs a targeted command against a path that matches ignore_paths and has no explicit project configuration in atlantis.yaml, the command is silently skipped (returns no projects). Paths with explicit project config are NOT affected — the ignore only applies to paths that would otherwise be auto-discovered. Fixes #6460 Signed-off-by: emanuelbesliu <32497562+emanuelbesliu@users.noreply.github.com> --- .../docs/repo-level-atlantis-yaml.md | 7 + .../docs/server-side-repo-config.md | 4 +- server/events/project_command_builder.go | 42 +++ server/events/project_command_builder_test.go | 272 ++++++++++++++++++ 4 files changed, 324 insertions(+), 1 deletion(-) diff --git a/runatlantis.io/docs/repo-level-atlantis-yaml.md b/runatlantis.io/docs/repo-level-atlantis-yaml.md index b0fbc2c7c8..b6ecc35138 100644 --- a/runatlantis.io/docs/repo-level-atlantis-yaml.md +++ b/runatlantis.io/docs/repo-level-atlantis-yaml.md @@ -418,6 +418,13 @@ autodiscover: Autodiscover can also be configured to skip over directories that match a path glob (as defined by the [doublestar path matching package](https://pkg.go.dev/github.com/bmatcuk/doublestar/v4)). +When `ignore_paths` is set, it applies to: +- Automatic project discovery during autoplan and `atlantis plan` (without `-d`) +- `atlantis apply` (without `-d`) when filtering pending plans +- Targeted commands like `atlantis plan -d ` and `atlantis apply -d `, if the path has no explicit project configuration + +This makes `ignore_paths` useful for **multi-instance setups** where each Atlantis instance manages a different directory subtree. For example, one instance can ignore `environments/prod/**` while another ignores `environments/nonprod/**`, preventing cross-instance interference on targeted commands. + ### Custom Backend Config See [Custom Workflow Use Cases: Custom Backend Config](custom-workflows.md#custom-backend-config) diff --git a/runatlantis.io/docs/server-side-repo-config.md b/runatlantis.io/docs/server-side-repo-config.md index bb422533b0..9b9b1e110b 100644 --- a/runatlantis.io/docs/server-side-repo-config.md +++ b/runatlantis.io/docs/server-side-repo-config.md @@ -100,7 +100,9 @@ repos: # If any part of this setting is set here, it overrides the entire setting in the repo config. autodiscover: mode: auto - # Optionally ignore some paths for autodiscovery by a glob path + # Optionally ignore some paths for autodiscovery by a glob path. + # Also applies to targeted commands (e.g. atlantis plan -d ) + # when the path has no explicit project configuration. ignore_paths: - foo/* diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 7aed29dca9..2c14d6f409 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -748,6 +748,28 @@ func (p *DefaultProjectCommandBuilder) buildProjectPlanCommand(ctx *command.Cont repoRelDir = cmd.RepoRelDir } + // Check ignore_paths for targeted plan commands against non-configured + // paths. This prevents targeted 'atlantis plan -d ' from running + // on paths the operator intended to exclude (e.g. multi-instance setups). + // Paths with explicit project config in atlantis.yaml are not affected. + if cmd.ProjectName == "" { + repoCfg, _, repoCfgErr := p.parseRepoCfg(ctx, defaultRepoDir) + if repoCfgErr == nil { + cleanDir := filepath.Clean(repoRelDir) + hasExplicitCfg := false + for _, proj := range repoCfg.Projects { + if filepath.Clean(proj.Dir) == cleanDir { + hasExplicitCfg = true + break + } + } + if !hasExplicitCfg && p.isAutoDiscoverPathIgnored(ctx, repoCfg, cleanDir) { + ctx.Log.Debug("ignoring targeted plan for dir '%s' due to autodiscover.ignore_paths", repoRelDir) + return pcc, nil + } + } + } + return p.buildProjectCommandCtx( ctx, command.Plan, @@ -934,6 +956,26 @@ func (p *DefaultProjectCommandBuilder) buildProjectCommand(ctx *command.Context, repoRelDir = cmd.RepoRelDir } + // Same ignore_paths check as buildProjectPlanCommand for targeted commands. + // Paths with explicit project config in atlantis.yaml are not affected. + if cmd.ProjectName == "" { + repoCfg, _, repoCfgErr := p.parseRepoCfg(ctx, repoDir) + if repoCfgErr == nil { + cleanDir := filepath.Clean(repoRelDir) + hasExplicitCfg := false + for _, proj := range repoCfg.Projects { + if filepath.Clean(proj.Dir) == cleanDir { + hasExplicitCfg = true + break + } + } + if !hasExplicitCfg && p.isAutoDiscoverPathIgnored(ctx, repoCfg, cleanDir) { + ctx.Log.Debug("ignoring targeted command for dir '%s' due to autodiscover.ignore_paths", repoRelDir) + return projCtx, nil + } + } + } + return p.buildProjectCommandCtx( ctx, cmd.Name, diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index f6af414b64..6594849562 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -682,6 +682,278 @@ projects: } } +// Test that autodiscover.ignore_paths blocks targeted plan/apply -d commands +// when the directory has no explicit project config (global config ignore_paths). +func TestDefaultProjectCommandBuilder_BuildTargetedCommand_IgnorePaths(t *testing.T) { + RegisterMockTestingT(t) + + tmpDir := DirStructure(t, map[string]any{ + "environments": map[string]any{ + "prod": map[string]any{ + "main.tf": nil, + }, + "nonprod": map[string]any{ + "main.tf": nil, + }, + }, + }) + + workingDir := mocks.NewMockWorkingDir() + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmpDir, nil) + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, nil) + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn([]string{"environments/prod/main.tf", "environments/nonprod/main.tf"}, nil) + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + + globalCfgArgs := valid.GlobalCfgArgs{AllowAllRepoSettings: true} + globalCfg := valid.NewGlobalCfgFromArgs(globalCfgArgs) + globalCfg.Repos[0].AutoDiscover = &valid.AutoDiscover{ + Mode: valid.AutoDiscoverEnabledMode, + IgnorePaths: []string{"environments/prod/**"}, + } + + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + + cmdCtx := &command.Context{Log: logger, Scope: scope} + + // Targeted plan -d to ignored path should return no projects + planCtxs, err := builder.BuildPlanCommands(cmdCtx, &events.CommentCommand{ + Name: command.Plan, + RepoRelDir: "environments/prod", + Workspace: "default", + }) + Ok(t, err) + Equals(t, 0, len(planCtxs)) + + // Targeted plan -d to non-ignored path should succeed + planCtxs, err = builder.BuildPlanCommands(cmdCtx, &events.CommentCommand{ + Name: command.Plan, + RepoRelDir: "environments/nonprod", + Workspace: "default", + }) + Ok(t, err) + Equals(t, 1, len(planCtxs)) + Equals(t, "environments/nonprod", planCtxs[0].RepoRelDir) + + // Targeted apply -d to ignored path should return no projects + applyCtxs, err := builder.BuildApplyCommands(cmdCtx, &events.CommentCommand{ + Name: command.Apply, + RepoRelDir: "environments/prod", + Workspace: "default", + }) + Ok(t, err) + Equals(t, 0, len(applyCtxs)) + + // Targeted apply -d to non-ignored path should succeed + applyCtxs, err = builder.BuildApplyCommands(cmdCtx, &events.CommentCommand{ + Name: command.Apply, + RepoRelDir: "environments/nonprod", + Workspace: "default", + }) + Ok(t, err) + Equals(t, 1, len(applyCtxs)) + Equals(t, "environments/nonprod", applyCtxs[0].RepoRelDir) +} + +// Test that autodiscover.ignore_paths set in repo-level atlantis.yaml blocks +// targeted plan/apply -d commands for non-configured projects. +func TestDefaultProjectCommandBuilder_BuildTargetedCommand_IgnorePathsRepoCfg(t *testing.T) { + RegisterMockTestingT(t) + + atlantisYAML := ` +version: 3 +autodiscover: + mode: enabled + ignore_paths: + - "environments/prod/**" +` + tmpDir := DirStructure(t, map[string]any{ + "environments": map[string]any{ + "prod": map[string]any{ + "main.tf": nil, + }, + "nonprod": map[string]any{ + "main.tf": nil, + }, + }, + }) + err := os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(atlantisYAML), 0600) + Ok(t, err) + + workingDir := mocks.NewMockWorkingDir() + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmpDir, nil) + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, nil) + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn([]string{"environments/prod/main.tf", "environments/nonprod/main.tf"}, nil) + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + + globalCfgArgs := valid.GlobalCfgArgs{AllowAllRepoSettings: true} + globalCfg := valid.NewGlobalCfgFromArgs(globalCfgArgs) + + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + + cmdCtx := &command.Context{Log: logger, Scope: scope} + + // Targeted plan -d to ignored path should return no projects + planCtxs, err := builder.BuildPlanCommands(cmdCtx, &events.CommentCommand{ + Name: command.Plan, + RepoRelDir: "environments/prod", + Workspace: "default", + }) + Ok(t, err) + Equals(t, 0, len(planCtxs)) + + // Non-ignored path should work + planCtxs, err = builder.BuildPlanCommands(cmdCtx, &events.CommentCommand{ + Name: command.Plan, + RepoRelDir: "environments/nonprod", + Workspace: "default", + }) + Ok(t, err) + Equals(t, 1, len(planCtxs)) + Equals(t, "environments/nonprod", planCtxs[0].RepoRelDir) +} + +// Test that targeted -d commands to a path with an explicit project config +// are NOT blocked by ignore_paths. +func TestDefaultProjectCommandBuilder_BuildTargetedCommand_IgnorePathsExplicitProjectAllowed(t *testing.T) { + RegisterMockTestingT(t) + + atlantisYAML := ` +version: 3 +autodiscover: + mode: enabled + ignore_paths: + - "environments/prod/**" +projects: +- name: prod-project + dir: environments/prod +` + tmpDir := DirStructure(t, map[string]any{ + "environments": map[string]any{ + "prod": map[string]any{ + "main.tf": nil, + }, + }, + }) + err := os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(atlantisYAML), 0600) + Ok(t, err) + + workingDir := mocks.NewMockWorkingDir() + When(workingDir.Clone(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), + Any[string]())).ThenReturn(tmpDir, nil) + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn(tmpDir, nil) + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), + Any[models.PullRequest]())).ThenReturn([]string{"environments/prod/main.tf"}, nil) + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + + globalCfgArgs := valid.GlobalCfgArgs{AllowAllRepoSettings: true} + globalCfg := valid.NewGlobalCfgFromArgs(globalCfgArgs) + + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + + cmdCtx := &command.Context{Log: logger, Scope: scope} + + // Targeted plan -d to ignored path with explicit config should succeed + planCtxs, err := builder.BuildPlanCommands(cmdCtx, &events.CommentCommand{ + Name: command.Plan, + RepoRelDir: "environments/prod", + Workspace: "default", + }) + Ok(t, err) + Equals(t, 1, len(planCtxs)) + Equals(t, "prod-project", planCtxs[0].ProjectName) +} + // Test building a plan and apply command for one project // with the RestrictFileList func TestDefaultProjectCommandBuilder_BuildSinglePlanApplyCommand_WithRestrictFileList(t *testing.T) { From baedc6eeaa00517de82746fd239541e48b8a1e15 Mon Sep 17 00:00:00 2001 From: emanuelbesliu <32497562+emanuelbesliu@users.noreply.github.com> Date: Tue, 12 May 2026 12:00:34 +0300 Subject: [PATCH 02/23] refactor: extract shouldIgnoreTargetedDir helper and gate behind autoDiscoverModeEnabled Address Copilot review feedback on PR #6466: - Extract duplicated ignore_paths logic into shouldIgnoreTargetedDir() - Gate the check behind autoDiscoverModeEnabled() for consistency - Add targeted apply -d assertions to repo-level config test Signed-off-by: emanuelbesliu <32497562+emanuelbesliu@users.noreply.github.com> --- server/events/project_command_builder.go | 65 ++++++++----------- server/events/project_command_builder_test.go | 19 ++++++ 2 files changed, 46 insertions(+), 38 deletions(-) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 2c14d6f409..55cb46d087 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -412,6 +412,27 @@ func (p *DefaultProjectCommandBuilder) parseRepoCfg(ctx *command.Context, repoDi return repoCfg, true, nil } +// shouldIgnoreTargetedDir checks whether a targeted -d command should be +// silently skipped because the path matches autodiscover.ignore_paths. It only +// applies when autodiscovery is active and the directory has no explicit project +// config in atlantis.yaml. +func (p *DefaultProjectCommandBuilder) shouldIgnoreTargetedDir(ctx *command.Context, repoDir string, repoRelDir string) bool { + repoCfg, _, err := p.parseRepoCfg(ctx, repoDir) + if err != nil { + return false + } + if !p.autoDiscoverModeEnabled(ctx, repoCfg) { + return false + } + cleanDir := filepath.Clean(repoRelDir) + for _, proj := range repoCfg.Projects { + if filepath.Clean(proj.Dir) == cleanDir { + return false // explicit project config overrides ignore_paths + } + } + return p.isAutoDiscoverPathIgnored(ctx, repoCfg, cleanDir) +} + // isAutoDiscoverPathIgnored determines whether this particular path is ignored for the purposes of auto discovery. // Global config ignore_paths takes precedence when explicitly set; otherwise falls through to repo config. func (p *DefaultProjectCommandBuilder) isAutoDiscoverPathIgnored(ctx *command.Context, repoCfg valid.RepoCfg, path string) bool { @@ -748,26 +769,9 @@ func (p *DefaultProjectCommandBuilder) buildProjectPlanCommand(ctx *command.Cont repoRelDir = cmd.RepoRelDir } - // Check ignore_paths for targeted plan commands against non-configured - // paths. This prevents targeted 'atlantis plan -d ' from running - // on paths the operator intended to exclude (e.g. multi-instance setups). - // Paths with explicit project config in atlantis.yaml are not affected. - if cmd.ProjectName == "" { - repoCfg, _, repoCfgErr := p.parseRepoCfg(ctx, defaultRepoDir) - if repoCfgErr == nil { - cleanDir := filepath.Clean(repoRelDir) - hasExplicitCfg := false - for _, proj := range repoCfg.Projects { - if filepath.Clean(proj.Dir) == cleanDir { - hasExplicitCfg = true - break - } - } - if !hasExplicitCfg && p.isAutoDiscoverPathIgnored(ctx, repoCfg, cleanDir) { - ctx.Log.Debug("ignoring targeted plan for dir '%s' due to autodiscover.ignore_paths", repoRelDir) - return pcc, nil - } - } + if cmd.ProjectName == "" && p.shouldIgnoreTargetedDir(ctx, defaultRepoDir, repoRelDir) { + ctx.Log.Debug("ignoring targeted plan for dir '%s' due to autodiscover.ignore_paths", repoRelDir) + return pcc, nil } return p.buildProjectCommandCtx( @@ -956,24 +960,9 @@ func (p *DefaultProjectCommandBuilder) buildProjectCommand(ctx *command.Context, repoRelDir = cmd.RepoRelDir } - // Same ignore_paths check as buildProjectPlanCommand for targeted commands. - // Paths with explicit project config in atlantis.yaml are not affected. - if cmd.ProjectName == "" { - repoCfg, _, repoCfgErr := p.parseRepoCfg(ctx, repoDir) - if repoCfgErr == nil { - cleanDir := filepath.Clean(repoRelDir) - hasExplicitCfg := false - for _, proj := range repoCfg.Projects { - if filepath.Clean(proj.Dir) == cleanDir { - hasExplicitCfg = true - break - } - } - if !hasExplicitCfg && p.isAutoDiscoverPathIgnored(ctx, repoCfg, cleanDir) { - ctx.Log.Debug("ignoring targeted command for dir '%s' due to autodiscover.ignore_paths", repoRelDir) - return projCtx, nil - } - } + if cmd.ProjectName == "" && p.shouldIgnoreTargetedDir(ctx, repoDir, repoRelDir) { + ctx.Log.Debug("ignoring targeted command for dir '%s' due to autodiscover.ignore_paths", repoRelDir) + return projCtx, nil } return p.buildProjectCommandCtx( diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 6594849562..7775f80627 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -872,6 +872,25 @@ autodiscover: Ok(t, err) Equals(t, 1, len(planCtxs)) Equals(t, "environments/nonprod", planCtxs[0].RepoRelDir) + + // Targeted apply -d to ignored path should return no projects + applyCtxs, err := builder.BuildApplyCommands(cmdCtx, &events.CommentCommand{ + Name: command.Apply, + RepoRelDir: "environments/prod", + Workspace: "default", + }) + Ok(t, err) + Equals(t, 0, len(applyCtxs)) + + // Targeted apply -d to non-ignored path should work + applyCtxs, err = builder.BuildApplyCommands(cmdCtx, &events.CommentCommand{ + Name: command.Apply, + RepoRelDir: "environments/nonprod", + Workspace: "default", + }) + Ok(t, err) + Equals(t, 1, len(applyCtxs)) + Equals(t, "environments/nonprod", applyCtxs[0].RepoRelDir) } // Test that targeted -d commands to a path with an explicit project config From 353e2f7e23109befe9148c59a593f35f5b18df33 Mon Sep 17 00:00:00 2001 From: emanuelbesliu <32497562+emanuelbesliu@users.noreply.github.com> Date: Tue, 12 May 2026 12:14:03 +0300 Subject: [PATCH 03/23] fix: address second round of Copilot review feedback - Skip ignore_paths check when -d uses glob patterns - Fix doc comment to reference repo config file generically - Add apply assertions to ExplicitProjectAllowed test Signed-off-by: emanuelbesliu <32497562+emanuelbesliu@users.noreply.github.com> --- server/events/project_command_builder.go | 7 +++++-- server/events/project_command_builder_test.go | 10 ++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 55cb46d087..faebb22685 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -415,8 +415,11 @@ func (p *DefaultProjectCommandBuilder) parseRepoCfg(ctx *command.Context, repoDi // shouldIgnoreTargetedDir checks whether a targeted -d command should be // silently skipped because the path matches autodiscover.ignore_paths. It only // applies when autodiscovery is active and the directory has no explicit project -// config in atlantis.yaml. +// config in the repo config file. func (p *DefaultProjectCommandBuilder) shouldIgnoreTargetedDir(ctx *command.Context, repoDir string, repoRelDir string) bool { + if valid.ContainsDirGlobPattern(repoRelDir) { + return false + } repoCfg, _, err := p.parseRepoCfg(ctx, repoDir) if err != nil { return false @@ -427,7 +430,7 @@ func (p *DefaultProjectCommandBuilder) shouldIgnoreTargetedDir(ctx *command.Cont cleanDir := filepath.Clean(repoRelDir) for _, proj := range repoCfg.Projects { if filepath.Clean(proj.Dir) == cleanDir { - return false // explicit project config overrides ignore_paths + return false } } return p.isAutoDiscoverPathIgnored(ctx, repoCfg, cleanDir) diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 7775f80627..e54d7cae41 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -971,6 +971,16 @@ projects: Ok(t, err) Equals(t, 1, len(planCtxs)) Equals(t, "prod-project", planCtxs[0].ProjectName) + + // Targeted apply -d to ignored path with explicit config should succeed + applyCtxs, err := builder.BuildApplyCommands(cmdCtx, &events.CommentCommand{ + Name: command.Apply, + RepoRelDir: "environments/prod", + Workspace: "default", + }) + Ok(t, err) + Equals(t, 1, len(applyCtxs)) + Equals(t, "prod-project", applyCtxs[0].ProjectName) } // Test building a plan and apply command for one project From 5908fd43ffb6227a49343a18939f002d6c35a4f8 Mon Sep 17 00:00:00 2001 From: emanuelbesliu <32497562+emanuelbesliu@users.noreply.github.com> Date: Tue, 12 May 2026 21:29:32 +0300 Subject: [PATCH 04/23] docs: clarify ignore_paths applies to all targeted -d commands Update docs to reflect that ignore_paths filtering covers all targeted -d commands (plan, apply, import, state rm, etc.), not just plan/apply. Signed-off-by: emanuelbesliu <32497562+emanuelbesliu@users.noreply.github.com> --- runatlantis.io/docs/repo-level-atlantis-yaml.md | 2 +- runatlantis.io/docs/server-side-repo-config.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/runatlantis.io/docs/repo-level-atlantis-yaml.md b/runatlantis.io/docs/repo-level-atlantis-yaml.md index b6ecc35138..a9948af8c4 100644 --- a/runatlantis.io/docs/repo-level-atlantis-yaml.md +++ b/runatlantis.io/docs/repo-level-atlantis-yaml.md @@ -421,7 +421,7 @@ Autodiscover can also be configured to skip over directories that match a path g When `ignore_paths` is set, it applies to: - Automatic project discovery during autoplan and `atlantis plan` (without `-d`) - `atlantis apply` (without `-d`) when filtering pending plans -- Targeted commands like `atlantis plan -d ` and `atlantis apply -d `, if the path has no explicit project configuration +- All targeted `-d` commands (`plan`, `apply`, `import`, `state rm`, etc.), if the path has no explicit project configuration This makes `ignore_paths` useful for **multi-instance setups** where each Atlantis instance manages a different directory subtree. For example, one instance can ignore `environments/prod/**` while another ignores `environments/nonprod/**`, preventing cross-instance interference on targeted commands. diff --git a/runatlantis.io/docs/server-side-repo-config.md b/runatlantis.io/docs/server-side-repo-config.md index 9b9b1e110b..54c009e066 100644 --- a/runatlantis.io/docs/server-side-repo-config.md +++ b/runatlantis.io/docs/server-side-repo-config.md @@ -101,7 +101,7 @@ repos: autodiscover: mode: auto # Optionally ignore some paths for autodiscovery by a glob path. - # Also applies to targeted commands (e.g. atlantis plan -d ) + # Also applies to all targeted -d commands (plan, apply, import, etc.) # when the path has no explicit project configuration. ignore_paths: - foo/* From ec1a774296a26408724aaec97cdb0b26013fc864 Mon Sep 17 00:00:00 2001 From: emanuelbesliu <32497562+emanuelbesliu@users.noreply.github.com> Date: Tue, 12 May 2026 21:53:34 +0300 Subject: [PATCH 05/23] docs: clarify ignore_paths applies only when autodiscovery is enabled Signed-off-by: emanuelbesliu <32497562+emanuelbesliu@users.noreply.github.com> --- runatlantis.io/docs/repo-level-atlantis-yaml.md | 2 +- runatlantis.io/docs/server-side-repo-config.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/runatlantis.io/docs/repo-level-atlantis-yaml.md b/runatlantis.io/docs/repo-level-atlantis-yaml.md index a9948af8c4..6d923b16a6 100644 --- a/runatlantis.io/docs/repo-level-atlantis-yaml.md +++ b/runatlantis.io/docs/repo-level-atlantis-yaml.md @@ -421,7 +421,7 @@ Autodiscover can also be configured to skip over directories that match a path g When `ignore_paths` is set, it applies to: - Automatic project discovery during autoplan and `atlantis plan` (without `-d`) - `atlantis apply` (without `-d`) when filtering pending plans -- All targeted `-d` commands (`plan`, `apply`, `import`, `state rm`, etc.), if the path has no explicit project configuration +- All targeted `-d` commands (`plan`, `apply`, `import`, `state rm`, etc.) when autodiscovery is enabled, if the path has no explicit project configuration This makes `ignore_paths` useful for **multi-instance setups** where each Atlantis instance manages a different directory subtree. For example, one instance can ignore `environments/prod/**` while another ignores `environments/nonprod/**`, preventing cross-instance interference on targeted commands. diff --git a/runatlantis.io/docs/server-side-repo-config.md b/runatlantis.io/docs/server-side-repo-config.md index 54c009e066..675fb2b539 100644 --- a/runatlantis.io/docs/server-side-repo-config.md +++ b/runatlantis.io/docs/server-side-repo-config.md @@ -101,8 +101,8 @@ repos: autodiscover: mode: auto # Optionally ignore some paths for autodiscovery by a glob path. - # Also applies to all targeted -d commands (plan, apply, import, etc.) - # when the path has no explicit project configuration. + # When autodiscovery is enabled, also applies to all targeted -d commands + # (plan, apply, import, etc.) when the path has no explicit project configuration. ignore_paths: - foo/* From 4050a68fc1a950730e9518176c98854d9a3e35ff Mon Sep 17 00:00:00 2001 From: Emanuel Besliu <32497562+emanuelbesliu@users.noreply.github.com> Date: Tue, 23 Jun 2026 13:26:56 +0300 Subject: [PATCH 06/23] docs: fix MD032 blanks-around-lists in repo-level-atlantis-yaml Signed-off-by: Emanuel Besliu <32497562+emanuelbesliu@users.noreply.github.com> --- runatlantis.io/docs/repo-level-atlantis-yaml.md | 1 + 1 file changed, 1 insertion(+) diff --git a/runatlantis.io/docs/repo-level-atlantis-yaml.md b/runatlantis.io/docs/repo-level-atlantis-yaml.md index 6d923b16a6..14e4414eb3 100644 --- a/runatlantis.io/docs/repo-level-atlantis-yaml.md +++ b/runatlantis.io/docs/repo-level-atlantis-yaml.md @@ -419,6 +419,7 @@ autodiscover: Autodiscover can also be configured to skip over directories that match a path glob (as defined by the [doublestar path matching package](https://pkg.go.dev/github.com/bmatcuk/doublestar/v4)). When `ignore_paths` is set, it applies to: + - Automatic project discovery during autoplan and `atlantis plan` (without `-d`) - `atlantis apply` (without `-d`) when filtering pending plans - All targeted `-d` commands (`plan`, `apply`, `import`, `state rm`, etc.) when autodiscovery is enabled, if the path has no explicit project configuration From 9fb766f072fcaf09001cbe4323057b7500aaa938 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 01:46:09 -0400 Subject: [PATCH 07/23] fix: honor ignored targeted dirs before restrict list Run the autodiscover.ignore_paths guard before restrict-file-list validation for targeted plan -d commands so ignored directories remain no-op even when they are not in the modified file list. Add regression coverage that verifies the ignored targeted dir returns no project contexts without calling the modified-file lookup. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/project_command_builder.go | 20 +++++------ server/events/project_command_builder_test.go | 35 +++++++++++++++++++ 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index faebb22685..1bd98b77e2 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -695,6 +695,16 @@ func (p *DefaultProjectCommandBuilder) buildProjectPlanCommand(ctx *command.Cont return pcc, err } + repoRelDir := DefaultRepoRelDir + if cmd.RepoRelDir != "" { + repoRelDir = cmd.RepoRelDir + } + + if cmd.ProjectName == "" && p.shouldIgnoreTargetedDir(ctx, defaultRepoDir, repoRelDir) { + ctx.Log.Debug("ignoring targeted plan for dir '%s' due to autodiscover.ignore_paths", repoRelDir) + return pcc, nil + } + if p.RestrictFileList { ctx.Log.Debug("'restrict-file-list' option is set, checking modified files") modifiedFiles, err := p.VCSClient.GetModifiedFiles(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull) @@ -767,16 +777,6 @@ func (p *DefaultProjectCommandBuilder) buildProjectPlanCommand(ctx *command.Cont } } - repoRelDir := DefaultRepoRelDir - if cmd.RepoRelDir != "" { - repoRelDir = cmd.RepoRelDir - } - - if cmd.ProjectName == "" && p.shouldIgnoreTargetedDir(ctx, defaultRepoDir, repoRelDir) { - ctx.Log.Debug("ignoring targeted plan for dir '%s' due to autodiscover.ignore_paths", repoRelDir) - return pcc, nil - } - return p.buildProjectCommandCtx( ctx, command.Plan, diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index e54d7cae41..819378bfc3 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -993,6 +993,8 @@ func TestDefaultProjectCommandBuilder_BuildSinglePlanApplyCommand_WithRestrictFi ModifiedFiles []string Cmd events.CommentCommand ExpErr string + ExpNoProjects bool + ExpSkipFileList bool }{ { Description: "planning a file outside of the changed files", @@ -1029,6 +1031,32 @@ func TestDefaultProjectCommandBuilder_BuildSinglePlanApplyCommand_WithRestrictFi }, ModifiedFiles: []string{"directory-1/main.tf"}, }, + { + Description: "planning an ignored targeted dir outside of the changed files", + Cmd: events.CommentCommand{ + Name: command.Plan, + RepoRelDir: "ignored", + Workspace: "default", + }, + AtlantisYAML: ` +version: 3 +autodiscover: + mode: enabled + ignore_paths: + - ignored/** +`, + DirectoryStructure: map[string]any{ + "ignored": map[string]any{ + "main.tf": nil, + }, + "directory-2": map[string]any{ + "main.tf": nil, + }, + }, + ModifiedFiles: []string{"directory-2/main.tf"}, + ExpNoProjects: true, + ExpSkipFileList: true, + }, { Description: "planning a project outside of the requested changed files", Cmd: events.CommentCommand{ @@ -1148,6 +1176,13 @@ projects: return } Ok(t, err) + if c.ExpNoProjects { + Equals(t, 0, len(actCtxs)) + if c.ExpSkipFileList { + vcsClient.VerifyWasCalled(Never()).GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]()) + } + return + } Equals(t, 1, len(actCtxs)) }) } From 76cf9ae37baf9fb10c67209c478147701fd93e65 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 09:13:00 -0400 Subject: [PATCH 08/23] fix: make ignored targeted dirs true no-ops Return a sentinel when targeted -d commands match autodiscover.ignore_paths so comment runners can distinguish ignored targets from ordinary zero-project results. Skip comments, combined status updates, and post-workflow hooks for ignored targeted plan/apply commands, and let API request parsing skip those targets instead of failing. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/controllers/api_controller.go | 5 +- server/events/apply_command_runner.go | 18 +++++++ server/events/apply_command_runner_test.go | 29 ++++++++++ server/events/command/context.go | 3 ++ server/events/command_runner.go | 32 ++--------- server/events/command_runner_test.go | 53 ++++++++++++++++++- server/events/plan_command_runner.go | 20 ++++++- server/events/plan_command_runner_test.go | 29 ++++++++++ server/events/project_command_builder.go | 10 +++- server/events/project_command_builder_test.go | 20 +++---- 10 files changed, 175 insertions(+), 44 deletions(-) diff --git a/server/controllers/api_controller.go b/server/controllers/api_controller.go index fbf953838f..e57dc6926b 100644 --- a/server/controllers/api_controller.go +++ b/server/controllers/api_controller.go @@ -82,7 +82,10 @@ func (a *APIRequest) getCommands(ctx *command.Context, cmdName command.Name, cmd for _, commentCommand := range cc { projectCmds, err := cmdBuilder(ctx, commentCommand) if err != nil { - return nil, nil, fmt.Errorf("failed to build command: %v", err) + if events.IsIgnoredTargetedDir(err) { + continue + } + return nil, nil, fmt.Errorf("failed to build command: %w", err) } cmds = append(cmds, projectCmds...) } diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go index b81641f31d..968a745be3 100644 --- a/server/events/apply_command_runner.go +++ b/server/events/apply_command_runner.go @@ -123,6 +123,11 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { var projectCmds []command.ProjectContext projectCmds, err = a.prjCmdBuilder.BuildApplyCommands(ctx, cmd) if err != nil { + if IsIgnoredTargetedDir(err) { + ctx.Log.Debug("ignoring targeted apply because the directory matches autodiscover.ignore_paths") + ctx.CommandSkipped = true + return + } if statusErr := a.commitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, cmd.CommandName()); statusErr != nil { ctx.Log.Warn("unable to update commit status: %s", statusErr) } @@ -164,6 +169,9 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { } return } + if len(projectCmds) > 0 { + a.updatePendingCommitStatus(ctx) + } result := runProjectCmdsWithCancellationTracker(ctx, projectCmds, a.cancellationTracker, a.parallelPoolSize, a.isParallelEnabled(projectCmds), a.prjCmdRunner.Apply) ctx.CommandHasErrors = result.HasErrors() @@ -186,6 +194,16 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { } } +func (a *ApplyCommandRunner) updatePendingCommitStatus(ctx *command.Context) { + if a.silenceVCSStatusNoProjects { + ctx.Log.Debug("silence enabled - not setting pending VCS status") + return + } + if err := a.commitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply); err != nil { + ctx.Log.Warn("unable to update commit status: %s", err) + } +} + func (a *ApplyCommandRunner) IsLocked() (bool, error) { lock, err := a.locker.CheckApplyLock() diff --git a/server/events/apply_command_runner_test.go b/server/events/apply_command_runner_test.go index dbcd833f4c..57552e729c 100644 --- a/server/events/apply_command_runner_test.go +++ b/server/events/apply_command_runner_test.go @@ -248,6 +248,35 @@ func TestApplyCommandRunner_IsSilenced(t *testing.T) { } } +func TestApplyCommandRunner_IgnoredTargetedDirNoOp(t *testing.T) { + RegisterMockTestingT(t) + vcsClient := setup(t) + scopeNull := metricstest.NewLoggingScope(t, logging.NewNoopLogger(t), "atlantis") + modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} + cmd := &events.CommentCommand{Name: command.Apply, RepoRelDir: "ignored"} + ctx := &command.Context{ + User: testdata.User, + Log: logging.NewNoopLogger(t), + Scope: scopeNull, + Pull: modelPull, + HeadRepo: testdata.GithubRepo, + Trigger: command.CommentTrigger, + } + + When(projectCommandBuilder.BuildApplyCommands(ctx, cmd)).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + + applyCommandRunner.Run(ctx, cmd) + Assert(t, ctx.CommandSkipped, "expected ignored targeted dir to mark the command skipped") + + vcsClient.VerifyWasCalled(Never()).CreateComment( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombined( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombinedCount( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name](), Any[models.ProjectCounts]()) + projectCommandRunner.VerifyWasCalled(Never()).Apply(Any[command.ProjectContext]()) +} + func TestApplyCommandRunner_ExecutionOrder(t *testing.T) { logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) diff --git a/server/events/command/context.go b/server/events/command/context.go index 7ccf923091..998d6cfa5b 100644 --- a/server/events/command/context.go +++ b/server/events/command/context.go @@ -55,4 +55,7 @@ type Context struct { // Set true if there were any errors during the command execution CommandHasErrors bool + + // Set true if the command was intentionally skipped without executing work. + CommandSkipped bool } diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 94eb585b40..0761630a0a 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -199,17 +199,6 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo Name: command.Autoplan, } - // Only set pending status if silence is not enabled - // The PlanCommandRunner will handle the final status decision based on project results - if !c.SilenceVCSStatusNoProjects { - // Update the combined plan commit status to pending - if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { - ctx.Log.Warn("unable to update plan commit status: %s", err) - } - } else { - ctx.Log.Debug("silence enabled - not setting pending VCS status") - } - preWorkflowHooksErr := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) if preWorkflowHooksErr != nil { @@ -500,24 +489,6 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead return } - // Only set pending status if silence is not enabled - // The command runners will handle the final status decision based on project results - if !c.SilenceVCSStatusNoProjects { - // Update the combined plan or apply commit status to pending - switch cmd.Name { - case command.Plan: - if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil { - ctx.Log.Warn("unable to update plan commit status: %s", err) - } - case command.Apply: - if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply); err != nil { - ctx.Log.Warn("unable to update apply commit status: %s", err) - } - } - } else { - ctx.Log.Debug("silence enabled - not setting pending VCS status") - } - preWorkflowHooksErr := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) if preWorkflowHooksErr != nil { @@ -551,6 +522,9 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) cmdRunner.Run(ctx, cmd) + if ctx.CommandSkipped { + return + } c.PostWorkflowHooksCommandRunner.RunPostHooks(ctx, cmd) // nolint: errcheck } diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 31b6e3e35f..3910107a8e 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -463,8 +463,57 @@ func TestRunCommentCommandApply_NoProjects_SilenceEnabled(t *testing.T) { ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Apply}) vcsClient.VerifyWasCalled(Never()).CreateComment( Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) - commitUpdater.VerifyWasCalledOnce().UpdateCombined( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Eq(models.PendingCommitStatus), Eq(command.Apply)) + commitUpdater.VerifyWasCalledOnce().UpdateCombinedCount( + Any[logging.SimpleLogging](), + Any[models.Repo](), + Any[models.PullRequest](), + Eq[models.CommitStatus](models.SuccessCommitStatus), + Eq[command.Name](command.Apply), + Eq(models.ProjectCounts{}), + ) +} + +func TestRunCommentCommand_IgnoredTargetedDirNoOp(t *testing.T) { + cases := []struct { + name string + command command.Name + }{ + { + name: "plan", + command: command.Plan, + }, + { + name: "apply", + command: command.Apply, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + vcsClient := setup(t) + pull := &github.PullRequest{State: github.Ptr("open")} + modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} + cmd := &events.CommentCommand{Name: c.command, RepoRelDir: "ignored"} + + When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) + When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) + if c.command == command.Plan { + When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + } else { + When(projectCommandBuilder.BuildApplyCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + } + + ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, cmd) + + vcsClient.VerifyWasCalled(Never()).CreateComment( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombined( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombinedCount( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name](), Any[models.ProjectCounts]()) + postWorkflowHooksCommandRunner.(*mocks.MockPostWorkflowHooksCommandRunner).VerifyWasCalled(Never()).RunPostHooks(Any[*command.Context](), Any[*events.CommentCommand]()) + }) + } } func TestRunCommentCommandApprovePolicy_NoProjects_SilenceEnabled(t *testing.T) { diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go index 63b242109a..f50af9807a 100644 --- a/server/events/plan_command_runner.go +++ b/server/events/plan_command_runner.go @@ -144,6 +144,7 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) { } return } + p.updatePendingCommitStatus(ctx, command.Plan) // discard previous plans that might not be relevant anymore ctx.Log.Debug("deleting previous plans and locks") @@ -213,6 +214,11 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) { projectCmds, err := p.prjCmdBuilder.BuildPlanCommands(ctx, cmd) if err != nil { + if IsIgnoredTargetedDir(err) { + ctx.Log.Debug("ignoring targeted plan because the directory matches autodiscover.ignore_paths") + ctx.CommandSkipped = true + return + } if statusErr := p.commitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Plan); statusErr != nil { ctx.Log.Warn("unable to update commit status: %s", statusErr) } @@ -262,8 +268,10 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) { } return } - projectCmds, policyCheckCmds := p.partitionProjectCmds(ctx, projectCmds) + if len(projectCmds) > 0 { + p.updatePendingCommitStatus(ctx, command.Plan) + } // if the plan is generic, new plans will be generated based on changes // discard previous plans that might not be relevant anymore @@ -328,6 +336,16 @@ func (p *PlanCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { } } +func (p *PlanCommandRunner) updatePendingCommitStatus(ctx *command.Context, commandName command.Name) { + if p.silenceVCSStatusNoProjects { + ctx.Log.Debug("silence enabled - not setting pending VCS status") + return + } + if err := p.commitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, commandName); err != nil { + ctx.Log.Warn("unable to update commit status: %s", err) + } +} + func (p *PlanCommandRunner) updateCommitStatus(ctx *command.Context, pullStatus models.PullStatus, commandName command.Name) { var numSuccess int var numErrored int diff --git a/server/events/plan_command_runner_test.go b/server/events/plan_command_runner_test.go index cb7e7a260f..449d10f149 100644 --- a/server/events/plan_command_runner_test.go +++ b/server/events/plan_command_runner_test.go @@ -163,6 +163,35 @@ func TestPlanCommandRunner_IsSilenced(t *testing.T) { } } +func TestPlanCommandRunner_IgnoredTargetedDirNoOp(t *testing.T) { + RegisterMockTestingT(t) + vcsClient := setup(t) + scopeNull := metricstest.NewLoggingScope(t, logging.NewNoopLogger(t), "atlantis") + modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} + cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored"} + ctx := &command.Context{ + User: testdata.User, + Log: logging.NewNoopLogger(t), + Scope: scopeNull, + Pull: modelPull, + HeadRepo: testdata.GithubRepo, + Trigger: command.CommentTrigger, + } + + When(projectCommandBuilder.BuildPlanCommands(ctx, cmd)).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + + planCommandRunner.Run(ctx, cmd) + Assert(t, ctx.CommandSkipped, "expected ignored targeted dir to mark the command skipped") + + vcsClient.VerifyWasCalled(Never()).CreateComment( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombined( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombinedCount( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name](), Any[models.ProjectCounts]()) + projectCommandRunner.VerifyWasCalled(Never()).Plan(Any[command.ProjectContext]()) +} + func TestPlanCommandRunner_ExecutionOrder(t *testing.T) { logger := logging.NewNoopLogger(t) RegisterMockTestingT(t) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 1bd98b77e2..a9238330ec 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -38,6 +38,12 @@ const ( DefaultAbortOnExecutionOrderFail = false ) +var ErrIgnoredTargetedDir = errors.New("targeted dir ignored by autodiscover.ignore_paths") + +func IsIgnoredTargetedDir(err error) bool { + return errors.Is(err, ErrIgnoredTargetedDir) +} + func NewInstrumentedProjectCommandBuilder( logger logging.SimpleLogging, policyChecksSupported bool, @@ -702,7 +708,7 @@ func (p *DefaultProjectCommandBuilder) buildProjectPlanCommand(ctx *command.Cont if cmd.ProjectName == "" && p.shouldIgnoreTargetedDir(ctx, defaultRepoDir, repoRelDir) { ctx.Log.Debug("ignoring targeted plan for dir '%s' due to autodiscover.ignore_paths", repoRelDir) - return pcc, nil + return pcc, ErrIgnoredTargetedDir } if p.RestrictFileList { @@ -965,7 +971,7 @@ func (p *DefaultProjectCommandBuilder) buildProjectCommand(ctx *command.Context, if cmd.ProjectName == "" && p.shouldIgnoreTargetedDir(ctx, repoDir, repoRelDir) { ctx.Log.Debug("ignoring targeted command for dir '%s' due to autodiscover.ignore_paths", repoRelDir) - return projCtx, nil + return projCtx, ErrIgnoredTargetedDir } return p.buildProjectCommandCtx( diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 819378bfc3..8c9455c55a 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -4,6 +4,7 @@ package events_test import ( + "errors" "os" "path/filepath" "sort" @@ -752,7 +753,7 @@ func TestDefaultProjectCommandBuilder_BuildTargetedCommand_IgnorePaths(t *testin RepoRelDir: "environments/prod", Workspace: "default", }) - Ok(t, err) + Assert(t, errors.Is(err, events.ErrIgnoredTargetedDir), "expected ignored targeted dir error, got %v", err) Equals(t, 0, len(planCtxs)) // Targeted plan -d to non-ignored path should succeed @@ -771,7 +772,7 @@ func TestDefaultProjectCommandBuilder_BuildTargetedCommand_IgnorePaths(t *testin RepoRelDir: "environments/prod", Workspace: "default", }) - Ok(t, err) + Assert(t, errors.Is(err, events.ErrIgnoredTargetedDir), "expected ignored targeted dir error, got %v", err) Equals(t, 0, len(applyCtxs)) // Targeted apply -d to non-ignored path should succeed @@ -860,7 +861,7 @@ autodiscover: RepoRelDir: "environments/prod", Workspace: "default", }) - Ok(t, err) + Assert(t, errors.Is(err, events.ErrIgnoredTargetedDir), "expected ignored targeted dir error, got %v", err) Equals(t, 0, len(planCtxs)) // Non-ignored path should work @@ -879,7 +880,7 @@ autodiscover: RepoRelDir: "environments/prod", Workspace: "default", }) - Ok(t, err) + Assert(t, errors.Is(err, events.ErrIgnoredTargetedDir), "expected ignored targeted dir error, got %v", err) Equals(t, 0, len(applyCtxs)) // Targeted apply -d to non-ignored path should work @@ -1171,18 +1172,19 @@ projects: Scope: scope, }, &cmd) - if c.ExpErr != "" { - ErrEquals(t, c.ExpErr, err) - return - } - Ok(t, err) if c.ExpNoProjects { + Assert(t, errors.Is(err, events.ErrIgnoredTargetedDir), "expected ignored targeted dir error, got %v", err) Equals(t, 0, len(actCtxs)) if c.ExpSkipFileList { vcsClient.VerifyWasCalled(Never()).GetModifiedFiles(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]()) } return } + if c.ExpErr != "" { + ErrEquals(t, c.ExpErr, err) + return + } + Ok(t, err) Equals(t, 1, len(actCtxs)) }) } From 29ced3b4f6b6fbd1e47c4dfd943b924aa962a785 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 09:23:57 -0400 Subject: [PATCH 09/23] fix: fail autoplan status on pre-hook errors Set the plan status to failed when autoplan stops on a pre-workflow hook error now that pending status is no longer set before command construction. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/command_runner.go | 12 ++---------- server/events/command_runner_test.go | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 0761630a0a..0b7313241e 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -211,16 +211,8 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo ctx.Log.Warn("Unable to create comment about pre-workflow hook failure: %s", err) } - // Update the plan or apply commit status to failed - switch cmd.Name { - case command.Plan: - if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Plan); err != nil { - ctx.Log.Warn("Unable to update plan commit status: %s", err) - } - case command.Apply: - if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Apply); err != nil { - ctx.Log.Warn("Unable to update apply commit status: %s", err) - } + if err := c.CommitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Plan); err != nil { + ctx.Log.Warn("Unable to update plan commit status: %s", err) } return diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 3910107a8e..a8fe4e7da7 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -923,7 +923,7 @@ func TestRunAutoplanCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_Tru pendingPlanFinder.VerifyWasCalled(Never()).DeletePlans(Any[string]()) // gomock will fail if lockingLocker.UnlockByPull is called unexpectedly (no EXPECT set) commitUpdater.VerifyWasCalledOnce().UpdateCombined(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), - Eq(models.PendingCommitStatus), Eq(command.Plan)) + Eq(models.FailedCommitStatus), Eq(command.Plan)) _, _, _, comment, _ := vcsClient.VerifyWasCalledOnce().CreateComment( Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()).GetCapturedArguments() From d0e7566511557b36aba92214afcc421b7a052b8a Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 09:39:55 -0400 Subject: [PATCH 10/23] fix: skip ignored targets before command side effects Classify ignored targeted dirs before plan review dismissal and apply lock handling, and make all targeted command runners treat the sentinel as a true skipped command. Keep API command and hook lists aligned when ignored paths are dropped. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/controllers/api_controller.go | 8 ++- server/controllers/api_controller_test.go | 50 ++++++++++++++ server/events/apply_command_runner.go | 59 ++++++++++------ server/events/apply_command_runner_test.go | 68 ++++++++++++------- .../events/approve_policies_command_runner.go | 11 +-- server/events/command_runner_test.go | 53 ++++++++++++--- server/events/import_command_runner.go | 3 + server/events/plan_command_runner.go | 15 ++-- server/events/plan_command_runner_test.go | 2 + server/events/project_command_builder.go | 9 +++ server/events/state_command_runner.go | 6 ++ server/events/version_command_runner.go | 3 + 12 files changed, 220 insertions(+), 67 deletions(-) diff --git a/server/controllers/api_controller.go b/server/controllers/api_controller.go index e57dc6926b..5943833c83 100644 --- a/server/controllers/api_controller.go +++ b/server/controllers/api_controller.go @@ -79,6 +79,7 @@ func (a *APIRequest) getCommands(ctx *command.Context, cmdName command.Name, cmd } cmds := make([]command.ProjectContext, 0) + keptCommentCommands := make([]*events.CommentCommand, 0) for _, commentCommand := range cc { projectCmds, err := cmdBuilder(ctx, commentCommand) if err != nil { @@ -87,10 +88,13 @@ func (a *APIRequest) getCommands(ctx *command.Context, cmdName command.Name, cmd } return nil, nil, fmt.Errorf("failed to build command: %w", err) } - cmds = append(cmds, projectCmds...) + for _, projectCmd := range projectCmds { + cmds = append(cmds, projectCmd) + keptCommentCommands = append(keptCommentCommands, commentCommand) + } } - return cmds, cc, nil + return cmds, keptCommentCommands, nil } func (a *APIController) apiReportError(w http.ResponseWriter, code int, err error) { diff --git a/server/controllers/api_controller_test.go b/server/controllers/api_controller_test.go index d0a7921cec..bd790ec53c 100644 --- a/server/controllers/api_controller_test.go +++ b/server/controllers/api_controller_test.go @@ -237,6 +237,56 @@ func TestAPIController_Plan_PreWorkflowHooksReceiveCorrectCommand(t *testing.T) command.Plan, capturedCmd.Name.String(), capturedCmd.Name) } +func TestAPIController_Plan_SkipsIgnoredPathsWithoutShiftingHookCommands(t *testing.T) { + ac, projectCommandBuilder, projectCommandRunner := setup(t) + preWorkflowHooksRunner := ac.PreWorkflowHooksCommandRunner.(*MockPreWorkflowHooksCommandRunner) + + When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())).Then(func(args []Param) ReturnValues { + commentCommand := args[1].(*events.CommentCommand) + if commentCommand.RepoRelDir == "ignored" { + return ReturnValues{[]command.ProjectContext{}, events.ErrIgnoredTargetedDir} + } + return ReturnValues{[]command.ProjectContext{{ + CommandName: command.Plan, + RepoRelDir: commentCommand.RepoRelDir, + Workspace: commentCommand.Workspace, + }}, nil} + }) + + body, _ := json.Marshal(controllers.APIRequest{ + Repository: "Repo", + Ref: "main", + Type: "Gitlab", + Paths: []struct { + Directory string + Workspace string + }{ + { + Directory: "ignored", + Workspace: "ignored-workspace", + }, + { + Directory: "kept", + Workspace: "kept-workspace", + }, + }, + }) + + req, _ := http.NewRequest("POST", "", bytes.NewBuffer(body)) + req.Header.Set(atlantisTokenHeader, atlantisToken) + w := httptest.NewRecorder() + ac.Plan(w, req) + ResponseContains(t, w, http.StatusOK, "") + + projectCommandRunner.VerifyWasCalledOnce().Plan(Any[command.ProjectContext]()) + _, capturedCmd := preWorkflowHooksRunner.VerifyWasCalledOnce(). + RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]()). + GetCapturedArguments() + + Equals(t, "kept", capturedCmd.RepoRelDir) + Equals(t, "kept-workspace", capturedCmd.Workspace) +} + // TestAPIController_Apply_PreWorkflowHooksReceiveCorrectCommand verifies that when // calling the Apply API endpoint, the pre-workflow hooks receive a CommentCommand // with Name set to command.Apply for the apply phase (and command.Plan for the diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go index 968a745be3..5be112a887 100644 --- a/server/events/apply_command_runner.go +++ b/server/events/apply_command_runner.go @@ -75,6 +75,25 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { baseRepo := ctx.Pull.BaseRepo pull := ctx.Pull + var projectCmds []command.ProjectContext + var projectCmdsBuilt bool + var projectCmdsErr error + if cmd.IsForSpecificProject() { + ctx.PullRequestStatus, err = a.pullReqStatusFetcher.FetchPullStatus(ctx.Log, pull) + if err != nil { + // On error we continue the request with mergeable assumed false. + // We want to continue because not all apply's will need this status, + // only if they rely on the mergeability requirement. + // All PullRequestStatus fields are set to false by default when error. + ctx.Log.Warn("unable to get pull request status: %s. Continuing with mergeable and approved assumed false", err) + } + projectCmds, projectCmdsErr = a.prjCmdBuilder.BuildApplyCommands(ctx, cmd) + projectCmdsBuilt = true + if MarkCommandSkippedIfIgnoredTargetedDir(ctx, cmd.CommandName(), projectCmdsErr) { + return + } + } + locked, err := a.IsLocked() if err != nil { ctx.Log.Err("checking global apply lock: %s", err) @@ -106,32 +125,30 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { return } - // Get the mergeable status before we set any build statuses of our own. - // We do this here because when we set a "Pending" status, if users have - // required the Atlantis status checks to pass, then we've now changed - // the mergeability status of the pull request. - // This sets the approved, mergeable, and sqlocked status in the context. - ctx.PullRequestStatus, err = a.pullReqStatusFetcher.FetchPullStatus(ctx.Log, pull) - if err != nil { - // On error we continue the request with mergeable assumed false. - // We want to continue because not all apply's will need this status, - // only if they rely on the mergeability requirement. - // All PullRequestStatus fields are set to false by default when error. - ctx.Log.Warn("unable to get pull request status: %s. Continuing with mergeable and approved assumed false", err) - } - - var projectCmds []command.ProjectContext - projectCmds, err = a.prjCmdBuilder.BuildApplyCommands(ctx, cmd) - if err != nil { - if IsIgnoredTargetedDir(err) { - ctx.Log.Debug("ignoring targeted apply because the directory matches autodiscover.ignore_paths") - ctx.CommandSkipped = true + if !projectCmdsBuilt { + // Get the mergeable status before we set any build statuses of our own. + // We do this here because when we set a "Pending" status, if users have + // required the Atlantis status checks to pass, then we've now changed + // the mergeability status of the pull request. + // This sets the approved, mergeable, and sqlocked status in the context. + ctx.PullRequestStatus, err = a.pullReqStatusFetcher.FetchPullStatus(ctx.Log, pull) + if err != nil { + // On error we continue the request with mergeable assumed false. + // We want to continue because not all apply's will need this status, + // only if they rely on the mergeability requirement. + // All PullRequestStatus fields are set to false by default when error. + ctx.Log.Warn("unable to get pull request status: %s. Continuing with mergeable and approved assumed false", err) + } + projectCmds, projectCmdsErr = a.prjCmdBuilder.BuildApplyCommands(ctx, cmd) + if MarkCommandSkippedIfIgnoredTargetedDir(ctx, cmd.CommandName(), projectCmdsErr) { return } + } + if projectCmdsErr != nil { if statusErr := a.commitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, cmd.CommandName()); statusErr != nil { ctx.Log.Warn("unable to update commit status: %s", statusErr) } - a.pullUpdater.updatePull(ctx, cmd, command.Result{Error: err}) + a.pullUpdater.updatePull(ctx, cmd, command.Result{Error: projectCmdsErr}) return } diff --git a/server/events/apply_command_runner_test.go b/server/events/apply_command_runner_test.go index 57552e729c..fd468c7375 100644 --- a/server/events/apply_command_runner_test.go +++ b/server/events/apply_command_runner_test.go @@ -249,32 +249,54 @@ func TestApplyCommandRunner_IsSilenced(t *testing.T) { } func TestApplyCommandRunner_IgnoredTargetedDirNoOp(t *testing.T) { - RegisterMockTestingT(t) - vcsClient := setup(t) - scopeNull := metricstest.NewLoggingScope(t, logging.NewNoopLogger(t), "atlantis") - modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} - cmd := &events.CommentCommand{Name: command.Apply, RepoRelDir: "ignored"} - ctx := &command.Context{ - User: testdata.User, - Log: logging.NewNoopLogger(t), - Scope: scopeNull, - Pull: modelPull, - HeadRepo: testdata.GithubRepo, - Trigger: command.CommentTrigger, + cases := []struct { + description string + setup func(*TestConfig) + }{ + { + description: "global lock backend errors", + setup: func(tc *TestConfig) { + tc.applyLockCheckerErr = errors.New("lock backend down") + }, + }, + { + description: "global apply lock is active", + setup: func(tc *TestConfig) { + tc.applyLockCheckerReturn = locking.ApplyCommandLock{Locked: true} + }, + }, } - When(projectCommandBuilder.BuildApplyCommands(ctx, cmd)).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + for _, c := range cases { + t.Run(c.description, func(t *testing.T) { + RegisterMockTestingT(t) + vcsClient := setup(t, c.setup) + scopeNull := metricstest.NewLoggingScope(t, logging.NewNoopLogger(t), "atlantis") + modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} + cmd := &events.CommentCommand{Name: command.Apply, RepoRelDir: "ignored"} + ctx := &command.Context{ + User: testdata.User, + Log: logging.NewNoopLogger(t), + Scope: scopeNull, + Pull: modelPull, + HeadRepo: testdata.GithubRepo, + Trigger: command.CommentTrigger, + } - applyCommandRunner.Run(ctx, cmd) - Assert(t, ctx.CommandSkipped, "expected ignored targeted dir to mark the command skipped") - - vcsClient.VerifyWasCalled(Never()).CreateComment( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) - commitUpdater.VerifyWasCalled(Never()).UpdateCombined( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]()) - commitUpdater.VerifyWasCalled(Never()).UpdateCombinedCount( - Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name](), Any[models.ProjectCounts]()) - projectCommandRunner.VerifyWasCalled(Never()).Apply(Any[command.ProjectContext]()) + When(projectCommandBuilder.BuildApplyCommands(ctx, cmd)).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + + applyCommandRunner.Run(ctx, cmd) + Assert(t, ctx.CommandSkipped, "expected ignored targeted dir to mark the command skipped") + + vcsClient.VerifyWasCalled(Never()).CreateComment( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombined( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombinedCount( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name](), Any[models.ProjectCounts]()) + projectCommandRunner.VerifyWasCalled(Never()).Apply(Any[command.ProjectContext]()) + }) + } } func TestApplyCommandRunner_ExecutionOrder(t *testing.T) { diff --git a/server/events/approve_policies_command_runner.go b/server/events/approve_policies_command_runner.go index 128ee0497c..c0b253d0b1 100644 --- a/server/events/approve_policies_command_runner.go +++ b/server/events/approve_policies_command_runner.go @@ -48,11 +48,10 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *CommentCom baseRepo := ctx.Pull.BaseRepo pull := ctx.Pull - if err := a.commitStatusUpdater.UpdateCombined(ctx.Log, baseRepo, pull, models.PendingCommitStatus, command.PolicyCheck); err != nil { - ctx.Log.Warn("unable to update commit status: %s", err) - } - projectCmds, err := a.prjCmdBuilder.BuildApprovePoliciesCommands(ctx, cmd) + if MarkCommandSkippedIfIgnoredTargetedDir(ctx, cmd.CommandName(), err) { + return + } if err != nil { if statusErr := a.commitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.PolicyCheck); statusErr != nil { ctx.Log.Warn("unable to update commit status: %s", statusErr) @@ -61,6 +60,10 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *CommentCom return } + if err := a.commitStatusUpdater.UpdateCombined(ctx.Log, baseRepo, pull, models.PendingCommitStatus, command.PolicyCheck); err != nil { + ctx.Log.Warn("unable to update commit status: %s", err) + } + if len(projectCmds) == 0 && a.SilenceNoProjects { ctx.Log.Info("determined there was no project to run approve_policies in") if !a.silenceVCSStatusNoProjects { diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index a8fe4e7da7..c9ae0f491c 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -59,6 +59,7 @@ var lockingLocker *lockingmocks.MockLocker var applyCommandRunner *events.ApplyCommandRunner var unlockCommandRunner *events.UnlockCommandRunner var importCommandRunner *events.ImportCommandRunner +var stateCommandRunner *events.StateCommandRunner var preWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner var postWorkflowHooksCommandRunner events.PostWorkflowHooksCommandRunner var cancellationTracker *mocks.MockCancellationTracker @@ -226,6 +227,12 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock testConfig.SilenceNoProjects, ) + stateCommandRunner = events.NewStateCommandRunner( + pullUpdater, + projectCommandBuilder, + projectCommandRunner, + ) + commentCommandRunnerByCmd := map[command.Name]events.CommentCommandRunner{ command.Plan: planCommandRunner, command.Apply: applyCommandRunner, @@ -233,6 +240,7 @@ func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.Mock command.Unlock: unlockCommandRunner, command.Version: versionCommandRunner, command.Import: importCommandRunner, + command.State: stateCommandRunner, } preWorkflowHooksCommandRunner = mocks.NewMockPreWorkflowHooksCommandRunner() @@ -475,16 +483,34 @@ func TestRunCommentCommandApply_NoProjects_SilenceEnabled(t *testing.T) { func TestRunCommentCommand_IgnoredTargetedDirNoOp(t *testing.T) { cases := []struct { - name string - command command.Name + name string + commandName command.Name + subName string }{ { - name: "plan", - command: command.Plan, + name: "plan", + commandName: command.Plan, + }, + { + name: "apply", + commandName: command.Apply, + }, + { + name: "approve_policies", + commandName: command.ApprovePolicies, + }, + { + name: "import", + commandName: command.Import, + }, + { + name: "version", + commandName: command.Version, }, { - name: "apply", - command: command.Apply, + name: "state rm", + commandName: command.State, + subName: "rm", }, } @@ -493,14 +519,23 @@ func TestRunCommentCommand_IgnoredTargetedDirNoOp(t *testing.T) { vcsClient := setup(t) pull := &github.PullRequest{State: github.Ptr("open")} modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} - cmd := &events.CommentCommand{Name: c.command, RepoRelDir: "ignored"} + cmd := &events.CommentCommand{Name: c.commandName, SubName: c.subName, RepoRelDir: "ignored"} When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) - if c.command == command.Plan { + switch c.commandName { + case command.Plan: When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) - } else { + case command.Apply: When(projectCommandBuilder.BuildApplyCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + case command.ApprovePolicies: + When(projectCommandBuilder.BuildApprovePoliciesCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + case command.Import: + When(projectCommandBuilder.BuildImportCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + case command.Version: + When(projectCommandBuilder.BuildVersionCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + case command.State: + When(projectCommandBuilder.BuildStateRmCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) } ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, cmd) diff --git a/server/events/import_command_runner.go b/server/events/import_command_runner.go index e34276db5f..e77b88bffe 100644 --- a/server/events/import_command_runner.go +++ b/server/events/import_command_runner.go @@ -50,6 +50,9 @@ func (v *ImportCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { var projectCmds []command.ProjectContext projectCmds, err = v.prjCmdBuilder.BuildImportCommands(ctx, cmd) + if MarkCommandSkippedIfIgnoredTargetedDir(ctx, cmd.CommandName(), err) { + return + } if err != nil { ctx.Log.Warn("Error %s", err) } diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go index f50af9807a..ed6f94b49f 100644 --- a/server/events/plan_command_runner.go +++ b/server/events/plan_command_runner.go @@ -206,19 +206,18 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) { ctx.Log.Warn("unable to get pull request status: %s. Continuing with mergeable and approved assumed false", err) } + projectCmds, err := p.prjCmdBuilder.BuildPlanCommands(ctx, cmd) + if MarkCommandSkippedIfIgnoredTargetedDir(ctx, command.Plan, err) { + return + } + if p.DiscardApprovalOnPlan { - if err = p.pullUpdater.VCSClient.DiscardReviews(ctx.Log, baseRepo, pull); err != nil { - ctx.Log.Err("failed to remove approvals: %s", err) + if discardErr := p.pullUpdater.VCSClient.DiscardReviews(ctx.Log, baseRepo, pull); discardErr != nil { + ctx.Log.Err("failed to remove approvals: %s", discardErr) } } - projectCmds, err := p.prjCmdBuilder.BuildPlanCommands(ctx, cmd) if err != nil { - if IsIgnoredTargetedDir(err) { - ctx.Log.Debug("ignoring targeted plan because the directory matches autodiscover.ignore_paths") - ctx.CommandSkipped = true - return - } if statusErr := p.commitStatusUpdater.UpdateCombined(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, command.Plan); statusErr != nil { ctx.Log.Warn("unable to update commit status: %s", statusErr) } diff --git a/server/events/plan_command_runner_test.go b/server/events/plan_command_runner_test.go index 449d10f149..4fec6357cf 100644 --- a/server/events/plan_command_runner_test.go +++ b/server/events/plan_command_runner_test.go @@ -166,6 +166,7 @@ func TestPlanCommandRunner_IsSilenced(t *testing.T) { func TestPlanCommandRunner_IgnoredTargetedDirNoOp(t *testing.T) { RegisterMockTestingT(t) vcsClient := setup(t) + planCommandRunner.DiscardApprovalOnPlan = true scopeNull := metricstest.NewLoggingScope(t, logging.NewNoopLogger(t), "atlantis") modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored"} @@ -189,6 +190,7 @@ func TestPlanCommandRunner_IgnoredTargetedDirNoOp(t *testing.T) { Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]()) commitUpdater.VerifyWasCalled(Never()).UpdateCombinedCount( Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name](), Any[models.ProjectCounts]()) + vcsClient.VerifyWasCalled(Never()).DiscardReviews(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]()) projectCommandRunner.VerifyWasCalled(Never()).Plan(Any[command.ProjectContext]()) } diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index a9238330ec..0a8f41171f 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -44,6 +44,15 @@ func IsIgnoredTargetedDir(err error) bool { return errors.Is(err, ErrIgnoredTargetedDir) } +func MarkCommandSkippedIfIgnoredTargetedDir(ctx *command.Context, commandName command.Name, err error) bool { + if !IsIgnoredTargetedDir(err) { + return false + } + ctx.Log.Debug("ignoring targeted %s because the directory matches autodiscover.ignore_paths", commandName.String()) + ctx.CommandSkipped = true + return true +} + func NewInstrumentedProjectCommandBuilder( logger logging.SimpleLogging, policyChecksSupported bool, diff --git a/server/events/state_command_runner.go b/server/events/state_command_runner.go index aaad6000c8..9ea9f01984 100644 --- a/server/events/state_command_runner.go +++ b/server/events/state_command_runner.go @@ -37,11 +37,17 @@ func (v *StateCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { Failure: fmt.Sprintf("unknown state subcommand %s", cmd.SubName), } } + if ctx.CommandSkipped { + return + } v.pullUpdater.updatePull(ctx, cmd, result) } func (v *StateCommandRunner) runRm(ctx *command.Context, cmd *CommentCommand) command.Result { projectCmds, err := v.prjCmdBuilder.BuildStateRmCommands(ctx, cmd) + if MarkCommandSkippedIfIgnoredTargetedDir(ctx, cmd.CommandName(), err) { + return command.Result{} + } if err != nil { ctx.Log.Warn("Error %s", err) } diff --git a/server/events/version_command_runner.go b/server/events/version_command_runner.go index db9c4b32f6..789e8f6f85 100644 --- a/server/events/version_command_runner.go +++ b/server/events/version_command_runner.go @@ -37,6 +37,9 @@ func (v *VersionCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { var err error var projectCmds []command.ProjectContext projectCmds, err = v.prjCmdBuilder.BuildVersionCommands(ctx, cmd) + if MarkCommandSkippedIfIgnoredTargetedDir(ctx, cmd.CommandName(), err) { + return + } if err != nil { ctx.Log.Warn("Error %s", err) } From ad3f98a7bb1bf7a0a86cd7393955e4417033e0ca Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 10:08:21 -0400 Subject: [PATCH 11/23] fix: skip ignored targets before hooks and API statuses Classify ignored targeted dirs before comment pre-workflow hooks and before non-plan builders require an existing workspace. Preserve skipped API requests as no-ops when every requested path is ignored. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/controllers/api_controller.go | 15 +++ server/controllers/api_controller_test.go | 73 ++++++++++++++ server/events/apply_command_runner.go | 4 + .../events/approve_policies_command_runner.go | 4 + server/events/command_runner.go | 21 ++++- server/events/command_runner_test.go | 16 +--- server/events/import_command_runner.go | 4 + .../mocks/mock_project_command_builder.go | 50 ++++++++++ server/events/plan_command_runner.go | 4 + server/events/project_command_builder.go | 73 +++++++++++++- server/events/project_command_builder_test.go | 94 +++++++++++++++++++ server/events/state_command_runner.go | 4 + server/events/version_command_runner.go | 4 + 13 files changed, 345 insertions(+), 21 deletions(-) diff --git a/server/controllers/api_controller.go b/server/controllers/api_controller.go index 5943833c83..774db292d8 100644 --- a/server/controllers/api_controller.go +++ b/server/controllers/api_controller.go @@ -80,19 +80,26 @@ func (a *APIRequest) getCommands(ctx *command.Context, cmdName command.Name, cmd cmds := make([]command.ProjectContext, 0) keptCommentCommands := make([]*events.CommentCommand, 0) + ignoredCommands := 0 + nonIgnoredCommands := 0 for _, commentCommand := range cc { projectCmds, err := cmdBuilder(ctx, commentCommand) if err != nil { if events.IsIgnoredTargetedDir(err) { + ignoredCommands++ continue } return nil, nil, fmt.Errorf("failed to build command: %w", err) } + nonIgnoredCommands++ for _, projectCmd := range projectCmds { cmds = append(cmds, projectCmd) keptCommentCommands = append(keptCommentCommands, commentCommand) } } + if ignoredCommands > 0 && nonIgnoredCommands == 0 { + return nil, nil, events.ErrIgnoredTargetedDir + } return cmds, keptCommentCommands, nil } @@ -255,6 +262,10 @@ func (a *APIController) apiSetup(ctx *command.Context, cmdName command.Name) err func (a *APIController) apiPlan(request *APIRequest, ctx *command.Context) (*command.Result, error) { cmds, cc, err := request.getCommands(ctx, command.Plan, a.ProjectCommandBuilder.BuildPlanCommands) + if events.IsIgnoredTargetedDir(err) { + ctx.CommandSkipped = true + return &command.Result{ProjectResults: []command.ProjectResult{}}, nil + } if err != nil { return nil, err } @@ -303,6 +314,10 @@ func (a *APIController) apiPlan(request *APIRequest, ctx *command.Context) (*com func (a *APIController) apiApply(request *APIRequest, ctx *command.Context) (*command.Result, error) { cmds, cc, err := request.getCommands(ctx, command.Apply, a.ProjectCommandBuilder.BuildApplyCommands) + if events.IsIgnoredTargetedDir(err) { + ctx.CommandSkipped = true + return &command.Result{ProjectResults: []command.ProjectResult{}}, nil + } if err != nil { return nil, err } diff --git a/server/controllers/api_controller_test.go b/server/controllers/api_controller_test.go index bd790ec53c..8af0c08af7 100644 --- a/server/controllers/api_controller_test.go +++ b/server/controllers/api_controller_test.go @@ -287,6 +287,79 @@ func TestAPIController_Plan_SkipsIgnoredPathsWithoutShiftingHookCommands(t *test Equals(t, "kept-workspace", capturedCmd.Workspace) } +func TestAPIController_Plan_AllIgnoredPathsNoOp(t *testing.T) { + ac, projectCommandBuilder, projectCommandRunner := setup(t) + commitStatusUpdater := ac.CommitStatusUpdater.(*MockCommitStatusUpdater) + + When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())). + ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + + body, _ := json.Marshal(controllers.APIRequest{ + Repository: "Repo", + Ref: "main", + Type: "Gitlab", + Paths: []struct { + Directory string + Workspace string + }{ + { + Directory: "ignored", + Workspace: "default", + }, + }, + }) + + req, _ := http.NewRequest("POST", "", bytes.NewBuffer(body)) + req.Header.Set(atlantisTokenHeader, atlantisToken) + w := httptest.NewRecorder() + ac.Plan(w, req) + ResponseContains(t, w, http.StatusOK, "") + + projectCommandRunner.VerifyWasCalled(Never()).Plan(Any[command.ProjectContext]()) + commitStatusUpdater.VerifyWasCalled(Never()).UpdateCombined( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]()) + commitStatusUpdater.VerifyWasCalled(Never()).UpdateCombinedCount( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name](), Any[models.ProjectCounts]()) +} + +func TestAPIController_Apply_AllIgnoredPathsNoOp(t *testing.T) { + ac, projectCommandBuilder, projectCommandRunner := setup(t) + commitStatusUpdater := ac.CommitStatusUpdater.(*MockCommitStatusUpdater) + + When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())). + ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + When(projectCommandBuilder.BuildApplyCommands(Any[*command.Context](), Any[*events.CommentCommand]())). + ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) + + body, _ := json.Marshal(controllers.APIRequest{ + Repository: "Repo", + Ref: "main", + Type: "Gitlab", + Paths: []struct { + Directory string + Workspace string + }{ + { + Directory: "ignored", + Workspace: "default", + }, + }, + }) + + req, _ := http.NewRequest("POST", "", bytes.NewBuffer(body)) + req.Header.Set(atlantisTokenHeader, atlantisToken) + w := httptest.NewRecorder() + ac.Apply(w, req) + ResponseContains(t, w, http.StatusOK, "") + + projectCommandRunner.VerifyWasCalled(Never()).Plan(Any[command.ProjectContext]()) + projectCommandRunner.VerifyWasCalled(Never()).Apply(Any[command.ProjectContext]()) + commitStatusUpdater.VerifyWasCalled(Never()).UpdateCombined( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]()) + commitStatusUpdater.VerifyWasCalled(Never()).UpdateCombinedCount( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name](), Any[models.ProjectCounts]()) +} + // TestAPIController_Apply_PreWorkflowHooksReceiveCorrectCommand verifies that when // calling the Apply API endpoint, the pre-workflow hooks receive a CommentCommand // with Name set to command.Apply for the apply phase (and command.Plan for the diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go index 5be112a887..e2877a2049 100644 --- a/server/events/apply_command_runner.go +++ b/server/events/apply_command_runner.go @@ -221,6 +221,10 @@ func (a *ApplyCommandRunner) updatePendingCommitStatus(ctx *command.Context) { } } +func (a *ApplyCommandRunner) ShouldSkipPreWorkflowHooks(ctx *command.Context, cmd *CommentCommand) bool { + return MarkCommandSkippedIfIgnoredTarget(ctx, command.Apply, cmd, a.prjCmdBuilder) +} + func (a *ApplyCommandRunner) IsLocked() (bool, error) { lock, err := a.locker.CheckApplyLock() diff --git a/server/events/approve_policies_command_runner.go b/server/events/approve_policies_command_runner.go index c0b253d0b1..fb707b18ac 100644 --- a/server/events/approve_policies_command_runner.go +++ b/server/events/approve_policies_command_runner.go @@ -95,6 +95,10 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *CommentCom a.updateCommitStatus(ctx, pullStatus) } +func (a *ApprovePoliciesCommandRunner) ShouldSkipPreWorkflowHooks(ctx *command.Context, cmd *CommentCommand) bool { + return MarkCommandSkippedIfIgnoredTarget(ctx, cmd.CommandName(), cmd, a.prjCmdBuilder) +} + func (a *ApprovePoliciesCommandRunner) updateCommitStatus(ctx *command.Context, pullStatus models.PullStatus) { var numSuccess int var numErrored int diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 0b7313241e..9360ded331 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -69,6 +69,10 @@ type CommentCommandRunner interface { Run(*command.Context, *CommentCommand) } +type PreWorkflowHooksSkipper interface { + ShouldSkipPreWorkflowHooks(*command.Context, *CommentCommand) bool +} + func buildCommentCommandRunner( cmdRunner *DefaultCommandRunner, cmdName command.Name, @@ -84,6 +88,11 @@ func buildCommentCommandRunner( return runner } +func shouldSkipPreWorkflowHooks(ctx *command.Context, cmdRunner CommentCommandRunner, cmd *CommentCommand) bool { + skipper, ok := cmdRunner.(PreWorkflowHooksSkipper) + return ok && skipper.ShouldSkipPreWorkflowHooks(ctx, cmd) +} + // DefaultCommandRunner is the first step when processing a comment command. type DefaultCommandRunner struct { VCSClient vcs.Client `validate:"required"` @@ -199,6 +208,11 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo Name: command.Autoplan, } + cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) + if shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) { + return + } + preWorkflowHooksErr := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) if preWorkflowHooksErr != nil { @@ -481,6 +495,11 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead return } + cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) + if shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) { + return + } + preWorkflowHooksErr := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) if preWorkflowHooksErr != nil { @@ -511,8 +530,6 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead ctx.Log.Err("'fail-on-pre-workflow-hook-error' not set so running %s command.", cmd.Name.String()) } - cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) - cmdRunner.Run(ctx, cmd) if ctx.CommandSkipped { return diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index c9ae0f491c..9008224a39 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -523,23 +523,11 @@ func TestRunCommentCommand_IgnoredTargetedDirNoOp(t *testing.T) { When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) - switch c.commandName { - case command.Plan: - When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) - case command.Apply: - When(projectCommandBuilder.BuildApplyCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) - case command.ApprovePolicies: - When(projectCommandBuilder.BuildApprovePoliciesCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) - case command.Import: - When(projectCommandBuilder.BuildImportCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) - case command.Version: - When(projectCommandBuilder.BuildVersionCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) - case command.State: - When(projectCommandBuilder.BuildStateRmCommands(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn([]command.ProjectContext{}, events.ErrIgnoredTargetedDir) - } + When(projectCommandBuilder.ShouldIgnoreTargetedDir(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn(true) ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, cmd) + preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalled(Never()).RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]()) vcsClient.VerifyWasCalled(Never()).CreateComment( Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) commitUpdater.VerifyWasCalled(Never()).UpdateCombined( diff --git a/server/events/import_command_runner.go b/server/events/import_command_runner.go index e77b88bffe..3bc9a81673 100644 --- a/server/events/import_command_runner.go +++ b/server/events/import_command_runner.go @@ -73,3 +73,7 @@ func (v *ImportCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { } v.pullUpdater.updatePull(ctx, cmd, result) } + +func (v *ImportCommandRunner) ShouldSkipPreWorkflowHooks(ctx *command.Context, cmd *CommentCommand) bool { + return MarkCommandSkippedIfIgnoredTarget(ctx, cmd.CommandName(), cmd, v.prjCmdBuilder) +} diff --git a/server/events/mocks/mock_project_command_builder.go b/server/events/mocks/mock_project_command_builder.go index 4395759917..fdaba5d798 100644 --- a/server/events/mocks/mock_project_command_builder.go +++ b/server/events/mocks/mock_project_command_builder.go @@ -159,6 +159,21 @@ func (mock *MockProjectCommandBuilder) BuildVersionCommands(ctx *command.Context return _ret0, _ret1 } +func (mock *MockProjectCommandBuilder) ShouldIgnoreTargetedDir(ctx *command.Context, comment *events.CommentCommand) bool { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockProjectCommandBuilder().") + } + _params := []pegomock.Param{ctx, comment} + _result := pegomock.GetGenericMockFrom(mock).Invoke("ShouldIgnoreTargetedDir", _params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem()}) + var _ret0 bool + if len(_result) != 0 { + if _result[0] != nil { + _ret0 = _result[0].(bool) + } + } + return _ret0 +} + func (mock *MockProjectCommandBuilder) VerifyWasCalledOnce() *VerifierMockProjectCommandBuilder { return &VerifierMockProjectCommandBuilder{ mock: mock, @@ -434,3 +449,38 @@ func (c *MockProjectCommandBuilder_BuildVersionCommands_OngoingVerification) Get } return } + +func (verifier *VerifierMockProjectCommandBuilder) ShouldIgnoreTargetedDir(ctx *command.Context, comment *events.CommentCommand) *MockProjectCommandBuilder_ShouldIgnoreTargetedDir_OngoingVerification { + _params := []pegomock.Param{ctx, comment} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "ShouldIgnoreTargetedDir", _params, verifier.timeout) + return &MockProjectCommandBuilder_ShouldIgnoreTargetedDir_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockProjectCommandBuilder_ShouldIgnoreTargetedDir_OngoingVerification struct { + mock *MockProjectCommandBuilder + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockProjectCommandBuilder_ShouldIgnoreTargetedDir_OngoingVerification) GetCapturedArguments() (*command.Context, *events.CommentCommand) { + ctx, comment := c.GetAllCapturedArguments() + return ctx[len(ctx)-1], comment[len(comment)-1] +} + +func (c *MockProjectCommandBuilder_ShouldIgnoreTargetedDir_OngoingVerification) GetAllCapturedArguments() (_param0 []*command.Context, _param1 []*events.CommentCommand) { + _params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(_params) > 0 { + if len(_params) > 0 { + _param0 = make([]*command.Context, len(c.methodInvocations)) + for u, param := range _params[0] { + _param0[u] = param.(*command.Context) + } + } + if len(_params) > 1 { + _param1 = make([]*events.CommentCommand, len(c.methodInvocations)) + for u, param := range _params[1] { + _param1[u] = param.(*events.CommentCommand) + } + } + } + return +} diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go index ed6f94b49f..b67522e196 100644 --- a/server/events/plan_command_runner.go +++ b/server/events/plan_command_runner.go @@ -335,6 +335,10 @@ func (p *PlanCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { } } +func (p *PlanCommandRunner) ShouldSkipPreWorkflowHooks(ctx *command.Context, cmd *CommentCommand) bool { + return MarkCommandSkippedIfIgnoredTarget(ctx, command.Plan, cmd, p.prjCmdBuilder) +} + func (p *PlanCommandRunner) updatePendingCommitStatus(ctx *command.Context, commandName command.Name) { if p.silenceVCSStatusNoProjects { ctx.Log.Debug("silence enabled - not setting pending VCS status") diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 0a8f41171f..2935d45849 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -53,6 +53,15 @@ func MarkCommandSkippedIfIgnoredTargetedDir(ctx *command.Context, commandName co return true } +func MarkCommandSkippedIfIgnoredTarget(ctx *command.Context, commandName command.Name, cmd *CommentCommand, ignorer ProjectTargetedDirIgnorer) bool { + if cmd.ProjectName != "" || cmd.RepoRelDir == "" || !ignorer.ShouldIgnoreTargetedDir(ctx, cmd) { + return false + } + ctx.Log.Debug("ignoring targeted %s because the directory matches autodiscover.ignore_paths", commandName.String()) + ctx.CommandSkipped = true + return true +} + func NewInstrumentedProjectCommandBuilder( logger logging.SimpleLogging, policyChecksSupported bool, @@ -167,6 +176,7 @@ func NewProjectCommandBuilder( } type ProjectPlanCommandBuilder interface { + ProjectTargetedDirIgnorer // BuildAutoplanCommands builds project commands that will run plan on // the projects determined to be modified. BuildAutoplanCommands(ctx *command.Context) ([]command.ProjectContext, error) @@ -177,6 +187,7 @@ type ProjectPlanCommandBuilder interface { } type ProjectApplyCommandBuilder interface { + ProjectTargetedDirIgnorer // BuildApplyCommands builds project Apply commands for this ctx and comment. If // comment doesn't specify one project then there may be multiple commands // to be run. @@ -184,11 +195,13 @@ type ProjectApplyCommandBuilder interface { } type ProjectApprovePoliciesCommandBuilder interface { + ProjectTargetedDirIgnorer // BuildApprovePoliciesCommands builds project PolicyCheck commands for this ctx and comment. BuildApprovePoliciesCommands(ctx *command.Context, comment *CommentCommand) ([]command.ProjectContext, error) } type ProjectVersionCommandBuilder interface { + ProjectTargetedDirIgnorer // BuildVersionCommands builds project Version commands for this ctx and comment. If // comment doesn't specify one project then there may be multiple commands // to be run. @@ -196,6 +209,7 @@ type ProjectVersionCommandBuilder interface { } type ProjectImportCommandBuilder interface { + ProjectTargetedDirIgnorer // BuildImportCommands builds project Import commands for this ctx and comment. If // comment doesn't specify one project then there may be multiple commands // to be run. @@ -203,12 +217,17 @@ type ProjectImportCommandBuilder interface { } type ProjectStateCommandBuilder interface { + ProjectTargetedDirIgnorer // BuildStateRmCommands builds project state rm commands for this ctx and comment. If // comment doesn't specify one project then there may be multiple commands // to be run. BuildStateRmCommands(ctx *command.Context, comment *CommentCommand) ([]command.ProjectContext, error) } +type ProjectTargetedDirIgnorer interface { + ShouldIgnoreTargetedDir(ctx *command.Context, comment *CommentCommand) bool +} + //go:generate go tool pegomock generate github.com/runatlantis/atlantis/server/events --package mocks -o mocks/mock_project_command_builder.go ProjectCommandBuilder // ProjectCommandBuilder builds commands that run on individual projects. @@ -439,6 +458,45 @@ func (p *DefaultProjectCommandBuilder) shouldIgnoreTargetedDir(ctx *command.Cont if err != nil { return false } + return p.shouldIgnoreTargetedDirFromCfg(ctx, repoCfg, repoRelDir) +} + +func (p *DefaultProjectCommandBuilder) ShouldIgnoreTargetedDir(ctx *command.Context, cmd *CommentCommand) bool { + if cmd.ProjectName != "" || cmd.RepoRelDir == "" || valid.ContainsDirGlobPattern(cmd.RepoRelDir) { + return false + } + repoCfg := p.repoCfgForTargetedIgnore(ctx) + return p.shouldIgnoreTargetedDirFromCfg(ctx, repoCfg, cmd.RepoRelDir) +} + +func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Context) valid.RepoCfg { + if p.VCSClient != nil { + repoCfgFile := p.GlobalCfg.RepoConfigFile(ctx.Pull.BaseRepo.ID()) + hasRepoCfg, repoCfgData, err := p.VCSClient.GetFileContent(ctx.Log, ctx.HeadRepo, ctx.Pull.HeadBranch, repoCfgFile) + if err != nil { + ctx.Log.Debug("unable to fetch %s while checking autodiscover.ignore_paths: %s", repoCfgFile, err) + } else if hasRepoCfg { + repoCfg, err := p.ParserValidator.ParseRepoCfgData(repoCfgData, p.GlobalCfg, ctx.Pull.BaseRepo.ID(), ctx.Pull.BaseBranch) + if err != nil { + ctx.Log.Debug("unable to parse %s while checking autodiscover.ignore_paths: %s", repoCfgFile, err) + } else { + return repoCfg + } + } + } + + repoDir, err := p.WorkingDir.GetWorkingDir(ctx.Pull.BaseRepo, ctx.Pull, DefaultWorkspace) + if err != nil { + return valid.RepoCfg{} + } + repoCfg, _, err := p.parseRepoCfg(ctx, repoDir) + if err != nil { + return valid.RepoCfg{} + } + return repoCfg +} + +func (p *DefaultProjectCommandBuilder) shouldIgnoreTargetedDirFromCfg(ctx *command.Context, repoCfg valid.RepoCfg, repoRelDir string) bool { if !p.autoDiscoverModeEnabled(ctx, repoCfg) { return false } @@ -958,6 +1016,16 @@ func (p *DefaultProjectCommandBuilder) buildProjectCommand(ctx *command.Context, } var projCtx []command.ProjectContext + repoRelDir := DefaultRepoRelDir + if cmd.RepoRelDir != "" { + repoRelDir = cmd.RepoRelDir + } + + if p.ShouldIgnoreTargetedDir(ctx, cmd) { + ctx.Log.Debug("ignoring targeted command for dir '%s' due to autodiscover.ignore_paths", repoRelDir) + return projCtx, ErrIgnoredTargetedDir + } + unlockFn, err := p.WorkingDirLocker.TryLock(ctx.Pull.BaseRepo.FullName, ctx.Pull.Num, workspace, DefaultRepoRelDir, cmd.ProjectName, cmd.Name) if err != nil { return projCtx, err @@ -973,11 +1041,6 @@ func (p *DefaultProjectCommandBuilder) buildProjectCommand(ctx *command.Context, return projCtx, err } - repoRelDir := DefaultRepoRelDir - if cmd.RepoRelDir != "" { - repoRelDir = cmd.RepoRelDir - } - if cmd.ProjectName == "" && p.shouldIgnoreTargetedDir(ctx, repoDir, repoRelDir) { ctx.Log.Debug("ignoring targeted command for dir '%s' due to autodiscover.ignore_paths", repoRelDir) return projCtx, ErrIgnoredTargetedDir diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 8c9455c55a..6a5c7669e8 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -786,6 +786,100 @@ func TestDefaultProjectCommandBuilder_BuildTargetedCommand_IgnorePaths(t *testin Equals(t, "environments/nonprod", applyCtxs[0].RepoRelDir) } +func TestDefaultProjectCommandBuilder_BuildTargetedNonPlanCommand_IgnorePathsWithoutWorkingDir(t *testing.T) { + RegisterMockTestingT(t) + + workingDir := mocks.NewMockWorkingDir() + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]())).ThenReturn("", os.ErrNotExist) + vcsClient := vcsmocks.NewMockClient() + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + + globalCfgArgs := valid.GlobalCfgArgs{AllowAllRepoSettings: true} + globalCfg := valid.NewGlobalCfgFromArgs(globalCfgArgs) + globalCfg.Repos[0].AutoDiscover = &valid.AutoDiscover{ + Mode: valid.AutoDiscoverEnabledMode, + IgnorePaths: []string{"environments/prod/**"}, + } + + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + + repo := models.Repo{FullName: "runatlantis/atlantis", Owner: "runatlantis", Name: "atlantis"} + cmdCtx := &command.Context{ + Log: logger, + Scope: scope, + Pull: models.PullRequest{BaseRepo: repo, Num: 1, HeadBranch: "feature", BaseBranch: "main"}, + HeadRepo: repo, + } + + cases := []struct { + description string + cmd events.CommentCommand + build func(*command.Context, *events.CommentCommand) ([]command.ProjectContext, error) + }{ + { + description: "apply", + cmd: events.CommentCommand{Name: command.Apply, RepoRelDir: "environments/prod", Workspace: "default"}, + build: builder.BuildApplyCommands, + }, + { + description: "approve policies", + cmd: events.CommentCommand{Name: command.ApprovePolicies, RepoRelDir: "environments/prod", Workspace: "default"}, + build: builder.BuildApprovePoliciesCommands, + }, + { + description: "import", + cmd: events.CommentCommand{Name: command.Import, RepoRelDir: "environments/prod", Workspace: "default"}, + build: builder.BuildImportCommands, + }, + { + description: "version", + cmd: events.CommentCommand{Name: command.Version, RepoRelDir: "environments/prod", Workspace: "default"}, + build: builder.BuildVersionCommands, + }, + { + description: "state rm", + cmd: events.CommentCommand{Name: command.State, SubName: "rm", RepoRelDir: "environments/prod", Workspace: "default"}, + build: builder.BuildStateRmCommands, + }, + } + + for _, c := range cases { + t.Run(c.description, func(t *testing.T) { + ctxs, err := c.build(cmdCtx, &c.cmd) + Assert(t, errors.Is(err, events.ErrIgnoredTargetedDir), "expected ignored targeted dir error, got %v", err) + Equals(t, 0, len(ctxs)) + }) + } +} + // Test that autodiscover.ignore_paths set in repo-level atlantis.yaml blocks // targeted plan/apply -d commands for non-configured projects. func TestDefaultProjectCommandBuilder_BuildTargetedCommand_IgnorePathsRepoCfg(t *testing.T) { diff --git a/server/events/state_command_runner.go b/server/events/state_command_runner.go index 9ea9f01984..318203bc92 100644 --- a/server/events/state_command_runner.go +++ b/server/events/state_command_runner.go @@ -53,3 +53,7 @@ func (v *StateCommandRunner) runRm(ctx *command.Context, cmd *CommentCommand) co } return runProjectCmds(projectCmds, v.prjCmdRunner.StateRm) } + +func (v *StateCommandRunner) ShouldSkipPreWorkflowHooks(ctx *command.Context, cmd *CommentCommand) bool { + return MarkCommandSkippedIfIgnoredTarget(ctx, cmd.CommandName(), cmd, v.prjCmdBuilder) +} diff --git a/server/events/version_command_runner.go b/server/events/version_command_runner.go index 789e8f6f85..e951fe7b47 100644 --- a/server/events/version_command_runner.go +++ b/server/events/version_command_runner.go @@ -61,6 +61,10 @@ func (v *VersionCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { v.pullUpdater.updatePull(ctx, cmd, result) } +func (v *VersionCommandRunner) ShouldSkipPreWorkflowHooks(ctx *command.Context, cmd *CommentCommand) bool { + return MarkCommandSkippedIfIgnoredTarget(ctx, cmd.CommandName(), cmd, v.prjCmdBuilder) +} + func (v *VersionCommandRunner) isParallelEnabled(cmds []command.ProjectContext) bool { return len(cmds) > 0 && cmds[0].ParallelPolicyCheckEnabled } From c0c510d8e9e6bcdf7fa5922186f3b67ce93ec7c7 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 10:31:33 -0400 Subject: [PATCH 12/23] fix: skip ignored targets before validations Classify ignored targeted directory commands before comment validations that can publish user, var-file, fork, or closed-PR errors. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/command_runner.go | 58 ++++++++++---------- server/events/command_runner_test.go | 81 ++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 29 deletions(-) diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 9360ded331..405294a4a8 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -437,6 +437,35 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead timer := scope.Timer(metrics.ExecutionTimeMetric).Start() defer timer.Stop() + headRepo, pull, err := c.ensureValidRepoMetadata(baseRepo, maybeHeadRepo, maybePull, user, pullNum, log) + if err != nil { + return + } + + status, err := c.PullStatusFetcher.GetPullStatus(pull) + + if err != nil { + log.Err("Unable to fetch pull status, this is likely a bug: %s", err) + } + + ctx := &command.Context{ + User: user, + Log: log, + Pull: pull, + PullStatus: status, + HeadRepo: headRepo, + Scope: scope, + Trigger: command.CommentTrigger, + PolicySet: cmd.PolicySet, + ClearPolicyApproval: cmd.ClearPolicyApproval, + TeamAllowlistChecker: c.TeamAllowlistChecker, + } + + cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) + if shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) { + return + } + // Check if the user who commented has the permissions to execute the 'plan' or 'apply' commands if c.TeamAllowlistChecker != nil && c.TeamAllowlistChecker.HasRules() { err := c.fetchUserTeams(log, baseRepo, &user) @@ -467,39 +496,10 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead return } - headRepo, pull, err := c.ensureValidRepoMetadata(baseRepo, maybeHeadRepo, maybePull, user, pullNum, log) - if err != nil { - return - } - - status, err := c.PullStatusFetcher.GetPullStatus(pull) - - if err != nil { - log.Err("Unable to fetch pull status, this is likely a bug: %s", err) - } - - ctx := &command.Context{ - User: user, - Log: log, - Pull: pull, - PullStatus: status, - HeadRepo: headRepo, - Scope: scope, - Trigger: command.CommentTrigger, - PolicySet: cmd.PolicySet, - ClearPolicyApproval: cmd.ClearPolicyApproval, - TeamAllowlistChecker: c.TeamAllowlistChecker, - } - if !c.validateCtxAndComment(ctx, cmd.Name) { return } - cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) - if shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) { - return - } - preWorkflowHooksErr := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) if preWorkflowHooksErr != nil { diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 9008224a39..b2a8d8aae6 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -539,6 +539,87 @@ func TestRunCommentCommand_IgnoredTargetedDirNoOp(t *testing.T) { } } +func TestRunCommentCommand_IgnoredTargetedDirSkipsValidationComments(t *testing.T) { + cases := []struct { + name string + cmd *events.CommentCommand + modelPull models.PullRequest + headRepo models.Repo + setup func(t *testing.T, vcsClient *vcsmocks.MockClient) + verify func(t *testing.T, vcsClient *vcsmocks.MockClient) + }{ + { + name: "team allowlist", + cmd: &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored"}, + setup: func(t *testing.T, vcsClient *vcsmocks.MockClient) { + checker, err := command.NewTeamAllowlistChecker("allowed-team:apply") + Ok(t, err) + ch.TeamAllowlistChecker = checker + }, + verify: func(t *testing.T, vcsClient *vcsmocks.MockClient) { + vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.User)) + }, + }, + { + name: "var file allowlist", + cmd: &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored", Flags: []string{"-var-file", "/tmp/outside.tfvars"}}, + setup: func(t *testing.T, vcsClient *vcsmocks.MockClient) { + checker, err := events.NewVarFileAllowlistChecker("") + Ok(t, err) + ch.VarFileAllowlistChecker = checker + }, + }, + { + name: "fork pull request", + cmd: &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored"}, + headRepo: models.Repo{ + Owner: "forkrepo", + FullName: "forkrepo/atlantis", + }, + }, + { + name: "closed pull request", + cmd: &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored"}, + modelPull: models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.ClosedPullState, Num: testdata.Pull.Num}, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + vcsClient := setup(t) + if c.modelPull.BaseRepo.ID() == "" { + c.modelPull = models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} + } + if c.headRepo.FullName == "" { + c.headRepo = testdata.GithubRepo + } + + if c.setup != nil { + c.setup(t, vcsClient) + } + + pull := &github.PullRequest{State: github.Ptr("open")} + When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) + When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(c.modelPull, c.modelPull.BaseRepo, c.headRepo, nil) + When(projectCommandBuilder.ShouldIgnoreTargetedDir(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn(true) + + ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, c.cmd) + + if c.verify != nil { + c.verify(t, vcsClient) + } + preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalled(Never()).RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]()) + vcsClient.VerifyWasCalled(Never()).CreateComment( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombined( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]()) + commitUpdater.VerifyWasCalled(Never()).UpdateCombinedCount( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name](), Any[models.ProjectCounts]()) + postWorkflowHooksCommandRunner.(*mocks.MockPostWorkflowHooksCommandRunner).VerifyWasCalled(Never()).RunPostHooks(Any[*command.Context](), Any[*events.CommentCommand]()) + }) + } +} + func TestRunCommentCommandApprovePolicy_NoProjects_SilenceEnabled(t *testing.T) { t.Log("if an approve_policy command is run on a pull request and SilenceNoProjects is enabled and we are silencing all comments if the modified files don't have a matching project") vcsClient := setup(t) From 1536eed6e176700cbcebceefdb79cc2a829e29f2 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 11:06:10 -0400 Subject: [PATCH 13/23] fix: use plan runner for autoplan hook checks Autoplan uses the plan command runner for execution, so pre-workflow hook skip checks must look up the registered plan runner instead of the synthetic autoplan command. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/command_runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 405294a4a8..2ff4fbbb79 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -208,7 +208,7 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo Name: command.Autoplan, } - cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) + cmdRunner := buildCommentCommandRunner(c, command.Plan) if shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) { return } From 3f55a3bc453001075fd719eb5d3b78b2fd666023 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 12:02:11 -0400 Subject: [PATCH 14/23] fix: preserve ignored target no-op boundaries Avoid API PR lock cleanup for all-ignored requests, let pre-workflow hooks generate explicit config before the final ignored-target skip, and keep expected ignored-target sentinels out of builder error metrics. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/controllers/api_controller.go | 17 +++- server/controllers/api_controller_test.go | 28 +++++-- server/events/command_runner.go | 14 +++- server/events/command_runner_test.go | 36 ++++++++- .../instrumented_project_command_builder.go | 8 +- ...strumented_project_command_builder_test.go | 79 +++++++++++++++++++ 6 files changed, 168 insertions(+), 14 deletions(-) create mode 100644 server/events/instrumented_project_command_builder_test.go diff --git a/server/controllers/api_controller.go b/server/controllers/api_controller.go index 774db292d8..c947d04133 100644 --- a/server/controllers/api_controller.go +++ b/server/controllers/api_controller.go @@ -131,7 +131,9 @@ func (a *APIController) Plan(w http.ResponseWriter, r *http.Request) { a.apiReportError(w, http.StatusInternalServerError, err) return } - defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, ctx.Pull.Num) // nolint: errcheck + if !ctx.CommandSkipped { + defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, ctx.Pull.Num) // nolint: errcheck + } if result.HasErrors() { code = http.StatusInternalServerError } @@ -161,11 +163,20 @@ func (a *APIController) Apply(w http.ResponseWriter, r *http.Request) { } // We must first make the plan for all projects - _, err = a.apiPlan(request, ctx) + result, err := a.apiPlan(request, ctx) if err != nil { a.apiReportError(w, http.StatusInternalServerError, err) return } + if ctx.CommandSkipped { + response, err := json.Marshal(result) + if err != nil { + a.apiReportError(w, http.StatusInternalServerError, err) + return + } + a.respond(w, logging.Warn, code, "%s", string(response)) + return + } defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, ctx.Pull.Num) // nolint: errcheck // The API apply endpoint runs plan first. Refresh PR status afterward so @@ -173,7 +184,7 @@ func (a *APIController) Apply(w http.ResponseWriter, r *http.Request) { a.populatePullRequestStatus(ctx) // We can now prepare and run the apply step - result, err := a.apiApply(request, ctx) + result, err = a.apiApply(request, ctx) if err != nil { a.apiReportError(w, http.StatusInternalServerError, err) return diff --git a/server/controllers/api_controller_test.go b/server/controllers/api_controller_test.go index 8af0c08af7..babb74f682 100644 --- a/server/controllers/api_controller_test.go +++ b/server/controllers/api_controller_test.go @@ -288,7 +288,9 @@ func TestAPIController_Plan_SkipsIgnoredPathsWithoutShiftingHookCommands(t *test } func TestAPIController_Plan_AllIgnoredPathsNoOp(t *testing.T) { - ac, projectCommandBuilder, projectCommandRunner := setup(t) + ac, projectCommandBuilder, projectCommandRunner := setup(t, func(config *apiControllerTestConfig) { + config.allowUnlockByPull = false + }) commitStatusUpdater := ac.CommitStatusUpdater.(*MockCommitStatusUpdater) When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())). @@ -323,7 +325,9 @@ func TestAPIController_Plan_AllIgnoredPathsNoOp(t *testing.T) { } func TestAPIController_Apply_AllIgnoredPathsNoOp(t *testing.T) { - ac, projectCommandBuilder, projectCommandRunner := setup(t) + ac, projectCommandBuilder, projectCommandRunner := setup(t, func(config *apiControllerTestConfig) { + config.allowUnlockByPull = false + }) commitStatusUpdater := ac.CommitStatusUpdater.(*MockCommitStatusUpdater) When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())). @@ -354,6 +358,7 @@ func TestAPIController_Apply_AllIgnoredPathsNoOp(t *testing.T) { projectCommandRunner.VerifyWasCalled(Never()).Plan(Any[command.ProjectContext]()) projectCommandRunner.VerifyWasCalled(Never()).Apply(Any[command.ProjectContext]()) + projectCommandBuilder.VerifyWasCalled(Never()).BuildApplyCommands(Any[*command.Context](), Any[*events.CommentCommand]()) commitStatusUpdater.VerifyWasCalled(Never()).UpdateCombined( Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest](), Any[models.CommitStatus](), Any[command.Name]()) commitStatusUpdater.VerifyWasCalled(Never()).UpdateCombinedCount( @@ -599,12 +604,25 @@ func TestAPIController_ListLocksEmpty(t *testing.T) { Equals(t, expected, result) } -func setup(t *testing.T) (controllers.APIController, *MockProjectCommandBuilder, *MockProjectCommandRunner) { +type apiControllerTestConfig struct { + allowUnlockByPull bool +} + +func setup(t *testing.T, options ...func(*apiControllerTestConfig)) (controllers.APIController, *MockProjectCommandBuilder, *MockProjectCommandRunner) { RegisterMockTestingT(t) + config := &apiControllerTestConfig{ + allowUnlockByPull: true, + } + for _, option := range options { + option(config) + } + gmockCtrl := gomock.NewController(t) locker := NewMockLocker(gmockCtrl) - // Allow incidental calls to UnlockByPull (called internally during plan/apply operations) - locker.EXPECT().UnlockByPull(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + if config.allowUnlockByPull { + // Allow incidental calls to UnlockByPull (called internally during plan/apply operations) + locker.EXPECT().UnlockByPull(gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() + } logger := logging.NewNoopLogger(t) parser := NewMockEventParsing() repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*") diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 2ff4fbbb79..24fec98eae 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -462,8 +462,16 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead } cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) + preWorkflowHooksRan := false + var preWorkflowHooksErr error if shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) { - return + ctx.CommandSkipped = false + preWorkflowHooksErr = c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) + preWorkflowHooksRan = true + ctx.CommandSkipped = false + if shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) { + return + } } // Check if the user who commented has the permissions to execute the 'plan' or 'apply' commands @@ -500,7 +508,9 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead return } - preWorkflowHooksErr := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) + if !preWorkflowHooksRan { + preWorkflowHooksErr = c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) + } if preWorkflowHooksErr != nil { if c.FailOnPreWorkflowHookError { diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index b2a8d8aae6..c6d413b0e7 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -517,6 +517,7 @@ func TestRunCommentCommand_IgnoredTargetedDirNoOp(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { vcsClient := setup(t) + ch.FailOnPreWorkflowHookError = true pull := &github.PullRequest{State: github.Ptr("open")} modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} cmd := &events.CommentCommand{Name: c.commandName, SubName: c.subName, RepoRelDir: "ignored"} @@ -524,10 +525,11 @@ func TestRunCommentCommand_IgnoredTargetedDirNoOp(t *testing.T) { When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) When(projectCommandBuilder.ShouldIgnoreTargetedDir(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn(true) + When(preWorkflowHooksCommandRunner.RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn(errors.New("err")) ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, cmd) - preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalled(Never()).RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]()) + preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalledOnce().RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]()) vcsClient.VerifyWasCalled(Never()).CreateComment( Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) commitUpdater.VerifyWasCalled(Never()).UpdateCombined( @@ -608,7 +610,7 @@ func TestRunCommentCommand_IgnoredTargetedDirSkipsValidationComments(t *testing. if c.verify != nil { c.verify(t, vcsClient) } - preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalled(Never()).RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]()) + preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalledOnce().RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]()) vcsClient.VerifyWasCalled(Never()).CreateComment( Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) commitUpdater.VerifyWasCalled(Never()).UpdateCombined( @@ -620,6 +622,36 @@ func TestRunCommentCommand_IgnoredTargetedDirSkipsValidationComments(t *testing. } } +func TestRunCommentCommand_IgnoredTargetedDirPreHooksCanGenerateExplicitConfig(t *testing.T) { + setup(t) + pull := &github.PullRequest{State: github.Ptr("open")} + modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} + cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored"} + projectCtx := command.ProjectContext{ + CommandName: command.Plan, + RepoRelDir: "ignored", + Workspace: events.DefaultWorkspace, + BaseRepo: testdata.GithubRepo, + Pull: modelPull, + } + ignoreChecks := 0 + + When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) + When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) + When(projectCommandBuilder.ShouldIgnoreTargetedDir(Any[*command.Context](), Any[*events.CommentCommand]())).Then(func(args []Param) ReturnValues { + ignoreChecks++ + return ReturnValues{ignoreChecks == 1} + }) + When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Eq(cmd))).ThenReturn([]command.ProjectContext{projectCtx}, nil) + When(projectCommandRunner.Plan(projectCtx)).ThenReturn(command.ProjectCommandOutput{PlanSuccess: &models.PlanSuccess{}}) + + ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, cmd) + + preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalledOnce().RunPreHooks(Any[*command.Context](), Eq(cmd)) + projectCommandBuilder.VerifyWasCalledOnce().BuildPlanCommands(Any[*command.Context](), Eq(cmd)) + projectCommandRunner.VerifyWasCalledOnce().Plan(projectCtx) +} + func TestRunCommentCommandApprovePolicy_NoProjects_SilenceEnabled(t *testing.T) { t.Log("if an approve_policy command is run on a pull request and SilenceNoProjects is enabled and we are silencing all comments if the modified files don't have a matching project") vcsClient := setup(t) diff --git a/server/events/instrumented_project_command_builder.go b/server/events/instrumented_project_command_builder.go index 2c037c6add..c9660ae338 100644 --- a/server/events/instrumented_project_command_builder.go +++ b/server/events/instrumented_project_command_builder.go @@ -74,8 +74,12 @@ func (b *InstrumentedProjectCommandBuilder) buildAndEmitStats( projectCmds, err := execute() if err != nil { - executionError.Inc(1) - b.Logger.Err("Error building %s commands: %s", command, err) + if IsIgnoredTargetedDir(err) { + executionSuccess.Inc(1) + } else { + executionError.Inc(1) + b.Logger.Err("Error building %s commands: %s", command, err) + } } else { executionSuccess.Inc(1) } diff --git a/server/events/instrumented_project_command_builder_test.go b/server/events/instrumented_project_command_builder_test.go new file mode 100644 index 0000000000..03ac497cd2 --- /dev/null +++ b/server/events/instrumented_project_command_builder_test.go @@ -0,0 +1,79 @@ +// Copyright 2026 The Atlantis Authors +// SPDX-License-Identifier: Apache-2.0 + +package events + +import ( + "testing" + + "github.com/runatlantis/atlantis/server/events/command" + "github.com/runatlantis/atlantis/server/logging" + "github.com/runatlantis/atlantis/server/metrics" + tally "github.com/uber-go/tally/v4" +) + +func TestInstrumentedProjectCommandBuilder_IgnoredTargetedDirIsNotErrorMetric(t *testing.T) { + scope := tally.NewTestScope("builder", nil) + builder := &InstrumentedProjectCommandBuilder{ + ProjectCommandBuilder: fakeInstrumentedProjectCommandBuilder{err: ErrIgnoredTargetedDir}, + Logger: logging.NewNoopLogger(t), + scope: scope, + } + + _, err := builder.BuildPlanCommands(&command.Context{}, &CommentCommand{}) + + if !IsIgnoredTargetedDir(err) { + t.Fatalf("expected ignored targeted dir error, got %v", err) + } + counters := scope.Snapshot().Counters() + if got := counterValue(counters, "builder."+metrics.ExecutionSuccessMetric+"+"); got != 1 { + t.Fatalf("expected success metric to be 1, got %d", got) + } + if got := counterValue(counters, "builder."+metrics.ExecutionErrorMetric+"+"); got != 0 { + t.Fatalf("expected error metric to be 0, got %d", got) + } +} + +func counterValue(counters map[string]tally.CounterSnapshot, name string) int64 { + counter, ok := counters[name] + if !ok { + return 0 + } + return counter.Value() +} + +type fakeInstrumentedProjectCommandBuilder struct { + err error +} + +func (f fakeInstrumentedProjectCommandBuilder) ShouldIgnoreTargetedDir(ctx *command.Context, comment *CommentCommand) bool { + return false +} + +func (f fakeInstrumentedProjectCommandBuilder) BuildAutoplanCommands(ctx *command.Context) ([]command.ProjectContext, error) { + return nil, f.err +} + +func (f fakeInstrumentedProjectCommandBuilder) BuildPlanCommands(ctx *command.Context, comment *CommentCommand) ([]command.ProjectContext, error) { + return nil, f.err +} + +func (f fakeInstrumentedProjectCommandBuilder) BuildApplyCommands(ctx *command.Context, comment *CommentCommand) ([]command.ProjectContext, error) { + return nil, f.err +} + +func (f fakeInstrumentedProjectCommandBuilder) BuildApprovePoliciesCommands(ctx *command.Context, comment *CommentCommand) ([]command.ProjectContext, error) { + return nil, f.err +} + +func (f fakeInstrumentedProjectCommandBuilder) BuildVersionCommands(ctx *command.Context, comment *CommentCommand) ([]command.ProjectContext, error) { + return nil, f.err +} + +func (f fakeInstrumentedProjectCommandBuilder) BuildImportCommands(ctx *command.Context, comment *CommentCommand) ([]command.ProjectContext, error) { + return nil, f.err +} + +func (f fakeInstrumentedProjectCommandBuilder) BuildStateRmCommands(ctx *command.Context, comment *CommentCommand) ([]command.ProjectContext, error) { + return nil, f.err +} From f61f28c2d72ff8e890fbbad396a60503b2fa8014 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 12:40:20 -0400 Subject: [PATCH 15/23] fix: validate ignored targets before hooks Run normal command validations before pre-workflow hooks even for initially ignored targeted directories, while keeping ignored-target validation failures silent. Prefer generated local repo config for the post-hook ignored-target recheck so explicit projects added by hooks are honored. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/command/context.go | 5 + server/events/command_runner.go | 104 ++++++++++-------- server/events/command_runner_test.go | 5 +- server/events/project_command_builder.go | 19 +++- server/events/project_command_builder_test.go | 83 ++++++++++++++ 5 files changed, 163 insertions(+), 53 deletions(-) diff --git a/server/events/command/context.go b/server/events/command/context.go index 998d6cfa5b..d07cc2d406 100644 --- a/server/events/command/context.go +++ b/server/events/command/context.go @@ -58,4 +58,9 @@ type Context struct { // Set true if the command was intentionally skipped without executing work. CommandSkipped bool + + // PreferLocalRepoCfgForTargetedIgnore makes targeted ignore checks read a + // cloned repo config before falling back to VCS content. This is used after + // pre-workflow hooks may have generated or updated atlantis.yaml. + PreferLocalRepoCfgForTargetedIgnore bool } diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 24fec98eae..af2e3cb42d 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -187,7 +187,7 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo PullStatus: status, Trigger: command.AutoTrigger, } - if !c.validateCtxAndComment(ctx, command.Autoplan) { + if !c.validateCtxAndComment(ctx, command.Autoplan, true) { return } if c.DisableAutoplan { @@ -412,6 +412,45 @@ func (c *DefaultCommandRunner) checkVarFilesInPlanCommandAllowlisted(cmd *Commen return c.VarFileAllowlistChecker.Check(cmd.Flags) } +func (c *DefaultCommandRunner) validateCommentCommand(ctx *command.Context, baseRepo models.Repo, pullNum int, user models.User, cmd *CommentCommand, shouldComment bool) bool { + // Check if the user who commented has the permissions to execute the 'plan' or 'apply' commands + if c.TeamAllowlistChecker != nil && c.TeamAllowlistChecker.HasRules() { + err := c.fetchUserTeams(ctx.Log, baseRepo, &user) + if err != nil { + c.Logger.Err("Unable to fetch user teams: %s", err) + return false + } + directUserTeams := append([]string(nil), user.Teams...) + + ok, err := c.checkUserPermissions(baseRepo, &user, cmd.Name.String()) + if err != nil { + c.Logger.Err("Unable to check user permissions: %s", err) + return false + } + if !ok { + if shouldComment { + c.commentUserDoesNotHavePermissions(baseRepo, pullNum, user, cmd) + } + return false + } + c.addPolicyCheckHierarchyTeamsForPlan(baseRepo, &user, cmd.Name, directUserTeams) + ctx.User = user + } + + // Check if the provided var files in a 'plan' command are allowlisted + if err := c.checkVarFilesInPlanCommandAllowlisted(cmd); err != nil { + if shouldComment { + errMsg := fmt.Sprintf("```\n%s\n```", err.Error()) + if commentErr := c.VCSClient.CreateComment(c.Logger, baseRepo, pullNum, errMsg, ""); commentErr != nil { + c.Logger.Err("unable to comment on pull request: %s", commentErr) + } + } + return false + } + + return c.validateCtxAndComment(ctx, cmd.Name, shouldComment) +} + // RunCommentCommand executes the command. // We take in a pointer for maybeHeadRepo because for some events there isn't // enough data to construct the Repo model and callers might want to wait until @@ -462,56 +501,23 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead } cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) - preWorkflowHooksRan := false - var preWorkflowHooksErr error - if shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) { + targetInitiallyIgnored := shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) + if targetInitiallyIgnored { ctx.CommandSkipped = false - preWorkflowHooksErr = c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) - preWorkflowHooksRan = true - ctx.CommandSkipped = false - if shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) { - return - } } - // Check if the user who commented has the permissions to execute the 'plan' or 'apply' commands - if c.TeamAllowlistChecker != nil && c.TeamAllowlistChecker.HasRules() { - err := c.fetchUserTeams(log, baseRepo, &user) - if err != nil { - c.Logger.Err("Unable to fetch user teams: %s", err) - return - } - directUserTeams := append([]string(nil), user.Teams...) - - ok, err := c.checkUserPermissions(baseRepo, &user, cmd.Name.String()) - if err != nil { - c.Logger.Err("Unable to check user permissions: %s", err) - return - } - if !ok { - c.commentUserDoesNotHavePermissions(baseRepo, pullNum, user, cmd) - return - } - c.addPolicyCheckHierarchyTeamsForPlan(baseRepo, &user, cmd.Name, directUserTeams) - } - - // Check if the provided var files in a 'plan' command are allowlisted - if err := c.checkVarFilesInPlanCommandAllowlisted(cmd); err != nil { - errMsg := fmt.Sprintf("```\n%s\n```", err.Error()) - if commentErr := c.VCSClient.CreateComment(c.Logger, baseRepo, pullNum, errMsg, ""); commentErr != nil { - c.Logger.Err("unable to comment on pull request: %s", commentErr) - } - return - } - - if !c.validateCtxAndComment(ctx, cmd.Name) { + if !c.validateCommentCommand(ctx, baseRepo, pullNum, user, cmd, !targetInitiallyIgnored) { return } - if !preWorkflowHooksRan { - preWorkflowHooksErr = c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) + preWorkflowHooksErr := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) + if targetInitiallyIgnored { + ctx.CommandSkipped = false + ctx.PreferLocalRepoCfgForTargetedIgnore = true + if shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) { + return + } } - if preWorkflowHooksErr != nil { if c.FailOnPreWorkflowHookError { ctx.Log.Err("'fail-on-pre-workflow-hook-error' set, so not running %s command.", cmd.Name.String()) @@ -664,9 +670,9 @@ func (c *DefaultCommandRunner) fetchUserTeams(logger logging.SimpleLogging, repo return nil } -func (c *DefaultCommandRunner) validateCtxAndComment(ctx *command.Context, commandName command.Name) bool { +func (c *DefaultCommandRunner) validateCtxAndComment(ctx *command.Context, commandName command.Name, shouldComment bool) bool { if !c.AllowForkPRs && ctx.HeadRepo.Owner != ctx.Pull.BaseRepo.Owner { - if c.SilenceForkPRErrors { + if c.SilenceForkPRErrors || !shouldComment { return false } ctx.Log.Info("command was run on a fork pull request which is disallowed") @@ -678,8 +684,10 @@ func (c *DefaultCommandRunner) validateCtxAndComment(ctx *command.Context, comma if ctx.Pull.State != models.OpenPullState && commandName != command.Unlock { ctx.Log.Info("command was run on closed pull request") - if err := c.VCSClient.CreateComment(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull.Num, "Atlantis commands can't be run on closed pull requests", ""); err != nil { - ctx.Log.Err("unable to comment: %s", err) + if shouldComment { + if err := c.VCSClient.CreateComment(ctx.Log, ctx.Pull.BaseRepo, ctx.Pull.Num, "Atlantis commands can't be run on closed pull requests", ""); err != nil { + ctx.Log.Err("unable to comment: %s", err) + } } return false } diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index c6d413b0e7..8864858bd8 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -557,9 +557,10 @@ func TestRunCommentCommand_IgnoredTargetedDirSkipsValidationComments(t *testing. checker, err := command.NewTeamAllowlistChecker("allowed-team:apply") Ok(t, err) ch.TeamAllowlistChecker = checker + When(vcsClient.GetTeamNamesForUser(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.User))).ThenReturn([]string{"blocked-team"}, nil) }, verify: func(t *testing.T, vcsClient *vcsmocks.MockClient) { - vcsClient.VerifyWasCalled(Never()).GetTeamNamesForUser(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.User)) + vcsClient.VerifyWasCalledOnce().GetTeamNamesForUser(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.User)) }, }, { @@ -610,7 +611,7 @@ func TestRunCommentCommand_IgnoredTargetedDirSkipsValidationComments(t *testing. if c.verify != nil { c.verify(t, vcsClient) } - preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalledOnce().RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]()) + preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalled(Never()).RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]()) vcsClient.VerifyWasCalled(Never()).CreateComment( Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) commitUpdater.VerifyWasCalled(Never()).UpdateCombined( diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 2935d45849..4679b98e7f 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -470,6 +470,12 @@ func (p *DefaultProjectCommandBuilder) ShouldIgnoreTargetedDir(ctx *command.Cont } func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Context) valid.RepoCfg { + if ctx.PreferLocalRepoCfgForTargetedIgnore { + if repoCfg, ok := p.repoCfgFromWorkingDir(ctx); ok { + return repoCfg + } + } + if p.VCSClient != nil { repoCfgFile := p.GlobalCfg.RepoConfigFile(ctx.Pull.BaseRepo.ID()) hasRepoCfg, repoCfgData, err := p.VCSClient.GetFileContent(ctx.Log, ctx.HeadRepo, ctx.Pull.HeadBranch, repoCfgFile) @@ -485,15 +491,22 @@ func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Con } } + if repoCfg, ok := p.repoCfgFromWorkingDir(ctx); ok { + return repoCfg + } + return valid.RepoCfg{} +} + +func (p *DefaultProjectCommandBuilder) repoCfgFromWorkingDir(ctx *command.Context) (valid.RepoCfg, bool) { repoDir, err := p.WorkingDir.GetWorkingDir(ctx.Pull.BaseRepo, ctx.Pull, DefaultWorkspace) if err != nil { - return valid.RepoCfg{} + return valid.RepoCfg{}, false } repoCfg, _, err := p.parseRepoCfg(ctx, repoDir) if err != nil { - return valid.RepoCfg{} + return valid.RepoCfg{}, false } - return repoCfg + return repoCfg, true } func (p *DefaultProjectCommandBuilder) shouldIgnoreTargetedDirFromCfg(ctx *command.Context, repoCfg valid.RepoCfg, repoRelDir string) bool { diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 6a5c7669e8..99e466bca9 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -880,6 +880,89 @@ func TestDefaultProjectCommandBuilder_BuildTargetedNonPlanCommand_IgnorePathsWit } } +func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirPrefersGeneratedLocalConfig(t *testing.T) { + RegisterMockTestingT(t) + + remoteAtlantisYAML := ` +version: 3 +autodiscover: + mode: enabled + ignore_paths: + - "environments/prod/**" +` + localAtlantisYAML := ` +version: 3 +autodiscover: + mode: enabled + ignore_paths: + - "environments/prod/**" +projects: +- dir: environments/prod +` + tmpDir := DirStructure(t, map[string]any{ + "environments": map[string]any{ + "prod": map[string]any{ + "main.tf": nil, + }, + }, + }) + err := os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(localAtlantisYAML), 0600) + Ok(t, err) + + workingDir := mocks.NewMockWorkingDir() + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Eq(events.DefaultWorkspace))).ThenReturn(tmpDir, nil) + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Eq("feature"), Eq(valid.DefaultAtlantisFile))).ThenReturn(true, []byte(remoteAtlantisYAML), nil) + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{AllowAllRepoSettings: true}) + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + baseRepo := models.Repo{Owner: "owner", Name: "repo", FullName: "owner/repo", VCSHost: models.VCSHost{Type: models.Github}} + ctx := &command.Context{ + Log: logger, + Scope: scope, + HeadRepo: baseRepo, + Pull: models.PullRequest{ + Num: 1, + BaseBranch: "main", + HeadBranch: "feature", + BaseRepo: baseRepo, + }, + } + cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "environments/prod", Workspace: events.DefaultWorkspace} + + Assert(t, builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected remote config without explicit project to ignore target") + + ctx.PreferLocalRepoCfgForTargetedIgnore = true + Assert(t, !builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected generated local explicit project to prevent ignored-target skip") +} + // Test that autodiscover.ignore_paths set in repo-level atlantis.yaml blocks // targeted plan/apply -d commands for non-configured projects. func TestDefaultProjectCommandBuilder_BuildTargetedCommand_IgnorePathsRepoCfg(t *testing.T) { From 3110355fccc3c599c81c9ac7b82dc6bb28465008 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 13:38:28 -0400 Subject: [PATCH 16/23] fix: avoid head config for merge checkout ignore checks Use the local merged checkout config for targeted ignore checks when checkout-strategy=merge is enabled. If no merged checkout exists yet, fail open so commands can clone and evaluate the merged config instead of silently skipping from head-branch config. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/github_app_working_dir.go | 7 + server/events/project_command_builder.go | 34 ++++- server/events/project_command_builder_test.go | 139 ++++++++++++++++++ server/events/working_dir.go | 4 + 4 files changed, 178 insertions(+), 6 deletions(-) diff --git a/server/events/github_app_working_dir.go b/server/events/github_app_working_dir.go index 73274aadff..ac27ab3c88 100644 --- a/server/events/github_app_working_dir.go +++ b/server/events/github_app_working_dir.go @@ -33,6 +33,13 @@ func (g *GithubAppWorkingDir) MergeAgain(logger logging.SimpleLogging, headRepo return g.WorkingDir.MergeAgain(logger, headRepo, p, workspace) } +func (g *GithubAppWorkingDir) CheckoutMergeEnabled() bool { + checkoutMerge, ok := g.WorkingDir.(interface { + CheckoutMergeEnabled() bool + }) + return ok && checkoutMerge.CheckoutMergeEnabled() +} + func (g *GithubAppWorkingDir) fixReposURL(p *models.PullRequest, headRepo *models.Repo) { // Realistically, this is a super brittle way of supporting clones using gh app installation tokens // This URL should be built during Repo creation and the struct should be immutable going forward. diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 4679b98e7f..9d5b8c0d1c 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -465,15 +465,26 @@ func (p *DefaultProjectCommandBuilder) ShouldIgnoreTargetedDir(ctx *command.Cont if cmd.ProjectName != "" || cmd.RepoRelDir == "" || valid.ContainsDirGlobPattern(cmd.RepoRelDir) { return false } - repoCfg := p.repoCfgForTargetedIgnore(ctx) + repoCfg, ok := p.repoCfgForTargetedIgnore(ctx) + if !ok { + return false + } return p.shouldIgnoreTargetedDirFromCfg(ctx, repoCfg, cmd.RepoRelDir) } -func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Context) valid.RepoCfg { +func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Context) (valid.RepoCfg, bool) { if ctx.PreferLocalRepoCfgForTargetedIgnore { if repoCfg, ok := p.repoCfgFromWorkingDir(ctx); ok { - return repoCfg + return repoCfg, true + } + } + + if p.needsLocalRepoCfgForTargetedIgnore(ctx) { + if repoCfg, ok := p.repoCfgFromWorkingDir(ctx); ok { + return repoCfg, true } + ctx.Log.Debug("not checking remote repo config for targeted ignore because merge checkout needs the merged local config") + return valid.RepoCfg{}, false } if p.VCSClient != nil { @@ -486,15 +497,15 @@ func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Con if err != nil { ctx.Log.Debug("unable to parse %s while checking autodiscover.ignore_paths: %s", repoCfgFile, err) } else { - return repoCfg + return repoCfg, true } } } if repoCfg, ok := p.repoCfgFromWorkingDir(ctx); ok { - return repoCfg + return repoCfg, true } - return valid.RepoCfg{} + return valid.RepoCfg{}, true } func (p *DefaultProjectCommandBuilder) repoCfgFromWorkingDir(ctx *command.Context) (valid.RepoCfg, bool) { @@ -509,6 +520,17 @@ func (p *DefaultProjectCommandBuilder) repoCfgFromWorkingDir(ctx *command.Contex return repoCfg, true } +func (p *DefaultProjectCommandBuilder) needsLocalRepoCfgForTargetedIgnore(ctx *command.Context) bool { + return ctx.Pull.Num > 0 && workingDirUsesMergeCheckout(p.WorkingDir) +} + +func workingDirUsesMergeCheckout(workingDir WorkingDir) bool { + checkoutMerge, ok := workingDir.(interface { + CheckoutMergeEnabled() bool + }) + return ok && checkoutMerge.CheckoutMergeEnabled() +} + func (p *DefaultProjectCommandBuilder) shouldIgnoreTargetedDirFromCfg(ctx *command.Context, repoCfg valid.RepoCfg, repoRelDir string) bool { if !p.autoDiscoverModeEnabled(ctx, repoCfg) { return false diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 99e466bca9..02f4185546 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -963,6 +963,145 @@ projects: Assert(t, !builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected generated local explicit project to prevent ignored-target skip") } +func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirMergeCheckoutUsesLocalConfig(t *testing.T) { + RegisterMockTestingT(t) + + remoteAtlantisYAML := "version: 3\n" + + "autodiscover:\n" + + " mode: enabled\n" + + " ignore_paths:\n" + + " - \"environments/prod/**\"\n" + localAtlantisYAML := "version: 3\n" + + "autodiscover:\n" + + " mode: enabled\n" + + " ignore_paths:\n" + + " - \"environments/prod/**\"\n" + + "projects:\n" + + "- dir: environments/prod\n" + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{AllowAllRepoSettings: true}) + terraformClient := tfclientmocks.NewMockClient() + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Eq("feature"), Eq(valid.DefaultAtlantisFile))).ThenReturn(true, []byte(remoteAtlantisYAML), nil) + + dataDir := t.TempDir() + workingDir := &events.FileWorkspace{ + DataDir: dataDir, + CheckoutMerge: true, + } + baseRepo := models.Repo{Owner: "owner", Name: "repo", FullName: "owner/repo", VCSHost: models.VCSHost{Type: models.Github}} + pull := models.PullRequest{ + Num: 1, + BaseBranch: "main", + HeadBranch: "feature", + BaseRepo: baseRepo, + } + repoDir := filepath.Join(dataDir, "repos", baseRepo.FullName, "1", events.DefaultWorkspace) + Ok(t, os.MkdirAll(repoDir, 0700)) + Ok(t, os.WriteFile(filepath.Join(repoDir, valid.DefaultAtlantisFile), []byte(localAtlantisYAML), 0600)) + + userConfig := defaultUserConfig + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + ctx := &command.Context{ + Log: logger, + Scope: scope, + HeadRepo: baseRepo, + Pull: pull, + } + cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "environments/prod", Workspace: events.DefaultWorkspace} + + Assert(t, !builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected merged local explicit project to prevent ignored-target skip") + vcsClient.VerifyWasCalled(Never()).GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Any[string](), Any[string]()) +} + +func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirMergeCheckoutWithoutLocalConfigFailsOpen(t *testing.T) { + RegisterMockTestingT(t) + + remoteAtlantisYAML := "version: 3\n" + + "autodiscover:\n" + + " mode: enabled\n" + + " ignore_paths:\n" + + " - \"environments/prod/**\"\n" + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{AllowAllRepoSettings: true}) + terraformClient := tfclientmocks.NewMockClient() + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Eq("feature"), Eq(valid.DefaultAtlantisFile))).ThenReturn(true, []byte(remoteAtlantisYAML), nil) + + workingDir := &events.FileWorkspace{ + DataDir: t.TempDir(), + CheckoutMerge: true, + } + userConfig := defaultUserConfig + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + baseRepo := models.Repo{Owner: "owner", Name: "repo", FullName: "owner/repo", VCSHost: models.VCSHost{Type: models.Github}} + ctx := &command.Context{ + Log: logger, + Scope: scope, + HeadRepo: baseRepo, + Pull: models.PullRequest{ + Num: 1, + BaseBranch: "main", + HeadBranch: "feature", + BaseRepo: baseRepo, + }, + } + cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "environments/prod", Workspace: events.DefaultWorkspace} + + Assert(t, !builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected merge checkout without local config to avoid pre-clone skip") + vcsClient.VerifyWasCalled(Never()).GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Any[string](), Any[string]()) +} + // Test that autodiscover.ignore_paths set in repo-level atlantis.yaml blocks // targeted plan/apply -d commands for non-configured projects. func TestDefaultProjectCommandBuilder_BuildTargetedCommand_IgnorePathsRepoCfg(t *testing.T) { diff --git a/server/events/working_dir.go b/server/events/working_dir.go index dd1ff13912..be84133a74 100644 --- a/server/events/working_dir.go +++ b/server/events/working_dir.go @@ -109,6 +109,10 @@ type FileWorkspace struct { CheckForUpstreamChanges bool } +func (w *FileWorkspace) CheckoutMergeEnabled() bool { + return w.CheckoutMerge +} + // Clone git clones headRepo, checks out the branch and then returns the absolute // path to the root of the cloned repo. // If the repo already exists and is at From a90f686af9d47655ae6150d212a570727e83e026 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 14:05:54 -0400 Subject: [PATCH 17/23] fix: validate pulls before targeted ignore config reads Validate fork, closed, and base-branch state before the early targeted-ignore check can ask the builder to read repo config. Also read targeted-ignore repo config at the PR head commit when available so skip decisions match the checkout ref. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/command_runner.go | 6 +- server/events/command_runner_test.go | 88 ++++++++++++++++--- server/events/project_command_builder.go | 9 +- server/events/project_command_builder_test.go | 63 +++++++++++++ 4 files changed, 150 insertions(+), 16 deletions(-) diff --git a/server/events/command_runner.go b/server/events/command_runner.go index af2e3cb42d..6b078cc8fd 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -448,7 +448,7 @@ func (c *DefaultCommandRunner) validateCommentCommand(ctx *command.Context, base return false } - return c.validateCtxAndComment(ctx, cmd.Name, shouldComment) + return true } // RunCommentCommand executes the command. @@ -500,6 +500,10 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead TeamAllowlistChecker: c.TeamAllowlistChecker, } + if !c.validateCtxAndComment(ctx, cmd.Name, true) { + return + } + cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) targetInitiallyIgnored := shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) if targetInitiallyIgnored { diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 8864858bd8..784f60e0de 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -572,25 +572,12 @@ func TestRunCommentCommand_IgnoredTargetedDirSkipsValidationComments(t *testing. ch.VarFileAllowlistChecker = checker }, }, - { - name: "fork pull request", - cmd: &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored"}, - headRepo: models.Repo{ - Owner: "forkrepo", - FullName: "forkrepo/atlantis", - }, - }, - { - name: "closed pull request", - cmd: &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored"}, - modelPull: models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.ClosedPullState, Num: testdata.Pull.Num}, - }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { vcsClient := setup(t) - if c.modelPull.BaseRepo.ID() == "" { + if c.modelPull.BaseRepo.FullName == "" { c.modelPull = models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} } if c.headRepo.FullName == "" { @@ -623,6 +610,79 @@ func TestRunCommentCommand_IgnoredTargetedDirSkipsValidationComments(t *testing. } } +func TestRunCommentCommand_IgnoredTargetedDirValidatesPullBeforeIgnoreCheck(t *testing.T) { + cases := []struct { + name string + modelPull models.PullRequest + headRepo models.Repo + setup func() + wantComment string + }{ + { + name: "fork pull request", + modelPull: models.PullRequest{ + BaseRepo: testdata.GithubRepo, + State: models.OpenPullState, + Num: testdata.Pull.Num, + }, + headRepo: models.Repo{ + Owner: "forkrepo", + FullName: "forkrepo/atlantis", + }, + wantComment: fmt.Sprintf("Atlantis commands can't be run on fork pull requests. To enable, set --%s or, to disable this message, set --%s", "allow-fork-prs-flag", ""), + }, + { + name: "closed pull request", + modelPull: models.PullRequest{ + BaseRepo: testdata.GithubRepo, + State: models.ClosedPullState, + Num: testdata.Pull.Num, + }, + headRepo: testdata.GithubRepo, + wantComment: "Atlantis commands can't be run on closed pull requests", + }, + { + name: "base branch mismatch", + modelPull: models.PullRequest{ + BaseRepo: testdata.GithubRepo, + State: models.OpenPullState, + Num: testdata.Pull.Num, + BaseBranch: "release", + }, + headRepo: testdata.GithubRepo, + setup: func() { + ch.GlobalCfg.Repos[0].BranchRegex = regexp.MustCompile("^main$") + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + vcsClient := setup(t) + if c.setup != nil { + c.setup() + } + + pull := &github.PullRequest{State: github.Ptr("open")} + When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) + When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(c.modelPull, c.modelPull.BaseRepo, c.headRepo, nil) + When(projectCommandBuilder.ShouldIgnoreTargetedDir(Any[*command.Context](), Any[*events.CommentCommand]())).ThenReturn(true) + + ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored"}) + + projectCommandBuilder.VerifyWasCalled(Never()).ShouldIgnoreTargetedDir(Any[*command.Context](), Any[*events.CommentCommand]()) + preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalled(Never()).RunPreHooks(Any[*command.Context](), Any[*events.CommentCommand]()) + if c.wantComment == "" { + vcsClient.VerifyWasCalled(Never()).CreateComment( + Any[logging.SimpleLogging](), Any[models.Repo](), Any[int](), Any[string](), Any[string]()) + } else { + vcsClient.VerifyWasCalledOnce().CreateComment( + Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(c.modelPull.Num), Eq(c.wantComment), Eq("")) + } + }) + } +} + func TestRunCommentCommand_IgnoredTargetedDirPreHooksCanGenerateExplicitConfig(t *testing.T) { setup(t) pull := &github.PullRequest{State: github.Ptr("open")} diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 9d5b8c0d1c..90339ecbc6 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -489,7 +489,7 @@ func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Con if p.VCSClient != nil { repoCfgFile := p.GlobalCfg.RepoConfigFile(ctx.Pull.BaseRepo.ID()) - hasRepoCfg, repoCfgData, err := p.VCSClient.GetFileContent(ctx.Log, ctx.HeadRepo, ctx.Pull.HeadBranch, repoCfgFile) + hasRepoCfg, repoCfgData, err := p.VCSClient.GetFileContent(ctx.Log, ctx.HeadRepo, targetedIgnoreConfigRef(ctx.Pull), repoCfgFile) if err != nil { ctx.Log.Debug("unable to fetch %s while checking autodiscover.ignore_paths: %s", repoCfgFile, err) } else if hasRepoCfg { @@ -508,6 +508,13 @@ func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Con return valid.RepoCfg{}, true } +func targetedIgnoreConfigRef(pull models.PullRequest) string { + if pull.HeadCommit != "" { + return pull.HeadCommit + } + return pull.HeadBranch +} + func (p *DefaultProjectCommandBuilder) repoCfgFromWorkingDir(ctx *command.Context) (valid.RepoCfg, bool) { repoDir, err := p.WorkingDir.GetWorkingDir(ctx.Pull.BaseRepo, ctx.Pull, DefaultWorkspace) if err != nil { diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 02f4185546..75572005d2 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -963,6 +963,69 @@ projects: Assert(t, !builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected generated local explicit project to prevent ignored-target skip") } +func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirUsesHeadCommitForRemoteConfig(t *testing.T) { + RegisterMockTestingT(t) + + atlantisYAML := "version: 3\n" + + "autodiscover:\n" + + " mode: enabled\n" + + " ignore_paths:\n" + + " - \"environments/prod/**\"\n" + + workingDir := mocks.NewMockWorkingDir() + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Eq(events.DefaultWorkspace))).ThenReturn("", os.ErrNotExist) + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Eq("abc123"), Eq(valid.DefaultAtlantisFile))).ThenReturn(true, []byte(atlantisYAML), nil) + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{AllowAllRepoSettings: true}) + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + baseRepo := models.Repo{Owner: "owner", Name: "repo", FullName: "owner/repo", VCSHost: models.VCSHost{Type: models.Github}} + ctx := &command.Context{ + Log: logger, + Scope: scope, + HeadRepo: baseRepo, + Pull: models.PullRequest{ + Num: 1, + BaseBranch: "main", + HeadBranch: "feature", + HeadCommit: "abc123", + BaseRepo: baseRepo, + }, + } + cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "environments/prod", Workspace: events.DefaultWorkspace} + + Assert(t, builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected remote head commit config to ignore target") + vcsClient.VerifyWasCalledOnce().GetFileContent(Any[logging.SimpleLogging](), Eq(baseRepo), Eq("abc123"), Eq(valid.DefaultAtlantisFile)) + vcsClient.VerifyWasCalled(Never()).GetFileContent(Any[logging.SimpleLogging](), Eq(baseRepo), Eq("feature"), Eq(valid.DefaultAtlantisFile)) +} + func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirMergeCheckoutUsesLocalConfig(t *testing.T) { RegisterMockTestingT(t) From 65c129ef1b7dd407f2790789172cda4878c18a75 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 14:22:41 -0400 Subject: [PATCH 18/23] fix: fail open on unknown targeted ignore config Avoid making early targeted-ignore skip decisions from stale local checkout data or empty config when the current ref config could not be fetched or parsed. Keep authoritative current-ref missing config as an empty config so global ignore_paths still work when the VCS confirms no repo config exists. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/project_command_builder.go | 8 +- server/events/project_command_builder_test.go | 130 ++++++++++++++++++ 2 files changed, 134 insertions(+), 4 deletions(-) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 90339ecbc6..afc3c5c586 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -492,20 +492,20 @@ func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Con hasRepoCfg, repoCfgData, err := p.VCSClient.GetFileContent(ctx.Log, ctx.HeadRepo, targetedIgnoreConfigRef(ctx.Pull), repoCfgFile) if err != nil { ctx.Log.Debug("unable to fetch %s while checking autodiscover.ignore_paths: %s", repoCfgFile, err) + return valid.RepoCfg{}, false } else if hasRepoCfg { repoCfg, err := p.ParserValidator.ParseRepoCfgData(repoCfgData, p.GlobalCfg, ctx.Pull.BaseRepo.ID(), ctx.Pull.BaseBranch) if err != nil { ctx.Log.Debug("unable to parse %s while checking autodiscover.ignore_paths: %s", repoCfgFile, err) + return valid.RepoCfg{}, false } else { return repoCfg, true } } + return valid.RepoCfg{}, true } - if repoCfg, ok := p.repoCfgFromWorkingDir(ctx); ok { - return repoCfg, true - } - return valid.RepoCfg{}, true + return valid.RepoCfg{}, false } func targetedIgnoreConfigRef(pull models.PullRequest) string { diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 75572005d2..0432bf258d 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -1026,6 +1026,136 @@ func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirUsesHeadCommitForRe vcsClient.VerifyWasCalled(Never()).GetFileContent(Any[logging.SimpleLogging](), Eq(baseRepo), Eq("feature"), Eq(valid.DefaultAtlantisFile)) } +func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirFailsOpenWhenRemoteConfigUnknown(t *testing.T) { + RegisterMockTestingT(t) + + staleLocalAtlantisYAML := "version: 3\n" + + "autodiscover:\n" + + " mode: enabled\n" + + " ignore_paths:\n" + + " - \"environments/prod/**\"\n" + tmpDir := DirStructure(t, map[string]any{ + "environments": map[string]any{ + "prod": map[string]any{ + "main.tf": nil, + }, + }, + }) + Ok(t, os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(staleLocalAtlantisYAML), 0600)) + + workingDir := mocks.NewMockWorkingDir() + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Eq(events.DefaultWorkspace))).ThenReturn(tmpDir, nil) + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Eq("abc123"), Eq(valid.DefaultAtlantisFile))).ThenReturn(false, []byte{}, errors.New("not implemented")) + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{AllowAllRepoSettings: true}) + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + baseRepo := models.Repo{Owner: "owner", Name: "repo", FullName: "owner/repo", VCSHost: models.VCSHost{Type: models.Github}} + ctx := &command.Context{ + Log: logger, + Scope: scope, + HeadRepo: baseRepo, + Pull: models.PullRequest{ + Num: 1, + BaseBranch: "main", + HeadBranch: "feature", + HeadCommit: "abc123", + BaseRepo: baseRepo, + }, + } + cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "environments/prod", Workspace: events.DefaultWorkspace} + + Assert(t, !builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected unknown current repo config to avoid pre-clone skip") + workingDir.VerifyWasCalled(Never()).GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]()) +} + +func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirAllowsAuthoritativeMissingRemoteConfig(t *testing.T) { + RegisterMockTestingT(t) + + workingDir := mocks.NewMockWorkingDir() + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Eq(events.DefaultWorkspace))).ThenReturn("", os.ErrNotExist) + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Eq("abc123"), Eq(valid.DefaultAtlantisFile))).ThenReturn(false, []byte{}, nil) + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{AllowAllRepoSettings: true}) + globalCfg.Repos[0].AutoDiscover = &valid.AutoDiscover{ + Mode: valid.AutoDiscoverEnabledMode, + IgnorePaths: []string{"environments/prod/**"}, + } + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + baseRepo := models.Repo{Owner: "owner", Name: "repo", FullName: "owner/repo", VCSHost: models.VCSHost{Type: models.Github}} + ctx := &command.Context{ + Log: logger, + Scope: scope, + HeadRepo: baseRepo, + Pull: models.PullRequest{ + Num: 1, + BaseBranch: "main", + HeadBranch: "feature", + HeadCommit: "abc123", + BaseRepo: baseRepo, + }, + } + cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "environments/prod", Workspace: events.DefaultWorkspace} + + Assert(t, builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected authoritative missing repo config to allow global ignored-target skip") + workingDir.VerifyWasCalled(Never()).GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]()) +} + func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirMergeCheckoutUsesLocalConfig(t *testing.T) { RegisterMockTestingT(t) From cfa71fd7f7f665990f5aef18276b157776c37c76 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 15:07:02 -0400 Subject: [PATCH 19/23] fix: avoid stale local config after no-op hooks Only prefer local repo config for the post-hook targeted-ignore recheck when configured pre-workflow hooks could have refreshed or generated that config. This keeps no-hook ignored targets from overriding an authoritative current-head ignore decision with stale working-dir state. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/command_runner.go | 9 ++++ server/events/command_runner_test.go | 42 ++++++++++++++++++- .../pre_workflow_hooks_command_runner.go | 25 ++++++++--- 3 files changed, 69 insertions(+), 7 deletions(-) diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 6b078cc8fd..723681c633 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -93,6 +93,11 @@ func shouldSkipPreWorkflowHooks(ctx *command.Context, cmdRunner CommentCommandRu return ok && skipper.ShouldSkipPreWorkflowHooks(ctx, cmd) } +func preWorkflowHooksConfigured(runner PreWorkflowHooksCommandRunner, ctx *command.Context) bool { + checker, ok := runner.(PreWorkflowHooksConfiguredChecker) + return ok && checker.HasPreWorkflowHooks(ctx) +} + // DefaultCommandRunner is the first step when processing a comment command. type DefaultCommandRunner struct { VCSClient vcs.Client `validate:"required"` @@ -514,9 +519,13 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead return } + preWorkflowHooksMayUpdateRepo := preWorkflowHooksConfigured(c.PreWorkflowHooksCommandRunner, ctx) preWorkflowHooksErr := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) if targetInitiallyIgnored { ctx.CommandSkipped = false + if !preWorkflowHooksMayUpdateRepo || preWorkflowHooksErr != nil { + return + } ctx.PreferLocalRepoCfgForTargetedIgnore = true if shouldSkipPreWorkflowHooks(ctx, cmdRunner, cmd) { return diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 784f60e0de..90cb88cf1c 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -78,6 +78,21 @@ type TestConfig struct { applyLockCheckerErr error } +type configuredPreWorkflowHooksCommandRunner struct { + hasHooks bool + err error + calls int +} + +func (r *configuredPreWorkflowHooksCommandRunner) RunPreHooks(_ *command.Context, _ *events.CommentCommand) error { + r.calls++ + return r.err +} + +func (r *configuredPreWorkflowHooksCommandRunner) HasPreWorkflowHooks(_ *command.Context) bool { + return r.hasHooks +} + func setup(t *testing.T, options ...func(testConfig *TestConfig)) *vcsmocks.MockClient { RegisterMockTestingT(t) @@ -683,8 +698,33 @@ func TestRunCommentCommand_IgnoredTargetedDirValidatesPullBeforeIgnoreCheck(t *t } } +func TestRunCommentCommand_IgnoredTargetedDirNoHooksDoesNotUseLocalConfig(t *testing.T) { + setup(t) + pull := &github.PullRequest{State: github.Ptr("open")} + modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} + cmd := &events.CommentCommand{Name: command.Apply, RepoRelDir: "ignored"} + ignoreChecks := 0 + + When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) + When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) + When(projectCommandBuilder.ShouldIgnoreTargetedDir(Any[*command.Context](), Any[*events.CommentCommand]())).Then(func(args []Param) ReturnValues { + ignoreChecks++ + return ReturnValues{ignoreChecks == 1} + }) + + ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, cmd) + + Equals(t, 1, ignoreChecks) + preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalledOnce().RunPreHooks(Any[*command.Context](), Eq(cmd)) + projectCommandBuilder.VerifyWasCalled(Never()).BuildApplyCommands(Any[*command.Context](), Any[*events.CommentCommand]()) + projectCommandRunner.VerifyWasCalled(Never()).Apply(Any[command.ProjectContext]()) +} + func TestRunCommentCommand_IgnoredTargetedDirPreHooksCanGenerateExplicitConfig(t *testing.T) { setup(t) + configuredPreHooks := &configuredPreWorkflowHooksCommandRunner{hasHooks: true} + preWorkflowHooksCommandRunner = configuredPreHooks + ch.PreWorkflowHooksCommandRunner = configuredPreHooks pull := &github.PullRequest{State: github.Ptr("open")} modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored"} @@ -708,7 +748,7 @@ func TestRunCommentCommand_IgnoredTargetedDirPreHooksCanGenerateExplicitConfig(t ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, cmd) - preWorkflowHooksCommandRunner.(*mocks.MockPreWorkflowHooksCommandRunner).VerifyWasCalledOnce().RunPreHooks(Any[*command.Context](), Eq(cmd)) + Equals(t, 1, configuredPreHooks.calls) projectCommandBuilder.VerifyWasCalledOnce().BuildPlanCommands(Any[*command.Context](), Eq(cmd)) projectCommandRunner.VerifyWasCalledOnce().Plan(projectCtx) } diff --git a/server/events/pre_workflow_hooks_command_runner.go b/server/events/pre_workflow_hooks_command_runner.go index f5e6f72b43..92d7912366 100644 --- a/server/events/pre_workflow_hooks_command_runner.go +++ b/server/events/pre_workflow_hooks_command_runner.go @@ -28,6 +28,10 @@ type PreWorkflowHooksCommandRunner interface { RunPreHooks(ctx *command.Context, cmd *CommentCommand) error } +type PreWorkflowHooksConfiguredChecker interface { + HasPreWorkflowHooks(ctx *command.Context) bool +} + // DefaultPreWorkflowHooksCommandRunner is the first step when processing a workflow hook commands. type DefaultPreWorkflowHooksCommandRunner struct { VCSClient vcs.Client `validate:"required"` @@ -41,12 +45,7 @@ type DefaultPreWorkflowHooksCommandRunner struct { // RunPreHooks runs pre_workflow_hooks when PR is opened or updated. func (w *DefaultPreWorkflowHooksCommandRunner) RunPreHooks(ctx *command.Context, cmd *CommentCommand) error { - preWorkflowHooks := make([]*valid.WorkflowHook, 0) - for _, repo := range w.GlobalCfg.Repos { - if repo.IDMatches(ctx.Pull.BaseRepo.ID()) && len(repo.PreWorkflowHooks) > 0 { - preWorkflowHooks = append(preWorkflowHooks, repo.PreWorkflowHooks...) - } - } + preWorkflowHooks := w.preWorkflowHooks(ctx) // short circuit any other calls if there are no pre-hooks configured if len(preWorkflowHooks) == 0 { @@ -97,6 +96,20 @@ func (w *DefaultPreWorkflowHooksCommandRunner) RunPreHooks(ctx *command.Context, return nil } +func (w *DefaultPreWorkflowHooksCommandRunner) HasPreWorkflowHooks(ctx *command.Context) bool { + return len(w.preWorkflowHooks(ctx)) > 0 +} + +func (w *DefaultPreWorkflowHooksCommandRunner) preWorkflowHooks(ctx *command.Context) []*valid.WorkflowHook { + preWorkflowHooks := make([]*valid.WorkflowHook, 0) + for _, repo := range w.GlobalCfg.Repos { + if repo.IDMatches(ctx.Pull.BaseRepo.ID()) && len(repo.PreWorkflowHooks) > 0 { + preWorkflowHooks = append(preWorkflowHooks, repo.PreWorkflowHooks...) + } + } + return preWorkflowHooks +} + func (w *DefaultPreWorkflowHooksCommandRunner) runHooks( ctx models.WorkflowHookCommandContext, preWorkflowHooks []*valid.WorkflowHook, From d82d2f0a6719ba168a7d8cbe55d3eca7f14e7f76 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 15:33:43 -0400 Subject: [PATCH 20/23] fix: fail open on stale merge checkout config Do not use pre-clone local repo config for targeted ignore decisions when checkout-strategy=merge is enabled, since that checkout may not have been refreshed to the current pull ref. Also allow non-fatal pre-workflow hook errors to recheck generated local config and continue through the existing non-blocking hook path. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/command_runner.go | 2 +- server/events/command_runner_test.go | 34 +++++++++++++++++++ server/events/project_command_builder.go | 3 -- server/events/project_command_builder_test.go | 16 +++------ 4 files changed, 39 insertions(+), 16 deletions(-) diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 723681c633..3249f06e28 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -523,7 +523,7 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead preWorkflowHooksErr := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmd) if targetInitiallyIgnored { ctx.CommandSkipped = false - if !preWorkflowHooksMayUpdateRepo || preWorkflowHooksErr != nil { + if !preWorkflowHooksMayUpdateRepo { return } ctx.PreferLocalRepoCfgForTargetedIgnore = true diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 90cb88cf1c..6092afea99 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -753,6 +753,40 @@ func TestRunCommentCommand_IgnoredTargetedDirPreHooksCanGenerateExplicitConfig(t projectCommandRunner.VerifyWasCalledOnce().Plan(projectCtx) } +func TestRunCommentCommand_IgnoredTargetedDirNonFatalPreHookErrorCanGenerateExplicitConfig(t *testing.T) { + setup(t) + configuredPreHooks := &configuredPreWorkflowHooksCommandRunner{hasHooks: true, err: errors.New("hook error")} + preWorkflowHooksCommandRunner = configuredPreHooks + ch.PreWorkflowHooksCommandRunner = configuredPreHooks + pull := &github.PullRequest{State: github.Ptr("open")} + modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} + cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "ignored"} + projectCtx := command.ProjectContext{ + CommandName: command.Plan, + RepoRelDir: "ignored", + Workspace: events.DefaultWorkspace, + BaseRepo: testdata.GithubRepo, + Pull: modelPull, + } + ignoreChecks := 0 + + When(githubGetter.GetPullRequest(Any[logging.SimpleLogging](), Eq(testdata.GithubRepo), Eq(testdata.Pull.Num))).ThenReturn(pull, nil) + When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) + When(projectCommandBuilder.ShouldIgnoreTargetedDir(Any[*command.Context](), Any[*events.CommentCommand]())).Then(func(args []Param) ReturnValues { + ignoreChecks++ + return ReturnValues{ignoreChecks == 1} + }) + When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Eq(cmd))).ThenReturn([]command.ProjectContext{projectCtx}, nil) + When(projectCommandRunner.Plan(projectCtx)).ThenReturn(command.ProjectCommandOutput{PlanSuccess: &models.PlanSuccess{}}) + + ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, cmd) + + Equals(t, 1, configuredPreHooks.calls) + Equals(t, 2, ignoreChecks) + projectCommandBuilder.VerifyWasCalledOnce().BuildPlanCommands(Any[*command.Context](), Eq(cmd)) + projectCommandRunner.VerifyWasCalledOnce().Plan(projectCtx) +} + func TestRunCommentCommandApprovePolicy_NoProjects_SilenceEnabled(t *testing.T) { t.Log("if an approve_policy command is run on a pull request and SilenceNoProjects is enabled and we are silencing all comments if the modified files don't have a matching project") vcsClient := setup(t) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index afc3c5c586..126c3d16c6 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -480,9 +480,6 @@ func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Con } if p.needsLocalRepoCfgForTargetedIgnore(ctx) { - if repoCfg, ok := p.repoCfgFromWorkingDir(ctx); ok { - return repoCfg, true - } ctx.Log.Debug("not checking remote repo config for targeted ignore because merge checkout needs the merged local config") return valid.RepoCfg{}, false } diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 0432bf258d..5767eca7b8 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -1156,28 +1156,20 @@ func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirAllowsAuthoritative workingDir.VerifyWasCalled(Never()).GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]()) } -func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirMergeCheckoutUsesLocalConfig(t *testing.T) { +func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirMergeCheckoutWithLocalConfigFailsOpen(t *testing.T) { RegisterMockTestingT(t) - remoteAtlantisYAML := "version: 3\n" + + staleLocalAtlantisYAML := "version: 3\n" + "autodiscover:\n" + " mode: enabled\n" + " ignore_paths:\n" + " - \"environments/prod/**\"\n" - localAtlantisYAML := "version: 3\n" + - "autodiscover:\n" + - " mode: enabled\n" + - " ignore_paths:\n" + - " - \"environments/prod/**\"\n" + - "projects:\n" + - "- dir: environments/prod\n" logger := logging.NewNoopLogger(t) scope := metricstest.NewLoggingScope(t, logger, "atlantis") globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{AllowAllRepoSettings: true}) terraformClient := tfclientmocks.NewMockClient() vcsClient := vcsmocks.NewMockClient() - When(vcsClient.GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Eq("feature"), Eq(valid.DefaultAtlantisFile))).ThenReturn(true, []byte(remoteAtlantisYAML), nil) dataDir := t.TempDir() workingDir := &events.FileWorkspace{ @@ -1193,7 +1185,7 @@ func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirMergeCheckoutUsesLo } repoDir := filepath.Join(dataDir, "repos", baseRepo.FullName, "1", events.DefaultWorkspace) Ok(t, os.MkdirAll(repoDir, 0700)) - Ok(t, os.WriteFile(filepath.Join(repoDir, valid.DefaultAtlantisFile), []byte(localAtlantisYAML), 0600)) + Ok(t, os.WriteFile(filepath.Join(repoDir, valid.DefaultAtlantisFile), []byte(staleLocalAtlantisYAML), 0600)) userConfig := defaultUserConfig builder := events.NewProjectCommandBuilder( @@ -1228,7 +1220,7 @@ func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirMergeCheckoutUsesLo } cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "environments/prod", Workspace: events.DefaultWorkspace} - Assert(t, !builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected merged local explicit project to prevent ignored-target skip") + Assert(t, !builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected merge checkout to avoid pre-clone skip from stale local config") vcsClient.VerifyWasCalled(Never()).GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Any[string](), Any[string]()) } From 7d8d94b8a3eaa45ff23dcdc0226a9603ecbf93a9 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 16:01:13 -0400 Subject: [PATCH 21/23] fix: preserve global ignores without file downloads When a VCS provider cannot download single files, keep server-side autodiscover.ignore_paths authoritative for targeted ignore routing instead of falling through to stale working-dir errors. Supported providers still fail open on fetch or parse errors so repo config uncertainty does not silently skip valid targets. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/project_command_builder.go | 13 +++- server/events/project_command_builder_test.go | 61 +++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 126c3d16c6..3f8ccfaa88 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -465,14 +465,14 @@ func (p *DefaultProjectCommandBuilder) ShouldIgnoreTargetedDir(ctx *command.Cont if cmd.ProjectName != "" || cmd.RepoRelDir == "" || valid.ContainsDirGlobPattern(cmd.RepoRelDir) { return false } - repoCfg, ok := p.repoCfgForTargetedIgnore(ctx) + repoCfg, ok := p.repoCfgForTargetedIgnore(ctx, cmd.RepoRelDir) if !ok { return false } return p.shouldIgnoreTargetedDirFromCfg(ctx, repoCfg, cmd.RepoRelDir) } -func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Context) (valid.RepoCfg, bool) { +func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Context, repoRelDir string) (valid.RepoCfg, bool) { if ctx.PreferLocalRepoCfgForTargetedIgnore { if repoCfg, ok := p.repoCfgFromWorkingDir(ctx); ok { return repoCfg, true @@ -489,6 +489,9 @@ func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Con hasRepoCfg, repoCfgData, err := p.VCSClient.GetFileContent(ctx.Log, ctx.HeadRepo, targetedIgnoreConfigRef(ctx.Pull), repoCfgFile) if err != nil { ctx.Log.Debug("unable to fetch %s while checking autodiscover.ignore_paths: %s", repoCfgFile, err) + if !p.VCSClient.SupportsSingleFileDownload(ctx.HeadRepo) && p.shouldIgnoreTargetedDirFromGlobalCfg(ctx, repoRelDir) { + return valid.RepoCfg{}, true + } return valid.RepoCfg{}, false } else if hasRepoCfg { repoCfg, err := p.ParserValidator.ParseRepoCfgData(repoCfgData, p.GlobalCfg, ctx.Pull.BaseRepo.ID(), ctx.Pull.BaseBranch) @@ -505,6 +508,12 @@ func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Con return valid.RepoCfg{}, false } +func (p *DefaultProjectCommandBuilder) shouldIgnoreTargetedDirFromGlobalCfg(ctx *command.Context, repoRelDir string) bool { + repoCfg := valid.RepoCfg{} + return p.autoDiscoverModeEnabled(ctx, repoCfg) && + p.isAutoDiscoverPathIgnored(ctx, repoCfg, filepath.Clean(repoRelDir)) +} + func targetedIgnoreConfigRef(pull models.PullRequest) string { if pull.HeadCommit != "" { return pull.HeadCommit diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 5767eca7b8..b9e8e722d3 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -1156,6 +1156,67 @@ func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirAllowsAuthoritative workingDir.VerifyWasCalled(Never()).GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]()) } +func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirUsesGlobalIgnoreWhenFileDownloadUnsupported(t *testing.T) { + RegisterMockTestingT(t) + + workingDir := mocks.NewMockWorkingDir() + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Eq(events.DefaultWorkspace))).ThenReturn("", os.ErrNotExist) + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Eq("abc123"), Eq(valid.DefaultAtlantisFile))).ThenReturn(false, []byte{}, errors.New("not implemented")) + When(vcsClient.SupportsSingleFileDownload(Any[models.Repo]())).ThenReturn(false) + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{AllowAllRepoSettings: true}) + globalCfg.Repos[0].AutoDiscover = &valid.AutoDiscover{ + Mode: valid.AutoDiscoverEnabledMode, + IgnorePaths: []string{"environments/prod/**"}, + } + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + baseRepo := models.Repo{Owner: "owner", Name: "repo", FullName: "owner/repo", VCSHost: models.VCSHost{Type: models.AzureDevops}} + ctx := &command.Context{ + Log: logger, + Scope: scope, + HeadRepo: baseRepo, + Pull: models.PullRequest{ + Num: 1, + BaseBranch: "main", + HeadBranch: "feature", + HeadCommit: "abc123", + BaseRepo: baseRepo, + }, + } + cmd := &events.CommentCommand{Name: command.Apply, RepoRelDir: "environments/prod", Workspace: events.DefaultWorkspace} + + Assert(t, builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected global ignored-target skip when VCS file download is unsupported") + workingDir.VerifyWasCalled(Never()).GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]()) +} + func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirMergeCheckoutWithLocalConfigFailsOpen(t *testing.T) { RegisterMockTestingT(t) From 1bcaadc92e27abe939ecd8a6b7679cf616d3513b Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 16:22:08 -0400 Subject: [PATCH 22/23] fix: respect glob project dirs for targeted ignores Treat glob-configured project dirs as explicit projects during targeted-ignore prechecks so paths like modules/foo are not skipped before clone-time project glob expansion can run. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/project_command_builder.go | 14 ++++- server/events/project_command_builder_test.go | 63 +++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 3f8ccfaa88..b9f49cf2cf 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -12,6 +12,7 @@ import ( "sort" "strings" + "github.com/bmatcuk/doublestar/v4" tally "github.com/uber-go/tally/v4" "github.com/runatlantis/atlantis/server/core/config/valid" @@ -550,13 +551,24 @@ func (p *DefaultProjectCommandBuilder) shouldIgnoreTargetedDirFromCfg(ctx *comma } cleanDir := filepath.Clean(repoRelDir) for _, proj := range repoCfg.Projects { - if filepath.Clean(proj.Dir) == cleanDir { + if projectDirMatchesTargetedDir(proj.Dir, cleanDir) { return false } } return p.isAutoDiscoverPathIgnored(ctx, repoCfg, cleanDir) } +func projectDirMatchesTargetedDir(projectDir string, cleanTargetDir string) bool { + cleanProjectDir := filepath.Clean(projectDir) + if cleanProjectDir == cleanTargetDir { + return true + } + if !valid.ContainsDirGlobPattern(cleanProjectDir) { + return false + } + return doublestar.MatchUnvalidated(filepath.ToSlash(cleanProjectDir), filepath.ToSlash(cleanTargetDir)) +} + // isAutoDiscoverPathIgnored determines whether this particular path is ignored for the purposes of auto discovery. // Global config ignore_paths takes precedence when explicitly set; otherwise falls through to repo config. func (p *DefaultProjectCommandBuilder) isAutoDiscoverPathIgnored(ctx *command.Context, repoCfg valid.RepoCfg, path string) bool { diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index b9e8e722d3..a040d63fe7 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -1026,6 +1026,69 @@ func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirUsesHeadCommitForRe vcsClient.VerifyWasCalled(Never()).GetFileContent(Any[logging.SimpleLogging](), Eq(baseRepo), Eq("feature"), Eq(valid.DefaultAtlantisFile)) } +func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirRespectsGlobProjectDirs(t *testing.T) { + RegisterMockTestingT(t) + + atlantisYAML := "version: 3\n" + + "autodiscover:\n" + + " mode: enabled\n" + + " ignore_paths:\n" + + " - \"modules/**\"\n" + + "projects:\n" + + "- dir: \"modules/*\"\n" + + workingDir := mocks.NewMockWorkingDir() + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Eq("abc123"), Eq(valid.DefaultAtlantisFile))).ThenReturn(true, []byte(atlantisYAML), nil) + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{AllowAllRepoSettings: true}) + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + baseRepo := models.Repo{Owner: "owner", Name: "repo", FullName: "owner/repo", VCSHost: models.VCSHost{Type: models.Github}} + ctx := &command.Context{ + Log: logger, + Scope: scope, + HeadRepo: baseRepo, + Pull: models.PullRequest{ + Num: 1, + BaseBranch: "main", + HeadBranch: "feature", + HeadCommit: "abc123", + BaseRepo: baseRepo, + }, + } + cmd := &events.CommentCommand{Name: command.Apply, RepoRelDir: "modules/foo", Workspace: events.DefaultWorkspace} + + Assert(t, !builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected glob-configured project dir to prevent ignored-target skip") + vcsClient.VerifyWasCalledOnce().GetFileContent(Any[logging.SimpleLogging](), Eq(baseRepo), Eq("abc123"), Eq(valid.DefaultAtlantisFile)) +} + func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirFailsOpenWhenRemoteConfigUnknown(t *testing.T) { RegisterMockTestingT(t) From 19e8aee67607600351f027da8e29ce09f07d4723 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Fri, 26 Jun 2026 16:55:31 -0400 Subject: [PATCH 23/23] fix: preserve explicit projects without file downloads Avoid treating global ignore_paths as authoritative when single-file config downloads are unsupported. Plan commands now fail open so the cloned config can be parsed, and non-plan commands inspect an existing local config before using the no-working-dir ignored-target no-op. Assisted-by: OpenAI Signed-off-by: Rui Chen --- server/events/project_command_builder.go | 25 +++- server/events/project_command_builder_test.go | 134 ++++++++++++++++++ 2 files changed, 152 insertions(+), 7 deletions(-) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index b9f49cf2cf..40470aadcb 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -466,16 +466,17 @@ func (p *DefaultProjectCommandBuilder) ShouldIgnoreTargetedDir(ctx *command.Cont if cmd.ProjectName != "" || cmd.RepoRelDir == "" || valid.ContainsDirGlobPattern(cmd.RepoRelDir) { return false } - repoCfg, ok := p.repoCfgForTargetedIgnore(ctx, cmd.RepoRelDir) + repoCfg, ok := p.repoCfgForTargetedIgnore(ctx, cmd) if !ok { return false } return p.shouldIgnoreTargetedDirFromCfg(ctx, repoCfg, cmd.RepoRelDir) } -func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Context, repoRelDir string) (valid.RepoCfg, bool) { +func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Context, cmd *CommentCommand) (valid.RepoCfg, bool) { + repoRelDir := cmd.RepoRelDir if ctx.PreferLocalRepoCfgForTargetedIgnore { - if repoCfg, ok := p.repoCfgFromWorkingDir(ctx); ok { + if repoCfg, ok, _ := p.repoCfgFromWorkingDir(ctx); ok { return repoCfg, true } } @@ -491,6 +492,16 @@ func (p *DefaultProjectCommandBuilder) repoCfgForTargetedIgnore(ctx *command.Con if err != nil { ctx.Log.Debug("unable to fetch %s while checking autodiscover.ignore_paths: %s", repoCfgFile, err) if !p.VCSClient.SupportsSingleFileDownload(ctx.HeadRepo) && p.shouldIgnoreTargetedDirFromGlobalCfg(ctx, repoRelDir) { + // Do not let global ignore_paths hide an explicit project that only the + // cloned repo config can prove. + if cmd.Name == command.Plan { + return valid.RepoCfg{}, false + } + if repoCfg, ok, missing := p.repoCfgFromWorkingDir(ctx); ok { + return repoCfg, true + } else if !missing { + return valid.RepoCfg{}, false + } return valid.RepoCfg{}, true } return valid.RepoCfg{}, false @@ -522,16 +533,16 @@ func targetedIgnoreConfigRef(pull models.PullRequest) string { return pull.HeadBranch } -func (p *DefaultProjectCommandBuilder) repoCfgFromWorkingDir(ctx *command.Context) (valid.RepoCfg, bool) { +func (p *DefaultProjectCommandBuilder) repoCfgFromWorkingDir(ctx *command.Context) (valid.RepoCfg, bool, bool) { repoDir, err := p.WorkingDir.GetWorkingDir(ctx.Pull.BaseRepo, ctx.Pull, DefaultWorkspace) if err != nil { - return valid.RepoCfg{}, false + return valid.RepoCfg{}, false, errors.Is(err, os.ErrNotExist) } repoCfg, _, err := p.parseRepoCfg(ctx, repoDir) if err != nil { - return valid.RepoCfg{}, false + return valid.RepoCfg{}, false, false } - return repoCfg, true + return repoCfg, true, false } func (p *DefaultProjectCommandBuilder) needsLocalRepoCfgForTargetedIgnore(ctx *command.Context) bool { diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index a040d63fe7..c4494246a4 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -1277,6 +1277,140 @@ func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirUsesGlobalIgnoreWhe cmd := &events.CommentCommand{Name: command.Apply, RepoRelDir: "environments/prod", Workspace: events.DefaultWorkspace} Assert(t, builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected global ignored-target skip when VCS file download is unsupported") + workingDir.VerifyWasCalledOnce().GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Eq(events.DefaultWorkspace)) +} + +func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirFileDownloadUnsupportedPreservesExplicitLocalProject(t *testing.T) { + RegisterMockTestingT(t) + + atlantisYAML := "version: 3\n" + + "projects:\n" + + "- name: prod-project\n" + + " dir: environments/prod\n" + tmpDir := DirStructure(t, map[string]any{ + "environments": map[string]any{ + "prod": map[string]any{ + "main.tf": nil, + }, + }, + }) + Ok(t, os.WriteFile(filepath.Join(tmpDir, valid.DefaultAtlantisFile), []byte(atlantisYAML), 0600)) + + workingDir := mocks.NewMockWorkingDir() + When(workingDir.GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Eq(events.DefaultWorkspace))).ThenReturn(tmpDir, nil) + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Eq("abc123"), Eq(valid.DefaultAtlantisFile))).ThenReturn(false, []byte{}, errors.New("not implemented")) + When(vcsClient.SupportsSingleFileDownload(Any[models.Repo]())).ThenReturn(false) + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{AllowAllRepoSettings: true}) + globalCfg.Repos[0].AutoDiscover = &valid.AutoDiscover{ + Mode: valid.AutoDiscoverEnabledMode, + IgnorePaths: []string{"environments/prod/**"}, + } + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + baseRepo := models.Repo{Owner: "owner", Name: "repo", FullName: "owner/repo", VCSHost: models.VCSHost{Type: models.AzureDevops}} + ctx := &command.Context{ + Log: logger, + Scope: scope, + HeadRepo: baseRepo, + Pull: models.PullRequest{ + Num: 1, + BaseBranch: "main", + HeadBranch: "feature", + HeadCommit: "abc123", + BaseRepo: baseRepo, + }, + } + cmd := &events.CommentCommand{Name: command.Apply, RepoRelDir: "environments/prod", Workspace: events.DefaultWorkspace} + + Assert(t, !builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected local explicit project to prevent ignored-target skip when file download is unsupported") + workingDir.VerifyWasCalledOnce().GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Eq(events.DefaultWorkspace)) +} + +func TestDefaultProjectCommandBuilder_ShouldIgnoreTargetedDirFileDownloadUnsupportedFailsOpenForPlan(t *testing.T) { + RegisterMockTestingT(t) + + workingDir := mocks.NewMockWorkingDir() + vcsClient := vcsmocks.NewMockClient() + When(vcsClient.GetFileContent(Any[logging.SimpleLogging](), Any[models.Repo](), Eq("abc123"), Eq(valid.DefaultAtlantisFile))).ThenReturn(false, []byte{}, errors.New("not implemented")) + When(vcsClient.SupportsSingleFileDownload(Any[models.Repo]())).ThenReturn(false) + + logger := logging.NewNoopLogger(t) + scope := metricstest.NewLoggingScope(t, logger, "atlantis") + globalCfg := valid.NewGlobalCfgFromArgs(valid.GlobalCfgArgs{AllowAllRepoSettings: true}) + globalCfg.Repos[0].AutoDiscover = &valid.AutoDiscover{ + Mode: valid.AutoDiscoverEnabledMode, + IgnorePaths: []string{"environments/prod/**"}, + } + terraformClient := tfclientmocks.NewMockClient() + userConfig := defaultUserConfig + builder := events.NewProjectCommandBuilder( + false, + &config.ParserValidator{}, + &events.DefaultProjectFinder{}, + vcsClient, + workingDir, + events.NewDefaultWorkingDirLocker(), + globalCfg, + &events.DefaultPendingPlanFinder{}, + &events.CommentParser{ExecutableName: "atlantis"}, + userConfig.SkipCloneNoChanges, + userConfig.EnableRegExpCmd, + userConfig.EnableAutoMerge, + userConfig.EnableParallelPlan, + userConfig.EnableParallelApply, + userConfig.AutoDetectModuleFiles, + userConfig.AutoplanFileList, + userConfig.RestrictFileList, + userConfig.SilenceNoProjects, + userConfig.IncludeGitUntrackedFiles, + userConfig.AutoDiscoverMode, + scope, + terraformClient, + ) + baseRepo := models.Repo{Owner: "owner", Name: "repo", FullName: "owner/repo", VCSHost: models.VCSHost{Type: models.AzureDevops}} + ctx := &command.Context{ + Log: logger, + Scope: scope, + HeadRepo: baseRepo, + Pull: models.PullRequest{ + Num: 1, + BaseBranch: "main", + HeadBranch: "feature", + HeadCommit: "abc123", + BaseRepo: baseRepo, + }, + } + cmd := &events.CommentCommand{Name: command.Plan, RepoRelDir: "environments/prod", Workspace: events.DefaultWorkspace} + + Assert(t, !builder.ShouldIgnoreTargetedDir(ctx, cmd), "expected plan to avoid pre-clone skip when file download is unsupported") workingDir.VerifyWasCalled(Never()).GetWorkingDir(Any[models.Repo](), Any[models.PullRequest](), Any[string]()) }