Merge pull request #61 from hasheddan/rel-1.9-bp-use-sa

[release-1.9] Fetch package pull secrets from Crossplane SA and propagate to Provider Deployments

cluster/charts/crossplane/templates/deployment.yaml

@@ -65,6 +65,10 @@ spec:
             valueFrom:
               fieldRef:
                 fieldPath: metadata.namespace
+          - name: POD_SERVICE_ACCOUNT
+            valueFrom:
+              fieldRef:
+                fieldPath: spec.serviceAccountName
           {{- if .Values.webhooks.enabled }}
           - name: "WEBHOOK_TLS_SECRET_NAME"
             value: webhook-tls-secret

cmd/crossplane/core/core.go

@@ -61,6 +61,7 @@ func (c *Command) Run() error {
 
 type startCommand struct {
 	Namespace            string `short:"n" help:"Namespace used to unpack and run packages." default:"crossplane-system" env:"POD_NAMESPACE"`
+	ServiceAccount       string `help:"Name of the Crossplane Service Account." default:"crossplane" env:"POD_SERVICE_ACCOUNT"`
 	CacheDir             string `short:"c" help:"Directory used for caching package images." default:"/cache" env:"CACHE_DIR"`
 	LeaderElection       bool   `short:"l" help:"Use leader election for the controller manager." default:"false" env:"LEADER_ELECTION"`
 	Registry             string `short:"r" help:"Default registry used to fetch packages when not specified in tag." default:"${default_registry}" env:"REGISTRY"`
@@ -130,6 +131,7 @@ func (c *startCommand) Run(s *runtime.Scheme, log logging.Logger) error { //noli
 		Options:              o,
 		Cache:                xpkg.NewFsPackageCache(c.CacheDir, afero.NewOsFs()),
 		Namespace:            c.Namespace,
+		ServiceAccount:       c.ServiceAccount,
 		DefaultRegistry:      c.Registry,
 		Features:             feats,
 		WebhookTLSSecretName: c.WebhookTLSSecretName,

internal/controller/pkg/controller/options.go

@@ -34,6 +34,9 @@ type Options struct {
 	// Namespace used to unpack and run packages.
 	Namespace string
 
+	// ServiceAccount is the core Crossplane ServiceAccount name.
+	ServiceAccount string
+
 	// DefaultRegistry used to pull packages.
 	DefaultRegistry string
 

internal/controller/pkg/manager/reconciler.go

@@ -156,7 +156,7 @@ func SetupProvider(mgr ctrl.Manager, o controller.Options) error {
 	if err != nil {
 		return errors.Wrap(err, errCreateK8sClient)
 	}
-	f, err := xpkg.NewK8sFetcher(cs, o.Namespace, o.FetcherOptions...)
+	f, err := xpkg.NewK8sFetcher(cs, append(o.FetcherOptions, xpkg.WithNamespace(o.Namespace), xpkg.WithServiceAccount(o.ServiceAccount))...)
 	if err != nil {
 		return errors.Wrap(err, errBuildFetcher)
 	}
@@ -191,7 +191,7 @@ func SetupConfiguration(mgr ctrl.Manager, o controller.Options) error {
 	if err != nil {
 		return errors.Wrap(err, "failed to initialize clientset")
 	}
-	fetcher, err := xpkg.NewK8sFetcher(clientset, o.Namespace, o.FetcherOptions...)
+	fetcher, err := xpkg.NewK8sFetcher(clientset, append(o.FetcherOptions, xpkg.WithNamespace(o.Namespace), xpkg.WithServiceAccount(o.ServiceAccount))...)
 	if err != nil {
 		return errors.Wrap(err, "cannot build fetcher")
 	}

internal/controller/pkg/resolver/reconciler.go

@@ -115,7 +115,7 @@ func Setup(mgr ctrl.Manager, o controller.Options) error {
 	if err != nil {
 		return errors.Wrap(err, "failed to initialize clientset")
 	}
-	f, err := xpkg.NewK8sFetcher(cs, o.Namespace, o.FetcherOptions...)
+	f, err := xpkg.NewK8sFetcher(cs, append(o.FetcherOptions, xpkg.WithNamespace(o.Namespace), xpkg.WithServiceAccount(o.ServiceAccount))...)
 	if err != nil {
 		return errors.Wrap(err, "cannot build fetcher")
 	}

internal/controller/pkg/revision/deployment.go

@@ -54,13 +54,14 @@ const (
 	upboundCTXValue = "uxp"
 )
 
-func buildProviderDeployment(provider *pkgmetav1.Provider, revision v1.PackageRevision, cc *v1alpha1.ControllerConfig, namespace string) (*corev1.ServiceAccount, *appsv1.Deployment, *corev1.Service) { // nolint:gocyclo
+func buildProviderDeployment(provider *pkgmetav1.Provider, revision v1.PackageRevision, cc *v1alpha1.ControllerConfig, namespace string, pullSecrets []corev1.LocalObjectReference) (*corev1.ServiceAccount, *appsv1.Deployment, *corev1.Service) { // nolint:gocyclo
 	s := &corev1.ServiceAccount{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            revision.GetName(),
 			Namespace:       namespace,
 			OwnerReferences: []metav1.OwnerReference{meta.AsController(meta.TypedReferenceTo(revision, v1.ProviderRevisionGroupVersionKind))},
 		},
+		ImagePullSecrets: pullSecrets,
 	}
 	pullPolicy := corev1.PullIfNotPresent
 	if revision.GetPackagePullPolicy() != nil {

internal/controller/pkg/revision/deployment_test.go

@@ -338,7 +338,7 @@ func TestBuildProviderDeployment(t *testing.T) {
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			sa, d, svc := buildProviderDeployment(tc.fields.provider, tc.fields.revision, tc.fields.cc, namespace)
+			sa, d, svc := buildProviderDeployment(tc.fields.provider, tc.fields.revision, tc.fields.cc, namespace, nil)
 
 			if diff := cmp.Diff(tc.want.sa, sa, cmpopts.IgnoreTypes([]metav1.OwnerReference{})); diff != "" {
 				t.Errorf("-want, +got:\n%s\n", diff)

internal/controller/pkg/revision/hook.go

@@ -36,7 +36,8 @@ import (
 const (
 	errNotProvider                   = "not a provider package"
 	errNotProviderRevision           = "not a provider revision"
-	errControllerConfig              = "cannot get referenced controller config"
+	errGetControllerConfig           = "cannot get referenced controller config"
+	errGetServiceAccount             = "cannot get Crossplane service account"
 	errDeleteProviderDeployment      = "cannot delete provider package deployment"
 	errDeleteProviderSA              = "cannot delete provider package service account"
 	errDeleteProviderService         = "cannot delete provider package service"
@@ -58,15 +59,17 @@ type Hooks interface {
 // ProviderHooks performs operations for a provider package that requires a
 // controller before and after the revision establishes objects.
 type ProviderHooks struct {
-	client    resource.ClientApplicator
-	namespace string
+	client         resource.ClientApplicator
+	namespace      string
+	serviceAccount string
 }
 
 // NewProviderHooks creates a new ProviderHooks.
-func NewProviderHooks(client resource.ClientApplicator, namespace string) *ProviderHooks {
+func NewProviderHooks(client resource.ClientApplicator, namespace, serviceAccount string) *ProviderHooks {
 	return &ProviderHooks{
-		client:    client,
-		namespace: namespace,
+		client:         client,
+		namespace:      namespace,
+		serviceAccount: serviceAccount,
 	}
 }
 
@@ -92,11 +95,10 @@ func (h *ProviderHooks) Pre(ctx context.Context, pkg runtime.Object, pr v1.Packa
 	if pr.GetDesiredState() != v1.PackageRevisionInactive {
 		return nil
 	}
-	cc, err := h.getControllerConfig(ctx, pr)
-	if err != nil {
-		return errors.Wrap(err, errControllerConfig)
-	}
-	s, d, svc := buildProviderDeployment(pkgProvider, pr, cc, h.namespace)
+
+	// NOTE(hasheddan): we avoid fetching pull secrets and controller config as
+	// they aren't needed to delete Deployment, ServiceAccount, and Service.
+	s, d, svc := buildProviderDeployment(pkgProvider, pr, nil, h.namespace, []corev1.LocalObjectReference{})
 	if err := h.client.Delete(ctx, d); resource.IgnoreNotFound(err) != nil {
 		return errors.Wrap(err, errDeleteProviderDeployment)
 	}
@@ -122,9 +124,13 @@ func (h *ProviderHooks) Post(ctx context.Context, pkg runtime.Object, pr v1.Pack
 	}
 	cc, err := h.getControllerConfig(ctx, pr)
 	if err != nil {
-		return errors.Wrap(err, errControllerConfig)
+		return err
+	}
+	ps, err := h.getSAPullSecrets(ctx)
+	if err != nil {
+		return err
 	}
-	s, d, svc := buildProviderDeployment(pkgProvider, pr, cc, h.namespace)
+	s, d, svc := buildProviderDeployment(pkgProvider, pr, cc, h.namespace, append(pr.GetPackagePullSecrets(), ps...))
 	if err := h.client.Apply(ctx, s); err != nil {
 		return errors.Wrap(err, errApplyProviderSA)
 	}
@@ -149,15 +155,24 @@ func (h *ProviderHooks) Post(ctx context.Context, pkg runtime.Object, pr v1.Pack
 	return nil
 }
 
+func (h *ProviderHooks) getSAPullSecrets(ctx context.Context) ([]corev1.LocalObjectReference, error) {
+	sa := &corev1.ServiceAccount{}
+	if err := h.client.Get(ctx, types.NamespacedName{
+		Namespace: h.namespace,
+		Name:      h.serviceAccount,
+	}, sa); err != nil {
+		return []corev1.LocalObjectReference{}, errors.Wrap(err, errGetServiceAccount)
+	}
+	return sa.ImagePullSecrets, nil
+}
+
 func (h *ProviderHooks) getControllerConfig(ctx context.Context, pr v1.PackageRevision) (*v1alpha1.ControllerConfig, error) {
-	var cc *v1alpha1.ControllerConfig
-	if pr.GetControllerConfigRef() != nil {
-		cc = &v1alpha1.ControllerConfig{}
-		if err := h.client.Get(ctx, types.NamespacedName{Name: pr.GetControllerConfigRef().Name}, cc); err != nil {
-			return nil, errors.Wrap(err, errControllerConfig)
-		}
+	if pr.GetControllerConfigRef() == nil {
+		return nil, nil
 	}
-	return cc, nil
+	cc := &v1alpha1.ControllerConfig{}
+	err := h.client.Get(ctx, types.NamespacedName{Name: pr.GetControllerConfigRef().Name}, cc)
+	return cc, errors.Wrap(err, errGetControllerConfig)
 }
 
 // ConfigurationHooks performs operations for a configuration package before and