fix: augment NO_PROXY with required internal exclusions in proxy-env ConfigMap#359
fix: augment NO_PROXY with required internal exclusions in proxy-env ConfigMap#359rooftopcellist wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe proxy environment ConfigMap template generates ChangesProxy environment configuration
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| no_proxy: '{{ no_proxy }}' | ||
| {% if http_proxy or https_proxy or no_proxy %} | ||
| {% set _no_proxy_list = (no_proxy | default('')).split(',') | map('trim') | select | list %} | ||
| {% set _required = ['localhost', '127.0.0.1', '::1', '[::1]', '.svc', '.svc.cluster.local', ansible_operator_meta.name, ansible_operator_meta.name + '.' + ansible_operator_meta.namespace] %} |
There was a problem hiding this comment.
@rooftopcellist This host name should also be in the exclusion list.
Thats the WebSocketServer that ansible-rulebook will connect to and it should not go thru the proxy server.
Both of these websocket_server_name and api_server_name
EDA_WEBSOCKET_BASE_URL: "ws://{{ websocket_server_name }}:{{ websocket_port }}"
EDA_WEBSOCKET_TOKEN_BASE_URL: "http://{{ api_server_name }}:{{ api_nginx_port }}"
There was a problem hiding this comment.
Thanks for the feedback, it now includes those:
- Standard:
localhost,127.0.0.1,::1,[::1],.svc, .svc.cluster.local - CR name + namespaced:
<cr-name>,<cr-name>.<namespace> - API server:
<cr-name>-api,<cr-name>-api.<namespace>(EDA_WEBSOCKET_TOKEN_BASE_URL) - WebSocket server:
<cr-name>-daphne,<cr-name>-daphne.<namespace>(EDA_WEBSOCKET_BASE_URL)
…ConfigMap When a cluster-wide egress proxy is configured on OCP, NO_PROXY is propagated verbatim from the operator pod environment. OCP's default NO_PROXY uses .svc/.svc.cluster.local suffix patterns that do not match the bare short service names used for internal communication, causing internal traffic to route through the corporate proxy. Augment NO_PROXY before writing it to the ConfigMap: preserve customer entries first, then append required exclusions (loopbacks, .svc suffixes, CR short name, namespaced CR name). Use unique filter for determinism across reconciliation loops. Emit NO_PROXY whenever any proxy var is set. Related: AAP-83822 Signed-off-by: Christian M. Adams <chadams@redhat.com>
893a4b6 to
7a1e8a7
Compare
EDA (via ansible-rulebook) contacts the gateway when launching Job Templates using EDA_CONTROLLER_URL, which resolves to automation_server_url in the EDA CR spec. If this URL's hostname is not in NO_PROXY, those calls are routed through the corporate proxy instead of staying in-cluster. Since automation_server_url is user-provided and may differ from the EDA CR name, the hostname cannot be statically inferred. Extract it from the configured URL at template render time and append it to the required exclusion list. Signed-off-by: Christian M. Adams <chadams@redhat.com>
|
4 participants
Summary
When a cluster-wide egress proxy is configured on OpenShift, the operator propagates
NO_PROXYverbatim from the operator pod environment into theproxy-envConfigMap that is mounted viaenvFrominto operand containers. OCP's defaultNO_PROXYuses.svcand.svc.cluster.localsuffix patterns, which do not match the bare short service names operators use for internal communication (e.g.http://<cr-name>). This causes internal traffic — including JWT key validation calls — to route through the corporate proxy instead of staying in-cluster, producing authentication failures and component unavailability.This fix augments
NO_PROXYbefore writing it to the ConfigMap:localhost,127.0.0.1,::1,[::1],.svc,.svc.cluster.local, the CR short name, and the namespaced CR nameNO_PROXYis now emitted whenever any proxy variable is set, not only whenNO_PROXYitself is non-emptyuniquefilter ensures deterministic ordering across reconciliation loops (prevents spurious pod restarts from changing ConfigMap checksums)Related
Test Plan
<cr-name>-proxy-envConfigMap is created andNO_PROXYvalue contains all required internal exclusionsNO_PROXYentries are preservedNO_PROXYvalue is byte-for-byte identical across consecutive reconciliation loopsSummary by CodeRabbit
NO_PROXY/no_proxywhenever any proxy setting is provided.no_proxyinput by splitting on commas, trimming whitespace, and removing empty entries.-apiand-daphnevariants).