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

Fix webserver ingress annotations #12619

Merged
merged 1 commit into from
Jan 5, 2021

Conversation

ecerulm
Copy link
Contributor

@ecerulm ecerulm commented Nov 25, 2020

The indentation under web.annotation was wrong (6 leading spaces
on first line, 4 on the next) leading to

Error converting YAMl to JSON: yaml: line 32: did not find expected key

when more that 1 annotation was provided.

@potiuk


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@boring-cyborg boring-cyborg bot added the area:helm-chart Airflow Helm Chart label Nov 25, 2020
@ecerulm
Copy link
Contributor Author

ecerulm commented Nov 25, 2020

I should note that this is a regression introduced by #12003 comment

@potiuk potiuk requested a review from mik-laj November 25, 2020 18:18
@mik-laj
Copy link
Member

mik-laj commented Nov 25, 2020

@ecerulm Can you add tests to avoid regression?

@ecerulm
Copy link
Contributor Author

ecerulm commented Nov 26, 2020

@mik-laj I've added two tests, I had to do other changes unrelated to the ingress annotations because the current template output did not validate :

  • upgrade the validation schemas from 1.12.9 to 1.14.0 (since 1.12.9 doesn't have ingress-networking-v1beta1.json
  • don't generate metadata.annotations if empty
  • don't generate spec.rules[0].paths[].path if empty

@github-actions
Copy link

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

The indentation under `web.annotations` was wrong (6 leading spaces
on first line, 4 on the rest) leading to

    Error converting YAMl to JSON: yaml: line 32: did not find expected key

when you run helm chart with value `ingres.web.annotations`
@ecerulm ecerulm force-pushed the fix_webserver_ingress_annotations branch from e4b5f45 to 5f90b2b Compare November 26, 2020 13:07
@ashb
Copy link
Member

ashb commented Jan 5, 2021

Since the chart hasn't yet been released upgrading the minimum kube version is no problem.

@ashb ashb merged commit 07670ec into apache:master Jan 5, 2021
@ecerulm ecerulm deleted the fix_webserver_ingress_annotations branch January 5, 2021 15:17
kaxil pushed a commit that referenced this pull request Jan 21, 2021
The indentation under `web.annotations` was wrong (6 leading spaces
on first line, 4 on the rest) leading to

    Error converting YAMl to JSON: yaml: line 32: did not find expected key

when you run helm chart with value `ingres.web.annotations`

(cherry picked from commit 07670ec)
cognifloyd pushed a commit to cognifloyd/stackstorm-k8s that referenced this pull request Feb 16, 2022
The indentation under `web.annotations` was wrong (6 leading spaces
on first line, 4 on the rest) leading to

    Error converting YAMl to JSON: yaml: line 32: did not find expected key

when you run helm chart with value `ingres.web.annotations`

Partial Commit Extracted From: https://github.com/apache/airflow
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants