cartographer
cartographer copied to clipboard
RFC: Add matchParams selector
Add matchParams selector Readable
✔️ Deploy Preview for elated-stonebraker-105904 canceled.
🔨 Explore the source changes: c557c5d7291b3801941cc7e95d5deb16f1005a1b
🔍 Inspect the deploy log: https://app.netlify.com/sites/elated-stonebraker-105904/deploys/621507c2195d720008057add
If we think that params are important enough to match on, they deserve top level support in a matchParams facet of the selector.
@scothis yeah @emmjohnson suggested something similar in our community meeting. I'm happy to take that direction with this RFC. @sclevine I think you mentioned in the meeting that you preferred the RFC as written, any objections to making the above change?
Since matchFields is generic and able to match any field within the resource, I'd rather not overload that syntax by creating shortcuts for params.
@scothis We already have this shortcut in other places, and matchFields
already uses non-standard notation for the field selector path (e.g., workload.spec
instead spec
). I suspect uses will assume that params
works with matchFields
, just as it works in other places. But no strong opinion.
Since matchFields is generic and able to match any field within the resource, I'd rather not overload that syntax by creating shortcuts for params.
Despite originally thinking that MatchParams was overkill, I think you're right. MatchFields has a clear context, and Params are the resolution of partial-tree. I think drawing a user's attention to this special case with this separate selector is more educational.
I think this rfc is important in it's ability to provide defaultable fields.
That's a compelling distinction as it is not possible with a vanilla field selector operating against the Workload structure directly. It requires a partial templating context to match against.
I'm still on the fence as to whether matchFields
would be better actually as a vanilla field selector operating against the Workload structure directly and the new behavior be defined within a matchParam
facet.
This strikes me as a bit of syntactic sugar. Params can be set in 4 places:
- In the template
- In the resource field of a supply chain
- At the top level of a supply chain
- In the workload
When specifying a MatchParam, the supply-chain author knows 3 of these values already. Ultimately they are just checking, has the supply chain overwritten the choices made at the other 3 levels. So they need only check if the param exists on the workload and if it has a given value.
I have no objections to the change, but I wonder at its utility.
I think I agree with @emmjohnson and @scothis that if we're going to inject the param hierarchy logic in this field, it deserves its own matchParams
key to be explicit.
But as @waciumawanjohi said, I struggle to understand the practical use case for this.
But as @waciumawanjohi said, I struggle to understand the practical use case for this.
@martyspiewak @waciumawanjohi as @squeedee pointed out this is about providing a path for defaultable fields. I've updated the yaml in the RFC to better illustrate this use case.