Add latest changes from gitlab-org/gitlab@master

app/models/clusters/cluster.rb

@@ -101,7 +101,7 @@ module Clusters
     scope :disabled, -> { where(enabled: false) }
     scope :user_provided, -> { where(provider_type: ::Clusters::Cluster.provider_types[:user]) }
     scope :gcp_provided, -> { where(provider_type: ::Clusters::Cluster.provider_types[:gcp]) }
-    scope :gcp_installed, -> { gcp_provided.includes(:provider_gcp).where(cluster_providers_gcp: { status: ::Clusters::Providers::Gcp.state_machines[:status].states[:created].value }) }
+    scope :gcp_installed, -> { gcp_provided.joins(:provider_gcp).merge(Clusters::Providers::Gcp.with_status(:created)) }
     scope :managed, -> { where(managed: true) }
 
     scope :default_environment, -> { where(environment_scope: DEFAULT_ENVIRONMENT) }

app/models/clusters/concerns/provider_status.rb

+# frozen_string_literal: true
+
+module Clusters
+  module Concerns
+    module ProviderStatus
+      extend ActiveSupport::Concern
+
+      included do
+        state_machine :status, initial: :scheduled do
+          state :scheduled, value: 1
+          state :creating, value: 2
+          state :created, value: 3
+          state :errored, value: 4
+
+          event :make_creating do
+            transition any - [:creating] => :creating
+          end
+
+          event :make_created do
+            transition any - [:created] => :created
+          end
+
+          event :make_errored do
+            transition any - [:errored] => :errored
+          end
+
+          before_transition any => [:errored, :created] do |provider|
+            provider.nullify_credentials
+          end
+
+          before_transition any => [:creating] do |provider, transition|
+            operation_id = transition.args.first
+            provider.assign_operation_id(operation_id) if operation_id
+          end
+
+          before_transition any => [:errored] do |provider, transition|
+            status_reason = transition.args.first
+            provider.status_reason = status_reason if status_reason
+          end
+        end
+
+        def on_creation?
+          scheduled? || creating?
+        end
+      end
+    end
+  end
+end

app/models/clusters/providers/gcp.rb

@@ -3,6 +3,8 @@
 module Clusters
   module Providers
     class Gcp < ApplicationRecord
+      include Clusters::Concerns::ProviderStatus
+
       self.table_name = 'cluster_providers_gcp'
 
       belongs_to :cluster, inverse_of: :provider_gcp, class_name: 'Clusters::Cluster'
@@ -35,50 +37,21 @@ module Clusters
           greater_than: 0
         }
 
-      state_machine :status, initial: :scheduled do
-        state :scheduled, value: 1
-        state :creating, value: 2
-        state :created, value: 3
-        state :errored, value: 4
-
-        event :make_creating do
-          transition any - [:creating] => :creating
-        end
-
-        event :make_created do
-          transition any - [:created] => :created
-        end
-
-        event :make_errored do
-          transition any - [:errored] => :errored
-        end
-
-        before_transition any => [:errored, :created] do |provider|
-          provider.access_token = nil
-          provider.operation_id = nil
-        end
-
-        before_transition any => [:creating] do |provider, transition|
-          operation_id = transition.args.first
-          raise ArgumentError.new('operation_id is required') unless operation_id.present?
-
-          provider.operation_id = operation_id
-        end
+      def api_client
+        return unless access_token
 
-        before_transition any => [:errored] do |provider, transition|
-          status_reason = transition.args.first
-          provider.status_reason = status_reason if status_reason
-        end
+        @api_client ||= GoogleApi::CloudPlatform::Client.new(access_token, nil)
       end
 
-      def on_creation?
-        scheduled? || creating?
+      def nullify_credentials
+        assign_attributes(
+          access_token: nil,
+          operation_id: nil
+        )
       end
 
-      def api_client
-        return unless access_token
-
-        @api_client ||= GoogleApi::CloudPlatform::Client.new(access_token, nil)
+      def assign_operation_id(operation_id)
+        assign_attributes(operation_id: operation_id)
       end
 
       def knative_pre_installed?

doc/development/README.md

@@ -94,6 +94,7 @@ description: 'Learn how to contribute to GitLab.'
 - [What requires downtime?](what_requires_downtime.md)
 - [SQL guidelines](sql.md) for working with SQL queries
 - [Migrations style guide](migration_style_guide.md) for creating safe SQL migrations
+- [Testing Rails migrations](testing_guide/testing_migrations_guide.md) guide
 - [Post deployment migrations](post_deployment_migrations.md)
 - [Background migrations](background_migrations.md)
 - [Swapping tables](swapping_tables.md)

doc/development/background_migrations.md

@@ -290,7 +290,9 @@ It is required to write tests for:
 - A cleanup migration.
 
 You can use the `:migration` RSpec tag when testing the migrations.
-See [README][migrations-readme].
+See the
+[Testing Rails migrations](testing_guide/testing_migrations_guide.md#testing-a-non-activerecordmigration-class)
+style guide.
 
 When you do that, keep in mind that `before` and `after` RSpec hooks are going
 to migrate you database down and up, which can result in other background

doc/development/migration_style_guide.md

@@ -407,10 +407,7 @@ end
 
 ## Testing
 
-Make sure that your migration works for databases with data. An
-empty database does not guarantee that your migration is correct.
-
-Make sure your migration can be reversed.
+See the [Testing Rails migrations](testing_guide/testing_migrations_guide.md) style guide.
 
 ## Data migration
 

doc/development/testing_guide/testing_levels.md

@@ -44,7 +44,7 @@ records should use stubs/doubles as much as possible.
 | `config/routes.rb`, `config/routes/` | `spec/routing/` | RSpec | |
 | `config/puma.example.development.rb`, `config/unicorn.rb.example` | `spec/rack_servers/` | RSpec | |
 | `db/` | `spec/db/` | RSpec | |
-| `db/{post_,}migrate/` | `spec/migrations/` | RSpec | More details at [`spec/migrations/README.md`](https://gitlab.com/gitlab-org/gitlab/blob/master/spec/migrations/README.md). |
+| `db/{post_,}migrate/` | `spec/migrations/` | RSpec | More details in the [Testing Rails migrations guide](testing_migrations_guide.md). |
 | `Gemfile` | `spec/dependencies/`, `spec/sidekiq/` | RSpec | |
 | `lib/` | `spec/lib/` | RSpec | |
 | `lib/tasks/` | `spec/tasks/` | RSpec | |

doc/development/testing_guide/testing_migrations_guide.md

+---
+type: reference
+---
+
+# Testing Rails migrations at GitLab
+
+In order to reliably check Rails migrations, we need to test them against
+a database schema.
+
+## When to write a migration test
+
+- Post migrations (`/db/post_migrate`) and background migrations
+  (`lib/gitlab/background_migration`) **must** have migration tests performed.
+- If your migration is a data migration then it **must** have a migration test.
+- Other migrations may have a migration test if necessary.
+
+## How does it work?
+
+Adding a `:migration` tag to a test signature enables some custom RSpec
+`before` and `after` hooks in our
+[`spec_helper.rb`](https://gitlab.com/gitlab-org/gitlab/blob/3b29908a64ff729c0cf6d93452fe00ab23079c75/spec%2Fspec_helper.rb#L259)
+to run.
+
+A `before` hook will revert all migrations to the point that a migration
+under test is not yet migrated.
+
+In other words, our custom RSpec hooks will find a previous migration, and
+migrate the database **down** to the previous migration version.
+
+With this approach you can test a migration against a database schema.
+
+An `after` hook will migrate the database **up** and reinstitute the latest
+schema version, so that the process does not affect subsequent specs and
+ensures proper isolation.
+
+## Testing an `ActiveRecord::Migration` class
+
+To test an `ActiveRecord::Migration` class (i.e., a
+regular migration `db/migrate` or a post-migration `db/post_migrate`), you
+will need to manually `require` the migration file because it is not
+autoloaded with Rails. Example:
+
+```ruby
+require Rails.root.join('db', 'post_migrate', '20170526185842_migrate_pipeline_stages.rb')
+```
+
+Use the `table` helper to create a temporary `ActiveRecord::Base`-derived model
+for a table. [FactoryBot](https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories)
+**should not** be used to create data for migration specs. For example, to
+create a record in the `projects` table:
+
+```ruby
+project = table(:projects).create!(id: 1, name: 'gitlab1', path: 'gitlab1')
+```
+
+Use the `migrate!` helper to run the migration that is under test.  It will not only
+run the migration, but will also bump the schema version in the `schema_migrations`
+table. It is necessary because in the `after` hook we trigger the rest of
+the migrations, and we need to know where to start. Example:
+
+```ruby
+it 'migrates successfully' do
+  # ... pre-migration expectations
+
+  migrate!
+
+  # ... post-migration expectations
+end
+```
+
+### Example database migration test
+
+This spec tests the
+[`db/post_migrate/20170526185842_migrate_pipeline_stages.rb`](https://gitlab.com/gitlab-org/gitlab/blob/v11.6.5/db/post_migrate/20170526185842_migrate_pipeline_stages.rb)
+migration. You can find the complete spec in
+[`spec/migrations/migrate_pipeline_stages_spec.rb`](https://gitlab.com/gitlab-org/gitlab/blob/v11.6.5/spec/migrations/migrate_pipeline_stages_spec.rb).
+
+```ruby
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170526185842_migrate_pipeline_stages.rb')
+
+describe MigratePipelineStages, :migration do
+  # Create test data - pipeline and CI/CD jobs.
+  let(:jobs) { table(:ci_builds) }
+  let(:stages) { table(:ci_stages) }
+  let(:pipelines) { table(:ci_pipelines) }
+  let(:projects) { table(:projects) }
+
+  before do
+    projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1')
+    pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a')
+    jobs.create!(id: 1, commit_id: 1, project_id: 123, stage_idx: 2, stage: 'build')
+    jobs.create!(id: 2, commit_id: 1, project_id: 123, stage_idx: 1, stage: 'test')
+  end
+
+  # Test the up migration.
+  it 'correctly migrates pipeline stages' do
+    expect(stages.count).to be_zero
+
+    migrate!
+
+    expect(stages.count).to eq 2
+    expect(stages.all.pluck(:name)).to match_array %w[test build]
+  end
+end
+```
+
+## Testing a non-`ActiveRecord::Migration` class
+
+To test a non-`ActiveRecord::Migration` test (a background migration),
+you will need to manually provide a required schema version. Please add a
+schema tag to a context that you want to switch the database schema within.
+
+Example:
+
+```ruby
+describe SomeClass, :migration, schema: 20170608152748 do
+  # ...
+end
+```
+
+### Example background migration test
+
+This spec tests the
+[`lib/gitlab/background_migration/archive_legacy_traces.rb`](https://gitlab.com/gitlab-org/gitlab/blob/v11.6.5/lib/gitlab/background_migration/archive_legacy_traces.rb)
+background migration. You can find the complete spec on
+[`spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb`](https://gitlab.com/gitlab-org/gitlab/blob/v11.6.5/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb)
+
+```ruby
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::ArchiveLegacyTraces, :migration, schema: 20180529152628 do
+  include TraceHelpers
+
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:builds) { table(:ci_builds) }
+  let(:job_artifacts) { table(:ci_job_artifacts) }
+
+  before do
+    namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1')
+    projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123)
+    @build = builds.create!(id: 1, project_id: 123, status: 'success', type: 'Ci::Build')
+  end
+
+  context 'when trace file exists at the right place' do
+    before do
+      create_legacy_trace(@build, 'trace in file')
+    end
+
+    it 'correctly archive legacy traces' do
+      expect(job_artifacts.count).to eq(0)
+      expect(File.exist?(legacy_trace_path(@build))).to be_truthy
+
+      described_class.new.perform(1, 1)
+
+      expect(job_artifacts.count).to eq(1)
+      expect(File.exist?(legacy_trace_path(@build))).to be_falsy
+      expect(File.read(archived_trace_path(job_artifacts.first))).to eq('trace in file')
+    end
+  end
+end
+```
+
+NOTE: **Note:**
+These tests do not run within a database transaction, as we use a deletion database
+cleanup strategy. Do not depend on a transaction being present.

doc/user/application_security/index.md

@@ -151,7 +151,7 @@ Clicking on this button will create a merge request to apply the solution onto t
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/9928) in [GitLab Ultimate](https://about.gitlab.com/pricing) 12.2.
 
 Merge Request Approvals can be configured to require approval from a member
-of your security team when a vulnerability would be introduced by a merge request.
+of your security team when a vulnerability, or a software license compliance violation would be introduced by a merge request.
 
 This threshold is defined as `high`, `critical`, or `unknown`
 severity. When any vulnerabilities are present within a merge request, an
@@ -178,6 +178,29 @@ An approval will be optional when a security report:
 - Contains no new vulnerabilities.
 - Contains only new vulnerabilities of `low` or `medium` severity.
 
+### Enabling License Approvals within a project
+
+To enable License Approvals, a [project approval rule](../project/merge_requests/merge_request_approvals.md#multiple-approval-rules-premium)
+must be created with the case-sensitive name `License-Check`. This approval
+group must be set with an "Approvals required" count greater than zero.
+
+Once this group has been added to your project, the approval rule will be enabled
+for all Merge Requests. To configure how this rule behaves, you can choose which
+licenses to `approve` or `blacklist` in the
+[project policies for License Compliance](license_compliance/index.md#project-policies-for-license-compliance) section.
+
+Any code changes made will cause the count of approvals required to reset.
+
+An approval will be required when a license report:
+
+- Contains a dependency that includes a software license that is `blacklisted`.
+- Is not generated during pipeline execution.
+
+An approval will be optional when a license report:
+
+- Contains no software license violations.
+- Contains only new licenses that are `approved` or unknown.
+
 <!-- ## Troubleshooting
 
 Include any troubleshooting steps that you can foresee. If you know beforehand what issues

doc/user/project/merge_requests/merge_request_approvals.md

@@ -337,6 +337,16 @@ of your security team when a vulnerability would be introduced by a merge reques
 For more information, see
 [Security approvals in merge requests](../../application_security/index.md#security-approvals-in-merge-requests-ultimate).
 
+## License compliance approvals in merge requests **(ULTIMATE)**
+
+> Introduced in [GitLab Ultimate](https://about.gitlab.com/pricing) 12.3.
+
+Merge Request Approvals can be configured to require approval from a member
+of your security team when a blacklisted software license would be introduced by a merge request.
+
+For more information, see
+[Security approvals in merge requests](../../application_security/index.md#security-approvals-in-merge-requests-ultimate).
+
 <!-- ## Troubleshooting
 
 Include any troubleshooting steps that you can foresee. If you know beforehand what issues

doc/user/project/quick_actions.md

@@ -64,8 +64,8 @@ The following quick actions are applicable to descriptions, discussions and thre
 | `/create_merge_request <branch name>` | ✓     |               |      | Create a new merge request starting from the current issue |
 | `/relate #issue1 #issue2`             | ✓     |               |      | Mark issues as related **(STARTER)** |
 | `/move <path/to/project>`             | ✓     |               |      | Move this issue to another project |
-| `/zoom <Zoom URL>`                    | ✓     |               |      | Add Zoom meeting to this issue. ([Introduced in GitLab 12.3](https://gitlab.com/gitlab-org/gitlab/merge_requests/16609). Must be enabled by feature flag `issue_zoom_integration` for self-hosted. Feature flag to be removed and available by default in 12.4.) |
-| `/remove_zoom`                        | ✓     |               |      | Remove Zoom meeting from this issue. ([Introduced in GitLab 12.3](https://gitlab.com/gitlab-org/gitlab/merge_requests/16609). Must be enabled by feature flag `issue_zoom_integration` for self-hosted. Feature flag to be removed and available by default in 12.4.) |
+| `/zoom <Zoom URL>`                    | ✓     |               |      | Add Zoom meeting to this issue. ([Introduced in GitLab 12.3](https://gitlab.com/gitlab-org/gitlab/merge_requests/16609). Must be enabled by feature flag `issue_zoom_integration` for self-hosted. [Feature flag to be removed and available by default in 12.4.](https://gitlab.com/gitlab-org/gitlab/issues/32133)) |
+| `/remove_zoom`                        | ✓     |               |      | Remove Zoom meeting from this issue. ([Introduced in GitLab 12.3](https://gitlab.com/gitlab-org/gitlab/merge_requests/16609). Must be enabled by feature flag `issue_zoom_integration` for self-hosted. [Feature flag to be removed and available by default in 12.4.](https://gitlab.com/gitlab-org/gitlab/issues/32133)) |
 | `/target_branch <local branch name>`  |       | ✓             |      | Set target branch |
 | `/wip`                                |       | ✓             |      | Toggle the Work In Progress status |
 | `/approve`                            |       | ✓             |      | Approve the merge request |

spec/migrations/README.md

-# Testing migrations
-
-In order to reliably test a migration, we need to test it against a database
-schema that this migration has been written for. In order to achieve that we
-have some _migration helpers_ and RSpec test tag, called `:migration`.
-
-If you want to write a test for a migration consider adding `:migration` tag to
-the test signature, like `describe SomeMigrationClass, :migration`.
-
-## How does it work?
-
-Adding a `:migration` tag to a test signature injects a few before / after
-hooks to the test.
-
-The most important change is that adding a `:migration` tag adds a `before`
-hook that will revert all migrations to the point that a migration under test
-is not yet migrated.
-
-In other words, our custom RSpec hooks will find a previous migration, and
-migrate the database **down** to the previous migration version.
-
-With this approach you can test a migration against a database schema that this
-migration has been written for.
-
-The `after` hook will migrate the database **up** and reinstitutes the latest
-schema version, so that the process does not affect subsequent specs and
-ensures proper isolation.
-
-## Available helpers
-
-Use `table` helper to create a temporary `ActiveRecord::Base` derived model
-for a table.
-
-See `spec/support/helpers/migrations_helpers.rb` for all the available helpers.
-
-## Testing a class that is an ActiveRecord::Migration
-
-In order to test a class that is an `ActiveRecord::Migration`, you will need to
-manually `require` the migration file because it is not autoloaded with Rails.
-
-Use `migrate!` helper to run the migration that is under test.  It will not only
-run migration, but will also bump the schema version in the `schema_migrations`
-table. It is necessary because in the `after` hook we trigger the rest of
-the migrations, and we need to know where to start.
-
-### Example
-
-This spec tests the [`db/post_migrate/20170526185842_migrate_pipeline_stages.rb`](https://gitlab.com/gitlab-org/gitlab-foss/blob/v11.6.5/db/post_migrate/20170526185842_migrate_pipeline_stages.rb) migration. You can find the complete spec on [`spec/migrations/migrate_pipeline_stages_spec.rb`](https://gitlab.com/gitlab-org/gitlab-foss/blob/v11.6.5/spec/migrations/migrate_pipeline_stages_spec.rb).
-
-```ruby
-require 'spec_helper'
-require Rails.root.join('db', 'post_migrate', '20170526185842_migrate_pipeline_stages.rb')
-
-describe MigratePipelineStages, :migration do
-
-  # Create test data - pipeline and CI/CD jobs.
-
-  let(:jobs) { table(:ci_builds) }
-  let(:stages) { table(:ci_stages) }
-  let(:pipelines) { table(:ci_pipelines) }
-  let(:projects) { table(:projects) }
-
-  before do
-    projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1')
-    pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a')
-    jobs.create!(id: 1, commit_id: 1, project_id: 123, stage_idx: 2, stage: 'build')
-    jobs.create!(id: 2, commit_id: 1, project_id: 123, stage_idx: 1, stage: 'test')
-  end
-
-  # Test the migration.
-
-  it 'correctly migrates pipeline stages' do
-    expect(stages.count).to be_zero
-
-    migrate!
-
-    expect(stages.count).to eq 2
-    expect(stages.all.pluck(:name)).to match_array %w[test build]
-  end
-end
-```
-
-## Testing a class that is not an ActiveRecord::Migration
-
-To test a class that is not an `ActiveRecord::Migration` (a background migration),
-you will need to manually provide a required schema version. Please add a
-schema tag to a context that you want to switch the database schema within.
-
-Example: `describe SomeClass, :migration, schema: 20170608152748`.
-
-### Example
-
-This spec tests the [`lib/gitlab/background_migration/archive_legacy_traces.rb`](https://gitlab.com/gitlab-org/gitlab-foss/blob/v11.6.5/lib/gitlab/background_migration/archive_legacy_traces.rb)
-background migration. You can find the complete spec on 
-[`spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb`](https://gitlab.com/gitlab-org/gitlab-foss/blob/v11.6.5/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb)
-
-```ruby
-require 'spec_helper'
-
-describe Gitlab::BackgroundMigration::ArchiveLegacyTraces, :migration, schema: 20180529152628 do
-  include TraceHelpers
-
-  let(:namespaces) { table(:namespaces) }
-  let(:projects) { table(:projects) }
-  let(:builds) { table(:ci_builds) }
-  let(:job_artifacts) { table(:ci_job_artifacts) }
-
-  before do
-    namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1')
-    projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123)
-    @build = builds.create!(id: 1, project_id: 123, status: 'success', type: 'Ci::Build')
-  end
-
-  context 'when trace file exsits at the right place' do
-    before do
-      create_legacy_trace(@build, 'trace in file')
-    end
-
-    it 'correctly archive legacy traces' do
-      expect(job_artifacts.count).to eq(0)
-      expect(File.exist?(legacy_trace_path(@build))).to be_truthy
-
-      described_class.new.perform(1, 1)
-
-      expect(job_artifacts.count).to eq(1)
-      expect(File.exist?(legacy_trace_path(@build))).to be_falsy
-      expect(File.read(archived_trace_path(job_artifacts.first))).to eq('trace in file')
-    end
-  end
-end
-```
-
-## Best practices
-
-1. Note that this type of tests do not run within the transaction, we use
-a deletion database cleanup strategy. Do not depend on transaction being
-present.
+This document was moved to [another location](https://docs.gitlab.com/ee/development/testing_guide/testing_migrations_guide.html).

spec/models/clusters/providers/gcp_spec.rb

@@ -6,6 +6,8 @@ describe Clusters::Providers::Gcp do
   it { is_expected.to belong_to(:cluster) }
   it { is_expected.to validate_presence_of(:zone) }
 
+  include_examples 'provider status', :cluster_provider_gcp
+
   describe 'default_value_for' do
     let(:gcp) { build(:cluster_provider_gcp) }
 
@@ -84,88 +86,6 @@ describe Clusters::Providers::Gcp do
     it { is_expected.not_to be_legacy_abac }
   end
 
-  describe '#state_machine' do
-    context 'when any => [:created]' do
-      let(:gcp) { build(:cluster_provider_gcp, :creating) }
-
-      before do
-        gcp.make_created
-      end
-
-      it 'nullify access_token and operation_id' do
-        expect(gcp.access_token).to be_nil
-        expect(gcp.operation_id).to be_nil
-        expect(gcp).to be_created
-      end
-    end
-
-    context 'when any => [:creating]' do
-      let(:gcp) { build(:cluster_provider_gcp) }
-
-      context 'when operation_id is present' do
-        let(:operation_id) { 'operation-xxx' }
-
-        before do
-          gcp.make_creating(operation_id)
-        end
-
-        it 'sets operation_id' do
-          expect(gcp.operation_id).to eq(operation_id)
-          expect(gcp).to be_creating
-        end
-      end
-
-      context 'when operation_id is nil' do
-        let(:operation_id) { nil }
-
-        it 'raises an error' do
-          expect { gcp.make_creating(operation_id) }
-            .to raise_error('operation_id is required')
-        end
-      end
-    end
-
-    context 'when any => [:errored]' do
-      let(:gcp) { build(:cluster_provider_gcp, :creating) }
-      let(:status_reason) { 'err msg' }
-
-      it 'nullify access_token and operation_id' do
-        gcp.make_errored(status_reason)
-
-        expect(gcp.access_token).to be_nil
-        expect(gcp.operation_id).to be_nil
-        expect(gcp.status_reason).to eq(status_reason)
-        expect(gcp).to be_errored
-      end
-
-      context 'when status_reason is nil' do
-        let(:gcp) { build(:cluster_provider_gcp, :errored) }
-
-        it 'does not set status_reason' do
-          gcp.make_errored(nil)
-
-          expect(gcp.status_reason).not_to be_nil
-        end
-      end
-    end
-  end
-
-  describe '#on_creation?' do
-    subject { gcp.on_creation? }
-
-    context 'when status is creating' do
-      let(:gcp) { create(:cluster_provider_gcp, :creating) }
-
-      it { is_expected.to be_truthy }
-    end
-
-    context 'when status is created' do
-      let(:gcp) { create(:cluster_provider_gcp, :created) }
-
-      it { is_expected.to be_falsey }
-    end
-  end
-
   describe '#knative_pre_installed?' do
     subject { gcp.knative_pre_installed? }
 
@@ -206,4 +126,31 @@ describe Clusters::Providers::Gcp do
       it { is_expected.to be_nil }
     end
   end
+
+  describe '#nullify_credentials' do
+    let(:provider) { create(:cluster_provider_gcp, :creating) }
+
+    before do
+      expect(provider.access_token).to be_present
+      expect(provider.operation_id).to be_present
+    end
+
+    it 'removes access_token and operation_id' do
+      provider.nullify_credentials
+
+      expect(provider.access_token).to be_nil
+      expect(provider.operation_id).to be_nil
+    end
+  end
+
+  describe '#assign_operation_id' do
+    let(:provider) { create(:cluster_provider_gcp, :scheduled) }
+    let(:operation_id) { 'operation-123' }
+
+    it 'sets operation_id' do
+      provider.assign_operation_id(operation_id)
+
+      expect(provider.operation_id).to eq(operation_id)
+    end
+  end
 end

spec/support/shared_examples/models/clusters/providers/provider_status.rb

+# frozen_string_literal: true
+
+shared_examples 'provider status' do |factory|
+  describe 'state_machine' do
+    context 'when any => [:created]' do
+      let(:provider) { build(factory, :creating) }
+
+      it 'nullifies API credentials' do
+        expect(provider).to receive(:nullify_credentials).and_call_original
+        provider.make_created
+
+        expect(provider).to be_created
+      end
+    end
+
+    context 'when any => [:creating]' do
+      let(:provider) { build(factory) }
+      let(:operation_id) { 'operation-xxx' }
+
+      it 'calls #operation_id on the provider' do
+        expect(provider).to receive(:assign_operation_id).with(operation_id).and_call_original
+
+        provider.make_creating(operation_id)
+      end
+    end
+
+    context 'when any => [:errored]' do
+      let(:provider) { build(factory, :creating) }
+      let(:status_reason) { 'err msg' }
+
+      it 'calls #nullify_credentials on the provider' do
+        expect(provider).to receive(:nullify_credentials).and_call_original
+
+        provider.make_errored(status_reason)
+      end
+
+      it 'sets a status reason' do
+        provider.make_errored(status_reason)
+
+        expect(provider.status_reason).to eq('err msg')
+      end
+
+      context 'when status_reason is nil' do
+        let(:provider) { build(factory, :errored) }
+
+        it 'does not set status_reason' do
+          provider.make_errored(nil)
+
+          expect(provider.status_reason).not_to be_nil
+        end
+      end
+    end
+  end
+
+  describe '#on_creation?' do
+    using RSpec::Parameterized::TableSyntax
+
+    subject { provider.on_creation? }
+
+    where(:status, :result) do
+      :scheduled | true
+      :creating  | true
+      :created   | false
+      :errored   | false
+    end
+
+    with_them do
+      let(:provider) { build(factory, status) }
+
+      it { is_expected.to eq result }
+    end
+  end
+end