Repository: knative-sandbox/kn-plugin-func
Pull request #623 (2021-11-03 16:08:55)

fix: ConfigMap/Secret key validation
Signed-off-by: Zbynek Roubalik <zroubali@redhat.com>

<!-- Thanks for sending a pull request! -->

# Changes

As per: https://kubernetes.io/docs/concepts/configuration/configmap/#configmap-object
> Each key under the data or the binaryData field must consist of alphanumeric characters, -, _ or .

Our validation only allowed alphanumeric and underscore before, fixing this.


codecov[bot]: # [Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/623?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) Report
> Merging [#623](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/623?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (556b471) into [main](https://codecov.io/gh/knative-sandbox/kn-plugin-func/commit/ac9de9dfc68969ecb825c9ee2a92aa9918ae5ea7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (ac9de9d) will **decrease** coverage by `0.11%`.
> The diff coverage is `0.00%`.

[![Impacted file tree graph](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/623/graphs/tree.svg?width=650&height=150&src=pr&token=da1B9bMsQ9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/623?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)

```diff
@@            Coverage Diff             @@
##             main     #623      +/-   ##
==========================================
- Coverage   36.22%   36.11%   -0.12%     
==========================================
  Files          38       38              
  Lines        3834     3846      +12     
==========================================
  Hits         1389     1389              
- Misses       2261     2273      +12     
  Partials      184      184              
```


| [Impacted Files](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/623?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) | Coverage Δ | |
|---|---|---|
| [cmd/config\_envs.go](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/623/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y21kL2NvbmZpZ19lbnZzLmdv) | `2.57% <0.00%> (-0.07%)` | :arrow_down: |
| [config.go](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/623/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-Y29uZmlnLmdv) | `61.82% <ø> (ø)` | |
| [utils/names.go](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/623/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-dXRpbHMvbmFtZXMuZ28=) | `72.72% <0.00%> (-27.28%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/623?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/623?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Last update [ac9de9d...556b471](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/623?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).


Patch:
diff --git a/cmd/config_envs.go b/cmd/config_envs.go
index 74293f13..fbf20fde 100644
--- a/cmd/config_envs.go
+++ b/cmd/config_envs.go
@@ -281,9 +281,11 @@ func runAddEnvsPrompt(ctx context.Context, f fn.Function) (err error) {
 				},
 			},
 			{
-				Name:     "key",
-				Prompt:   &survey.Input{Message: "Please specify a key from the selected ConfigMap:"},
-				Validate: survey.Required,
+				Name:   "key",
+				Prompt: &survey.Input{Message: "Please specify a key from the selected ConfigMap:"},
+				Validate: func(val interface{}) error {
+					return utils.ValidateConfigMapKey(val.(string))
+				},
 			},
 		}
 		answers := struct {
@@ -339,9 +341,11 @@ func runAddEnvsPrompt(ctx context.Context, f fn.Function) (err error) {
 				},
 			},
 			{
-				Name:     "key",
-				Prompt:   &survey.Input{Message: "Please specify a key from the selected Secret:"},
-				Validate: survey.Required,
+				Name:   "key",
+				Prompt: &survey.Input{Message: "Please specify a key from the selected Secret:"},
+				Validate: func(val interface{}) error {
+					return utils.ValidateSecretKey(val.(string))
+				},
 			},
 		}
 		answers := struct {
diff --git a/config.go b/config.go
index 911603fc..11624cfa 100644
--- a/config.go
+++ b/config.go
@@ -19,9 +19,9 @@ const ConfigFile = "func.yaml"
 
 var (
 	regWholeSecret      = regexp.MustCompile(`^{{\s*secret:((?:\w|['-]\w)+)\s*}}$`)
-	regKeyFromSecret    = regexp.MustCompile(`^{{\s*secret:((?:\w|['-]\w)+):(\w+)\s*}}$`)
+	regKeyFromSecret    = regexp.MustCompile(`^{{\s*secret:((?:\w|['-]\w)+):([-._a-zA-Z0-9]+)\s*}}$`)
 	regWholeConfigMap   = regexp.MustCompile(`^{{\s*configMap:((?:\w|['-]\w)+)\s*}}$`)
-	regKeyFromConfigMap = regexp.MustCompile(`^{{\s*configMap:((?:\w|['-]\w)+):(\w+)\s*}}$`)
+	regKeyFromConfigMap = regexp.MustCompile(`^{{\s*configMap:((?:\w|['-]\w)+):([-._a-zA-Z0-9]+)\s*}}$`)
 	regLocalEnv         = regexp.MustCompile(`^{{\s*env:(\w+)\s*}}$`)
 )
 
diff --git a/utils/names.go b/utils/names.go
index 74962ce6..40114e32 100644
--- a/utils/names.go
+++ b/utils/names.go
@@ -14,6 +14,12 @@ type ErrInvalidFunctionName error
 // ErrInvalidEnvVarName indicates the name did not pass env var name validation.
 type ErrInvalidEnvVarName error
 
+// ErrInvalidConfigMapKey indicates the key specified for ConfigMap did not pass validation.
+type ErrInvalidConfigMapKey error
+
+// ErrInvalidSecretKey indicates the key specified for ConfigMap did not pass validation.
+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
 
@@ -36,7 +42,7 @@ func ValidateFunctionName(name string) error {
 
 // ValidateEnvVarName validatest that the input name is a valid Kubernetes Environmet Variable name.
 // It must  must consist of alphabetic characters, digits, '_', '-', or '.', and must not start with a digit
-// (e.g. 'my.env-name',  or 'MY_ENV.NAME',  or 'MyEnvName1', regex used for validation is '[-._a-zA-Z][-._a-zA-Z0-9]*'))
+// (e.g. 'my.env-name',  or 'MY_ENV.NAME',  or 'MyEnvName1', regex used for validation is '[-._a-zA-Z][-._a-zA-Z0-9]*'
 func ValidateEnvVarName(name string) error {
 	if errs := validation.IsEnvVarName(name); len(errs) > 0 {
 		return ErrInvalidEnvVarName(errors.New(strings.Join(errs, "")))
@@ -45,6 +51,26 @@ func ValidateEnvVarName(name string) error {
 	return nil
 }
 
+// ValidateConfigMapKey validatest that the input ConfigMap key is valid.
+// It must  must consist of alphabetic characters, digits, '_', '-', or '.', regex used for validation is '[-._a-zA-Z0-9]+'
+func ValidateConfigMapKey(key string) error {
+	if errs := validation.IsConfigMapKey(key); len(errs) > 0 {
+		return ErrInvalidConfigMapKey(errors.New(strings.Join(errs, "")))
+	}
+
+	return nil
+}
+
+// ValidateSecretKey validatest that the input Secret key is valid.
+// It must  must consist of alphabetic characters, digits, '_', '-', or '.', regex used for validation is '[-._a-zA-Z0-9]+'
+func ValidateSecretKey(key string) error {
+	if errs := validation.IsConfigMapKey(key); len(errs) > 0 {
+		return ErrInvalidSecretKey(errors.New(strings.Join(errs, "")))
+	}
+
+	return nil
+}
+
 // ValidateLabelKey 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


Test patch:
diff --git a/config_test.go b/config_test.go
index 982ec6ce..b0790242 100644
--- a/config_test.go
+++ b/config_test.go
@@ -314,10 +314,22 @@ func Test_validateEnvs(t *testing.T) {
 	valueSecretKey := "{{ secret:mysecret:key }}"
 	valueSecretKey2 := "{{secret:my-secret:key }}"
 	valueSecretKey3 := "{{secret:my-secret:key2}}"
-	valueSecretKeyIncorrect := "{{ secret:my-secret:key.key }}"
+	valueSecretKey4 := "{{secret:my-secret:key-2}}"
+	valueSecretKey5 := "{{secret:my-secret:key.2}}"
+	valueSecretKey6 := "{{secret:my-secret:key_2}}"
+	valueSecretKey7 := "{{secret:my-secret:key_2-1}}"
+	valueSecretKey8 := "{{secret:my-secret:key_2-1.3}}"
+	valueSecretKeyIncorrect := "{{ secret:my-secret:key,key }}"
 	valueSecretKeyIncorrect2 := "{{ my-secret:key }}"
 	valueSecretKeyIncorrect3 := "{{ secret:my-secret:key }}foo"
 	valueConfigMapKey := "{{ configMap:myconfigmap:key }}"
+	valueConfigMapKey2 := "{{ configMap:myconfigmap:key }}"
+	valueConfigMapKey3 := "{{ configMap:myconfigmap:key2 }}"
+	valueConfigMapKey4 := "{{ configMap:myconfigmap:key-2 }}"
+	valueConfigMapKey5 := "{{ configMap:myconfigmap:key.2 }}"
+	valueConfigMapKey6 := "{{ configMap:myconfigmap:key_2 }}"
+	valueConfigMapKey7 := "{{ configMap:myconfigmap:key_2-1 }}"
+	valueConfigMapKey8 := "{{ configMap:myconfigmap:key_2.1 }}"
 
 	valueSecret := "{{ secret:my-secret }}"
 	valueSecret2 := "{{ secret:mysecret }}"
@@ -467,6 +479,44 @@ func Test_validateEnvs(t *testing.T) {
 			},
 			0,
 		},
+		{
+			"correct entry - multiple configMaps with key",
+			Envs{
+				Env{
+					Name:  &name,
+					Value: &valueConfigMapKey,
+				},
+				Env{
+					Name:  &name,
+					Value: &valueConfigMapKey2,
+				},
+				Env{
+					Name:  &name,
+					Value: &valueConfigMapKey3,
+				},
+				Env{
+					Name:  &name,
+					Value: &valueConfigMapKey4,
+				},
+				Env{
+					Name:  &name,
+					Value: &valueConfigMapKey5,
+				},
+				Env{
+					Name:  &name,
+					Value: &valueConfigMapKey6,
+				},
+				Env{
+					Name:  &name,
+					Value: &valueConfigMapKey7,
+				},
+				Env{
+					Name:  &name,
+					Value: &valueConfigMapKey8,
+				},
+			},
+			0,
+		},
 		{
 			"correct entry - multiple secrets with key",
 			Envs{
@@ -482,6 +532,26 @@ func Test_validateEnvs(t *testing.T) {
 					Name:  &name,
 					Value: &valueSecretKey3,
 				},
+				Env{
+					Name:  &name,
+					Value: &valueSecretKey4,
+				},
+				Env{
+					Name:  &name,
+					Value: &valueSecretKey5,
+				},
+				Env{
+					Name:  &name,
+					Value: &valueSecretKey6,
+				},
+				Env{
+					Name:  &name,
+					Value: &valueSecretKey7,
+				},
+				Env{
+					Name:  &name,
+					Value: &valueSecretKey8,
+				},
 			},
 			0,
 		},
diff --git a/utils/names_test.go b/utils/names_test.go
index 5a20d184..0e606d5d 100644
--- a/utils/names_test.go
+++ b/utils/names_test.go
@@ -82,6 +82,70 @@ func TestValidateEnvVarName(t *testing.T) {
 	}
 }
 
+func TestValidateConfigMapKey(t *testing.T) {
+	cases := []struct {
+		In    string
+		Valid bool
+	}{
+		{"", false},
+		{"*", false},
+		{"example", true},
+		{"example-com", true},
+		{"example.com", true},
+		{"-example-com", true},
+		{"example-com-", true},
+		{"Example", true},
+		{"Example_com", true},
+		{"Example_com.com", true},
+		{"EXAMPLE", true},
+		{";Example", false},
+		{":Example", false},
+		{",Example", false},
+	}
+
+	for _, c := range cases {
+		err := ValidateConfigMapKey(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 TestValidateSecretKey(t *testing.T) {
+	cases := []struct {
+		In    string
+		Valid bool
+	}{
+		{"", false},
+		{"*", false},
+		{"example", true},
+		{"example-com", true},
+		{"example.com", true},
+		{"-example-com", true},
+		{"example-com-", true},
+		{"Example", true},
+		{"Example_com", true},
+		{"Example_com.com", true},
+		{"EXAMPLE", true},
+		{";Example", false},
+		{":Example", false},
+		{",Example", false},
+	}
+
+	for _, c := range cases {
+		err := ValidateSecretKey(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 TestValidateLabelName(t *testing.T) {
 	cases := []struct {
 		In    string