Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDK v2.23.0 provider merging causes duplicate provider test failures #1064

Closed
dghubble opened this issue Sep 20, 2022 · 7 comments · Fixed by #1092
Closed

SDK v2.23.0 provider merging causes duplicate provider test failures #1064

dghubble opened this issue Sep 20, 2022 · 7 comments · Fixed by #1092
Assignees
Labels
bug Something isn't working subsystem/tests Issues and feature requests related to the testing framework.
Milestone

Comments

@dghubble
Copy link

SDK version

Upgrading from v2.22.0 to v2.23.0 shows the breakage. See poseidon/terraform-provider-matchbox#114

Relevant provider source code

Here's an example poseidon/terraform-provider-matchbox#114

The Providers field is set (required) and an HCL provider block is defined as part of the config. https://github.com/poseidon/terraform-provider-matchbox/blob/main/matchbox/resource_group_test.go#L45

Providers: testProviders,                   <--- provider
Steps: []resource.TestStep{
 {
  Config: srv.AddProviderConfig(groupWithAllFields),    <--- provider HCL
  Check: resource.ComposeAggregateTestCheckFunc(
...

The provider block is added to the config by AddProviderConfig to configure the provider with neccessary credentials for testing. Overall, I think its a pretty typical situation.

What changed is that SDK v2.23.0 added merging logic for providers in #1057 (file helper/resource/teststep_providers.go) https://github.com/hashicorp/terraform-plugin-sdk/blob/main/helper/resource/teststep_providers.go#L16 that introduces duplication.

I've been able to checkout sdk v2.23.0, add a replace directive to go.mod, show it breaks my provider, revert #1057, and show the problem is resolved.

Terraform Configuration Files

These details aren't important for repro, but I'll include just since the issue template asks.

provider "matchbox" {
  endpoint = "some-endpoint"
  client_cert = <<CERT
some cert
CERT
  client_key = <<KEY
some key
KEY
  ca         = <<CA
some ca
CA
}

resource "matchbox_profile" "default" {
  name   = "default"
  kernel = "foo"
}

Debug Output

Expected Behavior

Tests should continue working on v2.23.0 as they did on v2.22.0

Actual Behavior

SDK tests always see this as duplicated providers (although the SDK is the one adding the duplicate).

Error: Duplicate provider configuration
        
          on terraform_plugin_test.tf line 3:
           3: 		provider "matchbox" {
        
        A default (non-aliased) provider configuration for "matchbox" was already
        given at terraform_plugin_test.tf:1,1-20. If multiple configurations are
        required, set the "alias" argument for alternative configurations.

Steps to Reproduce

poseidon/terraform-provider-matchbox#114 is a great example. Rollback to v2.22.0 and it works again.

References

#1057 @bflad

@dghubble dghubble added the bug Something isn't working label Sep 20, 2022
@bflad bflad added the subsystem/tests Issues and feature requests related to the testing framework. label Sep 20, 2022
@bflad
Copy link
Member

bflad commented Sep 20, 2022

Hi @dghubble 👋 Thank you for reporting this and sorry the upgrade gave you unexpected issues.

Just to confirm things, if you run your testing with the TF_LOG environment variable set to TRACE and search for Setting Terraform configuration right before the testing errors out, does it show a configuration with multiple provider "matchbox" blocks? The logic that's adding the terraform/provider configuration blocks should only take affect for TestCase or TestStep that use the ExternalProviders field, but ... okay I found the code issue:

// [BF] The Providers field handling predates the logic being moved to this
// method. It's not entirely clear to me at this time why this field
// is being used and not the others, but leaving it here just in case
// it does have a special purpose that wasn't being unit tested prior.
for name := range c.Providers {
providerBlocks.WriteString(fmt.Sprintf("provider %q {}\n", name))
}

Apparently TestCase Providers field is treated special. 😖 My apologies for missing that. The configuration merging logic will need to account for that by skipping the providers blocks if provider is already found in the configuration.

@bflad
Copy link
Member

bflad commented Sep 20, 2022

It might be best if we just unilaterally skipped the provider source merge if the terraform or any provider configuration blocks are found in the given configurations.

@bflad
Copy link
Member

bflad commented Sep 20, 2022

Another workaround in your case without any changes in the SDK would be switching from the deprecated Providers field to the ProviderFactories field. That should skip the duplicate provider ... being written into the merged configuration.

@dghubble
Copy link
Author

Just to confirm things...

It doesn't show the exact configuration, but there seem to be two "matchbox" providers. That may be related to the original problem you were solving with hashicorp vs non-hashicorp.

[TRACE] sdk.helper_resource: Setting Terraform CLI reattach configuration: test_step_number=1 test_terraform_path=/usr/local/bin/terraform test_working_directory=/tmp/plugintest1199335445 tf_reattach_config="map[registry.terraform.io/-/matchbox:{grpc 5 557795 true {unix /tmp/plugin1472369595}} registry.terraform.io/hashicorp/matchbox:{grpc 5 557795 true {unix /tmp/plugin1472369595}}]" test_name=TestResourceProfile_Read

Another workaround in your case without any changes in the SDK would be switching from the deprecated Providers field to the ProviderFactories field

Interesting. Yeah, I can change to ProviderFactories and it doesn't cause the problem anymore. Nice

dghubble added a commit to poseidon/terraform-provider-matchbox that referenced this issue Sep 21, 2022
* Providers was deprecated in favor of ProviderFactories
* Workaround an issue with the terraform-plugin-sdk v2.23.0 that causes
duplicate provider blocks when UnitTest.Providers is used
* Update terraform-plugin-sdk from v2.22.0 to v2.23.0

Rel: hashicorp/terraform-plugin-sdk#1064
dghubble added a commit to poseidon/terraform-provider-matchbox that referenced this issue Sep 21, 2022
* Providers was deprecated in favor of ProviderFactories
* Workaround an issue with the terraform-plugin-sdk v2.23.0 that causes
duplicate provider blocks when UnitTest.Providers is used
* Update terraform-plugin-sdk from v2.22.0 to v2.23.0

Rel: hashicorp/terraform-plugin-sdk#1064
@varnastadeus
Copy link

varnastadeus commented Oct 11, 2022

@bflad we are having same issue:

Step 1/2 error: Error running pre-apply refresh: exit status 1
        
        Error: Duplicate provider configuration
        
          on terraform_plugin_test.tf line 14:
          14: provider "aws" {
        
        A default (non-aliased) provider configuration for "aws" was already given at
        terraform_plugin_test.tf:11,1-[15](https://github.com/castai/terraform-provider-castai/actions/runs/3226033532/jobs/5279063009#step:8:16). If multiple configurations are required,
        set the "alias" argument for alternative configurations.

we are using external providers

ExternalProviders: map[string]resource.ExternalProvider{
			"aws": {
				Source:            "hashicorp/aws",
				VersionConstraint: "~> 4.0",
			},
		},

and

provider "aws" {
  region = "eu-central-1"
}

switching to v2.22.0 works as expected

@bflad bflad self-assigned this Nov 8, 2022
@bflad bflad added this to the v2.24.1 milestone Nov 8, 2022
bflad added a commit that referenced this issue Nov 8, 2022
…estStep merged configuration

Reference: #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
              	"""
              )
```
@bflad
Copy link
Member

bflad commented Nov 8, 2022

Submitted #1092, which should prevent the testing framework from adding its own provider configuration blocks if it detects a TestStep.Config that already has them. 👍

bflad added a commit that referenced this issue Nov 14, 2022
…estStep merged configuration (#1092)

Reference: #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
              	"""
              )
```
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working subsystem/tests Issues and feature requests related to the testing framework.
Projects
None yet
3 participants