Skip to content

G
gitlab-ce
Commit b8410b37 authored by Furkan Ayhan's avatar Furkan Ayhan
Browse files
Options

Merge branch '351995-cleanup-variables-memoization' into 'master' 

Cleanup secret variables refactoring in variables builder

See merge request gitlab-org/gitlab!83428
parents 5ff1764a b668969c
Show whitespace changes
Inline Side-by-side
Showing with 123 additions and 194 deletions
config/feature_flags/development/ci_variables_builder_memoize_secret_variables.yml deleted 100644 → 0
View file @ 5ff1764a
---
name: ci_variables_builder_memoize_secret_variables
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79850
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351995
milestone: '14.8'
type: development
group: group::pipeline execution
default_enabled: false
lib/gitlab/ci/variables/builder.rb
View file @ b8410b37
......@@ -24,8 +24,8 @@ module Gitlab
variables.concat(user_variables(job.user))
variables.concat(job.dependency_variables) if dependencies
variables.concat(secret_instance_variables)
variables.concat(secret_group_variables(environment: environment, ref: job.git_ref))
variables.concat(secret_project_variables(environment: environment, ref: job.git_ref))
variables.concat(secret_group_variables(environment: environment))
variables.concat(secret_project_variables(environment: environment))
variables.concat(job.trigger_request.user_variables) if job.trigger_request
variables.concat(pipeline.variables)
variables.concat(pipeline.pipeline_schedule.job_variables) if pipeline.pipeline_schedule
......@@ -75,21 +75,21 @@ module Gitlab
end
end
def secret_group_variables(environment:, ref:)
if memoize_secret_variables?
memoized_secret_group_variables(environment: environment)
else
return [] unless project.group
project.group.ci_variables_for(ref, project, environment: environment)
def secret_group_variables(environment:)
strong_memoize_with(:secret_group_variables, environment) do
group_variables_builder
.secret_variables(
environment: environment,
protected_ref: protected_ref?)
end
end
def secret_project_variables(environment:, ref:)
if memoize_secret_variables?
memoized_secret_project_variables(environment: environment)
else
project.ci_variables_for(ref: ref, environment: environment)
def secret_project_variables(environment:)
strong_memoize_with(:secret_project_variables, environment) do
project_variables_builder
.secret_variables(
environment: environment,
protected_ref: protected_ref?)
end
end
......@@ -120,24 +120,6 @@ module Gitlab
end
end
def memoized_secret_project_variables(environment:)
strong_memoize_with(:secret_project_variables, environment) do
project_variables_builder
.secret_variables(
environment: environment,
protected_ref: protected_ref?)
end
end
def memoized_secret_group_variables(environment:)
strong_memoize_with(:secret_group_variables, environment) do
group_variables_builder
.secret_variables(
environment: environment,
protected_ref: protected_ref?)
end
end
def ci_node_total_value(job)
parallel = job.options&.dig(:parallel)
parallel = parallel.dig(:total) if parallel.is_a?(Hash)
......@@ -150,14 +132,6 @@ module Gitlab
end
end
def memoize_secret_variables?
strong_memoize(:memoize_secret_variables) do
::Feature.enabled?(:ci_variables_builder_memoize_secret_variables,
project,
default_enabled: :yaml)
end
end
def strong_memoize_with(name, *args)
container = strong_memoize(name) { {} }
......
spec/lib/gitlab/ci/variables/builder_spec.rb
View file @ b8410b37
......@@ -278,6 +278,14 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
end
shared_examples "secret CI variables" do
let(:protected_variable_item) do
Gitlab::Ci::Variables::Collection::Item.fabricate(protected_variable)
end
let(:unprotected_variable_item) do
Gitlab::Ci::Variables::Collection::Item.fabricate(unprotected_variable)
end
context 'when ref is branch' do
context 'when ref is protected' do
before do
......@@ -338,43 +346,20 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
let_it_be(:protected_variable) { create(:ci_instance_variable, protected: true) }
let_it_be(:unprotected_variable) { create(:ci_instance_variable, protected: false) }
let(:protected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(protected_variable) }
let(:unprotected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(unprotected_variable) }
include_examples "secret CI variables"
end
describe '#secret_group_variables' do
subject { builder.secret_group_variables(ref: job.git_ref, environment: job.expanded_environment_name) }
subject { builder.secret_group_variables(environment: job.expanded_environment_name) }
let_it_be(:protected_variable) { create(:ci_group_variable, protected: true, group: group) }
let_it_be(:unprotected_variable) { create(:ci_group_variable, protected: false, group: group) }
context 'with ci_variables_builder_memoize_secret_variables disabled' do
before do
stub_feature_flags(ci_variables_builder_memoize_secret_variables: false)
end
let(:protected_variable_item) { protected_variable }
let(:unprotected_variable_item) { unprotected_variable }
include_examples "secret CI variables"
end
context 'with ci_variables_builder_memoize_secret_variables enabled' do
before do
stub_feature_flags(ci_variables_builder_memoize_secret_variables: true)
end
let(:protected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(protected_variable) }
let(:unprotected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(unprotected_variable) }
include_examples "secret CI variables"
context 'variables memoization' do
let_it_be(:scoped_variable) { create(:ci_group_variable, group: group, environment_scope: 'scoped') }
let(:ref) { job.git_ref }
let(:environment) { job.expanded_environment_name }
let(:scoped_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(scoped_variable) }
......@@ -394,7 +379,7 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
end
2.times do
expect(builder.secret_group_variables(ref: ref, environment: 'production'))
expect(builder.secret_group_variables(environment: 'production'))
.to contain_exactly(unprotected_variable_item, protected_variable_item)
end
end
......@@ -422,45 +407,24 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
end
2.times do
expect(builder.secret_group_variables(ref: 'other', environment: nil))
expect(builder.secret_group_variables(environment: nil))
.to contain_exactly(unprotected_variable_item)
expect(builder.secret_group_variables(ref: 'other', environment: 'scoped'))
expect(builder.secret_group_variables(environment: 'scoped'))
.to contain_exactly(unprotected_variable_item, scoped_variable_item)
end
end
end
end
end
end
describe '#secret_project_variables' do
let_it_be(:protected_variable) { create(:ci_variable, protected: true, project: project) }
let_it_be(:unprotected_variable) { create(:ci_variable, protected: false, project: project) }
let(:ref) { job.git_ref }
let(:environment) { job.expanded_environment_name }
subject { builder.secret_project_variables(ref: ref, environment: environment) }
context 'with ci_variables_builder_memoize_secret_variables disabled' do
before do
stub_feature_flags(ci_variables_builder_memoize_secret_variables: false)
end
let(:protected_variable_item) { protected_variable }
let(:unprotected_variable_item) { unprotected_variable }
include_examples "secret CI variables"
end
context 'with ci_variables_builder_memoize_secret_variables enabled' do
before do
stub_feature_flags(ci_variables_builder_memoize_secret_variables: true)
end
let(:protected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(protected_variable) }
let(:unprotected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(unprotected_variable) }
subject { builder.secret_project_variables(environment: environment) }
include_examples "secret CI variables"
......@@ -485,7 +449,7 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
end
2.times do
expect(builder.secret_project_variables(ref: ref, environment: 'production'))
expect(builder.secret_project_variables(environment: 'production'))
.to contain_exactly(unprotected_variable_item, protected_variable_item)
end
end
......@@ -513,15 +477,14 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
end
2.times do
expect(builder.secret_project_variables(ref: 'other', environment: nil))
expect(builder.secret_project_variables(environment: nil))
.to contain_exactly(unprotected_variable_item)
expect(builder.secret_project_variables(ref: 'other', environment: 'scoped'))
expect(builder.secret_project_variables(environment: 'scoped'))
.to contain_exactly(unprotected_variable_item, scoped_variable_item)
end
end
end
end
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7