Skip to content

Commit 9523045

Browse files
authored
helper/resource: Check for existing provider configuration block in TestStep merged configuration (#1092)
Reference: hashicorp/terraform-plugin-sdk#1064 Previously: ``` --- FAIL: TestStepMergedConfig/teststep-externalproviders-config-with-provider-block-unquoted (0.00s) /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/resource/teststep_providers_test.go:643: unexpected difference:   (    """    ... // 6 identical lines    }    } -  -  provider "test" {} -        provider test {}    ... // 2 identical lines    """   ) --- FAIL: TestStepMergedConfig/teststep-externalproviders-config-with-provider-block-quoted (0.00s) /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/resource/teststep_providers_test.go:643: unexpected difference:   (    """    ... // 6 identical lines    }    } -  -  provider "test" {} -        provider "test" {}    ... // 2 identical lines    """   ) ```
1 parent ff2cdef commit 9523045

File tree

6 files changed

+296
-19
lines changed

6 files changed

+296
-19
lines changed

.changelog/1092.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
helper/resource: Prevented provider configuration already given error when `TestStep` type `Config` field already contained provider configuration block
3+
```

helper/resource/testcase_providers.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
// providerConfig takes the list of providers in a TestCase and returns a
1010
// config with only empty provider blocks. This is useful for Import, where no
1111
// config is provided, but the providers must be defined.
12-
func (c TestCase) providerConfig(_ context.Context) string {
12+
func (c TestCase) providerConfig(_ context.Context, skipProviderBlock bool) string {
1313
var providerBlocks, requiredProviderBlocks strings.Builder
1414

1515
// [BF] The Providers field handling predates the logic being moved to this
@@ -21,7 +21,9 @@ func (c TestCase) providerConfig(_ context.Context) string {
2121
}
2222

2323
for name, externalProvider := range c.ExternalProviders {
24-
providerBlocks.WriteString(fmt.Sprintf("provider %q {}\n", name))
24+
if !skipProviderBlock {
25+
providerBlocks.WriteString(fmt.Sprintf("provider %q {}\n", name))
26+
}
2527

2628
if externalProvider.Source == "" && externalProvider.VersionConstraint == "" {
2729
continue

helper/resource/testcase_providers_test.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@ func TestTestCaseProviderConfig(t *testing.T) {
1717
t.Parallel()
1818

1919
tests := map[string]struct {
20-
testCase TestCase
21-
expected string
20+
testCase TestCase
21+
skipProviderBlock bool
22+
expected string
2223
}{
2324
"externalproviders-and-protov5providerfactories": {
2425
testCase: TestCase{
@@ -103,6 +104,27 @@ provider "externaltest" {}
103104
},
104105
expected: `provider "test" {}`,
105106
},
107+
"externalproviders-skip-provider-block": {
108+
testCase: TestCase{
109+
ExternalProviders: map[string]ExternalProvider{
110+
"test": {
111+
Source: "registry.terraform.io/hashicorp/test",
112+
VersionConstraint: "1.2.3",
113+
},
114+
},
115+
},
116+
skipProviderBlock: true,
117+
expected: `
118+
terraform {
119+
required_providers {
120+
test = {
121+
source = "registry.terraform.io/hashicorp/test"
122+
version = "1.2.3"
123+
}
124+
}
125+
}
126+
`,
127+
},
106128
"externalproviders-source-and-versionconstraint": {
107129
testCase: TestCase{
108130
ExternalProviders: map[string]ExternalProvider{
@@ -205,7 +227,7 @@ provider "test" {}
205227
t.Run(name, func(t *testing.T) {
206228
t.Parallel()
207229

208-
got := test.testCase.providerConfig(context.Background())
230+
got := test.testCase.providerConfig(context.Background(), test.skipProviderBlock)
209231

210232
if diff := cmp.Diff(strings.TrimSpace(got), strings.TrimSpace(test.expected)); diff != "" {
211233
t.Errorf("unexpected difference: %s", diff)

helper/resource/testing_new.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest
8989
}()
9090

9191
if c.hasProviders(ctx) {
92-
err := wd.SetConfig(ctx, c.providerConfig(ctx))
92+
err := wd.SetConfig(ctx, c.providerConfig(ctx, false))
9393

9494
if err != nil {
9595
logging.HelperResourceError(ctx,
@@ -170,7 +170,7 @@ func runNewTest(ctx context.Context, t testing.T, c TestCase, helper *plugintest
170170
protov6: protov6ProviderFactories(c.ProtoV6ProviderFactories).merge(step.ProtoV6ProviderFactories),
171171
}
172172

173-
providerCfg := step.providerConfig(ctx)
173+
providerCfg := step.providerConfig(ctx, step.configHasProviderBlock(ctx))
174174

175175
err := wd.SetConfig(ctx, providerCfg)
176176

@@ -371,7 +371,7 @@ func testIDRefresh(ctx context.Context, t testing.T, c TestCase, wd *plugintest.
371371

372372
// Temporarily set the config to a minimal provider config for the refresh
373373
// test. After the refresh we can reset it.
374-
err := wd.SetConfig(ctx, c.providerConfig(ctx))
374+
err := wd.SetConfig(ctx, c.providerConfig(ctx, step.configHasProviderBlock(ctx)))
375375
if err != nil {
376376
t.Fatalf("Error setting import test config: %s", err)
377377
}

helper/resource/teststep_providers.go

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,24 @@ package resource
33
import (
44
"context"
55
"fmt"
6+
"regexp"
67
"strings"
78
)
89

10+
var configProviderBlockRegex = regexp.MustCompile(`provider "?[a-zA-Z0-9_-]+"? {`)
11+
12+
// configHasProviderBlock returns true if the Config has declared a provider
13+
// configuration block, e.g. provider "examplecloud" {...}
14+
func (s TestStep) configHasProviderBlock(_ context.Context) bool {
15+
return configProviderBlockRegex.MatchString(s.Config)
16+
}
17+
18+
// configHasTerraformBlock returns true if the Config has declared a terraform
19+
// configuration block, e.g. terraform {...}
20+
func (s TestStep) configHasTerraformBlock(_ context.Context) bool {
21+
return strings.Contains(s.Config, "terraform {")
22+
}
23+
924
// mergedConfig prepends any necessary terraform configuration blocks to the
1025
// TestStep Config.
1126
//
@@ -18,12 +33,16 @@ func (s TestStep) mergedConfig(ctx context.Context, testCase TestCase) string {
1833

1934
// Prevent issues with existing configurations containing the terraform
2035
// configuration block.
21-
if !strings.Contains(s.Config, "terraform {") {
22-
if testCase.hasProviders(ctx) {
23-
config.WriteString(testCase.providerConfig(ctx))
24-
} else {
25-
config.WriteString(s.providerConfig(ctx))
26-
}
36+
if s.configHasTerraformBlock(ctx) {
37+
config.WriteString(s.Config)
38+
39+
return config.String()
40+
}
41+
42+
if testCase.hasProviders(ctx) {
43+
config.WriteString(testCase.providerConfig(ctx, s.configHasProviderBlock(ctx)))
44+
} else {
45+
config.WriteString(s.providerConfig(ctx, s.configHasProviderBlock(ctx)))
2746
}
2847

2948
config.WriteString(s.Config)
@@ -34,11 +53,13 @@ func (s TestStep) mergedConfig(ctx context.Context, testCase TestCase) string {
3453
// providerConfig takes the list of providers in a TestStep and returns a
3554
// config with only empty provider blocks. This is useful for Import, where no
3655
// config is provided, but the providers must be defined.
37-
func (s TestStep) providerConfig(_ context.Context) string {
56+
func (s TestStep) providerConfig(_ context.Context, skipProviderBlock bool) string {
3857
var providerBlocks, requiredProviderBlocks strings.Builder
3958

4059
for name, externalProvider := range s.ExternalProviders {
41-
providerBlocks.WriteString(fmt.Sprintf("provider %q {}\n", name))
60+
if !skipProviderBlock {
61+
providerBlocks.WriteString(fmt.Sprintf("provider %q {}\n", name))
62+
}
4263

4364
if externalProvider.Source == "" && externalProvider.VersionConstraint == "" {
4465
continue

0 commit comments

Comments
 (0)