Skip to content

fix(chart): harmonize protocol usage in urls#890

Merged
tom-rm-meyer-ISST merged 8 commits intomainfrom
fix/885-helm
Jun 2, 2025
Merged

fix(chart): harmonize protocol usage in urls#890
tom-rm-meyer-ISST merged 8 commits intomainfrom
fix/885-helm

Conversation

@tom-rm-meyer-ISST
Copy link
Copy Markdown
Contributor

@tom-rm-meyer-ISST tom-rm-meyer-ISST commented May 28, 2025 •
edited
Loading

Description

solves #885, #886, #887

Additionally I

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

  • DEPENDENCIES are up-to-date. Dash license tool. Committers can open IP issues for restricted libs.
  • Copyright and license header are present on all affected files
  • If helm chart has been changed, the chart version has been bumped to either next major, minor or patch level (compared to released chart).

This was linked to issues May 28, 2025
[Improvement] inconsistent handling of urls in helm template orgins backend-deployment template lacks protocol #885
Closed
[Bug] helm chart does not include http or https in fallback for allowed origins #887
Closed
@tom-rm-meyer-ISST tom-rm-meyer-ISST marked this pull request as ready for review May 28, 2025 15:50
@tom-rm-meyer-ISST
Copy link
Copy Markdown
Contributor Author

tom-rm-meyer-ISST commented May 28, 2025

Crosschecked multiple times: now the upgrade run with the nameOverride runs through: i added a pre-upgrade hook with sufficient priviledges :)

ReneSchroederLJ
ReneSchroederLJ requested changes May 30, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ReneSchroederLJ ReneSchroederLJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. I just found some minor spelling mistakes and I had one question regarding the implementation of allowed origins.

Comment thread docs/admin/Migration_Guide.md Outdated
Comment thread docs/admin/Migration_Guide.md Outdated
Comment thread docs/admin/Migration_Guide.md Outdated
Comment thread charts/puris/templates/backend-deployment.yaml Outdated
@tom-rm-meyer-ISST
Copy link
Copy Markdown
Contributor Author

tom-rm-meyer-ISST commented Jun 2, 2025

Not sure why the run fails in the pr but in a separately triggered action, but it doesn't fail. Maybe the pr runner is smaller.

ReneSchroederLJ
ReneSchroederLJ approved these changes Jun 2, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ReneSchroederLJ ReneSchroederLJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you

mhellmeier
mhellmeier approved these changes Jun 2, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@mhellmeier mhellmeier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The failed pipelines are related to a timeout issue.

@tom-rm-meyer-ISST tom-rm-meyer-ISST merged commit e583824 into main Jun 2, 2025
13 of 15 checks passed
@tom-rm-meyer-ISST tom-rm-meyer-ISST deleted the fix/885-helm branch June 2, 2025 12:50
@tom-rm-meyer-ISST tom-rm-meyer-ISST linked an issue Jun 3, 2025 that may be closed by this pull request
[Bug] helm chart does not handle nameOverride in migration from R25.03 to R25.06 correctly #886
Closed
@tom-rm-meyer-ISST tom-rm-meyer-ISST mentioned this pull request Jun 3, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhellmeier mhellmeier mhellmeier approved these changes

@ReneSchroederLJ ReneSchroederLJ ReneSchroederLJ approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

3 participants

@tom-rm-meyer-ISST @mhellmeier @ReneSchroederLJ
⚡