diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-11-23 12:09:11 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-11-23 12:09:11 +0000 |
| commit | 15ea3fec22d1efd1945d390b21ff65461290dfc1 (patch) | |
| tree | f5703fc0c0c4ab49eef0cc5449fb08f570b7a16b | |
| parent | 8132e39e1b5a7829b8b7ed8bd0482e1812a9eeef (diff) | |
| download | gitlab-ce-15ea3fec22d1efd1945d390b21ff65461290dfc1.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
52 files changed, 824 insertions, 325 deletions
diff --git a/.gitignore b/.gitignore index 30cb231e83f..93fb0b1144b 100644 --- a/.gitignore +++ b/.gitignore @@ -77,6 +77,7 @@ eslint-report.html /.gitlab_kas_secret /webpack-report/ /crystalball/ +/deprecations/ /knapsack/ /rspec_flaky/ /locale/**/LC_MESSAGES diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 14b07dd4a2a..3962be8d84d 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -21,6 +21,7 @@ RUBY_GC_MALLOC_LIMIT: 67108864 RUBY_GC_MALLOC_LIMIT_MAX: 134217728 CRYSTALBALL: "true" + RECORD_DEPRECATIONS: "true" needs: ["setup-test-env", "retrieve-tests-metadata", "compile-test-assets"] script: - *base-script @@ -31,6 +32,7 @@ paths: - coverage/ - crystalball/ + - deprecations/ - knapsack/ - rspec_flaky/ - rspec_profiling/ diff --git a/.gitlab/issue_templates/Feature Flag Roll Out.md b/.gitlab/issue_templates/Feature Flag Roll Out.md index 584574bcbfe..8fda3297c38 100644 --- a/.gitlab/issue_templates/Feature Flag Roll Out.md +++ b/.gitlab/issue_templates/Feature Flag Roll Out.md @@ -36,8 +36,7 @@ If applicable, any groups/projects that are happy to have this feature turned on - [ ] Ensure that documentation has been updated - [ ] Enable on GitLab.com for individual groups/projects listed above and verify behaviour (`/chatops run feature set --project=gitlab-org/gitlab feature_name true`) - [ ] Coordinate a time to enable the flag with the SRE oncall and release managers - - In `#production` by pinging `@sre-oncall` - - In `#g_delivery` by pinging `@release-managers` + - In `#production` mention `@sre-oncall` and `@release-managers`. Once an SRE on call and Release Manager on call confirm, you can proceed with the rollout - [ ] Announce on the issue an estimated time this will be enabled on GitLab.com - [ ] Enable on GitLab.com by running chatops command in `#production` (`/chatops run feature set feature_name true`) - [ ] Cross post chatops Slack command to `#support_gitlab-com` ([more guidance when this is necessary in the dev docs](https://docs.gitlab.com/ee/development/feature_flags/controls.html#where-to-run-commands)) and in your team channel diff --git a/.rubocop.yml b/.rubocop.yml index 34d6fe5e434..125b2db5cf8 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -47,6 +47,10 @@ Cop/StaticTranslationDefinition: - 'spec/**/*' - 'ee/spec/**/*' +Lint/LastKeywordArgument: + Enabled: true + Safe: false + # This cop checks whether some constant value isn't a # mutable literal (e.g. array or hash). Style/MutableConstant: @@ -351,6 +351,7 @@ group :development do end group :development, :test do + gem 'deprecation_toolkit', '~> 1.5.1', require: false gem 'bullet', '~> 6.1.0' gem 'pry-byebug', '~> 3.9.0', platform: :mri gem 'pry-rails', '~> 0.3.9' diff --git a/Gemfile.lock b/Gemfile.lock index 807c24c0d31..f5d7c409c0c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -224,6 +224,8 @@ GEM declarative-option (0.1.0) default_value_for (3.3.0) activerecord (>= 3.2.0, < 6.1) + deprecation_toolkit (1.5.1) + activesupport (>= 4.2) derailed_benchmarks (1.7.0) benchmark-ips (~> 2) get_process_mem (~> 0) @@ -1302,6 +1304,7 @@ DEPENDENCIES database_cleaner (~> 1.7.0) deckar01-task_list (= 2.3.1) default_value_for (~> 3.3.0) + deprecation_toolkit (~> 1.5.1) derailed_benchmarks device_detector devise (~> 4.7.2) diff --git a/app/assets/javascripts/alerts_settings/components/alerts_settings_form_new.vue b/app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue index 3656fc4d7ec..bf2874b6cc7 100644 --- a/app/assets/javascripts/alerts_settings/components/alerts_settings_form_new.vue +++ b/app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue @@ -216,8 +216,12 @@ export default { return { name: this.currentIntegration?.name || '', active: this.currentIntegration?.active || false, - token: this.currentIntegration?.token || this.selectedIntegrationType.token, - url: this.currentIntegration?.url || this.selectedIntegrationType.url, + token: + this.currentIntegration?.token || + (this.selectedIntegrationType !== this.generic ? this.selectedIntegrationType.token : ''), + url: + this.currentIntegration?.url || + (this.selectedIntegrationType !== this.generic ? this.selectedIntegrationType.url : ''), apiUrl: this.currentIntegration?.apiUrl || '', }; }, @@ -246,8 +250,20 @@ export default { canEditPayload() { return this.hasSamplePayload && !this.resetSamplePayloadConfirmed; }, + isResetAuthKeyDisabled() { + return !this.active && !this.integrationForm.token !== ''; + }, isPayloadEditDisabled() { - return !this.active || this.canEditPayload; + return this.glFeatures.multipleHttpIntegrationsCustomMapping + ? !this.active || this.canEditPayload + : !this.active; + }, + isSubmitTestPayloadDisabled() { + return ( + !this.active || + Boolean(this.integrationTestPayload.error) || + this.integrationTestPayload.json === '' + ); }, }, watch: { @@ -257,7 +273,7 @@ export default { } this.selectedIntegration = val.type; this.active = val.active; - if (val.type === typeSet.http) this.getIntegrationMapping(val.id); + if (val.type === typeSet.http && this.showMappingBuilder) this.getIntegrationMapping(val.id); return this.integrationTypeSelect(); }, }, @@ -297,14 +313,8 @@ export default { }); }, submitWithTestPayload() { - return service - .updateTestAlert(this.testAlertPayload) - .then(() => { - this.submit(); - }) - .catch(() => { - this.$emit('test-payload-failure'); - }); + this.$emit('set-test-alert-payload', this.testAlertPayload); + this.submit(); }, submit() { // TODO: Will be removed in 13.7 as part of: https://gitlab.com/gitlab-org/gitlab/-/issues/273657 @@ -323,6 +333,7 @@ export default { return this.$emit('update-integration', integrationPayload); } + this.reset(); return this.$emit('create-new-integration', integrationPayload); }, reset() { @@ -539,7 +550,7 @@ export default { </template> </gl-form-input-group> - <gl-button v-gl-modal.authKeyModal :disabled="!active"> + <gl-button v-gl-modal.authKeyModal :disabled="isResetAuthKeyDisabled"> {{ $options.i18n.integrationFormSteps.step3.reset }} </gl-button> <gl-modal @@ -642,7 +653,7 @@ export default { <gl-button v-if="!isManagingOpsgenie" data-testid="integration-test-and-submit" - :disabled="Boolean(integrationTestPayload.error)" + :disabled="isSubmitTestPayloadDisabled" category="secondary" variant="success" class="gl-mx-3 js-no-auto-disable" diff --git a/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue b/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue index b9b397de05e..1ac63fd04a1 100644 --- a/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue +++ b/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue @@ -14,7 +14,8 @@ import resetHttpTokenMutation from '../graphql/mutations/reset_http_token.mutati import resetPrometheusTokenMutation from '../graphql/mutations/reset_prometheus_token.mutation.graphql'; import updateCurrentIntergrationMutation from '../graphql/mutations/update_current_intergration.mutation.graphql'; import IntegrationsList from './alerts_integrations_list.vue'; -import SettingsFormNew from './alerts_settings_form_new.vue'; +import AlertSettingsForm from './alerts_settings_form.vue'; +import service from '../services'; import { typeSet } from '../constants'; import { updateStoreAfterIntegrationDelete, @@ -35,6 +36,9 @@ export default { 'AlertsIntegrations|The integration has been successfully saved. Alerts from this new integration should now appear on your alerts list.', ), integrationRemoved: s__('AlertsIntegrations|The integration has been successfully removed.'), + alertSent: s__( + 'AlertsIntegrations|The test alert has been successfully sent, and should now be visible on your alerts list.', + ), }, components: { // TODO: Will be removed in 13.7 as part of: https://gitlab.com/gitlab-org/gitlab/-/issues/273657 @@ -42,7 +46,7 @@ export default { GlLink, GlSprintf, IntegrationsList, - SettingsFormNew, + AlertSettingsForm, }, inject: { generic: { @@ -89,6 +93,7 @@ export default { data() { return { isUpdating: false, + testAlertPayload: null, integrations: {}, currentIntegration: null, }; @@ -131,6 +136,19 @@ export default { if (error) { return createFlash({ message: error }); } + + if (this.testAlertPayload) { + const integration = + httpIntegrationCreate?.integration || prometheusIntegrationCreate?.integration; + + const payload = { + ...this.testAlertPayload, + endpoint: integration.url, + token: integration.token, + }; + return this.validateAlertPayload(payload); + } + return createFlash({ message: this.$options.i18n.changesSaved, type: FLASH_TYPES.SUCCESS, @@ -161,6 +179,13 @@ export default { if (error) { return createFlash({ message: error }); } + + if (this.testAlertPayload) { + return this.validateAlertPayload(); + } + + this.clearCurrentIntegration(); + return createFlash({ message: this.$options.i18n.changesSaved, type: FLASH_TYPES.SUCCESS, @@ -171,6 +196,7 @@ export default { }) .finally(() => { this.isUpdating = false; + this.testAlertPayload = null; }); }, resetToken({ type, variables }) { @@ -194,7 +220,13 @@ export default { const integration = httpIntegrationResetToken?.integration || prometheusIntegrationResetToken?.integration; - this.currentIntegration = integration; + + this.$apollo.mutate({ + mutation: updateCurrentIntergrationMutation, + variables: { + ...integration, + }, + }); return createFlash({ message: this.$options.i18n.changesSaved, @@ -262,8 +294,21 @@ export default { variables: {}, }); }, - testPayloadFailure() { - createFlash({ message: INTEGRATION_PAYLOAD_TEST_ERROR }); + setTestAlertPayload(payload) { + this.testAlertPayload = payload; + }, + validateAlertPayload(payload) { + return service + .updateTestAlert(payload ?? this.testAlertPayload) + .then(() => { + return createFlash({ + message: this.$options.i18n.alertSent, + type: FLASH_TYPES.SUCCESS, + }); + }) + .catch(() => { + createFlash({ message: INTEGRATION_PAYLOAD_TEST_ERROR }); + }); }, }, }; @@ -297,7 +342,7 @@ export default { @edit-integration="editIntegration" @delete-integration="deleteIntegration" /> - <settings-form-new + <alert-settings-form :loading="isUpdating" :can-add-integration="canAddIntegration" :can-manage-opsgenie="canManageOpsgenie" @@ -305,7 +350,7 @@ export default { @update-integration="updateIntegration" @reset-token="resetToken" @clear-current-integration="clearCurrentIntegration" - @test-payload-failure="testPayloadFailure" + @set-test-alert-payload="setTestAlertPayload" /> </div> </template> diff --git a/app/graphql/types/user_type.rb b/app/graphql/types/user_type.rb index 11c5369f726..ddba6589474 100644 --- a/app/graphql/types/user_type.rb +++ b/app/graphql/types/user_type.rb @@ -38,6 +38,8 @@ module Types feature_flag: :user_group_counts field :status, Types::UserStatusType, null: true, description: 'User status' + field :location, ::GraphQL::STRING_TYPE, null: true, + description: 'The location of the user.' field :project_memberships, Types::ProjectMemberType.connection_type, null: true, description: 'Project memberships of the user', method: :project_members diff --git a/app/models/terraform/state.rb b/app/models/terraform/state.rb index e6f295ee97a..19700587f09 100644 --- a/app/models/terraform/state.rb +++ b/app/models/terraform/state.rb @@ -3,7 +3,6 @@ module Terraform class State < ApplicationRecord include UsageStatistics - include FileStoreMounter include IgnorableColumns # These columns are being removed since geo replication falls to the versioned state # Tracking in https://gitlab.com/gitlab-org/gitlab/-/issues/258262 @@ -36,18 +35,8 @@ module Terraform default_value_for(:uuid, allows_nil: false) { SecureRandom.hex(UUID_LENGTH / 2) } - mount_file_store_uploader StateUploader - - def file_store - super || StateUploader.default_store - end - def latest_file - if versioning_enabled? - latest_version&.file - else - latest_version&.file || file - end + latest_version&.file end def locked? @@ -55,13 +44,14 @@ module Terraform end def update_file!(data, version:, build:) + # This check is required to maintain backwards compatibility with + # states that were created prior to versioning being supported. + # This can be removed in 14.0 when support for these states is dropped. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/258960 if versioning_enabled? create_new_version!(data: data, version: version, build: build) - elsif latest_version.present? - migrate_legacy_version!(data: data, version: version, build: build) else - self.file = data - save! + migrate_legacy_version!(data: data, version: version, build: build) end end diff --git a/app/models/terraform/state_version.rb b/app/models/terraform/state_version.rb index cc5d94b8e09..19d708616fc 100644 --- a/app/models/terraform/state_version.rb +++ b/app/models/terraform/state_version.rb @@ -10,9 +10,9 @@ module Terraform scope :ordered_by_version_desc, -> { order(version: :desc) } - default_value_for(:file_store) { VersionedStateUploader.default_store } + default_value_for(:file_store) { StateUploader.default_store } - mount_file_store_uploader VersionedStateUploader + mount_file_store_uploader StateUploader delegate :project_id, :uuid, to: :terraform_state, allow_nil: true diff --git a/app/models/user.rb b/app/models/user.rb index be64e057d59..22e62bc5f1e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -587,11 +587,13 @@ class User < ApplicationRecord sanitized_order_sql = Arel.sql(sanitize_sql_array([order, query: query])) - where( - fuzzy_arel_match(:name, query, lower_exact_match: true) - .or(fuzzy_arel_match(:username, query, lower_exact_match: true)) - .or(arel_table[:email].eq(query)) - ).reorder(sanitized_order_sql, :name) + search_query = if Feature.enabled?(:user_search_secondary_email) + search_with_secondary_emails(query) + else + search_without_secondary_emails(query) + end + + search_query.reorder(sanitized_order_sql, :name) end # Limits the result set to users _not_ in the given query/list of IDs. @@ -606,6 +608,18 @@ class User < ApplicationRecord reorder(:name) end + def search_without_secondary_emails(query) + return none if query.blank? + + query = query.downcase + + where( + fuzzy_arel_match(:name, query, lower_exact_match: true) + .or(fuzzy_arel_match(:username, query, lower_exact_match: true)) + .or(arel_table[:email].eq(query)) + ) + end + # searches user by given pattern # it compares name, email, username fields and user's secondary emails with given pattern # This method uses ILIKE on PostgreSQL. @@ -616,15 +630,16 @@ class User < ApplicationRecord query = query.downcase email_table = Email.arel_table - matched_by_emails_user_ids = email_table + matched_by_email_user_id = email_table .project(email_table[:user_id]) .where(email_table[:email].eq(query)) + .take(1) # at most 1 record as there is a unique constraint where( fuzzy_arel_match(:name, query) .or(fuzzy_arel_match(:username, query)) .or(arel_table[:email].eq(query)) - .or(arel_table[:id].in(matched_by_emails_user_ids)) + .or(arel_table[:id].eq(matched_by_email_user_id)) ) end diff --git a/app/services/users/approve_service.rb b/app/services/users/approve_service.rb index 27668e9430e..debd1e8cd17 100644 --- a/app/services/users/approve_service.rb +++ b/app/services/users/approve_service.rb @@ -7,8 +7,9 @@ module Users end def execute(user) - return error(_('You are not allowed to approve a user')) unless allowed? - return error(_('The user you are trying to approve is not pending an approval')) unless approval_required?(user) + return error(_('You are not allowed to approve a user'), :forbidden) unless allowed? + return error(_('The user you are trying to approve is not pending an approval'), :conflict) if user.active? + return error(_('The user you are trying to approve is not pending an approval'), :conflict) unless approval_required?(user) if user.activate # Resends confirmation email if the user isn't confirmed yet. @@ -18,9 +19,9 @@ module Users DeviseMailer.user_admin_approval(user).deliver_later after_approve_hook(user) - success + success(message: 'Success', http_status: :created) else - error(user.errors.full_messages.uniq.join('. ')) + error(user.errors.full_messages.uniq.join('. '), :unprocessable_entity) end end diff --git a/app/uploaders/terraform/state_uploader.rb b/app/uploaders/terraform/state_uploader.rb index 2306313fc82..d80725cb051 100644 --- a/app/uploaders/terraform/state_uploader.rb +++ b/app/uploaders/terraform/state_uploader.rb @@ -6,17 +6,33 @@ module Terraform storage_options Gitlab.config.terraform_state - delegate :project_id, to: :model + delegate :terraform_state, :project_id, to: :model # Use Lockbox to encrypt/decrypt the stored file (registers CarrierWave callbacks) encrypt(key: :key) def filename - "#{model.uuid}.tfstate" + # This check is required to maintain backwards compatibility with + # states that were created prior to versioning being supported. + # This can be removed in 14.0 when support for these states is dropped. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/258960 + if terraform_state.versioning_enabled? + "#{model.version}.tfstate" + else + "#{model.uuid}.tfstate" + end end def store_dir - project_id.to_s + # This check is required to maintain backwards compatibility with + # states that were created prior to versioning being supported. + # This can be removed in 14.0 when support for these states is dropped. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/258960 + if terraform_state.versioning_enabled? + Gitlab::HashedPath.new(model.uuid, root_hash: project_id) + else + project_id.to_s + end end def key diff --git a/app/uploaders/terraform/versioned_state_uploader.rb b/app/uploaders/terraform/versioned_state_uploader.rb deleted file mode 100644 index e50ab6c7dc6..00000000000 --- a/app/uploaders/terraform/versioned_state_uploader.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -module Terraform - class VersionedStateUploader < StateUploader - delegate :terraform_state, to: :model - - def filename - if terraform_state.versioning_enabled? - "#{model.version}.tfstate" - else - "#{model.uuid}.tfstate" - end - end - - def store_dir - if terraform_state.versioning_enabled? - Gitlab::HashedPath.new(model.uuid, root_hash: project_id) - else - project_id.to_s - end - end - end -end diff --git a/changelogs/unreleased/241267-add-partitioned-audit-events-indexes.yml b/changelogs/unreleased/241267-add-partitioned-audit-events-indexes.yml new file mode 100644 index 00000000000..ac580b87b55 --- /dev/null +++ b/changelogs/unreleased/241267-add-partitioned-audit-events-indexes.yml @@ -0,0 +1,5 @@ +--- +title: Add secondary indexes to partitioned audit_events +merge_request: 48270 +author: +type: added diff --git a/changelogs/unreleased/263107-user-admin-approval-approve-user-via-api.yml b/changelogs/unreleased/263107-user-admin-approval-approve-user-via-api.yml new file mode 100644 index 00000000000..424a600e298 --- /dev/null +++ b/changelogs/unreleased/263107-user-admin-approval-approve-user-via-api.yml @@ -0,0 +1,5 @@ +--- +title: Add API endoint for Administrators to approve pending users +merge_request: 47564 +author: +type: added diff --git a/changelogs/unreleased/ajk-graphql-user-location.yml b/changelogs/unreleased/ajk-graphql-user-location.yml new file mode 100644 index 00000000000..95c8660860c --- /dev/null +++ b/changelogs/unreleased/ajk-graphql-user-location.yml @@ -0,0 +1,5 @@ +--- +title: Add User.location field to GraphQL API +merge_request: 48059 +author: +type: changed diff --git a/changelogs/unreleased/alert-settings-form-json-bug.yml b/changelogs/unreleased/alert-settings-form-json-bug.yml new file mode 100644 index 00000000000..9bb46fe640b --- /dev/null +++ b/changelogs/unreleased/alert-settings-form-json-bug.yml @@ -0,0 +1,6 @@ +--- +title: Update alert setting form to handle JSON payload submit when mapping builder + is not enabled +merge_request: 48231 +author: +type: fixed diff --git a/changelogs/unreleased/cat-user-search-secondary-emails.yml b/changelogs/unreleased/cat-user-search-secondary-emails.yml new file mode 100644 index 00000000000..9a88b1b0031 --- /dev/null +++ b/changelogs/unreleased/cat-user-search-secondary-emails.yml @@ -0,0 +1,5 @@ +--- +title: Allow secondary emails in user search +merge_request: 47587 +author: +type: added diff --git a/config/feature_flags/development/ci_variable_expansion_in_rules_changes.yml b/config/feature_flags/development/user_search_secondary_email.yml index a3a66295896..65e0fd8ce97 100644 --- a/config/feature_flags/development/ci_variable_expansion_in_rules_changes.yml +++ b/config/feature_flags/development/user_search_secondary_email.yml @@ -1,7 +1,8 @@ --- -name: ci_variable_expansion_in_rules_changes -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45037 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/267192 +name: user_search_secondary_email +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47587 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/282137 +milestone: '13.7' type: development -group: group::pipeline authoring -default_enabled: true +group: group::access +default_enabled: false diff --git a/db/migrate/20201112215028_add_partitioned_audit_event_indexes.rb b/db/migrate/20201112215028_add_partitioned_audit_event_indexes.rb new file mode 100644 index 00000000000..d8b2833474b --- /dev/null +++ b/db/migrate/20201112215028_add_partitioned_audit_event_indexes.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class AddPartitionedAuditEventIndexes < ActiveRecord::Migration[6.0] + include Gitlab::Database::PartitioningMigrationHelpers + + DOWNTIME = false + + CREATED_AT_AUTHOR_ID_INDEX_NAME = 'analytics_index_audit_events_part_on_created_at_and_author_id' + ENTITY_ID_DESC_INDEX_NAME = 'idx_audit_events_part_on_entity_id_desc_author_id_created_at' + + disable_ddl_transaction! + + def up + add_concurrent_partitioned_index :audit_events_part_5fc467ac26, + [:created_at, :author_id], + name: CREATED_AT_AUTHOR_ID_INDEX_NAME + + add_concurrent_partitioned_index :audit_events_part_5fc467ac26, + [:entity_id, :entity_type, :id, :author_id, :created_at], + order: { id: :desc }, + name: ENTITY_ID_DESC_INDEX_NAME + end + + def down + remove_concurrent_partitioned_index_by_name :audit_events_part_5fc467ac26, ENTITY_ID_DESC_INDEX_NAME + + remove_concurrent_partitioned_index_by_name :audit_events_part_5fc467ac26, CREATED_AT_AUTHOR_ID_INDEX_NAME + end +end diff --git a/db/schema_migrations/20201112215028 b/db/schema_migrations/20201112215028 new file mode 100644 index 00000000000..07cedc7a146 --- /dev/null +++ b/db/schema_migrations/20201112215028 @@ -0,0 +1 @@ +d8d774e788eeaaecbda3cb7c5530926e74843d844bfad27b6a6e65bf5f89ac8a
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0cd8e6f5993..b10088d7ea9 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -20038,6 +20038,8 @@ CREATE INDEX active_billable_users ON users USING btree (id) WHERE (((state)::te CREATE INDEX analytics_index_audit_events_on_created_at_and_author_id ON audit_events USING btree (created_at, author_id); +CREATE INDEX analytics_index_audit_events_part_on_created_at_and_author_id ON ONLY audit_events_part_5fc467ac26 USING btree (created_at, author_id); + CREATE INDEX analytics_index_events_on_created_at_and_author_id ON events USING btree (created_at, author_id); CREATE INDEX analytics_repository_languages_on_project_id ON analytics_language_trend_repository_languages USING btree (project_id); @@ -20082,6 +20084,8 @@ CREATE INDEX finding_links_on_vulnerability_occurrence_id ON vulnerability_findi CREATE INDEX idx_audit_events_on_entity_id_desc_author_id_created_at ON audit_events USING btree (entity_id, entity_type, id DESC, author_id, created_at); +CREATE INDEX idx_audit_events_part_on_entity_id_desc_author_id_created_at ON ONLY audit_events_part_5fc467ac26 USING btree (entity_id, entity_type, id DESC, author_id, created_at); + CREATE INDEX idx_ci_pipelines_artifacts_locked ON ci_pipelines USING btree (ci_ref_id, id) WHERE (locked = 1); CREATE INDEX idx_container_exp_policies_on_project_id_next_run_at_enabled ON container_expiration_policies USING btree (project_id, next_run_at, enabled); diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 4ce05199d49..60ee98703aa 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -23031,6 +23031,11 @@ type User { id: ID! """ + The location of the user. + """ + location: String + + """ Human-readable name of the user """ name: String! diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index fa5ae0d8dcb..c1615dd9436 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -66795,6 +66795,20 @@ "deprecationReason": null }, { + "name": "location", + "description": "The location of the user.", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { "name": "name", "description": "Human-readable name of the user", "args": [ diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 4ee61debade..e4250701f82 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -3450,6 +3450,7 @@ Autogenerated return type of UpdateSnippet. | `groupCount` | Int | Group count for the user. Available only when feature flag `user_group_counts` is enabled | | `groupMemberships` | GroupMemberConnection | Group memberships of the user | | `id` | ID! | ID of the user | +| `location` | String | The location of the user. | | `name` | String! | Human-readable name of the user | | `projectMemberships` | ProjectMemberConnection | Project memberships of the user | | `snippets` | SnippetConnection | Snippets authored by the user | diff --git a/doc/api/users.md b/doc/api/users.md index 0d837924507..9b9005754d2 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -1275,8 +1275,8 @@ Parameters: Returns: - `201 OK` on success. -- `404 User Not Found` if user cannot be found. -- `403 Forbidden` when trying to activate a user blocked by admin or by LDAP synchronization. +- `404 User Not Found` if the user cannot be found. +- `403 Forbidden` if the user cannot be activated because they are blocked by an administrator or by LDAP synchronization. ### Get user contribution events @@ -1337,6 +1337,44 @@ Example response: ] ``` +## Approve user + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/263107) in GitLab 13.7. + +Approves the specified user. Available only for administrators. + +```plaintext +POST /users/:id/approve +``` + +Parameters: + +- `id` (required) - ID of specified user + +```shell +curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/users/42/approve" +``` + +Returns: + +- `201 OK` on success. +- `404 User Not Found` if user cannot be found. +- `403 Forbidden` if the user cannot be approved because they are blocked by an administrator or by LDAP synchronization. + +Example Responses: + +```json +{ "message": "Success" } +``` + +```json +{ "message": "404 User Not Found" } +``` + +```json +{ "message": "The user you are trying to approve is not pending an approval" } +``` + ## Get an impersonation token of a user > Requires admin permissions. diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 77684576154..5063d8da6e5 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1342,14 +1342,7 @@ if there is no `if:` statement that limits the job to branch or merge request pi ##### Variables in `rules:changes` > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/34272) in GitLab 13.6. -> - It was [deployed behind a feature flag](../../user/feature_flags.md), disabled by default. -> - [Became enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/267192) in GitLab 13.6. -> - It's enabled on GitLab.com. -> - It's recommended for production use. -> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-variables-support-in-ruleschanges). **(CORE ONLY)** - -CAUTION: **Warning:** -This feature might not be available to you. Check the **version history** note above for details. +> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/267192) in GitLab 13.7. Environment variables can be used in `rules:changes` expressions to determine when to add jobs to a pipeline: @@ -1368,25 +1361,6 @@ The `$` character can be used for both variables and paths. For example, if the `$DOCKERFILES_DIR` variable exists, its value is used. If it does not exist, the `$` is interpreted as being part of a path. -###### Enable or disable variables support in `rules:changes` **(CORE ONLY)** - -Variables support in `rules:changes` is under development, but is ready for production use. -It is deployed behind a feature flag that is **enabled by default**. -[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) -can opt to disable it. - -To enable it: - -```ruby -Feature.enable(:ci_variable_expansion_in_rules_changes) -``` - -To disable it: - -```ruby -Feature.disable(:ci_variable_expansion_in_rules_changes) -``` - #### `rules:exists` > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/24021) in GitLab 12.4. diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index 32b7f18c840..a10557d2db8 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -257,19 +257,22 @@ Users of GitLab 12.1 and earlier should use the command `gitlab-rake gitlab:back #### Excluding specific directories from the backup -You can choose what should be exempt from the backup by adding the environment -variable `SKIP`. The available options are: +You can exclude specific directories from the backup by adding the environment variable `SKIP`, whose values are a comma-separated list of the following options: - `db` (database) - `uploads` (attachments) -- `repositories` (Git repositories data) - `builds` (CI job output logs) - `artifacts` (CI job artifacts) - `lfs` (LFS objects) - `registry` (Container Registry images) - `pages` (Pages content) +- `repositories` (Git repositories data) -Use a comma to specify several options at the same time: +All wikis will be backed up as part of the `repositories` group. Non-existent wikis will be skipped during a backup. + +NOTE: **Note:** +When [backing up and restoring Helm Charts](https://docs.gitlab.com/charts/architecture/backup-restore.html), there is an additional option `packages`, which refers to any packages managed by the GitLab [package registry](../user/packages/package_registry/index.md). +For more information see [command line arguments](https://docs.gitlab.com/charts/architecture/backup-restore.html#command-line-arguments). All wikis are backed up as part of the `repositories` group. Non-existent wikis are skipped during a backup. diff --git a/doc/user/application_security/secret_detection/index.md b/doc/user/application_security/secret_detection/index.md index 5eba0fa44ba..153753ea0c0 100644 --- a/doc/user/application_security/secret_detection/index.md +++ b/doc/user/application_security/secret_detection/index.md @@ -283,3 +283,25 @@ Support for custom certificate authorities was introduced in the following versi ### Getting warning message `gl-secret-detection-report.json: no matching files` For information on this, see the [general Application Security troubleshooting section](../../../ci/pipelines/job_artifacts.md#error-message-no-files-to-upload). + +### Error: `Couldn't run the gitleaks command: exit status 2` + +This error is usually caused by the `GIT_DEPTH` value of 50 that is set for all [projects by default](../../../ci/pipelines/settings.md#git-shallow-clone). + +For example, if a pipeline is triggered from a Merge Request containing 60 commits while the `GIT_DEPTH` is set to 50, the Secret Detection job will fail as the clone will not have been deep enough to contain all of the relevant commits. + +You can confirm this to be the cause of the error by implementing a [logging level](../../application_security/secret_detection/index.md#logging-level) of `debug`. Once implemented, the logs should look similar to the following example, wherein an "object not found" error can be seen: + +```plaintext +ERRO[2020-11-18T18:05:52Z] object not found +[ERRO] [secrets] [2020-11-18T18:05:52Z] â–¶ Couldn't run the gitleaks command: exit status 2 +[ERRO] [secrets] [2020-11-18T18:05:52Z] â–¶ Gitleaks analysis failed: exit status 2 +``` + +If this is the case, we can resolve the issue by setting the [`GIT_DEPTH` variable](../../../ci/runners/README.md#shallow-cloning) to a higher value. In order to apply this only to the Secret Detection job, the following can be added to your `.gitlab-ci.yml`: + +```yaml +secret_detection: + variables: + GIT_DEPTH: 100 +``` diff --git a/lib/api/users.rb b/lib/api/users.rb index 501ed629c7e..8b9b82877f7 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -534,6 +534,24 @@ module API user.activate end + + desc 'Approve a pending user. Available only for admins.' + params do + requires :id, type: Integer, desc: 'The ID of the user' + end + post ':id/approve', feature_category: :authentication_and_authorization do + user = User.find_by(id: params[:id]) + not_found!('User') unless can?(current_user, :read_user, user) + + result = ::Users::ApproveService.new(current_user).execute(user) + + if result[:success] + result + else + render_api_error!(result[:message], result[:http_status]) + end + end + # rubocop: enable CodeReuse/ActiveRecord desc 'Deactivate an active user. Available only for admins.' params do diff --git a/lib/gitlab/ci/build/rules/rule/clause/changes.rb b/lib/gitlab/ci/build/rules/rule/clause/changes.rb index cbecce57163..9c2f6eea1dd 100644 --- a/lib/gitlab/ci/build/rules/rule/clause/changes.rb +++ b/lib/gitlab/ci/build/rules/rule/clause/changes.rb @@ -11,7 +11,7 @@ module Gitlab def satisfied_by?(pipeline, context) return true if pipeline.modified_paths.nil? - expanded_globs = expand_globs(pipeline, context) + expanded_globs = expand_globs(context) pipeline.modified_paths.any? do |path| expanded_globs.any? do |glob| File.fnmatch?(glob, path, File::FNM_PATHNAME | File::FNM_DOTMATCH | File::FNM_EXTGLOB) @@ -19,8 +19,7 @@ module Gitlab end end - def expand_globs(pipeline, context) - return @globs unless ::Feature.enabled?(:ci_variable_expansion_in_rules_changes, pipeline.project, default_enabled: true) + def expand_globs(context) return @globs unless context @globs.map do |glob| diff --git a/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml index 385959389de..e5b40e5f49a 100644 --- a/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml @@ -1,5 +1,5 @@ .auto-deploy: - image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v2.0.0-beta.2" + image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v2.0.0" dependencies: [] review: diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2a9eda83746..6e84c95b3ae 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2733,6 +2733,9 @@ msgstr "" msgid "AlertsIntegrations|The integration token could not be reset. Please try again." msgstr "" +msgid "AlertsIntegrations|The test alert has been successfully sent, and should now be visible on your alerts list." +msgstr "" + msgid "AlertsIntegrations|You have opted to delete the %{integrationName} integration. Do you want to proceed? It means you will no longer receive alerts from this endpoint in your alert list, and this action cannot be undone." msgstr "" diff --git a/rubocop/cop/lint/last_keyword_argument.rb b/rubocop/cop/lint/last_keyword_argument.rb new file mode 100644 index 00000000000..d8f8d03b552 --- /dev/null +++ b/rubocop/cop/lint/last_keyword_argument.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Lint + # This cop only works if there are files from deprecation_toolkit. You can + # generate these files by: + # + # 1. Running specs with RECORD_DEPRECATIONS=1 + # 1. Downloading the complete set of deprecations/ files from a CI + # pipeline (see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47720) + class LastKeywordArgument < Cop + MSG = 'Using the last argument as keyword parameters is deprecated'.freeze + + DEPRECATIONS_GLOB = File.expand_path('../../../deprecations/**/*.yml', __dir__) + KEYWORD_DEPRECATION_STR = 'maybe ** should be added to the call' + + def on_send(node) + arg = node.arguments.last + return unless arg + + return unless known_match?(processed_source.file_path, node.first_line, node.method_name.to_s) + + return if arg.children.first.respond_to?(:kwsplat_type?) && arg.children.first&.kwsplat_type? + + # parser thinks `a: :b, c: :d` is hash type, it's actually kwargs + return if arg.hash_type? && !arg.source.match(/\A{/) + + add_offense(arg) + end + + def autocorrect(arg) + lambda do |corrector| + if arg.hash_type? + kwarg = arg.source.sub(/\A{\s*/, '').sub(/\s*}\z/, '') + corrector.replace(arg, kwarg) + elsif arg.splat_type? + corrector.insert_before(arg, '*') + else + corrector.insert_before(arg, '**') + end + end + end + + private + + def known_match?(file_path, line_number, method_name) + file_path_from_root = file_path.sub(File.expand_path('../../..', __dir__), '') + + self.class.keyword_warnings.any? do |warning| + warning.include?("#{file_path_from_root}:#{line_number}") && warning.include?("called method `#{method_name}'") + end + end + + def self.keyword_warnings + @keyword_warnings ||= keywords_list + end + + def self.keywords_list + hash = Dir.glob(DEPRECATIONS_GLOB).each_with_object({}) do |file, hash| + hash.merge!(YAML.safe_load(File.read(file))) + end + + hash.values.flatten.select { |str| str.include?(KEYWORD_DEPRECATION_STR) }.uniq + end + end + end + end +end diff --git a/spec/deprecation_toolkit_env.rb b/spec/deprecation_toolkit_env.rb new file mode 100644 index 00000000000..bc90f67f0db --- /dev/null +++ b/spec/deprecation_toolkit_env.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +if ENV.key?('RECORD_DEPRECATIONS') + require 'deprecation_toolkit' + require 'deprecation_toolkit/rspec' + DeprecationToolkit::Configuration.test_runner = :rspec + DeprecationToolkit::Configuration.deprecation_path = 'deprecations' + DeprecationToolkit::Configuration.behavior = DeprecationToolkit::Behaviors::Record + + # Enable ruby deprecations for keywords, it's suppressed by default in Ruby 2.7.2 + Warning[:deprecated] = true + + kwargs_warnings = [ + # Taken from https://github.com/jeremyevans/ruby-warning/blob/1.1.0/lib/warning.rb#L18 + %r{warning: (?:Using the last argument (?:for `.+' )?as keyword parameters is deprecated; maybe \*\* should be added to the call|Passing the keyword argument (?:for `.+' )?as the last hash parameter is deprecated|Splitting the last argument (?:for `.+' )?into positional and keyword parameters is deprecated|The called method (?:`.+' )?is defined here)\n\z} + ] + DeprecationToolkit::Configuration.warnings_treated_as_deprecation = kwargs_warnings +end diff --git a/spec/factories/terraform/state.rb b/spec/factories/terraform/state.rb index c54a8aedbc6..fb63c845073 100644 --- a/spec/factories/terraform/state.rb +++ b/spec/factories/terraform/state.rb @@ -6,11 +6,6 @@ FactoryBot.define do sequence(:name) { |n| "state-#{n}" } - trait :with_file do - versioning_enabled { false } - file { fixture_file_upload('spec/fixtures/terraform/terraform.tfstate', 'application/json') } - end - trait :locked do sequence(:lock_xid) { |n| "lock-#{n}" } locked_at { Time.current } @@ -22,8 +17,5 @@ FactoryBot.define do create(:terraform_state_version, terraform_state: state) end end - - # Remove with https://gitlab.com/gitlab-org/gitlab/-/issues/235108 - factory :legacy_terraform_state, parent: :terraform_state, traits: [:with_file] end end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 1b430009ab5..50656d14eb7 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -35,6 +35,10 @@ FactoryBot.define do user_type { :alert_bot } end + trait :deactivated do + after(:build) { |user, _| user.deactivate! } + end + trait :project_bot do user_type { :project_bot } end diff --git a/spec/frontend/alerts_settings/__snapshots__/alerts_settings_form_new_spec.js.snap b/spec/frontend/alerts_settings/__snapshots__/alerts_settings_form_spec.js.snap index 98c79b406f2..a1ced8910b3 100644 --- a/spec/frontend/alerts_settings/__snapshots__/alerts_settings_form_new_spec.js.snap +++ b/spec/frontend/alerts_settings/__snapshots__/alerts_settings_form_spec.js.snap @@ -87,7 +87,7 @@ exports[`AlertsSettingsFormNew with default values renders the initial template <div class=\\"gl-display-flex gl-justify-content-start gl-py-3\\"><button data-testid=\\"integration-form-submit\\" type=\\"submit\\" class=\\"btn js-no-auto-disable btn-success btn-md gl-button\\"> <!----> <!----> <span class=\\"gl-button-text\\">Save integration - </span></button> <button data-testid=\\"integration-test-and-submit\\" type=\\"button\\" class=\\"btn gl-mx-3 js-no-auto-disable btn-success btn-md gl-button btn-success-secondary\\"> + </span></button> <button data-testid=\\"integration-test-and-submit\\" type=\\"button\\" disabled=\\"disabled\\" class=\\"btn gl-mx-3 js-no-auto-disable btn-success btn-md disabled gl-button btn-success-secondary\\"> <!----> <!----> <span class=\\"gl-button-text\\">Save and test payload</span></button> <button type=\\"reset\\" class=\\"btn js-no-auto-disable btn-default btn-md gl-button\\"> <!----> diff --git a/spec/frontend/alerts_settings/alerts_settings_form_new_spec.js b/spec/frontend/alerts_settings/alerts_settings_form_spec.js index fbd482b1906..dd7d32c5e75 100644 --- a/spec/frontend/alerts_settings/alerts_settings_form_new_spec.js +++ b/spec/frontend/alerts_settings/alerts_settings_form_spec.js @@ -8,7 +8,7 @@ import { GlFormTextarea, } from '@gitlab/ui'; import waitForPromises from 'helpers/wait_for_promises'; -import AlertsSettingsForm from '~/alerts_settings/components/alerts_settings_form_new.vue'; +import AlertsSettingsForm from '~/alerts_settings/components/alerts_settings_form.vue'; import { defaultAlertSettingsConfig } from './util'; import { typeSet } from '~/alerts_settings/constants'; diff --git a/spec/frontend/alerts_settings/alerts_settings_wrapper_spec.js b/spec/frontend/alerts_settings/alerts_settings_wrapper_spec.js index 91fb85e3b3e..fe187d9e8f9 100644 --- a/spec/frontend/alerts_settings/alerts_settings_wrapper_spec.js +++ b/spec/frontend/alerts_settings/alerts_settings_wrapper_spec.js @@ -1,11 +1,13 @@ import VueApollo from 'vue-apollo'; import { mount, createLocalVue } from '@vue/test-utils'; +import AxiosMockAdapter from 'axios-mock-adapter'; import createMockApollo from 'jest/helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import { GlLoadingIcon, GlAlert } from '@gitlab/ui'; import { useMockIntersectionObserver } from 'helpers/mock_dom_observer'; +import { GlLoadingIcon, GlAlert } from '@gitlab/ui'; +import axios from '~/lib/utils/axios_utils'; import AlertsSettingsWrapper from '~/alerts_settings/components/alerts_settings_wrapper.vue'; -import AlertsSettingsFormNew from '~/alerts_settings/components/alerts_settings_form_new.vue'; +import AlertsSettingsForm from '~/alerts_settings/components/alerts_settings_form.vue'; import IntegrationsList from '~/alerts_settings/components/alerts_integrations_list.vue'; import getIntegrationsQuery from '~/alerts_settings/graphql/queries/get_integrations.query.graphql'; import createHttpIntegrationMutation from '~/alerts_settings/graphql/mutations/create_http_integration.mutation.graphql'; @@ -113,17 +115,15 @@ describe('AlertsSettingsWrapper', () => { } afterEach(() => { - if (wrapper) { - wrapper.destroy(); - wrapper = null; - } + wrapper.destroy(); + wrapper = null; }); describe('rendered via default permissions', () => { it('renders the GraphQL alerts integrations list and new form', () => { createComponent(); expect(wrapper.find(IntegrationsList).exists()).toBe(true); - expect(wrapper.find(AlertsSettingsFormNew).exists()).toBe(true); + expect(wrapper.find(AlertsSettingsForm).exists()).toBe(true); }); it('uses a loading state inside the IntegrationsList table', () => { @@ -153,7 +153,7 @@ describe('AlertsSettingsWrapper', () => { jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue({ data: { createHttpIntegrationMutation: { integration: { id: '1' } } }, }); - wrapper.find(AlertsSettingsFormNew).vm.$emit('create-new-integration', { + wrapper.find(AlertsSettingsForm).vm.$emit('create-new-integration', { type: typeSet.http, variables: createHttpVariables, }); @@ -175,7 +175,7 @@ describe('AlertsSettingsWrapper', () => { jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue({ data: { updateHttpIntegrationMutation: { integration: { id: '1' } } }, }); - wrapper.find(AlertsSettingsFormNew).vm.$emit('update-integration', { + wrapper.find(AlertsSettingsForm).vm.$emit('update-integration', { type: typeSet.http, variables: updateHttpVariables, }); @@ -195,7 +195,7 @@ describe('AlertsSettingsWrapper', () => { jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue({ data: { resetHttpTokenMutation: { integration: { id: '1' } } }, }); - wrapper.find(AlertsSettingsFormNew).vm.$emit('reset-token', { + wrapper.find(AlertsSettingsForm).vm.$emit('reset-token', { type: typeSet.http, variables: { id: ID }, }); @@ -217,7 +217,7 @@ describe('AlertsSettingsWrapper', () => { jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue({ data: { createPrometheusIntegrationMutation: { integration: { id: '2' } } }, }); - wrapper.find(AlertsSettingsFormNew).vm.$emit('create-new-integration', { + wrapper.find(AlertsSettingsForm).vm.$emit('create-new-integration', { type: typeSet.prometheus, variables: createPrometheusVariables, }); @@ -239,7 +239,7 @@ describe('AlertsSettingsWrapper', () => { jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue({ data: { updatePrometheusIntegrationMutation: { integration: { id: '2' } } }, }); - wrapper.find(AlertsSettingsFormNew).vm.$emit('update-integration', { + wrapper.find(AlertsSettingsForm).vm.$emit('update-integration', { type: typeSet.prometheus, variables: updatePrometheusVariables, }); @@ -259,7 +259,7 @@ describe('AlertsSettingsWrapper', () => { jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue({ data: { resetPrometheusTokenMutation: { integration: { id: '1' } } }, }); - wrapper.find(AlertsSettingsFormNew).vm.$emit('reset-token', { + wrapper.find(AlertsSettingsForm).vm.$emit('reset-token', { type: typeSet.prometheus, variables: { id: ID }, }); @@ -279,7 +279,7 @@ describe('AlertsSettingsWrapper', () => { }); jest.spyOn(wrapper.vm.$apollo, 'mutate').mockRejectedValue(ADD_INTEGRATION_ERROR); - wrapper.find(AlertsSettingsFormNew).vm.$emit('create-new-integration', {}); + wrapper.find(AlertsSettingsForm).vm.$emit('create-new-integration', {}); await waitForPromises(); @@ -294,7 +294,7 @@ describe('AlertsSettingsWrapper', () => { jest.spyOn(wrapper.vm.$apollo, 'mutate').mockRejectedValue(RESET_INTEGRATION_TOKEN_ERROR); - wrapper.find(AlertsSettingsFormNew).vm.$emit('reset-token', {}); + wrapper.find(AlertsSettingsForm).vm.$emit('reset-token', {}); await waitForPromises(); expect(createFlash).toHaveBeenCalledWith({ message: RESET_INTEGRATION_TOKEN_ERROR }); @@ -308,23 +308,25 @@ describe('AlertsSettingsWrapper', () => { jest.spyOn(wrapper.vm.$apollo, 'mutate').mockRejectedValue(errorMsg); - wrapper.find(AlertsSettingsFormNew).vm.$emit('update-integration', {}); + wrapper.find(AlertsSettingsForm).vm.$emit('update-integration', {}); await waitForPromises(); expect(createFlash).toHaveBeenCalledWith({ message: UPDATE_INTEGRATION_ERROR }); }); it('shows an error alert when integration test payload fails ', async () => { + const mock = new AxiosMockAdapter(axios); + mock.onPost(/(.*)/).replyOnce(403); createComponent({ data: { integrations: { list: mockIntegrations }, currentIntegration: mockIntegrations[0] }, loading: false, }); - wrapper.find(AlertsSettingsFormNew).vm.$emit('test-payload-failure'); - - await waitForPromises(); - expect(createFlash).toHaveBeenCalledWith({ message: INTEGRATION_PAYLOAD_TEST_ERROR }); - expect(createFlash).toHaveBeenCalledTimes(1); + return wrapper.vm.validateAlertPayload({ endpoint: '', data: '', token: '' }).then(() => { + expect(createFlash).toHaveBeenCalledWith({ message: INTEGRATION_PAYLOAD_TEST_ERROR }); + expect(createFlash).toHaveBeenCalledTimes(1); + mock.restore(); + }); }); }); diff --git a/spec/graphql/mutations/boards/lists/create_spec.rb b/spec/graphql/mutations/boards/lists/create_spec.rb index b1fe9911c7b..7a638d11ed3 100644 --- a/spec/graphql/mutations/boards/lists/create_spec.rb +++ b/spec/graphql/mutations/boards/lists/create_spec.rb @@ -23,12 +23,12 @@ RSpec.describe Mutations::Boards::Lists::Create do describe '#ready?' do it 'raises an error if required arguments are missing' do - expect { mutation.ready?({ board_id: 'some id' }) } + expect { mutation.ready?(board_id: 'some id') } .to raise_error(Gitlab::Graphql::Errors::ArgumentError, /one and only one of/) end it 'raises an error if too many required arguments are specified' do - expect { mutation.ready?({ board_id: 'some id', backlog: true, label_id: 'some label' }) } + expect { mutation.ready?(board_id: 'some id', backlog: true, label_id: 'some label') } .to raise_error(Gitlab::Graphql::Errors::ArgumentError, /one and only one of/) end end diff --git a/spec/graphql/types/user_type_spec.rb b/spec/graphql/types/user_type_spec.rb index c8953d9ccb7..4e20efe40d6 100644 --- a/spec/graphql/types/user_type_spec.rb +++ b/spec/graphql/types/user_type_spec.rb @@ -21,6 +21,7 @@ RSpec.describe GitlabSchema.types['User'] do todos state status + location authoredMergeRequests assignedMergeRequests groupMemberships diff --git a/spec/models/terraform/state_spec.rb b/spec/models/terraform/state_spec.rb index ca8fe4cca3b..ef91e4a5a71 100644 --- a/spec/models/terraform/state_spec.rb +++ b/spec/models/terraform/state_spec.rb @@ -3,21 +3,13 @@ require 'spec_helper' RSpec.describe Terraform::State do - subject { create(:terraform_state, :with_file) } - - let(:terraform_state_file) { fixture_file('terraform/terraform.tfstate') } - - it { is_expected.to be_a FileStoreMounter } + subject { create(:terraform_state, :with_version) } it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:locked_by_user).class_name('User') } it { is_expected.to validate_presence_of(:project_id) } - before do - stub_terraform_state_object_storage - end - describe 'scopes' do describe '.ordered_by_name' do let_it_be(:project) { create(:project) } @@ -35,64 +27,18 @@ RSpec.describe Terraform::State do end end - describe '#file' do - context 'when a file exists' do - it 'does not use the default file' do - expect(subject.file.read).to eq(terraform_state_file) - end - end - end - - describe '#file_store' do - context 'when a value is set' do - it 'returns the value' do - [ObjectStorage::Store::LOCAL, ObjectStorage::Store::REMOTE].each do |store| - expect(build(:terraform_state, file_store: store).file_store).to eq(store) - end - end - end - end - - describe '#update_file_store' do - context 'when file is stored in object storage' do - it_behaves_like 'mounted file in object store' - end - - context 'when file is stored locally' do - before do - stub_terraform_state_object_storage(enabled: false) - end - - it_behaves_like 'mounted file in local store' - end - end - describe '#latest_file' do - subject { terraform_state.latest_file } - - context 'versioning is enabled' do - let(:terraform_state) { create(:terraform_state, :with_version) } - let(:latest_version) { terraform_state.latest_version } - - it { is_expected.to eq latest_version.file } - - context 'but no version exists yet' do - let(:terraform_state) { create(:terraform_state) } + let(:terraform_state) { create(:terraform_state, :with_version) } + let(:latest_version) { terraform_state.latest_version } - it { is_expected.to be_nil } - end - end - - context 'versioning is disabled' do - let(:terraform_state) { create(:terraform_state, :with_file) } + subject { terraform_state.latest_file } - it { is_expected.to eq terraform_state.file } + it { is_expected.to eq latest_version.file } - context 'and a version exists (migration to versioned in progress)' do - let!(:migrated_version) { create(:terraform_state_version, terraform_state: terraform_state) } + context 'but no version exists yet' do + let(:terraform_state) { create(:terraform_state) } - it { is_expected.to eq terraform_state.latest_version.file } - end + it { is_expected.to be_nil } end end @@ -115,39 +61,30 @@ RSpec.describe Terraform::State do end end - context 'versioning is disabled' do - let(:terraform_state) { create(:terraform_state, :with_file) } - - it 'modifies the existing state record' do - expect { subject }.not_to change { Terraform::StateVersion.count } + context 'versioning is disabled (migration to versioned in progress)' do + let(:terraform_state) { create(:terraform_state, versioning_enabled: false) } + let!(:migrated_version) { create(:terraform_state_version, terraform_state: terraform_state, version: 0) } - expect(terraform_state.latest_file.read).to eq(data) - end - - context 'and a version exists (migration to versioned in progress)' do - let!(:migrated_version) { create(:terraform_state_version, terraform_state: terraform_state, version: 0) } + it 'creates a new version, corrects the migrated version number, and marks the state as versioned' do + expect { subject }.to change { Terraform::StateVersion.count } - it 'creates a new version, corrects the migrated version number, and marks the state as versioned' do - expect { subject }.to change { Terraform::StateVersion.count } + expect(migrated_version.reload.version).to eq(1) + expect(migrated_version.file.read).to eq(fixture_file('terraform/terraform.tfstate')) - expect(migrated_version.reload.version).to eq(1) - expect(migrated_version.file.read).to eq(terraform_state_file) + expect(terraform_state.reload.latest_version.version).to eq(version) + expect(terraform_state.latest_version.file.read).to eq(data) + expect(terraform_state).to be_versioning_enabled + end - expect(terraform_state.reload.latest_version.version).to eq(version) - expect(terraform_state.latest_version.file.read).to eq(data) - expect(terraform_state).to be_versioning_enabled + context 'the current version cannot be determined' do + before do + migrated_version.update!(file: CarrierWaveStringFile.new('invalid-json')) end - context 'the current version cannot be determined' do - before do - migrated_version.update!(file: CarrierWaveStringFile.new('invalid-json')) - end - - it 'uses version - 1 to correct the migrated version number' do - expect { subject }.to change { Terraform::StateVersion.count } + it 'uses version - 1 to correct the migrated version number' do + expect { subject }.to change { Terraform::StateVersion.count } - expect(migrated_version.reload.version).to eq(2) - end + expect(migrated_version.reload.version).to eq(2) end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 19a6a3ce3c4..2c3b561c491 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2024,9 +2024,10 @@ RSpec.describe User do end describe '.search' do - let!(:user) { create(:user, name: 'user', username: 'usern', email: 'email@gmail.com') } - let!(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@gmail.com') } - let!(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@gmail.com') } + let_it_be(:user) { create(:user, name: 'user', username: 'usern', email: 'email@example.com') } + let_it_be(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@example.com') } + let_it_be(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@example.com') } + let_it_be(:email) { create(:email, user: user, email: 'alias@example.com') } describe 'name matching' do it 'returns users with a matching name with exact match first' do @@ -2056,7 +2057,7 @@ RSpec.describe User do end it 'does not return users with a partially matching Email' do - expect(described_class.search(user.email[0..2])).not_to include(user, user2) + expect(described_class.search(user.email[1...-1])).to be_empty end it 'returns users with a matching Email regardless of the casing' do @@ -2064,6 +2065,36 @@ RSpec.describe User do end end + describe 'secondary email matching' do + context 'feature flag :user_search_secondary_email is enabled' do + it 'returns users with a matching secondary email' do + expect(described_class.search(email.email)).to include(email.user) + end + + it 'does not return users with a matching part of secondary email' do + expect(described_class.search(email.email[1...-1])).to be_empty + end + + it 'returns users with a matching secondary email regardless of the casing' do + expect(described_class.search(email.email.upcase)).to include(email.user) + end + end + + context 'feature flag :user_search_secondary_email is disabled' do + before do + stub_feature_flags(user_search_secondary_email: false) + end + + it 'does not return users with a matching secondary email' do + expect(described_class.search(email.email)).not_to include(email.user) + end + + it 'does not return users with a matching part of secondary email' do + expect(described_class.search(email.email[1...-1])).to be_empty + end + end + end + describe 'username matching' do it 'returns users with a matching username' do expect(described_class.search(user.username)).to eq([user, user2]) @@ -2103,65 +2134,119 @@ RSpec.describe User do end end - describe '.search_with_secondary_emails' do - delegate :search_with_secondary_emails, to: :described_class + describe '.search_without_secondary_emails' do + let_it_be(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'someone.1@example.com' ) } + let_it_be(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'another.2@example.com' ) } + let_it_be(:email) { create(:email, user: another_user, email: 'alias@example.com') } + + it 'returns users with a matching name' do + expect(described_class.search_without_secondary_emails(user.name)).to eq([user]) + end + + it 'returns users with a partially matching name' do + expect(described_class.search_without_secondary_emails(user.name[0..2])).to eq([user]) + end + + it 'returns users with a matching name regardless of the casing' do + expect(described_class.search_without_secondary_emails(user.name.upcase)).to eq([user]) + end + + it 'returns users with a matching email' do + expect(described_class.search_without_secondary_emails(user.email)).to eq([user]) + end + + it 'does not return users with a partially matching email' do + expect(described_class.search_without_secondary_emails(user.email[1...-1])).to be_empty + end + + it 'returns users with a matching email regardless of the casing' do + expect(described_class.search_without_secondary_emails(user.email.upcase)).to eq([user]) + end + + it 'returns users with a matching username' do + expect(described_class.search_without_secondary_emails(user.username)).to eq([user]) + end + + it 'returns users with a partially matching username' do + expect(described_class.search_without_secondary_emails(user.username[0..2])).to eq([user]) + end + + it 'returns users with a matching username regardless of the casing' do + expect(described_class.search_without_secondary_emails(user.username.upcase)).to eq([user]) + end + + it 'does not return users with a matching whole secondary email' do + expect(described_class.search_without_secondary_emails(email.email)).not_to include(email.user) + end + + it 'does not return users with a matching part of secondary email' do + expect(described_class.search_without_secondary_emails(email.email[1...-1])).to be_empty + end - let!(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'john.doe@example.com' ) } - let!(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'albert.smith@example.com' ) } - let!(:email) do - create(:email, user: another_user, email: 'alias@example.com') + it 'returns no matches for an empty string' do + expect(described_class.search_without_secondary_emails('')).to be_empty + end + + it 'returns no matches for nil' do + expect(described_class.search_without_secondary_emails(nil)).to be_empty end + end + + describe '.search_with_secondary_emails' do + let_it_be(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'someone.1@example.com' ) } + let_it_be(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'another.2@example.com' ) } + let_it_be(:email) { create(:email, user: another_user, email: 'alias@example.com') } it 'returns users with a matching name' do - expect(search_with_secondary_emails(user.name)).to eq([user]) + expect(described_class.search_with_secondary_emails(user.name)).to eq([user]) end it 'returns users with a partially matching name' do - expect(search_with_secondary_emails(user.name[0..2])).to eq([user]) + expect(described_class.search_with_secondary_emails(user.name[0..2])).to eq([user]) end it 'returns users with a matching name regardless of the casing' do - expect(search_with_secondary_emails(user.name.upcase)).to eq([user]) + expect(described_class.search_with_secondary_emails(user.name.upcase)).to eq([user]) end it 'returns users with a matching email' do - expect(search_with_secondary_emails(user.email)).to eq([user]) + expect(described_class.search_with_secondary_emails(user.email)).to eq([user]) end it 'does not return users with a partially matching email' do - expect(search_with_secondary_emails(user.email[0..2])).not_to include([user]) + expect(described_class.search_with_secondary_emails(user.email[1...-1])).to be_empty end it 'returns users with a matching email regardless of the casing' do - expect(search_with_secondary_emails(user.email.upcase)).to eq([user]) + expect(described_class.search_with_secondary_emails(user.email.upcase)).to eq([user]) end it 'returns users with a matching username' do - expect(search_with_secondary_emails(user.username)).to eq([user]) + expect(described_class.search_with_secondary_emails(user.username)).to eq([user]) end it 'returns users with a partially matching username' do - expect(search_with_secondary_emails(user.username[0..2])).to eq([user]) + expect(described_class.search_with_secondary_emails(user.username[0..2])).to eq([user]) end it 'returns users with a matching username regardless of the casing' do - expect(search_with_secondary_emails(user.username.upcase)).to eq([user]) + expect(described_class.search_with_secondary_emails(user.username.upcase)).to eq([user]) end it 'returns users with a matching whole secondary email' do - expect(search_with_secondary_emails(email.email)).to eq([email.user]) + expect(described_class.search_with_secondary_emails(email.email)).to eq([email.user]) end it 'does not return users with a matching part of secondary email' do - expect(search_with_secondary_emails(email.email[1..4])).not_to include([email.user]) + expect(described_class.search_with_secondary_emails(email.email[1...-1])).to be_empty end it 'returns no matches for an empty string' do - expect(search_with_secondary_emails('')).to be_empty + expect(described_class.search_with_secondary_emails('')).to be_empty end it 'returns no matches for nil' do - expect(search_with_secondary_emails(nil)).to be_empty + expect(described_class.search_with_secondary_emails(nil)).to be_empty end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 98840d6238a..381e0b03589 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -2510,6 +2510,98 @@ RSpec.describe API::Users, :do_not_mock_admin_mode do end end + context 'approve pending user' do + shared_examples '404' do + it 'returns 404' do + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') + end + end + + describe 'POST /users/:id/approve' do + subject(:approve) { post api("/users/#{user_id}/approve", api_user) } + + let_it_be(:pending_user) { create(:user, :blocked_pending_approval) } + let_it_be(:deactivated_user) { create(:user, :deactivated) } + let_it_be(:blocked_user) { create(:user, :blocked) } + + context 'performed by a non-admin user' do + let(:api_user) { user } + let(:user_id) { pending_user.id } + + it 'is not authorized to perform the action' do + expect { approve }.not_to change { pending_user.reload.state } + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('You are not allowed to approve a user') + end + end + + context 'performed by an admin user' do + let(:api_user) { admin } + + context 'for a deactivated user' do + let(:user_id) { deactivated_user.id } + + it 'does not approve a deactivated user' do + expect { approve }.not_to change { deactivated_user.reload.state } + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response['message']).to eq('The user you are trying to approve is not pending an approval') + end + end + + context 'for an pending approval user' do + let(:user_id) { pending_user.id } + + it 'returns 201' do + expect { approve }.to change { pending_user.reload.state }.to('active') + expect(response).to have_gitlab_http_status(:created) + expect(json_response['message']).to eq('Success') + end + end + + context 'for an active user' do + let(:user_id) { user.id } + + it 'returns 201' do + expect { approve }.not_to change { user.reload.state } + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response['message']).to eq('The user you are trying to approve is not pending an approval') + end + end + + context 'for a blocked user' do + let(:user_id) { blocked_user.id } + + it 'returns 403' do + expect { approve }.not_to change { blocked_user.reload.state } + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response['message']).to eq('The user you are trying to approve is not pending an approval') + end + end + + context 'for a ldap blocked user' do + let(:user_id) { ldap_blocked_user.id } + + it 'returns 403' do + expect { approve }.not_to change { ldap_blocked_user.reload.state } + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response['message']).to eq('The user you are trying to approve is not pending an approval') + end + end + + context 'for a user that does not exist' do + let(:user_id) { non_existing_record_id } + + before do + approve + end + + it_behaves_like '404' + end + end + end + end + describe 'POST /users/:id/block' do let(:blocked_user) { create(:user, state: 'blocked') } diff --git a/spec/rubocop/cop/lint/last_keyword_argument_spec.rb b/spec/rubocop/cop/lint/last_keyword_argument_spec.rb new file mode 100644 index 00000000000..f942390569b --- /dev/null +++ b/spec/rubocop/cop/lint/last_keyword_argument_spec.rb @@ -0,0 +1,124 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require_relative '../../../../rubocop/cop/lint/last_keyword_argument' + +RSpec.describe RuboCop::Cop::Lint::LastKeywordArgument, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + before do + described_class.instance_variable_set(:@keyword_warnings, nil) + end + + context 'deprecation files does not exist' do + before do + allow(Dir).to receive(:glob).and_return([]) + allow(File).to receive(:exist?).and_return(false) + end + + it 'does not register an offense' do + expect_no_offenses(<<~SOURCE) + users.call(params) + SOURCE + end + end + + context 'deprecation files does exist' do + let(:create_spec_yaml) do + <<~YAML + --- + test_mutations/boards/lists/create#resolve_with_proper_permissions_backlog_list_creates_one_and_only_one_backlog: + - | + DEPRECATION WARNING: /Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/batch-loader-1.4.0/lib/batch_loader/graphql.rb:38: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call + /Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/batch-loader-1.4.0/lib/batch_loader.rb:26: warning: The called method `batch' is defined here + test_mutations/boards/lists/create#ready?_raises_an_error_if_required_arguments_are_missing: + - | + DEPRECATION WARNING: /Users/tkuah/code/ee-gdk/gitlab/create_service.rb:1: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call + /Users/tkuah/code/ee-gdk/gitlab/user.rb:17: warning: The called method `call' is defined here + YAML + end + + let(:projects_spec_yaml) do + <<~YAML + --- + test_api/projects_get_/projects_when_unauthenticated_behaves_like_projects_response_returns_an_array_of_projects: + - | + DEPRECATION WARNING: /Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/state_machines-activerecord-0.6.0/lib/state_machines/integrations/active_record.rb:511: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call + /Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.3.3/lib/active_record/suppressor.rb:43: warning: The called method `save' is defined here + - | + DEPRECATION WARNING: /Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rack-2.2.3/lib/rack/builder.rb:158: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call + /Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/grape-1.4.0/lib/grape/middleware/error.rb:30: warning: The called method `initialize' is defined here + YAML + end + + before do + allow(Dir).to receive(:glob).and_return(['deprecations/service/create_spec.yml', 'deprecations/api/projects_spec.yml']) + allow(File).to receive(:read).and_return(create_spec_yaml, projects_spec_yaml) + end + + it 'registers an offense' do + expect_offense(<<~SOURCE, 'create_service.rb') + users.call(params) + ^^^^^^ Using the last argument as keyword parameters is deprecated + SOURCE + + expect_correction(<<~SOURCE) + users.call(**params) + SOURCE + end + + it 'registers an offense and corrects by converting hash to kwarg' do + expect_offense(<<~SOURCE, 'create_service.rb') + users.call(id, { a: :b, c: :d }) + ^^^^^^^^^^^^^^^^ Using the last argument as keyword parameters is deprecated + SOURCE + + expect_correction(<<~SOURCE) + users.call(id, a: :b, c: :d) + SOURCE + end + + it 'registers an offense and corrects by converting splat to double splat' do + expect_offense(<<~SOURCE, 'create_service.rb') + users.call(id, *params) + ^^^^^^^ Using the last argument as keyword parameters is deprecated + SOURCE + + expect_correction(<<~SOURCE) + users.call(id, **params) + SOURCE + end + + it 'does not register an offense if already a kwarg', :aggregate_failures do + expect_no_offenses(<<~SOURCE, 'create_service.rb') + users.call(**params) + SOURCE + + expect_no_offenses(<<~SOURCE, 'create_service.rb') + users.call(id, a: :b, c: :d) + SOURCE + end + + it 'does not register an offense if the method name does not match' do + expect_no_offenses(<<~SOURCE, 'create_service.rb') + users.process(params) + SOURCE + end + + it 'does not register an offense if the line number does not match' do + expect_no_offenses(<<~SOURCE, 'create_service.rb') + users.process + users.call(params) + SOURCE + end + + it 'does not register an offense if the filename does not match' do + expect_no_offenses(<<~SOURCE, 'update_service.rb') + users.call(params) + SOURCE + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7051d975980..49775940584 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -8,6 +8,8 @@ if $".include?(File.expand_path('fast_spec_helper.rb', __dir__)) abort 'Aborting...' end +require './spec/deprecation_toolkit_env' + require './spec/simplecov_env' SimpleCovEnv.start! diff --git a/spec/support/helpers/stub_object_storage.rb b/spec/support/helpers/stub_object_storage.rb index 0fa83530672..dc54a21d0fa 100644 --- a/spec/support/helpers/stub_object_storage.rb +++ b/spec/support/helpers/stub_object_storage.rb @@ -94,13 +94,6 @@ module StubObjectStorage def stub_terraform_state_object_storage(**params) stub_object_storage_uploader(config: Gitlab.config.terraform_state.object_store, - uploader: Terraform::VersionedStateUploader, - remote_directory: 'terraform', - **params) - end - - def stub_terraform_state_version_object_storage(**params) - stub_object_storage_uploader(config: Gitlab.config.terraform_state.object_store, uploader: Terraform::StateUploader, remote_directory: 'terraform', **params) diff --git a/spec/uploaders/terraform/state_uploader_spec.rb b/spec/uploaders/terraform/state_uploader_spec.rb index dadfdf6e93f..bd8e7fbc016 100644 --- a/spec/uploaders/terraform/state_uploader_spec.rb +++ b/spec/uploaders/terraform/state_uploader_spec.rb @@ -3,23 +3,45 @@ require 'spec_helper' RSpec.describe Terraform::StateUploader do - subject { terraform_state.file } + subject { state_version.file } - let(:terraform_state) { create(:terraform_state, :with_file) } + let(:state_version) { create(:terraform_state_version) } before do stub_terraform_state_object_storage end describe '#filename' do - it 'contains the UUID of the terraform state record' do - expect(subject.filename).to include(terraform_state.uuid) + it 'contains the version of the terraform state record' do + expect(subject.filename).to eq("#{state_version.version}.tfstate") + end + + context 'legacy state with versioning disabled' do + let(:state) { create(:terraform_state, versioning_enabled: false) } + let(:state_version) { create(:terraform_state_version, terraform_state: state) } + + it 'contains the UUID of the terraform state record' do + expect(subject.filename).to eq("#{state_version.uuid}.tfstate") + end end end describe '#store_dir' do - it 'contains the ID of the project' do - expect(subject.store_dir).to include(terraform_state.project_id.to_s) + it 'hashes the project ID and UUID' do + expect(Gitlab::HashedPath).to receive(:new) + .with(state_version.uuid, root_hash: state_version.project_id) + .and_return(:store_dir) + + expect(subject.store_dir).to eq(:store_dir) + end + + context 'legacy state with versioning disabled' do + let(:state) { create(:terraform_state, versioning_enabled: false) } + let(:state_version) { create(:terraform_state_version, terraform_state: state) } + + it 'contains the ID of the project' do + expect(subject.store_dir).to include(state_version.project_id.to_s) + end end end @@ -27,7 +49,7 @@ RSpec.describe Terraform::StateUploader do it 'creates a digest with a secret key and the project id' do expect(OpenSSL::HMAC) .to receive(:digest) - .with('SHA256', Gitlab::Application.secrets.db_key_base, terraform_state.project_id.to_s) + .with('SHA256', Gitlab::Application.secrets.db_key_base, state_version.project_id.to_s) .and_return('digest') expect(subject.key).to eq('digest') diff --git a/spec/uploaders/terraform/versioned_state_uploader_spec.rb b/spec/uploaders/terraform/versioned_state_uploader_spec.rb deleted file mode 100644 index eeb54cb61c7..00000000000 --- a/spec/uploaders/terraform/versioned_state_uploader_spec.rb +++ /dev/null @@ -1,47 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Terraform::VersionedStateUploader do - subject { model.file } - - let(:model) { create(:terraform_state_version, :with_file) } - - before do - stub_terraform_state_object_storage - end - - describe '#filename' do - it 'contains the version of the terraform state record' do - expect(subject.filename).to eq("#{model.version}.tfstate") - end - - context 'legacy state with versioning disabled' do - let(:state) { create(:legacy_terraform_state) } - let(:model) { create(:terraform_state_version, terraform_state: state) } - - it 'contains the UUID of the terraform state record' do - expect(subject.filename).to eq("#{model.uuid}.tfstate") - end - end - end - - describe '#store_dir' do - it 'hashes the project ID and UUID' do - expect(Gitlab::HashedPath).to receive(:new) - .with(model.uuid, root_hash: model.project_id) - .and_return(:store_dir) - - expect(subject.store_dir).to eq(:store_dir) - end - - context 'legacy state with versioning disabled' do - let(:state) { create(:legacy_terraform_state) } - let(:model) { create(:terraform_state_version, terraform_state: state) } - - it 'contains the ID of the project' do - expect(subject.store_dir).to include(model.project_id.to_s) - end - end - end -end |