Copilot commented on code in PR #220:
URL:
https://github.com/apache/cloudstack-cloudmonkey/pull/220#discussion_r3902865221
##########
README.md:
##########
@@ -79,6 +79,27 @@ If cloudmonkey is being upgraded from a version lower than
v6.0.0, it must be no
that the cloudmonkey configuration path is changed from
`~/.cloudmonkey/config` to
`~/.cmk/config` and a default `localcloud` profile is created. One must first
set up basic configurations such as apikey/secretkey/username/password/url for
the required profile(s) as required
+### Environment Variables
+
+`cmk` supports environment variables that mirror its CLI flags. CLI flags take
+precedence over environment variables, which take precedence over values in the
+config file.
+
+| Environment variable | Flag | Description |
+|----------------------|------|-------------|
+| `CMK_CONFIG` | `-c` | Config file path |
+| `CMK_PROFILE` | `-p` | Server profile |
+| `CMK_URL` | `-u` | CloudStack's API endpoint URL |
+| `CMK_API_KEY` | `-k` | CloudStack user's API key |
+| `CMK_SECRET_KEY` | `-s` | CloudStack user's secret key |
+| `CMK_OUTPUT` | `-o` | API response output format |
+| `CMK_DEBUG` | `-d` | Enable debug mode when set to a boolean true value
(e.g. `true` or `1`) |
Review Comment:
The markdown table syntax is incorrect: each row starts with `||` instead of
`|`, which will render poorly in most Markdown renderers. Change the
header/separator/rows to use single leading/trailing pipes (e.g., `|
Environment variable | Flag | Description |`).
##########
cmk.go:
##########
@@ -53,6 +54,34 @@ func main() {
flag.Parse()
args := flag.Args()
+ // Fall back to environment variables for flags not passed on the
+ // command line; CLI flags take precedence over environment variables.
+ if *configFilePath == "" {
+ *configFilePath = os.Getenv(config.ConfigFileEnvVar)
+ }
+ if *profile == "" {
+ *profile = os.Getenv(config.ProfileEnvVar)
+ }
+ if *acsURL == config.DefaultACSAPIEndpoint {
+ if value := os.Getenv(config.URLEnvVar); value != "" {
+ *acsURL = value
+ }
+ }
+ if *apiKey == "" {
+ *apiKey = os.Getenv(config.APIKeyEnvVar)
+ }
+ if *secretKey == "" {
+ *secretKey = os.Getenv(config.SecretKeyEnvVar)
+ }
+ if *outputFormat == "" {
+ *outputFormat = os.Getenv(config.OutputEnvVar)
+ }
+ if !*debug {
+ if value, err :=
strconv.ParseBool(strings.TrimSpace(os.Getenv(config.DebugEnvVar))); err == nil
&& value {
+ *debug = true
+ }
+ }
Review Comment:
This precedence logic can incorrectly let env vars override an
explicitly-provided flag when the flag value equals the default. For example,
`-u http://localhost:8080/client/api` (explicit) still satisfies `*acsURL ==
DefaultACSAPIEndpoint`, so `CMK_URL` would override it. Similarly, `-d=false`
is explicitly set but `CMK_DEBUG=true` would flip it back on. To preserve
“flags take precedence”, track whether each flag was explicitly set (e.g., via
a custom `flag.Value` that records `Set` calls, or by scanning `os.Args` for
the relevant flag tokens) and only apply env fallbacks when the flag was not
set.
##########
cmk.go:
##########
@@ -53,6 +54,34 @@ func main() {
flag.Parse()
args := flag.Args()
+ // Fall back to environment variables for flags not passed on the
+ // command line; CLI flags take precedence over environment variables.
+ if *configFilePath == "" {
+ *configFilePath = os.Getenv(config.ConfigFileEnvVar)
+ }
+ if *profile == "" {
+ *profile = os.Getenv(config.ProfileEnvVar)
+ }
+ if *acsURL == config.DefaultACSAPIEndpoint {
+ if value := os.Getenv(config.URLEnvVar); value != "" {
+ *acsURL = value
+ }
+ }
+ if *apiKey == "" {
+ *apiKey = os.Getenv(config.APIKeyEnvVar)
+ }
+ if *secretKey == "" {
+ *secretKey = os.Getenv(config.SecretKeyEnvVar)
+ }
+ if *outputFormat == "" {
+ *outputFormat = os.Getenv(config.OutputEnvVar)
+ }
+ if !*debug {
Review Comment:
This precedence logic can incorrectly let env vars override an
explicitly-provided flag when the flag value equals the default. For example,
`-u http://localhost:8080/client/api` (explicit) still satisfies `*acsURL ==
DefaultACSAPIEndpoint`, so `CMK_URL` would override it. Similarly, `-d=false`
is explicitly set but `CMK_DEBUG=true` would flip it back on. To preserve
“flags take precedence”, track whether each flag was explicitly set (e.g., via
a custom `flag.Value` that records `Set` calls, or by scanning `os.Args` for
the relevant flag tokens) and only apply env fallbacks when the flag was not
set.
##########
cmd/command.go:
##########
@@ -65,13 +65,15 @@ CloudMonkey (cmk) 🐵 is a command line interface for Apache
CloudStack.
Allowed flags:
-h Show this help message or API doc when specified after an API
-v Print version
- -o API response output format: json, text, table, column, csv
- -p Server profile
- -d Enable debug mode
- -c Different config file path
- -u CloudStack's API endpoint URL
- -s CloudStack user's secret Key
- -k CloudStack user's API Key
+ -o API response output format: json, text, table, column, csv (env:
CMK_OUTPUT)
+ -p Server profile (env: CMK_PROFILE)
+ -d Enable debug mode (env: CMK_DEBUG)
+ -c Different config file path (env: CMK_CONFIG)
+ -u CloudStack's API endpoint URL (env: CMK_URL)
+ -s CloudStack user's secret Key (env: CMK_SECRET_KEY)
+ -k CloudStack user's API Key (env: CMK_API_KEY)
Review Comment:
Use consistent capitalization: change “secret Key” → “secret key” and “API
Key” → “API key” to match typical wording and the README.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
