feat: validate the data directory (#448)

* refactor: use external ctrlkit lib and bump dependencies Signed-off-by: arkbriar <[email protected]> * feat: validate the data directory Signed-off-by: arkbriar <[email protected]> * Update the regex Signed-off-by: arkbriar <[email protected]> --------- Signed-off-by: arkbriar <[email protected]>
risingwavelabs · May 30, 2023 · 957c93f · 957c93f
1 parent b62d781
commit 957c93f
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 0 deletions.
diff --git a/apis/risingwave/v1alpha1/risingwave_state_store_backend.go b/apis/risingwave/v1alpha1/risingwave_state_store_backend.go
@@ -251,6 +251,7 @@ type RisingWaveStateStoreBackend struct {
 	// DataDirectory is the directory to store the data in the object storage.
 	// Defaults to hummock.
 	// +kubebuilder:default=hummock
+	// +kubebuilder:validation:Pattern="^[0-9a-zA-Z_/-]{1,}$"
 	DataDirectory string `json:"dataDirectory,omitempty"`
 
 	// Memory indicates to store the data in memory. It's only for test usage and strongly discouraged to

diff --git a/config/crd/bases/risingwave.risingwavelabs.com_risingwaves.yaml b/config/crd/bases/risingwave.risingwavelabs.com_risingwaves.yaml
@@ -44108,6 +44108,7 @@ spec:
                     default: hummock
                     description: DataDirectory is the directory to store the data
                       in the object storage. Defaults to hummock.
+                    pattern: ^[0-9a-zA-Z_/-]{1,}$
                     type: string
                   gcs:
                     description: GCS storage spec.
@@ -44465,6 +44466,7 @@ spec:
                         default: hummock
                         description: DataDirectory is the directory to store the data
                           in the object storage. Defaults to hummock.
+                        pattern: ^[0-9a-zA-Z_/-]{1,}$
                         type: string
                       gcs:
                         description: GCS storage spec.

diff --git a/config/risingwave-operator-test.yaml b/config/risingwave-operator-test.yaml
@@ -51268,6 +51268,7 @@ spec:
                     default: hummock
                     description: DataDirectory is the directory to store the data
                       in the object storage. Defaults to hummock.
+                    pattern: ^[0-9a-zA-Z_/-]{1,}$
                     type: string
                   gcs:
                     description: GCS storage spec.
@@ -51625,6 +51626,7 @@ spec:
                         default: hummock
                         description: DataDirectory is the directory to store the data
                           in the object storage. Defaults to hummock.
+                        pattern: ^[0-9a-zA-Z_/-]{1,}$
                         type: string
                       gcs:
                         description: GCS storage spec.

diff --git a/config/risingwave-operator.yaml b/config/risingwave-operator.yaml
@@ -51268,6 +51268,7 @@ spec:
                     default: hummock
                     description: DataDirectory is the directory to store the data
                       in the object storage. Defaults to hummock.
+                    pattern: ^[0-9a-zA-Z_/-]{1,}$
                     type: string
                   gcs:
                     description: GCS storage spec.
@@ -51625,6 +51626,7 @@ spec:
                         default: hummock
                         description: DataDirectory is the directory to store the data
                           in the object storage. Defaults to hummock.
+                        pattern: ^[0-9a-zA-Z_/-]{1,}$
                         type: string
                       gcs:
                         description: GCS storage spec.

diff --git a/pkg/webhook/risingwave_mutating_webhook.go b/pkg/webhook/risingwave_mutating_webhook.go
@@ -18,6 +18,7 @@ package webhook
 
 import (
 	"context"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -32,6 +33,8 @@ type RisingWaveMutatingWebhook struct{}
 // Default implements admission.CustomDefaulter.
 func (m *RisingWaveMutatingWebhook) Default(ctx context.Context, obj runtime.Object) error {
 	ConvertToV1alpha2Features(obj.(*risingwavev1alpha1.RisingWave))
+	risingwave := obj.(*risingwavev1alpha1.RisingWave)
+	risingwave.Spec.StateStore.DataDirectory = strings.TrimRight(strings.TrimSpace(risingwave.Spec.StateStore.DataDirectory), "/")
 	return nil
 }
 

diff --git a/pkg/webhook/risingwave_validating_webhook.go b/pkg/webhook/risingwave_validating_webhook.go
@@ -199,6 +199,13 @@ func (v *RisingWaveValidatingWebhook) validateMetaStoreAndStateStore(path *field
 		fieldErrs = append(fieldErrs, field.Invalid(path.Child("stateStore"), stateStore, "multiple state store types"))
 	}
 
+	if strings.HasSuffix(stateStore.DataDirectory, "/") ||
+		strings.HasPrefix(stateStore.DataDirectory, "/") ||
+		strings.Contains(stateStore.DataDirectory, "//") ||
+		len(stateStore.DataDirectory) > 800 {
+		fieldErrs = append(fieldErrs, field.Invalid(path.Child("stateStore", "dataDirectory"), stateStore.DataDirectory, "must be a valid path"))
+	}
+
 	return fieldErrs
 }
 

diff --git a/pkg/webhook/risingwave_validating_webhook_test.go b/pkg/webhook/risingwave_validating_webhook_test.go
@@ -18,6 +18,7 @@ package webhook
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	kruisepubs "github.com/openkruise/kruise-api/apps/pub"
@@ -743,6 +744,48 @@ func Test_RisingWaveValidatingWebhook_ValidateCreate(t *testing.T) {
 			},
 			pass: true,
 		},
+		"invalid-data-dir-1": {
+			patch: func(r *risingwavev1alpha1.RisingWave) {
+				r.Spec.StateStore.DataDirectory = "/"
+			},
+			pass: false,
+		},
+		"invalid-data-dir-2": {
+			patch: func(r *risingwavev1alpha1.RisingWave) {
+				r.Spec.StateStore.DataDirectory = "/a"
+			},
+			pass: false,
+		},
+		"invalid-data-dir-3": {
+			patch: func(r *risingwavev1alpha1.RisingWave) {
+				r.Spec.StateStore.DataDirectory = "a/"
+			},
+			pass: false,
+		},
+		"invalid-data-dir-4": {
+			patch: func(r *risingwavev1alpha1.RisingWave) {
+				r.Spec.StateStore.DataDirectory = "a//b"
+			},
+			pass: false,
+		},
+		"invalid-data-dir-5": {
+			patch: func(r *risingwavev1alpha1.RisingWave) {
+				r.Spec.StateStore.DataDirectory = strings.Repeat("a", 801)
+			},
+			pass: false,
+		},
+		"valid-data-dir-1": {
+			patch: func(r *risingwavev1alpha1.RisingWave) {
+				r.Spec.StateStore.DataDirectory = "a"
+			},
+			pass: true,
+		},
+		"valid-data-dir-2": {
+			patch: func(r *risingwavev1alpha1.RisingWave) {
+				r.Spec.StateStore.DataDirectory = "a/b"
+			},
+			pass: true,
+		},
 	}
 
 	for name, tc := range testcases {