Fix devfile registry entry name and ingress url #203
Conversation
openshift-ci
bot
added
the
approved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #203 +/- ##
=======================================
Coverage 27.23% 27.23%
=======================================
Files 7 7
Lines 2012 2012
=======================================
Hits 548 548
Misses 1417 1417
Partials 47 47 ☔ View full report in Codecov by Sentry. |
michael-valdron
force-pushed
the
chart/fix-dr-entry-name
branch
2 times, most recently
from
414b63f to
65a1c02
Compare
Signed-off-by: Michael Valdron <[email protected]>
Signed-off-by: Michael Valdron <[email protected]>
michael-valdron
force-pushed
the
chart/fix-dr-entry-name
branch
from
9e42554 to
aafd64e
Compare
No changes needed. |
| @@ -17,6 +17,20 @@ | |||
| {{- .Values.hostnameOverride | default (printf "devfile-registry-%s" .Release.Namespace) -}} | |||
| {{- end -}} | |||
|
|
|||
| {{- define "devfileregistry.ingressHostname" -}} | |||
| {{- $hostname := .Values.hostnameOverride | default (printf "devfile-registry-%s" .Release.Namespace) -}} | |||
| {{- .Values.global.ingress.domain | printf "%s.%s" $hostname -}} | |||
There was a problem hiding this comment.
Will this format differ than the one from devfile/registry-operator#80? On the operator side the hostname is {hostName}.{ingressDomain} but here it looks like {ingressDomain}.{hostName}. Is this the case and if so does it matter or should they be identical in formatting?
There was a problem hiding this comment.
@Jdubrick The piping {{- .Values.global.ingress.domain | printf "%s.%s" $hostname -}} passes .Values.global.ingress.domain into the last argument of printf, so its still {hostName}.{ingressDomain}. I guess I could do {{- printf "%s.%s" $hostname .Values.global.ingress.domain -}} instead since the piping seems unnecessary.
There was a problem hiding this comment.
Ah I was under the impression it was going first, in that case I think it's all good, the piping gets the job done I just misunderstood.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Jdubrick, michael-valdron 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 |
Please specify the area for this PR
What does does this PR do / why we need it:
Fixes the devfile registry entry name for registry viewer component of the helm chart. Fixed name 'Community' changed to value of
{{ template "devfileregistry.name" . }}(the deployment name).Additionally, the ingress url is now define and built under
_template.tpl: https://github.com/michael-valdron/devfile-registry-support/blob/1df84bc08021587d954a3f6749699d77b40aaf28/deploy/chart/devfile-registry/templates/_template.tpl#L20-L27Currently, this is only built from the ingress domain for k8s and devfile/api#1467 needs resolving to either separate this field or not rely on manually setting the domain for OpenShift environments.
Which issue(s) this PR fixes:
Fixes #?
fixes devfile/api#1433
PR acceptance criteria:
Documentation (WIP)
How to test changes / Special notes to the reviewer: