diff options
186 files changed, 4216 insertions, 1183 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 5fea1768541..18455b77f5e 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.113.0 +0.114.0 diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 0ee843cc604..ae9a76b9249 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -7.2.0 +8.0.0 diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 0441d9f9e42..d3ffbe0415a 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -50,7 +50,7 @@ export default { }; }, computed: { - ...mapGetters('diffs', ['diffHasExpandedDiscussions']), + ...mapGetters('diffs', ['diffHasExpandedDiscussions', 'diffHasDiscussions']), hasExpandedDiscussions() { return this.diffHasExpandedDiscussions(this.diffFile); }, @@ -221,6 +221,7 @@ export default { v-if="diffFile.blob && diffFile.blob.readableText" > <button + :disabled="!diffHasDiscussions(diffFile)" :class="{ active: hasExpandedDiscussions }" :title="s__('MergeRequests|Toggle comments for this file')" class="js-btn-vue-toggle-comments btn" diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index 855de79adf8..9aec117c236 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -46,6 +46,14 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => { }; /** + * Checks if the diff has any discussion + * @param {Boolean} diff + * @returns {Boolean} + */ +export const diffHasDiscussions = (state, getters) => diff => + getters.getDiffFileDiscussions(diff).length > 0; + +/** * Returns an array with the discussions of the given diff * @param {Object} diff * @returns {Array} diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index eeceb99c8d2..73d7b8cb9cf 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -397,7 +397,7 @@ class ApplicationController < ActionController::Base # actually stored in the session and a token is needed # for every request. If you want the token to work as a # sign in token, you can simply remove store: false. - sign_in user, store: false + sign_in(user, store: false, message: :sessionless_sign_in) end end diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index 69a053d4246..dfa1da7872c 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -60,7 +60,7 @@ module AuthenticatesWithTwoFactor remember_me(user) if user_params[:remember_me] == '1' user.save! - sign_in(user) + sign_in(user, message: :two_factor_authenticated) else user.increment_failed_attempts! Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=OTP") @@ -77,7 +77,7 @@ module AuthenticatesWithTwoFactor session.delete(:challenge) remember_me(user) if user_params[:remember_me] == '1' - sign_in(user) + sign_in(user, message: :two_factor_authenticated) else user.increment_failed_attempts! Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=U2F") diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb index 3cb9e46b548..d172aee5436 100644 --- a/app/controllers/jwt_controller.rb +++ b/app/controllers/jwt_controller.rb @@ -54,6 +54,22 @@ class JwtController < ApplicationController end def auth_params - params.permit(:service, :scope, :account, :client_id) + params.permit(:service, :account, :client_id) + .merge(additional_params) + end + + def additional_params + { scopes: scopes_param }.compact + end + + # We have to parse scope here, because Docker Client does not send an array of scopes, + # but rather a flat list and we loose second scope when being processed by Rails: + # scope=scopeA&scope=scopeB + # + # This method makes to always return an array of scopes + def scopes_param + return unless params[:scope].present? + + Array(Rack::Utils.parse_query(request.query_string)['scope']) end end diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index ce03b2d8d1d..8a2bce6e7b5 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -160,7 +160,10 @@ class Projects::LabelsController < Projects::ApplicationController def find_labels @available_labels ||= - LabelsFinder.new(current_user, project_id: @project.id, include_ancestor_groups: params[:include_ancestor_groups]).execute + LabelsFinder.new(current_user, + project_id: @project.id, + include_ancestor_groups: params[:include_ancestor_groups], + search: params[:search]).execute end def authorize_admin_labels! diff --git a/app/controllers/projects/lfs_api_controller.rb b/app/controllers/projects/lfs_api_controller.rb index c64ccc3d473..a01351ba292 100644 --- a/app/controllers/projects/lfs_api_controller.rb +++ b/app/controllers/projects/lfs_api_controller.rb @@ -1,6 +1,8 @@ class Projects::LfsApiController < Projects::GitHttpClientController include LfsRequest + LFS_TRANSFER_CONTENT_TYPE = 'application/octet-stream'.freeze + skip_before_action :lfs_check_access!, only: [:deprecated] before_action :lfs_check_batch_operation!, only: [:batch] @@ -86,7 +88,10 @@ class Projects::LfsApiController < Projects::GitHttpClientController upload: { href: "#{project.http_url_to_repo}/gitlab-lfs/objects/#{object[:oid]}/#{object[:size]}", header: { - Authorization: request.headers['Authorization'] + Authorization: request.headers['Authorization'], + # git-lfs v2.5.0 sets the Content-Type based on the uploaded file. This + # ensures that Workhorse can intercept the request. + 'Content-Type': LFS_TRANSFER_CONTENT_TYPE }.compact } } diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 4ca42e2d4a2..ab8e2e35b98 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -89,6 +89,14 @@ class SessionsController < Devise::SessionsController ).increment end + ## + # We do have some duplication between lib/gitlab/auth/activity.rb here, but + # leaving this method here because of backwards compatibility. + # + def login_counter + @login_counter ||= Gitlab::Metrics.counter(:user_session_logins_total, 'User sign in count') + end + def log_failed_login Gitlab::AppLogger.info("Failed Login: username=#{user_params[:login]} ip=#{request.remote_ip}") end @@ -97,10 +105,6 @@ class SessionsController < Devise::SessionsController (options = env["warden.options"]) && options[:action] == "unauthenticated" end - def login_counter - @login_counter ||= Gitlab::Metrics.counter(:user_session_logins_total, 'User sign in count') - end - # Handle an "initial setup" state, where there's only one user, it's an admin, # and they require a password change. def check_initial_setup diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index afd1f824b32..1d05bf28438 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -14,6 +14,7 @@ class LabelsFinder < UnionFinder @skip_authorization = skip_authorization items = find_union(label_ids, Label) || Label.none items = with_title(items) + items = by_search(items) sort(items) end @@ -63,6 +64,12 @@ class LabelsFinder < UnionFinder items.where(title: title) end + def by_search(labels) + return labels unless search? + + labels.search(params[:search]) + end + # Gets redacted array of group ids # which can include the ancestors and descendants of the requested group. def group_ids_for(group) @@ -106,6 +113,10 @@ class LabelsFinder < UnionFinder params[:only_group_labels] end + def search? + params[:search].present? + end + def title params[:title] || params[:name] end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 358b896702b..da54988ccfe 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -148,6 +148,7 @@ module ApplicationSettingsHelper :after_sign_up_text, :akismet_api_key, :akismet_enabled, + :allow_local_requests_from_hooks_and_services, :authorized_keys_enabled, :auto_devops_enabled, :auto_devops_domain, @@ -174,6 +175,7 @@ module ApplicationSettingsHelper :ed25519_key_restriction, :email_author_in_body, :enabled_git_access_protocol, + :enforce_terms, :gitaly_timeout_default, :gitaly_timeout_medium, :gitaly_timeout_fast, @@ -182,6 +184,7 @@ module ApplicationSettingsHelper :help_page_hide_commercial_content, :help_page_support_url, :help_page_text, + :hide_third_party_offers, :home_page_url, :housekeeping_bitmaps_enabled, :housekeeping_enabled, @@ -203,6 +206,7 @@ module ApplicationSettingsHelper :metrics_port, :metrics_sample_interval, :metrics_timeout, + :mirror_available, :pages_domain_verification_enabled, :password_authentication_enabled_for_web, :password_authentication_enabled_for_git, @@ -233,15 +237,16 @@ module ApplicationSettingsHelper :sign_in_text, :signup_enabled, :terminal_max_session_time, - :throttle_unauthenticated_enabled, - :throttle_unauthenticated_requests_per_period, - :throttle_unauthenticated_period_in_seconds, - :throttle_authenticated_web_enabled, - :throttle_authenticated_web_requests_per_period, - :throttle_authenticated_web_period_in_seconds, + :terms, :throttle_authenticated_api_enabled, - :throttle_authenticated_api_requests_per_period, :throttle_authenticated_api_period_in_seconds, + :throttle_authenticated_api_requests_per_period, + :throttle_authenticated_web_enabled, + :throttle_authenticated_web_period_in_seconds, + :throttle_authenticated_web_requests_per_period, + :throttle_unauthenticated_enabled, + :throttle_unauthenticated_period_in_seconds, + :throttle_unauthenticated_requests_per_period, :two_factor_grace_period, :unique_ips_limit_enabled, :unique_ips_limit_per_user, @@ -249,12 +254,7 @@ module ApplicationSettingsHelper :usage_ping_enabled, :user_default_external, :user_oauth_applications, - :version_check_enabled, - :allow_local_requests_from_hooks_and_services, - :hide_third_party_offers, - :enforce_terms, - :terms, - :mirror_available + :version_check_enabled ] end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index f770b219422..a5b3116a48a 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -228,25 +228,27 @@ class ApplicationSetting < ActiveRecord::Base { after_sign_up_text: nil, akismet_enabled: false, + allow_local_requests_from_hooks_and_services: false, authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand container_registry_token_expire_delay: 5, default_artifacts_expire_in: '30 days', default_branch_protection: Settings.gitlab['default_branch_protection'], + default_group_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_projects_limit: Settings.gitlab['default_projects_limit'], default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], - default_group_visibility: Settings.gitlab.default_projects_features['visibility_level'], disabled_oauth_sign_in_sources: [], domain_whitelist: Settings.gitlab['domain_whitelist'], dsa_key_restriction: 0, ecdsa_key_restriction: 0, ed25519_key_restriction: 0, + gitaly_timeout_default: 55, + gitaly_timeout_fast: 10, + gitaly_timeout_medium: 30, gravatar_enabled: Settings.gravatar['enabled'], - help_page_text: nil, help_page_hide_commercial_content: false, - unique_ips_limit_per_user: 10, - unique_ips_limit_time_window: 3600, - unique_ips_limit_enabled: false, + help_page_text: nil, + hide_third_party_offers: false, housekeeping_bitmaps_enabled: true, housekeeping_enabled: true, housekeeping_full_repack_period: 50, @@ -257,12 +259,14 @@ class ApplicationSetting < ActiveRecord::Base koding_url: nil, max_artifacts_size: Settings.artifacts['max_size'], max_attachment_size: Settings.gitlab['max_attachment_size'], - password_authentication_enabled_for_web: Settings.gitlab['signin_enabled'], + mirror_available: true, password_authentication_enabled_for_git: true, + password_authentication_enabled_for_web: Settings.gitlab['signin_enabled'], performance_bar_allowed_group_id: nil, rsa_key_restriction: 0, plantuml_enabled: false, plantuml_url: nil, + polling_interval_multiplier: 1, project_export_enabled: true, recaptcha_enabled: false, repository_checks_enabled: true, @@ -277,25 +281,21 @@ class ApplicationSetting < ActiveRecord::Base sign_in_text: nil, signup_enabled: Settings.gitlab['signup_enabled'], terminal_max_session_time: 0, - throttle_unauthenticated_enabled: false, - throttle_unauthenticated_requests_per_period: 3600, - throttle_unauthenticated_period_in_seconds: 3600, - throttle_authenticated_web_enabled: false, - throttle_authenticated_web_requests_per_period: 7200, - throttle_authenticated_web_period_in_seconds: 3600, throttle_authenticated_api_enabled: false, - throttle_authenticated_api_requests_per_period: 7200, throttle_authenticated_api_period_in_seconds: 3600, + throttle_authenticated_api_requests_per_period: 7200, + throttle_authenticated_web_enabled: false, + throttle_authenticated_web_period_in_seconds: 3600, + throttle_authenticated_web_requests_per_period: 7200, + throttle_unauthenticated_enabled: false, + throttle_unauthenticated_period_in_seconds: 3600, + throttle_unauthenticated_requests_per_period: 3600, two_factor_grace_period: 48, - user_default_external: false, - polling_interval_multiplier: 1, + unique_ips_limit_enabled: false, + unique_ips_limit_per_user: 10, + unique_ips_limit_time_window: 3600, usage_ping_enabled: Settings.gitlab['usage_ping_enabled'], - gitaly_timeout_fast: 10, - gitaly_timeout_medium: 30, - gitaly_timeout_default: 55, - allow_local_requests_from_hooks_and_services: false, - hide_third_party_offers: false, - mirror_available: true + user_default_external: false } end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 35b20bc1e0b..93bbee49c09 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -8,8 +8,6 @@ module Ci include Importable include Gitlab::Utils::StrongMemoize - MissingDependenciesError = Class.new(StandardError) - belongs_to :project, inverse_of: :builds belongs_to :runner belongs_to :trigger_request @@ -17,6 +15,10 @@ module Ci has_many :deployments, as: :deployable + RUNNER_FEATURES = { + upload_multiple_artifacts: -> (build) { build.publishes_artifacts_reports? } + }.freeze + has_one :last_deployment, -> { order('deployments.id DESC') }, as: :deployable, class_name: 'Deployment' has_many :trace_sections, class_name: 'Ci::BuildTraceSection' has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id @@ -174,10 +176,6 @@ module Ci end end - before_transition any => [:running] do |build| - build.validates_dependencies! unless Feature.enabled?('ci_disable_validates_dependencies') - end - after_transition pending: :running do |build| build.ensure_metadata.update_timeout_state end @@ -581,10 +579,10 @@ module Ci options[:dependencies]&.empty? end - def validates_dependencies! - dependencies.each do |dependency| - raise MissingDependenciesError unless dependency.valid_dependency? - end + def has_valid_build_dependencies? + return true if Feature.enabled?('ci_disable_validates_dependencies') + + dependencies.all?(&:valid_dependency?) end def valid_dependency? @@ -594,6 +592,24 @@ module Ci true end + def runner_required_feature_names + strong_memoize(:runner_required_feature_names) do + RUNNER_FEATURES.select do |feature, method| + method.call(self) + end.keys + end + end + + def supported_runner?(features) + runner_required_feature_names.all? do |feature_name| + features&.dig(feature_name) + end + end + + def publishes_artifacts_reports? + options&.dig(:artifacts, :reports)&.any? + end + def hide_secrets(trace) return unless trace diff --git a/app/models/ci/build_runner_session.rb b/app/models/ci/build_runner_session.rb index 6f3be31d8e1..869dc0ccadf 100644 --- a/app/models/ci/build_runner_session.rb +++ b/app/models/ci/build_runner_session.rb @@ -17,7 +17,7 @@ module Ci { subprotocols: ['terminal.gitlab.com'].freeze, url: "#{url}/exec".sub("https://", "wss://"), - headers: { Authorization: authorization.presence }.compact, + headers: { Authorization: [authorization.presence] }.compact, ca_pem: certificate.presence } end diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb index b566edae7bb..58de3448577 100644 --- a/app/models/clusters/applications/helm.rb +++ b/app/models/clusters/applications/helm.rb @@ -1,26 +1,13 @@ -require 'openssl' - module Clusters module Applications class Helm < ActiveRecord::Base self.table_name = 'clusters_applications_helm' - attr_encrypted :ca_key, - mode: :per_attribute_iv, - key: Settings.attr_encrypted_db_key_base_truncated, - algorithm: 'aes-256-cbc' - include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus default_value_for :version, Gitlab::Kubernetes::Helm::HELM_VERSION - before_create :create_keys_and_certs - - def issue_client_cert - ca_cert_obj.issue - end - def set_initial_status return unless not_installable? @@ -28,41 +15,7 @@ module Clusters end def install_command - Gitlab::Kubernetes::Helm::InitCommand.new( - name: name, - files: files - ) - end - - def has_ssl? - ca_key.present? && ca_cert.present? - end - - private - - def files - { - 'ca.pem': ca_cert, - 'cert.pem': tiller_cert.cert_string, - 'key.pem': tiller_cert.key_string - } - end - - def create_keys_and_certs - ca_cert = Gitlab::Kubernetes::Helm::Certificate.generate_root - self.ca_key = ca_cert.key_string - self.ca_cert = ca_cert.cert_string - end - - def tiller_cert - @tiller_cert ||= ca_cert_obj.issue(expires_in: Gitlab::Kubernetes::Helm::Certificate::INFINITE_EXPIRY) - end - - def ca_cert_obj - return unless has_ssl? - - Gitlab::Kubernetes::Helm::Certificate - .from_strings(ca_key, ca_cert) + Gitlab::Kubernetes::Helm::InitCommand.new(name) end end end diff --git a/app/models/clusters/applications/ingress.rb b/app/models/clusters/applications/ingress.rb index 64810812531..27fc3b85465 100644 --- a/app/models/clusters/applications/ingress.rb +++ b/app/models/clusters/applications/ingress.rb @@ -32,9 +32,9 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name: name, + name, chart: chart, - files: files + values: values ) end diff --git a/app/models/clusters/applications/jupyter.rb b/app/models/clusters/applications/jupyter.rb index cff5a423acb..975d434e1a4 100644 --- a/app/models/clusters/applications/jupyter.rb +++ b/app/models/clusters/applications/jupyter.rb @@ -35,9 +35,9 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name: name, + name, chart: chart, - files: files, + values: values, repository: repository ) end diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index 22815cc1219..ea6ec4d6b03 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -43,10 +43,10 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name: name, + name, chart: chart, version: version, - files: files + values: values ) end diff --git a/app/models/clusters/applications/runner.rb b/app/models/clusters/applications/runner.rb index 4c894b5376d..e6f795f3e0b 100644 --- a/app/models/clusters/applications/runner.rb +++ b/app/models/clusters/applications/runner.rb @@ -28,9 +28,9 @@ module Clusters def install_command Gitlab::Kubernetes::Helm::InstallCommand.new( - name: name, + name, chart: chart, - files: files, + values: values, repository: repository ) end diff --git a/app/models/clusters/concerns/application_data.rb b/app/models/clusters/concerns/application_data.rb index d66f09d48b5..96ac757e99e 100644 --- a/app/models/clusters/concerns/application_data.rb +++ b/app/models/clusters/concerns/application_data.rb @@ -12,34 +12,8 @@ module Clusters File.read(chart_values_file) end - def files - @files ||= begin - files = { 'values.yaml': values } - - files.merge!(certificate_files) if cluster.application_helm.has_ssl? - - files - end - end - private - def certificate_files - { - 'ca.pem': ca_cert, - 'cert.pem': helm_cert.cert_string, - 'key.pem': helm_cert.key_string - } - end - - def ca_cert - cluster.application_helm.ca_cert - end - - def helm_cert - @helm_cert ||= cluster.application_helm.issue_client_cert - end - def chart_values_file "#{Rails.root}/vendor/#{name}/values.yaml" end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 97516079b66..8b1093655b7 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -46,7 +46,8 @@ class CommitStatus < ActiveRecord::Base api_failure: 2, stuck_or_timeout_failure: 3, runner_system_failure: 4, - missing_dependency_failure: 5 + missing_dependency_failure: 5, + runner_unsupported: 6 } ## diff --git a/app/models/concerns/atomic_internal_id.rb b/app/models/concerns/atomic_internal_id.rb index 4fef615e6e3..5e39676b24b 100644 --- a/app/models/concerns/atomic_internal_id.rb +++ b/app/models/concerns/atomic_internal_id.rb @@ -35,16 +35,21 @@ module AtomicInternalId define_method("ensure_#{scope}_#{column}!") do scope_value = association(scope).reader + value = read_attribute(column) - if read_attribute(column).blank? && scope_value - scope_attrs = { scope_value.class.table_name.singularize.to_sym => scope_value } - usage = self.class.table_name.to_sym + return value unless scope_value - new_iid = InternalId.generate_next(self, scope_attrs, usage, init) - write_attribute(column, new_iid) + scope_attrs = { scope_value.class.table_name.singularize.to_sym => scope_value } + usage = self.class.table_name.to_sym + + if value.present? + InternalId.track_greatest(self, scope_attrs, usage, value, init) + else + value = InternalId.generate_next(self, scope_attrs, usage, init) + write_attribute(column, value) end - read_attribute(column) + value end end end diff --git a/app/models/concerns/label_eventable.rb b/app/models/concerns/label_eventable.rb new file mode 100644 index 00000000000..d22d93448e4 --- /dev/null +++ b/app/models/concerns/label_eventable.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# == LabelEventable concern +# +# Contains functionality related to objects that support adding/removing labels. +# +# This concern is not used yet, it will be used for: +# https://gitlab.com/gitlab-org/gitlab-ce/issues/48483 + +module LabelEventable + extend ActiveSupport::Concern + + included do + has_many :resource_label_events + end +end diff --git a/app/models/internal_id.rb b/app/models/internal_id.rb index f50f28deffe..e5d0f94073c 100644 --- a/app/models/internal_id.rb +++ b/app/models/internal_id.rb @@ -1,6 +1,9 @@ # An InternalId is a strictly monotone sequence of integers # generated for a given scope and usage. # +# The monotone sequence may be broken if an ID is explicitly provided +# to `.track_greatest_and_save!` or `#track_greatest`. +# # For example, issues use their project to scope internal ids: # In that sense, scope is "project" and usage is "issues". # Generated internal ids for an issue are unique per project. @@ -25,13 +28,34 @@ class InternalId < ActiveRecord::Base # The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL). # As such, the increment is atomic and safe to be called concurrently. def increment_and_save! + update_and_save { self.last_value = (last_value || 0) + 1 } + end + + # Increments #last_value with new_value if it is greater than the current, + # and saves the record + # + # The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL). + # As such, the increment is atomic and safe to be called concurrently. + def track_greatest_and_save!(new_value) + update_and_save { self.last_value = [last_value || 0, new_value].max } + end + + private + + def update_and_save(&block) lock! - self.last_value = (last_value || 0) + 1 + yield save! last_value end class << self + def track_greatest(subject, scope, usage, new_value, init) + return new_value unless available? + + InternalIdGenerator.new(subject, scope, usage, init).track_greatest(new_value) + end + def generate_next(subject, scope, usage, init) # Shortcut if `internal_ids` table is not available (yet) # This can be the case in other (unrelated) migration specs @@ -94,6 +118,16 @@ class InternalId < ActiveRecord::Base end end + # Create a record in internal_ids if one does not yet exist + # and set its new_value if it is higher than the current last_value + # + # Note this will acquire a ROW SHARE lock on the InternalId record + def track_greatest(new_value) + subject.transaction do + (lookup || create_record).track_greatest_and_save!(new_value) + end + end + private # Retrieve InternalId record for (project, usage) combination, if it exists diff --git a/app/models/issue.rb b/app/models/issue.rb index 4715d942c8d..e4ed06f9a69 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -12,6 +12,7 @@ class Issue < ActiveRecord::Base include TimeTrackable include ThrottledTouch include IgnorableColumn + include LabelEventable ignore_column :assignee_id, :branch_name, :deleted_at diff --git a/app/models/label.rb b/app/models/label.rb index 7bbcaa121ca..7b08547fa6e 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -2,6 +2,7 @@ class Label < ActiveRecord::Base include CacheMarkdownField include Referable include Subscribable + include Gitlab::SQL::Pattern # Represents a "No Label" state used for filtering Issues and Merge # Requests that have no label assigned. @@ -103,6 +104,17 @@ class Label < ActiveRecord::Base nil end + # Searches for labels with a matching title or description. + # + # This method uses ILIKE on PostgreSQL and LIKE on MySQL. + # + # query - The search query as a String. + # + # Returns an ActiveRecord::Relation. + def self.search(query) + fuzzy_search(query, [:title, :description]) + end + def open_issues_count(user = nil) issues_count(user, state: 'opened') end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b4090fd8baf..124ff6b3f91 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -10,6 +10,7 @@ class MergeRequest < ActiveRecord::Base include EachBatch include ThrottledTouch include Gitlab::Utils::StrongMemoize + include LabelEventable ignore_column :locked_at, :ref_fetched, diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb index 5d4e3c34b39..ef4a26f0e7f 100644 --- a/app/models/project_statistics.rb +++ b/app/models/project_statistics.rb @@ -5,7 +5,7 @@ class ProjectStatistics < ActiveRecord::Base before_save :update_storage_size COLUMNS_TO_REFRESH = [:repository_size, :lfs_objects_size, :commit_count].freeze - INCREMENTABLE_COLUMNS = [:build_artifacts_size].freeze + INCREMENTABLE_COLUMNS = { build_artifacts_size: %i[storage_size] }.freeze def total_repository_size repository_size + lfs_objects_size @@ -38,11 +38,28 @@ class ProjectStatistics < ActiveRecord::Base self.storage_size = repository_size + lfs_objects_size + build_artifacts_size end + # Since this incremental update method does not call update_storage_size above, + # we have to update the storage_size here as additional column. + # Additional columns are updated depending on key => [columns], which allows + # to update statistics which are and also those which aren't included in storage_size + # or any other additional summary column in the future. def self.increment_statistic(project_id, key, amount) - raise ArgumentError, "Cannot increment attribute: #{key}" unless key.in?(INCREMENTABLE_COLUMNS) + raise ArgumentError, "Cannot increment attribute: #{key}" unless INCREMENTABLE_COLUMNS.key?(key) return if amount == 0 where(project_id: project_id) - .update_all(["#{key} = COALESCE(#{key}, 0) + (?)", amount]) + .columns_to_increment(key, amount) + end + + def self.columns_to_increment(key, amount) + updates = ["#{key} = COALESCE(#{key}, 0) + (#{amount})"] + + if (additional = INCREMENTABLE_COLUMNS[key]) + additional.each do |column| + updates << "#{column} = COALESCE(#{column}, 0) + (#{amount})" + end + end + + update_all(updates.join(', ')) end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 9873d9a6327..50f42be661b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -312,6 +312,8 @@ class Repository # types - An Array of file types (e.g. `:readme`) used to refresh extra # caches. def refresh_method_caches(types) + return if types.empty? + to_refresh = [] types.each do |type| diff --git a/app/models/resource_label_event.rb b/app/models/resource_label_event.rb new file mode 100644 index 00000000000..42c255fcd1e --- /dev/null +++ b/app/models/resource_label_event.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +# This model is not used yet, it will be used for: +# https://gitlab.com/gitlab-org/gitlab-ce/issues/48483 +class ResourceLabelEvent < ActiveRecord::Base + belongs_to :user + belongs_to :issue + belongs_to :merge_request + belongs_to :label + + validates :user, presence: true, on: :create + validates :label, presence: true, on: :create + validate :exactly_one_issuable + + enum action: { + add: 1, + remove: 2 + } + + def self.issuable_columns + %i(issue_id merge_request_id).freeze + end + + def issuable + issue || merge_request + end + + private + + def exactly_one_issuable + if self.class.issuable_columns.count { |attr| self[attr] } != 1 + errors.add(:base, "Exactly one of #{self.class.issuable_columns.join(', ')} is required") + end + end +end diff --git a/app/presenters/commit_status_presenter.rb b/app/presenters/commit_status_presenter.rb index 3a9088cfcb8..a08f34e2335 100644 --- a/app/presenters/commit_status_presenter.rb +++ b/app/presenters/commit_status_presenter.rb @@ -2,17 +2,19 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated CALLOUT_FAILURE_MESSAGES = { - unknown_failure: 'There is an unknown failure, please try again', - api_failure: 'There has been an API failure, please try again', - stuck_or_timeout_failure: 'There has been a timeout failure or the job got stuck. Check your timeout limits or try again', - runner_system_failure: 'There has been a runner system failure, please try again', - missing_dependency_failure: 'There has been a missing dependency failure' + unknown_failure: 'There is an unknown failure, please try again', + script_failure: nil, + api_failure: 'There has been an API failure, please try again', + stuck_or_timeout_failure: 'There has been a timeout failure or the job got stuck. Check your timeout limits or try again', + runner_system_failure: 'There has been a runner system failure, please try again', + missing_dependency_failure: 'There has been a missing dependency failure', + runner_unsupported: 'Your runner is outdated, please upgrade your runner' }.freeze presents :build def callout_failure_message - CALLOUT_FAILURE_MESSAGES[failure_reason.to_sym] + CALLOUT_FAILURE_MESSAGES.fetch(failure_reason.to_sym) end def recoverable? diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index 4fe04e4b206..63fd9d63ec4 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -132,6 +132,10 @@ class MergeRequestWidgetEntity < IssuableEntity can?(request.current_user, :create_note, merge_request) end + expose :can_create_issue do |merge_request| + can?(current_user, :create_issue, merge_request.project) + end + expose :can_update do |merge_request| can?(request.current_user, :update_merge_request, merge_request) end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 81857d0cb4c..893b37b831a 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -9,11 +9,11 @@ module Auth return error('UNAVAILABLE', status: 404, message: 'registry not enabled') unless registry.enabled - unless scope || current_user || project + unless scopes.any? || current_user || project return error('DENIED', status: 403, message: 'access forbidden') end - { token: authorized_token(scope).encoded } + { token: authorized_token(*scopes).encoded } end def self.full_access_token(*names) @@ -47,10 +47,12 @@ module Auth end end - def scope - return unless params[:scope] + def scopes + return [] unless params[:scopes] - @scope ||= process_scope(params[:scope]) + @scopes ||= params[:scopes].map do |scope| + process_scope(scope) + end.compact end def process_scope(scope) diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index f7ccec3a700..11f85627faf 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -41,16 +41,10 @@ module Ci begin # In case when 2 runners try to assign the same build, second runner will be declined # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method. - begin - build.runner_id = runner.id - build.runner_session_attributes = params[:session] if params[:session].present? - - build.run! + if assign_runner!(build, params) register_success(build) return Result.new(build, true) # rubocop:disable Cop/AvoidReturnFromBlocks - rescue Ci::Build::MissingDependenciesError - build.drop!(:missing_dependency_failure) end rescue StateMachines::InvalidTransition, ActiveRecord::StaleObjectError # We are looping to find another build that is not conflicting @@ -72,6 +66,24 @@ module Ci private + def assign_runner!(build, params) + build.runner_id = runner.id + build.runner_session_attributes = params[:session] if params[:session].present? + + unless build.has_valid_build_dependencies? + build.drop!(:missing_dependency_failure) + return false + end + + unless build.supported_runner?(params.dig(:info, :features)) + build.drop!(:runner_unsupported) + return false + end + + build.run! + true + end + def builds_for_shared_runner new_builds. # don't run projects which have not enabled shared runners and builds diff --git a/app/services/clusters/applications/check_installation_progress_service.rb b/app/services/clusters/applications/check_installation_progress_service.rb index a1165b0ab28..35f5cff0e0c 100644 --- a/app/services/clusters/applications/check_installation_progress_service.rb +++ b/app/services/clusters/applications/check_installation_progress_service.rb @@ -35,7 +35,7 @@ module Clusters def check_timeout if timeouted? begin - app.make_errored!('Installation timeouted') + app.make_errored!('Installation timed out') ensure remove_installation_pod end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index f0587667d37..e3640e38bfa 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -3,6 +3,7 @@ class GitPushService < BaseService attr_accessor :push_data, :push_commits include Gitlab::Access + include Gitlab::Utils::StrongMemoize # The N most recent commits to process in a single push payload. PROCESS_COMMIT_LIMIT = 100 @@ -21,14 +22,14 @@ class GitPushService < BaseService # 6. Checks if the project's main language has changed # def execute - @project.repository.after_create if @project.empty_repo? - @project.repository.after_push_commit(branch_name) + project.repository.after_create if project.empty_repo? + project.repository.after_push_commit(branch_name) if push_remove_branch? - @project.repository.after_remove_branch + project.repository.after_remove_branch @push_commits = [] elsif push_to_new_branch? - @project.repository.after_create_branch + project.repository.after_create_branch # Re-find the pushed commits. if default_branch? @@ -38,14 +39,14 @@ class GitPushService < BaseService # Use the pushed commits that aren't reachable by the default branch # as a heuristic. This may include more commits than are actually pushed, but # that shouldn't matter because we check for existing cross-references later. - @push_commits = @project.repository.commits_between(@project.default_branch, params[:newrev]) + @push_commits = project.repository.commits_between(project.default_branch, params[:newrev]) # don't process commits for the initial push to the default branch process_commit_messages end elsif push_to_existing_branch? # Collect data for this git push - @push_commits = @project.repository.commits_between(params[:oldrev], params[:newrev]) + @push_commits = project.repository.commits_between(params[:oldrev], params[:newrev]) process_commit_messages @@ -64,7 +65,7 @@ class GitPushService < BaseService end def update_gitattributes - @project.repository.copy_gitattributes(params[:ref]) + project.repository.copy_gitattributes(params[:ref]) end def update_caches @@ -88,7 +89,7 @@ class GitPushService < BaseService types = [] end - ProjectCacheWorker.perform_async(@project.id, types, [:commit_count, :repository_size]) + ProjectCacheWorker.perform_async(project.id, types, [:commit_count, :repository_size]) end def update_signatures @@ -121,10 +122,10 @@ class GitPushService < BaseService protected def update_remote_mirrors - return unless @project.has_remote_mirror? + return unless project.has_remote_mirror? - @project.mark_stuck_remote_mirrors_as_failed! - @project.update_remote_mirrors + project.mark_stuck_remote_mirrors_as_failed! + project.update_remote_mirrors end def execute_related_hooks @@ -132,14 +133,14 @@ class GitPushService < BaseService # could cause the last commit of a merge request to change. # UpdateMergeRequestsWorker - .perform_async(@project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref]) + .perform_async(project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref]) - EventCreateService.new.push(@project, current_user, build_push_data) - Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute(:push) + EventCreateService.new.push(project, current_user, build_push_data) + Ci::CreatePipelineService.new(project, current_user, build_push_data).execute(:push) SystemHookPushWorker.perform_async(build_push_data.dup, :push_hooks) - @project.execute_hooks(build_push_data.dup, :push_hooks) - @project.execute_services(build_push_data.dup, :push_hooks) + project.execute_hooks(build_push_data.dup, :push_hooks) + project.execute_services(build_push_data.dup, :push_hooks) if push_remove_branch? AfterBranchDeleteService @@ -149,52 +150,50 @@ class GitPushService < BaseService end def perform_housekeeping - housekeeping = Projects::HousekeepingService.new(@project) + housekeeping = Projects::HousekeepingService.new(project) housekeeping.increment! housekeeping.execute if housekeeping.needed? rescue Projects::HousekeepingService::LeaseTaken end def process_default_branch - @push_commits_count = project.repository.commit_count_for_ref(params[:ref]) - - offset = [@push_commits_count - PROCESS_COMMIT_LIMIT, 0].max + offset = [push_commits_count - PROCESS_COMMIT_LIMIT, 0].max @push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT) - @project.after_create_default_branch + project.after_create_default_branch end def build_push_data @push_data ||= Gitlab::DataBuilder::Push.build( - @project, + project, current_user, params[:oldrev], params[:newrev], params[:ref], @push_commits, - commits_count: @push_commits_count) + commits_count: push_commits_count) end def push_to_existing_branch? # Return if this is not a push to a branch (e.g. new commits) - Gitlab::Git.branch_ref?(params[:ref]) && !Gitlab::Git.blank_ref?(params[:oldrev]) + branch_ref? && !Gitlab::Git.blank_ref?(params[:oldrev]) end def push_to_new_branch? - Gitlab::Git.branch_ref?(params[:ref]) && Gitlab::Git.blank_ref?(params[:oldrev]) + strong_memoize(:push_to_new_branch) do + branch_ref? && Gitlab::Git.blank_ref?(params[:oldrev]) + end end def push_remove_branch? - Gitlab::Git.branch_ref?(params[:ref]) && Gitlab::Git.blank_ref?(params[:newrev]) - end - - def push_to_branch? - Gitlab::Git.branch_ref?(params[:ref]) + strong_memoize(:push_remove_branch) do + branch_ref? && Gitlab::Git.blank_ref?(params[:newrev]) + end end def default_branch? - Gitlab::Git.branch_ref?(params[:ref]) && - (Gitlab::Git.ref_name(params[:ref]) == project.default_branch || project.default_branch.nil?) + branch_ref? && + (branch_name == project.default_branch || project.default_branch.nil?) end def commit_user(commit) @@ -202,7 +201,21 @@ class GitPushService < BaseService end def branch_name - @branch_name ||= Gitlab::Git.ref_name(params[:ref]) + strong_memoize(:branch_name) do + Gitlab::Git.ref_name(params[:ref]) + end + end + + def branch_ref? + strong_memoize(:branch_ref) do + Gitlab::Git.branch_ref?(params[:ref]) + end + end + + def push_commits_count + strong_memoize(:push_commits_count) do + project.repository.commit_count_for_ref(params[:ref]) + end end def last_pushed_commits diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 3ff2d1d107d..dbadafc0f52 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -9,12 +9,12 @@ class GitTagPushService < BaseService @push_data = build_push_data - EventCreateService.new.push(project, current_user, @push_data) - Ci::CreatePipelineService.new(project, current_user, @push_data).execute(:push) + EventCreateService.new.push(project, current_user, push_data) + Ci::CreatePipelineService.new(project, current_user, push_data).execute(:push) - SystemHooksService.new.execute_hooks(build_system_push_data.dup, :tag_push_hooks) - project.execute_hooks(@push_data.dup, :tag_push_hooks) - project.execute_services(@push_data.dup, :tag_push_hooks) + SystemHooksService.new.execute_hooks(build_system_push_data, :tag_push_hooks) + project.execute_hooks(push_data.dup, :tag_push_hooks) + project.execute_services(push_data.dup, :tag_push_hooks) ProjectCacheWorker.perform_async(project.id, [], [:commit_count, :repository_size]) diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index c02dddf67b2..faa4c8a5a4f 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -37,6 +37,8 @@ module Issues end if issue.previous_changes.include?('confidential') + # don't enqueue immediately to prevent todos removal in case of a mistake + TodosDestroyer::ConfidentialIssueWorker.perform_in(1.hour, issue.id) if issue.confidential? create_confidentiality_note(issue) end diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index aca0ba66646..c186a5971dc 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -15,6 +15,8 @@ module Members notification_service.decline_access_request(member) end + enqeue_delete_todos(member) + after_execute(member: member) member @@ -22,6 +24,12 @@ module Members private + def enqeue_delete_todos(member) + type = member.is_a?(GroupMember) ? 'Group' : 'Project' + # don't enqueue immediately to prevent todos removal in case of a mistake + TodosDestroyer::EntityLeaveWorker.perform_in(1.hour, member.user_id, member.source_id, type) + end + def can_destroy_member?(member) can?(current_user, destroy_member_permission(member), member) end diff --git a/app/services/projects/create_from_template_service.rb b/app/services/projects/create_from_template_service.rb index f5c48e56880..8306d43ca7c 100644 --- a/app/services/projects/create_from_template_service.rb +++ b/app/services/projects/create_from_template_service.rb @@ -2,21 +2,27 @@ module Projects class CreateFromTemplateService < BaseService + include Gitlab::Utils::StrongMemoize + def initialize(user, params) @current_user, @params = user, params.dup end def execute - template_name = params.delete(:template_name) - file = Gitlab::ProjectTemplate.find(template_name).file + file = Gitlab::ProjectTemplate.find(template_name)&.file override_params = params.dup params[:file] = file GitlabProjectsImportService.new(current_user, params, override_params).execute - ensure file&.close end + + def template_name + strong_memoize(:template_name) do + params.delete(:template_name).presence + end + end end end diff --git a/app/services/projects/gitlab_projects_import_service.rb b/app/services/projects/gitlab_projects_import_service.rb index bc6e9caebb8..615dccc4685 100644 --- a/app/services/projects/gitlab_projects_import_service.rb +++ b/app/services/projects/gitlab_projects_import_service.rb @@ -5,6 +5,9 @@ # The latter will under the hood just import an archive supplied by GitLab. module Projects class GitlabProjectsImportService + include Gitlab::Utils::StrongMemoize + include Gitlab::TemplateHelper + attr_reader :current_user, :params def initialize(user, import_params, override_params = nil) @@ -12,39 +15,17 @@ module Projects end def execute - FileUtils.mkdir_p(File.dirname(import_upload_path)) - - file = params.delete(:file) - FileUtils.copy_entry(file.path, import_upload_path) - - @overwrite = params.delete(:overwrite) - data = {} - data[:override_params] = @override_params if @override_params - - if overwrite_project? - data[:original_path] = params[:path] - params[:path] += "-#{tmp_filename}" - end + prepare_template_environment(template_file&.path) - params[:import_type] = 'gitlab_project' - params[:import_source] = import_upload_path - params[:import_data] = { data: data } if data.present? + prepare_import_params ::Projects::CreateService.new(current_user, params).execute end private - def import_upload_path - @import_upload_path ||= Gitlab::ImportExport.import_upload_path(filename: tmp_filename) - end - - def tmp_filename - SecureRandom.hex - end - def overwrite_project? - @overwrite && project_with_same_full_path? + overwrite? && project_with_same_full_path? end def project_with_same_full_path? @@ -52,7 +33,38 @@ module Projects end def current_namespace - @current_namespace ||= Namespace.find_by(id: params[:namespace_id]) + strong_memoize(:current_namespace) do + Namespace.find_by(id: params[:namespace_id]) + end + end + + def overwrite? + strong_memoize(:overwrite) do + params.delete(:overwrite) + end + end + + def template_file + strong_memoize(:template_file) do + params.delete(:file) + end + end + + def prepare_import_params + data = {} + data[:override_params] = @override_params if @override_params + + if overwrite_project? + data[:original_path] = params[:path] + params[:path] += "-#{tmp_filename}" + end + + if template_file + params[:import_type] = 'gitlab_project' + params[:import_source] = import_upload_path + end + + params[:import_data] = { data: data } if data.present? end end end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index d3dc11435fe..31ab4fbe49e 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -25,13 +25,7 @@ module Projects return validation_failed! if project.errors.any? if project.update(params.except(:default_branch)) - if project.previous_changes.include?('path') - project.rename_repo - else - system_hook_service.execute_hooks_for(project, :update) - end - - update_pages_config if changing_pages_https_only? + after_update success else @@ -47,6 +41,30 @@ module Projects private + def after_update + todos_features_changes = %w( + issues_access_level + merge_requests_access_level + repository_access_level + ) + project_changed_feature_keys = project.project_feature.previous_changes.keys + + if project.previous_changes.include?(:visibility_level) && project.private? + # don't enqueue immediately to prevent todos removal in case of a mistake + TodosDestroyer::ProjectPrivateWorker.perform_in(1.hour, project.id) + elsif (project_changed_feature_keys & todos_features_changes).present? + TodosDestroyer::PrivateFeaturesWorker.perform_in(1.hour, project.id) + end + + if project.previous_changes.include?('path') + project.rename_repo + else + system_hook_service.execute_hooks_for(project, :update) + end + + update_pages_config if changing_pages_https_only? + end + def validation_failed! model_errors = project.errors.full_messages.to_sentence error_message = model_errors.presence || 'Project could not be updated!' diff --git a/app/services/resource_events/change_labels_service.rb b/app/services/resource_events/change_labels_service.rb new file mode 100644 index 00000000000..8edb0ddb3ed --- /dev/null +++ b/app/services/resource_events/change_labels_service.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +# This service is not used yet, it will be used for: +# https://gitlab.com/gitlab-org/gitlab-ce/issues/48483 +module ResourceEvents + class ChangeLabelsService + attr_reader :resource, :user + + def initialize(resource, user) + @resource, @user = resource, user + end + + def execute(added_labels: [], removed_labels: []) + label_hash = { + resource_column(resource) => resource.id, + user_id: user.id, + created_at: Time.now + } + + labels = added_labels.map do |label| + label_hash.merge(label_id: label.id, action: ResourceLabelEvent.actions['add']) + end + labels += removed_labels.map do |label| + label_hash.merge(label_id: label.id, action: ResourceLabelEvent.actions['remove']) + end + + Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, labels) + end + + private + + def resource_column(resource) + case resource + when Issue + :issue_id + when MergeRequest + :merge_request_id + else + raise ArgumentError, "Unknown resource type #{resource.class.name}" + end + end + end +end diff --git a/app/services/todos/destroy/base_service.rb b/app/services/todos/destroy/base_service.rb new file mode 100644 index 00000000000..dff5e1f30e5 --- /dev/null +++ b/app/services/todos/destroy/base_service.rb @@ -0,0 +1,33 @@ +module Todos + module Destroy + class BaseService + def execute + return unless todos_to_remove? + + without_authorized(todos).delete_all + end + + private + + def without_authorized(items) + items.where('user_id NOT IN (?)', authorized_users) + end + + def authorized_users + ProjectAuthorization.select(:user_id).where(project_id: project_ids) + end + + def todos + raise NotImplementedError + end + + def project_ids + raise NotImplementedError + end + + def todos_to_remove? + raise NotImplementedError + end + end + end +end diff --git a/app/services/todos/destroy/confidential_issue_service.rb b/app/services/todos/destroy/confidential_issue_service.rb new file mode 100644 index 00000000000..c5b66df057a --- /dev/null +++ b/app/services/todos/destroy/confidential_issue_service.rb @@ -0,0 +1,39 @@ +module Todos + module Destroy + class ConfidentialIssueService < ::Todos::Destroy::BaseService + extend ::Gitlab::Utils::Override + + attr_reader :issue + + def initialize(issue_id) + @issue = Issue.find_by(id: issue_id) + end + + private + + override :todos + def todos + Todo.where(target: issue) + .where('user_id != ?', issue.author_id) + .where('user_id NOT IN (?)', issue.assignees.select(:id)) + end + + override :todos_to_remove? + def todos_to_remove? + issue&.confidential? + end + + override :project_ids + def project_ids + issue.project_id + end + + override :authorized_users + def authorized_users + ProjectAuthorization.select(:user_id) + .where(project_id: project_ids) + .where('access_level >= ?', Gitlab::Access::REPORTER) + end + end + end +end diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb new file mode 100644 index 00000000000..2ff9f94b718 --- /dev/null +++ b/app/services/todos/destroy/entity_leave_service.rb @@ -0,0 +1,59 @@ +module Todos + module Destroy + class EntityLeaveService < ::Todos::Destroy::BaseService + extend ::Gitlab::Utils::Override + + attr_reader :user_id, :entity + + def initialize(user_id, entity_id, entity_type) + unless %w(Group Project).include?(entity_type) + raise ArgumentError.new("#{entity_type} is not an entity user can leave") + end + + @user_id = user_id + @entity = entity_type.constantize.find_by(id: entity_id) + end + + private + + override :todos + def todos + if entity.private? + Todo.where(project_id: project_ids, user_id: user_id) + else + project_ids.each do |project_id| + TodosDestroyer::PrivateFeaturesWorker.perform_async(project_id, user_id) + end + + Todo.where( + target_id: confidential_issues.select(:id), target_type: Issue, user_id: user_id + ) + end + end + + override :project_ids + def project_ids + case entity + when Project + [entity.id] + when Namespace + Project.select(:id).where(namespace_id: entity.self_and_descendants.select(:id)) + end + end + + override :todos_to_remove? + def todos_to_remove? + # if an entity is provided we want to check always at least private features + !!entity + end + + def confidential_issues + assigned_ids = IssueAssignee.select(:issue_id).where(user_id: user_id) + + Issue.where(project_id: project_ids, confidential: true) + .where('author_id != ?', user_id) + .where('id NOT IN (?)', assigned_ids) + end + end + end +end diff --git a/app/services/todos/destroy/private_features_service.rb b/app/services/todos/destroy/private_features_service.rb new file mode 100644 index 00000000000..4d8e2877bfb --- /dev/null +++ b/app/services/todos/destroy/private_features_service.rb @@ -0,0 +1,40 @@ +module Todos + module Destroy + class PrivateFeaturesService < ::Todos::Destroy::BaseService + attr_reader :project_ids, :user_id + + def initialize(project_ids, user_id = nil) + @project_ids = project_ids + @user_id = user_id + end + + def execute + ProjectFeature.where(project_id: project_ids).each do |project_features| + target_types = [] + target_types << Issue if private?(project_features.issues_access_level) + target_types << MergeRequest if private?(project_features.merge_requests_access_level) + target_types << Commit if private?(project_features.repository_access_level) + + next if target_types.empty? + + remove_todos(project_features.project_id, target_types) + end + end + + private + + def private?(feature_level) + feature_level == ProjectFeature::PRIVATE + end + + def remove_todos(project_id, target_types) + items = Todo.where(project_id: project_id) + items = items.where(user_id: user_id) if user_id + + items.where('user_id NOT IN (?)', authorized_users) + .where(target_type: target_types) + .delete_all + end + end + end +end diff --git a/app/services/todos/destroy/project_private_service.rb b/app/services/todos/destroy/project_private_service.rb new file mode 100644 index 00000000000..171933e7cbc --- /dev/null +++ b/app/services/todos/destroy/project_private_service.rb @@ -0,0 +1,30 @@ +module Todos + module Destroy + class ProjectPrivateService < ::Todos::Destroy::BaseService + extend ::Gitlab::Utils::Override + + attr_reader :project + + def initialize(project_id) + @project = Project.find_by(id: project_id) + end + + private + + override :todos + def todos + Todo.where(project_id: project_ids) + end + + override :project_ids + def project_ids + project.id + end + + override :todos_to_remove? + def todos_to_remove? + project&.private? + end + end + end +end diff --git a/app/views/projects/labels/index.html.haml b/app/views/projects/labels/index.html.haml index fb5b0fc15c9..768ce9bd103 100644 --- a/app/views/projects/labels/index.html.haml +++ b/app/views/projects/labels/index.html.haml @@ -2,32 +2,45 @@ - page_title "Labels" - can_admin_label = can?(current_user, :admin_label, @project) - hide_class = '' +- search = params[:search] - if can_admin_label - content_for(:header_content) do .nav-controls = link_to _('New label'), new_project_label_path(@project), class: "btn btn-new" -- if @labels.exists? || @prioritized_labels.exists? +- if @labels.exists? || @prioritized_labels.exists? || search.present? #promote-label-modal %div{ class: container_class } .top-area.adjust .nav-text = _('Labels can be applied to issues and merge requests.') - - if can_admin_label - = _('Star a label to make it a priority label. Order the prioritized labels to change their relative priority, by dragging.') - .labels-container.prepend-top-5 + .nav-controls + = form_tag project_labels_path(@project), method: :get do + .input-group + = search_field_tag :search, params[:search], { placeholder: _('Filter'), id: 'label-search', class: 'form-control search-text-input input-short', spellcheck: false } + %span.input-group-append + %button.btn.btn-default{ type: "submit", "aria-label" => _('Submit search') } + = icon("search") + + .labels-container.prepend-top-10 - if can_admin_label + - if search.blank? + %p.text-muted + = _('Star a label to make it a priority label. Order the prioritized labels to change their relative priority, by dragging.') -# Only show it in the first page - hide = @available_labels.empty? || (params[:page].present? && params[:page] != '1') .prioritized-labels{ class: ('hide' if hide) } %h5.prepend-top-10= _('Prioritized Labels') .content-list.manage-labels-list.js-prioritized-labels{ "data-url" => set_priorities_project_labels_path(@project) } - #js-priority-labels-empty-state.priority-labels-empty-state{ class: "#{'hidden' unless @prioritized_labels.empty?}" } + #js-priority-labels-empty-state.priority-labels-empty-state{ class: "#{'hidden' unless @prioritized_labels.empty? && search.blank?}" } = render 'shared/empty_states/priority_labels' - if @prioritized_labels.present? = render partial: 'shared/label', subject: @project, collection: @prioritized_labels, as: :label, locals: { force_priority: true } + - elsif search.present? + .nothing-here-block + = _('No prioritised labels with such name or description') - if @labels.present? .other-labels @@ -36,6 +49,18 @@ .content-list.manage-labels-list.js-other-labels = render partial: 'shared/label', subject: @project, collection: @labels, as: :label = paginate @labels, theme: 'gitlab' + - elsif search.present? + .other-labels + - if @available_labels.any? + %h5 + = _('Other Labels') + .nothing-here-block + = _('No other labels with such name or description') + - else + .nothing-here-block + = _('No labels with such name or description') + + - else = render 'shared/empty_states/labels' diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 4de35b9bd06..f2651cb54ec 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -73,6 +73,11 @@ - repository_check:repository_check_batch - repository_check:repository_check_single_repository +- todos_destroyer:todos_destroyer_confidential_issue +- todos_destroyer:todos_destroyer_entity_leave +- todos_destroyer:todos_destroyer_project_private +- todos_destroyer:todos_destroyer_private_features + - default - mailers # ActionMailer::DeliveryJob.queue_name diff --git a/app/workers/concerns/todos_destroyer_queue.rb b/app/workers/concerns/todos_destroyer_queue.rb new file mode 100644 index 00000000000..8e2b1d30579 --- /dev/null +++ b/app/workers/concerns/todos_destroyer_queue.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +## +# Concern for setting Sidekiq settings for the various Todos Destroyers. +# +module TodosDestroyerQueue + extend ActiveSupport::Concern + + included do + queue_namespace :todos_destroyer + end +end diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb index 8c64c513c74..82189a3c9f5 100644 --- a/app/workers/repository_import_worker.rb +++ b/app/workers/repository_import_worker.rb @@ -7,9 +7,9 @@ class RepositoryImportWorker include ProjectImportOptions def perform(project_id) - project = Project.find(project_id) + @project = Project.find(project_id) - return unless start_import(project) + return unless start_import Gitlab::Metrics.add_event(:import_repository) @@ -21,7 +21,7 @@ class RepositoryImportWorker return if service.async? if result[:status] == :error - fail_import(project, result[:message]) if project.gitlab_project_import? + fail_import(result[:message]) if template_import? raise result[:message] end @@ -31,14 +31,20 @@ class RepositoryImportWorker private - def start_import(project) + attr_reader :project + + def start_import return true if start(project) Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while importing.") false end - def fail_import(project, message) + def fail_import(message) project.mark_import_as_failed(message) end + + def template_import? + project.gitlab_project_import? + end end diff --git a/app/workers/todos_destroyer/confidential_issue_worker.rb b/app/workers/todos_destroyer/confidential_issue_worker.rb new file mode 100644 index 00000000000..9d640c14963 --- /dev/null +++ b/app/workers/todos_destroyer/confidential_issue_worker.rb @@ -0,0 +1,10 @@ +module TodosDestroyer + class ConfidentialIssueWorker + include ApplicationWorker + include TodosDestroyerQueue + + def perform(issue_id) + ::Todos::Destroy::ConfidentialIssueService.new(issue_id).execute + end + end +end diff --git a/app/workers/todos_destroyer/entity_leave_worker.rb b/app/workers/todos_destroyer/entity_leave_worker.rb new file mode 100644 index 00000000000..e62d9876f4a --- /dev/null +++ b/app/workers/todos_destroyer/entity_leave_worker.rb @@ -0,0 +1,10 @@ +module TodosDestroyer + class EntityLeaveWorker + include ApplicationWorker + include TodosDestroyerQueue + + def perform(user_id, entity_id, entity_type) + ::Todos::Destroy::EntityLeaveService.new(user_id, entity_id, entity_type).execute + end + end +end diff --git a/app/workers/todos_destroyer/private_features_worker.rb b/app/workers/todos_destroyer/private_features_worker.rb new file mode 100644 index 00000000000..f457d5e0471 --- /dev/null +++ b/app/workers/todos_destroyer/private_features_worker.rb @@ -0,0 +1,10 @@ +module TodosDestroyer + class PrivateFeaturesWorker + include ApplicationWorker + include TodosDestroyerQueue + + def perform(project_id, user_id = nil) + ::Todos::Destroy::PrivateFeaturesService.new(project_id, user_id).execute + end + end +end diff --git a/app/workers/todos_destroyer/project_private_worker.rb b/app/workers/todos_destroyer/project_private_worker.rb new file mode 100644 index 00000000000..7a853c36370 --- /dev/null +++ b/app/workers/todos_destroyer/project_private_worker.rb @@ -0,0 +1,10 @@ +module TodosDestroyer + class ProjectPrivateWorker + include ApplicationWorker + include TodosDestroyerQueue + + def perform(project_id) + ::Todos::Destroy::ProjectPrivateService.new(project_id).execute + end + end +end diff --git a/changelogs/unreleased/1756-set-iid-via-api.yml b/changelogs/unreleased/1756-set-iid-via-api.yml new file mode 100644 index 00000000000..680a9464ab4 --- /dev/null +++ b/changelogs/unreleased/1756-set-iid-via-api.yml @@ -0,0 +1,5 @@ +--- +title: Allow issues API to receive an internal ID (iid) on create +merge_request: 20626 +author: Jamie Schembri +type: fixed diff --git a/changelogs/unreleased/25990-interactive-web-terminals-authorization.yml b/changelogs/unreleased/25990-interactive-web-terminals-authorization.yml new file mode 100644 index 00000000000..0a2853c20c6 --- /dev/null +++ b/changelogs/unreleased/25990-interactive-web-terminals-authorization.yml @@ -0,0 +1,5 @@ +--- +title: Fix authorization for interactive web terminals +merge_request: 20811 +author: +type: fixed diff --git a/changelogs/unreleased/34572-ssh-certificates.yml b/changelogs/unreleased/34572-ssh-certificates.yml new file mode 100644 index 00000000000..76a08a188de --- /dev/null +++ b/changelogs/unreleased/34572-ssh-certificates.yml @@ -0,0 +1,5 @@ +--- +title: Add support for SSH certificate authentication +merge_request: 19911 +author: Ævar Arnfjörð Bjarmason +type: added diff --git a/changelogs/unreleased/48098-mutual-auth-cluster-applications.yml b/changelogs/unreleased/48098-mutual-auth-cluster-applications.yml deleted file mode 100644 index 36e39cf31db..00000000000 --- a/changelogs/unreleased/48098-mutual-auth-cluster-applications.yml +++ /dev/null @@ -1,6 +0,0 @@ ---- -title: Ensure installed Helm Tiller For GitLab Managed Apps Is protected by mutual - auth -merge_request: 20801 -author: -type: changed diff --git a/changelogs/unreleased/49161-disable-toggle-comments.yml b/changelogs/unreleased/49161-disable-toggle-comments.yml new file mode 100644 index 00000000000..5ec16191f07 --- /dev/null +++ b/changelogs/unreleased/49161-disable-toggle-comments.yml @@ -0,0 +1,5 @@ +--- +title: Disables toggle comments button if diff has no discussions +merge_request: +author: +type: other diff --git a/changelogs/unreleased/dz-labels-search.yml b/changelogs/unreleased/dz-labels-search.yml new file mode 100644 index 00000000000..49c1b6c1a86 --- /dev/null +++ b/changelogs/unreleased/dz-labels-search.yml @@ -0,0 +1,5 @@ +--- +title: Search for labels by title or description on project labels page +merge_request: 20749 +author: +type: added diff --git a/changelogs/unreleased/feature-gb-login-activity-metrics.yml b/changelogs/unreleased/feature-gb-login-activity-metrics.yml new file mode 100644 index 00000000000..5d687b984eb --- /dev/null +++ b/changelogs/unreleased/feature-gb-login-activity-metrics.yml @@ -0,0 +1,5 @@ +--- +title: Add more comprehensive metrics tracking authentication activity +merge_request: 20668 +author: +type: added diff --git a/changelogs/unreleased/fix-multiple-scopes.yml b/changelogs/unreleased/fix-multiple-scopes.yml new file mode 100644 index 00000000000..24e5172d9a1 --- /dev/null +++ b/changelogs/unreleased/fix-multiple-scopes.yml @@ -0,0 +1,5 @@ +--- +title: Support multiple scopes when authing container registry scopes +merge_request: 20617 +author: +type: fixed diff --git a/changelogs/unreleased/fix-storage-size-for-artifacts-change.yml b/changelogs/unreleased/fix-storage-size-for-artifacts-change.yml new file mode 100644 index 00000000000..6a3e1420726 --- /dev/null +++ b/changelogs/unreleased/fix-storage-size-for-artifacts-change.yml @@ -0,0 +1,5 @@ +--- +title: Update total storage size when changing size of artifacts +merge_request: 20697 +author: Peter Marko +type: fixed diff --git a/changelogs/unreleased/floating-avarage-commit-numbers.yml b/changelogs/unreleased/floating-avarage-commit-numbers.yml new file mode 100644 index 00000000000..7f91ab16af4 --- /dev/null +++ b/changelogs/unreleased/floating-avarage-commit-numbers.yml @@ -0,0 +1,5 @@ +--- +title: Show one digit after dot in commit_per_day value in charts page. +merge_request: +author: msdundar +type: changed diff --git a/changelogs/unreleased/jprovazn-resource-events.yml b/changelogs/unreleased/jprovazn-resource-events.yml new file mode 100644 index 00000000000..05643150f16 --- /dev/null +++ b/changelogs/unreleased/jprovazn-resource-events.yml @@ -0,0 +1,5 @@ +--- +title: Add new model for tracking label events. +merge_request: +author: +type: added diff --git a/changelogs/unreleased/mk-add-local-project-uploads-cleanup-task.yml b/changelogs/unreleased/mk-add-local-project-uploads-cleanup-task.yml new file mode 100644 index 00000000000..9d38b353a41 --- /dev/null +++ b/changelogs/unreleased/mk-add-local-project-uploads-cleanup-task.yml @@ -0,0 +1,5 @@ +--- +title: Add local project uploads cleanup task +merge_request: 20863 +author: +type: added diff --git a/changelogs/unreleased/rails5-fix-flaky-spec-user-uses-shortcuts.yml b/changelogs/unreleased/rails5-fix-flaky-spec-user-uses-shortcuts.yml new file mode 100644 index 00000000000..5f2504c604d --- /dev/null +++ b/changelogs/unreleased/rails5-fix-flaky-spec-user-uses-shortcuts.yml @@ -0,0 +1,5 @@ +--- +title: 'Rails5: fix flaky spec' +merge_request: 20953 +author: Jasper Maes +type: fixed diff --git a/changelogs/unreleased/runner-features.yml b/changelogs/unreleased/runner-features.yml new file mode 100644 index 00000000000..c5e0fff5a18 --- /dev/null +++ b/changelogs/unreleased/runner-features.yml @@ -0,0 +1,5 @@ +--- +title: Verify runner feature set +merge_request: 20664 +author: +type: added diff --git a/changelogs/unreleased/sh-lfs-fix-content-type.yml b/changelogs/unreleased/sh-lfs-fix-content-type.yml new file mode 100644 index 00000000000..a839be9b3ae --- /dev/null +++ b/changelogs/unreleased/sh-lfs-fix-content-type.yml @@ -0,0 +1,5 @@ +--- +title: Fix LFS uploads not working with git-lfs 2.5.0 +merge_request: 20923 +author: +type: fixed diff --git a/changelogs/unreleased/todos-visibility-change.yml b/changelogs/unreleased/todos-visibility-change.yml new file mode 100644 index 00000000000..b7632b94771 --- /dev/null +++ b/changelogs/unreleased/todos-visibility-change.yml @@ -0,0 +1,5 @@ +--- +title: Delete todos when user loses access to read the target +merge_request: 20665 +author: +type: other diff --git a/changelogs/unreleased/zj-remove-git-rake-tasks.yml b/changelogs/unreleased/zj-remove-git-rake-tasks.yml new file mode 100644 index 00000000000..8c90fc7d0fe --- /dev/null +++ b/changelogs/unreleased/zj-remove-git-rake-tasks.yml @@ -0,0 +1,5 @@ +--- +title: Remove gitlab:user:check_repos, gitlab:check_repo, gitlab:git:prune, gitlab:git:gc, and gitlab:git:repack +merge_request: 20806 +author: +type: removed diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb index 8cc36820d3c..d64b659c6d7 100644 --- a/config/initializers/warden.rb +++ b/config/initializers/warden.rb @@ -1,10 +1,20 @@ Rails.application.configure do |config| Warden::Manager.after_set_user(scope: :user) do |user, auth, opts| Gitlab::Auth::UniqueIpsLimiter.limit_user!(user) - end - Warden::Manager.before_failure(scope: :user) do |env, opts| - Gitlab::Auth::BlockedUserTracker.log_if_user_blocked(env) + activity = Gitlab::Auth::Activity.new(user, opts) + + case opts[:event] + when :authentication + activity.user_authenticated! + when :set_user + activity.user_authenticated! + activity.user_session_override! + when :fetch # rubocop:disable Lint/EmptyWhen + # We ignore session fetch events + else + activity.user_session_override! + end end Warden::Manager.after_authentication(scope: :user) do |user, auth, opts| @@ -15,7 +25,17 @@ Rails.application.configure do |config| ActiveSession.set(user, auth.request) end - Warden::Manager.before_logout(scope: :user) do |user, auth, opts| - ActiveSession.destroy(user || auth.user, auth.request.session.id) + Warden::Manager.before_failure(scope: :user) do |env, opts| + tracker = Gitlab::Auth::BlockedUserTracker.new(env) + tracker.log_blocked_user_activity! if tracker.user_blocked? + + Gitlab::Auth::Activity.new(tracker.user, opts).user_authentication_failed! + end + + Warden::Manager.before_logout(scope: :user) do |user_warden, auth, opts| + user = user_warden || auth.user + + ActiveSession.destroy(user, auth.request.session.id) + Gitlab::Auth::Activity.new(user, opts).user_session_destroyed! end end diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 70b584ff9e9..3c85cd07d46 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -45,6 +45,7 @@ - [github_import_advance_stage, 1] - [project_service, 1] - [delete_user, 1] + - [todos_destroyer, 1] - [delete_merged_branches, 1] - [authorized_projects, 1] - [expire_build_instance_artifacts, 1] diff --git a/db/migrate/20180612103626_add_columns_for_helm_tiller_certificates.rb b/db/migrate/20180612103626_add_columns_for_helm_tiller_certificates.rb deleted file mode 100644 index d9f15b6b67d..00000000000 --- a/db/migrate/20180612103626_add_columns_for_helm_tiller_certificates.rb +++ /dev/null @@ -1,11 +0,0 @@ -class AddColumnsForHelmTillerCertificates < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - def change - add_column :clusters_applications_helm, :encrypted_ca_key, :text - add_column :clusters_applications_helm, :encrypted_ca_key_iv, :text - add_column :clusters_applications_helm, :ca_cert, :text - end -end diff --git a/db/migrate/20180726172057_create_resource_label_events.rb b/db/migrate/20180726172057_create_resource_label_events.rb new file mode 100644 index 00000000000..2ef7078d898 --- /dev/null +++ b/db/migrate/20180726172057_create_resource_label_events.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CreateResourceLabelEvents < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :resource_label_events, id: :bigserial do |t| + t.integer :action, null: false + t.references :issue, null: true, index: true, foreign_key: { on_delete: :cascade } + t.references :merge_request, null: true, index: true, foreign_key: { on_delete: :cascade } + t.references :label, index: true, foreign_key: { on_delete: :nullify } + t.references :user, index: true, foreign_key: { on_delete: :nullify } + t.datetime_with_timezone :created_at, null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 0638de8c6ff..97e7e28df09 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180722103201) do +ActiveRecord::Schema.define(version: 20180726172057) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -636,9 +636,6 @@ ActiveRecord::Schema.define(version: 20180722103201) do t.integer "status", null: false t.string "version", null: false t.text "status_reason" - t.text "encrypted_ca_key" - t.text "encrypted_ca_key_iv" - t.text "ca_cert" end create_table "clusters_applications_ingress", force: :cascade do |t| @@ -1790,6 +1787,20 @@ ActiveRecord::Schema.define(version: 20180722103201) do add_index "remote_mirrors", ["last_successful_update_at"], name: "index_remote_mirrors_on_last_successful_update_at", using: :btree add_index "remote_mirrors", ["project_id"], name: "index_remote_mirrors_on_project_id", using: :btree + create_table "resource_label_events", id: :bigserial, force: :cascade do |t| + t.integer "action", null: false + t.integer "issue_id" + t.integer "merge_request_id" + t.integer "label_id" + t.integer "user_id" + t.datetime_with_timezone "created_at", null: false + end + + add_index "resource_label_events", ["issue_id"], name: "index_resource_label_events_on_issue_id", using: :btree + add_index "resource_label_events", ["label_id"], name: "index_resource_label_events_on_label_id", using: :btree + add_index "resource_label_events", ["merge_request_id"], name: "index_resource_label_events_on_merge_request_id", using: :btree + add_index "resource_label_events", ["user_id"], name: "index_resource_label_events_on_user_id", using: :btree + create_table "routes", force: :cascade do |t| t.integer "source_id", null: false t.string "source_type", null: false @@ -2340,6 +2351,10 @@ ActiveRecord::Schema.define(version: 20180722103201) do add_foreign_key "push_event_payloads", "events", name: "fk_36c74129da", on_delete: :cascade add_foreign_key "releases", "projects", name: "fk_47fe2a0596", on_delete: :cascade add_foreign_key "remote_mirrors", "projects", on_delete: :cascade + add_foreign_key "resource_label_events", "issues", on_delete: :cascade + add_foreign_key "resource_label_events", "labels", on_delete: :nullify + add_foreign_key "resource_label_events", "merge_requests", on_delete: :cascade + add_foreign_key "resource_label_events", "users", on_delete: :nullify add_foreign_key "services", "projects", name: "fk_71cce407f9", on_delete: :cascade add_foreign_key "snippets", "projects", name: "fk_be41fd4bb7", on_delete: :cascade add_foreign_key "subscriptions", "projects", on_delete: :cascade diff --git a/doc/administration/img/raketasks/check_repos_output.png b/doc/administration/img/raketasks/check_repos_output.png Binary files differdeleted file mode 100644 index 7fda2ba0c0f..00000000000 --- a/doc/administration/img/raketasks/check_repos_output.png +++ /dev/null diff --git a/doc/administration/index.md b/doc/administration/index.md index 88190b2df5f..112d14652af 100644 --- a/doc/administration/index.md +++ b/doc/administration/index.md @@ -106,6 +106,7 @@ created in snippets, wikis, and repos. - [Gitaly](gitaly/index.md): Configuring Gitaly, GitLab's Git repository storage service. - [Default labels](../user/admin_area/labels.html): Create labels that will be automatically added to every new project. - [Restrict the use of public or internal projects](../public_access/public_access.md#restricting-the-use-of-public-or-internal-projects): Restrict the use of visibility levels for users when they create a project or a snippet. +- [Custom project templates](https://docs.gitlab.com/ee/user/admin_area/custom_project_templates.html): Configure a set of projects to be used as custom templates when creating a new project. **[PREMIUM ONLY]** ### Repository settings diff --git a/doc/administration/operations/fast_ssh_key_lookup.md b/doc/administration/operations/fast_ssh_key_lookup.md index 89331238ce4..752a2774bd7 100644 --- a/doc/administration/operations/fast_ssh_key_lookup.md +++ b/doc/administration/operations/fast_ssh_key_lookup.md @@ -1,3 +1,10 @@ +# Consider using SSH certificates instead of, or in addition to this + +This document describes a drop-in replacement for the +`authorized_keys` file for normal (non-deploy key) users. Consider +using [ssh certificates](ssh_certificates.md), they are even faster, +but are not is not a drop-in replacement. + # Fast lookup of authorized SSH keys in the database > [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/1631) in diff --git a/doc/administration/operations/index.md b/doc/administration/operations/index.md index 5655b7efec6..e9cad99c4b0 100644 --- a/doc/administration/operations/index.md +++ b/doc/administration/operations/index.md @@ -14,4 +14,7 @@ that to prioritize important jobs. - [Sidekiq MemoryKiller](sidekiq_memory_killer.md): Configure Sidekiq MemoryKiller to restart Sidekiq. - [Unicorn](unicorn.md): Understand Unicorn and unicorn-worker-killer. -- [Speed up SSH operations](fast_ssh_key_lookup.md): Authorize SSH users via a fast, indexed lookup to the GitLab database. +- Speed up SSH operations by [Authorizing SSH users via a fast, +indexed lookup to the GitLab database](fast_ssh_key_lookup.md), and/or +by [doing away with user SSH keys stored on GitLab entirely in favor +of SSH certificates](ssh_certificates.md). diff --git a/doc/administration/operations/ssh_certificates.md b/doc/administration/operations/ssh_certificates.md new file mode 100644 index 00000000000..8968afba01b --- /dev/null +++ b/doc/administration/operations/ssh_certificates.md @@ -0,0 +1,165 @@ +# User lookup via OpenSSH's AuthorizedPrincipalsCommand + +> [Available in](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/19911) GitLab +> Community Edition 11.2. + +GitLab's default SSH authentication requires users to upload their ssh +public keys before they can use the SSH transport. + +In centralized (e.g. corporate) environments this can be a hassle +operationally, particularly if the SSH keys are temporary keys issued +to the user, e.g. ones that expire 24 hours after issuing. + +In such setups some external automated process is needed to constantly +upload the new keys to GitLab. + +> **Warning:** OpenSSH version 6.9+ is required because that version +introduced the `AuthorizedPrincipalsCommand` configuration option. If +using CentOS 6, you can [follow these +instructions](fast_ssh_key_lookup.html#compiling-a-custom-version-of-openssh-for-centos-6) +to compile an up-to-date version. + +## Why use OpenSSH certificates? + +By using OpenSSH certificates all the information about what user on +GitLab owns the key is encoded in the key itself, and OpenSSH itself +guarantees that users can't fake this, since they'd need to have +access to the private CA signing key. + +When correctly set up, this does away with the requirement of +uploading user SSH keys to GitLab entirely. + +## Setting up SSH certificate lookup via GitLab Shell + +How to fully setup SSH certificates is outside the scope of this +document. See [OpenSSH's +PROTOCOL.certkeys](https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?annotate=HEAD) +for how it works, and e.g. [RedHat's documentation about +it](https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/deployment_guide/sec-using_openssh_certificate_authentication). + +We assume that you already have SSH certificates set up, and have +added the `TrustedUserCAKeys` of your CA to your `sshd_config`, e.g.: + +``` +TrustedUserCAKeys /etc/security/mycompany_user_ca.pub +``` + +Usually `TrustedUserCAKeys` would not be scoped under a `Match User +git` in such a setup, since it would also be used for system logins to +the GitLab server itself, but your setup may vary. If the CA is only +used for GitLab consider putting this in the `Match User git` section +(described below). + +The SSH certificates being issued by that CA **MUST** have a "key id" +corresponding to that user's username on GitLab, e.g. (some output +omitted for brevity): + +``` +$ ssh-add -L | grep cert | ssh-keygen -L -f - +(stdin):1: + Type: ssh-rsa-cert-v01@openssh.com user certificate + Public key: RSA-CERT SHA256:[...] + Signing CA: RSA SHA256:[...] + Key ID: "aearnfjord" + Serial: 8289829611021396489 + Valid: from 2018-07-18T09:49:00 to 2018-07-19T09:50:34 + Principals: + sshUsers + [...] + [...] +``` + +Technically that's not strictly true, e.g. it could be +`prod-aearnfjord` if it's a SSH certificate you'd normally log in to +servers as the `prod-aearnfjord` user, but then you must specify your +own `AuthorizedPrincipalsCommand` to do that mapping instead of using +our provided default. + +The important part is that the `AuthorizedPrincipalsCommand` must be +able to map from the "key id" to a GitLab username in some way, the +default command we ship assumes there's a 1=1 mapping between the two, +since the whole point of this is to allow us to extract a GitLab +username from the key itself, instead of relying on something like the +default public key to username mapping. + +Then, in your `sshd_config` set up `AuthorizedPrincipalsCommand` for +the `git` user. Hopefully you can use the default one shipped with +GitLab: + +``` +Match User git + AuthorizedPrincipalsCommandUser root + AuthorizedPrincipalsCommand /opt/gitlab/embedded/service/gitlab-shell/bin/gitlab-shell-authorized-principals-check %i sshUsers +``` + +This command will emit output that looks something like: + +``` +command="/opt/gitlab/embedded/service/gitlab-shell/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL} +``` + +Where `{KEY_ID}` is the `%i` argument passed to the script +(e.g. `aeanfjord`), and `{PRINCIPAL}` is the principal passed to it +(e.g. `sshUsers`). + +You will need to customize the `sshUsers` part of that. It should be +some principal that's guaranteed to be part of the key for all users +who can log in to GitLab, or you must provide a list of principals, +one of which is going to be present for the user, e.g.: + +``` + [...] + AuthorizedPrincipalsCommand /opt/gitlab/embedded/service/gitlab-shell/bin/gitlab-shell-authorized-principals-check %i sshUsers windowsUsers +``` + +## Principals and security + +You can supply as many principals as you want, these will be turned +into multiple lines of `authorized_keys` output, as described in the +`AuthorizedPrincipalsFile` documentation in `sshd_config(5)`. + +Normally when using the `AuthorizedKeysCommand` with OpenSSH the +principal is some "group" that's allowed to log into that +server. However with GitLab it's only used to appease OpenSSH's +requirement for it, we effectively only care about the "key id" being +correct. Once that's extracted GitLab will enforce its own ACLs for +that user (e.g. what projects the user can access). + +So it's OK to e.g. be overly generous in what you accept, since if the +user e.g. has no access to GitLab at all it'll just error out with a +message about this being an invalid user. + +## Interaction with the `authorized_keys` file + +SSH certificates can be used in conjunction with the `authorized_keys` +file, and if setup as configured above the `authorized_keys` file will +still serve as a fallback. + +This is because if the `AuthorizedPrincipalsCommand` can't +authenticate the user, OpenSSH will fall back on +`~/.ssh/authorized_keys` (or the `AuthorizedKeysCommand`). + +Therefore there may still be a reason to use the ["Fast lookup of +authorized SSH keys in the database"](fast_ssh_key_lookup.html) method +in conjunction with this. Since you'll be using SSH certificates for +all your normal users, and relying on the `~/.ssh/authorized_keys` +fallback for deploy keys, if you make use of those. + +But you may find that there's no reason to do that, since all your +normal users will use the fast `AuthorizedPrincipalsCommand` path, and +only automated deployment key access will fall back on +`~/.ssh/authorized_keys`, or that you have a lot more keys for normal +users (especially if they're renewed) than you have deploy keys. + +## Other security caveats + +Users can still bypass SSH certificate authentication by manually +uploading an SSH public key to their profile, relying on the +`~/.ssh/authorized_keys` fallback to authenticate it. There's +currently no feature to prevent this, [but there's an open request for +adding it](https://gitlab.com/gitlab-org/gitlab-ce/issues/49218). + +Such a restriction can currently be hacked in by e.g. providing a +custom `AuthorizedKeysCommand` which checks if the discovered key-ID +returned from `gitlab-shell-authorized-keys-check` is a deploy key or +not (all non-deploy keys should be refused). diff --git a/doc/administration/raketasks/check.md b/doc/administration/raketasks/check.md index 0c145830f02..0ca1d77f1d0 100644 --- a/doc/administration/raketasks/check.md +++ b/doc/administration/raketasks/check.md @@ -1,4 +1,4 @@ -# Check Rake Tasks +# Integrity Check Rake Task ## Repository Integrity @@ -28,14 +28,8 @@ exactly which repositories are causing the trouble. ### Check all GitLab repositories ->**Note:** -> -> - `gitlab:repo:check` has been deprecated in favor of `gitlab:git:fsck` -> - [Deprecated][ce-15931] in GitLab 10.4. -> - `gitlab:repo:check` will be removed in the future. [Removal issue][ce-41699] - This task loops through all repositories on the GitLab server and runs the -3 integrity checks described previously. +integrity check described previously. **Omnibus Installation** @@ -49,33 +43,6 @@ sudo gitlab-rake gitlab:git:fsck sudo -u git -H bundle exec rake gitlab:git:fsck RAILS_ENV=production ``` -### Check repositories for a specific user - -This task checks all repositories that a specific user has access to. This is important -because sometimes you know which user is experiencing trouble but you don't know -which project might be the cause. - -If the rake task is executed without brackets at the end, you will be prompted -to enter a username. - -**Omnibus Installation** - -```bash -sudo gitlab-rake gitlab:user:check_repos -sudo gitlab-rake gitlab:user:check_repos[<username>] -``` - -**Source Installation** - -```bash -sudo -u git -H bundle exec rake gitlab:user:check_repos RAILS_ENV=production -sudo -u git -H bundle exec rake gitlab:user:check_repos[<username>] RAILS_ENV=production -``` - -Example output: - - - ## Uploaded Files Integrity Various types of files can be uploaded to a GitLab installation by users. @@ -167,5 +134,4 @@ The LDAP check Rake task will test the bind_dn and password credentials executed as part of the `gitlab:check` task, but can run independently. See [LDAP Rake Tasks - LDAP Check](ldap.md#check) for details. -[ce-15931]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/15931 -[ce-41699]: https://gitlab.com/gitlab-org/gitlab-ce/issues/41699 +[git-fsck]: https://git-scm.com/docs/git-fsck diff --git a/doc/api/issues.md b/doc/api/issues.md index 92fb3e9c307..103eaa5655f 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -463,6 +463,7 @@ POST /projects/:id/issues | Attribute | Type | Required | Description | |-------------------------------------------|----------------|----------|--------------| | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `iid` | integer/string | no | The internal ID of the project's issue (requires admin or project owner rights) | | `title` | string | yes | The title of an issue | | `description` | string | no | The description of an issue | | `confidential` | boolean | no | Set an issue to be confidential. Default is `false`. | diff --git a/doc/ci/examples/test-and-deploy-python-application-to-heroku.md b/doc/ci/examples/test-and-deploy-python-application-to-heroku.md index a433cd5a5dd..087b317ab73 100644 --- a/doc/ci/examples/test-and-deploy-python-application-to-heroku.md +++ b/doc/ci/examples/test-and-deploy-python-application-to-heroku.md @@ -46,7 +46,7 @@ This project has three jobs: ## Store API keys -You'll need to create two variables in `Project > Variables`: +You'll need to create two variables in `Settings > CI/CD > Variables` on your GitLab project settings: 1. `HEROKU_STAGING_API_KEY` - Heroku API key used to deploy staging app, 2. `HEROKU_PRODUCTION_API_KEY` - Heroku API key used to deploy production app. diff --git a/doc/gitlab-basics/create-project.md b/doc/gitlab-basics/create-project.md index dd8d95a3bca..2517908e5b1 100644 --- a/doc/gitlab-basics/create-project.md +++ b/doc/gitlab-basics/create-project.md @@ -43,7 +43,7 @@ When you create a new repo locally, instead of going to GitLab to manually create a new project and then push the repo, you can directly push it to GitLab to create the new project, all without leaving your terminal. If you have access to that -namespace, we will automatically create a new project under that GitLab namespace with its +namespace, we will automatically create a new project under that GitLab namespace with its visibility set to Private by default (you can later change it in the [project's settings](../public_access/public_access.md#how-to-change-project-visibility)). This can be done by using either SSH or HTTP: diff --git a/doc/install/installation.md b/doc/install/installation.md index 8c7f80fd8e8..ea01d88d85f 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -12,7 +12,7 @@ Since installations from source don't have Runit, Sidekiq can't be terminated an ## Select Version to Install -Make sure you view [this installation guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/install/installation.md) from the branch (version) of GitLab you would like to install (e.g., `11-1-stable`). +Make sure you view [this installation guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/install/installation.md) from the branch (version) of GitLab you would like to install (e.g., `11-2-stable`). You can select the branch in the version dropdown in the top left corner of GitLab (below the menu bar). If the highest number stable branch is unclear please check the [GitLab Blog](https://about.gitlab.com/blog/) for installation guide links by version. @@ -300,9 +300,9 @@ sudo usermod -aG redis git ### Clone the Source # Clone GitLab repository - sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 11-1-stable gitlab + sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 11-2-stable gitlab -**Note:** You can change `11-1-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server! +**Note:** You can change `11-2-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server! ### Configure It diff --git a/doc/raketasks/cleanup.md b/doc/raketasks/cleanup.md index cf891cd90ad..e2eb342361a 100644 --- a/doc/raketasks/cleanup.md +++ b/doc/raketasks/cleanup.md @@ -22,3 +22,34 @@ sudo gitlab-rake gitlab:cleanup:repos # installation from source bundle exec rake gitlab:cleanup:repos RAILS_ENV=production ``` + +Clean up local project upload files if they don't exist in GitLab database. The +task attempts to fix the file if it can find its project, otherwise it moves the +file to a lost and found directory. + +``` +# omnibus-gitlab +sudo gitlab-rake gitlab:cleanup:project_uploads + +# installation from source +bundle exec rake gitlab:cleanup:project_uploads RAILS_ENV=production +``` + +Example output: + +``` +$ sudo gitlab-rake gitlab:cleanup:project_uploads +I, [2018-07-27T12:08:27.671559 #89817] INFO -- : Looking for orphaned project uploads to clean up. Dry run... +D, [2018-07-27T12:08:28.293568 #89817] DEBUG -- : Processing batch of 500 project upload file paths, starting with /opt/gitlab/embedded/service/gitlab-rails/public/uploads/test.out +I, [2018-07-27T12:08:28.689869 #89817] INFO -- : Can move to lost and found /opt/gitlab/embedded/service/gitlab-rails/public/uploads/test.out -> /opt/gitlab/embedded/service/gitlab-rails/public/uploads/-/project-lost-found/test.out +I, [2018-07-27T12:08:28.755624 #89817] INFO -- : Can fix /opt/gitlab/embedded/service/gitlab-rails/public/uploads/foo/bar/89a0f7b0b97008a4a18cedccfdcd93fb/foo.txt -> /opt/gitlab/embedded/service/gitlab-rails/public/uploads/qux/foo/bar/89a0f7b0b97008a4a18cedccfdcd93fb/foo.txt +I, [2018-07-27T12:08:28.760257 #89817] INFO -- : Can move to lost and found /opt/gitlab/embedded/service/gitlab-rails/public/uploads/foo/bar/1dd6f0f7eefd2acc4c2233f89a0f7b0b/image.png -> /opt/gitlab/embedded/service/gitlab-rails/public/uploads/-/project-lost-found/foo/bar/1dd6f0f7eefd2acc4c2233f89a0f7b0b/image.png +I, [2018-07-27T12:08:28.764470 #89817] INFO -- : To cleanup these files run this command with DRY_RUN=false + +$ sudo gitlab-rake gitlab:cleanup:project_uploads DRY_RUN=false +I, [2018-07-27T12:08:32.944414 #89936] INFO -- : Looking for orphaned project uploads to clean up... +D, [2018-07-27T12:08:33.293568 #89817] DEBUG -- : Processing batch of 500 project upload file paths, starting with /opt/gitlab/embedded/service/gitlab-rails/public/uploads/test.out +I, [2018-07-27T12:08:33.689869 #89817] INFO -- : Did move to lost and found /opt/gitlab/embedded/service/gitlab-rails/public/uploads/test.out -> /opt/gitlab/embedded/service/gitlab-rails/public/uploads/-/project-lost-found/test.out +I, [2018-07-27T12:08:33.755624 #89817] INFO -- : Did fix /opt/gitlab/embedded/service/gitlab-rails/public/uploads/foo/bar/89a0f7b0b97008a4a18cedccfdcd93fb/foo.txt -> /opt/gitlab/embedded/service/gitlab-rails/public/uploads/qux/foo/bar/89a0f7b0b97008a4a18cedccfdcd93fb/foo.txt +I, [2018-07-27T12:08:33.760257 #89817] INFO -- : Did move to lost and found /opt/gitlab/embedded/service/gitlab-rails/public/uploads/foo/bar/1dd6f0f7eefd2acc4c2233f89a0f7b0b/image.png -> /opt/gitlab/embedded/service/gitlab-rails/public/uploads/-/project-lost-found/foo/bar/1dd6f0f7eefd2acc4c2233f89a0f7b0b/image.png +```
\ No newline at end of file diff --git a/doc/update/11.1-to-11.2.md b/doc/update/11.1-to-11.2.md new file mode 100644 index 00000000000..3edc7e6923e --- /dev/null +++ b/doc/update/11.1-to-11.2.md @@ -0,0 +1,378 @@ +--- +comments: false +--- + +# From 11.1 to 11.2 + +Make sure you view this update guide from the branch (version) of GitLab you would +like to install (e.g., `11-2-stable`. You can select the branch in the version +dropdown at the top left corner of GitLab (below the menu bar). + +If the highest number stable branch is unclear please check the +[GitLab Blog](https://about.gitlab.com/blog/archives.html) for installation +guide links by version. + +### 1. Stop server + +```bash +sudo service gitlab stop +``` + +### 2. Backup + +NOTE: If you installed GitLab from source, make sure `rsync` is installed. + +```bash +cd /home/git/gitlab + +sudo -u git -H bundle exec rake gitlab:backup:create RAILS_ENV=production +``` + +### 3. Update Ruby + +NOTE: GitLab 11.0 and higher only support Ruby 2.4.x and dropped support for Ruby 2.3.x. Be +sure to upgrade your interpreter if necessary. + +You can check which version you are running with `ruby -v`. + +Download Ruby and compile it: + +```bash +mkdir /tmp/ruby && cd /tmp/ruby +curl --remote-name --progress https://cache.ruby-lang.org/pub/ruby/2.4/ruby-2.4.4.tar.gz +echo 'ec82b0d53bd0adad9b19e6b45e44d54e9ec3f10c ruby-2.4.4.tar.gz' | shasum -c - && tar xzf ruby-2.4.4.tar.gz +cd ruby-2.4.4 + +./configure --disable-install-rdoc +make +sudo make install +``` + +Install Bundler: + +```bash +sudo gem install bundler --no-ri --no-rdoc +``` + +### 4. Update Node + +GitLab utilizes [webpack](http://webpack.js.org) to compile frontend assets. +This requires a minimum version of node v6.0.0. + +You can check which version you are running with `node -v`. If you are running +a version older than `v6.0.0` you will need to update to a newer version. You +can find instructions to install from community maintained packages or compile +from source at the nodejs.org website. + +<https://nodejs.org/en/download/> + +GitLab also requires the use of yarn `>= v1.2.0` to manage JavaScript +dependencies. + +```bash +curl --silent --show-error https://dl.yarnpkg.com/debian/pubkey.gpg | sudo apt-key add - +echo "deb https://dl.yarnpkg.com/debian/ stable main" | sudo tee /etc/apt/sources.list.d/yarn.list +sudo apt-get update +sudo apt-get install yarn +``` + +More information can be found on the [yarn website](https://yarnpkg.com/en/docs/install). + +### 5. Update Go + +NOTE: GitLab 11.0 and higher only supports Go 1.9.x and newer, and dropped support for Go +1.5.x through 1.8.x. Be sure to upgrade your installation if necessary. + +You can check which version you are running with `go version`. + +Download and install Go: + +```bash +# Remove former Go installation folder +sudo rm -rf /usr/local/go + +curl --remote-name --progress https://dl.google.com/go/go1.10.3.linux-amd64.tar.gz +echo 'fa1b0e45d3b647c252f51f5e1204aba049cde4af177ef9f2181f43004f901035 go1.10.3.linux-amd64.tar.gz' | shasum -a256 -c - && \ + sudo tar -C /usr/local -xzf go1.10.3.linux-amd64.tar.gz +sudo ln -sf /usr/local/go/bin/{go,godoc,gofmt} /usr/local/bin/ +rm go1.10.3.linux-amd64.tar.gz +``` + +### 6. Get latest code + +```bash +cd /home/git/gitlab + +sudo -u git -H git fetch --all --prune +sudo -u git -H git checkout -- db/schema.rb # local changes will be restored automatically +sudo -u git -H git checkout -- locale +``` + +For GitLab Community Edition: + +```bash +cd /home/git/gitlab + +sudo -u git -H git checkout 11-2-stable +``` + +OR + +For GitLab Enterprise Edition: + +```bash +cd /home/git/gitlab + +sudo -u git -H git checkout 11-2-stable-ee +``` + +### 7. Update gitlab-shell + +```bash +cd /home/git/gitlab-shell + +sudo -u git -H git fetch --all --tags --prune +sudo -u git -H git checkout v$(</home/git/gitlab/GITLAB_SHELL_VERSION) +sudo -u git -H bin/compile +``` + +### 8. Update gitlab-workhorse + +Install and compile gitlab-workhorse. GitLab-Workhorse uses +[GNU Make](https://www.gnu.org/software/make/). +If you are not using Linux you may have to run `gmake` instead of +`make` below. + +```bash +cd /home/git/gitlab-workhorse + +sudo -u git -H git fetch --all --tags --prune +sudo -u git -H git checkout v$(</home/git/gitlab/GITLAB_WORKHORSE_VERSION) +sudo -u git -H make +``` + +### 9. Update Gitaly + +#### New Gitaly configuration options required + +In order to function Gitaly needs some additional configuration information. Below we assume you installed Gitaly in `/home/git/gitaly` and GitLab Shell in `/home/git/gitlab-shell`. + +```shell +echo ' +[gitaly-ruby] +dir = "/home/git/gitaly/ruby" + +[gitlab-shell] +dir = "/home/git/gitlab-shell" +' | sudo -u git tee -a /home/git/gitaly/config.toml +``` + +#### Check Gitaly configuration + +Due to a bug in the `rake gitlab:gitaly:install` script your Gitaly +configuration file may contain syntax errors. The block name +`[[storages]]`, which may occur more than once in your `config.toml` +file, should be `[[storage]]` instead. + +```shell +sudo -u git -H sed -i.pre-10.1 's/\[\[storages\]\]/[[storage]]/' /home/git/gitaly/config.toml +``` + +#### Compile Gitaly + +```shell +cd /home/git/gitaly +sudo -u git -H git fetch --all --tags --prune +sudo -u git -H git checkout v$(</home/git/gitlab/GITALY_SERVER_VERSION) +sudo -u git -H make +``` + +### 10. Update gitlab-pages + +#### Only needed if you use GitLab Pages. + +Install and compile gitlab-pages. GitLab-Pages uses +[GNU Make](https://www.gnu.org/software/make/). +If you are not using Linux you may have to run `gmake` instead of +`make` below. + +```bash +cd /home/git/gitlab-pages + +sudo -u git -H git fetch --all --tags --prune +sudo -u git -H git checkout v$(</home/git/gitlab/GITLAB_PAGES_VERSION) +sudo -u git -H make +``` + +### 11. Update MySQL permissions + +If you are using MySQL you need to grant the GitLab user the necessary +permissions on the database: + +```bash +mysql -u root -p -e "GRANT TRIGGER ON \`gitlabhq_production\`.* TO 'git'@'localhost';" +``` + +If you use MySQL with replication, or just have MySQL configured with binary logging, +you will need to also run the following on all of your MySQL servers: + +```bash +mysql -u root -p -e "SET GLOBAL log_bin_trust_function_creators = 1;" +``` + +You can make this setting permanent by adding it to your `my.cnf`: + +``` +log_bin_trust_function_creators=1 +``` + +### 12. Update configuration files + +#### New configuration options for `gitlab.yml` + +There might be configuration options available for [`gitlab.yml`][yaml]. View them with the command below and apply them manually to your current `gitlab.yml`: + +```sh +cd /home/git/gitlab + +git diff origin/11-1-stable:config/gitlab.yml.example origin/11-2-stable:config/gitlab.yml.example +``` + +#### Nginx configuration + +Ensure you're still up-to-date with the latest NGINX configuration changes: + +```sh +cd /home/git/gitlab + +# For HTTPS configurations +git diff origin/11-1-stable:lib/support/nginx/gitlab-ssl origin/11-2-stable:lib/support/nginx/gitlab-ssl + +# For HTTP configurations +git diff origin/11-1-stable:lib/support/nginx/gitlab origin/11-2-stable:lib/support/nginx/gitlab +``` + +If you are using Strict-Transport-Security in your installation to continue using it you must enable it in your Nginx +configuration as GitLab application no longer handles setting it. + +If you are using Apache instead of NGINX please see the updated [Apache templates]. +Also note that because Apache does not support upstreams behind Unix sockets you +will need to let gitlab-workhorse listen on a TCP port. You can do this +via [/etc/default/gitlab]. + +[Apache templates]: https://gitlab.com/gitlab-org/gitlab-recipes/tree/master/web-server/apache +[/etc/default/gitlab]: https://gitlab.com/gitlab-org/gitlab-ce/blob/11-2-stable/lib/support/init.d/gitlab.default.example#L38 + +#### SMTP configuration + +If you're installing from source and use SMTP to deliver mail, you will need to add the following line +to config/initializers/smtp_settings.rb: + +```ruby +ActionMailer::Base.delivery_method = :smtp +``` + +See [smtp_settings.rb.sample] as an example. + +[smtp_settings.rb.sample]: https://gitlab.com/gitlab-org/gitlab-ce/blob/11-2-stable/config/initializers/smtp_settings.rb.sample#L13 + +#### Init script + +There might be new configuration options available for [`gitlab.default.example`][gl-example]. View them with the command below and apply them manually to your current `/etc/default/gitlab`: + +```sh +cd /home/git/gitlab + +git diff origin/11-1-stable:lib/support/init.d/gitlab.default.example origin/11-2-stable:lib/support/init.d/gitlab.default.example +``` + +Ensure you're still up-to-date with the latest init script changes: + +```bash +cd /home/git/gitlab + +sudo cp lib/support/init.d/gitlab /etc/init.d/gitlab +``` + +For Ubuntu 16.04.1 LTS: + +```bash +sudo systemctl daemon-reload +``` + +### 13. Install libs, migrations, etc. + +```bash +cd /home/git/gitlab + +# MySQL installations (note: the line below states '--without postgres') +sudo -u git -H bundle install --without postgres development test --deployment + +# PostgreSQL installations (note: the line below states '--without mysql') +sudo -u git -H bundle install --without mysql development test --deployment + +# Optional: clean up old gems +sudo -u git -H bundle clean + +# Run database migrations +sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production + +# Compile GetText PO files + +sudo -u git -H bundle exec rake gettext:compile RAILS_ENV=production + +# Update node dependencies and recompile assets +sudo -u git -H bundle exec rake yarn:install gitlab:assets:clean gitlab:assets:compile RAILS_ENV=production NODE_ENV=production + +# Clean up cache +sudo -u git -H bundle exec rake cache:clear RAILS_ENV=production +``` + +**MySQL installations**: Run through the `MySQL strings limits` and `Tables and data conversion to utf8mb4` [tasks](../install/database_mysql.md). + +### 14. Start application + +```bash +sudo service gitlab start +sudo service nginx restart +``` + +### 15. Check application status + +Check if GitLab and its environment are configured correctly: + +```bash +cd /home/git/gitlab + +sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production +``` + +To make sure you didn't miss anything run a more thorough check: + +```bash +cd /home/git/gitlab + +sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production +``` + +If all items are green, then congratulations, the upgrade is complete! + +## Things went south? Revert to previous version (11.1) + +### 1. Revert the code to the previous version + +Follow the [upgrade guide from 11.0 to 11.1](11.0-to-11.1.md), except for the +database migration (the backup is already migrated to the previous version). + +### 2. Restore from the backup + +```bash +cd /home/git/gitlab + +sudo -u git -H bundle exec rake gitlab:backup:restore RAILS_ENV=production +``` + +If you have more than one backup `*.tar` file(s) please add `BACKUP=timestamp_of_backup` to the command above. + +[yaml]: https://gitlab.com/gitlab-org/gitlab-ce/blob/11-2-stable/config/gitlab.yml.example +[gl-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/11-2-stable/lib/support/init.d/gitlab.default.example diff --git a/lib/api/internal.rb b/lib/api/internal.rb index a9803be9f69..516f25db15b 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -11,7 +11,8 @@ module API # # Params: # key_id - ssh key id for Git over SSH - # user_id - user id for Git over HTTP + # user_id - user id for Git over HTTP or over SSH in keyless SSH CERT mode + # username - user name for Git over SSH in keyless SSH cert mode # protocol - Git access protocol being used, e.g. HTTP or SSH # project - project full_path (not path on disk) # action - git action (git-upload-pack or git-receive-pack) @@ -28,6 +29,8 @@ module API Key.find_by(id: params[:key_id]) elsif params[:user_id] User.find_by(id: params[:user_id]) + elsif params[:username] + User.find_by_username(params[:username]) end protocol = params[:protocol] @@ -58,6 +61,7 @@ module API { status: true, gl_repository: gl_repository, + gl_id: Gitlab::GlId.gl_id(user), gl_username: user&.username, # This repository_path is a bogus value but gitlab-shell still requires @@ -71,10 +75,17 @@ module API post "/lfs_authenticate" do status 200 - key = Key.find(params[:key_id]) - key.update_last_used_at + if params[:key_id] + actor = Key.find(params[:key_id]) + actor.update_last_used_at + elsif params[:user_id] + actor = User.find_by(id: params[:user_id]) + raise ActiveRecord::RecordNotFound.new("No such user id!") unless actor + else + raise ActiveRecord::RecordNotFound.new("No key_id or user_id passed!") + end - token_handler = Gitlab::LfsToken.new(key) + token_handler = Gitlab::LfsToken.new(actor) { username: token_handler.actor_name, @@ -100,7 +111,7 @@ module API end # - # Discover user by ssh key or user id + # Discover user by ssh key, user id or username # get "/discover" do if params[:key_id] @@ -108,6 +119,8 @@ module API user = key.user elsif params[:user_id] user = User.find_by(id: params[:user_id]) + elsif params[:username] + user = User.find_by(username: params[:username]) end present user, with: Entities::UserSafe @@ -141,22 +154,30 @@ module API post '/two_factor_recovery_codes' do status 200 - key = Key.find_by(id: params[:key_id]) + if params[:key_id] + key = Key.find_by(id: params[:key_id]) - if key - key.update_last_used_at - else - break { 'success' => false, 'message' => 'Could not find the given key' } - end + if key + key.update_last_used_at + else + break { 'success' => false, 'message' => 'Could not find the given key' } + end - if key.is_a?(DeployKey) - break { success: false, message: 'Deploy keys cannot be used to retrieve recovery codes' } - end + if key.is_a?(DeployKey) + break { success: false, message: 'Deploy keys cannot be used to retrieve recovery codes' } + end + + user = key.user - user = key.user + unless user + break { success: false, message: 'Could not find a user for the given key' } + end + elsif params[:user_id] + user = User.find_by(id: params[:user_id]) - unless user - break { success: false, message: 'Could not find a user for the given key' } + unless user + break { success: false, message: 'Could not find the given user' } + end end unless user.two_factor_enabled? diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 25185d6edc8..bda05d1795b 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -162,6 +162,9 @@ module API desc: 'The IID of a merge request for which to resolve discussions' optional :discussion_to_resolve, type: String, desc: 'The ID of a discussion to resolve, also pass `merge_request_to_resolve_discussions_of`' + optional :iid, type: Integer, + desc: 'The internal ID of a project issue. Available only for admins and project owners.' + use :issue_params end post ':id/issues' do @@ -169,9 +172,10 @@ module API authorize! :create_issue, user_project - # Setting created_at time only allowed for admins and project owners + # Setting created_at time or iid only allowed for admins and project owners unless current_user.admin? || user_project.owner == current_user params.delete(:created_at) + params.delete(:iid) end issue_params = declared_params(include_missing: false) diff --git a/lib/api/runner.rb b/lib/api/runner.rb index c7595493e11..c9931c2d603 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -80,7 +80,15 @@ module API params do requires :token, type: String, desc: %q(Runner's authentication token) optional :last_update, type: String, desc: %q(Runner's queue last_update token) - optional :info, type: Hash, desc: %q(Runner's metadata) + optional :info, type: Hash, desc: %q(Runner's metadata) do + optional :name, type: String, desc: %q(Runner's name) + optional :version, type: String, desc: %q(Runner's version) + optional :revision, type: String, desc: %q(Runner's revision) + optional :platform, type: String, desc: %q(Runner's platform) + optional :architecture, type: String, desc: %q(Runner's architecture) + optional :executor, type: String, desc: %q(Runner's executor) + optional :features, type: Hash, desc: %q(Runner's features) + end optional :session, type: Hash, desc: %q(Runner's session data) do optional :url, type: String, desc: %q(Session's url) optional :certificate, type: String, desc: %q(Session's certificate) diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 1ca7d23203b..6601c268d79 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -20,116 +20,116 @@ module API success Entities::ApplicationSetting end params do + optional :admin_notification_email, type: String, desc: 'Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.' + optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' + optional :after_sign_out_path, type: String, desc: 'We will redirect users to this page after they sign out' + optional :akismet_enabled, type: Boolean, desc: 'Helps prevent bots from creating issues' + given akismet_enabled: ->(val) { val } do + requires :akismet_api_key, type: String, desc: 'Generate API key at http://www.akismet.com' + end + optional :clientside_sentry_enabled, type: Boolean, desc: 'Sentry can also be used for reporting and logging clientside exceptions. https://sentry.io/for/javascript/' + given clientside_sentry_enabled: ->(val) { val } do + requires :clientside_sentry_dsn, type: String, desc: 'Clientside Sentry Data Source Name' + end + optional :container_registry_token_expire_delay, type: Integer, desc: 'Authorization token duration (minutes)' + optional :default_artifacts_expire_in, type: String, desc: "Set the default expiration time for each job's artifacts" optional :default_branch_protection, type: Integer, values: [0, 1, 2], desc: 'Determine if developers can push to master' + optional :default_group_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default group visibility' optional :default_project_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default project visibility' + optional :default_projects_limit, type: Integer, desc: 'The maximum number of personal projects' optional :default_snippet_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default snippet visibility' - optional :default_group_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default group visibility' - optional :restricted_visibility_levels, type: Array[String], desc: 'Selected levels cannot be used by non-admin users for groups, projects or snippets. If the public level is restricted, user profiles are only visible to logged in users.' - optional :import_sources, type: Array[String], values: %w[github bitbucket gitlab google_code fogbugz git gitlab_project manifest], - desc: 'Enabled sources for code import during project creation. OmniAuth must be configured for GitHub, Bitbucket, and GitLab.com' optional :disabled_oauth_sign_in_sources, type: Array[String], desc: 'Disable certain OAuth sign-in sources' - optional :enabled_git_access_protocol, type: String, values: %w[ssh http nil], desc: 'Allow only the selected protocols to be used for Git access.' - optional :project_export_enabled, type: Boolean, desc: 'Enable project export' - optional :gravatar_enabled, type: Boolean, desc: 'Flag indicating if the Gravatar service is enabled' - optional :default_projects_limit, type: Integer, desc: 'The maximum number of personal projects' - optional :max_attachment_size, type: Integer, desc: 'Maximum attachment size in MB' - optional :session_expire_delay, type: Integer, desc: 'Session duration in minutes. GitLab restart is required to apply changes.' - optional :user_oauth_applications, type: Boolean, desc: 'Allow users to register any application to use GitLab as an OAuth provider' - optional :user_default_external, type: Boolean, desc: 'Newly registered users will by default be external' - optional :signup_enabled, type: Boolean, desc: 'Flag indicating if sign up is enabled' - optional :send_user_confirmation_email, type: Boolean, desc: 'Send confirmation email on sign-up' - optional :domain_whitelist, type: String, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' optional :domain_blacklist_enabled, type: Boolean, desc: 'Enable domain blacklist for sign ups' given domain_blacklist_enabled: ->(val) { val } do requires :domain_blacklist, type: String, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' end - optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' - optional :password_authentication_enabled_for_web, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' - optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 - optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 - mutually_exclusive :password_authentication_enabled_for_web, :password_authentication_enabled, :signin_enabled - optional :password_authentication_enabled_for_git, type: Boolean, desc: 'Flag indicating if password authentication is enabled for Git over HTTP(S)' - optional :performance_bar_allowed_group_path, type: String, desc: 'Path of the group that is allowed to toggle the performance bar.' - optional :performance_bar_allowed_group_id, type: String, desc: 'Depreated: Use :performance_bar_allowed_group_path instead. Path of the group that is allowed to toggle the performance bar.' # support legacy names, can be removed in v6 - optional :performance_bar_enabled, type: String, desc: 'Deprecated: Pass `performance_bar_allowed_group_path: nil` instead. Allow enabling the performance.' # support legacy names, can be removed in v6 - optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication' - given require_two_factor_authentication: ->(val) { val } do - requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication' - end - optional :home_page_url, type: String, desc: 'We will redirect non-logged in users to this page' - optional :after_sign_out_path, type: String, desc: 'We will redirect users to this page after they sign out' - optional :sign_in_text, type: String, desc: 'The sign in text of the GitLab application' + optional :domain_whitelist, type: String, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' + optional :email_author_in_body, type: Boolean, desc: 'Some email servers do not support overriding the email sender name. Enable this option to include the name of the author of the issue, merge request or comment in the email body instead.' + optional :enabled_git_access_protocol, type: String, values: %w[ssh http nil], desc: 'Allow only the selected protocols to be used for Git access.' + optional :gitaly_timeout_default, type: Integer, desc: 'Default Gitaly timeout, in seconds. Set to 0 to disable timeouts.' + optional :gitaly_timeout_fast, type: Integer, desc: 'Gitaly fast operation timeout, in seconds. Set to 0 to disable timeouts.' + optional :gitaly_timeout_medium, type: Integer, desc: 'Medium Gitaly timeout, in seconds. Set to 0 to disable timeouts.' + optional :gravatar_enabled, type: Boolean, desc: 'Flag indicating if the Gravatar service is enabled' optional :help_page_hide_commercial_content, type: Boolean, desc: 'Hide marketing-related entries from help' - optional :help_page_text, type: String, desc: 'Custom text displayed on the help page' optional :help_page_support_url, type: String, desc: 'Alternate support URL for help page' - optional :shared_runners_enabled, type: Boolean, desc: 'Enable shared runners for new projects' - given shared_runners_enabled: ->(val) { val } do - requires :shared_runners_text, type: String, desc: 'Shared runners text ' + optional :help_page_text, type: String, desc: 'Custom text displayed on the help page' + optional :home_page_url, type: String, desc: 'We will redirect non-logged in users to this page' + optional :housekeeping_enabled, type: Boolean, desc: 'Enable automatic repository housekeeping (git repack, git gc)' + given housekeeping_enabled: ->(val) { val } do + requires :housekeeping_bitmaps_enabled, type: Boolean, desc: "Creating pack file bitmaps makes housekeeping take a little longer but bitmaps should accelerate 'git clone' performance." + requires :housekeeping_full_repack_period, type: Integer, desc: "Number of Git pushes after which a full 'git repack' is run." + requires :housekeeping_gc_period, type: Integer, desc: "Number of Git pushes after which 'git gc' is run." + requires :housekeeping_incremental_repack_period, type: Integer, desc: "Number of Git pushes after which an incremental 'git repack' is run." + end + optional :html_emails_enabled, type: Boolean, desc: 'By default GitLab sends emails in HTML and plain text formats so mail clients can choose what format to use. Disable this option if you only want to send emails in plain text format.' + optional :import_sources, type: Array[String], values: %w[github bitbucket gitlab google_code fogbugz git gitlab_project manifest], + desc: 'Enabled sources for code import during project creation. OmniAuth must be configured for GitHub, Bitbucket, and GitLab.com' + optional :koding_enabled, type: Boolean, desc: 'Enable Koding' + given koding_enabled: ->(val) { val } do + requires :koding_url, type: String, desc: 'The Koding team URL' end optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size for each job's artifacts" - optional :default_artifacts_expire_in, type: String, desc: "Set the default expiration time for each job's artifacts" + optional :max_attachment_size, type: Integer, desc: 'Maximum attachment size in MB' optional :max_pages_size, type: Integer, desc: 'Maximum size of pages in MB' - optional :container_registry_token_expire_delay, type: Integer, desc: 'Authorization token duration (minutes)' - optional :prometheus_metrics_enabled, type: Boolean, desc: 'Enable Prometheus metrics' optional :metrics_enabled, type: Boolean, desc: 'Enable the InfluxDB metrics' given metrics_enabled: ->(val) { val } do requires :metrics_host, type: String, desc: 'The InfluxDB host' - requires :metrics_port, type: Integer, desc: 'The UDP port to use for connecting to InfluxDB' - requires :metrics_pool_size, type: Integer, desc: 'The amount of InfluxDB connections to open' - requires :metrics_timeout, type: Integer, desc: 'The amount of seconds after which an InfluxDB connection will time out' requires :metrics_method_call_threshold, type: Integer, desc: 'A method call is only tracked when it takes longer to complete than the given amount of milliseconds.' - requires :metrics_sample_interval, type: Integer, desc: 'The sampling interval in seconds' requires :metrics_packet_size, type: Integer, desc: 'The amount of points to store in a single UDP packet' + requires :metrics_pool_size, type: Integer, desc: 'The amount of InfluxDB connections to open' + requires :metrics_port, type: Integer, desc: 'The UDP port to use for connecting to InfluxDB' + requires :metrics_sample_interval, type: Integer, desc: 'The sampling interval in seconds' + requires :metrics_timeout, type: Integer, desc: 'The amount of seconds after which an InfluxDB connection will time out' end - optional :sidekiq_throttling_enabled, type: Boolean, desc: 'Enable Sidekiq Job Throttling' - given sidekiq_throttling_enabled: ->(val) { val } do - requires :sidekiq_throttling_queus, type: Array[String], desc: 'Choose which queues you wish to throttle' - requires :sidekiq_throttling_factor, type: Float, desc: 'The factor by which the queues should be throttled. A value between 0.0 and 1.0, exclusive.' + optional :password_authentication_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 + optional :password_authentication_enabled_for_web, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' + mutually_exclusive :password_authentication_enabled_for_web, :password_authentication_enabled, :signin_enabled + optional :password_authentication_enabled_for_git, type: Boolean, desc: 'Flag indicating if password authentication is enabled for Git over HTTP(S)' + optional :performance_bar_allowed_group_id, type: String, desc: 'Deprecated: Use :performance_bar_allowed_group_path instead. Path of the group that is allowed to toggle the performance bar.' # support legacy names, can be removed in v6 + optional :performance_bar_allowed_group_path, type: String, desc: 'Path of the group that is allowed to toggle the performance bar.' + optional :performance_bar_enabled, type: String, desc: 'Deprecated: Pass `performance_bar_allowed_group_path: nil` instead. Allow enabling the performance.' # support legacy names, can be removed in v6 + optional :plantuml_enabled, type: Boolean, desc: 'Enable PlantUML' + given plantuml_enabled: ->(val) { val } do + requires :plantuml_url, type: String, desc: 'The PlantUML server URL' end + optional :polling_interval_multiplier, type: BigDecimal, desc: 'Interval multiplier used by endpoints that perform polling. Set to 0 to disable polling.' + optional :project_export_enabled, type: Boolean, desc: 'Enable project export' + optional :prometheus_metrics_enabled, type: Boolean, desc: 'Enable Prometheus metrics' optional :recaptcha_enabled, type: Boolean, desc: 'Helps prevent bots from creating accounts' given recaptcha_enabled: ->(val) { val } do requires :recaptcha_site_key, type: String, desc: 'Generate site key at http://www.google.com/recaptcha' requires :recaptcha_private_key, type: String, desc: 'Generate private key at http://www.google.com/recaptcha' end - optional :akismet_enabled, type: Boolean, desc: 'Helps prevent bots from creating issues' - given akismet_enabled: ->(val) { val } do - requires :akismet_api_key, type: String, desc: 'Generate API key at http://www.akismet.com' + optional :repository_checks_enabled, type: Boolean, desc: "GitLab will periodically run 'git fsck' in all project and wiki repositories to look for silent disk corruption issues." + optional :repository_storages, type: Array[String], desc: 'Storage paths for new projects' + optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication' + given require_two_factor_authentication: ->(val) { val } do + requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication' end - optional :admin_notification_email, type: String, desc: 'Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.' + optional :restricted_visibility_levels, type: Array[String], desc: 'Selected levels cannot be used by non-admin users for groups, projects or snippets. If the public level is restricted, user profiles are only visible to logged in users.' + optional :send_user_confirmation_email, type: Boolean, desc: 'Send confirmation email on sign-up' optional :sentry_enabled, type: Boolean, desc: 'Sentry is an error reporting and logging tool which is currently not shipped with GitLab, get it here: https://getsentry.com' given sentry_enabled: ->(val) { val } do requires :sentry_dsn, type: String, desc: 'Sentry Data Source Name' end - optional :clientside_sentry_enabled, type: Boolean, desc: 'Sentry can also be used for reporting and logging clientside exceptions. https://sentry.io/for/javascript/' - given clientside_sentry_enabled: ->(val) { val } do - requires :clientside_sentry_dsn, type: String, desc: 'Clientside Sentry Data Source Name' - end - optional :repository_storages, type: Array[String], desc: 'Storage paths for new projects' - optional :repository_checks_enabled, type: Boolean, desc: "GitLab will periodically run 'git fsck' in all project and wiki repositories to look for silent disk corruption issues." - optional :koding_enabled, type: Boolean, desc: 'Enable Koding' - given koding_enabled: ->(val) { val } do - requires :koding_url, type: String, desc: 'The Koding team URL' - end - optional :plantuml_enabled, type: Boolean, desc: 'Enable PlantUML' - given plantuml_enabled: ->(val) { val } do - requires :plantuml_url, type: String, desc: 'The PlantUML server URL' + optional :session_expire_delay, type: Integer, desc: 'Session duration in minutes. GitLab restart is required to apply changes.' + optional :shared_runners_enabled, type: Boolean, desc: 'Enable shared runners for new projects' + given shared_runners_enabled: ->(val) { val } do + requires :shared_runners_text, type: String, desc: 'Shared runners text ' end - optional :version_check_enabled, type: Boolean, desc: 'Let GitLab inform you when an update is available.' - optional :email_author_in_body, type: Boolean, desc: 'Some email servers do not support overriding the email sender name. Enable this option to include the name of the author of the issue, merge request or comment in the email body instead.' - optional :html_emails_enabled, type: Boolean, desc: 'By default GitLab sends emails in HTML and plain text formats so mail clients can choose what format to use. Disable this option if you only want to send emails in plain text format.' - optional :housekeeping_enabled, type: Boolean, desc: 'Enable automatic repository housekeeping (git repack, git gc)' - given housekeeping_enabled: ->(val) { val } do - requires :housekeeping_bitmaps_enabled, type: Boolean, desc: "Creating pack file bitmaps makes housekeeping take a little longer but bitmaps should accelerate 'git clone' performance." - requires :housekeeping_incremental_repack_period, type: Integer, desc: "Number of Git pushes after which an incremental 'git repack' is run." - requires :housekeeping_full_repack_period, type: Integer, desc: "Number of Git pushes after which a full 'git repack' is run." - requires :housekeeping_gc_period, type: Integer, desc: "Number of Git pushes after which 'git gc' is run." + optional :sidekiq_throttling_enabled, type: Boolean, desc: 'Enable Sidekiq Job Throttling' + given sidekiq_throttling_enabled: ->(val) { val } do + requires :sidekiq_throttling_factor, type: Float, desc: 'The factor by which the queues should be throttled. A value between 0.0 and 1.0, exclusive.' + requires :sidekiq_throttling_queues, type: Array[String], desc: 'Choose which queues you wish to throttle' end + optional :sign_in_text, type: String, desc: 'The sign in text of the GitLab application' + optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 + optional :signup_enabled, type: Boolean, desc: 'Flag indicating if sign up is enabled' optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.' - optional :polling_interval_multiplier, type: BigDecimal, desc: 'Interval multiplier used by endpoints that perform polling. Set to 0 to disable polling.' - optional :gitaly_timeout_default, type: Integer, desc: 'Default Gitaly timeout, in seconds. Set to 0 to disable timeouts.' - optional :gitaly_timeout_medium, type: Integer, desc: 'Medium Gitaly timeout, in seconds. Set to 0 to disable timeouts.' - optional :gitaly_timeout_fast, type: Integer, desc: 'Gitaly fast operation timeout, in seconds. Set to 0 to disable timeouts.' optional :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.' + optional :user_default_external, type: Boolean, desc: 'Newly registered users will by default be external' + optional :user_oauth_applications, type: Boolean, desc: 'Allow users to register any application to use GitLab as an OAuth provider' + optional :version_check_enabled, type: Boolean, desc: 'Let GitLab inform you when an update is available.' ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type| optional :"#{type}_key_restriction", diff --git a/lib/gitlab/auth/activity.rb b/lib/gitlab/auth/activity.rb new file mode 100644 index 00000000000..9f84c578d4f --- /dev/null +++ b/lib/gitlab/auth/activity.rb @@ -0,0 +1,77 @@ +module Gitlab + module Auth + ## + # Metrics and logging for user authentication activity. + # + class Activity + extend Gitlab::Utils::StrongMemoize + + COUNTERS = { + user_authenticated: 'Counter of successful authentication events', + user_unauthenticated: 'Counter of authentication failures', + user_not_found: 'Counter of failed log-ins when user is unknown', + user_password_invalid: 'Counter of failed log-ins with invalid password', + user_session_override: 'Counter of manual log-ins and sessions overrides', + user_session_destroyed: 'Counter of user sessions being destroyed', + user_two_factor_authenticated: 'Counter of two factor authentications', + user_sessionless_authentication: 'Counter of sessionless authentications', + user_blocked: 'Counter of sign in attempts when user is blocked' + }.freeze + + def initialize(user, opts) + @user = user + @opts = opts + end + + def user_authentication_failed! + self.class.user_unauthenticated_counter_increment! + + case @opts[:message] + when :not_found_in_database + self.class.user_not_found_counter_increment! + when :invalid + self.class.user_password_invalid_counter_increment! + end + + self.class.user_blocked_counter_increment! if @user&.blocked? + end + + def user_authenticated! + self.class.user_authenticated_counter_increment! + end + + def user_session_override! + self.class.user_session_override_counter_increment! + + case @opts[:message] + when :two_factor_authenticated + self.class.user_two_factor_authenticated_counter_increment! + when :sessionless_sign_in + self.class.user_sessionless_authentication_counter_increment! + end + end + + def user_session_destroyed! + self.class.user_session_destroyed_counter_increment! + end + + def self.each_counter + COUNTERS.each_pair do |metric, description| + yield "#{metric}_counter", metric, description + end + end + + each_counter do |counter, metric, description| + define_singleton_method(counter) do + strong_memoize(counter) do + Gitlab::Metrics.counter("gitlab_auth_#{metric}_total".to_sym, description) + end + end + + define_singleton_method("#{counter}_increment!") do + public_send(counter).increment # rubocop:disable GitlabSecurity/PublicSend + end + end + end + end +end diff --git a/lib/gitlab/auth/blocked_user_tracker.rb b/lib/gitlab/auth/blocked_user_tracker.rb index 7609a7b04f6..b6d2adc834b 100644 --- a/lib/gitlab/auth/blocked_user_tracker.rb +++ b/lib/gitlab/auth/blocked_user_tracker.rb @@ -2,37 +2,58 @@ module Gitlab module Auth class BlockedUserTracker + include Gitlab::Utils::StrongMemoize + ACTIVE_RECORD_REQUEST_PARAMS = 'action_dispatch.request.request_parameters' - def self.log_if_user_blocked(env) - message = env.dig('warden.options', :message) + def initialize(env) + @env = env + end - # Devise calls User#active_for_authentication? on the User model and then - # throws an exception to Warden with User#inactive_message: - # https://github.com/plataformatec/devise/blob/v4.2.1/lib/devise/hooks/activatable.rb#L8 - # - # Since Warden doesn't pass the user record to the failure handler, we - # need to do a database lookup with the username. We can limit the - # lookups to happen when the user was blocked by checking the inactive - # message passed along by Warden. - return unless message == User::BLOCKED_MESSAGE + def user_blocked? + user&.blocked? + end - # Check for either LDAP or regular GitLab account logins - login = env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'username') || - env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'user', 'login') + def user + return unless has_user_blocked_message? - return unless login.present? + strong_memoize(:user) do + # Check for either LDAP or regular GitLab account logins + login = @env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'username') || + @env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'user', 'login') - user = User.by_login(login) + User.by_login(login) if login.present? + end + rescue TypeError + end - return unless user&.blocked? + def log_blocked_user_activity! + return unless user_blocked? - Gitlab::AppLogger.info("Failed login for blocked user: user=#{user.username} ip=#{env['REMOTE_ADDR']}") + Gitlab::AppLogger.info("Failed login for blocked user: user=#{user.username} ip=#{@env['REMOTE_ADDR']}") SystemHooksService.new.execute_hooks_for(user, :failed_login) - true rescue TypeError end + + private + + ## + # Devise calls User#active_for_authentication? on the User model and then + # throws an exception to Warden with User#inactive_message: + # https://github.com/plataformatec/devise/blob/v4.2.1/lib/devise/hooks/activatable.rb#L8 + # + # Since Warden doesn't pass the user record to the failure handler, we + # need to do a database lookup with the username. We can limit the + # lookups to happen when the user was blocked by checking the inactive + # message passed along by Warden. + # + def has_user_blocked_message? + strong_memoize(:user_blocked_message) do + message = @env.dig('warden.options', :message) + message == User::BLOCKED_MESSAGE + end + end end end end diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb index 155f4fc1343..703f0b9217b 100644 --- a/lib/gitlab/ci/status/build/failed.rb +++ b/lib/gitlab/ci/status/build/failed.rb @@ -4,12 +4,13 @@ module Gitlab module Build class Failed < Status::Extended REASONS = { - 'unknown_failure' => 'unknown failure', - 'script_failure' => 'script failure', - 'api_failure' => 'API failure', - 'stuck_or_timeout_failure' => 'stuck or timeout failure', - 'runner_system_failure' => 'runner system failure', - 'missing_dependency_failure' => 'missing dependency failure' + unknown_failure: 'unknown failure', + script_failure: 'script failure', + api_failure: 'API failure', + stuck_or_timeout_failure: 'stuck or timeout failure', + runner_system_failure: 'runner system failure', + missing_dependency_failure: 'missing dependency failure', + runner_unsupported: 'unsupported runner' }.freeze def status_tooltip @@ -31,7 +32,11 @@ module Gitlab end def description - "<br> (#{REASONS[subject.failure_reason]})" + "<br> (#{failure_reason_message})" + end + + def failure_reason_message + REASONS.fetch(subject.failure_reason.to_sym) end end end diff --git a/lib/gitlab/cleanup/project_upload_file_finder.rb b/lib/gitlab/cleanup/project_upload_file_finder.rb new file mode 100644 index 00000000000..2ee8b60e76a --- /dev/null +++ b/lib/gitlab/cleanup/project_upload_file_finder.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Gitlab + module Cleanup + class ProjectUploadFileFinder + FIND_BATCH_SIZE = 500 + ABSOLUTE_UPLOAD_DIR = FileUploader.root.freeze + EXCLUDED_SYSTEM_UPLOADS_PATH = "#{ABSOLUTE_UPLOAD_DIR}/-/*".freeze + EXCLUDED_HASHED_UPLOADS_PATH = "#{ABSOLUTE_UPLOAD_DIR}/@hashed/*".freeze + EXCLUDED_TMP_UPLOADS_PATH = "#{ABSOLUTE_UPLOAD_DIR}/tmp/*".freeze + + # Paths are relative to the upload directory + def each_file_batch(batch_size: FIND_BATCH_SIZE, &block) + cmd = build_find_command(ABSOLUTE_UPLOAD_DIR) + + Open3.popen2(*cmd) do |stdin, stdout, status_thread| + yield_paths_in_batches(stdout, batch_size, &block) + + raise "Find command failed" unless status_thread.value.success? + end + end + + private + + def yield_paths_in_batches(stdout, batch_size, &block) + paths = [] + + stdout.each_line("\0") do |line| + paths << line.chomp("\0") + + if paths.size >= batch_size + yield(paths) + paths = [] + end + end + + yield(paths) if paths.any? + end + + def build_find_command(search_dir) + cmd = %W[find -L #{search_dir} + -type f + ! ( -path #{EXCLUDED_SYSTEM_UPLOADS_PATH} -prune ) + ! ( -path #{EXCLUDED_HASHED_UPLOADS_PATH} -prune ) + ! ( -path #{EXCLUDED_TMP_UPLOADS_PATH} -prune ) + -print0] + + ionice = which_ionice + cmd = %W[#{ionice} -c Idle] + cmd if ionice + + log_msg = "find command: \"#{cmd.join(' ')}\"" + Rails.logger.info log_msg + + cmd + end + + def which_ionice + Gitlab::Utils.which('ionice') + rescue StandardError + # In this case, returning false is relatively safe, + # even though it isn't very nice + false + end + end + end +end diff --git a/lib/gitlab/cleanup/project_uploads.rb b/lib/gitlab/cleanup/project_uploads.rb new file mode 100644 index 00000000000..b88e00311d5 --- /dev/null +++ b/lib/gitlab/cleanup/project_uploads.rb @@ -0,0 +1,125 @@ +# frozen_string_literal: true + +module Gitlab + module Cleanup + class ProjectUploads + LOST_AND_FOUND = File.join(ProjectUploadFileFinder::ABSOLUTE_UPLOAD_DIR, '-', 'project-lost-found') + + attr_reader :logger + + def initialize(logger: nil) + @logger = logger || Rails.logger + end + + def run!(dry_run: true) + logger.info "Looking for orphaned project uploads to clean up#{'. Dry run' if dry_run}..." + + each_orphan_file do |path, upload_path| + result = cleanup(path, upload_path, dry_run) + + logger.info result + end + end + + private + + def cleanup(path, upload_path, dry_run) + # This happened in staging: + # `find` returned a path on which `File.delete` raised `Errno::ENOENT` + return "Cannot find file: #{path}" unless File.exist?(path) + + correct_path = upload_path && find_correct_path(upload_path) + + if correct_path + move(path, correct_path, 'fix', dry_run) + else + move_to_lost_and_found(path, dry_run) + end + end + + # Accepts a path in the form of "#{hex_secret}/#{filename}" + def find_correct_path(upload_path) + upload = Upload.find_by(uploader: 'FileUploader', path: upload_path) + return unless upload && upload.local? + + upload.absolute_path + rescue => e + logger.error e.message + + # absolute_path depends on a lot of code. If it doesn't work, then it + # it doesn't matter if the upload file is in the right place. Treat it + # as uncorrectable. + # I.e. the project record might be missing, which raises an exception. + nil + end + + def move_to_lost_and_found(path, dry_run) + new_path = path.sub(/\A#{ProjectUploadFileFinder::ABSOLUTE_UPLOAD_DIR}/, LOST_AND_FOUND) + + move(path, new_path, 'move to lost and found', dry_run) + end + + def move(path, new_path, prefix, dry_run) + action = "#{prefix} #{path} -> #{new_path}" + + if dry_run + "Can #{action}" + else + begin + FileUtils.mkdir_p(File.dirname(new_path)) + FileUtils.mv(path, new_path) + + "Did #{action}" + rescue => e + "Error during #{action}: #{e.inspect}" + end + end + end + + # Yields absolute paths of project upload files that are not in the + # uploads table + def each_orphan_file + ProjectUploadFileFinder.new.each_file_batch do |file_paths| + logger.debug "Processing batch of #{file_paths.size} project upload file paths, starting with #{file_paths.first}" + + file_paths.each do |path| + pup = ProjectUploadPath.from_path(path) + + yield(path, pup.upload_path) if pup.orphan? + end + end + end + + class ProjectUploadPath + PROJECT_FULL_PATH_REGEX = %r{\A#{FileUploader.root}/(.+)/(\h+/[^/]+)\z}.freeze + + attr_reader :full_path, :upload_path + + def initialize(full_path, upload_path) + @full_path = full_path + @upload_path = upload_path + end + + def self.from_path(path) + path_matched = path.match(PROJECT_FULL_PATH_REGEX) + return new(nil, nil) unless path_matched + + new(path_matched[1], path_matched[2]) + end + + def orphan? + return true if full_path.nil? || upload_path.nil? + + # It's possible to reduce to one query, but `where_full_path_in` is complex + !Upload.exists?(path: upload_path, model_id: project_id, model_type: 'Project', uploader: 'FileUploader') + end + + private + + def project_id + @project_id ||= Project.where_full_path_in([full_path]).pluck(:id) + end + end + end + end +end diff --git a/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb index 742118b76a8..e731e654f3c 100644 --- a/lib/gitlab/git_post_receive.rb +++ b/lib/gitlab/git_post_receive.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Gitlab class GitPostReceive include Gitlab::Identifier @@ -14,10 +16,11 @@ module Gitlab end def changes_refs - return enum_for(:changes_refs) unless block_given? + return changes unless block_given? changes.each do |change| - oldrev, newrev, ref = change.strip.split(' ') + change.strip! + oldrev, newrev, ref = change.split(' ') yield oldrev, newrev, ref end @@ -26,13 +29,10 @@ module Gitlab private def deserialize_changes(changes) - changes = utf8_encode_changes(changes) - changes.lines + utf8_encode_changes(changes).each_line end def utf8_encode_changes(changes) - changes = changes.dup - changes.force_encoding('UTF-8') return changes if changes.valid_encoding? diff --git a/lib/gitlab/graphs/commits.rb b/lib/gitlab/graphs/commits.rb index 3caf9036459..c4ffc19df09 100644 --- a/lib/gitlab/graphs/commits.rb +++ b/lib/gitlab/graphs/commits.rb @@ -18,7 +18,7 @@ module Gitlab end def commit_per_day - @commit_per_day ||= @commits.size / (@duration + 1) + @commit_per_day ||= (@commits.size.to_f / (@duration + 1)).round(1) end def collect_data diff --git a/lib/gitlab/import_sources.rb b/lib/gitlab/import_sources.rb index af9b880ef9e..45816bee176 100644 --- a/lib/gitlab/import_sources.rb +++ b/lib/gitlab/import_sources.rb @@ -22,24 +22,28 @@ module Gitlab class << self def options - @options ||= Hash[ImportTable.map { |importer| [importer.title, importer.name] }] + Hash[import_table.map { |importer| [importer.title, importer.name] }] end def values - @values ||= ImportTable.map(&:name) + import_table.map(&:name) end def importer_names - @importer_names ||= ImportTable.select(&:importer).map(&:name) + import_table.select(&:importer).map(&:name) end def importer(name) - ImportTable.find { |import_source| import_source.name == name }.importer + import_table.find { |import_source| import_source.name == name }.importer end def title(name) options.key(name) end + + def import_table + ImportTable + end end end end diff --git a/lib/gitlab/kubernetes/config_map.rb b/lib/gitlab/kubernetes/config_map.rb index 9e55dae137c..8a8a59a9cd4 100644 --- a/lib/gitlab/kubernetes/config_map.rb +++ b/lib/gitlab/kubernetes/config_map.rb @@ -1,15 +1,15 @@ module Gitlab module Kubernetes class ConfigMap - def initialize(name, files) + def initialize(name, values = "") @name = name - @files = files + @values = values end def generate resource = ::Kubeclient::Resource.new resource.metadata = metadata - resource.data = files + resource.data = { values: values } resource end @@ -19,7 +19,7 @@ module Gitlab private - attr_reader :name, :files + attr_reader :name, :values def metadata { diff --git a/lib/gitlab/kubernetes/helm/api.rb b/lib/gitlab/kubernetes/helm/api.rb index d65374cc23b..c4de9a398cc 100644 --- a/lib/gitlab/kubernetes/helm/api.rb +++ b/lib/gitlab/kubernetes/helm/api.rb @@ -9,7 +9,7 @@ module Gitlab def install(command) namespace.ensure_exists! - create_config_map(command) + create_config_map(command) if command.config_map? kubeclient.create_pod(command.pod_resource) end diff --git a/lib/gitlab/kubernetes/helm/base_command.rb b/lib/gitlab/kubernetes/helm/base_command.rb index afcfd109de0..f9ebe53d6af 100644 --- a/lib/gitlab/kubernetes/helm/base_command.rb +++ b/lib/gitlab/kubernetes/helm/base_command.rb @@ -1,7 +1,13 @@ module Gitlab module Kubernetes module Helm - module BaseCommand + class BaseCommand + attr_reader :name + + def initialize(name) + @name = name + end + def pod_resource Gitlab::Kubernetes::Helm::Pod.new(self, namespace).generate end @@ -18,32 +24,16 @@ module Gitlab HEREDOC end - def pod_name - "install-#{name}" - end - - def config_map_resource - Gitlab::Kubernetes::ConfigMap.new(name, files).generate + def config_map? + false end - def file_names - files.keys - end - - def name - raise "Not implemented" - end - - def files - raise "Not implemented" + def pod_name + "install-#{name}" end private - def files_dir - "/data/helm/#{name}/config" - end - def namespace Gitlab::Kubernetes::Helm::NAMESPACE end diff --git a/lib/gitlab/kubernetes/helm/certificate.rb b/lib/gitlab/kubernetes/helm/certificate.rb deleted file mode 100644 index c344add82cd..00000000000 --- a/lib/gitlab/kubernetes/helm/certificate.rb +++ /dev/null @@ -1,72 +0,0 @@ -module Gitlab - module Kubernetes - module Helm - class Certificate - INFINITE_EXPIRY = 1000.years - SHORT_EXPIRY = 30.minutes - - attr_reader :key, :cert - - def key_string - @key.to_s - end - - def cert_string - @cert.to_pem - end - - def self.from_strings(key_string, cert_string) - key = OpenSSL::PKey::RSA.new(key_string) - cert = OpenSSL::X509::Certificate.new(cert_string) - new(key, cert) - end - - def self.generate_root - _issue(signed_by: nil, expires_in: INFINITE_EXPIRY, certificate_authority: true) - end - - def issue(expires_in: SHORT_EXPIRY) - self.class._issue(signed_by: self, expires_in: expires_in, certificate_authority: false) - end - - private - - def self._issue(signed_by:, expires_in:, certificate_authority:) - key = OpenSSL::PKey::RSA.new(4096) - public_key = key.public_key - - subject = OpenSSL::X509::Name.parse("/C=US") - - cert = OpenSSL::X509::Certificate.new - cert.subject = subject - - cert.issuer = signed_by&.cert&.subject || subject - - cert.not_before = Time.now - cert.not_after = expires_in.from_now - cert.public_key = public_key - cert.serial = 0x0 - cert.version = 2 - - if certificate_authority - extension_factory = OpenSSL::X509::ExtensionFactory.new - extension_factory.subject_certificate = cert - extension_factory.issuer_certificate = cert - cert.add_extension(extension_factory.create_extension('subjectKeyIdentifier', 'hash')) - cert.add_extension(extension_factory.create_extension('basicConstraints', 'CA:TRUE', true)) - cert.add_extension(extension_factory.create_extension('keyUsage', 'cRLSign,keyCertSign', true)) - end - - cert.sign(signed_by&.key || key, OpenSSL::Digest::SHA256.new) - - new(key, cert) - end - - def initialize(key, cert) - @key = key - @cert = cert - end - end - end - end -end diff --git a/lib/gitlab/kubernetes/helm/init_command.rb b/lib/gitlab/kubernetes/helm/init_command.rb index a4546509515..a02e64561f6 100644 --- a/lib/gitlab/kubernetes/helm/init_command.rb +++ b/lib/gitlab/kubernetes/helm/init_command.rb @@ -1,16 +1,7 @@ module Gitlab module Kubernetes module Helm - class InitCommand - include BaseCommand - - attr_reader :name, :files - - def initialize(name:, files:) - @name = name - @files = files - end - + class InitCommand < BaseCommand def generate_script super + [ init_helm_command @@ -20,12 +11,7 @@ module Gitlab private def init_helm_command - tls_flags = "--tiller-tls" \ - " --tiller-tls-verify --tls-ca-cert #{files_dir}/ca.pem" \ - " --tiller-tls-cert #{files_dir}/cert.pem" \ - " --tiller-tls-key #{files_dir}/key.pem" - - "helm init #{tls_flags} >/dev/null" + "helm init >/dev/null" end end end diff --git a/lib/gitlab/kubernetes/helm/install_command.rb b/lib/gitlab/kubernetes/helm/install_command.rb index c7d6a9c5b4d..d2133a6d65b 100644 --- a/lib/gitlab/kubernetes/helm/install_command.rb +++ b/lib/gitlab/kubernetes/helm/install_command.rb @@ -1,16 +1,14 @@ module Gitlab module Kubernetes module Helm - class InstallCommand - include BaseCommand + class InstallCommand < BaseCommand + attr_reader :name, :chart, :version, :repository, :values - attr_reader :name, :files, :chart, :version, :repository - - def initialize(name:, chart:, files:, version: nil, repository: nil) + def initialize(name, chart:, values:, version: nil, repository: nil) @name = name @chart = chart @version = version - @files = files + @values = values @repository = repository end @@ -22,6 +20,14 @@ module Gitlab ].compact.join("\n") end + def config_map? + true + end + + def config_map_resource + Gitlab::Kubernetes::ConfigMap.new(name, values).generate + end + private def init_command @@ -33,27 +39,14 @@ module Gitlab end def script_command - "helm install" \ - "#{optional_tls_flags} " \ - "#{chart} " \ - "--name #{name}" \ - "#{optional_version_flag} " \ - "--namespace #{Gitlab::Kubernetes::Helm::NAMESPACE} " \ - "-f /data/helm/#{name}/config/values.yaml >/dev/null\n" + <<~HEREDOC + helm install #{chart} --name #{name}#{optional_version_flag} --namespace #{Gitlab::Kubernetes::Helm::NAMESPACE} -f /data/helm/#{name}/config/values.yaml >/dev/null + HEREDOC end def optional_version_flag " --version #{version}" if version end - - def optional_tls_flags - return unless files.key?(:'ca.pem') - - " --tls" \ - " --tls-ca-cert #{files_dir}/ca.pem" \ - " --tls-cert #{files_dir}/cert.pem" \ - " --tls-key #{files_dir}/key.pem" - end end end end diff --git a/lib/gitlab/kubernetes/helm/pod.rb b/lib/gitlab/kubernetes/helm/pod.rb index 6e5d3388405..1e12299eefd 100644 --- a/lib/gitlab/kubernetes/helm/pod.rb +++ b/lib/gitlab/kubernetes/helm/pod.rb @@ -10,8 +10,10 @@ module Gitlab def generate spec = { containers: [container_specification], restartPolicy: 'Never' } - spec[:volumes] = volumes_specification - spec[:containers][0][:volumeMounts] = volume_mounts_specification + if command.config_map? + spec[:volumes] = volumes_specification + spec[:containers][0][:volumeMounts] = volume_mounts_specification + end ::Kubeclient::Resource.new(metadata: metadata, spec: spec) end @@ -59,7 +61,7 @@ module Gitlab name: 'configuration-volume', configMap: { name: "values-content-configuration-#{command.name}", - items: command.file_names.map { |name| { key: name, path: name } } + items: [{ key: 'values', path: 'values.yaml' }] } } ] diff --git a/lib/gitlab/template_helper.rb b/lib/gitlab/template_helper.rb new file mode 100644 index 00000000000..3b8e45e0688 --- /dev/null +++ b/lib/gitlab/template_helper.rb @@ -0,0 +1,22 @@ +module Gitlab + module TemplateHelper + include Gitlab::Utils::StrongMemoize + + def prepare_template_environment(file_path) + return unless file_path.present? + + FileUtils.mkdir_p(File.dirname(import_upload_path)) + FileUtils.copy_entry(file_path, import_upload_path) + end + + def import_upload_path + strong_memoize(:import_upload_path) do + Gitlab::ImportExport.import_upload_path(filename: tmp_filename) + end + end + + def tmp_filename + SecureRandom.hex + end + end +end diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index a8acafa9cd9..e5b5f3548e4 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -385,14 +385,6 @@ namespace :gitlab do end end - namespace :repo do - desc "GitLab | Check the integrity of the repositories managed by GitLab" - task check: :gitlab_environment do - puts "This task is deprecated. Please use gitlab:git:fsck instead".color(:red) - Rake::Task["gitlab:git:fsck"].execute - end - end - namespace :orphans do desc 'Gitlab | Check for orphaned namespaces and repositories' task check: :gitlab_environment do @@ -422,23 +414,6 @@ namespace :gitlab do end end - namespace :user do - desc "GitLab | Check the integrity of a specific user's repositories" - task :check_repos, [:username] => :gitlab_environment do |t, args| - username = args[:username] || prompt("Check repository integrity for username? ".color(:blue)) - user = User.find_by(username: username) - if user - repo_dirs = user.authorized_projects.map do |p| - p.repository.path_to_repo - end - - repo_dirs.each { |repo_dir| check_repo_integrity(repo_dir) } - else - puts "\nUser '#{username}' not found".color(:red) - end - end - end - # Helper methods ########################## diff --git a/lib/tasks/gitlab/cleanup.rake b/lib/tasks/gitlab/cleanup.rake index 5e07b12ee1c..a2feb074b1d 100644 --- a/lib/tasks/gitlab/cleanup.rake +++ b/lib/tasks/gitlab/cleanup.rake @@ -7,9 +7,8 @@ namespace :gitlab do desc "GitLab | Cleanup | Clean namespaces" task dirs: :gitlab_environment do warn_user_is_not_gitlab - remove_flag = ENV['REMOVE'] - namespaces = Namespace.pluck(:path) + namespaces = Namespace.pluck(:path) namespaces << HASHED_REPOSITORY_NAME # add so that it will be ignored Gitlab.config.repositories.storages.each do |name, repository_storage| git_base_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository_storage.legacy_disk_path } @@ -31,7 +30,7 @@ namespace :gitlab do end all_dirs.each do |dir_path| - if remove_flag + if remove? if FileUtils.rm_rf dir_path puts "Removed...#{dir_path}".color(:red) else @@ -43,7 +42,7 @@ namespace :gitlab do end end - unless remove_flag + unless remove? puts "To cleanup this directories run this command with REMOVE=true".color(:yellow) end end @@ -104,5 +103,37 @@ namespace :gitlab do puts "To block these users run this command with BLOCK=true".color(:yellow) end end + + desc "GitLab | Cleanup | Clean orphaned project uploads" + task project_uploads: :gitlab_environment do + warn_user_is_not_gitlab + + cleaner = Gitlab::Cleanup::ProjectUploads.new(logger: logger) + cleaner.run!(dry_run: dry_run?) + + if dry_run? + logger.info "To clean up these files run this command with DRY_RUN=false".color(:yellow) + end + end + + def remove? + ENV['REMOVE'] == 'true' + end + + def dry_run? + ENV['DRY_RUN'] != 'false' + end + + def logger + return @logger if defined?(@logger) + + @logger = if Rails.env.development? || Rails.env.production? + Logger.new(STDOUT).tap do |stdout_logger| + stdout_logger.extend(ActiveSupport::Logger.broadcast(Rails.logger)) + end + else + Rails.logger + end + end end end diff --git a/lib/tasks/gitlab/git.rake b/lib/tasks/gitlab/git.rake index cb4f7e5c8a8..8a53b51d4fe 100644 --- a/lib/tasks/gitlab/git.rake +++ b/lib/tasks/gitlab/git.rake @@ -1,87 +1,24 @@ namespace :gitlab do namespace :git do - desc "GitLab | Git | Repack" - task repack: :gitlab_environment do - failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} repack -a --quiet), "Repacking repo") - if failures.empty? - puts "Done".color(:green) - else - output_failures(failures) - end - end - - desc "GitLab | Git | Run garbage collection on all repos" - task gc: :gitlab_environment do - failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} gc --auto --quiet), "Garbage Collecting") - if failures.empty? - puts "Done".color(:green) - else - output_failures(failures) - end - end - - desc "GitLab | Git | Prune all repos" - task prune: :gitlab_environment do - failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} prune), "Git Prune") - if failures.empty? - puts "Done".color(:green) - else - output_failures(failures) - end - end - desc 'GitLab | Git | Check all repos integrity' task fsck: :gitlab_environment do - failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} fsck --name-objects --no-progress), "Checking integrity") do |repo| - check_config_lock(repo) - check_ref_locks(repo) - end - - if failures.empty? - puts "Done".color(:green) - else - output_failures(failures) - end - end - - def perform_git_cmd(cmd, message) - puts "Starting #{message} on all repositories" - failures = [] - all_repos do |repo| - if system(*cmd, chdir: repo) - puts "Performed #{message} at #{repo}" - else - failures << repo + Project.find_each(batch_size: 100) do |project| + begin + project.repository.fsck + + rescue => e + failures << "#{project.full_path} on #{project.repository_storage}: #{e}" end - yield(repo) if block_given? + puts "Performed integrity check for #{project.repository.full_path}" end - failures - end - - def output_failures(failures) - puts "The following repositories reported errors:".color(:red) - failures.each { |f| puts "- #{f}" } - end - - def check_config_lock(repo_dir) - config_exists = File.exist?(File.join(repo_dir, 'config.lock')) - config_output = config_exists ? 'yes'.color(:red) : 'no'.color(:green) - - puts "'config.lock' file exists?".color(:yellow) + " ... #{config_output}" - end - - def check_ref_locks(repo_dir) - lock_files = Dir.glob(File.join(repo_dir, 'refs/heads/*.lock')) - - if lock_files.present? - puts "Ref lock files exist:".color(:red) - - lock_files.each { |lock_file| puts " #{lock_file}" } + if failures.empty? + puts "Done".color(:green) else - puts "No ref lock files exist".color(:green) + puts "The following repositories reported errors:".color(:red) + failures.each { |f| puts "- #{f}" } end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e63a1f129d6..7189d1fedaf 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2522,6 +2522,9 @@ msgstr "" msgid "Files (%{human_size})" msgstr "" +msgid "Filter" +msgstr "" + msgid "Filter by commit message" msgstr "" @@ -3562,12 +3565,21 @@ msgstr "" msgid "No files found." msgstr "" +msgid "No labels with such name or description" +msgstr "" + msgid "No merge requests found" msgstr "" msgid "No messages were logged" msgstr "" +msgid "No other labels with such name or description" +msgstr "" + +msgid "No prioritised labels with such name or description" +msgstr "" + msgid "No public groups" msgstr "" @@ -4898,6 +4910,9 @@ msgstr "" msgid "Submit as spam" msgstr "" +msgid "Submit search" +msgstr "" + msgid "Subscribe" msgstr "" diff --git a/qa/qa/factory/resource/kubernetes_cluster.rb b/qa/qa/factory/resource/kubernetes_cluster.rb index ef2ea72b170..1c9e5f94b22 100644 --- a/qa/qa/factory/resource/kubernetes_cluster.rb +++ b/qa/qa/factory/resource/kubernetes_cluster.rb @@ -44,11 +44,10 @@ module QA page.await_installed(:helm) page.install!(:ingress) if @install_ingress - page.install!(:prometheus) if @install_prometheus - page.install!(:runner) if @install_runner - page.await_installed(:ingress) if @install_ingress + page.install!(:prometheus) if @install_prometheus page.await_installed(:prometheus) if @install_prometheus + page.install!(:runner) if @install_runner page.await_installed(:runner) if @install_runner end end diff --git a/qa/qa/page/project/operations/kubernetes/show.rb b/qa/qa/page/project/operations/kubernetes/show.rb index e831edeb89e..4923304133e 100644 --- a/qa/qa/page/project/operations/kubernetes/show.rb +++ b/qa/qa/page/project/operations/kubernetes/show.rb @@ -16,7 +16,6 @@ module QA def install!(application_name) within(".js-cluster-application-row-#{application_name}") do - page.has_button?('Install', wait: 30) click_on 'Install' end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index f1165c73847..bad7a28556c 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -57,6 +57,10 @@ describe ApplicationController do end describe "#authenticate_user_from_personal_access_token!" do + before do + stub_authentication_activity_metrics(debug: false) + end + controller(described_class) do def index render text: 'authenticated' @@ -67,7 +71,13 @@ describe ApplicationController do context "when the 'personal_access_token' param is populated with the personal access token" do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + get :index, private_token: personal_access_token.token + expect(response).to have_gitlab_http_status(200) expect(response.body).to eq('authenticated') end @@ -75,15 +85,25 @@ describe ApplicationController do context "when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + @request.headers["PRIVATE-TOKEN"] = personal_access_token.token get :index + expect(response).to have_gitlab_http_status(200) expect(response.body).to eq('authenticated') end end it "doesn't log the user in otherwise" do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + get :index, private_token: "token" + expect(response.status).not_to eq(200) expect(response.body).not_to eq('authenticated') end @@ -174,6 +194,10 @@ describe ApplicationController do end describe '#authenticate_sessionless_user!' do + before do + stub_authentication_activity_metrics(debug: false) + end + describe 'authenticating a user from a feed token' do controller(described_class) do def index @@ -184,7 +208,13 @@ describe ApplicationController do context "when the 'feed_token' param is populated with the feed token" do context 'when the request format is atom' do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + get :index, feed_token: user.feed_token, format: :atom + expect(response).to have_gitlab_http_status 200 expect(response.body).to eq 'authenticated' end @@ -192,7 +222,13 @@ describe ApplicationController do context 'when the request format is ics' do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + get :index, feed_token: user.feed_token, format: :ics + expect(response).to have_gitlab_http_status 200 expect(response.body).to eq 'authenticated' end @@ -200,7 +236,11 @@ describe ApplicationController do context 'when the request format is neither atom nor ics' do it "doesn't log the user in" do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + get :index, feed_token: user.feed_token + expect(response.status).not_to have_gitlab_http_status 200 expect(response.body).not_to eq 'authenticated' end @@ -209,7 +249,11 @@ describe ApplicationController do context "when the 'feed_token' param is populated with an invalid feed token" do it "doesn't log the user" do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + get :index, feed_token: 'token', format: :atom + expect(response.status).not_to eq 200 expect(response.body).not_to eq 'authenticated' end diff --git a/spec/factories/clusters/applications/helm.rb b/spec/factories/clusters/applications/helm.rb index 7c4a440b9a9..3e4277e4ba6 100644 --- a/spec/factories/clusters/applications/helm.rb +++ b/spec/factories/clusters/applications/helm.rb @@ -32,21 +32,11 @@ FactoryBot.define do updated_at ClusterWaitForAppInstallationWorker::TIMEOUT.ago end - factory :clusters_applications_ingress, class: Clusters::Applications::Ingress do - cluster factory: %i(cluster with_installed_helm provided_by_gcp) - end - - factory :clusters_applications_prometheus, class: Clusters::Applications::Prometheus do - cluster factory: %i(cluster with_installed_helm provided_by_gcp) - end - - factory :clusters_applications_runner, class: Clusters::Applications::Runner do - cluster factory: %i(cluster with_installed_helm provided_by_gcp) - end - + factory :clusters_applications_ingress, class: Clusters::Applications::Ingress + factory :clusters_applications_prometheus, class: Clusters::Applications::Prometheus + factory :clusters_applications_runner, class: Clusters::Applications::Runner factory :clusters_applications_jupyter, class: Clusters::Applications::Jupyter do oauth_application factory: :oauth_application - cluster factory: %i(cluster with_installed_helm provided_by_gcp) end end end diff --git a/spec/factories/clusters/clusters.rb b/spec/factories/clusters/clusters.rb index bbeba8ce8b9..0430762c1ff 100644 --- a/spec/factories/clusters/clusters.rb +++ b/spec/factories/clusters/clusters.rb @@ -36,9 +36,5 @@ FactoryBot.define do trait :production_environment do sequence(:environment_scope) { |n| "production#{n}/*" } end - - trait :with_installed_helm do - application_helm factory: %i(clusters_applications_helm installed) - end end end diff --git a/spec/factories/resource_label_events.rb b/spec/factories/resource_label_events.rb new file mode 100644 index 00000000000..a67ad78c098 --- /dev/null +++ b/spec/factories/resource_label_events.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :resource_label_event do + user { issue.project.creator } + action :add + label + issue + end +end diff --git a/spec/features/projects/clusters/applications_spec.rb b/spec/features/projects/clusters/applications_spec.rb index 71d715237f5..a65ca662350 100644 --- a/spec/features/projects/clusters/applications_spec.rb +++ b/spec/features/projects/clusters/applications_spec.rb @@ -46,14 +46,12 @@ describe 'Clusters Applications', :js do end end - it 'they see status transition' do + it 'he sees status transition' do page.within('.js-cluster-application-row-helm') do # FE sends request and gets the response, then the buttons is "Install" expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Install') - wait_until_helm_created! - Clusters::Cluster.last.application_helm.make_installing! # FE starts polling and update the buttons to "Installing" @@ -85,7 +83,7 @@ describe 'Clusters Applications', :js do end end - it 'they see status transition' do + it 'he sees status transition' do page.within('.js-cluster-application-row-ingress') do # FE sends request and gets the response, then the buttons is "Install" expect(page).to have_css('.js-cluster-application-install-button[disabled]') @@ -118,14 +116,4 @@ describe 'Clusters Applications', :js do end end end - - def wait_until_helm_created! - retries = 0 - - while Clusters::Cluster.last.application_helm.nil? - raise "Timed out waiting for helm application to be created in DB" if (retries += 1) > 3 - - sleep(1) - end - end end diff --git a/spec/features/projects/labels/search_labels_spec.rb b/spec/features/projects/labels/search_labels_spec.rb new file mode 100644 index 00000000000..2d5a138c3cc --- /dev/null +++ b/spec/features/projects/labels/search_labels_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Search for labels', :js do + let(:user) { create(:user) } + let(:project) { create(:project) } + let!(:label1) { create(:label, title: 'Foo', description: 'Lorem ipsum', project: project) } + let!(:label2) { create(:label, title: 'Bar', description: 'Fusce consequat', project: project) } + + before do + project.add_maintainer(user) + sign_in(user) + + visit project_labels_path(project) + end + + it 'searches for label by title' do + fill_in 'label-search', with: 'Bar' + find('#label-search').native.send_keys(:enter) + + expect(page).to have_content(label2.title) + expect(page).to have_content(label2.description) + expect(page).not_to have_content(label1.title) + expect(page).not_to have_content(label1.description) + end + + it 'searches for label by title' do + fill_in 'label-search', with: 'Lorem' + find('#label-search').native.send_keys(:enter) + + expect(page).to have_content(label1.title) + expect(page).to have_content(label1.description) + expect(page).not_to have_content(label2.title) + expect(page).not_to have_content(label2.description) + end + + it 'shows nothing found message' do + fill_in 'label-search', with: 'nonexistent' + find('#label-search').native.send_keys(:enter) + + expect(page).to have_content('No labels with such name or description') + expect(page).not_to have_content(label1.title) + expect(page).not_to have_content(label1.description) + expect(page).not_to have_content(label2.title) + expect(page).not_to have_content(label2.description) + end + + context 'priority labels' do + let!(:label_priority) { create(:label_priority, label: label1, project: project) } + + it 'searches for priority label' do + fill_in 'label-search', with: 'Foo' + find('#label-search').native.send_keys(:enter) + + page.within('.prioritized-labels') do + expect(page).to have_content(label1.title) + expect(page).to have_content(label1.description) + end + + page.within('.other-labels') do + expect(page).to have_content('No other labels with such name or description') + end + end + + it 'searches for other label' do + fill_in 'label-search', with: 'Bar' + find('#label-search').native.send_keys(:enter) + + page.within('.prioritized-labels') do + expect(page).to have_content('No prioritised labels with such name or description') + end + + page.within('.other-labels') do + expect(page).to have_content(label2.title) + expect(page).to have_content(label2.description) + end + end + end +end diff --git a/spec/features/projects/user_uses_shortcuts_spec.rb b/spec/features/projects/user_uses_shortcuts_spec.rb index df9ee69aadb..64f9a4fcd39 100644 --- a/spec/features/projects/user_uses_shortcuts_spec.rb +++ b/spec/features/projects/user_uses_shortcuts_spec.rb @@ -9,6 +9,8 @@ describe 'User uses shortcuts', :js do sign_in(user) visit(project_path(project)) + + wait_for_requests end context 'when navigating to the Project pages' do diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 21891b9ccda..44758f862a8 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -3,28 +3,40 @@ require 'spec_helper' describe 'Login' do include TermsHelper - it 'Successful user signin invalidates password reset token' do - user = create(:user) + before do + stub_authentication_activity_metrics(debug: true) + end + + describe 'password reset token after successful sign in' do + it 'invalidates password reset token' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + user = create(:user) - expect(user.reset_password_token).to be_nil + expect(user.reset_password_token).to be_nil - visit new_user_password_path - fill_in 'user_email', with: user.email - click_button 'Reset password' + visit new_user_password_path + fill_in 'user_email', with: user.email + click_button 'Reset password' - user.reload - expect(user.reset_password_token).not_to be_nil + user.reload + expect(user.reset_password_token).not_to be_nil - find('a[href="#login-pane"]').click - gitlab_sign_in(user) - expect(current_path).to eq root_path + find('a[href="#login-pane"]').click + gitlab_sign_in(user) + expect(current_path).to eq root_path - user.reload - expect(user.reset_password_token).to be_nil + user.reload + expect(user.reset_password_token).to be_nil + end end describe 'initial login after setup' do it 'allows the initial admin to create a password' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + # This behavior is dependent on there only being one user User.delete_all @@ -56,6 +68,11 @@ describe 'Login' do describe 'with a blocked account' do it 'prevents the user from logging in' do + expect(authentication_metrics) + .to increment(:user_blocked_counter) + .and increment(:user_unauthenticated_counter) + .and increment(:user_session_destroyed_counter).twice + user = create(:user, :blocked) gitlab_sign_in(user) @@ -64,6 +81,11 @@ describe 'Login' do end it 'does not update Devise trackable attributes', :clean_gitlab_redis_shared_state do + expect(authentication_metrics) + .to increment(:user_blocked_counter) + .and increment(:user_unauthenticated_counter) + .and increment(:user_session_destroyed_counter).twice + user = create(:user, :blocked) expect { gitlab_sign_in(user) }.not_to change { user.reload.sign_in_count } @@ -72,13 +94,22 @@ describe 'Login' do describe 'with the ghost user' do it 'disallows login' do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + gitlab_sign_in(User.ghost) expect(page).to have_content('Invalid Login or password.') end it 'does not update Devise trackable attributes', :clean_gitlab_redis_shared_state do - expect { gitlab_sign_in(User.ghost) }.not_to change { User.ghost.reload.sign_in_count } + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + + expect { gitlab_sign_in(User.ghost) } + .not_to change { User.ghost.reload.sign_in_count } end end @@ -93,17 +124,30 @@ describe 'Login' do before do gitlab_sign_in(user, remember: true) + expect(page).to have_content('Two-Factor Authentication') end it 'does not show a "You are already signed in." error message' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code(user.current_otp) + expect(page).not_to have_content('You are already signed in.') end context 'using one-time code' do it 'allows login with valid code' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code(user.current_otp) + expect(current_path).to eq root_path end @@ -114,11 +158,20 @@ describe 'Login' do end it 'blocks login with invalid code' do + # TODO invalid 2FA code does not generate any events + # See gitlab-org/gitlab-ce#49785 + enter_code('foo') + expect(page).to have_content('Invalid two-factor code') end it 'allows login with invalid code, then valid code' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code('foo') expect(page).to have_content('Invalid two-factor code') @@ -139,16 +192,33 @@ describe 'Login' do context 'with valid code' do it 'allows login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code(codes.sample) + expect(current_path).to eq root_path end it 'invalidates the used code' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + expect { enter_code(codes.sample) } .to change { user.reload.otp_backup_codes.size }.by(-1) end it 'invalidates backup codes twice in a row' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter).twice + .and increment(:user_session_override_counter).twice + .and increment(:user_two_factor_authenticated_counter).twice + .and increment(:user_session_destroyed_counter) + random_code = codes.delete(codes.sample) expect { enter_code(random_code) } .to change { user.reload.otp_backup_codes.size }.by(-1) @@ -163,6 +233,9 @@ describe 'Login' do context 'with invalid code' do it 'blocks login' do + # TODO, invalid two factor authentication does not increment + # metrics / counters, see gitlab-org/gitlab-ce#49785 + code = codes.sample expect(user.invalidate_otp_backup_code!(code)).to eq true @@ -176,7 +249,7 @@ describe 'Login' do end end - context 'logging in via OAuth' do + context 'when logging in via OAuth' do let(:user) { create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')} let(:mock_saml_response) do File.read('spec/fixtures/authentication/saml_response.xml') @@ -185,49 +258,80 @@ describe 'Login' do before do stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config_with_upstream_two_factor_authn_contexts]) - gitlab_sign_in_via('saml', user, 'my-uid', mock_saml_response) end context 'when authn_context is worth two factors' do let(:mock_saml_response) do File.read('spec/fixtures/authentication/saml_response.xml') - .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') + .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', + 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') end it 'signs user in without prompting for second factor' do + # TODO, OAuth authentication does not fire events, + # see gitlab-org/gitlab-ce#49786 + + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + + sign_in_using_saml! + expect(page).not_to have_content('Two-Factor Authentication') expect(current_path).to eq root_path end end - context 'when authn_context is not worth two factors' do + context 'when two factor authentication is required' do it 'shows 2FA prompt after OAuth login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + + sign_in_using_saml! + expect(page).to have_content('Two-Factor Authentication') + enter_code(user.current_otp) + expect(current_path).to eq root_path end end + + def sign_in_using_saml! + gitlab_sign_in_via('saml', user, 'my-uid', mock_saml_response) + end end end describe 'without two-factor authentication' do - let(:user) { create(:user) } + context 'with correct username and password' do + let(:user) { create(:user) } - it 'allows basic login' do - gitlab_sign_in(user) - expect(current_path).to eq root_path - end + it 'allows basic login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) - it 'does not show a "You are already signed in." error message' do - gitlab_sign_in(user) - expect(page).not_to have_content('You are already signed in.') + gitlab_sign_in(user) + + expect(current_path).to eq root_path + expect(page).not_to have_content('You are already signed in.') + end end - it 'blocks invalid login' do - user = create(:user, password: 'not-the-default') + context 'with invalid username and password' do + let(:user) { create(:user, password: 'not-the-default') } - gitlab_sign_in(user) - expect(page).to have_content('Invalid Login or password.') + it 'blocks invalid login' do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + + gitlab_sign_in(user) + + expect(page).to have_content('Invalid Login or password.') + end end end @@ -243,18 +347,26 @@ describe 'Login' do context 'with grace period defined' do before do stub_application_setting(two_factor_grace_period: 48) - gitlab_sign_in(user) end context 'within the grace period' do it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content('The global settings require you to enable Two-Factor Authentication for your account. You need to do this before ') end it 'allows skipping two-factor configuration', :js do - expect(current_path).to eq profile_two_factor_auth_path + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path click_link 'Configure it later' expect(current_path).to eq root_path end @@ -264,6 +376,11 @@ describe 'Login' do let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) } it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The global settings require you to enable Two-Factor Authentication for your account.' @@ -271,6 +388,11 @@ describe 'Login' do end it 'disallows skipping two-factor configuration', :js do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).not_to have_link('Configure it later') end @@ -280,10 +402,14 @@ describe 'Login' do context 'without grace period defined' do before do stub_application_setting(two_factor_grace_period: 0) - gitlab_sign_in(user) end it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The global settings require you to enable Two-Factor Authentication for your account.' @@ -303,11 +429,15 @@ describe 'Login' do context 'with grace period defined' do before do stub_application_setting(two_factor_grace_period: 48) - gitlab_sign_in(user) end context 'within the grace period' do it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ @@ -316,8 +446,12 @@ describe 'Login' do end it 'allows skipping two-factor configuration', :js do - expect(current_path).to eq profile_two_factor_auth_path + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + gitlab_sign_in(user) + + expect(current_path).to eq profile_two_factor_auth_path click_link 'Configure it later' expect(current_path).to eq root_path end @@ -327,6 +461,11 @@ describe 'Login' do let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) } it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ @@ -335,6 +474,11 @@ describe 'Login' do end it 'disallows skipping two-factor configuration', :js do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).not_to have_link('Configure it later') end @@ -344,10 +488,14 @@ describe 'Login' do context 'without grace period defined' do before do stub_application_setting(two_factor_grace_period: 0) - gitlab_sign_in(user) end it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ @@ -431,6 +579,9 @@ describe 'Login' do end it 'asks to accept the terms on first login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -447,6 +598,9 @@ describe 'Login' do end it 'does not ask for terms when the user already accepted them' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + accept_terms(user) visit new_user_session_path @@ -467,6 +621,9 @@ describe 'Login' do context 'when the user did not enable 2FA' do it 'asks to set 2FA before asking to accept the terms' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -495,6 +652,11 @@ describe 'Login' do end it 'asks the user to accept the terms' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -518,6 +680,9 @@ describe 'Login' do end it 'asks the user to accept the terms before setting a new password' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -546,6 +711,10 @@ describe 'Login' do end it 'asks the user to accept the terms before setting an email' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + gitlab_sign_in_via('saml', user, 'my-uid') expect_to_be_on_terms_page diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb index 899d0d22819..eb2a4576e30 100644 --- a/spec/finders/labels_finder_spec.rb +++ b/spec/finders/labels_finder_spec.rb @@ -14,7 +14,7 @@ describe LabelsFinder do let(:project_4) { create(:project, :public) } let(:project_5) { create(:project, namespace: group_1) } - let!(:project_label_1) { create(:label, project: project_1, title: 'Label 1') } + let!(:project_label_1) { create(:label, project: project_1, title: 'Label 1', description: 'awesome label') } let!(:project_label_2) { create(:label, project: project_2, title: 'Label 2') } let!(:project_label_4) { create(:label, project: project_4, title: 'Label 4') } let!(:project_label_5) { create(:label, project: project_5, title: 'Label 5') } @@ -196,5 +196,19 @@ describe LabelsFinder do expect(finder.execute).to be_empty end end + + context 'search by title and description' do + it 'returns labels with a partially matching title' do + finder = described_class.new(user, search: '(group)') + + expect(finder.execute).to eq [group_label_1] + end + + it 'returns labels with a partially matching description' do + finder = described_class.new(user, search: 'awesome') + + expect(finder.execute).to eq [project_label_1] + end + end end end diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 3b741d51598..a2ac4d238c7 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -81,6 +81,7 @@ "can_revert_on_current_merge_request": { "type": ["boolean", "null"] }, "can_cherry_pick_on_current_merge_request": { "type": ["boolean", "null"] }, "can_create_note": { "type": "boolean" }, + "can_create_issue": { "type": "boolean" }, "can_update": { "type": "boolean" } }, "additionalProperties": false diff --git a/spec/javascripts/.eslintrc.yml b/spec/javascripts/.eslintrc.yml index 78e2f3b521f..5525c9f5bd0 100644 --- a/spec/javascripts/.eslintrc.yml +++ b/spec/javascripts/.eslintrc.yml @@ -30,7 +30,6 @@ rules: jasmine/no-spec-dupes: - warn - branch - no-console: off prefer-arrow-callback: off import/no-unresolved: - error diff --git a/spec/javascripts/datetime_utility_spec.js b/spec/javascripts/datetime_utility_spec.js index e224ed46d18..492171684dc 100644 --- a/spec/javascripts/datetime_utility_spec.js +++ b/spec/javascripts/datetime_utility_spec.js @@ -162,7 +162,6 @@ describe('getTimeframeWindowFrom', () => { const timeframe = datetimeUtility.getTimeframeWindowFrom(startDate, 5); expect(timeframe.length).toBe(5); timeframe.forEach((timeframeItem, index) => { - console.log(timeframeItem); expect(timeframeItem.getFullYear() === mockTimeframe[index].getFullYear()).toBe(true); expect(timeframeItem.getMonth() === mockTimeframe[index].getMonth()).toBe(true); expect(timeframeItem.getDate() === mockTimeframe[index].getDate()).toBeTruthy(); diff --git a/spec/javascripts/diffs/components/diff_file_header_spec.js b/spec/javascripts/diffs/components/diff_file_header_spec.js index 860a976e7cd..92b2004c4d7 100644 --- a/spec/javascripts/diffs/components/diff_file_header_spec.js +++ b/spec/javascripts/diffs/components/diff_file_header_spec.js @@ -11,7 +11,9 @@ const discussionFixture = 'merge_requests/diff_discussion.json'; describe('diff_file_header', () => { let vm; let props; + const diffDiscussionMock = getJSONFixture(discussionFixture)[0]; const Component = Vue.extend(DiffFileHeader); + const store = new Vuex.Store({ modules: { diffs: diffsModule, @@ -20,7 +22,6 @@ describe('diff_file_header', () => { }); beforeEach(() => { - const diffDiscussionMock = getJSONFixture(discussionFixture)[0]; const diffFile = convertObjectPropsToCamelCase(diffDiscussionMock.diff_file, { deep: true }); props = { diffFile, @@ -409,7 +410,7 @@ describe('diff_file_header', () => { }); describe('handles toggle discussions', () => { - it('dispatches toggleFileDiscussions when user clicks on toggle discussions button', () => { + it('renders a disabled button when diff has no discussions', () => { const propsCopy = Object.assign({}, props); propsCopy.diffFile.submodule = false; propsCopy.diffFile.blob = { @@ -428,11 +429,44 @@ describe('diff_file_header', () => { store, }); - spyOn(vm, 'toggleFileDiscussions'); - - vm.$el.querySelector('.js-btn-vue-toggle-comments').click(); - - expect(vm.toggleFileDiscussions).toHaveBeenCalled(); + expect( + vm.$el.querySelector('.js-btn-vue-toggle-comments').getAttribute('disabled'), + ).toEqual('disabled'); + }); + + describe('with discussions', () => { + it('dispatches toggleFileDiscussions when user clicks on toggle discussions button', () => { + const propsCopy = Object.assign({}, props); + propsCopy.diffFile.submodule = false; + propsCopy.diffFile.blob = { + id: '848ed9407c6730ff16edb3dd24485a0eea24292a', + path: 'lib/base.js', + name: 'base.js', + mode: '100644', + readableText: true, + icon: 'file-text-o', + }; + propsCopy.addMergeRequestButtons = true; + propsCopy.diffFile.deletedFile = true; + + const discussionGetter = () => [diffDiscussionMock]; + notesModule.getters.discussions = discussionGetter; + vm = mountComponentWithStore(Component, { + props: propsCopy, + store: new Vuex.Store({ + modules: { + diffs: diffsModule, + notes: notesModule, + }, + }), + }); + + spyOn(vm, 'toggleFileDiscussions'); + + vm.$el.querySelector('.js-btn-vue-toggle-comments').click(); + + expect(vm.toggleFileDiscussions).toHaveBeenCalled(); + }); }); }); }); diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js index 6210d4a7124..7706c32d24d 100644 --- a/spec/javascripts/diffs/store/getters_spec.js +++ b/spec/javascripts/diffs/store/getters_spec.js @@ -166,6 +166,24 @@ describe('Diffs Module Getters', () => { }); }); + describe('diffHasDiscussions', () => { + it('returns true when getDiffFileDiscussions returns discussions', () => { + expect( + getters.diffHasDiscussions(localState, { + getDiffFileDiscussions: () => [discussionMock], + })(diffFileMock), + ).toEqual(true); + }); + + it('returns false when getDiffFileDiscussions returns no discussions', () => { + expect( + getters.diffHasDiscussions(localState, { + getDiffFileDiscussions: () => [], + })(diffFileMock), + ).toEqual(false); + }); + }); + describe('getDiffFileDiscussions', () => { it('returns an array with discussions when fileHash matches and the discussion belongs to a diff', () => { discussionMock.diff_file.file_hash = diffFileMock.fileHash; diff --git a/spec/javascripts/pdf/page_spec.js b/spec/javascripts/pdf/page_spec.js index 9c686748c10..ff1bfd7f650 100644 --- a/spec/javascripts/pdf/page_spec.js +++ b/spec/javascripts/pdf/page_spec.js @@ -30,7 +30,7 @@ describe('Page component', () => { done(); }) .catch((error) => { - console.error(error); + done.fail(error); }); }); diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index 59e472789e2..e77236c40ef 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -1,4 +1,6 @@ -/* eslint-disable jasmine/no-global-setup, jasmine/no-unsafe-spy, no-underscore-dangle */ +/* eslint-disable + jasmine/no-global-setup, jasmine/no-unsafe-spy, no-underscore-dangle, no-console +*/ import $ from 'jquery'; import 'vendor/jasmine-jquery'; diff --git a/spec/lib/gitlab/auth/activity_spec.rb b/spec/lib/gitlab/auth/activity_spec.rb new file mode 100644 index 00000000000..07854cb1eba --- /dev/null +++ b/spec/lib/gitlab/auth/activity_spec.rb @@ -0,0 +1,30 @@ +require 'fast_spec_helper' + +describe Gitlab::Auth::Activity do + describe '.each_counter' do + it 'has all static counters defined' do + described_class.each_counter do |counter| + expect(described_class).to respond_to(counter) + end + end + + it 'has all static incrementers defined' do + described_class.each_counter do |counter| + expect(described_class).to respond_to("#{counter}_increment!") + end + end + + it 'has all counters starting with `user_`' do + described_class.each_counter do |counter| + expect(counter).to start_with('user_') + end + end + + it 'yields counter method, name and description' do + described_class.each_counter do |method, name, description| + expect(method).to eq "#{name}_counter" + expect(description).to start_with('Counter of') + end + end + end +end diff --git a/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb index 43b68e69131..13c09b9cb9b 100644 --- a/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb +++ b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb @@ -3,24 +3,30 @@ require 'spec_helper' describe Gitlab::Auth::BlockedUserTracker do set(:user) { create(:user) } - describe '.log_if_user_blocked' do + describe '#log_blocked_user_activity!' do it 'does not log if user failed to login due to undefined reason' do expect_any_instance_of(SystemHooksService).not_to receive(:execute_hooks_for) - expect(described_class.log_if_user_blocked({})).to be_nil + tracker = described_class.new({}) + + expect(tracker.user).to be_nil + expect(tracker.user_blocked?).to be_falsey + expect(tracker.log_blocked_user_activity!).to be_nil end it 'gracefully handles malformed environment variables' do - env = { 'warden.options' => 'test' } + tracker = described_class.new({ 'warden.options' => 'test' }) - expect(described_class.log_if_user_blocked(env)).to be_nil + expect(tracker.user).to be_nil + expect(tracker.user_blocked?).to be_falsey + expect(tracker.log_blocked_user_activity!).to be_nil end context 'failed login due to blocked user' do let(:base_env) { { 'warden.options' => { message: User::BLOCKED_MESSAGE } } } let(:env) { base_env.merge(request_env) } - subject { described_class.log_if_user_blocked(env) } + subject { described_class.new(env) } before do expect_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).with(user, :failed_login) @@ -32,14 +38,17 @@ describe Gitlab::Auth::BlockedUserTracker do it 'logs a blocked user' do user.block! - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.user_blocked?).to be true + expect(subject.log_blocked_user_activity!).to be_truthy end it 'logs a blocked user by e-mail' do user.block! env[described_class::ACTIVE_RECORD_REQUEST_PARAMS]['user']['login'] = user.email - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.log_blocked_user_activity!).to be_truthy end end @@ -49,13 +58,17 @@ describe Gitlab::Auth::BlockedUserTracker do it 'logs a blocked user' do user.block! - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.user_blocked?).to be true + expect(subject.log_blocked_user_activity!).to be_truthy end it 'logs a LDAP blocked user' do user.ldap_block! - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.user_blocked?).to be true + expect(subject.log_blocked_user_activity!).to be_truthy end end end diff --git a/spec/lib/gitlab/ci/status/build/failed_spec.rb b/spec/lib/gitlab/ci/status/build/failed_spec.rb index cadb424ea2c..b6676b40fd3 100644 --- a/spec/lib/gitlab/ci/status/build/failed_spec.rb +++ b/spec/lib/gitlab/ci/status/build/failed_spec.rb @@ -80,4 +80,31 @@ describe Gitlab::Ci::Status::Build::Failed do end end end + + describe 'covers all failure reasons' do + let(:status) { Gitlab::Ci::Status::Failed.new(build, user) } + let(:tooltip) { subject.status_tooltip } + + CommitStatus.failure_reasons.keys.each do |failure_reason| + context failure_reason do + before do + build.failure_reason = failure_reason + end + + it "is a valid status" do + expect { tooltip }.not_to raise_error + end + end + end + + context 'invalid failure message' do + before do + expect(build).to receive(:failure_reason) { 'invalid failure message' } + end + + it "is an invalid status" do + expect { tooltip }.to raise_error(/key not found:/) + end + end + end end diff --git a/spec/lib/gitlab/graphs/commits_spec.rb b/spec/lib/gitlab/graphs/commits_spec.rb index b2084f56640..530d4a981bf 100644 --- a/spec/lib/gitlab/graphs/commits_spec.rb +++ b/spec/lib/gitlab/graphs/commits_spec.rb @@ -29,7 +29,7 @@ describe Gitlab::Graphs::Commits do context 'with commits from yesterday and today' do subject { described_class.new([commit2, commit1_yesterday]) } describe '#commit_per_day' do - it { expect(subject.commit_per_day).to eq 1 } + it { expect(subject.commit_per_day).to eq 1.0 } end describe '#duration' do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index db5aab0cd76..c175dc1e4dd 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -7,6 +7,7 @@ issues: - updated_by - milestone - notes +- resource_label_events - label_links - labels - last_edited_by @@ -76,6 +77,7 @@ merge_requests: - updated_by - milestone - notes +- resource_label_events - label_links - labels - last_edited_by diff --git a/spec/lib/gitlab/kubernetes/config_map_spec.rb b/spec/lib/gitlab/kubernetes/config_map_spec.rb index fe65d03875f..e253b291277 100644 --- a/spec/lib/gitlab/kubernetes/config_map_spec.rb +++ b/spec/lib/gitlab/kubernetes/config_map_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::Kubernetes::ConfigMap do let(:kubeclient) { double('kubernetes client') } let(:application) { create(:clusters_applications_prometheus) } - let(:config_map) { described_class.new(application.name, application.files) } + let(:config_map) { described_class.new(application.name, application.values) } let(:namespace) { Gitlab::Kubernetes::Helm::NAMESPACE } let(:metadata) do @@ -15,7 +15,7 @@ describe Gitlab::Kubernetes::ConfigMap do end describe '#generate' do - let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: application.files) } + let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: { values: application.values }) } subject { config_map.generate } it 'should build a Kubeclient Resource' do diff --git a/spec/lib/gitlab/kubernetes/helm/api_spec.rb b/spec/lib/gitlab/kubernetes/helm/api_spec.rb index 341f71a3e49..6e9b4ca0869 100644 --- a/spec/lib/gitlab/kubernetes/helm/api_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/api_spec.rb @@ -39,7 +39,7 @@ describe Gitlab::Kubernetes::Helm::Api do end context 'with a ConfigMap' do - let(:resource) { Gitlab::Kubernetes::ConfigMap.new(application.name, application.files).generate } + let(:resource) { Gitlab::Kubernetes::ConfigMap.new(application.name, application.values).generate } it 'creates a ConfigMap on kubeclient' do expect(client).to receive(:create_config_map).with(resource).once diff --git a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb index d50616e95e8..7be8be54d5e 100644 --- a/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/base_command_spec.rb @@ -2,25 +2,7 @@ require 'spec_helper' describe Gitlab::Kubernetes::Helm::BaseCommand do let(:application) { create(:clusters_applications_helm) } - let(:test_class) do - Class.new do - include Gitlab::Kubernetes::Helm::BaseCommand - - def name - "test-class-name" - end - - def files - { - some: 'value' - } - end - end - end - - let(:base_command) do - test_class.new - end + let(:base_command) { described_class.new(application.name) } subject { base_command } @@ -36,9 +18,15 @@ describe Gitlab::Kubernetes::Helm::BaseCommand do end end + describe '#config_map?' do + subject { base_command.config_map? } + + it { is_expected.to be_falsy } + end + describe '#pod_name' do subject { base_command.pod_name } - it { is_expected.to eq('install-test-class-name') } + it { is_expected.to eq('install-helm') } end end diff --git a/spec/lib/gitlab/kubernetes/helm/certificate_spec.rb b/spec/lib/gitlab/kubernetes/helm/certificate_spec.rb deleted file mode 100644 index f8d00c95a36..00000000000 --- a/spec/lib/gitlab/kubernetes/helm/certificate_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Kubernetes::Helm::Certificate do - describe '.generate_root' do - subject { described_class.generate_root } - - it 'should generate a root CA that expires a long way in the future' do - expect(subject.cert.not_after).to be > 999.years.from_now - end - end - - describe '#issue' do - subject { described_class.generate_root.issue } - - it 'should generate a cert that expires soon' do - expect(subject.cert.not_after).to be < 60.minutes.from_now - end - - context 'passing in INFINITE_EXPIRY' do - subject { described_class.generate_root.issue(expires_in: described_class::INFINITE_EXPIRY) } - - it 'should generate a cert that expires a long way in the future' do - expect(subject.cert.not_after).to be > 999.years.from_now - end - end - end -end diff --git a/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb index dcbc046cf00..89e36a298f8 100644 --- a/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/init_command_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' describe Gitlab::Kubernetes::Helm::InitCommand do let(:application) { create(:clusters_applications_helm) } - let(:commands) { 'helm init --tiller-tls --tiller-tls-verify --tls-ca-cert /data/helm/helm/config/ca.pem --tiller-tls-cert /data/helm/helm/config/cert.pem --tiller-tls-key /data/helm/helm/config/key.pem >/dev/null' } + let(:commands) { 'helm init >/dev/null' } - subject { described_class.new(name: application.name, files: {}) } + subject { described_class.new(application.name) } it_behaves_like 'helm commands' end diff --git a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb index 51221e54d89..25c6fa3b9a3 100644 --- a/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/install_command_spec.rb @@ -1,82 +1,83 @@ require 'rails_helper' describe Gitlab::Kubernetes::Helm::InstallCommand do - let(:files) { { 'ca.pem': 'some file content' } } - let(:repository) { 'https://repository.example.com' } - let(:version) { '1.2.3' } - - let(:install_command) do - described_class.new( - name: 'app-name', - chart: 'chart-name', - files: files, - version: version, repository: repository - ) - end + let(:application) { create(:clusters_applications_prometheus) } + let(:namespace) { Gitlab::Kubernetes::Helm::NAMESPACE } + let(:install_command) { application.install_command } subject { install_command } - it_behaves_like 'helm commands' do - let(:commands) do - <<~EOS - helm init --client-only >/dev/null - helm repo add app-name https://repository.example.com - helm install --tls --tls-ca-cert /data/helm/app-name/config/ca.pem --tls-cert /data/helm/app-name/config/cert.pem --tls-key /data/helm/app-name/config/key.pem chart-name --name app-name --version 1.2.3 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null - EOS + context 'for ingress' do + let(:application) { create(:clusters_applications_ingress) } + + it_behaves_like 'helm commands' do + let(:commands) do + <<~EOS + helm init --client-only >/dev/null + helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null + EOS + end end end - context 'when there is no repository' do - let(:repository) { nil } + context 'for prometheus' do + let(:application) { create(:clusters_applications_prometheus) } it_behaves_like 'helm commands' do let(:commands) do <<~EOS helm init --client-only >/dev/null - helm install --tls --tls-ca-cert /data/helm/app-name/config/ca.pem --tls-cert /data/helm/app-name/config/cert.pem --tls-key /data/helm/app-name/config/key.pem chart-name --name app-name --version 1.2.3 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null + helm install #{application.chart} --name #{application.name} --version #{application.version} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null EOS end end end - context 'when there is no ca.pem file' do - let(:files) { { 'file.txt': 'some content' } } + context 'for runner' do + let(:ci_runner) { create(:ci_runner) } + let(:application) { create(:clusters_applications_runner, runner: ci_runner) } it_behaves_like 'helm commands' do let(:commands) do <<~EOS helm init --client-only >/dev/null - helm repo add app-name https://repository.example.com - helm install chart-name --name app-name --version 1.2.3 --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null + helm repo add #{application.name} #{application.repository} + helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null EOS end end end - context 'when there is no version' do - let(:version) { nil } + context 'for jupyter' do + let(:application) { create(:clusters_applications_jupyter) } it_behaves_like 'helm commands' do let(:commands) do <<~EOS helm init --client-only >/dev/null - helm repo add app-name https://repository.example.com - helm install --tls --tls-ca-cert /data/helm/app-name/config/ca.pem --tls-cert /data/helm/app-name/config/cert.pem --tls-key /data/helm/app-name/config/key.pem chart-name --name app-name --namespace gitlab-managed-apps -f /data/helm/app-name/config/values.yaml >/dev/null + helm repo add #{application.name} #{application.repository} + helm install #{application.chart} --name #{application.name} --namespace #{namespace} -f /data/helm/#{application.name}/config/values.yaml >/dev/null EOS end end end + describe '#config_map?' do + subject { install_command.config_map? } + + it { is_expected.to be_truthy } + end + describe '#config_map_resource' do let(:metadata) do { - name: "values-content-configuration-app-name", - namespace: 'gitlab-managed-apps', - labels: { name: "values-content-configuration-app-name" } + name: "values-content-configuration-#{application.name}", + namespace: namespace, + labels: { name: "values-content-configuration-#{application.name}" } } end - let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: files) } + let(:resource) { ::Kubeclient::Resource.new(metadata: metadata, data: { values: application.values }) } subject { install_command.config_map_resource } diff --git a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb index ec64193c0b2..43adc80d576 100644 --- a/spec/lib/gitlab/kubernetes/helm/pod_spec.rb +++ b/spec/lib/gitlab/kubernetes/helm/pod_spec.rb @@ -2,13 +2,14 @@ require 'rails_helper' describe Gitlab::Kubernetes::Helm::Pod do describe '#generate' do - let(:app) { create(:clusters_applications_prometheus) } + let(:cluster) { create(:cluster) } + let(:app) { create(:clusters_applications_prometheus, cluster: cluster) } let(:command) { app.install_command } let(:namespace) { Gitlab::Kubernetes::Helm::NAMESPACE } subject { described_class.new(command, namespace) } - context 'with a command' do + shared_examples 'helm pod' do it 'should generate a Kubeclient::Resource' do expect(subject.generate).to be_a_kind_of(Kubeclient::Resource) end @@ -40,6 +41,10 @@ describe Gitlab::Kubernetes::Helm::Pod do spec = subject.generate.spec expect(spec.restartPolicy).to eq('Never') end + end + + context 'with a install command' do + it_behaves_like 'helm pod' it 'should include volumes for the container' do container = subject.generate.spec.containers.first @@ -55,8 +60,24 @@ describe Gitlab::Kubernetes::Helm::Pod do it 'should mount configMap specification in the volume' do volume = subject.generate.spec.volumes.first expect(volume.configMap['name']).to eq("values-content-configuration-#{app.name}") - expect(volume.configMap['items'].first['key']).to eq(:'values.yaml') - expect(volume.configMap['items'].first['path']).to eq(:'values.yaml') + expect(volume.configMap['items'].first['key']).to eq('values') + expect(volume.configMap['items'].first['path']).to eq('values.yaml') + end + end + + context 'with a init command' do + let(:app) { create(:clusters_applications_helm, cluster: cluster) } + + it_behaves_like 'helm pod' + + it 'should not include volumeMounts inside the container' do + container = subject.generate.spec.containers.first + expect(container.volumeMounts).to be_nil + end + + it 'should not a volume inside the specification' do + spec = subject.generate.spec + expect(spec.volumes).to be_nil end end end diff --git a/spec/models/ci/build_runner_session_spec.rb b/spec/models/ci/build_runner_session_spec.rb index 7183957aa50..35622366829 100644 --- a/spec/models/ci/build_runner_session_spec.rb +++ b/spec/models/ci/build_runner_session_spec.rb @@ -29,7 +29,7 @@ describe Ci::BuildRunnerSession, model: true do it 'adds Authorization header if authorization is present' do subject.authorization = 'whatever' - expect(terminal_specification[:headers]).to include(Authorization: 'whatever') + expect(terminal_specification[:headers]).to include(Authorization: ['whatever']) end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index e4fa04baae6..6955f7f4cd8 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2409,18 +2409,18 @@ describe Ci::Build do end end - describe 'state transition: any => [:running]' do + describe '#has_valid_build_dependencies?' do shared_examples 'validation is active' do context 'when depended job has not been completed yet' do let!(:pre_stage_job) { create(:ci_build, :manual, pipeline: pipeline, name: 'test', stage_idx: 0) } - it { expect { job.run! }.not_to raise_error } + it { expect(job).to have_valid_build_dependencies } end context 'when artifacts of depended job has been expired' do let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) } - it { expect { job.run! }.to raise_error(Ci::Build::MissingDependenciesError) } + it { expect(job).not_to have_valid_build_dependencies } end context 'when artifacts of depended job has been erased' do @@ -2430,7 +2430,7 @@ describe Ci::Build do pre_stage_job.erase end - it { expect { job.run! }.to raise_error(Ci::Build::MissingDependenciesError) } + it { expect(job).not_to have_valid_build_dependencies } end end @@ -2438,12 +2438,13 @@ describe Ci::Build do context 'when depended job has not been completed yet' do let!(:pre_stage_job) { create(:ci_build, :manual, pipeline: pipeline, name: 'test', stage_idx: 0) } - it { expect { job.run! }.not_to raise_error } + it { expect(job).to have_valid_build_dependencies } end + context 'when artifacts of depended job has been expired' do let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) } - it { expect { job.run! }.not_to raise_error } + it { expect(job).to have_valid_build_dependencies } end context 'when artifacts of depended job has been erased' do @@ -2453,7 +2454,7 @@ describe Ci::Build do pre_stage_job.erase end - it { expect { job.run! }.not_to raise_error } + it { expect(job).to have_valid_build_dependencies } end end @@ -2469,13 +2470,13 @@ describe Ci::Build do context 'when "dependencies" keyword is not defined' do let(:options) { {} } - it { expect { job.run! }.not_to raise_error } + it { expect(job).to have_valid_build_dependencies } end context 'when "dependencies" keyword is empty' do let(:options) { { dependencies: [] } } - it { expect { job.run! }.not_to raise_error } + it { expect(job).to have_valid_build_dependencies } end context 'when "dependencies" keyword is specified' do @@ -2812,4 +2813,76 @@ describe Ci::Build do end end end + + describe '#publishes_artifacts_reports?' do + let(:build) { create(:ci_build, options: options) } + + subject { build.publishes_artifacts_reports? } + + context 'when artifacts reports are defined' do + let(:options) do + { artifacts: { reports: { junit: "junit.xml" } } } + end + + it { is_expected.to be_truthy } + end + + context 'when artifacts reports missing defined' do + let(:options) do + { artifacts: { paths: ["file.txt"] } } + end + + it { is_expected.to be_falsey } + end + + context 'when options are missing' do + let(:options) { nil } + + it { is_expected.to be_falsey } + end + end + + describe '#runner_required_feature_names' do + let(:build) { create(:ci_build, options: options) } + + subject { build.runner_required_feature_names } + + context 'when artifacts reports are defined' do + let(:options) do + { artifacts: { reports: { junit: "junit.xml" } } } + end + + it { is_expected.to include(:upload_multiple_artifacts) } + end + end + + describe '#supported_runner?' do + set(:build) { create(:ci_build) } + + subject { build.supported_runner?(runner_features) } + + context 'when feature is required by build' do + before do + expect(build).to receive(:runner_required_feature_names) do + [:upload_multiple_artifacts] + end + end + + context 'when runner provides given feature' do + let(:runner_features) do + { upload_multiple_artifacts: true } + end + + it { is_expected.to be_truthy } + end + + context 'when runner does not provide given feature' do + let(:runner_features) do + {} + end + + it { is_expected.to be_falsey } + end + end + end end diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb index e5b2bdc8a4e..0eb1e3876e2 100644 --- a/spec/models/clusters/applications/helm_spec.rb +++ b/spec/models/clusters/applications/helm_spec.rb @@ -6,24 +6,13 @@ describe Clusters::Applications::Helm do describe '.installed' do subject { described_class.installed } - let!(:installed_cluster) { create(:clusters_applications_helm, :installed) } + let!(:cluster) { create(:clusters_applications_helm, :installed) } before do create(:clusters_applications_helm, :errored) end - it { is_expected.to contain_exactly(installed_cluster) } - end - - describe '#issue_client_cert' do - let(:application) { create(:clusters_applications_helm) } - subject { application.issue_client_cert } - - it 'returns a new cert' do - is_expected.to be_kind_of(Gitlab::Kubernetes::Helm::Certificate) - expect(subject.cert_string).not_to eq(application.ca_cert) - expect(subject.key_string).not_to eq(application.ca_key) - end + it { is_expected.to contain_exactly(cluster) } end describe '#install_command' do @@ -36,16 +25,5 @@ describe Clusters::Applications::Helm do it 'should be initialized with 1 arguments' do expect(subject.name).to eq('helm') end - - it 'should have cert files' do - expect(subject.files[:'ca.pem']).to be_present - expect(subject.files[:'ca.pem']).to eq(helm.ca_cert) - - expect(subject.files[:'cert.pem']).to be_present - expect(subject.files[:'key.pem']).to be_present - - cert = OpenSSL::X509::Certificate.new(subject.files[:'cert.pem']) - expect(cert.not_after).to be > 999.years.from_now - end end end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index c76aae432f9..bb5b2ef3a47 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -74,43 +74,18 @@ describe Clusters::Applications::Ingress do expect(subject.name).to eq('ingress') expect(subject.chart).to eq('stable/nginx-ingress') expect(subject.version).to be_nil - expect(subject.files).to eq(ingress.files) + expect(subject.values).to eq(ingress.values) end end - describe '#files' do - let(:application) { ingress } - subject { application.files } - let(:values) { subject[:'values.yaml'] } + describe '#values' do + subject { ingress.values } - it 'should include ingress valid keys in values' do - expect(values).to include('image') - expect(values).to include('repository') - expect(values).to include('stats') - expect(values).to include('podAnnotations') - end - - context 'when the helm application does not have a ca_cert' do - before do - application.cluster.application_helm.ca_cert = nil - end - - it 'should not include cert files' do - expect(subject[:'ca.pem']).not_to be_present - expect(subject[:'cert.pem']).not_to be_present - expect(subject[:'key.pem']).not_to be_present - end - end - - it 'should include cert files' do - expect(subject[:'ca.pem']).to be_present - expect(subject[:'ca.pem']).to eq(application.cluster.application_helm.ca_cert) - - expect(subject[:'cert.pem']).to be_present - expect(subject[:'key.pem']).to be_present - - cert = OpenSSL::X509::Certificate.new(subject[:'cert.pem']) - expect(cert.not_after).to be < 60.minutes.from_now + it 'should include ingress valid keys' do + is_expected.to include('image') + is_expected.to include('repository') + is_expected.to include('stats') + is_expected.to include('podAnnotations') end end end diff --git a/spec/models/clusters/applications/jupyter_spec.rb b/spec/models/clusters/applications/jupyter_spec.rb index a8cf44ac8a8..65750141e65 100644 --- a/spec/models/clusters/applications/jupyter_spec.rb +++ b/spec/models/clusters/applications/jupyter_spec.rb @@ -38,46 +38,23 @@ describe Clusters::Applications::Jupyter do expect(subject.chart).to eq('jupyter/jupyterhub') expect(subject.version).to be_nil expect(subject.repository).to eq('https://jupyterhub.github.io/helm-chart/') - expect(subject.files).to eq(jupyter.files) + expect(subject.values).to eq(jupyter.values) end end - describe '#files' do - let(:application) { create(:clusters_applications_jupyter) } - subject { application.files } - let(:values) { subject[:'values.yaml'] } + describe '#values' do + let(:jupyter) { create(:clusters_applications_jupyter) } - it 'should include cert files' do - expect(subject[:'ca.pem']).to be_present - expect(subject[:'ca.pem']).to eq(application.cluster.application_helm.ca_cert) - - expect(subject[:'cert.pem']).to be_present - expect(subject[:'key.pem']).to be_present - - cert = OpenSSL::X509::Certificate.new(subject[:'cert.pem']) - expect(cert.not_after).to be < 60.minutes.from_now - end - - context 'when the helm application does not have a ca_cert' do - before do - application.cluster.application_helm.ca_cert = nil - end - - it 'should not include cert files' do - expect(subject[:'ca.pem']).not_to be_present - expect(subject[:'cert.pem']).not_to be_present - expect(subject[:'key.pem']).not_to be_present - end - end + subject { jupyter.values } it 'should include valid values' do - expect(values).to include('ingress') - expect(values).to include('hub') - expect(values).to include('rbac') - expect(values).to include('proxy') - expect(values).to include('auth') - expect(values).to match(/clientId: '?#{application.oauth_application.uid}/) - expect(values).to match(/callbackUrl: '?#{application.callback_url}/) + is_expected.to include('ingress') + is_expected.to include('hub') + is_expected.to include('rbac') + is_expected.to include('proxy') + is_expected.to include('auth') + is_expected.to include("clientId: #{jupyter.oauth_application.uid}") + is_expected.to include("callbackUrl: #{jupyter.callback_url}") end end end diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 313bd741f88..e4b61552033 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -153,44 +153,21 @@ describe Clusters::Applications::Prometheus do expect(command.name).to eq('prometheus') expect(command.chart).to eq('stable/prometheus') expect(command.version).to eq('6.7.3') - expect(command.files).to eq(prometheus.files) + expect(command.values).to eq(prometheus.values) end end - describe '#files' do - let(:application) { create(:clusters_applications_prometheus) } - subject { application.files } - let(:values) { subject[:'values.yaml'] } - - it 'should include cert files' do - expect(subject[:'ca.pem']).to be_present - expect(subject[:'ca.pem']).to eq(application.cluster.application_helm.ca_cert) - - expect(subject[:'cert.pem']).to be_present - expect(subject[:'key.pem']).to be_present - - cert = OpenSSL::X509::Certificate.new(subject[:'cert.pem']) - expect(cert.not_after).to be < 60.minutes.from_now - end - - context 'when the helm application does not have a ca_cert' do - before do - application.cluster.application_helm.ca_cert = nil - end + describe '#values' do + let(:prometheus) { create(:clusters_applications_prometheus) } - it 'should not include cert files' do - expect(subject[:'ca.pem']).not_to be_present - expect(subject[:'cert.pem']).not_to be_present - expect(subject[:'key.pem']).not_to be_present - end - end + subject { prometheus.values } it 'should include prometheus valid values' do - expect(values).to include('alertmanager') - expect(values).to include('kubeStateMetrics') - expect(values).to include('nodeExporter') - expect(values).to include('pushgateway') - expect(values).to include('serverFiles') + is_expected.to include('alertmanager') + is_expected.to include('kubeStateMetrics') + is_expected.to include('nodeExporter') + is_expected.to include('pushgateway') + is_expected.to include('serverFiles') end end end diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index 65aaa1ee882..b12500d0acd 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -33,55 +33,31 @@ describe Clusters::Applications::Runner do expect(subject.chart).to eq('runner/gitlab-runner') expect(subject.version).to be_nil expect(subject.repository).to eq('https://charts.gitlab.io') - expect(subject.files).to eq(gitlab_runner.files) + expect(subject.values).to eq(gitlab_runner.values) end end - describe '#files' do - let(:application) { create(:clusters_applications_runner, runner: ci_runner) } - - subject { application.files } - let(:values) { subject[:'values.yaml'] } - - it 'should include cert files' do - expect(subject[:'ca.pem']).to be_present - expect(subject[:'ca.pem']).to eq(application.cluster.application_helm.ca_cert) - - expect(subject[:'cert.pem']).to be_present - expect(subject[:'key.pem']).to be_present - - cert = OpenSSL::X509::Certificate.new(subject[:'cert.pem']) - expect(cert.not_after).to be < 60.minutes.from_now - end - - context 'when the helm application does not have a ca_cert' do - before do - application.cluster.application_helm.ca_cert = nil - end + describe '#values' do + let(:gitlab_runner) { create(:clusters_applications_runner, runner: ci_runner) } - it 'should not include cert files' do - expect(subject[:'ca.pem']).not_to be_present - expect(subject[:'cert.pem']).not_to be_present - expect(subject[:'key.pem']).not_to be_present - end - end + subject { gitlab_runner.values } it 'should include runner valid values' do - expect(values).to include('concurrent') - expect(values).to include('checkInterval') - expect(values).to include('rbac') - expect(values).to include('runners') - expect(values).to include('privileged: true') - expect(values).to include('image: ubuntu:16.04') - expect(values).to include('resources') - expect(values).to match(/runnerToken: '?#{ci_runner.token}/) - expect(values).to match(/gitlabUrl: '?#{Gitlab::Routing.url_helpers.root_url}/) + is_expected.to include('concurrent') + is_expected.to include('checkInterval') + is_expected.to include('rbac') + is_expected.to include('runners') + is_expected.to include('privileged: true') + is_expected.to include('image: ubuntu:16.04') + is_expected.to include('resources') + is_expected.to include("runnerToken: #{ci_runner.token}") + is_expected.to include("gitlabUrl: #{Gitlab::Routing.url_helpers.root_url}") end context 'without a runner' do let(:project) { create(:project) } - let(:cluster) { create(:cluster, :with_installed_helm, projects: [project]) } - let(:application) { create(:clusters_applications_runner, cluster: cluster) } + let(:cluster) { create(:cluster, projects: [project]) } + let(:gitlab_runner) { create(:clusters_applications_runner, cluster: cluster) } it 'creates a runner' do expect do @@ -90,18 +66,18 @@ describe Clusters::Applications::Runner do end it 'uses the new runner token' do - expect(values).to match(/runnerToken: '?#{application.reload.runner.token}/) + expect(subject).to include("runnerToken: #{gitlab_runner.reload.runner.token}") end it 'assigns the new runner to runner' do subject - expect(application.reload.runner).to be_project_type + expect(gitlab_runner.reload.runner).to be_project_type end end context 'with duplicated values on vendor/runner/values.yaml' do - let(:stub_values) do + let(:values) do { "concurrent" => 4, "checkInterval" => 3, @@ -120,11 +96,11 @@ describe Clusters::Applications::Runner do end before do - allow(application).to receive(:chart_values).and_return(stub_values) + allow(gitlab_runner).to receive(:chart_values).and_return(values) end it 'should overwrite values.yaml' do - expect(values).to match(/privileged: '?#{application.privileged}/) + is_expected.to include("privileged: #{gitlab_runner.privileged}") end end end diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb index 581fd0293cc..20600f5fa38 100644 --- a/spec/models/internal_id_spec.rb +++ b/spec/models/internal_id_spec.rb @@ -79,6 +79,46 @@ describe InternalId do end end + describe '.track_greatest' do + let(:value) { 9001 } + subject { described_class.track_greatest(issue, scope, usage, value, init) } + + context 'in the absence of a record' do + it 'creates a record if not yet present' do + expect { subject }.to change { described_class.count }.from(0).to(1) + end + end + + it 'stores record attributes' do + subject + + described_class.first.tap do |record| + expect(record.project).to eq(project) + expect(record.usage).to eq(usage.to_s) + expect(record.last_value).to eq(value) + end + end + + context 'with existing issues' do + before do + create(:issue, project: project) + described_class.delete_all + end + + it 'still returns the last value to that of the given value' do + expect(subject).to eq(value) + end + end + + context 'when value is less than the current last_value' do + it 'returns the current last_value' do + described_class.create!(**scope, usage: usage, last_value: 10_001) + + expect(subject).to eq 10_001 + end + end + end + describe '#increment_and_save!' do let(:id) { create(:internal_id) } subject { id.increment_and_save! } @@ -103,4 +143,30 @@ describe InternalId do end end end + + describe '#track_greatest_and_save!' do + let(:id) { create(:internal_id) } + let(:new_last_value) { 9001 } + subject { id.track_greatest_and_save!(new_last_value) } + + it 'returns new last value' do + expect(subject).to eq new_last_value + end + + it 'saves the record' do + subject + + expect(id.changed?).to be_falsey + end + + context 'when new last value is lower than the max' do + it 'does not update the last value' do + id.update!(last_value: 10_001) + + subject + + expect(id.reload.last_value).to eq 10_001 + end + end + end end diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index 8914845ea82..99670af786a 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -139,4 +139,20 @@ describe Label do end end end + + describe '.search' do + let(:label) { create(:label, title: 'bug', description: 'incorrect behavior') } + + it 'returns labels with a partially matching title' do + expect(described_class.search(label.title[0..2])).to eq([label]) + end + + it 'returns labels with a partially matching description' do + expect(described_class.search(label.description[0..5])).to eq([label]) + end + + it 'returns nothing' do + expect(described_class.search('feature')).to be_empty + end + end end diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index 38a3590ad12..64c39f09e33 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -128,6 +128,12 @@ describe ProjectStatistics do .by(13) end + it 'increases also storage size by that amount' do + expect { described_class.increment_statistic(project.id, :build_artifacts_size, 20) } + .to change { statistics.reload.storage_size } + .by(20) + end + context 'when the amount is 0' do it 'does not execute a query' do project diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb new file mode 100644 index 00000000000..4756caa1b97 --- /dev/null +++ b/spec/models/resource_label_event_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ResourceLabelEvent, type: :model do + subject { build(:resource_label_event) } + let(:issue) { create(:issue) } + let(:merge_request) { create(:merge_request) } + + describe 'associations' do + it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:issue) } + it { is_expected.to belong_to(:merge_request) } + it { is_expected.to belong_to(:label) } + end + + describe 'validations' do + it { is_expected.to be_valid } + it { is_expected.to validate_presence_of(:label) } + it { is_expected.to validate_presence_of(:user) } + + describe 'Issuable validation' do + it 'is invalid if issue_id and merge_request_id are missing' do + subject.attributes = { issue: nil, merge_request: nil } + + expect(subject).to be_invalid + end + + it 'is invalid if issue_id and merge_request_id are set' do + subject.attributes = { issue: issue, merge_request: merge_request } + + expect(subject).to be_invalid + end + + it 'is valid if only issue_id is set' do + subject.attributes = { issue: issue, merge_request: nil } + + expect(subject).to be_valid + end + + it 'is valid if only merge_request_id is set' do + subject.attributes = { merge_request: merge_request, issue: nil } + + expect(subject).to be_valid + end + end + end +end diff --git a/spec/presenters/commit_status_presenter_spec.rb b/spec/presenters/commit_status_presenter_spec.rb index f81ee44e371..2b7742ddbb8 100644 --- a/spec/presenters/commit_status_presenter_spec.rb +++ b/spec/presenters/commit_status_presenter_spec.rb @@ -12,4 +12,30 @@ describe CommitStatusPresenter do it 'inherits from Gitlab::View::Presenter::Delegated' do expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated) end + + describe 'covers all failure reasons' do + let(:message) { presenter.callout_failure_message } + + CommitStatus.failure_reasons.keys.each do |failure_reason| + context failure_reason do + before do + build.failure_reason = failure_reason + end + + it "is a valid status" do + expect { message }.not_to raise_error + end + end + end + + context 'invalid failure message' do + before do + expect(build).to receive(:failure_reason) { 'invalid failure message' } + end + + it "is an invalid status" do + expect { message }.to raise_error(/key not found:/) + end + end + end end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index a2cfa706f58..b537b6e1667 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -152,7 +152,7 @@ describe API::Internal do context 'user key' do it 'returns the correct information about the key' do - lfs_auth(key.id, project) + lfs_auth_key(key.id, project) expect(response).to have_gitlab_http_status(200) expect(json_response['username']).to eq(user.username) @@ -161,8 +161,30 @@ describe API::Internal do expect(json_response['repository_http_path']).to eq(project.http_url_to_repo) end + it 'returns the correct information about the user' do + lfs_auth_user(user.id, project) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['username']).to eq(user.username) + expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(user).token) + + expect(json_response['repository_http_path']).to eq(project.http_url_to_repo) + end + + it 'returns a 404 when no key or user is provided' do + lfs_auth_project(project) + + expect(response).to have_gitlab_http_status(404) + end + it 'returns a 404 when the wrong key is provided' do - lfs_auth(nil, project) + lfs_auth_key(key.id + 12345, project) + + expect(response).to have_gitlab_http_status(404) + end + + it 'returns a 404 when the wrong user is provided' do + lfs_auth_user(user.id + 12345, project) expect(response).to have_gitlab_http_status(404) end @@ -172,7 +194,7 @@ describe API::Internal do let(:key) { create(:deploy_key) } it 'returns the correct information about the key' do - lfs_auth(key.id, project) + lfs_auth_key(key.id, project) expect(response).to have_gitlab_http_status(200) expect(json_response['username']).to eq("lfs+deploy-key-#{key.id}") @@ -183,13 +205,29 @@ describe API::Internal do end describe "GET /internal/discover" do - it do + it "finds a user by key id" do get(api("/internal/discover"), key_id: key.id, secret_token: secret_token) expect(response).to have_gitlab_http_status(200) expect(json_response['name']).to eq(user.name) end + + it "finds a user by user id" do + get(api("/internal/discover"), user_id: user.id, secret_token: secret_token) + + expect(response).to have_gitlab_http_status(200) + + expect(json_response['name']).to eq(user.name) + end + + it "finds a user by username" do + get(api("/internal/discover"), username: user.username, secret_token: secret_token) + + expect(response).to have_gitlab_http_status(200) + + expect(json_response['name']).to eq(user.name) + end end describe "GET /internal/authorized_keys" do @@ -871,7 +909,15 @@ describe API::Internal do ) end - def lfs_auth(key_id, project) + def lfs_auth_project(project) + post( + api("/internal/lfs_authenticate"), + secret_token: secret_token, + project: project.full_path + ) + end + + def lfs_auth_key(key_id, project) post( api("/internal/lfs_authenticate"), key_id: key_id, @@ -879,4 +925,13 @@ describe API::Internal do project: project.full_path ) end + + def lfs_auth_user(user_id, project) + post( + api("/internal/lfs_authenticate"), + user_id: user_id, + secret_token: secret_token, + project: project.full_path + ) + end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 66eb18229fa..28ba00c7293 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -1002,6 +1002,38 @@ describe API::Issues do end end + context 'an internal ID is provided' do + context 'by an admin' do + it 'sets the internal ID on the new issue' do + post api("/projects/#{project.id}/issues", admin), + title: 'new issue', iid: 9001 + + expect(response).to have_gitlab_http_status(201) + expect(json_response['iid']).to eq 9001 + end + end + + context 'by an owner' do + it 'sets the internal ID on the new issue' do + post api("/projects/#{project.id}/issues", user), + title: 'new issue', iid: 9001 + + expect(response).to have_gitlab_http_status(201) + expect(json_response['iid']).to eq 9001 + end + end + + context 'by another user' do + it 'ignores the given internal ID' do + post api("/projects/#{project.id}/issues", user2), + title: 'new issue', iid: 9001 + + expect(response).to have_gitlab_http_status(201) + expect(json_response['iid']).not_to eq 9001 + end + end + end + it 'creates a new project issue' do post api("/projects/#{project.id}/issues", user), title: 'new issue', labels: 'label, label2', weight: 3, diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index 6f40a02aaa9..e042d772718 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -70,6 +70,25 @@ describe JwtController do it { expect(service_class).to have_received(:new).with(nil, user, parameters) } + context 'when passing a flat array of scopes' do + # We use this trick to make rails to generate a query_string: + # scope=scope1&scope=scope2 + # It works because :scope and 'scope' are the same as string, but different objects + let(:parameters) do + { + :service => service_name, + :scope => 'scope1', + 'scope' => 'scope2' + } + end + + let(:service_parameters) do + { service: service_name, scopes: %w(scope1 scope2) } + end + + it { expect(service_class).to have_received(:new).with(nil, user, service_parameters) } + end + context 'when user has 2FA enabled' do let(:user) { create(:user, :two_factor) } diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index c2378646f89..e349181b794 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -732,7 +732,7 @@ describe 'Git LFS API and storage' do expect(json_response['objects'].first['oid']).to eq(sample_oid) expect(json_response['objects'].first['size']).to eq(sample_size) expect(json_response['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.full_path}.git/gitlab-lfs/objects/#{sample_oid}/#{sample_size}") - expect(json_response['objects'].first['actions']['upload']['header']).to eq('Authorization' => authorization) + expect(json_response['objects'].first['actions']['upload']['header']).to eq({ 'Authorization' => authorization, 'Content-Type' => 'application/octet-stream' }) end end @@ -761,7 +761,7 @@ describe 'Git LFS API and storage' do expect(lfs_object.projects.pluck(:id)).not_to include(project.id) expect(lfs_object.projects.pluck(:id)).to include(other_project.id) expect(json_response['objects'].first['actions']['upload']['href']).to eq("#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}") - expect(json_response['objects'].first['actions']['upload']['header']).to eq('Authorization' => authorization) + expect(json_response['objects'].first['actions']['upload']['header']).to eq({ 'Authorization' => authorization, 'Content-Type' => 'application/octet-stream' }) end end @@ -796,7 +796,7 @@ describe 'Git LFS API and storage' do expect(json_response['objects'].first['oid']).to eq("91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897") expect(json_response['objects'].first['size']).to eq(1575078) expect(json_response['objects'].first['actions']['upload']['href']).to eq("#{project.http_url_to_repo}/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897/1575078") - expect(json_response['objects'].first['actions']['upload']['header']).to eq("Authorization" => authorization) + expect(json_response['objects'].first['actions']['upload']['header']).to eq({ 'Authorization' => authorization, 'Content-Type' => 'application/octet-stream' }) expect(json_response['objects'].last['oid']).to eq(sample_oid) expect(json_response['objects'].last['size']).to eq(sample_size) diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 037484931b8..c7f88e45c84 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -142,7 +142,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'for registry catalog' do let(:current_params) do - { scope: "registry:catalog:*" } + { scopes: ["registry:catalog:*"] } end context 'disallow browsing for users without Gitlab admin rights' do @@ -164,7 +164,7 @@ describe Auth::ContainerRegistryAuthenticationService do end let(:current_params) do - { scope: "repository:#{project.full_path}:push" } + { scopes: ["repository:#{project.full_path}:push"] } end it_behaves_like 'a pushable' @@ -177,7 +177,7 @@ describe Auth::ContainerRegistryAuthenticationService do end let(:current_params) do - { scope: "repository:#{project.full_path}:*" } + { scopes: ["repository:#{project.full_path}:*"] } end it_behaves_like 'an inaccessible' @@ -191,7 +191,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'when pulling from root level repository' do let(:current_params) do - { scope: "repository:#{project.full_path}:pull" } + { scopes: ["repository:#{project.full_path}:pull"] } end it_behaves_like 'a pullable' @@ -205,7 +205,7 @@ describe Auth::ContainerRegistryAuthenticationService do end let(:current_params) do - { scope: "repository:#{project.full_path}:*" } + { scopes: ["repository:#{project.full_path}:*"] } end it_behaves_like 'an inaccessible' @@ -218,7 +218,7 @@ describe Auth::ContainerRegistryAuthenticationService do end let(:current_params) do - { scope: "repository:#{project.full_path}:push,pull" } + { scopes: ["repository:#{project.full_path}:push,pull"] } end it_behaves_like 'a pullable' @@ -231,7 +231,7 @@ describe Auth::ContainerRegistryAuthenticationService do end let(:current_params) do - { scope: "repository:#{project.full_path}:pull,push" } + { scopes: ["repository:#{project.full_path}:pull,push"] } end it_behaves_like 'an inaccessible' @@ -244,7 +244,7 @@ describe Auth::ContainerRegistryAuthenticationService do end let(:current_params) do - { scope: "repository:#{project.full_path}:*" } + { scopes: ["repository:#{project.full_path}:*"] } end it_behaves_like 'an inaccessible' @@ -257,7 +257,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'allow anyone to pull images' do let(:current_params) do - { scope: "repository:#{project.full_path}:pull" } + { scopes: ["repository:#{project.full_path}:pull"] } end it_behaves_like 'a pullable' @@ -266,7 +266,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'disallow anyone to push images' do let(:current_params) do - { scope: "repository:#{project.full_path}:push" } + { scopes: ["repository:#{project.full_path}:push"] } end it_behaves_like 'an inaccessible' @@ -275,7 +275,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'disallow anyone to delete images' do let(:current_params) do - { scope: "repository:#{project.full_path}:*" } + { scopes: ["repository:#{project.full_path}:*"] } end it_behaves_like 'an inaccessible' @@ -284,7 +284,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'when repository name is invalid' do let(:current_params) do - { scope: 'repository:invalid:push' } + { scopes: ['repository:invalid:push'] } end it_behaves_like 'an inaccessible' @@ -298,7 +298,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'for internal user' do context 'allow anyone to pull images' do let(:current_params) do - { scope: "repository:#{project.full_path}:pull" } + { scopes: ["repository:#{project.full_path}:pull"] } end it_behaves_like 'a pullable' @@ -307,7 +307,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'disallow anyone to push images' do let(:current_params) do - { scope: "repository:#{project.full_path}:push" } + { scopes: ["repository:#{project.full_path}:push"] } end it_behaves_like 'an inaccessible' @@ -316,7 +316,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'disallow anyone to delete images' do let(:current_params) do - { scope: "repository:#{project.full_path}:*" } + { scopes: ["repository:#{project.full_path}:*"] } end it_behaves_like 'an inaccessible' @@ -328,7 +328,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'disallow anyone to pull or push images' do let(:current_user) { create(:user, external: true) } let(:current_params) do - { scope: "repository:#{project.full_path}:pull,push" } + { scopes: ["repository:#{project.full_path}:pull,push"] } end it_behaves_like 'an inaccessible' @@ -338,7 +338,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'disallow anyone to delete images' do let(:current_user) { create(:user, external: true) } let(:current_params) do - { scope: "repository:#{project.full_path}:*" } + { scopes: ["repository:#{project.full_path}:*"] } end it_behaves_like 'an inaccessible' @@ -364,7 +364,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'allow to delete images' do let(:current_params) do - { scope: "repository:#{current_project.full_path}:*" } + { scopes: ["repository:#{current_project.full_path}:*"] } end it_behaves_like 'a deletable' do @@ -397,7 +397,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'allow to pull and push images' do let(:current_params) do - { scope: "repository:#{current_project.full_path}:pull,push" } + { scopes: ["repository:#{current_project.full_path}:pull,push"] } end it_behaves_like 'a pullable and pushable' do @@ -411,7 +411,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'disallow to delete images' do let(:current_params) do - { scope: "repository:#{current_project.full_path}:*" } + { scopes: ["repository:#{current_project.full_path}:*"] } end it_behaves_like 'an inaccessible' do @@ -422,7 +422,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'for other projects' do context 'when pulling' do let(:current_params) do - { scope: "repository:#{project.full_path}:pull" } + { scopes: ["repository:#{project.full_path}:pull"] } end context 'allow for public' do @@ -489,7 +489,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'when pushing' do let(:current_params) do - { scope: "repository:#{project.full_path}:push" } + { scopes: ["repository:#{project.full_path}:push"] } end context 'disallow for all' do @@ -523,7 +523,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'disallow when pulling' do let(:current_params) do - { scope: "repository:#{project.full_path}:pull" } + { scopes: ["repository:#{project.full_path}:pull"] } end it_behaves_like 'an inaccessible' @@ -534,14 +534,66 @@ describe Auth::ContainerRegistryAuthenticationService do context 'registry catalog browsing authorized as admin' do let(:current_user) { create(:user, :admin) } + let(:project) { create(:project, :public) } let(:current_params) do - { scope: "registry:catalog:*" } + { scopes: ["registry:catalog:*"] } end it_behaves_like 'a browsable' end + context 'support for multiple scopes' do + let(:internal_project) { create(:project, :internal) } + let(:private_project) { create(:project, :private) } + + let(:current_params) do + { + scopes: [ + "repository:#{internal_project.full_path}:pull", + "repository:#{private_project.full_path}:pull" + ] + } + end + + context 'user has access to all projects' do + let(:current_user) { create(:user, :admin) } + + it_behaves_like 'a browsable' do + let(:access) do + [ + { 'type' => 'repository', + 'name' => internal_project.full_path, + 'actions' => ['pull'] }, + { 'type' => 'repository', + 'name' => private_project.full_path, + 'actions' => ['pull'] } + ] + end + end + end + + context 'user only has access to internal project' do + let(:current_user) { create(:user) } + + it_behaves_like 'a browsable' do + let(:access) do + [ + { 'type' => 'repository', + 'name' => internal_project.full_path, + 'actions' => ['pull'] } + ] + end + end + end + + context 'anonymous access is rejected' do + let(:current_user) { nil } + + it_behaves_like 'a forbidden' + end + end + context 'unauthorized' do context 'disallow to use scope-less authentication' do it_behaves_like 'a forbidden' @@ -550,7 +602,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'for invalid scope' do let(:current_params) do - { scope: 'invalid:aa:bb' } + { scopes: ['invalid:aa:bb'] } end it_behaves_like 'a forbidden' @@ -561,7 +613,7 @@ describe Auth::ContainerRegistryAuthenticationService do let(:project) { create(:project, :private) } let(:current_params) do - { scope: "repository:#{project.full_path}:pull" } + { scopes: ["repository:#{project.full_path}:pull"] } end it_behaves_like 'a forbidden' @@ -572,7 +624,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'when pulling and pushing' do let(:current_params) do - { scope: "repository:#{project.full_path}:pull,push" } + { scopes: ["repository:#{project.full_path}:pull,push"] } end it_behaves_like 'a pullable' @@ -581,7 +633,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'when pushing' do let(:current_params) do - { scope: "repository:#{project.full_path}:push" } + { scopes: ["repository:#{project.full_path}:push"] } end it_behaves_like 'a forbidden' @@ -591,7 +643,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'for registry catalog' do let(:current_params) do - { scope: "registry:catalog:*" } + { scopes: ["registry:catalog:*"] } end it_behaves_like 'a forbidden' @@ -601,7 +653,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'for deploy tokens' do let(:current_params) do - { scope: "repository:#{project.full_path}:pull" } + { scopes: ["repository:#{project.full_path}:pull"] } end context 'when deploy token has read_registry as a scope' do @@ -616,7 +668,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'when pushing' do let(:current_params) do - { scope: "repository:#{project.full_path}:push" } + { scopes: ["repository:#{project.full_path}:push"] } end it_behaves_like 'an inaccessible' @@ -632,7 +684,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'when pushing' do let(:current_params) do - { scope: "repository:#{project.full_path}:push" } + { scopes: ["repository:#{project.full_path}:push"] } end it_behaves_like 'an inaccessible' @@ -648,7 +700,7 @@ describe Auth::ContainerRegistryAuthenticationService do context 'when pushing' do let(:current_params) do - { scope: "repository:#{project.full_path}:push" } + { scopes: ["repository:#{project.full_path}:push"] } end it_behaves_like 'an inaccessible' @@ -734,4 +786,26 @@ describe Auth::ContainerRegistryAuthenticationService do end end end + + context 'user authorization' do + let(:current_user) { create(:user) } + + context 'with multiple scopes' do + let(:project) { create(:project) } + let(:project2) { create } + + context 'allow developer to push images' do + before do + project.add_developer(current_user) + end + + let(:current_params) do + { scopes: ["repository:#{project.full_path}:push"] } + end + + it_behaves_like 'a pushable' + it_behaves_like 'container repository factory' + end + end + end end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index dbb5e33bbdc..a6565709641 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -351,6 +351,38 @@ module Ci end end + context 'runner feature set is verified' do + let!(:pending_job) { create(:ci_build, :pending, pipeline: pipeline) } + + before do + expect_any_instance_of(Ci::Build).to receive(:runner_required_feature_names) do + [:runner_required_feature] + end + end + + subject { execute(specific_runner, params) } + + context 'when feature is missing by runner' do + let(:params) { {} } + + it 'does not pick the build and drops the build' do + expect(subject).to be_nil + expect(pending_job.reload).to be_failed + expect(pending_job).to be_runner_unsupported + end + end + + context 'when feature is supported by runner' do + let(:params) do + { info: { features: { runner_required_feature: true } } } + end + + it 'does pick job' do + expect(subject).not_to be_nil + end + end + end + context 'when "dependencies" keyword is specified' do shared_examples 'not pick' do it 'does not pick the build and drops the build' do @@ -403,6 +435,7 @@ module Ci it { expect(subject).to eq(pending_job) } end + context 'when artifacts of depended job has been expired' do let!(:pre_stage_job) { create(:ci_build, :success, :expired, pipeline: pipeline, name: 'test', stage_idx: 0) } diff --git a/spec/services/clusters/applications/check_installation_progress_service_spec.rb b/spec/services/clusters/applications/check_installation_progress_service_spec.rb index 6894c1797b0..986f11410fd 100644 --- a/spec/services/clusters/applications/check_installation_progress_service_spec.rb +++ b/spec/services/clusters/applications/check_installation_progress_service_spec.rb @@ -43,7 +43,7 @@ describe Clusters::Applications::CheckInstallationProgressService do service.execute expect(application).to be_errored - expect(application.status_reason).to match(/\btimeouted\b/) + expect(application.status_reason).to match(/\btimed out\b/) end end end diff --git a/spec/services/clusters/applications/install_service_spec.rb b/spec/services/clusters/applications/install_service_spec.rb index a744ec30b65..93199964a0e 100644 --- a/spec/services/clusters/applications/install_service_spec.rb +++ b/spec/services/clusters/applications/install_service_spec.rb @@ -47,7 +47,7 @@ describe Clusters::Applications::InstallService do end context 'when application cannot be persisted' do - let(:application) { create(:clusters_applications_helm, :scheduled) } + let(:application) { build(:clusters_applications_helm, :scheduled) } it 'make the application errored' do expect(application).to receive(:make_installing!).once.and_raise(ActiveRecord::RecordInvalid) diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index fa98d05c61b..5bcfef46b75 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -55,6 +55,8 @@ describe Issues::UpdateService, :mailer do end it 'updates the issue with the given params' do + expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in) + update_issue(opts) expect(issue).to be_valid @@ -74,6 +76,21 @@ describe Issues::UpdateService, :mailer do .to change { project.open_issues_count }.from(1).to(0) end + it 'enqueues ConfidentialIssueWorker when an issue is made confidential' do + expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(1.hour, issue.id) + + update_issue(confidential: true) + end + + it 'does not enqueue ConfidentialIssueWorker when an issue is made non confidential' do + # set confidentiality to true before the actual update + issue.update!(confidential: true) + + expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in) + + update_issue(confidential: false) + end + it 'updates open issue counter for assignees when issue is reassigned' do update_issue(assignee_ids: [user2.id]) diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index ef47b0a450b..0a5220c7c61 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -20,6 +20,11 @@ describe Members::DestroyService do end shared_examples 'a service destroying a member' do + before do + type = member.is_a?(GroupMember) ? 'Group' : 'Project' + expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(1.hour, member.user_id, member.source_id, type) + end + it 'destroys the member' do expect { described_class.new(current_user).execute(member, opts) }.to change { member.source.members_and_requesters.count }.by(-1) end diff --git a/spec/services/projects/create_from_template_service_spec.rb b/spec/services/projects/create_from_template_service_spec.rb index a43da01f37e..141ccf7c4d8 100644 --- a/spec/services/projects/create_from_template_service_spec.rb +++ b/spec/services/projects/create_from_template_service_spec.rb @@ -2,10 +2,11 @@ require 'spec_helper' describe Projects::CreateFromTemplateService do let(:user) { create(:user) } + let(:template_name) { 'rails' } let(:project_params) do { path: user.to_param, - template_name: 'rails', + template_name: template_name, description: 'project description', visibility_level: Gitlab::VisibilityLevel::PUBLIC } @@ -14,7 +15,10 @@ describe Projects::CreateFromTemplateService do subject { described_class.new(user, project_params) } it 'calls the importer service' do - expect_any_instance_of(Projects::GitlabProjectsImportService).to receive(:execute) + import_service_double = double + + allow(Projects::GitlabProjectsImportService).to receive(:new).and_return(import_service_double) + expect(import_service_double).to receive(:execute) subject.execute end @@ -26,6 +30,31 @@ describe Projects::CreateFromTemplateService do expect(project.import_scheduled?).to be(true) end + context 'when template is not present' do + let(:template_name) { 'non_existent' } + let(:project) { subject.execute } + + before do + expect(project).to be_saved + end + + it 'does not set import set import type' do + expect(project.import_type).to be nil + end + + it 'does not set import set import source' do + expect(project.import_source).to be nil + end + + it 'is not scheduled' do + expect(project.import_scheduled?).to be(false) + end + + it 'repository is empty' do + expect(project.repository.empty?).to be(true) + end + end + context 'the result project' do before do perform_enqueued_jobs do diff --git a/spec/services/projects/gitlab_projects_import_service_spec.rb b/spec/services/projects/gitlab_projects_import_service_spec.rb index 0a898e9b89b..a2061127698 100644 --- a/spec/services/projects/gitlab_projects_import_service_spec.rb +++ b/spec/services/projects/gitlab_projects_import_service_spec.rb @@ -6,60 +6,10 @@ describe Projects::GitlabProjectsImportService do let(:file) { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') } let(:overwrite) { false } let(:import_params) { { namespace_id: namespace.id, path: path, file: file, overwrite: overwrite } } + subject { described_class.new(namespace.owner, import_params) } describe '#execute' do - context 'with an invalid path' do - let(:path) { '/invalid-path/' } - - it 'returns an invalid project' do - project = subject.execute - - expect(project).not_to be_persisted - expect(project).not_to be_valid - end - end - - context 'with a valid path' do - it 'creates a project' do - project = subject.execute - - expect(project).to be_persisted - expect(project).to be_valid - end - end - - context 'override params' do - it 'stores them as import data when passed' do - project = described_class - .new(namespace.owner, import_params, description: 'Hello') - .execute - - expect(project.import_data.data['override_params']['description']).to eq('Hello') - end - end - - context 'when there is a project with the same path' do - let(:existing_project) { create(:project, namespace: namespace) } - let(:path) { existing_project.path} - - it 'does not create the project' do - project = subject.execute - - expect(project).to be_invalid - expect(project).not_to be_persisted - end - - context 'when overwrite param is set' do - let(:overwrite) { true } - - it 'creates a project in a temporary full_path' do - project = subject.execute - - expect(project).to be_valid - expect(project).to be_persisted - end - end - end + it_behaves_like 'gitlab projects import validations' end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index ecf1ba05618..e6871545a0b 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -15,6 +15,8 @@ describe Projects::UpdateService do context 'when changing visibility level' do context 'when visibility_level is INTERNAL' do it 'updates the project to internal' do + expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in) + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL) expect(result).to eq({ status: :success }) @@ -24,12 +26,30 @@ describe Projects::UpdateService do context 'when visibility_level is PUBLIC' do it 'updates the project to public' do + expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in) + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC) + expect(result).to eq({ status: :success }) expect(project).to be_public end end + context 'when visibility_level is PRIVATE' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it 'updates the project to private' do + expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(1.hour, project.id) + + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + expect(result).to eq({ status: :success }) + expect(project).to be_private + end + end + context 'when visibility levels are restricted to PUBLIC only' do before do stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) @@ -38,6 +58,7 @@ describe Projects::UpdateService do context 'when visibility_level is INTERNAL' do it 'updates the project to internal' do result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL) + expect(result).to eq({ status: :success }) expect(project).to be_internal end @@ -54,6 +75,7 @@ describe Projects::UpdateService do context 'when updated by an admin' do it 'updates the project to public' do result = update_project(project, admin, visibility_level: Gitlab::VisibilityLevel::PUBLIC) + expect(result).to eq({ status: :success }) expect(project).to be_public end @@ -166,6 +188,20 @@ describe Projects::UpdateService do end end + context 'when changing feature visibility to private' do + it 'updates the visibility correctly' do + expect(TodosDestroyer::PrivateFeaturesWorker) + .to receive(:perform_in).with(1.hour, project.id) + + result = update_project(project, user, project_feature_attributes: + { issues_access_level: ProjectFeature::PRIVATE } + ) + + expect(result).to eq({ status: :success }) + expect(project.project_feature.issues_access_level).to be(ProjectFeature::PRIVATE) + end + end + context 'when updating a project that contains container images' do before do stub_container_registry_config(enabled: true) diff --git a/spec/services/resource_events/change_labels_service_spec.rb b/spec/services/resource_events/change_labels_service_spec.rb new file mode 100644 index 00000000000..41b0fb3eea3 --- /dev/null +++ b/spec/services/resource_events/change_labels_service_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ResourceEvents::ChangeLabelsService do + set(:project) { create(:project) } + set(:author) { create(:user) } + let(:resource) { create(:issue, project: project) } + + describe '.change_labels' do + subject { described_class.new(resource, author).execute(added_labels: added, removed_labels: removed) } + + let(:labels) { create_list(:label, 2, project: project) } + + def expect_label_event(event, label, action) + expect(event.user).to eq(author) + expect(event.label).to eq(label) + expect(event.action).to eq(action) + end + + context 'when adding a label' do + let(:added) { [labels[0]] } + let(:removed) { [] } + + it 'creates new label event' do + expect { subject }.to change { resource.resource_label_events.count }.from(0).to(1) + + expect_label_event(resource.resource_label_events.first, labels[0], 'add') + end + end + + context 'when removing a label' do + let(:added) { [] } + let(:removed) { [labels[1]] } + + it 'creates new label event' do + expect { subject }.to change { resource.resource_label_events.count }.from(0).to(1) + + expect_label_event(resource.resource_label_events.first, labels[1], 'remove') + end + end + + context 'when both adding and removing labels' do + let(:added) { [labels[0]] } + let(:removed) { [labels[1]] } + + it 'creates all label events in a single query' do + expect(Gitlab::Database).to receive(:bulk_insert).once.and_call_original + expect { subject }.to change { resource.resource_label_events.count }.from(0).to(2) + end + end + end +end diff --git a/spec/services/todos/destroy/confidential_issue_service_spec.rb b/spec/services/todos/destroy/confidential_issue_service_spec.rb new file mode 100644 index 00000000000..54d1d7e83f1 --- /dev/null +++ b/spec/services/todos/destroy/confidential_issue_service_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe Todos::Destroy::ConfidentialIssueService do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:guest) { create(:user) } + let(:project_member) { create(:user) } + let(:issue) { create(:issue, project: project, author: author, assignees: [assignee]) } + + let!(:todo_issue_non_member) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_member) { create(:todo, user: project_member, target: issue, project: project) } + let!(:todo_issue_author) { create(:todo, user: author, target: issue, project: project) } + let!(:todo_issue_asignee) { create(:todo, user: assignee, target: issue, project: project) } + let!(:todo_issue_guest) { create(:todo, user: guest, target: issue, project: project) } + let!(:todo_another_non_member) { create(:todo, user: user, project: project) } + + describe '#execute' do + before do + project.add_developer(project_member) + project.add_guest(guest) + end + + subject { described_class.new(issue.id).execute } + + context 'when provided issue is confidential' do + before do + issue.update!(confidential: true) + end + + it 'removes issue todos for a user who is not a project member' do + expect { subject }.to change { Todo.count }.from(6).to(4) + + expect(user.todos).to match_array([todo_another_non_member]) + expect(author.todos).to match_array([todo_issue_author]) + expect(project_member.todos).to match_array([todo_issue_member]) + end + end + + context 'when provided issue is not confidential' do + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + end +end diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb new file mode 100644 index 00000000000..bad408a314e --- /dev/null +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -0,0 +1,135 @@ +require 'spec_helper' + +describe Todos::Destroy::EntityLeaveService do + let(:group) { create(:group, :private) } + let(:project) { create(:project, group: group) } + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:issue) { create(:issue, project: project) } + let(:mr) { create(:merge_request, source_project: project) } + + let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) } + let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_user2) { create(:todo, user: user2, target: issue, project: project) } + + describe '#execute' do + context 'when a user leaves a project' do + subject { described_class.new(user.id, project.id, 'Project').execute } + + context 'when project is private' do + it 'removes todos for the provided user' do + expect { subject }.to change { Todo.count }.from(3).to(1) + + expect(user.todos).to be_empty + expect(user2.todos).to match_array([todo_issue_user2]) + end + end + + context 'when project is not private' do + before do + group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end + + context 'when a user is not an author of confidential issue' do + before do + issue.update!(confidential: true) + end + + it 'removes only confidential issues todos' do + expect { subject }.to change { Todo.count }.from(3).to(2) + end + end + + context 'when a user is an author of confidential issue' do + before do + issue.update!(author: user, confidential: true) + end + + it 'removes only confidential issues todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when a user is an assignee of confidential issue' do + before do + issue.update!(confidential: true) + issue.assignees << user + end + + it 'removes only confidential issues todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'feature visibility check' do + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only users issue todos' do + expect { subject }.to change { Todo.count }.from(3).to(2) + end + end + end + end + end + + context 'when a user leaves a group' do + subject { described_class.new(user.id, group.id, 'Group').execute } + + context 'when group is private' do + it 'removes todos for the user' do + expect { subject }.to change { Todo.count }.from(3).to(1) + + expect(user.todos).to be_empty + expect(user2.todos).to match_array([todo_issue_user2]) + end + + context 'with nested groups', :nested_groups do + let(:subgroup) { create(:group, :private, parent: group) } + let(:subproject) { create(:project, group: subgroup) } + + let!(:todo_subproject_user) { create(:todo, user: user, project: subproject) } + let!(:todo_subproject_user2) { create(:todo, user: user2, project: subproject) } + + it 'removes todos for the user including subprojects todos' do + expect { subject }.to change { Todo.count }.from(5).to(2) + + expect(user.todos).to be_empty + expect(user2.todos) + .to match_array([todo_issue_user2, todo_subproject_user2]) + end + end + end + + context 'when group is not private' do + before do + issue.update!(confidential: true) + + group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end + + it 'removes only confidential issues todos' do + expect { subject }.to change { Todo.count }.from(3).to(2) + end + end + end + + context 'when entity type is not valid' do + it 'raises an exception' do + expect { described_class.new(user.id, group.id, 'GroupWrongly').execute } + .to raise_error(ArgumentError) + end + end + + context 'when entity was not found' do + it 'does not remove any todos' do + expect { described_class.new(user.id, 999999, 'Group').execute } + .not_to change { Todo.count } + end + end + end +end diff --git a/spec/services/todos/destroy/private_features_service_spec.rb b/spec/services/todos/destroy/private_features_service_spec.rb new file mode 100644 index 00000000000..be8b5bb3979 --- /dev/null +++ b/spec/services/todos/destroy/private_features_service_spec.rb @@ -0,0 +1,143 @@ +require 'spec_helper' + +describe Todos::Destroy::PrivateFeaturesService do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:another_user) { create(:user) } + let(:project_member) { create(:user) } + let(:issue) { create(:issue, project: project) } + let(:mr) { create(:merge_request, source_project: project) } + + let!(:todo_mr_non_member) { create(:todo, user: user, target: mr, project: project) } + let!(:todo_mr_non_member2) { create(:todo, user: another_user, target: mr, project: project) } + let!(:todo_mr_member) { create(:todo, user: project_member, target: mr, project: project) } + let!(:todo_issue_non_member) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_non_member2) { create(:todo, user: another_user, target: issue, project: project) } + let!(:todo_issue_member) { create(:todo, user: project_member, target: issue, project: project) } + let!(:commit_todo_non_member) { create(:on_commit_todo, user: user, project: project) } + let!(:commit_todo_non_member2) { create(:on_commit_todo, user: another_user, project: project) } + let!(:commit_todo_member) { create(:on_commit_todo, user: project_member, project: project) } + + before do + project.add_developer(project_member) + end + + context 'when user_id is provided' do + subject { described_class.new(project.id, user.id).execute } + + context 'when all feaures have same visibility as the project' do + it 'removes only user issue todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members but the user is a member' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.add_developer(user) + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(8) + end + end + + context 'when mrs, builds and repository are visible only to project members' do + before do + # builds and merge requests cannot have higher visibility than repository + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(repository_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user mr and commit todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs are visible only to project members' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user merge request todo' do + expect { subject }.to change { Todo.count }.from(9).to(8) + end + end + + context 'when mrs and issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user merge request and issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + end + + context 'when user_id is not provided' do + subject { described_class.new(project.id).execute } + + context 'when all feaures have same visibility as the project' do + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs, builds and repository are visible only to project members' do + before do + # builds and merge requests cannot have higher visibility than repository + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(repository_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members mr and commit todos' do + expect { subject }.to change { Todo.count }.from(9).to(5) + end + end + + context 'when mrs are visible only to project members' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members merge request todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs and issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members merge request and issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(5) + end + end + end +end diff --git a/spec/services/todos/destroy/project_private_service_spec.rb b/spec/services/todos/destroy/project_private_service_spec.rb new file mode 100644 index 00000000000..badf3f913a5 --- /dev/null +++ b/spec/services/todos/destroy/project_private_service_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Todos::Destroy::ProjectPrivateService do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:project_member) { create(:user) } + + let!(:todo_issue_non_member) { create(:todo, user: user, project: project) } + let!(:todo_issue_member) { create(:todo, user: project_member, project: project) } + let!(:todo_another_non_member) { create(:todo, user: user, project: project) } + + describe '#execute' do + before do + project.add_developer(project_member) + end + + subject { described_class.new(project.id).execute } + + context 'when a project set to private' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it 'removes issue todos for a user who is not a member' do + expect { subject }.to change { Todo.count }.from(3).to(1) + + expect(user.todos).to be_empty + expect(project_member.todos).to match_array([todo_issue_member]) + end + end + + context 'when project is not private' do + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + end +end diff --git a/spec/support/helpers/stub_metrics.rb b/spec/support/helpers/stub_metrics.rb new file mode 100644 index 00000000000..64983fdf222 --- /dev/null +++ b/spec/support/helpers/stub_metrics.rb @@ -0,0 +1,27 @@ +module StubMetrics + def authentication_metrics + Gitlab::Auth::Activity + end + + def stub_authentication_activity_metrics(debug: false) + authentication_metrics.each_counter do |name, metric, description| + allow(authentication_metrics).to receive(name) + .and_return(double("#{metric} - #{description}")) + end + + debug_authentication_activity_metrics if debug + end + + def debug_authentication_activity_metrics + authentication_metrics.tap do |metrics| + metrics.each_counter do |name, metric| + "#{name}_increment!".tap do |incrementer| + allow(metrics).to receive(incrementer).and_wrap_original do |method| + puts "Authentication activity metric incremented: #{name}" + method.call + end + end + end + end + end +end diff --git a/spec/support/matchers/metric_counter_matcher.rb b/spec/support/matchers/metric_counter_matcher.rb new file mode 100644 index 00000000000..22d5cd17e3f --- /dev/null +++ b/spec/support/matchers/metric_counter_matcher.rb @@ -0,0 +1,11 @@ +RSpec::Matchers.define :increment do |counter| + match do |adapter| + expect(adapter.send(counter)) + .to receive(:increment) + .exactly(@exactly || :once) + end + + chain :twice do + @exactly = :twice + end +end diff --git a/spec/support/rspec.rb b/spec/support/rspec.rb index 54b8df7aa19..9b8bcebcb3a 100644 --- a/spec/support/rspec.rb +++ b/spec/support/rspec.rb @@ -1,4 +1,5 @@ require_relative "helpers/stub_configuration" +require_relative "helpers/stub_metrics" require_relative "helpers/stub_object_storage" require_relative "helpers/stub_env" @@ -7,6 +8,7 @@ RSpec.configure do |config| config.raise_errors_for_deprecations! config.include StubConfiguration + config.include StubMetrics config.include StubObjectStorage config.include StubENV diff --git a/spec/support/shared_examples/gitlab_projects_import_service_shared_examples.rb b/spec/support/shared_examples/gitlab_projects_import_service_shared_examples.rb new file mode 100644 index 00000000000..b8db35a6ef9 --- /dev/null +++ b/spec/support/shared_examples/gitlab_projects_import_service_shared_examples.rb @@ -0,0 +1,54 @@ +shared_examples 'gitlab projects import validations' do + context 'with an invalid path' do + let(:path) { '/invalid-path/' } + + it 'returns an invalid project' do + project = subject.execute + + expect(project).not_to be_persisted + expect(project).not_to be_valid + end + end + + context 'with a valid path' do + it 'creates a project' do + project = subject.execute + + expect(project).to be_persisted + expect(project).to be_valid + end + end + + context 'override params' do + it 'stores them as import data when passed' do + project = described_class + .new(namespace.owner, import_params, description: 'Hello') + .execute + + expect(project.import_data.data['override_params']['description']).to eq('Hello') + end + end + + context 'when there is a project with the same path' do + let(:existing_project) { create(:project, namespace: namespace) } + let(:path) { existing_project.path} + + it 'does not create the project' do + project = subject.execute + + expect(project).to be_invalid + expect(project).not_to be_persisted + end + + context 'when overwrite param is set' do + let(:overwrite) { true } + + it 'creates a project in a temporary full_path' do + project = subject.execute + + expect(project).to be_valid + expect(project).to be_persisted + end + end + end +end diff --git a/spec/support/shared_examples/models/atomic_internal_id_spec.rb b/spec/support/shared_examples/models/atomic_internal_id_spec.rb index 7ab1041d17c..c659be8f13a 100644 --- a/spec/support/shared_examples/models/atomic_internal_id_spec.rb +++ b/spec/support/shared_examples/models/atomic_internal_id_spec.rb @@ -60,6 +60,20 @@ shared_examples_for 'AtomicInternalId' do |validate_presence: true| expect { subject }.not_to change { instance.public_send(internal_id_attribute) } end + + context 'when the instance has an internal ID set' do + let(:internal_id) { 9001 } + + it 'calls InternalId.update_last_value and sets the `last_value` to that of the instance' do + instance.send("#{internal_id_attribute}=", internal_id) + + expect(InternalId) + .to receive(:track_greatest) + .with(instance, scope_attrs, usage, internal_id, any_args) + .and_return(internal_id) + subject + end + end end end end diff --git a/spec/tasks/gitlab/cleanup_rake_spec.rb b/spec/tasks/gitlab/cleanup_rake_spec.rb index 2bf873c923f..ba08ece1b4b 100644 --- a/spec/tasks/gitlab/cleanup_rake_spec.rb +++ b/spec/tasks/gitlab/cleanup_rake_spec.rb @@ -5,7 +5,7 @@ describe 'gitlab:cleanup rake tasks' do Rake.application.rake_require 'tasks/gitlab/cleanup' end - describe 'cleanup' do + describe 'cleanup namespaces and repos' do let(:storages) do { 'default' => Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => 'tmp/tests/default_storage')) @@ -67,4 +67,319 @@ describe 'gitlab:cleanup rake tasks' do end end end + + describe 'cleanup:project_uploads' do + context 'orphaned project upload file' do + context 'when an upload record matching the secret and filename is found' do + context 'when the project is still in legacy storage' do + let!(:orphaned) { create(:upload, :issuable_upload, :with_file, model: build(:project, :legacy_storage)) } + let!(:correct_path) { orphaned.absolute_path } + let!(:other_project) { create(:project, :legacy_storage) } + let!(:orphaned_path) { correct_path.sub(/#{orphaned.model.full_path}/, other_project.full_path) } + + before do + FileUtils.mkdir_p(File.dirname(orphaned_path)) + FileUtils.mv(correct_path, orphaned_path) + end + + it 'moves the file to its proper location' do + expect(Rails.logger).to receive(:info).twice + expect(Rails.logger).to receive(:info).with("Did fix #{orphaned_path} -> #{correct_path}") + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(correct_path)).to be_falsey + + stub_env('DRY_RUN', 'false') + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(orphaned_path)).to be_falsey + expect(File.exist?(correct_path)).to be_truthy + end + + it 'a dry run does not move the file' do + expect(Rails.logger).to receive(:info).twice + expect(Rails.logger).to receive(:info).with("Can fix #{orphaned_path} -> #{correct_path}") + expect(Rails.logger).to receive(:info) + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(correct_path)).to be_falsey + + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(correct_path)).to be_falsey + end + + context 'when the project record is missing (Upload#absolute_path raises error)' do + let!(:lost_and_found_path) { File.join(FileUploader.root, '-', 'project-lost-found', other_project.full_path, orphaned.path) } + + before do + orphaned.model.delete + end + + it 'moves the file to lost and found' do + expect(Rails.logger).to receive(:info).twice + expect(Rails.logger).to receive(:info).with("Did move to lost and found #{orphaned_path} -> #{lost_and_found_path}") + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(lost_and_found_path)).to be_falsey + + stub_env('DRY_RUN', 'false') + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(orphaned_path)).to be_falsey + expect(File.exist?(lost_and_found_path)).to be_truthy + end + + it 'a dry run does not move the file' do + expect(Rails.logger).to receive(:info).twice + expect(Rails.logger).to receive(:info).with("Can move to lost and found #{orphaned_path} -> #{lost_and_found_path}") + expect(Rails.logger).to receive(:info) + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(lost_and_found_path)).to be_falsey + + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(lost_and_found_path)).to be_falsey + end + end + end + + context 'when the project was moved to hashed storage' do + let!(:orphaned) { create(:upload, :issuable_upload, :with_file) } + let!(:correct_path) { orphaned.absolute_path } + let!(:orphaned_path) { File.join(FileUploader.root, 'foo', 'bar', orphaned.path) } + + before do + FileUtils.mkdir_p(File.dirname(orphaned_path)) + FileUtils.mv(correct_path, orphaned_path) + end + + it 'moves the file to its proper location' do + expect(Rails.logger).to receive(:info).twice + expect(Rails.logger).to receive(:info).with("Did fix #{orphaned_path} -> #{correct_path}") + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(correct_path)).to be_falsey + + stub_env('DRY_RUN', 'false') + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(orphaned_path)).to be_falsey + expect(File.exist?(correct_path)).to be_truthy + end + + it 'a dry run does not move the file' do + expect(Rails.logger).to receive(:info).twice + expect(Rails.logger).to receive(:info).with("Can fix #{orphaned_path} -> #{correct_path}") + expect(Rails.logger).to receive(:info) + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(correct_path)).to be_falsey + + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(correct_path)).to be_falsey + end + end + end + + context 'when a matching upload record can not be found' do + context 'when the file path fits the known pattern' do + let!(:orphaned) { create(:upload, :issuable_upload, :with_file, model: build(:project, :legacy_storage)) } + let!(:orphaned_path) { orphaned.absolute_path } + let!(:lost_and_found_path) { File.join(FileUploader.root, '-', 'project-lost-found', orphaned.model.full_path, orphaned.path) } + + before do + orphaned.delete + end + + it 'moves the file to lost and found' do + expect(Rails.logger).to receive(:info).twice + expect(Rails.logger).to receive(:info).with("Did move to lost and found #{orphaned_path} -> #{lost_and_found_path}") + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(lost_and_found_path)).to be_falsey + + stub_env('DRY_RUN', 'false') + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(orphaned_path)).to be_falsey + expect(File.exist?(lost_and_found_path)).to be_truthy + end + + it 'a dry run does not move the file' do + expect(Rails.logger).to receive(:info).twice + expect(Rails.logger).to receive(:info).with("Can move to lost and found #{orphaned_path} -> #{lost_and_found_path}") + expect(Rails.logger).to receive(:info) + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(lost_and_found_path)).to be_falsey + + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(lost_and_found_path)).to be_falsey + end + end + + context 'when the file path does not fit the known pattern' do + let!(:invalid_path) { File.join('group', 'file.jpg') } + let!(:orphaned_path) { File.join(FileUploader.root, invalid_path) } + let!(:lost_and_found_path) { File.join(FileUploader.root, '-', 'project-lost-found', invalid_path) } + + before do + FileUtils.mkdir_p(File.dirname(orphaned_path)) + FileUtils.touch(orphaned_path) + end + + after do + File.delete(orphaned_path) if File.exist?(orphaned_path) + end + + it 'moves the file to lost and found' do + expect(Rails.logger).to receive(:info).twice + expect(Rails.logger).to receive(:info).with("Did move to lost and found #{orphaned_path} -> #{lost_and_found_path}") + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(lost_and_found_path)).to be_falsey + + stub_env('DRY_RUN', 'false') + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(orphaned_path)).to be_falsey + expect(File.exist?(lost_and_found_path)).to be_truthy + end + + it 'a dry run does not move the file' do + expect(Rails.logger).to receive(:info).twice + expect(Rails.logger).to receive(:info).with("Can move to lost and found #{orphaned_path} -> #{lost_and_found_path}") + expect(Rails.logger).to receive(:info) + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(lost_and_found_path)).to be_falsey + + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(orphaned_path)).to be_truthy + expect(File.exist?(lost_and_found_path)).to be_falsey + end + end + end + end + + context 'non-orphaned project upload file' do + it 'does not move the file' do + tracked = create(:upload, :issuable_upload, :with_file, model: build(:project, :legacy_storage)) + tracked_path = tracked.absolute_path + + expect(Rails.logger).not_to receive(:info).with(/move|fix/i) + expect(File.exist?(tracked_path)).to be_truthy + + stub_env('DRY_RUN', 'false') + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(tracked_path)).to be_truthy + end + end + + context 'ignorable cases' do + shared_examples_for 'does not move anything' do + it 'does not move even an orphan file' do + orphaned = create(:upload, :issuable_upload, :with_file, model: project) + orphaned_path = orphaned.absolute_path + orphaned.delete + + expect(File.exist?(orphaned_path)).to be_truthy + + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(orphaned_path)).to be_truthy + end + end + + # Because we aren't concerned about these, and can save a lot of + # processing time by ignoring them. If we wish to cleanup hashed storage + # directories, it should simply require removing this test and modifying + # the find command. + context 'when the file is already in hashed storage' do + let(:project) { create(:project) } + + before do + stub_env('DRY_RUN', 'false') + expect(Rails.logger).not_to receive(:info).with(/move|fix/i) + end + + it_behaves_like 'does not move anything' + end + + context 'when DRY_RUN env var is unset' do + let(:project) { create(:project, :legacy_storage) } + + it_behaves_like 'does not move anything' + end + + context 'when DRY_RUN env var is true' do + let(:project) { create(:project, :legacy_storage) } + + before do + stub_env('DRY_RUN', 'true') + end + + it_behaves_like 'does not move anything' + end + + context 'when DRY_RUN env var is foo' do + let(:project) { create(:project, :legacy_storage) } + + before do + stub_env('DRY_RUN', 'foo') + end + + it_behaves_like 'does not move anything' + end + + it 'does not move any non-project (FileUploader) uploads' do + stub_env('DRY_RUN', 'false') + + paths = [] + orphaned1 = create(:upload, :personal_snippet_upload, :with_file) + orphaned2 = create(:upload, :namespace_upload, :with_file) + orphaned3 = create(:upload, :attachment_upload, :with_file) + paths << orphaned1.absolute_path + paths << orphaned2.absolute_path + paths << orphaned3.absolute_path + Upload.delete_all + + expect(Rails.logger).not_to receive(:info).with(/move|fix/i) + paths.each do |path| + expect(File.exist?(path)).to be_truthy + end + + run_rake_task('gitlab:cleanup:project_uploads') + + paths.each do |path| + expect(File.exist?(path)).to be_truthy + end + end + + it 'does not move any uploads in tmp (which would interfere with ongoing upload activity)' do + stub_env('DRY_RUN', 'false') + + path = File.join(FileUploader.root, 'tmp', 'foo.jpg') + FileUtils.mkdir_p(File.dirname(path)) + FileUtils.touch(path) + + expect(Rails.logger).not_to receive(:info).with(/move|fix/i) + expect(File.exist?(path)).to be_truthy + + run_rake_task('gitlab:cleanup:project_uploads') + + expect(File.exist?(path)).to be_truthy + end + end + end end diff --git a/spec/tasks/gitlab/git_rake_spec.rb b/spec/tasks/gitlab/git_rake_spec.rb index d0263ad9a37..57b006e1a39 100644 --- a/spec/tasks/gitlab/git_rake_spec.rb +++ b/spec/tasks/gitlab/git_rake_spec.rb @@ -2,45 +2,19 @@ require 'rake_helper' describe 'gitlab:git rake tasks' do let(:base_path) { 'tmp/tests/default_storage' } - - before(:all) do - @default_storage_hash = Gitlab.config.repositories.storages.default.to_h - end + let!(:project) { create(:project, :repository) } before do Rake.application.rake_require 'tasks/gitlab/git' - storages = { 'default' => Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => base_path)) } - - path = Settings.absolute("#{base_path}/@hashed/1/2/test.git") - FileUtils.mkdir_p(path) - Gitlab::Popen.popen(%W[git -C #{path} init --bare]) - allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) allow_any_instance_of(String).to receive(:color) { |string, _color| string } stub_warn_user_is_not_gitlab end - after do - FileUtils.rm_rf(Settings.absolute(base_path)) - end - describe 'fsck' do it 'outputs the integrity check for a repo' do - expect { run_rake_task('gitlab:git:fsck') }.to output(%r{Performed Checking integrity at .*@hashed/1/2/test.git}).to_stdout - end - - it 'errors out about config.lock issues' do - FileUtils.touch(Settings.absolute("#{base_path}/@hashed/1/2/test.git/config.lock")) - - expect { run_rake_task('gitlab:git:fsck') }.to output(/file exists\? ... yes/).to_stdout - end - - it 'errors out about ref lock issues' do - FileUtils.mkdir_p(Settings.absolute("#{base_path}/@hashed/1/2/test.git/refs/heads")) - FileUtils.touch(Settings.absolute("#{base_path}/@hashed/1/2/test.git/refs/heads/blah.lock")) - - expect { run_rake_task('gitlab:git:fsck') }.to output(/Ref lock files exist:/).to_stdout + expect { run_rake_task('gitlab:git:fsck') }.to output(/Performed integrity check for/).to_stdout end end end diff --git a/spec/workers/todos_destroyer/confidential_issue_worker_spec.rb b/spec/workers/todos_destroyer/confidential_issue_worker_spec.rb new file mode 100644 index 00000000000..9d7c0b8f560 --- /dev/null +++ b/spec/workers/todos_destroyer/confidential_issue_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::ConfidentialIssueWorker do + it "calls the Todos::Destroy::ConfidentialIssueService with the params it was given" do + service = double + + expect(::Todos::Destroy::ConfidentialIssueService).to receive(:new).with(100).and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100) + end +end diff --git a/spec/workers/todos_destroyer/entity_leave_worker_spec.rb b/spec/workers/todos_destroyer/entity_leave_worker_spec.rb new file mode 100644 index 00000000000..955447906aa --- /dev/null +++ b/spec/workers/todos_destroyer/entity_leave_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::EntityLeaveWorker do + it "calls the Todos::Destroy::EntityLeaveService with the params it was given" do + service = double + + expect(::Todos::Destroy::EntityLeaveService).to receive(:new).with(100, 5, 'Group').and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100, 5, 'Group') + end +end diff --git a/spec/workers/todos_destroyer/private_features_worker_spec.rb b/spec/workers/todos_destroyer/private_features_worker_spec.rb new file mode 100644 index 00000000000..9599f5ee071 --- /dev/null +++ b/spec/workers/todos_destroyer/private_features_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::PrivateFeaturesWorker do + it "calls the Todos::Destroy::PrivateFeaturesService with the params it was given" do + service = double + + expect(::Todos::Destroy::PrivateFeaturesService).to receive(:new).with(100, nil).and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100) + end +end diff --git a/spec/workers/todos_destroyer/project_private_worker_spec.rb b/spec/workers/todos_destroyer/project_private_worker_spec.rb new file mode 100644 index 00000000000..15d926fa9d5 --- /dev/null +++ b/spec/workers/todos_destroyer/project_private_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::ProjectPrivateWorker do + it "calls the Todos::Destroy::ProjectPrivateService with the params it was given" do + service = double + + expect(::Todos::Destroy::ProjectPrivateService).to receive(:new).with(100).and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100) + end +end |
