Skip to content
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

Implement checksummed migrations #275

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

APshenkin
Copy link

This fixes #254

During migration apply we store checksum (sha256) of the migration file.
If file was changed, then error returned when try to apply migration.

Unfortunately, didn't find the way to add additional column for already existing migration tables, maybe somebody can come up with the sollution

@rubenv
Copy link
Owner

rubenv commented Oct 29, 2024

I can't possibly merge this without a migration path, otherwise it'll break everyone's code.

Plus it'd be a break in compatibility/behavior.

Finally: how do you recover from the migration error in case it occurs?

@APshenkin
Copy link
Author

APshenkin commented Oct 30, 2024

I can't possibly merge this without a migration path, otherwise it'll break everyone's code.

It will not break existing installations, as if your db doesn't have checksum column, it will not populate data for it (will double check)

Plus it'd be a break in compatibility/behavior.

Same, it should not break compatibility. For all old installations that have no checksum collumn, nothing will change

migration path

What I come up with is that users of the library can write their own custom migration that will add column checksum to migration table and fill it with proper values if they want to migrate to checksum verification

Finally: how do you recover from the migration error in case it occurs?

The story is that this integrity check prevents you modify migration files after they were applied to database. If you need to modify then something in your schema, you should create new migration file, that will do required changes

@rubenv
Copy link
Owner

rubenv commented Oct 30, 2024

It will not break existing installations, as if your db doesn't have checksum column, it will not populate data for it (will double check)

Fairly certain gorp will throw an error if it misses columns.

@APshenkin
Copy link
Author

@rubenv oh, yes, you are right. Just my tests had some cache that's why it didn't throw errors.

I'll try to play around this to add new column

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Missing integrity checks on migrations lead to corrupt DB schema state
2 participants