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

Modify detection of provider blocks in configuration #264

Closed
bendbennett opened this issue Jan 8, 2024 · 9 comments · Fixed by #265
Closed

Modify detection of provider blocks in configuration #264

bendbennett opened this issue Jan 8, 2024 · 9 comments · Fixed by #265
Labels
bug Something isn't working

Comments

@bendbennett
Copy link
Contributor

terraform-plugin-testing version

v1.6.0

Relevant provider source code

N/A

Terraform Configuration Files

The following are all allowed provider block declarations from the perspective of terraform validate.

provider"random"{}
provider "random"{}
provider random{}

Expected Behavior

Using the provider block configurations supplied should result in successfully executing tests.

Actual Behavior

The following error is raised:

        Error: Duplicate provider configuration
        
          on terraform_plugin_test.tf line 13:
          13: provider"random"{}
        
        A default (non-aliased) provider configuration for "random" was already given
        at terraform_plugin_test.tf:11,1-18. If multiple configurations are required,
        set the "alias" argument for alternative configurations.
    testing_new.go:80: Error retrieving state, there may be dangling resources: exit status 1

Steps to Reproduce

Run an acceptance test with a provider block configuration that uses the quoting and /or spacing described in Terraform Configuration Files.

@bendbennett bendbennett added the bug Something isn't working label Jan 8, 2024
@bflad
Copy link
Contributor

bflad commented Jan 8, 2024

I wonder if we should submit something upstream for provider"random"{} -- its probably fine for us to support the lack of whitespace after the provider block label, but the label-without-prior-whitespace feels very 😬 to me personally.

@bendbennett
Copy link
Contributor Author

There's actually no usage of provider"random"{} in the random provider. @austinvalle uncovered that these various forms of provider block declaration are all legitimate terraform configuration, so I thought it might be best to have the detection of these forms of declaration be taken into account in case they caused the errors described.

@bflad
Copy link
Contributor

bflad commented Jan 8, 2024

I have no issues with implementing the unquoted and whitespace missing after label cases (e.g. provider "LABEL"{), but I'm asking if we should submit an issue to https://github.com/hashicorp/terraform/issues to consider having terraform validate raise an error if it encounters provider"LABEL"{ without the whitespace before the label. It may be valid in HCL for whatever reason, but seems like an awkward Terraform configuration style to support.

@austinvalle
Copy link
Member

drive-by - 🚗 - terraform fmt will add those whitespaces in (which our addition of config files and directories will make it easier to take advantage of 🥳 )

image

Looking at the validate command docs, they emphasize that the config is only checked for syntactic correctness:

Validate runs checks that verify whether a configuration is syntactically valid and internally consistent, regardless of any provided variables or existing state.

I'd imagine based on how it's described that they'd be unlikely to also check formatting during that command.

Maybe we could add a test warning if we detect that terraform fmt -check says the formatting is wonky, since it's possible and probable that provider developers will be just using Go strings for their config. Although that might open another can of worms 😆

@austinvalle
Copy link
Member

At the very least, it's probably worth documenting (if we haven't already) that we dynamically add terraform configuration to their test configs if needed.

@bendbennett
Copy link
Contributor Author

I have no issues with implementing the unquoted and whitespace missing after label cases (e.g. provider "LABEL"{), but I'm asking if we should submit an issue to https://github.com/hashicorp/terraform/issues to consider having terraform validate raise an error if it encounters provider"LABEL"{ without the whitespace before the label. It may be valid in HCL for whatever reason, but seems like an awkward Terraform configuration style to support.

Ooops. Apologies, I completely misunderstood what you were getting at here. I've opened an issue for Consider having terraform validate raise an error if provider block defined without whitespace.

@bendbennett
Copy link
Contributor Author

drive-by - 🚗 - terraform fmt will add those whitespaces in (which our addition of config files and directories will make it easier to take advantage of 🥳 )

image

Looking at the validate command docs, they emphasize that the config is only checked for syntactic correctness:

Validate runs checks that verify whether a configuration is syntactically valid and internally consistent, regardless of any provided variables or existing state.

I'd imagine based on how it's described that they'd be unlikely to also check formatting during that command.

Maybe we could add a test warning if we detect that terraform fmt -check says the formatting is wonky, since it's possible and probable that provider developers will be just using Go strings for their config. Although that might open another can of worms 😆

Have added an issue for Consider running terraform validate against supplied configuration to track this.

@bendbennett
Copy link
Contributor Author

At the very least, it's probably worth documenting (if we haven't already) that we dynamically add terraform configuration to their test configs if needed.

Agreed. I've added an issue to track this Consider Adding Documentation Describing Dynamic Modification of Terraform Configuration.

bendbennett added a commit that referenced this issue Jan 9, 2024
…ration (#265)

* Modify regex to detect other legitimate forms of provider block declaration (#264)

* Add change log entry (#264)

* Update internal/teststep/config.go

Co-authored-by: Brian Flad <[email protected]>

* Modify regex to detect other legitimate forms of terraform block declaration (#264)

---------

Co-authored-by: Brian Flad <[email protected]>
Copy link

github-actions bot commented Feb 9, 2024

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 Feb 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
3 participants