diff options
50 files changed, 1186 insertions, 405 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue index 33a83aef057..d878a1fa2e0 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/actions.vue @@ -70,6 +70,7 @@ export default { variant="confirm" size="small" class="gl-display-none gl-md-display-block gl-float-left" + data-testid="extension-actions-button" @click="onClickAction(btn)" > {{ btn.text }} diff --git a/app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js b/app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js new file mode 100644 index 00000000000..d9868101e9d --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js @@ -0,0 +1,190 @@ +import { __, n__, s__, sprintf } from '~/locale'; +import axios from '~/lib/utils/axios_utils'; +import Poll from '~/lib/utils/poll'; +import { EXTENSION_ICONS } from '../../constants'; + +export default { + name: 'WidgetTerraform', + i18n: { + label: s__('Terraform|Terraform reports'), + loading: s__('Terraform|Loading Terraform reports...'), + error: s__('Terraform|Failed to load Terraform reports'), + reportGenerated: s__('Terraform|A Terraform report was generated in your pipelines.'), + namedReportGenerated: s__( + 'Terraform|The job %{strong_start}%{name}%{strong_end} generated a report.', + ), + reportChanges: s__( + 'Terraform|Reported Resource Changes: %{addNum} to add, %{changeNum} to change, %{deleteNum} to delete', + ), + reportFailed: s__('Terraform|A Terraform report failed to generate.'), + namedReportFailed: s__( + 'Terraform|The job %{strong_start}%{name}%{strong_end} failed to generate a report.', + ), + reportErrored: s__('Terraform|Generating the report caused an error.'), + fullLog: __('Full log'), + }, + expandEvent: 'i_testing_terraform_widget_total', + props: ['terraformReportsPath'], + computed: { + // Extension computed props + statusIcon() { + return EXTENSION_ICONS.warning; + }, + }, + methods: { + // Extension methods + summary({ valid = [], invalid = [] }) { + let title; + let subtitle = ''; + + const validText = sprintf( + n__( + 'Terraform|%{strong_start}%{number}%{strong_end} Terraform report was generated in your pipelines', + 'Terraform|%{strong_start}%{number}%{strong_end} Terraform reports were generated in your pipelines', + valid.length, + ), + { + number: valid.length, + }, + false, + ); + + const invalidText = sprintf( + n__( + 'Terraform|%{strong_start}%{number}%{strong_end} Terraform report failed to generate', + 'Terraform|%{strong_start}%{number}%{strong_end} Terraform reports failed to generate', + invalid.length, + ), + { + number: invalid.length, + }, + false, + ); + + if (valid.length) { + title = validText; + if (invalid.length) { + subtitle = sprintf(`<br>%{small_start}${invalidText}%{small_end}`); + } + } else { + title = invalidText; + } + + return `${title}${subtitle}`; + }, + fetchCollapsedData() { + return Promise.resolve(this.fetchPlans().then(this.prepareReports)); + }, + fetchFullData() { + const { valid, invalid } = this.collapsedData; + return Promise.resolve([...valid, ...invalid]); + }, + // Custom methods + fetchPlans() { + return new Promise((resolve) => { + const poll = new Poll({ + resource: { + fetchPlans: () => axios.get(this.terraformReportsPath), + }, + data: this.terraformReportsPath, + method: 'fetchPlans', + successCallback: ({ data }) => { + if (Object.keys(data).length > 0) { + poll.stop(); + + const result = Object.keys(data).map((key) => { + return data[key]; + }); + + resolve(result); + } + }, + errorCallback: () => { + const invalidData = { tf_report_error: 'api_error' }; + poll.stop(); + const result = [invalidData]; + resolve(result); + }, + }); + + poll.makeRequest(); + }); + }, + createReportRow(report, iconName) { + const addNum = Number(report.create); + const changeNum = Number(report.update); + const deleteNum = Number(report.delete); + const validPlanValues = addNum + changeNum + deleteNum >= 0; + + const actions = []; + + let title; + let subtitle; + + if (report.job_path) { + const action = { + href: report.job_path, + text: this.$options.i18n.fullLog, + target: '_blank', + }; + actions.push(action); + } + + if (validPlanValues) { + if (report.job_name) { + title = sprintf( + this.$options.i18n.namedReportGenerated, + { + name: report.job_name, + }, + false, + ); + } else { + title = this.$options.i18n.reportGenerated; + } + + subtitle = sprintf(`%{small_start}${this.$options.i18n.reportChanges}%{small_end}`, { + addNum, + changeNum, + deleteNum, + }); + } else { + if (report.job_name) { + title = sprintf( + this.$options.i18n.namedReportFailed, + { + name: report.job_name, + }, + false, + ); + } else { + title = this.$options.i18n.reportFailed; + } + + subtitle = sprintf(`%{small_start}${this.$options.i18n.reportErrored}%{small_end}`); + } + + return { + text: `${title} + <br> + ${subtitle}`, + icon: { name: iconName }, + actions, + }; + }, + prepareReports(reports) { + const valid = []; + const invalid = []; + + reports.forEach((report) => { + if (report.tf_report_error) { + invalid.push(this.createReportRow(report, EXTENSION_ICONS.error)); + } else { + valid.push(this.createReportRow(report, EXTENSION_ICONS.success)); + } + }); + + return { valid, invalid }; + }, + }, +}; diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index c98dc426224..83a07240403 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -1,6 +1,7 @@ <script> import { GlSafeHtmlDirective } from '@gitlab/ui'; import { isEmpty } from 'lodash'; +import { registerExtension } from '~/vue_merge_request_widget/components/extensions'; import MrWidgetApprovals from 'ee_else_ce/vue_merge_request_widget/components/approvals/approvals.vue'; import MRWidgetService from 'ee_else_ce/vue_merge_request_widget/services/mr_widget_service'; import MRWidgetStore from 'ee_else_ce/vue_merge_request_widget/stores/mr_widget_store'; @@ -43,6 +44,7 @@ import { STATE_MACHINE, stateToComponentMap } from './constants'; import eventHub from './event_hub'; import mergeRequestQueryVariablesMixin from './mixins/merge_request_query_variables'; import getStateQuery from './queries/get_state.query.graphql'; +import terraformExtension from './extensions/terraform'; export default { // False positive i18n lint: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/25 @@ -184,6 +186,9 @@ export default { shouldRenderSecurityReport() { return Boolean(this.mr.pipeline.id); }, + shouldRenderTerraformPlans() { + return Boolean(this.mr?.terraformReportsPath); + }, mergeError() { let { mergeError } = this.mr; @@ -230,6 +235,11 @@ export default { this.initPostMergeDeploymentsPolling(); } }, + shouldRenderTerraformPlans(newVal) { + if (newVal) { + this.registerTerraformPlans(); + } + }, }, mounted() { MRWidgetService.fetchInitialData() @@ -463,6 +473,11 @@ export default { dismissSuggestPipelines() { this.mr.isDismissedSuggestPipeline = true; }, + registerTerraformPlans() { + if (this.shouldRenderTerraformPlans && this.shouldShowExtension) { + registerExtension(terraformExtension); + } + }, }, }; </script> @@ -542,7 +557,10 @@ export default { :pipeline-path="mr.pipeline.path" /> - <terraform-plan v-if="mr.terraformReportsPath" :endpoint="mr.terraformReportsPath" /> + <terraform-plan + v-if="mr.terraformReportsPath && !shouldShowExtension" + :endpoint="mr.terraformReportsPath" + /> <grouped-accessibility-reports-app v-if="shouldShowAccessibilityReport" diff --git a/app/controllers/concerns/check_rate_limit.rb b/app/controllers/concerns/check_rate_limit.rb index 5ccdf843525..0eaf74fd3a9 100644 --- a/app/controllers/concerns/check_rate_limit.rb +++ b/app/controllers/concerns/check_rate_limit.rb @@ -8,6 +8,7 @@ # See lib/api/helpers/rate_limiter.rb for API version module CheckRateLimit def check_rate_limit!(key, scope:, redirect_back: false, **options) + return if bypass_header_set? return unless rate_limiter.throttled?(key, scope: scope, **options) rate_limiter.log_request(request, "#{key}_request_limit".to_sym, current_user) @@ -28,4 +29,8 @@ module CheckRateLimit def rate_limiter ::Gitlab::ApplicationRateLimiter end + + def bypass_header_set? + ::Gitlab::Throttle.bypass_header.present? && request.get_header(Gitlab::Throttle.bypass_header) == '1' + end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 7133233f083..0b1516f6f45 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -42,7 +42,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:restructured_mr_widget, project, default_enabled: :yaml) push_frontend_feature_flag(:mr_changes_fluid_layout, project, default_enabled: :yaml) push_frontend_feature_flag(:mr_attention_requests, project, default_enabled: :yaml) - + push_frontend_feature_flag(:refactor_mr_widgets_extensions, @project, default_enabled: :yaml) # Usage data feature flags push_frontend_feature_flag(:users_expanding_widgets_usage_data, @project, default_enabled: :yaml) push_frontend_feature_flag(:diff_settings_usage_data, default_enabled: :yaml) diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb index 75623d33ef5..fff17098c7b 100644 --- a/app/finders/group_members_finder.rb +++ b/app/finders/group_members_finder.rb @@ -1,14 +1,15 @@ # frozen_string_literal: true class GroupMembersFinder < UnionFinder - RELATIONS = %i(direct inherited descendants).freeze + RELATIONS = %i(direct inherited descendants shared_from_groups).freeze DEFAULT_RELATIONS = %i(direct inherited).freeze INVALID_RELATION_TYPE_ERROR_MSG = "is not a valid relation type. Valid relation types are #{RELATIONS.join(', ')}." RELATIONS_DESCRIPTIONS = { direct: 'Members in the group itself', inherited: "Members in the group's ancestor groups", - descendants: "Members in the group's subgroups" + descendants: "Members in the group's subgroups", + shared_from_groups: "Invited group's members" }.freeze include CreatedAtFilter @@ -28,11 +29,7 @@ class GroupMembersFinder < UnionFinder end def execute(include_relations: DEFAULT_RELATIONS) - return filter_members(group_members_list) if include_relations == [:direct] - groups = groups_by_relations(include_relations) - return GroupMember.none unless groups - members = all_group_members(groups).distinct_on_user_with_max_access_level filter_members(members) @@ -45,22 +42,14 @@ class GroupMembersFinder < UnionFinder def groups_by_relations(include_relations) check_relation_arguments!(include_relations) - case include_relations.sort - when [:inherited] - group.ancestors - when [:descendants] - group.descendants - when [:direct, :inherited] - group.self_and_ancestors - when [:descendants, :direct] - group.self_and_descendants - when [:descendants, :inherited] - find_union([group.ancestors, group.descendants], Group) - when [:descendants, :direct, :inherited] - group.self_and_hierarchy - else - nil - end + related_groups = [] + + related_groups << Group.by_id(group.id) if include_relations&.include?(:direct) + related_groups << group.ancestors if include_relations&.include?(:inherited) + related_groups << group.descendants if include_relations&.include?(:descendants) + related_groups << group.shared_with_groups.public_or_visible_to_user(user) if include_relations&.include?(:shared_from_groups) + + find_union(related_groups, Group) end def filter_members(members) diff --git a/app/models/integrations/jira.rb b/app/models/integrations/jira.rb index d46299de1be..816f5cbe177 100644 --- a/app/models/integrations/jira.rb +++ b/app/models/integrations/jira.rb @@ -303,11 +303,7 @@ module Integrations private def branch_name(commit) - if Feature.enabled?(:jira_use_first_ref_by_oid, project, default_enabled: :yaml) - commit.first_ref_by_oid(project.repository) - else - commit.ref_names(project.repository).first - end + commit.first_ref_by_oid(project.repository) end def server_info diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index 1ad4cb6d368..a8a4fbedc41 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -18,7 +18,7 @@ class GroupMember < Member default_scope { where(source_type: SOURCE_TYPE) } # rubocop:disable Cop/DefaultScope - scope :of_groups, ->(groups) { where(source_id: groups.select(:id)) } + scope :of_groups, ->(groups) { where(source_id: groups&.select(:id)) } scope :of_ldap_type, -> { where(ldap: true) } scope :count_users_by_group_id, -> { group(:source_id).count } scope :with_user, -> (user) { where(user: user) } diff --git a/app/views/projects/protected_branches/shared/_protected_branch.html.haml b/app/views/projects/protected_branches/shared/_protected_branch.html.haml index 02ec778b97c..3f8e560e994 100644 --- a/app/views/projects/protected_branches/shared/_protected_branch.html.haml +++ b/app/views/projects/protected_branches/shared/_protected_branch.html.haml @@ -5,7 +5,7 @@ %span.ref-name= protected_branch.name - if @project.root_ref?(protected_branch.name) - %span.badge.gl-badge.badge-pill.badge-info.d-inline default + = gl_badge_tag s_('ProtectedBranch|default'), variant: :info %div - if protected_branch.wildcard? @@ -20,4 +20,4 @@ - if can_admin_project %td - = link_to 'Unprotect', [@project, protected_branch, { update_section: 'js-protected-branches-settings' }], disabled: local_assigns[:disabled], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn gl-button btn-warning" + = link_to s_('ProtectedBranch|Unprotect'), [@project, protected_branch, { update_section: 'js-protected-branches-settings' }], disabled: local_assigns[:disabled], data: { confirm: s_('ProtectedBranch|Branch will be writable for developers. Are you sure?') }, method: :delete, class: "btn gl-button btn-warning" diff --git a/config/feature_flags/development/jira_use_first_ref_by_oid.yml b/config/feature_flags/development/rate_limit_user_by_id_endpoint.yml index 88db6c1ab4c..d5523b7541b 100644 --- a/config/feature_flags/development/jira_use_first_ref_by_oid.yml +++ b/config/feature_flags/development/rate_limit_user_by_id_endpoint.yml @@ -1,8 +1,8 @@ --- -name: jira_use_first_ref_by_oid -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72739 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343585 -milestone: '14.5' +name: rate_limit_user_by_id_endpoint +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73069 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/348796 +milestone: '14.6' type: development -group: group::integrations -default_enabled: true +group: group::optimize +default_enabled: false diff --git a/config/feature_flags/development/vulnerability_location_image_filter.yml b/config/feature_flags/development/vulnerability_location_image_filter.yml deleted file mode 100644 index 1bbc8e43d57..00000000000 --- a/config/feature_flags/development/vulnerability_location_image_filter.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: vulnerability_location_image_filter -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69867 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/340915 -milestone: '14.4' -type: development -group: group::container security -default_enabled: true diff --git a/config/initializers/7_prometheus_metrics.rb b/config/initializers/7_prometheus_metrics.rb index 8ef11b83131..15757c05bd0 100644 --- a/config/initializers/7_prometheus_metrics.rb +++ b/config/initializers/7_prometheus_metrics.rb @@ -70,17 +70,17 @@ if !Rails.env.test? && Gitlab::Metrics.prometheus_metrics_enabled? Gitlab::Cluster::LifecycleEvents.on_worker_start do defined?(::Prometheus::Client.reinitialize_on_pid_change) && ::Prometheus::Client.reinitialize_on_pid_change - - Gitlab::Metrics::Samplers::RubySampler.initialize_instance.start - Gitlab::Metrics::Samplers::DatabaseSampler.initialize_instance.start - Gitlab::Metrics::Samplers::ThreadsSampler.initialize_instance.start + logger = Gitlab::AppLogger + Gitlab::Metrics::Samplers::RubySampler.initialize_instance(logger: logger).start + Gitlab::Metrics::Samplers::DatabaseSampler.initialize_instance(logger: logger).start + Gitlab::Metrics::Samplers::ThreadsSampler.initialize_instance(logger: logger).start if Gitlab::Runtime.web_server? - Gitlab::Metrics::Samplers::ActionCableSampler.instance.start + Gitlab::Metrics::Samplers::ActionCableSampler.instance(logger: logger).start end if Gitlab.ee? && Gitlab::Runtime.sidekiq? - Gitlab::Metrics::Samplers::GlobalSearchSampler.instance.start + Gitlab::Metrics::Samplers::GlobalSearchSampler.instance(logger: logger).start end Gitlab::Ci::Parsers.instrument! diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 573659881e6..a168df4fd63 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -16555,6 +16555,7 @@ Group member relation. | <a id="groupmemberrelationdescendants"></a>`DESCENDANTS` | Members in the group's subgroups. | | <a id="groupmemberrelationdirect"></a>`DIRECT` | Members in the group itself. | | <a id="groupmemberrelationinherited"></a>`INHERITED` | Members in the group's ancestor groups. | +| <a id="groupmemberrelationshared_from_groups"></a>`SHARED_FROM_GROUPS` | Invited group's members. | ### `GroupPermission` diff --git a/doc/api/members.md b/doc/api/members.md index bc476980d2d..2ee1662c7be 100644 --- a/doc/api/members.md +++ b/doc/api/members.md @@ -88,15 +88,20 @@ Example response: ] ``` -## List all members of a group or project including inherited members +## List all members of a group or project including inherited and invited members -Gets a list of group or project members viewable by the authenticated user, including inherited members and permissions through ancestor groups. +Gets a list of group or project members viewable by the authenticated user, including inherited members, invited users, and permissions through ancestor groups. If a user is a member of this group or project and also of one or more ancestor groups, only its membership with the highest `access_level` is returned. ([Improved](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56677) in GitLab 13.11.) This represents the effective permission of the user. +Members from an invited group are returned if either: + +- The invited group is public. +- The requester is also a member of the invited group. + This function takes pagination parameters `page` and `per_page` to restrict the list of users. ```plaintext @@ -202,11 +207,11 @@ Example response: } ``` -## Get a member of a group or project, including inherited members +## Get a member of a group or project, including inherited and invited members > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/17744) in GitLab 12.4. -Gets a member of a group or project, including members inherited through ancestor groups. See the corresponding [endpoint to list all inherited members](#list-all-members-of-a-group-or-project-including-inherited-members) for details. +Gets a member of a group or project, including members inherited or invited through ancestor groups. See the corresponding [endpoint to list all inherited members](#list-all-members-of-a-group-or-project-including-inherited-and-invited-members) for details. ```plaintext GET /groups/:id/members/all/:user_id diff --git a/doc/development/feature_flags/controls.md b/doc/development/feature_flags/controls.md index 4843b58c3fd..91e27a3838c 100644 --- a/doc/development/feature_flags/controls.md +++ b/doc/development/feature_flags/controls.md @@ -303,6 +303,19 @@ and reduces confidence in our testing suite covering all possible combinations. Additionally, a feature flag overwritten in some of the environments can result in undefined and untested system behavior. +`development` type feature flags should have a short life-cycle because their purpose +is for rolling out a persistent change. `development` feature flags that are older +than 2 milestones are reported to engineering managers. The +[report tool](https://gitlab.com/gitlab-org/gitlab-feature-flag-alert) runs on a +monthly basis. For example, see [the report for December 2021](https://gitlab.com/gitlab-org/quality/triage-reports/-/issues/5480). + +If a `development` feature flag is still present in the codebase after 6 months we should +take one of the following actions: + +- Enable the feature flag by default and remove it. +- Convert it to an instance, group, or project setting. +- Revert the changes if it's still disabled and not needed anymore. + To remove a feature flag, open **one merge request** to make the changes. In the MR: 1. Add the ~"feature flag" label so release managers are aware the changes are hidden behind a feature flag. diff --git a/lib/api/helpers/members_helpers.rb b/lib/api/helpers/members_helpers.rb index c2710be6c03..6c20993431d 100644 --- a/lib/api/helpers/members_helpers.rb +++ b/lib/api/helpers/members_helpers.rb @@ -50,7 +50,7 @@ module API end def find_all_members_for_group(group) - GroupMembersFinder.new(group).execute + GroupMembersFinder.new(group, current_user).execute(include_relations: [:inherited, :direct, :shared_from_groups]) end def present_members(members) diff --git a/lib/api/helpers/rate_limiter.rb b/lib/api/helpers/rate_limiter.rb index 7d87c74097d..0ad4f089907 100644 --- a/lib/api/helpers/rate_limiter.rb +++ b/lib/api/helpers/rate_limiter.rb @@ -10,6 +10,7 @@ module API # See app/controllers/concerns/check_rate_limit.rb for Rails controllers version module RateLimiter def check_rate_limit!(key, scope:, **options) + return if bypass_header_set? return unless rate_limiter.throttled?(key, scope: scope, **options) rate_limiter.log_request(request, "#{key}_request_limit".to_sym, current_user) @@ -24,6 +25,10 @@ module API def rate_limiter ::Gitlab::ApplicationRateLimiter end + + def bypass_header_set? + ::Gitlab::Throttle.bypass_header.present? && request.get_header(Gitlab::Throttle.bypass_header) == '1' + end end end end diff --git a/lib/api/users.rb b/lib/api/users.rb index ce0a0e9b502..efecc7593d0 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -142,11 +142,15 @@ module API get ":id", feature_category: :users do forbidden!('Not authorized!') unless current_user + if Feature.enabled?(:rate_limit_user_by_id_endpoint, type: :development) + check_rate_limit! :users_get_by_id, scope: current_user unless current_user.admin? + end + user = User.find_by(id: params[:id]) not_found!('User') unless user && can?(current_user, :read_user, user) - opts = { with: current_user&.admin? ? Entities::UserDetailsWithAdmin : Entities::User, current_user: current_user } + opts = { with: current_user.admin? ? Entities::UserDetailsWithAdmin : Entities::User, current_user: current_user } user, opts = with_custom_attributes(user, opts) present user, opts diff --git a/lib/gitlab.rb b/lib/gitlab.rb index d93d7acbaad..2449554d3c0 100644 --- a/lib/gitlab.rb +++ b/lib/gitlab.rb @@ -1,10 +1,15 @@ # frozen_string_literal: true require 'pathname' +require 'forwardable' + +require_relative 'gitlab_edition' module Gitlab - def self.root - Pathname.new(File.expand_path('..', __dir__)) + class << self + extend Forwardable + + def_delegators :GitlabEdition, :root, :extensions, :ee?, :ee, :jh?, :jh end def self.version_info @@ -89,47 +94,6 @@ module Gitlab Rails.env.development? || Rails.env.test? end - def self.extensions - if jh? - %w[ee jh] - elsif ee? - %w[ee] - else - %w[] - end - end - - def self.ee? - @is_ee ||= - # We use this method when the Rails environment is not loaded. This - # means that checking the presence of the License class could result in - # this method returning `false`, even for an EE installation. - # - # The `FOSS_ONLY` is always `string` or `nil` - # Thus the nil or empty string will result - # in using default value: false - # - # The behavior needs to be synchronised with - # config/helpers/is_ee_env.js - root.join('ee/app/models/license.rb').exist? && - !%w[true 1].include?(ENV['FOSS_ONLY'].to_s) - end - - def self.jh? - @is_jh ||= - ee? && - root.join('jh').exist? && - !%w[true 1].include?(ENV['EE_ONLY'].to_s) - end - - def self.ee - yield if ee? - end - - def self.jh - yield if jh? - end - def self.http_proxy_env? HTTP_PROXY_ENV_VARS.any? { |name| ENV[name] } end diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb index fb90ad9e275..5125d566524 100644 --- a/lib/gitlab/application_rate_limiter.rb +++ b/lib/gitlab/application_rate_limiter.rb @@ -49,6 +49,7 @@ module Gitlab group_testing_hook: { threshold: 5, interval: 1.minute }, profile_add_new_email: { threshold: 5, interval: 1.minute }, web_hook_calls: { interval: 1.minute }, + users_get_by_id: { threshold: 10, interval: 1.minute }, profile_resend_email_confirmation: { threshold: 5, interval: 1.minute }, update_environment_canary_ingress: { threshold: 1, interval: 1.minute }, auto_rollback_deployment: { threshold: 1, interval: 3.minutes } diff --git a/lib/gitlab/experimentation.rb b/lib/gitlab/experimentation.rb index 4cc653bec43..7edda290204 100644 --- a/lib/gitlab/experimentation.rb +++ b/lib/gitlab/experimentation.rb @@ -30,14 +30,6 @@ module Gitlab module Experimentation EXPERIMENTS = { - remove_known_trial_form_fields_welcoming: { - tracking_category: 'Growth::Conversion::Experiment::RemoveKnownTrialFormFieldsWelcoming', - rollout_strategy: :user - }, - remove_known_trial_form_fields_noneditable: { - tracking_category: 'Growth::Conversion::Experiment::RemoveKnownTrialFormFieldsNoneditable', - rollout_strategy: :user - } }.freeze class << self diff --git a/lib/gitlab/metrics/samplers/action_cable_sampler.rb b/lib/gitlab/metrics/samplers/action_cable_sampler.rb index adce3030d0d..1f50371cae9 100644 --- a/lib/gitlab/metrics/samplers/action_cable_sampler.rb +++ b/lib/gitlab/metrics/samplers/action_cable_sampler.rb @@ -6,8 +6,8 @@ module Gitlab class ActionCableSampler < BaseSampler DEFAULT_SAMPLING_INTERVAL_SECONDS = 5 - def initialize(interval = nil, action_cable: ::ActionCable.server) - super(interval) + def initialize(action_cable: ::ActionCable.server, **options) + super(**options) @action_cable = action_cable end diff --git a/lib/gitlab/metrics/samplers/base_sampler.rb b/lib/gitlab/metrics/samplers/base_sampler.rb index 52d80c3c27e..b2a9de21145 100644 --- a/lib/gitlab/metrics/samplers/base_sampler.rb +++ b/lib/gitlab/metrics/samplers/base_sampler.rb @@ -9,7 +9,10 @@ module Gitlab attr_reader :interval # interval - The sampling interval in seconds. - def initialize(interval = nil) + # warmup - When true, takes a single sample eagerly before entering the sampling loop. + # This can be useful to ensure that all metrics files exist after `start` returns, + # since prometheus-client-mmap creates them lazily upon first access. + def initialize(interval: nil, logger: Logger.new($stdout), warmup: false, **options) interval ||= ENV[interval_env_key]&.to_i interval ||= self.class::DEFAULT_SAMPLING_INTERVAL_SECONDS interval_half = interval.to_f / 2 @@ -17,13 +20,16 @@ module Gitlab @interval = interval @interval_steps = (-interval_half..interval_half).step(0.1).to_a - super() + @logger = logger + @warmup = warmup + + super(**options) end def safe_sample sample rescue StandardError => e - ::Gitlab::AppLogger.warn("#{self.class}: #{e}, stopping") + @logger.warn("#{self.class}: #{e}, stopping") stop end @@ -63,6 +69,8 @@ module Gitlab def start_working @running = true + safe_sample if @warmup + true end diff --git a/lib/gitlab/metrics/samplers/ruby_sampler.rb b/lib/gitlab/metrics/samplers/ruby_sampler.rb index b1c5e9800da..d71ee671b8d 100644 --- a/lib/gitlab/metrics/samplers/ruby_sampler.rb +++ b/lib/gitlab/metrics/samplers/ruby_sampler.rb @@ -7,12 +7,12 @@ module Gitlab DEFAULT_SAMPLING_INTERVAL_SECONDS = 60 GC_REPORT_BUCKETS = [0.01, 0.05, 0.1, 0.2, 0.3, 0.5, 1].freeze - def initialize(*) + def initialize(...) GC::Profiler.clear metrics[:process_start_time_seconds].set(labels, Time.now.to_i) - super + super(...) end def metrics diff --git a/lib/gitlab_edition.rb b/lib/gitlab_edition.rb new file mode 100644 index 00000000000..6eb6b52c357 --- /dev/null +++ b/lib/gitlab_edition.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'pathname' + +module GitlabEdition + def self.root + Pathname.new(File.expand_path('..', __dir__)) + end + + def self.extensions + if jh? + %w[ee jh] + elsif ee? + %w[ee] + else + %w[] + end + end + + def self.ee? + @is_ee ||= + # We use this method when the Rails environment is not loaded. This + # means that checking the presence of the License class could result in + # this method returning `false`, even for an EE installation. + # + # The `FOSS_ONLY` is always `string` or `nil` + # Thus the nil or empty string will result + # in using default value: false + # + # The behavior needs to be synchronised with + # config/helpers/is_ee_env.js + root.join('ee/app/models/license.rb').exist? && + !%w[true 1].include?(ENV['FOSS_ONLY'].to_s) + end + + def self.jh? + @is_jh ||= + ee? && + root.join('jh').exist? && + !%w[true 1].include?(ENV['EE_ONLY'].to_s) + end + + def self.ee + yield if ee? + end + + def self.jh + yield if jh? + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e9656aaa82e..dfae714e26d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15473,6 +15473,9 @@ msgstr "" msgid "Full" msgstr "" +msgid "Full log" +msgstr "" + msgid "Full name" msgstr "" @@ -28537,6 +28540,9 @@ msgstr "" msgid "ProtectedBranch|Branch" msgstr "" +msgid "ProtectedBranch|Branch will be writable for developers. Are you sure?" +msgstr "" + msgid "ProtectedBranch|Branch:" msgstr "" @@ -28582,6 +28588,9 @@ msgstr "" msgid "ProtectedBranch|Toggle code owner approval" msgstr "" +msgid "ProtectedBranch|Unprotect" +msgstr "" + msgid "ProtectedBranch|What are protected branches?" msgstr "" @@ -34658,9 +34667,25 @@ msgid_plural "Terraform|%{number} Terraform reports were generated in your pipel msgstr[0] "" msgstr[1] "" +msgid "Terraform|%{strong_start}%{number}%{strong_end} Terraform report failed to generate" +msgid_plural "Terraform|%{strong_start}%{number}%{strong_end} Terraform reports failed to generate" +msgstr[0] "" +msgstr[1] "" + +msgid "Terraform|%{strong_start}%{number}%{strong_end} Terraform report was generated in your pipelines" +msgid_plural "Terraform|%{strong_start}%{number}%{strong_end} Terraform reports were generated in your pipelines" +msgstr[0] "" +msgstr[1] "" + msgid "Terraform|%{user} updated %{timeAgo}" msgstr "" +msgid "Terraform|A Terraform report failed to generate." +msgstr "" + +msgid "Terraform|A Terraform report was generated in your pipelines." +msgstr "" + msgid "Terraform|A report failed to generate." msgstr "" @@ -34691,6 +34716,9 @@ msgstr "" msgid "Terraform|Download JSON" msgstr "" +msgid "Terraform|Failed to load Terraform reports" +msgstr "" + msgid "Terraform|Generating the report caused an error." msgstr "" @@ -34703,6 +34731,9 @@ msgstr "" msgid "Terraform|Job status" msgstr "" +msgid "Terraform|Loading Terraform reports..." +msgstr "" + msgid "Terraform|Lock" msgstr "" @@ -34739,12 +34770,21 @@ msgstr "" msgid "Terraform|Terraform init command" msgstr "" +msgid "Terraform|Terraform reports" +msgstr "" + msgid "Terraform|The job %{name} failed to generate a report." msgstr "" msgid "Terraform|The job %{name} generated a report." msgstr "" +msgid "Terraform|The job %{strong_start}%{name}%{strong_end} failed to generate a report." +msgstr "" + +msgid "Terraform|The job %{strong_start}%{name}%{strong_end} generated a report." +msgstr "" + msgid "Terraform|To get access to this terraform state from your local computer, run the following command at the command line. The first line requires a personal access token with API read and write access. %{linkStart}How do I create a personal access token?%{linkEnd}." msgstr "" @@ -37243,9 +37283,6 @@ msgstr "" msgid "Trial|GitLab Ultimate trial (optional)" msgstr "" -msgid "Trial|Hi%{salutation}, your GitLab Ultimate trial lasts for 30 days, but you can keep your free GitLab account forever. We just need some additional information about %{company} to activate your trial." -msgstr "" - msgid "Trial|How many employees will use Gitlab?" msgstr "" @@ -37276,9 +37313,6 @@ msgstr "" msgid "Trial|Your GitLab Ultimate trial lasts for 30 days, but you can keep your free GitLab account forever. We just need some additional information to activate your trial." msgstr "" -msgid "Trial|your company" -msgstr "" - msgid "Trigger" msgstr "" diff --git a/metrics_server/dependencies.rb b/metrics_server/dependencies.rb index a459efef1ad..494b795b292 100644 --- a/metrics_server/dependencies.rb +++ b/metrics_server/dependencies.rb @@ -6,6 +6,7 @@ require 'fileutils' require 'active_support/concern' require 'active_support/inflector' +require 'active_support/core_ext/numeric/bytes' require 'prometheus/client' require 'rack' @@ -18,6 +19,9 @@ require_relative '../lib/gitlab/utils/strong_memoize' require_relative '../lib/prometheus/cleanup_multiproc_dir_service' require_relative '../lib/gitlab/metrics/prometheus' require_relative '../lib/gitlab/metrics' +require_relative '../lib/gitlab/metrics/system' +require_relative '../lib/gitlab/metrics/samplers/base_sampler' +require_relative '../lib/gitlab/metrics/samplers/ruby_sampler' require_relative '../lib/gitlab/metrics/exporter/base_exporter' require_relative '../lib/gitlab/metrics/exporter/sidekiq_exporter' require_relative '../lib/gitlab/health_checks/probes/collection' diff --git a/metrics_server/metrics_server.rb b/metrics_server/metrics_server.rb index 56fc20dcc9d..33b31326d2a 100644 --- a/metrics_server/metrics_server.rb +++ b/metrics_server/metrics_server.rb @@ -40,14 +40,20 @@ class MetricsServer # rubocop:disable Gitlab/NamespacedClass def start ::Prometheus::Client.configure do |config| config.multiprocess_files_dir = @metrics_dir + config.pid_provider = proc { "#{@target}_exporter" } end FileUtils.mkdir_p(@metrics_dir, mode: 0700) ::Prometheus::CleanupMultiprocDirService.new.execute if @wipe_metrics_dir - settings = Settings.new(Settings.monitoring[name]) + # We need to `warmup: true` since otherwise the sampler and exporter threads enter + # a race where not all Prometheus db files will be visible to the exporter, resulting + # in missing metrics. + # Warming up ensures that these files exist prior to the exporter starting up. + Gitlab::Metrics::Samplers::RubySampler.initialize_instance(warmup: true).start exporter_class = "Gitlab::Metrics::Exporter::#{@target.camelize}Exporter".constantize + settings = Settings.new(Settings.monitoring[name]) server = exporter_class.instance(settings, synchronous: true) server.start diff --git a/metrics_server/override_gitlab_current_settings.rb b/metrics_server/override_gitlab_current_settings.rb new file mode 100644 index 00000000000..1dc19b5da23 --- /dev/null +++ b/metrics_server/override_gitlab_current_settings.rb @@ -0,0 +1,21 @@ +# rubocop:disable Naming/FileName +# frozen_string_literal: true + +# We need to supply this outside of Rails because: +# RubySampler needs Gitlab::Metrics needs Gitlab::Metrics::Prometheus needs Gitlab::CurrentSettings needs ::Settings +# to check for `prometheus_metrics_enabled`. We therefore simply redirect it to our own Settings type. +module Gitlab + module CurrentSettings + class << self + def prometheus_metrics_enabled + # We make the simplified assumption that when the metrics-server runs, + # Prometheus metrics are enabled. Since the latter is a setting stored + # in the application database, we have no access to it here, so we need + # to hard-code it. + true + end + end + end +end + +# rubocop:enable Naming/FileName diff --git a/metrics_server/settings_overrides.rb b/metrics_server/settings_overrides.rb index 8572b4f86b0..b3fd39229d5 100644 --- a/metrics_server/settings_overrides.rb +++ b/metrics_server/settings_overrides.rb @@ -9,6 +9,11 @@ # Here we make the necessary constants available conditionally. require_relative 'override_rails_constants' unless Object.const_defined?('Rails') +# We need to supply this outside of Rails because: +# RubySampler needs Gitlab::Metrics needs Gitlab::Metrics::Prometheus needs Gitlab::CurrentSettings needs ::Settings +# to check for `prometheus_metrics_enabled`. We therefore simply redirect it to our own Settings type. +require_relative 'override_gitlab_current_settings' unless Object.const_defined?('Gitlab::CurrentSettings') + require_relative '../config/settings' # rubocop:enable Naming/FileName diff --git a/rubocop/code_reuse_helpers.rb b/rubocop/code_reuse_helpers.rb index 6ea12999cae..45cfa7ba78d 100644 --- a/rubocop/code_reuse_helpers.rb +++ b/rubocop/code_reuse_helpers.rb @@ -1,7 +1,15 @@ # frozen_string_literal: true +require 'forwardable' + +require_relative '../lib/gitlab_edition' + module RuboCop module CodeReuseHelpers + extend Forwardable + + def_delegators :GitlabEdition, :ee?, :jh? + # Returns true for a `(send const ...)` node. def send_to_constant?(node) node.type == :send && node.children&.first&.type == :const @@ -180,13 +188,5 @@ module RuboCop def rails_root File.expand_path('..', __dir__) end - - def ee? - File.exist?(File.expand_path('../ee/app/models/license.rb', __dir__)) && !%w[true 1].include?(ENV['FOSS_ONLY'].to_s) - end - - def jh? - ee? && Dir.exist?(File.expand_path('../jh', __dir__)) && !%w[true 1].include?(ENV['EE_ONLY'].to_s) - end end end diff --git a/scripts/used-feature-flags b/scripts/used-feature-flags index 552adbfbd9f..89ea99c6984 100755 --- a/scripts/used-feature-flags +++ b/scripts/used-feature-flags @@ -3,7 +3,7 @@ require 'set' require 'fileutils' -require_relative 'lib/gitlab' +require_relative '../lib/gitlab_edition' class String def red @@ -28,7 +28,7 @@ flags_paths = [ ] # For EE additionally process `ee/` feature flags -if Gitlab.ee? +if GitlabEdition.ee? flags_paths << 'ee/config/feature_flags/**/*.yml' # Geo feature flags are constructed dynamically and there's no explicit checks in the codebase so we mark all @@ -43,7 +43,7 @@ if Gitlab.ee? end # For JH additionally process `jh/` feature flags -if Gitlab.jh? +if GitlabEdition.jh? flags_paths << 'jh/config/feature_flags/**/*.yml' Dir.glob('jh/app/replicators/geo/*_replicator.rb').each_with_object(Set.new) do |path, memo| diff --git a/spec/controllers/concerns/check_rate_limit_spec.rb b/spec/controllers/concerns/check_rate_limit_spec.rb new file mode 100644 index 00000000000..34ececfe639 --- /dev/null +++ b/spec/controllers/concerns/check_rate_limit_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe CheckRateLimit do + let(:key) { :some_key } + let(:scope) { [:some, :scope] } + let(:request) { instance_double('Rack::Request') } + let(:user) { build_stubbed(:user) } + + let(:controller_class) do + Class.new do + include CheckRateLimit + + attr_reader :request, :current_user + + def initialize(request, current_user) + @request = request + @current_user = current_user + end + + def redirect_back_or_default(**args) + end + + def render(**args) + end + end + end + + subject { controller_class.new(request, user) } + + before do + allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?) + allow(::Gitlab::ApplicationRateLimiter).to receive(:log_request) + end + + describe '#check_rate_limit!' do + it 'calls ApplicationRateLimiter#throttled? with the right arguments' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(key, scope: scope).and_return(false) + expect(subject).not_to receive(:render) + + subject.check_rate_limit!(key, scope: scope) + end + + it 'renders error and logs request if throttled' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(key, scope: scope).and_return(true) + expect(::Gitlab::ApplicationRateLimiter).to receive(:log_request).with(request, "#{key}_request_limit".to_sym, user) + expect(subject).to receive(:render).with({ plain: _('This endpoint has been requested too many times. Try again later.'), status: :too_many_requests }) + + subject.check_rate_limit!(key, scope: scope) + end + + it 'redirects back if throttled and redirect_back option is set to true' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(key, scope: scope).and_return(true) + expect(::Gitlab::ApplicationRateLimiter).to receive(:log_request).with(request, "#{key}_request_limit".to_sym, user) + expect(subject).not_to receive(:render) + expect(subject).to receive(:redirect_back_or_default).with(options: { alert: _('This endpoint has been requested too many times. Try again later.') }) + + subject.check_rate_limit!(key, scope: scope, redirect_back: true) + end + + context 'when the bypass header is set' do + before do + allow(Gitlab::Throttle).to receive(:bypass_header).and_return('SOME_HEADER') + end + + it 'skips rate limit if set to "1"' do + allow(request).to receive(:get_header).with(Gitlab::Throttle.bypass_header).and_return('1') + + expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?) + expect(subject).not_to receive(:render) + + subject.check_rate_limit!(key, scope: scope) + end + + it 'does not skip rate limit if set to something else than "1"' do + allow(request).to receive(:get_header).with(Gitlab::Throttle.bypass_header).and_return('0') + + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?) + + subject.check_rate_limit!(key, scope: scope) + end + end + end +end diff --git a/spec/finders/group_members_finder_spec.rb b/spec/finders/group_members_finder_spec.rb index 0d797b7923c..a9a8e9d19b8 100644 --- a/spec/finders/group_members_finder_spec.rb +++ b/spec/finders/group_members_finder_spec.rb @@ -3,83 +3,93 @@ require 'spec_helper' RSpec.describe GroupMembersFinder, '#execute' do - let(:group) { create(:group) } - let(:sub_group) { create(:group, parent: group) } - let(:sub_sub_group) { create(:group, parent: sub_group) } - let(:user1) { create(:user) } - let(:user2) { create(:user) } - let(:user3) { create(:user) } - let(:user4) { create(:user) } - let(:user5) { create(:user, :two_factor_via_otp) } + let_it_be(:group) { create(:group) } + let_it_be(:sub_group) { create(:group, parent: group) } + let_it_be(:sub_sub_group) { create(:group, parent: sub_group) } + let_it_be(:public_shared_group) { create(:group, :public) } + let_it_be(:private_shared_group) { create(:group, :private) } + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:user3) { create(:user) } + let_it_be(:user4) { create(:user) } + let_it_be(:user5) { create(:user, :two_factor_via_otp) } + + let!(:link) do + create(:group_group_link, shared_group: group, shared_with_group: public_shared_group) + create(:group_group_link, shared_group: sub_group, shared_with_group: private_shared_group) + end let(:groups) do { - group: group, - sub_group: sub_group, - sub_sub_group: sub_sub_group + group: group, + sub_group: sub_group, + sub_sub_group: sub_sub_group, + public_shared_group: public_shared_group, + private_shared_group: private_shared_group } end context 'relations' do let!(:members) do { - user1_sub_sub_group: create(:group_member, :maintainer, group: sub_sub_group, user: user1), - user1_sub_group: create(:group_member, :developer, group: sub_group, user: user1), - user1_group: create(:group_member, :reporter, group: group, user: user1), - user2_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user2), - user2_sub_group: create(:group_member, :developer, group: sub_group, user: user2), - user2_group: create(:group_member, :maintainer, group: group, user: user2), - user3_sub_sub_group: create(:group_member, :developer, group: sub_sub_group, user: user3, expires_at: 1.day.from_now), - user3_sub_group: create(:group_member, :developer, group: sub_group, user: user3, expires_at: 2.days.from_now), - user3_group: create(:group_member, :reporter, group: group, user: user3), - user4_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user4), - user4_sub_group: create(:group_member, :developer, group: sub_group, user: user4, expires_at: 1.day.from_now), - user4_group: create(:group_member, :developer, group: group, user: user4, expires_at: 2.days.from_now) + user1_sub_sub_group: create(:group_member, :maintainer, group: sub_sub_group, user: user1), + user1_sub_group: create(:group_member, :developer, group: sub_group, user: user1), + user1_group: create(:group_member, :reporter, group: group, user: user1), + user1_public_shared_group: create(:group_member, :maintainer, group: public_shared_group, user: user1), + user1_private_shared_group: create(:group_member, :maintainer, group: private_shared_group, user: user1), + user2_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user2), + user2_sub_group: create(:group_member, :developer, group: sub_group, user: user2), + user2_group: create(:group_member, :maintainer, group: group, user: user2), + user2_public_shared_group: create(:group_member, :developer, group: public_shared_group, user: user2), + user2_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user2), + user3_sub_sub_group: create(:group_member, :developer, group: sub_sub_group, user: user3, expires_at: 1.day.from_now), + user3_sub_group: create(:group_member, :developer, group: sub_group, user: user3, expires_at: 2.days.from_now), + user3_group: create(:group_member, :reporter, group: group, user: user3), + user3_public_shared_group: create(:group_member, :reporter, group: public_shared_group, user: user3), + user3_private_shared_group: create(:group_member, :reporter, group: private_shared_group, user: user3), + user4_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user4), + user4_sub_group: create(:group_member, :developer, group: sub_group, user: user4, expires_at: 1.day.from_now), + user4_group: create(:group_member, :developer, group: group, user: user4, expires_at: 2.days.from_now), + user4_public_shared_group: create(:group_member, :developer, group: public_shared_group, user: user4), + user4_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user4) } end it 'raises an error if a non-supported relation type is used' do expect do described_class.new(group).execute(include_relations: [:direct, :invalid_relation_type]) - end.to raise_error(ArgumentError, "invalid_relation_type is not a valid relation type. Valid relation types are direct, inherited, descendants.") + end.to raise_error(ArgumentError, "invalid_relation_type is not a valid relation type. Valid relation types are direct, inherited, descendants, shared_from_groups.") end using RSpec::Parameterized::TableSyntax where(:subject_relations, :subject_group, :expected_members) do - nil | :group | [:user1_group, :user2_group, :user3_group, :user4_group] - [:direct] | :group | [:user1_group, :user2_group, :user3_group, :user4_group] - [:inherited] | :group | [] - [:descendants] | :group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] - [:direct, :inherited] | :group | [:user1_group, :user2_group, :user3_group, :user4_group] - [:direct, :descendants] | :group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:descendants, :inherited] | :group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] - [:direct, :descendants, :inherited] | :group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] - nil | :sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct] | :sub_group | [:user1_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] - [:inherited] | :sub_group | [:user1_group, :user2_group, :user3_group, :user4_group] - [:descendants] | :sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] - [:direct, :inherited] | :sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct, :descendants] | :sub_group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] - [:descendants, :inherited] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_sub_group, :user4_group] - [:direct, :descendants, :inherited] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] - nil | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct] | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] - [:inherited] | :sub_sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:descendants] | :sub_sub_group | [] - [:direct, :inherited] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct, :descendants] | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] - [:descendants, :inherited] | :sub_sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct, :descendants, :inherited] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] + [] | :group | [] + GroupMembersFinder::DEFAULT_RELATIONS | :group | [:user1_group, :user2_group, :user3_group, :user4_group] + [:direct] | :group | [:user1_group, :user2_group, :user3_group, :user4_group] + [:inherited] | :group | [] + [:descendants] | :group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] + [:shared_from_groups] | :group | [:user1_public_shared_group, :user2_public_shared_group, :user3_public_shared_group, :user4_public_shared_group] + [:direct, :inherited, :descendants, :shared_from_groups] | :group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_public_shared_group] + [] | :sub_group | [] + GroupMembersFinder::DEFAULT_RELATIONS | :sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] + [:direct] | :sub_group | [:user1_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] + [:inherited] | :sub_group | [:user1_group, :user2_group, :user3_group, :user4_group] + [:descendants] | :sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] + [:shared_from_groups] | :sub_group | [] + [:direct, :inherited, :descendants, :shared_from_groups] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] + [] | :sub_sub_group | [] + GroupMembersFinder::DEFAULT_RELATIONS | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] + [:direct] | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] + [:inherited] | :sub_sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] + [:descendants] | :sub_sub_group | [] + [:shared_from_groups] | :sub_sub_group | [] + [:direct, :inherited, :descendants, :shared_from_groups] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] end with_them do it 'returns correct members' do - result = if subject_relations - described_class.new(groups[subject_group]).execute(include_relations: subject_relations) - else - described_class.new(groups[subject_group]).execute - end + result = described_class.new(groups[subject_group]).execute(include_relations: subject_relations) expect(result.to_a).to match_array(expected_members.map { |name| members[name] }) end diff --git a/spec/frontend/vue_mr_widget/components/terraform/mock_data.js b/spec/frontend/vue_mr_widget/components/terraform/mock_data.js index ae280146c22..8e46af5dfd6 100644 --- a/spec/frontend/vue_mr_widget/components/terraform/mock_data.js +++ b/spec/frontend/vue_mr_widget/components/terraform/mock_data.js @@ -1,6 +1,6 @@ export const invalidPlanWithName = { job_name: 'Invalid Plan', - job_path: '/path/to/ci/logs/1', + job_path: '/path/to/ci/logs/3', tf_report_error: 'api_error', }; @@ -20,12 +20,12 @@ export const validPlanWithoutName = { create: 10, update: 20, delete: 30, - job_path: '/path/to/ci/logs/1', + job_path: '/path/to/ci/logs/2', }; export const plans = { invalid_plan_one: invalidPlanWithName, - invalid_plan_two: invalidPlanWithName, + invalid_plan_two: invalidPlanWithoutName, valid_plan_one: validPlanWithName, valid_plan_two: validPlanWithoutName, }; diff --git a/spec/frontend/vue_mr_widget/extentions/terraform/index_spec.js b/spec/frontend/vue_mr_widget/extentions/terraform/index_spec.js new file mode 100644 index 00000000000..f8ea6fc23a2 --- /dev/null +++ b/spec/frontend/vue_mr_widget/extentions/terraform/index_spec.js @@ -0,0 +1,178 @@ +import MockAdapter from 'axios-mock-adapter'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import axios from '~/lib/utils/axios_utils'; +import Poll from '~/lib/utils/poll'; +import extensionsContainer from '~/vue_merge_request_widget/components/extensions/container'; +import { registerExtension } from '~/vue_merge_request_widget/components/extensions'; +import terraformExtension from '~/vue_merge_request_widget/extensions/terraform'; +import { + plans, + validPlanWithName, + validPlanWithoutName, + invalidPlanWithName, + invalidPlanWithoutName, +} from '../../components/terraform/mock_data'; + +describe('Terraform extension', () => { + let wrapper; + let mock; + + const endpoint = '/path/to/terraform/report.json'; + const successStatusCode = 200; + const errorStatusCode = 500; + + const findListItem = (at) => wrapper.findAllByTestId('extension-list-item').at(at); + + registerExtension(terraformExtension); + + const mockPollingApi = (response, body, header) => { + mock.onGet(endpoint).reply(response, body, header); + }; + + const createComponent = () => { + wrapper = mountExtended(extensionsContainer, { + propsData: { + mr: { + terraformReportsPath: endpoint, + }, + }, + }); + return axios.waitForAll(); + }; + + beforeEach(() => { + mock = new MockAdapter(axios); + }); + + afterEach(() => { + wrapper.destroy(); + mock.restore(); + }); + + describe('summary', () => { + describe('while loading', () => { + const loadingText = 'Loading Terraform reports...'; + it('should render loading text', async () => { + mockPollingApi(successStatusCode, plans, {}); + createComponent(); + + expect(wrapper.text()).toContain(loadingText); + await waitForPromises(); + expect(wrapper.text()).not.toContain(loadingText); + }); + }); + + describe('when the fetching fails', () => { + beforeEach(() => { + mockPollingApi(errorStatusCode, null, {}); + return createComponent(); + }); + + it('should generate one invalid plan and render correct summary text', () => { + expect(wrapper.text()).toContain('1 Terraform report failed to generate'); + }); + }); + + describe('when the fetching succeeds', () => { + describe.each` + responseType | response | summaryTitle | summarySubtitle + ${'1 invalid report'} | ${{ 0: invalidPlanWithName }} | ${'1 Terraform report failed to generate'} | ${''} + ${'2 valid reports'} | ${{ 0: validPlanWithName, 1: validPlanWithName }} | ${'2 Terraform reports were generated in your pipelines'} | ${''} + ${'1 valid and 2 invalid reports'} | ${{ 0: validPlanWithName, 1: invalidPlanWithName, 2: invalidPlanWithName }} | ${'Terraform report was generated in your pipelines'} | ${'2 Terraform reports failed to generate'} + `('and received $responseType', ({ response, summaryTitle, summarySubtitle }) => { + beforeEach(async () => { + mockPollingApi(successStatusCode, response, {}); + return createComponent(); + }); + + it(`should render correct summary text`, () => { + expect(wrapper.text()).toContain(summaryTitle); + + if (summarySubtitle) { + expect(wrapper.text()).toContain(summarySubtitle); + } + }); + }); + }); + }); + + describe('expanded data', () => { + beforeEach(async () => { + mockPollingApi(successStatusCode, plans, {}); + await createComponent(); + + wrapper.findByTestId('toggle-button').trigger('click'); + }); + + describe.each` + reportType | title | subtitle | logLink | lineNumber + ${'a valid report with name'} | ${`The job ${validPlanWithName.job_name} generated a report.`} | ${`Reported Resource Changes: ${validPlanWithName.create} to add, ${validPlanWithName.update} to change, ${validPlanWithName.delete} to delete`} | ${validPlanWithName.job_path} | ${0} + ${'a valid report without name'} | ${'A Terraform report was generated in your pipelines.'} | ${`Reported Resource Changes: ${validPlanWithoutName.create} to add, ${validPlanWithoutName.update} to change, ${validPlanWithoutName.delete} to delete`} | ${validPlanWithoutName.job_path} | ${1} + ${'an invalid report with name'} | ${`The job ${invalidPlanWithName.job_name} failed to generate a report.`} | ${'Generating the report caused an error.'} | ${invalidPlanWithName.job_path} | ${2} + ${'an invalid report without name'} | ${'A Terraform report failed to generate.'} | ${'Generating the report caused an error.'} | ${invalidPlanWithoutName.job_path} | ${3} + `('renders correct text for $reportType', ({ title, subtitle, logLink, lineNumber }) => { + it('renders correct text', () => { + expect(findListItem(lineNumber).text()).toContain(title); + expect(findListItem(lineNumber).text()).toContain(subtitle); + }); + + it(`${logLink ? 'renders' : "doesn't render"} the log link`, () => { + const logText = 'Full log'; + if (logLink) { + expect( + findListItem(lineNumber) + .find('[data-testid="extension-actions-button"]') + .attributes('href'), + ).toBe(logLink); + } else { + expect(findListItem(lineNumber).text()).not.toContain(logText); + } + }); + }); + }); + + describe('polling', () => { + let pollRequest; + let pollStop; + + beforeEach(() => { + pollRequest = jest.spyOn(Poll.prototype, 'makeRequest'); + pollStop = jest.spyOn(Poll.prototype, 'stop'); + }); + + afterEach(() => { + pollRequest.mockRestore(); + pollStop.mockRestore(); + }); + + describe('successful poll', () => { + beforeEach(() => { + mockPollingApi(successStatusCode, plans, {}); + + return createComponent(); + }); + + it('does not make additional requests after poll is successful', () => { + expect(pollRequest).toHaveBeenCalledTimes(1); + expect(pollStop).toHaveBeenCalledTimes(1); + }); + }); + + describe('polling fails', () => { + beforeEach(() => { + mockPollingApi(errorStatusCode, null, {}); + return createComponent(); + }); + + it('generates one broken plan', () => { + expect(wrapper.text()).toContain('1 Terraform report failed to generate'); + }); + + it('does not make additional requests after poll is unsuccessful', () => { + expect(pollRequest).toHaveBeenCalledTimes(1); + expect(pollStop).toHaveBeenCalledTimes(1); + }); + }); + }); +}); diff --git a/spec/graphql/types/group_member_relation_enum_spec.rb b/spec/graphql/types/group_member_relation_enum_spec.rb index 315809ef75e..89ee8c574c4 100644 --- a/spec/graphql/types/group_member_relation_enum_spec.rb +++ b/spec/graphql/types/group_member_relation_enum_spec.rb @@ -6,6 +6,6 @@ RSpec.describe Types::GroupMemberRelationEnum do specify { expect(described_class.graphql_name).to eq('GroupMemberRelation') } it 'exposes all the existing group member relation type values' do - expect(described_class.values.keys).to contain_exactly('DIRECT', 'INHERITED', 'DESCENDANTS') + expect(described_class.values.keys).to contain_exactly('DIRECT', 'INHERITED', 'DESCENDANTS', 'SHARED_FROM_GROUPS') end end diff --git a/spec/lib/api/helpers/rate_limiter_spec.rb b/spec/lib/api/helpers/rate_limiter_spec.rb new file mode 100644 index 00000000000..2fed1cf3604 --- /dev/null +++ b/spec/lib/api/helpers/rate_limiter_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Helpers::RateLimiter do + let(:key) { :some_key } + let(:scope) { [:some, :scope] } + let(:request) { instance_double('Rack::Request') } + let(:user) { build_stubbed(:user) } + + let(:api_class) do + Class.new do + include API::Helpers::RateLimiter + + attr_reader :request, :current_user + + def initialize(request, current_user) + @request = request + @current_user = current_user + end + + def render_api_error!(**args) + end + end + end + + subject { api_class.new(request, user) } + + before do + allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?) + allow(::Gitlab::ApplicationRateLimiter).to receive(:log_request) + end + + describe '#check_rate_limit!' do + it 'calls ApplicationRateLimiter#throttled? with the right arguments' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(key, scope: scope).and_return(false) + expect(subject).not_to receive(:render_api_error!) + + subject.check_rate_limit!(key, scope: scope) + end + + it 'renders api error and logs request if throttled' do + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(key, scope: scope).and_return(true) + expect(::Gitlab::ApplicationRateLimiter).to receive(:log_request).with(request, "#{key}_request_limit".to_sym, user) + expect(subject).to receive(:render_api_error!).with({ error: _('This endpoint has been requested too many times. Try again later.') }, 429) + + subject.check_rate_limit!(key, scope: scope) + end + + context 'when the bypass header is set' do + before do + allow(Gitlab::Throttle).to receive(:bypass_header).and_return('SOME_HEADER') + end + + it 'skips rate limit if set to "1"' do + allow(request).to receive(:get_header).with(Gitlab::Throttle.bypass_header).and_return('1') + + expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?) + expect(subject).not_to receive(:render_api_error!) + + subject.check_rate_limit!(key, scope: scope) + end + + it 'does not skip rate limit if set to something else than "1"' do + allow(request).to receive(:get_header).with(Gitlab::Throttle.bypass_header).and_return('0') + + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?) + + subject.check_rate_limit!(key, scope: scope) + end + end + end +end diff --git a/spec/lib/gitlab/metrics/samplers/action_cable_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/action_cable_sampler_spec.rb index d834b796179..e1e4877cd50 100644 --- a/spec/lib/gitlab/metrics/samplers/action_cable_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/action_cable_sampler_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Gitlab::Metrics::Samplers::ActionCableSampler do let(:action_cable) { instance_double(ActionCable::Server::Base) } - subject { described_class.new(action_cable: action_cable) } + subject { described_class.new(action_cable: action_cable, logger: double) } it_behaves_like 'metrics sampler', 'ACTION_CABLE_SAMPLER' diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index 6f1e0480197..a4877208bcf 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -84,7 +84,7 @@ RSpec.describe Gitlab::Metrics::Samplers::RubySampler do end describe '#sample_gc' do - let!(:sampler) { described_class.new(5) } + let!(:sampler) { described_class.new } let(:gc_reports) { [{ GC_TIME: 0.1 }, { GC_TIME: 0.2 }, { GC_TIME: 0.3 }] } diff --git a/spec/lib/gitlab_edition_spec.rb b/spec/lib/gitlab_edition_spec.rb new file mode 100644 index 00000000000..2f1316819ec --- /dev/null +++ b/spec/lib/gitlab_edition_spec.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabEdition do + before do + # Make sure the ENV is clean + stub_env('FOSS_ONLY', nil) + stub_env('EE_ONLY', nil) + + described_class.instance_variable_set(:@is_ee, nil) + described_class.instance_variable_set(:@is_jh, nil) + end + + after do + described_class.instance_variable_set(:@is_ee, nil) + described_class.instance_variable_set(:@is_jh, nil) + end + + describe '.root' do + it 'returns the root path of the app' do + expect(described_class.root).to eq(Pathname.new(File.expand_path('../..', __dir__))) + end + end + + describe 'extensions' do + context 'when .jh? is true' do + before do + allow(described_class).to receive(:jh?).and_return(true) + end + + it 'returns %w[ee jh]' do + expect(described_class.extensions).to match_array(%w[ee jh]) + end + end + + context 'when .ee? is true' do + before do + allow(described_class).to receive(:jh?).and_return(false) + allow(described_class).to receive(:ee?).and_return(true) + end + + it 'returns %w[ee]' do + expect(described_class.extensions).to match_array(%w[ee]) + end + end + + context 'when neither .jh? and .ee? are true' do + before do + allow(described_class).to receive(:jh?).and_return(false) + allow(described_class).to receive(:ee?).and_return(false) + end + + it 'returns the exyensions according to the current edition' do + expect(described_class.extensions).to be_empty + end + end + end + + describe '.ee? and .jh?' do + def stub_path(*paths, **arguments) + root = Pathname.new('dummy') + pathname = double(:path, **arguments) + + allow(described_class) + .to receive(:root) + .and_return(root) + + allow(root).to receive(:join) + + paths.each do |path| + allow(root) + .to receive(:join) + .with(path) + .and_return(pathname) + end + end + + describe '.ee?' do + context 'for EE' do + before do + stub_path('ee/app/models/license.rb', exist?: true) + end + + context 'when using FOSS_ONLY=1' do + before do + stub_env('FOSS_ONLY', '1') + end + + it 'returns not to be EE' do + expect(described_class).not_to be_ee + end + end + + context 'when using FOSS_ONLY=0' do + before do + stub_env('FOSS_ONLY', '0') + end + + it 'returns to be EE' do + expect(described_class).to be_ee + end + end + + context 'when using default FOSS_ONLY' do + it 'returns to be EE' do + expect(described_class).to be_ee + end + end + end + + context 'for CE' do + before do + stub_path('ee/app/models/license.rb', exist?: false) + end + + it 'returns not to be EE' do + expect(described_class).not_to be_ee + end + end + end + + describe '.jh?' do + context 'for JH' do + before do + stub_path( + 'ee/app/models/license.rb', + 'jh', + exist?: true) + end + + context 'when using default FOSS_ONLY and EE_ONLY' do + it 'returns to be JH' do + expect(described_class).to be_jh + end + end + + context 'when using FOSS_ONLY=1' do + before do + stub_env('FOSS_ONLY', '1') + end + + it 'returns not to be JH' do + expect(described_class).not_to be_jh + end + end + + context 'when using EE_ONLY=1' do + before do + stub_env('EE_ONLY', '1') + end + + it 'returns not to be JH' do + expect(described_class).not_to be_jh + end + end + end + end + end +end diff --git a/spec/lib/gitlab_spec.rb b/spec/lib/gitlab_spec.rb index 869eaf26772..49ba4debe31 100644 --- a/spec/lib/gitlab_spec.rb +++ b/spec/lib/gitlab_spec.rb @@ -3,9 +3,19 @@ require 'spec_helper' RSpec.describe Gitlab do - describe '.root' do - it 'returns the root path of the app' do - expect(described_class.root).to eq(Pathname.new(File.expand_path('../..', __dir__))) + %w[root extensions ee? jh?].each do |method_name| + it "delegates #{method_name} to GitlabEdition" do + expect(GitlabEdition).to receive(method_name) + + described_class.public_send(method_name) + end + end + + %w[ee jh].each do |method_name| + it "delegates #{method_name} to GitlabEdition" do + expect(GitlabEdition).to receive(method_name) + + described_class.public_send(method_name) {} end end @@ -248,121 +258,6 @@ RSpec.describe Gitlab do end end - describe 'ee? and jh?' do - before do - # Make sure the ENV is clean - stub_env('FOSS_ONLY', nil) - stub_env('EE_ONLY', nil) - - described_class.instance_variable_set(:@is_ee, nil) - described_class.instance_variable_set(:@is_jh, nil) - end - - after do - described_class.instance_variable_set(:@is_ee, nil) - described_class.instance_variable_set(:@is_jh, nil) - end - - def stub_path(*paths, **arguments) - root = Pathname.new('dummy') - pathname = double(:path, **arguments) - - allow(described_class) - .to receive(:root) - .and_return(root) - - allow(root).to receive(:join) - - paths.each do |path| - allow(root) - .to receive(:join) - .with(path) - .and_return(pathname) - end - end - - describe '.ee?' do - context 'for EE' do - before do - stub_path('ee/app/models/license.rb', exist?: true) - end - - context 'when using FOSS_ONLY=1' do - before do - stub_env('FOSS_ONLY', '1') - end - - it 'returns not to be EE' do - expect(described_class).not_to be_ee - end - end - - context 'when using FOSS_ONLY=0' do - before do - stub_env('FOSS_ONLY', '0') - end - - it 'returns to be EE' do - expect(described_class).to be_ee - end - end - - context 'when using default FOSS_ONLY' do - it 'returns to be EE' do - expect(described_class).to be_ee - end - end - end - - context 'for CE' do - before do - stub_path('ee/app/models/license.rb', exist?: false) - end - - it 'returns not to be EE' do - expect(described_class).not_to be_ee - end - end - end - - describe '.jh?' do - context 'for JH' do - before do - stub_path( - 'ee/app/models/license.rb', - 'jh', - exist?: true) - end - - context 'when using default FOSS_ONLY and EE_ONLY' do - it 'returns to be JH' do - expect(described_class).to be_jh - end - end - - context 'when using FOSS_ONLY=1' do - before do - stub_env('FOSS_ONLY', '1') - end - - it 'returns not to be JH' do - expect(described_class).not_to be_jh - end - end - - context 'when using EE_ONLY=1' do - before do - stub_env('EE_ONLY', '1') - end - - it 'returns not to be JH' do - expect(described_class).not_to be_jh - end - end - end - end - end - describe '.http_proxy_env?' do it 'returns true when lower case https' do stub_env('https_proxy', 'https://my.proxy') diff --git a/spec/lib/sidebars/projects/panel_spec.rb b/spec/lib/sidebars/projects/panel_spec.rb index 2e79ced7039..7e69a2dfe52 100644 --- a/spec/lib/sidebars/projects/panel_spec.rb +++ b/spec/lib/sidebars/projects/panel_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Sidebars::Projects::Panel do - let(:project) { build(:project) } + let_it_be(:project) { create(:project) } + let(:context) { Sidebars::Projects::Context.new(current_user: nil, container: project) } subject { described_class.new(context) } diff --git a/spec/metrics_server/metrics_server_spec.rb b/spec/metrics_server/metrics_server_spec.rb index 4e3c6900875..4f0d5bc5bb2 100644 --- a/spec/metrics_server/metrics_server_spec.rb +++ b/spec/metrics_server/metrics_server_spec.rb @@ -8,18 +8,24 @@ require_relative '../support/helpers/next_instance_of' RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath include NextInstanceOf + let(:metrics_dir) { Dir.mktmpdir } + before do # We do not want this to have knock-on effects on the test process. allow(Gitlab::ProcessManagement).to receive(:modify_signals) end + after do + FileUtils.rm_rf(metrics_dir, secure: true) + end + describe '.spawn' do context 'when in parent process' do it 'forks into a new process and detaches it' do expect(Process).to receive(:fork).and_return(99) expect(Process).to receive(:detach).with(99) - described_class.spawn('sidekiq', metrics_dir: 'path/to/metrics') + described_class.spawn('sidekiq', metrics_dir: metrics_dir) end end @@ -35,13 +41,13 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath expect(server).to receive(:start) end - described_class.spawn('sidekiq', metrics_dir: 'path/to/metrics') + described_class.spawn('sidekiq', metrics_dir: metrics_dir) end it 'resets signal handlers from parent process' do expect(Gitlab::ProcessManagement).to receive(:modify_signals).with(%i[A B], 'DEFAULT') - described_class.spawn('sidekiq', metrics_dir: 'path/to/metrics', trapped_signals: %i[A B]) + described_class.spawn('sidekiq', metrics_dir: metrics_dir, trapped_signals: %i[A B]) end end end @@ -50,9 +56,9 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath let(:exporter_class) { Class.new(Gitlab::Metrics::Exporter::BaseExporter) } let(:exporter_double) { double('fake_exporter', start: true) } let(:prometheus_config) { ::Prometheus::Client.configuration } - let(:metrics_dir) { Dir.mktmpdir } let(:settings) { { "fake_exporter" => { "enabled" => true } } } let!(:old_metrics_dir) { prometheus_config.multiprocess_files_dir } + let(:ruby_sampler_double) { double(Gitlab::Metrics::Samplers::RubySampler) } subject(:metrics_server) { described_class.new('fake', metrics_dir, true)} @@ -60,11 +66,13 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath stub_const('Gitlab::Metrics::Exporter::FakeExporter', exporter_class) expect(exporter_class).to receive(:instance).with(settings['fake_exporter'], synchronous: true).and_return(exporter_double) expect(Settings).to receive(:monitoring).and_return(settings) + + allow(Gitlab::Metrics::Samplers::RubySampler).to receive(:initialize_instance).and_return(ruby_sampler_double) + allow(ruby_sampler_double).to receive(:start) end after do Gitlab::Metrics.reset_registry! - FileUtils.rm_rf(metrics_dir, secure: true) prometheus_config.multiprocess_files_dir = old_metrics_dir end @@ -72,6 +80,7 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath metrics_server.start expect(prometheus_config.multiprocess_files_dir).to eq metrics_dir + expect(::Prometheus::Client.configuration.pid_provider.call).to eq 'fake_exporter' end it 'ensures that metrics directory exists in correct mode (0700)' do @@ -105,5 +114,11 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath metrics_server.start end + + it 'starts a RubySampler instance' do + expect(ruby_sampler_double).to receive(:start) + + subject.start + end end end diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 9163a7ef845..e80fa6e3b70 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -937,18 +937,6 @@ RSpec.describe Integrations::Jira do end end - context 'with jira_use_first_ref_by_oid feature flag disabled' do - before do - stub_feature_flags(jira_use_first_ref_by_oid: false) - end - - it 'creates a comment and remote link on Jira' do - expect(subject).to eq(success_message) - expect(WebMock).to have_requested(:post, comment_url).with(body: comment_body).once - expect(WebMock).to have_requested(:post, remote_link_url).once - end - end - it 'tracks usage' do expect(Gitlab::UsageDataCounters::HLLRedisCounter) .to receive(:track_event) diff --git a/spec/requests/api/graphql/group/group_members_spec.rb b/spec/requests/api/graphql/group/group_members_spec.rb index 31cb0393d7f..06afb5b9a49 100644 --- a/spec/requests/api/graphql/group/group_members_spec.rb +++ b/spec/requests/api/graphql/group/group_members_spec.rb @@ -56,12 +56,16 @@ RSpec.describe 'getting group members information' do context 'member relations' do let_it_be(:child_group) { create(:group, :public, parent: parent_group) } let_it_be(:grandchild_group) { create(:group, :public, parent: child_group) } + let_it_be(:invited_group) { create(:group, :public) } let_it_be(:child_user) { create(:user) } let_it_be(:grandchild_user) { create(:user) } + let_it_be(:invited_user) { create(:user) } + let_it_be(:group_link) { create(:group_group_link, shared_group: child_group, shared_with_group: invited_group) } before_all do child_group.add_guest(child_user) grandchild_group.add_guest(grandchild_user) + invited_group.add_guest(invited_user) end it 'returns direct members' do @@ -71,6 +75,13 @@ RSpec.describe 'getting group members information' do expect_array_response(child_user) end + it 'returns invited members plus inherited members' do + fetch_members(group: child_group, args: { relations: [:DIRECT, :INHERITED, :SHARED_FROM_GROUPS] }) + + expect(graphql_errors).to be_nil + expect_array_response(invited_user, user_1, user_2, child_user) + end + it 'returns direct and inherited members' do fetch_members(group: child_group, args: { relations: [:DIRECT, :INHERITED] }) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index b93df2f3bae..0fb0150ecc9 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -498,6 +498,10 @@ RSpec.describe API::Users do describe "GET /users/:id" do let_it_be(:user2, reload: true) { create(:user, username: 'another_user') } + before do + allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:users_get_by_id, scope: user).and_return(false) + end + it "returns a user by id" do get api("/users/#{user.id}", user) @@ -593,6 +597,55 @@ RSpec.describe API::Users do expect(json_response).not_to have_key('sign_in_count') end + context 'when the rate limit is not exceeded' do + it 'returns a success status' do + expect(Gitlab::ApplicationRateLimiter) + .to receive(:throttled?).with(:users_get_by_id, scope: user) + .and_return(false) + + get api("/users/#{user.id}", user) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when the rate limit is exceeded' do + context 'when feature flag is enabled' do + it 'returns "too many requests" status' do + expect(Gitlab::ApplicationRateLimiter) + .to receive(:throttled?).with(:users_get_by_id, scope: user) + .and_return(true) + + get api("/users/#{user.id}", user) + + expect(response).to have_gitlab_http_status(:too_many_requests) + end + + it 'still allows admin users' do + expect(Gitlab::ApplicationRateLimiter) + .not_to receive(:throttled?) + + get api("/users/#{user.id}", admin) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(rate_limit_user_by_id_endpoint: false) + end + + it 'does not throttle the request' do + expect(Gitlab::ApplicationRateLimiter).not_to receive(:throttled?) + + get api("/users/#{user.id}", user) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + context 'when job title is present' do let(:job_title) { 'Fullstack Engineer' } diff --git a/spec/rubocop/code_reuse_helpers_spec.rb b/spec/rubocop/code_reuse_helpers_spec.rb index 3220cff1681..d437ada85ee 100644 --- a/spec/rubocop/code_reuse_helpers_spec.rb +++ b/spec/rubocop/code_reuse_helpers_spec.rb @@ -315,76 +315,11 @@ RSpec.describe RuboCop::CodeReuseHelpers do end end - describe '#ee?' do - before do - stub_env('FOSS_ONLY', nil) - allow(File).to receive(:exist?).with(ee_file_path) { true } - end - - it 'returns true when ee/app/models/license.rb exists' do - expect(cop.ee?).to eq(true) - end - end - - describe '#jh?' do - context 'when jh directory exists and EE_ONLY is not set' do - before do - stub_env('EE_ONLY', nil) - - allow(Dir).to receive(:exist?).with(File.expand_path('../../jh', __dir__)) { true } - end - - context 'when ee/app/models/license.rb exists' do - before do - allow(File).to receive(:exist?).with(ee_file_path) { true } - end - - context 'when FOSS_ONLY is not set' do - before do - stub_env('FOSS_ONLY', nil) - end - - it 'returns true' do - expect(cop.jh?).to eq(true) - end - end - - context 'when FOSS_ONLY is set to 1' do - before do - stub_env('FOSS_ONLY', '1') - end + %w[ee? jh?].each do |method_name| + it "delegates #{method_name} to GitlabEdition" do + expect(GitlabEdition).to receive(method_name) - it 'returns false' do - expect(cop.jh?).to eq(false) - end - end - end - - context 'when ee/app/models/license.rb not exist' do - before do - allow(File).to receive(:exist?).with(ee_file_path) { false } - end - - context 'when FOSS_ONLY is not set' do - before do - stub_env('FOSS_ONLY', nil) - end - - it 'returns true' do - expect(cop.jh?).to eq(false) - end - end - - context 'when FOSS_ONLY is set to 1' do - before do - stub_env('FOSS_ONLY', '1') - end - - it 'returns false' do - expect(cop.jh?).to eq(false) - end - end - end + cop.public_send(method_name) end end end diff --git a/spec/support/shared_examples/metrics/sampler_shared_examples.rb b/spec/support/shared_examples/metrics/sampler_shared_examples.rb index ebf199c3a8d..cec540cd120 100644 --- a/spec/support/shared_examples/metrics/sampler_shared_examples.rb +++ b/spec/support/shared_examples/metrics/sampler_shared_examples.rb @@ -2,26 +2,98 @@ RSpec.shared_examples 'metrics sampler' do |env_prefix| context 'when sampling interval is passed explicitly' do - subject { described_class.new(42) } + subject(:sampler) { described_class.new(interval: 42, logger: double) } - specify { expect(subject.interval).to eq(42) } + specify { expect(sampler.interval).to eq(42) } end context 'when sampling interval is passed through the environment' do - subject { described_class.new } + subject(:sampler) { described_class.new(logger: double) } before do stub_env("#{env_prefix}_INTERVAL_SECONDS", '42') end - specify { expect(subject.interval).to eq(42) } + specify { expect(sampler.interval).to eq(42) } end context 'when no sampling interval is passed anywhere' do - subject { described_class.new } + subject(:sampler) { described_class.new(logger: double) } it 'uses the hardcoded default' do - expect(subject.interval).to eq(described_class::DEFAULT_SAMPLING_INTERVAL_SECONDS) + expect(sampler.interval).to eq(described_class::DEFAULT_SAMPLING_INTERVAL_SECONDS) + end + end + + describe '#start' do + include WaitHelpers + + subject(:sampler) { described_class.new(interval: 0.1) } + + it 'calls the sample method on the sampler thread' do + sampling_threads = [] + expect(sampler).to receive(:sample).at_least(:once) { sampling_threads << Thread.current } + + sampler.start + + wait_for('sampler has sampled', max_wait_time: 3) { sampling_threads.any? } + expect(sampling_threads.first.name).to eq(sampler.thread_name) + + sampler.stop + end + + context 'with warmup set to true' do + subject(:sampler) { described_class.new(interval: 0.1, warmup: true) } + + it 'calls the sample method first on the caller thread' do + sampling_threads = [] + current_thread = Thread.current + # Instead of sampling, we're keeping track of which thread the sampling happened on. + # We want the first sample to be on the spec thread, which would mean a blocking sample + # before the actual sampler thread starts. + expect(sampler).to receive(:sample).at_least(:once) { sampling_threads << Thread.current } + + sampler.start + + wait_for('sampler has sampled', max_wait_time: 3) { sampling_threads.size == 2 } + + expect(sampling_threads.first).to be(current_thread) + expect(sampling_threads.last.name).to eq(sampler.thread_name) + + sampler.stop + end + end + end + + describe '#safe_sample' do + let(:logger) { Logger.new(File::NULL) } + + subject(:sampler) { described_class.new(logger: logger) } + + it 'calls #sample once' do + expect(sampler).to receive(:sample) + + sampler.safe_sample + end + + context 'when sampling fails with error' do + before do + expect(sampler).to receive(:sample).and_raise "something failed" + end + + it 'recovers from errors' do + expect { sampler.safe_sample }.not_to raise_error + end + + context 'with logger' do + let(:logger) { double('logger') } + + it 'logs errors' do + expect(logger).to receive(:warn).with(an_instance_of(String)) + + expect { sampler.safe_sample }.not_to raise_error + end + end end end end diff --git a/spec/views/shared/nav/_sidebar.html.haml_spec.rb b/spec/views/shared/nav/_sidebar.html.haml_spec.rb index 2eeebdff7a8..0eb945f5624 100644 --- a/spec/views/shared/nav/_sidebar.html.haml_spec.rb +++ b/spec/views/shared/nav/_sidebar.html.haml_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe 'shared/nav/_sidebar.html.haml' do - let(:project) { build(:project, id: non_existing_record_id) } + let_it_be(:project) { create(:project) } + let(:context) { Sidebars::Projects::Context.new(current_user: nil, container: project) } let(:sidebar) { Sidebars::Projects::Panel.new(context) } |