Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow ACLs to be reloaded with SIGHUP #601

Merged
merged 11 commits into from
Jun 3, 2022
Merged

Allow ACLs to be reloaded with SIGHUP #601

merged 11 commits into from
Jun 3, 2022

Conversation

kradalby
Copy link
Collaborator

@kradalby kradalby commented May 31, 2022 •
edited
Loading

This PR partially resolves #173, allowing the users to send a reload signal (SIGHUP) to headscale to reload the ACL policies.

In addition, sending a SIGTERM, SIGINT etc, will now attempt to shutdown headscale gracefully.

  • read the CONTRIBUTING guidelines
  • raised a GitHub issue or discussed it on the projects chat beforehand
  • added unit tests
  • added integration tests
  • updated documentation if needed
  • updated CHANGELOG.md

kradalby added 9 commits May 31, 2022 14:28
@kradalby
Improve signal handling
0b4f59b 
This commit starts to wire up better signal handling, it starts with
handling shutdown a bit better, using the graceful shutdown for all the
listeners we use.

It also adds the initial switch case for handling config and acl reload,
which is to be implemented.
@kradalby
Fix logtail config function name
3e078f0
@kradalby
Make config get function global
5bfae22
@kradalby
Make ACL policy part of the config struct
24e4787
@kradalby
Use config object instead of viper for policy path
6b1482d
@kradalby
Rename abspath function to describe what it does
0612927
@kradalby
Move Abspath function to headscale utils
36dca35
@kradalby
Support reloading ACLs with SIGHUP
2feed18 
Also continously listen for signals, not just once.
@kradalby
Update changelog
6f32b80
@kradalby kradalby marked this pull request as ready for review May 31, 2022 12:32
@kradalby kradalby requested a review from juanfont as a code owner May 31, 2022 12:32
@kradalby kradalby mentioned this pull request May 31, 2022
Closed
@kradalby kradalby merged commit 0797148 into juanfont:main Jun 3, 2022
@kradalby kradalby deleted the signals-reload-acl branch June 3, 2022 08:48
@GoodiesHQ
Copy link

Given that this is the case, is there any possible issue with setting a cronjob for running docker kill --signal HUP headscale every 1-5 minutes or so? I have tested with running it manually and it seems to be very quick (granted, I only have a very small ACL file, so maybe the time taken scales with complexity). Using the Headscale-UI program to assign tags to nodes is great, but it doesn't seem to take effect until it is reloaded. Since there is currently no API support for ACLs, you can't add users to groups with it, either.

@kradalby
Copy link
Collaborator Author

@GoodiesHQ I would assume that is fine, how performant/problematic is hard to predict, but there shouldnt be anything wrong with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juanfont juanfont juanfont approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add reload and test config commands
3 participants
@kradalby @GoodiesHQ @juanfont