Allow setting pod annotations separately from the deployment in kratos helm chart

sazzle2611 commented 2 years ago

Preflight checklist

[X] I could not find a solution in the existing issues, docs, nor discussions.
[X] I agree to follow this project's Code of Conduct.
[X] I have read and am following this repository's Contribution Guidelines.
[ ] This issue affects my Ory Cloud project.
[ ] I have joined the Ory Community Slack.
[ ] I am signed up to the Ory Security Patch Newsletter.

Describe your problem

deployment.annotations adds annotations to the deployment, statefulset, service and pod template but we want to only add the annotations to the pod template.

Istio is giving us many warnings like these 2

Warning [IST0107] (Deployment default/kratos-kratos) Misplaced annotation: sidecar.istio.io/proxyCPU can only be applied to Pod
Warning [IST0107] (Service default/kratos-kratos-courier) Misplaced annotation: sidecar.istio.io/proxyMemoryLimit can only be applied to Pod

It would also be useful to be able to define the courier annotations separately

Describe your ideal solution

Separate pod and courier pod annotations

Workarounds or alternatives

Don't think there are any

Version

v0.9.0-alpha.2

Additional Context

No response

Demonsthere commented 2 years ago

Hi there!

Pod labels and annotations are taken from the deployment level and propagate downwards https://github.com/ory/k8s/blob/master/helm/charts/kratos/templates/deployment-kratos.yaml#L32-L42 you should be able to set the annotations using those :)

sazzle2611 commented 2 years ago

Hey @Demonsthere thanks for getting back to me.

Yes it does successfully set the pod annotations but I don't want the same annotations in the deployment or the service. The wanings we're getting are because those annotations should only be in the pod.

Okay it's not 100% critical as it is only a warning and I don't think it effects how anything works but I like to keep an eye on istio warnings and errors in case we've set something up wrong. We intend to use Kratos for all our authentication and at the moment have 5 deployments, each produces 8 warnings (4x istio annotations, warnings for each annotation in service & deployment) so at the moment we are at 40 warning messages.

Actually just double checked and the annotations aren't in the kratos-public or kratos-admin service as you can set these separately but they are in the kratos-courier service.

Would be great to have the flexibility to set them all separately

Demonsthere commented 2 years ago

I agree, noisy notifications are a pain and can obscure the important messages, could you change the description of the issue to better reflect what would you like to achieve?

sazzle2611 commented 2 years ago

@Demonsthere is that description okay?

ory / k8s