Skip to content

Support raising an Error if table schema for a table is not known

What does this MR do and why?

The method Gitlab::Database::GitlabSchema.table_schema(<table_name>) tries to figure out to which table schema the table belongs: main or ci. It is using the database dictionary db/docs and some sane defaults and returns one of :gitlab_internal :gitlab_main, :gitlab_ci, :gitlab_shared

If this method fails to find a table schema, it will return a symbol, for example:

[2] pry(main)> Gitlab::Database::GitlabSchema.table_schema("new_table_that_was_not_in_data_directory")
=> :undefined_new_table_that_was_not_in_data_directory

After merge of !99287 (merged), it is now possible to specify the return value of table_schema method: it can be nil or the old behaviour :undefined_<table_name>.

This MR will add a table_schema! method: it will raise an error in case a schema is not found. This way, developers are aware that a newly created table should be added to the database directory db/docs

Apart from that, this MR will also support specifying the desired behaviour of table_schemas method: return nil or undefined_<table_name>.

Impact of this change

I ran git grep .table_schema and manually removed irrelavant matches. This leaves us with this list of calls to Gitlab::Database::GitlabSchema.table_schema and Gitlab::Database::GitlabSchema.table_schemas. For each of this usages, I checked if should allow nil or if we should raise an Error and explained my motivation

method table_schema

File change? Reasoning
app/models/concerns/cross_database_modification.rb no change Behaviour will not change
lib/gitlab/database/loose_foreign_keys.rb raise an error Developers should not create loose foreign keys for undefined schemas so let's raise an error.
scripts/decomposition/generate-loose-foreign-key raise an error This should fail if a table does not map to a schema

method table_schemas

method Gitlab::Database::GitlabSchema.table_schemas is looping over a list of tables and calls table_schema method on each of them. The default value of undefined parameter for table method will be used in this method as well.

File change? Reasoning
lib/gitlab/database/query_analyzers/gitlab_schemas_metrics.rb no change no change needed
lib/gitlab/database/query_analyzers/gitlab_schemas_validate_connection.rb no change no change needed
lib/gitlab/database/query_analyzers/prevent_cross_database_modification.rb no change no change needed
lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb no change no change needed

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

How to set up and validate locally

  1. Create a new table CREATE TABLE public.things (id serial primary key);
  2. GitlabSchemasValidateConnection: In Rails Console:
    1. Thread.current[:query_analyzer_enabled_analyzers].append(::Gitlab::Database::QueryAnalyzers::GitlabSchemasValidateConnection)
    2. Project.joins("inner join things on 1=1")
    3. This should raise an error, having unknown_schema in it: #<Gitlab::Database::QueryAnalyzers::GitlabSchemasValidateConnection::CrossSchemaAccessError: The query tried to access ["projects", "things"] (of gitlab_main,unknown_schema) which is outside of allowed schemas ([:gitlab_main, :gitlab_shared, :gitlab_internal]) for the current connection 'main'>
    4. Project.joins(:users) will not raise an error

MR acceptance checklist

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

Related to #381499 (closed)

Edited by Rutger Wessels

Merge request reports