Skip to content

Commit 1ed1f8e

Browse files
committed
Update pgBackRest repo option logic
When taking a backup, PGO tries to help by not allowing the user to pass the "--repo" option. However, the current method for catching this results in catching any option that begins with "--repo", which prevents users from passing in perfectly valid options. This commit corrects the flag check to only block on exact matches of "--repo". Issue: [sc-16128]
1 parent 613913d commit 1ed1f8e

File tree

2 files changed

+24
-6
lines changed

2 files changed

+24
-6
lines changed

internal/controller/postgrescluster/pgbackrest.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,7 +1011,9 @@ func (r *Reconciler) reconcileRestoreJob(ctx context.Context,
10111011
for _, opt := range options {
10121012
var msg string
10131013
switch {
1014-
case strings.Contains(opt, "--repo"):
1014+
// Since '--repo' can be set with or without an equals ('=') sign, we check for both
1015+
// usage patterns.
1016+
case strings.Contains(opt, "--repo=") || strings.Contains(opt, "--repo "):
10151017
msg = "Option '--repo' is not allowed: please use the 'repoName' field instead."
10161018
case strings.Contains(opt, "--stanza"):
10171019
msg = "Option '--stanza' is not allowed: the operator will automatically set this " +
@@ -2200,9 +2202,11 @@ func (r *Reconciler) reconcileManualBackup(ctx context.Context,
22002202
// and not using the "--repo" option in the "manual.options" field. Therefore, record a
22012203
// warning event and return if a "--repo" option is found. Reconciliation will then be
22022204
// reattempted when "--repo" is removed from "manual.options" and the spec is updated.
2205+
// Since '--repo' can be set with or without an equals ('=') sign, we check for both
2206+
// usage patterns.
22032207
backupOpts := postgresCluster.Spec.Backups.PGBackRest.Manual.Options
22042208
for _, opt := range backupOpts {
2205-
if strings.Contains(opt, "--repo") {
2209+
if strings.Contains(opt, "--repo=") || strings.Contains(opt, "--repo ") {
22062210
r.Recorder.Eventf(postgresCluster, corev1.EventTypeWarning, "InvalidManualBackup",
22072211
"Option '--repo' is not allowed: please use the 'repoName' field instead.",
22082212
repoName)

internal/controller/postgrescluster/pgbackrest_test.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1831,13 +1831,27 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
18311831
expectedClusterCondition: nil,
18321832
},
18331833
}, {
1834-
desc: "invalid option: repo",
1834+
desc: "invalid option: --repo=",
18351835
dataSource: &v1beta1.DataSource{PostgresCluster: &v1beta1.PostgresClusterDataSource{
1836-
ClusterName: "invalid-repo-option", RepoName: "repo1",
1837-
Options: []string{"--repo"},
1836+
ClusterName: "invalid-repo-option-equals", RepoName: "repo1",
1837+
Options: []string{"--repo="},
18381838
}},
18391839
clusterBootstrapped: false,
1840-
sourceClusterName: "invalid-repo-option",
1840+
sourceClusterName: "invalid-repo-option-equals",
1841+
sourceClusterRepos: []v1beta1.PGBackRestRepo{{Name: "repo1"}},
1842+
result: testResult{
1843+
configCount: 1, jobCount: 0, pvcCount: 1,
1844+
invalidSourceRepo: false, invalidSourceCluster: false, invalidOptions: true,
1845+
expectedClusterCondition: nil,
1846+
},
1847+
}, {
1848+
desc: "invalid option: --repo ",
1849+
dataSource: &v1beta1.DataSource{PostgresCluster: &v1beta1.PostgresClusterDataSource{
1850+
ClusterName: "invalid-repo-option-space", RepoName: "repo1",
1851+
Options: []string{"--repo "},
1852+
}},
1853+
clusterBootstrapped: false,
1854+
sourceClusterName: "invalid-repo-option-space",
18411855
sourceClusterRepos: []v1beta1.PGBackRestRepo{{Name: "repo1"}},
18421856
result: testResult{
18431857
configCount: 1, jobCount: 0, pvcCount: 1,

0 commit comments

Comments
 (0)