feat: add first-class HTTP proxy support for importer - #100
Merged
Conversation
Add top-level proxy configuration (httpProxy, httpsProxy, noProxy) that injects HTTP_PROXY, HTTPS_PROXY, and NO_PROXY env vars into the importer container. Required for environments where direct egress to github.com is blocked and git operations must route through a proxy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reviewer's GuideAdds configurable HTTP proxy support to the importer by introducing a top-level proxy configuration and wiring it into the Helm chart so HTTP(S)/NO proxy environment variables are conditionally injected, plus tests and helper template to verify behavior. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The helper
trustification.application.proxy.envVarsexpects adictwith arootkey, but the importer deployment passes$moddirectly—double-check that$modhas the expected shape (including.root) or adjust the helper/usage for consistency with other helpers. - Many tools respect lowercase proxy variables (
http_proxy,https_proxy,no_proxy) in addition to the uppercase forms; consider optionally injecting these as well to match common proxy environments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The helper `trustification.application.proxy.envVars` expects a `dict` with a `root` key, but the importer deployment passes `$mod` directly—double-check that `$mod` has the expected shape (including `.root`) or adjust the helper/usage for consistency with other helpers.
- Many tools respect lowercase proxy variables (`http_proxy`, `https_proxy`, `no_proxy`) in addition to the uppercase forms; consider optionally injecting these as well to match common proxy environments.
## Individual Comments
### Comment 1
<location path="charts/trustify/templates/helpers/_proxy.tpl" line_range="10-18" />
<code_context>
+{{- define "trustification.application.proxy.envVars" -}}
+{{- with .root.Values.proxy }}
+{{- with .httpProxy }}
+- name: HTTP_PROXY
+ value: {{ . | quote }}
+{{- end }}
+{{- with .httpsProxy }}
+- name: HTTPS_PROXY
+ value: {{ . | quote }}
+{{- end }}
+{{- with .noProxy }}
+- name: NO_PROXY
+ value: {{ . | quote }}
+{{- end }}
</code_context>
<issue_to_address>
**suggestion:** Consider setting lowercase proxy env vars alongside the uppercase ones.
Some clients only honor lowercase proxy variables (`http_proxy`, `https_proxy`, `no_proxy`). Emitting the lowercase variants alongside the uppercase ones would make this helper more robust and avoid proxy misconfigurations for those tools.
Suggested implementation:
```
{{/*
Proxy env-vars for containers that need HTTP proxy support.
Arguments (dict):
* root - .
*/}}
{{- define "trustification.application.proxy.envVars" -}}
{{- with .root.Values.proxy }}
{{- with .httpProxy }}
- name: HTTP_PROXY
value: {{ . | quote }}
- name: http_proxy
value: {{ . | quote }}
{{- end }}
{{- with .httpsProxy }}
- name: HTTPS_PROXY
value: {{ . | quote }}
- name: https_proxy
value: {{ . | quote }}
{{- end }}
{{- with .noProxy }}
- name: NO_PROXY
value: {{ . | quote }}
- name: no_proxy
value: {{ . | quote }}
{{- end }}
{{- end }}
{{- end }}
```
None required; this change is self-contained in the Helm helper and will emit both upper- and lower-case proxy environment variables wherever `trustification.application.proxy.envVars` is used.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
I think AI is right, we might just want to set those too. |
Some clients only honor lowercase proxy variables (http_proxy, https_proxy, no_proxy). Emit both variants for broader compatibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
Agree on that too - Updated the helm :) |
ctron
approved these changes
Jul 16, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Add top-level proxy configuration (httpProxy, httpsProxy, noProxy) that injects HTTP_PROXY, HTTPS_PROXY, and NO_PROXY env vars into the importer container. Required for environments where direct egress to github.com is blocked and git operations must route through a proxy.
Summary by Sourcery
Add configurable HTTP proxy support for the importer by introducing a top-level proxy configuration that maps to standard proxy environment variables.
New Features:
Tests: