diff options
28 files changed, 81 insertions, 74 deletions
@@ -360,7 +360,7 @@ group :development, :test do gem 'awesome_print', require: false gem 'database_cleaner', '~> 1.7.0' - gem 'factory_bot_rails', '~> 5.1.0' + gem 'factory_bot_rails', '~> 6.1.0' gem 'rspec-rails', '~> 4.0.1' # Prevent occasions where minitest is not bundled in packaged versions of ruby (see #3826) diff --git a/Gemfile.lock b/Gemfile.lock index 539657482c2..d3d0e996915 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -321,11 +321,11 @@ GEM expression_parser (0.9.0) extended-markdown-filter (0.6.0) html-pipeline (~> 2.0) - factory_bot (5.1.0) - activesupport (>= 4.2.0) - factory_bot_rails (5.1.0) - factory_bot (~> 5.1.0) - railties (>= 4.2.0) + factory_bot (6.1.0) + activesupport (>= 5.0.0) + factory_bot_rails (6.1.0) + factory_bot (~> 6.1.0) + railties (>= 5.0.0) faraday (1.0.1) multipart-post (>= 1.2, < 3) faraday-cookie_jar (0.0.7) @@ -1331,7 +1331,7 @@ DEPENDENCIES email_spec (~> 2.2.0) erubi (~> 1.9.0) escape_utils (~> 1.1) - factory_bot_rails (~> 5.1.0) + factory_bot_rails (~> 6.1.0) faraday (~> 1.0) faraday_middleware-aws-sigv4 (~> 0.3.0) fast_blank diff --git a/app/models/terraform/state.rb b/app/models/terraform/state.rb index 1b99f310e1a..efbbd86ae4a 100644 --- a/app/models/terraform/state.rb +++ b/app/models/terraform/state.rb @@ -27,6 +27,8 @@ module Terraform validates :uuid, presence: true, uniqueness: true, length: { is: UUID_LENGTH }, format: { with: HEX_REGEXP, message: 'only allows hex characters' } + before_destroy :ensure_state_is_unlocked + default_value_for(:uuid, allows_nil: false) { SecureRandom.hex(UUID_LENGTH / 2) } def latest_file @@ -87,6 +89,13 @@ module Terraform new_version.save! end + def ensure_state_is_unlocked + return unless locked? + + errors.add(:base, s_("Terraform|You cannot remove the State file because it's locked. Unlock the State file first before removing it.")) + throw :abort # rubocop:disable Cop/BanCatchThrow + end + def parse_serial(file) Gitlab::Json.parse(file)["serial"] rescue JSON::ParserError diff --git a/changelogs/unreleased/id-update-factory-bot.yml b/changelogs/unreleased/id-update-factory-bot.yml new file mode 100644 index 00000000000..3a943237678 --- /dev/null +++ b/changelogs/unreleased/id-update-factory-bot.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade factory_bot_rails to 6.1.0 +merge_request: 50875 +author: +type: other diff --git a/changelogs/unreleased/state-lock-delete-validation.yml b/changelogs/unreleased/state-lock-delete-validation.yml new file mode 100644 index 00000000000..eb4c8337bf2 --- /dev/null +++ b/changelogs/unreleased/state-lock-delete-validation.yml @@ -0,0 +1,5 @@ +--- +title: Prevent locked Terraform states from being deleted +merge_request: 50798 +author: +type: changed diff --git a/config/feature_flags/experiment/null_hypothesis.yml b/config/feature_flags/experiment/null_hypothesis.yml new file mode 100644 index 00000000000..716b0711ef1 --- /dev/null +++ b/config/feature_flags/experiment/null_hypothesis.yml @@ -0,0 +1,7 @@ +--- +name: null_hypothesis +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45840 +rollout_issue_url: +type: experiment +group: group::adoption +default_enabled: false diff --git a/doc/.vale/gitlab/Admin.yml b/doc/.vale/gitlab/Admin.yml index 6d01882138a..96325ad2ef4 100644 --- a/doc/.vale/gitlab/Admin.yml +++ b/doc/.vale/gitlab/Admin.yml @@ -1,7 +1,7 @@ --- # Warning: gitlab.Admin # -# You should not use "admin", but "Admin Area" is OK. +# Checks for "admin" and recommends using the full word instead. "Admin Area" is OK. # # For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles extends: substitution diff --git a/doc/.vale/gitlab/AlertBoxStyle.yml b/doc/.vale/gitlab/AlertBoxStyle.yml index 78db7fa99ae..92c3a16cab8 100644 --- a/doc/.vale/gitlab/AlertBoxStyle.yml +++ b/doc/.vale/gitlab/AlertBoxStyle.yml @@ -1,9 +1,8 @@ --- # Error: gitlab.AlertBoxStyle # -# Makes sure alert boxes are used with block quotes. +# Makes sure alert boxes are used with block quotes. Checks for 3 formatting issues: # -# Checks for 3 formatting issues: # - Alert boxes inside a block quote (">") # - Alert boxes with the note text on the same line # - Alert boxes using words other than "NOTE" or "WARNING" diff --git a/doc/.vale/gitlab/British.yml b/doc/.vale/gitlab/British.yml index 65842ad7aa5..152723ead26 100644 --- a/doc/.vale/gitlab/British.yml +++ b/doc/.vale/gitlab/British.yml @@ -1,7 +1,7 @@ --- # Error: gitlab.British # -# Checks that US spelling is used over British spelling. +# Checks that US spelling is used instead of British spelling. # # For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles extends: substitution diff --git a/doc/.vale/gitlab/CurlStringsQuoted.yml b/doc/.vale/gitlab/CurlStringsQuoted.yml index 32bf662f2ae..c0bc8c18c93 100644 --- a/doc/.vale/gitlab/CurlStringsQuoted.yml +++ b/doc/.vale/gitlab/CurlStringsQuoted.yml @@ -1,7 +1,7 @@ --- # Error: gitlab.CurlStringsQuoted # -# Ensures all code blocks using curl quote any URL strings. +# Ensures all code blocks using `curl` wrap URL strings in quotation marks. # # For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles extends: existence diff --git a/doc/.vale/gitlab/FutureTense.yml b/doc/.vale/gitlab/FutureTense.yml index d7001d57899..fc414a9c0fd 100644 --- a/doc/.vale/gitlab/FutureTense.yml +++ b/doc/.vale/gitlab/FutureTense.yml @@ -1,8 +1,7 @@ --- # Suggestion: gitlab.FutureTense # -# Checks for use of future tense in sentences. Present tense is preferred as -# much as possible. +# Checks for use of future tense in sentences. Present tense is strongly preferred. # # For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles extends: existence diff --git a/doc/.vale/gitlab/OxfordComma.yml b/doc/.vale/gitlab/OxfordComma.yml index 0425d9e1dd0..1716145b26a 100644 --- a/doc/.vale/gitlab/OxfordComma.yml +++ b/doc/.vale/gitlab/OxfordComma.yml @@ -1,8 +1,7 @@ --- # Warning: gitlab.OxfordComma # -# Checks for the lack of an Oxford comma. In some cases, will catch overly -# complex sentence structures with lots of commas. +# Checks for the lack of an Oxford comma. In some cases, will catch overly complex sentence structures with lots of commas. # # For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles extends: existence diff --git a/doc/.vale/gitlab/ReferenceLinks.yml b/doc/.vale/gitlab/ReferenceLinks.yml index 0df1f359107..160ed2e8e14 100644 --- a/doc/.vale/gitlab/ReferenceLinks.yml +++ b/doc/.vale/gitlab/ReferenceLinks.yml @@ -1,7 +1,7 @@ --- # Error: gitlab.ReferenceLinks # -# Checks for the presence of reference-style links that must be inline. +# Checks for reference-style links that should be converted to inline links. # # For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles extends: existence diff --git a/doc/.vale/gitlab/SentenceSpacing.yml b/doc/.vale/gitlab/SentenceSpacing.yml index 884d4b57508..0288abe834f 100644 --- a/doc/.vale/gitlab/SentenceSpacing.yml +++ b/doc/.vale/gitlab/SentenceSpacing.yml @@ -1,10 +1,7 @@ --- # Error: gitlab.SentenceSpacing # -# Checks for the following in common content scenarios: -# -# - No spaces. -# - More than one space. +# Checks for incorrect spacing (no spaces, or more than one space) around punctuation. # # For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles extends: existence diff --git a/doc/.vale/gitlab/Spelling.yml b/doc/.vale/gitlab/Spelling.yml index 602b7cd11e6..4ebaf7bfb70 100644 --- a/doc/.vale/gitlab/Spelling.yml +++ b/doc/.vale/gitlab/Spelling.yml @@ -1,8 +1,7 @@ --- # Warning: gitlab.Spelling # -# Checks for possible spelling mistakes in content, not code. May find false positives -# due to links using angle brackets: <https://example.com>. These can be ignored. +# Checks for possible spelling mistakes in content, not code. Results from links using angle brackets (<https://example.com>) should be corrected. # # If a word is flagged as a spelling mistake incorrectly, such as a product name, # you can submit an MR to update `spelling-exceptions.txt` with the missing word. diff --git a/doc/.vale/gitlab/SubstitutionSuggestions.yml b/doc/.vale/gitlab/SubstitutionSuggestions.yml index 82e3e789864..735a39119d7 100644 --- a/doc/.vale/gitlab/SubstitutionSuggestions.yml +++ b/doc/.vale/gitlab/SubstitutionSuggestions.yml @@ -1,8 +1,8 @@ --- # Suggestion: gitlab.SubstitutionSuggestions # -# Suggests better options for frequently misused terms at GitLab that are -# often - but not always - incorrect. +# Suggests better options for frequently misused terms that are often - but not always - incorrect. +# SubstitutionWarning.yml and Substitutions.yml also exist. # # For a list of all options, see https://errata-ai.github.io/vale/styles/ extends: substitution diff --git a/doc/.vale/gitlab/SubstitutionWarning.yml b/doc/.vale/gitlab/SubstitutionWarning.yml index ed0f8b498fe..61fd0148fd8 100644 --- a/doc/.vale/gitlab/SubstitutionWarning.yml +++ b/doc/.vale/gitlab/SubstitutionWarning.yml @@ -1,8 +1,8 @@ --- # Warning: gitlab.SubstitutionWarning # -# Warns against using common shorthand for terms. -# For substitutions flagged as errors, see Substitutions.yml +# Checks for misused terms or common shorthand that should never be used at GitLab, but can't be flagged as errors. +# Substitutions.yml and SubstitionSuggestions.yml also exist. # # For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles extends: substitution diff --git a/doc/.vale/gitlab/Substitutions.yml b/doc/.vale/gitlab/Substitutions.yml index 0a56524e3f5..987785c7a22 100644 --- a/doc/.vale/gitlab/Substitutions.yml +++ b/doc/.vale/gitlab/Substitutions.yml @@ -1,8 +1,8 @@ --- # Error: gitlab.Substitutions # -# Checks for use of some of the top misused terms at GitLab. -# For substitutions only flagged as warnings, see SubstitutionWarning.yml +# Checks for misused terms that should never be used at GitLab. +# SubstitutionWarning.yml and SubstitionSuggestions.yml also exist. # # For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles extends: substitution diff --git a/lib/feature/shared.rb b/lib/feature/shared.rb index a3f02156d94..17dfe26bd82 100644 --- a/lib/feature/shared.rb +++ b/lib/feature/shared.rb @@ -57,8 +57,6 @@ class Feature default_enabled: false, example: <<-EOS experiment(:my_experiment, project: project, actor: current_user) { ...variant code... } - # or - Gitlab::Experimentation.in_experiment_group?(:my_experiment, subject: current_user) EOS } }.freeze diff --git a/lib/gitlab/experimentation/experiment.rb b/lib/gitlab/experimentation/experiment.rb index 36cd673a38f..e594c3bedeb 100644 --- a/lib/gitlab/experimentation/experiment.rb +++ b/lib/gitlab/experimentation/experiment.rb @@ -3,21 +3,17 @@ module Gitlab module Experimentation class Experiment - FEATURE_FLAG_SUFFIX = "_experiment_percentage" - attr_reader :key, :tracking_category, :use_backwards_compatible_subject_index def initialize(key, **params) @key = key @tracking_category = params[:tracking_category] @use_backwards_compatible_subject_index = params[:use_backwards_compatible_subject_index] + + @experiment_percentage = Feature.get(:"#{key}_experiment_percentage").percentage_of_time_value # rubocop:disable Gitlab/AvoidFeatureGet end def active? - # TODO: just touch a feature flag - # Temporary change, we will change `experiment_percentage` in future to `Feature.enabled? - Feature.enabled?(feature_flag_name, type: :experiment, default_enabled: :yaml) - ::Gitlab.dev_env_or_com? && experiment_percentage > 0 end @@ -29,17 +25,7 @@ module Gitlab private - def experiment_percentage - feature_flag.percentage_of_time_value - end - - def feature_flag - Feature.get(feature_flag_name) # rubocop:disable Gitlab/AvoidFeatureGet - end - - def feature_flag_name - :"#{key}#{FEATURE_FLAG_SUFFIX}" - end + attr_reader :experiment_percentage end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 36f1b690507..3e76854169a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -27549,6 +27549,9 @@ msgstr "" msgid "Terraform|You are about to remove the State file %{name}. This will permanently delete all the State versions and history. The infrastructure provisioned previously\twill remain intact, only the state file with all its versions are to be removed. This action is non-revertible." msgstr "" +msgid "Terraform|You cannot remove the State file because it's locked. Unlock the State file first before removing it." +msgstr "" + msgid "Test" msgstr "" diff --git a/spec/lib/gitlab/experimentation/experiment_spec.rb b/spec/lib/gitlab/experimentation/experiment_spec.rb index 008e6699597..7b1d1763010 100644 --- a/spec/lib/gitlab/experimentation/experiment_spec.rb +++ b/spec/lib/gitlab/experimentation/experiment_spec.rb @@ -14,10 +14,8 @@ RSpec.describe Gitlab::Experimentation::Experiment do end before do - skip_feature_flags_yaml_validation - skip_default_enabled_yaml_check - feature = double('FeatureFlag', percentage_of_time_value: percentage, enabled?: true) - allow(Feature).to receive(:get).with(:experiment_key_experiment_percentage).and_return(feature) + feature = double('FeatureFlag', percentage_of_time_value: percentage ) + expect(Feature).to receive(:get).with(:experiment_key_experiment_percentage).and_return(feature) end subject(:experiment) { described_class.new(:experiment_key, **params) } diff --git a/spec/lib/gitlab/experimentation_spec.rb b/spec/lib/gitlab/experimentation_spec.rb index b503960b8c7..a68c050d829 100644 --- a/spec/lib/gitlab/experimentation_spec.rb +++ b/spec/lib/gitlab/experimentation_spec.rb @@ -38,8 +38,6 @@ RSpec.describe Gitlab::Experimentation do } }) - skip_feature_flags_yaml_validation - skip_default_enabled_yaml_check Feature.enable_percentage_of_time(:backwards_compatible_test_experiment_experiment_percentage, enabled_percentage) Feature.enable_percentage_of_time(:test_experiment_experiment_percentage, enabled_percentage) allow(Gitlab).to receive(:com?).and_return(true) diff --git a/spec/models/terraform/state_spec.rb b/spec/models/terraform/state_spec.rb index ef91e4a5a71..ed311314086 100644 --- a/spec/models/terraform/state_spec.rb +++ b/spec/models/terraform/state_spec.rb @@ -27,6 +27,22 @@ RSpec.describe Terraform::State do end end + describe '#destroy' do + let(:terraform_state) { create(:terraform_state) } + let(:user) { terraform_state.project.creator } + + it 'deletes when the state is unlocked' do + expect(terraform_state.destroy).to be_truthy + end + + it 'fails to delete when the state is locked', :aggregate_failures do + terraform_state.update!(lock_xid: SecureRandom.uuid, locked_by_user: user, locked_at: Time.current) + + expect(terraform_state.destroy).to be_falsey + expect(terraform_state.errors.full_messages).to eq(["You cannot remove the State file because it's locked. Unlock the State file first before removing it."]) + end + end + describe '#latest_file' do let(:terraform_state) { create(:terraform_state, :with_version) } let(:latest_version) { terraform_state.latest_version } diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 1d99f042c06..34f34c0b850 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -196,8 +196,8 @@ RSpec.describe 'Rack Attack global throttles' do let(:api_partial_url) { '/todos' } context 'with the token in the query string' do - let(:request_args) { [api(api_partial_url, personal_access_token: token)] } - let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token)] } + let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } + let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } it_behaves_like 'rate-limited token-authenticated requests' end @@ -258,8 +258,8 @@ RSpec.describe 'Rack Attack global throttles' do let(:api_partial_url) { '/todos' } context 'with the token in the query string' do - let(:request_args) { [api(api_partial_url, oauth_access_token: token)] } - let(:other_user_request_args) { [api(api_partial_url, oauth_access_token: other_user_token)] } + let(:request_args) { [api(api_partial_url, oauth_access_token: token), {}] } + let(:other_user_request_args) { [api(api_partial_url, oauth_access_token: other_user_token), {}] } it_behaves_like 'rate-limited token-authenticated requests' end @@ -376,8 +376,8 @@ RSpec.describe 'Rack Attack global throttles' do end context 'with the token in the query string' do - let(:request_args) { [api(api_partial_url, personal_access_token: token)] } - let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token)] } + let(:request_args) { [api(api_partial_url, personal_access_token: token), {}] } + let(:other_user_request_args) { [api(api_partial_url, personal_access_token: other_user_token), {}] } it_behaves_like 'rate-limited token-authenticated requests' end diff --git a/spec/support/helpers/stub_experiments.rb b/spec/support/helpers/stub_experiments.rb index 408d16a7c08..247692d83ee 100644 --- a/spec/support/helpers/stub_experiments.rb +++ b/spec/support/helpers/stub_experiments.rb @@ -11,7 +11,6 @@ module StubExperiments allow(Gitlab::Experimentation).to receive(:active?).and_call_original experiments.each do |experiment_key, enabled| - Feature.persist_used!("#{experiment_key}#{feature_flag_suffix}") allow(Gitlab::Experimentation).to receive(:active?).with(experiment_key) { enabled } end end @@ -26,14 +25,7 @@ module StubExperiments allow(Gitlab::Experimentation).to receive(:in_experiment_group?).and_call_original experiments.each do |experiment_key, enabled| - Feature.persist_used!("#{experiment_key}#{feature_flag_suffix}") allow(Gitlab::Experimentation).to receive(:in_experiment_group?).with(experiment_key, anything) { enabled } end end - - private - - def feature_flag_suffix - Gitlab::Experimentation::Experiment::FEATURE_FLAG_SUFFIX - end end diff --git a/spec/support/helpers/stub_feature_flags.rb b/spec/support/helpers/stub_feature_flags.rb index 77f31169ecb..7f30a2a70cd 100644 --- a/spec/support/helpers/stub_feature_flags.rb +++ b/spec/support/helpers/stub_feature_flags.rb @@ -66,8 +66,4 @@ module StubFeatureFlags def skip_feature_flags_yaml_validation allow(Feature::Definition).to receive(:valid_usage!) end - - def skip_default_enabled_yaml_check - allow(Feature::Definition).to receive(:default_enabled?).and_return(false) - end end diff --git a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb index 5d300d38e4a..3b039049ca9 100644 --- a/spec/support/shared_examples/requests/rack_attack_shared_examples.rb +++ b/spec/support/shared_examples/requests/rack_attack_shared_examples.rb @@ -154,10 +154,11 @@ RSpec.shared_examples 'rate-limited token-authenticated requests' do end def make_request(args) + path, options = args if request_method == 'POST' - post(*args) + post(path, **options) else - get(*args) + get(path, **options) end end end |