Split GEP-713 into a Memorandum and separate GEPs for Direct and Inherited Policy

[youngnick]

/kind gep

What this PR does / why we need it:

This PR splits up GEP-713 (Metaresources and Policy Attachment), changing GEP-713 into a Memorandum about the general idea of Polkicy Attachment, with GEP-2648 and GEP-2649 being Direct Attached and Inherited Policy designs respectively.

The idea is to allow us to iterate on and graduate each type of Policy separately.

This is a big change, so I'll summarize the changes here:

• GEP-713 is now a Memorandum GEP
• GEP-2648 defines Direct Attached Policies and includes some changes to required and recommended status designs for that type
• GEP-2649 now holds the documentation about defaults and overrides, and what makes an Inherited Policy
• The definition of what makes a Policy Direct or Inherited has been updated:
  • Direct Policies now must only affect the targeted object
  • Inherited Policies are any Policy that affects objects other than the targeted one

Which issue(s) this PR fixes:
Updates #713
Updates #2648
Updates #2649

Does this PR introduce a user-facing change?:

The recommendations for the Policy Attachment pattern have been substantially updated and clarified.

I'm pretty sure this makes sense, but I've read this over so many times now that I can't see it properly, and I need review. Please feel free to ping me about this to discuss, or bring it to #sig-network-gateway-api or the community meeting.

[youngnick]

I've merged in #2442's changes to the Direct Attached Policy document.

[youngnick]

And also fixed @gcs278's comments. Thanks!

[gcs278]

+1 for the revised GEP organization. I couldn't find any comments other than superficial markdown rendering issues and some minor typos. Reviewing these GEP updates helps me prepare for getting the Session Persistence Policy Attachment GEP through. Hope this helps.

[youngnick]

I've got two TODOs left that I will get to early next week, both are relatively minor though, so please review away.

- Add changelog including the original PRs plus Flynn's one
- Update example to be BackendTLSPolicy

These TODOs are now done, this is ready for final review.

[howardjohn]

I don't think its clear what it means to "only affect the single object". I don't feel like there are any definitions for what the scope of such objects are.

To make things more concrete, what if I apply a BackendTimeoutPolicy to a Gateway. What are the implications here?

[youngnick]

If the backend timeouts are set per-backend, then it's an Inherited Policy, because there's no backend config in Gateway.

The intent here is to tightly constrain what a Direct Attached Policy is so that we don't have to boil the ocean of finding a good way to tell everyone whose objects are affected by a policy about that in the status of all the resources.

It's also about ensuring that we don't have "Direct Attached" Policy objects that effectively set defaults, even if the defaults stanza isn't used.

[youngnick]

I added some more text here.

[howardjohn]

The goals make sense,I am just trying to fully grok the meaning of this since it seems like this is an important aspect

What does it mean to have a timeout set per-backend?

I can say logically that a timeout seems to apply to a backend in my mental model. And maybe in proxy implementations. But I could have an implementation that has one global value "timeout", and Gateway is the only logical place to put that policy in that implementation. Is that ok, or does that violate this?

What if my data plane implementation does happen to have per-backend config, but I don't want to deal with it so I only support gateway level config - is that ok?

More broadly, what exactly defined what is the scope of impact of an object? Is it the ability to attach a single type to multiple places? Is it the implementation details of how a policy is implemented? Is it a users mental model of how it works?

[robscott]

I think this question overlaps with the session persistence GEP @gcs278 has been working on (#2634). In that case we've found that some implementations (Envoy or GRPC-based in particular) can only attach this config at Route level. So even if a user attaches the config at the Backend layer, the underlying implementation is going to be implementing it at the Routing layer. Would that go against this guideline?

Similarly, that GEP currently proposes that Routes will include inline overrides that could override the values specified on a Service. That's not really policy at that point, but it is some config overriding a direct-attached policy which I believe would be a new concept for us.

To be clear, I actually like the approach that GEP is proposing, I'm just trying to make sure that the corresponding changes we're making here can line up with that so we don't end up with conflicting GEPs.

[howardjohn]

Do we need take in policies that are additive? They don't make sense to have defaults and overrides, but do target multiple layers. Authorization policies are likely to fall info this category

[robscott]

🤔 that's a good point. Are you referring to policies that essentially just have implicit defaulting behavior all the way down the stack? For example:

policy attached to GW:

foo: 1
bar: 2

policy attached to Route:

bar: 4
baz : 3

computed result:

foo: 1
bar: 4
baz: 3

[howardjohn]

I was thinking:
GatewaY:

rules: [a]

Route:

rules: [b]

Result: rules: [a,b]

[youngnick]

At the end of the day, what I'm trying to do is make it clear that Direct Policies have an easier time with status. If you do any cross object inheritance as mentioned in the previous two comments, those count as inherited, because you have to solve the Inherited status problem to have full status.

"Direct Policies can be attached at exactly one layer" is a good start, but if they have knock-on effects further down the hierarchy, then again, they have the same status problem as Inherited Policies.

Maybe I just make it more explicit here:

• Direct Policies can be attached at exactly one layer
• Direct Policies can have effects only on the layer they attach to
• Direct Policies can effects only on the object withing the layer that they attach to

This stops the "but my Direct Policy affected another Route attached to the same Gateway when attached to a Route" problem which could arise if we only had the first two items.

[youngnick]

Oh, and @howardjohn, additive policies being Inherited is definitely intended, that's one of the things I was worried that the original definition missed. That kind of implicit default still has all the status problems of a more explicit Inherited policy with a defined defaults section, so I wanted to make sure that we caught that in the definitions.

[youngnick]

Turned out I had missed some of @guicassolato's great feedback. Fixed now!

[candita]

Annotation or label?

[youngnick]

I've updated to label, although I originally chose annotation because these are machine-generated and machine-added. I updated because it seems useful to be able to search objects to find policy affected ones (which you can't do with annotations).

[candita]

No section on targeted resources? If it's not possible or not recommended, then can you add a note with the same title as GEP-2648? Same for PolicyAncestorStatus or a discussion of why it isn't applicable?

[youngnick]

I've been trying to keep this PR as small as possible, since it's already enormous. But I'll add some updates to the status sections to roll your and @robscott's feedback in.

[youngnick]

I added some information suggesting the PolicyAffected condition, as in Direct Attached Policy, but I want us all to spend some time considering apiserver fanout load concerns before going ahead with any more recommendations here.

[kate-osborn]

What does "capabilities" mean here? Can you include an example?

[youngnick]

Clarified and added.

[youngnick]

As discussed in #2927, I've marked GEP-2658 and GEP-2649 as provisional, and included notices at the top of each file that the file may change soon.

Hopefully this lets us get this one across the line for now, PTAL @robscott @howardjohn @mikemorris @kflynn @guicassolato @kate-osborn.

[mikemorris]

@candita I don't think this clarification actually solves for your use case, where, if I understand correctly, you're trying to select multiple backendRef matches of the same named object when referenced by multiple rules in the same HTTPRoute?

Refs #2813 (comment), #2813 (comment) and the implementation of BackendTLSPolicy in #2448.

I'm unclear from the implementation whether that policy is intended to directly target a Service object, or instead as it seems you're implying, to target a Service only in its capacity as a backendRef for a specific HTTPRoute object.

After reading through the merged GEP-1897 it feels pretty clear that the intended usage of BackendTLSPolicy is to target a Service directly, and sectionName would only be for scoping the policy to a specific port on a multi-port Service. If there's a need to narrow the scope of which Gateways, HTTPRoutes or HTTPRouteRules should respect a BackendTLSPolicy, I think we would want to add something like a from clause as suggested in #2813 (comment)

[youngnick]

I added a "SHOULD NOT use backendRef" above, which should clear this up, I think.

[candita]

@mikemorris I wanted information on what was considered a SectionName in the object. I heard from Rob that we have known SectionNames for each object. I do hope we have this documented somewhere.

[mikemorris]

Opened #2946 to improve documentation for SectionName

[robscott]

Thanks for all the work on this @youngnick!

I think this is a great update that represents where we are now, but also really appreciate you starting the broader discussion in #2927 to cover what's next (and linking it from all the GEPs here).

[robscott]

but I believe the same name can be used in different stanzas that are different objects, which breaks the application of policy to section name. For example, couldn't I have a rule and a backendRef with the same name

Although rule names on Routes are still provisional (x-ref #2593), so far we've clearly defined what sectionName means within each Gateway API resource it's relevant, and I think the same would apply to Routes. I think sectionName would be used exclusively to refer to rule names, and that rule names would need to be unique within a Route when they were specified. I think the discussion in #2927 could also lead to the ability to have multiple targets for the same policy.

The idea of what I'd call multi-dimensional targets (only x within y) is interesting but I'd rather hold off on that until we've got a better handle on policy attachment targeting a single resource.

[robscott]

When we do have nesting/namespacing available like Routes, can we suggest that implementations populate a unique section for their controller instead of adding conditions to an existing set of conditions?

[robscott]

controllers are supposed to update Conditions lists additively, putting back any Condition they don't understand or update as they found it

This seems to require a controller to be aware of any condition it may have theoretically set in the past instead of assuming full ownership of the conditions and replacing with the current set of conditions it knows about. I generally would prefer some form of namespacing/nesting in status as broadly as possible that allows each controller to clearly own separate lists. This PR doesn't need to solve this, but it would be nice to have some kind of note somewhere in the GEP covering this as a follow up area and/or unresolved.

[mikemorris]

Appreciate all the work you've put into this @youngnick, and sorry I haven't been more involved earlier to help support! This feels like it's in adequate shape to get in as provisional while we continue to work on refining the structure over in #2927

[shaneutt]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mikemorris, robscott, shaneutt, youngnick

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [robscott,shaneutt,youngnick]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[robscott]

/hold cancel