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
64 changes: 64 additions & 0 deletions pkg/reconciler/namespace/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ package namespace

import (
"context"
"errors"
"fmt"

"github.com/kelseyhightower/envconfig"
"k8s.io/client-go/tools/cache"
"knative.dev/eventing/pkg/reconciler/namespace/resources"
"knative.dev/eventing/pkg/utils"
Expand All @@ -36,12 +38,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 +57,8 @@ const (
brokerCreated = "BrokerCreated"
serviceAccountCreated = "BrokerServiceAccountCreated"
serviceAccountRBACCreated = "BrokerServiceAccountRBACCreated"
envConfigProcessed = "EnvConfigProcessed"
secretCopied = "SecretCopied"
)

var (
Expand Down Expand Up @@ -141,6 +150,13 @@ 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 {

var env envConfig
if err := envconfig.Process("", &env); err != nil {
r.Recorder.Event(ns, corev1.EventTypeNormal, envConfigProcessed,
bvennam marked this conversation as resolved.
Show resolved Hide resolved
fmt.Sprintf("Failed to process env var %s", err))
}

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 +190,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, "default", env.BrokerPullSecretName, ns.Name, sa.Name)
bvennam marked this conversation as resolved.
Show resolved Hide resolved
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 +261,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) {
tgtNSSvcAccI := r.KubeClientSet.CoreV1().ServiceAccounts(tgtNS)
srcSecI := r.KubeClientSet.CoreV1().Secrets(srcNS)
tgtNSSecI := r.KubeClientSet.CoreV1().Secrets(tgtNS)

// First try to find the secret we're supposed to copy
srcSecret, err := srcSecI.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 := tgtNSSecI.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 = tgtNSSvcAccI.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
}