Skip to content

Commit

Permalink
Fix JuicefsRuntime: escape customized string before constructing comm…
Browse files Browse the repository at this point in the history
…ands (#3761)

* add escapeBashStr

Signed-off-by: xixi <hexilee@juicedata.io>

* avoid bash -c in operations

Signed-off-by: xixi <hexilee@juicedata.io>

* fix GetUsedSpace and GetFileCount

Signed-off-by: xixi <hexilee@juicedata.io>

* move EscapeBashStr to pkg/utils/security

Signed-off-by: xixi <hexilee@juicedata.io>

* add left

Signed-off-by: xixi <hexilee@juicedata.io>

* resume GetFileCount

Signed-off-by: xixi <hexilee@juicedata.io>

* Escape value.Configs.Name

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Fix unit tests

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Upgrade juicefs helm chart version to 0.2.16

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

---------

Signed-off-by: xixi <hexilee@juicedata.io>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Co-authored-by: xixi <hexilee@juicedata.io>
Co-authored-by: trafalgarzzz <trafalgarz@outlook.com>
  • Loading branch information
3 people authored Mar 14, 2024
1 parent 68ea529 commit 02b7cd8
Show file tree
Hide file tree
Showing 8 changed files with 173 additions and 38 deletions.
2 changes: 1 addition & 1 deletion charts/juicefs/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: juicefs
apiVersion: v2
description: FileSystem aimed for data analytics and machine learning in any cloud.
version: 0.2.15
version: 0.2.16
appVersion: v1.0.0
home: https://juicefs.com/
maintainers:
Expand Down
23 changes: 14 additions & 9 deletions pkg/ddc/juicefs/operations/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

"github.com/fluid-cloudnative/fluid/pkg/utils/cmdguard"
"github.com/fluid-cloudnative/fluid/pkg/utils/kubeclient"
"github.com/fluid-cloudnative/fluid/pkg/utils/security"
)

type JuiceFileUtils struct {
Expand Down Expand Up @@ -113,12 +114,11 @@ func (j JuiceFileUtils) Count(juiceSubPath string) (total int64, err error) {
func (j JuiceFileUtils) GetFileCount(juiceSubPath string) (fileCount int64, err error) {
var (
//strs = "du -ah juiceSubPath |grep ^- |wc -l "
strs = fmt.Sprintf("ls -lR %s |grep ^- |wc -l ", juiceSubPath)
strs = fmt.Sprintf("ls -lR %s |grep ^- |wc -l ", security.EscapeBashStr(juiceSubPath))
command = []string{"bash", "-c", strs}
stdout string
stderr string
)

stdout, stderr, err = j.exec(command)
if err != nil {
err = fmt.Errorf("execute command %v with expectedErr: %v stdout %s and stderr %s", command, err, stdout, stderr)
Expand Down Expand Up @@ -249,11 +249,10 @@ func (j JuiceFileUtils) GetMetric(juicefsPath string) (metrics string, err error
}

// GetUsedSpace Get used space in byte
// use "df --block-size=1 |grep <juicefsPath>'"
// equal to `df --block-size=1 | grep juicefsPath`
func (j JuiceFileUtils) GetUsedSpace(juicefsPath string) (usedSpace int64, err error) {
var (
strs = fmt.Sprintf(`df --block-size=1 |grep %s`, juicefsPath)
command = []string{"bash", "-c", strs}
command = []string{"df", "--block-size=1"}
stdout string
stderr string
)
Expand All @@ -264,9 +263,15 @@ func (j JuiceFileUtils) GetUsedSpace(juicefsPath string) (usedSpace int64, err e
return
}

var str string
lines := strings.Split(stdout, "\n")
for _, line := range lines {
if strings.Contains(line, juicefsPath) {
str = line
break
}
}
// [<Filesystem> <Size> <Used> <Avail> <Use>% <Mounted on>]
str := strings.TrimSuffix(stdout, "\n")

data := strings.Fields(str)
if len(data) != 6 {
err = fmt.Errorf("failed to parse %s in GetUsedSpace method", data)
Expand Down Expand Up @@ -354,8 +359,8 @@ func (j JuiceFileUtils) QueryMetaDataInfoIntoFile(key KeyOfMetaDataFile, filenam
j.log.Error(errors.New("the key not in metadatafile"), "key", key)
}
var (
str = "sed -n '" + line + "' " + filename
command = []string{"bash", "-c", str}
str = "'" + line + "' " + filename
command = []string{"sed", "-n", str}
stdout string
stderr string
)
Expand Down
4 changes: 2 additions & 2 deletions pkg/ddc/juicefs/operations/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ func TestJuiceFileUtils_GetUsedSpace(t *testing.T) {
t.Fatal(err.Error())
}
a := &JuiceFileUtils{log: fake.NullLogger()}
_, err = a.GetUsedSpace("/tmp")
_, err = a.GetUsedSpace("/runtime-mnt/juicefs/kube-system/jfsdemo/juicefs-fuse")
if err == nil {
t.Error("check failure, want err, got nil")
}
Expand All @@ -457,7 +457,7 @@ func TestJuiceFileUtils_GetUsedSpace(t *testing.T) {
if err != nil {
t.Fatal(err.Error())
}
usedSpace, err := a.GetUsedSpace("/tmp")
usedSpace, err := a.GetUsedSpace("/runtime-mnt/juicefs/kube-system/jfsdemo/juicefs-fuse")
if err != nil {
t.Errorf("check failure, want nil, got err: %v", err)
}
Expand Down
21 changes: 17 additions & 4 deletions pkg/ddc/juicefs/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/fluid-cloudnative/fluid/pkg/ddc/base/portallocator"
"github.com/fluid-cloudnative/fluid/pkg/utils"
"github.com/fluid-cloudnative/fluid/pkg/utils/docker"
"github.com/fluid-cloudnative/fluid/pkg/utils/security"
"github.com/fluid-cloudnative/fluid/pkg/utils/transfromer"
)

Expand Down Expand Up @@ -202,26 +203,38 @@ func (j *JuiceFSEngine) genWorkerMount(value *JuiceFS, workerOptionMap map[strin
}
workerOptionMap["metrics"] = fmt.Sprintf("0.0.0.0:%d", metricsPort)
}
mountArgsWorker = []string{common.JuiceFSCeMountPath, value.Source, value.Worker.MountPath, "-o", strings.Join(genArgs(workerOptionMap), ",")}
mountArgsWorker = []string{
common.JuiceFSCeMountPath,
value.Source,
security.EscapeBashStr(value.Worker.MountPath),
"-o",
security.EscapeBashStr(strings.Join(genArgs(workerOptionMap), ",")),
}
} else {
workerOptionMap["foreground"] = ""
// do not update config again
workerOptionMap["no-update"] = ""

// start independent cache cluster, refer to [juicefs cache sharing](https://juicefs.com/docs/cloud/cache/#client_cache_sharing)
// fuse and worker use the same cache-group, fuse use no-sharing
cacheGroup := fmt.Sprintf("%s-%s", j.namespace, value.FullnameOverride)
cacheGroup := fmt.Sprintf("%s-%s", j.namespace, security.EscapeBashStr(value.FullnameOverride))
if _, ok := workerOptionMap["cache-group"]; ok {
cacheGroup = workerOptionMap["cache-group"]
}
workerOptionMap["cache-group"] = cacheGroup
delete(workerOptionMap, "no-sharing")

mountArgsWorker = []string{common.JuiceFSMountPath, value.Source, value.Worker.MountPath, "-o", strings.Join(genArgs(workerOptionMap), ",")}
mountArgsWorker = []string{
common.JuiceFSMountPath,
value.Source,
security.EscapeBashStr(value.Worker.MountPath),
"-o",
security.EscapeBashStr(strings.Join(genArgs(workerOptionMap), ",")),
}
}

value.Worker.Command = strings.Join(mountArgsWorker, " ")
value.Worker.StatCmd = "stat -c %i " + value.Worker.MountPath
value.Worker.StatCmd = "stat -c %i " + security.EscapeBashStr(value.Worker.MountPath)
}

func (j *JuiceFSEngine) transformPlacementMode(dataset *datav1alpha1.Dataset, value *JuiceFS) {
Expand Down
47 changes: 30 additions & 17 deletions pkg/ddc/juicefs/transform_fuse.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1"
"github.com/fluid-cloudnative/fluid/pkg/common"
"github.com/fluid-cloudnative/fluid/pkg/utils"
"github.com/fluid-cloudnative/fluid/pkg/utils/security"
)

func (j *JuiceFSEngine) transformFuse(runtime *datav1alpha1.JuiceFSRuntime, dataset *datav1alpha1.Dataset, value *JuiceFS) (err error) {
Expand All @@ -36,7 +37,7 @@ func (j *JuiceFSEngine) transformFuse(runtime *datav1alpha1.JuiceFSRuntime, data
}
mount := dataset.Spec.Mounts[0]

value.Configs.Name = mount.Name
value.Configs.Name = security.EscapeBashStr(mount.Name)

// transform image
image := runtime.Spec.Fuse.Image
Expand Down Expand Up @@ -129,7 +130,7 @@ func (j *JuiceFSEngine) transformFuseNodeSelector(runtime *datav1alpha1.JuiceFSR
func (j *JuiceFSEngine) genValue(mount datav1alpha1.Mount, tiredStoreLevel *datav1alpha1.Level, value *JuiceFS,
sharedOptions map[string]string, sharedEncryptOptions []datav1alpha1.EncryptOption) (map[string]string, error) {
options := make(map[string]string)
value.Configs.Name = mount.Name
value.Configs.Name = security.EscapeBashStr(mount.Name)
value.Configs.EncryptEnvOptions = make([]EncryptEnvOption, 0)
source := ""

Expand Down Expand Up @@ -238,7 +239,7 @@ func (j *JuiceFSEngine) genValue(mount datav1alpha1.Mount, tiredStoreLevel *data
}

if source == "" {
source = mount.Name
source = security.EscapeBashStr(mount.Name)
}

// transform source
Expand Down Expand Up @@ -355,7 +356,13 @@ func (j *JuiceFSEngine) genFuseMount(value *JuiceFS, optionMap map[string]string
}
optionMap["metrics"] = fmt.Sprintf("0.0.0.0:%d", metricsPort)
}
mountArgs = []string{common.JuiceFSCeMountPath, value.Source, value.Fuse.MountPath, "-o", strings.Join(genArgs(optionMap), ",")}
mountArgs = []string{
common.JuiceFSCeMountPath,
value.Source,
security.EscapeBashStr(value.Fuse.MountPath),
"-o",
security.EscapeBashStr(strings.Join(genArgs(optionMap), ",")),
}
} else {
if readonly {
optionMap["attrcacheto"] = "7200"
Expand All @@ -374,11 +381,17 @@ func (j *JuiceFSEngine) genFuseMount(value *JuiceFS, optionMap map[string]string
optionMap["cache-group"] = cacheGroup
optionMap["no-sharing"] = ""

mountArgs = []string{common.JuiceFSMountPath, value.Source, value.Fuse.MountPath, "-o", strings.Join(genArgs(optionMap), ",")}
mountArgs = []string{
common.JuiceFSMountPath,
value.Source,
security.EscapeBashStr(value.Fuse.MountPath),
"-o",
security.EscapeBashStr(strings.Join(genArgs(optionMap), ",")),
}
}

value.Fuse.Command = strings.Join(mountArgs, " ")
value.Fuse.StatCmd = "stat -c %i " + value.Fuse.MountPath
value.Fuse.StatCmd = "stat -c %i " + security.EscapeBashStr(value.Fuse.MountPath)
return nil
}

Expand Down Expand Up @@ -408,7 +421,7 @@ func (j *JuiceFSEngine) genFormatCmd(value *JuiceFS, config *[]string, options m
for _, option := range *config {
o := strings.TrimSpace(option)
if o != "" {
args = append(args, fmt.Sprintf("--%s", o))
args = append(args, fmt.Sprintf("--%s", security.EscapeBashStr(o)))
}
}
}
Expand All @@ -424,20 +437,20 @@ func (j *JuiceFSEngine) genFormatCmd(value *JuiceFS, config *[]string, options m
args = append(args, "--no-update")
}
if value.Configs.Storage != "" {
args = append(args, fmt.Sprintf("--storage=%s", value.Configs.Storage))
args = append(args, fmt.Sprintf("--storage=%s", security.EscapeBashStr(value.Configs.Storage)))
}
if value.Configs.Bucket != "" {
args = append(args, fmt.Sprintf("--bucket=%s", value.Configs.Bucket))
args = append(args, fmt.Sprintf("--bucket=%s", security.EscapeBashStr(value.Configs.Bucket)))
}
formatOpts := ceFilter.filterOption(options)
for k, v := range formatOpts {
args = append(args, fmt.Sprintf("--%s=%s", k, v))
args = append(args, fmt.Sprintf("--%s=%s", security.EscapeBashStr(k), security.EscapeBashStr(v)))
}
encryptOptions := ceFilter.filterEncryptEnvOptions(value.Configs.EncryptEnvOptions)
for _, v := range encryptOptions {
args = append(args, fmt.Sprintf("--%s=${%s}", v.Name, v.EnvName))
args = append(args, fmt.Sprintf("--%s=${%s}", security.EscapeBashStr(v.Name), v.EnvName))
}
args = append(args, value.Source, value.Configs.Name)
args = append(args, value.Source, security.EscapeBashStr(value.Configs.Name))
cmd := append([]string{common.JuiceCeCliPath, "format"}, args...)
value.Configs.FormatCmd = strings.Join(cmd, " ")
return
Expand All @@ -455,15 +468,15 @@ func (j *JuiceFSEngine) genFormatCmd(value *JuiceFS, config *[]string, options m
args = append(args, "--secretkey=${SECRET_KEY}")
}
if value.Configs.Bucket != "" {
args = append(args, fmt.Sprintf("--bucket=%s", value.Configs.Bucket))
args = append(args, fmt.Sprintf("--bucket=%s", security.EscapeBashStr(value.Configs.Bucket)))
}
formatOpts := eeFilter.filterOption(options)
for k, v := range formatOpts {
args = append(args, fmt.Sprintf("--%s=%s", k, v))
args = append(args, fmt.Sprintf("--%s=%s", security.EscapeBashStr(k), security.EscapeBashStr(v)))
}
encryptOptions := eeFilter.filterEncryptEnvOptions(value.Configs.EncryptEnvOptions)
for _, v := range encryptOptions {
args = append(args, fmt.Sprintf("--%s=${%s}", v.Name, v.EnvName))
args = append(args, fmt.Sprintf("--%s=${%s}", security.EscapeBashStr(v.Name), v.EnvName))
}
args = append(args, value.Source)
cmd := append([]string{common.JuiceCliPath, "auth"}, args...)
Expand Down Expand Up @@ -499,13 +512,13 @@ func (j *JuiceFSEngine) genQuotaCmd(value *JuiceFS, mount datav1alpha1.Mount) er
if value.Edition == CommunityEdition {
// ce
// juicefs quota set ${metaurl} --path ${path} --capacity ${capacity}
value.Configs.QuotaCmd = fmt.Sprintf("%s quota set %s --path %s --capacity %d", common.JuiceCeCliPath, value.Source, value.Fuse.SubPath, qs)
value.Configs.QuotaCmd = fmt.Sprintf("%s quota set %s --path %s --capacity %d", common.JuiceCeCliPath, value.Source, security.EscapeBashStr(value.Fuse.SubPath), qs)
return nil
}
// ee
// juicefs quota set ${metaurl} --path ${path} --capacity ${capacity}
cli := common.JuiceCliPath
value.Configs.QuotaCmd = fmt.Sprintf("%s quota set %s --path %s --capacity %d", cli, value.Source, value.Fuse.SubPath, qs)
value.Configs.QuotaCmd = fmt.Sprintf("%s quota set %s --path %s --capacity %d", cli, value.Source, security.EscapeBashStr(value.Fuse.SubPath), qs)
return nil
}
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/ddc/juicefs/ufs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ func mockExecCommandInContainerForTotalFileNums() (stdout string, stderr string,
}

func mockExecCommandInContainerForUsedStorageBytes() (stdout string, stderr string, err error) {
r := `JuiceFS:test 207300683100160 41460043776 207259223056384 1% /data`
r := `JuiceFS:test 207300683100160 41460043776 207259223056384 1% /juicefs/juicefs/test/juicefs-fuse`
return r, "", nil
}

func TestTotalStorageBytes(t *testing.T) {
statefulSet := &appsv1.StatefulSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-worker",
Namespace: "fluid",
Namespace: "juicefs",
},
Spec: appsv1.StatefulSetSpec{
Selector: &metav1.LabelSelector{
Expand All @@ -57,7 +57,7 @@ func TestTotalStorageBytes(t *testing.T) {
var pod = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test-work-0",
Namespace: "fluid",
Namespace: "juicefs",
Labels: map[string]string{"a": "b"},
},
Status: corev1.PodStatus{
Expand Down Expand Up @@ -93,11 +93,11 @@ func TestTotalStorageBytes(t *testing.T) {
name: "test",
fields: fields{
name: "test",
namespace: "fluid",
namespace: "juicefs",
runtime: &datav1alpha1.JuiceFSRuntime{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "fluid",
Namespace: "juicefs",
},
},
},
Expand Down
63 changes: 63 additions & 0 deletions pkg/utils/security/escape.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
Copyright 2023 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.
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 security

import (
"fmt"
"strings"
)

// According to https://www.gnu.org/software/bash/manual/html_node/ANSI_002dC-Quoting.html#ANSI_002dC-Quoting
// a -> a
// a b -> a b
// $a -> $'$a'
// $'a' -> $'$\'$a'\'
func EscapeBashStr(s string) string {
if !containsOne(s, []rune{'$', '`', '&', ';', '>', '|', '(', ')'}) {
return s
}
s = strings.ReplaceAll(s, `\`, `\\`)
s = strings.ReplaceAll(s, `'`, `\'`)
if strings.Contains(s, `\\`) {
s = strings.ReplaceAll(s, `\\\\`, `\\`)
s = strings.ReplaceAll(s, `\\\'`, `\'`)
s = strings.ReplaceAll(s, `\\"`, `\"`)
s = strings.ReplaceAll(s, `\\a`, `\a`)
s = strings.ReplaceAll(s, `\\b`, `\b`)
s = strings.ReplaceAll(s, `\\e`, `\e`)
s = strings.ReplaceAll(s, `\\E`, `\E`)
s = strings.ReplaceAll(s, `\\n`, `\n`)
s = strings.ReplaceAll(s, `\\r`, `\r`)
s = strings.ReplaceAll(s, `\\t`, `\t`)
s = strings.ReplaceAll(s, `\\v`, `\v`)
s = strings.ReplaceAll(s, `\\?`, `\?`)
}
return fmt.Sprintf(`$'%s'`, s)
}

func containsOne(target string, chars []rune) bool {
charMap := make(map[rune]bool, len(chars))
for _, c := range chars {
charMap[c] = true
}
for _, s := range target {
if charMap[s] {
return true
}
}
return false
}
Loading

0 comments on commit 02b7cd8

Please sign in to comment.