Skip to content

Commit d0a80f0

Browse files
committed
Reconcile a Service Account for the pgBackRest Repo Host
Currently, the pgBackRest repo host uses the 'default' service account. However, EKS's IAM role integration requires a specific annotation to enable this feature. This change adds a new SA for the repo host to allow PGO to reconcile a SA with this annotation, thus allowing the IAM integration to work as expected. fixes #4006 Issue: PGO-2123
1 parent 92f035e commit d0a80f0

File tree

3 files changed

+94
-10
lines changed

3 files changed

+94
-10
lines changed

internal/controller/postgrescluster/pgbackrest.go

+43-6
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,9 @@ type RepoResources struct {
122122
// strategy.
123123
func (r *Reconciler) applyRepoHostIntent(ctx context.Context, postgresCluster *v1beta1.PostgresCluster,
124124
repoHostName string, repoResources *RepoResources,
125-
observedInstances *observedInstances) (*appsv1.StatefulSet, error) {
125+
observedInstances *observedInstances, saName string) (*appsv1.StatefulSet, error) {
126126

127-
repo, err := r.generateRepoHostIntent(ctx, postgresCluster, repoHostName, repoResources, observedInstances)
127+
repo, err := r.generateRepoHostIntent(ctx, postgresCluster, repoHostName, repoResources, observedInstances, saName)
128128
if err != nil {
129129
return nil, err
130130
}
@@ -567,7 +567,7 @@ func (r *Reconciler) setScheduledJobStatus(ctx context.Context,
567567
// as needed to create and reconcile a pgBackRest dedicated repository host within the kubernetes
568568
// cluster.
569569
func (r *Reconciler) generateRepoHostIntent(ctx context.Context, postgresCluster *v1beta1.PostgresCluster,
570-
repoHostName string, repoResources *RepoResources, observedInstances *observedInstances,
570+
repoHostName string, repoResources *RepoResources, observedInstances *observedInstances, saName string,
571571
) (*appsv1.StatefulSet, error) {
572572

573573
annotations := naming.Merge(
@@ -681,6 +681,8 @@ func (r *Reconciler) generateRepoHostIntent(ctx context.Context, postgresCluster
681681

682682
repo.Spec.Template.Spec.SecurityContext = postgres.PodSecurityContext(postgresCluster)
683683

684+
repo.Spec.Template.Spec.ServiceAccountName = saName
685+
684686
pgbackrest.AddServerToRepoPod(ctx, postgresCluster, &repo.Spec.Template.Spec)
685687

686688
if pgbackrest.RepoHostVolumeDefined(postgresCluster) {
@@ -1380,10 +1382,18 @@ func (r *Reconciler) reconcilePGBackRest(ctx context.Context,
13801382
return result, nil
13811383
}
13821384

1385+
// reconcile the RBAC required to run the pgBackRest Repo Host
1386+
repoHostSA, err := r.reconcileRepoHostRBAC(ctx, postgresCluster)
1387+
if err != nil {
1388+
log.Error(err, "unable to reconcile pgBackRest repo host RBAC")
1389+
result.Requeue = true
1390+
return result, nil
1391+
}
1392+
13831393
var repoHost *appsv1.StatefulSet
13841394
var repoHostName string
13851395
// reconcile the pgbackrest repository host
1386-
repoHost, err = r.reconcileDedicatedRepoHost(ctx, postgresCluster, repoResources, instances)
1396+
repoHost, err = r.reconcileDedicatedRepoHost(ctx, postgresCluster, repoResources, instances, repoHostSA.GetName())
13871397
if err != nil {
13881398
log.Error(err, "unable to reconcile pgBackRest repo host")
13891399
result.Requeue = true
@@ -2118,12 +2128,39 @@ func (r *Reconciler) reconcilePGBackRestRBAC(ctx context.Context,
21182128
return sa, nil
21192129
}
21202130

2131+
// +kubebuilder:rbac:groups="",resources="serviceaccounts",verbs={create,patch}
2132+
2133+
// reconcileRepoHostRBAC reconciles the ServiceAccount for the pgBackRest repo host
2134+
func (r *Reconciler) reconcileRepoHostRBAC(ctx context.Context,
2135+
postgresCluster *v1beta1.PostgresCluster) (*corev1.ServiceAccount, error) {
2136+
2137+
sa := &corev1.ServiceAccount{ObjectMeta: naming.RepoHostRBAC(postgresCluster)}
2138+
sa.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ServiceAccount"))
2139+
2140+
if err := r.setControllerReference(postgresCluster, sa); err != nil {
2141+
return nil, errors.WithStack(err)
2142+
}
2143+
2144+
sa.Annotations = naming.Merge(postgresCluster.Spec.Metadata.GetAnnotationsOrNil(),
2145+
postgresCluster.Spec.Backups.PGBackRest.Metadata.GetAnnotationsOrNil())
2146+
sa.Labels = naming.Merge(postgresCluster.Spec.Metadata.GetLabelsOrNil(),
2147+
postgresCluster.Spec.Backups.PGBackRest.Metadata.GetLabelsOrNil(),
2148+
naming.PGBackRestLabels(postgresCluster.GetName()))
2149+
2150+
if err := r.apply(ctx, sa); err != nil {
2151+
return nil, errors.WithStack(err)
2152+
}
2153+
2154+
return sa, nil
2155+
}
2156+
21212157
// reconcileDedicatedRepoHost is responsible for reconciling a pgBackRest dedicated repository host
21222158
// StatefulSet according to a specific PostgresCluster custom resource.
21232159
func (r *Reconciler) reconcileDedicatedRepoHost(ctx context.Context,
21242160
postgresCluster *v1beta1.PostgresCluster,
21252161
repoResources *RepoResources,
2126-
observedInstances *observedInstances) (*appsv1.StatefulSet, error) {
2162+
observedInstances *observedInstances,
2163+
saName string) (*appsv1.StatefulSet, error) {
21272164

21282165
log := logging.FromContext(ctx).WithValues("reconcileResource", "repoHost")
21292166

@@ -2164,7 +2201,7 @@ func (r *Reconciler) reconcileDedicatedRepoHost(ctx context.Context,
21642201
}
21652202
repoHostName := repoResources.hosts[0].Name
21662203
repoHost, err := r.applyRepoHostIntent(ctx, postgresCluster, repoHostName, repoResources,
2167-
observedInstances)
2204+
observedInstances, saName)
21682205
if err != nil {
21692206
log.Error(err, "reconciling repository host")
21702207
return nil, err

internal/controller/postgrescluster/pgbackrest_test.go

+42-4
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,8 @@ schedulerName: default-scheduler
328328
securityContext:
329329
fsGroup: 26
330330
fsGroupChangePolicy: OnRootMismatch
331+
serviceAccount: hippocluster-repohost
332+
serviceAccountName: hippocluster-repohost
331333
shareProcessNamespace: true
332334
terminationGracePeriodSeconds: 30
333335
tolerations:
@@ -724,6 +726,42 @@ func TestReconcilePGBackRestRBAC(t *testing.T) {
724726
assert.Assert(t, foundSubject)
725727
}
726728

729+
func TestReconcileRepoHostRBAC(t *testing.T) {
730+
// Garbage collector cleans up test resources before the test completes
731+
if strings.EqualFold(os.Getenv("USE_EXISTING_CLUSTER"), "true") {
732+
t.Skip("USE_EXISTING_CLUSTER: Test fails due to garbage collection")
733+
}
734+
735+
ctx := context.Background()
736+
_, tClient := setupKubernetes(t)
737+
require.ParallelCapacity(t, 0)
738+
739+
r := &Reconciler{Client: tClient, Owner: client.FieldOwner(t.Name())}
740+
741+
clusterName := "hippocluster"
742+
clusterUID := "hippouid"
743+
744+
ns := setupNamespace(t, tClient)
745+
746+
// create a PostgresCluster to test with
747+
postgresCluster := fakePostgresCluster(clusterName, ns.GetName(), clusterUID, true)
748+
postgresCluster.Status.PGBackRest = &v1beta1.PGBackRestStatus{
749+
Repos: []v1beta1.RepoStatus{{Name: "repo1", StanzaCreated: false}},
750+
}
751+
752+
serviceAccount, err := r.reconcileRepoHostRBAC(ctx, postgresCluster)
753+
assert.NilError(t, err)
754+
assert.Assert(t, serviceAccount != nil)
755+
756+
// verify the service account has been created
757+
sa := &corev1.ServiceAccount{}
758+
err = tClient.Get(ctx, types.NamespacedName{
759+
Name: naming.RepoHostRBAC(postgresCluster).Name,
760+
Namespace: postgresCluster.GetNamespace(),
761+
}, sa)
762+
assert.NilError(t, err)
763+
}
764+
727765
func TestReconcileStanzaCreate(t *testing.T) {
728766
cfg, tClient := setupKubernetes(t)
729767
require.ParallelCapacity(t, 0)
@@ -2672,12 +2710,12 @@ func TestGenerateRepoHostIntent(t *testing.T) {
26722710

26732711
t.Run("empty", func(t *testing.T) {
26742712
_, err := r.generateRepoHostIntent(ctx, &v1beta1.PostgresCluster{}, "", &RepoResources{},
2675-
&observedInstances{})
2713+
&observedInstances{}, "")
26762714
assert.NilError(t, err)
26772715
})
26782716

26792717
cluster := &v1beta1.PostgresCluster{}
2680-
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, &observedInstances{})
2718+
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, &observedInstances{}, "")
26812719
assert.NilError(t, err)
26822720

26832721
t.Run("ServiceAccount", func(t *testing.T) {
@@ -2698,7 +2736,7 @@ func TestGenerateRepoHostIntent(t *testing.T) {
26982736
},
26992737
}
27002738
observed := &observedInstances{forCluster: []*Instance{{Pods: []*corev1.Pod{{}}}}}
2701-
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, observed)
2739+
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, observed, "")
27022740
assert.NilError(t, err)
27032741
assert.Equal(t, *sts.Spec.Replicas, int32(1))
27042742
})
@@ -2710,7 +2748,7 @@ func TestGenerateRepoHostIntent(t *testing.T) {
27102748
},
27112749
}
27122750
observed := &observedInstances{forCluster: []*Instance{{}}}
2713-
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, observed)
2751+
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, observed, "")
27142752
assert.NilError(t, err)
27152753
assert.Equal(t, *sts.Spec.Replicas, int32(0))
27162754
})

internal/naming/names.go

+9
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,15 @@ func PGBackRestRBAC(cluster *v1beta1.PostgresCluster) metav1.ObjectMeta {
490490
}
491491
}
492492

493+
// RepoHostRBAC returns the ObjectMeta necessary to lookup the ServiceAccount for
494+
// the pgBackRest Repo Host
495+
func RepoHostRBAC(cluster *v1beta1.PostgresCluster) metav1.ObjectMeta {
496+
return metav1.ObjectMeta{
497+
Namespace: cluster.Namespace,
498+
Name: cluster.Name + "-repohost",
499+
}
500+
}
501+
493502
// PGBackRestRepoVolume returns the ObjectMeta for a pgBackRest repository volume
494503
func PGBackRestRepoVolume(cluster *v1beta1.PostgresCluster,
495504
repoName string) metav1.ObjectMeta {

0 commit comments

Comments
 (0)