Skip to content

Commit c39bc04

Browse files
philrhurstphilrhurst
authored
Merge pull request #3 from CrunchyData/main
upstream changes
2 parents 6eecd0b + d0a80f0 commit c39bc04

File tree

13 files changed

+131
-16
lines changed

13 files changed

+131
-16
lines changed

‎.github/workflows/govulncheck.yaml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ jobs:
3838
uses: github/codeql-action/upload-sarif@v3
3939
with:
4040
sarif_file: 'govulncheck-results.sarif'
41-
# TODO: https://go.dev/issue/70157
42-
if: ${{ false }}
4341

4442
# Print any detected vulnerabilities to the workflow log. This step fails
4543
# when the tool detects a vulnerability in code that is called.

‎internal/controller/postgrescluster/pgbackrest.go‎

Lines changed: 43 additions & 6 deletions
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‎

Lines changed: 42 additions & 4 deletions
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/controller/postgrescluster/pgmonitor_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ func TestReconcilePGMonitorExporterStatus(t *testing.T) {
602602
podExecCalled: false,
603603
// Status was generated manually for this test case
604604
// TODO (jmckulk): add code to generate status
605-
status: v1beta1.MonitoringStatus{ExporterConfiguration: "6d874c58df"},
605+
status: v1beta1.MonitoringStatus{ExporterConfiguration: "5c5f955485"},
606606
statusChangedAfterReconcile: false,
607607
}} {
608608
t.Run(test.name, func(t *testing.T) {

‎internal/naming/names.go‎

Lines changed: 9 additions & 0 deletions
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 {

‎internal/pgaudit/postgres.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ func EnableInPostgreSQL(ctx context.Context, exec postgres.Executor) error {
3535

3636
stdout, stderr, err := exec.ExecInAllDatabases(ctx,
3737
// Quiet the NOTICE from IF EXISTS, and install the pgAudit event triggers.
38+
// Use the default setting for "synchronous_commit".
3839
// - https://www.postgresql.org/docs/current/runtime-config-client.html
40+
// - https://www.postgresql.org/docs/current/runtime-config-wal.html
3941
// - https://github.com/pgaudit/pgaudit#settings
4042
`SET client_min_messages = WARNING; CREATE EXTENSION IF NOT EXISTS pgaudit;`,
4143
map[string]string{

‎internal/pgbouncer/postgres.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ func DisableInPostgreSQL(ctx context.Context, exec postgres.Executor) error {
6868
// - https://www.postgresql.org/docs/current/runtime-config-client.html
6969
`SET client_min_messages = WARNING;`,
7070

71+
// Do not wait for changes to be replicated. [Since PostgreSQL v9.1]
72+
// - https://www.postgresql.org/docs/current/runtime-config-wal.html
73+
`SET synchronous_commit = LOCAL;`,
74+
7175
// Drop the following objects in a transaction.
7276
`BEGIN;`,
7377

@@ -102,7 +106,7 @@ SELECT pg_catalog.format('DROP OWNED BY %I CASCADE', :'username')
102106
// Remove the PgBouncer user now that the objects and other privileges are gone.
103107
stdout, stderr, err = exec.ExecInDatabasesFromQuery(ctx,
104108
`SELECT pg_catalog.current_database()`,
105-
`SET client_min_messages = WARNING; DROP ROLE IF EXISTS :"username";`,
109+
`SET client_min_messages = WARNING; SET synchronous_commit = LOCAL; DROP ROLE IF EXISTS :"username";`,
106110
map[string]string{
107111
"username": postgresqlUser,
108112

@@ -130,6 +134,10 @@ func EnableInPostgreSQL(
130134
// - https://www.postgresql.org/docs/current/runtime-config-client.html
131135
`SET client_min_messages = WARNING;`,
132136

137+
// Do not wait for changes to be replicated. [Since PostgreSQL v9.1]
138+
// - https://www.postgresql.org/docs/current/runtime-config-wal.html
139+
`SET synchronous_commit = LOCAL;`,
140+
133141
// Create the following objects in a transaction so that permissions
134142
// are correct before any other session sees them.
135143
// - https://www.postgresql.org/docs/current/ddl-priv.html

‎internal/pgbouncer/postgres_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ func TestDisableInPostgreSQL(t *testing.T) {
4949
assert.NilError(t, err)
5050
assert.Equal(t, string(b), strings.TrimSpace(`
5151
SET client_min_messages = WARNING;
52+
SET synchronous_commit = LOCAL;
5253
BEGIN;
5354
DROP FUNCTION IF EXISTS :"namespace".get_auth(username TEXT);
5455
DROP SCHEMA IF EXISTS :"namespace" CASCADE;
@@ -90,7 +91,7 @@ COMMIT;`))
9091

9192
b, err := io.ReadAll(stdin)
9293
assert.NilError(t, err)
93-
assert.Equal(t, string(b), `SET client_min_messages = WARNING; DROP ROLE IF EXISTS :"username";`)
94+
assert.Equal(t, string(b), `SET client_min_messages = WARNING; SET synchronous_commit = LOCAL; DROP ROLE IF EXISTS :"username";`)
9495
gomega.NewWithT(t).Expect(command).To(gomega.ContainElements(
9596
`--set=username=_crunchypgbouncer`,
9697
), "expected query parameters")
@@ -135,6 +136,7 @@ func TestEnableInPostgreSQL(t *testing.T) {
135136
assert.NilError(t, err)
136137
assert.Equal(t, string(b), strings.TrimSpace(`
137138
SET client_min_messages = WARNING;
139+
SET synchronous_commit = LOCAL;
138140
BEGIN;
139141
SELECT pg_catalog.format('CREATE ROLE %I NOLOGIN', :'username')
140142
WHERE NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = :'username')

‎internal/pgmonitor/postgres.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ func EnableExporterInPostgreSQL(ctx context.Context, exec postgres.Executor,
7979
// - https://www.postgresql.org/docs/current/runtime-config-client.html
8080
`SET client_min_messages = WARNING;`,
8181

82+
// Do not wait for changes to be replicated. [Since PostgreSQL v9.1]
83+
// - https://www.postgresql.org/docs/current/runtime-config-wal.html
84+
`SET synchronous_commit = LOCAL;`,
85+
8286
// Exporter expects that extension(s) to be installed in all databases
8387
// pg_stat_statements: https://access.crunchydata.com/documentation/pgmonitor/latest/exporter/
8488
"CREATE EXTENSION IF NOT EXISTS pg_stat_statements;",
@@ -103,6 +107,10 @@ func EnableExporterInPostgreSQL(ctx context.Context, exec postgres.Executor,
103107
// - https://www.postgresql.org/docs/current/runtime-config-client.html
104108
`SET client_min_messages = WARNING;`,
105109

110+
// Do not wait for changes to be replicated. [Since PostgreSQL v9.1]
111+
// - https://www.postgresql.org/docs/current/runtime-config-wal.html
112+
`SET synchronous_commit = LOCAL;`,
113+
106114
// Setup.sql file from the exporter image. sql is specific
107115
// to the PostgreSQL version
108116
setup,

‎internal/postgis/postgis.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ func EnableInPostgreSQL(ctx context.Context, exec postgres.Executor) error {
2626
// - https://www.postgresql.org/docs/current/runtime-config-client.html
2727
`SET client_min_messages = WARNING;`,
2828

29+
// Do not wait for changes to be replicated. [Since PostgreSQL v9.1]
30+
// - https://www.postgresql.org/docs/current/runtime-config-wal.html
31+
`SET synchronous_commit = LOCAL;`,
32+
2933
`CREATE EXTENSION IF NOT EXISTS postgis;`,
3034
`CREATE EXTENSION IF NOT EXISTS postgis_topology;`,
3135
`CREATE EXTENSION IF NOT EXISTS fuzzystrmatch;`,

0 commit comments

Comments
 (0)