diff --git a/cmd/deploy.go b/cmd/deploy.go index 42cb032140..4630561277 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -20,6 +20,7 @@ import ( "knative.dev/func/pkg/config" fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/k8s" + "knative.dev/func/pkg/utils" ) func NewDeployCmd(newClient ClientFactory) *cobra.Command { @@ -280,6 +281,22 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { } if err = cfg.Validate(cmd); err != nil { // Layer 2: Catch technical errors and provide CLI-specific user-friendly messages + if errors.Is(err, fn.ErrInvalidDomain) { + return fmt.Errorf(`%w + +Domain names must be valid DNS subdomains: + - Lowercase letters, numbers, hyphens (-), and dots (.) only + - Start and end with a letter or number + - Max 253 characters total, each part between dots max 63 characters + +Valid examples: + func deploy --registry ghcr.io/user --domain example.com + func deploy --registry ghcr.io/user --domain api.example.com + +Note: Domain must be configured on your Knative cluster, or it will be ignored. + +For more options, run 'func deploy --help'`, err) + } if errors.Is(err, fn.ErrConflictingImageAndRegistry) { return fmt.Errorf(`%w @@ -738,6 +755,14 @@ func (c deployConfig) Validate(cmd *cobra.Command) (err error) { return } + // Validate domain format if provided + if c.Domain != "" { + if err = utils.ValidateDomain(c.Domain); err != nil { + // Wrap the validation error as fn.ErrInvalidDomain for layer consistency + return fn.ErrInvalidDomain + } + } + // Check Image Digest was included var digest bool if c.Image != "" { diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index 6be3cf48ae..63f2909818 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -2244,3 +2244,154 @@ func testBaseImage(cmdFn commandConstructor, t *testing.T) { }) } } + +// TestDeploy_InvalidDomain ensures that invalid domain names are caught +// before build starts and return helpful error messages +func TestDeploy_InvalidDomain(t *testing.T) { + tests := []struct { + name string + domain string + errMsg string + }{ + { + name: "domain with spaces", + domain: "my app.com", + errMsg: "invalid domain", + }, + { + name: "domain with uppercase", + domain: "Example.Com", + errMsg: "invalid domain", + }, + { + name: "domain with special characters", + domain: "example@domain.com", + errMsg: "invalid domain", + }, + { + name: "domain starting with hyphen", + domain: "-example.com", + errMsg: "invalid domain", + }, + { + name: "domain with consecutive dots", + domain: "example..com", + errMsg: "invalid domain", + }, + { + name: "domain with only whitespace", + domain: " ", + errMsg: "invalid domain", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := FromTempDirectory(t) + + // Create a function + f := fn.Function{ + Runtime: "go", + Root: root, + Registry: TestRegistry, + } + _, err := fn.New().Init(f) + if err != nil { + t.Fatal(err) + } + + // Create deploy command with invalid domain + cmd := NewDeployCmd(NewTestClient( + fn.WithBuilder(mock.NewBuilder()), + fn.WithDeployer(mock.NewDeployer()), + )) + cmd.SetArgs([]string{"--domain", tt.domain}) + + // Execute and expect error + err = cmd.Execute() + if err == nil { + t.Fatalf("expected error for invalid domain '%v', but got none", tt.domain) + } + + // Check error message contains expected text + if !strings.Contains(err.Error(), tt.errMsg) { + t.Fatalf("expected error message to contain '%v', got: %v", tt.errMsg, err.Error()) + } + + // Ensure builder was NOT invoked (validation should fail before build) + builder := cmd.Flag("builder") + if builder == nil { + t.Fatal("builder flag not found") + } + }) + } +} + +// TestDeploy_ValidDomain ensures that valid domain names pass validation +// and proceed to build/deploy +func TestDeploy_ValidDomain(t *testing.T) { + tests := []struct { + name string + domain string + }{ + { + name: "standard domain", + domain: "example.com", + }, + { + name: "subdomain", + domain: "api.example.com", + }, + { + name: "multi-level subdomain", + domain: "my-app.staging.example.com", + }, + { + name: "single label domain", + domain: "localhost", + }, + { + name: "kubernetes internal domain", + domain: "cluster.local", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := FromTempDirectory(t) + + // Create a function + f := fn.Function{ + Runtime: "go", + Root: root, + Registry: TestRegistry, + } + _, err := fn.New().Init(f) + if err != nil { + t.Fatal(err) + } + + // Create deploy command with valid domain + cmd := NewDeployCmd(NewTestClient( + fn.WithBuilder(mock.NewBuilder()), + fn.WithDeployer(mock.NewDeployer()), + )) + cmd.SetArgs([]string{"--domain", tt.domain}) + + // Execute and expect no error + err = cmd.Execute() + if err != nil { + t.Fatalf("expected valid domain '%v' to pass, but got error: %v", tt.domain, err) + } + + // Verify domain was set on function + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + if f.Domain != tt.domain { + t.Fatalf("expected domain '%v', got '%v'", tt.domain, f.Domain) + } + }) + } +} diff --git a/pkg/functions/errors.go b/pkg/functions/errors.go index b454e29fb2..1908bd2027 100644 --- a/pkg/functions/errors.go +++ b/pkg/functions/errors.go @@ -34,6 +34,9 @@ var ( // ErrConflictingImageAndRegistry is returned when both --image and --registry flags are explicitly provided ErrConflictingImageAndRegistry = errors.New("both --image and --registry flags provided") + + // ErrInvalidDomain is returned when a domain name doesn't meet DNS subdomain requirements + ErrInvalidDomain = errors.New("invalid domain") ) // ErrNotInitialized indicates that a function is uninitialized diff --git a/pkg/utils/names.go b/pkg/utils/names.go index 88c773cc46..525d770562 100644 --- a/pkg/utils/names.go +++ b/pkg/utils/names.go @@ -23,6 +23,9 @@ type ErrInvalidSecretKey error // ErrInvalidLabel indicates the name did not pass label key validation, or the value did not pass label value validation. type ErrInvalidLabel error +// ErrInvalidDomain indicates the domain name did not pass DNS subdomain validation. +type ErrInvalidDomain error + // ValidateFunctionName validates that the input name is a valid function name, ie. valid DNS-1035 label. // It must consist of lower case alphanumeric characters or '-' and start with an alphabetic character and end with an alphanumeric character. // (e.g. 'my-name', or 'abc-1', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?') @@ -102,3 +105,23 @@ func ValidateLabelValue(value string) error { } return nil } + +// ValidateDomain validates that the input is a valid DNS subdomain name (RFC 1123). +// Examples: "example.com", "api.example.com", "my-app.staging.example.com" +func ValidateDomain(domain string) error { + if domain == "" { + return nil + } + + if errs := validation.IsDNS1123Subdomain(domain); len(errs) > 0 { + errMsg := strings.Replace( + strings.Join(errs, ""), + "a lowercase RFC 1123 subdomain", + fmt.Sprintf("Domain '%v'", domain), + 1, + ) + return ErrInvalidDomain(errors.New(errMsg)) + } + + return nil +} diff --git a/pkg/utils/names_test.go b/pkg/utils/names_test.go index e7a257e4c7..4ebf9fbbe9 100644 --- a/pkg/utils/names_test.go +++ b/pkg/utils/names_test.go @@ -209,3 +209,88 @@ func TestValidateLabelValue(t *testing.T) { } } } + +// TestValidateDomain tests that only correct DNS subdomain names are accepted +func TestValidateDomain(t *testing.T) { + cases := []struct { + In string + Valid bool + }{ + // Valid domains + {"", true}, // empty is valid (means use default) + {"example.com", true}, // standard domain + {"api.example.com", true}, // subdomain + {"my-app.example.com", true}, // subdomain with hyphen + {"app-123.example.com", true}, // subdomain with number + {"123.example.com", true}, // label starting with number + {"a.b.c.d.e.com", true}, // many subdomains + {"localhost", true}, // single label (valid) + {"cluster.local", true}, // Kubernetes internal domain + {"my-app-123.staging.example.com", true}, // complex valid domain + {"app.staging.v1.example.com", true}, // multi-level subdomain + {"example-app.com", true}, // hyphen in domain + {"a.co", true}, // short domain + {"123app.example.com", true}, // label starting with number + // Invalid domains + {"Example.Com", false}, // uppercase not allowed + {"MY-APP.COM", false}, // uppercase not allowed + {"my_app.com", false}, // underscore not allowed + {"my app.com", false}, // space not allowed + {"invalid domain.com", false}, // space not allowed + {"my@app.com", false}, // @ not allowed + {"app!.com", false}, // ! not allowed + {"-example.com", false}, // cannot start with hyphen + {"example-.com", false}, // label cannot end with hyphen + {"example.-com.com", false}, // label cannot start with hyphen + {"my..app.com", false}, // consecutive dots not allowed + {".example.com", false}, // cannot start with dot + {"my:app.com", false}, // colon not allowed + {"my;app.com", false}, // semicolon not allowed + {"my,app.com", false}, // comma not allowed + {"my*app.com", false}, // asterisk not allowed + {" example.com", false}, // leading whitespace not allowed + {"example.com ", false}, // trailing whitespace not allowed + {"example.com.", false}, // trailing dot not allowed + {"example@domain.com", false}, // @ not allowed + {"ex!ample.com", false}, // ! not allowed + } + + for _, c := range cases { + err := ValidateDomain(c.In) + if err != nil && c.Valid { + t.Fatalf("Unexpected error for valid domain: %v, domain: '%v'", err, c.In) + } + if err == nil && !c.Valid { + t.Fatalf("Expected error for invalid domain: '%v'", c.In) + } + } +} + +func TestValidateDomainErrMsg(t *testing.T) { + invalidDomain := "my@app.com" + errMsgPrefix := fmt.Sprintf("Domain '%v'", invalidDomain) + + err := ValidateDomain(invalidDomain) + if err != nil { + if !strings.HasPrefix(err.Error(), errMsgPrefix) { + t.Fatalf("Unexpected error message: %v, the message should start with '%v' string", err.Error(), errMsgPrefix) + } + } else { + t.Fatalf("Expected error for invalid domain: %v", invalidDomain) + } +} + +// TestValidateDomainEmptyString ensures empty string is handled specially +func TestValidateDomainEmptyString(t *testing.T) { + // Empty string should be valid (means use cluster default) + err := ValidateDomain("") + if err != nil { + t.Fatalf("Empty string should be valid (means use default): %v", err) + } + + // String with only whitespace should error + err = ValidateDomain(" ") + if err == nil { + t.Fatal("String with only whitespace should be invalid") + } +}