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

Set proxy-injector, tap-injector and jaeger-injector mutating webhook rules scope to Namespaced #12195

Merged
merged 5 commits into from Mar 28, 2024

Conversation

mdnfiras
Copy link
Contributor

@mdnfiras mdnfiras commented Mar 4, 2024

Subject: Setting the scope for mutating webhooks rules

Problem: The linkerd-proxy-injector-webhook-config, linkerd-jaeger-injector-webhook-config, and linkerd-tap-injector-webhook-config mutating webhooks raise a warning on GKE that says "Update webhook to no longer intercept system requests." in the GCP console recommendation section. This is because the scope is set to *. This also happens if scope is Namespaced, and kube-system and kube-node-lease namespaces are not excluded using namespaceSelector.

Solution: Setting the scope to Namespaced for all webhooks, and the user can set the namespaceSelector in the helm values.

Validation: This should not change the webhooks behaviour as all webhooks are triggered only by pod/service creation requests, and pods/services are namespaced resources.

Fixes #12193

@mdnfiras mdnfiras requested a review from a team as a code owner March 4, 2024 14:56
@mdnfiras mdnfiras force-pushed the feature/webhooks-scope-namespaced branch 2 times, most recently from 961815f to 28880c2 Compare March 4, 2024 15:06
Copy link
Member

@alpeb alpeb left a comment

Choose a reason for hiding this comment

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

Great fix, thanks!
To fix the tests, please run go test ./... -update to recreate the test fixtures 😉

@alpeb
Copy link
Member

alpeb commented Mar 18, 2024

... and also please address the DCO

@mdnfiras mdnfiras force-pushed the feature/webhooks-scope-namespaced branch 3 times, most recently from 44e6544 to 50af07d Compare March 25, 2024 17:46
@mdnfiras
Copy link
Contributor Author

@alpeb
done, i think.. I'm not very familiar with testing in go 😃

Subject: Setting the scope for mutating webhooks rules

Problem: The linkerd-proxy-injector-webhook-config, linkerd-jaeger-injector-webhook-config, and linkerd-tap-injector-webhook-config mutating webhooks raise a warning on GKE that says "Update webhook to no longer intercept system requests." in the GCP console recommendation section. This is because the scope is set to *. This also happens if scope is Namespaced, and kube-system and kube-node-lease namespaces are not excluded using namespaceSelector.

Solution: Setting the scope to Namespaced for both webhooks, and the user can set the namespaceSelector in the helm values.

Validation: This should not change the webhooks behaviour as all webhooks are triggered only by pod/service creation requests, and pods/services are namespaced resources.

Fixes linkerd#12193

Signed-off-by: f.medini <f.medini@nyris.io>
@mdnfiras mdnfiras force-pushed the feature/webhooks-scope-namespaced branch from 50af07d to 5e93d04 Compare March 26, 2024 08:58
jaeger/cmd/testdata/install_collector_disabled.golden Outdated Show resolved Hide resolved
jaeger/cmd/testdata/install_collector_disabled.golden Outdated Show resolved Hide resolved
jaeger/cmd/testdata/install_default.golden Outdated Show resolved Hide resolved
jaeger/cmd/testdata/install_default.golden Outdated Show resolved Hide resolved
jaeger/cmd/testdata/install_default.golden Outdated Show resolved Hide resolved
viz/cmd/testdata/install_proxy_resources.golden Outdated Show resolved Hide resolved
viz/cmd/testdata/install_proxy_resources.golden Outdated Show resolved Hide resolved
viz/cmd/testdata/install_proxy_resources.golden Outdated Show resolved Hide resolved
viz/cmd/testdata/install_proxy_resources.golden Outdated Show resolved Hide resolved
viz/cmd/testdata/install_proxy_resources.golden Outdated Show resolved Hide resolved
@mdnfiras mdnfiras changed the title Set tap-injector and jaeger-injector mutating webhook rules scope to Namespaced Set proxy-injector, tap-injector and jaeger-injector mutating webhook rules scope to Namespaced Mar 27, 2024
Copy link
Member

@alpeb alpeb left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @mdnfiras !

@alpeb alpeb merged commit aaab29c into linkerd:main Mar 28, 2024
35 checks passed
the-wondersmith pushed a commit to the-wondersmith/linkerd2 that referenced this pull request Apr 24, 2024
… rules scope to Namespaced (linkerd#12195)

* Set mutating webhook rules scope to Namespaced

Problem: The linkerd-proxy-injector-webhook-config, linkerd-jaeger-injector-webhook-config, and linkerd-tap-injector-webhook-config mutating webhooks raise a warning on GKE that says "Update webhook to no longer intercept system requests." in the GCP console recommendation section. This is because the scope is set to *. This also happens if scope is Namespaced, and kube-system and kube-node-lease namespaces are not excluded using namespaceSelector.

Solution: Setting the scope to Namespaced for all webhooks, and the user can set the namespaceSelector in the helm values.

Validation: This should not change the webhooks behaviour as all webhooks are triggered only by pod/service creation requests, and pods/services are namespaced resources.

Fixes linkerd#12193

---------

Signed-off-by: f.medini <f.medini@nyris.io>
Co-authored-by: Alejandro Pedraza <alejandro@buoyant.io>
Signed-off-by: Mark S <the@wondersmith.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

linkerd-jaeger-injector-webhook-config mutating webhook raises warning on GKE
2 participants