Skip to content

Commit 2b3ee90

Browse files
vdovhanychvdovhanych
committed
fixup! feat: add autogroup:member, autogroup:tagged
1 parent 7d07148 commit 2b3ee90

File tree

3 files changed

+109
-48
lines changed

3 files changed

+109
-48
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ working in v1 and not tested might be broken in v2 (and vice versa).
123123
- Add documentation for routes
124124
[#2496](https://github.com/juanfont/headscale/pull/2496)
125125
- Add support for `autogroup:member`, `autogroup:tagged`
126+
[#2572](https://github.com/juanfont/headscale/pull/2572)
126127

127128
## 0.25.1 (2025-02-25)
128129

‎hscontrol/policy/v2/types.go

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -382,10 +382,10 @@ func (p Prefix) Resolve(_ *Policy, _ types.Users, nodes types.Nodes) (*netipx.IP
382382
type AutoGroup string
383383

384384
const (
385-
AutoGroupMember AutoGroup = "autogroup:member"
386385
AutoGroupInternet AutoGroup = "autogroup:internet"
387-
AutoGroupTagged AutoGroup = "autogroup:tagged"
386+
AutoGroupMember AutoGroup = "autogroup:member"
388387
AutoGroupNonRoot AutoGroup = "autogroup:nonroot"
388+
AutoGroupTagged AutoGroup = "autogroup:tagged"
389389

390390
// These are not yet implemented.
391391
AutoGroupSelf AutoGroup = "autogroup:self"
@@ -394,8 +394,8 @@ const (
394394
var autogroups = []AutoGroup{
395395
AutoGroupInternet,
396396
AutoGroupMember,
397-
AutoGroupTagged,
398397
AutoGroupNonRoot,
398+
AutoGroupTagged,
399399
}
400400

401401
func (ag AutoGroup) Validate() error {
@@ -423,27 +423,59 @@ func (ag AutoGroup) Resolve(p *Policy, users types.Users, nodes types.Nodes) (*n
423423

424424
case AutoGroupMember:
425425
// autogroup:member represents all untagged devices in the tailnet.
426+
tagMap, err := resolveTagOwners(p, users, nodes)
427+
if err != nil {
428+
return nil, err
429+
}
430+
426431
for _, node := range nodes {
432+
// Skip if node has forced tags
427433
if len(node.ForcedTags) != 0 {
428434
continue
429435
}
436+
437+
// Skip if node has any allowed requested tags
438+
hasAllowedTag := false
430439
if node.Hostinfo != nil && len(node.Hostinfo.RequestTags) != 0 {
440+
for _, tag := range node.Hostinfo.RequestTags {
441+
if tagips, ok := tagMap[Tag(tag)]; ok && node.InIPSet(tagips) {
442+
hasAllowedTag = true
443+
break
444+
}
445+
}
446+
}
447+
if hasAllowedTag {
431448
continue
432449
}
450+
451+
// Node is a member if it has no forced tags and no allowed requested tags
433452
node.AppendToIPSet(&build)
434453
}
435454

436455
return build.IPSet()
437456

438457
case AutoGroupTagged:
439458
// autogroup:tagged represents all devices with a tag in the tailnet.
459+
tagMap, err := resolveTagOwners(p, users, nodes)
460+
if err != nil {
461+
return nil, err
462+
}
463+
440464
for _, node := range nodes {
465+
// Include if node has forced tags
441466
if len(node.ForcedTags) != 0 {
442467
node.AppendToIPSet(&build)
443468
continue
444469
}
470+
471+
// Include if node has any allowed requested tags
445472
if node.Hostinfo != nil && len(node.Hostinfo.RequestTags) != 0 {
446-
node.AppendToIPSet(&build)
473+
for _, tag := range node.Hostinfo.RequestTags {
474+
if _, ok := tagMap[Tag(tag)]; ok {
475+
node.AppendToIPSet(&build)
476+
break
477+
}
478+
}
447479
}
448480
}
449481

@@ -958,6 +990,7 @@ type Policy struct {
958990
}
959991

960992
var (
993+
// TODO(kradalby): Add these checks for tagOwners and autoApprovers
961994
autogroupForSrc = []AutoGroup{AutoGroupMember, AutoGroupTagged}
962995
autogroupForDst = []AutoGroup{AutoGroupInternet, AutoGroupMember, AutoGroupTagged}
963996
autogroupForSSHSrc = []AutoGroup{AutoGroupMember, AutoGroupTagged}
@@ -1280,19 +1313,3 @@ func unmarshalPolicy(b []byte) (*Policy, error) {
12801313
const (
12811314
expectedTokenItems = 2
12821315
)
1283-
1284-
var allIPSet *netipx.IPSet
1285-
1286-
func allIPs() *netipx.IPSet {
1287-
if allIPSet != nil {
1288-
return allIPSet
1289-
}
1290-
1291-
var build netipx.IPSetBuilder
1292-
build.AddPrefix(netip.MustParsePrefix("::/0"))
1293-
build.AddPrefix(netip.MustParsePrefix("0.0.0.0/0"))
1294-
1295-
allTheIps, _ := build.IPSet()
1296-
1297-
return allTheIps
1298-
}

‎hscontrol/policy/v2/types_test.go

Lines changed: 71 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ func TestUnmarshalPolicy(t *testing.T) {
359359
],
360360
}
361361
`,
362-
wantErr: `AutoGroup is invalid, got: "autogroup:invalid", must be one of [autogroup:internet autogroup:member autogroup:tagged autogroup:nonroot]`,
362+
wantErr: `AutoGroup is invalid, got: "autogroup:invalid", must be one of [autogroup:internet autogroup:member autogroup:nonroot autogroup:tagged]`,
363363
},
364364
{
365365
name: "undefined-hostname-errors-2490",
@@ -766,85 +766,128 @@ func TestResolvePolicy(t *testing.T) {
766766
want: []netip.Prefix{tsaddr.AllIPv4(), tsaddr.AllIPv6()},
767767
},
768768
{
769-
name: "autogroup-member-basic",
769+
name: "autogroup-member-comprehensive",
770770
toResolve: ptr.To(AutoGroup(AutoGroupMember)),
771771
nodes: types.Nodes{
772+
// Node with no tags (should be included)
772773
{
773774
User: users["testuser"],
774775
IPv4: ap("100.100.101.1"),
775776
},
777+
// Node with forced tags (should be excluded)
776778
{
777779
User: users["testuser"],
778780
ForcedTags: []string{"tag:test"},
779781
IPv4: ap("100.100.101.2"),
780782
},
783+
// Node with allowed requested tag (should be excluded)
781784
{
782785
User: users["testuser"],
783786
Hostinfo: &tailcfg.Hostinfo{
784787
RequestTags: []string{"tag:test"},
785788
},
786789
IPv4: ap("100.100.101.3"),
787790
},
791+
// Node with non-allowed requested tag (should be included)
788792
{
789-
User: users["notme"],
793+
User: users["testuser"],
794+
Hostinfo: &tailcfg.Hostinfo{
795+
RequestTags: []string{"tag:notallowed"},
796+
},
790797
IPv4: ap("100.100.101.4"),
791798
},
792-
},
793-
want: []netip.Prefix{mp("100.100.101.1/32"), mp("100.100.101.4/32")},
794-
},
795-
{
796-
name: "autogroup-member-multiple-users",
797-
toResolve: ptr.To(AutoGroup(AutoGroupMember)),
798-
nodes: types.Nodes{
799-
{
800-
User: users["user1"],
801-
IPv4: ap("100.100.101.1"),
802-
},
803-
{
804-
User: users["user2"],
805-
IPv4: ap("100.100.101.2"),
806-
},
799+
// Node with multiple requested tags, one allowed (should be excluded)
807800
{
808-
User: users["user3"],
809-
ForcedTags: []string{"tag:test"},
810-
IPv4: ap("100.100.101.3"),
801+
User: users["testuser"],
802+
Hostinfo: &tailcfg.Hostinfo{
803+
RequestTags: []string{"tag:test", "tag:notallowed"},
804+
},
805+
IPv4: ap("100.100.101.5"),
811806
},
807+
// Node with multiple requested tags, none allowed (should be included)
812808
{
813-
User: users["user4"],
809+
User: users["testuser"],
814810
Hostinfo: &tailcfg.Hostinfo{
815-
RequestTags: []string{"tag:test"},
811+
RequestTags: []string{"tag:notallowed1", "tag:notallowed2"},
816812
},
817-
IPv4: ap("100.100.101.4"),
813+
IPv4: ap("100.100.101.6"),
814+
},
815+
},
816+
pol: &Policy{
817+
TagOwners: TagOwners{
818+
Tag("tag:test"): Owners{ptr.To(Username("testuser@"))},
818819
},
819820
},
820-
want: []netip.Prefix{mp("100.100.101.1/32"), mp("100.100.101.2/32")},
821+
want: []netip.Prefix{
822+
mp("100.100.101.1/32"), // No tags
823+
mp("100.100.101.4/32"), // Non-allowed requested tag
824+
mp("100.100.101.6/32"), // Multiple non-allowed requested tags
825+
},
821826
},
822827
{
823828
name: "autogroup-tagged",
824829
toResolve: ptr.To(AutoGroup(AutoGroupTagged)),
825830
nodes: types.Nodes{
831+
// Node with no tags (should be excluded)
826832
{
827833
User: users["testuser"],
828834
IPv4: ap("100.100.101.1"),
829835
},
836+
// Node with forced tag (should be included)
830837
{
831838
User: users["testuser"],
832839
ForcedTags: []string{"tag:test"},
833840
IPv4: ap("100.100.101.2"),
834841
},
842+
// Node with allowed requested tag (should be included)
835843
{
836844
User: users["testuser"],
837845
Hostinfo: &tailcfg.Hostinfo{
838846
RequestTags: []string{"tag:test"},
839847
},
840848
IPv4: ap("100.100.101.3"),
841849
},
850+
// Node with non-allowed requested tag (should be excluded)
842851
{
843-
User: users["notme"],
852+
User: users["testuser"],
853+
Hostinfo: &tailcfg.Hostinfo{
854+
RequestTags: []string{"tag:notallowed"},
855+
},
844856
IPv4: ap("100.100.101.4"),
845857
},
858+
// Node with multiple requested tags, one allowed (should be included)
859+
{
860+
User: users["testuser"],
861+
Hostinfo: &tailcfg.Hostinfo{
862+
RequestTags: []string{"tag:test", "tag:notallowed"},
863+
},
864+
IPv4: ap("100.100.101.5"),
865+
},
866+
// Node with multiple requested tags, none allowed (should be excluded)
867+
{
868+
User: users["testuser"],
869+
Hostinfo: &tailcfg.Hostinfo{
870+
RequestTags: []string{"tag:notallowed1", "tag:notallowed2"},
871+
},
872+
IPv4: ap("100.100.101.6"),
873+
},
874+
// Node with multiple forced tags (should be included)
875+
{
876+
User: users["testuser"],
877+
ForcedTags: []string{"tag:test", "tag:other"},
878+
IPv4: ap("100.100.101.7"),
879+
},
880+
},
881+
pol: &Policy{
882+
TagOwners: TagOwners{
883+
Tag("tag:test"): Owners{ptr.To(Username("testuser@"))},
884+
},
885+
},
886+
want: []netip.Prefix{
887+
mp("100.100.101.2/31"), // Forced tag and allowed requested tag consecutive IPs are put in 31 prefix
888+
mp("100.100.101.5/32"), // Multiple requested tags, one allowed
889+
mp("100.100.101.7/32"), // Multiple forced tags
846890
},
847-
want: []netip.Prefix{mp("100.100.101.2/31")},
848891
},
849892
{
850893
name: "autogroup-invalid",
@@ -1013,7 +1056,7 @@ func TestResolveAutoApprovers(t *testing.T) {
10131056
name: "mixed-routes-and-exit-nodes",
10141057
policy: &Policy{
10151058
Groups: Groups{
1016-
"group:testgroup": Usernames{"user1", "user2"},
1059+
"group:testgroup": Usernames{"user1@", "user2@"},
10171060
},
10181061
AutoApprovers: AutoApproverPolicy{
10191062
Routes: map[netip.Prefix]AutoApprovers{

0 commit comments

Comments
 (0)