Azure / AKS

Azure Kubernetes Service
https://azure.github.io/AKS/
1.96k stars 305 forks source link

control-plane label on admission webhooks definitions #1771

Closed dprotaso closed 7 months ago

dprotaso commented 4 years ago

Turns out Knative doesn't run on AKS 1.17 because there's some Azure component that's mutating our webhooks which we did not expect.

See https://github.com/knative/pkg/issues/1590 for details.

I read https://docs.microsoft.com/en-us/azure/aks/faq#can-i-use-admission-controller-webhooks-on-aks and I'm curious why the selector like

namespaceSelector:
    matchExpressions:
    - key: control-plane
      operator: DoesNotExist

is necessary when the question states https://docs.microsoft.com/en-us/azure/aks/faq#can-admission-controller-webhooks-impact-kube-system-and-internal-aks-namespaces

To protect the stability of the system and prevent custom admission controllers from impacting internal services in the kube-system, namespace AKS has an Admissions Enforcer, which automatically excludes kube-system and AKS internal namespaces. This service ensures the custom admission controllers don't affect the services running in kube-system.

Since there's an Admissions Enforcer preventing these kube-system and AKS namespaces modifications is it necessary to mutate our webhook definitions?

This behaviour could break other projects & products who don't expect these external mutations - ie. Tekton CD is broken as well

ghost commented 4 years ago

Triage required from @Azure/aks-pm

mattmoor commented 4 years ago

cc @ImJasonH

ghost commented 4 years ago

Triage required from @Azure/aks-pm

palma21 commented 4 years ago

The selector is not necessary to be added be added manually as it will be added automatically, but I believe from the issue in knative the issue was the two reconciliations conflicting.

Seems it has been solved there, is there something we can do to help further?

@samkreter

dprotaso commented 4 years ago

Seems it has been solved there, is there something we can do to help further?

Pushing the resolution of this on each project doesn't scale in my opinion. What we've done in Knative is a workaround.

is there something we can do to help further?

I'd recommend stop mutating the webhook spec

ghost commented 4 years ago

Action required from @Azure/aks-pm

ghost commented 4 years ago

Issue needing attention of @Azure/aks-leads

ghost commented 4 years ago

Issue needing attention of @Azure/aks-leads

ghost commented 4 years ago

Issue needing attention of @Azure/aks-leads

ghost commented 3 years ago

Issue needing attention of @Azure/aks-leads

tomwganem commented 3 years ago

Hi there,

Our corporate security policy requires us to scan all images that we currently run, including images we don't control the life-cycle for. This includes all the images in the kube-system namespace for aks. We tried to implement a solution that utilizes a webhook to automatically change the registry of any pod in a namespace to a private one, and then we built some automation to mirror images. But because of this issue, we are unable to implement this for the kube-system namespace.

That is to say, I understand why it's implemented like this, but I would like the option to turn it off. Will this be addressed in the future?

ghost commented 3 years ago

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs within 15 days of this comment.

ghost commented 3 years ago

This issue will now be closed because it hasn't had any activity for 15 days after stale. dprotaso feel free to comment again on the next 7 days to reopen or open a new issue after that time if you still have a question/issue or suggestion.

slenky commented 3 years ago

Reopen please? It's still an issue.

dprotaso commented 3 years ago

@Azure/aks-leads can you re-open this issue?

gambtho commented 11 months ago

Based on this PR, https://github.com/knative/pkg/pull/1949, it appears this is no longer an issue with knative/tekton

Issues like kubeflow, https://github.com/kubeflow/pipelines/issues/5244, (use of the control-plane label by other projects) we will remove in a upcoming update by relying on "managedby: aks" as our label for protecting namespaces from webhook changes.

For the issue mentioned by @tomwganem -- you can use the label/annotation admissions.enforcer/disabled: true to allow your webhook to operate on kube-system and other AKS managed namespaces, but this is not recommended

Any other scenarios that remain a problem?