Skip to content

Commit 29f166c

Browse files
jar-bjar-b
authored
internal/vcr: add vcr support semgrep rules (#42855)
These rules will apply autofixes that enable VCR support for a given service package. At this time the intent is to manually apply the rules one service at a time and run acceptance tests (both with and without VCR enabled) to verify no breakage has occurred. A new Make target, `make semgrep-vcr`, has been added to simplify how this process is completed. ```console % PKG=logs make semgrep-vcr ``` Because certain autofixes apply changes inside blocks which are modified by other rules, this command may need to be run twice to fully complete the process. Additionally, the resulting code may need to be reformatted and imports may need to be tidied, depending on the scope of the change. ```console % make fmt ``` ```console % goimports -w internal/service/logs/ ```
1 parent 905f2f1 commit 29f166c

File tree

3 files changed

+153
-0
lines changed

3 files changed

+153
-0
lines changed

‎GNUmakefile

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,14 @@ semgrep-validate: ## Validate Semgrep configuration files
574574
--config .ci/.semgrep-service-name3.yml \
575575
--config .ci/semgrep/
576576

577+
semgrep-vcr: ## Enable VCR support with Semgrep --autofix
578+
@echo "make: Enable VCR support with Semgrep --autofix"
579+
@echo "WARNING: Because some autofixes are inside code blocks replaced by other rules,"
580+
@echo "this target may need to be run twice."
581+
@semgrep $(SEMGREP_ARGS) --autofix \
582+
$(if $(filter-out $(origin PKG), undefined),--include $(PKG_NAME),) \
583+
--config internal/vcr/.semgrep-vcr.yml
584+
577585
skaff: prereq-go ## Install skaff
578586
@echo "make: Installing skaff..."
579587
cd skaff && $(GO_VER) install github.com/hashicorp/terraform-provider-aws/skaff
@@ -929,6 +937,7 @@ yamllint: ## [CI] YAML Linting / yamllint
929937
semgrep-naming \
930938
semgrep-service-naming \
931939
semgrep-validate \
940+
semgrep-vcr \
932941
semgrep \
933942
skaff-check-compile \
934943
skaff \

‎docs/makefile-cheat-sheet.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ Variables are often defined before the `make` call on the same line, such as `MY
150150
| `semgrep-naming-cae`<sup>D</sup> | Semgrep Checks / Naming Scan Caps/`AWS`/EC2 | ✔️ | | `K`, `PKG`, `PKG_NAME`, `SEMGREP_ARGS` |
151151
| `semgrep-service-naming`<sup>D</sup> | Semgrep Checks / Service Name Scan A-Z | ✔️ | | `K`, `PKG`, `PKG_NAME`, `SEMGREP_ARGS` |
152152
| `semgrep-validate` | Validate Semgrep configuration files | | | |
153+
| `semgrep-vcr` | Enable VCR support with Semgrep --autofix | | | `K`, `PKG`, `PKG_NAME`, `SEMGREP_ARGS` |
153154
| `skaff`<sup>D</sup> | Install skaff | | | `GO_VER` |
154155
| `skaff-check-compile` | Skaff Checks / Compile skaff | ✔️ | | |
155156
| `sweep`<sup>D</sup> | Run sweepers | | | `GO_VER`, `SWEEP_DIR`, `SWEEP_TIMEOUT`, `SWEEP`, `SWEEPARGS` |

‎internal/vcr/.semgrep-vcr.yml

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
# Copyright (c) HashiCorp, Inc.
2+
# SPDX-License-Identifier: MPL-2.0
3+
4+
rules:
5+
- id: use-acctest-test
6+
languages: [go]
7+
message: "Use acctest.Test instead of resource.Test for VCR-compatible acceptance testing"
8+
severity: WARNING
9+
pattern: |
10+
resource.Test($T, $TC)
11+
fix: |
12+
acctest.Test(ctx, $T, $TC)
13+
paths:
14+
include:
15+
- "**/*_test.go"
16+
17+
- id: use-acctest-paralleltest
18+
languages: [go]
19+
message: "Use acctest.ParallelTest instead of resource.ParallelTest for VCR-compatible acceptance testing"
20+
severity: WARNING
21+
pattern: |
22+
resource.ParallelTest($T, $TC)
23+
fix: |
24+
acctest.ParallelTest(ctx, $T, $TC)
25+
paths:
26+
include:
27+
- "**/*_test.go"
28+
29+
- id: use-acctest-randomwithprefix
30+
languages: [go]
31+
message: "Use acctest.RandomWithPrefix instead of sdkacctest.RandomWithPrefix for VCR-compatible acceptance testing"
32+
severity: WARNING
33+
pattern: |
34+
sdkacctest.RandomWithPrefix($P)
35+
fix: |
36+
acctest.RandomWithPrefix(t, $P)
37+
paths:
38+
include:
39+
- "**/*_test.go"
40+
41+
- id: use-acctest-randint
42+
languages: [go]
43+
message: "Use acctest.RandInt instead of sdkacctest.RandInt for VCR-compatible acceptance testing"
44+
severity: WARNING
45+
pattern: |
46+
sdkacctest.RandInt()
47+
fix: |
48+
acctest.RandInt(t)
49+
paths:
50+
include:
51+
- "**/*_test.go"
52+
53+
- id: use-acctest-providermeta
54+
languages: [go]
55+
message: "Use acctest.ProviderMeta instead of acctest.Provider.Meta for VCR-compatible acceptance testing"
56+
severity: WARNING
57+
pattern: |
58+
acctest.Provider.Meta().(*conns.AWSClient).$C(ctx)
59+
fix: |
60+
acctest.ProviderMeta(ctx, t).$C(ctx)
61+
paths:
62+
include:
63+
- "**/*_test.go"
64+
65+
- id: add-testing-t-to-destroy-testcheckfunc
66+
languages: [go]
67+
message: "Add a testing.T argument into destroy TestCheckFunc helpers for VCR-compatible acceptance testing"
68+
severity: WARNING
69+
patterns:
70+
- pattern: |
71+
func $F(ctx context.Context) resource.TestCheckFunc
72+
- metavariable-regex:
73+
metavariable: $F
74+
regex: (testAccCheck.*Destroy.*)
75+
fix: |
76+
func $F(ctx context.Context, t *testing.T) resource.TestCheckFunc
77+
paths:
78+
include:
79+
- "**/*_test.go"
80+
81+
- id: add-testing-t-to-exists-testcheckfunc
82+
languages: [go]
83+
message: "Add a testing.T argument into exists TestCheckFunc helpers for VCR-compatible acceptance testing"
84+
severity: WARNING
85+
patterns:
86+
- pattern: |
87+
func $F(ctx context.Context, $...ARGS) resource.TestCheckFunc
88+
- pattern-not: |
89+
func $F(ctx context.Context) resource.TestCheckFunc
90+
- pattern-not: |
91+
func $F(..., t *testing.T, ...) resource.TestCheckFunc
92+
- metavariable-regex:
93+
metavariable: $F
94+
regex: (testAccCheck.*Exists$)
95+
fix: |
96+
func $F(ctx context.Context, t *testing.T, $...ARGS) resource.TestCheckFunc
97+
paths:
98+
include:
99+
- "**/*_test.go"
100+
101+
# NOTE: because this matched pattern is inside another section which is replaced by a previous rule,
102+
# autofix may need to be run twice in order to apply this change.
103+
- id: pass-testing-t-to-destroy-testcheckfunc
104+
languages: [go]
105+
message: "Pass testing.T argument into destroy TestCheckFunc helpers for VCR-compatible acceptance testing"
106+
severity: WARNING
107+
patterns:
108+
- pattern-inside: "resource.TestCase{ ... }"
109+
- pattern: |
110+
$F(ctx)
111+
- metavariable-regex:
112+
metavariable: $F
113+
regex: (testAccCheck.*Destroy.*)
114+
fix: |
115+
$F(ctx, t)
116+
paths:
117+
include:
118+
- "**/*_test.go"
119+
120+
# NOTE: because this matched pattern is inside another section which is replaced by a previous rule,
121+
# autofix may need to be run twice in order to apply this change.
122+
- id: pass-testing-t-to-exists-testcheckfunc
123+
languages: [go]
124+
message: "Pass testing.T argument into exists TestCheckFunc helpers for VCR-compatible acceptance testing"
125+
severity: WARNING
126+
patterns:
127+
- pattern-either:
128+
- pattern-inside: "Check: resource.ComposeTestCheckFunc( ... )"
129+
- pattern-inside: "Check: resource.ComposeAggregateTestCheckFunc( ... )"
130+
- pattern: |
131+
$F(ctx, $...ARGS)
132+
- pattern-not: |
133+
$F(ctx)
134+
- pattern-not: |
135+
$F(..., t, ...)
136+
- metavariable-regex:
137+
metavariable: $F
138+
regex: (testAccCheck.*Exists$)
139+
fix: |
140+
$F(ctx, t, $...ARGS)
141+
paths:
142+
include:
143+
- "**/*_test.go"

0 commit comments

Comments
 (0)