-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
New entry point parser. #2248
New entry point parser. #2248
Conversation
f97ba7f
to
70e56b4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ldez This PR will be very helpful!
Few suggestions. Let me know WDYT about them.
config[name] = field[1] | ||
} else { | ||
if strings.EqualFold(name, "TLS") { | ||
config["tls_acme"] = "TLS" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TLS should be in lower case (re use a possible constant?)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this case, this must be in Upper case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
|
||
var configTLS *TLS | ||
if len(result["TLS"]) > 0 { | ||
if len(result["tls"]) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT about using constants for all fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use constant for 2 strings seems overkill
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm talking about all the strings used in entrypoint definition (tls
, redirect_entrypoint
...) because they are used in many places, in many files.
IMHO, it's easier to modify one place.
"redirect_replacement": "RedirectReplacement", | ||
"whitelistsourcerange": "WhiteListSourceRange", | ||
"proxyprotocol_trustedips": "192.168.0.1", | ||
"compress": "true", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re use constants? 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @ldez 😻
LGTM
70e56b4
to
bfe9ab6
Compare
Description
Fix a problem when you write
--entryPoints='Name:http Address::8080'
(you must add a space at the end :--entryPoints='Name:http Address::8080 '
)Side effect: