From ad19aa970ed4cdaed3e490ba50b5796ae8b881bf Mon Sep 17 00:00:00 2001 From: Allen Zhong Date: Wed, 19 Jan 2022 11:24:45 +0800 Subject: [PATCH 1/3] cluster: fix malformed commands in local executor --- pkg/cluster/executor/local.go | 11 +++++++---- pkg/cluster/executor/local_test.go | 4 ++-- pkg/cluster/spec/grafana.go | 12 ++++++------ pkg/cluster/spec/monitoring.go | 8 ++++---- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/pkg/cluster/executor/local.go b/pkg/cluster/executor/local.go index 9471e9552c..4db7a4670f 100644 --- a/pkg/cluster/executor/local.go +++ b/pkg/cluster/executor/local.go @@ -41,11 +41,14 @@ var _ ctxt.Executor = &Local{} // Execute implements Executor interface. func (l *Local) Execute(ctx context.Context, cmd string, sudo bool, timeout ...time.Duration) ([]byte, []byte, error) { + // change wd to default home + cmd = fmt.Sprintf("cd; %s", cmd) + // try to acquire root permission if l.Sudo || sudo { - cmd = fmt.Sprintf("/usr/bin/sudo -H -u root bash -c 'cd; %s'", cmd) + cmd = fmt.Sprintf("/usr/bin/sudo -H -u root bash -c \"%s\"", strings.ReplaceAll(cmd, "\"", "\\\"")) } else { - cmd = fmt.Sprintf("/usr/bin/sudo -H -u %s bash -c 'cd; %s'", l.Config.User, cmd) + cmd = fmt.Sprintf("/usr/bin/sudo -H -u %s bash -c \"%s\"", l.Config.User, strings.ReplaceAll(cmd, "\"", "\\\"")) } // set a basic PATH in case it's empty on login @@ -67,7 +70,7 @@ func (l *Local) Execute(ctx context.Context, cmd string, sudo bool, timeout ...t defer cancel() } - command := exec.CommandContext(ctx, "/bin/sh", "-c", cmd) + command := exec.CommandContext(ctx, "/bin/bash", "-c", cmd) stdout := new(bytes.Buffer) stderr := new(bytes.Buffer) @@ -117,7 +120,7 @@ func (l *Local) Transfer(ctx context.Context, src, dst string, download bool, li cmd = fmt.Sprintf("/usr/bin/sudo -H -u root bash -c \"cp %[1]s %[2]s && chown %[3]s:$(id -g -n %[3]s) %[2]s\"", src, dst, l.Config.User) } - command := exec.Command("/bin/sh", "-c", cmd) + command := exec.Command("/bin/bash", "-c", cmd) stdout := new(bytes.Buffer) stderr := new(bytes.Buffer) command.Stdout = stdout diff --git a/pkg/cluster/executor/local_test.go b/pkg/cluster/executor/local_test.go index 60fe8141b0..f5fce78499 100644 --- a/pkg/cluster/executor/local_test.go +++ b/pkg/cluster/executor/local_test.go @@ -86,8 +86,8 @@ func TestLocalExecuteWithQuotes(t *testing.T) { defer os.RemoveAll(deployDir) cmds := []string{ - fmt.Sprintf(`find %s -type f -exec sed -i "s/\${DS_.*-CLUSTER}/hello/g" {} \;`, deployDir), - fmt.Sprintf(`find %s -type f -exec sed -i "s/DS_.*-CLUSTER/hello/g" {} \;`, deployDir), + fmt.Sprintf(`find %s -type f -exec sed -i 's/\${DS_.*-CLUSTER}/hello/g' {} \;`, deployDir), + fmt.Sprintf(`find %s -type f -exec sed -i 's/DS_.*-CLUSTER/hello/g' {} \;`, deployDir), `ls '/tmp'`, } for _, cmd := range cmds { diff --git a/pkg/cluster/spec/grafana.go b/pkg/cluster/spec/grafana.go index 53a9dfe8e2..42af998589 100644 --- a/pkg/cluster/spec/grafana.go +++ b/pkg/cluster/spec/grafana.go @@ -268,12 +268,12 @@ func (i *GrafanaInstance) initDashboards(ctx context.Context, e ctxt.Executor, s // Deal with the cluster name for _, cmd := range []string{ - `find %s -type f -exec sed -i "s/\${DS_.*-CLUSTER}/%s/g" {} \;`, - `find %s -type f -exec sed -i "s/DS_.*-CLUSTER/%s/g" {} \;`, - `find %s -type f -exec sed -i "s/\${DS_LIGHTNING}/%s/g" {} \;`, - `find %s -type f -exec sed -i "s/DS_LIGHTNING/%s/g" {} \;`, - `find %s -type f -exec sed -i "s/test-cluster/%s/g" {} \;`, - `find %s -type f -exec sed -i "s/Test-Cluster/%s/g" {} \;`, + `find %s -type f -exec sed -i 's/\${DS_.*-CLUSTER}/%s/g' {} \;`, + `find %s -type f -exec sed -i 's/DS_.*-CLUSTER/%s/g' {} \;`, + `find %s -type f -exec sed -i 's/\${DS_LIGHTNING}/%s/g' {} \;`, + `find %s -type f -exec sed -i 's/DS_LIGHTNING/%s/g' {} \;`, + `find %s -type f -exec sed -i 's/test-cluster/%s/g' {} \;`, + `find %s -type f -exec sed -i 's/Test-Cluster/%s/g' {} \;`, } { cmd := fmt.Sprintf(cmd, dashboardsDir, clusterName) _, stderr, err := e.Execute(ctx, cmd, false) diff --git a/pkg/cluster/spec/monitoring.go b/pkg/cluster/spec/monitoring.go index 9f0de67fcc..32d2f66372 100644 --- a/pkg/cluster/spec/monitoring.go +++ b/pkg/cluster/spec/monitoring.go @@ -404,7 +404,7 @@ func (i *MonitorInstance) installRules(ctx context.Context, e ctxt.Executor, dep `find %[1]s -type f -name "*.rules.yml" -delete`, `find %[2]s/dm-master/conf -type f -name "*.rules.yml" -exec cp {} %[1]s \;`, "rm -rf %[2]s", - `find %[1]s -maxdepth 1 -type f -name "*.rules.yml" -exec sed -i "s/ENV_LABELS_ENV/%[3]s/g" {} \;`, + `find %[1]s -maxdepth 1 -type f -name "*.rules.yml" -exec sed -i 's/ENV_LABELS_ENV/%[3]s/g' {} \;`, } _, stderr, err = e.Execute(ctx, fmt.Sprintf(strings.Join(cmds, " && "), targetDir, tmp, clusterName), false) if err != nil { @@ -420,7 +420,7 @@ func (i *MonitorInstance) initRules(ctx context.Context, e ctxt.Executor, spec * "mkdir -p %[1]s/conf", `find %[1]s/conf -type f -name "*.rules.yml" -delete`, `find %[1]s/bin/prometheus -maxdepth 1 -type f -name "*.rules.yml" -exec cp {} %[1]s/conf/ \;`, - `find %[1]s/conf -maxdepth 1 -type f -name "*.rules.yml" -exec sed -i -e "s/ENV_LABELS_ENV/%[2]s/g" {} \;`, + `find %[1]s/conf -maxdepth 1 -type f -name "*.rules.yml" -exec sed -i -e 's/ENV_LABELS_ENV/%[2]s/g' {} \;`, } _, stderr, err := e.Execute(ctx, fmt.Sprintf(strings.Join(cmds, " && "), paths.Deploy, clusterName), false) @@ -438,8 +438,8 @@ func (i *MonitorInstance) initRules(ctx context.Context, e ctxt.Executor, spec * } // only need to render the cluster name cmds = []string{ - `find %[1]s/conf -maxdepth 1 -type f -name "*.rules.yml" -exec sed -i -e "s/env: [^ ]*/env: %[2]s/g" {} \;`, - `find %[1]s/conf -maxdepth 1 -type f -name "*.rules.yml" -exec sed -i -e "s/cluster: [^ ]*,/cluster: %[2]s,/g" {} \;`, + `find %[1]s/conf -maxdepth 1 -type f -name "*.rules.yml" -exec sed -i -e 's/env: [^ ]*/env: %[2]s/g' {} \;`, + `find %[1]s/conf -maxdepth 1 -type f -name "*.rules.yml" -exec sed -i -e 's/cluster: [^ ]*,/cluster: %[2]s,/g' {} \;`, } _, stderr, err := e.Execute(ctx, fmt.Sprintf(strings.Join(cmds, " && "), paths.Deploy, clusterName), false) if err != nil { From bfe5759f6bbfee4f06c40413cc39c1d07e2946a9 Mon Sep 17 00:00:00 2001 From: Allen Zhong Date: Wed, 26 Jan 2022 16:47:57 +0800 Subject: [PATCH 2/3] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Qiμ$hiЯuí <39378935+srstack@users.noreply.github.com> --- pkg/cluster/executor/local.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/cluster/executor/local.go b/pkg/cluster/executor/local.go index 4db7a4670f..8e04aa7553 100644 --- a/pkg/cluster/executor/local.go +++ b/pkg/cluster/executor/local.go @@ -43,11 +43,16 @@ var _ ctxt.Executor = &Local{} func (l *Local) Execute(ctx context.Context, cmd string, sudo bool, timeout ...time.Duration) ([]byte, []byte, error) { // change wd to default home cmd = fmt.Sprintf("cd; %s", cmd) + // get current user name + user, err := user.Current() + if err != nil { + return nil, nil, err + } // try to acquire root permission if l.Sudo || sudo { cmd = fmt.Sprintf("/usr/bin/sudo -H -u root bash -c \"%s\"", strings.ReplaceAll(cmd, "\"", "\\\"")) - } else { + } else if l.Config.User != user.Name { cmd = fmt.Sprintf("/usr/bin/sudo -H -u %s bash -c \"%s\"", l.Config.User, strings.ReplaceAll(cmd, "\"", "\\\"")) } From 4836a9b33ab8fda69233a88a17ef65f1839c85f3 Mon Sep 17 00:00:00 2001 From: Allen Zhong Date: Thu, 27 Jan 2022 14:48:13 +0800 Subject: [PATCH 3/3] fix build error --- pkg/cluster/executor/local.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cluster/executor/local.go b/pkg/cluster/executor/local.go index 8e04aa7553..9a3a13f475 100644 --- a/pkg/cluster/executor/local.go +++ b/pkg/cluster/executor/local.go @@ -82,7 +82,7 @@ func (l *Local) Execute(ctx context.Context, cmd string, sudo bool, timeout ...t command.Stdout = stdout command.Stderr = stderr - err := command.Run() + err = command.Run() zap.L().Info("LocalCommand", zap.String("cmd", cmd),