Repository: knative/pkg
Pull request #1601 (2020-08-10 21:40:23)

Fix comment to ensure we tell the truth to the world
Otherwise we return the temp object that loop variable points at.

/assign mattmoor

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1601#" title="Author self-approved">vagababov</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[apis/duck/OWNERS](https://github.com/knative/pkg/blob/master/apis/duck/OWNERS)~~ [vagababov]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/apis/duck/v1/status_types.go b/apis/duck/v1/status_types.go
index 5ea757694..bb662eeea 100644
--- a/apis/duck/v1/status_types.go
+++ b/apis/duck/v1/status_types.go
@@ -74,7 +74,7 @@ func (*Conditions) GetFullType() ducktypes.Populatable {
 	return &KResource{}
 }
 
-// GetCondition fetches the condition of the specified type.
+// GetCondition fetches a copy of the condition of the specified type.
 func (s *Status) GetCondition(t apis.ConditionType) *apis.Condition {
 	for _, cond := range s.Conditions {
 		if cond.Type == t {
diff --git a/configmap/parse.go b/configmap/parse.go
index 1ef675818..eba04a11c 100644
--- a/configmap/parse.go
+++ b/configmap/parse.go
@@ -23,6 +23,8 @@ import (
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/resource"
+	"k8s.io/apimachinery/pkg/api/validation"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
@@ -144,6 +146,47 @@ func AsQuantity(key string, target **resource.Quantity) ParseFunc {
 	}
 }
 
+// AsOptionalNamespacedName parses the value at key as a types.NamespacedName into the target, if it exists
+// The namespace and name are both required and expected to be valid DNS labels
+func AsOptionalNamespacedName(key string, target **types.NamespacedName) ParseFunc {
+	return func(data map[string]string) error {
+		if _, ok := data[key]; !ok {
+			return nil
+		}
+
+		*target = &types.NamespacedName{}
+		return AsNamespacedName(key, *target)(data)
+	}
+}
+
+// AsNamespacedName parses the value at key as a types.NamespacedName into the target, if it exists
+// The namespace and name are both required and expected to be valid DNS labels
+func AsNamespacedName(key string, target *types.NamespacedName) ParseFunc {
+	return func(data map[string]string) error {
+		raw, ok := data[key]
+		if !ok {
+			return nil
+		}
+
+		v := strings.SplitN(raw, string(types.Separator), 3)
+
+		if len(v) != 2 {
+			return fmt.Errorf("failed to parse %q: expected 'namespace/name' format", key)
+		}
+
+		for _, val := range v {
+			if errs := validation.ValidateNamespaceName(val, false); len(errs) > 0 {
+				return fmt.Errorf("failed to parse %q: %s", key, strings.Join(errs, ", "))
+			}
+		}
+
+		target.Namespace = v[0]
+		target.Name = v[1]
+
+		return nil
+	}
+}
+
 // Parse parses the given map using the parser functions passed in.
 func Parse(data map[string]string, parsers ...ParseFunc) error {
 	for _, parse := range parsers {


Test patch:
diff --git a/configmap/parse_test.go b/configmap/parse_test.go
index 26fb3332d..8bf6cb98e 100644
--- a/configmap/parse_test.go
+++ b/configmap/parse_test.go
@@ -22,6 +22,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"k8s.io/apimachinery/pkg/api/resource"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
@@ -35,6 +36,9 @@ type testConfig struct {
 	dur time.Duration
 	set sets.String
 	qua *resource.Quantity
+
+	nsn  types.NamespacedName
+	onsn *types.NamespacedName
 }
 
 func TestParse(t *testing.T) {
@@ -57,6 +61,9 @@ func TestParse(t *testing.T) {
 			"test-duration": "1m",
 			"test-set":      "a,b,c",
 			"test-quantity": "500m",
+
+			"test-namespaced-name":          "some-namespace/some-name",
+			"test-optional-namespaced-name": "some-other-namespace/some-other-name",
 		},
 		want: testConfig{
 			str: "foo.bar",
@@ -68,6 +75,14 @@ func TestParse(t *testing.T) {
 			dur: time.Minute,
 			set: sets.NewString("a", "b", "c"),
 			qua: &fiveHundredM,
+			nsn: types.NamespacedName{
+				Name:      "some-name",
+				Namespace: "some-namespace",
+			},
+			onsn: &types.NamespacedName{
+				Name:      "some-other-name",
+				Namespace: "some-other-namespace",
+			},
 		},
 	}, {
 		name: "respect defaults",
@@ -133,6 +148,18 @@ func TestParse(t *testing.T) {
 			"test-quantity": "foo",
 		},
 		expectErr: true,
+	}, {
+		name: "types.NamespacedName bad dns name error",
+		data: map[string]string{
+			"test-namespaced-name": "some.bad.name/blah.bad.name",
+		},
+		expectErr: true,
+	}, {
+		name: "types.NamespacedName bad segment count error",
+		data: map[string]string{
+			"test-namespaced-name": "default/resource/whut",
+		},
+		expectErr: true,
 	}}
 
 	for _, test := range tests {
@@ -147,6 +174,8 @@ func TestParse(t *testing.T) {
 				AsDuration("test-duration", &test.conf.dur),
 				AsStringSet("test-set", &test.conf.set),
 				AsQuantity("test-quantity", &test.conf.qua),
+				AsNamespacedName("test-namespaced-name", &test.conf.nsn),
+				AsOptionalNamespacedName("test-optional-namespaced-name", &test.conf.onsn),
 			); (err == nil) == test.expectErr {
 				t.Fatal("Failed to parse data:", err)
 			}