Make configuration consistent #186
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
loshz
previously approved these changes
Jun 17, 2019
steakunderscore
requested changes
Jun 20, 2019
| @@ -29,7 +29,7 @@ import ( | |||
| // Options holds Configuration Options that can be set by Command Line Flag, | |||
| // or Config File | |||
| type Options struct { | |||
| ProxyPrefix string `flag:"proxy-prefix" cfg:"proxy-prefix" env:"OAUTH2_PROXY_PROXY_PREFIX"` | |||
| ProxyPrefix string `flag:"proxy-prefix" cfg:"proxy_prefix" env:"OAUTH2_PROXY_PROXY_PREFIX"` | |||
| ProxyWebSockets bool `flag:"proxy-websockets" cfg:"proxy_websockets" env:"OAUTH2_PROXY_PROXY_WEBSOCKETS"` | |||
| HTTPAddress string `flag:"http-address" cfg:"http_address" env:"OAUTH2_PROXY_HTTP_ADDRESS"` | |||
| HTTPSAddress string `flag:"https-address" cfg:"https_address" env:"OAUTH2_PROXY_HTTPS_ADDRESS"` | |||
There was a problem hiding this comment.
TLSCertFile and TLSKeyFile are missing the -file part in their flags.
(sorry comment is on the wrong line because github doesn't let comments happen in the right place)
There was a problem hiding this comment.
Very good spot, I had completely missed this, thanks! @syscll Do we change the flag or the cfg and env? 🤔
There was a problem hiding this comment.
My two cents are to shift to -tls-cert-file and -tls-key-file to be consistent with -htpasswd-file and -jwt-key-file. It just leaves the exception of -logging-filename, but it's a little bit exceptional in that it defaults to stdout. But I'm not strongly opinionated about which way to go.
karlskewes
reviewed
Jun 21, 2019
3 tasks
JoelSpeed
force-pushed
the
consistent-config
branch
from
6ceaa4c
to
874c147
Compare
|
@syscll @steakunderscore @kskewes I've fixed up all the comments on this PR now, are you all happy for me to merge? |
3 tasks
JoelSpeed
added a commit
that referenced
this pull request
Jul 17, 2019
Correct TLS Flags broken in #186
ploxiln
added a commit
to ploxiln/oauth2_proxy
that referenced
this pull request
May 26, 2020
config: * proxy-prefix -> proxy_prefix * google_group -> google_groups flags: * tls-cert -> tls-cert-file * tls-key -> tls-key-file flags always use dashes, config options always use underscores flags are singular if they can be specified multiple times, config options are plural if they take a list inspired by oauth2-proxy/oauth2-proxy#186 Co-authored-by: Joel Speed <[email protected]>
ploxiln
added a commit
to ploxiln/oauth2_proxy
that referenced
this pull request
May 27, 2020
config: * proxy-prefix -> proxy_prefix * google_group -> google_groups * github_team -> github_teams flags: * tls-cert -> tls-cert-file * tls-key -> tls-key-file flags always use dashes, config options always use underscores flags are singular if they can be specified multiple times, config options are plural if they take a list inspired by oauth2-proxy/oauth2-proxy#186 Co-authored-by: Joel Speed <[email protected]>
ploxiln
added a commit
to ploxiln/oauth2_proxy
that referenced
this pull request
May 27, 2020
config: * proxy-prefix -> proxy_prefix * google_group -> google_groups * github_team -> github_teams flags: * tls-cert -> tls-cert-file * tls-key -> tls-key-file flags always use dashes, config options always use underscores flags are singular if they can be specified multiple times, config options are plural if they take a list inspired by oauth2-proxy/oauth2-proxy#186 Co-authored-by: Joel Speed <[email protected]>
ploxiln
added a commit
to ploxiln/oauth2_proxy
that referenced
this pull request
May 27, 2020
config: * proxy-prefix -> proxy_prefix * google_group -> google_groups * github_team -> github_teams flags: * tls-cert -> tls-cert-file * tls-key -> tls-key-file flags always use dashes, config options always use underscores flags are singular if they can be specified multiple times, config options are plural if they take a list inspired by oauth2-proxy/oauth2-proxy#186 Co-authored-by: Joel Speed <[email protected]>
ploxiln
added a commit
to ploxiln/oauth2_proxy
that referenced
this pull request
May 27, 2020
config: * proxy-prefix -> proxy_prefix * google_group -> google_groups * github_team -> github_teams flags: * tls-cert -> tls-cert-file * tls-key -> tls-key-file flags always use dashes, config options always use underscores flags are singular if they can be specified multiple times, config options are plural if they take a list inspired by oauth2-proxy/oauth2-proxy#186 Co-authored-by: Joel Speed <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Some configuration options have been inconsistent in the past, this PR fixes that
Motivation and Context
Some options did not fit the pattern and I would like to make some future changes that automate some of this stuff, so having consistency will make this easier
Fixes #49
How Has This Been Tested?
It hasn't, but it's minor and shouldn't need testing
Checklist: