From 0a750103b1993c0c805cafb9bab8388bb8019569 Mon Sep 17 00:00:00 2001 From: "Vinicius C." Date: Thu, 16 Jan 2025 04:16:07 +0000 Subject: [PATCH] Validate Atmos Log Levels (#930) * log level invalid cover * fixes log level wip * added validation for parser * logger level fixes * update logger * checkpoint * checkpoint wip * final logger improvements --------- Co-authored-by: Andriy Knysh --- pkg/config/utils.go | 16 +++++ pkg/logger/logger.go | 56 ++++++++------- pkg/utils/log_utils.go | 2 +- tests/fixtures/invalid-log-level/atmos.yaml | 8 +++ tests/fixtures/valid-log-level/atmos.yaml | 8 +++ tests/test-cases/log-level-validation.yaml | 80 +++++++++++++++++++++ 6 files changed, 143 insertions(+), 27 deletions(-) create mode 100644 tests/fixtures/invalid-log-level/atmos.yaml create mode 100644 tests/fixtures/valid-log-level/atmos.yaml create mode 100644 tests/test-cases/log-level-validation.yaml diff --git a/pkg/config/utils.go b/pkg/config/utils.go index d78601618c..8d658d4436 100644 --- a/pkg/config/utils.go +++ b/pkg/config/utils.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/charmbracelet/log" + "github.com/cloudposse/atmos/pkg/logger" "github.com/cloudposse/atmos/pkg/schema" "github.com/cloudposse/atmos/pkg/store" u "github.com/cloudposse/atmos/pkg/utils" @@ -358,6 +359,11 @@ func processEnvVars(atmosConfig *schema.AtmosConfiguration) error { logsLevel := os.Getenv("ATMOS_LOGS_LEVEL") if len(logsLevel) > 0 { u.LogTrace(*atmosConfig, fmt.Sprintf("Found ENV var ATMOS_LOGS_LEVEL=%s", logsLevel)) + // Validate the log level before setting it + if _, err := logger.ParseLogLevel(logsLevel); err != nil { + return err + } + // Only set the log level if validation passes atmosConfig.Logs.Level = logsLevel } @@ -396,6 +402,12 @@ func checkConfig(atmosConfig schema.AtmosConfiguration) error { return errors.New("at least one path must be provided in 'stacks.included_paths' config or ATMOS_STACKS_INCLUDED_PATHS' ENV variable") } + if len(atmosConfig.Logs.Level) > 0 { + if _, err := logger.ParseLogLevel(atmosConfig.Logs.Level); err != nil { + return err + } + } + return nil } @@ -473,6 +485,10 @@ func processCommandLineArgs(atmosConfig *schema.AtmosConfiguration, configAndSta u.LogTrace(*atmosConfig, fmt.Sprintf("Using command line argument '%s' as path to Atmos JSON Schema", configAndStacksInfo.AtmosManifestJsonSchema)) } if len(configAndStacksInfo.LogsLevel) > 0 { + if _, err := logger.ParseLogLevel(configAndStacksInfo.LogsLevel); err != nil { + return err + } + // Only set the log level if validation passes atmosConfig.Logs.Level = configAndStacksInfo.LogsLevel u.LogTrace(*atmosConfig, fmt.Sprintf("Using command line argument '%s=%s'", LogsLevelFlag, configAndStacksInfo.LogsLevel)) } diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 833885f6f6..5fe37fa2f6 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -20,6 +20,15 @@ const ( LogLevelWarning LogLevel = "Warning" ) +// logLevelOrder defines the order of log levels from most verbose to least verbose +var logLevelOrder = map[LogLevel]int{ + LogLevelTrace: 0, + LogLevelDebug: 1, + LogLevelInfo: 2, + LogLevelWarning: 3, + LogLevelOff: 4, +} + type Logger struct { LogLevel LogLevel File string @@ -45,18 +54,14 @@ func ParseLogLevel(logLevel string) (LogLevel, error) { return LogLevelInfo, nil } - switch LogLevel(logLevel) { // Convert logLevel to type LogLevel - case LogLevelTrace: - return LogLevelTrace, nil - case LogLevelDebug: - return LogLevelDebug, nil - case LogLevelInfo: - return LogLevelInfo, nil - case LogLevelWarning: - return LogLevelWarning, nil - default: - return LogLevelInfo, fmt.Errorf("invalid log level '%s'. Supported log levels are Trace, Debug, Info, Warning, Off", logLevel) + validLevels := []LogLevel{LogLevelTrace, LogLevelDebug, LogLevelInfo, LogLevelWarning, LogLevelOff} + for _, level := range validLevels { + if LogLevel(logLevel) == level { + return level, nil + } } + + return "", fmt.Errorf("Error: Invalid log level '%s'. Valid options are: %v", logLevel, validLevels) } func (l *Logger) log(logColor *color.Color, message string) { @@ -104,7 +109,7 @@ func (l *Logger) SetLogLevel(logLevel LogLevel) error { } func (l *Logger) Error(err error) { - if err != nil { + if err != nil && l.LogLevel != LogLevelOff { _, err2 := theme.Colors.Error.Fprintln(color.Error, err.Error()+"\n") if err2 != nil { color.Red("Error logging the error:") @@ -115,35 +120,34 @@ func (l *Logger) Error(err error) { } } +// isLevelEnabled checks if a given log level should be enabled based on the logger's current level +func (l *Logger) isLevelEnabled(level LogLevel) bool { + if l.LogLevel == LogLevelOff { + return false + } + return logLevelOrder[level] >= logLevelOrder[l.LogLevel] +} + func (l *Logger) Trace(message string) { - if l.LogLevel == LogLevelTrace { + if l.isLevelEnabled(LogLevelTrace) { l.log(theme.Colors.Info, message) } } func (l *Logger) Debug(message string) { - if l.LogLevel == LogLevelTrace || - l.LogLevel == LogLevelDebug { - + if l.isLevelEnabled(LogLevelDebug) { l.log(theme.Colors.Info, message) } } func (l *Logger) Info(message string) { - if l.LogLevel == LogLevelTrace || - l.LogLevel == LogLevelDebug || - l.LogLevel == LogLevelInfo { - - l.log(theme.Colors.Default, message) + if l.isLevelEnabled(LogLevelInfo) { + l.log(theme.Colors.Info, message) } } func (l *Logger) Warning(message string) { - if l.LogLevel == LogLevelTrace || - l.LogLevel == LogLevelDebug || - l.LogLevel == LogLevelInfo || - l.LogLevel == LogLevelWarning { - + if l.isLevelEnabled(LogLevelWarning) { l.log(theme.Colors.Warning, message) } } diff --git a/pkg/utils/log_utils.go b/pkg/utils/log_utils.go index af3007f184..63907a6e11 100644 --- a/pkg/utils/log_utils.go +++ b/pkg/utils/log_utils.go @@ -48,7 +48,7 @@ func LogErrorAndExit(atmosConfig schema.AtmosConfiguration, err error) { // LogError logs errors to std.Error func LogError(atmosConfig schema.AtmosConfiguration, err error) { if err != nil { - _, printErr := theme.Colors.Error.Fprintln(color.Error, err.Error()+"\n") + _, printErr := theme.Colors.Error.Fprintln(color.Error, err.Error()) if printErr != nil { theme.Colors.Error.Println("Error logging the error:") theme.Colors.Error.Printf("%s\n", printErr) diff --git a/tests/fixtures/invalid-log-level/atmos.yaml b/tests/fixtures/invalid-log-level/atmos.yaml new file mode 100644 index 0000000000..3af5102492 --- /dev/null +++ b/tests/fixtures/invalid-log-level/atmos.yaml @@ -0,0 +1,8 @@ +logs: + level: XTrace + file: /dev/stdout + +stacks: + base_path: stacks + included_paths: + - "**/*" \ No newline at end of file diff --git a/tests/fixtures/valid-log-level/atmos.yaml b/tests/fixtures/valid-log-level/atmos.yaml new file mode 100644 index 0000000000..9f4cc8c3c6 --- /dev/null +++ b/tests/fixtures/valid-log-level/atmos.yaml @@ -0,0 +1,8 @@ +logs: + level: Trace + file: /dev/stdout + +stacks: + base_path: stacks + included_paths: + - "**/*" \ No newline at end of file diff --git a/tests/test-cases/log-level-validation.yaml b/tests/test-cases/log-level-validation.yaml new file mode 100644 index 0000000000..f7172a33bf --- /dev/null +++ b/tests/test-cases/log-level-validation.yaml @@ -0,0 +1,80 @@ +tests: + - name: "Invalid Log Level in Config File" + enabled: true + description: "Test validation of invalid log level in atmos.yaml config file" + workdir: "fixtures/invalid-log-level" + command: "atmos" + args: + - terraform + - plan + - test + - -s + - test + expect: + exit_code: 1 + stderr: + - "Error: Invalid log level 'XTrace'. Valid options are: \\[Trace Debug Info Warning Off\\]" + + - name: "Invalid Log Level in Environment Variable" + enabled: true + description: "Test validation of invalid log level in ATMOS_LOGS_LEVEL env var" + workdir: "../" + command: "atmos" + args: + - terraform + - plan + - test + - -s + - test + env: + ATMOS_LOGS_LEVEL: XTrace + expect: + exit_code: 1 + stderr: + - "Error: Invalid log level 'XTrace'. Valid options are: \\[Trace Debug Info Warning Off\\]" + + - name: "Valid Log Level in Config File" + enabled: true + description: "Test validation of valid log level in atmos.yaml config file" + workdir: "fixtures/valid-log-level" + command: "atmos" + args: + - terraform + - plan + - test + - -s + - test + expect: + exit_code: 0 + + - name: "Valid Log Level in Environment Variable" + enabled: true + description: "Test validation of valid log level in ATMOS_LOGS_LEVEL env var" + workdir: "../" + command: "atmos" + args: + - terraform + - plan + - test + - -s + - test + env: + ATMOS_LOGS_LEVEL: Debug + expect: + exit_code: 0 + + - name: "Valid Log Level in Command Line Flag" + enabled: true + description: "Test validation of valid log level in --logs-level flag" + workdir: "../" + command: "atmos" + args: + - --logs-level + - Info + - terraform + - plan + - test + - -s + - test + expect: + exit_code: 0 \ No newline at end of file