diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-08-02 06:08:52 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-08-02 06:08:52 +0300 |
commit | 123582839259a70910bd0e9109e57ed65d71cb23 (patch) | |
tree | da3c33793182af05f67bf27e071168df94711376 | |
parent | a0e1fa9aada57f1fde912890e96f867ac540ab03 (diff) |
Add latest changes from gitlab-org/gitlab@master
47 files changed, 587 insertions, 227 deletions
diff --git a/app/controllers/concerns/kas_cookie.rb b/app/controllers/concerns/kas_cookie.rb index 06a4ee873f8..fafd426da7a 100644 --- a/app/controllers/concerns/kas_cookie.rb +++ b/app/controllers/concerns/kas_cookie.rb @@ -8,11 +8,10 @@ module KasCookie next unless ::Gitlab::Kas::UserAccess.enabled? next unless Settings.gitlab.content_security_policy['enabled'] - kas_url = ::Gitlab::Kas.tunnel_url next if URI(kas_url).host == ::Gitlab.config.gitlab.host # already allowed, no need for exception - kas_url += '/' unless kas_url.end_with?('/') - p.connect_src(*Array.wrap(p.directives['connect-src']), kas_url) + p.connect_src(*Array.wrap(p.directives['connect-src']), kas_ws_url.sub(%r{/?$}, '/')) + p.connect_src(*Array.wrap(p.directives['connect-src']), kas_url.sub(%r{/?$}, '/')) end end @@ -26,4 +25,14 @@ module KasCookie cookies[::Gitlab::Kas::COOKIE_KEY] = cookie_data end + + private + + def kas_url + ::Gitlab::Kas.tunnel_url + end + + def kas_ws_url + ::Gitlab::Kas.tunnel_ws_url + end end diff --git a/app/finders/autocomplete/group_users_finder.rb b/app/finders/autocomplete/group_users_finder.rb new file mode 100644 index 00000000000..f515a443e9a --- /dev/null +++ b/app/finders/autocomplete/group_users_finder.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +# This finder returns all users that are related to a given group because: +# 1. They are members of the group, its sub-groups, or its ancestor groups +# 2. They are members of a group that is invited to the group, its sub-groups, or its ancestors +# 3. They are members of a project that belongs to the group +# 4. They are members of a group that is invited to the group's descendant projects +# +# These users are not necessarily members of the given group and may not have access to the group +# so this should not be used for access control +module Autocomplete + class GroupUsersFinder + include Gitlab::Utils::StrongMemoize + + def initialize(group:) + @group = group + end + + def execute + members = Member + .with(group_hierarchy_cte.to_arel) # rubocop:disable CodeReuse/ActiveRecord + .with(descendant_projects_cte.to_arel) # rubocop:disable CodeReuse/ActiveRecord + .from_union(member_relations, remove_duplicates: false) + + User + .id_in(members.select(:user_id)) + .allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/420387") + end + + private + + def member_relations + relations = [ + members_from_group_hierarchy.select(:user_id), + members_from_descendant_projects.select(:user_id) + ] + + if Feature.enabled?(:include_descendant_shares_in_user_autocomplete, @group) + relations << members_from_hierarchy_group_shares.select(:user_id) + relations << members_from_descendant_project_shares.select(:user_id) + else + relations << @group.members_from_self_and_ancestor_group_shares.reselect(:user_id) + end + + relations + end + + def members_from_group_hierarchy + GroupMember + .with_source_id(group_hierarchy_ids) + .without_invites_and_requests + end + + def members_from_hierarchy_group_shares + invited_groups = GroupGroupLink.for_shared_groups(group_hierarchy_ids).select(:shared_with_group_id) + + GroupMember + .with_source_id(invited_groups) + .without_invites_and_requests + end + + def members_from_descendant_projects + ProjectMember + .with_source_id(descendant_project_ids) + .without_invites_and_requests + end + + def members_from_descendant_project_shares + descendant_project_invited_groups = ProjectGroupLink.for_projects(descendant_project_ids).select(:group_id) + + GroupMember + .with_source_id(descendant_project_invited_groups) + .without_invites_and_requests + end + + def group_hierarchy_cte + Gitlab::SQL::CTE.new(:group_hierarchy, @group.self_and_hierarchy.select(:id)) + end + strong_memoize_attr :group_hierarchy_cte + + def group_hierarchy_ids + Namespace.from(group_hierarchy_cte.table).select(:id) # rubocop:disable CodeReuse/ActiveRecord + end + + def descendant_projects_cte + Gitlab::SQL::CTE.new(:descendant_projects, @group.all_projects.select(:id)) + end + strong_memoize_attr :descendant_projects_cte + + def descendant_project_ids + Project.from(descendant_projects_cte.table).select(:id) # rubocop:disable CodeReuse/ActiveRecord + end + end +end diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 8e5f3f030fb..7058bfd5650 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -6,6 +6,7 @@ class ApplicationRecord < ActiveRecord::Base include LegacyBulkInsert include CrossDatabaseModification include SensitiveSerializableHash + include ResetOnUnionError self.abstract_class = true diff --git a/app/models/concerns/reset_on_union_error.rb b/app/models/concerns/reset_on_union_error.rb new file mode 100644 index 00000000000..42e350b0bed --- /dev/null +++ b/app/models/concerns/reset_on_union_error.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module ResetOnUnionError + extend ActiveSupport::Concern + + MAX_RESET_PERIOD = 10.minutes + + included do |base| + base.rescue_from ActiveRecord::StatementInvalid, with: :reset_on_union_error + + base.class_attribute :previous_reset_columns_from_error + end + + class_methods do + def reset_on_union_error(exception) + if reset_on_statement_invalid?(exception) + class_to_be_reset = base_class + + class_to_be_reset.reset_column_information + Gitlab::ErrorTracking.log_exception(exception, { reset_model_name: class_to_be_reset.name }) + + class_to_be_reset.previous_reset_columns_from_error = Time.current + end + + raise + end + + def reset_on_statement_invalid?(exception) + return false unless exception.message.include?("each UNION query must have the same number of columns") + + return false if base_class.previous_reset_columns_from_error? && + base_class.previous_reset_columns_from_error > MAX_RESET_PERIOD.ago + + Feature.enabled?(:reset_column_information_on_statement_invalid, type: :ops) + end + end +end diff --git a/app/models/group.rb b/app/models/group.rb index b0b9c852d11..5fb0980ec71 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -635,7 +635,7 @@ class Group < Namespace end # Returns all members that are part of the group, it's subgroups, and ancestor groups - def direct_and_indirect_members + def hierarchy_members GroupMember .active_without_invites_and_requests .where(source_id: self_and_hierarchy.reorder(nil).select(:id)) @@ -667,31 +667,6 @@ class Group < Namespace .reorder(nil) end - # Returns all users that are members of the group because: - # 1. They belong to the group - # 2. They belong to a project that belongs to the group - # 3. They belong to a sub-group or project in such sub-group - # 4. They belong to an ancestor group - # 5. They belong to a group that is shared with this group, if share_with_groups is true - def direct_and_indirect_users(share_with_groups: false) - members = if share_with_groups - # We only need :user_id column, but - # `members_from_self_and_ancestor_group_shares` needs more - # columns to make the CTE query work. - GroupMember.from_union([ - direct_and_indirect_members.select(:user_id, :source_type, :type), - members_from_self_and_ancestor_group_shares.reselect(:user_id, :source_type, :type) - ]) - else - direct_and_indirect_members - end - - User.from_union([ - User.where(id: members.select(:user_id)).reorder(nil), - project_users_with_descendants - ]) - end - def users_count members.count end @@ -944,7 +919,7 @@ class Group < Namespace end def update_two_factor_requirement_for_members - direct_and_indirect_members.find_each(&:update_two_factor_requirement) + hierarchy_members.find_each(&:update_two_factor_requirement) end def readme_project @@ -957,6 +932,42 @@ class Group < Namespace end strong_memoize_attr :group_readme + def members_from_self_and_ancestor_group_shares + group_group_link_table = GroupGroupLink.arel_table + group_member_table = GroupMember.arel_table + + source_ids = + if has_parent? + self_and_ancestors.reorder(nil).select(:id) + else + id + end + + group_group_links_query = GroupGroupLink.where(shared_group_id: source_ids) + cte = Gitlab::SQL::CTE.new(:group_group_links_cte, group_group_links_query) + cte_alias = cte.table.alias(GroupGroupLink.table_name) + + # Instead of members.access_level, we need to maximize that access_level at + # the respective group_group_links.group_access. + member_columns = GroupMember.attribute_names.map do |column_name| + if column_name == 'access_level' + smallest_value_arel([cte_alias[:group_access], group_member_table[:access_level]], 'access_level') + else + group_member_table[column_name] + end + end + + GroupMember + .with(cte.to_arel) + .select(*member_columns) + .from([group_member_table, cte.alias_to(group_group_link_table)]) + .where(group_member_table[:requested_at].eq(nil)) + .where(group_member_table[:source_id].eq(group_group_link_table[:shared_with_group_id])) + .where(group_member_table[:source_type].eq('Namespace')) + .where(group_member_table[:state].eq(::Member::STATE_ACTIVE)) + .non_minimal_access + end + private def feature_flag_enabled_for_self_or_ancestor?(feature_flag) @@ -1017,42 +1028,6 @@ class Group < Namespace errors.add(:require_two_factor_authentication, _('is forbidden by a top-level group')) end - def members_from_self_and_ancestor_group_shares - group_group_link_table = GroupGroupLink.arel_table - group_member_table = GroupMember.arel_table - - source_ids = - if has_parent? - self_and_ancestors.reorder(nil).select(:id) - else - id - end - - group_group_links_query = GroupGroupLink.where(shared_group_id: source_ids) - cte = Gitlab::SQL::CTE.new(:group_group_links_cte, group_group_links_query) - cte_alias = cte.table.alias(GroupGroupLink.table_name) - - # Instead of members.access_level, we need to maximize that access_level at - # the respective group_group_links.group_access. - member_columns = GroupMember.attribute_names.map do |column_name| - if column_name == 'access_level' - smallest_value_arel([cte_alias[:group_access], group_member_table[:access_level]], 'access_level') - else - group_member_table[column_name] - end - end - - GroupMember - .with(cte.to_arel) - .select(*member_columns) - .from([group_member_table, cte.alias_to(group_group_link_table)]) - .where(group_member_table[:requested_at].eq(nil)) - .where(group_member_table[:source_id].eq(group_group_link_table[:shared_with_group_id])) - .where(group_member_table[:source_type].eq('Namespace')) - .where(group_member_table[:state].eq(::Member::STATE_ACTIVE)) - .non_minimal_access - end - def smallest_value_arel(args, column_alias) Arel::Nodes::As.new( Arel::Nodes::NamedFunction.new('LEAST', args), diff --git a/app/models/group_group_link.rb b/app/models/group_group_link.rb index dba52aa51cd..13f74b938af 100644 --- a/app/models/group_group_link.rb +++ b/app/models/group_group_link.rb @@ -13,6 +13,7 @@ class GroupGroupLink < ApplicationRecord validates :group_access, inclusion: { in: Gitlab::Access.all_values }, presence: true scope :non_guests, -> { where('group_access > ?', Gitlab::Access::GUEST) } + scope :for_shared_groups, -> (group_ids) { where(shared_group_id: group_ids) } scope :with_owner_or_maintainer_access, -> do where(group_access: [Gitlab::Access::OWNER, Gitlab::Access::MAINTAINER]) diff --git a/app/models/project_group_link.rb b/app/models/project_group_link.rb index 9f9447c1de2..aee0afa875f 100644 --- a/app/models/project_group_link.rb +++ b/app/models/project_group_link.rb @@ -16,6 +16,7 @@ class ProjectGroupLink < ApplicationRecord scope :non_guests, -> { where('group_access > ?', Gitlab::Access::GUEST) } scope :in_group, -> (group_ids) { where(group_id: group_ids) } + scope :for_projects, -> (project_ids) { where(project_id: project_ids) } alias_method :shared_with_group, :group alias_method :shared_from, :project diff --git a/app/services/groups/participants_service.rb b/app/services/groups/participants_service.rb index e939d27d464..a2238264295 100644 --- a/app/services/groups/participants_service.rb +++ b/app/services/groups/participants_service.rb @@ -13,7 +13,7 @@ module Groups participants_in_noteable + all_members + groups + - group_members + group_hierarchy_users render_participants_as_hash(participants.uniq) end @@ -26,12 +26,10 @@ module Groups [{ username: "all", name: "All Group Members", count: group.users_count }] end - def group_members + def group_hierarchy_users return [] unless group - sorted( - group.direct_and_indirect_users(share_with_groups: group.member?(current_user)) - ) + sorted(Autocomplete::GroupUsersFinder.new(group: group).execute) end end end diff --git a/app/services/security/ci_configuration/base_create_service.rb b/app/services/security/ci_configuration/base_create_service.rb index 9c3cd6b25fb..a205a68532b 100644 --- a/app/services/security/ci_configuration/base_create_service.rb +++ b/app/services/security/ci_configuration/base_create_service.rb @@ -2,6 +2,8 @@ module Security module CiConfiguration + CiContentParseError = Class.new(StandardError) + class BaseCreateService attr_reader :branch_name, :current_user, :project, :name @@ -34,6 +36,10 @@ module Security track_event(attributes_for_commit) ServiceResponse.success(payload: { branch: branch_name, success_path: successful_change_path }) + rescue CiContentParseError => e + Gitlab::ErrorTracking.track_exception(e) + + ServiceResponse.error(message: e.message) rescue Gitlab::Git::PreReceiveError => e ServiceResponse.error(message: e.message) rescue StandardError @@ -59,12 +65,12 @@ module Security @gitlab_ci_yml ||= project.ci_config_for(root_ref) YAML.safe_load(@gitlab_ci_yml) if @gitlab_ci_yml rescue Psych::BadAlias - raise Gitlab::Graphql::Errors::MutationError, - _(".gitlab-ci.yml with aliases/anchors is not supported. Please change the CI configuration manually.") + raise CiContentParseError, _(".gitlab-ci.yml with aliases/anchors is not supported. " \ + "Please change the CI configuration manually.") rescue Psych::Exception => e Gitlab::AppLogger.error("Failed to process existing .gitlab-ci.yml: #{e.message}") - raise Gitlab::Graphql::Errors::MutationError, - "#{name} merge request creation mutation failed" + + raise CiContentParseError, "#{name} merge request creation failed" end def successful_change_path diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index a8328304e73..2c7b50fc131 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -25,7 +25,29 @@ module ObjectStorage end class DirectUploadStorage < ::CarrierWave::Storage::Fog + extend ::Gitlab::Utils::Override + + # This override only applies to object storage uploaders (e.g JobArtifactUploader). + # - The DirectUploadStorage is only used when object storage is enabled. See `#storage_for` + # - This method is called in two possible ways: + # - When a model (e.g. JobArtifact) is saved + # - When uploader.replace_file_without_saving! is called directly + # - For example, see `Gitlab::Geo::Replication::BlobDownloader#download_file` + # - We need this override to add the special behavior that bypasses + # CarrierWave's default storing mechanism, which copies a tempfile + # to its final location. In the case of files that are directly uploaded + # by Workhorse to the final location (determined by presence of `<mounted_as>_final_path`) in + # the object storage, the extra copy/delete step of CarrierWave + # is unnecessary. + # - We also need to ensure to only bypass the default store behavior if the file given + # is a `CarrierWave::Storage::Fog::File` (uploaded to object storage) and with `<mounted_as>_final_path` + # defined. For everything else, we want to still use the default CarrierWave storage behavior. + # - For example, during Geo replication of job artifacts, `replace_file_without_saving!` is + # called with a sanitized Tempfile. In this case, we want to use the default behavior of + # moving the tempfile to its final location and let CarrierWave upload the file to object storage. + override :store! def store!(file) + return super unless file.is_a?(::CarrierWave::Storage::Fog::File) return super unless @uploader.direct_upload_final_path.present? # The direct_upload_final_path is defined which means diff --git a/config/feature_flags/development/include_descendant_shares_in_user_autocomplete.yml b/config/feature_flags/development/include_descendant_shares_in_user_autocomplete.yml new file mode 100644 index 00000000000..3544c474a20 --- /dev/null +++ b/config/feature_flags/development/include_descendant_shares_in_user_autocomplete.yml @@ -0,0 +1,8 @@ +--- +name: include_descendant_shares_in_user_autocomplete +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/126559 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/419669 +milestone: '16.3' +type: development +group: group::project management +default_enabled: false diff --git a/config/initializers/active_record_relation_union_reset.rb b/config/initializers/active_record_relation_union_reset.rb deleted file mode 100644 index b8b3d634a61..00000000000 --- a/config/initializers/active_record_relation_union_reset.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -module ActiveRecordRelationUnionReset - MAX_RESET_PERIOD = 10.minutes - - def exec_queries - super - rescue ActiveRecord::StatementInvalid => e - if reset_on_statement_invalid?(e) - class_to_be_reset = klass.base_class - - class_to_be_reset.reset_column_information - Gitlab::ErrorTracking.log_exception(e, { reset_model_name: class_to_be_reset.name }) - - class_to_be_reset.previous_reset_columns_from_error = Time.now - end - - raise - end - - private - - def reset_on_statement_invalid?(exception) - return false unless exception.message.include?("each UNION query must have the same number of columns") - - return false if klass.base_class.previous_reset_columns_from_error? && - klass.base_class.previous_reset_columns_from_error > MAX_RESET_PERIOD.ago - - Feature.enabled?(:reset_column_information_on_statement_invalid, type: :ops) - end -end - -ActiveSupport.on_load(:active_record) do - ActiveRecord::Base.class_attribute :previous_reset_columns_from_error - ActiveRecord::Relation.prepend(ActiveRecordRelationUnionReset) -end diff --git a/config/metrics/counts_28d/20210216175016_analytics_total_unique_counts_monthly.yml b/config/metrics/counts_28d/20210216175016_analytics_total_unique_counts_monthly.yml index 53c10d5f253..b6c1f2ea507 100644 --- a/config/metrics/counts_28d/20210216175016_analytics_total_unique_counts_monthly.yml +++ b/config/metrics/counts_28d/20210216175016_analytics_total_unique_counts_monthly.yml @@ -42,5 +42,6 @@ tier: - free - premium - ultimate -performance_indicator_type: [] +performance_indicator_type: +- customer_health_score milestone: '<13.9' diff --git a/config/metrics/counts_28d/20210216175434_project_clusters_enabled.yml b/config/metrics/counts_28d/20210216175434_project_clusters_enabled.yml index 9000f412607..b027957e130 100644 --- a/config/metrics/counts_28d/20210216175434_project_clusters_enabled.yml +++ b/config/metrics/counts_28d/20210216175434_project_clusters_enabled.yml @@ -18,5 +18,6 @@ tier: - premium - ultimate name: 'count_distinct_user_id_from_enabled_clusters_attached_to_projects' -performance_indicator_type: [] +performance_indicator_type: +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/counts_28d/20210216180622_incident_management_total_unique_counts_monthly.yml b/config/metrics/counts_28d/20210216180622_incident_management_total_unique_counts_monthly.yml index 5ee19950f49..28a9d320746 100644 --- a/config/metrics/counts_28d/20210216180622_incident_management_total_unique_counts_monthly.yml +++ b/config/metrics/counts_28d/20210216180622_incident_management_total_unique_counts_monthly.yml @@ -43,4 +43,5 @@ performance_indicator_type: - smau - gmau - paid_gmau +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/counts_28d/20210216181504_issues_edit_total_unique_counts_monthly.yml b/config/metrics/counts_28d/20210216181504_issues_edit_total_unique_counts_monthly.yml index a9bc74e96e6..22ed6967e32 100644 --- a/config/metrics/counts_28d/20210216181504_issues_edit_total_unique_counts_monthly.yml +++ b/config/metrics/counts_28d/20210216181504_issues_edit_total_unique_counts_monthly.yml @@ -59,4 +59,5 @@ performance_indicator_type: - smau - gmau - paid_gmau +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/counts_28d/20210216184259_p_terraform_state_api_unique_users_monthly.yml b/config/metrics/counts_28d/20210216184259_p_terraform_state_api_unique_users_monthly.yml index b0c5b961263..84c8d986b18 100644 --- a/config/metrics/counts_28d/20210216184259_p_terraform_state_api_unique_users_monthly.yml +++ b/config/metrics/counts_28d/20210216184259_p_terraform_state_api_unique_users_monthly.yml @@ -20,5 +20,6 @@ tier: - free - premium - ultimate -performance_indicator_type: [] +performance_indicator_type: +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/counts_28d/20210216184937_user_packages_total_unique_counts_monthly.yml b/config/metrics/counts_28d/20210216184937_user_packages_total_unique_counts_monthly.yml index 8e6c286aa32..a34abaf4b9c 100644 --- a/config/metrics/counts_28d/20210216184937_user_packages_total_unique_counts_monthly.yml +++ b/config/metrics/counts_28d/20210216184937_user_packages_total_unique_counts_monthly.yml @@ -35,4 +35,5 @@ tier: performance_indicator_type: - smau - gmau +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/counts_all/20210204124932_clusters.yml b/config/metrics/counts_all/20210204124932_clusters.yml index 54011fb87ce..1193654a75f 100644 --- a/config/metrics/counts_all/20210204124932_clusters.yml +++ b/config/metrics/counts_all/20210204124932_clusters.yml @@ -16,4 +16,6 @@ tier: - free - premium - ultimate +performance_indicator_type: +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/counts_all/20210216175026_service_desk_issues.yml b/config/metrics/counts_all/20210216175026_service_desk_issues.yml index 3106b6bf7bb..0a68a3db1ee 100644 --- a/config/metrics/counts_all/20210216175026_service_desk_issues.yml +++ b/config/metrics/counts_all/20210216175026_service_desk_issues.yml @@ -14,5 +14,6 @@ distribution: - ee tier: - free -performance_indicator_type: [] +performance_indicator_type: +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/counts_all/20210216175312_clusters_applications_cilium.yml b/config/metrics/counts_all/20210216175312_clusters_applications_cilium.yml index bd067804cbb..3126519fb36 100644 --- a/config/metrics/counts_all/20210216175312_clusters_applications_cilium.yml +++ b/config/metrics/counts_all/20210216175312_clusters_applications_cilium.yml @@ -17,6 +17,7 @@ tier: - free - premium - ultimate -performance_indicator_type: [] +performance_indicator_type: +- customer_health_score milestone: "<13.9" milestone_removed: "14.4" diff --git a/config/metrics/counts_all/20210216175446_network_policy_forwards.yml b/config/metrics/counts_all/20210216175446_network_policy_forwards.yml index b1141edc39c..d0fa2c434d2 100644 --- a/config/metrics/counts_all/20210216175446_network_policy_forwards.yml +++ b/config/metrics/counts_all/20210216175446_network_policy_forwards.yml @@ -19,5 +19,6 @@ tier: - free - premium - ultimate -performance_indicator_type: [] +performance_indicator_type: +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/counts_all/20210216175448_network_policy_drops.yml b/config/metrics/counts_all/20210216175448_network_policy_drops.yml index 28bf9101656..53e2abe35c0 100644 --- a/config/metrics/counts_all/20210216175448_network_policy_drops.yml +++ b/config/metrics/counts_all/20210216175448_network_policy_drops.yml @@ -19,5 +19,6 @@ tier: - free - premium - ultimate -performance_indicator_type: [] +performance_indicator_type: +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/counts_all/20210216175537_ci_pipelines.yml b/config/metrics/counts_all/20210216175537_ci_pipelines.yml index 8a9a3ce892a..89f1a578b0b 100644 --- a/config/metrics/counts_all/20210216175537_ci_pipelines.yml +++ b/config/metrics/counts_all/20210216175537_ci_pipelines.yml @@ -16,4 +16,6 @@ tier: - free - premium - ultimate +performance_indicator_type: +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/counts_all/20210216180230_projects_jira_cloud_active.yml b/config/metrics/counts_all/20210216180230_projects_jira_cloud_active.yml index c82411850d5..283bda46c86 100644 --- a/config/metrics/counts_all/20210216180230_projects_jira_cloud_active.yml +++ b/config/metrics/counts_all/20210216180230_projects_jira_cloud_active.yml @@ -16,7 +16,8 @@ tier: - free - premium - ultimate -performance_indicator_type: [] +performance_indicator_type: +- customer_health_score milestone: "<13.9" removed_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112571 milestone_removed: "15.10" diff --git a/config/metrics/counts_all/20210216180234_projects_jira_dvcs_server_active.yml b/config/metrics/counts_all/20210216180234_projects_jira_dvcs_server_active.yml index bfb402c257e..2e1bbeca9a2 100644 --- a/config/metrics/counts_all/20210216180234_projects_jira_dvcs_server_active.yml +++ b/config/metrics/counts_all/20210216180234_projects_jira_dvcs_server_active.yml @@ -19,5 +19,6 @@ tier: - free - premium - ultimate -performance_indicator_type: [] +performance_indicator_type: +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/counts_all/20210216180628_projects_imported_from_github.yml b/config/metrics/counts_all/20210216180628_projects_imported_from_github.yml index 32f91f6faf6..3bbad9b4e62 100644 --- a/config/metrics/counts_all/20210216180628_projects_imported_from_github.yml +++ b/config/metrics/counts_all/20210216180628_projects_imported_from_github.yml @@ -16,5 +16,6 @@ tier: - free - premium - ultimate -performance_indicator_type: [] +performance_indicator_type: +- customer_health_score milestone: "<13.9" diff --git a/config/metrics/settings/20210204124856_instance_auto_devops_enabled.yml b/config/metrics/settings/20210204124856_instance_auto_devops_enabled.yml index df53d2784d2..4e035f5bb5a 100644 --- a/config/metrics/settings/20210204124856_instance_auto_devops_enabled.yml +++ b/config/metrics/settings/20210204124856_instance_auto_devops_enabled.yml @@ -16,5 +16,6 @@ tier: - free - premium - ultimate -performance_indicator_type: [] +performance_indicator_type: +- customer_health_score milestone: "<13.9" diff --git a/doc/user/project/settings/import_export_troubleshooting.md b/doc/user/project/settings/import_export_troubleshooting.md index c3abfa015a6..79401a7734a 100644 --- a/doc/user/project/settings/import_export_troubleshooting.md +++ b/doc/user/project/settings/import_export_troubleshooting.md @@ -187,6 +187,13 @@ e = Projects::ImportExport::ExportService.new(p,u) e.send(:version_saver).send(:save) e.send(:repo_saver).send(:save) +e.send(:avatar_saver).send(:save) +e.send(:project_tree_saver).send(:save) +e.send(:uploads_saver).send(:save) +e.send(:wiki_repo_saver).send(:save) +e.send(:lfs_saver).send(:save) +e.send(:snippets_repo_saver).send(:save) +e.send(:design_repo_saver).send(:save) ## continue using `e.send(:exporter_name).send(:save)` going through the list of exporters # The following line should show you the export_path similar to /var/opt/gitlab/gitlab-rails/shared/tmp/gitlab_exports/@hashed/49/94/4994.... diff --git a/lib/gitlab/kas.rb b/lib/gitlab/kas.rb index 8809795d9a1..255d8802c1c 100644 --- a/lib/gitlab/kas.rb +++ b/lib/gitlab/kas.rb @@ -55,6 +55,13 @@ module Gitlab uri.to_s end + def tunnel_ws_url + return tunnel_url if ws? + return tunnel_url.sub('https', 'wss') if ssl? + + tunnel_url.sub('http', 'ws') + end + # Return GitLab KAS internal_url # # @return [String] internal_url @@ -68,6 +75,16 @@ module Gitlab def enabled? !!Gitlab.config['gitlab_kas']&.fetch('enabled', false) end + + private + + def ssl? + URI(tunnel_url).scheme === 'https' + end + + def ws? + URI(tunnel_url).scheme.start_with?('ws') + end end end end diff --git a/qa/qa/page/project/new.rb b/qa/qa/page/project/new.rb index 8ea0b57ef3e..56650d5a782 100644 --- a/qa/qa/page/project/new.rb +++ b/qa/qa/page/project/new.rb @@ -6,6 +6,7 @@ module QA class New < Page::Base include Page::Component::Project::Templates include Page::Component::VisibilitySetting + include QA::Page::Component::Dropdown include Layout::Flash include Page::Component::Import::Selection @@ -50,7 +51,7 @@ module QA def choose_namespace(namespace) click_element :select_namespace_dropdown fill_element :select_namespace_dropdown_search_field, namespace - within_element(:select_namespace_dropdown) { click_button namespace } + select_item(namespace, css: '.gl-dropdown-item') end def click_import_project diff --git a/qa/qa/specs/features/browser_ui/9_data_stores/group/transfer_project_spec.rb b/qa/qa/specs/features/browser_ui/9_data_stores/group/transfer_project_spec.rb index 02e1598d6e7..387c3caaba4 100644 --- a/qa/qa/specs/features/browser_ui/9_data_stores/group/transfer_project_spec.rb +++ b/qa/qa/specs/features/browser_ui/9_data_stores/group/transfer_project_spec.rb @@ -2,7 +2,7 @@ module QA RSpec.describe 'Data Stores' do - describe 'Project transfer between groups', :reliable, product_group: :tenant_scale do + describe 'Project transfer', :reliable, product_group: :tenant_scale do let(:source_group) do Resource::Group.fabricate_via_api! do |group| group.path = "source-group-#{SecureRandom.hex(8)}" diff --git a/qa/qa/specs/features/browser_ui/9_data_stores/project/add_project_member_spec.rb b/qa/qa/specs/features/browser_ui/9_data_stores/project/add_project_member_spec.rb index 0beb297ffdb..6b9c47683d5 100644 --- a/qa/qa/specs/features/browser_ui/9_data_stores/project/add_project_member_spec.rb +++ b/qa/qa/specs/features/browser_ui/9_data_stores/project/add_project_member_spec.rb @@ -2,8 +2,8 @@ module QA RSpec.describe 'Data Stores', :reliable, product_group: :tenant_scale do - describe 'Add project member' do - it 'user adds project member', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347887' do + describe 'Project Member' do + it 'adds a project member', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347887' do Flow::Login.sign_in user = Resource::User.fabricate_or_use(Runtime::Env.gitlab_qa_username_1, Runtime::Env.gitlab_qa_password_1) diff --git a/qa/qa/specs/features/browser_ui/9_data_stores/project/create_project_badge_spec.rb b/qa/qa/specs/features/browser_ui/9_data_stores/project/create_project_badge_spec.rb index 87492af089e..3a39737a3b5 100644 --- a/qa/qa/specs/features/browser_ui/9_data_stores/project/create_project_badge_spec.rb +++ b/qa/qa/specs/features/browser_ui/9_data_stores/project/create_project_badge_spec.rb @@ -2,7 +2,7 @@ module QA RSpec.describe 'Data Stores' do - describe 'Create project badge', :reliable, product_group: :tenant_scale do + describe 'Project badge', :reliable, product_group: :tenant_scale do let(:badge_name) { "project-badge-#{SecureRandom.hex(8)}" } let(:expected_badge_link_url) { "#{Runtime::Scenario.gitlab_address}/#{project.path_with_namespace}" } let(:expected_badge_image_url) do @@ -21,7 +21,7 @@ module QA project.visit! end - it 'creates project badge successfully', + it 'creates project badge', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/350065' do Resource::ProjectBadge.fabricate! do |badge| badge.name = badge_name diff --git a/qa/qa/specs/features/browser_ui/9_data_stores/project/dashboard_images_spec.rb b/qa/qa/specs/features/browser_ui/9_data_stores/project/dashboard_images_spec.rb index f2136773f59..2ea96dfef95 100644 --- a/qa/qa/specs/features/browser_ui/9_data_stores/project/dashboard_images_spec.rb +++ b/qa/qa/specs/features/browser_ui/9_data_stores/project/dashboard_images_spec.rb @@ -16,7 +16,7 @@ module QA user.remove_via_api! end - it 'loads all images' do + it do Flow::Login.sign_in(as: user) Page::Dashboard::Welcome.perform do |welcome| @@ -32,14 +32,14 @@ module QA describe 'Check for broken images', :requires_admin, :reliable do context( - 'when logged in as a new user', + 'when a new user logs in', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347885' ) do it_behaves_like 'loads all images', false end context( - 'when logged in as a new admin', + 'when a new admin logs in', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347884' ) do it_behaves_like 'loads all images', true diff --git a/qa/qa/specs/features/browser_ui/9_data_stores/project/invite_group_to_project_spec.rb b/qa/qa/specs/features/browser_ui/9_data_stores/project/invite_group_to_project_spec.rb index c7501d437eb..7c756f9fb77 100644 --- a/qa/qa/specs/features/browser_ui/9_data_stores/project/invite_group_to_project_spec.rb +++ b/qa/qa/specs/features/browser_ui/9_data_stores/project/invite_group_to_project_spec.rb @@ -4,7 +4,7 @@ module QA RSpec.describe 'Data Stores' do describe 'Invite group', :reliable, product_group: :tenant_scale do shared_examples 'invites group to project' do - it 'verifies group is added and members can access project with correct access level' do + it 'grants group and members correct access level' do Page::Project::Menu.perform(&:click_members) Page::Project::Members.perform do |project_members| project_members.invite_group(group.path, 'Developer') diff --git a/qa/qa/specs/features/browser_ui/9_data_stores/project/project_owner_permissions_spec.rb b/qa/qa/specs/features/browser_ui/9_data_stores/project/project_owner_permissions_spec.rb index 2793b0440a4..21234f270ec 100644 --- a/qa/qa/specs/features/browser_ui/9_data_stores/project/project_owner_permissions_spec.rb +++ b/qa/qa/specs/features/browser_ui/9_data_stores/project/project_owner_permissions_spec.rb @@ -13,7 +13,7 @@ module QA Resource::User.fabricate_or_use(Runtime::Env.gitlab_qa_username_2, Runtime::Env.gitlab_qa_password_2) end - shared_examples 'when user is added as owner' do |project_type, testcase| + shared_examples 'adds user as owner' do |project_type, testcase| let!(:issue) do Resource::Issue.fabricate_via_api! do |issue| issue.api_client = owner_api_client @@ -27,7 +27,7 @@ module QA Flow::Login.sign_in(as: owner) end - it "has owner role with owner permissions", testcase: testcase do + it "has owner role and permissions", testcase: testcase do Page::Dashboard::Projects.perform do |projects| projects.filter_by_name(project.name) @@ -44,7 +44,7 @@ module QA end end - shared_examples 'when user is added as maintainer' do |testcase| + shared_examples 'adds user as maintainer' do |testcase| let!(:issue) do Resource::Issue.fabricate_via_api! do |issue| issue.api_client = owner_api_client @@ -82,8 +82,8 @@ module QA end end - it_behaves_like 'when user is added as owner', :personal_project, 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/352542' - it_behaves_like 'when user is added as maintainer', 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/352607' + it_behaves_like 'adds user as owner', :personal_project, 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/352542' + it_behaves_like 'adds user as maintainer', 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/352607' end context 'for group projects' do @@ -96,8 +96,8 @@ module QA end end - it_behaves_like 'when user is added as owner', :group_project, 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/366436' - it_behaves_like 'when user is added as maintainer', 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/366435' + it_behaves_like 'adds user as owner', :group_project, 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/366436' + it_behaves_like 'adds user as maintainer', 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/366435' end end end diff --git a/qa/qa/specs/features/browser_ui/9_data_stores/project/view_project_activity_spec.rb b/qa/qa/specs/features/browser_ui/9_data_stores/project/view_project_activity_spec.rb index 4945ef533a4..5cdb88407fb 100644 --- a/qa/qa/specs/features/browser_ui/9_data_stores/project/view_project_activity_spec.rb +++ b/qa/qa/specs/features/browser_ui/9_data_stores/project/view_project_activity_spec.rb @@ -3,22 +3,24 @@ module QA RSpec.describe 'Data Stores' do describe 'Project activity', :reliable, product_group: :tenant_scale do - it 'user creates an event in the activity page upon Git push', - testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347879' do - Flow::Login.sign_in + context 'with git push' do + it 'creates an event in the activity page', + testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347879' do + Flow::Login.sign_in - project = Resource::Repository::ProjectPush.fabricate! do |push| - push.file_name = 'README.md' - push.file_content = '# This is a test project' - push.commit_message = 'Add README.md' - end.project + project = Resource::Repository::ProjectPush.fabricate! do |push| + push.file_name = 'README.md' + push.file_content = '# This is a test project' + push.commit_message = 'Add README.md' + end.project - project.visit! - Page::Project::Menu.perform(&:click_activity) - Page::Project::Activity.perform do |activity| - activity.click_push_events + project.visit! + Page::Project::Menu.perform(&:click_activity) + Page::Project::Activity.perform do |activity| + activity.click_push_events - expect(activity).to have_content("pushed new branch #{project.default_branch}") + expect(activity).to have_content("pushed new branch #{project.default_branch}") + end end end end diff --git a/qa/qa/specs/features/browser_ui/9_data_stores/user/parent_group_access_termination_spec.rb b/qa/qa/specs/features/browser_ui/9_data_stores/user/parent_group_access_termination_spec.rb index 7e88f3a9ac3..76b7accbeeb 100644 --- a/qa/qa/specs/features/browser_ui/9_data_stores/user/parent_group_access_termination_spec.rb +++ b/qa/qa/specs/features/browser_ui/9_data_stores/user/parent_group_access_termination_spec.rb @@ -25,7 +25,7 @@ module QA end end - context 'when parent group membership is terminated' do + context 'with terminated parent group membership' do before do group.add_member(user) @@ -40,7 +40,7 @@ module QA end end - it 'is not allowed to edit the project files', + it 'can not edit the project files', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347866' do Flow::Login.sign_in(as: user) project.visit! diff --git a/spec/controllers/concerns/kas_cookie_spec.rb b/spec/controllers/concerns/kas_cookie_spec.rb index c9490508690..355d1e04b86 100644 --- a/spec/controllers/concerns/kas_cookie_spec.rb +++ b/spec/controllers/concerns/kas_cookie_spec.rb @@ -89,12 +89,16 @@ RSpec.describe KasCookie, feature_category: :deployment_management do end context 'when KAS is on subdomain' do - let_it_be(:kas_tunnel_url) { 'ws://kas.gitlab.example.com/k8s-proxy/' } + let_it_be(:kas_tunnel_url) { 'http://kas.gitlab.example.com/k8s-proxy/' } it 'adds KAS url to CSP connect-src directive' do expect(kas_csp_connect_src).to include(::Gitlab::Kas.tunnel_url) end + it 'adds websocket connections' do + expect(kas_csp_connect_src).to include('ws://kas.gitlab.example.com/k8s-proxy/') + end + context 'when content_security_policy is disabled' do let(:content_security_policy_enabled) { false } @@ -104,6 +108,14 @@ RSpec.describe KasCookie, feature_category: :deployment_management do end end + context 'when KAS tunnel has ssl' do + let_it_be(:kas_tunnel_url) { 'https://kas.gitlab.example.com/k8s-proxy/' } + + it 'adds websocket connections' do + expect(kas_csp_connect_src).to include('wss://kas.gitlab.example.com/k8s-proxy/') + end + end + context 'when KAS tunnel url is configured without trailing slash' do let_it_be(:kas_tunnel_url) { 'ws://kas.gitlab.example.com/k8s-proxy' } diff --git a/spec/finders/autocomplete/group_users_finder_spec.rb b/spec/finders/autocomplete/group_users_finder_spec.rb new file mode 100644 index 00000000000..9ad1b43ea22 --- /dev/null +++ b/spec/finders/autocomplete/group_users_finder_spec.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Autocomplete::GroupUsersFinder, feature_category: :team_planning do + let_it_be(:parent_group) { create(:group) } + let_it_be(:group) { create(:group, parent: parent_group) } + let_it_be(:subgroup) { create(:group, parent: group) } + + let_it_be(:parent_group_project) { create(:project, namespace: parent_group) } + let_it_be(:group_project) { create(:project, namespace: group) } + let_it_be(:subgroup_project) { create(:project, namespace: subgroup) } + + let(:finder) { described_class.new(group: group) } + + describe '#execute' do + context 'with group members' do + let_it_be(:parent_group_member) { create(:user).tap { |u| parent_group.add_developer(u) } } + let_it_be(:group_member) { create(:user).tap { |u| group.add_developer(u) } } + let_it_be(:subgroup_member) { create(:user).tap { |u| subgroup.add_developer(u) } } + + let_it_be(:other_group) { create(:group) } + let_it_be(:other_group_member) { create(:user).tap { |u| other_group.add_developer(u) } } + + it 'returns members of groups in the hierarchy' do + expect(finder.execute).to contain_exactly( + parent_group_member, + group_member, + subgroup_member + ) + end + end + + context 'with project members' do + let_it_be(:parent_group_project_member) { create(:user).tap { |u| parent_group_project.add_developer(u) } } + let_it_be(:group_project_member) { create(:user).tap { |u| group_project.add_developer(u) } } + let_it_be(:subgroup_project_member) { create(:user).tap { |u| subgroup_project.add_developer(u) } } + + it 'returns members of descendant projects' do + expect(finder.execute).to contain_exactly( + group_project_member, + subgroup_project_member + ) + end + end + + context 'with invited group members' do + let_it_be(:invited_group) { create(:group) } + let_it_be(:invited_group_user) { create(:user).tap { |u| invited_group.add_developer(u) } } + + it 'returns members of groups invited to this group' do + create(:group_group_link, shared_group: group, shared_with_group: invited_group) + + expect(finder.execute).to contain_exactly(invited_group_user) + end + + it 'returns members of groups invited to an ancestor group' do + create(:group_group_link, shared_group: parent_group, shared_with_group: invited_group) + + expect(finder.execute).to contain_exactly(invited_group_user) + end + + it 'returns members of groups invited to a descendant group' do + create(:group_group_link, shared_group: subgroup, shared_with_group: invited_group) + + expect(finder.execute).to contain_exactly(invited_group_user) + end + + it 'returns members of groups invited to a child project' do + create(:project_group_link, project: group_project, group: invited_group) + + expect(finder.execute).to contain_exactly(invited_group_user) + end + + it 'returns members of groups invited to a descendant project' do + create(:project_group_link, project: subgroup_project, group: invited_group) + + expect(finder.execute).to contain_exactly(invited_group_user) + end + + it 'does not return members of groups invited to a project of an ancestor group' do + create(:project_group_link, project: parent_group_project, group: invited_group) + + expect(finder.execute).to be_empty + end + + context 'when include_descendant_shares_in_user_autocomplete is disabled' do + before do + stub_feature_flags(include_descendant_shares_in_user_autocomplete: false) + end + + it 'does not return members of groups invited to a descendant group' do + create(:group_group_link, shared_group: subgroup, shared_with_group: invited_group) + + expect(finder.execute).to be_empty + end + + it 'does not return members of groups invited to a child project' do + create(:project_group_link, project: group_project, group: invited_group) + + expect(finder.execute).to be_empty + end + + it 'does not return members of groups invited to a descendant project' do + create(:project_group_link, project: subgroup_project, group: invited_group) + + expect(finder.execute).to be_empty + end + end + end + end +end diff --git a/spec/lib/gitlab/kas_spec.rb b/spec/lib/gitlab/kas_spec.rb index 1b42b031c42..69d9ca7d4ed 100644 --- a/spec/lib/gitlab/kas_spec.rb +++ b/spec/lib/gitlab/kas_spec.rb @@ -132,6 +132,52 @@ RSpec.describe Gitlab::Kas do end end + describe '.tunnel_ws_url' do + before do + stub_config(gitlab_kas: { external_url: external_url }) + end + + let(:external_url) { 'xyz' } + + subject { described_class.tunnel_ws_url } + + context 'with a gitlab_kas.external_k8s_proxy_url setting' do + let(:external_k8s_proxy_url) { 'http://abc' } + + before do + stub_config(gitlab_kas: { external_k8s_proxy_url: external_k8s_proxy_url }) + end + + it { is_expected.to eq('ws://abc') } + end + + context 'without a gitlab_kas.external_k8s_proxy_url setting' do + context 'external_url uses wss://' do + let(:external_url) { 'wss://kas.gitlab.example.com' } + + it { is_expected.to eq('wss://kas.gitlab.example.com/k8s-proxy') } + end + + context 'external_url uses ws://' do + let(:external_url) { 'ws://kas.gitlab.example.com' } + + it { is_expected.to eq('ws://kas.gitlab.example.com/k8s-proxy') } + end + + context 'external_url uses grpcs://' do + let(:external_url) { 'grpcs://kas.gitlab.example.com' } + + it { is_expected.to eq('wss://kas.gitlab.example.com/k8s-proxy') } + end + + context 'external_url uses grpc://' do + let(:external_url) { 'grpc://kas.gitlab.example.com' } + + it { is_expected.to eq('ws://kas.gitlab.example.com/k8s-proxy') } + end + end + end + describe '.internal_url' do it 'returns gitlab_kas internal_url config' do expect(described_class.internal_url).to eq(Gitlab.config.gitlab_kas.internal_url) diff --git a/spec/initializers/active_record_relation_union_reset_spec.rb b/spec/models/concerns/reset_on_union_error_spec.rb index 013dfa1b49b..70993b92c90 100644 --- a/spec/initializers/active_record_relation_union_reset_spec.rb +++ b/spec/models/concerns/reset_on_union_error_spec.rb @@ -2,10 +2,9 @@ require 'spec_helper' -# rubocop:disable Database/MultipleDatabases -RSpec.describe ActiveRecordRelationUnionReset, :delete, feature_category: :shared do +RSpec.describe ResetOnUnionError, :delete, feature_category: :shared do let(:test_unioned_model) do - Class.new(ActiveRecord::Base) do + Class.new(ApplicationRecord) do include FromUnion self.table_name = '_test_unioned_model' @@ -17,7 +16,7 @@ RSpec.describe ActiveRecordRelationUnionReset, :delete, feature_category: :share end before(:context) do - ActiveRecord::Base.connection.execute(<<~SQL) + ApplicationRecord.connection.execute(<<~SQL) CREATE TABLE _test_unioned_model ( id serial NOT NULL PRIMARY KEY, created_at timestamptz NOT NULL @@ -26,7 +25,7 @@ RSpec.describe ActiveRecordRelationUnionReset, :delete, feature_category: :share end after(:context) do - ActiveRecord::Base.connection.execute(<<~SQL) + ApplicationRecord.connection.execute(<<~SQL) DROP TABLE _test_unioned_model SQL end @@ -44,13 +43,13 @@ RSpec.describe ActiveRecordRelationUnionReset, :delete, feature_category: :share before do load_query - ActiveRecord::Base.connection.execute(<<~SQL) + ApplicationRecord.connection.execute(<<~SQL) ALTER TABLE _test_unioned_model ADD COLUMN _test_new_column int; SQL end after do - ActiveRecord::Base.connection.execute(<<~SQL) + ApplicationRecord.connection.execute(<<~SQL) ALTER TABLE _test_unioned_model DROP COLUMN _test_new_column; SQL @@ -131,4 +130,3 @@ RSpec.describe ActiveRecordRelationUnionReset, :delete, feature_category: :share end end end -# rubocop:enable Database/MultipleDatabases diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index eb604456511..23e72f6663a 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -1820,14 +1820,14 @@ RSpec.describe Group, feature_category: :groups_and_projects do let_it_be(:developer) { group.add_member(create(:user), GroupMember::DEVELOPER) } let_it_be(:other_developer) { group.add_member(create(:user), GroupMember::DEVELOPER) } - describe '#direct_and_indirect_members' do + describe '#hierarchy_members' do it 'returns parents members' do - expect(group.direct_and_indirect_members).to include(developer) - expect(group.direct_and_indirect_members).to include(maintainer) + expect(group.hierarchy_members).to include(developer) + expect(group.hierarchy_members).to include(maintainer) end it 'returns descendant members' do - expect(group.direct_and_indirect_members).to include(other_developer) + expect(group.hierarchy_members).to include(other_developer) end end @@ -1878,52 +1878,6 @@ RSpec.describe Group, feature_category: :groups_and_projects do end end - context 'user-related methods' do - let_it_be(:user_a) { create(:user) } - let_it_be(:user_b) { create(:user) } - let_it_be(:user_c) { create(:user) } - let_it_be(:user_d) { create(:user) } - - let_it_be(:group) { create(:group) } - let_it_be(:nested_group) { create(:group, parent: group) } - let_it_be(:deep_nested_group) { create(:group, parent: nested_group) } - let_it_be(:project) { create(:project, namespace: group) } - - let_it_be(:another_group) { create(:group) } - let_it_be(:another_user) { create(:user) } - - before_all do - group.add_developer(user_a) - group.add_developer(user_c) - nested_group.add_developer(user_b) - deep_nested_group.add_developer(user_a) - project.add_developer(user_d) - - another_group.add_developer(another_user) - - create(:group_group_link, shared_group: group, shared_with_group: another_group) - end - - describe '#direct_and_indirect_users' do - it 'returns member users on every nest level without duplication' do - expect(group.direct_and_indirect_users).to contain_exactly(user_a, user_b, user_c, user_d) - expect(nested_group.direct_and_indirect_users).to contain_exactly(user_a, user_b, user_c) - expect(deep_nested_group.direct_and_indirect_users).to contain_exactly(user_a, user_b, user_c) - end - - it 'does not return members of projects belonging to ancestor groups' do - expect(nested_group.direct_and_indirect_users).not_to include(user_d) - end - - context 'when share_with_groups is true' do - it 'also returns members of groups invited to this group' do - expect(group.direct_and_indirect_users(share_with_groups: true)) - .to contain_exactly(user_a, user_b, user_c, user_d, another_user) - end - end - end - end - describe '#project_users_with_descendants' do let(:user_a) { create(:user) } let(:user_b) { create(:user) } diff --git a/spec/services/groups/participants_service_spec.rb b/spec/services/groups/participants_service_spec.rb index 0b370ca9fd8..8359bf1670f 100644 --- a/spec/services/groups/participants_service_spec.rb +++ b/spec/services/groups/participants_service_spec.rb @@ -66,15 +66,7 @@ RSpec.describe Groups::ParticipantsService, feature_category: :groups_and_projec subject(:usernames) { service_result.pluck(:username) } - context 'when current_user is not a member' do - let(:service) { described_class.new(group, create(:user)) } - - it { is_expected.not_to include(private_group_member.username) } - end - - context 'when current_user is a member' do - it { is_expected.to include(private_group_member.username) } - end + it { is_expected.to include(private_group_member.username) } end end diff --git a/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb b/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb index 9a7ecd8827b..fd2c7455c5f 100644 --- a/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb +++ b/spec/support/shared_examples/services/security/ci_configuration/create_service_shared_examples.rb @@ -111,11 +111,14 @@ RSpec.shared_examples_for 'services security ci configuration create service' do YAML end - it 'fails with error' do + it 'returns a ServiceResponse error' do expect(project).to receive(:ci_config_for).and_return(unsupported_yaml) - expect { result }.to raise_error(Gitlab::Graphql::Errors::MutationError, - _(".gitlab-ci.yml with aliases/anchors is not supported. Please change the CI configuration manually.")) + expect(result).to be_kind_of(ServiceResponse) + expect(result.status).to eq(:error) + expect(result.message).to eq( + _(".gitlab-ci.yml with aliases/anchors is not supported. Please change the CI configuration manually.") + ) end end @@ -133,11 +136,13 @@ RSpec.shared_examples_for 'services security ci configuration create service' do YAML end - it 'fails with error' do + it 'returns a ServiceResponse error' do expect(project).to receive(:ci_config_for).and_return(invalid_yaml) expect(YAML).to receive(:safe_load).and_raise(Psych::Exception) - expect { result }.to raise_error(Gitlab::Graphql::Errors::MutationError, /merge request creation mutation failed/) + expect(result).to be_kind_of(ServiceResponse) + expect(result.status).to eq(:error) + expect(result.message).to match(/merge request creation failed/) end end diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index 8c33224968d..576f6deeec6 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -1210,6 +1210,86 @@ RSpec.describe ObjectStorage, :clean_gitlab_redis_shared_state, feature_category end end + describe '#replace_file_without_saving!' do + context 'when object storage and direct upload is enabled' do + let(:upload_path) { 'some/path/123' } + + let!(:fog_connection) do + stub_uploads_object_storage(uploader_class, enabled: true, direct_upload: true) + end + + let!(:fog_file) do + fog_connection.directories.new(key: 'uploads').files.create( # rubocop:disable Rails/SaveBang + key: upload_path, + body: 'old content' + ) + end + + before do + uploader.object_store = described_class::Store::REMOTE + end + + # This scenario can happen when replicating object storage uploads. + # See Geo::Replication::BlobDownloader#download_file + # A SanitizedFile from a Tempfile will be passed to replace_file_without_saving! + context 'and given file is not a CarrierWave::Storage::Fog::File' do + let(:temp_file) { Tempfile.new("test") } + let(:new_file) { CarrierWave::SanitizedFile.new(temp_file) } + + before do + temp_file.write('new content') + temp_file.close + FileUtils.touch(temp_file) + allow(object).to receive(:file_final_path).and_return(file_final_path) + end + + after do + FileUtils.rm_f(temp_file) + end + + shared_examples 'skipping triggers for local file' do + it 'allows file to be replaced without triggering any callbacks' do + expect(uploader).not_to receive(:with_callbacks) + + uploader.replace_file_without_saving!(new_file) + end + + it 'does not trigger pending upload checks' do + expect(ObjectStorage::PendingDirectUpload).not_to receive(:complete) + + uploader.replace_file_without_saving!(new_file) + end + end + + context 'and uploader model has the file_final_path' do + let(:file_final_path) { upload_path } + + it_behaves_like 'skipping triggers for local file' + + it 'uses default CarrierWave behavior and uploads the file to object storage using the final path' do + uploader.replace_file_without_saving!(new_file) + + updated_content = fog_connection.directories.get('uploads').files.get(upload_path).body + expect(updated_content).to eq('new content') + end + end + + context 'and uploader model has no file_final_path' do + let(:file_final_path) { nil } + + it_behaves_like 'skipping triggers for local file' + + it 'uses default CarrierWave behavior and uploads the file to object storage using the uploader store path' do + uploader.replace_file_without_saving!(new_file) + + content = fog_connection.directories.get('uploads').files.get(uploader.store_path).body + expect(content).to eq('new content') + end + end + end + end + end + describe '.generate_final_store_path' do let(:root_id) { 12345 } let(:expected_root_hashed_path) { Gitlab::HashedPath.new(root_hash: root_id) } |