-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
make acl_policy_path fatal if policy.path is not set #2041
Conversation
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.
just a tiny change to config comments
e7e8943
to
8cc239e
Compare
8cc239e
to
b6a3012
Compare
WalkthroughThe recent changes focus on enhancing configuration management within the codebase. Key modifications include correcting documentation typos, enforcing stricter checks for deprecated configuration keys, and introducing new test cases for policy path validation. Additionally, several environment variable keys have been renamed for clarity, reflecting a broader restructuring in how policies are managed. These updates aim to improve system reliability and documentation accuracy, ensuring better user experience and maintenance. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
The alias manager in Cobra has some sharp edges and it seem to not work as we expect, causing it to have some silent failures with both config files and ENV. This commits makes the new policy.path variable mandatory and fixes the issue where the config file would not load. Signed-off-by: Kristoffer Dalby <[email protected]>
b6a3012
to
6a4546e
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- config-example.yaml (1 hunks)
- hscontrol/types/config.go (1 hunks)
- hscontrol/types/config_test.go (1 hunks)
- hscontrol/types/testdata/policy-path-is-loaded.yaml (1 hunks)
- integration/hsic/config.go (1 hunks)
- integration/hsic/hsic.go (1 hunks)
Files skipped from review due to trivial changes (2)
- config-example.yaml
- integration/hsic/hsic.go
Additional comments not posted (4)
hscontrol/types/testdata/policy-path-is-loaded.yaml (1)
1-18
: Configuration file looks good.This new YAML file provides a clear and consistent configuration template for testing purposes, aligning with the PR's objectives to manage policy configurations effectively.
integration/hsic/config.go (1)
16-16
: Verify the usage of the renamed configuration key.The key
"HEADSCALE_ACL_POLICY_PATH"
has been renamed to"HEADSCALE_POLICY_PATH"
. Ensure that all references to the old key have been updated throughout the codebase to maintain consistency.Verification successful
Renaming of Configuration Key Verified Successfully
The old key
HEADSCALE_ACL_POLICY_PATH
has been removed, and the new keyHEADSCALE_POLICY_PATH
is consistently used in the codebase. The references inintegration/hsic/config.go
andintegration/hsic/hsic.go
confirm the successful update.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the renamed configuration key have been updated. # Test: Search for the old key. Expect: No occurrences of the old key. rg --type go $'HEADSCALE_ACL_POLICY_PATH'Length of output: 40
Script:
#!/bin/bash # Description: Verify the usage of the new configuration key. # Test: Search for the new key. Expect: Occurrences of the new key. rg --type go $'HEADSCALE_POLICY_PATH'Length of output: 193
hscontrol/types/config_test.go (1)
164-182
: New test case is well-implemented.The "policy-path-is-loaded" test case effectively verifies the loading of policy configurations, enhancing the test coverage for configuration management.
hscontrol/types/config.go (1)
295-295
: Stricter configuration management is a positive change.The shift from warnings to fatal errors for deprecated configuration keys enforces stricter adherence to updated configurations, enhancing reliability.
Ensure that this change is communicated to users to prevent unexpected failures due to deprecated configurations.
Beta2 is working fine |
this commit changes and streamlines the dns_config into a new
key, dns. It removes a combination of outdates and incompatible
configuration options that made it easy to confuse what headscale
could and could not do, or what to expect from ones configuration.
It has to go in after #2034 with a rebase as it contains that commit because of
some helpers used there.
Fixes #2024
Signed-off-by: Kristoffer Dalby [email protected]
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores