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
table_schema
method 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 |
table_schemas
method 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
- Create a new table
CREATE TABLE public.things (id serial primary key);
-
GitlabSchemasValidateConnection
: In Rails Console:Thread.current[:query_analyzer_enabled_analyzers].append(::Gitlab::Database::QueryAnalyzers::GitlabSchemasValidateConnection)
Project.joins("inner join things on 1=1")
- 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'>
-
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.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #381499 (closed)