Open alexsnaps opened 3 weeks ago
As part of the refactoring Value.ResolveFor
would be changed to not take a JSON literal in anymore, but the WellKnownAttributes
directly... so they still can be "cached" (kept around) on the call site, but the callee would now be the one to decide how to marshal that data...
type Value interface {
ResolveFor(attributes *WellKnownAttributes) (interface{}, error)
}
// where WellKnownAttributes are obtained from the AuthPipeline, just as the JSON currently is with GetAuthorizationJSON
value.ResolveFor(authPipeline.GetWellKnowAttributes())
Also note that currently, because of how data "gets out of CEL", we have this
exp, err := NewExpression("timestamp(\"2024-12-25T12:00:00Z\") + duration('123456m')")
v, err := exp.ResolveFor("{}")
// v is a `string` of value `2025-03-21T05:36:00Z` , i.e. a RFC3339 date format
which I think is a desirable outcome here... but only given this.
This has become more painful because of the introduction of CEL, but
request.time
is made available as the struct that backs it:map[nanos:68000 seconds:1.730319299e+09]
. With CEL having first class support it is probably expected (and documented) to be available as a properTimestamp
. Even in JSON tho, it would be probably be best represented in RFC3339 format.Proposal
protojson
deals with these conversions properly. Which is already used to produce the intermediary JSON result coming back from a CEL Value, as well as feeding outWellKnownAttributes
into the interpreter.request
itself is protobuf as it comes fromenvoy/service/auth/v3
'sAttributeContext
which is autogenerated from a.proto
.I'm thinking we should have our own
WellKnownAttributes
spec'ed as protobuf and useprotojson
to marshal that data. With that approach, no json marshaling would actually to inject theWellKnownAttributes
into the CEL interpreter, but they would be made available as protobufMessage
s directly.So the proposal is to refactor all of
WellKnownAttributes
into.proto
, and use that generated code to produce proper typed JSON/CEL Values. wdyt?