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

mutatingWebhook is being flagged as unsafe #461

Closed
serdarkkts opened this issue Oct 11, 2023 · 4 comments · Fixed by #499
Closed

mutatingWebhook is being flagged as unsafe #461

serdarkkts opened this issue Oct 11, 2023 · 4 comments · Fixed by #499
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@serdarkkts
Copy link
Contributor

The cloud-sql-proxy-operator mutating webhook can intercept pods in system-managed namespaces. Because of that GKE is flagging the webhook as unsafe.

From the GKE documentation:

If a webhook is intercepting any resources in system-managed namespaces, or certain types of resources, GKE considers this unsafe and recommends that you update the webhooks to avoid intercepting these resources.

The recommended action is to change the scope of the resources to namespaced from * and exclude the system-managed namespaces with the help of the namespaceSelector on the MutatingWebhookConfiguration resource (Since Kubernetes v1.21). However, I am not sure if this would affect any potential use cases of the cloud-sql-proxy-operator.

What are your opinions on this?

@hessjcg hessjcg added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Oct 11, 2023
@hessjcg
Copy link
Collaborator

hessjcg commented Oct 11, 2023

Thank you for reporting this. I agree that the operator has no reason to intercept pods in system namespaces, and I'd like to change the operator setup so that it works correctly. I have a suggestions on how to accomplish this:

  • Make the default installation YAML for the MutatingWebhookConfiguration use Kubernetes 1.21 namespaceSelector
  • Provide an alternative installation yaml for Kubernetes < 1.21, providing an explicit list of namespaces, and instructions to the user on how to edit that list.
  • When we build the helm chart, allow the users to explicitly list monitored namespaces in the chart's values. (Feature Request: Helm Chart #343)

@enocom enocom added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Nov 17, 2023
@enocom
Copy link
Member

enocom commented Nov 17, 2023

I think this is a bug -- we shouldn't be causing GKE to flag the Operator. Adjusting the priority down, though, as it's not going to interfere with operations.

@enocom
Copy link
Member

enocom commented Nov 17, 2023

Here's a screenshot of the warning for reference:

image

@enocom
Copy link
Member

enocom commented Nov 22, 2023

Based on GKE's version support, I think v1.21 (April 8, 2021) is already unsupported. So we're good to use a > v1.21 solution here (i.e. namespaceSelector).

Reference: https://cloud.google.com/kubernetes-engine/versioning#version-support

@enocom enocom assigned enocom and unassigned hessjcg Nov 28, 2023
enocom added a commit that referenced this issue Dec 8, 2023
enocom added a commit that referenced this issue Dec 8, 2023
enocom added a commit that referenced this issue Dec 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants