Repository: knative/pkg
Pull request #2462 (2022-03-14 16:46:07)

[release-1.3] Take pointer to map in AsOptionalMap for consistency
This is an automated cherry-pick of #2460

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2462#" title="Author self-approved">knative-prow-robot</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **rhuss** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @rhuss` in a comment when ready.

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

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

- **[OWNERS](https://github.com/knative/pkg/blob/release-1.3/OWNERS)**

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":["rhuss"]} -->
dprotaso: /lgtm
/approve
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2462#issuecomment-1067059644" title="Approved">dprotaso</a>*, *<a href="https://github.com/knative/pkg/pull/2462#" title="Author self-approved">knative-prow-robot</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:

- ~~[OWNERS](https://github.com/knative/pkg/blob/release-1.3/OWNERS)~~ [dprotaso]

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/configmap/parse.go b/configmap/parse.go
index b2f8d4f20..e733e3838 100644
--- a/configmap/parse.go
+++ b/configmap/parse.go
@@ -245,13 +245,21 @@ func Parse(data map[string]string, parsers ...ParseFunc) error {
 	return nil
 }
 
-// AsOptionalMap parses the data into the target as a map[string]string, if it exists.
+// CollectMapEntriesWithPrefix parses the data into the target as a map[string]string, if it exists.
 // The map is represented as a list of key-value pairs with a common prefix.
-func AsOptionalMap(prefix string, target map[string]string) ParseFunc {
+func CollectMapEntriesWithPrefix(prefix string, target *map[string]string) ParseFunc {
+	if target == nil {
+		panic("target cannot be nil")
+	}
+
 	return func(data map[string]string) error {
 		for k, v := range data {
 			if strings.HasPrefix(k, prefix) && len(k) > len(prefix)+1 {
-				target[k[len(prefix)+1: /* remove dot `.` */]] = v
+				if *target == nil {
+					m := make(map[string]string, 2)
+					*target = m
+				}
+				(*target)[k[len(prefix)+1: /* remove dot `.` */]] = v
 			}
 		}
 		return nil
diff --git a/leaderelection/config.go b/leaderelection/config.go
index 13e0832e5..11010014a 100644
--- a/leaderelection/config.go
+++ b/leaderelection/config.go
@@ -57,7 +57,7 @@ func NewConfigFromMap(data map[string]string) (*Config, error) {
 
 		cm.AsUint32("buckets", &config.Buckets),
 
-		cm.AsOptionalMap("map-lease-prefix", config.LeaseNamesPrefixMapping),
+		cm.CollectMapEntriesWithPrefix("map-lease-prefix", &config.LeaseNamesPrefixMapping),
 	); err != nil {
 		return nil, err
 	}
@@ -101,11 +101,10 @@ func (c *Config) GetComponentConfig(name string) ComponentConfig {
 
 func defaultConfig() *Config {
 	return &Config{
-		Buckets:                 1,
-		LeaseDuration:           60 * time.Second,
-		RenewDeadline:           40 * time.Second,
-		RetryPeriod:             10 * time.Second,
-		LeaseNamesPrefixMapping: make(map[string]string),
+		Buckets:       1,
+		LeaseDuration: 60 * time.Second,
+		RenewDeadline: 40 * time.Second,
+		RetryPeriod:   10 * time.Second,
 	}
 }
 


Test patch:
diff --git a/configmap/parse_test.go b/configmap/parse_test.go
index 725b11bd2..b948b3f01 100644
--- a/configmap/parse_test.go
+++ b/configmap/parse_test.go
@@ -76,9 +76,6 @@ func TestParse(t *testing.T) {
 			"test-dict.k":  "v",
 			"test-dict.k1": "v1",
 		},
-		conf: testConfig{
-			dict: map[string]string{},
-		},
 		want: testConfig{
 			str:    "foo.bar",
 			toggle: true,
@@ -218,13 +215,13 @@ func TestParse(t *testing.T) {
 				AsQuantity("test-quantity", &test.conf.qua),
 				AsNamespacedName("test-namespaced-name", &test.conf.nsn),
 				AsOptionalNamespacedName("test-optional-namespaced-name", &test.conf.onsn),
-				AsOptionalMap("test-dict", test.conf.dict),
+				CollectMapEntriesWithPrefix("test-dict", &test.conf.dict),
 			); (err == nil) == test.expectErr {
 				t.Fatal("Failed to parse data:", err)
 			}
 
-			if !cmp.Equal(test.conf, test.want, cmp.AllowUnexported(testConfig{})) {
-				t.Fatalf("parsed = %v, want %v", test.conf, test.want)
+			if diff := cmp.Diff(test.want, test.conf, cmp.AllowUnexported(testConfig{})); diff != "" {
+				t.Fatal("(-want, +got)", diff)
 			}
 		})
 	}
diff --git a/leaderelection/config_test.go b/leaderelection/config_test.go
index 52b96e4c5..949e8ed0f 100644
--- a/leaderelection/config_test.go
+++ b/leaderelection/config_test.go
@@ -39,11 +39,10 @@ const (
 
 func okConfig() *Config {
 	return &Config{
-		Buckets:                 1,
-		LeaseDuration:           15 * time.Second,
-		RenewDeadline:           10 * time.Second,
-		RetryPeriod:             2 * time.Second,
-		LeaseNamesPrefixMapping: map[string]string{},
+		Buckets:       1,
+		LeaseDuration: 15 * time.Second,
+		RenewDeadline: 10 * time.Second,
+		RetryPeriod:   2 * time.Second,
 	}
 }
 
@@ -124,11 +123,10 @@ func TestNewConfigMapFromData(t *testing.T) {
 			"buckets":       "5",
 		},
 		expected: &Config{
-			Buckets:                 5,
-			LeaseDuration:           2 * time.Second,
-			RenewDeadline:           3 * time.Second,
-			RetryPeriod:             4 * time.Second,
-			LeaseNamesPrefixMapping: map[string]string{},
+			Buckets:       5,
+			LeaseDuration: 2 * time.Second,
+			RenewDeadline: 3 * time.Second,
+			RetryPeriod:   4 * time.Second,
 		},
 	}, {
 		name: "prioritize new keys",
@@ -142,11 +140,10 @@ func TestNewConfigMapFromData(t *testing.T) {
 			"buckets":        "7",
 		},
 		expected: &Config{
-			Buckets:                 7,
-			LeaseDuration:           1 * time.Second,
-			RenewDeadline:           2 * time.Second,
-			RetryPeriod:             3 * time.Second,
-			LeaseNamesPrefixMapping: map[string]string{},
+			Buckets:       7,
+			LeaseDuration: 1 * time.Second,
+			RenewDeadline: 2 * time.Second,
+			RetryPeriod:   3 * time.Second,
 		},
 	}}
 
@@ -186,11 +183,10 @@ func TestNewConfigFromMap(t *testing.T) {
 			"buckets":        "5",
 		},
 		want: Config{
-			Buckets:                 5,
-			LeaseDuration:           15 * time.Second,
-			RenewDeadline:           40 * time.Second,
-			RetryPeriod:             10 * time.Second,
-			LeaseNamesPrefixMapping: map[string]string{},
+			Buckets:       5,
+			LeaseDuration: 15 * time.Second,
+			RenewDeadline: 40 * time.Second,
+			RetryPeriod:   10 * time.Second,
 		},
 	}, {
 		name: "ok config, prefix map",