From dc986e660f2fde77261b50d661389de0da0e3618 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Fri, 3 Dec 2021 09:39:18 -0300 Subject: [PATCH] cli:chore - deprecate monitor-retry-count flag Previously we control all execution of tools using a `monitor` package, but this package contained a lot of data races that was fixed on #477 and the monitor retry count flag was no longer necessary, since this was used to control the timeout counter. This commit mark monitor-retry-count flag as hidden and deprecated, so new users will not see this flag and users that is currently using this flag on pipelines will see a warning saying to use just --analysis-timeout flag. Signed-off-by: Matheus Alcantara --- cmd/app/start/start.go | 25 ++++++++++++++++------- config/config.go | 15 ++++++-------- config/config_test.go | 14 ++----------- internal/controllers/analyzer/analyzer.go | 10 ++------- internal/helpers/messages/panic.go | 2 ++ internal/usecases/cli/cli.go | 1 - 6 files changed, 30 insertions(+), 37 deletions(-) diff --git a/cmd/app/start/start.go b/cmd/app/start/start.go index 4764fe07b..fd367266b 100644 --- a/cmd/app/start/start.go +++ b/cmd/app/start/start.go @@ -85,12 +85,7 @@ func (s *Start) CreateStartCommand() *cobra.Command { RunE: s.runE, } - startCmd.PersistentFlags(). - Int64P( - "monitor-retry-count", "m", - s.configs.MonitorRetryInSeconds, - "The number of retries for the monitor.", - ) + startCmd.PersistentFlags().Int64P("monitor-retry-count", "m", 0, "The number of retries for the monitor.") startCmd.PersistentFlags(). StringP( @@ -269,7 +264,23 @@ func (s *Start) CreateStartCommand() *cobra.Command { ) } - return startCmd + return s.setDeprecatedFlags(startCmd) +} + +func (s *Start) setDeprecatedFlags(cmd *cobra.Command) *cobra.Command { + flags := cmd.PersistentFlags() + + if err := flags.MarkHidden("monitor-retry-count"); err != nil { + logger.LogPanic(fmt.Sprintf(messages.PanicMarkHiddenFlag, "monitor-retry-count"), err) + } + + if err := flags.MarkDeprecated( + "monitor-retry-count", "monitor component no longer exists in Horusec. Use only --analysis-timeout.", + ); err != nil { + logger.LogPanic(fmt.Sprintf(messages.PanicMarkDeprecatedFlag, "monitor-retry-count"), err) + } + + return cmd } func (s *Start) runE(cmd *cobra.Command, _ []string) error { diff --git a/config/config.go b/config/config.go index 59d1dc364..2c183c076 100644 --- a/config/config.go +++ b/config/config.go @@ -45,7 +45,6 @@ const ( EnvHorusecAPIUri = "HORUSEC_CLI_HORUSEC_API_URI" EnvTimeoutInSecondsRequest = "HORUSEC_CLI_TIMEOUT_IN_SECONDS_REQUEST" EnvTimeoutInSecondsAnalysis = "HORUSEC_CLI_TIMEOUT_IN_SECONDS_ANALYSIS" - EnvMonitorRetryInSeconds = "HORUSEC_CLI_MONITOR_RETRY_IN_SECONDS" EnvRepositoryAuthorization = "HORUSEC_CLI_REPOSITORY_AUTHORIZATION" EnvPrintOutputType = "HORUSEC_CLI_PRINT_OUTPUT_TYPE" EnvJSONOutputFilePath = "HORUSEC_CLI_JSON_OUTPUT_FILEPATH" @@ -72,6 +71,9 @@ const ( EnvLogFilePath = "HORUSEC_CLI_LOG_FILE_PATH" EnvEnableOwaspDependencyCheck = "HORUSEC_CLI_ENABLE_OWASP_DEPENDENCY_CHECK" EnvEnableShellCheck = "HORUSEC_CLI_ENABLE_SHELLCHECK" + + // Deprecated: Monitor component no longer exists in Horusec. + EnvMonitorRetryInSeconds = "HORUSEC_CLI_MONITOR_RETRY_IN_SECONDS" ) type GlobalOptions struct { @@ -98,7 +100,6 @@ type StartOptions struct { ContainerBindProjectPath string `json:"container_bind_project_path"` TimeoutInSecondsRequest int64 `json:"timeout_in_seconds_request"` TimeoutInSecondsAnalysis int64 `json:"timeout_in_seconds_analysis"` - MonitorRetryInSeconds int64 `json:"monitor_retry_in_seconds"` ReturnErrorIfFoundVulnerability bool `json:"return_error_if_found_vulnerability"` EnableGitHistoryAnalysis bool `json:"enable_git_history_analysis"` CertInsecureSkipVerify bool `json:"cert_insecure_skip_verify"` @@ -116,6 +117,9 @@ type StartOptions struct { Headers map[string]string `json:"headers"` WorkDir *workdir.WorkDir `json:"work_dir"` CustomImages customimages.CustomImages `json:"custom_images"` + + // Deprecated: Monitor component no longer exists in Horusec. + MonitorRetryInSeconds int64 `json:"monitor_retry_in_seconds"` } type Config struct { @@ -147,7 +151,6 @@ func New() *Config { HorusecAPIUri: "http://0.0.0.0:8000", TimeoutInSecondsRequest: 300, TimeoutInSecondsAnalysis: 600, - MonitorRetryInSeconds: 15, RepositoryAuthorization: uuid.Nil.String(), PrintOutputType: "", JSONOutputFilePath: "", @@ -189,7 +192,6 @@ func (c *Config) LoadGlobalFlags(cmd *cobra.Command) *Config { // //nolint:funlen func (c *Config) LoadStartFlags(cmd *cobra.Command) *Config { - c.MonitorRetryInSeconds = c.extractFlagValueInt64(cmd, "monitor-retry-count", c.MonitorRetryInSeconds) c.PrintOutputType = c.extractFlagValueString(cmd, "output-format", c.PrintOutputType) c.JSONOutputFilePath = c.extractFlagValueString(cmd, "json-output-file", c.JSONOutputFilePath) c.SeveritiesToIgnore = c.extractFlagValueStringSlice(cmd, "ignore-severity", c.SeveritiesToIgnore) @@ -242,9 +244,6 @@ func (c *Config) LoadFromConfigFile() *Config { c.TimeoutInSecondsAnalysis = valueordefault.GetInt64ValueOrDefault( viper.GetInt64(c.toLowerCamel(EnvTimeoutInSecondsAnalysis)), c.TimeoutInSecondsAnalysis, ) - c.MonitorRetryInSeconds = valueordefault.GetInt64ValueOrDefault( - viper.GetInt64(c.toLowerCamel(EnvMonitorRetryInSeconds)), c.MonitorRetryInSeconds, - ) c.RepositoryAuthorization = valueordefault.GetStringValueOrDefault( viper.GetString(c.toLowerCamel(EnvRepositoryAuthorization)), c.RepositoryAuthorization, ) @@ -328,7 +327,6 @@ func (c *Config) LoadFromEnvironmentVariables() *Config { c.HorusecAPIUri = env.GetEnvOrDefault(EnvHorusecAPIUri, c.HorusecAPIUri) c.TimeoutInSecondsRequest = env.GetEnvOrDefaultInt64(EnvTimeoutInSecondsRequest, c.TimeoutInSecondsRequest) c.TimeoutInSecondsAnalysis = env.GetEnvOrDefaultInt64(EnvTimeoutInSecondsAnalysis, c.TimeoutInSecondsAnalysis) - c.MonitorRetryInSeconds = env.GetEnvOrDefaultInt64(EnvMonitorRetryInSeconds, c.MonitorRetryInSeconds) c.RepositoryAuthorization = env.GetEnvOrDefault(EnvRepositoryAuthorization, c.RepositoryAuthorization) c.PrintOutputType = env.GetEnvOrDefault(EnvPrintOutputType, c.PrintOutputType) c.JSONOutputFilePath = env.GetEnvOrDefault(EnvJSONOutputFilePath, c.JSONOutputFilePath) @@ -432,7 +430,6 @@ func (c *Config) ToMapLowerCase() map[string]interface{} { c.toLowerCamel(EnvHorusecAPIUri): c.HorusecAPIUri, c.toLowerCamel(EnvTimeoutInSecondsRequest): c.TimeoutInSecondsRequest, c.toLowerCamel(EnvTimeoutInSecondsAnalysis): c.TimeoutInSecondsAnalysis, - c.toLowerCamel(EnvMonitorRetryInSeconds): c.MonitorRetryInSeconds, c.toLowerCamel(EnvRepositoryAuthorization): c.RepositoryAuthorization, c.toLowerCamel(EnvPrintOutputType): c.PrintOutputType, c.toLowerCamel(EnvJSONOutputFilePath): c.JSONOutputFilePath, diff --git a/config/config_test.go b/config/config_test.go index 10583778f..4d2c18030 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -53,7 +53,6 @@ func TestNewHorusecConfig(t *testing.T) { assert.Equal(t, "http://0.0.0.0:8000", configs.HorusecAPIUri) assert.Equal(t, int64(300), configs.TimeoutInSecondsRequest) assert.Equal(t, int64(600), configs.TimeoutInSecondsAnalysis) - assert.Equal(t, int64(15), configs.MonitorRetryInSeconds) assert.Equal(t, uuid.Nil.String(), configs.RepositoryAuthorization) assert.Equal(t, "", configs.PrintOutputType) assert.Equal(t, "", configs.JSONOutputFilePath) @@ -93,7 +92,6 @@ func TestNewHorusecConfig(t *testing.T) { assert.Equal(t, "http://new-viper.horusec.com", configs.HorusecAPIUri) assert.Equal(t, int64(20), configs.TimeoutInSecondsRequest) assert.Equal(t, int64(100), configs.TimeoutInSecondsAnalysis) - assert.Equal(t, int64(10), configs.MonitorRetryInSeconds) assert.Equal(t, "8beffdca-636e-4d73-a22f-b0f7c3cff1c4", configs.RepositoryAuthorization) assert.Equal(t, "json", configs.PrintOutputType) assert.Equal(t, "./output.json", configs.JSONOutputFilePath) @@ -135,7 +133,6 @@ func TestNewHorusecConfig(t *testing.T) { assert.Equal(t, "http://new-viper.horusec.com", configs.HorusecAPIUri) assert.Equal(t, int64(20), configs.TimeoutInSecondsRequest) assert.Equal(t, int64(100), configs.TimeoutInSecondsAnalysis) - assert.Equal(t, int64(10), configs.MonitorRetryInSeconds) assert.Equal(t, "8beffdca-636e-4d73-a22f-b0f7c3cff1c4", configs.RepositoryAuthorization) assert.Equal(t, "json", configs.PrintOutputType) assert.Equal(t, "./output.json", configs.JSONOutputFilePath) @@ -165,7 +162,6 @@ func TestNewHorusecConfig(t *testing.T) { assert.NoError(t, os.Setenv(config.EnvHorusecAPIUri, "http://horusec.com")) assert.NoError(t, os.Setenv(config.EnvTimeoutInSecondsRequest, "99")) assert.NoError(t, os.Setenv(config.EnvTimeoutInSecondsAnalysis, "999")) - assert.NoError(t, os.Setenv(config.EnvMonitorRetryInSeconds, "20")) assert.NoError(t, os.Setenv(config.EnvRepositoryAuthorization, authorization)) assert.NoError(t, os.Setenv(config.EnvPrintOutputType, "sonarqube")) assert.NoError(t, os.Setenv(config.EnvJSONOutputFilePath, filepath.Join(os.TempDir(), "output-sonarqube.json"))) @@ -196,7 +192,6 @@ func TestNewHorusecConfig(t *testing.T) { assert.Equal(t, "http://horusec.com", configs.HorusecAPIUri) assert.Equal(t, int64(99), configs.TimeoutInSecondsRequest) assert.Equal(t, int64(999), configs.TimeoutInSecondsAnalysis) - assert.Equal(t, int64(20), configs.MonitorRetryInSeconds) assert.Equal(t, authorization, configs.RepositoryAuthorization) assert.Equal(t, "sonarqube", configs.PrintOutputType) assert.Equal(t, filepath.Join(os.TempDir(), "output-sonarqube.json"), configs.JSONOutputFilePath) @@ -238,7 +233,6 @@ func TestNewHorusecConfig(t *testing.T) { assert.Equal(t, "http://new-viper.horusec.com", configs.HorusecAPIUri) assert.Equal(t, int64(20), configs.TimeoutInSecondsRequest) assert.Equal(t, int64(100), configs.TimeoutInSecondsAnalysis) - assert.Equal(t, int64(10), configs.MonitorRetryInSeconds) assert.Equal(t, "8beffdca-636e-4d73-a22f-b0f7c3cff1c4", configs.RepositoryAuthorization) assert.Equal(t, "json", configs.PrintOutputType) assert.Equal(t, "./output.json", configs.JSONOutputFilePath) @@ -267,7 +261,6 @@ func TestNewHorusecConfig(t *testing.T) { assert.NoError(t, os.Setenv(config.EnvTimeoutInSecondsRequest, "99")) assert.NoError(t, os.Setenv(config.EnvTimeoutInSecondsAnalysis, "999")) - assert.NoError(t, os.Setenv(config.EnvMonitorRetryInSeconds, "20")) assert.NoError(t, os.Setenv(config.EnvRepositoryAuthorization, authorization)) assert.NoError(t, os.Setenv(config.EnvPrintOutputType, "sonarqube")) assert.NoError(t, os.Setenv(config.EnvJSONOutputFilePath, filepath.Join(os.TempDir(), "output-sonarqube.json"))) @@ -293,7 +286,6 @@ func TestNewHorusecConfig(t *testing.T) { assert.Equal(t, configFilePath, configs.ConfigFilePath) assert.Equal(t, int64(99), configs.TimeoutInSecondsRequest) assert.Equal(t, int64(999), configs.TimeoutInSecondsAnalysis) - assert.Equal(t, int64(20), configs.MonitorRetryInSeconds) assert.Equal(t, authorization, configs.RepositoryAuthorization) assert.Equal(t, "sonarqube", configs.PrintOutputType) assert.Equal(t, filepath.Join(os.TempDir(), "output-sonarqube.json"), configs.JSONOutputFilePath) @@ -378,7 +370,6 @@ func TestNewHorusecConfig(t *testing.T) { assert.Equal(t, []string{"ignore-test-1", "ignore-test-2"}, configs.FilesOrPathsToIgnore) assert.Equal(t, true, configs.CertInsecureSkipVerify) assert.Equal(t, filepath.Join(wd, "tmp", "json-output-file-test.json"), configs.JSONOutputFilePath) - assert.Equal(t, int64(123), configs.MonitorRetryInSeconds) assert.Equal(t, "json", configs.PrintOutputType) assert.Equal(t, "repository-name-test", configs.RepositoryName) assert.Equal(t, int64(123), configs.TimeoutInSecondsRequest) @@ -418,7 +409,6 @@ func TestConfig_Bytes(t *testing.T) { assert.NoError(t, os.Setenv(config.EnvCertPath, "cert-path")) assert.NoError(t, os.Setenv(config.EnvTimeoutInSecondsRequest, "99")) assert.NoError(t, os.Setenv(config.EnvTimeoutInSecondsAnalysis, "999")) - assert.NoError(t, os.Setenv(config.EnvMonitorRetryInSeconds, "20")) assert.NoError(t, os.Setenv(config.EnvRepositoryAuthorization, repositoryAuthorization)) assert.NoError(t, os.Setenv(config.EnvPrintOutputType, "sonarqube")) assert.NoError(t, os.Setenv(config.EnvJSONOutputFilePath, filepath.Join(os.TempDir(), "output-sonarqube.json"))) @@ -459,7 +449,6 @@ func TestConfig_Bytes(t *testing.T) { "container_bind_project_path": "./my-path", "timeout_in_seconds_request": 99, "timeout_in_seconds_analysis": 999, - "monitor_retry_in_seconds": 20, "return_error_if_found_vulnerability": false, "enable_git_history_analysis": false, "cert_insecure_skip_verify": false, @@ -591,6 +580,7 @@ func TestConfig_Bytes(t *testing.T) { "ruby": "", "shell": "" }, + "monitor_retry_in_seconds": 0, "version": "{{VERSION_NOT_FOUND}}" }` // Add scape slashes when running on Windows. @@ -614,7 +604,6 @@ func TestConfig_Bytes(t *testing.T) { "container_bind_project_path": "", "timeout_in_seconds_request": 0, "timeout_in_seconds_analysis": 0, - "monitor_retry_in_seconds": 0, "return_error_if_found_vulnerability": false, "enable_git_history_analysis": false, "cert_insecure_skip_verify": false, @@ -632,6 +621,7 @@ func TestConfig_Bytes(t *testing.T) { "headers": null, "work_dir": null, "custom_images": null, + "monitor_retry_in_seconds": 0, "version": "" }`) cfg := config.Config{} diff --git a/internal/controllers/analyzer/analyzer.go b/internal/controllers/analyzer/analyzer.go index d2c8e3613..2a4eb1553 100644 --- a/internal/controllers/analyzer/analyzer.go +++ b/internal/controllers/analyzer/analyzer.go @@ -258,23 +258,17 @@ func (a *Analyzer) startDetectVulnerabilities(langs []languages.Language) { wg.Wait() }() - timeout := a.config.TimeoutInSecondsAnalysis - timer := time.After(time.Duration(timeout) * time.Second) - retry := a.config.MonitorRetryInSeconds - tick := time.NewTicker(time.Duration(retry) * time.Second) - defer tick.Stop() + timeout := time.After(time.Duration(a.config.TimeoutInSecondsAnalysis) * time.Second) for { select { case <-done: a.loading.Stop() return - case <-timer: + case <-timeout: a.docker.DeleteContainersFromAPI() a.config.IsTimeout = true a.loading.Stop() return - case <-tick.C: - timeout -= retry } } } diff --git a/internal/helpers/messages/panic.go b/internal/helpers/messages/panic.go index fc21eb45d..67751f920 100644 --- a/internal/helpers/messages/panic.go +++ b/internal/helpers/messages/panic.go @@ -21,4 +21,6 @@ const ( MsgPanicGetFlagValue = "{HORUSEC_CLI} Error on getting flag value, check and try again: " MsgPanicNotConnectDocker = "{HORUSEC_CLI} Error when try connect in docker." MsgPanicGetConfigFilePath = "{HORUSEC-CLI} Error on get config file path." + PanicMarkHiddenFlag = "{HORUSEC-CLI} Internal error occurred to hidden %s flag: " + PanicMarkDeprecatedFlag = "{HORUSEC-CLI} Internal error occurred to mark %s flag as deprecated: " ) diff --git a/internal/usecases/cli/cli.go b/internal/usecases/cli/cli.go index 44b17c55b..527056b42 100644 --- a/internal/usecases/cli/cli.go +++ b/internal/usecases/cli/cli.go @@ -44,7 +44,6 @@ func ValidateConfig(cfg *config.Config) error { validation.Field(&cfg.HorusecAPIUri, validation.Required, validation.By(checkIfIsURL(cfg.HorusecAPIUri))), validation.Field(&cfg.TimeoutInSecondsRequest, validation.Required, validation.Min(10)), validation.Field(&cfg.TimeoutInSecondsAnalysis, validation.Required, validation.Min(10)), - validation.Field(&cfg.MonitorRetryInSeconds, validation.Required, validation.Min(10)), validation.Field(&cfg.RepositoryAuthorization, validation.Required, is.UUID), validation.Field(&cfg.PrintOutputType, validation.In(outputtype.JSON, outputtype.SonarQube, outputtype.Text)), validation.Field(&cfg.JSONOutputFilePath, validation.By(validateJSONOutputFilePath(cfg))),