Merge pull request #752 from imjaroiswebdev/TFPROVDEV-77-update-serv-…

…name-validation [TFPROVDEV-77] Update name validation logic for technical services
PagerDuty · Oct 6, 2023 · 96a6a10 · 96a6a10
2 parents daed960 + 8f85b51
commit 96a6a10
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 24 deletions.
diff --git a/pagerduty/resource_pagerduty_escalation_policy.go b/pagerduty/resource_pagerduty_escalation_policy.go
@@ -25,7 +25,7 @@ func resourcePagerDutyEscalationPolicy() *schema.Resource {
 			"name": {
 				Type:             schema.TypeString,
 				Required:         true,
-				ValidateDiagFunc: validateCantBeBlankOrNotPrintableChars,
+				ValidateDiagFunc: validateIsAllowedString(NoNonPrintableCharsOrSpecialChars),
 			},
 			"description": {
 				Type:     schema.TypeString,

diff --git a/pagerduty/resource_pagerduty_escalation_policy_test.go b/pagerduty/resource_pagerduty_escalation_policy_test.go
@@ -113,7 +113,7 @@ func TestAccPagerDutyEscalationPolicy_Basic(t *testing.T) {
 func TestAccPagerDutyEscalationPolicy_FormatValidation(t *testing.T) {
 	username := fmt.Sprintf("tf-%s", acctest.RandString(5))
 	email := fmt.Sprintf("%[email protected]", username)
-	errMessageMatcher := "Name must not be blank, nor contain the characters.*, or any non-printable characters. White spaces at the end are also not allowed."
+	errMessageMatcher := "Name can not be blank, nor contain the characters.*, or any non-printable characters. Trailing white spaces are not allowed either."
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -150,6 +150,12 @@ func TestAccPagerDutyEscalationPolicy_FormatValidation(t *testing.T) {
 				PlanOnly:    true,
 				ExpectError: regexp.MustCompile(errMessageMatcher),
 			},
+			// Name with non printable characters
+			{
+				Config:      testAccCheckPagerDutyEscalationPolicyConfig(username, email, "this name has a non printable\\n character"),
+				PlanOnly:    true,
+				ExpectError: regexp.MustCompile(errMessageMatcher),
+			},
 		},
 	})
 }

diff --git a/pagerduty/resource_pagerduty_service.go b/pagerduty/resource_pagerduty_service.go
@@ -37,7 +37,7 @@ func resourcePagerDutyService() *schema.Resource {
 			"name": {
 				Type:             schema.TypeString,
 				Required:         true,
-				ValidateDiagFunc: validateCantBeBlankOrNotPrintableChars,
+				ValidateDiagFunc: validateIsAllowedString(NoNonPrintableChars),
 			},
 			"html_url": {
 				Type:     schema.TypeString,

diff --git a/pagerduty/resource_pagerduty_service_test.go b/pagerduty/resource_pagerduty_service_test.go
@@ -134,7 +134,7 @@ func TestAccPagerDutyService_FormatValidation(t *testing.T) {
 	username := fmt.Sprintf("tf-%s", acctest.RandString(5))
 	email := fmt.Sprintf("%[email protected]", username)
 	escalationPolicy := fmt.Sprintf("tf-%s", acctest.RandString(5))
-	errMessageMatcher := "Name must not be blank, nor contain the characters.*, or any non-printable characters. White spaces at the end are also not allowed."
+	errMessageMatcher := "Name can not be blank, nor contain non-printable characters. Trailing white spaces are not allowed either."
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -153,12 +153,6 @@ func TestAccPagerDutyService_FormatValidation(t *testing.T) {
 				PlanOnly:    true,
 				ExpectError: regexp.MustCompile(errMessageMatcher),
 			},
-			// Name with & in it
-			{
-				Config:      testAccCheckPagerDutyServiceConfig(username, email, escalationPolicy, "this name has an ampersand (&)"),
-				PlanOnly:    true,
-				ExpectError: regexp.MustCompile(errMessageMatcher),
-			},
 			// Name with one white space at the end
 			{
 				Config:      testAccCheckPagerDutyServiceConfig(username, email, escalationPolicy, "this name has a white space at the end "),
@@ -171,6 +165,12 @@ func TestAccPagerDutyService_FormatValidation(t *testing.T) {
 				PlanOnly:    true,
 				ExpectError: regexp.MustCompile(errMessageMatcher),
 			},
+			// Name with non printable characters
+			{
+				Config:      testAccCheckPagerDutyServiceConfig(username, email, escalationPolicy, "this name has a non printable\\n character"),
+				PlanOnly:    true,
+				ExpectError: regexp.MustCompile(errMessageMatcher),
+			},
 		},
 	})
 }

diff --git a/pagerduty/util.go b/pagerduty/util.go
@@ -6,9 +6,9 @@ import (
 	"log"
 	"math"
 	"reflect"
-	"regexp"
 	"strings"
 	"time"
+	"unicode"
 
 	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
@@ -114,22 +114,65 @@ func validateValueDiagFunc(values []string) schema.SchemaValidateDiagFunc {
 	}
 }
 
-func validateCantBeBlankOrNotPrintableChars(v interface{}, p cty.Path) diag.Diagnostics {
-	var diags diag.Diagnostics
+type StringContentValidationMode int64
 
-	value := v.(string)
-	matcher := regexp.MustCompile(`^$|^[ ]+$|[/\\<>&]|\s+$`)
-	matches := matcher.MatchString(value)
+const (
+	NoContentValidation StringContentValidationMode = iota
+	NoNonPrintableChars
+	NoNonPrintableCharsOrSpecialChars
+)
 
-	if matches {
-		diags = append(diags, diag.Diagnostic{
-			Severity:      diag.Error,
-			Summary:       "Name must not be blank, nor contain the characters '\\', '/', '&', '<', '>', or any non-printable characters. White spaces at the end are also not allowed.",
-			AttributePath: p,
-		})
-	}
+// validateIsAllowedString will always validate if string provided is not empty,
+// neither has trailing white spaces. Additionally the string content validation
+// will be done based on the `mode` set.
+//
+//	mode: NoContentValidation | NoNonPrintableChars | NoNonPrintableCharsOrSpecialChars
+func validateIsAllowedString(mode StringContentValidationMode) schema.SchemaValidateDiagFunc {
+	return func(v interface{}, p cty.Path) diag.Diagnostics {
+		var diags diag.Diagnostics
+
+		fillDiags := func() {
+			summary := "Name can not be blank. Trailing white spaces are not allowed either."
+			switch mode {
+			case NoNonPrintableChars:
+				summary = "Name can not be blank, nor contain non-printable characters. Trailing white spaces are not allowed either."
+			case NoNonPrintableCharsOrSpecialChars:
+				summary = "Name can not be blank, nor contain the characters '\\', '/', '&', '<', '>', or any non-printable characters. Trailing white spaces are not allowed either."
+			}
+			diags = append(diags, diag.Diagnostic{
+				Severity:      diag.Error,
+				Summary:       summary,
+				AttributePath: p,
+			})
+		}
 
-	return diags
+		value := v.(string)
+		if value == "" {
+			fillDiags()
+			return diags
+		}
+
+		for _, char := range value {
+			if (mode == NoNonPrintableChars || mode == NoNonPrintableCharsOrSpecialChars) && !unicode.IsPrint(char) {
+				fillDiags()
+				return diags
+			}
+			if mode == NoNonPrintableCharsOrSpecialChars {
+				switch char {
+				case '\\', '/', '&', '<', '>':
+					fillDiags()
+					return diags
+				}
+			}
+		}
+
+		if strings.HasSuffix(value, " ") {
+			fillDiags()
+			return diags
+		}
+
+		return diags
+	}
 }
 
 // Takes the result of flatmap.Expand for an array of strings