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

Backport of Fix mock provider ImportState data race into v1.6 #33923

Conversation

teamterraform
Copy link
Contributor

Backport

This PR is auto-generated from #33921 to be assessed for backporting due to the inclusion of the label 1.6-backport.

The below text is copied from the body of the original PR.


Fixes #33919

Fixes a bug in the mock provider that caused a data race during tests. The ImportResourceStateResponse.ImportedResources slice from the mock provider was being written by the mock setter and read by the actual resource instance.

During real provider operation, there is no sharing of data, so the race condition is not present outside testing with the mock provider.


This PR also removes the mock provider file from the legacy/terraform package, largely because I accidentally edited this file while fixing this bug and spent a few minutes trying to work out why the changes weren't doing anything.

briskt and others added 30 commits May 2, 2023 09:19
backend/s3(docs): expand access control examples
…te-locking-regression-for-local-users

Ignore remote version conflict for force-unlock
Fix local anchor link (should be to Syntax section).
A typo put the check for `GetProviderSchemaOptional` into the wrong `if`
expression, and was missed partly because of the named return which we
also remove here.
move `GetProviderSchemaOptional` check to correct block
Terraform 1.5 introduced a new check result kind, which causes a decode
failure when read by Terraform 1.3 and 1.4. This means that a 1.5+ state
file cannot be read by the `terraform_remote_state` data source in 1.3
or 1.4 series.

Aborting state decode at this point isn't strictly necessary, as we
don't stand to lose important data by failing to round-trip these check
results. Instead we can ignore those check results, allowing decode to
elide them from the state.

This has no negative effect on the remote state data source, which only
exposes root module outputs anyway.
…-compatibility

statefile: Ignore unknown check results on decode
Changes the release manifest format to more closely match the releases API V1 (example https://api.releases.hashicorp.com/v1/releases/terraform-cloudplugin/0.1.0-prototype)

- The new format doesn't carry the SHASUM for each build, so it made the matching_sums check in releaseauth redundant.
- Added tests for checksum parsing
- Added ID-based selection of signature file
backend/s3: add allowed/forbidden account ID parameters
…-not-enable-snapshots-when-x-terraform-snapshot-interval-is-not-sent
liamcervante and others added 20 commits September 14, 2023 08:32
* implement module testing via TFC

* ready for review

* fix static checks

* licence headers
…ables (#33861)

* Add additional validation around unknown values in test variables

* address comments
Even though import block evaluation is done in conjunction with their
target resource block, their evaluation scope must always be the
containing module of the import block. Since import blocks currently are
only valid in the root module, that means the context must always always
be the root module.
When an import block is being evaluated by a resource within a module,
the references for the import evaluation must be scoped to the root
module where the block was written. In order to accomplish that without
adding a new import node type to the graph, we instead can opt to add a
new method which separates the import references from the config
references. The ReferenceTransformer can then use the correct root
module path to lookup the referenced subjects when they come from
ImportReferences as opposed to References.
backend/s3: Add wrong bucket region handling to all of S3 client
import blocks are evaluated in the root module
…irements

configschema: CoerceValue error for omitted attribute requirements
backend/s3: set required or optional for single nested attributes
This commit fixes a bug in the mock provider that caused a data race
during tests. The ImportResourceStateResponse.ImportedResources slice from the mock provider was being written by the mock setter and read by the actual resource instance.

During real provider operation, there is no sharing of data, so the race
condition is not present outside testing with the mock provider.
@teamterraform teamterraform requested a review from a team as a code owner September 21, 2023 16:35
@teamterraform teamterraform force-pushed the backport/mock-importstate-data-race/adversely-prompt-halibut branch from e4a7ac3 to 8528dc4 Compare September 21, 2023 16:35
@teamterraform teamterraform force-pushed the backport/mock-importstate-data-race/adversely-prompt-halibut branch 2 times, most recently from 6a0c843 to e5d41fa Compare September 21, 2023 16:35
@kmoe
Copy link
Member

kmoe commented Sep 21, 2023

Bot went insane. Superseded by #33924.

Copy link
Contributor

github-actions bot commented Dec 9, 2023

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, 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 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.