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.

Sign up for GitHub

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

Jump to bottom

feat: basic auth API #2199

Merged
merged 7 commits into from Nov 17, 2023
Merged

feat: basic auth API #2199

merged 7 commits into from Nov 17, 2023

Conversation

zhaohuabing
Copy link
Member

@zhaohuabing zhaohuabing commented Nov 16, 2023 •
edited

Related to: #1660

This PR adds API for HTTP Basic Auth:

- apiVersion: gateway.envoyproxy.io/v1alpha1
  kind: SecurityPolicy
  metadata:
    name: basic-auth
    namespace: default
  spec:
    basicAuth:
      users:
        name: "htpasswd-secret"
    targetRef:
      group: gateway.networking.k8s.io
      kind: HTTPRoute
      name: httproute-1

@zhaohuabing zhaohuabing requested a review from a team as a code owner November 16, 2023 16:20
@codecov Codecov
Copy link

codecov bot commented Nov 16, 2023 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (94fe41c) 64.19% compared to head (9fd58c0) 64.21%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2199      +/-   ##
==========================================
+ Coverage   64.19%   64.21%   +0.02%     
==========================================
  Files         107      107              
  Lines       14924    14924              
==========================================
+ Hits         9580     9583       +3     
+ Misses       4768     4766       -2     
+ Partials      576      575       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zhaohuabing
basic auth API
023d83a 
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
@zhaohuabing
use secret to store users
667a3d3 
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
@zhaohuabing
address comments
97538d8 
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
@zhaohuabing
address comments
16e3a69 
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
@zhaohuabing
fix test
2f9bcbe 
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
@zhaohuabing
Copy link
Member Author

@saltbo

arkodg
arkodg reviewed Nov 16, 2023
View reviewed changes
api/v1alpha1/basic_auth_types.go
// Right now, only SHA hash algorithm is supported.
// Reference to https://httpd.apache.org/docs/2.4/programs/htpasswd.html
// for more details.
Users gwapiv1b1.SecretObjectReference `json:"users"`
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer if this field is called Htpasswd

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer Users because it's a user list, htpasswd is just the file format. The field should be named after its meaning instead of format.

Envoy also uses users in the HTTP Basic Auth filter config.

https://github.com/envoyproxy/envoy/blob/6d8c70b030700950503ea4e56b706e33bb76f934/api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto#L35

@arkodg
Copy link
Contributor

arkodg commented Nov 16, 2023

ptal @envoyproxy/gateway-maintainers

arkodg
arkodg previously approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

@arkodg arkodg requested review from a team, LanceEa, chauhanshubham and Xunzhuo and removed request for a team November 16, 2023 20:26
@zhaohuabing
address comments
a974984 
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
@zhaohuabing
fix generate
9fd58c0 
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
@zhaohuabing
Copy link
Member Author

/retest

@zirain zirain merged commit 1789d98 into envoyproxy:main Nov 17, 2023
20 checks passed
@zhaohuabing zhaohuabing mentioned this pull request Nov 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zirain zirain zirain approved these changes

@arkodg arkodg arkodg approved these changes

@LanceEa LanceEa Awaiting requested review from LanceEa LanceEa was automatically assigned from envoyproxy/gateway-reviewers

@chauhanshubham chauhanshubham Awaiting requested review from chauhanshubham chauhanshubham was automatically assigned from envoyproxy/gateway-reviewers

@Xunzhuo Xunzhuo Awaiting requested review from Xunzhuo Xunzhuo was automatically assigned from envoyproxy/gateway-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@zhaohuabing @arkodg @zirain