https://www.ory.sh/ logo
Join Slack
Powered by
<@U04PDLR0P37> It would be great if you could take...
# contributors
m
@enough-yak-81379 It would be great if you could take a look at this and see if I'm going in the right direction. If you think it's ok, I can do a full implementation. https://github.com/dlahn/k8s/commit/84b3b346751610cebb432cda1d98ce87074a113f
e
Hi there! This seems a simple approach, but does not address the dsn value inside the secret. imho we need to address the specific cases: • default: the chart deploys a single secret with dsn and other secrets, loads as envs • the chart expects a side loaded secret which contains everything • new: the chart loads 2 secrets (dns, others). Here i think we should support also the case that we create the secret as well as load it, to have the same behaviour as with the default case
that is why i would gravitate towards a refactor of the config and splitting the secret management like
Copy code
yaml
secret:
  create: true # charts creates the secret vs user creates the secret
  dsn:
    create: false # dsn is inside the default secret vs dedicated secret
m
What if.. instead.. we looked to see if DSN is defined in extraEnv and omit it if it is?
e
🤔 interesting approach. You mean, add a function to the current DSN env to iterate over all envs in extraEnvs and if we find DSN, then use that, otherwise fallback to the standard one? That may work with the approach that objects like couriers can derive their envs from deployments
to be honest i am a little puzzled here. If we template the chart using the test values from the repo
Copy code
helm template kratos helm/charts/kratos -f hacks/values/kratos.yaml | subl
in the resulting yamls we don’t see env duplication
Untitled.yaml
Untitled.yaml
can the duplicated dsn come from helm running a client-side merge and not from the template itself?
m
The duplicate issue only comes when we use an external secret and add DSN as an extraEnv because we don't want to use the one within the secret.
I know the duplicate only generates a warning if you apply it manually, but in ArgoCD it fails, and from what I have been reading, it is highly discouraged to have duplicates.
If you are ok with looking for DSN in extraEnv and using that, we are happy to make a PR for this.
e
i am aware of that, we use ot as a workaround internally and it can be done, but its not the best approach
i actually started to work on it today
Copy code
{{/*
Check if list contains object
*/}}
{{- define "kratos.extraEnvContainsEnvName" -}}
  {{- $extraEnvs := index . 0 -}}
  {{- $envName := index . 1 -}}
  {{- range $k, $v := $extraEnvs -}}
    {{- if eq $v.name $envName -}}
      found
    {{- end -}}
  {{- end -}}
{{- end -}}
the approach is simple but the downside is we have to touch a lot of templates to implement this behaviour
m
yes. It is the most elegant thing we could think of though
e
i have opened a draft with that https://github.com/ory/k8s/pull/689
feel free to check it out and see if it works for you
👍 1
i have added it to courier right now, but i would like to make it more elegant: move the function to a common library chart and include from there
and use in the rest of the charts
m
This is the approach we were talking about yesterday, so this seems like it would work well for us. I'll pass it on to the team as well.
e
m
sweet
I
I'll put it into staging
e
damn, not so soon, need to tinker with the release pipeline after adding the lib chart 😕
m
aha
It seems to work for us. Thank you!
❤️ 1
10 Views
Open in Slack
PreviousNext
Powered by