Skip to content

Commit 3bb865e

Browse files
Changbing Zhaoadchalla
Changbing Zhao
authored and
adchalla
committed
Fix the command builder bug to handle space char in input value
cr: https://code.amazon.com/reviews/CR-118040195
1 parent ec670ac commit 3bb865e

File tree

3 files changed

+78
-48
lines changed

3 files changed

+78
-48
lines changed

‎agent/plugins/updatessmagent/updateagent.go‎

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -119,16 +119,17 @@ func runUpdateAgent(
119119
output.AppendInfof("Successfully downloaded updater version %s\n", updaterVersion)
120120

121121
//Generate update command base on the update detail
122-
cmd := ""
123-
if cmd, err = generateUpdateCmd(
122+
cmd, err := generateUpdateCmd(
124123
&pluginInput,
125124
updaterVersion,
126125
config.MessageId,
127126
config.UpstreamServiceName,
128127
pluginConfig.StdoutFileName,
129128
pluginConfig.StderrFileName,
130129
fileutil.BuildS3Path(output.GetIOConfig().OutputS3KeyPrefix, config.PluginID),
131-
output.GetIOConfig().OutputS3BucketName); err != nil {
130+
output.GetIOConfig().OutputS3BucketName)
131+
132+
if err != nil {
132133
output.MarkAsFailed(err)
133134
return
134135
}
@@ -163,7 +164,7 @@ func runUpdateAgent(
163164
appconfig.UpdaterArtifactsRoot, pluginInput.UpdaterName, updaterVersion)
164165

165166
for retryCounter := 1; retryCounter <= noOfRetries; retryCounter++ {
166-
pid, _, err = util.ExeCommand(
167+
pid, _, err = util.ExeCommandWithSlice(
167168
log,
168169
cmd,
169170
workDir,
@@ -310,35 +311,38 @@ func generateUpdateCmd(
310311
stdout string,
311312
stderr string,
312313
keyPrefix string,
313-
bucketName string) (cmd string, err error) {
314-
updaterPath := updateutil.UpdaterFilePath(appconfig.UpdaterArtifactsRoot, pluginInput.UpdaterName, updaterVersion)
315-
cmd = updaterPath + " -update"
316-
317-
cmd = updateutil.BuildUpdateCommand(cmd, updateconstants.SourceVersionCmd, currentAgentVersion)
318-
cmd = updateutil.BuildUpdateCommand(cmd, updateconstants.TargetVersionCmd, pluginInput.TargetVersion)
319-
320-
cmd = updateutil.BuildUpdateCommand(cmd, updateconstants.PackageNameCmd, pluginInput.AgentName)
321-
cmd = updateutil.BuildUpdateCommand(cmd, updateconstants.MessageIDCmd, messageID)
322-
323-
cmd = updateutil.BuildUpdateCommand(cmd, updateconstants.StdoutFileName, stdout)
324-
cmd = updateutil.BuildUpdateCommand(cmd, updateconstants.StderrFileName, stderr)
325-
326-
cmd = updateutil.BuildUpdateCommand(cmd, updateconstants.OutputKeyPrefixCmd, keyPrefix)
327-
cmd = updateutil.BuildUpdateCommand(cmd, updateconstants.OutputBucketNameCmd, bucketName)
328-
329-
cmd = updateutil.BuildUpdateCommand(cmd, updateconstants.ManifestFileUrlCmd, pluginInput.Source)
330-
331-
cmd = updateutil.BuildUpdateCommand(cmd, updateconstants.UpstreamServiceName, string(upstreamServiceName))
314+
bucketName string) (cmd []string, err error) {
332315

316+
// quick return
333317
allowDowngrade, err := strconv.ParseBool(pluginInput.AllowDowngrade)
334318
if err != nil {
335-
return "", err
319+
return cmd, err
336320
}
337321

322+
updaterPath := updateutil.UpdaterFilePath(appconfig.UpdaterArtifactsRoot, pluginInput.UpdaterName, updaterVersion)
323+
cmd = append(cmd, updaterPath)
324+
cmd = append(cmd, "-update")
325+
338326
// Tell the updater if downgrade is not allowed
339327
if !allowDowngrade {
340-
cmd += " -" + updateconstants.DisableDowngradeCmd
328+
cmd = append(cmd, "-"+updateconstants.DisableDowngradeCmd)
341329
}
342330

331+
cmd = append(cmd, "-"+updateconstants.SourceVersionCmd, currentAgentVersion)
332+
cmd = append(cmd, "-"+updateconstants.TargetVersionCmd, pluginInput.TargetVersion)
333+
334+
cmd = append(cmd, "-"+updateconstants.PackageNameCmd, pluginInput.AgentName)
335+
cmd = append(cmd, "-"+updateconstants.MessageIDCmd, messageID)
336+
337+
cmd = append(cmd, "-"+updateconstants.StdoutFileName, stdout)
338+
cmd = append(cmd, "-"+updateconstants.StderrFileName, stderr)
339+
340+
cmd = append(cmd, "-"+updateconstants.OutputKeyPrefixCmd, keyPrefix)
341+
cmd = append(cmd, "-"+updateconstants.OutputBucketNameCmd, bucketName)
342+
343+
cmd = append(cmd, "-"+updateconstants.ManifestFileUrlCmd, pluginInput.Source)
344+
345+
cmd = append(cmd, "-"+updateconstants.UpstreamServiceName, string(upstreamServiceName))
346+
343347
return
344348
}

‎agent/plugins/updatessmagent/updateagent_test.go‎

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -47,37 +47,37 @@ func TestGenerateUpdateCmd(t *testing.T) {
4747
pluginInput := createStubPluginInput()
4848

4949
result, err := generateUpdateCmd(pluginInput,
50-
"3.0.0.0", "messageID", contracts.MessageGatewayService, "stdout", "stderr", "prefix", "bucket")
50+
"3.0.0.0", "messageID with space", contracts.MessageGatewayService, "stdout", "stderr", "prefix", "bucket")
5151

5252
assert.NoError(t, err)
53-
assert.Contains(t, result, "3.0.0.0")
54-
assert.Contains(t, result, "messageID")
55-
assert.Contains(t, result, "MessageGatewayService")
56-
assert.Contains(t, result, "stdout")
57-
assert.Contains(t, result, "stderr")
58-
assert.Contains(t, result, "prefix")
59-
assert.Contains(t, result, "bucket")
60-
assert.Contains(t, result, "manifest")
61-
assert.NotContains(t, result, "-"+updateconstants.DisableDowngradeCmd)
53+
assert.EqualValues(t, 22, len(result))
54+
assert.Contains(t, result[0], "3.0.0.0")
55+
assert.EqualValues(t, "messageID with space", result[9])
56+
assert.EqualValues(t, "stdout", result[11])
57+
assert.EqualValues(t, "stderr", result[13])
58+
assert.EqualValues(t, "prefix", result[15])
59+
assert.EqualValues(t, "bucket", result[17])
60+
assert.EqualValues(t, "testSource", result[19])
61+
assert.EqualValues(t, "MessageGatewayService", result[21])
6262
}
6363

6464
func TestGenerateUpdateCmdNoDowngrade(t *testing.T) {
6565
pluginInput := createStubPluginInput()
6666
pluginInput.AllowDowngrade = "false"
6767

6868
result, err := generateUpdateCmd(pluginInput,
69-
"3.0.0.0", "messageID", contracts.MessageGatewayService, "stdout", "stderr", "prefix", "bucket")
70-
69+
"3.0.0.0", "messageID with space", contracts.MessageGatewayService, "stdout", "stderr", "prefix", "bucket")
7170
assert.NoError(t, err)
72-
assert.Contains(t, result, "3.0.0.0")
73-
assert.Contains(t, result, "messageID")
74-
assert.Contains(t, result, "MessageGatewayService")
75-
assert.Contains(t, result, "stdout")
76-
assert.Contains(t, result, "stderr")
77-
assert.Contains(t, result, "prefix")
78-
assert.Contains(t, result, "bucket")
79-
assert.Contains(t, result, "manifest")
80-
assert.Contains(t, result, "-"+updateconstants.DisableDowngradeCmd)
71+
assert.EqualValues(t, 23, len(result))
72+
assert.Contains(t, result[0], "3.0.0.0")
73+
assert.EqualValues(t, "messageID with space", result[10])
74+
assert.EqualValues(t, "stdout", result[12])
75+
assert.EqualValues(t, "stderr", result[14])
76+
assert.EqualValues(t, "prefix", result[16])
77+
assert.EqualValues(t, "bucket", result[18])
78+
assert.EqualValues(t, "testSource", result[20])
79+
assert.EqualValues(t, "MessageGatewayService", result[22])
80+
assert.EqualValues(t, result[2], "-"+updateconstants.DisableDowngradeCmd)
8181
}
8282

8383
func TestGenerateUpdateCmdInvalidDowngrade(t *testing.T) {
@@ -629,6 +629,18 @@ func (u *fakeUtility) ExeCommand(
629629
return u.pid, exitCode, u.execCommandError
630630
}
631631

632+
func (u *fakeUtility) ExeCommandWithSlice(
633+
log log.T,
634+
cmd []string,
635+
updateRoot string,
636+
workingDir string,
637+
stdOut string,
638+
stdErr string,
639+
isAsync bool) (pid int, exitCode updateconstants.UpdateScriptExitCode, err error) {
640+
u.retryCounter++
641+
return u.pid, exitCode, u.execCommandError
642+
}
643+
632644
func (u *fakeUtility) ExecCommandWithOutput(
633645
log log.T,
634646
cmd string,

‎agent/updateutil/updateutil.go‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import (
4949
type T interface {
5050
CreateUpdateDownloadFolder() (folder string, err error)
5151
ExeCommand(log log.T, cmd string, workingDir string, updaterRoot string, stdOut string, stdErr string, isAsync bool) (pid int, exitCode updateconstants.UpdateScriptExitCode, err error)
52+
ExeCommandWithSlice(log log.T, cmd []string, workingDir string, updaterRoot string, stdOut string, stdErr string, isAsync bool) (pid int, exitCode updateconstants.UpdateScriptExitCode, err error)
5253
ExecCommandWithOutput(log log.T, cmd string, workingDir string, updaterRoot string, stdOut string, stdErr string) (pId int, updExitCode updateconstants.UpdateScriptExitCode, stdoutBytes *bytes.Buffer, errorBytes *bytes.Buffer, cmdErr error)
5354
IsServiceRunning(log log.T, i updateinfo.T) (result bool, err error)
5455
IsWorkerRunning(log log.T) (result bool, err error)
@@ -194,11 +195,24 @@ func (util *Utility) ExeCommand(
194195
isAsync bool) (int, updateconstants.UpdateScriptExitCode, error) { // pid, exitCode, error
195196

196197
parts := strings.Fields(cmd)
198+
return util.ExeCommandWithSlice(log, parts, workingDir, outputRoot, stdOut, stdErr, isAsync)
199+
}
200+
201+
// ExeCommand executes shell command
202+
func (util *Utility) ExeCommandWithSlice(
203+
log log.T,
204+
cmd []string,
205+
workingDir string,
206+
outputRoot string,
207+
stdOut string,
208+
stdErr string,
209+
isAsync bool) (int, updateconstants.UpdateScriptExitCode, error) { // pid, exitCode, error
210+
197211
pid := -1
198212
var updateExitCode updateconstants.UpdateScriptExitCode = -1
199213

200214
if isAsync {
201-
command := execCommand(parts[0], parts[1:]...)
215+
command := execCommand(cmd[0], cmd[1:]...)
202216
command.Dir = workingDir
203217
util.setCommandEnvironmentVariables(command)
204218
prepareProcess(command)
@@ -209,7 +223,7 @@ func (util *Utility) ExeCommand(
209223
}
210224
pid = GetCommandPid(command)
211225
} else {
212-
tempCmd := setPlatformSpecificCommand(parts)
226+
tempCmd := setPlatformSpecificCommand(cmd)
213227
command := execCommand(tempCmd[0], tempCmd[1:]...)
214228
command.Dir = workingDir
215229
stdoutWriter, stderrWriter, err := setExeOutErr(outputRoot, stdOut, stdErr)

0 commit comments

Comments
 (0)