From 8b9e8d8fbfb302d860d1e849cee828e010c5ca28 Mon Sep 17 00:00:00 2001 From: Lukas Berk Date: Wed, 17 Feb 2021 16:27:09 -0500 Subject: [PATCH 1/4] Fixup ping source dataMaxSize configmap - Configmap should be basic key/value pair, not a string for a yaml struct - Provide example dataMaxSize configmap config - Correct example-checksum annotation - No mashalling from yaml needed, strconv directly to int64 - Add unit testing for ConfigMap reading - Remove erroneous broker parsing comments/function from copied implementation - Rename configmap to match naming conventions - update bsize len assignment to match int64 size --- .../core/configmaps/default-pingsource.yaml | 26 +++- pkg/apis/sources/config/ping_defaults.go | 29 ++--- pkg/apis/sources/config/ping_defaults_test.go | 120 ++++++++++++++++++ .../config/testdata/config-ping-defaults.yaml | 41 ++++++ pkg/apis/sources/v1alpha2/ping_validation.go | 2 +- pkg/apis/sources/v1beta1/ping_validation.go | 2 +- pkg/apis/sources/v1beta2/ping_validation.go | 4 +- 7 files changed, 196 insertions(+), 28 deletions(-) create mode 100644 pkg/apis/sources/config/ping_defaults_test.go create mode 100644 pkg/apis/sources/config/testdata/config-ping-defaults.yaml diff --git a/config/core/configmaps/default-pingsource.yaml b/config/core/configmaps/default-pingsource.yaml index ef294239927..9df08b32935 100644 --- a/config/core/configmaps/default-pingsource.yaml +++ b/config/core/configmaps/default-pingsource.yaml @@ -15,13 +15,29 @@ apiVersion: v1 kind: ConfigMap metadata: - name: config-ping-webhook + name: config-ping-defaults namespace: knative-eventing labels: eventing.knative.dev/release: devel annotations: - knative.dev/example-checksum: "6eaeecba" + knative.dev/example-checksum: "01b3f6e0" data: - ping-config: | - # dataMaxSize: 10 # Max number of bytes allowed to be sent for message excluding any base64 decoding. - # Default is no limit set for data + _example: | + ################################ + # # + # EXAMPLE CONFIGURATION # + # # + ################################ + + # This block is not actually functional configuration, + # but serves to illustrate the available configuration + # options and document them in a way that is accessible + # to users that `kubectl edit` this config map. + # + # These sample configuration options may be copied out of + # this example block and unindented to be in the data block + # to actually change the configuration. + + # Max number of bytes allowed to be sent for message excluding any + # base64 decoding. Default is no limit set for data + dataMaxSize: 4096 diff --git a/pkg/apis/sources/config/ping_defaults.go b/pkg/apis/sources/config/ping_defaults.go index 77aa40ca87f..949e5c9fa6d 100644 --- a/pkg/apis/sources/config/ping_defaults.go +++ b/pkg/apis/sources/config/ping_defaults.go @@ -17,21 +17,18 @@ limitations under the License. package config import ( - "encoding/json" "fmt" + "strconv" corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/yaml" ) const ( // PingDefaultsConfigName is the name of config map for the default // configs that pings should use. - PingDefaultsConfigName = "config-ping-webhook" + PingDefaultsConfigName = "config-ping-defaults" - // PingDefaulterKey is the key in the ConfigMap to get the name of the default - // Ping CRD. - PingDefaulterKey = "ping-config" + DataMaxSizeKey = "dataMaxSize" PingDataMaxSize = -1 ) @@ -40,25 +37,19 @@ const ( func NewPingDefaultsConfigFromMap(data map[string]string) (*PingDefaults, error) { nc := &PingDefaults{DataMaxSize: PingDataMaxSize} - // Parse out the Broker Configuration Cluster default section - value, present := data[PingDefaulterKey] + // Parse out the MaxSizeKey + value, present := data[DataMaxSizeKey] if !present || value == "" { - return nil, fmt.Errorf("ConfigMap is missing (or empty) key: %q : %v", PingDefaulterKey, data) + return nc, nil } - if err := parseEntry(value, nc); err != nil { + int64Value, err := strconv.ParseInt(value, 0, 64) + if err != nil { return nil, fmt.Errorf("Failed to parse the entry: %s", err) } + nc.DataMaxSize = int64Value return nc, nil } -func parseEntry(entry string, out interface{}) error { - j, err := yaml.YAMLToJSON([]byte(entry)) - if err != nil { - return fmt.Errorf("ConfigMap's value could not be converted to JSON: %s : %v", err, entry) - } - return json.Unmarshal(j, &out) -} - // NewPingDefaultsConfigFromConfigMap creates a PingDefaults from the supplied configMap func NewPingDefaultsConfigFromConfigMap(config *corev1.ConfigMap) (*PingDefaults, error) { return NewPingDefaultsConfigFromMap(config.Data) @@ -66,7 +57,7 @@ func NewPingDefaultsConfigFromConfigMap(config *corev1.ConfigMap) (*PingDefaults // PingDefaults includes the default values to be populated by the webhook. type PingDefaults struct { - DataMaxSize int `json:"dataMaxSize"` + DataMaxSize int64 `json:"dataMaxSize"` } func (d *PingDefaults) GetPingConfig() *PingDefaults { diff --git a/pkg/apis/sources/config/ping_defaults_test.go b/pkg/apis/sources/config/ping_defaults_test.go new file mode 100644 index 00000000000..a037160c0a5 --- /dev/null +++ b/pkg/apis/sources/config/ping_defaults_test.go @@ -0,0 +1,120 @@ +/* +Copyright 2021 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + . "knative.dev/pkg/configmap/testing" + "knative.dev/pkg/system" + _ "knative.dev/pkg/system/testing" +) + +func TestNewPingDefaultsConfigFromConfigMap(t *testing.T) { + _, example := ConfigMapsFromTestFile(t, PingDefaultsConfigName) + if _, err := NewPingDefaultsConfigFromConfigMap(example); err != nil { + t.Error("NewPingDefaultsConfigFromMap(example) =", err) + } +} + +func TestPingDefaultsConfiguration(t *testing.T) { + testCases := []struct { + name string + wantErr bool + wantDefault int64 + config *corev1.ConfigMap + }{{ + name: "default config", + wantErr: false, + wantDefault: PingDataMaxSize, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: PingDefaultsConfigName, + }, + Data: map[string]string{}, + }, + }, { + name: "example text", + wantErr: false, + wantDefault: PingDataMaxSize, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: PingDefaultsConfigName, + }, + Data: map[string]string{ + "_example": ` + ################################ + # # + # EXAMPLE CONFIGURATION # + # # + ################################ + + # Max number of bytes allowed to be sent for message excluding any + # base64 decoding. Default is no limit set for data + dataMaxSize: 4096 +`, + }, + }, + }, { + name: "specific dataMaxSize", + wantErr: false, + wantDefault: 1337, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: PingDefaultsConfigName, + }, + Data: map[string]string{ + "dataMaxSize": "1337", + }, + }, + }, { + name: "dangling key/value pair", + wantErr: true, + wantDefault: PingDataMaxSize, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: PingDefaultsConfigName, + }, + Data: map[string]string{ + "dataMaxSize": "#nothing to see here", + }, + }, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actualDefault, err := NewPingDefaultsConfigFromConfigMap(tc.config) + + if (err != nil) != tc.wantErr { + t.Fatalf("Test: %q: NewPingDefaultsConfigFromMap() error = %v, wantErr %v", tc.name, err, tc.wantErr) + } + if !tc.wantErr { + if diff := cmp.Diff(tc.wantDefault, actualDefault.DataMaxSize); diff != "" { + t.Error("unexpected value (-want, +got)", diff) + } + } + }) + } +} diff --git a/pkg/apis/sources/config/testdata/config-ping-defaults.yaml b/pkg/apis/sources/config/testdata/config-ping-defaults.yaml new file mode 100644 index 00000000000..c63675b9136 --- /dev/null +++ b/pkg/apis/sources/config/testdata/config-ping-defaults.yaml @@ -0,0 +1,41 @@ +# Copyright 2020 The Knative Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: config-ping-defaults + namespace: knative-eventing + labels: + eventing.knative.dev/release: devel +data: + _example: | + ################################ + # # + # EXAMPLE CONFIGURATION # + # # + ################################ + + # This block is not actually functional configuration, + # but serves to illustrate the available configuration + # options and document them in a way that is accessible + # to users that `kubectl edit` this config map. + # + # These sample configuration options may be copied out of + # this example block and unindented to be in the data block + # to actually change the configuration. + + # Max number of bytes allowed to be sent for message excluding any + # base64 decoding. Default is no limit set for data + dataMaxSize: 4096 diff --git a/pkg/apis/sources/v1alpha2/ping_validation.go b/pkg/apis/sources/v1alpha2/ping_validation.go index 8d048949461..84a01f3ae90 100644 --- a/pkg/apis/sources/v1alpha2/ping_validation.go +++ b/pkg/apis/sources/v1alpha2/ping_validation.go @@ -43,7 +43,7 @@ func (cs *PingSourceSpec) Validate(ctx context.Context) *apis.FieldError { pingConfig := config.FromContextOrDefaults(ctx) pingDefaults := pingConfig.PingDefaults.GetPingConfig() - if bsize := len(cs.JsonData); pingDefaults.DataMaxSize > -1 && bsize > pingDefaults.DataMaxSize { + if bsize := int64(len(cs.JsonData)); pingDefaults.DataMaxSize > -1 && bsize > pingDefaults.DataMaxSize { fe := apis.ErrInvalidValue(fmt.Sprintf("the jsonData length of %d bytes exceeds limit set at %d.", bsize, pingDefaults.DataMaxSize), "jsonData") errs = errs.Also(fe) } diff --git a/pkg/apis/sources/v1beta1/ping_validation.go b/pkg/apis/sources/v1beta1/ping_validation.go index df07d799b46..b4ca07f4b27 100644 --- a/pkg/apis/sources/v1beta1/ping_validation.go +++ b/pkg/apis/sources/v1beta1/ping_validation.go @@ -53,7 +53,7 @@ func (cs *PingSourceSpec) Validate(ctx context.Context) *apis.FieldError { pingDefaults := pingConfig.PingDefaults.GetPingConfig() - if bsize := len(cs.JsonData); pingDefaults.DataMaxSize > -1 && bsize > pingDefaults.DataMaxSize { + if bsize := int64(len(cs.JsonData)); pingDefaults.DataMaxSize > -1 && bsize > pingDefaults.DataMaxSize { fe := apis.ErrInvalidValue(fmt.Sprintf("the jsonData length of %d bytes exceeds limit set at %d.", bsize, pingDefaults.DataMaxSize), "jsonData") errs = errs.Also(fe) } diff --git a/pkg/apis/sources/v1beta2/ping_validation.go b/pkg/apis/sources/v1beta2/ping_validation.go index 189a38ccbdb..5bbd2d7f0ef 100644 --- a/pkg/apis/sources/v1beta2/ping_validation.go +++ b/pkg/apis/sources/v1beta2/ping_validation.go @@ -63,7 +63,7 @@ func (cs *PingSourceSpec) Validate(ctx context.Context) *apis.FieldError { if cs.Data != "" && cs.DataBase64 != "" { errs = errs.Also(apis.ErrMultipleOneOf("data", "dataBase64")) } else if cs.DataBase64 != "" { - if bsize := len(cs.DataBase64); pingDefaults.DataMaxSize > -1 && bsize > pingDefaults.DataMaxSize { + if bsize := int64(len(cs.DataBase64)); pingDefaults.DataMaxSize > -1 && bsize > pingDefaults.DataMaxSize { fe := apis.ErrInvalidValue(fmt.Sprintf("the dataBase64 length of %d bytes exceeds limit set at %d.", bsize, pingDefaults.DataMaxSize), "dataBase64") errs = errs.Also(fe) } @@ -80,7 +80,7 @@ func (cs *PingSourceSpec) Validate(ctx context.Context) *apis.FieldError { } } } else if cs.Data != "" { - if bsize := len(cs.Data); pingDefaults.DataMaxSize > -1 && bsize > pingDefaults.DataMaxSize { + if bsize := int64(len(cs.Data)); pingDefaults.DataMaxSize > -1 && bsize > pingDefaults.DataMaxSize { fe := apis.ErrInvalidValue(fmt.Sprintf("the data length of %d bytes exceeds limit set at %d.", bsize, pingDefaults.DataMaxSize), "data") errs = errs.Also(fe) } From 309a6eda59724c50ec46d7efa35b3b55b07ab8e5 Mon Sep 17 00:00:00 2001 From: Lukas Berk Date: Wed, 17 Feb 2021 17:01:23 -0500 Subject: [PATCH 2/4] Update dataMaxSize example to -1 (default value) --- config/core/configmaps/default-pingsource.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/core/configmaps/default-pingsource.yaml b/config/core/configmaps/default-pingsource.yaml index 9df08b32935..20169206380 100644 --- a/config/core/configmaps/default-pingsource.yaml +++ b/config/core/configmaps/default-pingsource.yaml @@ -40,4 +40,4 @@ data: # Max number of bytes allowed to be sent for message excluding any # base64 decoding. Default is no limit set for data - dataMaxSize: 4096 + dataMaxSize: -1 From ee25a2c50192f860b7d6e87997325910fdd6268e Mon Sep 17 00:00:00 2001 From: Lukas Berk Date: Wed, 17 Feb 2021 17:13:53 -0500 Subject: [PATCH 3/4] nit: remove extra space in pingsource configmap example --- config/core/configmaps/default-pingsource.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/core/configmaps/default-pingsource.yaml b/config/core/configmaps/default-pingsource.yaml index 20169206380..8c25e4fbb1e 100644 --- a/config/core/configmaps/default-pingsource.yaml +++ b/config/core/configmaps/default-pingsource.yaml @@ -39,5 +39,5 @@ data: # to actually change the configuration. # Max number of bytes allowed to be sent for message excluding any - # base64 decoding. Default is no limit set for data + # base64 decoding. Default is no limit set for data dataMaxSize: -1 From 07455dbc645ff7718632da9c99fb24236454093e Mon Sep 17 00:00:00 2001 From: Lukas Berk Date: Wed, 17 Feb 2021 17:21:53 -0500 Subject: [PATCH 4/4] rename PingDataMaxSize DefaultDataMaxSize --- pkg/apis/sources/config/ping_defaults.go | 6 +++--- pkg/apis/sources/config/ping_defaults_test.go | 6 +++--- pkg/apis/sources/config/store.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/apis/sources/config/ping_defaults.go b/pkg/apis/sources/config/ping_defaults.go index 949e5c9fa6d..e3a1fff15eb 100644 --- a/pkg/apis/sources/config/ping_defaults.go +++ b/pkg/apis/sources/config/ping_defaults.go @@ -30,12 +30,12 @@ const ( DataMaxSizeKey = "dataMaxSize" - PingDataMaxSize = -1 + DefaultDataMaxSize = -1 ) // NewPingDefaultsConfigFromMap creates a Defaults from the supplied Map func NewPingDefaultsConfigFromMap(data map[string]string) (*PingDefaults, error) { - nc := &PingDefaults{DataMaxSize: PingDataMaxSize} + nc := &PingDefaults{DataMaxSize: DefaultDataMaxSize} // Parse out the MaxSizeKey value, present := data[DataMaxSizeKey] @@ -62,7 +62,7 @@ type PingDefaults struct { func (d *PingDefaults) GetPingConfig() *PingDefaults { if d.DataMaxSize < 0 { - d.DataMaxSize = PingDataMaxSize + d.DataMaxSize = DefaultDataMaxSize } return d diff --git a/pkg/apis/sources/config/ping_defaults_test.go b/pkg/apis/sources/config/ping_defaults_test.go index a037160c0a5..424c647d66b 100644 --- a/pkg/apis/sources/config/ping_defaults_test.go +++ b/pkg/apis/sources/config/ping_defaults_test.go @@ -44,7 +44,7 @@ func TestPingDefaultsConfiguration(t *testing.T) { }{{ name: "default config", wantErr: false, - wantDefault: PingDataMaxSize, + wantDefault: DefaultDataMaxSize, config: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.Namespace(), @@ -55,7 +55,7 @@ func TestPingDefaultsConfiguration(t *testing.T) { }, { name: "example text", wantErr: false, - wantDefault: PingDataMaxSize, + wantDefault: DefaultDataMaxSize, config: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.Namespace(), @@ -91,7 +91,7 @@ func TestPingDefaultsConfiguration(t *testing.T) { }, { name: "dangling key/value pair", wantErr: true, - wantDefault: PingDataMaxSize, + wantDefault: DefaultDataMaxSize, config: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.Namespace(), diff --git a/pkg/apis/sources/config/store.go b/pkg/apis/sources/config/store.go index 3b5a567d6c9..ceab865de48 100644 --- a/pkg/apis/sources/config/store.go +++ b/pkg/apis/sources/config/store.go @@ -47,7 +47,7 @@ func FromContextOrDefaults(ctx context.Context) *Config { } pingDefaults, err := NewPingDefaultsConfigFromMap(map[string]string{}) if err != nil || pingDefaults == nil { - pingDefaults = &PingDefaults{DataMaxSize: PingDataMaxSize} + pingDefaults = &PingDefaults{DataMaxSize: DefaultDataMaxSize} pingDefaults.GetPingConfig() }