From 5986b7f7e16e1cd0078e4987cabb11cbad73db40 Mon Sep 17 00:00:00 2001 From: Chad Patel Date: Tue, 17 Dec 2024 15:38:23 -0600 Subject: [PATCH] fix bugs and linter errors --- .../amazon-cloudwatch-agent.go | 1 + tool/cmdwrapper/cmdwrapper.go | 4 +- tool/cmdwrapper/cmdwrapper_test.go | 2 - tool/downloader/downloader.go | 110 ++++++++---------- tool/downloader/flags/flags.go | 10 +- tool/wizard/flags/flags.go | 18 +-- tool/wizard/wizard.go | 11 +- translator/cmdutil/translator.go | 18 +-- translator/cmdutil/translatorutil_test.go | 2 +- translator/flags/flags.go | 14 +-- 10 files changed, 89 insertions(+), 101 deletions(-) diff --git a/cmd/amazon-cloudwatch-agent/amazon-cloudwatch-agent.go b/cmd/amazon-cloudwatch-agent/amazon-cloudwatch-agent.go index 24a937a335..5d34a92a0c 100644 --- a/cmd/amazon-cloudwatch-agent/amazon-cloudwatch-agent.go +++ b/cmd/amazon-cloudwatch-agent/amazon-cloudwatch-agent.go @@ -41,6 +41,7 @@ import ( "github.com/aws/amazon-cloudwatch-agent/internal/version" cwaLogger "github.com/aws/amazon-cloudwatch-agent/logger" "github.com/aws/amazon-cloudwatch-agent/logs" + _ "github.com/aws/amazon-cloudwatch-agent/plugins" // do not remove, necessary for telegraf to know what plugins are used "github.com/aws/amazon-cloudwatch-agent/profiler" "github.com/aws/amazon-cloudwatch-agent/receiver/adapter" "github.com/aws/amazon-cloudwatch-agent/service/configprovider" diff --git a/tool/cmdwrapper/cmdwrapper.go b/tool/cmdwrapper/cmdwrapper.go index b547b33721..161ba5c1dc 100644 --- a/tool/cmdwrapper/cmdwrapper.go +++ b/tool/cmdwrapper/cmdwrapper.go @@ -15,7 +15,6 @@ import ( ) type Flag struct { - Name string DefaultValue string Description string } @@ -28,7 +27,7 @@ var execCommand = exec.Command func AddFlags(prefix string, flagConfigs map[string]Flag) map[string]*string { flags := make(map[string]*string) for key, flagConfig := range flagConfigs { - flagName := flagConfig.Name + flagName := key if prefix != "" { flagName = prefix + delimiter + flagName } @@ -52,6 +51,7 @@ func ExecuteAgentCommand(command string, flags map[string]*string) error { cmd := execCommand(paths.AgentBinaryPath, args...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr + cmd.Stdin = os.Stdin if err := cmd.Run(); err != nil { var exitErr *exec.ExitError diff --git a/tool/cmdwrapper/cmdwrapper_test.go b/tool/cmdwrapper/cmdwrapper_test.go index ebfa0c628e..7b7797d61a 100644 --- a/tool/cmdwrapper/cmdwrapper_test.go +++ b/tool/cmdwrapper/cmdwrapper_test.go @@ -26,7 +26,6 @@ func TestAddFlags(t *testing.T) { prefix: "", flagConfigs: map[string]Flag{ "test": { - Name: "test", DefaultValue: "default", Description: "test description", }, @@ -40,7 +39,6 @@ func TestAddFlags(t *testing.T) { prefix: "prefix", flagConfigs: map[string]Flag{ "test": { - Name: "test", DefaultValue: "default", Description: "test description", }, diff --git a/tool/downloader/downloader.go b/tool/downloader/downloader.go index ddc711a872..079e9d7f4d 100644 --- a/tool/downloader/downloader.go +++ b/tool/downloader/downloader.go @@ -28,18 +28,8 @@ const ( locationFile = "file" locationSeparator = ":" - - exitErrorMessage = "Fail to fetch the config!" ) -func EscapeFilePath(filePath string) (escapedFilePath string) { - escapedFilePath = filepath.ToSlash(filePath) - escapedFilePath = strings.Replace(escapedFilePath, "/", "_", -1) - escapedFilePath = strings.Replace(escapedFilePath, " ", "_", -1) - escapedFilePath = strings.Replace(escapedFilePath, ":", "_", -1) - return -} - func RunDownloaderFromFlags(flags map[string]*string) error { return RunDownloader( *flags["mode"], @@ -50,19 +40,14 @@ func RunDownloaderFromFlags(flags map[string]*string) error { ) } -/** - * multi-config: - * default, append: download config to the dir and append .tmp suffix - * remove: remove the config from the dir - */ -func RunDownloader( - mode string, - downloadLocation string, - outputDir string, - inputConfig string, - multiConfig string, -) error { - // Initialize common config +func RunDownloader(mode, downloadLocation, outputDir, inputConfig, multiConfig string) error { + defer func() { + if r := recover(); r != nil { + fmt.Println("Fail to fetch the config!") + os.Exit(1) + } + }() + cc := commonconfig.New() if inputConfig != "" { f, err := os.Open(inputConfig) @@ -93,8 +78,6 @@ func RunDownloader( // Detect agent mode and region mode = sdkutil.DetectAgentMode(mode) region, _ := util.DetectRegion(mode, cc.CredentialsMap()) - - // Validate region if region == "" && downloadLocation != locationDefault { if mode == config.ModeEC2 { return fmt.Errorf("please check if you can access the metadata service. For example, on linux, run 'wget -q -O - http://169.254.169.254/latest/meta-data/instance-id && echo'") @@ -102,40 +85,22 @@ func RunDownloader( return fmt.Errorf("please make sure the credentials and region set correctly on your hosts") } - // Clean up output directory - err := filepath.Walk(outputDir, func(path string, info os.FileInfo, err error) error { - if err != nil { - return fmt.Errorf("cannot access %v: %v", path, err) - } - if info.IsDir() { - if strings.EqualFold(path, outputDir) { - return nil - } - fmt.Printf("Sub dir %v will be ignored.", path) - return filepath.SkipDir - } - if filepath.Ext(path) == constants.FileSuffixTmp { - return os.Remove(path) - } - return nil - }) + err := cleanupOutputDir(outputDir) if err != nil { - return err + return fmt.Errorf("failed to clean up output directory: %v", err) } - // Parse download location locationArray := strings.SplitN(downloadLocation, locationSeparator, 2) if locationArray == nil || len(locationArray) < 2 && downloadLocation != locationDefault { return fmt.Errorf("downloadLocation %s is malformed", downloadLocation) } - // Process configuration based on location type var config, outputFilePath string switch locationArray[0] { case locationDefault: outputFilePath = locationDefault if multiConfig != "remove" { - config, err = defaultJsonConfig(mode) + config, err = defaultJSONConfig(mode) } case locationSSM: outputFilePath = locationSSM + "_" + EscapeFilePath(locationArray[1]) @@ -152,32 +117,29 @@ func RunDownloader( } if err != nil { - return err + return fmt.Errorf("fail to fetch/remove json config: %v", err) } - // Handle configuration based on multiConfig setting - if multiConfig == "remove" { - outputPath := filepath.Join(outputDir, outputFilePath) - if err := os.Remove(outputPath); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("failed to remove file %s: %v", outputPath, err) - } - } else { + if multiConfig != "remove" { outputPath := filepath.Join(outputDir, outputFilePath+constants.FileSuffixTmp) if err := os.WriteFile(outputPath, []byte(config), 0644); err != nil { - return fmt.Errorf("failed to write to file %s: %v", outputPath, err) + return fmt.Errorf("failed to write the json file %v: %v", outputPath, err) + } + } else { + outputPath := filepath.Join(outputDir, outputFilePath) + if err := os.Remove(outputPath); err != nil { + return fmt.Errorf("failed to remove the json file %v: %v", outputPath, err) } } return nil } -func defaultJsonConfig(mode string) (string, error) { +func defaultJSONConfig(mode string) (string, error) { return config.DefaultJsonConfig(config.ToValidOs(""), mode), nil } func downloadFromSSM(region, parameterStoreName, mode string, credsConfig map[string]string) (string, error) { - fmt.Printf("Region: %v\n", region) - fmt.Printf("credsConfig: %v\n", credsConfig) var ses *session.Session credsMap := util.GetCredentials(mode, credsConfig) profile, profileOk := credsMap[commonconfig.CredentialProfile] @@ -196,8 +158,7 @@ func downloadFromSSM(region, parameterStoreName, mode string, credsConfig map[st ses, err := session.NewSession(rootconfig) if err != nil { - fmt.Printf("Error in creating session: %v\n", err) - return "", err + return "", fmt.Errorf("error in creating session: %v", err) } ssmClient := ssm.New(ses) @@ -207,8 +168,7 @@ func downloadFromSSM(region, parameterStoreName, mode string, credsConfig map[st } output, err := ssmClient.GetParameter(&input) if err != nil { - fmt.Printf("Error in retrieving parameter store content: %v\n", err) - return "", err + return "", fmt.Errorf("error in retrieving parameter store content: %v", err) } return *output.Parameter.Value, nil @@ -218,3 +178,29 @@ func readFromFile(filePath string) (string, error) { bytes, err := os.ReadFile(filePath) return string(bytes), err } + +func EscapeFilePath(filePath string) string { + escapedFilePath := filepath.ToSlash(filePath) + escapedFilePath = strings.Replace(escapedFilePath, "/", "_", -1) + escapedFilePath = strings.Replace(escapedFilePath, " ", "_", -1) + escapedFilePath = strings.Replace(escapedFilePath, ":", "_", -1) + return escapedFilePath +} + +func cleanupOutputDir(outputDir string) error { + return filepath.Walk(outputDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return fmt.Errorf("cannot access %v: %v", path, err) + } + if info.IsDir() { + if strings.EqualFold(path, outputDir) { + return nil + } + return filepath.SkipDir + } + if filepath.Ext(path) == constants.FileSuffixTmp { + return os.Remove(path) + } + return nil + }) +} diff --git a/tool/downloader/flags/flags.go b/tool/downloader/flags/flags.go index cb55c5eed0..2fb06cfa9c 100644 --- a/tool/downloader/flags/flags.go +++ b/tool/downloader/flags/flags.go @@ -8,9 +8,9 @@ import "github.com/aws/amazon-cloudwatch-agent/tool/cmdwrapper" const Command = "config-downloader" var DownloaderFlags = map[string]cmdwrapper.Flag{ - "mode": {"mode", "ec2", "Please provide the mode, i.e. ec2, onPremise, onPrem, auto"}, - "download-source": {"download-source", "", "Download source. Example: \"ssm:my-parameter-store-name\" for an EC2 SSM Parameter Store Name holding your CloudWatch Agent configuration."}, - "output-dir": {"output-dir", "", "Path of output json config directory."}, - "config": {"config", "", "Please provide the common-config file"}, - "multi-config": {"multi-config", "default", "valid values: default, append, remove"}, + "mode": {DefaultValue: "ec2", Description: "Please provide the mode, i.e. ec2, onPremise, onPrem, auto"}, + "download-source": {DefaultValue: "", Description: "Download source. Example: \"ssm:my-parameter-store-name\" for an EC2 SSM Parameter Store Name holding your CloudWatch Agent configuration."}, + "output-dir": {DefaultValue: "", Description: "Path of output json config directory."}, + "config": {DefaultValue: "", Description: "Please provide the common-config file"}, + "multi-config": {DefaultValue: "default", Description: "valid values: default, append, remove"}, } diff --git a/tool/wizard/flags/flags.go b/tool/wizard/flags/flags.go index 8eb6728af0..95223047f3 100644 --- a/tool/wizard/flags/flags.go +++ b/tool/wizard/flags/flags.go @@ -18,13 +18,13 @@ const ( ) var WizardFlags = map[string]cmdwrapper.Flag{ - "is-non-interactive-windows-migration": {"isNonInteractiveWindowsMigration", "false", "If true, it will use command line args to bypass the wizard. Default value is false."}, - "is-non-interactive-linux-migration": {"isNonInteractiveLinuxMigration", "false", "If true, it will do the linux config migration. Default value is false."}, - "traces-only": {"tracesOnly", "false", "If true, only trace configuration will be generated"}, - "use-parameter-store": {"useParameterStore", "false", "If true, it will use the parameter store for the migrated config storage."}, - "non-interactive-xray-migration": {"nonInteractiveXrayMigration", "false", "If true, then this is part of non Interactive xray migration tool."}, - "config-file-path": {"configFilePath", "", fmt.Sprintf("The path of the old config file. Default is %s on Windows or %s on Linux", DefaultFilePathWindowsConfiguration, DefaultFilePathLinuxConfiguration)}, - "config-output-path": {"configOutputPath", "", "Specifies where to write the configuration file generated by the wizard"}, - "parameter-store-name": {"parameterStoreName", "", "The parameter store name. Default is AmazonCloudWatch-windows"}, - "parameter-store-region": {"parameterStoreRegion", "", "The parameter store region. Default is us-east-1"}, + "is-non-interactive-windows-migration": {DefaultValue: "false", Description: "If true, it will use command line args to bypass the wizard. Default value is false."}, + "is-non-interactive-linux-migration": {DefaultValue: "false", Description: "If true, it will do the linux config migration. Default value is false."}, + "traces-only": {DefaultValue: "false", Description: "If true, only trace configuration will be generated"}, + "use-parameter-store": {DefaultValue: "false", Description: "If true, it will use the parameter store for the migrated config storage."}, + "non-interactive-xray-migration": {DefaultValue: "false", Description: "If true, then this is part of non Interactive xray migration tool."}, + "config-file-path": {DefaultValue: "", Description: fmt.Sprintf("The path of the old config file. Default is %s on Windows or %s on Linux", DefaultFilePathWindowsConfiguration, DefaultFilePathLinuxConfiguration)}, + "config-output-path": {DefaultValue: "", Description: "Specifies where to write the configuration file generated by the wizard"}, + "parameter-store-name": {DefaultValue: "", Description: "The parameter store name. Default is AmazonCloudWatch-windows"}, + "parameter-store-region": {DefaultValue: "", Description: "The parameter store region. Default is us-east-1"}, } diff --git a/tool/wizard/wizard.go b/tool/wizard/wizard.go index b8e8600785..26d6947895 100644 --- a/tool/wizard/wizard.go +++ b/tool/wizard/wizard.go @@ -22,7 +22,7 @@ import ( ) type IMainProcessor interface { - VerifyProcessor(processor interface{}) + VerifyProcessor(_processor interface{}) } type MainProcessorStruct struct{} @@ -42,7 +42,10 @@ type Params struct { } func init() { - stdin.Scanln = func(a ...interface{}) (n int, err error) { + stdin.Scanln = func(a ...interface{}) (int, error) { + var n int + var err error + scanner := bufio.NewScanner(os.Stdin) scanner.Scan() if len(a) > 0 { @@ -50,7 +53,7 @@ func init() { n = len(*a[0].(*string)) } err = scanner.Err() - return + return n, err } processors.StartProcessor = basicInfo.Processor } @@ -140,5 +143,5 @@ func startProcessing(configOutputPath string, isNonInteractiveWindowsMigration, } } -func (p *MainProcessorStruct) VerifyProcessor(processor interface{}) { +func (p *MainProcessorStruct) VerifyProcessor(interface{}) { } diff --git a/translator/cmdutil/translator.go b/translator/cmdutil/translator.go index 27d28dd9db..f8896f4c1a 100644 --- a/translator/cmdutil/translator.go +++ b/translator/cmdutil/translator.go @@ -47,17 +47,17 @@ func RunTranslator(flags map[string]*string) error { return ct.Translate() } -func NewConfigTranslator(inputOs, inputJsonFile, inputJsonDir, inputTomlFile, inputMode, inputConfig, multiConfig string) (*ConfigTranslator, error) { +func NewConfigTranslator(inputOs, inputJSONFile, inputJSONDir, inputTOMLFile, inputMode, inputConfig, multiConfig string) (*ConfigTranslator, error) { ct := ConfigTranslator{ ctx: context.CurrentContext(), } ct.ctx.SetOs(inputOs) - ct.ctx.SetInputJsonFilePath(inputJsonFile) - ct.ctx.SetInputJsonDirPath(inputJsonDir) + ct.ctx.SetInputJsonFilePath(inputJSONFile) + ct.ctx.SetInputJsonDirPath(inputJSONDir) ct.ctx.SetMultiConfig(multiConfig) - ct.ctx.SetOutputTomlFilePath(inputTomlFile) + ct.ctx.SetOutputTomlFilePath(inputTOMLFile) if inputConfig != "" { f, err := os.Open(inputConfig) @@ -97,7 +97,7 @@ func (ct *ConfigTranslator) Translate() error { } }() - mergedJsonConfigMap, err := GenerateMergedJsonConfigMap(ct.ctx) + mergedJSONConfigMap, err := GenerateMergedJsonConfigMap(ct.ctx) if err != nil { return fmt.Errorf("failed to generate merged json config: %v", err) } @@ -105,7 +105,7 @@ func (ct *ConfigTranslator) Translate() error { if !ct.ctx.RunInContainer() { current, err := user.Current() if err == nil && current.Name == "****" { - runAsUser, err := userutil.DetectRunAsUser(mergedJsonConfigMap) + runAsUser, err := userutil.DetectRunAsUser(mergedJSONConfigMap) if err != nil { return fmt.Errorf("failed to detectRunAsUser") } @@ -116,11 +116,11 @@ func (ct *ConfigTranslator) Translate() error { tomlConfigPath := GetTomlConfigPath(ct.ctx.OutputTomlFilePath()) tomlConfigDir := filepath.Dir(tomlConfigPath) yamlConfigPath := filepath.Join(tomlConfigDir, yamlConfigFileName) - tomlConfig, err := TranslateJsonMapToTomlConfig(mergedJsonConfigMap) + tomlConfig, err := TranslateJsonMapToTomlConfig(mergedJSONConfigMap) if err != nil { return fmt.Errorf("failed to generate TOML configuration validation content: %v", err) } - yamlConfig, err := TranslateJsonMapToYamlConfig(mergedJsonConfigMap) + yamlConfig, err := TranslateJsonMapToYamlConfig(mergedJSONConfigMap) if err != nil && !errors.Is(err, pipeline.ErrNoPipelines) { return fmt.Errorf("failed to generate YAML configuration validation content: %v", err) } @@ -133,7 +133,7 @@ func (ct *ConfigTranslator) Translate() error { log.Println(exitSuccessMessage) envConfigPath := filepath.Join(tomlConfigDir, envConfigFileName) - TranslateJsonMapToEnvConfigFile(mergedJsonConfigMap, envConfigPath) + TranslateJsonMapToEnvConfigFile(mergedJSONConfigMap, envConfigPath) return nil } diff --git a/translator/cmdutil/translatorutil_test.go b/translator/cmdutil/translatorutil_test.go index 0fc4e22ea5..4dbfa2c06e 100644 --- a/translator/cmdutil/translatorutil_test.go +++ b/translator/cmdutil/translatorutil_test.go @@ -223,7 +223,7 @@ func checkIfSchemaValidateAsExpected(t *testing.T, jsonInputPath string, shouldS t.Logf("Type: %v \n", errorDetail.Type()) t.Logf("Value: %v \n", errorDetail.Value()) if _, ok := actualErrorMap[errorDetail.Type()]; ok { - actualErrorMap[errorDetail.Type()] += 1 + actualErrorMap[errorDetail.Type()]++ } else { actualErrorMap[errorDetail.Type()] = 1 } diff --git a/translator/flags/flags.go b/translator/flags/flags.go index 2313f742cc..b82a3debcd 100644 --- a/translator/flags/flags.go +++ b/translator/flags/flags.go @@ -8,11 +8,11 @@ import "github.com/aws/amazon-cloudwatch-agent/tool/cmdwrapper" const TranslatorCommand = "config-translator" var TranslatorFlags = map[string]cmdwrapper.Flag{ - "os": {"os", "", "Please provide the os preference, valid value: windows/linux."}, - "input": {"input", "", "Please provide the path of input agent json config file"}, - "input-dir": {"input-dir", "", "Please provide the path of input agent json config directory."}, - "output": {"output", "", "Please provide the path of the output CWAgent config file"}, - "mode": {"mode", "ec2", "Please provide the mode, i.e. ec2, onPremise, onPrem, auto"}, - "config": {"config", "", "Please provide the common-config file"}, - "multi-config": {"multi-config", "remove", "valid values: default, append, remove"}, + "os": {DefaultValue: "", Description: "Please provide the os preference, valid value: windows/linux."}, + "input": {DefaultValue: "", Description: "Please provide the path of input agent json config file"}, + "input-dir": {DefaultValue: "", Description: "Please provide the path of input agent json config directory."}, + "output": {DefaultValue: "", Description: "Please provide the path of the output CWAgent config file"}, + "mode": {DefaultValue: "ec2", Description: "Please provide the mode, i.e. ec2, onPremise, onPrem, auto"}, + "config": {DefaultValue: "", Description: "Please provide the common-config file"}, + "multi-config": {DefaultValue: "remove", Description: "valid values: default, append, remove"}, }