Skip to content
This repository has been archived by the owner on Feb 2, 2024. It is now read-only.

feat: add fallback value for janus-idp.hostname for openshift clusters #169

Merged
merged 4 commits into from
Jan 23, 2024

Conversation

Zaperex
Copy link
Member

@Zaperex Zaperex commented Jan 9, 2024

Description of the change

Adds a fallback value for janus-idp.hostname for openshift clusters when both global.host and global.clusterRouterBase are not provided.

Existing or Associated Issue(s)

Fixes #167

Additional Information

Checklist

  • Chart version bumped in Chart.yaml according to semver.
  • Variables are documented in the values.yaml and added to the README.md. The pre-commit utility can be used to generate the necessary content. Use pre-commit run -a to apply changes.
  • JSON Schema template updated and re-generated the raw schema via pre-commit hook.
  • List tests pass for Chart using the Chart Testing tool and the ct lint command.

@Zaperex Zaperex requested a review from a team January 9, 2024 16:44
@masayag
Copy link

masayag commented Jan 9, 2024

Thank you @Zaperex

@masayag
Copy link

masayag commented Jan 22, 2024

@Zaperex any updates for this PR?

Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

One last thing: the default value for global.clusterRouterBase is apps.example.com, which means that if I want to use this new feature, I need to explicitly set it to an empty string in my values.yaml file, right?
Would it make sense to set the default value to an empty string instead, so that most users won´t even need to think about this? On OpenShift, without having to set global.clusterRouterBase, I would get the right hostname value.

charts/backstage/templates/_helpers.tpl Outdated Show resolved Hide resolved
charts/backstage/templates/_helpers.tpl Show resolved Hide resolved
charts/backstage/README.md Outdated Show resolved Hide resolved
@Zaperex
Copy link
Member Author

Zaperex commented Jan 22, 2024

whoops it seems the test charts github action isn't running in an openshift cluster, which is why it's failing with the new default value for global.clusterRouterBase

charts/backstage/templates/_helpers.tpl Outdated Show resolved Hide resolved
@Zaperex Zaperex requested a review from rm3l January 23, 2024 14:56
@openshift-ci openshift-ci bot added the lgtm label Jan 23, 2024
Copy link

openshift-ci bot commented Jan 23, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rm3l

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit d83df53 into janus-idp:main Jan 23, 2024
4 checks passed
@Zaperex Zaperex deleted the simplify-hostname branch January 23, 2024 15:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify the definition of the application host name
3 participants