ory / k8s

Kubernetes Helm Charts for the ORY ecosystem.
https://k8s.ory.sh/helm
Apache License 2.0
335 stars 262 forks source link

Incorrect value for issuer/URLS_SELF_ISSUER in hydra template deployment.yaml #708

Closed Prusias closed 6 days ago

Prusias commented 1 week ago

Preflight checklist

Ory Network Project

No response

Describe the bug

The deployment.yaml template of the hydra chart contains the following code:

env:
            {{- $issuer := include "hydra.config.urls.issuer" . -}}
            {{- if $issuer }}
            - name: URLS_SELF_ISSUER
              value: {{ $issuer | quote }}

This points to the issuer value being set at hydra.config.urls.issuer.

However, the documentation states that it should be set as hydra.config.urls.self.issuer. The values.yaml gives a half example, namely hydra.config.urls.self: {}

Interestingly, setting the issuer using hydra.config.urls.self.issuer works fine, and updates the URLS_SELF_ISSUER environment value in the deployment. Setting any value on hydra.config.urls.issuer will cause the container to crash on start-up.

Reproducing the bug

Set the hydra.config.urls.issuer value in your deployment to anything.

Relevant log output

││ The configuration contains values or keys which are invalid: 
││ urls: map[issuer:<url> self:map[issuer:http://127.0.0.1:4444/]
││       ^-- additionalProperties "issuer" not allowed 
││
││ time=2024-10-09T14:31:39Z level=error msg=Unable to instantiate configuration. audience=application error=map[message:I[#/urls] S[#/properties/urls/additionalProperties] additionalProperties "issuer" not allowed] service_name=Ory Hydra service_version=v2.2.0
││ Error: I[#/urls] S[#/properties/urls/additionalProperties] additionalProperties "issuer" not allowed
││ Usage:
││   hydra serve all [flags] 

Relevant configuration

hydra:
  config:
    urls:
      issuer: <url>

Version

0.49.0

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Kubernetes with Helm

Additional Context

No response

Demonsthere commented 6 days ago

Hi there! I think this was a change in hydra, that we missed in the chart 😓

Demonsthere commented 6 days ago

🤔 Wait a moment, the {{- $issuer := include "hydra.config.urls.issuer" . -}} is just a named template

{{/*
Generate the urls.issuer value
*/}}
{{- define "hydra.config.urls.issuer" -}}
{{- if .Values.hydra.config.urls.self.issuer -}}
{{- .Values.hydra.config.urls.self.issuer }}
{{- else if .Values.ingress.public.enabled -}}
{{- $host := index .Values.ingress.public.hosts 0 -}}
http{{ if $.Values.ingress.public.tls }}s{{ end }}://{{ $host.host }}
{{- else if contains "ClusterIP" .Values.service.public.type -}}
http://127.0.0.1:{{ .Values.service.public.port }}/
{{- end -}}
{{- end -}}

and here we can see, that if defined .Values.hydra.config.urls.self.issue it is used

Prusias commented 6 days ago

It seems that I got confused by the helm template then :see_no_evil:, and totally missed it was including a value, rather than using .Values. I think this is not actually an issue then!

The only minor suggestion I could give is to make the example in the values.yaml a bit more explicit. Though it is possible to figure out, of course.

Demonsthere commented 6 days ago

Sure thing :) Since you are fresh on the topic, would you mind adding something to the values, which would be helpful to yourself from the past? :D

Prusias commented 6 days ago

I'd suggest something like the following, just to make it a tiny bit more clear. But I'm not sure if this is necessary.

    urls:
      # -- Configure the urls used by hydra itself, such as the issuer.
      # self: {
      #   issuer: "https://public.hydra.localhost:4444/"
      # }
      self: {}
Demonsthere commented 6 days ago

Docs updated in https://github.com/ory/k8s/commit/e51f83d2b9437269768ec1b5e471a882ac954188 :)