-
Notifications
You must be signed in to change notification settings - Fork 313
Feat/falco proxy env #918
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
base: master
Are you sure you want to change the base?
Feat/falco proxy env #918
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jvlerner The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @jvlerner! It looks like this is your first PR to falcosecurity/charts 🎉 |
a570359 to
952d4f1
Compare
Signed-off-by: jvlerner <jvlerner@uolinc.com>
952d4f1 to
a2449e7
Compare
42467d8 to
b133844
Compare
Signed-off-by: jvlerner <jvlerner@uolinc.com>
b133844 to
0edbf77
Compare
29f8ff2 to
d46bc4f
Compare
Signed-off-by: jvlerner <jvlerner@uolinc.com>
d46bc4f to
12691a9
Compare
|
My bad guys, I missed adding the proxy link to the ignore list. Just fixed it |
ekoops
left a comment
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.
Hey, I overall looks good! I just added some comments. Let me know if you have any question or doubt! 😄
| env: | ||
| {{- include "falco.renderTemplate" ( dict "value" .Values.falcoctl.artifact.install.env "context" $) | nindent 4 }} | ||
| {{- include "falco.proxyEnv" . | nindent 2 }} | ||
| {{- if .Values.falcoctl.artifact.install.env }} |
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.
Is this if statement required here? I have hadn't chance to check it, but what happen if we render an empty env list?
| {{- include "falco.proxyEnv" . | nindent 2 }} | ||
| {{- include "falco.renderTemplate" ( dict "value" .Values.falcoctl.artifact.follow.env "context" $) | nindent 4 }} |
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.
Similarly to my previous comment, is there any reason why you didn't add the {{- if .Values.falcoctl.artifact.install.env }} check here?
| {{- if .Values.proxy.enabled }} | ||
| {{- include "falco.proxyEnv" . | nindent 8 }} | ||
| {{- end }} |
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.
You are already checking {{- if .Values.proxy.enabled }} in the helper. At this point, do we want to keep the check in the helper or do we want to repeat it? I guess it is better to remove it from the helper, but I'm open to other opinion on this!
| {{- if .Values.proxy.enabled }} | ||
| {{- include "falco.proxyEnv" . | nindent 4 }} | ||
| {{- end }} |
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.
Same as above
| | proxy.http | string | `"http://proxy.dominio.com:3128"` | HTTP proxy URL used for outbound HTTP requests. | | ||
| | proxy.https | string | `"http://proxy.dominio.com:3128"` | HTTPS proxy URL used for outbound HTTPS requests. | |
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.
Sorry, I guess my missed these two here in the previous review cycle 😔
|
Hey @jvlerner any update on this? |
What type of PR is this?
/kind feature
/kind chart-release
Any specific area of the project related to this PR?
/area falco-chart
What this PR does / why we need it:
Which issue(s) this PR fixes:
Currently, in environments with restricted internet access, containers often fail to reach external resources due to missing proxy configuration. This impacts components like falcoctl, which require outbound connectivity to fetch artifacts or updates.
This PR introduces support for injecting proxy-related environment variables (HTTP_PROXY, HTTPS_PROXY, NO_PROXY) into all relevant containers and initContainers when proxy is enabled via values.yaml.
The implementation uses a reusable Helm helper to ensure consistency and maintainability across templates.
Fixes #917
Special notes for your reviewer:
Checklist