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

chore: move --upstream-force-h2c out of --insecure-listen-address #139 #140

Merged
merged 2 commits into from
Sep 20, 2021
Merged

chore: move --upstream-force-h2c out of --insecure-listen-address #139 #140

merged 2 commits into from
Sep 20, 2021

Conversation

andylibrian
Copy link
Contributor

@andylibrian andylibrian commented Aug 17, 2021

Upstream-force-h2c is a flag for upstream, while insecure-listen-address is for the proxy endpoint. Both can work
separately. One use case is having a secure terminating endpoint with kube-rbac-proxy while having an insecure upstream.

Signed-off-by: Andy Librian andylibrian@gmail.com

ref: #139

Upstream-force-h2c is a flag for upstream, while
insecure-listen-address is for the proxy endpoint. Both can work
separately. One use case is having a secure terminating endpoint with
kube-rbac-proxy while having an insecure upstream.

Signed-off-by: Andy Librian <andylibrian@gmail.com>
@s-urbaniak
Copy link
Collaborator

@andylibrian loooks great so far 👍 do you think it Is feasible to add an e2e test for this? Potentially we could contribute to https://github.com/brancz/prometheus-example-app to allow h2c requests.

@andylibrian
Copy link
Contributor Author

andylibrian commented Aug 17, 2021

I'm ok to add a test. This would be a new directory (something like h2c-upstream) along with existing basics, tokenrequest, etc. Sounds good to you?

@s-urbaniak
Copy link
Collaborator

@andylibrian that would be perfect, yes

andylibrian added a commit to andylibrian/prometheus-example-app that referenced this pull request Aug 17, 2021
ref: brancz/kube-rbac-proxy#140

Signed-off-by: Andy Librian andylibrian@gmail.com
@andylibrian
Copy link
Contributor Author

@s-urbaniak I opened a PR in the example repo: brancz/prometheus-example-app#11

Do you mind taking a look?

@brancz
Copy link
Owner

brancz commented Aug 31, 2021

Merged and pushed the new example app version

@s-urbaniak
Copy link
Collaborator

@brancz thank you! 🙏 @andylibrian how do you feel finalizing this here? :)

@andylibrian
Copy link
Contributor Author

Thanks @brancz!

@s-urbaniak, I'm still keen to add the test, hopefully next week

@s-urbaniak
Copy link
Collaborator

@andylibrian perfect, thank you and no rush! 🙏

@andylibrian
Copy link
Contributor Author

@s-urbaniak I've added e2e test for h2c upstream. Do you mind taking a look?

@andylibrian
Copy link
Contributor Author

The failing golint seems to be flakey:

run golangci-lint
  Running [/home/runner/golangci-lint-1.42.1-linux-amd64/golangci-lint run --out-format=github-actions] in [] ...
  level=error msg="Timeout exceeded: try increasing it by passing --timeout option"

Can we restart it?

@s-urbaniak
Copy link
Collaborator

@andylibrian i didn't do a thorough review yet, but do you mind fixing the linting issue first?

Signed-off-by: Andy Librian <andylibrian@gmail.com>
@andylibrian
Copy link
Contributor Author

@s-urbaniak I ran golangci-lint run locally on this branch and it passed. I think the build was flaky. I was requesting to restart the build manually, but I went ahead with force pushing this branch hoping that it triggered another build.

@s-urbaniak
Copy link
Collaborator

@andylibrian thank you! We will have a look and I'll merge once e2e is green 👍

@s-urbaniak s-urbaniak merged commit c03fe45 into brancz:master Sep 20, 2021
@andylibrian andylibrian deleted the move-upstream-force-h2c-out-of-insecure-listen-address branch September 21, 2021 22:44
@andylibrian
Copy link
Contributor Author

@s-urbaniak Thank you for merging it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants