Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -551,8 +551,25 @@ spec:
gtid:
type: string
type:
enum:
- latest
- date
- transaction
- skip
type: string
type: object
x-kubernetes-validations:
- message: 'Date is required for type ''date'' and should be in format
YYYY-MM-DD HH:MM:SS with valid ranges (MM: 01-12, DD: 01-31, HH:
00-23, MM/SS: 00-59)'
rule: self.type != 'date' || (has(self.date) && self.date.matches('^[0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])
([01][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]$'))
- message: GTID is required for types 'transaction' and 'skip'
rule: (self.type != 'transaction' && self.type != 'skip') || (has(self.gtid)
&& size(self.gtid) > 0)
- message: Date and GTID should not be set when type is 'latest'
rule: self.type != 'latest' || ((!has(self.date) || size(self.date)
== 0) && (!has(self.gtid) || size(self.gtid) == 0))
pxcCluster:
type: string
resources:
Expand Down
17 changes: 17 additions & 0 deletions deploy/bundle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -940,8 +940,25 @@ spec:
gtid:
type: string
type:
enum:
- latest
- date
- transaction
- skip
type: string
type: object
x-kubernetes-validations:
- message: 'Date is required for type ''date'' and should be in format
YYYY-MM-DD HH:MM:SS with valid ranges (MM: 01-12, DD: 01-31, HH:
00-23, MM/SS: 00-59)'
rule: self.type != 'date' || (has(self.date) && self.date.matches('^[0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])
([01][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]$'))
- message: GTID is required for types 'transaction' and 'skip'
rule: (self.type != 'transaction' && self.type != 'skip') || (has(self.gtid)
&& size(self.gtid) > 0)
- message: Date and GTID should not be set when type is 'latest'
rule: self.type != 'latest' || ((!has(self.date) || size(self.date)
== 0) && (!has(self.gtid) || size(self.gtid) == 0))
pxcCluster:
type: string
resources:
Expand Down
17 changes: 17 additions & 0 deletions deploy/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -940,8 +940,25 @@ spec:
gtid:
type: string
type:
enum:
- latest
- date
- transaction
- skip
type: string
type: object
x-kubernetes-validations:
- message: 'Date is required for type ''date'' and should be in format
YYYY-MM-DD HH:MM:SS with valid ranges (MM: 01-12, DD: 01-31, HH:
00-23, MM/SS: 00-59)'
rule: self.type != 'date' || (has(self.date) && self.date.matches('^[0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])
([01][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]$'))
- message: GTID is required for types 'transaction' and 'skip'
rule: (self.type != 'transaction' && self.type != 'skip') || (has(self.gtid)
&& size(self.gtid) > 0)
- message: Date and GTID should not be set when type is 'latest'
rule: self.type != 'latest' || ((!has(self.date) || size(self.date)
== 0) && (!has(self.gtid) || size(self.gtid) == 0))
pxcCluster:
type: string
resources:
Expand Down
17 changes: 17 additions & 0 deletions deploy/cw-bundle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -940,8 +940,25 @@ spec:
gtid:
type: string
type:
enum:
- latest
- date
- transaction
- skip
type: string
type: object
x-kubernetes-validations:
- message: 'Date is required for type ''date'' and should be in format
YYYY-MM-DD HH:MM:SS with valid ranges (MM: 01-12, DD: 01-31, HH:
00-23, MM/SS: 00-59)'
rule: self.type != 'date' || (has(self.date) && self.date.matches('^[0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])
([01][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]$'))
- message: GTID is required for types 'transaction' and 'skip'
rule: (self.type != 'transaction' && self.type != 'skip') || (has(self.gtid)
&& size(self.gtid) > 0)
- message: Date and GTID should not be set when type is 'latest'
rule: self.type != 'latest' || ((!has(self.date) || size(self.date)
== 0) && (!has(self.gtid) || size(self.gtid) == 0))
pxcCluster:
type: string
resources:
Expand Down
11 changes: 7 additions & 4 deletions pkg/apis/pxc/v1/pxc_prestore_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package v1

import (
"errors"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand All @@ -29,11 +28,15 @@ type PerconaXtraDBClusterRestoreStatus struct {
Unsafe UnsafeFlags `json:"unsafeFlags,omitempty"`
}

// +kubebuilder:validation:XValidation:rule="self.type != 'date' || (has(self.date) && self.date.matches('^[0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01]) ([01][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]$'))",message="Date is required for type 'date' and should be in format YYYY-MM-DD HH:MM:SS with valid ranges (MM: 01-12, DD: 01-31, HH: 00-23, MM/SS: 00-59)"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we discussed the kubebuilder validation testing through envtest, are we going to update that test in this PR?

// +kubebuilder:validation:XValidation:rule="(self.type != 'transaction' && self.type != 'skip') || (has(self.gtid) && size(self.gtid) > 0)",message="GTID is required for types 'transaction' and 'skip'"
// +kubebuilder:validation:XValidation:rule="self.type != 'latest' || ((!has(self.date) || size(self.date) == 0) && (!has(self.gtid) || size(self.gtid) == 0))",message="Date and GTID should not be set when type is 'latest'"
type PITR struct {
BackupSource *PXCBackupStatus `json:"backupSource"`
Type string `json:"type"`
Date string `json:"date"`
GTID string `json:"gtid"`
// +kubebuilder:validation:Enum={latest,date,transaction,skip}
Type string `json:"type"`
Date string `json:"date"`
GTID string `json:"gtid"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
111 changes: 111 additions & 0 deletions pkg/controller/pxcrestore/pitr_validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package pxcrestore

import (
"context"
"os"
"path/filepath"
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"

"github.com/percona/percona-xtradb-cluster-operator/pkg/apis"
pxcv1 "github.com/percona/percona-xtradb-cluster-operator/pkg/apis/pxc/v1"
)

var (
cfg *rest.Config
k8sClient client.Client
testEnv *envtest.Environment
)

func TestPxcrestore(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "PerconaXtraDBClusterRestore Suite")
}

var _ = BeforeSuite(func() {
logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))

Expect(os.Setenv("WATCH_NAMESPACE", "default")).NotTo(HaveOccurred())

By("bootstrapping test environment")
testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")},
ErrorIfCRDPathMissing: true,
}

var err error
cfg, err = testEnv.Start()
Expect(err).NotTo(HaveOccurred())
Expect(cfg).NotTo(BeNil())

err = apis.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred())

k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
Expect(err).NotTo(HaveOccurred())
Expect(k8sClient).NotTo(BeNil())
})

var _ = AfterSuite(func() {
By("tearing down the test environment")
Expect(os.Unsetenv("WATCH_NAMESPACE")).NotTo(HaveOccurred())
Expect(testEnv.Stop()).NotTo(HaveOccurred())
})

var _ = Describe("PerconaXtraDBClusterRestore PITR CRD validation", Ordered, func() {
ctx := context.Background()
const ns = "pitr-validation"

BeforeAll(func() {
Expect(k8sClient.Create(ctx, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: ns},
})).To(Succeed())
})

AfterAll(func() {
_ = k8sClient.Delete(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}})
})

newRestore := func(name string, pitr *pxcv1.PITR) *pxcv1.PerconaXtraDBClusterRestore {
return &pxcv1.PerconaXtraDBClusterRestore{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns},
Spec: pxcv1.PerconaXtraDBClusterRestoreSpec{
PXCCluster: "cluster1",
BackupName: "backup1",
PITR: pitr,
},
}
}

DescribeTable("valid PITR configurations",
func(name string, pitr *pxcv1.PITR) {
Expect(k8sClient.Create(ctx, newRestore(name, pitr))).To(Succeed())
},
Entry("type latest", "valid-latest", &pxcv1.PITR{Type: "latest"}),
Entry("type date with valid format", "valid-date", &pxcv1.PITR{Type: "date", Date: "2024-01-15 12:30:00"}),
Entry("type transaction with gtid", "valid-transaction", &pxcv1.PITR{Type: "transaction", GTID: "abc123:1-10"}),
)

DescribeTable("invalid PITR configurations",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

til DescribeTable nice

func(name string, pitr *pxcv1.PITR, errMsg string) {
err := k8sClient.Create(ctx, newRestore(name, pitr))
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring(errMsg))
},
Entry("type date with empty date", "invalid-date-empty", &pxcv1.PITR{Type: "date"}, "Date is required"),
Entry("type date with wrong format", "invalid-date-format", &pxcv1.PITR{Type: "date", Date: "15-01-2024 12:30:00"}, "format YYYY-MM-DD"),
Entry("type latest with date set", "invalid-latest-date", &pxcv1.PITR{Type: "latest", Date: "2024-01-15 12:30:00"}, "Date and GTID should not be set"),
Entry("type transaction without gtid", "invalid-transaction-no-gtid", &pxcv1.PITR{Type: "transaction"}, "GTID is required"),
Entry("unknown type", "invalid-unknown-type", &pxcv1.PITR{Type: "unknown"}, "Unsupported value"),
)
})
Loading