Fix for missing pull secrets in auto created namespaces

config/500-controller.yaml

@@ -56,6 +56,8 @@ spec:
             value: knative.dev/eventing/cmd/broker/filter
           - name: BROKER_FILTER_SERVICE_ACCOUNT
             value: eventing-broker-filter
+          - name: BROKER_IMAGE_PULL_SECRET_NAME
+            value: broker-image-pull-secret
         ports:
           - containerPort: 9090
             name: metrics

pkg/reconciler/namespace/controller.go

@@ -19,6 +19,7 @@ package namespace
 import (
 	"context"
 
+	"github.com/kelseyhightower/envconfig"
 	"knative.dev/pkg/configmap"
 	"knative.dev/pkg/controller"
 	"knative.dev/pkg/tracker"
@@ -31,6 +32,10 @@ import (
 	"knative.dev/pkg/client/injection/kube/informers/rbac/v1/rolebinding"
 )
 
+type envConfig struct {
+	BrokerPullSecretName string `envconfig:"BROKER_IMAGE_PULL_SECRET_NAME" required:"false"`
+}
+
 const (
 	// ReconcilerName is the name of the reconciler
 	ReconcilerName = "Namespace" // TODO: Namespace is not a very good name for this controller.
@@ -56,6 +61,13 @@ func NewController(
 		Base:            reconciler.NewBase(ctx, controllerAgentName, cmw),
 		namespaceLister: namespaceInformer.Lister(),
 	}
+
+	var env envConfig
+	if err := envconfig.Process("", &env); err != nil {
+		r.Logger.Info("no broker image pull secret name defined")
+	}
+	r.brokerPullSecretName = env.BrokerPullSecretName
+
 	impl := controller.NewImpl(r, r.Logger, ReconcilerName)
 	// TODO: filter label selector: on InjectionEnabledLabels()
 

pkg/reconciler/namespace/namespace.go

@@ -50,6 +50,7 @@ const (
 	brokerCreated             = "BrokerCreated"
 	serviceAccountCreated     = "BrokerServiceAccountCreated"
 	serviceAccountRBACCreated = "BrokerServiceAccountRBACCreated"
+	secretCopied              = "SecretCopied"
 )
 
 var (
@@ -61,6 +62,8 @@ var (
 type Reconciler struct {
 	*reconciler.Base
 
+	brokerPullSecretName string
+
 	// listers index properties about resources
 	namespaceLister      corev1listers.NamespaceLister
 	serviceAccountLister corev1listers.ServiceAccountLister
@@ -141,6 +144,7 @@ func (r *Reconciler) reconcile(ctx context.Context, ns *corev1.Namespace) error
 // reconcileServiceAccountAndRoleBinding reconciles the service account and role binding for
 // Namespace 'ns'.
 func (r *Reconciler) reconcileServiceAccountAndRoleBindings(ctx context.Context, ns *corev1.Namespace, saName, rbName, clusterRoleName, configClusterRoleName string) error {
+
 	sa, err := r.reconcileBrokerServiceAccount(ctx, ns, resources.MakeServiceAccount(ns.Name, saName))
 	if err != nil {
 		return fmt.Errorf("service account '%s': %v", saName, err)
@@ -174,6 +178,27 @@ func (r *Reconciler) reconcileServiceAccountAndRoleBindings(ctx context.Context,
 		return fmt.Errorf("track role binding '%s': %v", rb.Name, err)
 	}
 
+	if sa.Name == resources.IngressServiceAccountName || sa.Name == resources.FilterServiceAccountName {
+		// check for existence of brokerPullSecret, and skip copy if it already exists
+		secretFound := false
+		for _, v := range sa.ImagePullSecrets {
+			if fmt.Sprintf("%s", v) == ("{" + r.brokerPullSecretName + "}") {
+				secretFound = true
+				break
+			}
+		}
+		if (!secretFound) {
+			_, err := utils.CopySecret(r.KubeClientSet.CoreV1(), system.Namespace(), r.brokerPullSecretName, ns.Name, sa.Name)
+			if err != nil {
+				r.Recorder.Event(ns, corev1.EventTypeNormal, secretCopied,
+					fmt.Sprintf("Error copying secret: %s", err))
+			} else {
+				r.Recorder.Event(ns, corev1.EventTypeNormal, secretCopied,
+					fmt.Sprintf("Secret copied into namespace: %s", sa.Name))
+			}
+		}
+	}
+
 	return nil
 }
 

pkg/reconciler/namespace/namespace_test.go

@@ -33,6 +33,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/scheme"
+	clientgotesting "k8s.io/client-go/testing"
 	"knative.dev/eventing/pkg/apis/eventing/v1alpha1"
 	eventingv1alpha1 "knative.dev/eventing/pkg/apis/eventing/v1alpha1"
 	"knative.dev/eventing/pkg/reconciler"
@@ -43,7 +44,8 @@ import (
 )
 
 const (
-	testNS = "test-namespace"
+	testNS                    = "test-namespace"
+	brokerImagePullSecretName = "broker-image-pull-secret"
 )
 
 var (
@@ -80,8 +82,15 @@ func TestAllCases(t *testing.T) {
 	rbFilterConfigEvent := Eventf(corev1.EventTypeNormal, "BrokerServiceAccountRBACCreated", "RoleBinding 'knative-testing/eventing-broker-filter-test-namespace' created for the Broker")
 	brokerEvent := Eventf(corev1.EventTypeNormal, "BrokerCreated", "Default eventing.knative.dev Broker created.")
 	nsEvent := Eventf(corev1.EventTypeNormal, "NamespaceReconciled", "Namespace reconciled: \"test-namespace\"")
+	secretEventFilter := Eventf(corev1.EventTypeNormal, "SecretCopied", "Secret copied into namespace: eventing-broker-filter")
+	secretEventIngress := Eventf(corev1.EventTypeNormal, "SecretCopied", "Secret copied into namespace: eventing-broker-ingress")
+
+	// Patches
+	ingressPatch := createPatch(testNS, "eventing-broker-ingress")
+	filterPatch := createPatch(testNS, "eventing-broker-filter")
 
 	// Object
+	secret := resources.MakeSecret(brokerImagePullSecretName)
 	broker := resources.MakeBroker(testNS)
 	saIngress := resources.MakeServiceAccount(testNS, resources.IngressServiceAccountName)
 	rbIngress := resources.MakeRoleBinding(resources.IngressRoleBindingName, testNS, resources.MakeServiceAccount(testNS, resources.IngressServiceAccountName), resources.IngressClusterRoleName)
@@ -137,20 +146,29 @@ func TestAllCases(t *testing.T) {
 			saIngressEvent,
 			rbIngressEvent,
 			rbIngressConfigEvent,
+			secretEventIngress,
 			saFilterEvent,
 			rbFilterEvent,
 			rbFilterConfigEvent,
+			secretEventFilter,
 			brokerEvent,
 			nsEvent,
 		},
 		WantCreates: []runtime.Object{
 			broker,
+			secret,
 			saIngress,
 			rbIngress,
 			rbIngressConfig,
+			secret,
 			saFilter,
 			rbFilter,
 			rbFilterConfig,
+			secret,
+		},
+		WantPatches: []clientgotesting.PatchActionImpl{
+			ingressPatch,
+			filterPatch,
 		},
 	}, {
 		Name: "Namespace enabled, broker exists",
@@ -167,18 +185,27 @@ func TestAllCases(t *testing.T) {
 			saIngressEvent,
 			rbIngressEvent,
 			rbIngressConfigEvent,
+			secretEventIngress,
 			saFilterEvent,
 			rbFilterEvent,
 			rbFilterConfigEvent,
+			secretEventFilter,
 			nsEvent,
 		},
 		WantCreates: []runtime.Object{
+			secret,
 			saIngress,
 			rbIngress,
 			rbIngressConfig,
+			secret,
 			saFilter,
 			rbFilter,
 			rbFilterConfig,
+			secret,
+		},
+		WantPatches: []clientgotesting.PatchActionImpl{
+			ingressPatch,
+			filterPatch,
 		},
 	}, {
 		Name: "Namespace enabled, broker exists with no label",
@@ -210,19 +237,28 @@ func TestAllCases(t *testing.T) {
 		WantEvents: []string{
 			rbIngressEvent,
 			rbIngressConfigEvent,
+			secretEventIngress,
 			saFilterEvent,
 			rbFilterEvent,
 			rbFilterConfigEvent,
+			secretEventFilter,
 			brokerEvent,
 			nsEvent,
 		},
 		WantCreates: []runtime.Object{
 			broker,
+			secret,
 			rbIngress,
 			rbIngressConfig,
+			secret,
 			saFilter,
 			rbFilter,
 			rbFilterConfig,
+			secret,
+		},
+		WantPatches: []clientgotesting.PatchActionImpl{
+			ingressPatch,
+			filterPatch,
 		},
 	}, {
 		Name: "Namespace enabled, ingress role binding exists",
@@ -238,18 +274,27 @@ func TestAllCases(t *testing.T) {
 		WantErr:                 false,
 		WantEvents: []string{
 			saIngressEvent,
+			secretEventIngress,
 			saFilterEvent,
 			rbFilterEvent,
 			rbFilterConfigEvent,
+			secretEventFilter,
 			brokerEvent,
 			nsEvent,
 		},
 		WantCreates: []runtime.Object{
 			broker,
+			secret,
 			saIngress,
+			secret,
 			saFilter,
 			rbFilter,
 			rbFilterConfig,
+			secret,
+		},
+		WantPatches: []clientgotesting.PatchActionImpl{
+			ingressPatch,
+			filterPatch,
 		},
 	}, {
 		Name: "Namespace enabled, filter service account exists",
@@ -266,18 +311,27 @@ func TestAllCases(t *testing.T) {
 			saIngressEvent,
 			rbIngressEvent,
 			rbIngressConfigEvent,
+			secretEventIngress,
 			rbFilterEvent,
 			rbFilterConfigEvent,
+			secretEventFilter,
 			brokerEvent,
 			nsEvent,
 		},
 		WantCreates: []runtime.Object{
 			broker,
+			secret,
 			saIngress,
 			rbIngress,
 			rbIngressConfig,
+			secret,
 			rbFilter,
 			rbFilterConfig,
+			secret,
+		},
+		WantPatches: []clientgotesting.PatchActionImpl{
+			ingressPatch,
+			filterPatch,
 		},
 	}, {
 		Name: "Namespace enabled, filter role binding exists",
@@ -295,30 +349,67 @@ func TestAllCases(t *testing.T) {
 			saIngressEvent,
 			rbIngressEvent,
 			rbIngressConfigEvent,
+			secretEventIngress,
 			saFilterEvent,
+			secretEventFilter,
 			brokerEvent,
 			nsEvent,
 		},
 		WantCreates: []runtime.Object{
 			broker,
+			secret,
 			saIngress,
 			rbIngress,
 			rbIngressConfig,
+			secret,
 			saFilter,
+			secret,
+		},
+		WantPatches: []clientgotesting.PatchActionImpl{
+			ingressPatch,
+			filterPatch,
 		},
 	},
 	// TODO: we need a existing default un-owned test.
 	}
 
 	logger := logtesting.TestLogger(t)
+	// used to determine which test we are on
+	testNum := 0
 	table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler {
-		return &Reconciler{
+
+		r := &Reconciler{
 			Base:                 reconciler.NewBase(ctx, controllerAgentName, cmw),
 			namespaceLister:      listers.GetNamespaceLister(),
 			brokerLister:         listers.GetBrokerLister(),
 			serviceAccountLister: listers.GetServiceAccountLister(),
 			roleBindingLister:    listers.GetRoleBindingLister(),
 			tracker:              tracker.New(func(types.NamespacedName) {}, 0),
+			brokerPullSecretName: brokerImagePullSecretName,
+		}
+
+		// only create secret in required tests
+		createSecretTests := []string{"Namespace enabled", "Namespace enabled, broker exists",
+			"Namespace enabled, ingress service account exists", "Namespace enabled, ingress role binding exists",
+			"Namespace enabled, filter service account exists", "Namespace enabled, filter role binding exists"}
+
+		for _, theTest := range createSecretTests {
+			if theTest == table[testNum].Name {
+				// create the required secret in knative-eventing to be copied into required namespaces
+				tgtNSSecrets := r.KubeClientSet.CoreV1().Secrets(system.Namespace())
+				tgtNSSecrets.Create(resources.MakeSecret(brokerImagePullSecretName))
+				break
+			}
 		}
+		testNum++
+		return r
 	}, false, logger))
 }
+
+func createPatch(namespace string, name string) clientgotesting.PatchActionImpl {
+	patch := clientgotesting.PatchActionImpl{}
+	patch.Namespace = namespace
+	patch.Name = name
+	patch.Patch = []byte(`{"imagePullSecrets":[{"name":"` + brokerImagePullSecretName + `"}]}`)
+	return patch
+}

pkg/reconciler/namespace/resources/secret.go

@@ -0,0 +1,31 @@
+/*
+Copyright 2019 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 resources
+
+import (
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// MakeSecret creates a Secret object.
+func MakeSecret(name string) *corev1.Secret {
+	return &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+		},
+	}
+}