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.

Sign up for GitHub

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

Jump to bottom

Https proxy env #259

Merged
merged 4 commits into from
Dec 12, 2023
Merged

Https proxy env #259

merged 4 commits into from
Dec 12, 2023

Conversation

samitiilikainen
Copy link
Contributor

@samitiilikainen samitiilikainen commented Dec 8, 2023 •
edited
Loading

  1. Parses the YAML and replaces the placeholders in the parsed structure instead of string replace
  2. Supports having known empty values in the YAML to be replaced with env values (HTTPS_PROXY, BORED_HTTPS_PROXY) in a backwards compatible way (old versions just get the empty string)

There are two different envs for proxy:

  • BORED_HTTPS_PROXY is used when it's meant to be just passed through (bored agent updater setting it for the actual agent). This allows the updater's k8s client applying kubernetes resources not to use a proxy.
  • HTTPS_PROXY used in the agent deployment

Fixes #1208 (once the HTTPS_PROXY/BORED_HTTPS_PROXY also exists in lenscloud bored yaml files)

@samitiilikainen
Parse yaml and replace values placeholders values in parsed structure
2dae152 
Signed-off-by: Sami Tiilikainen <[email protected]>
@samitiilikainen
Add support for replacing known empty values from config
0c4defb 
A backwards compatible way to define HTTPS_PROXY as an empty value.
Only this version knows how to replace it but older versions won't break
from having the empty string.

Signed-off-by: Sami Tiilikainen <[email protected]>
panuhorsmalahti
panuhorsmalahti reviewed Dec 8, 2023
View reviewed changes
src/__tests__/updater-mock.yml
value: $LENS_BACKEND_URL
- name: AUTO_UPDATE_URL
value: $AUTO_UPDATE_URL
command:
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we will add HTTPS_PROXY also to the updater.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added and then renamed to BORED_HTTPS_PROXY so that it can be passed through but not used by the updater itself

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update the PR description to explain this logic? For future reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@samitiilikainen
Add proxy in mock to updater too
33391fc 
Signed-off-by: Sami Tiilikainen <[email protected]>
@samitiilikainen
Rename env for the updater
e8522b7 
When the env is just used for the updater to set in configs, not to use,
call it BORED_HTTPS_PROXY

Signed-off-by: Sami Tiilikainen <[email protected]>
@samitiilikainen samitiilikainen marked this pull request as ready for review December 12, 2023 07:47
@samitiilikainen samitiilikainen merged commit 364454a into main Dec 12, 2023
3 checks passed
@samitiilikainen samitiilikainen deleted the https-proxy-env branch December 12, 2023 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@panuhorsmalahti panuhorsmalahti panuhorsmalahti approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@samitiilikainen @panuhorsmalahti