diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-14 06:09:17 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-14 06:09:17 +0000 |
| commit | 8e42824b115f56679b9c791570b27d6184fecad9 (patch) | |
| tree | 57be23d99f10ab34dce4645622211caa13eae236 | |
| parent | d91ff791fb4a0b595b2b3f1adc6e9dd55899e320 (diff) | |
| download | gitlab-ce-8e42824b115f56679b9c791570b27d6184fecad9.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
45 files changed, 657 insertions, 200 deletions
diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_autocomplete_tokens.js b/app/assets/javascripts/ci_variable_list/components/ci_variable_autocomplete_tokens.js index 9022bf51514..3f25e3df305 100644 --- a/app/assets/javascripts/ci_variable_list/components/ci_variable_autocomplete_tokens.js +++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_autocomplete_tokens.js @@ -1,28 +1,14 @@ -import { __ } from '~/locale'; - import { AWS_ACCESS_KEY_ID, AWS_DEFAULT_REGION, AWS_SECRET_ACCESS_KEY } from '../constants'; export const awsTokens = { [AWS_ACCESS_KEY_ID]: { name: AWS_ACCESS_KEY_ID, - /* Checks for exactly twenty characters that match key. - Based on greps suggested by Amazon at: - https://aws.amazon.com/blogs/security/a-safer-way-to-distribute-aws-credentials-to-ec2/ - */ - validation: val => /^[A-Za-z0-9]{20}$/.test(val), - invalidMessage: __('This variable does not match the expected pattern.'), }, [AWS_DEFAULT_REGION]: { name: AWS_DEFAULT_REGION, }, [AWS_SECRET_ACCESS_KEY]: { name: AWS_SECRET_ACCESS_KEY, - /* Checks for exactly forty characters that match secret. - Based on greps suggested by Amazon at: - https://aws.amazon.com/blogs/security/a-safer-way-to-distribute-aws-credentials-to-ec2/ - */ - validation: val => /^[A-Za-z0-9/+=]{40}$/.test(val), - invalidMessage: __('This variable does not match the expected pattern.'), }, }; diff --git a/app/assets/javascripts/error_tracking_settings/components/error_tracking_form.vue b/app/assets/javascripts/error_tracking_settings/components/error_tracking_form.vue index 0be42519092..0de67a8bcc7 100644 --- a/app/assets/javascripts/error_tracking_settings/components/error_tracking_form.vue +++ b/app/assets/javascripts/error_tracking_settings/components/error_tracking_form.vue @@ -59,14 +59,14 @@ export default { </div> <div class="col-4 col-md-3 gl-pl-0"> <loading-button - class="js-error-tracking-connect prepend-left-5 d-inline-flex" + class="js-error-tracking-connect gl-ml-2 d-inline-flex" :label="isLoadingProjects ? __('Connecting') : __('Connect')" :loading="isLoadingProjects" @click="fetchProjects" /> <icon v-show="connectSuccessful" - class="js-error-tracking-connect-success prepend-left-5 text-success align-middle" + class="js-error-tracking-connect-success gl-ml-2 text-success align-middle" :aria-label="__('Projects Successfully Retrieved')" name="check-circle" /> diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js index 0121fcf2859..14efc2acbac 100644 --- a/app/assets/javascripts/gfm_auto_complete.js +++ b/app/assets/javascripts/gfm_auto_complete.js @@ -30,7 +30,7 @@ export function membersBeforeSave(members) { const imgAvatar = `<img src="${member.avatar_url}" alt="${member.username}" class="avatar ${rectAvatarClass} avatar-inline center s26"/>`; const txtAvatar = `<div class="avatar ${rectAvatarClass} center avatar-inline s26">${autoCompleteAvatar}</div>`; const avatarIcon = member.mentionsDisabled - ? spriteIcon('notifications-off', 's16 vertical-align-middle prepend-left-5') + ? spriteIcon('notifications-off', 's16 vertical-align-middle gl-ml-2') : ''; return { diff --git a/app/assets/javascripts/ide/components/file_row_extra.vue b/app/assets/javascripts/ide/components/file_row_extra.vue index 51509cd5fe6..f7cf7a5b251 100644 --- a/app/assets/javascripts/ide/components/file_row_extra.vue +++ b/app/assets/javascripts/ide/components/file_row_extra.vue @@ -76,7 +76,7 @@ export default { data-container="body" data-placement="right" name="file-modified" - class="prepend-left-5 ide-file-modified" + class="gl-ml-2 ide-file-modified" /> </span> <changed-file-icon diff --git a/app/assets/javascripts/issuables_list/index.js b/app/assets/javascripts/issuables_list/index.js index 6bfb885a8af..6b0c56c8dbd 100644 --- a/app/assets/javascripts/issuables_list/index.js +++ b/app/assets/javascripts/issuables_list/index.js @@ -36,7 +36,7 @@ function mountIssuableListRootApp() { } function mountIssuablesListApp() { - if (!gon.features?.vueIssuablesList) { + if (!gon.features?.vueIssuablesList && !gon.features?.jiraIntegration) { return; } diff --git a/app/assets/javascripts/jobs/components/job_log_controllers.vue b/app/assets/javascripts/jobs/components/job_log_controllers.vue index bcec83a7aee..a68174d8e1d 100644 --- a/app/assets/javascripts/jobs/components/job_log_controllers.vue +++ b/app/assets/javascripts/jobs/components/job_log_controllers.vue @@ -77,7 +77,7 @@ export default { <gl-link v-if="rawPath" :href="rawPath" - class="js-raw-link text-plain text-underline prepend-left-5" + class="js-raw-link text-plain text-underline gl-ml-2" >{{ s__('Job|Complete Raw') }}</gl-link > </template> diff --git a/app/assets/javascripts/reports/components/report_section.vue b/app/assets/javascripts/reports/components/report_section.vue index 91a8329a6fe..63af8a5a9ac 100644 --- a/app/assets/javascripts/reports/components/report_section.vue +++ b/app/assets/javascripts/reports/components/report_section.vue @@ -179,7 +179,7 @@ export default { <div> {{ headerText }} <slot :name="slotName"></slot> - <popover v-if="hasPopover" :options="popoverOptions" class="prepend-left-5" /> + <popover v-if="hasPopover" :options="popoverOptions" class="gl-ml-2" /> </div> <slot name="subHeading"></slot> </div> diff --git a/app/helpers/dashboard_helper.rb b/app/helpers/dashboard_helper.rb index b38feb0fb6c..7bf3795d73a 100644 --- a/app/helpers/dashboard_helper.rb +++ b/app/helpers/dashboard_helper.rb @@ -41,7 +41,7 @@ module DashboardHelper if doc_href.present? link_to_doc = link_to(sprite_icon('question', size: 16), doc_href, - class: 'prepend-left-5', title: _('Documentation'), + class: 'gl-ml-2', title: _('Documentation'), target: '_blank', rel: 'noopener noreferrer') concat(link_to_doc) diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index c3f9d834685..f1d03602fc0 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -128,7 +128,7 @@ class JiraService < IssueTrackerService end def new_issue_url - "#{url}/secure/CreateIssue.jspa" + "#{url}/secure/CreateIssue!default.jspa" end alias_method :original_url, :url diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index cdb88058d8c..39b39bd2fce 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -341,6 +341,7 @@ class ProjectPolicy < BasePolicy enable :update_alert_management_alert enable :create_design enable :destroy_design + enable :read_terraform_state end rule { can?(:developer_access) & user_confirmed? }.policy do diff --git a/app/services/terraform/remote_state_handler.rb b/app/services/terraform/remote_state_handler.rb index d180a3a2432..d2c44d4a265 100644 --- a/app/services/terraform/remote_state_handler.rb +++ b/app/services/terraform/remote_state_handler.rb @@ -5,26 +5,17 @@ module Terraform include Gitlab::OptimisticLocking StateLockedError = Class.new(StandardError) + UnauthorizedError = Class.new(StandardError) - # rubocop: disable CodeReuse/ActiveRecord def find_with_lock - raise ArgumentError unless params[:name].present? - - state = Terraform::State.find_by(project: project, name: params[:name]) - raise ActiveRecord::RecordNotFound.new("Couldn't find state") unless state - - retry_optimistic_lock(state) { |state| yield state } if state && block_given? - state - end - # rubocop: enable CodeReuse/ActiveRecord - - def create_or_find! - raise ArgumentError unless params[:name].present? - - Terraform::State.create_or_find_by(project: project, name: params[:name]) + retrieve_with_lock(find_only: true) do |state| + yield state if block_given? + end end def handle_with_lock + raise UnauthorizedError unless can_modify_state? + retrieve_with_lock do |state| raise StateLockedError unless lock_matches?(state) @@ -36,6 +27,7 @@ module Terraform def lock! raise ArgumentError if params[:lock_id].blank? + raise UnauthorizedError unless can_modify_state? retrieve_with_lock do |state| raise StateLockedError if state.locked? @@ -49,6 +41,8 @@ module Terraform end def unlock! + raise UnauthorizedError unless can_modify_state? + retrieve_with_lock do |state| # force-unlock does not pass ID, so we ignore it if it is missing raise StateLockedError unless params[:lock_id].nil? || lock_matches?(state) @@ -63,8 +57,21 @@ module Terraform private - def retrieve_with_lock - create_or_find!.tap { |state| retry_optimistic_lock(state) { |state| yield state } } + def retrieve_with_lock(find_only: false) + create_or_find!(find_only: find_only).tap { |state| retry_optimistic_lock(state) { |state| yield state } } + end + + def create_or_find!(find_only:) + raise ArgumentError unless params[:name].present? + + find_params = { project: project, name: params[:name] } + + if find_only + Terraform::State.find_by(find_params) || # rubocop: disable CodeReuse/ActiveRecord + raise(ActiveRecord::RecordNotFound.new("Couldn't find state")) + else + Terraform::State.create_or_find_by(find_params) + end end def lock_matches?(state) @@ -73,5 +80,9 @@ module Terraform ActiveSupport::SecurityUtils .secure_compare(state.lock_xid.to_s, params[:lock_id].to_s) end + + def can_modify_state? + current_user.can?(:admin_terraform_state, project) + end end end diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 2e9be28df86..5b6243f17cd 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -8,13 +8,13 @@ = link_to project_tree_path(@project, branch.name), class: 'item-title str-truncated-100 ref-name gl-ml-3 qa-branch-name' do = branch.name - if branch.name == @repository.root_ref - %span.badge.badge-primary.prepend-left-5 default + %span.badge.badge-primary.gl-ml-2 default - elsif merged - %span.badge.badge-info.has-tooltip.prepend-left-5{ title: s_('Branches|Merged into %{default_branch}') % { default_branch: @repository.root_ref } } + %span.badge.badge-info.has-tooltip.gl-ml-2{ title: s_('Branches|Merged into %{default_branch}') % { default_branch: @repository.root_ref } } = s_('Branches|merged') - if protected_branch?(@project, branch) - %span.badge.badge-success.prepend-left-5 + %span.badge.badge-success.gl-ml-2 = s_('Branches|protected') = render_if_exists 'projects/branches/diverged_from_upstream', branch: branch diff --git a/app/views/projects/commit/_limit_exceeded_message.html.haml b/app/views/projects/commit/_limit_exceeded_message.html.haml index 7d3c0582d0b..ace1be787fb 100644 --- a/app/views/projects/commit/_limit_exceeded_message.html.haml +++ b/app/views/projects/commit/_limit_exceeded_message.html.haml @@ -1,4 +1,4 @@ -.has-tooltip{ class: "limit-box limit-box-#{objects} prepend-left-5", data: { title: _('Project has too many %{label_for_message} to search') % { label_for_message: label_for_message } } } +.has-tooltip{ class: "limit-box limit-box-#{objects} gl-ml-2", data: { title: _('Project has too many %{label_for_message} to search') % { label_for_message: label_for_message } } } .limit-icon - if objects == :branch = sprite_icon('fork', size: 12) diff --git a/app/views/projects/diffs/_stats.html.haml b/app/views/projects/diffs/_stats.html.haml index f5fbede7c3b..0e2a1165ad3 100644 --- a/app/views/projects/diffs/_stats.html.haml +++ b/app/views/projects/diffs/_stats.html.haml @@ -4,7 +4,7 @@ Showing %button.diff-stats-summary-toggler.js-diff-stats-dropdown{ type: "button", data: { toggle: "dropdown", display: "static" } }< = pluralize(diff_files.size, "changed file") - = icon("caret-down", class: "prepend-left-5") + = icon("caret-down", class: "gl-ml-2") %span.diff-stats-additions-deletions-expanded#diff-stats with %strong.cgreen= pluralize(sum_added_lines, 'addition') diff --git a/app/views/projects/protected_branches/shared/_matching_branch.html.haml b/app/views/projects/protected_branches/shared/_matching_branch.html.haml index 2c76bf87945..9145be5d2f2 100644 --- a/app/views/projects/protected_branches/shared/_matching_branch.html.haml +++ b/app/views/projects/protected_branches/shared/_matching_branch.html.haml @@ -3,7 +3,7 @@ = link_to matching_branch.name, project_ref_path(@project, matching_branch.name), class: 'ref-name' - if @project.root_ref?(matching_branch.name) - %span.badge.badge-info.prepend-left-5 default + %span.badge.badge-info.gl-ml-2 default %td - commit = @project.commit(matching_branch.name) = link_to(commit.short_id, project_commit_path(@project, commit.id), class: 'commit-sha') diff --git a/app/views/projects/protected_tags/shared/_matching_tag.html.haml b/app/views/projects/protected_tags/shared/_matching_tag.html.haml index 133c76cd2ad..bf030d36cd6 100644 --- a/app/views/projects/protected_tags/shared/_matching_tag.html.haml +++ b/app/views/projects/protected_tags/shared/_matching_tag.html.haml @@ -3,7 +3,7 @@ = link_to matching_tag.name, project_ref_path(@project, matching_tag.name), class: 'ref-name' - if @project.root_ref?(matching_tag.name) - %span.badge.badge-info.prepend-left-5 default + %span.badge.badge-info.gl-ml-2 default %td - commit = @project.commit(matching_tag.name) = link_to(commit.short_id, project_commit_path(@project, commit.id), class: 'commit-sha') diff --git a/app/views/projects/protected_tags/shared/_protected_tag.html.haml b/app/views/projects/protected_tags/shared/_protected_tag.html.haml index cc6f0309123..b0563163c9c 100644 --- a/app/views/projects/protected_tags/shared/_protected_tag.html.haml +++ b/app/views/projects/protected_tags/shared/_protected_tag.html.haml @@ -3,7 +3,7 @@ %span.ref-name= protected_tag.name - if @project.root_ref?(protected_tag.name) - %span.badge.badge-info.prepend-left-5 default + %span.badge.badge-info.gl-ml-2 default %td - if protected_tag.wildcard? - matching_tags = protected_tag.matching(repository.tags) diff --git a/app/views/projects/starrers/_starrer.html.haml b/app/views/projects/starrers/_starrer.html.haml index 377d62f8abd..d8a2c72d9ce 100644 --- a/app/views/projects/starrers/_starrer.html.haml +++ b/app/views/projects/starrers/_starrer.html.haml @@ -13,7 +13,7 @@ %span.cgray= starrer.user.to_reference - if starrer.user == current_user - %span.badge.badge-success.prepend-left-5= _("It's you") + %span.badge.badge-success.gl-ml-2= _("It's you") .block-truncated = time_ago_with_tooltip(starrer.starred_since) diff --git a/app/views/projects/tree/_tree_row.html.haml b/app/views/projects/tree/_tree_row.html.haml index 8a27ea66523..300cd5423bf 100644 --- a/app/views/projects/tree/_tree_row.html.haml +++ b/app/views/projects/tree/_tree_row.html.haml @@ -14,7 +14,7 @@ %a.str-truncated{ href: fast_project_blob_path(@project, tree_join(@id || @commit.id, tree_row_name)), title: tree_row_name } %span= tree_row_name - if @lfs_blob_ids.include?(tree_row.id) - %span.badge.label-lfs.prepend-left-5 LFS + %span.badge.label-lfs.gl-ml-2 LFS - elsif tree_row_type == :commit = tree_icon('archive', tree_row.mode, tree_row.name) diff --git a/app/views/search/results/_issue.html.haml b/app/views/search/results/_issue.html.haml index 1f055cdfa31..b88e9a75053 100644 --- a/app/views/search/results/_issue.html.haml +++ b/app/views/search/results/_issue.html.haml @@ -4,7 +4,7 @@ = link_to namespace_project_issue_path(issue.project.namespace.becomes(Namespace), issue.project, issue) do %span.term.str-truncated= issue.title - if issue.closed? - %span.badge.badge-danger.prepend-left-5= _("Closed") + %span.badge.badge-danger.gl-ml-2= _("Closed") .float-right ##{issue.iid} - if issue.description.present? .description.term diff --git a/app/views/search/results/_merge_request.html.haml b/app/views/search/results/_merge_request.html.haml index 074bb9bce8d..45b6cb06753 100644 --- a/app/views/search/results/_merge_request.html.haml +++ b/app/views/search/results/_merge_request.html.haml @@ -3,9 +3,9 @@ = link_to namespace_project_merge_request_path(merge_request.target_project.namespace.becomes(Namespace), merge_request.target_project, merge_request) do %span.term.str-truncated= merge_request.title - if merge_request.merged? - %span.badge.badge-primary.prepend-left-5= _("Merged") + %span.badge.badge-primary.gl-ml-2= _("Merged") - elsif merge_request.closed? - %span.badge.badge-danger.prepend-left-5= _("Closed") + %span.badge.badge-danger.gl-ml-2= _("Closed") .float-right= merge_request.to_reference - if merge_request.description.present? .description.term diff --git a/app/views/shared/issuable/form/_branch_chooser.html.haml b/app/views/shared/issuable/form/_branch_chooser.html.haml index 3794a3b3845..1823c5279e5 100644 --- a/app/views/shared/issuable/form/_branch_chooser.html.haml +++ b/app/views/shared/issuable/form/_branch_chooser.html.haml @@ -18,7 +18,7 @@ - elsif issuable.for_fork? %code= issuable.target_project_path + ":" - unless issuable.new_record? - %span.dropdown.prepend-left-5.d-inline-block + %span.dropdown.gl-ml-2.d-inline-block = form.hidden_field(:target_branch, { class: 'target_branch js-target-branch-select ref-name mw-xl', data: { placeholder: _('Select branch'), endpoint: refs_project_path(@project, sort: 'updated_desc', find: 'branches') }}) diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index dbb8a1198df..cc318ecded7 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -62,7 +62,7 @@ - if show_controls && member.source == current_resource - if member.can_resend_invite? - = link_to icon('paper-plane'), polymorphic_path([:resend_invite, member]), + = link_to sprite_icon('paper-airplane', size: 16), polymorphic_path([:resend_invite, member]), method: :post, class: 'btn btn-default align-self-center mr-sm-2', title: _('Resend invite') diff --git a/changelogs/unreleased/216785-terraform-plan-developer-access.yml b/changelogs/unreleased/216785-terraform-plan-developer-access.yml new file mode 100644 index 00000000000..d7720cc1c13 --- /dev/null +++ b/changelogs/unreleased/216785-terraform-plan-developer-access.yml @@ -0,0 +1,5 @@ +--- +title: Allow developer role read-only access to Terraform state +merge_request: 33573 +author: +type: added diff --git a/changelogs/unreleased/sh-loosen-aws-tokens.yml b/changelogs/unreleased/sh-loosen-aws-tokens.yml new file mode 100644 index 00000000000..846bb634cc8 --- /dev/null +++ b/changelogs/unreleased/sh-loosen-aws-tokens.yml @@ -0,0 +1,5 @@ +--- +title: Remove CI/CD variable validations on AWS keys +merge_request: 36679 +author: +type: fixed diff --git a/changelogs/unreleased/stackprof.yml b/changelogs/unreleased/stackprof.yml new file mode 100644 index 00000000000..11361381d2d --- /dev/null +++ b/changelogs/unreleased/stackprof.yml @@ -0,0 +1,5 @@ +--- +title: Trigger stackprof by sending a SIGUSR2 signal +merge_request: 35993 +author: +type: performance diff --git a/config/initializers/stackprof.rb b/config/initializers/stackprof.rb new file mode 100644 index 00000000000..85980987cb6 --- /dev/null +++ b/config/initializers/stackprof.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +# trigger stackprof by sending a SIGUSR2 signal +# +# default settings: +# * collect raw samples +# * sample at 100hz (every 10k microseconds) +# * timeout profile after 30 seconds +# * write to $TMPDIR/stackprof.$PID.$RAND.profile + +if Gitlab::Utils.to_boolean(ENV['STACKPROF_ENABLED'].to_s) + Gitlab::Cluster::LifecycleEvents.on_worker_start do + require 'stackprof' + require 'tmpdir' + + Gitlab::AppJsonLogger.info "stackprof: listening on SIGUSR2 signal" + + # create a pipe in order to propagate signal out of the signal handler + # see also: https://cr.yp.to/docs/selfpipe.html + read, write = IO.pipe + + # create a separate thread that polls for signals on the pipe. + # + # this way we do not execute in signal handler context, which + # lifts restrictions and also serializes the calls in a thread-safe + # manner. + # + # it's very similar to a goroutine and channel design. + # + # another nice benefit of this method is that we can timeout the + # IO.select call, allowing the profile to automatically stop after + # a given interval (by default 30 seconds), avoiding unbounded memory + # growth from a profile that was started and never stopped. + t = Thread.new do + timeout_s = ENV['STACKPROF_TIMEOUT_S']&.to_i || 30 + current_timeout_s = nil + loop do + got_value = IO.select([read], nil, nil, current_timeout_s) + read.getbyte if got_value + + if StackProf.running? + stackprof_file_prefix = ENV['STACKPROF_FILE_PREFIX'] || Dir.tmpdir + stackprof_out_file = "#{stackprof_file_prefix}/stackprof.#{Process.pid}.#{SecureRandom.hex(6)}.profile" + + Gitlab::AppJsonLogger.info( + event: "stackprof", + message: "stopping profile", + output_filename: stackprof_out_file, + pid: Process.pid, + timeout_s: timeout_s, + timed_out: got_value.nil? + ) + + StackProf.stop + StackProf.results(stackprof_out_file) + current_timeout_s = nil + else + Gitlab::AppJsonLogger.info( + event: "stackprof", + message: "starting profile", + pid: Process.pid + ) + + StackProf.start( + raw: Gitlab::Utils.to_boolean(ENV['STACKPROF_RAW'] || 'true'), + interval: ENV['STACKPROF_INTERVAL_US']&.to_i || 10_000 + ) + current_timeout_s = timeout_s + end + end + end + t.abort_on_exception = true + + # in the case of puma, this will override the existing SIGUSR2 signal handler + # that can be used to trigger a restart. + # + # puma cluster has two types of restarts: + # * SIGUSR1: phased restart + # * SIGUSR2: restart + # + # phased restart is not supported in our configuration, because we use + # preload_app. this means we will always perform a normal restart. + # additionally, phased restart is not supported when sending a SIGUSR2 + # directly to a puma worker (as opposed to the master process). + # + # the result is that the behaviour of SIGUSR1 and SIGUSR2 is identical in + # our configuration, and we can always use a SIGUSR1 to perform a restart. + # + # thus, it is acceptable for us to re-appropriate the SIGUSR2 signal, and + # override the puma behaviour. + # + # see also: + # * https://github.com/puma/puma/blob/master/docs/signals.md#puma-signals + # * https://github.com/phusion/unicorn/blob/master/SIGNALS + # * https://github.com/mperham/sidekiq/wiki/Signals + Signal.trap('SIGUSR2') do + write.write('.') + end + end +end diff --git a/db/fixtures/development/26_packages.rb b/db/fixtures/development/26_packages.rb new file mode 100644 index 00000000000..6096fd8962a --- /dev/null +++ b/db/fixtures/development/26_packages.rb @@ -0,0 +1,159 @@ +# frozen_string_literal: true + +class Gitlab::Seeder::Packages + attr_reader :project + + def initialize(project) + @project = project + end + + def seed_packages(package_type) + send("seed_#{package_type}_packages") + end + + def seed_npm_packages + 5.times do |i| + name = "@#{@project.root_namespace.path}/npm_package_#{SecureRandom.hex}" + version = "1.12.#{i}" + + params = Gitlab::Json.parse(read_fixture_file('npm', 'payload.json') + .gsub('@root/npm-test', name) + .gsub('1.0.1', version)) + .with_indifferent_access + + ::Packages::Npm::CreatePackageService.new(project, project.owner, params).execute + + print '.' + end + end + + def seed_maven_packages + 5.times do |i| + name = "my/company/app/maven-app-#{i}" + version = "1.0.#{i}-SNAPSHOT" + + params = { + name: name, + version: version, + path: "#{name}/#{version}" + } + + pkg = ::Packages::Maven::CreatePackageService.new(project, project.owner, params).execute + + %w(maven-metadata.xml my-app-1.0-20180724.124855-1.pom my-app-1.0-20180724.124855-1.jar).each do |filename| + with_cloned_fixture_file('maven', filename) do |filepath| + file_params = { + file: UploadedFile.new(filepath, filename: filename), + file_name: filename, + file_sha1: '1234567890', + size: 100.kilobytes + } + ::Packages::CreatePackageFileService.new(pkg, file_params).execute + end + end + + print '.' + end + end + + def seed_conan_packages + 5.times do |i| + name = "my-conan-pkg-#{i}" + version = "2.0.#{i}" + + params = { + package_name: name, + package_version: version, + package_username: ::Packages::Conan::Metadatum.package_username_from(full_path: project.full_path), + package_channel: 'stable' + } + + pkg = ::Packages::Conan::CreatePackageService.new(project, project.owner, params).execute + + fixtures = { + 'recipe_files' => %w(conanfile.py conanmanifest.txt), + 'package_files' => %w(conanmanifest.txt conaninfo.txt conan_package.tgz) + } + + fixtures.each do |folder, filenames| + filenames.each do |filename| + with_cloned_fixture_file(File.join('conan', folder), filename) do |filepath| + file = UploadedFile.new(filepath, filename: filename) + file_params = { + file_name: filename, + 'file.sha1': '1234567890', + 'file.size': 100.kilobytes, + 'file.md5': '12345', + recipe_revision: '0', + package_revision: '0', + conan_package_reference: '123456789', + conan_file_type: :package_file + } + ::Packages::Conan::CreatePackageFileService.new(pkg, file, file_params).execute + end + end + end + + print '.' + end + end + + def seed_nuget_packages + 5.times do |i| + name = "MyNugetApp.Package#{i}" + version = "4.2.#{i}" + + pkg = ::Packages::Nuget::CreatePackageService.new(project, project.owner, {}).execute + # when using ::Packages::Nuget::CreatePackageService, packages have a fixed name and a fixed version. + pkg.update!(name: name, version: version) + + filename = 'package.nupkg' + with_cloned_fixture_file('nuget', filename) do |filepath| + file_params = { + file: UploadedFile.new(filepath, filename: filename), + file_name: filename, + file_sha1: '1234567890', + size: 100.kilobytes + } + ::Packages::CreatePackageFileService.new(pkg, file_params).execute + end + + print '.' + end + end + + private + + def read_fixture_file(package_type, file) + File.read(fixture_path(package_type, file)) + end + + def fixture_path(package_type, file) + Rails.root.join('spec', 'fixtures', 'packages', package_type, file) + end + + def with_cloned_fixture_file(package_type, file) + Dir.mktmpdir do |dirpath| + cloned_path = File.join(dirpath, file) + FileUtils.cp(fixture_path(package_type, file), cloned_path) + yield cloned_path + end + end +end + +Gitlab::Seeder.quiet do + flag = 'SEED_ALL_PACKAGE_TYPES' + + puts "Use the `#{flag}` environment variable to seed packages of all types." unless ENV[flag] + + package_types = ENV[flag] ? %i[npm maven conan nuget] : [:npm] + + Project.not_mass_generated.sample(5).each do |project| + puts "\nSeeding packages for the '#{project.full_path}' project" + seeder = Gitlab::Seeder::Packages.new(project) + + package_types.each do |package_type| + seeder.seed_packages(package_type) + end + end +end diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 3007706d203..4f9a1d8dd27 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -233,13 +233,12 @@ be updated or viewed by project members with [maintainer permissions](../../user ### Custom variables validated by GitLab Some variables are listed in the UI so you can choose them more quickly. -GitLab validates the values of these variables to ensure they are in the correct format. | Variable | Allowed Values | Introduced in | |-------------------------|----------------------------------------------------|---------------| -| `AWS_ACCESS_KEY_ID` | 20 characters: letters, digits | 12.10 | +| `AWS_ACCESS_KEY_ID` | Any | 12.10 | | `AWS_DEFAULT_REGION` | Any | 12.10 | -| `AWS_SECRET_ACCESS_KEY` | 40 characters: letters, digits, special characters | 12.10 | +| `AWS_SECRET_ACCESS_KEY` | Any | 12.10 | NOTE: **Note:** When you store credentials, there are security implications. If you are using AWS keys, diff --git a/doc/development/performance.md b/doc/development/performance.md index 26e107eb2a1..2841a7c339a 100644 --- a/doc/development/performance.md +++ b/doc/development/performance.md @@ -36,7 +36,6 @@ graphs/dashboards. GitLab provides built-in tools to help improve performance and availability: - [Profiling](profiling.md). - - [Sherlock](profiling.md#sherlock). - [Distributed Tracing](distributed_tracing.md) - [GitLab Performance Monitoring](../administration/monitoring/performance/index.md). - [Request Profiling](../administration/monitoring/performance/request_profiling.md). @@ -108,16 +107,24 @@ In short: ## Profiling By collecting snapshots of process state at regular intervals, profiling allows -you to see where time is spent in a process. The [StackProf](https://github.com/tmm1/stackprof) -gem is included in GitLab's development environment, allowing you to investigate -the behavior of suspect code in detail. +you to see where time is spent in a process. The +[Stackprof](https://github.com/tmm1/stackprof) gem is included in GitLab, +allowing you to profile which code is running on CPU in detail. -It's important to note that profiling an application *alters its performance*, -and will generally be done *in an unrepresentative environment*. In particular, -a method is not necessarily troublesome just because it's executed many times, -or takes a long time to execute. Profiles are tools you can use to better -understand what is happening in an application - using that information wisely -is up to you! +It's important to note that profiling an application *alters its performance*. +Different profiling strategies have different overheads. Stackprof is a sampling +profiler. It will sample stack traces from running threads at a configurable +frequency (e.g. 100hz, that is 100 stacks per second). This type of profiling +has quite a low (albeit non-zero) overhead and is generally considered to be +safe for production. + +### Development + +A profiler can be a very useful tool during development, even if it does run *in +an unrepresentative environment*. In particular, a method is not necessarily +troublesome just because it's executed many times, or takes a long time to +execute. Profiles are tools you can use to better understand what is happening +in an application - using that information wisely is up to you! Keeping that in mind, to create a profile, identify (or create) a spec that exercises the troublesome code path, then run it using the `bin/rspec-stackprof` @@ -211,11 +218,56 @@ application code, these profiles can be used to investigate slow tests as well. However, for smaller runs (like this example), this means that the cost of setting up the test suite will tend to dominate. -It's also possible to modify the application code in-place to output profiles -whenever a particular code path is triggered without going through the test -suite first. See the -[StackProf documentation](https://github.com/tmm1/stackprof/blob/master/README.md) -for details. +### Production + +Stackprof can also be used to profile production workloads. + +In order to enable production profiling for Ruby processes, you can set the `STACKPROF_ENABLED` environment variable to `true`. + +The following configuration options can be configured: + +- `STACKPROF_ENABLED`: Enables stackprof signal handler on SIGUSR2 signal. + Defaults to `false`. +- `STACKPROF_INTERVAL_US`: Sampling interval in microseconds. Defaults to + `10000` μs (100hz). +- `STACKPROF_FILE_PREFIX`: File path prefix where profiles are stored. Defaults + to `$TMPDIR` (often corresponds to `/tmp`). +- `STACKPROF_TIMEOUT_S`: Profiling timeout in seconds. Profiling will + automatically stop after this time has elapsed. Defaults to `30`. +- `STACKPROF_RAW`: Whether to collect raw samples or only aggregates. Raw + samples are needed to generate flamegraphs, but they do have a higher memory + and disk overhead. Defaults to `true`. + +Once enabled, profiling can be triggered by sending a `SIGUSR2` signal to the +Ruby process. The process will begin sampling stacks. Profiling can be stopped +by sending another `SIGUSR2`. Alternatively, it will automatically stop after +the timeout. + +Once profiling stops, the profile is written out to disk at +`$STACKPROF_FILE_PREFIX/stackprof.$PID.$RAND.profile`. It can then be inspected +further via the `stackprof` command line tool, as described in the previous +section. + +Currently supported profiling targets are: + +- Puma worker +- Sidekiq + +NOTE: **Note:** The Puma master process is not supported. Neither is Unicorn. +Sending SIGUSR2 to either of those will trigger restarts. In the case of Puma, +take care to only send the signal to Puma workers. + +This can be done via `pkill -USR2 puma:`. The `:` disambiguates between `puma +4.3.3.gitlab.2 ...` (the master process) from `puma: cluster worker 0: ...` (the +worker processes), selecting the latter. + +Production profiles can be especially noisy. It can be helpful to visualize them +as a [flamegraph](https://github.com/brendangregg/FlameGraph). This can be done +via: + +```shell +bundle exec stackprof --stackcollapse /tmp/stackprof.55769.c6c3906452.profile | flamegraph.pl > flamegraph.svg +``` ## RSpec profiling diff --git a/doc/development/telemetry/index.md b/doc/development/telemetry/index.md index 16b393a09cd..75f3f102932 100644 --- a/doc/development/telemetry/index.md +++ b/doc/development/telemetry/index.md @@ -20,6 +20,7 @@ Telemetry Guide: 1. [Our tracking tools](#our-tracking-tools) 1. [What data can be tracked](#what-data-can-be-tracked) 1. [Telemetry systems overview](#telemetry-systems-overview) + 1. [Snowflake data warehouse](#snowflake-data-warehouse) [Usage Ping Guide](usage_ping.md) @@ -169,3 +170,19 @@ The differences between GitLab.com and self-managed are summarized below: | Self-Managed | **{dotted-circle}**(1) | **{dotted-circle}**(1) | **{check-circle}** | **{dotted-circle}** | **{dotted-circle}** | Note (1): Snowplow JS and Snowplow Ruby are available on self-managed, however, the Snowplow Collector endpoint is set to a self-managed Snowplow Collector which GitLab Inc does not have access to. + +## Snowflake data warehouse + +The Snowflake data warehouse is where we keep all of GitLab Inc's data. + +### Data sources + +There are several data sources available in Snowflake and Sisense each representing a different view of the data along the transformation pipeline. + +| Source | Description | Access | +| ------ | ------ | ------ | +| raw | These tables are the raw data source | Access via Snowflake | +| analytics_staging | These tables have undergone little to no data transformation, meaning they're basically clones of the raw data source | Access via Snowflake or Sisense | +| analytics | These tables have typically undergone more data transformation. They will typically end in `_xf` to represent the fact that they are transformed | Access via Snowflake or Sisense | + +If you are a Product Manager interested in the raw data, you will likely focus on the `analytics` and `analytics_staging` sources. The raw source is limited to the data and infrastructure teams. For more information, please see [Data For Product Managers: What's the difference between analytics_staging and analytics?](https://about.gitlab.com/handbook/business-ops/data-team/programs/data-for-product-managers/#whats-the-difference-between-analytics_staging-and-analytics) diff --git a/doc/user/infrastructure/index.md b/doc/user/infrastructure/index.md index 96e205daf9b..06cd290f5ad 100644 --- a/doc/user/infrastructure/index.md +++ b/doc/user/infrastructure/index.md @@ -36,6 +36,14 @@ To get started with a GitLab-managed Terraform State, there are two different op - [Use a local machine](#get-started-using-local-development). - [Use GitLab CI](#get-started-using-gitlab-ci). +## Permissions for using Terraform + +In GitLab version 13.1, [Maintainer access](../permissions.md) was required to use a +GitLab managed Terraform state backend. In GitLab versions 13.2 and greater, +[Maintainer access](../permissions.md) is required to lock, unlock and write to the state +(using `terraform apply`), while [Developer access](../permissions.md) is required to read +the state (using `terraform plan -lock=false`). + ## Get started using local development If you plan to only run `terraform plan` and `terraform apply` commands from your @@ -54,8 +62,7 @@ local machine, this is a simple way to get started: ``` 1. Create a [Personal Access Token](../profile/personal_access_tokens.md) with - the `api` scope. The Terraform backend is restricted to users with - [Maintainer access](../permissions.md) to the repository. + the `api` scope. 1. On your local machine, run `terraform init`, passing in the following options, replacing `<YOUR-PROJECT-NAME>`, `<YOUR-PROJECT-ID>`, `<YOUR-USERNAME>` and @@ -89,10 +96,6 @@ Next, [configure the backend](#configure-the-backend). After executing the `terraform init` command, you must configure the Terraform backend and the CI YAML file: -CAUTION: **Important:** -The Terraform backend is restricted to users with [Maintainer access](../permissions.md) -to the repository. - 1. In your Terraform project, define the [HTTP backend](https://www.terraform.io/docs/backends/types/http.html) by adding the following code block in a `.tf` file (such as `backend.tf`) to define the remote backend: diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 17a8dc83257..ba62cf81847 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -142,6 +142,8 @@ The following table depicts the various user permission levels in a project. | Manage clusters | | | | ✓ | ✓ | | Manage Project Operations | | | | ✓ | ✓ | | View Pods logs | | | | ✓ | ✓ | +| Read Terraform state | | | ✓ | ✓ | ✓ | +| Manage Terraform state | | | | ✓ | ✓ | | Manage license policy **(ULTIMATE)** | | | | ✓ | ✓ | | Edit comments (posted by any user) | | | | ✓ | ✓ | | Manage Error Tracking | | | | ✓ | ✓ | diff --git a/lib/api/terraform/state.rb b/lib/api/terraform/state.rb index 6b9809c76a4..f6e966defce 100644 --- a/lib/api/terraform/state.rb +++ b/lib/api/terraform/state.rb @@ -11,7 +11,7 @@ module API before do authenticate! - authorize! :admin_terraform_state, user_project + authorize! :read_terraform_state, user_project end params do @@ -46,6 +46,8 @@ module API desc 'Add a new terraform state or update an existing one' route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth post do + authorize! :admin_terraform_state, user_project + data = request.body.read no_content! if data.empty? @@ -59,6 +61,8 @@ module API desc 'Delete a terraform state of a certain name' route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth delete do + authorize! :admin_terraform_state, user_project + remote_state_handler.handle_with_lock do |state| state.destroy! status :ok @@ -77,6 +81,8 @@ module API requires :Path, type: String, desc: 'Terraform path' end post '/lock' do + authorize! :admin_terraform_state, user_project + status_code = :ok lock_info = { 'Operation' => params[:Operation], @@ -108,6 +114,8 @@ module API optional :ID, type: String, limit: 255, desc: 'Terraform state lock ID' end delete '/lock' do + authorize! :admin_terraform_state, user_project + remote_state_handler.unlock! status :ok rescue ::Terraform::RemoteStateHandler::StateLockedError diff --git a/lib/tasks/gitlab/packages/migrate.rake b/lib/tasks/gitlab/packages/migrate.rake new file mode 100644 index 00000000000..cd6dcf78da3 --- /dev/null +++ b/lib/tasks/gitlab/packages/migrate.rake @@ -0,0 +1,23 @@ +require 'logger' + +desc "GitLab | Packages | Migrate packages files to remote storage" +namespace :gitlab do + namespace :packages do + task migrate: :environment do + logger = Logger.new(STDOUT) + logger.info('Starting transfer of package files to object storage') + + unless ::Packages::PackageFileUploader.object_store_enabled? + raise 'Object store is disabled for packages feature' + end + + ::Packages::PackageFile.with_files_stored_locally.find_each(batch_size: 10) do |package_file| + package_file.file.migrate!(::Packages::PackageFileUploader::Store::REMOTE) + + logger.info("Transferred package file #{package_file.id} of size #{package_file.size.to_i.bytes} to object storage") + rescue => e + logger.error("Failed to transfer package file #{package_file.id} with error: #{e.message}") + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cfb37fb12a4..9b9a1c8a0c5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6871,6 +6871,9 @@ msgstr "" msgid "Create new file or directory" msgstr "" +msgid "Create new issue in Jira" +msgstr "" + msgid "Create new label" msgstr "" @@ -24066,9 +24069,6 @@ msgstr "" msgid "This variable can not be masked." msgstr "" -msgid "This variable does not match the expected pattern." -msgstr "" - msgid "This will help us personalize your onboarding experience." msgstr "" diff --git a/spec/factories/terraform/state.rb b/spec/factories/terraform/state.rb index 74950ccf93e..46784581180 100644 --- a/spec/factories/terraform/state.rb +++ b/spec/factories/terraform/state.rb @@ -9,5 +9,11 @@ FactoryBot.define do trait :with_file do 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 } + locked_by_user { create(:user) } + end end end diff --git a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js index 02b1ad84752..27ded020692 100644 --- a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js +++ b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js @@ -4,7 +4,6 @@ import { GlDeprecatedButton } from '@gitlab/ui'; import { AWS_ACCESS_KEY_ID } from '~/ci_variable_list/constants'; import CiVariableModal from '~/ci_variable_list/components/ci_variable_modal.vue'; import CiKeyField from '~/ci_variable_list/components/ci_key_field.vue'; -import { awsTokens } from '~/ci_variable_list/components/ci_variable_autocomplete_tokens'; import createStore from '~/ci_variable_list/store'; import mockData from '../services/mock_data'; import ModalStub from '../stubs'; @@ -176,29 +175,6 @@ describe('Ci variable modal', () => { describe('Validations', () => { const maskError = 'This variable can not be masked.'; - describe('when the key state is invalid', () => { - beforeEach(() => { - const [variable] = mockData.mockVariables; - const invalidKeyVariable = { - ...variable, - key: AWS_ACCESS_KEY_ID, - value: 'AKIAIOSFODNN7EXAMPLEjdhy', - secret_value: 'AKIAIOSFODNN7EXAMPLEjdhy', - }; - createComponent(mount); - store.state.variable = invalidKeyVariable; - }); - - it('disables the submit button', () => { - expect(addOrUpdateButton(1).attributes('disabled')).toBeTruthy(); - }); - - it('shows the correct error text', () => { - const errorText = awsTokens[AWS_ACCESS_KEY_ID].invalidMessage; - expect(findModal().text()).toContain(errorText); - }); - }); - describe('when the mask state is invalid', () => { beforeEach(() => { const [variable] = mockData.mockVariables; @@ -222,39 +198,14 @@ describe('Ci variable modal', () => { }); }); - describe('when the mask and key states are invalid', () => { - beforeEach(() => { - const [variable] = mockData.mockVariables; - const invalidMaskandKeyVariable = { - ...variable, - key: AWS_ACCESS_KEY_ID, - value: 'AKIAIOSFODNN7EXAMPLEjdhyd:;', - secret_value: 'AKIAIOSFODNN7EXAMPLEjdhyd:;', - masked: true, - }; - createComponent(mount); - store.state.variable = invalidMaskandKeyVariable; - }); - - it('disables the submit button', () => { - expect(addOrUpdateButton(1).attributes('disabled')).toBeTruthy(); - }); - - it('shows the correct error text', () => { - const errorText = awsTokens[AWS_ACCESS_KEY_ID].invalidMessage; - expect(findModal().text()).toContain(maskError); - expect(findModal().text()).toContain(errorText); - }); - }); - describe('when both states are valid', () => { beforeEach(() => { const [variable] = mockData.mockVariables; const validMaskandKeyVariable = { ...variable, key: AWS_ACCESS_KEY_ID, - value: 'AKIAIOSFODNN7EXAMPLE', - secret_value: 'AKIAIOSFODNN7EXAMPLE', + value: '12345678', + secret_value: '87654321', masked: true, }; createComponent(mount); @@ -265,12 +216,6 @@ describe('Ci variable modal', () => { it('does not disable the submit button', () => { expect(addOrUpdateButton(1).attributes('disabled')).toBeFalsy(); }); - - it('shows no error text', () => { - const errorText = awsTokens[AWS_ACCESS_KEY_ID].invalidMessage; - expect(findModal().text()).not.toContain(maskError); - expect(findModal().text()).not.toContain(errorText); - }); }); }); }); diff --git a/spec/frontend/gfm_auto_complete_spec.js b/spec/frontend/gfm_auto_complete_spec.js index f58615000ee..869347128e5 100644 --- a/spec/frontend/gfm_auto_complete_spec.js +++ b/spec/frontend/gfm_auto_complete_spec.js @@ -312,7 +312,7 @@ describe('GfmAutoComplete', () => { title: 'My Group', search: 'my-group My Group', icon: - '<svg class="s16 vertical-align-middle prepend-left-5"><use xlink:href="undefined#notifications-off" /></svg>', + '<svg class="s16 vertical-align-middle gl-ml-2"><use xlink:href="undefined#notifications-off" /></svg>', }, ]); }); diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index e0b8b861462..cfc2c920cd2 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -724,7 +724,7 @@ RSpec.describe JiraService do describe '#new_issue_url' do it 'handles trailing slashes' do - expect(service.new_issue_url).to eq('http://jira.test.com/path/secure/CreateIssue.jspa') + expect(service.new_issue_url).to eq('http://jira.test.com/path/secure/CreateIssue!default.jspa') end end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 00ca5915601..dc6ed94309b 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -46,6 +46,7 @@ RSpec.describe ProjectPolicy do resolve_note create_container_image update_container_image destroy_container_image daily_statistics create_environment update_environment create_deployment update_deployment create_release update_release create_metrics_dashboard_annotation delete_metrics_dashboard_annotation update_metrics_dashboard_annotation + read_terraform_state ] end diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index 2ddbfe6fa95..c6cba39314b 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -59,10 +59,11 @@ RSpec.describe API::Terraform::State do context 'with developer permissions' do let(:current_user) { developer } - it 'returns forbidden if the user cannot access the state' do + it 'returns terraform state belonging to a project of given state name' do request - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(state.file.read) end end end @@ -94,10 +95,11 @@ RSpec.describe API::Terraform::State do context 'with developer permissions' do let(:job) { create(:ci_build, project: project, user: developer) } - it 'returns forbidden if the user cannot access the state' do + it 'returns terraform state belonging to a project of given state name' do request - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(state.file.read) end end end @@ -235,9 +237,43 @@ RSpec.describe API::Terraform::State do expect(response).to have_gitlab_http_status(:ok) end + + context 'state is already locked' do + before do + state.update!(lock_xid: 'locked', locked_by_user: current_user) + end + + it 'returns an error' do + request + + expect(response).to have_gitlab_http_status(:conflict) + end + end + + context 'user does not have permission to lock the state' do + let(:current_user) { developer } + + it 'returns an error' do + request + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end describe 'DELETE /projects/:id/terraform/state/:name/lock' do + let(:params) do + { + ID: lock_id, + Version: '0.1', + Operation: 'OperationTypePlan', + Info: '', + Who: "#{current_user.username}", + Created: Time.now.utc.iso8601(6), + Path: '' + } + end + before do state.lock_xid = '123-456' state.save! @@ -246,7 +282,7 @@ RSpec.describe API::Terraform::State do subject(:request) { delete api("#{state_path}/lock"), headers: auth_header, params: params } context 'with the correct lock id' do - let(:params) { { ID: '123-456' } } + let(:lock_id) { '123-456' } it 'removes the terraform state lock' do request @@ -266,7 +302,7 @@ RSpec.describe API::Terraform::State do end context 'with an incorrect lock id' do - let(:params) { { ID: '456-789' } } + let(:lock_id) { '456-789' } it 'returns an error' do request @@ -276,7 +312,7 @@ RSpec.describe API::Terraform::State do end context 'with a longer than 255 character lock id' do - let(:params) { { ID: '0' * 256 } } + let(:lock_id) { '0' * 256 } it 'returns an error' do request @@ -284,5 +320,16 @@ RSpec.describe API::Terraform::State do expect(response).to have_gitlab_http_status(:bad_request) end end + + context 'user does not have permission to unlock the state' do + let(:lock_id) { '123-456' } + let(:current_user) { developer } + + it 'returns an error' do + request + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end end diff --git a/spec/services/terraform/remote_state_handler_spec.rb b/spec/services/terraform/remote_state_handler_spec.rb index d3a84155e19..c47367feb14 100644 --- a/spec/services/terraform/remote_state_handler_spec.rb +++ b/spec/services/terraform/remote_state_handler_spec.rb @@ -4,7 +4,10 @@ require 'spec_helper' RSpec.describe Terraform::RemoteStateHandler do let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user) } + let_it_be(:developer) { create(:user, developer_projects: [project]) } + let_it_be(:maintainer) { create(:user, maintainer_projects: [project]) } + + let_it_be(:user) { maintainer } describe '#find_with_lock' do context 'without a state name' do @@ -34,33 +37,6 @@ RSpec.describe Terraform::RemoteStateHandler do end end - describe '#create_or_find!' do - it 'requires passing a state name' do - handler = described_class.new(project, user) - - expect { handler.create_or_find! }.to raise_error(ArgumentError) - end - - it 'allows to create states with same name in different projects' do - project_b = create(:project) - - state_a = described_class.new(project, user, name: 'my-state').create_or_find! - state_b = described_class.new(project_b, user, name: 'my-state').create_or_find! - - expect(state_a).to be_persisted - expect(state_b).to be_persisted - expect(state_a.id).not_to eq state_b.id - end - - it 'loads the same state upon subsequent call in the project scope' do - state_a = described_class.new(project, user, name: 'my-state').create_or_find! - state_b = described_class.new(project, user, name: 'my-state').create_or_find! - - expect(state_a).to be_persisted - expect(state_a.id).to eq state_b.id - end - end - context 'when state locking is not being used' do subject { described_class.new(project, user, name: 'my-state') } @@ -74,7 +50,7 @@ RSpec.describe Terraform::RemoteStateHandler do end it 'returns the state object itself' do - state = subject.create_or_find! + state = subject.handle_with_lock expect(state.name).to eq 'my-state' end @@ -89,10 +65,9 @@ RSpec.describe Terraform::RemoteStateHandler do context 'when using locking' do describe '#handle_with_lock' do - it 'handles a locked state using exclusive read lock' do - handler = described_class - .new(project, user, name: 'new-state', lock_id: 'abc-abc') + subject(:handler) { described_class.new(project, user, name: 'new-state', lock_id: 'abc-abc') } + it 'handles a locked state using exclusive read lock' do handler.lock! state = handler.handle_with_lock do |state| @@ -101,20 +76,35 @@ RSpec.describe Terraform::RemoteStateHandler do expect(state.name).to eq 'new-name' end - end - it 'raises exception if lock has not been acquired before' do - handler = described_class - .new(project, user, name: 'new-state', lock_id: 'abc-abc') + it 'raises exception if lock has not been acquired before' do + expect { handler.handle_with_lock } + .to raise_error(described_class::StateLockedError) + end + + context 'user does not have permission to modify state' do + let(:user) { developer } - expect { handler.handle_with_lock } - .to raise_error(described_class::StateLockedError) + it 'raises an exception' do + expect { handler.handle_with_lock } + .to raise_error(described_class::UnauthorizedError) + end + end end describe '#lock!' do - it 'allows to lock state if it does not exist yet' do - handler = described_class.new(project, user, name: 'new-state', lock_id: 'abc-abc') + let(:lock_id) { 'abc-abc' } + + subject(:handler) do + described_class.new( + project, + user, + name: 'new-state', + lock_id: lock_id + ) + end + it 'allows to lock state if it does not exist yet' do state = handler.lock! expect(state).to be_persisted @@ -122,22 +112,61 @@ RSpec.describe Terraform::RemoteStateHandler do end it 'allows to lock state if it exists and is not locked' do - state = described_class.new(project, user, name: 'new-state').create_or_find! - handler = described_class.new(project, user, name: 'new-state', lock_id: 'abc-abc') + state = create(:terraform_state, project: project, name: 'new-state') handler.lock! - expect(state.reload.lock_xid).to eq 'abc-abc' + expect(state.reload.lock_xid).to eq lock_id expect(state).to be_locked end it 'raises an exception when trying to unlocked state locked by someone else' do - described_class.new(project, user, name: 'new-state', lock_id: 'abc-abc').lock! - - handler = described_class.new(project, user, name: 'new-state', lock_id: '12a-23f') + described_class.new(project, user, name: 'new-state', lock_id: '12a-23f').lock! expect { handler.lock! }.to raise_error(described_class::StateLockedError) end end + + describe '#unlock!' do + let(:lock_id) { 'abc-abc' } + + subject(:handler) do + described_class.new( + project, + user, + name: 'new-state', + lock_id: lock_id + ) + end + + before do + create(:terraform_state, :locked, project: project, name: 'new-state', lock_xid: 'abc-abc') + end + + it 'unlocks the state' do + state = handler.unlock! + + expect(state.lock_xid).to be_nil + end + + context 'with no lock ID (force-unlock)' do + let(:lock_id) { } + + it 'unlocks the state' do + state = handler.unlock! + + expect(state.lock_xid).to be_nil + end + end + + context 'with different lock ID' do + let(:lock_id) { 'other' } + + it 'raises an exception' do + expect { handler.unlock! } + .to raise_error(described_class::StateLockedError) + end + end + end end end diff --git a/spec/support/helpers/stub_object_storage.rb b/spec/support/helpers/stub_object_storage.rb index b473cdaefc1..3f4c70ce389 100644 --- a/spec/support/helpers/stub_object_storage.rb +++ b/spec/support/helpers/stub_object_storage.rb @@ -1,6 +1,25 @@ # frozen_string_literal: true module StubObjectStorage + def stub_packages_object_storage(**params) + stub_object_storage_uploader(config: ::Gitlab.config.packages.object_store, + uploader: ::Packages::PackageFileUploader, + remote_directory: 'packages', + **params) + end + + def stub_dependency_proxy_object_storage(**params) + stub_object_storage_uploader(config: ::Gitlab.config.dependency_proxy.object_store, + uploader: ::DependencyProxy::FileUploader, + remote_directory: 'dependency_proxy', + **params) + end + + def stub_object_storage_pseudonymizer + stub_object_storage(connection_params: Pseudonymizer::Uploader.object_store_credentials, + remote_directory: Pseudonymizer::Uploader.remote_directory) + end + def stub_object_storage_uploader( config:, uploader:, @@ -89,8 +108,3 @@ module StubObjectStorage EOS end end - -require_relative '../../../ee/spec/support/helpers/ee/stub_object_storage' if - Dir.exist?("#{__dir__}/../../../ee") - -StubObjectStorage.prepend_if_ee('EE::StubObjectStorage') diff --git a/spec/tasks/gitlab/packages/migrate_rake_spec.rb b/spec/tasks/gitlab/packages/migrate_rake_spec.rb new file mode 100644 index 00000000000..0a296eb0808 --- /dev/null +++ b/spec/tasks/gitlab/packages/migrate_rake_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'rake_helper' + +RSpec.describe 'gitlab:packages namespace rake task' do + before :all do + Rake.application.rake_require 'tasks/gitlab/packages/migrate' + end + + describe 'migrate' do + let(:local) { ObjectStorage::Store::LOCAL } + let(:remote) { ObjectStorage::Store::REMOTE } + let!(:package_file) { create(:package_file, :pom, file_store: local) } + + def packages_migrate + run_rake_task('gitlab:packages:migrate') + end + + context 'object storage disabled' do + before do + stub_packages_object_storage(enabled: false) + end + + it "doesn't migrate files" do + expect { packages_migrate }.to raise_error('Object store is disabled for packages feature') + end + end + + context 'object storage enabled' do + before do + stub_packages_object_storage + end + + it 'migrates local file to object storage' do + expect { packages_migrate }.to change { package_file.reload.file_store }.from(local).to(remote) + end + end + end +end |