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

grpc-proxy: add option to allow non empty CN #11974

Closed
wants to merge 1 commit into from

Conversation

Sh4d1
Copy link

@Sh4d1 Sh4d1 commented Jun 4, 2020

In some cases, grpc-proxy can be used with CN based auth.

Fixes #11970

Signed-off-by: Patrik Cyvoct patrik@ptrk.io

In some cases, grpc-proxy can be used with CN based auth.

Fixes etcd-io#11970

Signed-off-by: Patrik Cyvoct <patrik@ptrk.io>
@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2020

Codecov Report

Merging #11974 into master will increase coverage by 1.04%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11974      +/-   ##
==========================================
+ Coverage   66.34%   67.38%   +1.04%     
==========================================
  Files         403      403              
  Lines       37245    39044    +1799     
==========================================
+ Hits        24709    26309    +1600     
- Misses      11017    11113      +96     
- Partials     1519     1622     +103     
Impacted Files Coverage Δ
etcdmain/grpc_proxy.go 16.11% <50.00%> (+0.25%) ⬆️
client/keys.go 69.34% <0.00%> (-22.12%) ⬇️
clientv3/naming/grpc.go 63.15% <0.00%> (-12.29%) ⬇️
etcdmain/gateway.go 24.13% <0.00%> (-9.74%) ⬇️
auth/simple_token.go 81.51% <0.00%> (-7.57%) ⬇️
clientv3/leasing/util.go 91.66% <0.00%> (-6.67%) ⬇️
client/client.go 70.58% <0.00%> (-2.95%) ⬇️
etcdserver/storage.go 57.77% <0.00%> (-1.20%) ⬇️
etcdserver/cluster_util.go 64.88% <0.00%> (-0.89%) ⬇️
etcdserver/api/v2store/store.go 88.57% <0.00%> (-0.01%) ⬇️
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 732df43...68272be. Read the comment docs.

@stale
Copy link

stale bot commented Sep 2, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 2, 2020
@Sh4d1
Copy link
Author

Sh4d1 commented Sep 2, 2020

@tangcong maybe?

@stale stale bot removed the stale label Sep 2, 2020
@ptabor
Copy link
Contributor

ptabor commented Sep 7, 2020

Suggested how to fix that problem in: #11970 (comment)
I think we shouldn't introduce new public flags and handle it internally.

@tangcong
Copy link
Contributor

tangcong commented Sep 7, 2020

I will take a closer look later, this issue(Jun 4) does not seem to be caused by pr 12114 that is submitted in July. I agree with your idea, currently healthcheck is not optimal. @ptabor

@ptabor
Copy link
Contributor

ptabor commented Sep 13, 2020

The fix in #12273 has been merged. I assume the problem is solved and we can close this PR now, as stated in #11970 (comment). Thank you.

@Sh4d1
Copy link
Author

Sh4d1 commented Sep 13, 2020

True my bad 😄

@Sh4d1 Sh4d1 closed this Sep 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Grpc proxy with tls auth
4 participants