fluid icon indicating copy to clipboard operation
fluid copied to clipboard

Validate fields mount name and mount path in Dataset

Open zhang-x-z opened this issue 2 years ago • 3 comments

Ⅰ. Describe what this PR does

This PR validate fields mount.Name and mount.Path when creating dataset.

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Start new Dataset Controller in local and apply different datasets, check the status of these datasets.

  • Test Case 1: Create valid dataset with single mount.
$ cat test-dataset-1.yaml 
apiVersion: data.fluid.io/v1alpha1
kind: Dataset
metadata:
  name: demo
spec:
  mounts:
    - mountPoint: https://mirrors.bit.edu.cn/apache/spark/
      name: spark
      path: /test
$ kubectl apply -f test-dataset-1.yaml 
dataset.data.fluid.io/demo created
$ kubectl get dataset demo
NAME   UFS TOTAL SIZE   CACHED   CACHE CAPACITY   CACHED PERCENTAGE   PHASE      AGE
demo                                                                  NotBound   7s

Dataset status switch to NotBound successfully.

  • Test Case 2: Create valid dataset with multi mounts.
$ cat test-dataset-2.yaml
apiVersion: data.fluid.io/v1alpha1
kind: Dataset
metadata:
  name: demo
spec:
  mounts:
    - mountPoint: https://mirrors.bit.edu.cn/apache/spark/
      name: spark
    - mountPoint: https://mirrors.bit.edu.cn/apache/flink/
      name: flink
$ kubectl apply -f test-dataset-2.yaml 
dataset.data.fluid.io/demo created
$ kubectl get dataset demo
NAME   UFS TOTAL SIZE   CACHED   CACHE CAPACITY   CACHED PERCENTAGE   PHASE      AGE
demo                                                                  NotBound   7s

Dataset status switch to NotBound successfully.

  • Test Case 3: Create invalid mount name in dataset with single mount.
$ cat test-dataset-3.yaml 
apiVersion: data.fluid.io/v1alpha1
kind: Dataset
metadata:
  name: demo
spec:
  mounts:
    - mountPoint: https://mirrors.bit.edu.cn/apache/spark/
      name: $(cat /proc/self/status | grep CapEff > /test.txt)
      path: /test
$ kubectl apply -f test-dataset-3.yaml 
dataset.data.fluid.io/demo created
$ kubectl get dataset demo
NAME   UFS TOTAL SIZE   CACHED   CACHE CAPACITY   CACHED PERCENTAGE   PHASE   AGE
demo                                                                          66s

Dataset status didn't switch to NotBound after a long time. Logs in Dataset Controller:

ERROR	datasetctl.Dataset	dataset/dataset_controller.go:109	Failed to create dataset	{"dataset": "default/demo", "DatasetCreateError": "default/demo", "error": "spec.mounts.name: Invalid value: \"$(cat /proc/self/status | grep CapEff > /test.txt)\": a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name',  or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')"}
  • Test Case 4: Create invalid mount path in dataset with single mount.
$ cat test-dataset-4.yaml 
apiVersion: data.fluid.io/v1alpha1
kind: Dataset
metadata:
  name: demo
spec:
  mounts:
    - mountPoint: https://mirrors.bit.edu.cn/apache/spark/
      name: spark
      path: /$(cat /proc/self/status | grep CapEff > /test.txt)/test
$ kubectl apply -f test-dataset-4.yaml 
dataset.data.fluid.io/demo created
$ kubectl get dataset demo
NAME   UFS TOTAL SIZE   CACHED   CACHE CAPACITY   CACHED PERCENTAGE   PHASE   AGE
demo                                                                          92s

Dataset status didn't switch to NotBound after a long time. Logs in Dataset Controller:

ERROR	datasetctl.Dataset	dataset/dataset_controller.go:109	Failed to create dataset	{"dataset": "default/demo", "DatasetCreateError": "default/demo", "error": "spec.mounts.path: Invalid value: \"/$(cat /proc/self/status | grep CapEff > /test.txt)/test\": every part of the path shuold follow the relaxed DNS (RFC 1123) rule which additionally allows upper case alphabetic character and character '_'"}
  • Test Case 5: Create invalid mount path in the second mount of the dataset.
$ cat test-dataset-5.yaml 
apiVersion: data.fluid.io/v1alpha1
kind: Dataset
metadata:
  name: demo
spec:
  mounts:
    - mountPoint: https://mirrors.bit.edu.cn/apache/spark/
      name: spark
    - mountPoint: https://mirrors.bit.edu.cn/apache/flink/
      name: flink
      path: /$(cat /proc/self/status | grep CapEff > /test.txt)/test2
$ kubectl apply -f test-dataset-5.yaml 
dataset.data.fluid.io/demo created
$ kubectl get dataset demo
NAME   UFS TOTAL SIZE   CACHED   CACHE CAPACITY   CACHED PERCENTAGE   PHASE   AGE
demo                                                                          95s

Dataset status didn't switch to NotBound after a long time. Logs in Dataset Controller:

ERROR	datasetctl.Dataset	dataset/dataset_controller.go:109	Failed to create dataset	{"dataset": "default/demo", "DatasetCreateError": "default/demo", "error": "spec.mounts.path: Invalid value: \"/$(cat /proc/self/status | grep CapEff > /test.txt)/test2\": every part of the path shuold follow the relaxed DNS (RFC 1123) rule which additionally allows upper case alphabetic character and character '_'"}
  • Test Case 6: Test disable validation.
$ cat test-dataset-6.yaml 
apiVersion: data.fluid.io/v1alpha1
kind: Dataset
metadata:
  name: demo
spec:
  mounts:
    - mountPoint: https://mirrors.bit.edu.cn/apache/spark/
      name: ${TEST}
      path: /test
$ kubectl apply -f test-dataset-6.yaml 
dataset.data.fluid.io/demo created
$ kubectl get dataset demo
NAME   UFS TOTAL SIZE   CACHED   CACHE CAPACITY   CACHED PERCENTAGE   PHASE      AGE
demo                                                                  NotBound   7s

Dataset status switch to NotBound successfully.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

zhang-x-z avatar Jan 11 '24 08:01 zhang-x-z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign cheyang for approval by writing /assign @cheyang in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

fluid-e2e-bot[bot] avatar Jan 11 '24 08:01 fluid-e2e-bot[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 64.49%. Comparing base (453b086) to head (e13a16a). Report is 126 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3687      +/-   ##
==========================================
+ Coverage   64.47%   64.49%   +0.02%     
==========================================
  Files         471      472       +1     
  Lines       28140    28179      +39     
==========================================
+ Hits        18142    18175      +33     
- Misses       7844     7848       +4     
- Partials     2154     2156       +2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 11 '24 09:01 codecov[bot]

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

sonarqubecloud[bot] avatar Jan 17 '24 06:01 sonarqubecloud[bot]