search

tumblr / k8s-sidecar-injector

Kubernetes sidecar injection service
Apache License 2.0
345 stars 75 forks source link

Status annotation ignores custom annotation namespace #20

Closed noahgoldman closed 5 years ago

noahgoldman commented 5 years ago

What's going on?

Setting a custom annotation namespace -annotation-namespace does not effect the injector.tumblr.com/status annotation.

The root cause seems quite clear from reading the source. The /status annotation is set in webhook.go#L462, using the config.InjectionStatusAnnotation package-level variable. This variable is hardcoded to use annotationNamespaceDefault, which is set to "injector.tumblr.com". This pretty clearly explains why the user-specified configuration is ignored.

Interestingly, both /request and /status are properly formatted using AnnotationNamespace in (*WebhookServer).getSidecarConfigurationRequested. Seems like that configuration format just needs to be used in both places.

Expected Behavior

Setting -annotation-namespace=sidecar-injector.eks.qcinternal.io should cause Pods with injected sidecars to have the annotation sidecar-injector.eks.qcinternal.io/status: injected. Instead, we see injector.tumblr.com/status: injected. The annotation setting which sidecar configuration to use is sidecar-injector.eks.qcinternal.io/request.

Reproducer

The injector is launched with the following arguments:

    - --v
    - "2"
    - --tls-cert-file
    - /var/lib/tls-cert/tls.crt
    - --tls-key-file
    - /var/lib/tls-cert/tls.key
    - --annotation-namespace
    - sidecar-injector.eks.qcinternal.io
    - --configmap-labels
    - app.kubernetes.io/instance=k8s-sidecar-injector-batch-production-blue,app.kubernetes.io/component=sidecar-config

I'm going to omit sidecar configurations in particular, as the root cause seems quite obvious and the configurations are for internal tools. I can provide similar information if necessary.

Version Deets

byxorna commented 5 years ago

@noahgoldman thanks for this issue! Great catch - I definitely missed this while making the injector more modular. Let me see if i can get a quick PR up for you.

More info shortly!

byxorna commented 5 years ago

@noahgoldman can you check out the :latest build now? Hopefully this is fixed :)