Skip to content

Commit 26a1e00

Browse files
timofurrertimofurrer
authored
Merge pull request #895 from timofurrer/feature/validate-group-variable-mask-371
resource/gitlab_group_variable: better error message for invalid masked variable values. Closes #371
2 parents 7799be7 + e1c2d47 commit 26a1e00

6 files changed

+192
-36
lines changed

‎internal/provider/resource_gitlab_group_variable.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ func resourceGitlabGroupVariableCreate(ctx context.Context, d *schema.ResourceDa
9898

9999
_, _, err := client.GroupVariables.CreateVariable(group, &options, gitlab.WithContext(ctx))
100100
if err != nil {
101-
return diag.FromErr(err)
101+
return augmentVariableClientError(d, err)
102102
}
103103

104104
keyScope := fmt.Sprintf("%s:%s", key, environmentScope)
@@ -137,7 +137,7 @@ func resourceGitlabGroupVariableRead(ctx context.Context, d *schema.ResourceData
137137
d.SetId("")
138138
return nil
139139
}
140-
return diag.FromErr(err)
140+
return augmentVariableClientError(d, err)
141141
}
142142

143143
d.Set("key", v.Key)
@@ -178,7 +178,7 @@ func resourceGitlabGroupVariableUpdate(ctx context.Context, d *schema.ResourceDa
178178
withEnvironmentScopeFilter(ctx, environmentScope),
179179
)
180180
if err != nil {
181-
return diag.FromErr(err)
181+
return augmentVariableClientError(d, err)
182182
}
183183
return resourceGitlabGroupVariableRead(ctx, d, meta)
184184
}
@@ -197,7 +197,7 @@ func resourceGitlabGroupVariableDelete(ctx context.Context, d *schema.ResourceDa
197197
withEnvironmentScopeFilter(ctx, environmentScope),
198198
)
199199
if err != nil {
200-
return diag.FromErr(err)
200+
return augmentVariableClientError(d, err)
201201
}
202202

203203
return nil

‎internal/provider/resource_gitlab_group_variable_test.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package provider
33
import (
44
"context"
55
"fmt"
6+
"regexp"
67
"testing"
78

89
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
@@ -58,6 +59,49 @@ func TestAccGitlabGroupVariable_basic(t *testing.T) {
5859
}),
5960
),
6061
},
62+
// Update the group variable to enable "masked" for a value that does not meet masking requirements, and expect an error with no state change.
63+
// ref: https://docs.gitlab.com/ce/ci/variables/README.html#masked-variable-requirements
64+
{
65+
Config: testAccGitlabGroupVariableUpdateConfigMaskedBad(rString),
66+
Check: resource.ComposeTestCheckFunc(
67+
testAccCheckGitlabGroupVariableExists("gitlab_group_variable.foo", &groupVariable),
68+
testAccCheckGitlabGroupVariableAttributes(&groupVariable, &testAccGitlabGroupVariableExpectedAttributes{
69+
Key: fmt.Sprintf("key_%s", rString),
70+
Value: fmt.Sprintf("value-%s", rString),
71+
EnvironmentScope: "*",
72+
}),
73+
),
74+
ExpectError: regexp.MustCompile(regexp.QuoteMeta(
75+
"Invalid value for a masked variable. Check the masked variable requirements: https://docs.gitlab.com/ee/ci/variables/#masked-variable-requirements",
76+
)),
77+
},
78+
// Update the group variable to to enable "masked" and meet masking requirements
79+
// ref: https://docs.gitlab.com/ce/ci/variables/README.html#masked-variable-requirements
80+
{
81+
Config: testAccGitlabGroupVariableUpdateConfigMaskedGood(rString),
82+
Check: resource.ComposeTestCheckFunc(
83+
testAccCheckGitlabGroupVariableExists("gitlab_group_variable.foo", &groupVariable),
84+
testAccCheckGitlabGroupVariableAttributes(&groupVariable, &testAccGitlabGroupVariableExpectedAttributes{
85+
Key: fmt.Sprintf("key_%s", rString),
86+
Value: fmt.Sprintf("value-%s", rString),
87+
EnvironmentScope: "*",
88+
Masked: true,
89+
}),
90+
),
91+
},
92+
// Update the group variable to toggle the options back
93+
{
94+
Config: testAccGitlabGroupVariableConfig(rString),
95+
Check: resource.ComposeTestCheckFunc(
96+
testAccCheckGitlabGroupVariableExists("gitlab_group_variable.foo", &groupVariable),
97+
testAccCheckGitlabGroupVariableAttributes(&groupVariable, &testAccGitlabGroupVariableExpectedAttributes{
98+
Key: fmt.Sprintf("key_%s", rString),
99+
Value: fmt.Sprintf("value-%s", rString),
100+
EnvironmentScope: "*",
101+
Protected: false,
102+
}),
103+
),
104+
},
61105
},
62106
})
63107
}
@@ -291,3 +335,40 @@ resource "gitlab_group_variable" "b" {
291335
}
292336
`, rString, rString, rString, valueA, scopeA, rString, valueB, scopeB)
293337
}
338+
339+
func testAccGitlabGroupVariableUpdateConfigMaskedBad(rString string) string {
340+
return fmt.Sprintf(`
341+
resource "gitlab_group" "foo" {
342+
name = "foo%v"
343+
path = "foo%v"
344+
}
345+
346+
resource "gitlab_group_variable" "foo" {
347+
group = "${gitlab_group.foo.id}"
348+
key = "key_%s"
349+
value = <<EOF
350+
value-%s"
351+
i am multiline
352+
EOF
353+
variable_type = "env_var"
354+
masked = true
355+
}
356+
`, rString, rString, rString, rString)
357+
}
358+
359+
func testAccGitlabGroupVariableUpdateConfigMaskedGood(rString string) string {
360+
return fmt.Sprintf(`
361+
resource "gitlab_group" "foo" {
362+
name = "foo%v"
363+
path = "foo%v"
364+
}
365+
366+
resource "gitlab_group_variable" "foo" {
367+
group = "${gitlab_group.foo.id}"
368+
key = "key_%s"
369+
value = "value-%s"
370+
variable_type = "env_var"
371+
masked = true
372+
}
373+
`, rString, rString, rString, rString)
374+
}

‎internal/provider/resource_gitlab_instance_variable.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func resourceGitlabInstanceVariableCreate(ctx context.Context, d *schema.Resourc
8181

8282
_, _, err := client.InstanceVariables.CreateVariable(&options, gitlab.WithContext(ctx))
8383
if err != nil {
84-
return diag.FromErr(err)
84+
return augmentVariableClientError(d, err)
8585
}
8686

8787
d.SetId(key)
@@ -103,7 +103,7 @@ func resourceGitlabInstanceVariableRead(ctx context.Context, d *schema.ResourceD
103103
d.SetId("")
104104
return nil
105105
}
106-
return diag.FromErr(err)
106+
return augmentVariableClientError(d, err)
107107
}
108108

109109
d.Set("key", v.Key)
@@ -133,7 +133,7 @@ func resourceGitlabInstanceVariableUpdate(ctx context.Context, d *schema.Resourc
133133

134134
_, _, err := client.InstanceVariables.UpdateVariable(key, options, gitlab.WithContext(ctx))
135135
if err != nil {
136-
return diag.FromErr(err)
136+
return augmentVariableClientError(d, err)
137137
}
138138
return resourceGitlabInstanceVariableRead(ctx, d, meta)
139139
}
@@ -145,7 +145,7 @@ func resourceGitlabInstanceVariableDelete(ctx context.Context, d *schema.Resourc
145145

146146
_, err := client.InstanceVariables.RemoveVariable(key, gitlab.WithContext(ctx))
147147
if err != nil {
148-
return diag.FromErr(err)
148+
return augmentVariableClientError(d, err)
149149
}
150150

151151
return nil

‎internal/provider/resource_gitlab_instance_variable_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package provider
22

33
import (
44
"fmt"
5+
"regexp"
56
"testing"
67

78
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
@@ -54,6 +55,46 @@ func TestAccGitlabInstanceVariable_basic(t *testing.T) {
5455
}),
5556
),
5657
},
58+
// Update the instance variable to enable "masked" for a value that does not meet masking requirements, and expect an error with no state change.
59+
// ref: https://docs.gitlab.com/ce/ci/variables/README.html#masked-variable-requirements
60+
{
61+
Config: testAccGitlabInstanceVariableUpdateConfigMaskedBad(rString),
62+
Check: resource.ComposeTestCheckFunc(
63+
testAccCheckGitlabInstanceVariableExists("gitlab_instance_variable.foo", &instanceVariable),
64+
testAccCheckGitlabInstanceVariableAttributes(&instanceVariable, &testAccGitlabInstanceVariableExpectedAttributes{
65+
Key: fmt.Sprintf("key_%s", rString),
66+
Value: fmt.Sprintf("value-%s", rString),
67+
}),
68+
),
69+
ExpectError: regexp.MustCompile(regexp.QuoteMeta(
70+
"Invalid value for a masked variable. Check the masked variable requirements: https://docs.gitlab.com/ee/ci/variables/#masked-variable-requirements",
71+
)),
72+
},
73+
// Update the instance variable to to enable "masked" and meet masking requirements
74+
// ref: https://docs.gitlab.com/ce/ci/variables/README.html#masked-variable-requirements
75+
{
76+
Config: testAccGitlabInstanceVariableUpdateConfigMaskedGood(rString),
77+
Check: resource.ComposeTestCheckFunc(
78+
testAccCheckGitlabInstanceVariableExists("gitlab_instance_variable.foo", &instanceVariable),
79+
testAccCheckGitlabInstanceVariableAttributes(&instanceVariable, &testAccGitlabInstanceVariableExpectedAttributes{
80+
Key: fmt.Sprintf("key_%s", rString),
81+
Value: fmt.Sprintf("value-%s", rString),
82+
Masked: true,
83+
}),
84+
),
85+
},
86+
// Update the instance variable to toggle the options back
87+
{
88+
Config: testAccGitlabInstanceVariableConfig(rString),
89+
Check: resource.ComposeTestCheckFunc(
90+
testAccCheckGitlabInstanceVariableExists("gitlab_instance_variable.foo", &instanceVariable),
91+
testAccCheckGitlabInstanceVariableAttributes(&instanceVariable, &testAccGitlabInstanceVariableExpectedAttributes{
92+
Key: fmt.Sprintf("key_%s", rString),
93+
Value: fmt.Sprintf("value-%s", rString),
94+
Protected: false,
95+
}),
96+
),
97+
},
5798
},
5899
})
59100
}
@@ -129,3 +170,26 @@ resource "gitlab_instance_variable" "foo" {
129170
}
130171
`, rString, rString)
131172
}
173+
174+
func testAccGitlabInstanceVariableUpdateConfigMaskedBad(rString string) string {
175+
return fmt.Sprintf(`
176+
resource "gitlab_instance_variable" "foo" {
177+
key = "key_%s"
178+
value = <<EOF
179+
value-%s"
180+
i am multiline
181+
EOF
182+
masked = true
183+
}
184+
`, rString, rString)
185+
}
186+
187+
func testAccGitlabInstanceVariableUpdateConfigMaskedGood(rString string) string {
188+
return fmt.Sprintf(`
189+
resource "gitlab_instance_variable" "foo" {
190+
key = "key_%s"
191+
value = "value-%s"
192+
masked = true
193+
}
194+
`, rString, rString)
195+
}

‎internal/provider/resource_gitlab_project_variable.go

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"errors"
66
"log"
7-
"net/http"
87
"strings"
98

109
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
@@ -107,7 +106,7 @@ func resourceGitlabProjectVariableCreate(ctx context.Context, d *schema.Resource
107106

108107
_, _, err := client.ProjectVariables.CreateVariable(project, &options, gitlab.WithContext(ctx))
109108
if err != nil {
110-
return augmentProjectVariableClientError(d, err)
109+
return augmentVariableClientError(d, err)
111110
}
112111

113112
d.SetId(id)
@@ -149,7 +148,7 @@ func resourceGitlabProjectVariableRead(ctx context.Context, d *schema.ResourceDa
149148
d.SetId("")
150149
return nil
151150
}
152-
return augmentProjectVariableClientError(d, err)
151+
return augmentVariableClientError(d, err)
153152
}
154153

155154
d.Set("key", v.Key)
@@ -184,7 +183,7 @@ func resourceGitlabProjectVariableUpdate(ctx context.Context, d *schema.Resource
184183

185184
_, _, err := client.ProjectVariables.UpdateVariable(project, key, options, withEnvironmentScopeFilter(ctx, environmentScope))
186185
if err != nil {
187-
return augmentProjectVariableClientError(d, err)
186+
return augmentVariableClientError(d, err)
188187
}
189188

190189
return resourceGitlabProjectVariableRead(ctx, d, meta)
@@ -202,30 +201,7 @@ func resourceGitlabProjectVariableDelete(ctx context.Context, d *schema.Resource
202201
// destroying or updating scoped variables.
203202
// ref: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39209
204203
_, err := client.ProjectVariables.RemoveVariable(project, key, nil, withEnvironmentScopeFilter(ctx, environmentScope))
205-
return augmentProjectVariableClientError(d, err)
206-
}
207-
208-
func augmentProjectVariableClientError(d *schema.ResourceData, err error) diag.Diagnostics {
209-
// Masked values will commonly error due to their strict requirements, and the error message from the GitLab API is not very informative,
210-
// so we return a custom error message in this case.
211-
if d.Get("masked").(bool) && isInvalidValueError(err) {
212-
log.Printf("[ERROR] %v", err)
213-
return diag.Errorf("Invalid value for a masked variable. Check the masked variable requirements: https://docs.gitlab.com/ee/ci/variables/#masked-variable-requirements")
214-
}
215-
216-
if err != nil {
217-
return diag.FromErr(err)
218-
}
219-
220-
return nil
221-
}
222-
223-
func isInvalidValueError(err error) bool {
224-
var httpErr *gitlab.ErrorResponse
225-
return errors.As(err, &httpErr) &&
226-
httpErr.Response.StatusCode == http.StatusBadRequest &&
227-
strings.Contains(httpErr.Message, "value") &&
228-
strings.Contains(httpErr.Message, "invalid")
204+
return augmentVariableClientError(d, err)
229205
}
230206

231207
var errProjectVariableNotExist = errors.New("project variable does not exist")

‎internal/provider/variable_helpers.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package provider
2+
3+
import (
4+
"errors"
5+
"log"
6+
"net/http"
7+
"strings"
8+
9+
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
10+
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
11+
"github.com/xanzy/go-gitlab"
12+
)
13+
14+
func augmentVariableClientError(d *schema.ResourceData, err error) diag.Diagnostics {
15+
// Masked values will commonly error due to their strict requirements, and the error message from the GitLab API is not very informative,
16+
// so we return a custom error message in this case.
17+
if d.Get("masked").(bool) && isInvalidValueError(err) {
18+
log.Printf("[ERROR] %v", err)
19+
return diag.Errorf("Invalid value for a masked variable. Check the masked variable requirements: https://docs.gitlab.com/ee/ci/variables/#masked-variable-requirements")
20+
}
21+
22+
if err != nil {
23+
return diag.FromErr(err)
24+
}
25+
26+
return nil
27+
}
28+
29+
func isInvalidValueError(err error) bool {
30+
var httpErr *gitlab.ErrorResponse
31+
return errors.As(err, &httpErr) &&
32+
httpErr.Response.StatusCode == http.StatusBadRequest &&
33+
strings.Contains(httpErr.Message, "value") &&
34+
strings.Contains(httpErr.Message, "invalid")
35+
}

0 commit comments

Comments
 (0)