k8s-sidecar-injector icon indicating copy to clipboard operation
k8s-sidecar-injector copied to clipboard

Ignored namespaces aren't skipped for Deployment pods

Open ribbybibby opened this issue 4 years ago • 3 comments

What's going on?

The metadata of the CREATE request object doesn't always contain the namespace or the name of the pod. This seems to be the case when the pod is launched on behalf of a Deployment. It doesn't seem to be the case with StatefulSets or a bare Pod. I haven't tested Jobs or CronJobs or any other controllers.

The check for ignored namespaces uses metadata.namespace to perform the comparison, so pods in kube-system and kube-public aren't skipped for Deployment pods.

Additionally, some logging statements are missing the namespace and name:

I0925 13:20:51.062652       1 webhook.go:165] Pod / annotation injector.tumblr.com/request is missing, skipping injection

Expected Behavior

List of ignored namespaces should be respected for all pod admission requests, regardless of the source.

Reproducer

This is the Deployment I've been using to test:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: test 
  namespace: kube-system
spec:
  replicas: 1
  selector:
    matchLabels:
      app: test
  template:
    metadata:
      annotations:
        injector.tumblr.com/request: my-sidecar
      labels:
        app: test
    spec:
      containers:
        - name: test
          image: alpine
          command:
            - ash
            - -c
            - |
              while true; do sleep 86400; done

Version Deets

  • Kubernetes Version: 1.18.5, 1.19.1
  • k8s-sidecar-injector Version: release-v0.5.0

ribbybibby avatar Sep 25 '20 13:09 ribbybibby

I've logged this PR to address the issue: https://github.com/tumblr/k8s-sidecar-injector/pull/53.

I'm also interested in #44 though. Is there any utility in having the ignored namespaces in the code at all? Could this not be achieved more flexibly with a namespaceSelector on the webhook itself? That would unblock people (like myself) who would like to use the injector in kube-system.

ribbybibby avatar Sep 25 '20 14:09 ribbybibby

Could this not be achieved more flexibly with a namespaceSelector on the webhook itself?

I personally think that will be a great improvement.

komapa avatar Sep 26 '20 05:09 komapa

Updated #53 to remove the ignored namespaces feature from the code altogether.

ribbybibby avatar Sep 28 '20 07:09 ribbybibby