Skip to content

Commit c84f34c

Browse files
andrewlecuyerjkatz
andrewlecuyer
authored and
jkatz
committed
Refactored the validation logic for the pgBackRest
storage type that can be specified when taking a pgBackRest backup or performing a pgBackRest restore. Specifically, the validation logic can now be effectively shared between the API server and the Operator. This allows backup, restore and clone functionality across both layers to share this validation logic.
1 parent 910c4ca commit c84f34c

File tree

9 files changed

+98
-76
lines changed

9 files changed

+98
-76
lines changed

‎apis/cr/v1/task.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ const (
8585
BackupTypeBootstrap string = "bootstrap"
8686
)
8787

88+
// BackrestStorageTypes defines the valid types of storage that can be utilized
89+
// with pgBackRest
90+
var BackrestStorageTypes = []string{"local", "s3"}
91+
8892
// PgtaskSpec ...
8993
// swagger:ignore
9094
type PgtaskSpec struct {

‎apiserver/backrestservice/backrestimpl.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"time"
2323

2424
"github.com/crunchydata/postgres-operator/apiserver/backupoptions"
25+
"github.com/crunchydata/postgres-operator/util"
2526

2627
crv1 "github.com/crunchydata/postgres-operator/apis/cr/v1"
2728
"github.com/crunchydata/postgres-operator/apiserver"
@@ -103,7 +104,7 @@ func CreateBackup(request *msgs.CreateBackrestBackupRequest, ns, pgouser string)
103104
return resp
104105
}
105106

106-
err = apiserver.ValidateBackrestStorageTypeOnBackupRestore(request.BackrestStorageType,
107+
err = util.ValidateBackrestStorageTypeOnBackupRestore(request.BackrestStorageType,
107108
cluster.Spec.UserLabels[config.LABEL_BACKREST_STORAGE_TYPE], false)
108109
if err != nil {
109110
resp.Status.Code = msgs.Error
@@ -437,7 +438,7 @@ func Restore(request *msgs.RestoreRequest, ns, pgouser string) msgs.RestoreRespo
437438

438439
// ensure the backrest storage type specified for the backup is valid and enabled in the
439440
// cluster
440-
err = apiserver.ValidateBackrestStorageTypeOnBackupRestore(request.BackrestStorageType,
441+
err = util.ValidateBackrestStorageTypeOnBackupRestore(request.BackrestStorageType,
441442
cluster.Spec.UserLabels[config.LABEL_BACKREST_STORAGE_TYPE], true)
442443
if err != nil {
443444
resp.Status.Code = msgs.Error

‎apiserver/cloneservice/cloneimpl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func Clone(request *msgs.CloneRequest, namespace, pgouser string) msgs.CloneResp
108108
}
109109

110110
// clone is a form of restore, so validate using ValidateBackrestStorageTypeOnBackupRestore
111-
err = apiserver.ValidateBackrestStorageTypeOnBackupRestore(request.BackrestStorageSource,
111+
err = util.ValidateBackrestStorageTypeOnBackupRestore(request.BackrestStorageSource,
112112
sourcePgcluster.Spec.UserLabels[config.LABEL_BACKREST_STORAGE_TYPE], true)
113113
if err != nil {
114114
response.Status.Code = msgs.Error

‎apiserver/clusterservice/clusterimpl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1420,7 +1420,7 @@ func validateBackrestStorageTypeOnCreate(request *msgs.CreateClusterRequest) err
14201420

14211421
requestBackRestStorageType := request.BackrestStorageType
14221422

1423-
if requestBackRestStorageType != "" && !apiserver.IsValidBackrestStorageType(requestBackRestStorageType) {
1423+
if requestBackRestStorageType != "" && !util.IsValidBackrestStorageType(requestBackRestStorageType) {
14241424
return fmt.Errorf("Invalid value provided for pgBackRest storage type. The following values are allowed: %s",
14251425
"\""+strings.Join(apiserver.GetBackrestStorageTypes(), "\", \"")+"\"")
14261426
} else if strings.Contains(requestBackRestStorageType, "s3") && isMissingS3Config(request) {

‎apiserver/common.go

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ limitations under the License.
1717

1818
import (
1919
"errors"
20-
"fmt"
2120
"strconv"
22-
"strings"
2321

2422
crv1 "github.com/crunchydata/postgres-operator/apis/cr/v1"
2523
msgs "github.com/crunchydata/postgres-operator/apiservermsgs"
@@ -149,32 +147,6 @@ func CreateRMDataTask(clusterName, replicaName, taskName string, deleteBackups,
149147

150148
}
151149

152-
// IsValidBackrestStorageType determines if the storageType string contains valid pgBackRest
153-
// storage type values
154-
func IsValidBackrestStorageType(storageType string) bool {
155-
isValid := true
156-
for _, storageType := range strings.Split(storageType, ",") {
157-
if !IsStringOneOf(storageType, GetBackrestStorageTypes()...) {
158-
isValid = false
159-
break
160-
}
161-
}
162-
return isValid
163-
}
164-
165-
// IsStringOneOf tests to see string testVal is included in the list
166-
// of strings provided using acceptedVals
167-
func IsStringOneOf(testVal string, acceptedVals ...string) bool {
168-
isOneOf := false
169-
for _, val := range acceptedVals {
170-
if testVal == val {
171-
isOneOf = true
172-
break
173-
}
174-
}
175-
return isOneOf
176-
}
177-
178150
func GetBackrestStorageTypes() []string {
179151
return backrestStorageTypes
180152
}
@@ -191,32 +163,3 @@ func IsValidPVC(pvcName, ns string) bool {
191163

192164
return true
193165
}
194-
195-
// ValidateBackrestStorageTypeOnBackupRestore checks to see if the pgbackrest storage type provided
196-
// when performing either pgbackrest backup or restore is valid. This includes ensuring the value
197-
// provided is a valid storage type (e.g. "s3" and/or "local"). This also includes ensuring the
198-
// storage type specified (e.g. "s3" or "local") is enabled in the current cluster. And finally,
199-
// validation is ocurring for a restore, the ensure only one storage type is selected.
200-
func ValidateBackrestStorageTypeOnBackupRestore(requestBackRestStorageType,
201-
clusterBackRestStorageType string, restore bool) error {
202-
203-
if requestBackRestStorageType != "" && !IsValidBackrestStorageType(requestBackRestStorageType) {
204-
return fmt.Errorf("Invalid value provided for pgBackRest storage type. The following values are allowed: %s",
205-
"\""+strings.Join(GetBackrestStorageTypes(), "\", \"")+"\"")
206-
} else if requestBackRestStorageType != "" && strings.Contains(requestBackRestStorageType, "s3") &&
207-
!strings.Contains(clusterBackRestStorageType, "s3") {
208-
return errors.New("Storage type 's3' not allowed. S3 storage is not enabled for pgBackRest in this cluster")
209-
} else if (requestBackRestStorageType == "" || strings.Contains(requestBackRestStorageType, "local")) &&
210-
(clusterBackRestStorageType != "" && !strings.Contains(clusterBackRestStorageType, "local")) {
211-
return errors.New("Storage type 'local' not allowed. Local storage is not enabled for pgBackRest in this cluster. " +
212-
"If this cluster uses S3 storage only, specify 's3' for the pgBackRest storage type.")
213-
}
214-
215-
// storage type validation that is only applicable for restores
216-
if restore && requestBackRestStorageType != "" && len(strings.Split(requestBackRestStorageType, ",")) > 1 {
217-
return fmt.Errorf("Multiple storage types cannot be selected cannot be select when performing a restore. Please "+
218-
"select one of the following: %s", "\""+strings.Join(GetBackrestStorageTypes(), "\", \"")+"\"")
219-
}
220-
221-
return nil
222-
}

‎apiserver/scheduleservice/scheduleimpl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ type scheduleRequest struct {
4141
func (s scheduleRequest) createBackRestSchedule(cluster *crv1.Pgcluster, ns string) *PgScheduleSpec {
4242
name := fmt.Sprintf("%s-%s-%s", cluster.Name, s.Request.ScheduleType, s.Request.PGBackRestType)
4343

44-
err := apiserver.ValidateBackrestStorageTypeOnBackupRestore(s.Request.BackrestStorageType,
44+
err := util.ValidateBackrestStorageTypeOnBackupRestore(s.Request.BackrestStorageType,
4545
cluster.Spec.UserLabels[config.LABEL_BACKREST_STORAGE_TYPE], false)
4646
if err != nil {
4747
s.Response.Status.Code = msgs.Error

‎operator/cluster/clone.go

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,8 @@ func cloneStep1(clientset *kubernetes.Clientset, client *rest.RESTClient, namesp
221221
// if 's3' storage was selected for the clone, ensure it is enabled in the current pg cluster.
222222
// also, if 'local' was selected, or if no storage type was selected, ensure the cluster is using
223223
// local storage
224-
err = validateBackrestStorageTypeClone(sourceClusterBackrestStorageType, cloneBackrestStorageType)
224+
err = util.ValidateBackrestStorageTypeOnBackupRestore(cloneBackrestStorageType,
225+
sourceClusterBackrestStorageType, true)
225226
if err != nil {
226227
log.Error(err)
227228
PublishCloneEvent(events.EventCloneClusterFailure, namespace, task, err.Error())
@@ -946,19 +947,6 @@ func waitForDeploymentReady(clientset *kubernetes.Clientset, namespace, deployme
946947
}
947948
}
948949

949-
// validateBackrestStorageTypeClone verifies that 's3' is enabled in the cluster being cloned
950-
// when 's3' is selected as the storage type for the new/cloned cluster
951-
func validateBackrestStorageTypeClone(sourceClusterBackrestStorageType,
952-
cloneBackrestStorageType string) error {
953-
954-
if !strings.Contains(sourceClusterBackrestStorageType, cloneBackrestStorageType) {
955-
return fmt.Errorf("The backrest storage source '%s' selected for the cloned cluster does "+
956-
"not match a storage type in source cluster storage type '%s'",
957-
cloneBackrestStorageType, sourceClusterBackrestStorageType)
958-
}
959-
return nil
960-
}
961-
962950
// getS3Param returns either the value provided by 'sourceClusterS3param' if not en empty string,
963951
// otherwise return the equivlant value from the pgo.yaml global configuration filer
964952
func getS3Param(sourceClusterS3param, pgoConfigParam string) string {

‎util/backrest.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package util
2+
3+
/*
4+
Copyright 2019 Crunchy Data Solutions, Inc.
5+
Licensed under the Apache License, Version 2.0 (the "License");
6+
you may not use this file except in compliance with the License.
7+
You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
import (
19+
"errors"
20+
"fmt"
21+
"strings"
22+
23+
crv1 "github.com/crunchydata/postgres-operator/apis/cr/v1"
24+
)
25+
26+
// ValidateBackrestStorageTypeOnBackupRestore checks to see if the pgbackrest storage type provided
27+
// when performing either pgbackrest backup or restore is valid. This includes ensuring the value
28+
// provided is a valid storage type (e.g. "s3" and/or "local"). This also includes ensuring the
29+
// storage type specified (e.g. "s3" or "local") is enabled in the current cluster. And finally,
30+
// validation is ocurring for a restore, the ensure only one storage type is selected.
31+
func ValidateBackrestStorageTypeOnBackupRestore(newBackRestStorageType,
32+
currentBackRestStorageType string, restore bool) error {
33+
34+
if newBackRestStorageType != "" && !IsValidBackrestStorageType(newBackRestStorageType) {
35+
return fmt.Errorf("Invalid value provided for pgBackRest storage type. The following "+
36+
"values are allowed: %s", "\""+strings.Join(crv1.BackrestStorageTypes, "\", \"")+"\"")
37+
} else if newBackRestStorageType != "" &&
38+
strings.Contains(newBackRestStorageType, "s3") &&
39+
!strings.Contains(currentBackRestStorageType, "s3") {
40+
return errors.New("Storage type 's3' not allowed. S3 storage is not enabled for " +
41+
"pgBackRest in thiscluster")
42+
} else if (newBackRestStorageType == "" ||
43+
strings.Contains(newBackRestStorageType, "local")) &&
44+
(currentBackRestStorageType != "" &&
45+
!strings.Contains(currentBackRestStorageType, "local")) {
46+
return errors.New("Storage type 'local' not allowed. Local storage is not enabled for " +
47+
"pgBackRest in this cluster. If this cluster uses S3 storage only, specify 's3' " +
48+
"for the pgBackRest storage type.")
49+
}
50+
51+
// storage type validation that is only applicable for restores
52+
if restore && newBackRestStorageType != "" &&
53+
len(strings.Split(newBackRestStorageType, ",")) > 1 {
54+
return fmt.Errorf("Multiple storage types cannot be selected cannot be select when "+
55+
"performing a restore. Please select one of the following: %s",
56+
"\""+strings.Join(crv1.BackrestStorageTypes, "\", \"")+"\"")
57+
}
58+
59+
return nil
60+
}
61+
62+
// IsValidBackrestStorageType determines if the storage source string contains valid pgBackRest
63+
// storage type values
64+
func IsValidBackrestStorageType(storageType string) bool {
65+
isValid := true
66+
for _, storageType := range strings.Split(storageType, ",") {
67+
if !IsStringOneOf(storageType, crv1.BackrestStorageTypes...) {
68+
isValid = false
69+
break
70+
}
71+
}
72+
return isValid
73+
}

‎util/util.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,3 +374,16 @@ func NewClient(cfg *rest.Config) (*rest.RESTClient, *runtime.Scheme, error) {
374374

375375
return client, scheme, nil
376376
}
377+
378+
// IsStringOneOf tests to see string testVal is included in the list
379+
// of strings provided using acceptedVals
380+
func IsStringOneOf(testVal string, acceptedVals ...string) bool {
381+
isOneOf := false
382+
for _, val := range acceptedVals {
383+
if testVal == val {
384+
isOneOf = true
385+
break
386+
}
387+
}
388+
return isOneOf
389+
}

0 commit comments

Comments
 (0)