Fix integration test failure from result formatting and remove a stray instance of results truncation in plugin

cr https://cr.amazon.com/r/6458012/

Author: mattford-amazon

agent/contracts/plugin.go

@@ -136,9 +136,8 @@ func (out *PluginOutput) MarkAsSuccessWithReboot() {
 }
 
 // AppendInfo adds info to PluginOutput StandardOut.
-func (out *PluginOutput) AppendInfo(log log.T, format string, params ...interface{}) {
-	if len(format) > 0 {
-		message := fmt.Sprintf(format, params...)
+func (out *PluginOutput) AppendInfo(log log.T, message string) {
+	if len(message) > 0 {
 		log.Info(message)
 		if len(out.Stdout) > 0 {
 			out.Stdout = fmt.Sprintf("%v\n%v", out.Stdout, message)
@@ -148,10 +147,17 @@ func (out *PluginOutput) AppendInfo(log log.T, format string, params ...interfac
 	}
 }
 
-// AppendError adds errors to PluginOutput StandardErr.
-func (out *PluginOutput) AppendError(log log.T, format string, params ...interface{}) {
+// AppendInfof adds info to PluginOutput StandardOut with formatting parameters.
+func (out *PluginOutput) AppendInfof(log log.T, format string, params ...interface{}) {
 	if len(format) > 0 {
 		message := fmt.Sprintf(format, params...)
+		out.AppendInfo(log, message)
+	}
+}
+
+// AppendError adds errors to PluginOutput StandardErr.
+func (out *PluginOutput) AppendError(log log.T, message string) {
+	if len(message) > 0 {
 		log.Error(message)
 		if len(out.Stderr) > 0 {
 			out.Stderr = fmt.Sprintf("%v\n%v", out.Stderr, message)
@@ -161,6 +167,14 @@ func (out *PluginOutput) AppendError(log log.T, format string, params ...interfa
 	}
 }
 
+// AppendErrorf adds errors to PluginOutput StandardErr with formatting parameters.
+func (out *PluginOutput) AppendErrorf(log log.T, format string, params ...interface{}) {
+	if len(format) > 0 {
+		message := fmt.Sprintf(format, params...)
+		out.AppendError(log, message)
+	}
+}
+
 // TruncateOutput truncates the output
 func TruncateOutput(stdout string, stderr string, capacity int) (response string) {
 	outputSize := len(stdout)

agent/contracts/plugin_test.go

@@ -100,3 +100,30 @@ func TestAppendInfo(t *testing.T) {
 	assert.Contains(t, output.Stdout, "Info message")
 	assert.Contains(t, output.Stdout, "Second entry")
 }
+
+func TestAppendSpecialChars(t *testing.T) {
+	output := PluginOutput{}
+
+	var testString = "%v`~!@#$%^&*()-_=+[{]}|\\;:'\",<.>/?"
+	output.AppendInfo(logger, testString)
+	output.AppendError(logger, testString)
+
+	assert.Contains(t, output.Stdout, testString)
+	assert.Contains(t, output.Stderr, testString)
+}
+
+func TestAppendFormat(t *testing.T) {
+	output := PluginOutput{}
+
+	var testString = "%v`~!@#$%^&*()-_=+[{]}|\\;:'\",<.>/?%%"
+
+	// The first % is a %v - a variable to be replaced and we provided a value for it.
+	// The second % isn't escaped and is treated as a fmt parameter, but no value is provided for it.
+	// The double %% is an escaped single literal %.
+	var testStringFormatted = "foo`~!@#$%!^(MISSING)&*()-_=+[{]}|\\;:'\",<.>/?%"
+	output.AppendInfof(logger, testString, "foo")
+	output.AppendErrorf(logger, testString, "foo")
+
+	assert.Contains(t, output.Stdout, testStringFormatted)
+	assert.Contains(t, output.Stderr, testStringFormatted)
+}

agent/plugins/configurepackage/configurepackage.go

@@ -156,7 +156,7 @@ func runConfigurePackage(
 		// if already installed, exit
 		if version == installedVersion {
 			// TODO:MF: validate that installed version is basically valid - has manifest and at least one other non-etag file or folder?
-			output.AppendInfo(log, "%v %v is already installed", input.Name, version)
+			output.AppendInfof(log, "%v %v is already installed", input.Name, version)
 			output.MarkAsSucceeded()
 			return
 		}
@@ -182,14 +182,14 @@ func runConfigurePackage(
 			// even though that may or may not be the package that installed it - it is our only decent option
 			_, ensureErr := manager.ensurePackage(context, configUtil, input.Name, installedVersion, &output)
 			if ensureErr != nil {
-				output.AppendError(log, "unable to obtain package: %v", ensureErr)
+				output.AppendErrorf(log, "unable to obtain package: %v", ensureErr)
 			} else {
 				result, err := manager.runUninstallPackagePre(context,
 					input.Name,
 					installedVersion,
 					&output)
 				if err != nil {
-					output.AppendError(log, "failed to uninstall currently installed version of package: %v", err)
+					output.AppendErrorf(log, "failed to uninstall currently installed version of package: %v", err)
 				} else {
 					if result == contracts.ResultStatusSuccessAndReboot || result == contracts.ResultStatusPassedAndReboot {
 						// Reboot before continuing
@@ -211,12 +211,12 @@ func runConfigurePackage(
 		if err != nil {
 			output.MarkAsFailed(log, fmt.Errorf("failed to install package: %v", err))
 		} else if result == contracts.ResultStatusSuccessAndReboot || result == contracts.ResultStatusPassedAndReboot {
-			output.AppendInfo(log, "Successfully installed %v %v", input.Name, version)
+			output.AppendInfof(log, "Successfully installed %v %v", input.Name, version)
 			output.MarkAsSuccessWithReboot()
 		} else if result != contracts.ResultStatusSuccess {
 			output.MarkAsFailed(log, fmt.Errorf("install action state was %v and not %v", result, contracts.ResultStatusSuccess))
 		} else {
-			output.AppendInfo(log, "Successfully installed %v %v", input.Name, version)
+			output.AppendInfof(log, "Successfully installed %v %v", input.Name, version)
 			output.MarkAsSucceeded()
 		}
 
@@ -227,7 +227,7 @@ func runConfigurePackage(
 				installedVersion,
 				&output)
 			if err != nil {
-				output.AppendError(log, "failed to clean up currently installed version of package: %v", err)
+				output.AppendErrorf(log, "failed to clean up currently installed version of package: %v", err)
 			}
 		}
 
@@ -265,12 +265,12 @@ func runConfigurePackage(
 
 		result := contracts.MergeResultStatus(resultPre, resultPost)
 		if result == contracts.ResultStatusSuccessAndReboot || result == contracts.ResultStatusPassedAndReboot {
-			output.AppendInfo(log, "Successfully uninstalled %v %v", input.Name, version)
+			output.AppendInfof(log, "Successfully uninstalled %v %v", input.Name, version)
 			output.MarkAsSuccessWithReboot()
 		} else if result != contracts.ResultStatusSuccess {
 			output.MarkAsFailed(log, fmt.Errorf("uninstall action state was %v and not %v", result, contracts.ResultStatusSuccess))
 		} else {
-			output.AppendInfo(log, "Successfully uninstalled %v %v", input.Name, version)
+			output.AppendInfof(log, "Successfully uninstalled %v %v", input.Name, version)
 			output.MarkAsSucceeded()
 		}
 	default:
@@ -444,7 +444,7 @@ func (m *configurePackage) downloadPackage(context context.T,
 		return "", errors.New(errMessage)
 	}
 
-	output.AppendInfo(log, "Successfully downloaded %v", downloadInput.SourceURL)
+	output.AppendInfof(log, "Successfully downloaded %v", downloadInput.SourceURL)
 
 	return downloadOutput.LocalFilePath, nil
 }
@@ -502,7 +502,7 @@ func (m *configurePackage) executeAction(context context.T,
 
 	log := context.Log()
 	if actionExists {
-		output.AppendInfo(log, "Initiating %v %v %v", packageName, version, actionName)
+		output.AppendInfof(log, "Initiating %v %v %v", packageName, version, actionName)
 		file, err := filesysdep.ReadFile(fileLocation)
 		if err != nil {
 			return true, contracts.ResultStatusFailed, err
@@ -521,10 +521,10 @@ func (m *configurePackage) executeAction(context context.T,
 		for _, pluginOut := range pluginOutputs {
 			log.Debugf("Plugin %v ResultStatus %v", pluginOut.PluginName, pluginOut.Status)
 			if pluginOut.StandardOutput != "" {
-				output.AppendInfo(log, "%v output: %v", actionName, pluginOut.StandardOutput)
+				output.AppendInfof(log, "%v output: %v", actionName, pluginOut.StandardOutput)
 			}
 			if pluginOut.StandardError != "" {
-				output.AppendInfo(log, "%v errors: %v", actionName, pluginOut.StandardError)
+				output.AppendInfof(log, "%v errors: %v", actionName, pluginOut.StandardError)
 			}
 			if pluginOut.Error != nil {
 				output.MarkAsFailed(log, pluginOut.Error)
@@ -613,12 +613,12 @@ func (p *Plugin) Execute(context context.T, config contracts.Configuration, canc
 			} else {
 				if err := filesysdep.WriteFile(outFile, out[0].Stdout); err != nil {
 					log.Debugf("Error writing to %v", outFile)
-					out[0].AppendError(log, "Error saving stdout: %v", err.Error())
+					out[0].AppendErrorf(log, "Error saving stdout: %v", err.Error())
 				}
 				errFile := filepath.Join(config.OrchestrationDirectory, p.StderrFileName)
 				if err := filesysdep.WriteFile(errFile, out[0].Stderr); err != nil {
 					log.Debugf("Error writing to %v", errFile)
-					out[0].AppendError(log, "Error saving stderr: %v", err.Error())
+					out[0].AppendErrorf(log, "Error saving stderr: %v", err.Error())
 				}
 			}
 			uploadErrs := p.UploadOutputToS3Bucket(log,

agent/plugins/refreshassociation/refreshassociation.go

@@ -15,7 +15,6 @@
 package refreshassociation
 
 import (
-	"bytes"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -161,16 +160,6 @@ func (p *Plugin) runCommandsRawInput(log log.T, rawPluginInput interface{}, orch
 		log.Error(err)
 	}
 
-	// read (a prefix of) the standard output/error
-	out.Stdout, err = pluginutil.ReadPrefix(bytes.NewBufferString(out.Stdout), p.MaxStdoutLength, p.OutputTruncatedSuffix)
-	if err != nil {
-		log.Error(err)
-	}
-	out.Stderr, err = pluginutil.ReadPrefix(bytes.NewBufferString(out.Stderr), p.MaxStderrLength, p.OutputTruncatedSuffix)
-	if err != nil {
-		log.Error(err)
-	}
-
 	// Upload output to S3
 	uploadOutputToS3BucketErrors := p.ExecuteUploadOutputToS3Bucket(log, pluginInput.ID, orchestrationDirectory, outputS3BucketName, outputS3KeyPrefix, false, "", out.Stdout, out.Stderr)
 	if len(uploadOutputToS3BucketErrors) > 0 {
@@ -280,7 +269,7 @@ func (p *Plugin) runCommands(log log.T, pluginInput RefreshAssociationPluginInpu
 	if applyAll {
 		out.AppendInfo(log, "All associations have been requested to execute immediately")
 	} else {
-		out.AppendInfo(log, "Associations %v have been requested to execute immediately", pluginInput.AssociationIds)
+		out.AppendInfof(log, "Associations %v have been requested to execute immediately", pluginInput.AssociationIds)
 	}
 
 	signal.ExecuteAssociation(log)

agent/plugins/updateec2config/updateec2config.go

@@ -222,7 +222,7 @@ func runUpdateAgent(
 		return
 	}
 
-	out.AppendInfo(log, "Updating %v from %v to %v",
+	out.AppendInfof(log, "Updating %v from %v to %v",
 		pluginInput.AgentName,
 		agentVersion,
 		targetVersion)
@@ -427,7 +427,7 @@ func (m *updateManager) downloadManifest(log log.T,
 		downloadOutput.LocalFilePath == "" {
 		return nil, downloadErr
 	}
-	out.AppendInfo(log, "Successfully downloaded %v", downloadInput.SourceURL)
+	out.AppendInfof(log, "Successfully downloaded %v", downloadInput.SourceURL)
 	return ParseManifest(log, downloadOutput.LocalFilePath)
 }
 
@@ -470,15 +470,15 @@ func (m *updateManager) downloadUpdater(log log.T,
 		return version, errors.New(errMessage)
 	}
 
-	out.AppendInfo(log, "Successfully downloaded %v", downloadInput.SourceURL)
+	out.AppendInfof(log, "Successfully downloaded %v", downloadInput.SourceURL)
 	if uncompressErr := fileUncompress(
 		downloadOutput.LocalFilePath,
 		updateutil.UpdateArtifactFolder(appconfig.EC2UpdateArtifactsRoot, updaterPackageName, version)); uncompressErr != nil {
 		return version, fmt.Errorf("failed to uncompress updater package, %v, %v",
 			downloadOutput.LocalFilePath,
 			uncompressErr.Error())
 	}
-	out.AppendInfo(log, "Successfully downloaded %v", downloadInput.SourceURL)
+	out.AppendInfof(log, "Successfully downloaded %v", downloadInput.SourceURL)
 
 	return version, nil
 }
@@ -502,7 +502,7 @@ func (m *updateManager) validateUpdate(log log.T,
 	}
 
 	if pluginInput.TargetVersion == currentVersion {
-		out.AppendInfo(log, "%v %v has already been installed, update skipped",
+		out.AppendInfof(log, "%v %v has already been installed, update skipped",
 			pluginInput.AgentName,
 			currentVersion)
 		out.MarkAsSucceeded()

agent/plugins/updatessmagent/updateagent.go

@@ -176,7 +176,7 @@ func runUpdateAgent(
 	if len(targetVersion) == 0 {
 		targetVersion = "latest"
 	}
-	out.AppendInfo(log, "Updating %v from %v to %v",
+	out.AppendInfof(log, "Updating %v from %v to %v",
 		pluginInput.AgentName,
 		version.Version,
 		targetVersion)
@@ -331,7 +331,7 @@ func (m *updateManager) downloadManifest(log log.T,
 		downloadOutput.LocalFilePath == "" {
 		return nil, downloadErr
 	}
-	out.AppendInfo(log, "Successfully downloaded %v", downloadInput.SourceURL)
+	out.AppendInfof(log, "Successfully downloaded %v", downloadInput.SourceURL)
 	return ParseManifest(log, downloadOutput.LocalFilePath, context, pluginInput.AgentName)
 }
 
@@ -373,7 +373,7 @@ func (m *updateManager) downloadUpdater(log log.T,
 		}
 		return version, errors.New(errMessage)
 	}
-	out.AppendInfo(log, "Successfully downloaded %v", downloadInput.SourceURL)
+	out.AppendInfof(log, "Successfully downloaded %v", downloadInput.SourceURL)
 	if uncompressErr := fileUncompress(
 		downloadOutput.LocalFilePath,
 		updateutil.UpdateArtifactFolder(appconfig.UpdaterArtifactsRoot, updaterPackageName, version)); uncompressErr != nil {
@@ -404,7 +404,7 @@ func (m *updateManager) validateUpdate(log log.T,
 	}
 
 	if pluginInput.TargetVersion == currentVersion {
-		out.AppendInfo(log, "%v %v has already been installed, update skipped",
+		out.AppendInfof(log, "%v %v has already been installed, update skipped",
 			pluginInput.AgentName,
 			currentVersion)
 		out.MarkAsSucceeded()