Skip to content

Commit f7cb0ea

Browse files
timofurrertimofurrer
committed
resource/gitlab_group_variable: better error message for invalid masked variable values. Closes #371
1 parent 7799be7 commit f7cb0ea

File tree

4 files changed

+124
-32
lines changed

4 files changed

+124
-32
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_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)