Skip to content

Commit 8f2d1a0

Browse files
timofurrertimofurrer
authored
Merge pull request #866 from timofurrer/bugfix-861
Add failing group variable update test to test issue #861
2 parents 9cb5889 + bc41c70 commit 8f2d1a0

File tree

4 files changed

+67
-48
lines changed

4 files changed

+67
-48
lines changed

‎internal/provider/environment_scope_helper.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package provider
2+
3+
import (
4+
"context"
5+
"net/url"
6+
7+
"github.com/hashicorp/go-retryablehttp"
8+
"github.com/xanzy/go-gitlab"
9+
)
10+
11+
// withEnvironmentScopeFilter adds the environment scope filter query parameter to the URL.
12+
// This function is supposed to be used as `gitlab.RequestOptionFunc` parameter.
13+
// The parameter is documented in the upstream GitLab API docs:
14+
// https://docs.gitlab.com/ee/api/project_level_variables.html#the-filter-parameter
15+
func withEnvironmentScopeFilter(ctx context.Context, environmentScope string) gitlab.RequestOptionFunc {
16+
return func(req *retryablehttp.Request) error {
17+
*req = *req.WithContext(ctx)
18+
query, err := url.ParseQuery(req.Request.URL.RawQuery)
19+
if err != nil {
20+
return err
21+
}
22+
query.Set("filter[environment_scope]", environmentScope)
23+
req.Request.URL.RawQuery = query.Encode()
24+
return nil
25+
}
26+
}

‎internal/provider/resource_gitlab_group_variable.go‎

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,11 @@ import (
66
"log"
77
"strings"
88

9-
"github.com/hashicorp/go-retryablehttp"
109
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
1110
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1211
gitlab "github.com/xanzy/go-gitlab"
1312
)
1413

15-
// modifyRequestAddEnvironmentFilter returns a RequestOptionFunc function that
16-
// can be passed to the go-gitlab library calls to add the environment scope to
17-
// requests to lookup, modification, and deletion requests. Since gitlab 13.11,
18-
// an environment variable key is no longer unique and is composit-keyed with
19-
// the scope.
20-
// See https://docs.gitlab.com/ee/ci/variables/#add-a-cicd-variable-to-a-group
21-
func modifyRequestAddEnvironmentFilter(scope string) gitlab.RequestOptionFunc {
22-
return func(r *retryablehttp.Request) error {
23-
queryParams := r.URL.Query()
24-
queryParams.Add("filter[environment_scope]", scope)
25-
r.URL.RawQuery = queryParams.Encode()
26-
return nil
27-
}
28-
}
29-
3014
var _ = registerResource("gitlab_group_variable", func() *schema.Resource {
3115
return &schema.Resource{
3216
Description: "This resource allows you to create and manage CI/CD variables for your GitLab groups.\n" +
@@ -145,7 +129,7 @@ func resourceGitlabGroupVariableRead(ctx context.Context, d *schema.ResourceData
145129
group,
146130
key,
147131
gitlab.WithContext(ctx),
148-
modifyRequestAddEnvironmentFilter(scope),
132+
withEnvironmentScopeFilter(ctx, scope),
149133
)
150134
if err != nil {
151135
if is404(err) {
@@ -191,7 +175,7 @@ func resourceGitlabGroupVariableUpdate(ctx context.Context, d *schema.ResourceDa
191175
key,
192176
options,
193177
gitlab.WithContext(ctx),
194-
modifyRequestAddEnvironmentFilter(environmentScope),
178+
withEnvironmentScopeFilter(ctx, environmentScope),
195179
)
196180
if err != nil {
197181
return diag.FromErr(err)
@@ -210,7 +194,7 @@ func resourceGitlabGroupVariableDelete(ctx context.Context, d *schema.ResourceDa
210194
group,
211195
key,
212196
gitlab.WithContext(ctx),
213-
modifyRequestAddEnvironmentFilter(environmentScope),
197+
withEnvironmentScopeFilter(ctx, environmentScope),
214198
)
215199
if err != nil {
216200
return diag.FromErr(err)

‎internal/provider/resource_gitlab_group_variable_test.go‎

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package provider
22

33
import (
4+
"context"
45
"fmt"
56
"testing"
67

@@ -65,64 +66,87 @@ func TestAccGitlabGroupVariable_scope(t *testing.T) {
6566
var groupVariableA, groupVariableB gitlab.GroupVariable
6667
rString := acctest.RandString(5)
6768

69+
defaultValueA := fmt.Sprintf("value-%s-a", rString)
70+
defaultValueB := fmt.Sprintf("value-%s-b", rString)
6871
resource.Test(t, resource.TestCase{
6972
PreCheck: func() { testAccPreCheck(t) },
7073
ProviderFactories: providerFactories,
7174
CheckDestroy: testAccCheckGitlabGroupVariableDestroy,
7275
Steps: []resource.TestStep{
7376
// Create a group and variables with same keys, different scopes
7477
{
75-
Config: testAccGitlabGroupVariableScopeConfig(rString, "*", "review/*"),
78+
Config: testAccGitlabGroupVariableScopeConfig(rString, "*", "review/*", defaultValueA, defaultValueB),
7679
SkipFunc: isRunningInCE,
7780
Check: resource.ComposeTestCheckFunc(
7881
testAccCheckGitlabGroupVariableExists("gitlab_group_variable.a", &groupVariableA),
7982
testAccCheckGitlabGroupVariableExists("gitlab_group_variable.b", &groupVariableB),
8083
testAccCheckGitlabGroupVariableAttributes(&groupVariableA, &testAccGitlabGroupVariableExpectedAttributes{
8184
Key: fmt.Sprintf("key_%s", rString),
82-
Value: fmt.Sprintf("value-%s-a", rString),
85+
Value: defaultValueA,
8386
EnvironmentScope: "*",
8487
}),
8588
testAccCheckGitlabGroupVariableAttributes(&groupVariableB, &testAccGitlabGroupVariableExpectedAttributes{
8689
Key: fmt.Sprintf("key_%s", rString),
87-
Value: fmt.Sprintf("value-%s-b", rString),
90+
Value: defaultValueB,
8891
EnvironmentScope: "review/*",
8992
}),
9093
),
9194
},
9295
// Change a variable's scope
9396
{
94-
Config: testAccGitlabGroupVariableScopeConfig(rString, "my-new-scope", "review/*"),
97+
Config: testAccGitlabGroupVariableScopeConfig(rString, "my-new-scope", "review/*", defaultValueA, defaultValueB),
9598
SkipFunc: isRunningInCE,
9699
Check: resource.ComposeTestCheckFunc(
97100
testAccCheckGitlabGroupVariableExists("gitlab_group_variable.a", &groupVariableA),
98101
testAccCheckGitlabGroupVariableExists("gitlab_group_variable.b", &groupVariableB),
99102
testAccCheckGitlabGroupVariableAttributes(&groupVariableA, &testAccGitlabGroupVariableExpectedAttributes{
100103
Key: fmt.Sprintf("key_%s", rString),
101-
Value: fmt.Sprintf("value-%s-a", rString),
104+
Value: defaultValueA,
102105
EnvironmentScope: "my-new-scope",
103106
}),
104107
testAccCheckGitlabGroupVariableAttributes(&groupVariableB, &testAccGitlabGroupVariableExpectedAttributes{
105108
Key: fmt.Sprintf("key_%s", rString),
106-
Value: fmt.Sprintf("value-%s-b", rString),
109+
Value: defaultValueB,
107110
EnvironmentScope: "review/*",
108111
}),
109112
),
110113
},
111114
// Change both variables scopes at the same time
112115
{
113-
Config: testAccGitlabGroupVariableScopeConfig(rString, "my-new-new-scope", "review/hello-world"),
116+
Config: testAccGitlabGroupVariableScopeConfig(rString, "my-new-new-scope", "review/hello-world", defaultValueA, defaultValueB),
114117
SkipFunc: isRunningInCE,
115118
Check: resource.ComposeTestCheckFunc(
116119
testAccCheckGitlabGroupVariableExists("gitlab_group_variable.a", &groupVariableA),
117120
testAccCheckGitlabGroupVariableExists("gitlab_group_variable.b", &groupVariableB),
118121
testAccCheckGitlabGroupVariableAttributes(&groupVariableA, &testAccGitlabGroupVariableExpectedAttributes{
119122
Key: fmt.Sprintf("key_%s", rString),
120-
Value: fmt.Sprintf("value-%s-a", rString),
123+
Value: defaultValueA,
121124
EnvironmentScope: "my-new-new-scope",
122125
}),
123126
testAccCheckGitlabGroupVariableAttributes(&groupVariableB, &testAccGitlabGroupVariableExpectedAttributes{
124127
Key: fmt.Sprintf("key_%s", rString),
125-
Value: fmt.Sprintf("value-%s-b", rString),
128+
Value: defaultValueB,
129+
EnvironmentScope: "review/hello-world",
130+
}),
131+
),
132+
},
133+
// Change value of one variable
134+
{
135+
Config: testAccGitlabGroupVariableScopeConfig(rString, "my-new-new-scope", "review/hello-world", defaultValueA, fmt.Sprintf("new-value-for-b-%s", rString)),
136+
// SkipFunc: isRunningInCE,
137+
// NOTE(TF): this test sporadically fails because of this: https://gitlab.com/gitlab-org/gitlab/-/issues/333296
138+
SkipFunc: func() (bool, error) { return true, nil },
139+
Check: resource.ComposeTestCheckFunc(
140+
testAccCheckGitlabGroupVariableExists("gitlab_group_variable.a", &groupVariableA),
141+
testAccCheckGitlabGroupVariableExists("gitlab_group_variable.b", &groupVariableB),
142+
testAccCheckGitlabGroupVariableAttributes(&groupVariableA, &testAccGitlabGroupVariableExpectedAttributes{
143+
Key: fmt.Sprintf("key_%s", rString),
144+
Value: defaultValueA,
145+
EnvironmentScope: "my-new-new-scope",
146+
}),
147+
testAccCheckGitlabGroupVariableAttributes(&groupVariableB, &testAccGitlabGroupVariableExpectedAttributes{
148+
Key: fmt.Sprintf("key_%s", rString),
149+
Value: fmt.Sprintf("new-value-for-b-%s", rString),
126150
EnvironmentScope: "review/hello-world",
127151
}),
128152
),
@@ -146,7 +170,7 @@ func testAccCheckGitlabGroupVariableExists(n string, groupVariable *gitlab.Group
146170
if key == "" {
147171
return fmt.Errorf("No variable key is set")
148172
}
149-
gotVariable, _, err := testGitlabClient.GroupVariables.GetVariable(repoName, key, modifyRequestAddEnvironmentFilter(rs.Primary.Attributes["environment_scope"]))
173+
gotVariable, _, err := testGitlabClient.GroupVariables.GetVariable(repoName, key, withEnvironmentScopeFilter(context.Background(), rs.Primary.Attributes["environment_scope"]))
150174
if err != nil {
151175
return err
152176
}
@@ -245,7 +269,7 @@ resource "gitlab_group_variable" "foo" {
245269
`, rString, rString, rString, rString)
246270
}
247271

248-
func testAccGitlabGroupVariableScopeConfig(rString, scopeA, scopeB string) string {
272+
func testAccGitlabGroupVariableScopeConfig(rString, scopeA, scopeB string, valueA, valueB string) string {
249273
return fmt.Sprintf(`
250274
resource "gitlab_group" "foo" {
251275
name = "foo%v"
@@ -255,15 +279,15 @@ resource "gitlab_group" "foo" {
255279
resource "gitlab_group_variable" "a" {
256280
group = "${gitlab_group.foo.id}"
257281
key = "key_%s"
258-
value = "value-%s-a"
282+
value = "%s"
259283
environment_scope = "%s"
260284
}
261285
262286
resource "gitlab_group_variable" "b" {
263287
group = "${gitlab_group.foo.id}"
264288
key = "key_%s"
265-
value = "value-%s-b"
289+
value = "%s"
266290
environment_scope = "%s"
267291
}
268-
`, rString, rString, rString, rString, scopeA, rString, rString, scopeB)
292+
`, rString, rString, rString, valueA, scopeA, rString, valueB, scopeB)
269293
}

‎internal/provider/resource_gitlab_project_variable.go‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@ import (
55
"errors"
66
"log"
77
"net/http"
8-
"net/url"
98
"strings"
109

11-
retryablehttp "github.com/hashicorp/go-retryablehttp"
1210
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
1311
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1412
gitlab "github.com/xanzy/go-gitlab"
@@ -230,19 +228,6 @@ func isInvalidValueError(err error) bool {
230228
strings.Contains(httpErr.Message, "invalid")
231229
}
232230

233-
func withEnvironmentScopeFilter(ctx context.Context, environmentScope string) gitlab.RequestOptionFunc {
234-
return func(req *retryablehttp.Request) error {
235-
*req = *req.WithContext(ctx)
236-
query, err := url.ParseQuery(req.Request.URL.RawQuery)
237-
if err != nil {
238-
return err
239-
}
240-
query.Set("filter[environment_scope]", environmentScope)
241-
req.Request.URL.RawQuery = query.Encode()
242-
return nil
243-
}
244-
}
245-
246231
var errProjectVariableNotExist = errors.New("project variable does not exist")
247232

248233
func getProjectVariable(ctx context.Context, client *gitlab.Client, project interface{}, key, environmentScope string) (*gitlab.ProjectVariable, error) {

0 commit comments

Comments
 (0)