Added --broker-config flag to broker create command (#1700)

* Added --broker-config flag to broker create command

* Added unit tests

* Added comments for exported functions and regenerated docs

* Changed format for spec config

docs/cmd/kn_broker_create.md

@@ -16,6 +16,17 @@ kn broker create NAME
   # Create a broker 'mybroker' in the 'myproject' namespace and with a broker class of 'Kafka'
   kn broker create mybroker --namespace myproject --class Kafka
 
+  # Create a broker 'mybroker' in the myproject namespace with config referencing a configmap in current namespace
+  kn broker create mybroker --namespace myproject --class Kafka --broker-config cm:spec-cm
+  OR
+  kn broker create mybroker --namespace myproject --class Kafka --broker-config spec-cm
+
+  # Create a broker 'mybroker' in the myproject namespace with config referencing secret named spec-sc in test namespace
+  kn broker create mybroker --namespace myproject --class Kafka --broker-config sc:spec-sc:test
+
+  # Create a broker 'mybroker' in the myproject namespace with config referencing RabbitmqCluster mycluster in test namespace
+  kn broker create mybroker --namespace myproject --class Kafka --broker-config rabbitmq.com/v1beta1:RabbitmqCluster:mycluster:test
+
 ```
 
 ### Options
@@ -23,6 +34,12 @@ kn broker create NAME
 ```
       --backoff-delay string     The delay before retrying.
       --backoff-policy string    The retry backoff policy (linear, exponential).
+      --broker-config string     Reference to the broker configuration For example, a pointer to a ConfigMap (cm:, configmap:), Secret(sc:, secret:), RabbitmqCluster(rmq:, rabbitmq: rabbitmqcluster:) etc. It should be used in conjunction with --class flag. The format for specifying the object is a colon separated string consisting of at most 4 slices:
+                                 Length 1: <object-name> (the object will be assumed to be ConfigMap with the same name)
+                                 Length 2: <kind>:<object-name> (the APIVersion will be determined for ConfigMap, Secret, and RabbitmqCluster types)
+                                 Length 3: <kind>:<object-name>:<namespace> (the APIVersion will be determined only for ConfigMap, Secret, and RabbitmqCluster types. Otherwise it will be interpreted as:
+                                 <apiVersion>:<kind>:<object-name>)
+                                 Length 4: <apiVersion>:<kind>:<object-name>:<namespace>
       --class string             Broker class like 'MTChannelBasedBroker' or 'Kafka' (if available).
       --dl-sink string           The sink receiving event that could not be sent to a destination.
   -h, --help                     help for create

pkg/eventing/v1/client.go

@@ -511,6 +511,13 @@ func (b *BrokerBuilder) RetryAfterMax(max *string) *BrokerBuilder {
 
 }
 
+// Config for the broker builder
+func (b *BrokerBuilder) Config(config *duckv1.KReference) *BrokerBuilder {
+	b.broker.Spec.Config = config
+	return b
+
+}
+
 // Build to return an instance of broker object
 func (b *BrokerBuilder) Build() *eventingv1.Broker {
 	return b.broker

pkg/eventing/v1/client_test.go

@@ -39,6 +39,12 @@ import (
 var (
 	testNamespace = "test-ns"
 	testClass     = "test-class"
+	testConfig    = &duckv1.KReference{
+		Kind:       "ConfigMap",
+		Namespace:  "test-ns",
+		Name:       "test-cm",
+		APIVersion: "v1",
+	}
 )
 
 func setup() (fakeSvr fake.FakeEventingV1, client KnEventingClient) {
@@ -337,6 +343,7 @@ func TestBrokerCreate(t *testing.T) {
 	brokerObjWithClass := newBrokerWithClass(name)
 	brokerObjWithDeliveryOptions := newBrokerWithDeliveryOptions(name)
 	brokerObjWithNilDeliveryOptions := newBrokerWithNilDeliveryOptions(name)
+	brokerObjWithConfig := newBrokerWithConfig(name)
 
 	server.AddReactor("create", "brokers",
 		func(a client_testing.Action) (bool, runtime.Object, error) {
@@ -413,6 +420,11 @@ func TestBrokerCreate(t *testing.T) {
 			assert.NilError(t, err)
 		}
 	})
+
+	t.Run("create broker with spec", func(t *testing.T) {
+		err := client.CreateBroker(context.Background(), brokerObjWithConfig)
+		assert.NilError(t, err)
+	})
 }
 
 func TestBrokerGet(t *testing.T) {
@@ -700,6 +712,13 @@ func newBrokerWithClass(name string) *eventingv1.Broker {
 		Class(testClass).
 		Build()
 }
+func newBrokerWithConfig(name string) *eventingv1.Broker {
+	return NewBrokerBuilder(name).
+		Namespace(testNamespace).
+		Class(testClass).
+		Config(testConfig).
+		Build()
+}
 
 func newBrokerWithDeliveryOptions(name string) *eventingv1.Broker {
 	sink := &duckv1.Destination{

pkg/kn/commands/broker/broker_test.go

@@ -133,3 +133,11 @@ func createBrokerWithBackoffDelay(brokerName, delay string) *v1beta1.Broker {
 func createBrokerWithRetryAfterMax(brokerName, timeout string) *v1beta1.Broker {
 	return clientv1beta1.NewBrokerBuilder(brokerName).Namespace("default").RetryAfterMax(&timeout).Build()
 }
+
+func createBrokerWithConfig(brokerName string, config *duckv1.KReference) *v1beta1.Broker {
+	return clientv1beta1.NewBrokerBuilder(brokerName).Namespace("default").Class("Kafka").Config(config).Build()
+}
+
+func createBrokerWithConfigAndClass(brokerName, class string, config *duckv1.KReference) *v1beta1.Broker {
+	return clientv1beta1.NewBrokerBuilder(brokerName).Namespace("default").Class(class).Config(config).Build()
+}

pkg/kn/commands/broker/config_flags.go

@@ -0,0 +1,129 @@
+// Copyright © 2022 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 broker
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/spf13/cobra"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
+)
+
+type ConfigType int
+
+// Known config types for broker
+const (
+	ConfigMapType ConfigType = iota
+	SecretType
+	RabbitMqType
+)
+
+var (
+	// KReferenceMapping is mapping between the known config kinds to a basic
+	// default KReference value
+	KReferenceMapping = map[ConfigType]*duckv1.KReference{
+		ConfigMapType: {Kind: "ConfigMap", APIVersion: "v1"},
+		SecretType:    {Kind: "Secret", APIVersion: "v1"},
+		RabbitMqType:  {Kind: "RabbitmqCluster", APIVersion: "rabbitmq.com/v1beta1"},
+	}
+	ConfigTypeMapping = map[string]ConfigType{
+		"cm":              ConfigMapType,
+		"configmap":       ConfigMapType,
+		"sc":              SecretType,
+		"secret":          SecretType,
+		"rabbitmqcluster": RabbitMqType,
+		"rabbitmq":        RabbitMqType,
+		"rmq":             RabbitMqType,
+	}
+)
+
+// ConfigFlags represents the broker config
+type ConfigFlags struct {
+	BrokerConfig string
+}
+
+// Add is used to add the broker config flag to a command
+func (c *ConfigFlags) Add(cmd *cobra.Command) {
+	cmd.Flags().StringVar(&c.BrokerConfig, "broker-config", "", "Reference to the broker configuration "+
+		"For example, a pointer to a ConfigMap (cm:, configmap:), Secret(sc:, secret:), RabbitmqCluster(rmq:, rabbitmq: rabbitmqcluster:) etc. "+
+		"It should be used in conjunction with --class flag. "+
+		"The format for specifying the object is a colon separated string consisting of at most 4 slices:\n"+
+		"Length 1: <object-name> (the object will be assumed to be ConfigMap with the same name)\n"+
+		"Length 2: <kind>:<object-name> (the APIVersion will be determined for ConfigMap, Secret, and RabbitmqCluster types)\n"+
+		"Length 3: <kind>:<object-name>:<namespace> (the APIVersion will be determined only for ConfigMap, Secret, "+
+		"and RabbitmqCluster types. Otherwise it will be interpreted as:\n"+
+		"<apiVersion>:<kind>:<object-name>)\n"+
+		"Length 4: <apiVersion>:<kind>:<object-name>:<namespace>")
+}
+
+// GetBrokerConfigReference parses the broker config
+// and return the appropriate KReference object
+func (c *ConfigFlags) GetBrokerConfigReference() (*duckv1.KReference, error) {
+	config := c.BrokerConfig
+	slices := strings.SplitN(config, ":", 4)
+	if len(slices) == 1 {
+		// If no APIVersion or Kind is specified, assume Configmap
+		return &duckv1.KReference{
+			Kind:       "ConfigMap",
+			Name:       slices[0],
+			APIVersion: "v1",
+		}, nil
+	} else if len(slices) == 2 {
+		// If only two slices are present, it should resolve to
+		// kind:name
+		kind := slices[0]
+		name := slices[1]
+		kRef := getDefaultKReference(kind)
+		if kRef.APIVersion == "" {
+			return nil, fmt.Errorf("APIVersion could not be determined for kind %q. Provide config in format: \"<apiVersion>:<kind>:<name>:<namespace>\"", kind)
+		}
+		kRef.Name = name
+		return kRef, nil
+	} else if len(slices) == 3 {
+		// 3 slices could resolve to either of the following:
+		// 1. <kind>:<name>:<namespace>
+		// 2. <apiVersion>:<kind>:<name>
+
+		var kRef *duckv1.KReference
+		if kRef = getDefaultKReference(slices[0]); kRef.APIVersion == "" {
+			return &duckv1.KReference{
+				Kind:       slices[1],
+				Name:       slices[2],
+				APIVersion: slices[0],
+			}, nil
+		}
+
+		kRef.Name = slices[1]
+		kRef.Namespace = slices[2]
+		return kRef, nil
+	} else {
+		// 4 slices should resolve into <apiVersion>:<kind>:<name>:<namespace>
+		return &duckv1.KReference{
+			APIVersion: slices[0],
+			Kind:       slices[1],
+			Name:       slices[2],
+			Namespace:  slices[3],
+		}, nil
+	}
+}
+
+func getDefaultKReference(kind string) *duckv1.KReference {
+	k := strings.ToLower(kind)
+	if configType, ok := ConfigTypeMapping[k]; ok {
+		return KReferenceMapping[configType]
+	}
+	return &duckv1.KReference{Kind: kind}
+}

pkg/kn/commands/broker/config_flags_test.go

@@ -0,0 +1,110 @@
+// Copyright © 2022 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 broker
+
+import (
+	"testing"
+
+	"github.com/spf13/cobra"
+	"gotest.tools/v3/assert"
+	v1 "knative.dev/pkg/apis/duck/v1"
+)
+
+func TestConfigFlags_Add(t *testing.T) {
+	testCmd := &cobra.Command{}
+	c := ConfigFlags{}
+	c.Add(testCmd)
+
+	assert.NilError(t, testCmd.Flags().Set("broker-config", "mock-config"))
+	assert.Equal(t, c.BrokerConfig, "mock-config")
+}
+
+func TestConfigFlags_GetBrokerConfigReference(t *testing.T) {
+	tests := []struct {
+		name          string
+		argument      string
+		expectedKRef  *v1.KReference
+		expectedError string
+	}{{
+		name:     "no kind specified",
+		argument: "mock-name",
+		expectedKRef: &v1.KReference{
+			Kind:       "ConfigMap",
+			Namespace:  "",
+			Name:       "mock-name",
+			APIVersion: "v1",
+			Group:      "",
+		},
+	},
+		{
+			name:     "only configmap kind and name specified",
+			argument: "cm:mock-name",
+			expectedKRef: &v1.KReference{
+				Kind:       "ConfigMap",
+				Namespace:  "",
+				Name:       "mock-name",
+				APIVersion: "v1",
+				Group:      "",
+			},
+		},
+		{
+			name:     "only rabbitmq kind and name specified",
+			argument: "rabbitmqcluster:mock-name",
+			expectedKRef: &v1.KReference{
+				Kind:       "RabbitmqCluster",
+				Namespace:  "",
+				Name:       "mock-name",
+				APIVersion: "rabbitmq.com/v1beta1",
+				Group:      "",
+			},
+		}, {
+			name:          "only kind (unknown) and name specified without specifying API version",
+			argument:      "unknown:mock-name",
+			expectedKRef:  nil,
+			expectedError: "APIVersion could not be determined for kind \"unknown\"",
+		},
+		{
+			name:     "kind, name, and namespace specified",
+			argument: "secret:mock-name:mock-namespace",
+			expectedKRef: &v1.KReference{
+				Kind:       "Secret",
+				Namespace:  "mock-namespace",
+				Name:       "mock-name",
+				APIVersion: "v1",
+				Group:      "",
+			},
+		},
+		{
+			name:     "apiVersion, kind, name, and namespace",
+			argument: "rabbitmq.com/v1beta1:RabbitmqCluster:test-cluster:test-ns",
+			expectedKRef: &v1.KReference{
+				Kind:       "RabbitmqCluster",
+				Namespace:  "test-ns",
+				Name:       "test-cluster",
+				APIVersion: "rabbitmq.com/v1beta1",
+			},
+		},
+	}
+	for _, tt := range tests {
+		c := ConfigFlags{BrokerConfig: tt.argument}
+		actualKRef, actualErr := c.GetBrokerConfigReference()
+		assert.DeepEqual(t, tt.expectedKRef, actualKRef)
+		if tt.expectedError == "" {
+			assert.NilError(t, actualErr)
+		} else {
+			assert.ErrorContains(t, actualErr, tt.expectedError)
+		}
+	}
+}

pkg/kn/commands/broker/create.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/spf13/cobra"
 	v1 "knative.dev/eventing/pkg/apis/duck/v1"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
 
 	clientv1beta1 "knative.dev/client/pkg/eventing/v1"
 	"knative.dev/client/pkg/kn/commands"
@@ -33,6 +34,17 @@ var createExample = `
 
   # Create a broker 'mybroker' in the 'myproject' namespace and with a broker class of 'Kafka'
   kn broker create mybroker --namespace myproject --class Kafka
+
+  # Create a broker 'mybroker' in the myproject namespace with config referencing a configmap in current namespace
+  kn broker create mybroker --namespace myproject --class Kafka --broker-config cm:spec-cm
+  OR
+  kn broker create mybroker --namespace myproject --class Kafka --broker-config spec-cm
+
+  # Create a broker 'mybroker' in the myproject namespace with config referencing secret named spec-sc in test namespace
+  kn broker create mybroker --namespace myproject --class Kafka --broker-config sc:spec-sc:test
+
+  # Create a broker 'mybroker' in the myproject namespace with config referencing RabbitmqCluster mycluster in test namespace
+  kn broker create mybroker --namespace myproject --class Kafka --broker-config rabbitmq.com/v1beta1:RabbitmqCluster:mycluster:test
 `
 
 // NewBrokerCreateCommand represents command to create new broker instance
@@ -41,6 +53,7 @@ func NewBrokerCreateCommand(p *commands.KnParams) *cobra.Command {
 	var className string
 
 	var deliveryFlags DeliveryOptionFlags
+	var configFlags ConfigFlags
 	cmd := &cobra.Command{
 		Use:     "create NAME",
 		Short:   "Create a broker",
@@ -73,6 +86,19 @@ func NewBrokerCreateCommand(p *commands.KnParams) *cobra.Command {
 
 			backoffPolicy := v1.BackoffPolicyType(deliveryFlags.BackoffPolicy)
 
+			var configReference *duckv1.KReference
+
+			if cmd.Flags().Changed("broker-config") {
+				if !cmd.Flags().Changed("class") {
+					return fmt.Errorf("cannot set broker-config without setting class")
+				}
+
+				configReference, err = configFlags.GetBrokerConfigReference()
+				if err != nil {
+					return err
+				}
+			}
+
 			brokerBuilder := clientv1beta1.
 				NewBrokerBuilder(name).
 				Namespace(namespace).
@@ -82,7 +108,8 @@ func NewBrokerCreateCommand(p *commands.KnParams) *cobra.Command {
 				Timeout(&deliveryFlags.Timeout).
 				BackoffPolicy(&backoffPolicy).
 				BackoffDelay(&deliveryFlags.BackoffDelay).
-				RetryAfterMax(&deliveryFlags.RetryAfterMax)
+				RetryAfterMax(&deliveryFlags.RetryAfterMax).
+				Config(configReference)
 
 			err = eventingClient.CreateBroker(cmd.Context(), brokerBuilder.Build())
 			if err != nil {
@@ -96,6 +123,7 @@ func NewBrokerCreateCommand(p *commands.KnParams) *cobra.Command {
 	}
 	commands.AddNamespaceFlags(cmd.Flags(), false)
 	cmd.Flags().StringVar(&className, "class", "", "Broker class like 'MTChannelBasedBroker' or 'Kafka' (if available).")
+	configFlags.Add(cmd)
 	deliveryFlags.Add(cmd)
 	return cmd
 }

pkg/kn/commands/broker/create_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"gotest.tools/v3/assert"
+	v1 "knative.dev/pkg/apis/duck/v1"
 
 	clienteventingv1 "knative.dev/client/pkg/eventing/v1"
 	"knative.dev/client/pkg/util"
@@ -61,6 +62,70 @@ func TestBrokerCreateWithClass(t *testing.T) {
 	eventingRecorder.Validate()
 }
 
+func TestBrokerCreateWithConfig(t *testing.T) {
+	eventingClient := clienteventingv1.NewMockKnEventingClient(t)
+
+	eventingRecorder := eventingClient.Recorder()
+
+	config := &v1.KReference{
+		Kind:       "ConfigMap",
+		Namespace:  "",
+		Name:       "test-config",
+		APIVersion: "v1",
+	}
+	secretConfig := &v1.KReference{
+		Kind:       "Secret",
+		Name:       "test-secret",
+		Namespace:  "test-ns",
+		APIVersion: "v1",
+	}
+	rabbitConfig := &v1.KReference{
+		Kind:       "RabbitmqCluster",
+		Namespace:  "test-ns",
+		Name:       "test-cluster",
+		APIVersion: "rabbitmq.com/v1beta1",
+	}
+
+	eventingRecorder.CreateBroker(createBrokerWithConfig(brokerName, config), nil)
+	// 1 slice
+	out, err := executeBrokerCommand(eventingClient, "create", brokerName, "--broker-config", "test-config",
+		"--class", "Kafka")
+	assert.NilError(t, err, "Broker should be created")
+	assert.Assert(t, util.ContainsAll(out, "Broker", brokerName, "created", "namespace", "default"))
+
+	eventingRecorder.CreateBroker(createBrokerWithConfig(brokerName, config), nil)
+	// 2 slices
+	out, err = executeBrokerCommand(eventingClient, "create", brokerName, "--broker-config", "cm:test-config",
+		"--class", "Kafka")
+	assert.NilError(t, err, "Broker should be created")
+	assert.Assert(t, util.ContainsAll(out, "Broker", brokerName, "created", "namespace", "default"))
+
+	eventingRecorder.CreateBroker(createBrokerWithConfig(brokerName, secretConfig), nil)
+
+	// 3 slices
+	out, err = executeBrokerCommand(eventingClient, "create", brokerName, "--broker-config", "secret:test-secret:test-ns",
+		"--class", "Kafka")
+	assert.NilError(t, err, "Broker should be created")
+	assert.Assert(t, util.ContainsAll(out, "Broker", brokerName, "created", "namespace", "default"))
+
+	// 4 slices
+	eventingRecorder.CreateBroker(createBrokerWithConfigAndClass(brokerName, "RabbitMQBroker", rabbitConfig), nil)
+	out, err = executeBrokerCommand(eventingClient, "create", brokerName, "--broker-config", "rabbitmq.com/v1beta1:RabbitmqCluster:test-cluster:test-ns",
+		"--class", "RabbitMQBroker")
+	assert.NilError(t, err, "Broker should be created")
+	assert.Assert(t, util.ContainsAll(out, "Broker", brokerName, "created", "namespace", "default"))
+
+	eventingRecorder.CreateBroker(createBrokerWithConfig(brokerName, &v1.KReference{Kind: "ConfigMap", APIVersion: "v1"}), nil)
+	out, err = executeBrokerCommand(eventingClient, "create", brokerName, "--broker-config", "", "--class", "Kafka")
+	assert.NilError(t, err, "Broker should be created with default configmap as config")
+	assert.Assert(t, util.ContainsAll(out, "Broker", brokerName, "created", "namespace", "default"))
+
+	_, err = executeBrokerCommand(eventingClient, "create", brokerName, "--broker-config", "")
+	assert.ErrorContains(t, err, "cannot set broker-config without setting class")
+
+	eventingRecorder.Validate()
+}
+
 func TestBrokerCreateWithError(t *testing.T) {
 	eventingClient := clienteventingv1.NewMockKnEventingClient(t)