fix: proper construction of SA groups for tier lookup

[bartoszmajsak]

#110 brought SA group construction so that they are not explicitly required in the configmap.

This PR fixes a stupid bug that previous PR brought with better test coverage.

Summary by CodeRabbit

• Bug Fixes

  • Corrected handling of Kubernetes ServiceAccount group names to use the proper plural prefix, ensuring accurate tier/group mapping and expected permissions for service-account-based access.

• Tests

  • Expanded test coverage for service account group scenarios, including premium tiers and mixed group memberships, to validate correct precedence and behavior.

[coderabbitai]

Walkthrough

Updated the service account group prefix from "system:serviceaccount:" to "system:serviceaccounts:" in tier mapping logic. Adjusted and expanded tests to reflect the new group string, including scenarios for premium tier and mixed groups. No control flow or API changes.

Changes

Cohort / File(s)	Summary
Tier mapping logic maas-api/internal/tier/mapper.go	Change constructed Kubernetes service account group from singular to plural prefix: system:serviceaccounts:...
Tier mapping tests maas-api/internal/tier/mapper_test.go	Update expected SA group strings to plural form; add premium tier SA test; add mixed groups test ensuring premium selection logic holds.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my nose at plural names,
Hop from singular to broader frames.
Now serviceaccounts lead the way,
Premium carrots win the day. 🥕
Tests burrow deep, confirm the trails—
A tidy hop where nothing fails.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly and concisely describes the primary change of fixing the construction of service account groups for tier lookup, directly matching the key modification in the code and test updates.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eeabb25 and 1405fdf.

📒 Files selected for processing (2)

• maas-api/internal/tier/mapper.go (1 hunks)
• maas-api/internal/tier/mapper_test.go (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: build

🔇 Additional comments (3)

maas-api/internal/tier/mapper_test.go (2)

44-44: LGTM!

The test case correctly updated to use the plural system:serviceaccounts: prefix, consistent with the fix in mapper.go.

---

48-53: LGTM! Good test coverage addition.

This new test case verifies that the inferred service account group for the premium tier works correctly with the updated plural prefix.

maas-api/internal/tier/mapper.go (1)

112-112: Correct service account group prefix The change to system:serviceaccounts:<namespace> aligns with Kubernetes RBAC conventions for service account groups.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Update the test description to match the test logic.

The test case correctly verifies that premium tier wins among mixed groups including service accounts. However, the description still mentions "developer" tier, which doesn't match the test logic.

Apply this diff to fix the description:

 		{
 			name:         "multiple groups - service account groups",
 			groups:       []string{"system:serviceaccounts", "system:serviceaccounts:test-tenant-tier-premium", "system:authenticated"},
 			expectedTier: "premium",
-			description:  "User belongs to both premium and developer - developer has higher level (15 > 10)",
+			description:  "Service account with premium namespace group should resolve to premium tier",
 		},

🤖 Prompt for AI Agents

In maas-api/internal/tier/mapper_test.go around lines 96 to 101, the test case
description incorrectly mentions "developer" even though the test data checks
that the "premium" tier wins among mixed service account groups; update the
description string to accurately state that premium wins (e.g., "User belongs to
multiple groups including premium - premium has higher level and should win") so
the description matches the test logic.