diff --git a/pkg/common/env_names.go b/pkg/common/env_names.go index 20dfbdaab3a..5cae04218e5 100644 --- a/pkg/common/env_names.go +++ b/pkg/common/env_names.go @@ -28,4 +28,6 @@ const ( EnvEnableRuntimeInfoCache = "ENABLE_RUNTIMEINFO_CACHE" EnvRuntimeInfoCacheTTL = "RUNTIMEINFO_CACHE_TTL" + + EnvEnableMountValidation = "ENABLE_MOUNT_VALIDATION" ) diff --git a/pkg/controllers/v1alpha1/dataset/dataset_controller.go b/pkg/controllers/v1alpha1/dataset/dataset_controller.go index eb65f468e01..531d75c428d 100644 --- a/pkg/controllers/v1alpha1/dataset/dataset_controller.go +++ b/pkg/controllers/v1alpha1/dataset/dataset_controller.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Fluid Author. +Copyright 2024 The Fluid Author. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -18,12 +18,8 @@ import ( "context" "errors" "reflect" - "strings" "time" - "k8s.io/apimachinery/pkg/util/validation" - "k8s.io/apimachinery/pkg/util/validation/field" - "github.com/fluid-cloudnative/fluid/pkg/common" "github.com/fluid-cloudnative/fluid/pkg/controllers/deploy" "github.com/fluid-cloudnative/fluid/pkg/ddc/base" @@ -41,6 +37,7 @@ import ( datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1" "github.com/fluid-cloudnative/fluid/pkg/utils" + fluidvalidation "github.com/fluid-cloudnative/fluid/pkg/utils/validation" ) const ( @@ -124,9 +121,12 @@ func (r *DatasetReconciler) reconcileDataset(ctx reconcileRequestContext, needRe log := ctx.Log.WithName("reconcileDataset") log.V(1).Info("process the dataset", "dataset", ctx.Dataset) - // 0. Validate name is prefixed with a number such as "20-hbase". - if errs := validation.IsDNS1035Label(ctx.Dataset.ObjectMeta.Name); len(ctx.Dataset.ObjectMeta.Name) > 0 && len(errs) > 0 { - err := field.Invalid(field.NewPath("metadata").Child("name"), ctx.Dataset.ObjectMeta.Name, strings.Join(errs, ",")) + // 0. Validate the dataset. + // Users can set this environment variable to 'false' to skip the validation of the mount field in dataset + // Default is true + if err := fluidvalidation.IsValidDataset( + ctx.Dataset, utils.GetBoolValueFromEnv(common.EnvEnableMountValidation, true), + ); err != nil { ctx.Log.Error(err, "Failed to create dataset", "DatasetCreateError", ctx) r.Recorder.Eventf(&ctx.Dataset, v1.EventTypeWarning, common.ErrorCreateDataset, "Failed to create dataset because err: %v", err) return utils.RequeueIfError(err) diff --git a/pkg/utils/validation/dataset.go b/pkg/utils/validation/dataset.go new file mode 100644 index 00000000000..9e3501c03f1 --- /dev/null +++ b/pkg/utils/validation/dataset.go @@ -0,0 +1,55 @@ +/* +Copyright 2024 The Fluid Author. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "strings" + + "github.com/fluid-cloudnative/fluid/api/v1alpha1" + "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func IsValidDataset(dataset v1alpha1.Dataset, enableMountValidation bool) error { + if errs := validation.IsDNS1035Label(dataset.ObjectMeta.Name); len(dataset.ObjectMeta.Name) > 0 && len(errs) > 0 { + return field.Invalid(field.NewPath("metadata").Child("name"), dataset.ObjectMeta.Name, strings.Join(errs, ",")) + } + + // 0.1 Validate the mount name and mount path + // Users can set the environment variable to 'false' to disable this validation + // Default is true + if !enableMountValidation { + return nil + } + for _, mount := range dataset.Spec.Mounts { + // The field mount.Name and mount.Path is optional + // Empty name or path is allowed + if len(mount.Name) != 0 { + // If users set the mount.Name, it should comply with the DNS1035 rule. + if errs := validation.IsDNS1035Label(mount.Name); len(errs) > 0 { + return field.Invalid(field.NewPath("spec").Child("mounts").Child("name"), mount.Name, strings.Join(errs, ",")) + } + } + if len(mount.Path) != 0 { + // If users set the mount.Path, check it. + if err := IsValidMountPath(mount.Path); err != nil { + return field.Invalid(field.NewPath("spec").Child("mounts").Child("path"), mount.Path, err.Error()) + } + } + } + return nil +} diff --git a/pkg/utils/validation/dataset_test.go b/pkg/utils/validation/dataset_test.go new file mode 100644 index 00000000000..1ba7bc6d9ee --- /dev/null +++ b/pkg/utils/validation/dataset_test.go @@ -0,0 +1,182 @@ +/* +Copyright 2024 The Fluid Author. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "testing" + + "github.com/fluid-cloudnative/fluid/api/v1alpha1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const mountPoint1 string = "https://mirrors.bit.edu.cn/apache/spark/" +const validMountName1 string = "spark" + +const mountPoint2 string = "https://mirrors.bit.edu.cn/apache/flink/" +const validMountName2 string = "flink" + +const validMountPath1 string = "/test" +const validMountPath2 string = "mnt/test" + +func TestIsValidDatasetWithValidDataset(t *testing.T) { + type testCase struct { + name string + input v1alpha1.Dataset + enableMountValidation bool + } + + testCases := []testCase{ + { + name: "validDatasetWithSingleMount", + enableMountValidation: true, + input: v1alpha1.Dataset{ + ObjectMeta: v1.ObjectMeta{ + Name: "demo", + }, + Spec: v1alpha1.DatasetSpec{ + Mounts: []v1alpha1.Mount{ + { + MountPoint: mountPoint1, + Name: validMountName1, + Path: validMountPath1, + }, + }, + }, + }, + }, + { + name: "validDatasetWithMultiMount", + enableMountValidation: true, + input: v1alpha1.Dataset{ + Spec: v1alpha1.DatasetSpec{ + Mounts: []v1alpha1.Mount{ + { + MountPoint: mountPoint1, + Name: validMountName1, + }, + { + MountPoint: mountPoint2, + Name: validMountName2, + Path: validMountPath2, + }, + }, + }, + }, + }, + { + name: "validDatasetWithDisableMountValidation", + enableMountValidation: false, + input: v1alpha1.Dataset{ + Spec: v1alpha1.DatasetSpec{ + Mounts: []v1alpha1.Mount{ + { + MountPoint: mountPoint1, + Path: "/${TEST}", + }, + }, + }, + }, + }, + } + + for _, test := range testCases { + got := IsValidDataset(test.input, test.enableMountValidation) + if got != nil { + t.Errorf("testcase %s failed, expect no error happened, but got an error: %s", test.name, got.Error()) + } + } +} + +func TestIsValidDatasetWithInvalidDataset(t *testing.T) { + type testCase struct { + name string + input v1alpha1.Dataset + } + + testCases := []testCase{ + { + name: "invalidDatasetMountName", + input: v1alpha1.Dataset{ + Spec: v1alpha1.DatasetSpec{ + Mounts: []v1alpha1.Mount{ + { + MountPoint: mountPoint1, + Name: "$(cat /etc/passwd > /test.txt)", + Path: validMountPath1, + }, + }, + }, + }, + }, + { + name: "invalidDatasetName", + input: v1alpha1.Dataset{ + ObjectMeta: v1.ObjectMeta{ + Name: "20-hbase", + }, + Spec: v1alpha1.DatasetSpec{ + Mounts: []v1alpha1.Mount{ + { + MountPoint: mountPoint1, + Name: validMountName1, + Path: validMountPath1, + }, + }, + }, + }, + }, + { + name: "invalidDatasetMountPath", + input: v1alpha1.Dataset{ + Spec: v1alpha1.DatasetSpec{ + Mounts: []v1alpha1.Mount{ + { + MountPoint: mountPoint1, + Name: validMountName1, + Path: "/$(cat /etc/passwd > /test.txt)", + }, + }, + }, + }, + }, + { + name: "invalidDatasetMountPathInSecondMount", + input: v1alpha1.Dataset{ + Spec: v1alpha1.DatasetSpec{ + Mounts: []v1alpha1.Mount{ + { + MountPoint: mountPoint1, + Name: validMountName1, + }, + { + MountPoint: mountPoint2, + Name: validMountName2, + Path: "/test/$(cat /etc/passwd > /test.txt)", + }, + }, + }, + }, + }, + } + + for _, test := range testCases { + got := IsValidDataset(test.input, true) + if got == nil { + t.Errorf("testcase %s failed, expect an error happened, but got no error", test.name) + } + } +} diff --git a/pkg/utils/validation/validation.go b/pkg/utils/validation/path.go similarity index 55% rename from pkg/utils/validation/validation.go rename to pkg/utils/validation/path.go index c212fabeff2..424724a11be 100644 --- a/pkg/utils/validation/validation.go +++ b/pkg/utils/validation/path.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Fluid Authors. +Copyright 2024 The Fluid Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -24,6 +24,22 @@ import ( "k8s.io/apimachinery/pkg/util/validation" ) +const invalidPartOfPathErrMsg string = "every part of the path shuold follow the relaxed DNS (RFC 1123) rule which additionally allows upper case alphabetic character and character '_'" + +func isValidPartsOfPath(partsOfPath []string) error { + for _, part := range partsOfPath { + // Convert characters to lowercase and replace underscores with hyphens + part = strings.ToLower(part) + part = strings.Replace(part, "_", "-", -1) + + // If the component fails the DNS 1123 conformity test, the function returns an error + if len(validation.IsDNS1123Label(part)) > 0 { + return fmt.Errorf(invalidPartOfPathErrMsg) + } + } + return nil +} + const invalidMountRootErrMsgFmt string = "invalid mount root path '%s': %s" func IsValidMountRoot(path string) error { @@ -37,18 +53,32 @@ func IsValidMountRoot(path string) error { // The path is an absolute path and to avoid an empty part, we omit the first '/' parts := strings.Split(filepath.Clean(path)[1:], string(filepath.Separator)) - for _, part := range parts { - // Convert characters to lowercase and replace underscores with hyphens - part = strings.ToLower(part) - part = strings.Replace(part, "_", "-", -1) + // Validate every part of the path + if err := isValidPartsOfPath(parts); err != nil { + return fmt.Errorf(invalidMountRootErrMsgFmt, path, err.Error()) + } - // If the component fails the DNS 1123 conformity test, the function returns an error - errs := validation.IsDNS1123Label(part) - if len(errs) > 0 { - return fmt.Errorf(invalidMountRootErrMsgFmt, path, "every directory name in the mount root path shuold follow the relaxed DNS (RFC 1123) rule which additionally allows upper case alphabetic character and character '_'") - } + // If all components pass the check, the function returns nil + return nil +} + +func IsValidMountPath(path string) error { + if len(path) == 0 { + return fmt.Errorf("the mount path is empty") + } + // Normalize the path and split it into components + path = filepath.Clean(path) + if filepath.IsAbs(path) { + // The path is an absolute path and to avoid an empty part, we omit the first '/' + path = path[1:] + } + parts := strings.Split(path, string(filepath.Separator)) + + // Validate every part of the path + if err := isValidPartsOfPath(parts); err != nil { + return err } - // If all components pass the DNS 1123 check, the function returns nil + // If all components pass the check, the function returns nil return nil } diff --git a/pkg/utils/validation/path_test.go b/pkg/utils/validation/path_test.go new file mode 100644 index 00000000000..0abb96e2c72 --- /dev/null +++ b/pkg/utils/validation/path_test.go @@ -0,0 +1,170 @@ +/* +Copyright 2024 The Fluid Author. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "fmt" + "testing" +) + +func TestIsValidMountRootWithValidPath(t *testing.T) { + + type testCase struct { + name string + input string + } + + testCases := []testCase{ + { + name: "validPath-1", + input: "/runtime-mnt//alluxio/default/hbase", + }, + { + name: "validPath-2", + input: "/opt/20-Runtime-Mnt_1/./alluxio/default/hbase", + }, + } + + for _, test := range testCases { + got := IsValidMountRoot(test.input) + if got != nil { + t.Errorf("testcase %s failed, expect no error happened, but got an error: %s", test.name, got.Error()) + } + } +} + +const invalidTestPath1 string = "/$test/alluxio/default/hbase" +const invalidTestPath2 string = "/test/(alluxio)/default/hbase" +const invalidTestPath3 string = "/test/alluxio/def;ault/hbase" + +func TestIsValidMountRootWithInvalidPath(t *testing.T) { + + type testCase struct { + name string + input string + expect error + } + + testCases := []testCase{ + { + name: "invalidPath-1", + input: invalidTestPath1, + expect: fmt.Errorf(invalidMountRootErrMsgFmt, invalidTestPath1, invalidPartOfPathErrMsg), + }, + { + name: "invalidPath-2", + input: invalidTestPath2, + expect: fmt.Errorf(invalidMountRootErrMsgFmt, invalidTestPath2, invalidPartOfPathErrMsg), + }, + { + name: "invalidPath-3", + input: invalidTestPath3, + expect: fmt.Errorf(invalidMountRootErrMsgFmt, invalidTestPath3, invalidPartOfPathErrMsg), + }, + { + name: "invalidPath-4", + input: "", + expect: fmt.Errorf(invalidMountRootErrMsgFmt, "", "the mount root path is empty"), + }, + { + name: "invalidPath-5", + input: "runtime-mnt/default", + expect: fmt.Errorf(invalidMountRootErrMsgFmt, "runtime-mnt/default", "the mount root path must be an absolute path"), + }, + } + + for _, test := range testCases { + got := IsValidMountRoot(test.input) + if got == nil { + t.Errorf("testcase %s failed, expect an error happened, but got no error", test.name) + } + if got.Error() != test.expect.Error() { + t.Errorf("testcase %s failed, expect error: %v, but got error: %v", test.name, test.expect, got) + } + } +} + +func TestIsValidMountPathWithValidPath(t *testing.T) { + type testCase struct { + name string + input string + } + + testCases := []testCase{ + { + name: "validPath-1", + input: "/runtime-mnt//alluxio/default/hbase", + }, + { + name: "validPath-2", + input: "/opt/20-Runtime-Mnt_1/./alluxio/default/hbase", + }, + { + name: "validPath-3", + input: "runtime-mnt/test", + }, + } + + for _, test := range testCases { + got := IsValidMountPath(test.input) + if got != nil { + t.Errorf("testcase %s failed, expect no error happened, but got an error: %s", test.name, got.Error()) + } + } +} + +func TestIsValidMountPathWithInvalidPath(t *testing.T) { + + type testCase struct { + name string + input string + expect error + } + + testCases := []testCase{ + { + name: "invalidPath-1", + input: invalidTestPath1, + expect: fmt.Errorf(invalidPartOfPathErrMsg), + }, + { + name: "invalidPath-2", + input: invalidTestPath2, + expect: fmt.Errorf(invalidPartOfPathErrMsg), + }, + { + name: "invalidPath-3", + input: invalidTestPath3, + expect: fmt.Errorf(invalidPartOfPathErrMsg), + }, + { + name: "invalidPath-4", + input: "", + expect: fmt.Errorf("the mount path is empty"), + }, + } + + for _, test := range testCases { + got := IsValidMountPath(test.input) + if got == nil { + t.Errorf("testcase %s failed, expect an error happened, but got no error", test.name) + } + if got.Error() != test.expect.Error() { + t.Errorf("testcase %s failed, expect error: %v, but got error: %v", test.name, test.expect, got) + } + } +} diff --git a/pkg/utils/validation/validation_test.go b/pkg/utils/validation/validation_test.go deleted file mode 100644 index c802a0df21d..00000000000 --- a/pkg/utils/validation/validation_test.go +++ /dev/null @@ -1,98 +0,0 @@ -/* -Copyright 2023 The Fluid Author. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package validation - -import ( - "fmt" - "path/filepath" - "testing" -) - -func TestIsSafePathWithSafePath(t *testing.T) { - - type testCase struct { - name string - input string - } - - testCases := []testCase{ - { - name: "validPath-1", - input: "/runtime-mnt//alluxio/default/hbase", - }, - { - name: "validPath-2", - input: "/opt/20-Runtime-Mnt_1/./alluxio/default/hbase", - }, - } - - for _, test := range testCases { - tt := filepath.Clean(test.input) - print(tt) - got := IsValidMountRoot(test.input) - if got != nil { - t.Errorf("testcase %s failed, expect no error happened, but got an error: %s", test.name, got.Error()) - } - } -} - -func TestIsSafePathWithInvalidPath(t *testing.T) { - - type testCase struct { - name string - input string - expect error - } - - testCases := []testCase{ - { - name: "invalidPath-1", - input: "/$test/alluxio/default/hbase", - expect: fmt.Errorf(invalidMountRootErrMsgFmt, "/$test/alluxio/default/hbase", "every directory name in the mount root path shuold follow the relaxed DNS (RFC 1123) rule which additionally allows upper case alphabetic character and character '_'"), - }, - { - name: "invalidPath-2", - input: "/test/(alluxio)/default/hbase", - expect: fmt.Errorf(invalidMountRootErrMsgFmt, "/test/(alluxio)/default/hbase", "every directory name in the mount root path shuold follow the relaxed DNS (RFC 1123) rule which additionally allows upper case alphabetic character and character '_'"), - }, - { - name: "invalidPath-3", - input: "/test/alluxio/def;ault/hbase", - expect: fmt.Errorf(invalidMountRootErrMsgFmt, "/test/alluxio/def;ault/hbase", "every directory name in the mount root path shuold follow the relaxed DNS (RFC 1123) rule which additionally allows upper case alphabetic character and character '_'"), - }, - { - name: "invalidPath-4", - input: "", - expect: fmt.Errorf(invalidMountRootErrMsgFmt, "", "the mount root path is empty"), - }, - { - name: "invalidPath-5", - input: "runtime-mnt/default", - expect: fmt.Errorf(invalidMountRootErrMsgFmt, "runtime-mnt/default", "the mount root path must be an absolute path"), - }, - } - - for _, test := range testCases { - got := IsValidMountRoot(test.input) - if got == nil { - t.Errorf("testcase %s failed, expect an error happened, but got no error", test.name) - } - if got.Error() != test.expect.Error() { - t.Errorf("testcase %s failed, expect error: %v, but got error: %v", test.name, test.expect, got) - } - } -}