gitlab_project_variable: fix nondeterministic reads when using environment_scope #409
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
armsnyder
force-pushed
the
project-variable-scope
branch
10 times, most recently
from
21db403 to
285ad4e
Compare
|
@roidelapluie @nicholasklick @sfang97 This is the continuation of #324, which has merge conflicts. I simplified the resource implementation greatly from that PR (see PR description for why). On the other hand the test file is a lot to review, but I wanted to make sure the resource was fully covered. ❤️ |
armsnyder
force-pushed
the
project-variable-scope
branch
from
285ad4e to
aebe687
Compare
armsnyder
force-pushed
the
project-variable-scope
branch
from
aebe687 to
a9bcbee
Compare
- remove indirection to build and parse the resource id - link to issue in the resource docs - small test cleanup by making a isGitLabVersionLessThan SkipFunc
|
awesome |
roidelapluie
approved these changes
Sep 13, 2020
|
@roidelapluie @armsnyder if this is good to go can we merge it? |
This was referenced Sep 15, 2020
|
Any ETA for the release? |
|
@abelmokadem Soon™! It's our first time releasing to the Terraform Registry, but we're making progress in the #411 issue. |
abelmokadem
reviewed
Sep 18, 2020
| @@ -12,6 +12,10 @@ This resource allows you to create and manage CI/CD variables for your GitLab pr | |||
| For further information on variables, consult the [gitlab | |||
| documentation](https://docs.gitlab.com/ce/ci/variables/README.html#variables). | |||
|
|
|||
| ~> **Important:** If your GitLab version is older than 13.4, you may see nondeterministic behavior | |||
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Supersedes #324
Fixes #213
Fixes #195
Fixes #278
tl;dr I borrowed just the read function logic from #324 and added the
environment_scopefilter query to the update and delete functions.This PR fixes a behavior where
gitlab_project_variableresources for keys with multiple environment scopes were not reading data from a unique variable, but could read from any variable with the specified key, causing unexpected drift.For users with the feature flag on or who are using GitLab 13.4+, this also lets them change environment scopes and remove variables with environment scopes, for
gitlab_project_variableresources that have non-unique keys.Note that one big difference between this and #324 is that I decided to only modify the read function, and not modify the update or delete functions, apart from adding the
environment_scopefilter query. I thought that those functions as implemented in #324 were not idempotent-safe. I would rather wait for a proper fix from GitLab (see previous paragraph) than try to work around the API in the provider for older GitLab versions. Theenvironment_scopefilter query in those functions ensures that people using the feature flag or on future versions of GitLab will have a better time.I tried using a
MigrateStatein the schema to handle the ID format change, but it didn't work when I tested it. I ended up with an ID format error. Instead, I have the read function handle both ID formats.Note that the
gitlab/resource_gitlab_project_variable_test.gofile is easier to review side-by-side than inline, since I rewrote it from scratch to make sure it was exercising all behaviors.I've tested this locally with GitLab v13.1 and v13.4 to make sure both versions work.
Incidentally this PR is a step toward #205, by having a destroy test that leaves the GitLab project alone and checks that the resources under test are removed from GitLab.