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

Support dynamic addresses for envoy proxy statsd_url configurations #8561

Closed
woz5999 opened this issue Aug 25, 2020 · 2 comments · Fixed by #8564
Closed

Support dynamic addresses for envoy proxy statsd_url configurations #8561

woz5999 opened this issue Aug 25, 2020 · 2 comments · Fixed by #8564
Labels
theme/kubernetes Consul-helm/kubernetes related questions theme/telemetry Anything related to telemetry or observability

Comments

@woz5999
Copy link
Contributor

woz5999 commented Aug 25, 2020

Feature Description

Either/or:

  • Provide the ability to resolve the statsd_url host address from an existing environment variable, e.g.
envoy_statsd_url: udp://${HOST_IP}:1234

Use Case(s)

envoy_statsd_url and envoy_dogstatsd_url must take an ip address and not a hostname. This presents problems when using daemonset statsd agents, because the config for each proxy must point to the local HOST_IP. The Connect Inject workflow does not provide any method to dynamically inject the appropriate address into the statsd_url configurations.

While the consul connect enoy command supports reading the entire config value from an environment variable, there's no meaningful way to populate such a variable in the injection sidecar container. The environment variable HOST_IP is already present in the container, but the dynamic env var configuration option requires that the target env var contain the entire configuration url, e.g. udp://ip:port.

The code here could be updated to support expanding environment variables in the config string. I believe this option would be the easiest to implement and I'd be happy to submit a PR with the changes. My only concern is if there was a reason this functionality was explicitly considered yet not implemented initially. From the docs: "It is not currently possible to use environment variables as only part of the URL." If there's a security concern or something, perhaps maintaining an allowlist of accepted env vars would be sufficient, e.g. only allow expansion of HOST_IP.

Alternatively, providing the ability to pass additional arbitrary env vars into the inject container would allow users to leverage the intended dynamic environment config option for these settings. This seems pretty messy and involved.

@jsosulska jsosulska added theme/kubernetes Consul-helm/kubernetes related questions theme/telemetry Anything related to telemetry or observability labels Sep 17, 2020
@woz5999
Copy link
Contributor Author

woz5999 commented Sep 29, 2020

@jsosulska what's the best way to get the PR for this reviewed?

@woz5999
Copy link
Contributor Author

woz5999 commented Oct 16, 2020

@mkeeler @rboyer what's the best way to get the PR for this reviewed?

dnephin pushed a commit to woz5999/consul that referenced this issue Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/kubernetes Consul-helm/kubernetes related questions theme/telemetry Anything related to telemetry or observability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants