-
Notifications
You must be signed in to change notification settings - Fork 318
CHEF-25540: Add config file support for HAB_AUTH_TOKEN and HAB_REFRESH_CHANNEL #9996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
👷 Deploy Preview for chef-habitat processing.
|
|
mwrock
left a comment
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.
We will also want to set the refresh channel via the config for the studio commands as well just like we do with origin, token and url.
We should add a prompt to hab setup for configuring the refresh channel. This is how the other config values can be initially set.
Hi @mwrock , |
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.
Pull request overview
This PR implements support for configuring HAB_AUTH_TOKEN and HAB_REFRESH_CHANNEL in the CLI configuration file (~/.hab/etc/cli.toml) as fallbacks when environment variables are not set, maintaining proper precedence: CLI arguments > environment variables > config file > defaults.
Key Changes:
- Added
refresh_channelfield toCliConfigstruct with TOML serialization support - Implemented utility functions for configuration precedence handling
- Updated error messages to reference the config file option
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
components/hab/src/lib.rs |
Added REFRESH_CHANNEL_ENVVAR constant |
components/hab/src/command/studio/enter.rs |
Integrated refresh channel config with studio environment setup |
components/hab/src/command/cli/setup.rs |
Added interactive setup for default refresh channel configuration |
components/hab/src/cli_v4/utils.rs |
Implemented precedence handling utilities and comprehensive unit tests |
components/hab/src/cli_v4/pkg/build.rs |
Updated build command to use config file fallback with proper precedence |
components/docs-chef-io/content/habitat/environment_variables.md |
Updated documentation to reference config file options |
components/common/src/cli_config.rs |
Added refresh_channel field and unit tests for serialization |
components/docs-chef-io/content/habitat/environment_variables.md
Outdated
Show resolved
Hide resolved
…H_CHANNEL - Added refresh_channel field to CliConfig struct in cli_config.rs - Implemented utility functions for config fallback with proper precedence - Updated hab pkg build to use config file when env vars not set - Added comprehensive unit tests for new functionality - Updated error messages to mention config file options - Updated environment variables documentation with config file info - Maintains precedence: CLI args > env vars > config file > defaults Signed-off-by: dikshagupta1 <dikshagupta1303@gmail.com>
Signed-off-by: dikshagupta1 <dikshagupta1303@gmail.com>
Signed-off-by: dikshagupta1 <dikshagupta1303@gmail.com>
9980b27 to
4a99a97
Compare
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
components/docs-chef-io/content/habitat/environment_variables.md
Outdated
Show resolved
Hide resolved
components/hab/src/cli_v4/utils.rs
Outdated
| bldr_auth_token_from_args_env_or_load(opt).ok() | ||
| } | ||
|
|
||
| pub(crate) fn refresh_channel_from_args_env_or_load(opt: Option<String>) -> Result<String, Error> { |
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.
I'd have this name in in _or_config
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.
Do you mean to change it to bldr_auth_token_in _or_config??
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.
sorry I meant refresh_channel_from_args_env_or_config
| https://www.habitat.sh/docs/using-habitat/#using-channels")?; | ||
| if ask_default_refresh_channel(ui)? { | ||
| ui.br()?; | ||
| ui.para("Enter your default Habitat refresh channel (e.g., 'stable', 'unstable').")?; |
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.
Id replace unstable with base as an example
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.
addressed in 49c6608
| write_cli_config_refresh_channel(&refresh_channel)?; | ||
| } else { | ||
| ui.para("Alright, maybe another time. You can also set a `HAB_REFRESH_CHANNEL` \ | ||
| environment variable or use the `--channel` flag when using the cli.")?; |
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.
i think you mean --refresh-channel
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.
Oh yes, that was a mistake. Thankyou for noticing it.
addressed in 49c6608
| ui.heading("Habitat Refresh Channel")?; | ||
| ui.para("The Habitat refresh channel determines which channel to use when refreshing \ | ||
| packages during builds and other operations. Common values include 'stable' for \ | ||
| stable releases and 'unstable' for latest development builds. If you set a \ |
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.
base instead of unstable as a common example.
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.
addressed in 49c6608
Signed-off-by: dikshagupta1 <dikshagupta1303@gmail.com>
Signed-off-by: dikshagupta1 <dikshagupta1303@gmail.com>
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
| unsafe { env::remove_var("HAB_REFRESH_CHANNEL") }; | ||
|
|
||
| // Set env var | ||
| unsafe { env::set_var("HAB_REFRESH_CHANNEL", "staging") }; |
Copilot
AI
Dec 5, 2025
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.
The use of unsafe blocks for environment variable manipulation in tests is unnecessary. Rust's std::env::set_var and std::env::remove_var are safe functions and don't require unsafe blocks. Remove the unsafe wrappers around these function calls.
| assert_eq!(result, Some("staging".to_string())); | ||
|
|
||
| // Clean up | ||
| unsafe { env::remove_var("HAB_REFRESH_CHANNEL") }; |
Copilot
AI
Dec 5, 2025
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.
The use of unsafe blocks for environment variable manipulation in tests is unnecessary. Rust's std::env::set_var and std::env::remove_var are safe functions and don't require unsafe blocks. Remove the unsafe wrappers around these function calls.
| #[test] | ||
| fn test_refresh_channel_fallback_to_none() { | ||
| // Clean env var to ensure fallback behavior | ||
| unsafe { env::remove_var("HAB_REFRESH_CHANNEL") }; |
Copilot
AI
Dec 5, 2025
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.
The use of unsafe blocks for environment variable manipulation in tests is unnecessary. Rust's std::env::set_var and std::env::remove_var are safe functions and don't require unsafe blocks. Remove the unsafe wrappers around these function calls.
| unsafe { env::remove_var("HAB_REFRESH_CHANNEL") }; | ||
|
|
||
| // Set env var | ||
| unsafe { env::set_var("HAB_REFRESH_CHANNEL", "env_channel") }; |
Copilot
AI
Dec 5, 2025
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.
The use of unsafe blocks for environment variable manipulation in tests is unnecessary. Rust's std::env::set_var and std::env::remove_var are safe functions and don't require unsafe blocks. Remove the unsafe wrappers around these function calls.
| assert_eq!(result, Some("cli_channel".to_string())); | ||
|
|
||
| // Clean up | ||
| unsafe { env::remove_var("HAB_REFRESH_CHANNEL") }; |
Copilot
AI
Dec 5, 2025
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.
The use of unsafe blocks for environment variable manipulation in tests is unnecessary. Rust's std::env::set_var and std::env::remove_var are safe functions and don't require unsafe blocks. Remove the unsafe wrappers around these function calls.
| docker: bool, | ||
|
|
||
| /// Channel used to retrieve plan dependencies for Chef supported origins | ||
| /// Uses value from the `HAB_REFRESH_CHANNEL` env variable or cli.toml config file if not set |
Copilot
AI
Dec 5, 2025
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.
The documentation comment should accurately describe the full precedence chain. Update to mention CLI argument takes priority: 'Uses value from --refresh-channel CLI arg, HAB_REFRESH_CHANNEL env variable, cli.toml config file, or defaults to "base" if not set'.
| /// Uses value from the `HAB_REFRESH_CHANNEL` env variable or cli.toml config file if not set | |
| /// Uses value from the `--refresh-channel` CLI argument, `HAB_REFRESH_CHANNEL` environment variable, `cli.toml` config file, or defaults to `"base"` if not set |
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.
We definitely want to indicate that base is the default. Also we need to add a period(.) to the comment on line 71 because the --help text concatenates these two comments.
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.
Addressed in commit ee85dc2
| docker: bool, | ||
|
|
||
| /// Channel used to retrieve plan dependencies for Chef supported origins | ||
| /// Uses value from the `HAB_REFRESH_CHANNEL` env variable or cli.toml config file if not set |
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.
We definitely want to indicate that base is the default. Also we need to add a period(.) to the comment on line 71 because the --help text concatenates these two comments.
| ui.heading("Habitat Refresh Channel")?; | ||
| ui.para("The Habitat refresh channel determines which channel to use for core packages \ | ||
| during a build. Common values include 'stable' for stable releases and 'base' \ | ||
| for latest development builds. If you set a refresh channel here, it will be used \ |
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.
for latest development builds -> for latest suported packages.
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.
Addressed in commit ee85dc2
| ui.para("If you would like to save a default refresh channel for use by the Habitat \ | ||
| client, please enter your preferred channel. Otherwise, just enter No.")?; | ||
| ui.para("For more information on Habitat channels, please read the documentation at \ | ||
| https://www.habitat.sh/docs/using-habitat/#using-channels")?; |
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.
I think we should delete this line and the one above it because the link looks irrelevant. I know the v2 docs will have https://docs.chef.io/habitat/2.0-rc1/supported_packages/package_refresh_strategy/ but I don't think we want ot add an rc link here.
Also we should mention that base is the default.
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.
Addresses in commit ee85dc2
Signed-off-by: dikshagupta1 <dikshagupta1303@gmail.com>
|
|
This all looks good now. Just a couple CI rustfmt and sonar issues you need to address. |









Summary
This PR implements support for configuring HAB_AUTH_TOKEN and HAB_REFRESH_CHANNEL in the CLI configuration file (~/.hab/etc/cli.toml) as requested in CHEF-25540. Users can now set these values in the config file as fallbacks when environment variables are not set, while maintaining the proper precedence chain.
Changes Made
Configuration Precedence
The implementation maintains the correct precedence chain:
Testing
Example Configuration
Users can now add the following to ~/.hab/etc/cli.toml:
Jira Link
CHEF-25540