diff --git a/internal/controller/pkg/revision/runtime_function.go b/internal/controller/pkg/revision/runtime_function.go index b0bed843912..804e0203cbb 100644 --- a/internal/controller/pkg/revision/runtime_function.go +++ b/internal/controller/pkg/revision/runtime_function.go @@ -137,7 +137,7 @@ func (h *FunctionHooks) Post(ctx context.Context, pkg runtime.Object, pr v1.Pack // `deploymentTemplate.spec.template.spec.serviceAccountName` in the // DeploymentRuntimeConfig. if sa.Name == d.Spec.Template.Spec.ServiceAccountName { - if err := h.client.Apply(ctx, sa); err != nil { + if err := applySA(ctx, h.client, sa); err != nil { return errors.Wrap(err, errApplyFunctionSA) } } diff --git a/internal/controller/pkg/revision/runtime_function_test.go b/internal/controller/pkg/revision/runtime_function_test.go index 87cd9e39e0a..0c9f5f73f5e 100644 --- a/internal/controller/pkg/revision/runtime_function_test.go +++ b/internal/controller/pkg/revision/runtime_function_test.go @@ -398,6 +398,56 @@ func TestFunctionPostHook(t *testing.T) { }, }, }, + "SuccessWithExtraSecret": { + reason: "Should not return error if successfully applied service account with additional secret.", + args: args{ + pkg: &pkgmetav1beta1.Function{}, + rev: &v1beta1.FunctionRevision{ + Spec: v1beta1.FunctionRevisionSpec{ + PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, + DesiredState: v1.PackageRevisionActive, + }, + }, + }, + manifests: &MockManifestBuilder{ + ServiceAccountFn: func(_ ...ServiceAccountOverride) *corev1.ServiceAccount { + return &corev1.ServiceAccount{} + }, + DeploymentFn: func(_ string, _ ...DeploymentOverride) *appsv1.Deployment { + return &appsv1.Deployment{} + }, + }, + client: &test.MockClient{ + MockGet: func(_ context.Context, _ client.ObjectKey, obj client.Object) error { + if sa, ok := obj.(*corev1.ServiceAccount); ok { + sa.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "test_secret"}} + } + return nil + }, + MockPatch: func(_ context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) error { + if d, ok := obj.(*appsv1.Deployment); ok { + d.Status.Conditions = []appsv1.DeploymentCondition{{ + Type: appsv1.DeploymentAvailable, + Status: corev1.ConditionTrue, + }} + return nil + } + return nil + }, + }, + }, + want: want{ + rev: &v1beta1.FunctionRevision{ + Spec: v1beta1.FunctionRevisionSpec{ + PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: functionImage, + DesiredState: v1.PackageRevisionActive, + }, + }, + }, + }, + }, "SuccessfulWithExternallyManagedSA": { reason: "Should be successful without creating an SA, when the SA is managed externally", args: args{ diff --git a/internal/controller/pkg/revision/runtime_provider.go b/internal/controller/pkg/revision/runtime_provider.go index 0be6d63c18c..151309a492c 100644 --- a/internal/controller/pkg/revision/runtime_provider.go +++ b/internal/controller/pkg/revision/runtime_provider.go @@ -24,6 +24,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane/crossplane-runtime/pkg/errors" @@ -154,7 +155,7 @@ func (h *ProviderHooks) Post(ctx context.Context, pkg runtime.Object, pr v1.Pack // `deploymentTemplate.spec.template.spec.serviceAccountName` in the // DeploymentRuntimeConfig. if sa.Name == d.Spec.Template.Spec.ServiceAccountName { - if err := h.client.Apply(ctx, sa); err != nil { + if err := applySA(ctx, h.client, sa); err != nil { return errors.Wrap(err, errApplyProviderSA) } } @@ -292,3 +293,23 @@ func getProviderImage(pm *pkgmetav1.Provider, pr v1.PackageRevisionWithRuntime, return ref.Name(), nil } + +// applySA creates/updates a ServiceAccount and includes any image pull secrets +// that have been added by external controllers. +func applySA(ctx context.Context, cl resource.ClientApplicator, sa *corev1.ServiceAccount) error { + oldSa := &corev1.ServiceAccount{} + if err := cl.Get(ctx, types.NamespacedName{Name: sa.Name, Namespace: sa.Namespace}, oldSa); err == nil { + // Add pull secrets created by other controllers + existingSecrets := make(map[string]bool) + for _, secret := range sa.ImagePullSecrets { + existingSecrets[secret.Name] = true + } + + for _, secret := range oldSa.ImagePullSecrets { + if !existingSecrets[secret.Name] { + sa.ImagePullSecrets = append(sa.ImagePullSecrets, secret) + } + } + } + return cl.Apply(ctx, sa) +} diff --git a/internal/controller/pkg/revision/runtime_provider_test.go b/internal/controller/pkg/revision/runtime_provider_test.go index 467c99f360e..5e2bb025311 100644 --- a/internal/controller/pkg/revision/runtime_provider_test.go +++ b/internal/controller/pkg/revision/runtime_provider_test.go @@ -468,6 +468,56 @@ func TestProviderPostHook(t *testing.T) { }, }, }, + "SuccessWithExtraSecret": { + reason: "Should not return error if successfully applied service account with additional secret.", + args: args{ + pkg: &pkgmetav1.Provider{}, + rev: &v1.ProviderRevision{ + Spec: v1.ProviderRevisionSpec{ + PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, + DesiredState: v1.PackageRevisionActive, + }, + }, + }, + manifests: &MockManifestBuilder{ + ServiceAccountFn: func(_ ...ServiceAccountOverride) *corev1.ServiceAccount { + return &corev1.ServiceAccount{} + }, + DeploymentFn: func(_ string, _ ...DeploymentOverride) *appsv1.Deployment { + return &appsv1.Deployment{} + }, + }, + client: &test.MockClient{ + MockGet: func(_ context.Context, _ client.ObjectKey, obj client.Object) error { + if sa, ok := obj.(*corev1.ServiceAccount); ok { + sa.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "test_secret"}} + } + return nil + }, + MockPatch: func(_ context.Context, obj client.Object, _ client.Patch, _ ...client.PatchOption) error { + if d, ok := obj.(*appsv1.Deployment); ok { + d.Status.Conditions = []appsv1.DeploymentCondition{{ + Type: appsv1.DeploymentAvailable, + Status: corev1.ConditionTrue, + }} + return nil + } + return nil + }, + }, + }, + want: want{ + rev: &v1.ProviderRevision{ + Spec: v1.ProviderRevisionSpec{ + PackageRevisionSpec: v1.PackageRevisionSpec{ + Package: providerImage, + DesiredState: v1.PackageRevisionActive, + }, + }, + }, + }, + }, "SuccessfulWithExternallyManagedSA": { reason: "Should be successful without creating an SA, when the SA is managed externally", args: args{