diff options
| -rw-r--r-- | app/models/clusters/cluster.rb | 2 | ||||
| -rw-r--r-- | app/models/clusters/concerns/provider_status.rb | 48 | ||||
| -rw-r--r-- | app/models/clusters/providers/gcp.rb | 51 | ||||
| -rw-r--r-- | doc/development/README.md | 1 | ||||
| -rw-r--r-- | doc/development/background_migrations.md | 4 | ||||
| -rw-r--r-- | doc/development/migration_style_guide.md | 5 | ||||
| -rw-r--r-- | doc/development/testing_guide/testing_levels.md | 2 | ||||
| -rw-r--r-- | doc/development/testing_guide/testing_migrations_guide.md | 167 | ||||
| -rw-r--r-- | doc/user/application_security/index.md | 25 | ||||
| -rw-r--r-- | doc/user/project/merge_requests/merge_request_approvals.md | 10 | ||||
| -rw-r--r-- | doc/user/project/quick_actions.md | 4 | ||||
| -rw-r--r-- | spec/migrations/README.md | 138 | ||||
| -rw-r--r-- | spec/models/clusters/providers/gcp_spec.rb | 111 | ||||
| -rw-r--r-- | spec/support/shared_examples/models/clusters/providers/provider_status.rb | 73 |
14 files changed, 373 insertions, 268 deletions
diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 49bed479c02..72f6acc0aa4 100644 --- a/app/models/clusters/cluster.rb +++ b/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) } diff --git a/app/models/clusters/concerns/provider_status.rb b/app/models/clusters/concerns/provider_status.rb new file mode 100644 index 00000000000..4d1974777ea --- /dev/null +++ b/app/models/clusters/concerns/provider_status.rb @@ -0,0 +1,48 @@ +# 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 diff --git a/app/models/clusters/providers/gcp.rb b/app/models/clusters/providers/gcp.rb index 043765f79ac..f871674676f 100644 --- a/app/models/clusters/providers/gcp.rb +++ b/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? diff --git a/doc/development/README.md b/doc/development/README.md index 0d1168c4450..4c9e111bc99 100644 --- a/doc/development/README.md +++ b/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) diff --git a/doc/development/background_migrations.md b/doc/development/background_migrations.md index 364e276b6cc..0a08360b727 100644 --- a/doc/development/background_migrations.md +++ b/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 diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index a0e11d20339..46db00993a3 100644 --- a/doc/development/migration_style_guide.md +++ b/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 diff --git a/doc/development/testing_guide/testing_levels.md b/doc/development/testing_guide/testing_levels.md index c59ca66cfd9..13659d66180 100644 --- a/doc/development/testing_guide/testing_levels.md +++ b/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 | | diff --git a/doc/development/testing_guide/testing_migrations_guide.md b/doc/development/testing_guide/testing_migrations_guide.md new file mode 100644 index 00000000000..03dd7fc7851 --- /dev/null +++ b/doc/development/testing_guide/testing_migrations_guide.md @@ -0,0 +1,167 @@ +--- +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. diff --git a/doc/user/application_security/index.md b/doc/user/application_security/index.md index 0e52496ec43..d1d46e6ef96 100644 --- a/doc/user/application_security/index.md +++ b/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 diff --git a/doc/user/project/merge_requests/merge_request_approvals.md b/doc/user/project/merge_requests/merge_request_approvals.md index 6f8d821e1c6..942747c8d81 100644 --- a/doc/user/project/merge_requests/merge_request_approvals.md +++ b/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 diff --git a/doc/user/project/quick_actions.md b/doc/user/project/quick_actions.md index 43479aff526..bc16ea52578 100644 --- a/doc/user/project/quick_actions.md +++ b/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 | diff --git a/spec/migrations/README.md b/spec/migrations/README.md index 4d86d30080a..5065df3fbef 100644 --- a/spec/migrations/README.md +++ b/spec/migrations/README.md @@ -1,137 +1 @@ -# 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). diff --git a/spec/models/clusters/providers/gcp_spec.rb b/spec/models/clusters/providers/gcp_spec.rb index 7ac1bbfafd8..15e152519b4 100644 --- a/spec/models/clusters/providers/gcp_spec.rb +++ b/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 diff --git a/spec/support/shared_examples/models/clusters/providers/provider_status.rb b/spec/support/shared_examples/models/clusters/providers/provider_status.rb new file mode 100644 index 00000000000..af758b07d96 --- /dev/null +++ b/spec/support/shared_examples/models/clusters/providers/provider_status.rb @@ -0,0 +1,73 @@ +# 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 |