Skip to content
Snippets Groups Projects

Add background migration to expiry all OAuth tokens

Merged Imre Farkas requested to merge if-oauth_tokens_migration_to_expire into master

What does this MR do and why?

:exclamation: This is a breaking change for %15.0. :exclamation:

In !86362 (merged), we are removing the backward compatibility for non-expiring OAuth tokens introduced. This MR makes sure that all existing tokens have an expiry, otherwise they would never expire.

Related to #340848 (closed)

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

Migration output

up

$ rake db:migrate:up:main VERSION=20220428133724                                                                                                                                                                
== 20220428133724 ScheduleExpireOAuthTokens: migrating ========================
== 20220428133724 ScheduleExpireOAuthTokens: migrated (0.0431s) ===============

down

$ rake db:migrate:up:main VERSION=20220428133724                                                                                                                                                                
== 20220428133724 ScheduleExpireOAuthTokens: migrating ========================
== 20220428133724 ScheduleExpireOAuthTokens: migrated (0.0431s) ===============

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Imre Farkas

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Imre Farkas
  • Imre Farkas requested review from @alexbuijs

    requested review from @alexbuijs

  • Imre Farkas requested review from @jdrpereira

    requested review from @jdrpereira

  • João Pereira approved this merge request

    approved this merge request

  • :wave: @jdrpereira, thanks for approving this merge request.

    This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.

    For more info, please refer to the following links:

  • João Pereira requested review from @krasio

    requested review from @krasio

  • Alex Buijs
  • Alex Buijs approved this merge request

    approved this merge request

  • Alex Buijs requested review from @.luke and removed review request for @alexbuijs

    requested review from @.luke and removed review request for @alexbuijs

  • Krasimir Angelov
  • Database migrations

    Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access).

    Migration Type Total runtime Result DB size change
    20220428133724 - ScheduleExpireOAuthTokens Post deploy 1.5 s :white_check_mark: +0.00 B
    Runtime Histogram for all migrations
    Query Runtime Count
    0 seconds - 0.01 seconds 0
    0.01 seconds - 0.1 seconds 4
    0.1 seconds - 1 second 0
    1 second - 5 minutes 0
    5 minutes + 0

    Migration: 20220428133724 - ScheduleExpireOAuthTokens

    • Type: Post deploy
    • Duration: 1.5 s
    • Database size change: +0.00 B
    Query Calls Total Time Max Time Mean Time Rows
    SELECT MAX("id")
    FROM "oauth_access_tokens"
    /*application:test,db_config_name:main*/
    1 5.8 ms 5.8 ms 5.8 ms 1
    INSERT INTO "batched_background_migrations" ("created_at", "updated_at", "max_value", "batch_size", "sub_batch_size", "interval", "status", "job_class_name", "table_name", "column_name", "total_tuple_count", "started_at") VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING "id" /*application:test,db_config_name:main*/
    1 0.1 ms 0.1 ms 0.1 ms 1
    SELECT $1 AS one
    FROM "batched_background_migrations"
    WHERE "batched_background_migrations"."job_class_name" = $2 AND "batched_background_migrations"."table_name" = $3 AND "batched_background_migrations"."column_name" = $4 AND (job_arguments = $5)
    LIMIT $6 /*application:test,db_config_name:main*/
    1 0.0 ms 0.0 ms 0.0 ms 0
    SELECT $1 AS one
    FROM "batched_background_migrations"
    WHERE "batched_background_migrations"."job_arguments" = $2 AND "batched_background_migrations"."job_class_name" = $3 AND "batched_background_migrations"."table_name" = $4 AND "batched_background_migrations"."column_name" = $5
    LIMIT $6 /*application:test,db_config_name:main*/
    1 0.0 ms 0.0 ms 0.0 ms 0
    Histogram for ScheduleExpireOAuthTokens
    Query Runtime Count
    0 seconds - 0.01 seconds 0
    0.01 seconds - 0.1 seconds 4
    0.1 seconds - 1 second 0
    1 second - 5 minutes 0
    5 minutes + 0

    Background migrations


    Other migrations pending on GitLab.com
    Migration Type Total runtime Result DB size change
    20220413235818 - AddMaxSeatsUsedChangedAtToGitlabSubscriptions Regular 1.2 s :white_check_mark: +0.00 B
    20220414203622 - AddIndexForColumnsUserCustomAttribute Regular 17.3 s :white_check_mark: +188.74 MiB
    20220419223906 - AddArkoseNamespaceToApplicationSettings Regular 2.2 s :white_check_mark: +0.00 B
    20220420173247 - AddGroupInheritanceTypeToPeAuthorizable Regular 1.3 s :white_check_mark: +0.00 B
    20220422220507 - RemoveTmpIndexSupportingLeakyRegexCleanup Regular 2.8 s :white_check_mark: -88.00 KiB
    20220324165436 - ScheduleBackfillProjectSettings Post deploy 1.8 s :white_check_mark: +0.00 B

    Clone Details

    Clone ID Clone Created At Clone Data Timestamp Expected Removal Time
    database-testing-1185746 2022-05-05T02:19:42Z 2022-04-26T01:21:49Z 2022-05-05 14:23:20 +0000

    Artifacts


    Brought to you by gitlab-org/database-team/gitlab-com-database-testing. Epic

  • Luke Duncalfe
  • Luke Duncalfe
  • Luke Duncalfe removed review request for @.luke

    removed review request for @.luke

  • @ifarkas thanks for adding the breaking change label!

    This merge request introduces breaking changes. Learn more about breaking changes.

    It's important to identify how the breaking change was introduced. To estimate the impact, try to assess the following:

    • Are there existing users depending on this feature?
      • Are self-managed customers affected?
      • To verify and quantify usage, use Grafana or Kibana.
      • If you're not sure about how to query the data, contact the infrastructure team on their Slack channel, #infrastructure-lounge
    • Was sufficient time given to communicate the change?
    • Changes in the permissions, the API schema, and the API response might affect existing 3rd party integrations.
      • Reach out to the Support team or Technical Account Managers and ask about the possible impact of this change.
  • Imre Farkas added 1014 commits

    added 1014 commits

    Compare with previous version

  • Imre Farkas requested review from @.luke

    requested review from @.luke

  • Luke Duncalfe approved this merge request

    approved this merge request

  • Luke Duncalfe removed review request for @.luke

    removed review request for @.luke

  • Imre Farkas added 225 commits

    added 225 commits

    Compare with previous version

  • Imre Farkas added 57 commits

    added 57 commits

    Compare with previous version

  • Krasimir Angelov approved this merge request

    approved this merge request

  • added databaseapproved label and removed databasereviewed label

  • Imre Farkas resolved all threads

    resolved all threads

  • Imre Farkas enabled an automatic merge when the pipeline for 8a420731 succeeds

    enabled an automatic merge when the pipeline for 8a420731 succeeds

  • merged

  • Imre Farkas mentioned in commit 832d65d8

    mentioned in commit 832d65d8

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading