Skip to content

Commit def079d

Browse files
authored
Ensure pgBackRest Settings Work When Local & S3 Storage is Enabled
This update ensures that the 's3-verify-tls' & 's3-uri-style' settings work as expected when both local & S3 storage are enabled for pgBackRest when creating a pgcluster. Currently, when a new cluster is created using Local and AWS S3 storage (e.g. by specifiying --pgbackrest-storage-type=local,s3 with the pgo create command), stanza creation fails and the cluster is not initialized. Since this is failing because the 'non S3' pgBackRest command runs when the repo1-s3-verify-tls flag is detected in the environment. This fix updates the pgBackRest flag implementation so that the repo1-s3-verify-tls setting is applied only for the second pgbackrest command only, i.e. the command using --repo-type-s3, and not apply it when running the first pgbackrest command for the local repository. This is done by using the --no-repo1-s3-verify-tls command flag rather than the relevant environment variable or the related configuration file setting. This method allows both commands to execute as expected.
1 parent 7e66338 commit def079d

File tree

16 files changed

+136
-52
lines changed

16 files changed

+136
-52
lines changed

bin/pgo-backrest-repo/pgo-backrest-repo.sh

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,7 @@ then
6464
then
6565
printf "repo1-s3-uri-style=%s\n" "${PGBACKREST_REPO1_S3_URI_STYLE}" >> /etc/pgbackrest/pgbackrest.conf
6666
fi
67-
68-
if [[ "${PGBACKREST_REPO1_S3_VERIFY_TLS}" != "" ]]
69-
then
70-
printf "repo1-s3-verify-tls=%s\n" "${PGBACKREST_REPO1_S3_VERIFY_TLS}" >> /etc/pgbackrest/pgbackrest.conf
71-
fi
72-
67+
7368
fi
7469

7570
mkdir -p ~/.ssh/

conf/postgres-operator/backrest-job.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,10 @@
6262
}, {
6363
"name": "PGHA_PGBACKREST_LOCAL_S3_STORAGE",
6464
"value": "{{.BackrestLocalAndS3Storage}}"
65-
}, {
65+
},{
66+
"name": "PGHA_PGBACKREST_S3_VERIFY_TLS",
67+
"value": "{{.PgbackrestS3VerifyTLS}}"
68+
},{
6669
"name": "PGBACKREST_LOG_PATH",
6770
"value": "/tmp"
6871
}, {

conf/postgres-operator/backrest-restore-job.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,6 @@
8080
}, {
8181
"name": "PGBACKREST_REPO1_HOST",
8282
"value": "{{.PgbackrestRepo1Host}}"
83-
}, {
84-
"name": "PGBACKREST_REPO_TYPE",
85-
"value": "{{.PgbackrestRepoType}}"
8683
}, {
8784
"name": "PGBACKREST_LOG_PATH",
8885
"value": "/tmp"

conf/postgres-operator/pgbackrest-s3-env-vars.json

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@
3939
{
4040
"name": "PGBACKREST_REPO1_S3_URI_STYLE",
4141
"value": "{{.PgbackrestS3URIStyle}}"
42-
},
43-
{
44-
"name": "PGBACKREST_REPO1_S3_VERIFY_TLS",
42+
},{
43+
"name": "PGHA_PGBACKREST_S3_VERIFY_TLS",
4544
"value": "{{.PgbackrestS3VerifyTLS}}"
4645
},

installers/ansible/roles/pgo-operator/files/pgo-configs/backrest-job.json

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,13 @@
5959
}, {
6060
"name": "PGBACKREST_REPO_TYPE",
6161
"value": "{{.PgbackrestRepoType}}"
62-
}, {
62+
},{
6363
"name": "PGHA_PGBACKREST_LOCAL_S3_STORAGE",
6464
"value": "{{.BackrestLocalAndS3Storage}}"
65-
}, {
65+
},{
66+
"name": "PGHA_PGBACKREST_S3_VERIFY_TLS",
67+
"value": "{{.PgbackrestS3VerifyTLS}}"
68+
},{
6669
"name": "PGBACKREST_LOG_PATH",
6770
"value": "/tmp"
6871
}, {

installers/ansible/roles/pgo-operator/files/pgo-configs/pgbackrest-s3-env-vars.json

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@
3939
{
4040
"name": "PGBACKREST_REPO1_S3_URI_STYLE",
4141
"value": "{{.PgbackrestS3URIStyle}}"
42-
},
43-
{
44-
"name": "PGBACKREST_REPO1_S3_VERIFY_TLS",
42+
},{
43+
"name": "PGHA_PGBACKREST_S3_VERIFY_TLS",
4544
"value": "{{.PgbackrestS3VerifyTLS}}"
4645
},

internal/apiserver/backrestservice/backrestimpl.go

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@ import (
2020
"errors"
2121
"fmt"
2222
"io/ioutil"
23+
"strconv"
2324
"strings"
2425
"time"
2526

2627
"github.com/crunchydata/postgres-operator/internal/apiserver/backupoptions"
28+
"github.com/crunchydata/postgres-operator/internal/operator"
2729
"github.com/crunchydata/postgres-operator/internal/util"
2830

2931
"github.com/crunchydata/postgres-operator/internal/apiserver"
@@ -44,7 +46,11 @@ var pgBackRestInfoCommand = []string{"pgbackrest", "info", "--output", "json"}
4446

4547
// repoTypeFlagS3 is used for getting the pgBackRest info for a repository that
4648
// is stored in S3
47-
var repoTypeFlagS3 = []string{"--repo-type", "s3"}
49+
var repoTypeFlagS3 = []string{"--repo1-type", "s3"}
50+
51+
// noRepoS3VerifyTLS is used to disable SSL certificate verification when getting
52+
// the pgBackRest info for a repository that is stored in S3
53+
var noRepoS3VerifyTLS = "--no-repo1-s3-verify-tls"
4854

4955
// CreateBackup ...
5056
// pgo backup mycluster
@@ -212,7 +218,7 @@ func CreateBackup(request *msgs.CreateBackrestBackupRequest, ns, pgouser string)
212218

213219
err = kubeapi.Createpgtask(apiserver.RESTClient,
214220
getBackupParams(cluster.ObjectMeta.Labels[config.LABEL_PG_CLUSTER_IDENTIFIER], clusterName, taskName, crv1.PgtaskBackrestBackup, podname, "database",
215-
util.GetValueOrDefault(cluster.Spec.PGOImagePrefix, apiserver.Pgo.Pgo.PGOImagePrefix), request.BackupOpts, request.BackrestStorageType, jobName, ns, pgouser),
221+
util.GetValueOrDefault(cluster.Spec.PGOImagePrefix, apiserver.Pgo.Pgo.PGOImagePrefix), request.BackupOpts, request.BackrestStorageType, operator.GetS3VerifyTLSSetting(&cluster), jobName, ns, pgouser),
216222
ns)
217223
if err != nil {
218224
resp.Status.Code = msgs.Error
@@ -226,7 +232,7 @@ func CreateBackup(request *msgs.CreateBackrestBackupRequest, ns, pgouser string)
226232
return resp
227233
}
228234

229-
func getBackupParams(identifier, clusterName, taskName, action, podName, containerName, imagePrefix, backupOpts, backrestStorageType, jobName, ns, pgouser string) *crv1.Pgtask {
235+
func getBackupParams(identifier, clusterName, taskName, action, podName, containerName, imagePrefix, backupOpts, backrestStorageType, s3VerifyTLS, jobName, ns, pgouser string) *crv1.Pgtask {
230236
var newInstance *crv1.Pgtask
231237
spec := crv1.PgtaskSpec{}
232238
spec.Name = taskName
@@ -244,6 +250,7 @@ func getBackupParams(identifier, clusterName, taskName, action, podName, contain
244250
spec.Parameters[config.LABEL_BACKREST_COMMAND] = action
245251
spec.Parameters[config.LABEL_BACKREST_OPTS] = backupOpts
246252
spec.Parameters[config.LABEL_BACKREST_STORAGE_TYPE] = backrestStorageType
253+
spec.Parameters[config.LABEL_BACKREST_S3_VERIFY_TLS] = s3VerifyTLS
247254

248255
newInstance = &crv1.Pgtask{
249256
ObjectMeta: metav1.ObjectMeta{
@@ -404,8 +411,10 @@ func ShowBackrest(name, selector, ns string) msgs.ShowBackrestResponse {
404411
StorageType: storageType,
405412
}
406413

414+
verifyTLS, _ := strconv.ParseBool(operator.GetS3VerifyTLSSetting(&c))
415+
407416
// get the pgBackRest info using this legacy function
408-
info, err := getInfo(c.Name, storageType, podname, ns)
417+
info, err := getInfo(c.Name, storageType, podname, ns, verifyTLS)
409418

410419
// see if the function returned successfully, and if so, unmarshal the JSON
411420
if err != nil {
@@ -433,13 +442,17 @@ func ShowBackrest(name, selector, ns string) msgs.ShowBackrestResponse {
433442
return response
434443
}
435444

436-
func getInfo(clusterName, storageType, podname, ns string) (string, error) {
445+
func getInfo(clusterName, storageType, podname, ns string, verifyTLS bool) (string, error) {
437446
log.Debug("backrest info command requested")
438447

439448
cmd := pgBackRestInfoCommand
440449

441450
if storageType == "s3" {
442451
cmd = append(cmd, repoTypeFlagS3...)
452+
453+
if !verifyTLS {
454+
cmd = append(cmd, noRepoS3VerifyTLS)
455+
}
443456
}
444457

445458
output, stderr, err := kubeapi.ExecToPodThroughAPI(apiserver.RESTConfig, apiserver.Clientset, cmd, containername, podname, ns, nil)
@@ -572,6 +585,7 @@ func getRestoreParams(request *msgs.RestoreRequest, ns string, cluster crv1.Pgcl
572585
spec.Parameters[config.LABEL_PGBACKREST_REPO_PATH] = "/backrestrepo/" + request.FromCluster + "-backrest-shared-repo"
573586
spec.Parameters[config.LABEL_PGBACKREST_REPO_HOST] = request.FromCluster + "-backrest-shared-repo"
574587
spec.Parameters[config.LABEL_BACKREST_STORAGE_TYPE] = request.BackrestStorageType
588+
spec.Parameters[config.LABEL_BACKREST_S3_VERIFY_TLS] = operator.GetS3VerifyTLSSetting(&cluster)
575589

576590
// validate & parse nodeLabel if exists
577591
if request.NodeLabel != "" {

internal/apiserver/backupoptions/pgbackrestoptions.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ var pgBackRestOptsDenyList = []string{
5454
"--repo-s3-endpoint",
5555
"--repo-s3-host",
5656
"--repo-s3-region",
57-
"--repo-s3-verify-tls",
57+
"--no-repo-s3-verify-tls",
5858
"--repo-s3-uri-style",
5959
"--stanza",
6060
"--tablespace-map",

internal/apiserver/clusterservice/clusterimpl.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,17 +1374,26 @@ func getClusterParams(request *msgs.CreateClusterRequest, name string, userLabel
13741374
spec.SyncReplication = request.SyncReplication
13751375

13761376
// set pgBackRest S3 settings in the spec if included in the request
1377+
// otherwise set to the default configuration value
13771378
if request.BackrestS3Bucket != "" {
13781379
spec.BackrestS3Bucket = request.BackrestS3Bucket
1380+
} else {
1381+
spec.BackrestS3Bucket = apiserver.Pgo.Cluster.BackrestS3Bucket
13791382
}
13801383
if request.BackrestS3Endpoint != "" {
13811384
spec.BackrestS3Endpoint = request.BackrestS3Endpoint
1385+
} else {
1386+
spec.BackrestS3Endpoint = apiserver.Pgo.Cluster.BackrestS3Endpoint
13821387
}
13831388
if request.BackrestS3Region != "" {
13841389
spec.BackrestS3Region = request.BackrestS3Region
1390+
} else {
1391+
spec.BackrestS3Region = apiserver.Pgo.Cluster.BackrestS3Region
13851392
}
13861393
if request.BackrestS3URIStyle != "" {
13871394
spec.BackrestS3URIStyle = request.BackrestS3URIStyle
1395+
} else {
1396+
spec.BackrestS3URIStyle = apiserver.Pgo.Cluster.BackrestS3URIStyle
13881397
}
13891398

13901399
// if the pgbackrest-s3-verify-tls flag was set, update the CR spec
@@ -1395,6 +1404,8 @@ func getClusterParams(request *msgs.CreateClusterRequest, name string, userLabel
13951404
} else {
13961405
spec.BackrestS3VerifyTLS = "true"
13971406
}
1407+
} else {
1408+
spec.BackrestS3VerifyTLS = apiserver.Pgo.Cluster.BackrestS3VerifyTLS
13981409
}
13991410

14001411
// set the data source that should be utilized to bootstrap the cluster

internal/config/labels.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ const LABEL_BACKREST_BACKUP_OPTS = "backrest-backup-opts"
8080
const LABEL_BACKREST_OPTS = "backrest-opts"
8181
const LABEL_BACKREST_PITR_TARGET = "backrest-pitr-target"
8282
const LABEL_BACKREST_STORAGE_TYPE = "backrest-storage-type"
83+
const LABEL_BACKREST_S3_VERIFY_TLS = "backrest-s3-verify-tls"
8384
const LABEL_BADGER = "crunchy-pgbadger"
8485
const LABEL_BADGER_CCPIMAGE = "crunchy-pgbadger"
8586
const LABEL_BACKUP_TYPE_BACKREST = "pgbackrest"

0 commit comments

Comments
 (0)