Dealing with secret names that are too long #463
Labels
No Label
abra
abra-gandi
awaiting-feedback
backups
bug
build
ci/cd
community organising
contributing
coopcloud.tech
democracy
design
documentation
duplicate
enhancement
finance
funding
good first issue
help wanted
installer
kadabra
performance
proposal
question
recipes.coopcloud.tech
security
test
wontfix
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: coop-cloud/organising#463
Loading…
Reference in New Issue
No description provided.
Delete Branch "%!s(<nil>)"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
In docker a secret name can have a maximum of 64 characters.
The matrix recipe contains a secret name called "registration_shared_secret_v1" that is already quite long with 30 characters. So only 34 characters are left for the domain part of the secret.
With this secret we got some problems as we have a domain that is already 30 characters long. Together with the
matrix.subdomain we have 37 characters.Now we are not able to setup the matrix recipe for this domain without either changing the recipe or changing the domain.
For the future I think there should be a length restriction on secret naming conventions and there should be a warning for long domains, so that you never choose a too long domain together with abra.
Ah yes, that's true 😬 Sucks that you ran into this...
I vaguely remember dealing with this on the
STACK_NAMEwhich we internally trim viaa7ce71d6cf/pkg/config/app.go (L60-L63). 45 was an arbitrary choice which leaves enough space on the end for other things...Do you want to put a PR into the synapse recipe to rename
registration_shared_secrettoreg_shared_secret? It might save others future pain for the current code. We can write release notes and help people migrate their secrets.Do you have ideas where we can put the length restriction for the secret naming that will be picked up early? We have recipe linting but unsure how many people use it.
The long domain name warning could help alright. Any ideas on what length we should start to warn at? Sometimes you can change a sub-domain but if you have a long domain, it could get annoying?
Thanks for opening!
Ah, I see
7b1b5c37ed00241b95c14b361247af278dea99cd(synapserecipe) now 👍Yes I already changed the synapse recipe with a release note.
Not sure what is a good length.
Here I listed the current secret name lengths.
About the half of the secret names are less equal 12 chars. With
SECRET_<secret_name>_VERSIONthey expand to 27 chars, so 37 chars are left for the complete subdomain. I would say this could be a good number.A warning should be shown for domains about 30 chars, and an error for subdomains longer than 37 chars.
The downside is that we need to change the secrets of about 33 recipes.
I think the ideal place for the length restriction should be the linting. Isn't the linting automatically executed before parsing the recipe?
Including via linting sounds good! We only lint for errors before
upgrade/deploy/rollback, so we might want to tweak that, potentially controversial 🙃 Numbers look good. At least people will be informed and can take action from there instead of running into a docker runtime error.Too long secret namesto Dealing with secret names that are too longHey,
I was looking at potentially picking this up but I want to make sure I understand the problem and the proposed solution before I start. From my limited understanding we have 2 problems:
From my limited understanding of the code it seems like linting can solve problem 1 but not problem 2. Linting seems to take place on the recipe rather than the app where it's the combination of the two things that causes the problem. Obviously linting secrets bigger than a certain size could help but someone can always come up with a very long domain name and encounter the same problem.
Hopefully I've managed to grasp the issue though I'm still not 100% on how the final secret names are generated : ) So if linting doesn't check the app name, do we want a variant that does? Do we want a separate check before deployment for this specific scenario? Do we want to just lint the secret names in the recipe to just reduce the probability of long domain names causing a problem? Should I give up and try and tackle something else that boils my brain a little less 😅
Let me know what you think : )
heyyy @rix, that's great! this is a difficult one alright. you've summed it up perfectly!
As @moritz mentioned:
I think we should try implement a linting rule (https://git.coopcloud.tech/coop-cloud/abra/src/branch/main/pkg/lint/recipe.go).
If
Level: "error"then1e6a6e6174/pkg/lint/recipe.go (L166-L195)will fail deployments. So, let's keep it at"warn"as mentioned by @moritz:AFAIU then people won't be plagued by this warning on deploy/upgrade/etc. but when they lint the recipe, they'll see they need to take action. As mentioned in #463 (comment) I don't think we should go on a renaming rampage as part of this ticket, people can update their own recipes.
We should also document this in the recipe maintainers handbook?
If you want to implement this additional rule, feel free! might be nice but if we have already something for the domain name, not sure how useful it will be.
Thanks!
Work underway coop-cloud/abra#359 👏
coop-cloud/abra#359 is merged! I pushed a few fixups. I also pushed
064a26e182for docs. There might be more we're missing, so please do comment if you have more to add. Closing for now, thanks all.