feat: deprecate autoincrement primary key in hydra_client

[grantzvolsky]

This patch deprecates the hydra_client.pk autoincrement primary key in favour of a new UUID primary key column. The values of the new column are generated in application code instead of being generated in the database.

This is the first step towards resolving #2781. The issue will be resolved when we remove the deprecated column.

CAUTION: The patch includes a database migration that must be applied offline while simultaneously updating all hydra instances to the latest version.

Related issue(s)

#2781
@aeneasr

Checklist

• I have read the contributing guidelines.
• I am following the
  contributing code guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security. vulnerability, I
  confirm that I got green light (please contact
  security@ory.sh) from the maintainers to push
  the changes.
• [Tests already existed] I have added tests that prove my fix is effective or that my feature
  works.
• [Nothing to document other than an entry in the changelog] I have added or changed the documentation.

Further Comments

[aeneasr]

Looking good :)

[aeneasr]

Are the test passing for you locally?

[aeneasr]

If you need help with the DB tests LMK

[grantzvolsky]

Are the test passing for you locally?

The tests are passing most of the time, but I randomly get this[1] error with ~5% probability. Is this test randomly failing a known issue?

[1]:
Test: TestManagers/package=consent/janitor=mysql/case=flush-consent-request-limit/case=limit/step=validate
Erro: An error is expected but got nil.
https://app.circleci.com/pipelines/github/ory/hydra/2992/workflows/4fef669c-6357-4031-9902-38594d68b5a3/jobs/33318

[grantzvolsky]

If you need help with the DB tests LMK

I've just added this1 test to make sure that the migrations result in a table with the correct columns. Should I add more tests, e.g. to make sure that the backfilled UUIDs are valid UUIDv4?

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (v2.x@8373bba). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             v2.x    #2784   +/-   ##
=======================================
  Coverage        ?   52.87%           
=======================================
  Files           ?      236           
  Lines           ?    14185           
  Branches        ?        0           
=======================================
  Hits            ?     7500           
  Misses          ?     6053           
  Partials        ?      632           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8373bba...1d20c54. Read the comment docs.

[aeneasr]

to make sure that the backfilled UUIDs are valid UUIDv4?

I think that would make sense! It would also be good to ensure that new client creations use the new UUID key format. You could test this by just creating a client and also fetching it.

The tests are passing most of the time, but I randomly get this[1] error with ~5% probability. Is this test randomly failing a known issue?

Test: TestManagers/package=consent/janitor=mysql/case=flush-consent-request-limit/case=limit/step=validate

Yes, this is a known issue /cc @Benehiko @flavioleggio

[aeneasr]

LGTM :)

[aeneasr]

@grantzvolsky I changed base to v2.x which we will use to have all these migration changes so that we can still do releases in case of bug fixes and maintenances :)