Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: GEP-1494 #3293

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

doc: GEP-1494 #3293

wants to merge 1 commit into from

Conversation

jgao1025
Copy link
Contributor

@jgao1025 jgao1025 commented Aug 25, 2024

What type of PR is this?

/kind documentation

What this PR does / why we need it:

#1494

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:
None

@k8s-ci-robot
Copy link
Contributor

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Aug 25, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jgao1025
Once this PR has been reviewed and has the lgtm label, please assign youngnick for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 25, 2024
@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/gep PRs related to Gateway Enhancement Proposal(GEP) labels Aug 25, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @jgao1025. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 25, 2024
@k8s-ci-robot
Copy link
Contributor

@jgao1025: The label(s) kind/draft cannot be applied, because the repository doesn't have them.

In response to this:

/kind draft

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@jgao1025
Copy link
Contributor Author

/hold

@jgao1025 jgao1025 changed the title doc: GEP-1494 DRAFT: doc: GEP-1494 Aug 25, 2024
@jgao1025 jgao1025 changed the title DRAFT: doc: GEP-1494 doc: GEP-1494 Aug 25, 2024
@jgao1025 jgao1025 marked this pull request as draft August 25, 2024 12:25
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 25, 2024
Copy link
Contributor

@youngnick youngnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jgao1025, this is a really good start. I should also note that getting the tone and goals right for a GEP is a seriously hard task, so please don't feel discouraged by the volume of feedback. I think that you understand the ask, it's just a matter of writing the document so that it's clear for future readers what this is all about.


## TLDR

Allow authentication works as a policy that can be attached to both the Gateway and HTTPRoute.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although most of the discussion to date has been about a Policy object, I think that we should leave the method open until we get to the design phase, so we need to try and phrase this more like a user journey or story.

Maybe something more like "Provide a method for configuring Gateway API implementaions to add Authentication for north-south traffic. The method may also include Authorization config if practical".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although most of the discussion to date has been about a Policy object, I think that we should leave the method open until we get to the design phase, so we need to try and phrase this more like a user journey or story.

Maybe something more like "Provide a method for configuring Gateway API implementaions to add Authentication for north-south traffic. The method may also include Authorization config if practical".

Hi Nick, thanks for your insightful comments! I initially viewed this document as a reference for developers to implement the code, but now it seems more aligned with the storytelling phase. Could you clarify which stage you're referring to as the 'design phase' and the current stage in the GEPs process document [1]?

[1]. https://gateway-api.sigs.k8s.io/geps/overview/#process

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, great point, maybe we haven't explained this very well in that doc. The design phase I'm referring to there is the phase between Provisional and Implementable - when we actually design the API to be used and have it ready for implementations to do the work to actually implement it (which is the Implementable state).

The aim of the Provisional state is to outline the what, who, and why, along with any relevant background information. This is somewhat similar to the story generation phase of Agile development, but with a slightly larger scope (because we need to record the background as well).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provide a method for configuring Gateway API implementaions to add Authentication for north-south traffic. The method may also include Authorization config if practical

Okay. Got it. Does this GEP also include the gRPC protocol? I will revise this to read "Provide a method for configuring Gateway API implementaions to add HTTP Authentication for north-south traffic. The method may also include Authorization config if practical". Does it sound good?

Comment on lines +14 to +16
1). In the default policy, it can add an ExtensionFilter that specifies the auth so that Gateway can be attached to.
2). Individual HTTPRoutes can choose to opt out by specifying an empty extension, or a null extension.
3). It can also have disabled authentication.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I think it's important to focus on the user story rather than details.

The thing that users (well, HTTPRoute owners) want to be able to do is to ask their Gateway API implementation to ensure that requests matching some pattern (such as a path like /admin) all have auth added if not present before requests are allowed to be forwarded to the backend.

So I think that we need a very high-level definition of that goal in here as the first item.

I think the second point is a really good one - that HTTPRoute owners must also be able to require that certain route rules do not require authentication.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think opting out (at the level of HTTPRoutes) on a config otherwise enforced as a default (set at the level of the Gateway) is a need that extrapolates the auth domain, but rather a pattern that we may see at any kind of hierarchical config.

I would dedicate some careful thought on this, possibly out of the scope of this GEP.

Prior art: Kuadrant specifies the concept of Unsetting for its AuthPolicy and RateLimitPolicy kinds.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think opting out (at the level of HTTPRoutes) on a config otherwise enforced as a default (set at the level of the Gateway) is a need that extrapolates the auth domain, but rather a pattern that we may see at any kind of hierarchical config.

I would dedicate some careful thought on this, possibly out of the scope of this GEP.

Prior art: Kuadrant specifies the concept of Unsetting for its AuthPolicy and RateLimitPolicy kinds.

My proposal is to implement one of the following options for authentication (AuthN): a default policy, an override policy, or a disabled policy. I want to keep it simple, but I’m wondering if I should consider making it more complex...


## Non-Goals

1). How authentication is implemented.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the implementation of the authentication will eventually be required in this GEP, so it isn't really correct to put this in here.

Maybe, let's add a ## Deferred Goals section for now, to mark goals that we'll get to later in this process. (I think I'll open a PR to the GEP template to add this as well).

Comment on lines +26 to +34
The Gateway API aims to establish a centralized authentication framework that impacts both the Gateway and HTTPRoute. This solution is designed to achieve the following objectives:

1). The Gateway API should support multiple authentication methods, including Multi-Platform Authentication, traditional Authentication, and no authentication at all. Multi-Platform Authentication includes Single Sign-On (SSO), OAuth, JWT tokens, and more, while traditional Authentication involves the use of usernames and passwords.

2). The authentication policy attachment mechanism should work at both the Gateway and HTTPRoute levels. The following conditions are considered:

- Inherited Policy attachment: attach the Policy to a Gateway and default auth settings (but still allow them to be overridden by individual HTTPRoutes)
- Direct Policy attachment: attach the Policy to a Gateway and override auth settings (which will prevent them being changed on individual HTTPRoutes)
- Not have a Policy at all and just set the settings on individual HTTPRoutes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really great start and definitely information we need to consider, but I think that we again need to go a bit higher-level for this Introduction section.

What this section is about is giving someone looking at this GEP some reasons why we are adding this feature. What do users want and why is it important that it's handled in Gateway API? (For this case, it's something like "it's really useful to be able to take having to worry about authentication off application developer's todo lists, and handle it correctly for them with a minimum of provided config").

Then, the other really important part is to work through what the state of the area looks like, reviewing both data planes (proxies) and what Implementation Specific things Gateway API implementations are doing in this area.

## Non-Goals

1). How authentication is implemented.
2). No authorization is required in this GEP.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this goal isn't leaking some current approaches of separating authentication and authorisation into completely sets of configuration. This is not necessarily a bad practice but tends to lead to additional requirements on how to export and data (typically user info) from one layer to the other. After all, there's only so much use case for authorisation completely disconnected from authentication.

I can see this separation eventually bumping into requests for defining, e.g., a principal entity that "magically" becomes available in an authorisation policy/filter, knowing an authentication step was enforced before. Similarly, authorisation may depend on jwt under the assumption that a JSON Web Token has been validated already.

At the very least, it open room for discussing sequencing/dependencies in the enforcement of different configs.

I would recommend checking out Kuadrant's AuthPolicy and Envoy Gateway's SecurityPolicy as a references (prior art). Both see auth as a rather unified domain, I reckon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this goal isn't leaking some current approaches of separating authentication and authorisation into completely sets of configuration. This is not necessarily a bad practice but tends to lead to additional requirements on how to export and data (typically user info) from one layer to the other. After all, there's only so much use case for authorisation completely disconnected from authentication.

I can see this separation eventually bumping into requests for defining, e.g., a principal entity that "magically" becomes available in an authorisation policy/filter, knowing an authentication step was enforced before. Similarly, authorisation may depend on jwt under the assumption that a JSON Web Token has been validated already.

At the very least, it open room for discussing sequencing/dependencies in the enforcement of different configs.

I would recommend checking out Kuadrant's AuthPolicy and Envoy Gateway's SecurityPolicy as a references (prior art). Both see auth as a rather unified domain, I reckon.

Yes. I think that authentication and authorization are generally regarded as separate entities across various domains. I personally would want to see such seperation. This separation allows security teams to do better control and enforcement over ACL services in authorization, such as implementing a least privilege policy.

However, I do realise that this approach may introduce an additional layer of complexity, but I think at the current stage, we are more than just to talk different possiblities than to think of implement these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/documentation Categorizes issue or PR as related to documentation. kind/gep PRs related to Gateway Enhancement Proposal(GEP) needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants