Enhance Atmos CLI: Add Support for Custom Base Path and Config Paths #1091
+826
−321
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| logs: | ||
| # Can also be set using 'ATMOS_LOGS_FILE' ENV var, or '--logs-file' command-line argument | ||
| # File or standard file descriptor to write logs to | ||
| # Logs can be written to any file or any standard file descriptor, including `/dev/stdout`, `/dev/stderr` and `/dev/null` | ||
| file: "/dev/stderr" | ||
| # Supported log levels: Trace, Debug, Info, Warning, Off | ||
| # Can also be set using 'ATMOS_LOGS_LEVEL' ENV var, or '--logs-level' command-line argument | ||
| level: Info |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| package config | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
|
|
||
| log "github.com/charmbracelet/log" | ||
| "github.com/cloudposse/atmos/pkg/schema" | ||
| "github.com/spf13/viper" | ||
| ) | ||
|
|
||
| var ( | ||
| ErrExpectedDirOrPattern = errors.New("--config-path expected directory found file") | ||
| ErrFileNotFound = errors.New("file not found") | ||
| ErrExpectedFile = errors.New("--config expected file found directory") | ||
| ErrAtmosArgConfigNotFound = errors.New("atmos configuration not found") | ||
| ErrAtmosFilesDIrConfigNotFound = errors.New("`atmos.yaml` or `.atmos.yaml` configuration file not found on directory") | ||
| ) | ||
|
|
||
| // loadConfigFromCLIArgs handles the loading of configurations provided via --config-path and --config. | ||
| func loadConfigFromCLIArgs(v *viper.Viper, configAndStacksInfo *schema.ConfigAndStacksInfo, atmosConfig *schema.AtmosConfiguration) error { | ||
| log.Debug("loading config from command line arguments") | ||
|
|
||
| configFilesArgs := configAndStacksInfo.AtmosConfigFilesFromArg | ||
| configDirsArgs := configAndStacksInfo.AtmosConfigDirsFromArg | ||
| var configPaths []string | ||
|
|
||
| // Merge all config from --config files | ||
| if len(configFilesArgs) > 0 { | ||
| if err := mergeFiles(v, configFilesArgs); err != nil { | ||
| return err | ||
| } | ||
| configPaths = append(configPaths, configFilesArgs...) | ||
| } | ||
|
|
||
| // Merge config from --config-path directories | ||
| if len(configDirsArgs) > 0 { | ||
| paths, err := mergeConfigFromDirectories(v, configDirsArgs) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| configPaths = append(configPaths, paths...) | ||
| } | ||
|
|
||
| // Check if any config files were found from command line arguments | ||
| if len(configPaths) == 0 { | ||
| log.Debug("no config files found from command line arguments") | ||
| return ErrAtmosArgConfigNotFound | ||
| } | ||
|
|
||
| if err := v.Unmarshal(atmosConfig); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| atmosConfig.CliConfigPath = connectPaths(configPaths) | ||
| return nil | ||
| } | ||
|
|
||
| // mergeFiles merges config files from the provided paths. | ||
| func mergeFiles(v *viper.Viper, configFilePaths []string) error { | ||
| err := validatedIsFiles(configFilePaths) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| for _, configPath := range configFilePaths { | ||
| err := mergeConfigFile(configPath, v) | ||
| if err != nil { | ||
| log.Debug("error loading config file", "path", configPath, "error", err) | ||
| return err | ||
| } | ||
| log.Debug("config file merged", "path", configPath) | ||
| if err := mergeDefaultImports(configPath, v); err != nil { | ||
| log.Debug("error process imports", "path", configPath, "error", err) | ||
| } | ||
| if err := mergeImports(v); err != nil { | ||
| log.Debug("error process imports", "file", configPath, "error", err) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
Comment on lines
+61
to
+82
There was a problem hiding this comment. 🛠️ Refactor suggestion Merge files with partial error handling. Errors in 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 70-72: pkg/config/load_config_args.go#L70-L72 [warning] 78-79: pkg/config/load_config_args.go#L78-L79 |
||
|
|
||
| // mergeConfigFromDirectories merges config files from the provided directories. | ||
| func mergeConfigFromDirectories(v *viper.Viper, dirPaths []string) ([]string, error) { | ||
| if err := validatedIsDirs(dirPaths); err != nil { | ||
| return nil, err | ||
| } | ||
| var configPaths []string | ||
| for _, confDirPath := range dirPaths { | ||
| err := mergeConfig(v, confDirPath, CliConfigFileName, true) | ||
| if err != nil { | ||
| log.Debug("Failed to find atmos config", "path", confDirPath, "error", err) | ||
| switch err.(type) { | ||
| case viper.ConfigFileNotFoundError: | ||
| log.Debug("Failed to found atmos config", "file", filepath.Join(confDirPath, CliConfigFileName)) | ||
| default: | ||
| return nil, err | ||
| } | ||
| } | ||
| if err == nil { | ||
| log.Debug("atmos config file merged", "path", v.ConfigFileUsed()) | ||
| configPaths = append(configPaths, confDirPath) | ||
| continue | ||
| } | ||
| err = mergeConfig(v, confDirPath, DotCliConfigFileName, true) | ||
| if err != nil { | ||
| log.Debug("Failed to found .atmos config", "path", filepath.Join(confDirPath, CliConfigFileName), "error", err) | ||
| return nil, fmt.Errorf("%w: %s", ErrAtmosFilesDIrConfigNotFound, confDirPath) | ||
| } | ||
| log.Debug(".atmos config file merged", "path", v.ConfigFileUsed()) | ||
| configPaths = append(configPaths, confDirPath) | ||
| } | ||
| return configPaths, nil | ||
| } | ||
|
|
||
| func validatedIsDirs(dirPaths []string) error { | ||
| for _, dirPath := range dirPaths { | ||
| stat, err := os.Stat(dirPath) | ||
| if err != nil { | ||
| log.Debug("--config-path directory not found", "path", dirPath) | ||
| return err | ||
| } | ||
| if !stat.IsDir() { | ||
| log.Debug("--config-path expected directory found file", "path", dirPath) | ||
| return ErrAtmosDIrConfigNotFound | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func validatedIsFiles(files []string) error { | ||
| for _, filePath := range files { | ||
| stat, err := os.Stat(filePath) | ||
| if err != nil { | ||
| log.Debug("--config file not found", "path", filePath) | ||
| return ErrFileNotFound | ||
| } | ||
| if stat.IsDir() { | ||
| log.Debug("--config expected file found directors", "path", filePath) | ||
| return ErrExpectedFile | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func connectPaths(paths []string) string { | ||
| if len(paths) == 1 { | ||
| return paths[0] | ||
| } | ||
| var result string | ||
| for _, path := range paths { | ||
| result += path + ";" | ||
| } | ||
| return result | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Implemented CLI flags for configuration but missing test coverage.
The implementation properly retrieves the new CLI flags (
--base-path,--config, and--config-path) and handles potential errors. This aligns with the PR objectives of enhancing configuration flexibility.However, the error handling code for these flags (lines 210-211, 214-215, 218-219) lacks test coverage according to static analysis.
🏁 Script executed:
Length of output: 487
Action: Add missing tests for CLI flag error handling in internal/exec/utils.go
The implementation for retrieving the CLI flags (
--base-path,--config, and--config-path) is correctly handling error conditions. However, our investigation didn't reveal any tests covering the error paths in the code (lines 208–219). Please add test cases simulating error scenarios forGetString()andGetStringSlice()to ensure that these branches are properly exercised.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 210-211: internal/exec/utils.go#L210-L211
Added lines #L210 - L211 were not covered by tests
[warning] 214-215: internal/exec/utils.go#L214-L215
Added lines #L214 - L215 were not covered by tests
[warning] 218-219: internal/exec/utils.go#L218-L219
Added lines #L218 - L219 were not covered by tests