fix: correct URL validation and centralize logic

Author: ssncferreira

provider/app.go

@@ -2,7 +2,7 @@ package provider
 
 import (
 	"context"
-	"net/url"
+	"github.com/coder/terraform-provider-coder/v2/provider/helpers"
 	"regexp"
 
 	"github.com/google/uuid"
@@ -93,15 +93,9 @@ func appResource() *schema.Resource {
 				Description: "A URL to an icon that will display in the dashboard. View built-in " +
 					"icons [here](https://github.com/coder/coder/tree/main/site/static/icon). Use a " +
 					"built-in icon with `\"${data.coder_workspace.me.access_url}/icon/<path>\"`.",
-				ForceNew: true,
-				Optional: true,
-				ValidateFunc: func(i any, s string) ([]string, []error) {
-					_, err := url.Parse(s)
-					if err != nil {
-						return nil, []error{err}
-					}
-					return nil, nil
-				},
+				ForceNew:     true,
+				Optional:     true,
+				ValidateFunc: helpers.ValidateURL,
 			},
 			"slug": {
 				Type: schema.TypeString,

provider/app_test.go

@@ -478,4 +478,61 @@ func TestApp(t *testing.T) {
 			})
 		}
 	})
+
+	t.Run("Icon", func(t *testing.T) {
+		t.Parallel()
+
+		cases := []struct {
+			name        string
+			icon        string
+			expectError *regexp.Regexp
+		}{
+			{
+				name: "Empty",
+				icon: "",
+			},
+			{
+				name: "ValidURL",
+				icon: "/icon/region.svg",
+			},
+			{
+				name:        "InvalidURL",
+				icon:        "/icon%.svg",
+				expectError: regexp.MustCompile("invalid URL escape"),
+			},
+		}
+
+		for _, c := range cases {
+			c := c
+
+			t.Run(c.name, func(t *testing.T) {
+				t.Parallel()
+
+				config := fmt.Sprintf(`
+				provider "coder" {}
+				resource "coder_agent" "dev" {
+					os = "linux"
+					arch = "amd64"
+				}
+				resource "coder_app" "code-server" {
+					agent_id = coder_agent.dev.id
+					slug = "code-server"
+					display_name = "Testing"
+					url = "http://localhost:13337"
+					open_in = "slim-window"
+					icon = "%s"
+				}
+				`, c.icon)
+
+				resource.Test(t, resource.TestCase{
+					ProviderFactories: coderFactory(),
+					IsUnitTest:        true,
+					Steps: []resource.TestStep{{
+						Config:      config,
+						ExpectError: c.expectError,
+					}},
+				})
+			})
+		}
+	})
 }

provider/helpers/validation.go

@@ -0,0 +1,19 @@
+package helpers
+
+import (
+	"fmt"
+	"net/url"
+)
+
+// ValidateURL checks that the given value is a valid URL string.
+// Example: for `icon = "/icon/region.svg"`, value is `/icon/region.svg` and label is `icon`.
+func ValidateURL(value interface{}, label string) ([]string, []error) {
+	val, ok := value.(string)
+	if !ok {
+		return nil, []error{fmt.Errorf("expected %q to be a string", label)}
+	}
+	if _, err := url.Parse(val); err != nil {
+		return nil, []error{err}
+	}
+	return nil, nil
+}

provider/metadata.go

@@ -2,8 +2,7 @@ package provider
 
 import (
 	"context"
-	"net/url"
-
+	"github.com/coder/terraform-provider-coder/v2/provider/helpers"
 	"github.com/google/uuid"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -56,15 +55,9 @@ func metadataResource() *schema.Resource {
 				Description: "A URL to an icon that will display in the dashboard. View built-in " +
 					"icons [here](https://github.com/coder/coder/tree/main/site/static/icon). Use a " +
 					"built-in icon with `\"${data.coder_workspace.me.access_url}/icon/<path>\"`.",
-				ForceNew: true,
-				Optional: true,
-				ValidateFunc: func(i interface{}, s string) ([]string, []error) {
-					_, err := url.Parse(s)
-					if err != nil {
-						return nil, []error{err}
-					}
-					return nil, nil
-				},
+				ForceNew:     true,
+				Optional:     true,
+				ValidateFunc: helpers.ValidateURL,
 			},
 			"daily_cost": {
 				Type: schema.TypeInt,

provider/parameter.go

@@ -6,7 +6,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
-	"net/url"
+	"github.com/coder/terraform-provider-coder/v2/provider/helpers"
 	"os"
 	"regexp"
 	"strconv"
@@ -223,15 +223,9 @@ func parameterDataSource() *schema.Resource {
 				Description: "A URL to an icon that will display in the dashboard. View built-in " +
 					"icons [here](https://github.com/coder/coder/tree/main/site/static/icon). Use a " +
 					"built-in icon with `\"${data.coder_workspace.me.access_url}/icon/<path>\"`.",
-				ForceNew: true,
-				Optional: true,
-				ValidateFunc: func(i interface{}, s string) ([]string, []error) {
-					_, err := url.Parse(s)
-					if err != nil {
-						return nil, []error{err}
-					}
-					return nil, nil
-				},
+				ForceNew:     true,
+				Optional:     true,
+				ValidateFunc: helpers.ValidateURL,
 			},
 			"option": {
 				Type:        schema.TypeList,
@@ -263,15 +257,9 @@ func parameterDataSource() *schema.Resource {
 							Description: "A URL to an icon that will display in the dashboard. View built-in " +
 								"icons [here](https://github.com/coder/coder/tree/main/site/static/icon). Use a " +
 								"built-in icon with `\"${data.coder_workspace.me.access_url}/icon/<path>\"`.",
-							ForceNew: true,
-							Optional: true,
-							ValidateFunc: func(i interface{}, s string) ([]string, []error) {
-								_, err := url.Parse(s)
-								if err != nil {
-									return nil, []error{err}
-								}
-								return nil, nil
-							},
+							ForceNew:     true,
+							Optional:     true,
+							ValidateFunc: helpers.ValidateURL,
 						},
 					},
 				},

provider/parameter_test.go

@@ -234,10 +234,6 @@ func TestParameter(t *testing.T) {
 					icon = "/icon/code.svg"
 					description = "Something!"
 				}
-				option {
-					name = "2"
-					value = "2"
-				}
 			}
 			`,
 		Check: func(state *terraform.ResourceState) {
@@ -665,7 +661,33 @@ data "coder_parameter" "region" {
 			}
 			`,
 		ExpectError: regexp.MustCompile("ephemeral parameter requires the default property"),
-	}} {
+	}, {
+		Name: "InvalidIconURL",
+		Config: `
+			data "coder_parameter" "region" {
+				name = "Region"
+				type = "string"
+				icon = "/icon%.svg"
+			}
+			`,
+		ExpectError: regexp.MustCompile("invalid URL escape"),
+	}, {
+		Name: "OptionInvalidIconURL",
+		Config: `
+			data "coder_parameter" "region" {
+				name = "Region"
+				type = "string"
+				option {
+					name = "1"
+					value = "1"
+					icon = "/icon%.svg"
+					description = "Something!"
+				}
+			}
+			`,
+		ExpectError: regexp.MustCompile("foo"),
+	},
+	} {
 		tc := tc
 		t.Run(tc.Name, func(t *testing.T) {
 			t.Parallel()

provider/provider.go

@@ -2,6 +2,7 @@ package provider
 
 import (
 	"context"
+	"github.com/coder/terraform-provider-coder/v2/provider/helpers"
 	"net/url"
 	"reflect"
 	"strings"
@@ -26,14 +27,8 @@ func New() *schema.Provider {
 				Optional:    true,
 				// The "CODER_AGENT_URL" environment variable is used by default
 				// as the Access URL when generating scripts.
-				DefaultFunc: schema.EnvDefaultFunc("CODER_AGENT_URL", "https://mydeployment.coder.com"),
-				ValidateFunc: func(i interface{}, s string) ([]string, []error) {
-					_, err := url.Parse(s)
-					if err != nil {
-						return nil, []error{err}
-					}
-					return nil, nil
-				},
+				DefaultFunc:  schema.EnvDefaultFunc("CODER_AGENT_URL", "https://mydeployment.coder.com"),
+				ValidateFunc: helpers.ValidateURL,
 			},
 		},
 		ConfigureContextFunc: func(c context.Context, resourceData *schema.ResourceData) (interface{}, diag.Diagnostics) {