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

Status annotation ignores custom annotation namespace #20

Closed
noahgoldman opened this issue Aug 12, 2019 · 2 comments · Fixed by #21
Closed

Status annotation ignores custom annotation namespace #20

noahgoldman opened this issue Aug 12, 2019 · 2 comments · Fixed by #21
Assignees

Comments

@noahgoldman
Copy link
Contributor

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

  • Kubernetes Version: v1.13.8
  • k8s-sidecar-injector Version: v0.1.7
@byxorna
Copy link
Contributor

byxorna commented Aug 13, 2019

@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 byxorna self-assigned this Aug 13, 2019
byxorna added a commit that referenced this issue Aug 21, 2019
@byxorna
Copy link
Contributor

byxorna commented Aug 21, 2019

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

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 a pull request may close this issue.

2 participants