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

Improve annotations customization in the chart #13643

Closed
4 tasks
FloChehab opened this issue Jan 12, 2021 · 11 comments
Closed
4 tasks

Improve annotations customization in the chart #13643

FloChehab opened this issue Jan 12, 2021 · 11 comments
Assignees
Labels
area:helm-chart Airflow Helm Chart kind:feature Feature Requests

Comments

@FloChehab
Copy link
Contributor

FloChehab commented Jan 12, 2021

Description

As discussed in #13616 (comment), we would need to improve the customization of annotations across all objects of the chart.

Use case / motivation

We often need to paremetrize the annotations and we would rather do it in one PR with all the objects rather than one small PR everytime.

Are you willing to submit a PR?

@mik-laj I am ready to be assigned.

Related Issues
Not to my knowledge.


What need to be tackled before starting dev:

  • On what objects do we want annotations?
  • What terminology do we choose in the values.yaml? (annotations or extraAnnotations or both or something else?)
  • Have a table with the mapping of each object and the value path for the annotation parametrization?
  • Should we drop the value airflowPodAnnotations ?
@FloChehab
Copy link
Contributor Author

First:

Objects that should have parametrizable annotations:

  • Pod (inside deployment and StatefulSet),
  • Service,
  • ServiceAccount,
  • Ingress.

Is this list too much ? Am I missing something?

@FloChehab
Copy link
Contributor Author

FloChehab commented Jan 12, 2021

(I have added more info in the issue description, feel free to edit)

@mik-laj
Copy link
Member

mik-laj commented Jan 12, 2021

@dimberman @XD-DENG @potiuk Do you have any thoughts on this? I saw that these changes often come back in different forms, and I think we can do it once, but good.

@XD-DENG
Copy link
Member

XD-DENG commented Jan 12, 2021

Absolutely a good idea 👍

@potiuk
Copy link
Member

potiuk commented Jan 12, 2021

Absolutely !

@mik-laj
Copy link
Member

mik-laj commented Jan 29, 2021

We have a ticket that improves the annotation handling for SA a bit. Anyone willing to do a review and say if this change is sufficient for this ticket as well?
#11769

@FloChehab
Copy link
Contributor Author

Hello,

Regarding the terminology

What terminology do we choose in the values.yaml? (annotations or extraAnnotations or both or something else?)

I checked the chart and extraAnnotations is only used when there are other annotations related to prometheus that are already present (for the pgbouncer-service & the statsd-service). My position on this would be to remove such annotations related to prometheus and use the annotations terminology everywhere.

@FloChehab
Copy link
Contributor Author

Hello, just wanted to let you now that I should have time to work on this issue in couple of weeks.

@dimberman
Copy link
Contributor

@FloChehab awesome! Please let us know when it's ready to review :)

@FloChehab
Copy link
Contributor Author

Hello, time flies!

I am going to have a quick lool at #15238 ; also, some related work seems to be going on in #14152 .

@dimberman
Copy link
Contributor

@FloChehab sounds good let me know where I can help!

kaxil added a commit that referenced this issue Apr 30, 2021
This PR builds off of and supersedes @jaydesl's work on his [PR](#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: #11755
related: #13643 

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
@kaxil kaxil closed this as completed in 6d64cc5 May 12, 2021
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Sep 17, 2021
This PR builds off of and supersedes @jaydesl's work on his [PR](apache/airflow#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: apache/airflow#11755
related: apache/airflow#13643

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
GitOrigin-RevId: 8655d66cea977102862379d9894810b1e836f7a8
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Sep 17, 2021
This PR adds a new field (`airflowConfigAnnotations`) that allows users to add `annotations` to the main `configmap.yaml` file.

I ended up setting up a new testing file as I didn't find a file where this specifically fit, but if it should be moved elsewhere let me know.

closes apache/airflow#13643

GitOrigin-RevId: 6d64cc54a6b7d1b22d0de89b5815035e21bfaf8c
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Sep 23, 2021
This PR builds off of and supersedes @jaydesl's work on his [PR](apache/airflow#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: apache/airflow#11755
related: apache/airflow#13643

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
GitOrigin-RevId: 8655d66cea977102862379d9894810b1e836f7a8
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Sep 23, 2021
This PR adds a new field (`airflowConfigAnnotations`) that allows users to add `annotations` to the main `configmap.yaml` file.

I ended up setting up a new testing file as I didn't find a file where this specifically fit, but if it should be moved elsewhere let me know.

closes apache/airflow#13643

GitOrigin-RevId: 6d64cc54a6b7d1b22d0de89b5815035e21bfaf8c
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Nov 27, 2021
This PR builds off of and supersedes @jaydesl's work on his [PR](apache/airflow#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: apache/airflow#11755
related: apache/airflow#13643

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
GitOrigin-RevId: 8655d66cea977102862379d9894810b1e836f7a8
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Nov 27, 2021
This PR adds a new field (`airflowConfigAnnotations`) that allows users to add `annotations` to the main `configmap.yaml` file.

I ended up setting up a new testing file as I didn't find a file where this specifically fit, but if it should be moved elsewhere let me know.

closes apache/airflow#13643

GitOrigin-RevId: 6d64cc54a6b7d1b22d0de89b5815035e21bfaf8c
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Mar 10, 2022
This PR builds off of and supersedes @jaydesl's work on his [PR](apache/airflow#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: apache/airflow#11755
related: apache/airflow#13643

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
GitOrigin-RevId: 8655d66cea977102862379d9894810b1e836f7a8
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Mar 10, 2022
This PR adds a new field (`airflowConfigAnnotations`) that allows users to add `annotations` to the main `configmap.yaml` file.

I ended up setting up a new testing file as I didn't find a file where this specifically fit, but if it should be moved elsewhere let me know.

closes apache/airflow#13643

GitOrigin-RevId: 6d64cc54a6b7d1b22d0de89b5815035e21bfaf8c
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jun 4, 2022
This PR builds off of and supersedes @jaydesl's work on his [PR](apache/airflow#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: apache/airflow#11755
related: apache/airflow#13643

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
GitOrigin-RevId: 8655d66cea977102862379d9894810b1e836f7a8
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jun 4, 2022
This PR adds a new field (`airflowConfigAnnotations`) that allows users to add `annotations` to the main `configmap.yaml` file.

I ended up setting up a new testing file as I didn't find a file where this specifically fit, but if it should be moved elsewhere let me know.

closes apache/airflow#13643

GitOrigin-RevId: 6d64cc54a6b7d1b22d0de89b5815035e21bfaf8c
kosteev pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jul 9, 2022
This PR builds off of and supersedes @jaydesl's work on his [PR](apache/airflow#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: apache/airflow#11755
related: apache/airflow#13643

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
GitOrigin-RevId: 8655d66cea977102862379d9894810b1e836f7a8
kosteev pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jul 9, 2022
This PR adds a new field (`airflowConfigAnnotations`) that allows users to add `annotations` to the main `configmap.yaml` file.

I ended up setting up a new testing file as I didn't find a file where this specifically fit, but if it should be moved elsewhere let me know.

closes apache/airflow#13643

GitOrigin-RevId: 6d64cc54a6b7d1b22d0de89b5815035e21bfaf8c
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Aug 27, 2022
This PR builds off of and supersedes @jaydesl's work on his [PR](apache/airflow#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: apache/airflow#11755
related: apache/airflow#13643

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
GitOrigin-RevId: 8655d66cea977102862379d9894810b1e836f7a8
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Aug 27, 2022
This PR adds a new field (`airflowConfigAnnotations`) that allows users to add `annotations` to the main `configmap.yaml` file.

I ended up setting up a new testing file as I didn't find a file where this specifically fit, but if it should be moved elsewhere let me know.

closes apache/airflow#13643

GitOrigin-RevId: 6d64cc54a6b7d1b22d0de89b5815035e21bfaf8c
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Oct 4, 2022
This PR builds off of and supersedes @jaydesl's work on his [PR](apache/airflow#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: apache/airflow#11755
related: apache/airflow#13643

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
GitOrigin-RevId: 8655d66cea977102862379d9894810b1e836f7a8
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Oct 4, 2022
This PR adds a new field (`airflowConfigAnnotations`) that allows users to add `annotations` to the main `configmap.yaml` file.

I ended up setting up a new testing file as I didn't find a file where this specifically fit, but if it should be moved elsewhere let me know.

closes apache/airflow#13643

GitOrigin-RevId: 6d64cc54a6b7d1b22d0de89b5815035e21bfaf8c
aglipska pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Oct 7, 2022
This PR builds off of and supersedes @jaydesl's work on his [PR](apache/airflow#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: apache/airflow#11755
related: apache/airflow#13643

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
GitOrigin-RevId: 8655d66cea977102862379d9894810b1e836f7a8
aglipska pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Oct 7, 2022
This PR adds a new field (`airflowConfigAnnotations`) that allows users to add `annotations` to the main `configmap.yaml` file.

I ended up setting up a new testing file as I didn't find a file where this specifically fit, but if it should be moved elsewhere let me know.

closes apache/airflow#13643

GitOrigin-RevId: 6d64cc54a6b7d1b22d0de89b5815035e21bfaf8c
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Dec 7, 2022
This PR builds off of and supersedes @jaydesl's work on his [PR](apache/airflow#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: apache/airflow#11755
related: apache/airflow#13643

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
GitOrigin-RevId: 8655d66cea977102862379d9894810b1e836f7a8
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Dec 7, 2022
This PR adds a new field (`airflowConfigAnnotations`) that allows users to add `annotations` to the main `configmap.yaml` file.

I ended up setting up a new testing file as I didn't find a file where this specifically fit, but if it should be moved elsewhere let me know.

closes apache/airflow#13643

GitOrigin-RevId: 6d64cc54a6b7d1b22d0de89b5815035e21bfaf8c
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jan 27, 2023
This PR builds off of and supersedes @jaydesl's work on his [PR](apache/airflow#11769) to move forward with properly following [helm's rbac best practices](https://helm.sh/docs/chart_best_practices/rbac/). This PR updates every potential pod that can be deployed to include the option to either create or use an existing service account. This is the first step towards supporting environments where users have the [PodSecurityPolicy](https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#podsecuritypolicy) admission controller enabled without forcing such users to provide any additional permissions to the default service account in the namespace this is deployed to.

closes: apache/airflow#11755
related: apache/airflow#13643

Co-authored-by: jaydesl <jay.deslauriers@gmail.com>
Co-authored-by: Ian Stanton <ian@astronomer.io>
Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
GitOrigin-RevId: 8655d66cea977102862379d9894810b1e836f7a8
leahecole pushed a commit to GoogleCloudPlatform/composer-airflow that referenced this issue Jan 27, 2023
This PR adds a new field (`airflowConfigAnnotations`) that allows users to add `annotations` to the main `configmap.yaml` file.

I ended up setting up a new testing file as I didn't find a file where this specifically fit, but if it should be moved elsewhere let me know.

closes apache/airflow#13643

GitOrigin-RevId: 6d64cc54a6b7d1b22d0de89b5815035e21bfaf8c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:helm-chart Airflow Helm Chart kind:feature Feature Requests
Projects
None yet
Development

No branches or pull requests

5 participants