diff options
32 files changed, 481 insertions, 251 deletions
diff --git a/.gitlab/issue_templates/SHA256_Bug.md b/.gitlab/issue_templates/SHA256_Bug.md new file mode 100644 index 00000000000..18638c23b81 --- /dev/null +++ b/.gitlab/issue_templates/SHA256_Bug.md @@ -0,0 +1,23 @@ +<!-- Title suggestion: [Sha 256] <issue description> --> + +This issue describes an anomaly with the GitLab application with projects that +use SHA256 as the hashing algorithm in the repository. + +## Where in the application are you seeing this issue? + +### URL of the page + +<!-- Provide the URL of the page in question --> + +### How did you expect the application to behave? + +<!-- Provide a description of how you expected the application to behave, look, +etc --> + +### How did the application behave? + + +<!-- Provide a description of how the application actually behaved --> + +/epic https://gitlab.com/groups/gitlab-org/-/epics/10981 +/label ~"group::gitaly" ~"group::gitaly::git" ~"type::bug" diff --git a/app/assets/stylesheets/framework/emojis.scss b/app/assets/stylesheets/framework/emojis.scss index d3986f31d52..92cb509148f 100644 --- a/app/assets/stylesheets/framework/emojis.scss +++ b/app/assets/stylesheets/framework/emojis.scss @@ -27,7 +27,7 @@ gl-emoji { .emoji-picker-category-header { @include gl-sticky; - background-color: $white-transparent; + background: linear-gradient(to bottom, $white 50%, transparent 100%); } .emoji-picker-emoji { diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index eb627b036fe..c99e985e18c 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -603,6 +603,6 @@ span.idiff { right: 0; top: -$gradient-size; height: $gradient-size; - background: linear-gradient(to top, $white, transparentize($white, 1)); + background: linear-gradient(to top, $white, transparent); } } diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index ab8547c3fef..3770a9b68a0 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -182,7 +182,6 @@ $t-gray-a-24: rgba($gray-950, 0.24) !default; $white: #fff !default; $white-normal: $gray-50 !default; $white-dark: darken($gray-50, 2) !default; -$white-transparent: rgba($white, 0.8) !default; $black: #000 !default; $black-transparent: $t-gray-a-24 !default; diff --git a/app/models/bulk_imports/batch_tracker.rb b/app/models/bulk_imports/batch_tracker.rb index eb7fe9f9913..8561dfacb6e 100644 --- a/app/models/bulk_imports/batch_tracker.rb +++ b/app/models/bulk_imports/batch_tracker.rb @@ -8,6 +8,8 @@ module BulkImports validates :batch_number, presence: true, uniqueness: { scope: :tracker_id } + scope :by_last_updated, -> { order(updated_at: :desc) } + state_machine :status, initial: :created do state :created, value: 0 state :started, value: 1 diff --git a/app/models/bulk_imports/tracker.rb b/app/models/bulk_imports/tracker.rb index b06583c8e06..b5092591019 100644 --- a/app/models/bulk_imports/tracker.rb +++ b/app/models/bulk_imports/tracker.rb @@ -24,7 +24,6 @@ class BulkImports::Tracker < ApplicationRecord delegate :file_extraction_pipeline?, :abort_on_failure?, to: :pipeline_class DEFAULT_PAGE_SIZE = 500 - STALE_AFTER = 4.hours scope :next_pipeline_trackers_for, -> (entity_id) { entity_scope = where(bulk_import_entity_id: entity_id) @@ -88,8 +87,4 @@ class BulkImports::Tracker < ApplicationRecord transition [:created, :started] => :timeout end end - - def stale? - created_at < STALE_AFTER.ago - end end diff --git a/app/services/ml/model_versions/update_model_version_service.rb b/app/services/ml/model_versions/update_model_version_service.rb new file mode 100644 index 00000000000..a0de87792f8 --- /dev/null +++ b/app/services/ml/model_versions/update_model_version_service.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Ml + module ModelVersions + class UpdateModelVersionService + def initialize(project, name, version, description) + @project = project + @name = name + @version = version + @description = description + end + + def execute + model_version = Ml::ModelVersion + .by_project_id_name_and_version(@project.id, @name, @version) + + return ServiceResponse.error(message: 'Model not found') unless model_version.present? + + result = model_version.update(description: @description) + + return ServiceResponse.error(message: 'Model update failed') unless result + + ServiceResponse.success(payload: model_version) + end + end + end +end diff --git a/app/workers/bulk_imports/finish_batched_pipeline_worker.rb b/app/workers/bulk_imports/finish_batched_pipeline_worker.rb index 73ae2188ac4..980cdb98f32 100644 --- a/app/workers/bulk_imports/finish_batched_pipeline_worker.rb +++ b/app/workers/bulk_imports/finish_batched_pipeline_worker.rb @@ -6,6 +6,7 @@ module BulkImports include ExceptionBacktrace REQUEUE_DELAY = 5.seconds + STALE_AFTER = 4.hours idempotent! deduplicate :until_executing @@ -18,24 +19,21 @@ module BulkImports @tracker = Tracker.find(pipeline_tracker_id) @context = ::BulkImports::Pipeline::Context.new(tracker) - return unless tracker.batched? - return unless tracker.started? + return unless tracker.batched? && tracker.started? + + @sorted_batches = tracker.batches.by_last_updated + return fail_stale_tracker_and_batches if most_recent_batch_stale? + return re_enqueue if import_in_progress? - if tracker.stale? - logger.error(log_attributes(message: 'Tracker stale. Failing batches and tracker')) - tracker.batches.map(&:fail_op!) - tracker.fail_op! - else - tracker.pipeline_class.new(@context).on_finish - logger.info(log_attributes(message: 'Tracker finished')) - tracker.finish! - end + tracker.pipeline_class.new(@context).on_finish + logger.info(log_attributes(message: 'Tracker finished')) + tracker.finish! end private - attr_reader :tracker + attr_reader :tracker, :sorted_batches def re_enqueue with_context(bulk_import_entity_id: tracker.entity.id) do @@ -44,7 +42,19 @@ module BulkImports end def import_in_progress? - tracker.batches.any? { |b| b.started? || b.created? } + sorted_batches.any? { |b| b.started? || b.created? } + end + + def most_recent_batch_stale? + return false unless sorted_batches.any? + + sorted_batches.first.updated_at < STALE_AFTER.ago + end + + def fail_stale_tracker_and_batches + logger.error(log_attributes(message: 'Batch stale. Failing batches and tracker')) + sorted_batches.map(&:fail_op!) + tracker.fail_op! end def logger diff --git a/db/migrate/20231114231330_add_released_at_to_catalog_resource_versions.rb b/db/migrate/20231114231330_add_released_at_to_catalog_resource_versions.rb new file mode 100644 index 00000000000..8984eaef9ca --- /dev/null +++ b/db/migrate/20231114231330_add_released_at_to_catalog_resource_versions.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class AddReleasedAtToCatalogResourceVersions < Gitlab::Database::Migration[2.2] + milestone '16.7' + + disable_ddl_transaction! + + OLD_INDEX = 'index_catalog_resource_versions_on_catalog_resource_id' + NEW_INDEX = 'index_catalog_resource_versions_on_resource_id_and_released_at' + + def up + # This will be denormalized with data from the `releases` table + add_column :catalog_resource_versions, :released_at, :datetime_with_timezone, default: '1970-01-01', null: false + + remove_concurrent_index_by_name :catalog_resource_versions, OLD_INDEX + add_concurrent_index :catalog_resource_versions, [:catalog_resource_id, :released_at], name: NEW_INDEX + end + + def down + remove_concurrent_index_by_name :catalog_resource_versions, NEW_INDEX + add_concurrent_index :catalog_resource_versions, :catalog_resource_id, name: OLD_INDEX + + remove_column :catalog_resource_versions, :released_at + end +end diff --git a/db/post_migrate/20231115104943_remove_service_access_tokens_category_column.rb b/db/post_migrate/20231115104943_remove_service_access_tokens_category_column.rb new file mode 100644 index 00000000000..42879d1e70d --- /dev/null +++ b/db/post_migrate/20231115104943_remove_service_access_tokens_category_column.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class RemoveServiceAccessTokensCategoryColumn < Gitlab::Database::Migration[2.2] + milestone '16.7' + + def change + remove_column :service_access_tokens, :category, :integer, limit: 2, default: 0, null: false + end +end diff --git a/db/schema_migrations/20231114231330 b/db/schema_migrations/20231114231330 new file mode 100644 index 00000000000..6debfc70f4f --- /dev/null +++ b/db/schema_migrations/20231114231330 @@ -0,0 +1 @@ +0bff5e9182931ab42dd71c0b130172cde5acc7ee37c50e77b3f160507d556ce1
\ No newline at end of file diff --git a/db/schema_migrations/20231115104943 b/db/schema_migrations/20231115104943 new file mode 100644 index 00000000000..d9023806426 --- /dev/null +++ b/db/schema_migrations/20231115104943 @@ -0,0 +1 @@ +dc8c51691062b08e02ef0c48a835a7b65a699012ef19c5635da700b0c550a375
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index c56525ef09e..875cf3a4a23 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13573,7 +13573,8 @@ CREATE TABLE catalog_resource_versions ( release_id bigint NOT NULL, catalog_resource_id bigint NOT NULL, project_id bigint NOT NULL, - created_at timestamp with time zone NOT NULL + created_at timestamp with time zone NOT NULL, + released_at timestamp with time zone DEFAULT '1970-01-01 00:00:00+00'::timestamp with time zone NOT NULL ); CREATE SEQUENCE catalog_resource_versions_id_seq @@ -23371,7 +23372,6 @@ CREATE TABLE service_access_tokens ( id bigint NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, - category smallint DEFAULT 0 NOT NULL, encrypted_token bytea NOT NULL, encrypted_token_iv bytea NOT NULL, expires_at timestamp with time zone NOT NULL @@ -31849,12 +31849,12 @@ CREATE INDEX index_catalog_resource_components_on_project_id ON catalog_resource CREATE INDEX index_catalog_resource_components_on_version_id ON catalog_resource_components USING btree (version_id); -CREATE INDEX index_catalog_resource_versions_on_catalog_resource_id ON catalog_resource_versions USING btree (catalog_resource_id); - CREATE INDEX index_catalog_resource_versions_on_project_id ON catalog_resource_versions USING btree (project_id); CREATE UNIQUE INDEX index_catalog_resource_versions_on_release_id ON catalog_resource_versions USING btree (release_id); +CREATE INDEX index_catalog_resource_versions_on_resource_id_and_released_at ON catalog_resource_versions USING btree (catalog_resource_id, released_at); + CREATE UNIQUE INDEX index_catalog_resources_on_project_id ON catalog_resources USING btree (project_id); CREATE INDEX index_catalog_resources_on_state ON catalog_resources USING btree (state); diff --git a/doc/development/github_importer.md b/doc/development/github_importer.md index dd6b5c419f3..9cb95316a4e 100644 --- a/doc/development/github_importer.md +++ b/doc/development/github_importer.md @@ -297,6 +297,54 @@ The code for this resides in: - `lib/gitlab/github_import/user_finder.rb` - `lib/gitlab/github_import/caching.rb` +## Increasing Sidekiq interrupts + +When a Sidekiq process shut downs, it waits for a period of time for running +jobs to finish before it then interrupts them. An interrupt terminates +the job and requeues it again. Our +[vendored `sidekiq-reliable-fetcher` gem](https://gitlab.com/gitlab-org/gitlab/-/blob/master/vendor/gems/sidekiq-reliable-fetch/README.md) +puts a limit of `3` interrupts before a job is no longer requeued and is +permanently terminated. Jobs that have been interrupted log a +`json.interrupted_count` in Kibana. + +This limit offers protection from jobs that can never be completed in +the time between Sidekiq restarts. + +For large imports, our GitHub [stage](#stages) workers (namespaced in +`Stage::`) take many hours to finish. By default, the import is at risk +of failing because of `sidekiq-reliable-fetcher` permanently stopping these +workers before they can complete. + +Stage workers that pick up from where they left off when restarted can +increase the interrupt limit of `sidekiq-reliable-fetcher` to `20` by +calling `.resumes_work_when_interrupted!`: + +```ruby +module Gitlab + module GithubImport + module Stage + class MyWorker + resumes_work_when_interrupted! + + # ... + end + end + end +end +``` + +Stage workers that do not fully resume their work when restarted should +not call this method. For example, a worker that skips already imported +objects, but starts its loop from the beginning each time. + +Examples of stage workers that do resume work fully are ones that +execute services that: + +- [Continue paging](https://gitlab.com/gitlab-org/gitlab/-/blob/487521cc/lib/gitlab/github_import/parallel_scheduling.rb#L114-117) + an endpoint from where it left off. +- [Continue their loop](https://gitlab.com/gitlab-org/gitlab/-/blob/487521cc26c1e2bdba4fc67c14478d2b2a5f2bfa/lib/gitlab/github_import/importer/attachments/issues_importer.rb#L27) + from where it left off. + ## Mapping labels and milestones To reduce pressure on the database we do not query it when setting labels and diff --git a/lib/api/admin/dictionary.rb b/lib/api/admin/dictionary.rb index 038c122c021..b013d584c1c 100644 --- a/lib/api/admin/dictionary.rb +++ b/lib/api/admin/dictionary.rb @@ -31,30 +31,12 @@ module API desc: 'The table name' end get do - not_found!('Table not found') unless File.exist?(safe_file_path!) + table_dictionary = ::Gitlab::Database::Dictionary.entry(params[:table_name]) + not_found!('Table not found') unless table_dictionary present table_dictionary, with: Entities::Dictionary::Table end end - - helpers do - def table_name - params[:table_name] - end - - def table_dictionary - YAML.load_file(safe_file_path!).with_indifferent_access - end - - def safe_file_path! - dir = Gitlab::Database::GitlabSchema.dictionary_paths.first.to_s - path = Rails.root.join(dir, "#{table_name}.yml").to_s - - Gitlab::PathTraversal.check_allowed_absolute_path_and_path_traversal!(path, [dir]) - - path - end - end end end end diff --git a/lib/api/entities/ml/mlflow/model_versions/responses/update.rb b/lib/api/entities/ml/mlflow/model_versions/responses/update.rb new file mode 100644 index 00000000000..a357174e043 --- /dev/null +++ b/lib/api/entities/ml/mlflow/model_versions/responses/update.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module API + module Entities + module Ml + module Mlflow + module ModelVersions + module Responses + class Update < Grape::Entity + expose :model_version, with: Types::ModelVersion + end + end + end + end + end + end +end diff --git a/lib/api/ml/mlflow/api_helpers.rb b/lib/api/ml/mlflow/api_helpers.rb index aefa156717c..4501941a3a3 100644 --- a/lib/api/ml/mlflow/api_helpers.rb +++ b/lib/api/ml/mlflow/api_helpers.rb @@ -16,6 +16,10 @@ module API not_found! unless can?(current_user, :read_model_registry, user_project) end + def check_api_model_registry_write! + unauthorized! unless can?(current_user, :write_model_registry, user_project) + end + def resource_not_found! render_structured_api_error!({ error_code: 'RESOURCE_DOES_NOT_EXIST' }, 404) end @@ -28,6 +32,10 @@ module API render_structured_api_error!({ error_code: 'INVALID_PARAMETER_VALUE', message: message }, 400) end + def update_failed! + render_structured_api_error!({ error_code: 'UPDATE_FAILED' }, 400) + end + def experiment_repository ::Ml::ExperimentTracking::ExperimentRepository.new(user_project, current_user) end diff --git a/lib/api/ml/mlflow/model_versions.rb b/lib/api/ml/mlflow/model_versions.rb index 04ada5204fd..a50dd4b005a 100644 --- a/lib/api/ml/mlflow/model_versions.rb +++ b/lib/api/ml/mlflow/model_versions.rb @@ -25,6 +25,28 @@ module API response = { model_version: model_version } present response, with: Entities::Ml::Mlflow::ModelVersions::Responses::Get end + + desc 'Updates a Model Version.' do + success Entities::Ml::Mlflow::ModelVersions::Responses::Update + detail 'https://mlflow.org/docs/2.6.0/rest-api.html#update-modelversion' + end + params do + # These params are actually required, however it is listed as optional here + # we can send a custom error response required by MLFlow + optional :name, type: String, desc: 'Model version name' + optional :version, type: String, desc: 'Model version number' + optional :description, type: String, desc: 'Model version description' + end + patch 'update', urgency: :low do + check_api_model_registry_write! + invalid_parameter! unless params[:name] && params[:version] && params[:description] + result = ::Ml::ModelVersions::UpdateModelVersionService.new( + user_project, params[:name], params[:version], params[:description] + ).execute + update_failed! unless result.success? + response = { model_version: result.payload } + present response, with: Entities::Ml::Mlflow::ModelVersions::Responses::Update + end end end end diff --git a/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml index 6898923bc53..3f8bcbbc635 100644 --- a/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml @@ -1,5 +1,5 @@ variables: - AUTO_BUILD_IMAGE_VERSION: 'v1.49.0' + AUTO_BUILD_IMAGE_VERSION: 'v1.50.0' build: stage: build diff --git a/lib/gitlab/ci/templates/Jobs/Build.latest.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Build.latest.gitlab-ci.yml index 6898923bc53..3f8bcbbc635 100644 --- a/lib/gitlab/ci/templates/Jobs/Build.latest.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Build.latest.gitlab-ci.yml @@ -1,5 +1,5 @@ variables: - AUTO_BUILD_IMAGE_VERSION: 'v1.49.0' + AUTO_BUILD_IMAGE_VERSION: 'v1.50.0' build: stage: build diff --git a/lib/gitlab/database/dictionary.rb b/lib/gitlab/database/dictionary.rb index 7b0c8560a26..c766010a80f 100644 --- a/lib/gitlab/database/dictionary.rb +++ b/lib/gitlab/database/dictionary.rb @@ -3,57 +3,87 @@ module Gitlab module Database class Dictionary - def initialize(file_path) - @file_path = file_path - @data = YAML.load_file(file_path) + def self.entries(scope = '') + @entries ||= {} + @entries[scope] ||= Dir.glob(dictionary_path_globs(scope)).map do |file_path| + dictionary = Entry.new(file_path) + dictionary.validate! + dictionary + end end - def name_and_schema - [key_name, gitlab_schema.to_sym] + def self.entry(name, scope = '') + entries(scope).find do |entry| + entry.key_name == name + end end - def table_name - data['table_name'] + private_class_method def self.dictionary_path_globs(scope) + dictionary_paths.map { |path| Rails.root.join(path, scope, '*.yml') } end - def view_name - data['view_name'] + private_class_method def self.dictionary_paths + ::Gitlab::Database.all_database_connections + .values.map(&:db_docs_dir).uniq end - def milestone - data['milestone'] - end + class Entry + def initialize(file_path) + @file_path = file_path + @data = YAML.load_file(file_path) + end - def gitlab_schema - data['gitlab_schema'] - end + def name_and_schema + [key_name, gitlab_schema.to_sym] + end - def schema?(schema_name) - gitlab_schema == schema_name.to_s - end + def table_name + data['table_name'] + end - def key_name - table_name || view_name - end + def feature_categories + data['feature_categories'] + end - def validate! - return true unless gitlab_schema.nil? + def view_name + data['view_name'] + end - raise( - GitlabSchema::UnknownSchemaError, - "#{file_path} must specify a valid gitlab_schema for #{key_name}. " \ - "See #{help_page_url}" - ) - end + def milestone + data['milestone'] + end + + def gitlab_schema + data['gitlab_schema'] + end + + def schema?(schema_name) + gitlab_schema == schema_name.to_s + end + + def key_name + table_name || view_name + end + + def validate! + return true unless gitlab_schema.nil? + + raise( + GitlabSchema::UnknownSchemaError, + "#{file_path} must specify a valid gitlab_schema for #{key_name}. " \ + "See #{help_page_url}" + ) + end - private + private - attr_reader :file_path, :data + attr_reader :file_path, :data - def help_page_url - # rubocop:disable Gitlab/DocUrl -- link directly to docs.gitlab.com, always - 'https://docs.gitlab.com/ee/development/database/database_dictionary.html' - # rubocop:enable Gitlab/DocUrl + def help_page_url + # rubocop:disable Gitlab/DocUrl -- link directly to docs.gitlab.com, always + 'https://docs.gitlab.com/ee/development/database/database_dictionary.html' + # rubocop:enable Gitlab/DocUrl + end end end end diff --git a/lib/gitlab/database/gitlab_schema.rb b/lib/gitlab/database/gitlab_schema.rb index ecb45622061..7c48aea929f 100644 --- a/lib/gitlab/database/gitlab_schema.rb +++ b/lib/gitlab/database/gitlab_schema.rb @@ -121,15 +121,6 @@ module Gitlab end end - def self.dictionary_paths - Gitlab::Database.all_database_connections - .values.map(&:db_docs_dir).uniq - end - - def self.dictionary_path_globs(scope) - self.dictionary_paths.map { |path| Rails.root.join(path, scope, '*.yml') } - end - def self.views_and_tables_to_schema @views_and_tables_to_schema ||= self.tables_to_schema.merge(self.views_to_schema) end @@ -139,32 +130,24 @@ module Gitlab end def self.deleted_tables_to_schema - @deleted_tables_to_schema ||= self.build_dictionary('deleted_tables').map(&:name_and_schema).to_h + @deleted_tables_to_schema ||= ::Gitlab::Database::Dictionary.entries('deleted_tables').map(&:name_and_schema).to_h end def self.deleted_views_to_schema - @deleted_views_to_schema ||= self.build_dictionary('deleted_views').map(&:name_and_schema).to_h + @deleted_views_to_schema ||= ::Gitlab::Database::Dictionary.entries('deleted_views').map(&:name_and_schema).to_h end def self.tables_to_schema - @tables_to_schema ||= self.build_dictionary('').map(&:name_and_schema).to_h + @tables_to_schema ||= ::Gitlab::Database::Dictionary.entries.map(&:name_and_schema).to_h end def self.views_to_schema - @views_to_schema ||= self.build_dictionary('views').map(&:name_and_schema).to_h + @views_to_schema ||= ::Gitlab::Database::Dictionary.entries('views').map(&:name_and_schema).to_h end def self.schema_names @schema_names ||= self.views_and_tables_to_schema.values.to_set end - - def self.build_dictionary(scope) - Dir.glob(dictionary_path_globs(scope)).map do |file_path| - dictionary = Dictionary.new(file_path) - dictionary.validate! - dictionary - end - end end end end diff --git a/spec/fixtures/achievements.yml b/spec/fixtures/achievements.yml deleted file mode 100644 index a24cf42413b..00000000000 --- a/spec/fixtures/achievements.yml +++ /dev/null @@ -1,10 +0,0 @@ ---- -table_name: achievements -classes: -- Achievements::Achievement -feature_categories: -- feature_category_example -description: Achievements which can be created by namespaces to award them to users -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/105871 -milestone: '15.7' -gitlab_schema: gitlab_main diff --git a/spec/lib/gitlab/database/dictionary_spec.rb b/spec/lib/gitlab/database/dictionary_spec.rb index 6d2de41468b..261cf27ed69 100644 --- a/spec/lib/gitlab/database/dictionary_spec.rb +++ b/spec/lib/gitlab/database/dictionary_spec.rb @@ -3,81 +3,104 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Dictionary, feature_category: :database do - subject(:database_dictionary) { described_class.new(file_path) } + describe '.entries' do + it 'all tables and views are unique' do + table_and_view_names = described_class.entries('') + table_and_view_names += described_class.entries('views') + + # ignore gitlab_internal due to `ar_internal_metadata`, `schema_migrations` + table_and_view_names = table_and_view_names + .reject { |database_dictionary| database_dictionary.schema?('gitlab_internal') } + + duplicated_tables = table_and_view_names + .group_by(&:key_name) + .select { |_, schemas| schemas.count > 1 } + .keys + + expect(duplicated_tables).to be_empty, \ + "Duplicated table(s) #{duplicated_tables.to_a} found in #{described_class}.views_and_tables_to_schema. " \ + "Any duplicated table must be removed from db/docs/ or ee/db/docs/. " \ + "More info: https://docs.gitlab.com/ee/development/database/database_dictionary.html" + end + end - context 'for a table' do - let(:file_path) { 'db/docs/application_settings.yml' } + describe '::Entry' do + subject(:database_dictionary) { described_class::Entry.new(file_path) } - describe '#name_and_schema' do - it 'returns the name of the table and its gitlab schema' do - expect(database_dictionary.name_and_schema).to match_array(['application_settings', :gitlab_main_clusterwide]) + context 'for a table' do + let(:file_path) { 'db/docs/application_settings.yml' } + + describe '#name_and_schema' do + it 'returns the name of the table and its gitlab schema' do + expect(database_dictionary.name_and_schema).to match_array(['application_settings', :gitlab_main_clusterwide]) + end end - end - describe '#table_name' do - it 'returns the name of the table' do - expect(database_dictionary.table_name).to eq('application_settings') + describe '#table_name' do + it 'returns the name of the table' do + expect(database_dictionary.table_name).to eq('application_settings') + end end - end - describe '#view_name' do - it 'returns nil' do - expect(database_dictionary.view_name).to be_nil + describe '#view_name' do + it 'returns nil' do + expect(database_dictionary.view_name).to be_nil + end end - end - describe '#milestone' do - it 'returns the milestone in which the table was introduced' do - expect(database_dictionary.milestone).to eq('7.7') + describe '#milestone' do + it 'returns the milestone in which the table was introduced' do + expect(database_dictionary.milestone).to eq('7.7') + end end - end - describe '#gitlab_schema' do - it 'returns the gitlab_schema of the table' do - expect(database_dictionary.table_name).to eq('application_settings') + describe '#gitlab_schema' do + it 'returns the gitlab_schema of the table' do + expect(database_dictionary.table_name).to eq('application_settings') + end end - end - describe '#schema?' do - it 'checks if the given schema matches the schema of the table' do - expect(database_dictionary.schema?('gitlab_main')).to eq(false) - expect(database_dictionary.schema?('gitlab_main_clusterwide')).to eq(true) + describe '#schema?' do + it 'checks if the given schema matches the schema of the table' do + expect(database_dictionary.schema?('gitlab_main')).to eq(false) + expect(database_dictionary.schema?('gitlab_main_clusterwide')).to eq(true) + end end - end - describe '#key_name' do - it 'returns the value of the name of the table' do - expect(database_dictionary.key_name).to eq('application_settings') + describe '#key_name' do + it 'returns the value of the name of the table' do + expect(database_dictionary.key_name).to eq('application_settings') + end end - end - describe '#validate!' do - it 'raises an error if the gitlab_schema is empty' do - allow(database_dictionary).to receive(:gitlab_schema).and_return(nil) + describe '#validate!' do + it 'raises an error if the gitlab_schema is empty' do + allow(database_dictionary).to receive(:gitlab_schema).and_return(nil) - expect { database_dictionary.validate! }.to raise_error(Gitlab::Database::GitlabSchema::UnknownSchemaError) + expect { database_dictionary.validate! }.to raise_error(Gitlab::Database::GitlabSchema::UnknownSchemaError) + end end end - end - context 'for a view' do - let(:file_path) { 'db/docs/views/postgres_constraints.yml' } + context 'for a view' do + let(:file_path) { 'db/docs/views/postgres_constraints.yml' } - describe '#table_name' do - it 'returns nil' do - expect(database_dictionary.table_name).to be_nil + describe '#table_name' do + it 'returns nil' do + expect(database_dictionary.table_name).to be_nil + end end - end - describe '#view_name' do - it 'returns the name of the view' do - expect(database_dictionary.view_name).to eq('postgres_constraints') + describe '#view_name' do + it 'returns the name of the view' do + expect(database_dictionary.view_name).to eq('postgres_constraints') + end end - end - describe '#key_name' do - it 'returns the value of the name of the view' do - expect(database_dictionary.key_name).to eq('postgres_constraints') + describe '#key_name' do + it 'returns the value of the name of the view' do + expect(database_dictionary.key_name).to eq('postgres_constraints') + end end end end diff --git a/spec/lib/gitlab/database/gitlab_schema_spec.rb b/spec/lib/gitlab/database/gitlab_schema_spec.rb index a47e53c18a5..3573ce7ee14 100644 --- a/spec/lib/gitlab/database/gitlab_schema_spec.rb +++ b/spec/lib/gitlab/database/gitlab_schema_spec.rb @@ -1,13 +1,6 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.shared_examples 'validate path globs' do |path_globs| - it 'returns an array of path globs' do - expect(path_globs).to be_an(Array) - expect(path_globs).to all(be_an(Pathname)) - end -end - RSpec.shared_examples 'validate schema data' do |tables_and_views| it 'all tables and views have assigned a known gitlab_schema' do expect(tables_and_views).to all( @@ -88,32 +81,6 @@ RSpec.describe Gitlab::Database::GitlabSchema, feature_category: :database do end end end - - it 'all tables and views are unique' do - table_and_view_names = described_class.build_dictionary('') - table_and_view_names += described_class.build_dictionary('views') - - # ignore gitlab_internal due to `ar_internal_metadata`, `schema_migrations` - table_and_view_names = table_and_view_names - .reject { |database_dictionary| database_dictionary.schema?('gitlab_internal') } - - duplicated_tables = table_and_view_names - .group_by(&:key_name) - .select { |_, schemas| schemas.count > 1 } - .keys - - expect(duplicated_tables).to be_empty, \ - "Duplicated table(s) #{duplicated_tables.to_a} found in #{described_class}.views_and_tables_to_schema. " \ - "Any duplicated table must be removed from db/docs/ or ee/db/docs/. " \ - "More info: https://docs.gitlab.com/ee/development/database/database_dictionary.html" - end - end - - describe '.dictionary_path_globs' do - include_examples 'validate path globs', described_class.dictionary_path_globs('') - include_examples 'validate path globs', described_class.dictionary_path_globs('views') - include_examples 'validate path globs', described_class.dictionary_path_globs('deleted_views') - include_examples 'validate path globs', described_class.dictionary_path_globs('deleted_tables') end describe '.tables_to_schema' do diff --git a/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb b/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb index 648213dc152..d1d7aa12c46 100644 --- a/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb +++ b/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb @@ -50,16 +50,16 @@ RSpec.describe 'new tables with gitlab_main schema', feature_category: :cell do end def tables_having_gitlab_main_schema(starting_from_milestone:) - selected_data = gitlab_main_schema_tables.select do |database_dictionary| - database_dictionary.milestone.to_f >= starting_from_milestone + selected_data = gitlab_main_schema_tables.select do |entry| + entry.milestone.to_f >= starting_from_milestone end selected_data.map(&:table_name) end def gitlab_main_schema_tables - ::Gitlab::Database::GitlabSchema.build_dictionary('').select do |database_dictionary| - database_dictionary.schema?('gitlab_main') + ::Gitlab::Database::Dictionary.entries.select do |entry| + entry.schema?('gitlab_main') end end end diff --git a/spec/requests/api/admin/dictionary_spec.rb b/spec/requests/api/admin/dictionary_spec.rb index effd3572423..b35aacd6ba0 100644 --- a/spec/requests/api/admin/dictionary_spec.rb +++ b/spec/requests/api/admin/dictionary_spec.rb @@ -29,29 +29,13 @@ RSpec.describe API::Admin::Dictionary, feature_category: :database do end end - context 'with a malicious table_name' do - it 'returns an error' do - get api("/admin/databases/main/dictionary/tables/%2E%2E%2Fpasswords.yml", admin, admin_mode: true) - - expect(response).to have_gitlab_http_status(:error) - end - end - context 'when the params are correct' do - let(:dictionary_dir) { Rails.root.join('spec/fixtures') } - let(:path_file) { Rails.root.join(dictionary_dir, 'achievements.yml') } - it 'fetches the table dictionary' do - allow(Gitlab::Database::GitlabSchema).to receive(:dictionary_paths).and_return([dictionary_dir]) - - expect(Gitlab::PathTraversal).to receive(:check_allowed_absolute_path_and_path_traversal!).twice.with( - path_file.to_s, [dictionary_dir.to_s]).and_call_original - show_table_dictionary aggregate_failures "testing response" do expect(json_response['table_name']).to eq('achievements') - expect(json_response['feature_categories']).to eq(['feature_category_example']) + expect(json_response['feature_categories']).to eq(['user_profile']) end end end diff --git a/spec/requests/api/ml/mlflow/model_versions_spec.rb b/spec/requests/api/ml/mlflow/model_versions_spec.rb index 9813ed95ab3..26432d3e25d 100644 --- a/spec/requests/api/ml/mlflow/model_versions_spec.rb +++ b/spec/requests/api/ml/mlflow/model_versions_spec.rb @@ -83,4 +83,55 @@ RSpec.describe API::Ml::Mlflow::ModelVersions, feature_category: :mlops do it_behaves_like 'MLflow|Requires read_api scope' end end + + describe 'UPDATE /projects/:id/ml/mlflow/api/2.0/mlflow/model-versions/update' do + let(:params) { { name: name, version: version, description: 'description-text' } } + let(:request) { patch api(route), params: params, headers: headers } + + let(:route) do + "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/model-versions/update" + end + + it 'returns the model version', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + expect(json_response['model_version']).not_to be_nil + expect(json_response['model_version']['name']).to eq(name) + expect(json_response['model_version']['version']).to eq(version) + end + + describe 'Error States' do + context 'when has access' do + context 'and model name in incorrect' do + let(:params) { { name: 'invalid-name', version: version, description: 'description-text' } } + + it 'throws error 400' do + is_expected.to have_gitlab_http_status(:bad_request) + end + end + + context 'and version in incorrect' do + let(:params) { { name: name, version: 'invalid-version', description: 'description-text' } } + + it 'throws error 400' do + is_expected.to have_gitlab_http_status(:bad_request) + end + end + + context 'when user lacks write_model_registry rights' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?) + .with(current_user, :write_model_registry, project) + .and_return(false) + end + + it "is Not Found" do + is_expected.to have_gitlab_http_status(:unauthorized) + end + end + end + + it_behaves_like 'MLflow|shared model registry error cases' + end + end end diff --git a/spec/services/ml/model_versions/update_model_version_service_spec.rb b/spec/services/ml/model_versions/update_model_version_service_spec.rb new file mode 100644 index 00000000000..99ea8b81df3 --- /dev/null +++ b/spec/services/ml/model_versions/update_model_version_service_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ml::ModelVersions::UpdateModelVersionService, feature_category: :mlops do + let_it_be(:existing_version) { create(:ml_model_versions) } + + let(:project) { existing_version.project } + let(:name) { existing_version.name } + let(:version) { existing_version.version } + let(:description) { 'A model version description' } + + subject(:execute_service) { described_class.new(project, name, version, description).execute } + + describe '#execute' do + context 'when model version exists' do + it { is_expected.to be_success } + + it 'updates the model version description' do + execute_service + + expect(execute_service.payload.description).to eq(description) + end + end + + context 'when description is invalid' do + let(:description) { 'a' * 501 } + + it { is_expected.to be_error } + end + + context 'when model does not exist' do + let(:name) { 'a_new_model' } + + it { is_expected.to be_error } + end + + context 'when model version does not exist' do + let(:name) { '2.0.0' } + + it { is_expected.to be_error } + end + end +end diff --git a/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb b/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb index 7978f43610d..20bc20c8912 100644 --- a/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb @@ -90,19 +90,6 @@ RSpec.shared_examples 'MLflow|shared model registry error cases' do is_expected.to have_gitlab_http_status(:not_found) end end - - context 'when model registry is unavailable' do - before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?) - .with(current_user, :read_model_registry, project) - .and_return(false) - end - - it "is Not Found" do - is_expected.to have_gitlab_http_status(:not_found) - end - end end RSpec.shared_examples 'MLflow|Bad Request on missing required' do |keys| diff --git a/spec/workers/bulk_imports/finish_batched_pipeline_worker_spec.rb b/spec/workers/bulk_imports/finish_batched_pipeline_worker_spec.rb index 59ae4205c0f..959b063e061 100644 --- a/spec/workers/bulk_imports/finish_batched_pipeline_worker_spec.rb +++ b/spec/workers/bulk_imports/finish_batched_pipeline_worker_spec.rb @@ -33,6 +33,8 @@ RSpec.describe BulkImports::FinishBatchedPipelineWorker, feature_category: :impo ) end + let!(:batch_1) { create(:bulk_import_batch_tracker, :finished, tracker: pipeline_tracker) } + subject(:worker) { described_class.new } describe '#perform' do @@ -45,27 +47,27 @@ RSpec.describe BulkImports::FinishBatchedPipelineWorker, feature_category: :impo end end - context 'when import is in progress' do - it 'marks the tracker as finished' do - expect_next_instance_of(BulkImports::Logger) do |logger| - expect(logger).to receive(:info).with( - a_hash_including('message' => 'Tracker finished') - ) - end - - expect { subject.perform(pipeline_tracker.id) } - .to change { pipeline_tracker.reload.finished? } - .from(false).to(true) + it 'marks the tracker as finished' do + expect_next_instance_of(BulkImports::Logger) do |logger| + expect(logger).to receive(:info).with( + a_hash_including('message' => 'Tracker finished') + ) end - it "calls the pipeline's `#on_finish`" do - expect_next_instance_of(pipeline_class) do |pipeline| - expect(pipeline).to receive(:on_finish) - end + expect { subject.perform(pipeline_tracker.id) } + .to change { pipeline_tracker.reload.finished? } + .from(false).to(true) + end - subject.perform(pipeline_tracker.id) + it "calls the pipeline's `#on_finish`" do + expect_next_instance_of(pipeline_class) do |pipeline| + expect(pipeline).to receive(:on_finish) end + subject.perform(pipeline_tracker.id) + end + + context 'when import is in progress' do it 're-enqueues for any started batches' do create(:bulk_import_batch_tracker, :started, tracker: pipeline_tracker) @@ -88,14 +90,14 @@ RSpec.describe BulkImports::FinishBatchedPipelineWorker, feature_category: :impo end context 'when pipeline tracker is stale' do - let(:pipeline_tracker) { create(:bulk_import_tracker, :started, :batched, :stale, entity: entity) } + before do + batch_1.update!(updated_at: 5.hours.ago) + end it 'fails pipeline tracker and its batches' do - create(:bulk_import_batch_tracker, :finished, tracker: pipeline_tracker) - expect_next_instance_of(BulkImports::Logger) do |logger| expect(logger).to receive(:error).with( - a_hash_including('message' => 'Tracker stale. Failing batches and tracker') + a_hash_including('message' => 'Batch stale. Failing batches and tracker') ) end diff --git a/workhorse/.tool-versions b/workhorse/.tool-versions index ff2d5626984..83d3f70dc6b 100644 --- a/workhorse/.tool-versions +++ b/workhorse/.tool-versions @@ -1 +1 @@ -golang 1.21.2 +golang 1.21.4 |