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

Add support for injecting HTTPS_PROXY env var in agent containers #211

Merged
merged 2 commits into from
Feb 19, 2021
Merged

Add support for injecting HTTPS_PROXY env var in agent containers #211

merged 2 commits into from
Feb 19, 2021

Conversation

danielfm
Copy link
Contributor

Why is this PR necessary, what does it do?

This PR aims to add to the injector the ability of setting the HTTPS_PROXY environment variable in the init/sidecar containers in order to allow requests to Vault to be made via a proxy.

References:

Fixes: #205

Notes:

If the reviewers feel there are tests missing, please let me know! I may have missed something.

I'll try to run this version of the injector in a test cluster to validate the workflow, but in the mean time, feel free to comment.

@hashicorp-cla
Copy link

hashicorp-cla commented Jan 15, 2021

CLA assistant check
All committers have signed the CLA.

@danielfm
Copy link
Contributor Author

danielfm commented Jan 15, 2021

I managed to validate this change in a local Kubernetes cluster.

I validated these flows:

  • Without proxy configuration
    • Works as it always did
  • With AGENT_INJECT_PROXY_ADDR env var set to the injector + no annotations
    • Agent requests are forwarded to Vault via the proxy address specified in the env var
  • With AGENT_INJECT_PROXY_ADDR env var set to the injector + annotation overwriting the proxy address
    • Agent requests are forwarded to Vault via the proxy address specified in the annotation
  • No AGENT_INJECT_PROXY_ADDR env var set to the injector + annotation setting the proxy address
    • Same as above

Please let me know if you want me to perform any other manual tests.

@danielfm
Copy link
Contributor Author

danielfm commented Jan 18, 2021

One question I have is to whether it makes sense to inject the same value in the HTTP_PROXY environment variable, or even allow for different proxy addresses for HTTP and HTTPS.

Since the recommended Vault server setup is to enable TLS, maybe this PR is good enough as is, but it may not work for development/test Vault clusters without TLS enabled, as requests are sent to Vault in plain HTTP.

Let me know what you think.

@jasonodonnell jasonodonnell self-requested a review January 20, 2021 22:05
@jasonodonnell
Copy link
Contributor

All looks good here @danielfm, do you mind resolving the conflicts?

This struct seems to be used for defining the Vault agent
configuration file[1], but since it doesn't have a specific flag for
setting up a proxy address, this field is useless.

[1] https://www.vaultproject.io/docs/agent
@danielfm
Copy link
Contributor Author

@jasonodonnell done.

@jasonodonnell jasonodonnell self-requested a review February 19, 2021 19:47
Copy link
Contributor

@jasonodonnell jasonodonnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@jasonodonnell jasonodonnell merged commit c2343ab into hashicorp:master Feb 19, 2021
@danielfm danielfm deleted the https-proxy-support branch February 19, 2021 19:57
RemcoBuddelmeijer pushed a commit to RemcoBuddelmeijer/vault-k8s that referenced this pull request Feb 22, 2022
…shicorp#211)

* Add support for injecting HTTPS_PROXY env var in agent containers

* Remove unnecessary field

This struct seems to be used for defining the Vault agent
configuration file[1], but since it doesn't have a specific flag for
setting up a proxy address, this field is useless.

[1] https://www.vaultproject.io/docs/agent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for injecting HTTPS_PROXY environment variable to the init and sidecar containers
3 participants