policy/v2: tolerate tag-owner cycles by resolving to empty

SaaS accepts tagOwners with circular references (tag:a -> tag:b ->
tag:a, tag:a -> tag:a) by dropping the cycle edge instead of failing
the policy. Each tag whose only path back is via the cycle resolves
to the empty owner set; a sibling non-tag owner survives. headscale
previously rejected the policy with ErrCircularReference at parse
time. Drop the sentinel, change flattenTags to return an empty owner
list on revisit, and update flattenTagOwners tests to capture the
new behaviour including the sibling-survives edge case.
This commit is contained in:
Kristoffer Dalby
2026-05-13 09:57:54 +00:00
parent 9f362d5be9
commit aea64b34de
4 changed files with 58 additions and 38 deletions
+10 -19
View File
@@ -1478,27 +1478,18 @@ func (pm *PolicyManager) invalidateGlobalPolicyCache(newNodes views.Slice[types.
}
}
// flattenTags flattens the TagOwners by resolving nested tags and detecting cycles.
// It will return a Owners list where all the Tag types have been resolved to their underlying Owners.
// flattenTags flattens the TagOwners by resolving nested tags. Cycles
// in the ownership graph (tag:a -> tag:b -> tag:a, or tag:a -> tag:a)
// are tolerated to match SaaS: the cycle-causing edge is dropped, the
// remaining owners propagate, and the cycle itself contributes no
// addresses. Non-cycle owners on the cycled tags still resolve.
// Undefined-tag references remain a hard error.
func flattenTags(tagOwners TagOwners, tag Tag, visiting map[Tag]bool, chain []Tag) (Owners, error) {
if visiting[tag] {
cycleStart := 0
for i, t := range chain {
if t == tag {
cycleStart = i
break
}
}
cycleTags := make([]string, len(chain[cycleStart:]))
for i, t := range chain[cycleStart:] {
cycleTags[i] = string(t)
}
slices.Sort(cycleTags)
return nil, fmt.Errorf("%w: %s", ErrCircularReference, strings.Join(cycleTags, " -> "))
// Cycle: this tag is already on the resolution stack. SaaS
// drops the edge instead of failing, so we return an empty
// owner set and let the caller continue with any siblings.
return nil, nil
}
visiting[tag] = true
@@ -87,18 +87,6 @@ var sshSkipReasons = map[string]string{
// equivalent for the user:*@passkey wildcard pattern.
"ssh-b5": "user:*@passkey wildcard not supported in headscale",
"ssh-d10": "user:*@passkey wildcard not supported in headscale",
// TAG_OWNER_CYCLE_TOLERATED (2 tests)
//
// SaaS accepts tagOwners with circular references (tag:a -> tag:b,
// tag:b -> tag:a or tag:a -> tag:a) and resolves them as the empty
// owner set. headscale's TagOwnerSet resolver eagerly detects
// circular tag references and rejects the policy at parse time.
// Pick a side and adjust: either match SaaS by treating cycles as
// "no owners" instead of an error, or document that headscale is
// intentionally stricter and remove these scenarios.
"ssh-tag-owner-cycle": "headscale rejects tagOwner cycles that SaaS accepts as empty owner set",
"ssh-tag-owner-self-reference": "headscale rejects self-referencing tagOwner that SaaS accepts as empty owner set",
}
// sshRejectSkipReasons documents APIResponseCode != 200 captures where
-2
View File
@@ -34,8 +34,6 @@ const Wildcard = Asterix(0)
var ErrAutogroupSelfRequiresPerNodeResolution = errors.New("autogroup:self requires per-node resolution and cannot be resolved in this context")
var ErrCircularReference = errors.New("circular reference detected")
var ErrUndefinedTagReference = errors.New("references undefined tag")
// SSH validation errors.
+48 -5
View File
@@ -4153,13 +4153,45 @@ func TestFlattenTagOwners(t *testing.T) {
wantErr: "",
},
{
name: "circular-reference",
// SaaS tolerates tag:a <-> tag:b cycles by dropping the
// cycle edge; both tags resolve to an empty owner set
// because neither chain reaches a non-tag owner.
name: "circular-reference-resolves-to-empty",
input: TagOwners{
Tag("tag:a"): Owners{new(Tag("tag:b"))},
Tag("tag:b"): Owners{new(Tag("tag:a"))},
},
want: nil,
wantErr: "circular reference detected: tag:a -> tag:b",
want: TagOwners{
Tag("tag:a"): nil,
Tag("tag:b"): nil,
},
wantErr: "",
},
{
// tag:a -> tag:a self-reference: the only owner is the
// cycle edge itself; result is empty.
name: "self-reference-resolves-to-empty",
input: TagOwners{
Tag("tag:a"): Owners{new(Tag("tag:a"))},
},
want: TagOwners{
Tag("tag:a"): nil,
},
wantErr: "",
},
{
// Cycle plus a sibling non-tag owner: the cycle edge
// drops out, the sibling owner survives.
name: "cycle-plus-sibling-keeps-sibling",
input: TagOwners{
Tag("tag:a"): Owners{new(Tag("tag:b")), new(Username("alice@example.com"))},
Tag("tag:b"): Owners{new(Tag("tag:a"))},
},
want: TagOwners{
Tag("tag:a"): Owners{new(Username("alice@example.com"))},
Tag("tag:b"): Owners{new(Username("alice@example.com"))},
},
wantErr: "",
},
{
name: "mixed-owners",
@@ -4218,7 +4250,9 @@ func TestFlattenTagOwners(t *testing.T) {
wantErr: "",
},
{
name: "tag-long-circular-chain",
// Long cycle: every tag eventually points back to itself.
// Each tag resolves to the empty owner set.
name: "tag-long-circular-chain-resolves-to-empty",
input: TagOwners{
Tag("tag:a"): Owners{new(Tag("tag:g"))},
Tag("tag:b"): Owners{new(Tag("tag:a"))},
@@ -4228,7 +4262,16 @@ func TestFlattenTagOwners(t *testing.T) {
Tag("tag:f"): Owners{new(Tag("tag:e"))},
Tag("tag:g"): Owners{new(Tag("tag:f"))},
},
wantErr: "circular reference detected: tag:a -> tag:b -> tag:c -> tag:d -> tag:e -> tag:f -> tag:g",
want: TagOwners{
Tag("tag:a"): nil,
Tag("tag:b"): nil,
Tag("tag:c"): nil,
Tag("tag:d"): nil,
Tag("tag:e"): nil,
Tag("tag:f"): nil,
Tag("tag:g"): nil,
},
wantErr: "",
},
{
name: "undefined-tag-reference",