Further changes to Policy Attachment

[youngnick]

In a dedicated meeting today about Policy Attachment, some community members discussed where Policy Attachment is at (as of writing, #2813 is still in progress, but we hope to merge this in its current state soon, and then to make further changes based on the content of this discussion). Meeting notes are available at https://docs.google.com/document/d/1hF0JuhL-XNCqgmZ_MGP4tgneFxVWEz44niVhM0skBjg/edit?usp=sharing

#2813 splits GEP-713 into a Memorandum GEP, and then separate GEPs for Direct Policy Attachment and Inherited Policy Attachment.

Direct Policy Attachment is a strict subset of Policies that meet a set of rules (defined in GEP-2648 in the PR), and is intended to make us able to graduate restricted Policies included in the API (currently BackendTLSPolicy, but GEP-1619 is proposing to also introduce a BackendLoadBalancerPolicy, which also meets the Direct criteria).

Additionally, the requirements for Inherited Policy have been loosened such that defaults or overrides stanzas are no longer required. Instead, Policy owners can define how fields should work.

In our discussion, we agreed that we, the community, would like to consider the following four things (as suggested by @robscott, thanks Rob!):

• if defaults/overrides are not specified, defaulting is assumed
• allow plural 'targetRefs' instead of targetRefs with maxLen of ~16?
• copy kuadrant's concept of merge strategies - what we currently refer to as "direct" sets this to "none". Implementations can specify impl-specific strategies and maybe add that to a GW API library that powers tools like gwctl
• direct and inherited are no longer different forms of policy attachment, just different merge strategies

I'll make four separate comments outlining my thoughts on these four options, that we can use as starting points for further discussion, so we can keep all of this in the same place.

We also agreed that #2813 will have GEP-2648 and GEP-2649 changed to Provisional, since it seems likely from today's discussion that the distinction may be less relevant in the future.

[youngnick]

Discussion thread for:

If defaults or overrides are not specified, defaulting is assumed.

This means that, in the absence of structs named defaults or overrides, fields in a Policy should be assumed to be default values rather than override ones.

[youngnick]

This seems like a straightforward change that could be merged into the existing GEPs very quickly. +1 from me.

[robscott]

+1

[youngnick]

Discussion thread for:

Allow plural targetRefs instead of targetRef with maxLen of ~16?

[robscott]

Yep, I think the big detail to get right here is status, but agree with your proposed approach for that, +1 on this direction.

[mikemorris]

For a specific example of how this is helpful even for the narrow scope of BackendTLSPolicy, consider how a typical blue/green deployment pattern with HTTPRoute typically involves splitting traffic off to a differently-named foo-v2 Service temporarily - to avoid disruption, this new Service would likely also need the same policy applied so the Gateway client knows to continue using TLS to connect.

This would allow temporarily adding an additional targetRef to the existing policy to enable this rather than creating a duplicate policy and risking config sprawl/zombie policies after consolidating back to the original name following a successful rollout.

I think the question of whether PolicyAncestorStatus is an appropriate way to represent the status of affected resources is an intersecting but separate consideration that should be discussed at more length over in #2923

[candita]

#2755 came to my attention recently. Another issue to consider with muitiple targetRefs.

[guicassolato]

I believe use case for this definitely exists, another one likely being HTTPRoutes that have to be split into 2+ due to max number of HTTPRouteRules allowed.

Without plural targetRefs, I fear some policies could end up being designed in a way just to model N-1 relations between targets and actual policy specs. E.g.:

• target-1 ← policy-1 → spec
• target-2 ← policy-2 → spec

With targetRefs, +1 for namespacing the status by controller > targetRef.

Additionally, I wonder if we also want Accepted (PartiallyAccepted?) status conditions for the policy as a whole or would users always read the status at the granularity of each targetRef (following the pattern adopted for parentRefs)?

[mikemorris]

Additionally, I wonder if we also want Accepted (PartiallyAccepted?) status conditions for the policy as a whole or would users always read the status at the granularity of each targetRef (following the pattern adopted for parentRefs)?

targetRef granularity feels preferable to me as effects of applied policy is already difficult to reason about without introducing partial acceptance, but either might be workable.

[youngnick]

Discussion thread for:

Copy Kuadrant's concept of merge strategies - what we currently refer to as "direct" sets this to "none". Implementations can specify impl-specific strategies and maybe add that to a GW API library that powers tools like gwctl

and

direct and inherited are no longer different forms of policy attachment, just different merge strategies

[robscott]

That is already state of the art for kubernetes objects and has a variety of tooling

@howardjohn What are you referring to in this context?

[howardjohn]

Putting merging method indicators into OpenAPI schemas and consuming them in tooling like kubectl/kustomize/etc

[mikemorris]

+1 to the merge strategy being a status field to communicate to a user or generic tool such as gwctl how this policy should be expected to resolve, and typically configured by the policy implementation author in the CRD directly.

If some implementations allow selection of merge strategies on certain policies, they could express the default behavior in this same way, and additionally allow an end user to override the default strategy in something like a spec.policyMeta.strategy field, which would then be reflected in the status.

[guicassolato]

Sorry for taking so long to comment on this one in particular.

I must say that Kuadrant’s concept of merge strategies has not been battle tested yet, albeit fairly well documented IMO.

We started with the non-merge (aka "atomic") strategy for inherited policies, with defaults and overrides and all, which have been implemented and are well covered by tests. We're currently working on expanding the list of scenarios, to better experiment with the use cases for more granular merges, including for complex setups such as (though not only) multiple gateway parents of a route, targeting sections of a resource, etc.

[guicassolato]

We've had another iteration on this which resulted in the presentation of the Policy Machinery project in the Gateway API community call on Jun 2nd.

TL;DR – Without changing much from the current design guideline for policy specs – where a spec basically consists of targetRef + a free "spec proper" optionally wrapped within a defaults or overrides block – there may be room to accommodate the standard and default merge strategies, as well as vendor-specific (custom) ones IMHO.

Two standard merge strategies can be:

• No merge at all → basically the case of Direct Policy as currently known
• JSON Patch strategy → can be the default for Inherited, as implemented by gwctl already

On top of that, I honestly believe implementations will require to define their own custom merge strategies. This is definitely the case of Kuadrant. I wish that could be something we can support without breaking compliance.

With that in mind, I've explored with this approach of the policy type itself being what implements the merge strategy. Gateway API could provide the default ones, NoMerge and JSONPatch, but always via interfaces that allow implementations to customise while still compliant, as well as tools such as gwctl to reproduce the behavior.

I'll leave here as reference this full example of a custom controller that computes effective policies for 4 kinds of policies based on custom merge strategies, using the Policy Machinery. 2 kinds of policies attach to Gateways with or without sectionName and implement the default no-merge strategy; 2 others attach to Gateways and HTTPRoute both with or without sectionName and implement 4 custom strategies: full ("atomic") default, atomic overrides, merge default named policy rules, merge override named policy rules.

Happy to jump into a call with whoever wants to learn more or you can watch the recording of a demo here: https://youtu.be/UNBMwM_M1fw?si=odvI8oZsHun5rtSE (28'28'')

[mikemorris]

Spinning out my comment from #2813 (comment), I'd like to propose that targeting both a resource as a whole and a subsection of the same resource by sectionName be disallowed for Direct Policies (or policies with merge strategy none if that proposal advances), and only supported when a compositional merge strategy is defined (either by the policy implementer in the CRD, or by an end-user author in the policy spec if supported).

Simultaneously targeting separate non-overlapping subsections of a resource should be allowed for Direct Policies (or with merge strategy none), but overlapping targets which would require some sort of inheritance logic should instead be rejected and set a Conflicted status condition.

[guicassolato]

direct and inherited are no longer different forms of policy attachment, just different merge strategies

This is another piece I would like to touch on, slightly separate from the details of the merge strategy (though not entirely), hence a separate comment for it.

I can see how a merge strategy none is part of distinguishing Direct vs Inherited. The point I want to raise is whether this feature of the Direct class (i.e. no merge strategy defined) suffices to characterize it or not. I guess this cross relates to introducing plural targetRefs too, in some way, as well as to other aspects used in the past in the definitions of these classes of Policies, e.g., allowing/not allowing multiple Policies of a kind to target a same resource, etc.

TL;DR – I want to propose Policy CRDs to declare explicitly what kind of resource instances of the Policy ultimately affect – not to be confused with the kinds a Policy allows in its targetRef. I.e. what kind of resource the Policy actually augments the behavior of.

This is something we somewhat get "for free" with Direct Policies today. From current definition: Direct Policies "only affect the object they are attached to; that is, the object specified in their targetRef." IOW, there's no transitivity; targeting a resource with a Direct Policy is never a mean to affecting something else other than the targeted resource itself. I suppose multiple targetRefs in a Direct Policy must all be of the same kind then or perhaps there will be Policy specs generic enough to apply to more than one kind without requiring 2 Policy CRDs?

Inherited Policies in contrast can target a resource as a mean to ultimately affect other resources, typically lower in the hierarchy. Again from current definition: "Any object that affects settings outside of the object that it attaches to is an Inherited Policy." But also "If a Policy can be used as an Inherited Policy, it MUST be treated as an Inherited Policy, regardless of whether a specific instance of the Policy is only affecting a single object." However, currently there's no straightforward way to tell what level of the hierarchy an Inherited Policy ultimately affects. An Inherited Policy that targets a Gateway could be aiming to augment the behaviour of the Routes or the behaviour of the Backends under that Gateway, for instance.

Regarding the merge strategy as the differentiator, I can also see Inherited Policies instances or even entire Inherited Policy kinds that want to enforce the semantics of indivisible ("atomic") specs, i.e., where either one or another Policy instance ultimately affecting a same resource wins, but never a merge of the two Policy instances, whilst still affecting the object indirectly (transitively). Maybe this could be captured from a merge strategy atomic that differentiates from none, though it's not hard to imagine some confusion between these 2 no-merge merge strategies.

Expanding a little further, maybe we do not need to differentiate so harshly between Direct and Inherited or maybe we do. But I believe we definitely want users of Policies to know:

1. What is the resource kind ultimately being targeted (regardless of what's in the targetRef)?
2. What kinds of resources are allowed in a targetRef?
3. Does the Policy kind support multiple targetRefs?
4. Does the Policy kind support sectionName in the targetRefs?
5. Can multiple Policies of a kind target a same resource/sectionName (directly or indirectly)?
6. Does the Policy kind do merges?

Should a distinction between Direct and Inherited really be needed, I see 2 possible paths from here:

• A: We stay with the current definitions of Direct and Inherited, and we answer the above trying not to change nor extend the current definitions if possible, maybe only fill the gaps.
• B: We redefine Direct vs Inherited from the above, even if we end up sticking with parts of the current definitions afterwards.

As a final note, I want to acknowledge the distinction between Direct and Inherited for the practicality of promoting and battle testing these concepts independently. But since now we may be entering a bit of a gray zone between what's one thing and what's the other, we may want to restate the characteristics of Policies in general at a more granular level.

[guicassolato]

On more discussion thread for:

Constraints: Policies that specify boundaries to what can be declared in the target objects and/or in other Policies of the same kind affecting the same target objects

Literally copying the definition from Kuadrant:

specialization of an override that, rather than declaring concrete values, specify higher level constraints (e.g., min value, max value, enums) for lower level values, with the semantics of "clipping" lower level values so they are enforced, in an override fashion, to be the boundaries dictated by the constraints; typically employed for constraining numeric values and regular patterns (e.g. limited sets)

The typical use case is:

As a platform engineer, I want to set an Inherited Policy at a Gateway that specifies a max/min/enum value allowed at a given field of the object specs ultimately affected by my Policy or of other Policies, lower in the hierarchy, ultimately targeting the same object affected by my Policy, where those other lower object specs and Policies are controlled by other users, such as an App Developer who owns a Route attached to my Gateway and/or who writes a Policy targeting this Route beneath my Gateway. The max/min/enum value I specify in my Policy is not the value itself, nor a default value to be enforced in case not specified at the lower level; it is a limit (boundary) for what can be specified at the lower level by the owners of these other resources.

Kuadrant's idea to implement this (currently under scrutiny) is to combine an override with conditional application of the Policy. Here's an example: https://docs.kuadrant.io/0.7.0/architecture/rfcs/0009-defaults-and-overrides/#examples-e-override-policy-rules-setting-constraints-to-other-at-lower-level

[maleck13]

allow plural 'targetRefs' instead of targetRefs with maxLen of ~16?

I think that in this vein it would also be ideal to have as part of targetRef a way to exclude a certain listener without needing to name each section name individually. So some form of sectionName selector that allows you to express a set listeners to target (perhaps via a pattern) and exclude others concisely.

This would be similar or could re-use the concept of labels and label selectors

[guicassolato]

Prior art: https://gateway.envoyproxy.io/docs/api/extension_types/#targetselector

[mikemorris]

(perhaps via a pattern)

...makes me think the ask here would actually be closer to evaluating something like matchExpressions against Listener name keys. A simpler matchLabels would be more concise than partially-duplicative parentRefs to select multiple Listeners on a single Gateway, but I don't think would actually enable patterns like e.g. applying rate-limiting policy multiple Listeners on a Gateway but only those whose name contains metered-

[youngnick]

I can see how that would be useful, but it's also really important to remember complexity budget here. What happens when you have multiple policies targeting multiple Gateways with multiple Listeners, where each Policy selects some subset of listeners based on the listener name field? That puts us back into "gee we better have a good Status solution" territory.

[maleck13]

@youngnick I agree on the complexity problem, but I wonder does the complexity just get shifted though to the user without it? As in they have to create a policy per listener or hey have to maintain the list of all sectionNames minus the one they don't want to target?