Skip to content

Commit

Permalink
Reconcile a Service Account for the pgBackRest Repo Host
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@tjmoore4
tjmoore4 committed Jan 15, 2025
1 parent 92f035e commit 6e1adb1
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 10 deletions.
49 changes: 43 additions & 6 deletions internal/controller/postgrescluster/pgbackrest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,9 @@ type RepoResources struct {
// strategy.
func (r *Reconciler) applyRepoHostIntent(ctx context.Context, postgresCluster *v1beta1.PostgresCluster,
repoHostName string, repoResources *RepoResources,
observedInstances *observedInstances) (*appsv1.StatefulSet, error) {
observedInstances *observedInstances, saName string) (*appsv1.StatefulSet, error) {

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

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

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

repo.Spec.Template.Spec.ServiceAccountName = saName

pgbackrest.AddServerToRepoPod(ctx, postgresCluster, &repo.Spec.Template.Spec)

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

// reconcile the RBAC required to run the pgBackRest Repo Host
repoHostSA, err := r.reconcileRepoHostRBAC(ctx, postgresCluster)
if err != nil {
log.Error(err, "unable to reconcile pgBackRest repo host RBAC")
result.Requeue = true
return result, nil
}

var repoHost *appsv1.StatefulSet
var repoHostName string
// reconcile the pgbackrest repository host
repoHost, err = r.reconcileDedicatedRepoHost(ctx, postgresCluster, repoResources, instances)
repoHost, err = r.reconcileDedicatedRepoHost(ctx, postgresCluster, repoResources, instances, repoHostSA.GetName())
if err != nil {
log.Error(err, "unable to reconcile pgBackRest repo host")
result.Requeue = true
Expand Down Expand Up @@ -2118,12 +2128,39 @@ func (r *Reconciler) reconcilePGBackRestRBAC(ctx context.Context,
return sa, nil
}

// +kubebuilder:rbac:groups="",resources="serviceaccounts",verbs={create,patch}

// reconcileRepoHostRBAC reconciles the ServiceAccount for the pgBackRest repo host
func (r *Reconciler) reconcileRepoHostRBAC(ctx context.Context,
postgresCluster *v1beta1.PostgresCluster) (*corev1.ServiceAccount, error) {

sa := &corev1.ServiceAccount{ObjectMeta: naming.RepoHostRBAC(postgresCluster)}
sa.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ServiceAccount"))

if err := r.setControllerReference(postgresCluster, sa); err != nil {
return nil, errors.WithStack(err)
}

sa.Annotations = naming.Merge(postgresCluster.Spec.Metadata.GetAnnotationsOrNil(),
postgresCluster.Spec.Backups.PGBackRest.Metadata.GetAnnotationsOrNil())
sa.Labels = naming.Merge(postgresCluster.Spec.Metadata.GetLabelsOrNil(),
postgresCluster.Spec.Backups.PGBackRest.Metadata.GetLabelsOrNil(),
naming.PGBackRestLabels(postgresCluster.GetName()))

if err := r.apply(ctx, sa); err != nil {
return nil, errors.WithStack(err)
}

return sa, nil
}

// reconcileDedicatedRepoHost is responsible for reconciling a pgBackRest dedicated repository host
// StatefulSet according to a specific PostgresCluster custom resource.
func (r *Reconciler) reconcileDedicatedRepoHost(ctx context.Context,
postgresCluster *v1beta1.PostgresCluster,
repoResources *RepoResources,
observedInstances *observedInstances) (*appsv1.StatefulSet, error) {
observedInstances *observedInstances,
saName string) (*appsv1.StatefulSet, error) {

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

Expand Down Expand Up @@ -2164,7 +2201,7 @@ func (r *Reconciler) reconcileDedicatedRepoHost(ctx context.Context,
}
repoHostName := repoResources.hosts[0].Name
repoHost, err := r.applyRepoHostIntent(ctx, postgresCluster, repoHostName, repoResources,
observedInstances)
observedInstances, saName)
if err != nil {
log.Error(err, "reconciling repository host")
return nil, err
Expand Down
46 changes: 42 additions & 4 deletions internal/controller/postgrescluster/pgbackrest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,8 @@ schedulerName: default-scheduler
securityContext:
fsGroup: 26
fsGroupChangePolicy: OnRootMismatch
serviceAccount: hippocluster-repohost
serviceAccountName: hippocluster-repohost
shareProcessNamespace: true
terminationGracePeriodSeconds: 30
tolerations:
Expand Down Expand Up @@ -724,6 +726,42 @@ func TestReconcilePGBackRestRBAC(t *testing.T) {
assert.Assert(t, foundSubject)
}

func TestReconcileRepoHostRBAC(t *testing.T) {
// Garbage collector cleans up test resources before the test completes
if strings.EqualFold(os.Getenv("USE_EXISTING_CLUSTER"), "true") {
t.Skip("USE_EXISTING_CLUSTER: Test fails due to garbage collection")
}

ctx := context.Background()
_, tClient := setupKubernetes(t)
require.ParallelCapacity(t, 0)

r := &Reconciler{Client: tClient, Owner: client.FieldOwner(t.Name())}

clusterName := "hippocluster"
clusterUID := "hippouid"

ns := setupNamespace(t, tClient)

// create a PostgresCluster to test with
postgresCluster := fakePostgresCluster(clusterName, ns.GetName(), clusterUID, true)
postgresCluster.Status.PGBackRest = &v1beta1.PGBackRestStatus{
Repos: []v1beta1.RepoStatus{{Name: "repo1", StanzaCreated: false}},
}

serviceAccount, err := r.reconcileRepoHostRBAC(ctx, postgresCluster)
assert.NilError(t, err)
assert.Assert(t, serviceAccount != nil)

// verify the service account has been created
sa := &corev1.ServiceAccount{}
err = tClient.Get(ctx, types.NamespacedName{
Name: naming.RepoHostRBAC(postgresCluster).Name,
Namespace: postgresCluster.GetNamespace(),
}, sa)
assert.NilError(t, err)
}

func TestReconcileStanzaCreate(t *testing.T) {
cfg, tClient := setupKubernetes(t)
require.ParallelCapacity(t, 0)
Expand Down Expand Up @@ -2672,12 +2710,12 @@ func TestGenerateRepoHostIntent(t *testing.T) {

t.Run("empty", func(t *testing.T) {
_, err := r.generateRepoHostIntent(ctx, &v1beta1.PostgresCluster{}, "", &RepoResources{},
&observedInstances{})
&observedInstances{}, "")
assert.NilError(t, err)
})

cluster := &v1beta1.PostgresCluster{}
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, &observedInstances{})
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, &observedInstances{}, "")
assert.NilError(t, err)

t.Run("ServiceAccount", func(t *testing.T) {
Expand All @@ -2698,7 +2736,7 @@ func TestGenerateRepoHostIntent(t *testing.T) {
},
}
observed := &observedInstances{forCluster: []*Instance{{Pods: []*corev1.Pod{{}}}}}
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, observed)
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, observed, "")
assert.NilError(t, err)
assert.Equal(t, *sts.Spec.Replicas, int32(1))
})
Expand All @@ -2710,7 +2748,7 @@ func TestGenerateRepoHostIntent(t *testing.T) {
},
}
observed := &observedInstances{forCluster: []*Instance{{}}}
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, observed)
sts, err := r.generateRepoHostIntent(ctx, cluster, "", &RepoResources{}, observed, "")
assert.NilError(t, err)
assert.Equal(t, *sts.Spec.Replicas, int32(0))
})
Expand Down
9 changes: 9 additions & 0 deletions internal/naming/names.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,15 @@ func PGBackRestRBAC(cluster *v1beta1.PostgresCluster) metav1.ObjectMeta {
}
}

// RepoHostRBAC returns the ObjectMeta necessary to lookup the ServiceAccount for
// the pgBackRest Repo Host
func RepoHostRBAC(cluster *v1beta1.PostgresCluster) metav1.ObjectMeta {
return metav1.ObjectMeta{
Namespace: cluster.Namespace,
Name: cluster.Name + "-repohost",
}
}

// PGBackRestRepoVolume returns the ObjectMeta for a pgBackRest repository volume
func PGBackRestRepoVolume(cluster *v1beta1.PostgresCluster,
repoName string) metav1.ObjectMeta {
Expand Down

0 comments on commit 6e1adb1

Please sign in to comment.