Skip to content

Commit 7220584

Browse files
Add LimitRange visibility and namespace links (#1696)
Adds dedicated LimitRange rendering, namespace summaries, and contextual links from Pods and workload details. This surfaces admission defaults and constraints that were previously hidden, while keeping the distinction clear between no rules and permission-denied reads. Includes dispatch wiring and focused test coverage for the new renderer behavior. ## Description This solves issue number #1660 ## Type of change - [ ] Bug fix (non-breaking change that fixes an issue) - [x] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## How has this been tested? Describe the tests you ran to verify your changes. - [ ] Tested locally with minikube/kind - [x] Tested against a remote cluster - [x] Added/updated unit tests ## Checklist - [ ] My code follows the project's coding standards - [ ] I have performed a self-review of my code - [ ] I have added comments where necessary - [ ] My changes generate no new warnings - [ ] Any dependent changes have been merged ## Related issues Fixes #(issue number) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes authorization behavior for LimitRange list endpoints (explicit 403 vs empty results) and adds new RBAC gates on get/list; UI impact is additive but mis-handled errors could still confuse operators about admission rules. > > **Overview** > Surfaces **LimitRange** admission defaults and constraints in the UI and tightens how the API reports unreadable namespaces. > > **Backend:** List/get for `limitranges` now runs dedicated RBAC checks (list/get SAR per namespace scope) and returns **403 with an explicit error** when denied, instead of the legacy empty-list/table response used for most kinds—so the UI cannot treat “no permission” as “no rules.” List/get also require the cache to **cover the requested namespace** (`KindCoversNamespace`) and honor **deferred cache warming** via 503 while LimitRanges are still loading. > > **UI:** Adds a **LimitRange** detail renderer (per-type tables for min/max/default/ratio, verbatim quantities), a **namespace Limit Ranges** section (empty vs loading vs 403 vs stale cache), and **contextual links** from Pod and workload resource sections to relevant LimitRange objects (PVC-only rules filtered out for pod links). The web app wires `useNamespaceLimitRanges` and passes data through the existing renderer props; **403 is shown in the LimitRange section** rather than hidden like quotas. > > **Tests:** Server access/cache coverage/warming tests plus renderer, dispatch, and `podLimitRangeNames` unit tests. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3255995. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Nadav Erell <nadaverell@gmail.com>
1 parent 3aee7f1 commit 7220584

15 files changed

Lines changed: 738 additions & 10 deletions
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package server
2+
3+
import (
4+
"encoding/json"
5+
"net/http"
6+
"testing"
7+
8+
"github.com/skyhook-io/radar/internal/auth"
9+
"github.com/skyhook-io/radar/internal/k8s"
10+
"github.com/skyhook-io/radar/pkg/k8score"
11+
corev1 "k8s.io/api/core/v1"
12+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/runtime"
14+
"k8s.io/client-go/kubernetes/fake"
15+
k8stesting "k8s.io/client-go/testing"
16+
)
17+
18+
func TestLimitRangeResourceAccess(t *testing.T) {
19+
useTestResourceCache(t, fake.NewClientset(
20+
&corev1.LimitRange{ObjectMeta: metav1.ObjectMeta{Name: "rules", Namespace: "default"}},
21+
))
22+
tests := []struct {
23+
name, path, verb string
24+
namespaces []string
25+
grants map[string]bool
26+
want int
27+
}{
28+
{"list denied", "/api/resources/limitranges?namespace=default", "list", []string{"default"}, map[string]bool{"default": false}, 403},
29+
{"list allowed", "/api/resources/limitranges?namespace=default", "list", []string{"default"}, map[string]bool{"default": true}, 200},
30+
{"get denied", "/api/resources/limitranges/default/rules", "get", []string{"default"}, map[string]bool{"default": false}, 403},
31+
{"get allowed", "/api/resources/limitranges/default/rules", "get", []string{"default"}, map[string]bool{"default": true}, 200},
32+
{"namespace denied is not empty", "/api/resources/limitranges?namespace=default", "list", []string{"other"}, map[string]bool{"default": true}, 403},
33+
{"cluster pod visibility does not grant rules", "/api/resources/limitranges", "list", nil, map[string]bool{"": false}, 403},
34+
{"cluster grant", "/api/resources/limitranges", "list", nil, map[string]bool{"": true}, 200},
35+
{"mixed grants cannot claim completeness", "/api/resources/limitranges?namespaces=default,other", "list", []string{"default", "other"}, map[string]bool{"default": true, "other": false}, 403},
36+
{"table denial is explicit", "/api/resources/limitranges?namespace=default&table=1", "list", []string{"default"}, map[string]bool{"default": false}, 403},
37+
}
38+
for _, tt := range tests {
39+
t.Run(tt.name, func(t *testing.T) {
40+
env := newAuthTestServer(t)
41+
perms := &auth.UserPermissions{AllowedNamespaces: tt.namespaces}
42+
for ns, allowed := range tt.grants {
43+
perms.SetCanI(tt.verb, "", "limitranges", ns, allowed)
44+
}
45+
env.srv.permCache.Set("reader", nil, perms)
46+
resp := env.authGet(t, tt.path, "reader", "")
47+
defer resp.Body.Close()
48+
if resp.StatusCode != tt.want {
49+
t.Fatalf("status = %d, want %d", resp.StatusCode, tt.want)
50+
}
51+
if tt.want == http.StatusForbidden {
52+
var body map[string]string
53+
if err := json.NewDecoder(resp.Body).Decode(&body); err != nil || body["error"] == "" {
54+
t.Fatalf("expected explicit error, got %v (%v)", body, err)
55+
}
56+
} else if tt.verb == "list" {
57+
var items []corev1.LimitRange
58+
if err := json.NewDecoder(resp.Body).Decode(&items); err != nil || len(items) != 1 || items[0].Name != "rules" {
59+
t.Fatalf("expected rules, got %v (%v)", items, err)
60+
}
61+
}
62+
})
63+
}
64+
}
65+
66+
func TestLimitRangeCacheCoverage(t *testing.T) {
67+
core, err := k8score.NewResourceCache(k8score.CacheConfig{
68+
Client: fake.NewClientset(),
69+
ResourceScopes: map[string]k8score.ResourceScope{
70+
k8score.LimitRanges: {Enabled: true, Namespace: "default"},
71+
},
72+
})
73+
if err != nil {
74+
t.Fatal(err)
75+
}
76+
cache := k8s.GetResourceCache()
77+
original := cache.ResourceCache
78+
cache.ResourceCache = core
79+
t.Cleanup(func() { cache.ResourceCache = original; core.Stop() })
80+
81+
for _, tt := range []struct {
82+
path string
83+
want int
84+
}{
85+
{"/api/resources/limitranges?namespace=default", 200},
86+
{"/api/resources/limitranges?namespace=other", 403},
87+
{"/api/resources/limitranges?namespaces=default,other", 403},
88+
{"/api/resources/limitranges", 403},
89+
{"/api/resources/limitranges/other/unknown", 403},
90+
{"/api/resources/limitranges/default/unknown", 404},
91+
} {
92+
t.Run(tt.path, func(t *testing.T) {
93+
resp, err := http.Get(testServer.URL + tt.path)
94+
if err != nil {
95+
t.Fatal(err)
96+
}
97+
defer resp.Body.Close()
98+
if resp.StatusCode != tt.want {
99+
t.Fatalf("status = %d, want %d", resp.StatusCode, tt.want)
100+
}
101+
})
102+
}
103+
}
104+
105+
func TestLimitRangeCacheWarming(t *testing.T) {
106+
client := fake.NewClientset()
107+
release := make(chan struct{})
108+
client.PrependReactor("list", "limitranges", func(action k8stesting.Action) (bool, runtime.Object, error) {
109+
<-release
110+
return true, &corev1.LimitRangeList{}, nil
111+
})
112+
core, err := k8score.NewResourceCache(k8score.CacheConfig{
113+
Client: client,
114+
ResourceScopes: map[string]k8score.ResourceScope{k8score.LimitRanges: {Enabled: true}},
115+
DeferredTypes: map[string]bool{k8score.LimitRanges: true},
116+
})
117+
if err != nil {
118+
close(release)
119+
t.Fatal(err)
120+
}
121+
cache := k8s.GetResourceCache()
122+
original := cache.ResourceCache
123+
cache.ResourceCache = core
124+
t.Cleanup(func() { close(release); cache.ResourceCache = original; core.Stop() })
125+
for _, path := range []string{"/api/resources/limitranges?namespace=default", "/api/resources/limitranges/default/rules"} {
126+
resp, err := http.Get(testServer.URL + path)
127+
if err != nil {
128+
t.Fatal(err)
129+
}
130+
resp.Body.Close()
131+
if resp.StatusCode != http.StatusServiceUnavailable {
132+
t.Fatalf("%s: status = %d, want 503", path, resp.StatusCode)
133+
}
134+
}
135+
}

‎internal/server/server.go‎

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2033,6 +2033,18 @@ func (s *Server) preflightResourceList(r *http.Request, kind, group string, name
20332033
}
20342034
}
20352035

2036+
if (kind == "limitranges" || kind == "limitrange") && group == "" {
2037+
scopes := namespaces
2038+
if scopes == nil {
2039+
scopes = []string{""}
2040+
}
2041+
for _, ns := range scopes {
2042+
if !s.canRead(r, "", "limitranges", ns, "list") {
2043+
return nil, http.StatusForbidden, "insufficient permissions to list limitranges", false
2044+
}
2045+
}
2046+
}
2047+
20362048
return namespaces, 0, "", true
20372049
}
20382050

@@ -2063,11 +2075,15 @@ func (s *Server) handleListResources(w http.ResponseWriter, r *http.Request) {
20632075
// DiscoverNamespaces if needed). canRead below relies on it.
20642076
namespaces := s.parseNamespacesForUser(r)
20652077

2066-
// Shared RBAC gate. REST converts denies to 200 with `[]` (legacy shape
2067-
// the frontend tolerates and that doesn't leak kind existence); the AI path
2068-
// returns the explicit status.
2069-
finalNamespaces, _, _, ok := s.preflightResourceList(r, kind, group, namespaces)
2078+
// Most REST lists preserve the empty-list denial response. LimitRange
2079+
// lookups need an explicit denial so an unreadable namespace is not
2080+
// presented as having no admission rules.
2081+
finalNamespaces, status, msg, ok := s.preflightResourceList(r, kind, group, namespaces)
20702082
if !ok {
2083+
if kind == "limitranges" && group == "" {
2084+
s.writeError(w, status, msg)
2085+
return
2086+
}
20712087
// Denied, not empty. writeResourceList resolves columns for an empty
20722088
// list, and that lookup runs as Radar's own identity — so routing a
20732089
// denial through it would answer a caller who cannot list the kind with
@@ -2402,10 +2418,20 @@ func (s *Server) handleListResources(w http.ResponseWriter, r *http.Request) {
24022418
}
24032419
result, err = cache.IngressClasses().List(labels.Everything())
24042420
case "limitranges":
2405-
if cache.LimitRanges() == nil {
2421+
if cache.IsDeferredPending("limitranges") || cache.LimitRanges() == nil {
24062422
notReadyOrForbidden("limitranges")
24072423
return
24082424
}
2425+
scopes := namespaces
2426+
if scopes == nil {
2427+
scopes = []string{""}
2428+
}
2429+
for _, ns := range scopes {
2430+
if !cache.KindCoversNamespace("limitranges", ns) {
2431+
s.writeError(w, http.StatusForbidden, "Radar does not have LimitRange visibility for the requested namespace scope")
2432+
return
2433+
}
2434+
}
24092435
result, err = listPerNs(
24102436
func() (any, error) { return cache.LimitRanges().List(labels.Everything()) },
24112437
func(ns string) (any, error) {
@@ -2544,6 +2570,9 @@ func (s *Server) preflightResourceGet(r *http.Request, kind, namespace, name, gr
25442570
if (kind == "secrets" || kind == "secret") && !s.canRead(r, "", "secrets", namespace, "get") {
25452571
return http.StatusForbidden, fmt.Sprintf("no access to secrets in namespace %q", namespace), false
25462572
}
2573+
if (kind == "limitranges" || kind == "limitrange") && group == "" && !s.canRead(r, "", "limitranges", namespace, "get") {
2574+
return http.StatusForbidden, fmt.Sprintf("no access to limitranges in namespace %q", namespace), false
2575+
}
25472576
default:
25482577
// Empty namespace and not a recognized cluster-scoped kind: an empty
25492578
// namespace means the target is cluster-scoped, but ClassifyKindScope
@@ -2786,10 +2815,14 @@ func (s *Server) handleGetResource(w http.ResponseWriter, r *http.Request) {
27862815
}
27872816
resource, err = cache.IngressClasses().Get(name)
27882817
case "limitranges", "limitrange":
2789-
if cache.LimitRanges() == nil {
2818+
if cache.IsDeferredPending("limitranges") || cache.LimitRanges() == nil {
27902819
notReadyOrForbiddenGet("limitranges")
27912820
return
27922821
}
2822+
if !cache.KindCoversNamespace("limitranges", namespace) {
2823+
s.writeError(w, http.StatusForbidden, "Radar does not have LimitRange visibility for the requested namespace")
2824+
return
2825+
}
27932826
resource, err = cache.LimitRanges().LimitRanges(namespace).Get(name)
27942827
case "resourcequotas", "resourcequota":
27952828
if cache.ResourceQuotas() == nil {
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { renderToString } from 'react-dom/server'
3+
import { LimitRangeRenderer, NamespaceLimitRangeLink, NamespaceLimitRangesSection } from './LimitRangeRenderer'
4+
5+
function err(message: string, status?: number): Error {
6+
return Object.assign(new Error(message), status === undefined ? {} : { status })
7+
}
8+
9+
const multiType = {
10+
apiVersion: 'v1',
11+
kind: 'LimitRange',
12+
metadata: { name: 'team-limits', namespace: 'dev' },
13+
spec: {
14+
limits: [
15+
{
16+
type: 'Container',
17+
min: { cpu: '10m', memory: '16Mi' },
18+
max: { cpu: '2', memory: '2Gi' },
19+
default: { cpu: '500m', memory: '512Mi' },
20+
defaultRequest: { cpu: '100m', memory: '128Mi' },
21+
maxLimitRequestRatio: { cpu: '4' },
22+
},
23+
{ type: 'Pod', max: { cpu: '4' } },
24+
{ type: 'PersistentVolumeClaim', min: { storage: '1Gi' }, max: { storage: '100Gi' } },
25+
],
26+
},
27+
}
28+
29+
describe('LimitRangeRenderer', () => {
30+
it('renders every declared entry type with its own rules', () => {
31+
const html = renderToString(<LimitRangeRenderer data={multiType} />)
32+
expect(html).toContain('Container')
33+
expect(html).toContain('Pod')
34+
expect(html).toContain('PersistentVolumeClaim')
35+
expect(html).toContain('100m')
36+
expect(html).toContain('2Gi')
37+
expect(html).toContain('storage')
38+
})
39+
40+
// Units and resource names carry the meaning; a reformatted quantity is a
41+
// different claim from the one the object makes.
42+
it('shows quantities verbatim', () => {
43+
const html = renderToString(<LimitRangeRenderer data={multiType} />)
44+
expect(html).toContain('16Mi')
45+
expect(html).toContain('10m')
46+
expect(html).not.toContain('0.01 cores')
47+
})
48+
49+
// An absent facet is not a limit of zero, and a limit of zero is a real rule.
50+
it('keeps an unset value distinct from an explicit zero', () => {
51+
const html = renderToString(<LimitRangeRenderer data={{
52+
spec: { limits: [{ type: 'Container', min: { cpu: '0' }, max: { cpu: '1', memory: '1Gi' } }] },
53+
}} />)
54+
// memory has a max but no min, so its min cell is the unset marker.
55+
expect(html).toContain('Not set')
56+
expect(html).toContain('>0<')
57+
})
58+
59+
it('drops facets the entry never declares instead of showing empty columns', () => {
60+
const html = renderToString(<LimitRangeRenderer data={{
61+
spec: { limits: [{ type: 'Pod', max: { cpu: '4' } }] },
62+
}} />)
63+
expect(html).toContain('Max')
64+
expect(html).not.toContain('Default request')
65+
expect(html).not.toContain('Max limit/request')
66+
})
67+
68+
it('states that a LimitRange with no entries constrains nothing', () => {
69+
const html = renderToString(<LimitRangeRenderer data={{ spec: {} }} />)
70+
expect(html).toContain('constrains nothing')
71+
})
72+
})
73+
74+
describe('NamespaceLimitRangesSection', () => {
75+
it('summarizes each LimitRange and links to it', () => {
76+
const html = renderToString(
77+
<NamespaceLimitRangesSection limitRanges={[multiType]} namespace="dev" onNavigate={() => {}} />,
78+
)
79+
expect(html).toContain('team-limits')
80+
expect(html).toContain('Container')
81+
expect(html).toContain('default request')
82+
})
83+
84+
it('tells an empty namespace apart from one still loading', () => {
85+
const empty = renderToString(<NamespaceLimitRangesSection limitRanges={[]} namespace="dev" />)
86+
expect(empty).toContain('Limit Ranges (0)')
87+
expect(empty).toContain('No LimitRanges found')
88+
expect(empty).toContain('Other admission policies may still apply')
89+
90+
const loading = renderToString(<NamespaceLimitRangesSection loading namespace="dev" />)
91+
expect(loading).toContain('Loading limit ranges')
92+
expect(loading).not.toContain('No LimitRanges found')
93+
})
94+
95+
// "You can't see the rules" and "there are no rules" lead to opposite
96+
// conclusions about what will happen at admission.
97+
it('reports a denied read rather than implying no rules exist', () => {
98+
const html = renderToString(
99+
<NamespaceLimitRangesSection error={err('forbidden', 403)} namespace="dev" />,
100+
)
101+
expect(html).toContain('permission')
102+
expect(html).not.toContain('No LimitRanges found')
103+
})
104+
105+
it('keeps cached rules visible with a stale warning when a refresh fails', () => {
106+
const html = renderToString(
107+
<NamespaceLimitRangesSection limitRanges={[multiType]} error={err('unavailable', 500)} namespace="dev" />,
108+
)
109+
expect(html).toContain('team-limits')
110+
expect(html).toContain('Showing previously loaded rules')
111+
expect(html).toContain('unavailable')
112+
expect(html).not.toContain('This may be incomplete')
113+
})
114+
115+
it('hides cached rules when access is denied', () => {
116+
const html = renderToString(
117+
<NamespaceLimitRangesSection limitRanges={[multiType]} error={err('forbidden', 403)} namespace="dev" />,
118+
)
119+
expect(html).toContain('permission')
120+
expect(html).not.toContain('team-limits')
121+
expect(html).not.toContain('No LimitRanges found')
122+
})
123+
124+
it('keeps a fetch fault loud', () => {
125+
const html = renderToString(
126+
<NamespaceLimitRangesSection error={err('boom', 500)} namespace="dev" />,
127+
)
128+
expect(html).toContain('boom')
129+
expect(html).toContain('text-red-400')
130+
})
131+
})
132+
133+
describe('NamespaceLimitRangeLink', () => {
134+
it('links every LimitRange that governs the namespace', () => {
135+
const html = renderToString(
136+
<NamespaceLimitRangeLink namespace="dev" names={['team-limits', 'storage-limits']} scope="pod" onNavigate={() => {}} />,
137+
)
138+
expect(html).toContain('Namespace defaults')
139+
expect(html).toContain('team-limits')
140+
expect(html).toContain('storage-limits')
141+
})
142+
143+
// Nothing to point at yet is not the same as pointing at nothing.
144+
it('renders nothing when the lookup produced no names', () => {
145+
expect(renderToString(<NamespaceLimitRangeLink namespace="dev" names={[]} scope="pod" />)).toBe('')
146+
expect(renderToString(<NamespaceLimitRangeLink namespace="dev" scope="pod" />)).toBe('')
147+
})
148+
149+
it('offers the scope explanation without adding inline prose', () => {
150+
const html = renderToString(
151+
<NamespaceLimitRangeLink namespace="dev" names={['team-limits']} scope="workload" onNavigate={() => {}} />,
152+
)
153+
expect(html).toContain('About namespace defaults and constraints')
154+
expect(html).not.toContain('not the workload object itself')
155+
})
156+
})

0 commit comments

Comments
 (0)