-
-
Notifications
You must be signed in to change notification settings - Fork 790
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move DKIM keys to database #2952
base: master
Are you sure you want to change the base?
Conversation
Thanks for submitting this pull request. bors try Note: if this build fails, read this. |
tryBuild succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
7be325d
to
9c4bc69
Compare
Ah; I have code for doing this on a branch somewhere... IMHO we can't merge it as is: we want to address the other related DKIM tickets too: #2687 #2618 #2053 #2050 #1700 maybe even #1519 We need a separate table (as the same keys could be used cross-domains -like for ARC-), we probably want additional fields (keytype, usage, generation timestamp, status, ...) |
I see. But wouldn’t having the keys i the database even without changing the current behaviour that much, make a migration to the changes you mentioned easier? |
I don't mind incremental improvements... but the database schema should be as close to the end-state as possible. The last thing we want is to have to lost information in the process and make migration more complex as a result. At the very least, we want a separate table with a column for the selector, a timestamp it was generated/imported on, and a way to link it to the domain it was enabled for (keeping in mind that there may be multiple DKIM keys) |
Here is what I had on my branch if you need inspiration:
|
This PR solves issue #2949.
Moving the DKIM keys to database removes the requirement for shared storage in the admin.
Currently a migration path from files to database is missing. My idea is it to add this to the migration file, but i am not to deeply into the python framework here to know if this is a good idea or not. If you have some tips i happily add this feature so that this PR can get merged.