Skip to content

Commit f9ec8a7

Browse files
fix: disable auto-merge instead of failing CI for Renovate PRs with plan changes (#3575)
* fix: disable auto-merge instead of failing CI for Renovate PRs with plan changes Replace validateRenovateChange() (which fails CI) with disableAutoMergeForRenovateChange() (which disables auto-merge on the PR via GraphQL). CI no longer fails, and the renovate-change label is deprecated. Add accept_change_by_renovate setting to tfaction.yaml per-target config to opt out of auto-merge disabling. Ref: #3571 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: use .default(false) for accept_change_by_renovate in Zod schema Make the default explicit at the schema level, removing the need for manual ?? false fallback in get-target-config. Widen getJobConfig's parameter type to accept both TargetConfig and TargetGroup. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: fix test --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent a66a231 commit f9ec8a7

File tree

11 files changed

+152
-28
lines changed

11 files changed

+152
-28
lines changed

install/github-comment.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,12 @@ post:
9292
9393
renovate-plan-change:
9494
template: |
95-
## :x: {{if .Vars.tfaction_target}}{{.Vars.tfaction_target}}: {{end}}Renovate's PR must be `No change`
95+
## :warning: {{if .Vars.tfaction_target}}{{.Vars.tfaction_target}}: {{end}}Auto-merge was disabled because `terraform plan` has changes
9696
9797
{{template "link" .}}
9898
99-
In the pull request created by Renovate, the result of `terraform plan` must be `No change` to enable automerge safely.
100-
If you allow changes, please set the pull request label `renovate-change` and rerun CI.
99+
In the pull request created by Renovate, `terraform plan` detected changes. Auto-merge has been disabled for safety.
100+
Please review the plan and merge the pull request manually if the changes are expected.
101101
102102
exec:
103103
default:

renovate.json5

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,4 @@
66
"github>aquaproj/aqua-renovate-config#2.10.0",
77
"github>aquaproj/aqua-renovate-config:file#2.10.0(aqua/.*\\.yaml)",
88
],
9-
addLabels: ["renovate-change"],
109
}

src/actions/get-target-config/index.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ test("default", async () => {
2222
["enable_trivy", true],
2323
["enable_terraform_docs", false],
2424
["destroy", false],
25+
["accept_change_by_renovate", false],
2526
["tflint_fix", false],
2627
["terraform_command", "terraform"],
2728
]),
@@ -71,6 +72,7 @@ test("config", async () => {
7172
["terraform_command", "tofu"],
7273
["aws_role_session_name", "test"],
7374
["destroy", false],
75+
["accept_change_by_renovate", false],
7476
["enable_terraform_docs", false],
7577
]),
7678
};
@@ -170,6 +172,7 @@ test("tfmigrate plan", async () => {
170172
["enable_trivy", true],
171173
["enable_terraform_docs", false],
172174
["destroy", false],
175+
["accept_change_by_renovate", false],
173176
["tflint_fix", false],
174177
["terraform_command", "terraform"],
175178
["aws_assume_role_arn", "arn:aws:iam::123:role/tfmigrate-plan"],
@@ -222,6 +225,7 @@ test("tfmigrate apply", async () => {
222225
["enable_trivy", true],
223226
["enable_terraform_docs", false],
224227
["destroy", false],
228+
["accept_change_by_renovate", false],
225229
["tflint_fix", false],
226230
["terraform_command", "terraform"],
227231
["aws_assume_role_arn", "arn:aws:iam::123:role/tfmigrate-apply"],
@@ -274,6 +278,7 @@ test("terraform apply", async () => {
274278
["enable_trivy", true],
275279
["enable_terraform_docs", false],
276280
["destroy", false],
281+
["accept_change_by_renovate", false],
277282
["tflint_fix", false],
278283
["terraform_command", "terraform"],
279284
["aws_assume_role_arn", "arn:aws:iam::123:role/terraform-apply"],
@@ -325,6 +330,7 @@ test("explicit aws_role_session_name overrides auto-generation", async () => {
325330
["enable_trivy", true],
326331
["enable_terraform_docs", false],
327332
["destroy", false],
333+
["accept_change_by_renovate", false],
328334
["tflint_fix", false],
329335
["terraform_command", "terraform"],
330336
["aws_role_session_name", "custom-session-name"],
@@ -374,6 +380,7 @@ test("terraform_docs enabled in root config", async () => {
374380
["enable_trivy", true],
375381
["enable_terraform_docs", true],
376382
["destroy", false],
383+
["accept_change_by_renovate", false],
377384
["tflint_fix", false],
378385
["terraform_command", "terraform"],
379386
["aws_role_session_name", "tfaction-plan-tests_aws_foo_dev-" + runID],
@@ -427,6 +434,7 @@ test("environment variables from targetGroup", async () => {
427434
["enable_trivy", true],
428435
["enable_terraform_docs", false],
429436
["destroy", false],
437+
["accept_change_by_renovate", false],
430438
["tflint_fix", false],
431439
["terraform_command", "terraform"],
432440
["aws_role_session_name", "tfaction-plan-tests_aws_foo_dev-" + runID],
@@ -480,6 +488,7 @@ test("environment variables from root config", async () => {
480488
["enable_trivy", true],
481489
["enable_terraform_docs", false],
482490
["destroy", false],
491+
["accept_change_by_renovate", false],
483492
["tflint_fix", false],
484493
["terraform_command", "terraform"],
485494
["aws_role_session_name", "tfaction-plan-tests_aws_foo_dev-" + runID],
@@ -546,6 +555,7 @@ test("gcp configuration", async () => {
546555
"projects/456/locations/global/workloadIdentityPools/pool/providers/provider",
547556
],
548557
["destroy", false],
558+
["accept_change_by_renovate", false],
549559
["enable_terraform_docs", false],
550560
]),
551561
};
@@ -599,6 +609,7 @@ test("gcp_access_token_scopes", async () => {
599609
["enable_trivy", true],
600610
["enable_terraform_docs", false],
601611
["destroy", false],
612+
["accept_change_by_renovate", false],
602613
["tflint_fix", false],
603614
["terraform_command", "terraform"],
604615
["aws_role_session_name", "tfaction-plan-tests_aws_foo_dev-" + runID],
@@ -652,6 +663,7 @@ test("s3_bucket_name_tfmigrate_history", async () => {
652663
["enable_trivy", true],
653664
["enable_terraform_docs", false],
654665
["destroy", false],
666+
["accept_change_by_renovate", false],
655667
["tflint_fix", false],
656668
["terraform_command", "terraform"],
657669
["aws_role_session_name", "tfaction-plan-tests_aws_foo_dev-" + runID],
@@ -702,6 +714,7 @@ test("gcs_bucket_name_tfmigrate_history", async () => {
702714
["enable_trivy", true],
703715
["enable_terraform_docs", false],
704716
["destroy", false],
717+
["accept_change_by_renovate", false],
705718
["tflint_fix", false],
706719
["terraform_command", "terraform"],
707720
["aws_role_session_name", "tfaction-plan-tests_aws_foo_dev-" + runID],
@@ -749,6 +762,7 @@ test("providers_lock_opts override", async () => {
749762
["enable_trivy", true],
750763
["enable_terraform_docs", false],
751764
["destroy", false],
765+
["accept_change_by_renovate", false],
752766
["tflint_fix", false],
753767
["terraform_command", "terraform"],
754768
["aws_role_session_name", "tfaction-plan-tests_aws_foo_dev-" + runID],
@@ -848,6 +862,7 @@ test("only workingDir provided - target derived from workingDir", async () => {
848862
["enable_trivy", true],
849863
["enable_terraform_docs", false],
850864
["destroy", false],
865+
["accept_change_by_renovate", false],
851866
["tflint_fix", false],
852867
["terraform_command", "terraform"],
853868
["aws_role_session_name", "tfaction-plan-tests_aws_foo_dev-" + runID],
@@ -896,6 +911,7 @@ test("tflint_fix enabled in root config", async () => {
896911
["enable_trivy", true],
897912
["enable_terraform_docs", false],
898913
["destroy", false],
914+
["accept_change_by_renovate", false],
899915
["tflint_fix", true],
900916
["terraform_command", "terraform"],
901917
["aws_role_session_name", "tfaction-plan-tests_aws_foo_dev-" + runID],
@@ -950,6 +966,7 @@ test("target group env overrides root env", async () => {
950966
["enable_trivy", true],
951967
["enable_terraform_docs", false],
952968
["destroy", false],
969+
["accept_change_by_renovate", false],
953970
["tflint_fix", false],
954971
["terraform_command", "terraform"],
955972
["aws_role_session_name", "tfaction-plan-tests_aws_foo_dev-" + runID],

src/actions/get-target-config/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export interface TargetConfig {
4242
// Only for non-scaffold_working_dir job types
4343
destroy?: boolean;
4444
enable_terraform_docs?: boolean;
45+
accept_change_by_renovate?: boolean;
4546

4647
// Additional envs from config (dynamic)
4748
env?: Record<string, string>;
@@ -190,6 +191,7 @@ export const getTargetConfig = async (
190191
}
191192

192193
result.destroy = wdConfig.destroy ? true : false;
194+
result.accept_change_by_renovate = wdConfig.accept_change_by_renovate;
193195
result.enable_terraform_docs =
194196
wdConfig?.terraform_docs?.enabled ??
195197
config?.terraform_docs?.enabled ??

src/actions/plan/run.test.ts

Lines changed: 77 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
22
import * as fs from "fs";
33
import * as path from "path";
44
import * as core from "@actions/core";
5+
import * as github from "@actions/github";
56
import {
67
runTerraformPlan,
78
runTfmigratePlan,
9+
disableAutoMergeForRenovateChange,
810
main,
911
type RunInputs,
1012
} from "./run";
@@ -20,6 +22,10 @@ vi.mock("@actions/core", () => ({
2022
info: vi.fn(),
2123
}));
2224

25+
vi.mock("@actions/github", () => ({
26+
getOctokit: vi.fn(),
27+
}));
28+
2329
vi.mock("@actions/artifact", () => ({
2430
DefaultArtifactClient: class MockArtifactClient {
2531
uploadArtifact = vi.fn().mockResolvedValue({});
@@ -78,6 +84,12 @@ const createMockExecutor = () => ({
7884

7985
type MockExecutor = ReturnType<typeof createMockExecutor>;
8086

87+
// Helper to create a mock octokit with graphql
88+
const createMockOctokit = () => ({
89+
graphql: vi.fn().mockResolvedValue({}),
90+
rest: {},
91+
});
92+
8193
// Base inputs for tests
8294
const createBaseInputs = (executor: MockExecutor) => ({
8395
githubToken: "test-token",
@@ -86,6 +98,7 @@ const createBaseInputs = (executor: MockExecutor) => ({
8698
destroy: false,
8799
tfCommand: "terraform",
88100
target: "aws/test/dev",
101+
acceptChangeByRenovate: false,
89102
executor: executor as unknown as aqua.Executor,
90103
});
91104

@@ -248,9 +261,18 @@ describe("runTerraformPlan", () => {
248261
);
249262
});
250263

251-
it("validates renovate change when PR author is renovate and no renovate-change label", async () => {
264+
it("disables auto-merge when renovate PR has changes and auto_merge is enabled", async () => {
265+
const mockOctokit = createMockOctokit();
266+
vi.mocked(github.getOctokit).mockReturnValue(
267+
mockOctokit as unknown as ReturnType<typeof github.getOctokit>,
268+
);
252269
vi.mocked(fs.existsSync).mockReturnValue(true);
253-
vi.mocked(fs.readFileSync).mockReturnValue("label1\nlabel2\n");
270+
vi.mocked(fs.readFileSync).mockReturnValue(
271+
JSON.stringify({
272+
node_id: "PR_123",
273+
auto_merge: { merge_method: "squash" },
274+
}),
275+
);
254276

255277
mockExecutor.getExecOutput
256278
.mockResolvedValueOnce({
@@ -271,8 +293,11 @@ describe("runTerraformPlan", () => {
271293
ciInfoTempDir: "/tmp/ci-info",
272294
};
273295

274-
await expect(runTerraformPlan(inputs)).rejects.toThrow(
275-
"Renovate PR must have 'No change' or 'renovate-change' label",
296+
const result = await runTerraformPlan(inputs);
297+
expect(result.detailedExitcode).toBe(2);
298+
expect(mockOctokit.graphql).toHaveBeenCalledWith(
299+
expect.stringContaining("disablePullRequestAutoMerge"),
300+
{ nodeId: "PR_123" },
276301
);
277302
expect(mockExecutor.exec).toHaveBeenCalledWith(
278303
"github-comment",
@@ -281,10 +306,13 @@ describe("runTerraformPlan", () => {
281306
);
282307
});
283308

284-
it("skips renovate validation when renovate-change label exists", async () => {
309+
it("skips disabling auto-merge when auto_merge is not enabled", async () => {
285310
vi.mocked(fs.existsSync).mockReturnValue(true);
286311
vi.mocked(fs.readFileSync).mockReturnValue(
287-
"label1\nrenovate-change\nlabel2\n",
312+
JSON.stringify({
313+
node_id: "PR_123",
314+
auto_merge: null,
315+
}),
288316
);
289317

290318
mockExecutor.getExecOutput
@@ -307,9 +335,10 @@ describe("runTerraformPlan", () => {
307335

308336
const result = await runTerraformPlan(inputs);
309337
expect(result.detailedExitcode).toBe(2);
338+
expect(mockExecutor.exec).not.toHaveBeenCalled();
310339
});
311340

312-
it("skips renovate validation when PR author is not renovate", async () => {
341+
it("skips renovate check when PR author is not renovate", async () => {
313342
mockExecutor.getExecOutput
314343
.mockResolvedValueOnce({
315344
exitCode: 2,
@@ -383,6 +412,47 @@ describe("runTerraformPlan", () => {
383412
});
384413
});
385414

415+
describe("disableAutoMergeForRenovateChange", () => {
416+
let mockExecutor: MockExecutor;
417+
418+
beforeEach(() => {
419+
vi.clearAllMocks();
420+
mockExecutor = createMockExecutor();
421+
});
422+
423+
afterEach(() => {
424+
vi.restoreAllMocks();
425+
});
426+
427+
it("skips when pr.json not found", async () => {
428+
vi.mocked(fs.existsSync).mockReturnValue(false);
429+
430+
const inputs = {
431+
...createBaseInputs(mockExecutor),
432+
prAuthor: "renovate[bot]",
433+
ciInfoTempDir: "/tmp/ci-info",
434+
};
435+
436+
await disableAutoMergeForRenovateChange(inputs);
437+
expect(core.warning).toHaveBeenCalledWith(
438+
"PR JSON file not found: /tmp/ci-info/pr.json",
439+
);
440+
expect(mockExecutor.exec).not.toHaveBeenCalled();
441+
});
442+
443+
it("skips when acceptChangeByRenovate is true", async () => {
444+
const inputs = {
445+
...createBaseInputs(mockExecutor),
446+
prAuthor: "renovate[bot]",
447+
ciInfoTempDir: "/tmp/ci-info",
448+
acceptChangeByRenovate: true,
449+
};
450+
451+
await disableAutoMergeForRenovateChange(inputs);
452+
expect(mockExecutor.exec).not.toHaveBeenCalled();
453+
});
454+
});
455+
386456
describe("runTfmigratePlan", () => {
387457
let mockExecutor: MockExecutor;
388458
let tempDir: string;

0 commit comments

Comments
 (0)