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

Add CI checks in separate PR #1347

Merged
merged 12 commits into from
Aug 21, 2024
Merged

Add CI checks in separate PR #1347

merged 12 commits into from
Aug 21, 2024

Conversation

GAlexIHU
Copy link
Contributor

@GAlexIHU GAlexIHU commented Aug 12, 2024

Overview

Adds checks to validate the integrity of migrations:

  1. Simple up-down-up on empty database (part of go test ...)
  2. Validates that ent schema was generated (internal/ent/db in sync with internal/ent/schema)
  3. Validates that migrations directory is in sync with schema (no diff)
  4. Runs atlas lint for last 10 migration files checking for migrations being non-destructive and linear (10 is arbitrary)
  5. Validates atlas checksum integrity (migration files weren't compromised)

The checks can also be run locally via make migrate-check

Fixes the atlas nix derivation to use platform specific version

Note: it's templated locally, not the ideal solution but I have some skill issues with nix

Follow Up Items

  • We should test that migrations work not just up-down-up but with a base of the latest release

Notes for reviewer

The checks are implemented in dagger, and for dependency management a nix-in-dagger solution was used. This is somewhat questionable, the 2 main drawbacks are:

  • doesn't really fit in the conceptual paradigm of dagger
  • nix core + devenv deps have to be pulled no matter how lightweight the environment is otherwise

(this latter is decently mitigated by caching but still)

The benefit form my point of view is that it's an exact replica the local environment by using the same derivation(s)

@GAlexIHU GAlexIHU added the release-note/misc Miscellaneous changes label Aug 12, 2024
@GAlexIHU GAlexIHU marked this pull request as ready for review August 14, 2024 11:58
@@ -72,6 +72,42 @@ jobs:
engine.stderr.log
retention-days: 14

migrations:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Todo: this should run as part of CI as well. Eventually, I'd like to see these separate jobs go away and run a single CI pipeline. Individual functions should be available as a convenience feature for local dev.

ci/migrate.go Outdated
Comment on lines 32 to 46
p.Go(syncFunc(nix.
WithServiceBinding("postgres", postgresNamed("no-diff")).
WithExec([]string{"sh", "-c", "nix develop --impure .#dagger -c atlas migrate --env ci diff test"}).
WithExec([]string{"sh", "-c", "nix develop --impure .#dagger -c sh -c 'if [[ -n $(git status --porcelain ./tools/migrate/migrations) ]]; then echo \"Migrations directory is dirty\"; exit 1; fi' "}),
))
// Always lint last 10 migrations
p.Go(syncFunc(nix.
WithServiceBinding("postgres", postgresNamed("last-10")).
WithExec([]string{"sh", "-c", "nix develop --impure .#dagger -c atlas migrate --env ci lint --latest 10"}),
))
// Validate checksum is intact
p.Go(syncFunc(nix.
WithServiceBinding("postgres", postgresNamed("validate")).
WithExec([]string{"sh", "-c", "nix develop --impure .#dagger -c atlas migrate --env ci validate"}),
))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably just use this image: https://hub.docker.com/r/arigaio/atlas

ci/migrate.go Outdated

// Always validate schema is generated
p.Go(syncFunc(nix.
WithExec([]string{"sh", "-c", "nix develop --impure .#dagger -c go generate ./internal/ent/..."}).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use the go moule to generate the code and then extract the directory using .Container().Directory().

ci/migrate.go Outdated
// Always validate schema is generated
p.Go(syncFunc(nix.
WithExec([]string{"sh", "-c", "nix develop --impure .#dagger -c go generate ./internal/ent/..."}).
WithExec([]string{"sh", "-c", "nix develop --impure .#dagger -c sh -c 'if [[ -n $(git status --porcelain ./internal/ent) ]]; then echo \"Ent schema wasnt generated\"; exit 1; fi' "}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can either use Git here, but you could also try using dagger.Directory.Diff to see if there is a diff between the first and the last version. That would utilize caching more.

@GAlexIHU GAlexIHU merged commit c1b8bb9 into feat/migrations Aug 21, 2024
17 checks passed
@GAlexIHU GAlexIHU deleted the feat/migrations-ci branch August 21, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc Miscellaneous changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants