Attach broker delivery ConfigMap to controller context (#1887) (#2143)

tommyreddad · web-flow · commit ae0bf760bdcd · 2021-02-12T15:23:26.000-08:00
* Broker reconciler has delivery config store

* Rename broker config package to brokerdelivery

* Fix broker reconciler controller unit test failure

* Use string builder for configmap testing
diff --git a/cmd/controller/wire.go b/cmd/controller/wire.go
@@ -18,6 +18,7 @@ package main
 import (
 	"context"
 
+	"github.com/google/knative-gcp/pkg/apis/configs/brokerdelivery"
 	"github.com/google/knative-gcp/pkg/apis/configs/dataresidency"
 	"github.com/google/knative-gcp/pkg/apis/configs/gcpauth"
 	"github.com/google/knative-gcp/pkg/reconciler/broker"
@@ -43,6 +44,7 @@ func InitializeControllers(ctx context.Context) ([]injection.ControllerConstruct
 		Controllers,
 		ClientOptions,
 		iam.PolicyManagerSet,
+		wire.Struct(new(brokerdelivery.StoreSingleton)),
 		wire.Struct(new(gcpauth.StoreSingleton)),
 		wire.Struct(new(dataresidency.StoreSingleton)),
 		auditlogs.NewConstructor,
diff --git a/cmd/controller/wire_gen.go b/cmd/controller/wire_gen.go
diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go
@@ -21,7 +21,7 @@ import (
 	"log"
 
 	brokerv1beta1 "github.com/google/knative-gcp/pkg/apis/broker/v1beta1"
-	"github.com/google/knative-gcp/pkg/apis/configs/broker"
+	"github.com/google/knative-gcp/pkg/apis/configs/brokerdelivery"
 	"github.com/google/knative-gcp/pkg/apis/configs/dataresidency"
 	"github.com/google/knative-gcp/pkg/apis/configs/gcpauth"
 	"github.com/google/knative-gcp/pkg/apis/events"
@@ -92,16 +92,16 @@ var types = map[schema.GroupVersionKind]resourcesemantics.GenericCRD{
 
 type defaultingAdmissionController func(context.Context, configmap.Watcher) *controller.Impl
 
-func newDefaultingAdmissionConstructor(brokers *broker.StoreSingleton, gcpas *gcpauth.StoreSingleton) defaultingAdmissionController {
+func newDefaultingAdmissionConstructor(brokerdeliverys *brokerdelivery.StoreSingleton, gcpas *gcpauth.StoreSingleton) defaultingAdmissionController {
 	return func(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
-		return newDefaultingAdmissionController(ctx, cmw, brokers.Store(ctx, cmw), gcpas.Store(ctx, cmw))
+		return newDefaultingAdmissionController(ctx, cmw, brokerdeliverys.Store(ctx, cmw), gcpas.Store(ctx, cmw))
 	}
 }
 
-func newDefaultingAdmissionController(ctx context.Context, cmw configmap.Watcher, brokers *broker.Store, gcpas *gcpauth.Store) *controller.Impl {
+func newDefaultingAdmissionController(ctx context.Context, cmw configmap.Watcher, brokerdeliverys *brokerdelivery.Store, gcpas *gcpauth.Store) *controller.Impl {
 	// Decorate contexts with the current state of the config.
 	ctxFunc := func(ctx context.Context) context.Context {
-		return brokers.ToContext(gcpas.ToContext(ctx))
+		return brokerdeliverys.ToContext(gcpas.ToContext(ctx))
 	}
 
 	return defaulting.NewAdmissionController(ctx,
@@ -125,16 +125,16 @@ func newDefaultingAdmissionController(ctx context.Context, cmw configmap.Watcher
 
 type validationController func(context.Context, configmap.Watcher) *controller.Impl
 
-func newValidationConstructor(brokers *broker.StoreSingleton, gcpas *gcpauth.StoreSingleton) validationController {
+func newValidationConstructor(brokerdeliverys *brokerdelivery.StoreSingleton, gcpas *gcpauth.StoreSingleton) validationController {
 	return func(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
-		return newValidationAdmissionController(ctx, cmw, brokers.Store(ctx, cmw), gcpas.Store(ctx, cmw))
+		return newValidationAdmissionController(ctx, cmw, brokerdeliverys.Store(ctx, cmw), gcpas.Store(ctx, cmw))
 	}
 }
 
-func newValidationAdmissionController(ctx context.Context, cmw configmap.Watcher, brokers *broker.Store, gcpas *gcpauth.Store) *controller.Impl {
+func newValidationAdmissionController(ctx context.Context, cmw configmap.Watcher, brokerdeliverys *brokerdelivery.Store, gcpas *gcpauth.Store) *controller.Impl {
 	// A function that infuses the context passed to Validate/SetDefaults with custom metadata.
 	ctxFunc := func(ctx context.Context) context.Context {
-		return brokers.ToContext(gcpas.ToContext(ctx))
+		return brokerdeliverys.ToContext(gcpas.ToContext(ctx))
 	}
 
 	return validation.NewAdmissionController(ctx,
@@ -173,21 +173,21 @@ func NewConfigValidationController(ctx context.Context, _ configmap.Watcher) *co
 			logging.ConfigMapName():        logging.NewConfigFromConfigMap,
 			leaderelection.ConfigMapName(): leaderelection.NewConfigFromConfigMap,
 			gcpauth.ConfigMapName():        gcpauth.NewDefaultsConfigFromConfigMap,
-			broker.ConfigMapName():         broker.NewDefaultsConfigFromConfigMap,
+			brokerdelivery.ConfigMapName(): brokerdelivery.NewDefaultsConfigFromConfigMap,
 			dataresidency.ConfigMapName():  dataresidency.NewDefaultsConfigFromConfigMap,
 		},
 	)
 }
 
 type conversionController func(context.Context, configmap.Watcher) *controller.Impl
 
-func newConversionConstructor(brokers *broker.StoreSingleton, gcpas *gcpauth.StoreSingleton) conversionController {
+func newConversionConstructor(brokerdeliverys *brokerdelivery.StoreSingleton, gcpas *gcpauth.StoreSingleton) conversionController {
 	return func(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
-		return newConversionController(ctx, cmw, brokers.Store(ctx, cmw), gcpas.Store(ctx, cmw))
+		return newConversionController(ctx, cmw, brokerdeliverys.Store(ctx, cmw), gcpas.Store(ctx, cmw))
 	}
 }
 
-func newConversionController(ctx context.Context, cmw configmap.Watcher, brokers *broker.Store, gcpas *gcpauth.Store) *controller.Impl {
+func newConversionController(ctx context.Context, cmw configmap.Watcher, brokerdeliverys *brokerdelivery.Store, gcpas *gcpauth.Store) *controller.Impl {
 	var (
 		eventsv1alpha1_    = eventsv1alpha1.SchemeGroupVersion.Version
 		eventsv1beta1_     = eventsv1beta1.SchemeGroupVersion.Version
@@ -201,7 +201,7 @@ func newConversionController(ctx context.Context, cmw configmap.Watcher, brokers
 
 	// Decorate contexts with the current state of the config.
 	ctxFunc := func(ctx context.Context) context.Context {
-		return brokers.ToContext(gcpas.ToContext(ctx))
+		return brokerdeliverys.ToContext(gcpas.ToContext(ctx))
 	}
 
 	return conversion.NewConversionController(ctx,
diff --git a/cmd/webhook/wire.go b/cmd/webhook/wire.go
@@ -18,7 +18,7 @@ package main
 import (
 	"context"
 
-	"github.com/google/knative-gcp/pkg/apis/configs/broker"
+	"github.com/google/knative-gcp/pkg/apis/configs/brokerdelivery"
 	"github.com/google/knative-gcp/pkg/apis/configs/gcpauth"
 	"github.com/google/wire"
 	"knative.dev/pkg/injection"
@@ -27,7 +27,7 @@ import (
 func InitializeControllers(ctx context.Context) ([]injection.ControllerConstructor, error) {
 	panic(wire.Build(
 		Controllers,
-		wire.Struct(new(broker.StoreSingleton)),
+		wire.Struct(new(brokerdelivery.StoreSingleton)),
 		wire.Struct(new(gcpauth.StoreSingleton)),
 		newConversionConstructor,
 		newDefaultingAdmissionConstructor,
diff --git a/cmd/webhook/wire_gen.go b/cmd/webhook/wire_gen.go
diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh
@@ -76,7 +76,7 @@ ${GOPATH}/bin/deepcopy-gen \
   -O zz_generated.deepcopy \
   --go-header-file ${REPO_ROOT_DIR}/hack/boilerplate/boilerplate.go.txt \
   -i github.com/google/knative-gcp/pkg/apis/configs/gcpauth \
-  -i github.com/google/knative-gcp/pkg/apis/configs/broker \
+  -i github.com/google/knative-gcp/pkg/apis/configs/brokerdelivery \
   -i github.com/google/knative-gcp/pkg/apis/configs/dataresidency \
 
 # TODO(yolocs): generate autoscaling v2beta2 in knative/pkg.
diff --git a/pkg/apis/broker/v1beta1/broker_defaults.go b/pkg/apis/broker/v1beta1/broker_defaults.go
@@ -23,14 +23,14 @@ import (
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/logging"
 
-	"github.com/google/knative-gcp/pkg/apis/configs/broker"
+	"github.com/google/knative-gcp/pkg/apis/configs/brokerdelivery"
 )
 
 // SetDefaults sets the default field values for a Broker.
 func (b *Broker) SetDefaults(ctx context.Context) {
 	// Apply the default Broker delivery settings from the context.
 	withNS := apis.WithinParent(ctx, b.ObjectMeta)
-	deliverySpecDefaults := broker.FromContextOrDefaults(withNS).BrokerDeliverySpecDefaults
+	deliverySpecDefaults := brokerdelivery.FromContextOrDefaults(withNS).BrokerDeliverySpecDefaults
 	if deliverySpecDefaults == nil {
 		logging.FromContext(ctx).Error("Failed to get the BrokerDeliverySpecDefaults")
 		return
diff --git a/pkg/apis/broker/v1beta1/broker_defaults_test.go b/pkg/apis/broker/v1beta1/broker_defaults_test.go
@@ -23,7 +23,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	"github.com/google/knative-gcp/pkg/apis/configs/broker"
+	"github.com/google/knative-gcp/pkg/apis/configs/brokerdelivery"
 	eventingduckv1beta1 "knative.dev/eventing/pkg/apis/duck/v1beta1"
 	eventingv1beta1 "knative.dev/eventing/pkg/apis/eventing/v1beta1"
 	"knative.dev/pkg/apis"
@@ -39,11 +39,11 @@ var (
 	nsDefaultedRetry              int32 = 10
 	customRetry                   int32 = 5
 
-	defaultConfig = &broker.Config{
-		BrokerDeliverySpecDefaults: &broker.Defaults{
+	defaultConfig = &brokerdelivery.Config{
+		BrokerDeliverySpecDefaults: &brokerdelivery.Defaults{
 			// NamespaceDefaultsConfig are the default Broker Configs for each namespace.
 			// Namespace is the key, the value is the KReference to the config.
-			NamespaceDefaults: map[string]broker.ScopedDefaults{
+			NamespaceDefaults: map[string]brokerdelivery.ScopedDefaults{
 				"mynamespace": {
 					DeliverySpec: &eventingduckv1beta1.DeliverySpec{
 						BackoffDelay:  &nsDefaultedBackoffDelay,
@@ -80,7 +80,7 @@ var (
 					},
 				},
 			},
-			ClusterDefaults: broker.ScopedDefaults{
+			ClusterDefaults: brokerdelivery.ScopedDefaults{
 				DeliverySpec: &eventingduckv1beta1.DeliverySpec{
 					BackoffDelay:  &clusterDefaultedBackoffDelay,
 					BackoffPolicy: &clusterDefaultedBackoffPolicy,
@@ -256,7 +256,7 @@ func TestBroker_SetDefaults(t *testing.T) {
 	}
 	for n, tc := range testCases {
 		t.Run(n, func(t *testing.T) {
-			tc.initial.SetDefaults(broker.ToContext(context.Background(), defaultConfig))
+			tc.initial.SetDefaults(brokerdelivery.ToContext(context.Background(), defaultConfig))
 			if diff := cmp.Diff(tc.expected, tc.initial); diff != "" {
 				t.Fatalf("Unexpected defaults (-want, +got): %s", diff)
 			}
diff --git a/pkg/apis/configs/brokerdelivery/br_delivery_defaults.go b/pkg/apis/configs/brokerdelivery/br_delivery_defaults.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package broker
+package brokerdelivery
 
 import (
 	"encoding/json"
diff --git a/pkg/apis/configs/brokerdelivery/br_delivery_defaults_test.go b/pkg/apis/configs/brokerdelivery/br_delivery_defaults_test.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package broker
+package brokerdelivery
 
 import (
 	"testing"
diff --git a/pkg/apis/configs/brokerdelivery/defaults.go b/pkg/apis/configs/brokerdelivery/defaults.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package broker
+package brokerdelivery
 
 import (
 	eventingduckv1beta1 "knative.dev/eventing/pkg/apis/duck/v1beta1"
diff --git a/pkg/apis/configs/brokerdelivery/doc.go b/pkg/apis/configs/brokerdelivery/doc.go
@@ -16,5 +16,5 @@ limitations under the License.
 
 // +k8s:deepcopy-gen=package
 
-// broker holds the typed objects that define the schemas for default Broker delivery settings.
-package broker
+// brokerdelivery holds the typed objects that define the schemas for default Broker delivery settings.
+package brokerdelivery
diff --git a/pkg/apis/configs/brokerdelivery/singleton.go b/pkg/apis/configs/brokerdelivery/singleton.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package broker
+package brokerdelivery
 
 import (
 	"context"
diff --git a/pkg/apis/configs/brokerdelivery/singleton_test.go b/pkg/apis/configs/brokerdelivery/singleton_test.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package broker
+package brokerdelivery
 
 import (
 	"context"
diff --git a/pkg/apis/configs/brokerdelivery/store.go b/pkg/apis/configs/brokerdelivery/store.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package broker
+package brokerdelivery
 
 import (
 	"context"
diff --git a/pkg/apis/configs/brokerdelivery/store_test.go b/pkg/apis/configs/brokerdelivery/store_test.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package broker
+package brokerdelivery
 
 import (
 	"context"
diff --git a/pkg/apis/configs/brokerdelivery/testdata/config-br-delivery.yaml b/pkg/apis/configs/brokerdelivery/testdata/config-br-delivery.yaml
diff --git a/pkg/apis/configs/brokerdelivery/zz_generated.deepcopy.go b/pkg/apis/configs/brokerdelivery/zz_generated.deepcopy.go
diff --git a/pkg/reconciler/broker/controller.go b/pkg/reconciler/broker/controller.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"os"
 
+	"github.com/google/knative-gcp/pkg/apis/configs/brokerdelivery"
 	"github.com/google/knative-gcp/pkg/apis/configs/dataresidency"
 
 	"cloud.google.com/go/pubsub"
@@ -53,13 +54,13 @@ const (
 type Constructor injection.ControllerConstructor
 
 // NewConstructor creates a constructor to make a Broker controller.
-func NewConstructor(dataresidencyss *dataresidency.StoreSingleton) Constructor {
+func NewConstructor(brokerdeliveryss *brokerdelivery.StoreSingleton, dataresidencyss *dataresidency.StoreSingleton) Constructor {
 	return func(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
-		return newController(ctx, cmw, dataresidencyss.Store(ctx, cmw))
+		return newController(ctx, cmw, brokerdeliveryss.Store(ctx, cmw), dataresidencyss.Store(ctx, cmw))
 	}
 }
 
-func newController(ctx context.Context, cmw configmap.Watcher, drs *dataresidency.Store) *controller.Impl {
+func newController(ctx context.Context, cmw configmap.Watcher, brds *brokerdelivery.Store, drs *dataresidency.Store) *controller.Impl {
 	brokerInformer := brokerinformer.Get(ctx)
 	bcInformer := brokercellinformer.Get(ctx)
 
@@ -91,7 +92,12 @@ func newController(ctx context.Context, cmw configmap.Watcher, drs *dataresidenc
 		dataresidencyStore: drs,
 	}
 
-	impl := brokerreconciler.NewImpl(ctx, r, brokerv1beta1.BrokerClass)
+	impl := brokerreconciler.NewImpl(ctx, r, brokerv1beta1.BrokerClass,
+		func(impl *controller.Impl) controller.Options {
+			return controller.Options{
+				ConfigStore: brds,
+			}
+		})
 
 	r.Logger.Info("Setting up event handlers")
 
diff --git a/pkg/reconciler/broker/controller_test.go b/pkg/reconciler/broker/controller_test.go
@@ -19,6 +19,7 @@ package broker
 import (
 	"testing"
 
+	"github.com/google/knative-gcp/pkg/apis/configs/brokerdelivery"
 	"github.com/google/knative-gcp/pkg/apis/configs/dataresidency"
 	. "github.com/google/knative-gcp/pkg/reconciler/testing"
 
@@ -39,7 +40,7 @@ import (
 func TestNew(t *testing.T) {
 	ctx, _ := SetupFakeContext(t)
 
-	c := NewConstructor(&dataresidency.StoreSingleton{})(ctx, configmap.NewStaticWatcher(
+	c := NewConstructor(&brokerdelivery.StoreSingleton{}, &dataresidency.StoreSingleton{})(ctx, configmap.NewStaticWatcher(
 		&corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      logging.ConfigMapName(),
@@ -61,6 +62,7 @@ func TestNew(t *testing.T) {
 			},
 			Data: map[string]string{},
 		},
+		NewBrokerDeliveryConfigMapFromDeliverySpec(nil),
 		NewDataresidencyConfigMapFromRegions([]string{}),
 	))
 
diff --git a/pkg/reconciler/testing/configmap.go b/pkg/reconciler/testing/configmap.go
