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(openshift): allow usage of the route with the default router #2646

Merged
merged 2 commits into from
Dec 10, 2024

Conversation

leiicamundi
Copy link
Contributor

@leiicamundi leiicamundi commented Dec 5, 2024

Which problem does the PR fix?

This PR introduces a fix for #2620

What's in this PR?

The fix is implemented by checking the className, if it contains "openshift-" and if no secret is set, then the tls block is completely removed, allowing fallback on the TLS of the Router.
It does not break the other mechanisms related to the https scheme.

For the context, here's an example usage https://github.com/camunda/camunda-deployment-references/blob/12cfae837701d9bc1c7e583400bfd897eee60efc/aws/rosa-hcp/camunda-versions/8.7/procedure/install/helm-values/domain.yml#L12 (don't pay attention to the version, it's a 8.6 file, I've not tested it with 8.7, but it should work also)

Checklist

Please make sure to follow our Contributing Guide.

Before opening the PR:

  • In the repo's root dir, run make go.update-golden-only.
  • There is no other open pull request for the same update/change.
  • Tests for charts are added (if needed). <= we don't test the default Router currently, I've tested it as part of the infraex reference arch
  • In-repo documentation are updated (if needed). <= The associated documentation will be updated with the introduction of ROSA specific reference arch

After opening the PR:

  • Did you sign our CLA (Contributor License Agreement)? It will show once you open the PR.
  • Did all checks/tests pass in the PR?

@leiicamundi
Copy link
Contributor Author

Hey @hamza-m-masood, can you please review this PR? It's a dependency of P1 Multiregion Openshift topic

@hamza-m-masood
Copy link
Contributor

Hey @leiicamundi !
I did not test this for myself but the change makes logical sense. I will approve

@leiicamundi
Copy link
Contributor Author

Thank you :) I'm merging it then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants