fixup

Signed-off-by: Lance Ball <lball@redhat.com>

cmd/config_labels.go

@@ -162,14 +162,14 @@ func runAddLabelsPrompt(ctx context.Context, f fn.Function) (err error) {
 				Name:   "name",
 				Prompt: &survey.Input{Message: "Please specify the label name:"},
 				Validate: func(val interface{}) error {
-					return utils.ValidateLabel(val.(string), "")
+					return utils.ValidateLabelName(val.(string))
 				},
 			},
 			{
 				Name:   "value",
 				Prompt: &survey.Input{Message: "Please specify the label value:"},
 				Validate: func(val interface{}) error {
-					return utils.ValidateLabel("", val.(string))
+					return utils.ValidateLabelValue(val.(string))
 				}},
 		}
 		answers := struct {
@@ -195,14 +195,14 @@ func runAddLabelsPrompt(ctx context.Context, f fn.Function) (err error) {
 				Name:   "name",
 				Prompt: &survey.Input{Message: "Please specify the label name:"},
 				Validate: func(val interface{}) error {
-					return utils.ValidateLabel(val.(string), "")
+					return utils.ValidateLabelName(val.(string))
 				},
 			},
 			{
 				Name:   "value",
 				Prompt: &survey.Input{Message: "Please specify the local environment variable:"},
 				Validate: func(val interface{}) error {
-					return utils.ValidateLabel("", val.(string))
+					return utils.ValidateLabelValue(val.(string))
 				},
 			},
 		}

config.go

@@ -351,7 +351,9 @@ func ValidateLabels(labels Pairs) (errors []string) {
 			errors = append(errors, fmt.Sprintf("label entry #%d is missing value field, only name '%s' is set", i, *label.Name))
 		} else {
 
-			if err := utils.ValidateLabel(*label.Name, *label.Value); err != nil {
+			if err := utils.ValidateLabelName(*label.Name); err != nil {
+				errors = append(errors, fmt.Sprintf("label entry #%d has invalid name or value set: %q %q; %s", i, *label.Name, *label.Value, err.Error()))
+			} else if err := utils.ValidateLabelValue(*label.Value); err != nil {
 				errors = append(errors, fmt.Sprintf("label entry #%d has invalid name or value set: %q %q; %s", i, *label.Name, *label.Value, err.Error()))
 			}
 
@@ -362,6 +364,12 @@ func ValidateLabels(labels Pairs) (errors []string) {
 						fmt.Sprintf(
 							"label entry #%d with name '%s' has invalid value field set, it has '%s', but allowed is only '{{ env:MY_ENV }}'",
 							i, *label.Name, *label.Value))
+				} else {
+					match := regLocalEnv.FindStringSubmatch(*label.Value)
+					value := os.Getenv(match[1])
+					if err := utils.ValidateLabelValue(value); err != nil {
+						errors = append(errors, fmt.Sprintf("label entry #%d with name '%s' has invalid value when the environment is evaluated: '%s'", i, *label.Name, match[1]))
+					}
 				}
 			}
 		}

config_test.go

@@ -3,6 +3,7 @@
 package function
 
 import (
+	"os"
 	"testing"
 
 	"knative.dev/pkg/ptr"
@@ -551,6 +552,12 @@ func Test_validateLabels(t *testing.T) {
 	valueLocalEnvIncorrect2 := "{{ MY_ENV }}"
 	valueLocalEnvIncorrect3 := "{{env:MY_ENV}}foo"
 
+	os.Setenv("BAD_EXAMPLE", ":invalid")
+	valueLocalEnvIncorrect4 := "{{env:BAD_EXAMPLE}}"
+
+	os.Setenv("GOOD_EXAMPLE", "valid")
+	valueLocalEnv4 := "{{env:GOOD_EXAMPLE}}"
+
 	tests := []struct {
 		name   string
 		labels Pairs
@@ -690,6 +697,25 @@ func Test_validateLabels(t *testing.T) {
 			},
 			3,
 		},
+		{
+			"correct entry - good environment variable value",
+			Pairs{
+				Pair{
+					Name:  &name,
+					Value: &valueLocalEnv4,
+				},
+			},
+			0,
+		}, {
+			"incorrect entry - bad environment variable value",
+			Pairs{
+				Pair{
+					Name:  &name,
+					Value: &valueLocalEnvIncorrect4,
+				},
+			},
+			1,
+		},
 		{
 			"correct entry - all combinations",
 			Pairs{

utils/names.go

@@ -44,26 +44,34 @@ func ValidateEnvVarName(name string) error {
 	return nil
 }
 
-// ValidateLabel validates that the input name is a valid Kubernetes key.
+// ValidateLabelName validates that the input name is a valid Kubernetes key.
 // Valid label names have two segments: an optional prefix and name, separated by a slash (/).
 // The name segment is required and must be 63 characters or less, beginning and ending with
 // an alphanumeric character ([a-z0-9A-Z]) with dashes (-), underscores (_), dots (.), and
 // alphanumerics between. The prefix is optional. If specified, the prefix must be a DNS subdomain:
 // a series of DNS labels separated by dots (.), not longer than 253 characters in total, followed
 // by a slash (/).
+func ValidateLabelName(name string) error {
+	errs := validation.IsQualifiedName(name)
+	if len(errs) > 0 {
+		return ErrInvalidLabel(errors.New(strings.Join(errs, "")))
+	}
+	return nil
+}
+
+// ValidateLabelValue ensures that the input is a Kubernetes label value
 // Valid label values must be 63 characters or less (can be empty),
 // unless empty, must begin and end with an alphanumeric character ([a-z0-9A-Z]),
 // could contain dashes (-), underscores (_), dots (.), and alphanumerics between.
 // Label values may also come from the environment and therefore, could be enclosed with {{}}
 // Treat this as a special case.
-func ValidateLabel(name, value string) error {
-	errs := validation.IsQualifiedName(name)
+func ValidateLabelValue(value string) error {
+	var errs []string
 	if !strings.HasPrefix(value, "{{") {
 		errs = append(errs, validation.IsValidLabelValue(value)...)
 	}
 	if len(errs) > 0 {
 		return ErrInvalidLabel(errors.New(strings.Join(errs, "")))
 	}
-
 	return nil
 }

utils/names_test.go

@@ -2,7 +2,9 @@
 
 package utils
 
-import "testing"
+import (
+	"testing"
+)
 
 // TestValidateFunctionName tests that only correct function names are accepted
 func TestValidateFunctionName(t *testing.T) {
@@ -62,3 +64,66 @@ func TestValidateEnvVarName(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateLabelName(t *testing.T) {
+	cases := []struct {
+		In    string
+		Valid bool
+	}{
+		{"", false},
+		{"*", false},
+		{"example", true},
+		{"example-com", true},
+		{"example.com", true},
+		{"-example-com", false},
+		{"example-com-", false},
+		{"Example", true},
+		{"EXAMPLE", true},
+		{"example.com/example", true},
+		{";Example", false},
+		{":Example", false},
+		{",Example", false},
+	}
+
+	for _, c := range cases {
+		err := ValidateLabelName(c.In)
+		if err != nil && c.Valid {
+			t.Fatalf("Unexpected error: %v, for '%v'", err, c.In)
+		}
+		if err == nil && !c.Valid {
+			t.Fatalf("Expected error for invalid entry: %v", c.In)
+		}
+	}
+}
+
+func TestValidateLabelValue(t *testing.T) {
+	cases := []struct {
+		In    string
+		Valid bool
+	}{
+		{"", true},
+		{"*", false},
+		{"example", true},
+		{"example-com", true},
+		{"example.com", true},
+		{"-example-com", false},
+		{"example-com-", false},
+		{"Example", true},
+		{"EXAMPLE", true},
+		{"example.com/example", false},
+		{";Example", false},
+		{":Example", false},
+		{",Example", false},
+		{"{{env.EXAMPLE}}", true},
+	}
+
+	for _, c := range cases {
+		err := ValidateLabelValue(c.In)
+		if err != nil && c.Valid {
+			t.Fatalf("Unexpected error: %v, for '%v'", err, c.In)
+		}
+		if err == nil && !c.Valid {
+			t.Fatalf("Expected error for invalid entry: %v", c.In)
+		}
+	}
+}