Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for missing pull secrets in auto created namespaces #2245

Merged
merged 13 commits into from
Dec 13, 2019
2 changes: 2 additions & 0 deletions config/500-controller.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
bvennam marked this conversation as resolved.
Show resolved Hide resolved
ports:
- containerPort: 9090
name: metrics
Expand Down
8 changes: 8 additions & 0 deletions pkg/reconciler/namespace/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package namespace
import (
"context"

"github.com/kelseyhightower/envconfig"
"knative.dev/pkg/configmap"
"knative.dev/pkg/controller"
"knative.dev/pkg/tracker"
Expand Down Expand Up @@ -56,6 +57,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()

Expand Down
59 changes: 59 additions & 0 deletions pkg/reconciler/namespace/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package namespace

import (
"context"
"errors"
"fmt"

"k8s.io/client-go/tools/cache"
Expand All @@ -36,12 +37,17 @@ import (
apierrs "k8s.io/apimachinery/pkg/api/errors"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"knative.dev/eventing/pkg/apis/eventing/v1alpha1"
"knative.dev/eventing/pkg/logging"
"knative.dev/eventing/pkg/reconciler"
"knative.dev/pkg/controller"
)

type envConfig struct {
bvennam marked this conversation as resolved.
Show resolved Hide resolved
BrokerPullSecretName string `envconfig:"BROKER_IMAGE_PULL_SECRET_NAME" required:"false"`
}

const (
namespaceReconciled = "NamespaceReconciled"
namespaceReconcileFailure = "NamespaceReconcileFailure"
Expand All @@ -50,6 +56,8 @@ const (
brokerCreated = "BrokerCreated"
serviceAccountCreated = "BrokerServiceAccountCreated"
serviceAccountRBACCreated = "BrokerServiceAccountRBACCreated"
envConfigProcessed = "EnvConfigProcessed"
secretCopied = "SecretCopied"
)

var (
Expand All @@ -61,6 +69,8 @@ var (
type Reconciler struct {
*reconciler.Base

brokerPullSecretName string

// listers index properties about resources
namespaceLister corev1listers.NamespaceLister
serviceAccountLister corev1listers.ServiceAccountLister
Expand Down Expand Up @@ -141,6 +151,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)
Expand Down Expand Up @@ -174,6 +185,14 @@ 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 {
bvennam marked this conversation as resolved.
Show resolved Hide resolved
_, err := CopySecret(r, system.Namespace(), r.brokerPullSecretName, ns.Name, sa.Name)
if err != nil {
r.Recorder.Event(ns, corev1.EventTypeNormal, secretCopied,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably not be an EventTypeNormal, but let's fix this in a followup PR. I can do this.

fmt.Sprintf("Error copying secret: %s", err))
}
}

return nil
}

Expand Down Expand Up @@ -237,3 +256,43 @@ func (r *Reconciler) reconcileBroker(ctx context.Context, ns *corev1.Namespace)
// Don't update anything that is already present.
return current, nil
}

func CopySecret(r *Reconciler, srcNS string, srcSecretName string, tgtNS string, svcAccount string) (*corev1.Secret, error) {
tgtNamespaceSvcAcct := r.KubeClientSet.CoreV1().ServiceAccounts(tgtNS)
srcSecrets := r.KubeClientSet.CoreV1().Secrets(srcNS)
tgtNamespaceSecrets := r.KubeClientSet.CoreV1().Secrets(tgtNS)

// First try to find the secret we're supposed to copy
srcSecret, err := srcSecrets.Get(srcSecretName, metav1.GetOptions{})
if err != nil {
return nil, err
}

// check for nil source secret
if srcSecret == nil {
return nil, errors.New("error copying secret")
bvennam marked this conversation as resolved.
Show resolved Hide resolved
}

// Found the secret, so now make a copy in our new namespace
newSecret, err := tgtNamespaceSecrets.Create(
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: srcSecretName,
},
Data: srcSecret.Data,
Type: srcSecret.Type,
})

// If the secret already exists then that's ok - may have already been created
if err != nil && !apierrs.IsAlreadyExists(err) {
return nil, fmt.Errorf("error copying the Secret: %s", err)
}

_, err = tgtNamespaceSvcAcct.Patch(svcAccount, types.StrategicMergePatchType,
[]byte(`{"imagePullSecrets":[{"name":"`+srcSecretName+`"}]}`))
if err != nil {
return nil, fmt.Errorf("patch failed on NS/SA (%s/%s): %s",
tgtNS, srcSecretName, err)
}
return newSecret, nil
}