diff --git a/provider/app.go b/provider/app.go index adbbf0e7..52996a72 100644 --- a/provider/app.go +++ b/provider/app.go @@ -2,13 +2,14 @@ package provider import ( "context" - "net/url" "regexp" "github.com/google/uuid" "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + + "github.com/coder/terraform-provider-coder/v2/provider/helpers" ) var ( @@ -93,15 +94,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/\"`.", - 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, diff --git a/provider/app_test.go b/provider/app_test.go index aeb42d08..2b9a5580 100644 --- a/provider/app_test.go +++ b/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, + }}, + }) + }) + } + }) } diff --git a/provider/helpers/validation.go b/provider/helpers/validation.go new file mode 100644 index 00000000..9cc21b89 --- /dev/null +++ b/provider/helpers/validation.go @@ -0,0 +1,22 @@ +package helpers + +import ( + "fmt" + "net/url" +) + +// ValidateURL validates that value is a valid URL string. +// Accepts empty strings, local file paths, file:// URLs, and http/https URLs. +// Example: for `icon = "/icon/region.svg"`, value is `/icon/region.svg` and label is `icon`. +func ValidateURL(value any, 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 +} diff --git a/provider/helpers/validation_test.go b/provider/helpers/validation_test.go new file mode 100644 index 00000000..557bae41 --- /dev/null +++ b/provider/helpers/validation_test.go @@ -0,0 +1,151 @@ +package helpers + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestValidateURL(t *testing.T) { + tests := []struct { + name string + value any + label string + expectError bool + errorContains string + }{ + // Valid cases + { + name: "empty string", + value: "", + label: "url", + expectError: false, + }, + { + name: "valid http URL", + value: "http://example.com", + label: "url", + expectError: false, + }, + { + name: "valid https URL", + value: "https://example.com/path", + label: "url", + expectError: false, + }, + { + name: "absolute file path", + value: "/path/to/file", + label: "url", + expectError: false, + }, + { + name: "relative file path", + value: "./file.txt", + label: "url", + expectError: false, + }, + { + name: "relative path up directory", + value: "../config.json", + label: "url", + expectError: false, + }, + { + name: "simple filename", + value: "file.txt", + label: "url", + expectError: false, + }, + { + name: "URL with query params", + value: "https://example.com/search?q=test", + label: "url", + expectError: false, + }, + { + name: "URL with fragment", + value: "https://example.com/page#section", + label: "url", + expectError: false, + }, + + // Various URL schemes that url.Parse accepts + { + name: "file URL scheme", + value: "file:///path/to/file", + label: "url", + expectError: false, + }, + { + name: "ftp scheme", + value: "ftp://files.example.com/file.txt", + label: "url", + expectError: false, + }, + { + name: "mailto scheme", + value: "mailto:user@example.com", + label: "url", + expectError: false, + }, + { + name: "tel scheme", + value: "tel:+1234567890", + label: "url", + expectError: false, + }, + { + name: "data scheme", + value: "data:text/plain;base64,SGVsbG8=", + label: "url", + expectError: false, + }, + + // Invalid cases + { + name: "non-string type - int", + value: 123, + label: "url", + expectError: true, + errorContains: "expected \"url\" to be a string", + }, + { + name: "non-string type - nil", + value: nil, + label: "config_url", + expectError: true, + errorContains: "expected \"config_url\" to be a string", + }, + { + name: "invalid URL with spaces", + value: "http://example .com", + label: "url", + expectError: true, + errorContains: "invalid character", + }, + { + name: "malformed URL", + value: "http://[::1:80", + label: "endpoint", + expectError: true, + errorContains: "missing ']'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + warnings, errors := ValidateURL(tt.value, tt.label) + + if tt.expectError { + require.Len(t, errors, 1, "expected an error but got none") + require.Contains(t, errors[0].Error(), tt.errorContains) + } else { + require.Empty(t, errors, "expected no errors but got: %v", errors) + } + + // Should always return nil for warnings + require.Nil(t, warnings, "expected warnings to be nil but got: %v", warnings) + }) + } +} diff --git a/provider/metadata.go b/provider/metadata.go index 535c700c..5ed6d478 100644 --- a/provider/metadata.go +++ b/provider/metadata.go @@ -2,12 +2,13 @@ package provider import ( "context" - "net/url" "github.com/google/uuid" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "golang.org/x/xerrors" + + "github.com/coder/terraform-provider-coder/v2/provider/helpers" ) func metadataResource() *schema.Resource { @@ -56,15 +57,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/\"`.", - 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, diff --git a/provider/parameter.go b/provider/parameter.go index c8284da1..2d3ea413 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -6,7 +6,6 @@ import ( "encoding/hex" "encoding/json" "fmt" - "net/url" "os" "regexp" "strconv" @@ -19,6 +18,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/mitchellh/mapstructure" "golang.org/x/xerrors" + + "github.com/coder/terraform-provider-coder/v2/provider/helpers" ) var ( @@ -223,15 +224,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/\"`.", - 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 +258,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/\"`.", - 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, }, }, }, diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 9b5e76f1..35f045b9 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -665,7 +665,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("invalid URL escape"), + }, + } { tc := tc t.Run(tc.Name, func(t *testing.T) { t.Parallel() diff --git a/provider/provider.go b/provider/provider.go index 43e3a6ac..a0ef63f9 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -10,6 +10,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "golang.org/x/xerrors" + + "github.com/coder/terraform-provider-coder/v2/provider/helpers" ) type config struct { @@ -26,14 +28,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) {