From e1317dadde0c2b7f2554f63407193594a2c4a2d6 Mon Sep 17 00:00:00 2001 From: ekapratama93 Date: Thu, 12 Dec 2024 20:59:39 +0700 Subject: [PATCH 1/4] fix: status always success when using custom policy Signed-off-by: ekapratama93 --- server/core/runtime/run_step_runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index 76629ba460..85b80ccbba 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -88,9 +88,9 @@ func (r *RunStepRunner) Run( err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, output) if !ctx.CustomPolicyCheck { ctx.Log.Debug("error: %s", err) - return "", err } ctx.Log.Debug("Treating custom policy tool error exit code as a policy failure. Error output: %s", err) + return "", err } switch postProcessOutput { From 443069ee67a989c9fb46ec645d16daeac2e9ecec Mon Sep 17 00:00:00 2001 From: bakuljajan Date: Fri, 13 Dec 2024 11:02:13 +0700 Subject: [PATCH 2/4] chore: enable custom policy on test Signed-off-by: bakuljajan --- server/core/runtime/run_step_runner_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/server/core/runtime/run_step_runner_test.go b/server/core/runtime/run_step_runner_test.go index 4672fa2bb0..e2383461df 100644 --- a/server/core/runtime/run_step_runner_test.go +++ b/server/core/runtime/run_step_runner_test.go @@ -144,6 +144,7 @@ func TestRunStepRunner_Run(t *testing.T) { TerraformVersion: projVersion, ProjectName: c.ProjectName, EscapedCommentArgs: []string{"-target=resource1", "-target=resource2"}, + CustomPolicyCheck: true, } out, err := r.Run(ctx, nil, c.Command, tmpDir, map[string]string{"test": "var"}, true, valid.PostProcessRunOutputShow) if c.ExpErr != "" { From 8c24141451d252682b654a80946c3366a2f97968 Mon Sep 17 00:00:00 2001 From: bakuljajan Date: Sat, 25 Jan 2025 22:48:04 +0700 Subject: [PATCH 3/4] fix: test both case & move debug log on else block Signed-off-by: bakuljajan --- server/core/runtime/run_step_runner.go | 7 +- server/core/runtime/run_step_runner_test.go | 146 ++++++++++---------- 2 files changed, 77 insertions(+), 76 deletions(-) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index 5759d1930e..63ae3228ba 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -95,9 +95,10 @@ func (r *RunStepRunner) Run( err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, output) if !ctx.CustomPolicyCheck { ctx.Log.Debug("error: %s", err) - } - ctx.Log.Debug("Treating custom policy tool error exit code as a policy failure. Error output: %s", err) - return "", err + } else { + ctx.Log.Debug("Treating custom policy tool error exit code as a policy failure. Error output: %s", err) + } + return "", err } switch postProcessOutput { diff --git a/server/core/runtime/run_step_runner_test.go b/server/core/runtime/run_step_runner_test.go index b22ad8092c..51289f8c49 100644 --- a/server/core/runtime/run_step_runner_test.go +++ b/server/core/runtime/run_step_runner_test.go @@ -100,90 +100,90 @@ func TestRunStepRunner_Run(t *testing.T) { ExpOut: "args=-target=resource1,-target=resource2\n", }, } + for _, customPolicyCheck := range []bool{false, true} { + for _, c := range cases { + var projVersion *version.Version + var err error - for _, c := range cases { + projVersion, err = version.NewVersion("v0.11.0") - var projVersion *version.Version - var err error - - projVersion, err = version.NewVersion("v0.11.0") + if c.Version != "" { + projVersion, err = version.NewVersion(c.Version) + Ok(t, err) + } - if c.Version != "" { - projVersion, err = version.NewVersion(c.Version) Ok(t, err) - } - Ok(t, err) - - projTFDistribution := "terraform" - if c.Distribution != "" { - projTFDistribution = c.Distribution - } + projTFDistribution := "terraform" + if c.Distribution != "" { + projTFDistribution = c.Distribution + } - defaultVersion, _ := version.NewVersion("0.8") + defaultVersion, _ := version.NewVersion("0.8") - RegisterMockTestingT(t) - terraform := tfclientmocks.NewMockClient() - defaultDistribution := tf.NewDistributionTerraformWithDownloader(mocks.NewMockDownloader()) - When(terraform.EnsureVersion(Any[logging.SimpleLogging](), Any[tf.Distribution](), Any[*version.Version]())). - ThenReturn(nil) + RegisterMockTestingT(t) + terraform := tfclientmocks.NewMockClient() + defaultDistribution := tf.NewDistributionTerraformWithDownloader(mocks.NewMockDownloader()) + When(terraform.EnsureVersion(Any[logging.SimpleLogging](), Any[tf.Distribution](), Any[*version.Version]())). + ThenReturn(nil) - logger := logging.NewNoopLogger(t) - projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler() - tmpDir := t.TempDir() + logger := logging.NewNoopLogger(t) + projectCmdOutputHandler := jobmocks.NewMockProjectCommandOutputHandler() + tmpDir := t.TempDir() - r := runtime.RunStepRunner{ - TerraformExecutor: terraform, - DefaultTFDistribution: defaultDistribution, - DefaultTFVersion: defaultVersion, - TerraformBinDir: "/bin/dir", - ProjectCmdOutputHandler: projectCmdOutputHandler, - } - t.Run(c.Command, func(t *testing.T) { - ctx := command.ProjectContext{ - BaseRepo: models.Repo{ - Name: "basename", - Owner: "baseowner", - }, - HeadRepo: models.Repo{ - Name: "headname", - Owner: "headowner", - }, - Pull: models.PullRequest{ - Num: 2, - URL: "https://github.com/runatlantis/atlantis/pull/2", - HeadBranch: "add-feat", - HeadCommit: "12345abcdef", - BaseBranch: "main", - Author: "acme", - }, - User: models.User{ - Username: "acme-user", - }, - Log: logger, - Workspace: "myworkspace", - RepoRelDir: "mydir", - TerraformDistribution: &projTFDistribution, - TerraformVersion: projVersion, - ProjectName: c.ProjectName, - EscapedCommentArgs: []string{"-target=resource1", "-target=resource2"}, - CustomPolicyCheck: true, - } - out, err := r.Run(ctx, nil, c.Command, tmpDir, map[string]string{"test": "var"}, true, valid.PostProcessRunOutputShow) - if c.ExpErr != "" { - ErrContains(t, c.ExpErr, err) - return + r := runtime.RunStepRunner{ + TerraformExecutor: terraform, + DefaultTFDistribution: defaultDistribution, + DefaultTFVersion: defaultVersion, + TerraformBinDir: "/bin/dir", + ProjectCmdOutputHandler: projectCmdOutputHandler, } - Ok(t, err) - // Replace $DIR in the exp with the actual temp dir. We do this - // here because when constructing the cases we don't yet know the - // temp dir. - expOut := strings.Replace(c.ExpOut, "$DIR", tmpDir, -1) - Equals(t, expOut, out) + t.Run(fmt.Sprintf("%s_CustomPolicyCheck=%v", c.Command, customPolicyCheck), func(t *testing.T) { + ctx := command.ProjectContext{ + BaseRepo: models.Repo{ + Name: "basename", + Owner: "baseowner", + }, + HeadRepo: models.Repo{ + Name: "headname", + Owner: "headowner", + }, + Pull: models.PullRequest{ + Num: 2, + URL: "https://github.com/runatlantis/atlantis/pull/2", + HeadBranch: "add-feat", + HeadCommit: "12345abcdef", + BaseBranch: "main", + Author: "acme", + }, + User: models.User{ + Username: "acme-user", + }, + Log: logger, + Workspace: "myworkspace", + RepoRelDir: "mydir", + TerraformDistribution: &projTFDistribution, + TerraformVersion: projVersion, + ProjectName: c.ProjectName, + EscapedCommentArgs: []string{"-target=resource1", "-target=resource2"}, + CustomPolicyCheck: customPolicyCheck, + } + out, err := r.Run(ctx, nil, c.Command, tmpDir, map[string]string{"test": "var"}, true, valid.PostProcessRunOutputShow) + if c.ExpErr != "" { + ErrContains(t, c.ExpErr, err) + return + } + Ok(t, err) + // Replace $DIR in the exp with the actual temp dir. We do this + // here because when constructing the cases we don't yet know the + // temp dir. + expOut := strings.Replace(c.ExpOut, "$DIR", tmpDir, -1) + Equals(t, expOut, out) - terraform.VerifyWasCalledOnce().EnsureVersion(Eq(logger), NotEq(defaultDistribution), Eq(projVersion)) - terraform.VerifyWasCalled(Never()).EnsureVersion(Eq(logger), Eq(defaultDistribution), Eq(defaultVersion)) + terraform.VerifyWasCalledOnce().EnsureVersion(Eq(logger), NotEq(defaultDistribution), Eq(projVersion)) + terraform.VerifyWasCalled(Never()).EnsureVersion(Eq(logger), Eq(defaultDistribution), Eq(defaultVersion)) - }) + }) + } } } From bd8259cc54b7105f50d47a1efdaeb33f8b3b3947 Mon Sep 17 00:00:00 2001 From: bakuljajan Date: Sat, 25 Jan 2025 22:53:54 +0700 Subject: [PATCH 4/4] fix: indentation Signed-off-by: bakuljajan --- server/core/runtime/run_step_runner.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/core/runtime/run_step_runner.go b/server/core/runtime/run_step_runner.go index 63ae3228ba..5fa32896d7 100644 --- a/server/core/runtime/run_step_runner.go +++ b/server/core/runtime/run_step_runner.go @@ -95,10 +95,10 @@ func (r *RunStepRunner) Run( err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, output) if !ctx.CustomPolicyCheck { ctx.Log.Debug("error: %s", err) - } else { - ctx.Log.Debug("Treating custom policy tool error exit code as a policy failure. Error output: %s", err) - } - return "", err + } else { + ctx.Log.Debug("Treating custom policy tool error exit code as a policy failure. Error output: %s", err) + } + return "", err } switch postProcessOutput {