provide -admin-access-log-path #5858
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ type cmd struct { | |
| // flags | ||
| proxyID string | ||
| sidecarFor string | ||
| adminAccessLogPath string | ||
| adminBind string | ||
| envoyBin string | ||
| bootstrap bool | ||
|
|
@@ -67,6 +68,10 @@ func (c *cmd) init() { | |
| "The full path to the envoy binary to run. By default will just search "+ | ||
| "$PATH. Ignored if -bootstrap is used.") | ||
|
|
||
| c.flags.StringVar(&c.adminAccessLogPath, "admin-access-log-path", "/dev/null", | ||
| "The path to write the access log for the administration server. If no access "+ | ||
| "log is desired specify \"/dev/null\".") | ||
|
|
||
| c.flags.StringVar(&c.adminBind, "admin-bind", "localhost:19000", | ||
| "The address:port to start envoy's admin server on. Envoy requires this "+ | ||
| "but care must be taken to ensure it's not exposed to an untrusted network "+ | ||
|
|
@@ -258,13 +263,19 @@ func (c *cmd) templateArgs() (*BootstrapTplArgs, error) { | |
| cluster = c.sidecarFor | ||
| } | ||
|
|
||
| adminAccessLogPath := c.adminAccessLogPath | ||
| if len(adminAccessLogPath) == 0 { | ||
|
There was a problem hiding this comment. Would this be a little clearer with There was a problem hiding this comment. I always wonder about the same thing and I read the first answer of https://stackoverflow.com/questions/18594330/what-is-the-best-way-to-test-for-an-empty-string-in-go. If you find There was a problem hiding this comment. I think Thanks! |
||
| adminAccessLogPath = "/dev/null" | ||
| } | ||
|
|
||
| return &BootstrapTplArgs{ | ||
| ProxyCluster: cluster, | ||
| ProxyID: c.proxyID, | ||
| AgentAddress: agentIP.String(), | ||
| AgentPort: agentPort, | ||
| AgentTLS: useTLS, | ||
| AgentCAFile: httpCfg.TLSConfig.CAFile, | ||
| AdminAccessLogPath: adminAccessLogPath, | ||
| AdminBindAddress: adminBindIP.String(), | ||
| AdminBindPort: adminPort, | ||
| Token: httpCfg.Token, | ||
|
|
||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| { | ||
| "admin": { | ||
| "access_log_path": "/some/path/access.log", | ||
| "address": { | ||
| "socket_address": { | ||
| "address": "127.0.0.1", | ||
| "port_value": 19000 | ||
| } | ||
| } | ||
| }, | ||
| "node": { | ||
| "cluster": "test-proxy", | ||
| "id": "test-proxy" | ||
| }, | ||
| "static_resources": { | ||
| "clusters": [ | ||
| { | ||
| "name": "local_agent", | ||
| "connect_timeout": "1s", | ||
| "type": "STATIC", | ||
| "http2_protocol_options": {}, | ||
| "hosts": [ | ||
| { | ||
| "socket_address": { | ||
| "address": "127.0.0.1", | ||
| "port_value": 8502 | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| }, | ||
| "stats_config": { | ||
| "stats_tags": [ | ||
| { | ||
| "tag_name": "local_cluster", | ||
| "fixed_value": "test-proxy" | ||
| } | ||
| ], | ||
| "use_all_default_tags": true | ||
| }, | ||
| "dynamic_resources": { | ||
| "lds_config": { "ads": {} }, | ||
| "cds_config": { "ads": {} }, | ||
| "ads_config": { | ||
| "api_type": "GRPC", | ||
| "grpc_services": { | ||
| "initial_metadata": [ | ||
| { | ||
| "key": "x-consul-token", | ||
| "value": "" | ||
| } | ||
| ], | ||
| "envoy_grpc": { | ||
| "cluster_name": "local_agent" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
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.
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.
Should we just make /dev/null be the default so specifying empty is also valid? This is kinda internal only but we do a bunch of default setting before the template already so this seems like a reasonable approach here too.
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 made it so that it can also be left empty and then the default would be picked up.