headscale icon indicating copy to clipboard operation
headscale copied to clipboard

Published 20 hours ago • verified publisher icon juanfont

Add support for PreAuthKey tags/automatic tagging

Open tsujamin opened this issue 2 years ago • 6 comments

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

These changes add the ability to automatically tag nodes when activated with a preauthkey as is available in SaaS

CreatePreAuthKey and like APIs were expanded to optionally accept a list of tags and the CLI arguments were updated. The database schema was also updated with a new table to store tags associated with preauth keys.

Keys associated to a preauthkey are applied to a new or existing Machine's ForcedKeys when registered/refreshed

tsujamin avatar Aug 25 '22 21:08 tsujamin

just to note that there's a small test break I'll need to push a change to depending on which order this and #763 get merged (if either is accepted

tsujamin avatar Sep 03 '22 23:09 tsujamin

fixed broken test from merge

tsujamin avatar Sep 06 '22 10:09 tsujamin

np will have a crack tomorrow - just noticed my local linting is broken too so will address both

tsujamin avatar Sep 07 '22 10:09 tsujamin

np will have a crack tomorrow - just noticed my local linting is broken too so will address both

Thanks ! Maybe you'll want to wait for an owner review first :smile:

restanrm avatar Sep 07 '22 10:09 restanrm

np will have a crack tomorrow - just noticed my local linting is broken too so will address both

Thanks ! Maybe you'll want to wait for an owner review first smile

Sorry this comment was for another PR. No need for approval to add tests they are always welcome :smile:

restanrm avatar Sep 07 '22 11:09 restanrm

haha all good, fwiw just about to push a fix for the linting issues but not the unit test as the point where tags is applied is in protocol_common.go as part of machine registration/refresh as this most closely follows SaaS behaviour.

Let me know if an equivalent integration test is desired - just a bit of extra effort to stand up a new test case in there 🥲

tsujamin avatar Sep 07 '22 12:09 tsujamin

@kradalby thanks for the review - have merged in your recommendations

tsujamin avatar Sep 23 '22 08:09 tsujamin

Seems like there is a lint error, and could you add changelog entry for this one as well?

kradalby avatar Sep 23 '22 08:09 kradalby

fixed, sorry my vscode linter re-added that space

tsujamin avatar Sep 23 '22 08:09 tsujamin