Merge pull request #380 from armsnyder/371-masked-validation

gitlab_project_variable: friendlier error when masked variable is invalid

Author: armsnyder

gitlab/resource_gitlab_project_variable.go

@@ -1,7 +1,10 @@
 package gitlab
 
 import (
+	"errors"
 	"log"
+	"net/http"
+	"strings"
 
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
 	gitlab "github.com/xanzy/go-gitlab"
@@ -82,7 +85,7 @@ func resourceGitlabProjectVariableCreate(d *schema.ResourceData, meta interface{
 
 	_, _, err := client.ProjectVariables.CreateVariable(project, &options)
 	if err != nil {
-		return err
+		return augmentProjectVariableClientError(d, err)
 	}
 
 	d.SetId(buildTwoPartID(&project, &key))
@@ -102,7 +105,7 @@ func resourceGitlabProjectVariableRead(d *schema.ResourceData, meta interface{})
 
 	v, _, err := client.ProjectVariables.GetVariable(project, key)
 	if err != nil {
-		return err
+		return augmentProjectVariableClientError(d, err)
 	}
 
 	d.Set("key", v.Key)
@@ -140,7 +143,7 @@ func resourceGitlabProjectVariableUpdate(d *schema.ResourceData, meta interface{
 
 	_, _, err := client.ProjectVariables.UpdateVariable(project, key, options)
 	if err != nil {
-		return err
+		return augmentProjectVariableClientError(d, err)
 	}
 
 	return resourceGitlabProjectVariableRead(d, meta)
@@ -153,5 +156,24 @@ func resourceGitlabProjectVariableDelete(d *schema.ResourceData, meta interface{
 	log.Printf("[DEBUG] Delete gitlab project variable %s/%s", project, key)
 
 	_, err := client.ProjectVariables.RemoveVariable(project, key)
+	return augmentProjectVariableClientError(d, err)
+}
+
+func augmentProjectVariableClientError(d *schema.ResourceData, err error) error {
+	// Masked values will commonly error due to their strict requirements, and the error message from the GitLab API is not very informative,
+	// so we return a custom error message in this case.
+	if d.Get("masked").(bool) && isInvalidValueError(err) {
+		log.Printf("[ERROR] %v", err)
+		return errors.New("Invalid value for a masked variable. Check the masked variable requirements: https://docs.gitlab.com/ee/ci/variables/#masked-variable-requirements")
+	}
+
 	return err
 }
+
+func isInvalidValueError(err error) bool {
+	var httpErr *gitlab.ErrorResponse
+	return errors.As(err, &httpErr) &&
+		httpErr.Response.StatusCode == http.StatusBadRequest &&
+		strings.Contains(httpErr.Message, "value") &&
+		strings.Contains(httpErr.Message, "invalid")
+}

gitlab/resource_gitlab_project_variable_test.go

@@ -2,6 +2,7 @@ package gitlab
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/helper/acctest"
@@ -54,6 +55,46 @@ func TestAccGitlabProjectVariable_basic(t *testing.T) {
 					}),
 				),
 			},
+			// Update the project variable to enable "masked" for a value that does not meet masking requirements, and expect an error with no state change.
+			// ref: https://docs.gitlab.com/ce/ci/variables/README.html#masked-variable-requirements
+			{
+				Config: testAccGitlabProjectVariableUpdateConfigMaskedBad(rString),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGitlabProjectVariableExists("gitlab_project_variable.foo", &projectVariable),
+					testAccCheckGitlabProjectVariableAttributes(&projectVariable, &testAccGitlabProjectVariableExpectedAttributes{
+						Key:   fmt.Sprintf("key_%s", rString),
+						Value: fmt.Sprintf("value-%s", rString),
+					}),
+				),
+				ExpectError: regexp.MustCompile(regexp.QuoteMeta(
+					"Invalid value for a masked variable. Check the masked variable requirements: https://docs.gitlab.com/ee/ci/variables/#masked-variable-requirements",
+				)),
+			},
+			// Update the project variable to to enable "masked" and meet masking requirements
+			// ref: https://docs.gitlab.com/ce/ci/variables/README.html#masked-variable-requirements
+			{
+				Config: testAccGitlabProjectVariableUpdateConfigMaskedGood(rString),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGitlabProjectVariableExists("gitlab_project_variable.foo", &projectVariable),
+					testAccCheckGitlabProjectVariableAttributes(&projectVariable, &testAccGitlabProjectVariableExpectedAttributes{
+						Key:    fmt.Sprintf("key_%s", rString),
+						Value:  fmt.Sprintf("value-%s", rString),
+						Masked: true,
+					}),
+				),
+			},
+			// Update the project variable to toggle the options back
+			{
+				Config: testAccGitlabProjectVariableConfig(rString),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGitlabProjectVariableExists("gitlab_project_variable.foo", &projectVariable),
+					testAccCheckGitlabProjectVariableAttributes(&projectVariable, &testAccGitlabProjectVariableExpectedAttributes{
+						Key:       fmt.Sprintf("key_%s", rString),
+						Value:     fmt.Sprintf("value-%s", rString),
+						Protected: false,
+					}),
+				),
+			},
 		},
 	})
 }
@@ -88,6 +129,7 @@ type testAccGitlabProjectVariableExpectedAttributes struct {
 	Key       string
 	Value     string
 	Protected bool
+	Masked    bool
 }
 
 func testAccCheckGitlabProjectVariableAttributes(variable *gitlab.ProjectVariable, want *testAccGitlabProjectVariableExpectedAttributes) resource.TestCheckFunc {
@@ -104,6 +146,10 @@ func testAccCheckGitlabProjectVariableAttributes(variable *gitlab.ProjectVariabl
 			return fmt.Errorf("got protected %t; want %t", variable.Protected, want.Protected)
 		}
 
+		if variable.Masked != want.Masked {
+			return fmt.Errorf("got masked %t; want %t", variable.Masked, want.Masked)
+		}
+
 		return nil
 	}
 }
@@ -171,3 +217,48 @@ resource "gitlab_project_variable" "foo" {
 }
 	`, rString, rString, rString)
 }
+
+func testAccGitlabProjectVariableUpdateConfigMaskedBad(rString string) string {
+	return fmt.Sprintf(`
+resource "gitlab_project" "foo" {
+  name = "foo-%s"
+  description = "Terraform acceptance tests"
+
+  # So that acceptance tests can be run in a gitlab organization
+  # with no billing
+  visibility_level = "public"
+}
+
+resource "gitlab_project_variable" "foo" {
+  project = "${gitlab_project.foo.id}"
+  key = "key_%s"
+  value = <<EOF
+value-%s"
+i am multiline
+EOF
+  variable_type = "env_var"
+  masked = true
+}
+	`, rString, rString, rString)
+}
+
+func testAccGitlabProjectVariableUpdateConfigMaskedGood(rString string) string {
+	return fmt.Sprintf(`
+resource "gitlab_project" "foo" {
+  name = "foo-%s"
+  description = "Terraform acceptance tests"
+
+  # So that acceptance tests can be run in a gitlab organization
+  # with no billing
+  visibility_level = "public"
+}
+
+resource "gitlab_project_variable" "foo" {
+  project = "${gitlab_project.foo.id}"
+  key = "key_%s"
+  value = "value-%s"
+  variable_type = "env_var"
+  masked = true
+}
+	`, rString, rString, rString)
+}