Add domain validation to catch invalid names before build/deploy (#3152)

* validate invalid domain name to catch the error instead of building

Signed-off-by: RayyanSeliya <rayyanseliya786@gmail.com>

* fix the linting spacing issues

Signed-off-by: RayyanSeliya <rayyanseliya786@gmail.com>

* goimports

Signed-off-by: RayyanSeliya <rayyanseliya786@gmail.com>

---------

Signed-off-by: RayyanSeliya <rayyanseliya786@gmail.com>

Author: RayyanSeliya

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 != "" {

cmd/deploy_test.go

@@ -2258,3 +2258,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)
+			}
+		})
+	}
+}

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

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
+}

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")
+	}
+}