Skip to content

Commit 4ca3983

Browse files
refactor: fail early when no tier mapping configmap is found (#137)
Previously, missing tier mapping configmaps were silently defaulted to `free`, which could mislead users and hide configuration issues. By failing fast, cluster operators are immediately informed of the problem and can correct it, avoiding confusion and unintended behavior. The component now explicitly fails when the configmap is missing instead of falling back to the `free` tier. Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com>
1 parent e315444 commit 4ca3983

File tree

4 files changed

+31
-49
lines changed

4 files changed

+31
-49
lines changed

maas-api/internal/tier/handler_test.go

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ func TestHandler_PostTierLookup_BadRequest(t *testing.T) {
170170
}
171171
}
172172

173-
func TestHandler_PostTierLookup_ConfigMapMissing_ShouldDefaultEveryUserToFreeTier(t *testing.T) {
173+
func TestHandler_PostTierLookup_ConfigMapMissing_ShouldError(t *testing.T) {
174174
mapper := createTestMapper(false) // No ConfigMap
175175
router := fixtures.SetupTierTestRouter(mapper)
176176

@@ -182,16 +182,7 @@ func TestHandler_PostTierLookup_ConfigMapMissing_ShouldDefaultEveryUserToFreeTie
182182
req.Header.Set("Content-Type", "application/json")
183183
router.ServeHTTP(w, req)
184184

185-
if w.Code != http.StatusOK {
186-
t.Errorf("expected status %d, got %d", http.StatusOK, w.Code)
187-
}
188-
189-
var response tier.LookupResponse
190-
if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil {
191-
t.Fatalf("failed to unmarshal response: %v", err)
192-
}
193-
194-
if response.Tier != "free" {
195-
t.Errorf("expected tier 'free', got %s", response.Tier)
185+
if w.Code == http.StatusOK {
186+
t.Errorf("expected status %d, got %d", http.StatusServiceUnavailable, w.Code)
196187
}
197188
}

maas-api/internal/tier/mapper.go

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,6 @@ import (
1717
"k8s.io/apimachinery/pkg/api/errors"
1818
)
1919

20-
var defaultTier = Tier{
21-
Name: "free",
22-
Level: 0,
23-
Groups: []string{
24-
"system:authenticated",
25-
},
26-
}
27-
2820
// Mapper handles tier-to-group mapping lookups
2921
type Mapper struct {
3022
tenantName string
@@ -38,22 +30,19 @@ func NewMapper(clientset kubernetes.Interface, tenantName, namespace string) *Ma
3830
}
3931
}
4032

41-
func (m *Mapper) Namespaces(ctx context.Context) map[string]string {
33+
func (m *Mapper) Namespace(ctx context.Context, tier string) (string, error) {
4234
tiers, err := m.loadTierConfig(ctx)
4335
if err != nil {
44-
if errors.IsNotFound(err) {
45-
tiers = []Tier{defaultTier}
46-
}
36+
return "", err
4737
}
4838

49-
namespaces := make(map[string]string, len(tiers))
50-
5139
for i := range tiers {
52-
tier := &tiers[i]
53-
namespaces[tier.Name] = m.projectedNsName(tier)
40+
if tiers[i].Name == tier {
41+
return m.ProjectedNsName(&tiers[i]), nil
42+
}
5443
}
5544

56-
return namespaces
45+
return "", fmt.Errorf("tier %s not found", tier)
5746
}
5847

5948
// GetTierForGroups returns the highest level tier for a user with multiple group memberships.
@@ -68,8 +57,7 @@ func (m *Mapper) GetTierForGroups(ctx context.Context, groups ...string) (string
6857
tiers, err := m.loadTierConfig(ctx)
6958
if err != nil {
7059
if errors.IsNotFound(err) {
71-
log.Printf("tier mapping %s not found, defaulting to 'free' tier", constant.TierMappingConfigMap)
72-
return "free", nil
60+
return "", fmt.Errorf("tier mapping not found, provide configuration in %s", constant.TierMappingConfigMap)
7361
}
7462
log.Printf("Failed to load tier configuration from ConfigMap %s: %v", constant.TierMappingConfigMap, err)
7563
return "", fmt.Errorf("failed to load tier configuration: %w", err)
@@ -90,6 +78,15 @@ func (m *Mapper) GetTierForGroups(ctx context.Context, groups ...string) (string
9078
return "", &GroupNotFoundError{Group: fmt.Sprintf("groups [%s]", strings.Join(groups, ", "))}
9179
}
9280

81+
// ProjectedSAGroup returns the projected SA group for a tier.
82+
func (m *Mapper) ProjectedSAGroup(tier *Tier) string {
83+
return fmt.Sprintf("system:serviceaccounts:%s", m.ProjectedNsName(tier))
84+
}
85+
86+
func (m *Mapper) ProjectedNsName(tier *Tier) string {
87+
return fmt.Sprintf("%s-tier-%s", m.tenantName, tier.Name)
88+
}
89+
9390
func (m *Mapper) loadTierConfig(ctx context.Context) ([]Tier, error) {
9491
cm, err := m.configMapClient.Get(ctx, constant.TierMappingConfigMap, metav1.GetOptions{})
9592
if err != nil {
@@ -109,12 +106,8 @@ func (m *Mapper) loadTierConfig(ctx context.Context) ([]Tier, error) {
109106

110107
for i := range tiers {
111108
tier := &tiers[i]
112-
tier.Groups = append(tier.Groups, fmt.Sprintf("system:serviceaccounts:%s", m.projectedNsName(tier)))
109+
tier.Groups = append(tier.Groups, m.ProjectedSAGroup(tier))
113110
}
114111

115112
return tiers, nil
116113
}
117-
118-
func (m *Mapper) projectedNsName(tier *Tier) string {
119-
return fmt.Sprintf("%s-tier-%s", m.tenantName, tier.Name)
120-
}

maas-api/internal/tier/mapper_test.go

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -155,17 +155,12 @@ func TestMapper_GetTierForGroups(t *testing.T) {
155155
func TestMapper_GetTierForGroups_MissingConfigMap(t *testing.T) {
156156
ctx := t.Context()
157157

158-
clientset := fake.NewSimpleClientset()
158+
clientset := fake.NewClientset()
159159
mapper := tier.NewMapper(clientset, testTenant, testNamespace)
160160

161-
// Should default to free mappedTier when ConfigMap is missing
162-
mappedTier, err := mapper.GetTierForGroups(ctx, "any-group", "another-group")
163-
if err != nil {
164-
t.Errorf("unexpected error: %v", err)
165-
}
166-
167-
if mappedTier != "free" {
168-
t.Errorf("expected default mappedTier 'free', got %s", mappedTier)
161+
_, err := mapper.GetTierForGroups(ctx, "any-group", "another-group")
162+
if err == nil {
163+
t.Errorf("expected error")
169164
}
170165
}
171166

maas-api/internal/token/manager.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ func (m *Manager) RevokeTokens(ctx context.Context, user *UserContext) error {
8484
return fmt.Errorf("failed to determine user tier for %s: %w", user.Username, err)
8585
}
8686

87-
namespace := m.tierMapper.Namespaces(ctx)[userTier]
87+
namespace, errNS := m.tierMapper.Namespace(ctx, userTier)
88+
if errNS != nil {
89+
return fmt.Errorf("failed to determine namespace for user %s: %w", user.Username, errNS)
90+
}
8891

8992
saName, errName := m.sanitizeServiceAccountName(user.Username)
9093
if errName != nil {
@@ -117,9 +120,9 @@ func (m *Manager) RevokeTokens(ctx context.Context, user *UserContext) error {
117120
// ensureTierNamespace creates a tier-based namespace if it doesn't exist.
118121
// It takes a tier name, formats it as {instance}-tier-{tier}, and returns the namespace name.
119122
func (m *Manager) ensureTierNamespace(ctx context.Context, tier string) (string, error) {
120-
namespace := m.tierMapper.Namespaces(ctx)[tier]
121-
if namespace == "" {
122-
return "", fmt.Errorf("no namespace mapping found for tier %q", tier)
123+
namespace, errNs := m.tierMapper.Namespace(ctx, tier)
124+
if errNs != nil {
125+
return "", fmt.Errorf("failed to determine namespace for tier %q: %w", tier, errNs)
123126
}
124127

125128
_, err := m.namespaceLister.Get(namespace)

0 commit comments

Comments
 (0)