-
Notifications
You must be signed in to change notification settings - Fork 70
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
Expose client http scheme as an env variable #58
Conversation
ce58042
to
691177f
Compare
691177f
to
8276bf9
Compare
Download the artifacts for this pull request: Or use the following Docker image |
@aymanbagabas thanks for the fixes! one question I have, as I've been testing the branch. I have to
Which means the fact that I have charm behind a proxy/tls terminator isn't fully transparent to the client. Is that correct? |
Yes, that's correct. Since you're using Caddy for tls termination, the charm server should run without tls (http). But you still need to instruct the charm client to use https to communicate properly with the server. |
@aymanbagabas would it be useful/possible to have the client always use https by default unless instructed otherwise? or even something like the patch suggested in #37, so the client does the protocol upgrade automatically without user intervention? once you distribute the client you also need to make clients/users aware that they need to Note: I think having the charm server designed to be the TLS termination is totally fine, I don't expect it to change because someone decided to complicate its life putting it behind a proxy 😉. |
As an alternative to my hacky patch in #37, I've been testing adding a diff --git a/client/client.go b/client/client.go
index 4fa77d6..8187e59 100644
--- a/client/client.go
+++ b/client/client.go
@@ -25,12 +25,10 @@ var nameValidator = regexp.MustCompile("^[a-zA-Z0-9]{1,50}$")
// Config contains the Charm client configuration.
type Config struct {
- Host string `env:"CHARM_HOST" default:"cloud.charm.sh"`
- SSHPort int `env:"CHARM_SSH_PORT" default:"35353"`
- HTTPPort int `env:"CHARM_HTTP_PORT" default:"35354"`
- HTTPScheme string `env:"CHARM_HTTP_SCHEME" default:"https"`
- Debug bool `env:"CHARM_DEBUG" default:"false"`
- Logfile string `env:"CHARM_LOGFILE" default:""`
+ ServerURL string `env:"CHARM_SERVER_URL" default:"https://cloud.charm.sh:35354"`
+ SSHPort int `env:"CHARM_SSH_PORT" default:"35353"`
+ Debug bool `env:"CHARM_DEBUG" default:"false"`
+ Logfile string `env:"CHARM_LOGFILE" default:""`
} |
I realized today that the new JWKS URI will also be invalid when external TLS termination or proxying (docker) is happening, since the server scheme could be I'm solving it with a CHARM_SERVER_PUBLIC_URL for now, that could potentially be used by the client I guess. |
Add the ability to set the client HTTP scheme and default to the server's scheme when the environment variable is not provided.
Fixes: #37