Skip to content

Commit 800ec0e

Browse files
authored
refactor(datalayer): key AttributeMap by DataKey instead of string (#2190)
* refactor(datalayer): key AttributeMap by DataKey instead of string Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com> * enforce(requestcontrol): scope DataProducer Put/Get to declared keys Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com> * enforce(framework): confine plugin attribute access to declared keys Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com> * refactor(framework): confine plugin attribute access to declared keys Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com> * refactor(datalayer): surface plugin data-scope violations Move runtime confinement alongside the static dependency validation it mirrors, so both halves of the Produces()/Consumes() contract live in one package. A rejected access surfaced only as a log line, and a rejected read logged one per endpoint per request. Count every rejection under llm_d_epp_plugin_data_scope_violations_total and log the first offence of each kind per invocation, so a misdeclared plugin stays visible in production without depending on log verbosity. Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com> * fix(endpointattribute): reject the combined Attribute/Producer spelling The attribute name and its producer are separate parameters, but they were once a single "Attribute/Producer" string. A key built from that spelling matches nothing: the read resolves as absent, so the filter keeps every endpoint and the scorer returns zero, with no error anywhere. Reject it at construction and name the split in the message. A producer pointer tells an omitted producer from one set to the empty string, so an attribute whose own name contains a slash stays reachable. Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com> --------- Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com>
1 parent 275883d commit 800ec0e

110 files changed

Lines changed: 1506 additions & 352 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

deploy/config/sim-epp-gpu-config.yaml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ plugins:
1313
- type: endpoint-attribute-filter
1414
name: gpu-utilization-filter
1515
parameters:
16-
attribute: "GPUUtilization/dcgm-extractor"
16+
attribute: "GPUUtilization"
17+
producer: "dcgm-extractor"
1718
onMissing: "Pass"
1819
fallbackOnEmpty: true
1920
algorithm:
@@ -24,7 +25,8 @@ plugins:
2425
- type: endpoint-attribute-scorer
2526
name: gpu-utilization-scorer
2627
parameters:
27-
attributeKey: "GPUUtilization/dcgm-extractor"
28+
attributeKey: "GPUUtilization"
29+
producer: "dcgm-extractor"
2830
algorithm:
2931
type: "linear_lower_is_better"
3032
normalization:

pkg/epp/datalayer/data_graph_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import (
3535
"github.com/llm-d/llm-d-router/pkg/epp/util"
3636
)
3737

38-
const mockProducedDataKey = "mockProducedData"
38+
var mockProducedDataKey = fwkplugin.NewDataKey("mockProducedData", "mock")
3939

4040
type mockDataProducerP struct {
4141
name string
Lines changed: 268 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,268 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
// Runtime confinement of a plugin's endpoint attribute access to the DataKeys
18+
// it declares. Typing AttributeMap by DataKey stops a plugin naming a key it
19+
// never declared in source; the scope here stops it reaching a key declared by
20+
// some other plugin, which is what makes Produces() and Consumes() a contract
21+
// rather than documentation. ValidateAndOrderDataDependencies enforces the same
22+
// contract statically, over the declarations alone.
23+
//
24+
// The scope covers endpoint attributes reached through the extension points
25+
// that receive a []scheduling.Endpoint: filters, scorers, and DataProducers.
26+
// Two paths are deliberately outside it:
27+
//
28+
// - The per-request store (InferenceRequest.PutAttribute/GetAttribute). It is
29+
// typed by DataKey but not confined, because in-tree plugins currently
30+
// exchange request attributes they do not declare.
31+
// - Datalayer extractors, which write through Endpoint.GetAttributes()
32+
// directly rather than through a scoped endpoint.
33+
34+
package datalayer
35+
36+
import (
37+
"fmt"
38+
"sync"
39+
40+
"github.com/go-logr/logr"
41+
42+
"github.com/llm-d/llm-d-router/pkg/common/observability/logging"
43+
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
44+
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
45+
fwksched "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/scheduling"
46+
"github.com/llm-d/llm-d-router/pkg/epp/metrics"
47+
)
48+
49+
// Violations records what a plugin did outside its declarations during one
50+
// extension-point invocation. Every endpoint wrapper produced by a single Scope
51+
// call shares one instance, so a plugin that reaches for the same undeclared key
52+
// on each of a hundred candidates is reported once rather than a hundred times.
53+
//
54+
// The lock is taken only on the rejection path. A conforming plugin never
55+
// touches it, so confinement costs an allowed access nothing beyond the map
56+
// lookup.
57+
type Violations struct {
58+
mu sync.Mutex
59+
write error
60+
readReported bool
61+
}
62+
63+
// Write returns the first write rejected during the invocation, or nil when the
64+
// plugin stayed inside its declarations. Extension points that can fail a
65+
// request turn this into an error; those without an error channel ignore it.
66+
func (v *Violations) Write() error {
67+
v.mu.Lock()
68+
defer v.mu.Unlock()
69+
return v.write
70+
}
71+
72+
// recordWrite stores the first rejected write and reports whether this was it.
73+
func (v *Violations) recordWrite(err error) bool {
74+
v.mu.Lock()
75+
defer v.mu.Unlock()
76+
if v.write != nil {
77+
return false
78+
}
79+
v.write = err
80+
return true
81+
}
82+
83+
// recordRead reports whether this is the first rejected read of the invocation.
84+
func (v *Violations) recordRead() bool {
85+
v.mu.Lock()
86+
defer v.mu.Unlock()
87+
if v.readReported {
88+
return false
89+
}
90+
v.readReported = true
91+
return true
92+
}
93+
94+
// ScopedEndpoint wraps a scheduling.Endpoint and confines attribute access to a
95+
// plugin's declared keys. Writes outside Produces() are dropped and recorded;
96+
// reads outside Consumes() (plus the plugin's own Produces(), since a producer
97+
// may read back what it wrote) resolve as absent.
98+
//
99+
// Reads resolve as absent rather than failing because the read paths -- Filter,
100+
// Score -- have no error channel, and every consumer already handles a missing
101+
// optional attribute. Writes have one: a producer's violation is surfaced
102+
// through Violations.Write and turned into an error by the caller that ran it.
103+
//
104+
// Either way the rejection increments a counter, so an extension point that
105+
// cannot fail the request still makes a misdeclared plugin visible in
106+
// production rather than leaving it to log verbosity.
107+
type ScopedEndpoint struct {
108+
inner fwksched.Endpoint
109+
allowedPut map[fwkplugin.DataKey]struct{}
110+
allowedGet map[fwkplugin.DataKey]struct{}
111+
typedName fwkplugin.TypedName
112+
extensionPoint string
113+
logger logr.Logger
114+
violations *Violations
115+
}
116+
117+
var _ fwksched.Endpoint = &ScopedEndpoint{}
118+
119+
func (e *ScopedEndpoint) GetMetadata() *fwkdl.EndpointMetadata { return e.inner.GetMetadata() }
120+
func (e *ScopedEndpoint) GetMetrics() *fwkdl.Metrics { return e.inner.GetMetrics() }
121+
func (e *ScopedEndpoint) String() string { return e.inner.String() }
122+
123+
// Unwrap returns the underlying endpoint. Callers hand plugin results back to
124+
// the framework unwrapped so endpoint identity -- which the scheduler relies on
125+
// to key score maps -- survives the round trip.
126+
func (e *ScopedEndpoint) Unwrap() fwksched.Endpoint { return e.inner }
127+
128+
func (e *ScopedEndpoint) Put(key fwkplugin.DataKey, value fwkdl.Cloneable) {
129+
if _, ok := e.allowedPut[key]; !ok {
130+
e.reject(metrics.DataScopeAccessWrite, fmt.Errorf(
131+
"plugin %q wrote undeclared DataKey %q; add it to Produces()", e.typedName.String(), key))
132+
return
133+
}
134+
e.inner.Put(key, value)
135+
}
136+
137+
func (e *ScopedEndpoint) Get(key fwkplugin.DataKey) (fwkdl.Cloneable, bool) {
138+
if _, ok := e.allowedGet[key]; !ok {
139+
e.reject(metrics.DataScopeAccessRead, fmt.Errorf(
140+
"plugin %q read undeclared DataKey %q; add it to Consumes()", e.typedName.String(), key))
141+
return nil, false
142+
}
143+
return e.inner.Get(key)
144+
}
145+
146+
// reject counts the violation and logs it. The first offence of each kind in an
147+
// invocation is logged at Error, the rest at DEBUG: a plugin that misreaches on
148+
// every candidate endpoint would otherwise emit one error line per endpoint per
149+
// request.
150+
func (e *ScopedEndpoint) reject(access string, err error) {
151+
metrics.RecordPluginDataScopeViolation(e.extensionPoint, e.typedName.Type, e.typedName.Name, access)
152+
153+
first := false
154+
if access == metrics.DataScopeAccessWrite {
155+
first = e.violations.recordWrite(err)
156+
} else {
157+
first = e.violations.recordRead()
158+
}
159+
if first {
160+
e.logger.Error(err, "Rejected access outside the plugin's declared keys",
161+
"extensionPoint", e.extensionPoint, "access", access)
162+
return
163+
}
164+
e.logger.V(logging.DEBUG).Info("Rejected access outside the plugin's declared keys",
165+
"extensionPoint", e.extensionPoint, "access", access, "error", err.Error())
166+
}
167+
168+
// Keys returns only the declared keys that are present, so enumeration cannot
169+
// be used to discover an attribute the plugin may not read.
170+
func (e *ScopedEndpoint) Keys() []fwkplugin.DataKey {
171+
var keys []fwkplugin.DataKey
172+
for _, key := range e.inner.Keys() {
173+
if _, ok := e.allowedGet[key]; ok {
174+
keys = append(keys, key)
175+
}
176+
}
177+
return keys
178+
}
179+
180+
// Clone returns a copy holding only the declared keys, so cloning cannot be
181+
// used to read around the scope.
182+
func (e *ScopedEndpoint) Clone() fwkdl.AttributeMap {
183+
clone := fwkdl.NewAttributes()
184+
for _, key := range e.Keys() {
185+
if value, ok := e.inner.Get(key); ok {
186+
clone.Put(key, value)
187+
}
188+
}
189+
return clone
190+
}
191+
192+
// Scope confines endpoints to what plugin declares. extensionPoint labels the
193+
// violation counter, so a misdeclared plugin is attributable to the point that
194+
// ran it. The returned Violations accumulates what the plugin did outside its
195+
// declarations, for callers whose extension point can report it.
196+
//
197+
// A plugin that declares nothing reaches nothing: an absent declaration is a
198+
// statement that the plugin exchanges no data, not a request to be exempt.
199+
func Scope(logger logr.Logger, extensionPoint string, plugin fwkplugin.Plugin, endpoints []fwksched.Endpoint) ([]fwksched.Endpoint, *Violations) {
200+
produces := map[fwkplugin.DataKey]any{}
201+
if producer, ok := plugin.(fwkplugin.ProducerPlugin); ok {
202+
produces = producer.Produces()
203+
}
204+
205+
allowedPut := make(map[fwkplugin.DataKey]struct{}, len(produces))
206+
allowedGet := make(map[fwkplugin.DataKey]struct{}, len(produces))
207+
for key := range produces {
208+
allowedPut[key] = struct{}{}
209+
// A producer may read back its own output.
210+
allowedGet[key] = struct{}{}
211+
}
212+
if consumer, ok := plugin.(fwkplugin.ConsumerPlugin); ok {
213+
deps := consumer.Consumes()
214+
for key := range deps.Required {
215+
allowedGet[key] = struct{}{}
216+
}
217+
for key := range deps.Optional {
218+
allowedGet[key] = struct{}{}
219+
}
220+
}
221+
222+
violations := &Violations{}
223+
typedName := plugin.TypedName()
224+
// One backing array rather than an allocation per endpoint: this runs for
225+
// every filter and scorer on every request, over the whole candidate set.
226+
wrappers := make([]ScopedEndpoint, len(endpoints))
227+
scoped := make([]fwksched.Endpoint, len(endpoints))
228+
for i, endpoint := range endpoints {
229+
wrappers[i] = ScopedEndpoint{
230+
inner: endpoint,
231+
allowedPut: allowedPut,
232+
allowedGet: allowedGet,
233+
typedName: typedName,
234+
extensionPoint: extensionPoint,
235+
logger: logger,
236+
violations: violations,
237+
}
238+
scoped[i] = &wrappers[i]
239+
}
240+
return scoped, violations
241+
}
242+
243+
// Unscope restores the underlying endpoints of a plugin's result.
244+
func Unscope(endpoints []fwksched.Endpoint) []fwksched.Endpoint {
245+
unscoped := make([]fwksched.Endpoint, len(endpoints))
246+
for i, endpoint := range endpoints {
247+
unscoped[i] = unwrap(endpoint)
248+
}
249+
return unscoped
250+
}
251+
252+
// UnscopeScores rekeys a scorer's result by the underlying endpoints. The
253+
// scheduler sums scores across scorers in a map keyed by endpoint, so a wrapper
254+
// left in a key would split one endpoint's score into several entries.
255+
func UnscopeScores(scores map[fwksched.Endpoint]float64) map[fwksched.Endpoint]float64 {
256+
unscoped := make(map[fwksched.Endpoint]float64, len(scores))
257+
for endpoint, score := range scores {
258+
unscoped[unwrap(endpoint)] = score
259+
}
260+
return unscoped
261+
}
262+
263+
func unwrap(endpoint fwksched.Endpoint) fwksched.Endpoint {
264+
if scoped, ok := endpoint.(*ScopedEndpoint); ok {
265+
return scoped.inner
266+
}
267+
return endpoint
268+
}
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package datalayer
18+
19+
import (
20+
"fmt"
21+
"testing"
22+
23+
"k8s.io/apimachinery/pkg/types"
24+
25+
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
26+
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
27+
fwksched "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/scheduling"
28+
)
29+
30+
// The scheduler runs every filter and scorer over the whole candidate set, so
31+
// the per-plugin cost here is multiplied by the plugin count on each request.
32+
func benchEndpoints(count int) []fwksched.Endpoint {
33+
endpoints := make([]fwksched.Endpoint, count)
34+
for i := range endpoints {
35+
attrs := fwkdl.NewAttributes()
36+
attrs.Put(consumedKey, cloneableStr("v"))
37+
endpoints[i] = fwksched.NewEndpoint(
38+
&fwkdl.EndpointMetadata{ID: types.NamespacedName{Name: fmt.Sprintf("ep-%d", i)}},
39+
&fwkdl.Metrics{}, attrs)
40+
}
41+
return endpoints
42+
}
43+
44+
func benchPlugin() fwkplugin.Plugin {
45+
p := &producerConsumerPlugin{}
46+
p.produces = map[fwkplugin.DataKey]any{producedKey: nil}
47+
p.consumes = &fwkplugin.DataDependencies{Optional: map[fwkplugin.DataKey]any{consumedKey: nil}}
48+
return p
49+
}
50+
51+
func BenchmarkScope(b *testing.B) {
52+
for _, count := range []int{10, 100} {
53+
b.Run(fmt.Sprintf("endpoints=%d", count), func(b *testing.B) {
54+
endpoints, plugin := benchEndpoints(count), benchPlugin()
55+
b.ReportAllocs()
56+
b.ResetTimer()
57+
for i := 0; i < b.N; i++ {
58+
scoped, _ := Scope(testLogger(), "test-extension-point", plugin, endpoints)
59+
_ = Unscope(scoped)
60+
}
61+
})
62+
}
63+
}
64+
65+
func BenchmarkUnscopeScores(b *testing.B) {
66+
for _, count := range []int{10, 100} {
67+
b.Run(fmt.Sprintf("endpoints=%d", count), func(b *testing.B) {
68+
endpoints, plugin := benchEndpoints(count), benchPlugin()
69+
scoped, _ := Scope(testLogger(), "test-extension-point", plugin, endpoints)
70+
scores := make(map[fwksched.Endpoint]float64, len(scoped))
71+
for i, endpoint := range scoped {
72+
scores[endpoint] = float64(i)
73+
}
74+
b.ReportAllocs()
75+
b.ResetTimer()
76+
for i := 0; i < b.N; i++ {
77+
_ = UnscopeScores(scores)
78+
}
79+
})
80+
}
81+
}

0 commit comments

Comments
 (0)