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

Adding insecure server boolean to chart. #83

Merged
merged 6 commits into from
Apr 21, 2020

Conversation

bjoernw
Copy link
Contributor

@bjoernw bjoernw commented Apr 19, 2020

The chart was lacking support for a way to run pomerium in insecure mode. My use case is that I would like to fold pomerium into my istio mesh and since istio already handles mutual-tls I did not need/want pomerium to do that as well. Since pomerium itself already supports an insecure mode I just needed to expose this toggle in the chart.

However, just setting environment variables INSECURE_SERVER and GRPC_INSECURE wasn't enough to get past the failing liveness and readiness checks which still had a scheme of "HTTPS" regardless of the value of INSECURE_SERVER or GRPC_INSECURE.

Here I create a single flag config.insecure that abstracts those details away.

Let me know what you think.

@CLAassistant
Copy link

CLAassistant commented Apr 19, 2020

CLA assistant check
All committers have signed the CLA.

Signed-off-by: Bjoern Weidlich <bweidlich@ripple.com>
desimone
desimone previously approved these changes Apr 19, 2020
Copy link
Contributor

@desimone desimone left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the contribution @bjoernw

#### DANGER: You have disabled TLS!
####
#### Please only do this if you absolutely know what you are doing.
##############################################################################
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Love the disclaimer.

@desimone desimone added the enhancement New feature or request label Apr 19, 2020
@desimone
Copy link
Contributor

@bjoernw just needs a version bump

@bjoernw
Copy link
Contributor Author

bjoernw commented Apr 20, 2020

@desimone What version do you want me to bump this to? 8.3.0? Should I capture my change in the changelog in the readme?

@desimone
Copy link
Contributor

@desimone What version do you want me to bump this to? 8.3.0?

Seems good to me, going to ping @travisgroth to make sure this is a minor not a major version bump. I don't think it breaks existing so... should be fine.

Should I capture my change in the changelog in the readme?

Please do!

Signed-off-by: Bjoern Weidlich <bweidlich@ripple.com>
@bjoernw
Copy link
Contributor Author

bjoernw commented Apr 21, 2020

@desimone bumped it to 8.4.0 and added changelog. Looks good?

Copy link
Contributor

@desimone desimone left a comment

Choose a reason for hiding this comment

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

One small typo, otherwise LGTM!

charts/pomerium/README.md Outdated Show resolved Hide resolved
Bjoern Weidlich added 3 commits April 21, 2020 11:16
Signed-off-by: Bjoern Weidlich <bweidlich@ripple.com>
Signed-off-by: Bjoern Weidlich <bweidlich@ripple.com>
@desimone
Copy link
Contributor

Thanks @bjoernw !

@desimone desimone merged commit ef518d5 into pomerium:master Apr 21, 2020
@bjoernw bjoernw deleted the adding_inscure_toggle branch April 21, 2020 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants