diff options
40 files changed, 1132 insertions, 306 deletions
diff --git a/app/controllers/admin/dev_ops_report_controller.rb b/app/controllers/admin/dev_ops_report_controller.rb index 4ebc643be33..4178e51fb13 100644 --- a/app/controllers/admin/dev_ops_report_controller.rb +++ b/app/controllers/admin/dev_ops_report_controller.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true class Admin::DevOpsReportController < Admin::ApplicationController - include Analytics::UniqueVisitsHelper + include RedisTracking helper_method :show_adoption? - track_unique_visits :show, target_id: 'i_analytics_dev_ops_score' + track_redis_hll_event :show, name: 'i_analytics_dev_ops_score', if: -> { should_track_devops_score? } feature_category :devops_reports @@ -18,6 +18,10 @@ class Admin::DevOpsReportController < Admin::ApplicationController def show_adoption? false end + + def should_track_devops_score? + true + end end Admin::DevOpsReportController.prepend_if_ee('EE::Admin::DevOpsReportController') diff --git a/app/helpers/analytics/unique_visits_helper.rb b/app/helpers/analytics/unique_visits_helper.rb index 337a5dc9536..4aa8907f578 100644 --- a/app/helpers/analytics/unique_visits_helper.rb +++ b/app/helpers/analytics/unique_visits_helper.rb @@ -16,7 +16,7 @@ module Analytics def track_visit(target_id) return unless visitor_id - Gitlab::Analytics::UniqueVisits.new.track_visit(visitor_id, target_id) + Gitlab::Analytics::UniqueVisits.new.track_visit(target_id, values: visitor_id) end class_methods do diff --git a/app/services/authorized_project_update/find_records_due_for_refresh_service.rb b/app/services/authorized_project_update/find_records_due_for_refresh_service.rb new file mode 100644 index 00000000000..d6de10fb8aa --- /dev/null +++ b/app/services/authorized_project_update/find_records_due_for_refresh_service.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +module AuthorizedProjectUpdate + # Service for finding the authorized_projects records of a user that needs addition or removal. + # + # Usage: + # + # user = User.find_by(username: 'alice') + # service = AuthorizedProjectUpdate::FindRecordsDueForRefreshService.new(some_user) + # service.execute + class FindRecordsDueForRefreshService + def initialize(user, source: nil, incorrect_auth_found_callback: nil, missing_auth_found_callback: nil) + @user = user + @source = source + @incorrect_auth_found_callback = incorrect_auth_found_callback + @missing_auth_found_callback = missing_auth_found_callback + end + + def execute + current = current_authorizations_per_project + fresh = fresh_access_levels_per_project + + # Projects that have more than one authorizations associated with + # the user needs to be deleted. + # The correct authorization is added to the ``add`` array in the + # next stage. + remove = projects_with_duplicates + current.except!(*projects_with_duplicates) + + remove |= current.each_with_object([]) do |(project_id, row), array| + # rows not in the new list or with a different access level should be + # removed. + if !fresh[project_id] || fresh[project_id] != row.access_level + if incorrect_auth_found_callback + incorrect_auth_found_callback.call(project_id, row.access_level) + end + + array << row.project_id + end + end + + add = fresh.each_with_object([]) do |(project_id, level), array| + # rows not in the old list or with a different access level should be + # added. + if !current[project_id] || current[project_id].access_level != level + if missing_auth_found_callback + missing_auth_found_callback.call(project_id, level) + end + + array << [user.id, project_id, level] + end + end + + [remove, add] + end + + def fresh_access_levels_per_project + fresh_authorizations.each_with_object({}) do |row, hash| + hash[row.project_id] = row.access_level + end + end + + def current_authorizations_per_project + current_authorizations.index_by(&:project_id) + end + + def current_authorizations + @current_authorizations ||= user.project_authorizations.select(:project_id, :access_level) + end + + def fresh_authorizations + Gitlab::ProjectAuthorizations.new(user).calculate + end + + private + + attr_reader :user, :source, :incorrect_auth_found_callback, :missing_auth_found_callback + + def projects_with_duplicates + @projects_with_duplicates ||= current_authorizations + .group_by(&:project_id) + .select { |project_id, authorizations| authorizations.count > 1 } + .keys + end + end +end diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb index 070713929e4..d28ff45bfdf 100644 --- a/app/services/users/refresh_authorized_projects_service.rb +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -51,38 +51,12 @@ module Users # This method returns the updated User object. def execute_without_lease - current = current_authorizations_per_project - fresh = fresh_access_levels_per_project - - # Delete projects that have more than one authorizations associated with - # the user. The correct authorization is added to the ``add`` array in the - # next stage. - remove = projects_with_duplicates - current.except!(*projects_with_duplicates) - - remove |= current.each_with_object([]) do |(project_id, row), array| - # rows not in the new list or with a different access level should be - # removed. - if !fresh[project_id] || fresh[project_id] != row.access_level - if incorrect_auth_found_callback - incorrect_auth_found_callback.call(project_id, row.access_level) - end - - array << row.project_id - end - end - - add = fresh.each_with_object([]) do |(project_id, level), array| - # rows not in the old list or with a different access level should be - # added. - if !current[project_id] || current[project_id].access_level != level - if missing_auth_found_callback - missing_auth_found_callback.call(project_id, level) - end - - array << [user.id, project_id, level] - end - end + remove, add = AuthorizedProjectUpdate::FindRecordsDueForRefreshService.new( + user, + source: source, + incorrect_auth_found_callback: incorrect_auth_found_callback, + missing_auth_found_callback: missing_auth_found_callback + ).execute update_authorizations(remove, add) end @@ -104,6 +78,10 @@ module Users user.reset end + private + + attr_reader :incorrect_auth_found_callback, :missing_auth_found_callback + def log_refresh_details(remove, add) Gitlab::AppJsonLogger.info(event: 'authorized_projects_refresh', user_id: user.id, @@ -115,34 +93,5 @@ module Users 'authorized_projects_refresh.rows_deleted_slice': remove.first(5), 'authorized_projects_refresh.rows_added_slice': add.first(5)) end - - def fresh_access_levels_per_project - fresh_authorizations.each_with_object({}) do |row, hash| - hash[row.project_id] = row.access_level - end - end - - def current_authorizations_per_project - current_authorizations.index_by(&:project_id) - end - - def current_authorizations - @current_authorizations ||= user.project_authorizations.select(:project_id, :access_level) - end - - def fresh_authorizations - Gitlab::ProjectAuthorizations.new(user).calculate - end - - private - - attr_reader :incorrect_auth_found_callback, :missing_auth_found_callback - - def projects_with_duplicates - @projects_with_duplicates ||= current_authorizations - .group_by(&:project_id) - .select { |project_id, authorizations| authorizations.count > 1 } - .keys - end end end diff --git a/config/feature_flags/development/ci_workflow_rules_variables.yml b/config/feature_flags/development/ci_workflow_rules_variables.yml new file mode 100644 index 00000000000..8915d109c83 --- /dev/null +++ b/config/feature_flags/development/ci_workflow_rules_variables.yml @@ -0,0 +1,8 @@ +--- +name: ci_workflow_rules_variables +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52085 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/300997 +milestone: '13.11' +type: development +group: group::pipeline authoring +default_enabled: false diff --git a/config/feature_flags/development/usage_data_i_analytics_dev_ops_adoption.yml b/config/feature_flags/development/usage_data_i_analytics_dev_ops_adoption.yml new file mode 100644 index 00000000000..80e7e971aad --- /dev/null +++ b/config/feature_flags/development/usage_data_i_analytics_dev_ops_adoption.yml @@ -0,0 +1,8 @@ +--- +name: usage_data_i_analytics_dev_ops_adoption +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57104 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326450 +milestone: '13.11' +type: development +group: group::optimize +default_enabled: true diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 1ecd17b50ed..36ebedf61d1 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -172,6 +172,7 @@ a preconfigured `workflow: rules` entry. - [`when`](#when): Specify what to do when the `if` rule evaluates to true. - To run a pipeline, set to `always`. - To prevent pipelines from running, set to `never`. +- [`variables`](#workflowrulesvariables): If not defined, uses the [variables defined elsewhere](#variables). When no rules evaluate to true, the pipeline does not run. @@ -222,6 +223,54 @@ request pipelines. If your rules match both branch pipelines and merge request pipelines, [duplicate pipelines](#avoid-duplicate-pipelines) can occur. +#### `workflow:rules:variables` + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/294232) in GitLab 13.11. +> - It's [deployed behind a feature flag](../../user/feature_flags.md), disabled by default. +> - It's disabled on GitLab.com. +> - It's not recommended for production use. +> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-workflowrulesvariables). **(CORE ONLY)** + +WARNING: +This feature might not be available to you. Check the **version history** note above for details. + +You can use [`variables`](#variables) in `workflow:rules:` to define variables for specific pipeline conditions. + +For example: + +```yaml +variables: + DEPLOY_VARIABLE: "default-deploy" + +workflow: + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + DEPLOY_VARIABLE: "deploy-production" # Override globally-defined DEPLOY_VARIABLE + - if: $CI_COMMIT_REF_NAME =~ /feature/ + variables: + IS_A_FEATURE: "true" # Define a new variable. +``` + +##### Enable or disable workflow:rules:variables **(CORE ONLY)** + +rules:variables is under development and not ready for production use. +It is deployed behind a feature flag that is **disabled by default**. +[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) +can enable it. + +To enable it: + +```ruby +Feature.enable(:ci_workflow_rules_variables) +``` + +To disable it: + +```ruby +Feature.disable(:ci_workflow_rules_variables) +``` + #### `workflow:rules` templates > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/217732) in GitLab 13.0. diff --git a/doc/development/usage_ping/dictionary.md b/doc/development/usage_ping/dictionary.md index 17ab4ba49a1..24cd5e10f89 100644 --- a/doc/development/usage_ping/dictionary.md +++ b/doc/development/usage_ping/dictionary.md @@ -7364,6 +7364,30 @@ Status: `data_available` Tiers: +### `redis_hll_counters.analytics.i_analytics_dev_ops_adoption_monthly` + +Counts visits to DevOps Adoption page per month + +[YAML definition](https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/config/metrics/counts_28d/20210401092244_i_analytics_dev_ops_adoption_monthly.yml) + +Group: `group::optimize` + +Status: `implemented` + +Tiers: `premium`, `ultimate` + +### `redis_hll_counters.analytics.i_analytics_dev_ops_adoption_weekly` + +Counts visits to DevOps Adoption page per week + +[YAML definition](https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/config/metrics/counts_7d/20210401092244_i_analytics_dev_ops_adoption_weekly.yml) + +Group: `group::optimize` + +Status: `implemented` + +Tiers: `premium`, `ultimate` + ### `redis_hll_counters.analytics.i_analytics_dev_ops_score_monthly` Missing description diff --git a/lib/gitlab/analytics/unique_visits.rb b/lib/gitlab/analytics/unique_visits.rb index e367d33d743..723486231b1 100644 --- a/lib/gitlab/analytics/unique_visits.rb +++ b/lib/gitlab/analytics/unique_visits.rb @@ -3,8 +3,8 @@ module Gitlab module Analytics class UniqueVisits - def track_visit(visitor_id, target_id, time = Time.zone.now) - Gitlab::UsageDataCounters::HLLRedisCounter.track_event(target_id, values: visitor_id, time: time) + def track_visit(*args, **kwargs) + Gitlab::UsageDataCounters::HLLRedisCounter.track_event(*args, **kwargs) end # Returns number of unique visitors for given targets in given time frame diff --git a/lib/gitlab/ci/config/entry/processable.rb b/lib/gitlab/ci/config/entry/processable.rb index 9584d19bdec..1cdf84ef4d3 100644 --- a/lib/gitlab/ci/config/entry/processable.rb +++ b/lib/gitlab/ci/config/entry/processable.rb @@ -124,7 +124,9 @@ module Gitlab stage: stage_value, extends: extends, rules: rules_value, - variables: root_and_job_variables_value, + variables: root_and_job_variables_value, # https://gitlab.com/gitlab-org/gitlab/-/issues/300581 + job_variables: job_variables, + root_variables_inheritance: root_variables_inheritance, only: only_value, except: except_value, resource_group: resource_group }.compact @@ -139,6 +141,18 @@ module Gitlab root_variables.merge(variables_value.to_h) end + def job_variables + return unless ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + + variables_value.to_h + end + + def root_variables_inheritance + return unless ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + + inherit_entry&.variables_entry&.value + end + def manual_action? self.when == 'manual' end diff --git a/lib/gitlab/ci/config/normalizer/matrix_strategy.rb b/lib/gitlab/ci/config/normalizer/matrix_strategy.rb index 5a23836d8a0..48b6fca598a 100644 --- a/lib/gitlab/ci/config/normalizer/matrix_strategy.rb +++ b/lib/gitlab/ci/config/normalizer/matrix_strategy.rb @@ -43,9 +43,10 @@ module Gitlab { name: name, instance: instance, - variables: variables, + variables: variables, # https://gitlab.com/gitlab-org/gitlab/-/issues/300581 + job_variables: job_variables, parallel: { total: total } - } + }.compact end def name @@ -60,6 +61,12 @@ module Gitlab private attr_reader :job_name, :instance, :variables, :total + + def job_variables + return unless ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + + variables + end end end end diff --git a/lib/gitlab/ci/pipeline/chain/seed.rb b/lib/gitlab/ci/pipeline/chain/seed.rb index 46524e8999f..560e62ce850 100644 --- a/lib/gitlab/ci/pipeline/chain/seed.rb +++ b/lib/gitlab/ci/pipeline/chain/seed.rb @@ -11,6 +11,10 @@ module Gitlab def perform! raise ArgumentError, 'missing YAML processor result' unless @command.yaml_processor_result + if ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + raise ArgumentError, 'missing workflow rules result' unless @command.workflow_rules_result + end + # Allocate next IID. This operation must be outside of transactions of pipeline creations. pipeline.ensure_project_iid! pipeline.ensure_ci_ref! @@ -47,7 +51,13 @@ module Gitlab end def root_variables - @command.yaml_processor_result.root_variables + if ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + ::Gitlab::Ci::Variables::Helpers.merge_variables( + @command.yaml_processor_result.root_variables, @command.workflow_rules_result.variables + ) + else + @command.yaml_processor_result.root_variables + end end end end diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index b95a23d2263..722ef1d658e 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/lib/gitlab/ci/pipeline/seed/build.rb @@ -18,6 +18,8 @@ module Gitlab @previous_stages = previous_stages @needs_attributes = dig(:needs_attributes) @resource_group_key = attributes.delete(:resource_group_key) + @job_variables = @seed_attributes.delete(:job_variables) + @root_variables_inheritance = @seed_attributes.delete(:root_variables_inheritance) { true } @using_rules = attributes.key?(:rules) @using_only = attributes.key?(:only) @@ -31,6 +33,8 @@ module Gitlab .new(attributes.delete(:rules), default_when: 'on_success') @cache = Gitlab::Ci::Build::Cache .new(attributes.delete(:cache), @pipeline) + + recalculate_yaml_variables! end def name @@ -207,6 +211,14 @@ module Gitlab { options: { allow_failure_criteria: nil } } end + + def recalculate_yaml_variables! + return unless ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + + @seed_attributes[:yaml_variables] = Gitlab::Ci::Variables::Helpers.inherit_yaml_variables( + from: @context.root_variables, to: @job_variables, inheritance: @root_variables_inheritance + ) + end end end end diff --git a/lib/gitlab/ci/variables/helpers.rb b/lib/gitlab/ci/variables/helpers.rb index e2a54f90ecb..2c3457e0265 100644 --- a/lib/gitlab/ci/variables/helpers.rb +++ b/lib/gitlab/ci/variables/helpers.rb @@ -25,6 +25,20 @@ module Gitlab vars.to_a.map { |var| [var[:key].to_s, var[:value]] }.to_h end + + def inherit_yaml_variables(from:, to:, inheritance:) + merge_variables(apply_inheritance(from, inheritance), to) + end + + private + + def apply_inheritance(variables, inheritance) + case inheritance + when true then variables + when false then {} + when Array then variables.select { |var| inheritance.include?(var[:key]) } + end + end end end end diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb index c266dfd75d3..f96a6629849 100644 --- a/lib/gitlab/ci/yaml_processor/result.rb +++ b/lib/gitlab/ci/yaml_processor/result.rb @@ -69,7 +69,9 @@ module Gitlab when: job[:when] || 'on_success', environment: job[:environment_name], coverage_regex: job[:coverage], - yaml_variables: transform_to_yaml_variables(job[:variables]), + yaml_variables: transform_to_yaml_variables(job[:variables]), # https://gitlab.com/gitlab-org/gitlab/-/issues/300581 + job_variables: transform_to_yaml_variables(job[:job_variables]), + root_variables_inheritance: job[:root_variables_inheritance], needs_attributes: job.dig(:needs, :job), interruptible: job[:interruptible], only: job[:only], diff --git a/lib/gitlab/usage_data_counters/known_events/common.yml b/lib/gitlab/usage_data_counters/known_events/common.yml index a6a2c62a483..76db86d1435 100644 --- a/lib/gitlab/usage_data_counters/known_events/common.yml +++ b/lib/gitlab/usage_data_counters/known_events/common.yml @@ -91,6 +91,11 @@ redis_slot: analytics aggregation: weekly feature_flag: track_unique_visits +- name: i_analytics_dev_ops_adoption + category: analytics + redis_slot: analytics + aggregation: weekly + feature_flag: track_unique_visits - name: g_analytics_merge_request category: analytics redis_slot: analytics diff --git a/lib/gitlab/web_ide/config/entry/terminal.rb b/lib/gitlab/web_ide/config/entry/terminal.rb index 403e308d45b..514fca1435c 100644 --- a/lib/gitlab/web_ide/config/entry/terminal.rb +++ b/lib/gitlab/web_ide/config/entry/terminal.rb @@ -10,6 +10,7 @@ module Gitlab class Terminal < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Configurable include ::Gitlab::Config::Entry::Attributable + include Gitlab::Utils::StrongMemoize # By default the build will finish in a few seconds, not giving the webide # enough time to connect to the terminal. This default script provides @@ -51,23 +52,34 @@ module Gitlab private def to_hash - { tag_list: tags || [], - yaml_variables: yaml_variables, + { + tag_list: tags || [], + yaml_variables: yaml_variables, # https://gitlab.com/gitlab-org/gitlab/-/issues/300581 + job_variables: job_variables, options: { image: image_value, services: services_value, before_script: before_script_value, script: script_value || DEFAULT_SCRIPT - }.compact } + }.compact + }.compact end def yaml_variables - return unless variables_value + strong_memoize(:yaml_variables) do + next unless variables_value - variables_value.map do |key, value| - { key: key.to_s, value: value, public: true } + variables_value.map do |key, value| + { key: key.to_s, value: value, public: true } + end end end + + def job_variables + return unless ::Feature.enabled?(:ci_workflow_rules_variables, default_enabled: :yaml) + + yaml_variables + end end end end diff --git a/spec/controllers/admin/dev_ops_report_controller_spec.rb b/spec/controllers/admin/dev_ops_report_controller_spec.rb index 913921b9630..142db175a15 100644 --- a/spec/controllers/admin/dev_ops_report_controller_spec.rb +++ b/spec/controllers/admin/dev_ops_report_controller_spec.rb @@ -5,7 +5,13 @@ require 'spec_helper' RSpec.describe Admin::DevOpsReportController do describe 'show_adoption?' do it 'is always false' do - expect(controller.show_adoption?).to be false + expect(controller.show_adoption?).to be_falsey + end + end + + describe 'should_track_devops_score?' do + it 'is always true' do + expect(controller.should_track_devops_score?).to be_truthy end end diff --git a/spec/lib/gitlab/analytics/unique_visits_spec.rb b/spec/lib/gitlab/analytics/unique_visits_spec.rb index 6ac58e13f4c..f4d5c0b1eca 100644 --- a/spec/lib/gitlab/analytics/unique_visits_spec.rb +++ b/spec/lib/gitlab/analytics/unique_visits_spec.rb @@ -24,18 +24,18 @@ RSpec.describe Gitlab::Analytics::UniqueVisits, :clean_gitlab_redis_shared_state describe '#track_visit' do it 'tracks the unique weekly visits for targets' do - unique_visits.track_visit(visitor1_id, target1_id, 7.days.ago) - unique_visits.track_visit(visitor1_id, target1_id, 7.days.ago) - unique_visits.track_visit(visitor2_id, target1_id, 7.days.ago) + unique_visits.track_visit(target1_id, values: visitor1_id, time: 7.days.ago) + unique_visits.track_visit(target1_id, values: visitor1_id, time: 7.days.ago) + unique_visits.track_visit(target1_id, values: visitor2_id, time: 7.days.ago) - unique_visits.track_visit(visitor2_id, target2_id, 7.days.ago) - unique_visits.track_visit(visitor1_id, target2_id, 8.days.ago) - unique_visits.track_visit(visitor1_id, target2_id, 15.days.ago) + unique_visits.track_visit(target2_id, values: visitor2_id, time: 7.days.ago) + unique_visits.track_visit(target2_id, values: visitor1_id, time: 8.days.ago) + unique_visits.track_visit(target2_id, values: visitor1_id, time: 15.days.ago) - unique_visits.track_visit(visitor3_id, target4_id, 7.days.ago) + unique_visits.track_visit(target4_id, values: visitor3_id, time: 7.days.ago) - unique_visits.track_visit(visitor3_id, target5_id, 15.days.ago) - unique_visits.track_visit(visitor2_id, target5_id, 15.days.ago) + unique_visits.track_visit(target5_id, values: visitor3_id, time: 15.days.ago) + unique_visits.track_visit(target5_id, values: visitor2_id, time: 15.days.ago) expect(unique_visits.unique_visits_for(targets: target1_id)).to eq(2) expect(unique_visits.unique_visits_for(targets: target2_id)).to eq(1) @@ -61,7 +61,7 @@ RSpec.describe Gitlab::Analytics::UniqueVisits, :clean_gitlab_redis_shared_state end it 'sets the keys in Redis to expire automatically after 12 weeks' do - unique_visits.track_visit(visitor1_id, target1_id) + unique_visits.track_visit(target1_id, values: visitor1_id) Gitlab::Redis::SharedState.with do |redis| redis.scan_each(match: "{#{target1_id}}-*").each do |key| @@ -74,7 +74,7 @@ RSpec.describe Gitlab::Analytics::UniqueVisits, :clean_gitlab_redis_shared_state invalid_target_id = "x_invalid" expect do - unique_visits.track_visit(visitor1_id, invalid_target_id) + unique_visits.track_visit(invalid_target_id, values: visitor1_id) end.to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownEvent) end end diff --git a/spec/lib/gitlab/ci/config/entry/bridge_spec.rb b/spec/lib/gitlab/ci/config/entry/bridge_spec.rb index 179578fe0a8..d294eca7f15 100644 --- a/spec/lib/gitlab/ci/config/entry/bridge_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/bridge_spec.rb @@ -107,6 +107,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Bridge do stage: 'test', only: { refs: %w[branches tags] }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage) end end @@ -130,6 +132,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Bridge do stage: 'test', only: { refs: %w[branches tags] }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage) end end @@ -284,6 +288,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Bridge do parallel: { matrix: [{ 'PROVIDER' => ['aws'], 'STACK' => %w(monitoring app1) }, { 'PROVIDER' => ['gcp'], 'STACK' => %w(data) }] }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage ) end diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index a4167003987..ffcd029172a 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -663,6 +663,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do after_script: %w[cleanup], only: { refs: %w[branches tags] }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage) end end diff --git a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb index ac6b589ec6b..cb73044b62b 100644 --- a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb @@ -100,6 +100,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Jobs do stage: 'test', trigger: { project: 'my/project' }, variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage }, regular_job: { @@ -109,6 +111,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Jobs do script: ['something'], stage: 'test', variables: {}, + job_variables: {}, + root_variables_inheritance: true, scheduling_type: :stage }) end diff --git a/spec/lib/gitlab/ci/config/entry/processable_spec.rb b/spec/lib/gitlab/ci/config/entry/processable_spec.rb index 04e80450263..6914eb15279 100644 --- a/spec/lib/gitlab/ci/config/entry/processable_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/processable_spec.rb @@ -382,8 +382,24 @@ RSpec.describe Gitlab::Ci::Config::Entry::Processable do context 'with only job variables' do it 'does return defined variables' do expect(entry.value).to include( + variables: { 'A' => 'job', 'B' => 'job' }, + job_variables: { 'A' => 'job', 'B' => 'job' }, + root_variables_inheritance: true + ) + end + end + + context 'when FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'does not return job_variables and root_variables_inheritance' do + expect(entry.value).to include( variables: { 'A' => 'job', 'B' => 'job' } ) + expect(entry.value).not_to have_key(:job_variables) + expect(entry.value).not_to have_key(:root_variables_inheritance) end end @@ -394,9 +410,11 @@ RSpec.describe Gitlab::Ci::Config::Entry::Processable do ).value end - it 'does return all variables and overwrite them' do + it 'does return job and root variables' do expect(entry.value).to include( - variables: { 'A' => 'job', 'B' => 'job', 'C' => 'root', 'D' => 'root' } + variables: { 'A' => 'job', 'B' => 'job', 'C' => 'root', 'D' => 'root' }, + job_variables: { 'A' => 'job', 'B' => 'job' }, + root_variables_inheritance: true ) end @@ -408,9 +426,11 @@ RSpec.describe Gitlab::Ci::Config::Entry::Processable do } end - it 'does return only job variables' do + it 'does return job and root variables' do expect(entry.value).to include( - variables: { 'A' => 'job', 'B' => 'job' } + variables: { 'A' => 'job', 'B' => 'job' }, + job_variables: { 'A' => 'job', 'B' => 'job' }, + root_variables_inheritance: false ) end end @@ -423,9 +443,11 @@ RSpec.describe Gitlab::Ci::Config::Entry::Processable do } end - it 'does return only job variables' do + it 'does return job and root variables' do expect(entry.value).to include( - variables: { 'A' => 'job', 'B' => 'job', 'D' => 'root' } + variables: { 'A' => 'job', 'B' => 'job', 'D' => 'root' }, + job_variables: { 'A' => 'job', 'B' => 'job' }, + root_variables_inheritance: ['D'] ) end end @@ -493,9 +515,26 @@ RSpec.describe Gitlab::Ci::Config::Entry::Processable do name: :rspec, stage: 'test', only: { refs: %w[branches tags] }, - variables: {} + variables: {}, + job_variables: {}, + root_variables_inheritance: true ) end + + context 'when FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'does not return job_variables and root_variables_inheritance' do + expect(entry.value).to eq( + name: :rspec, + stage: 'test', + only: { refs: %w[branches tags] }, + variables: {} + ) + end + end end end end diff --git a/spec/lib/gitlab/ci/config/entry/root_spec.rb b/spec/lib/gitlab/ci/config/entry/root_spec.rb index 7b38c21788f..041eb748fc9 100644 --- a/spec/lib/gitlab/ci/config/entry/root_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/root_spec.rb @@ -133,6 +133,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do stage: 'test', cache: [{ key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }], variables: { 'VAR' => 'root', 'VAR2' => 'val 2' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -147,6 +149,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do stage: 'test', cache: [{ key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }], variables: { 'VAR' => 'root', 'VAR2' => 'val 2' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -163,6 +167,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do cache: [{ key: "k", untracked: true, paths: ["public/"], policy: "pull-push", when: 'on_success' }], only: { refs: %w(branches tags) }, variables: { 'VAR' => 'job', 'VAR2' => 'val 2' }, + job_variables: { 'VAR' => 'job' }, + root_variables_inheritance: true, after_script: [], ignore: false, scheduling_type: :stage } @@ -188,6 +194,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do stage: 'test', cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }, variables: { 'VAR' => 'root', 'VAR2' => 'val 2' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -202,6 +210,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do stage: 'test', cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }, variables: { 'VAR' => 'root', 'VAR2' => 'val 2' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -218,6 +228,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do cache: { key: "k", untracked: true, paths: ["public/"], policy: "pull-push", when: 'on_success' }, only: { refs: %w(branches tags) }, variables: { 'VAR' => 'job', 'VAR2' => 'val 2' }, + job_variables: { 'VAR' => 'job' }, + root_variables_inheritance: true, after_script: [], ignore: false, scheduling_type: :stage } @@ -267,6 +279,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do stage: 'test', cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }, variables: { 'VAR' => 'root' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -279,6 +293,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do stage: 'test', cache: { key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }, variables: { 'VAR' => 'job' }, + job_variables: { 'VAR' => 'job' }, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -311,6 +327,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do stage: 'test', cache: [{ key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }], variables: { 'VAR' => 'root' }, + job_variables: {}, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, @@ -323,6 +341,8 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do stage: 'test', cache: [{ key: 'k', untracked: true, paths: ['public/'], policy: 'pull-push', when: 'on_success' }], variables: { 'VAR' => 'job' }, + job_variables: { 'VAR' => 'job' }, + root_variables_inheritance: true, ignore: false, after_script: ['make clean'], only: { refs: %w[branches tags] }, diff --git a/spec/lib/gitlab/ci/config/normalizer/matrix_strategy_spec.rb b/spec/lib/gitlab/ci/config/normalizer/matrix_strategy_spec.rb index fbf86927bd9..0e3ea697a32 100644 --- a/spec/lib/gitlab/ci/config/normalizer/matrix_strategy_spec.rb +++ b/spec/lib/gitlab/ci/config/normalizer/matrix_strategy_spec.rb @@ -1,8 +1,12 @@ # frozen_string_literal: true require 'fast_spec_helper' +require 'support/helpers/stubbed_feature' +require 'support/helpers/stub_feature_flags' RSpec.describe Gitlab::Ci::Config::Normalizer::MatrixStrategy do + include StubFeatureFlags + describe '.applies_to?' do subject { described_class.applies_to?(config) } @@ -49,6 +53,10 @@ RSpec.describe Gitlab::Ci::Config::Normalizer::MatrixStrategy do variables: { 'PROVIDER' => 'aws', 'STACK' => 'app1' + }, + job_variables: { + 'PROVIDER' => 'aws', + 'STACK' => 'app1' } }, { @@ -58,6 +66,10 @@ RSpec.describe Gitlab::Ci::Config::Normalizer::MatrixStrategy do variables: { 'PROVIDER' => 'aws', 'STACK' => 'app2' + }, + job_variables: { + 'PROVIDER' => 'aws', + 'STACK' => 'app2' } }, { @@ -67,6 +79,10 @@ RSpec.describe Gitlab::Ci::Config::Normalizer::MatrixStrategy do variables: { 'PROVIDER' => 'ovh', 'STACK' => 'app' + }, + job_variables: { + 'PROVIDER' => 'ovh', + 'STACK' => 'app' } }, { @@ -76,6 +92,10 @@ RSpec.describe Gitlab::Ci::Config::Normalizer::MatrixStrategy do variables: { 'PROVIDER' => 'gcp', 'STACK' => 'app' + }, + job_variables: { + 'PROVIDER' => 'gcp', + 'STACK' => 'app' } } ] @@ -87,5 +107,54 @@ RSpec.describe Gitlab::Ci::Config::Normalizer::MatrixStrategy do ['test: [aws, app1]', 'test: [aws, app2]', 'test: [gcp, app]', 'test: [ovh, app]'] ) end + + context 'when the FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'excludes job_variables' do + expect(subject.map(&:attributes)).to match_array( + [ + { + name: 'test: [aws, app1]', + instance: 1, + parallel: { total: 4 }, + variables: { + 'PROVIDER' => 'aws', + 'STACK' => 'app1' + } + }, + { + name: 'test: [aws, app2]', + instance: 2, + parallel: { total: 4 }, + variables: { + 'PROVIDER' => 'aws', + 'STACK' => 'app2' + } + }, + { + name: 'test: [ovh, app]', + instance: 3, + parallel: { total: 4 }, + variables: { + 'PROVIDER' => 'ovh', + 'STACK' => 'app' + } + }, + { + name: 'test: [gcp, app]', + instance: 4, + parallel: { total: 4 }, + variables: { + 'PROVIDER' => 'gcp', + 'STACK' => 'app' + } + } + ] + ) + end + end end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb index 1326b3b0fb5..bd5b6d53b01 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/seed_spec.rb @@ -194,5 +194,39 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Seed do expect(pipeline.variables.size).to eq(0) end end + + describe '#root_variables' do + let(:config) do + { + variables: { VAR1: 'var 1' }, + workflow: { + rules: [{ if: '$CI_PIPELINE_SOURCE', + variables: { VAR1: 'overridden var 1' } }, + { when: 'always' }] + }, + rspec: { script: 'rake' } + } + end + + let(:rspec_variables) { command.pipeline_seed.stages[0].statuses[0].variables.to_hash } + + it 'sends root variable with overridden by rules' do + run_chain + + expect(rspec_variables['VAR1']).to eq('overridden var 1') + end + + context 'when the FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'sends root variable' do + run_chain + + expect(rspec_variables['VAR1']).to eq('var 1') + end + end + end end end diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index 10f2f42e268..f97935feb86 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -77,8 +77,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do let(:attributes) do { name: 'rspec', ref: 'master', - yaml_variables: [{ key: 'VAR1', value: 'var 1', public: true }, - { key: 'VAR2', value: 'var 2', public: true }], + job_variables: [{ key: 'VAR1', value: 'var 1', public: true }, + { key: 'VAR2', value: 'var 2', public: true }], rules: [{ if: '$VAR == null', variables: { VAR1: 'new var 1', VAR3: 'var 3' } }] } end @@ -304,14 +304,98 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do end end + context 'with workflow:rules:[variables:]' do + let(:attributes) do + { name: 'rspec', + ref: 'master', + yaml_variables: [{ key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }], + job_variables: [{ key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }], + root_variables_inheritance: root_variables_inheritance } + end + + context 'when the pipeline has variables' do + let(:root_variables) do + [{ key: 'VAR1', value: 'var overridden pipeline 1', public: true }, + { key: 'VAR2', value: 'var pipeline 2', public: true }, + { key: 'VAR3', value: 'var pipeline 3', public: true }, + { key: 'VAR4', value: 'new var pipeline 4', public: true }] + end + + context 'when root_variables_inheritance is true' do + let(:root_variables_inheritance) { true } + + it 'returns calculated yaml variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR1', value: 'var overridden pipeline 1', public: true }, + { key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }, + { key: 'VAR4', value: 'new var pipeline 4', public: true }] + ) + end + + context 'when FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'returns existing yaml variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }] + ) + end + end + end + + context 'when root_variables_inheritance is false' do + let(:root_variables_inheritance) { false } + + it 'returns job variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }] + ) + end + end + + context 'when root_variables_inheritance is an array' do + let(:root_variables_inheritance) { %w(VAR1 VAR2 VAR3) } + + it 'returns calculated yaml variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR1', value: 'var overridden pipeline 1', public: true }, + { key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }] + ) + end + end + end + + context 'when the pipeline has not a variable' do + let(:root_variables_inheritance) { true } + + it 'returns seed yaml variables' do + expect(subject[:yaml_variables]).to match_array( + [{ key: 'VAR2', value: 'var 2', public: true }, + { key: 'VAR3', value: 'var 3', public: true }]) + end + end + end + context 'when the job rule depends on variables' do let(:attributes) do { name: 'rspec', ref: 'master', yaml_variables: [{ key: 'VAR1', value: 'var 1', public: true }], + job_variables: [{ key: 'VAR1', value: 'var 1', public: true }], + root_variables_inheritance: root_variables_inheritance, rules: rules } end + let(:root_variables_inheritance) { true } + context 'when the rules use job variables' do let(:rules) do [{ if: '$VAR1 == "var 1"', variables: { VAR1: 'overridden var 1', VAR2: 'new var 2' } }] @@ -322,6 +406,29 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do { key: 'VAR2', value: 'new var 2', public: true }) end end + + context 'when the rules use root variables' do + let(:root_variables) do + [{ key: 'VAR2', value: 'var pipeline 2', public: true }] + end + + let(:rules) do + [{ if: '$VAR2 == "var pipeline 2"', variables: { VAR1: 'overridden var 1', VAR2: 'overridden var 2' } }] + end + + it 'recalculates the variables' do + expect(subject[:yaml_variables]).to contain_exactly({ key: 'VAR1', value: 'overridden var 1', public: true }, + { key: 'VAR2', value: 'overridden var 2', public: true }) + end + + context 'when the root_variables_inheritance is false' do + let(:root_variables_inheritance) { false } + + it 'does not recalculate the variables' do + expect(subject[:yaml_variables]).to contain_exactly({ key: 'VAR1', value: 'var 1', public: true }) + end + end + end end end diff --git a/spec/lib/gitlab/ci/variables/helpers_spec.rb b/spec/lib/gitlab/ci/variables/helpers_spec.rb index b45abf8c0e1..f13b334c10e 100644 --- a/spec/lib/gitlab/ci/variables/helpers_spec.rb +++ b/spec/lib/gitlab/ci/variables/helpers_spec.rb @@ -100,4 +100,50 @@ RSpec.describe Gitlab::Ci::Variables::Helpers do it { is_expected.to eq(result) } end end + + describe '.inherit_yaml_variables' do + let(:from) do + [{ key: 'key1', value: 'value1' }, + { key: 'key2', value: 'value2' }] + end + + let(:to) do + [{ key: 'key2', value: 'value22' }, + { key: 'key3', value: 'value3' }] + end + + let(:inheritance) { true } + + let(:result) do + [{ key: 'key1', value: 'value1', public: true }, + { key: 'key2', value: 'value22', public: true }, + { key: 'key3', value: 'value3', public: true }] + end + + subject { described_class.inherit_yaml_variables(from: from, to: to, inheritance: inheritance) } + + it { is_expected.to eq(result) } + + context 'when inheritance is false' do + let(:inheritance) { false } + + let(:result) do + [{ key: 'key2', value: 'value22', public: true }, + { key: 'key3', value: 'value3', public: true }] + end + + it { is_expected.to eq(result) } + end + + context 'when inheritance is array' do + let(:inheritance) { ['key2'] } + + let(:result) do + [{ key: 'key2', value: 'value22', public: true }, + { key: 'key3', value: 'value3', public: true }] + end + + it { is_expected.to eq(result) } + end + end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 4f45eb2c985..91e232ee2c1 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -43,6 +43,8 @@ module Gitlab allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -74,6 +76,8 @@ module Gitlab allow_failure: false, when: 'on_success', yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -111,7 +115,9 @@ module Gitlab tag_list: %w[A B], allow_failure: false, when: "on_success", - yaml_variables: [] + yaml_variables: [], + job_variables: [], + root_variables_inheritance: true }) end end @@ -158,6 +164,8 @@ module Gitlab allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -347,6 +355,8 @@ module Gitlab allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage, options: { script: ["rspec"] }, only: { refs: ["branches"] } }] }, @@ -359,6 +369,8 @@ module Gitlab allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage, options: { script: ["cap prod"] }, only: { refs: ["tags"] } }] }, @@ -853,6 +865,8 @@ module Gitlab allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -886,6 +900,8 @@ module Gitlab allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -915,6 +931,8 @@ module Gitlab allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -942,6 +960,8 @@ module Gitlab allow_failure: false, when: "on_success", yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -951,7 +971,10 @@ module Gitlab describe 'Variables' do subject { Gitlab::Ci::YamlProcessor.new(YAML.dump(config)).execute } - let(:build_variables) { subject.builds.first[:yaml_variables] } + let(:build) { subject.builds.first } + let(:yaml_variables) { build[:yaml_variables] } + let(:job_variables) { build[:job_variables] } + let(:root_variables_inheritance) { build[:root_variables_inheritance] } context 'when global variables are defined' do let(:variables) do @@ -967,10 +990,12 @@ module Gitlab end it 'returns global variables' do - expect(build_variables).to contain_exactly( + expect(yaml_variables).to contain_exactly( { key: 'VAR1', value: 'value1', public: true }, { key: 'VAR2', value: 'value2', public: true } ) + expect(job_variables).to eq([]) + expect(root_variables_inheritance).to eq(true) end end @@ -979,7 +1004,7 @@ module Gitlab { 'VAR1' => 'global1', 'VAR3' => 'global3', 'VAR4' => 'global4' } end - let(:job_variables) do + let(:build_variables) do { 'VAR1' => 'value1', 'VAR2' => 'value2' } end @@ -987,20 +1012,25 @@ module Gitlab { before_script: ['pwd'], variables: global_variables, - rspec: { script: 'rspec', variables: job_variables, inherit: inherit } + rspec: { script: 'rspec', variables: build_variables, inherit: inherit } } end context 'when no inheritance is specified' do let(:inherit) { } - it 'returns all unique variables' do - expect(build_variables).to contain_exactly( - { key: 'VAR4', value: 'global4', public: true }, + it 'returns all variables' do + expect(yaml_variables).to contain_exactly( + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true }, { key: 'VAR3', value: 'global3', public: true }, + { key: 'VAR4', value: 'global4', public: true } + ) + expect(job_variables).to contain_exactly( { key: 'VAR1', value: 'value1', public: true }, { key: 'VAR2', value: 'value2', public: true } ) + expect(root_variables_inheritance).to eq(true) end end @@ -1008,22 +1038,32 @@ module Gitlab let(:inherit) { { variables: false } } it 'does not inherit variables' do - expect(build_variables).to contain_exactly( + expect(yaml_variables).to contain_exactly( { key: 'VAR1', value: 'value1', public: true }, { key: 'VAR2', value: 'value2', public: true } ) + expect(job_variables).to contain_exactly( + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true } + ) + expect(root_variables_inheritance).to eq(false) end end context 'when specific variables are to inherited' do let(:inherit) { { variables: %w[VAR1 VAR4] } } - it 'returns all unique variables and inherits only specified variables' do - expect(build_variables).to contain_exactly( - { key: 'VAR4', value: 'global4', public: true }, + it 'returns all variables and inherits only specified variables' do + expect(yaml_variables).to contain_exactly( + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true }, + { key: 'VAR4', value: 'global4', public: true } + ) + expect(job_variables).to contain_exactly( { key: 'VAR1', value: 'value1', public: true }, { key: 'VAR2', value: 'value2', public: true } ) + expect(root_variables_inheritance).to eq(%w[VAR1 VAR4]) end end end @@ -1042,10 +1082,15 @@ module Gitlab end it 'returns job variables' do - expect(build_variables).to contain_exactly( + expect(yaml_variables).to contain_exactly( + { key: 'VAR1', value: 'value1', public: true }, + { key: 'VAR2', value: 'value2', public: true } + ) + expect(job_variables).to contain_exactly( { key: 'VAR1', value: 'value1', public: true }, { key: 'VAR2', value: 'value2', public: true } ) + expect(root_variables_inheritance).to eq(true) end end @@ -1068,8 +1113,11 @@ module Gitlab # When variables config is empty, we assume this is a valid # configuration, see issue #18775 # - expect(build_variables).to be_an_instance_of(Array) - expect(build_variables).to be_empty + expect(yaml_variables).to be_an_instance_of(Array) + expect(yaml_variables).to be_empty + + expect(job_variables).to eq([]) + expect(root_variables_inheritance).to eq(true) end end end @@ -1084,8 +1132,11 @@ module Gitlab end it 'returns empty array' do - expect(build_variables).to be_an_instance_of(Array) - expect(build_variables).to be_empty + expect(yaml_variables).to be_an_instance_of(Array) + expect(yaml_variables).to be_empty + + expect(job_variables).to eq([]) + expect(root_variables_inheritance).to eq(true) end end end @@ -1717,6 +1768,8 @@ module Gitlab when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -2080,6 +2133,8 @@ module Gitlab when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage ) expect(subject.builds[4]).to eq( @@ -2095,6 +2150,8 @@ module Gitlab when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :dag ) end @@ -2122,6 +2179,8 @@ module Gitlab when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage ) expect(subject.builds[4]).to eq( @@ -2139,6 +2198,8 @@ module Gitlab when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :dag ) end @@ -2162,6 +2223,8 @@ module Gitlab when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :dag ) end @@ -2193,6 +2256,8 @@ module Gitlab when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :dag ) end @@ -2391,6 +2456,8 @@ module Gitlab when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end @@ -2438,6 +2505,8 @@ module Gitlab when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) expect(subject.second).to eq({ @@ -2451,6 +2520,8 @@ module Gitlab when: "on_success", allow_failure: false, yaml_variables: [], + job_variables: [], + root_variables_inheritance: true, scheduling_type: :stage }) end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index e835cec7728..31066419ee4 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -1291,6 +1291,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do 'p_analytics_repo' => 123, 'i_analytics_cohorts' => 123, 'i_analytics_dev_ops_score' => 123, + 'i_analytics_dev_ops_adoption' => 123, 'i_analytics_instance_statistics' => 123, 'p_analytics_merge_request' => 123, 'g_analytics_merge_request' => 123, diff --git a/spec/lib/gitlab/web_ide/config/entry/global_spec.rb b/spec/lib/gitlab/web_ide/config/entry/global_spec.rb index 3e29bf89785..8dbe64af1c7 100644 --- a/spec/lib/gitlab/web_ide/config/entry/global_spec.rb +++ b/spec/lib/gitlab/web_ide/config/entry/global_spec.rb @@ -83,6 +83,7 @@ RSpec.describe Gitlab::WebIde::Config::Entry::Global do expect(global.terminal_value).to eq({ tag_list: [], yaml_variables: [], + job_variables: [], options: { before_script: ['ls'], script: ['sleep 10s'], diff --git a/spec/lib/gitlab/web_ide/config/entry/terminal_spec.rb b/spec/lib/gitlab/web_ide/config/entry/terminal_spec.rb index 0df0f56f440..b0a52a094ac 100644 --- a/spec/lib/gitlab/web_ide/config/entry/terminal_spec.rb +++ b/spec/lib/gitlab/web_ide/config/entry/terminal_spec.rb @@ -142,6 +142,7 @@ RSpec.describe Gitlab::WebIde::Config::Entry::Terminal do .to eq( tag_list: ['webide'], yaml_variables: [{ key: 'KEY', value: 'value', public: true }], + job_variables: [{ key: 'KEY', value: 'value', public: true }], options: { image: { name: "ruby:2.5" }, services: [{ name: "mysql" }], @@ -150,6 +151,26 @@ RSpec.describe Gitlab::WebIde::Config::Entry::Terminal do } ) end + + context 'when FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'returns correct value without job_variables' do + expect(entry.value) + .to eq( + tag_list: ['webide'], + yaml_variables: [{ key: 'KEY', value: 'value', public: true }], + options: { + image: { name: "ruby:2.5" }, + services: [{ name: "mysql" }], + before_script: %w[ls pwd], + script: ['sleep 100'] + } + ) + end + end end end end diff --git a/spec/services/authorized_project_update/find_records_due_for_refresh_service_spec.rb b/spec/services/authorized_project_update/find_records_due_for_refresh_service_spec.rb new file mode 100644 index 00000000000..cdc93eaa365 --- /dev/null +++ b/spec/services/authorized_project_update/find_records_due_for_refresh_service_spec.rb @@ -0,0 +1,260 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do + # We're using let! here so that any expectations for the service class are not + # triggered twice. + let!(:project) { create(:project) } + + let(:user) { project.namespace.owner } + let(:service) { described_class.new(user) } + + describe '#execute' do + context 'callbacks' do + let(:callback) { double('callback') } + + context 'incorrect_auth_found_callback callback' do + let(:user) { create(:user) } + let(:service) do + described_class.new(user, + incorrect_auth_found_callback: callback) + end + + it 'is called' do + access_level = Gitlab::Access::DEVELOPER + create(:project_authorization, user: user, project: project, access_level: access_level) + + expect(callback).to receive(:call).with(project.id, access_level).once + + service.execute + end + end + + context 'missing_auth_found_callback callback' do + let(:service) do + described_class.new(user, + missing_auth_found_callback: callback) + end + + it 'is called' do + ProjectAuthorization.delete_all + + expect(callback).to receive(:call).with(project.id, Gitlab::Access::MAINTAINER).once + + service.execute + end + end + end + + context 'finding project authorizations due for refresh' do + context 'when there are changes to be made' do + before do + user.project_authorizations.delete_all + end + + it 'finds projects authorizations that needs to be refreshed' do + project2 = create(:project) + user.project_authorizations + .create!(project: project2, access_level: Gitlab::Access::MAINTAINER) + + to_be_removed = [project2.id] + to_be_added = [[user.id, project.id, Gitlab::Access::MAINTAINER]] + + expect(service.execute).to eq([to_be_removed, to_be_added]) + end + + it 'finds duplicate entries that has to be removed' do + [Gitlab::Access::MAINTAINER, Gitlab::Access::REPORTER].each do |access_level| + user.project_authorizations.create!(project: project, access_level: access_level) + end + + to_be_removed = [project.id] + to_be_added = [[user.id, project.id, Gitlab::Access::MAINTAINER]] + + expect(service.execute).to eq([to_be_removed, to_be_added]) + end + + it 'finds entries with wrong access levels' do + user.project_authorizations + .create!(project: project, access_level: Gitlab::Access::DEVELOPER) + + to_be_removed = [project.id] + to_be_added = [[user.id, project.id, Gitlab::Access::MAINTAINER]] + + expect(service.execute).to eq([to_be_removed, to_be_added]) + end + end + + context 'when there are no changes to be made' do + it 'returns empty arrays' do + expect(service.execute).to eq([[], []]) + end + end + end + end + + describe '#fresh_access_levels_per_project' do + let(:hash) { service.fresh_access_levels_per_project } + + it 'returns a Hash' do + expect(hash).to be_an_instance_of(Hash) + end + + it 'sets the keys to the project IDs' do + expect(hash.keys).to eq([project.id]) + end + + it 'sets the values to the access levels' do + expect(hash.values).to eq([Gitlab::Access::MAINTAINER]) + end + + context 'personal projects' do + it 'includes the project with the right access level' do + expect(hash[project.id]).to eq(Gitlab::Access::MAINTAINER) + end + end + + context 'projects the user is a member of' do + let!(:other_project) { create(:project) } + + before do + other_project.team.add_reporter(user) + end + + it 'includes the project with the right access level' do + expect(hash[other_project.id]).to eq(Gitlab::Access::REPORTER) + end + end + + context 'projects of groups the user is a member of' do + let(:group) { create(:group) } + let!(:other_project) { create(:project, group: group) } + + before do + group.add_owner(user) + end + + it 'includes the project with the right access level' do + expect(hash[other_project.id]).to eq(Gitlab::Access::OWNER) + end + end + + context 'projects of subgroups of groups the user is a member of' do + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + let!(:other_project) { create(:project, group: nested_group) } + + before do + group.add_maintainer(user) + end + + it 'includes the project with the right access level' do + expect(hash[other_project.id]).to eq(Gitlab::Access::MAINTAINER) + end + end + + context 'projects shared with groups the user is a member of' do + let(:group) { create(:group) } + let(:other_project) { create(:project) } + let!(:project_group_link) { create(:project_group_link, project: other_project, group: group, group_access: Gitlab::Access::GUEST) } + + before do + group.add_maintainer(user) + end + + it 'includes the project with the right access level' do + expect(hash[other_project.id]).to eq(Gitlab::Access::GUEST) + end + end + + context 'projects shared with subgroups of groups the user is a member of' do + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + let(:other_project) { create(:project) } + let!(:project_group_link) { create(:project_group_link, project: other_project, group: nested_group, group_access: Gitlab::Access::DEVELOPER) } + + before do + group.add_maintainer(user) + end + + it 'includes the project with the right access level' do + expect(hash[other_project.id]).to eq(Gitlab::Access::DEVELOPER) + end + end + end + + describe '#current_authorizations_per_project' do + let(:hash) { service.current_authorizations_per_project } + + it 'returns a Hash' do + expect(hash).to be_an_instance_of(Hash) + end + + it 'sets the keys to the project IDs' do + expect(hash.keys).to eq([project.id]) + end + + it 'sets the values to the project authorization rows' do + expect(hash.values.length).to eq(1) + + value = hash.values[0] + + expect(value.project_id).to eq(project.id) + expect(value.access_level).to eq(Gitlab::Access::MAINTAINER) + end + end + + describe '#current_authorizations' do + context 'without authorizations' do + it 'returns an empty list' do + user.project_authorizations.delete_all + + expect(service.current_authorizations.empty?).to eq(true) + end + end + + context 'with an authorization' do + let(:row) { service.current_authorizations.take } + + it 'returns the currently authorized projects' do + expect(service.current_authorizations.length).to eq(1) + end + + it 'includes the project ID for every row' do + expect(row.project_id).to eq(project.id) + end + + it 'includes the access level for every row' do + expect(row.access_level).to eq(Gitlab::Access::MAINTAINER) + end + end + end + + describe '#fresh_authorizations' do + it 'returns the new authorized projects' do + expect(service.fresh_authorizations.length).to eq(1) + end + + it 'returns the highest access level' do + project.team.add_guest(user) + + rows = service.fresh_authorizations.to_a + + expect(rows.length).to eq(1) + expect(rows.first.access_level).to eq(Gitlab::Access::MAINTAINER) + end + + context 'every returned row' do + let(:row) { service.fresh_authorizations.take } + + it 'includes the project ID' do + expect(row.project_id).to eq(project.id) + end + + it 'includes the access level' do + expect(row.access_level).to eq(Gitlab::Access::MAINTAINER) + end + end + end +end diff --git a/spec/services/authorized_project_update/recalculate_for_user_range_service_spec.rb b/spec/services/authorized_project_update/recalculate_for_user_range_service_spec.rb index 0c944cad40c..95e2c0380bf 100644 --- a/spec/services/authorized_project_update/recalculate_for_user_range_service_spec.rb +++ b/spec/services/authorized_project_update/recalculate_for_user_range_service_spec.rb @@ -7,12 +7,14 @@ RSpec.describe AuthorizedProjectUpdate::RecalculateForUserRangeService do let_it_be(:users) { create_list(:user, 2) } it 'calls Users::RefreshAuthorizedProjectsService' do - users.each do |user| + user_ids = users.map(&:id) + + User.where(id: user_ids).select(:id).each do |user| expect(Users::RefreshAuthorizedProjectsService).to( receive(:new).with(user, source: described_class.name).and_call_original) end - range = users.map(&:id).minmax + range = user_ids.minmax described_class.new(*range).execute end end diff --git a/spec/services/ci/create_pipeline_service/rules_spec.rb b/spec/services/ci/create_pipeline_service/rules_spec.rb index 66a3aa7771c..33ec6aacc44 100644 --- a/spec/services/ci/create_pipeline_service/rules_spec.rb +++ b/spec/services/ci/create_pipeline_service/rules_spec.rb @@ -151,11 +151,29 @@ RSpec.describe Ci::CreatePipelineService do context 'variables:' do let(:config) do <<-EOY - job: + variables: + VAR4: workflow var 4 + VAR5: workflow var 5 + VAR7: workflow var 7 + + workflow: + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + VAR4: overridden workflow var 4 + - if: $CI_COMMIT_REF_NAME =~ /feature/ + variables: + VAR5: overridden workflow var 5 + VAR6: new workflow var 6 + VAR7: overridden workflow var 7 + - when: always + + job1: script: "echo job1" variables: - VAR1: my var 1 - VAR2: my var 2 + VAR1: job var 1 + VAR2: job var 2 + VAR5: job var 5 rules: - if: $CI_COMMIT_REF_NAME =~ /master/ variables: @@ -164,45 +182,117 @@ RSpec.describe Ci::CreatePipelineService do variables: VAR2: overridden var 2 VAR3: new var 3 + VAR7: overridden var 7 + - when: on_success + + job2: + script: "echo job2" + inherit: + variables: [VAR4, VAR6, VAR7] + variables: + VAR4: job var 4 + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + VAR7: overridden var 7 - when: on_success EOY end - let(:job) { pipeline.builds.find_by(name: 'job') } + let(:job1) { pipeline.builds.find_by(name: 'job1') } + let(:job2) { pipeline.builds.find_by(name: 'job2') } + + let(:variable_keys) { %w(VAR1 VAR2 VAR3 VAR4 VAR5 VAR6 VAR7) } + + context 'when no match' do + let(:ref) { 'refs/heads/wip' } + + it 'does not affect vars' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['job var 1', 'job var 2', nil, 'workflow var 4', 'job var 5', nil, 'workflow var 7'] + ) + + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, nil, 'workflow var 7'] + ) + end + end context 'when matching to the first rule' do let(:ref) { 'refs/heads/master' } - it 'overrides VAR1' do - variables = job.scoped_variables.to_hash + it 'overrides variables' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['overridden var 1', 'job var 2', nil, 'overridden workflow var 4', 'job var 5', nil, 'workflow var 7'] + ) - expect(variables['VAR1']).to eq('overridden var 1') - expect(variables['VAR2']).to eq('my var 2') - expect(variables['VAR3']).to be_nil + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, nil, 'overridden var 7'] + ) + end + + context 'when FF ci_workflow_rules_variables is disabled' do + before do + stub_feature_flags(ci_workflow_rules_variables: false) + end + + it 'does not affect workflow variables but job variables' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['overridden var 1', 'job var 2', nil, 'workflow var 4', 'job var 5', nil, 'workflow var 7'] + ) + + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, nil, 'overridden var 7'] + ) + end end end context 'when matching to the second rule' do let(:ref) { 'refs/heads/feature' } - it 'overrides VAR2 and adds VAR3' do - variables = job.scoped_variables.to_hash + it 'overrides variables' do + expect(job1.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + ['job var 1', 'overridden var 2', 'new var 3', 'workflow var 4', 'job var 5', 'new workflow var 6', 'overridden var 7'] + ) - expect(variables['VAR1']).to eq('my var 1') - expect(variables['VAR2']).to eq('overridden var 2') - expect(variables['VAR3']).to eq('new var 3') + expect(job2.scoped_variables.to_hash.values_at(*variable_keys)).to eq( + [nil, nil, nil, 'job var 4', nil, 'new workflow var 6', 'overridden workflow var 7'] + ) end end - context 'when no match' do - let(:ref) { 'refs/heads/wip' } + context 'using calculated workflow var in job rules' do + let(:config) do + <<-EOY + variables: + VAR1: workflow var 4 + + workflow: + rules: + - if: $CI_COMMIT_REF_NAME =~ /master/ + variables: + VAR1: overridden workflow var 4 + - when: always + + job: + script: "echo job1" + rules: + - if: $VAR1 =~ "overridden workflow var 4" + variables: + VAR1: overridden var 1 + - when: on_success + EOY + end - it 'does not affect vars' do - variables = job.scoped_variables.to_hash + let(:job) { pipeline.builds.find_by(name: 'job') } + + context 'when matching the first workflow condition' do + let(:ref) { 'refs/heads/master' } - expect(variables['VAR1']).to eq('my var 1') - expect(variables['VAR2']).to eq('my var 2') - expect(variables['VAR3']).to be_nil + it 'uses VAR1 of job rules result' do + expect(job.scoped_variables.to_hash['VAR1']).to eq('overridden var 1') + end end end end diff --git a/spec/services/ide/terminal_config_service_spec.rb b/spec/services/ide/terminal_config_service_spec.rb index d6c4f7a2a69..2bfc8a7ff3c 100644 --- a/spec/services/ide/terminal_config_service_spec.rb +++ b/spec/services/ide/terminal_config_service_spec.rb @@ -47,6 +47,7 @@ RSpec.describe Ide::TerminalConfigService do terminal: { tag_list: [], yaml_variables: [], + job_variables: [], options: { script: ["sleep 60"] } }) end @@ -61,6 +62,7 @@ RSpec.describe Ide::TerminalConfigService do terminal: { tag_list: [], yaml_variables: [], + job_variables: [], options: { before_script: ["ls"], script: ["sleep 60"] } }) end diff --git a/spec/services/users/refresh_authorized_projects_service_spec.rb b/spec/services/users/refresh_authorized_projects_service_spec.rb index 1e74ff3d9eb..a8ad0d02f60 100644 --- a/spec/services/users/refresh_authorized_projects_service_spec.rb +++ b/spec/services/users/refresh_authorized_projects_service_spec.rb @@ -163,168 +163,4 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do service.update_authorizations([], [[user.id, project.id, Gitlab::Access::MAINTAINER]]) end end - - describe '#fresh_access_levels_per_project' do - let(:hash) { service.fresh_access_levels_per_project } - - it 'returns a Hash' do - expect(hash).to be_an_instance_of(Hash) - end - - it 'sets the keys to the project IDs' do - expect(hash.keys).to eq([project.id]) - end - - it 'sets the values to the access levels' do - expect(hash.values).to eq([Gitlab::Access::MAINTAINER]) - end - - context 'personal projects' do - it 'includes the project with the right access level' do - expect(hash[project.id]).to eq(Gitlab::Access::MAINTAINER) - end - end - - context 'projects the user is a member of' do - let!(:other_project) { create(:project) } - - before do - other_project.team.add_reporter(user) - end - - it 'includes the project with the right access level' do - expect(hash[other_project.id]).to eq(Gitlab::Access::REPORTER) - end - end - - context 'projects of groups the user is a member of' do - let(:group) { create(:group) } - let!(:other_project) { create(:project, group: group) } - - before do - group.add_owner(user) - end - - it 'includes the project with the right access level' do - expect(hash[other_project.id]).to eq(Gitlab::Access::OWNER) - end - end - - context 'projects of subgroups of groups the user is a member of' do - let(:group) { create(:group) } - let(:nested_group) { create(:group, parent: group) } - let!(:other_project) { create(:project, group: nested_group) } - - before do - group.add_maintainer(user) - end - - it 'includes the project with the right access level' do - expect(hash[other_project.id]).to eq(Gitlab::Access::MAINTAINER) - end - end - - context 'projects shared with groups the user is a member of' do - let(:group) { create(:group) } - let(:other_project) { create(:project) } - let!(:project_group_link) { create(:project_group_link, project: other_project, group: group, group_access: Gitlab::Access::GUEST) } - - before do - group.add_maintainer(user) - end - - it 'includes the project with the right access level' do - expect(hash[other_project.id]).to eq(Gitlab::Access::GUEST) - end - end - - context 'projects shared with subgroups of groups the user is a member of' do - let(:group) { create(:group) } - let(:nested_group) { create(:group, parent: group) } - let(:other_project) { create(:project) } - let!(:project_group_link) { create(:project_group_link, project: other_project, group: nested_group, group_access: Gitlab::Access::DEVELOPER) } - - before do - group.add_maintainer(user) - end - - it 'includes the project with the right access level' do - expect(hash[other_project.id]).to eq(Gitlab::Access::DEVELOPER) - end - end - end - - describe '#current_authorizations_per_project' do - let(:hash) { service.current_authorizations_per_project } - - it 'returns a Hash' do - expect(hash).to be_an_instance_of(Hash) - end - - it 'sets the keys to the project IDs' do - expect(hash.keys).to eq([project.id]) - end - - it 'sets the values to the project authorization rows' do - expect(hash.values.length).to eq(1) - - value = hash.values[0] - - expect(value.project_id).to eq(project.id) - expect(value.access_level).to eq(Gitlab::Access::MAINTAINER) - end - end - - describe '#current_authorizations' do - context 'without authorizations' do - it 'returns an empty list' do - user.project_authorizations.delete_all - - expect(service.current_authorizations.empty?).to eq(true) - end - end - - context 'with an authorization' do - let(:row) { service.current_authorizations.take } - - it 'returns the currently authorized projects' do - expect(service.current_authorizations.length).to eq(1) - end - - it 'includes the project ID for every row' do - expect(row.project_id).to eq(project.id) - end - - it 'includes the access level for every row' do - expect(row.access_level).to eq(Gitlab::Access::MAINTAINER) - end - end - end - - describe '#fresh_authorizations' do - it 'returns the new authorized projects' do - expect(service.fresh_authorizations.length).to eq(1) - end - - it 'returns the highest access level' do - project.team.add_guest(user) - - rows = service.fresh_authorizations.to_a - - expect(rows.length).to eq(1) - expect(rows.first.access_level).to eq(Gitlab::Access::MAINTAINER) - end - - context 'every returned row' do - let(:row) { service.fresh_authorizations.take } - - it 'includes the project ID' do - expect(row.project_id).to eq(project.id) - end - - it 'includes the access level' do - expect(row.access_level).to eq(Gitlab::Access::MAINTAINER) - end - end - end end diff --git a/spec/support/shared_examples/controllers/unique_visits_shared_examples.rb b/spec/support/shared_examples/controllers/unique_visits_shared_examples.rb index 428389a9a01..3f97c031e27 100644 --- a/spec/support/shared_examples/controllers/unique_visits_shared_examples.rb +++ b/spec/support/shared_examples/controllers/unique_visits_shared_examples.rb @@ -4,27 +4,30 @@ RSpec.shared_examples 'tracking unique visits' do |method| let(:request_params) { {} } it 'tracks unique visit if the format is HTML' do - expect_any_instance_of(Gitlab::Analytics::UniqueVisits).to receive(:track_visit).with(instance_of(String), target_id) + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event).with(target_id, values: kind_of(String)) get method, params: request_params, format: :html end it 'tracks unique visit if DNT is not enabled' do - expect_any_instance_of(Gitlab::Analytics::UniqueVisits).to receive(:track_visit).with(instance_of(String), target_id) + expect(Gitlab::UsageDataCounters::HLLRedisCounter) + .to receive(:track_event).with(target_id, values: kind_of(String)) + request.headers['DNT'] = '0' get method, params: request_params, format: :html end it 'does not track unique visit if DNT is enabled' do - expect_any_instance_of(Gitlab::Analytics::UniqueVisits).not_to receive(:track_visit) + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) request.headers['DNT'] = '1' get method, params: request_params, format: :html end it 'does not track unique visit if the format is JSON' do - expect_any_instance_of(Gitlab::Analytics::UniqueVisits).not_to receive(:track_visit) + expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event) get method, params: request_params, format: :json end diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb index 38dc6f0aebe..464fef13360 100644 --- a/spec/tooling/danger/project_helper_spec.rb +++ b/spec/tooling/danger/project_helper_spec.rb @@ -140,6 +140,7 @@ RSpec.describe Tooling::Danger::ProjectHelper do 'ee/db/geo/post_migrate/foo' | [:database, :migration] 'app/models/project_authorization.rb' | [:database] 'app/services/users/refresh_authorized_projects_service.rb' | [:database] + 'app/services/authorized_project_update/find_records_due_for_refresh_service.rb' | [:database] 'lib/gitlab/background_migration.rb' | [:database] 'lib/gitlab/background_migration/foo' | [:database] 'ee/lib/gitlab/background_migration/foo' | [:database] diff --git a/tooling/danger/project_helper.rb b/tooling/danger/project_helper.rb index 5f8103d659c..298aeb08737 100644 --- a/tooling/danger/project_helper.rb +++ b/tooling/danger/project_helper.rb @@ -78,6 +78,7 @@ module Tooling %r{\A(ee/)?db/(geo/)?(migrate|post_migrate)/} => [:database, :migration], %r{\A(ee/)?db/(?!fixtures)[^/]+} => :database, %r{\A(ee/)?lib/gitlab/(database|background_migration|sql|github_import)(/|\.rb)} => :database, + %r{\A(app/services/authorized_project_update/find_records_due_for_refresh_service)(/|\.rb)} => :database, %r{\A(app/models/project_authorization|app/services/users/refresh_authorized_projects_service)(/|\.rb)} => :database, %r{\A(ee/)?app/finders/} => :database, %r{\Arubocop/cop/migration(/|\.rb)} => :database, |