Adds logic to create projectSeed and expose to build services #336
Conversation
| @@ -15,3 +16,22 @@ metadata: | |||
| {{- include "lagoon-core.labels" . | nindent 4 }} | |||
| stringData: | |||
| JWTSECRET: {{ $jwtSecret | quote }} | |||
| --- | |||
| {{/* | |||
| This additonally complex logic is intended to handle the projectsecret: | |||
There was a problem hiding this comment.
Do we need to add this extra complexity to the chart? The problem with doing this is that it tightly couples the chart to the current implementation in Lagoon which causes the bug described in uselagoon/lagoon#2858.
Given that uselagoon/lagoon#2859 already prefers PROJECTSECRET and falls back to JWTSECRET to generate project secrets, could we just add a simple option to set PROJECTSECRET in the chart? Would that combined with uselagoon/lagoon#2859 solve uselagoon/lagoon#2858?
I don't think we need to worry about backwards compatibility since the bug is only exposed in very specific circumstances i.e. when rotating JWTSECRET. So you won't expose the bug just by upgrading lagoon-core. And we can just document that if you do rotate JWTSECRET and use k8up, then you also need to set PRJOJECTSECRET.
There was a problem hiding this comment.
ok - I'll think on this a little more.
ideally, the project secret is only handled in the chart, not in the API fallback.
I think also on a fresh install the two should be intentionally different, not the same.
There was a problem hiding this comment.
ok - I've coordinated with the Lagoon PR, and we've renamed this PROJECTSEED - and it is autogenerated on first Lagoon install, or set to JWTSECRET if Lagoon is already installed (for maximal backwards compatibility).
Having a PROJECTSEED available in the charts that is differentiated from JWTSECRET may allow further possibilities.
There was a problem hiding this comment.
Ok, got it. I think we can simplify the logic a bit, and maybe consider sticking PROJECTSEED in the same lagoon-core-jwtsecret?
{{- $projectSeed := coalesce .Values.projectSeed (index $data "PROJECTSEED" | default "" | b64dec) (index $data "JWTSECRET" | default "" | b64dec) (randAlpha 32) }}
...
PROJECTSEED: {{ $projectSeed | quote }}
There was a problem hiding this comment.
So, I've simplified the logic, less ternarys are generally a good thing.
I've kept the projectSeed as a separate secret to avoid potential confusion if we tuck it in with the jwtSecret. Ideally, there should probably be a single lagoon-core secrets that all these things live it, but that can be a future consideration.
There was a problem hiding this comment.
Ideally, there should probably be a single lagoon-core secrets that all these things live it, but that can be a future consideration.
Agreed, but maybe we can just do it as part of this PR? We can just remove the -jwtsecret suffix and that should be enough?
There was a problem hiding this comment.
ok - I've got a rewrite that creates lagoon-core-secrets, copies in the existing jwtSecret, and also handles the projectSeed - and have repointed all the downstream jwtsecret refs to it - in final testing now.
I'll obvs have to write a release note about it, but I think it makes sense, and gives us a -secrets we can use down the track for more.
tobybellwood
changed the title
|
Hey @tobybellwood is this PR nearly ready for review/merge? |
|
This is included in the 210 release branch #341 now. Will close this |
lagoon-charts update for uselagoon/lagoon#2858
gist of testing process (old PROJECTSECRET terminology notwithstanding) at https://gist.github.com/tobybellwood/4abfc42b5f2ef1e9f1632cef7df3d2a8
closes #335
The logic here creates a projectSeed on first install, or from the current jwtSecret if Lagoon is already installed (for backwards compatibility), unless a projectSeed has been specified in the values, or already exists.
I've added the PROJECTSEED to the two services most likely to interact with a build, API and webhooks2tasks