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

Clarify documentation and prevent activating Ingress and Gateway controllers if l7 proxy is disabled #22698

Closed
5 tasks done
youngnick opened this issue Dec 13, 2022 · 10 comments
Closed
5 tasks done
Assignees
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. area/servicemesh GH issues or PRs regarding servicemesh kind/bug This is a bug in the Cilium logic. sig/agent Cilium agent related.

Comments

@youngnick
Copy link
Contributor

youngnick commented Dec 13, 2022

This issue is effectively an epic covering work to handle what happens when Ingress or Gateway API support is enabled when L7 proxy support has already been explicitly disabled (which is required for Wireguard support, but could also be done for other reasons).

When this happens, a few problems can arise:

The fact that the last one has all Cilium pods panic and crash seems pretty bad.

Actions to take here are at least:

@youngnick youngnick added kind/bug This is a bug in the Cilium logic. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. area/servicemesh GH issues or PRs regarding servicemesh labels Dec 13, 2022
@pchaigno
Copy link
Member

Should we split this up into the 4 above tasks and mark good-first-issues? You should find people to work on this in the community if we do that IMO.

@christarazi christarazi added the sig/agent Cilium agent related. label Dec 15, 2022
@afzal442
Copy link

Hey there, would like to look into it.

cc @pchaigno

@youngnick
Copy link
Contributor Author

Thanks @pchaigno, good call. I've split out three of the tasks, and marked them good-first-issue. @afzal442, please feel free to pick up any of those sub-tasks!

@amolkavitkar
Copy link

case option.Config.EnableL7Proxy:

isn't this is suppose to log a error if wireguard is enabled with L7proxy

@pchaigno
Copy link
Member

pchaigno commented Jan 2, 2023

@amolkavitkar That's correct and this logic works, but the present issue is about the Ingress and Gateway controllers when the L7 proxy is disabled, not about WireGuard. WireGuard is just one of the reasons why the L7 proxy might be disabled.

@youngnick
Copy link
Contributor Author

#18773 is another example of a similar behavior.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not
had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Apr 22, 2023
@youngnick youngnick removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Apr 25, 2023
@youngnick youngnick self-assigned this May 1, 2023
@youngnick
Copy link
Contributor Author

With #25215 merged, this one is resolved. As of 1.13, it's not possible to disable l7 proxy and enable Ingress or Gateway API, it will be rejected by Helm.

@jibi
Copy link
Member

jibi commented May 25, 2023

With #25215 merged, this one is resolved. As of 1.13, it's not possible to disable l7 proxy and enable Ingress or Gateway API, it will be rejected by Helm.

reopening this as I think there's a typo in https://github.com/cilium/cilium/pull/25215/files: l7proxy should be l7Proxy (we dropped that PR from v1.12 #25452 and v1.13 #25346 backports but probably the notification from the PR description got lost)

@jibi jibi reopened this May 25, 2023
@youngnick
Copy link
Contributor Author

@sayboras closed this one in #25570.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. area/servicemesh GH issues or PRs regarding servicemesh kind/bug This is a bug in the Cilium logic. sig/agent Cilium agent related.
Projects
None yet
Development

No branches or pull requests

6 participants