feat: add autogroup:self #2789
feat: add autogroup:self #2789
Conversation
07942bc to
b007db8
Compare
80fba7a to
2235b67
Compare
nblock
left a comment
There was a problem hiding this comment.
Thank you very much for tackling autogroup:self and also documenting it!
Left a few minor comments and suggestions.
docs/ref/acls.md
Outdated
|
|
||
| Includes devices where the same user is authenticated on both the source and destination. Does not include tagged devices. Can only be used in ACL destinations. | ||
|
|
||
| Available as of [#2789](https://github.com/juanfont/headscale/pull/2789). |
There was a problem hiding this comment.
removed the line completely, not really needed there.
| // invalidateAutogroupSelfCache intelligently clears only the cache entries that need to be | ||
| // invalidated when using autogroup:self policies. This is much more efficient than clearing | ||
| // the entire cache. | ||
| func (pm *PolicyManager) invalidateAutogroupSelfCache(oldNodes, newNodes views.Slice[types.NodeView]) { |
There was a problem hiding this comment.
Not sure how easy, but would be nice to have some tests for this function
There was a problem hiding this comment.
Shouldn't be that hard, i'll write a test for this functionality.
There was a problem hiding this comment.
There was a problem hiding this comment.
One comment, otherwise I'm happy with letting this in to let it be tested.
I think we should write up in the changelog that this is an experiment and that you as a user needs to be careful and help test it and that they must understand that a bug might give more access than what is expected.
And a note about that it is quite computational expensive.
2235b67 to
a49a882
Compare
kradalby
left a comment
There was a problem hiding this comment.
Let's take it for a spin!
|
I've been looking forward to this one, thank you for making it happen! |
Same here @benley 😆 |
|
Hip, hip hooray. Thanks all! |
|
@vdovhanych thanks for implementing this! ❤️ |
This adds the autogroup:self feature and should close #2618
While the feature should now work, I still feel it needs a bit more work to be "production" ready. I am concerned about the headscale's performance when this autogroup is used in the ACL.
Update1:
I added and updated the documentation for the ACL rules, listing supported autogroups and providing some examples of usage.
Update2:
I was trying out the build on some test environments when I realized that SSH would not work with the
autogroup:selfand that it also needed special handling for filtering the nodes. I tried to address it with ddf4cc6Update3:
changed some stuff again for the SSH rules to work with autogroup:self (this was done after I tried to evaluate adding the feature with the AI agent, this was a suggestion, but it would not work without it) 80fba7a