diff options
Diffstat (limited to 'app')
24 files changed, 321 insertions, 36 deletions
diff --git a/app/assets/javascripts/clusters/components/applications.vue b/app/assets/javascripts/clusters/components/applications.vue index 704515cf70c..fe2ad562ad5 100644 --- a/app/assets/javascripts/clusters/components/applications.vue +++ b/app/assets/javascripts/clusters/components/applications.vue @@ -129,9 +129,6 @@ export default { crossplaneInstalled() { return this.applications.crossplane.status === APPLICATION_STATUS.INSTALLED; }, - enableClusterApplicationElasticStack() { - return gon.features && gon.features.enableClusterApplicationElasticStack; - }, ingressModSecurityDescription() { const escapedUrl = _.escape(this.ingressModSecurityHelpPath); @@ -655,7 +652,6 @@ Crossplane runs inside your Kubernetes cluster and supports secure connectivity </div> </application-row> <application-row - v-if="enableClusterApplicationElasticStack" id="elastic_stack" :logo-url="elasticStackLogo" :title="applications.elastic_stack.title" diff --git a/app/assets/javascripts/repository/components/breadcrumbs.vue b/app/assets/javascripts/repository/components/breadcrumbs.vue index f306910df05..9d6eda55c1e 100644 --- a/app/assets/javascripts/repository/components/breadcrumbs.vue +++ b/app/assets/javascripts/repository/components/breadcrumbs.vue @@ -107,10 +107,16 @@ export default { return acc.concat({ name, path, - to: `/-/tree/${this.ref}${path}`, + to: `/-/tree/${escape(this.ref)}${path}`, }); }, - [{ name: this.projectShortPath, path: '/', to: `/-/tree/${this.ref}/` }], + [ + { + name: this.projectShortPath, + path: '/', + to: `/-/tree/${escape(this.ref)}/`, + }, + ], ); }, canCreateMrFromFork() { diff --git a/app/assets/javascripts/repository/components/table/parent_row.vue b/app/assets/javascripts/repository/components/table/parent_row.vue index c919f2d42cb..096c25a693f 100644 --- a/app/assets/javascripts/repository/components/table/parent_row.vue +++ b/app/assets/javascripts/repository/components/table/parent_row.vue @@ -28,7 +28,7 @@ export default { return splitArray.join('/'); }, parentRoute() { - return { path: `/-/tree/${this.commitRef}/${this.parentPath}` }; + return { path: `/-/tree/${escape(this.commitRef)}/${this.parentPath}` }; }, }, methods: { diff --git a/app/assets/javascripts/repository/components/table/row.vue b/app/assets/javascripts/repository/components/table/row.vue index a0a9a5657a8..e7bbca957c1 100644 --- a/app/assets/javascripts/repository/components/table/row.vue +++ b/app/assets/javascripts/repository/components/table/row.vue @@ -90,7 +90,7 @@ export default { }, computed: { routerLinkTo() { - return this.isFolder ? { path: `/-/tree/${this.ref}/${this.path}` } : null; + return this.isFolder ? { path: `/-/tree/${escape(this.ref)}/${this.path}` } : null; }, iconName() { return `fa-${getIconName(this.type, this.path)}`; diff --git a/app/assets/javascripts/repository/log_tree.js b/app/assets/javascripts/repository/log_tree.js index 2a66659fecb..aefa4963d5f 100644 --- a/app/assets/javascripts/repository/log_tree.js +++ b/app/assets/javascripts/repository/log_tree.js @@ -27,7 +27,10 @@ export function fetchLogsTree(client, path, offset, resolver = null) { fetchpromise = axios .get( - `${gon.relative_url_root}/${projectPath}/-/refs/${ref}/logs_tree/${path.replace(/^\//, '')}`, + `${gon.relative_url_root}/${projectPath}/-/refs/${escape(ref)}/logs_tree/${path.replace( + /^\//, + '', + )}`, { params: { format: 'json', offset }, }, diff --git a/app/assets/javascripts/repository/router.js b/app/assets/javascripts/repository/router.js index fa544444be8..6c31c9bae82 100644 --- a/app/assets/javascripts/repository/router.js +++ b/app/assets/javascripts/repository/router.js @@ -12,7 +12,7 @@ export default function createRouter(base, baseRef) { base: joinPaths(gon.relative_url_root || '', base), routes: [ { - path: `/-/tree/${baseRef}(/.*)?`, + path: `/-/tree/${escape(baseRef)}(/.*)?`, name: 'treePath', component: TreePage, props: route => ({ diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb index 52a5f801bad..2c9ee69c8c4 100644 --- a/app/controllers/clusters/clusters_controller.rb +++ b/app/controllers/clusters/clusters_controller.rb @@ -12,9 +12,6 @@ class Clusters::ClustersController < Clusters::BaseController before_action :authorize_update_cluster!, only: [:update] before_action :authorize_admin_cluster!, only: [:destroy, :clear_cache] before_action :update_applications_status, only: [:cluster_status] - before_action only: [:show] do - push_frontend_feature_flag(:enable_cluster_application_elastic_stack) - end helper_method :token_in_session diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index c0c8474232a..72e76f56540 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -64,6 +64,10 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic options = additional_attributes.merge(diff_view: diff_view) + if @merge_request.project.context_commits_enabled? + options[:context_commits] = @merge_request.context_commits + end + render json: DiffsSerializer.new(request).represent(diffs, options) end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 7cdda071813..e21930c9cfe 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -116,6 +116,15 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo } end + def context_commits + return render_404 unless project.context_commits_enabled? + + # Get commits from repository + # or from cache if already merged + commits = ContextCommitsFinder.new(project, @merge_request, { search: params[:search], limit: params[:limit], offset: params[:offset] }).execute + render json: CommitEntity.represent(commits, { type: :full, request: merge_request }) + end + def test_reports reports_response(@merge_request.compare_test_reports) end diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb index 2ed29b937ad..19a8df3e9a5 100644 --- a/app/controllers/projects/repositories_controller.rb +++ b/app/controllers/projects/repositories_controller.rb @@ -81,7 +81,7 @@ class Projects::RepositoriesController < Projects::ApplicationController def assign_archive_vars if params[:id] - @ref, @filename = extract_ref(params[:id]) + @ref, @filename = extract_ref_and_filename(params[:id]) else @ref = params[:ref] @filename = nil @@ -89,6 +89,26 @@ class Projects::RepositoriesController < Projects::ApplicationController rescue InvalidPathError render_404 end + + # path can be of the form: + # master + # master/first.zip + # master/first/second.tar.gz + # master/first/second/third.zip + # + # In the archive case, we know that the last value is always the filename, so we + # do a greedy match to extract the ref. This avoid having to pull all ref names + # from Redis. + def extract_ref_and_filename(id) + path = id.strip + data = path.match(/(.*)\/(.*)/) + + if data + [data[1], data[2]] + else + [path, nil] + end + end end Projects::RepositoriesController.prepend_if_ee('EE::Projects::RepositoriesController') diff --git a/app/finders/context_commits_finder.rb b/app/finders/context_commits_finder.rb new file mode 100644 index 00000000000..f1b3eb43e84 --- /dev/null +++ b/app/finders/context_commits_finder.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +class ContextCommitsFinder + def initialize(project, merge_request, params = {}) + @project = project + @merge_request = merge_request + @search = params[:search] + @limit = (params[:limit] || 40).to_i + @offset = (params[:offset] || 0).to_i + end + + def execute + commits = init_collection + commits = filter_existing_commits(commits) + + commits + end + + private + + attr_reader :project, :merge_request, :search, :limit, :offset + + def init_collection + commits = + if search.present? + search_commits + else + project.repository.commits(merge_request.source_branch, { limit: limit, offset: offset }) + end + + commits + end + + def filter_existing_commits(commits) + commits.select! { |commit| already_included_ids.exclude?(commit.id) } + + commits + end + + def search_commits + key = search.strip + commits = [] + if Commit.valid_hash?(key) + mr_existing_commits_ids = merge_request.commits.map(&:id) + if mr_existing_commits_ids.exclude? key + commit_by_sha = project.repository.commit(key) + commits = [commit_by_sha] if commit_by_sha + end + else + commits = project.repository.find_commits_by_message(search, nil, nil, 20) + end + + commits + end + + def already_included_ids + mr_existing_commits_ids = merge_request.commits.map(&:id) + mr_context_commits_ids = merge_request.context_commits.map(&:id) + + mr_existing_commits_ids + mr_context_commits_ids + end +end diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 620a63fdc46..4c3c4931387 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -29,6 +29,8 @@ module DiffHelper if action_name == 'diff_for_path' options[:expanded] = true options[:paths] = params.values_at(:old_path, :new_path) + elsif action_name == 'show' + options[:include_context_commits] = true unless @project.context_commits_enabled? end options diff --git a/app/models/concerns/cached_commit.rb b/app/models/concerns/cached_commit.rb new file mode 100644 index 00000000000..183d5728743 --- /dev/null +++ b/app/models/concerns/cached_commit.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module CachedCommit + extend ActiveSupport::Concern + + def to_hash + Gitlab::Git::Commit::SERIALIZE_KEYS.each_with_object({}) do |key, hash| + hash[key] = public_send(key) # rubocop:disable GitlabSecurity/PublicSend + end + end + + # We don't save these, because they would need a table or a serialised + # field. They aren't used anywhere, so just pretend the commit has no parents. + def parent_ids + [] + end +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 48c5c0152b5..3174a3269b4 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -34,6 +34,8 @@ class MergeRequest < ApplicationRecord has_internal_id :iid, scope: :target_project, track_if: -> { !importing? }, init: ->(s) { s&.target_project&.merge_requests&.maximum(:iid) } has_many :merge_request_diffs + has_many :merge_request_context_commits + has_many :merge_request_context_commit_diff_files, through: :merge_request_context_commits, source: :diff_files has_many :merge_request_milestones has_many :milestones, through: :merge_request_milestones @@ -399,6 +401,10 @@ class MergeRequest < ApplicationRecord "#{project.to_reference_base(from, full: full)}#{reference}" end + def context_commits + @context_commits ||= merge_request_context_commits.map(&:to_commit) + end + def commits(limit: nil) return merge_request_diff.commits(limit: limit) if persisted? diff --git a/app/models/merge_request_context_commit.rb b/app/models/merge_request_context_commit.rb new file mode 100644 index 00000000000..eecb10e6dbc --- /dev/null +++ b/app/models/merge_request_context_commit.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class MergeRequestContextCommit < ApplicationRecord + include CachedCommit + include ShaAttribute + + belongs_to :merge_request + has_many :diff_files, class_name: 'MergeRequestContextCommitDiffFile' + + sha_attribute :sha + + validates :sha, presence: true + validates :sha, uniqueness: { message: 'has already been added' } + + # delete all MergeRequestContextCommit & MergeRequestContextCommitDiffFile for given merge_request & commit SHAs + def self.delete_bulk(merge_request, commits) + commit_ids = commits.map(&:sha) + merge_request.merge_request_context_commits.where(sha: commit_ids).delete_all + end + + # create MergeRequestContextCommit by given commit sha and it's diff file record + def self.bulk_insert(*args) + Gitlab::Database.bulk_insert('merge_request_context_commits', *args) + end + + def to_commit + # Here we are storing the commit sha because to_hash removes the sha parameter and we lose + # the reference, this happens because we are storing the ID in db and the Commit class replaces + # id with sha and removes it, so in our case it will be some incremented integer which is not + # what we want + commit_hash = attributes.except('id').to_hash + commit_hash['id'] = sha + Commit.from_hash(commit_hash, merge_request.target_project) + end +end diff --git a/app/models/merge_request_context_commit_diff_file.rb b/app/models/merge_request_context_commit_diff_file.rb new file mode 100644 index 00000000000..9dce7c53ab6 --- /dev/null +++ b/app/models/merge_request_context_commit_diff_file.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class MergeRequestContextCommitDiffFile < ApplicationRecord + include Gitlab::EncodingHelper + include ShaAttribute + include DiffFile + + belongs_to :merge_request_context_commit, inverse_of: :diff_files + + sha_attribute :sha + alias_attribute :id, :sha + + # create MergeRequestContextCommitDiffFile by given diff file record(s) + def self.bulk_insert(*args) + Gitlab::Database.bulk_insert('merge_request_context_commit_diff_files', *args) + end +end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index fa633a1a725..ffe95e8f034 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -560,6 +560,10 @@ class MergeRequestDiff < ApplicationRecord opening_external_diff do collection = merge_request_diff_files + if options[:include_context_commits] + collection += merge_request.merge_request_context_commit_diff_files + end + if paths = options[:paths] collection = collection.where('old_path IN (?) OR new_path IN (?)', paths, paths) end diff --git a/app/models/merge_request_diff_commit.rb b/app/models/merge_request_diff_commit.rb index b897bbc8cf5..aa41a68f184 100644 --- a/app/models/merge_request_diff_commit.rb +++ b/app/models/merge_request_diff_commit.rb @@ -2,6 +2,7 @@ class MergeRequestDiffCommit < ApplicationRecord include ShaAttribute + include CachedCommit belongs_to :merge_request_diff @@ -9,8 +10,6 @@ class MergeRequestDiffCommit < ApplicationRecord alias_attribute :id, :sha def self.create_bulk(merge_request_diff_id, commits) - sha_attribute = Gitlab::Database::ShaAttribute.new - rows = commits.map.with_index do |commit, index| # See #parent_ids. commit_hash = commit.to_hash.except(:parent_ids) @@ -19,7 +18,7 @@ class MergeRequestDiffCommit < ApplicationRecord commit_hash.merge( merge_request_diff_id: merge_request_diff_id, relative_order: index, - sha: sha_attribute.serialize(sha), # rubocop:disable Cop/ActiveRecordSerialize + sha: Gitlab::Database::ShaAttribute.serialize(sha), # rubocop:disable Cop/ActiveRecordSerialize authored_date: Gitlab::Database.sanitize_timestamp(commit_hash[:authored_date]), committed_date: Gitlab::Database.sanitize_timestamp(commit_hash[:committed_date]) ) @@ -27,16 +26,4 @@ class MergeRequestDiffCommit < ApplicationRecord Gitlab::Database.bulk_insert(self.table_name, rows) end - - def to_hash - Gitlab::Git::Commit::SERIALIZE_KEYS.each_with_object({}) do |key, hash| - hash[key] = public_send(key) # rubocop:disable GitlabSecurity/PublicSend - end - end - - # We don't save these, because they would need a table or a serialised - # field. They aren't used anywhere, so just pretend the commit has no parents. - def parent_ids - [] - end end diff --git a/app/models/project.rb b/app/models/project.rb index 3aa8430f3a2..8cb35904d92 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -763,6 +763,10 @@ class Project < ApplicationRecord Feature.enabled?(:unlink_fork_network_upon_visibility_decrease, self, default_enabled: true) end + def context_commits_enabled? + Feature.enabled?(:context_commits, default_enabled: true) + end + def empty_repo? repository.empty? end diff --git a/app/serializers/diffs_entity.rb b/app/serializers/diffs_entity.rb index 88e09ae8c0b..02f78180fb0 100644 --- a/app/serializers/diffs_entity.rb +++ b/app/serializers/diffs_entity.rb @@ -24,6 +24,10 @@ class DiffsEntity < Grape::Entity ) end + expose :context_commits, using: API::Entities::Commit, if: -> (diffs, options) { merge_request&.project&.context_commits_enabled? } do |diffs| + options[:context_commits] + end + expose :merge_request_diff, using: MergeRequestDiffEntity do |diffs| options[:merge_request_diff] end diff --git a/app/services/clusters/applications/base_service.rb b/app/services/clusters/applications/base_service.rb index 844da11e5cb..2585d815e07 100644 --- a/app/services/clusters/applications/base_service.rb +++ b/app/services/clusters/applications/base_service.rb @@ -58,7 +58,7 @@ module Clusters end def instantiate_application - raise_invalid_application_error if invalid_application? + raise_invalid_application_error if unknown_application? builder || raise(InvalidApplicationError, "invalid application: #{application_name}") end @@ -67,10 +67,6 @@ module Clusters raise(InvalidApplicationError, "invalid application: #{application_name}") end - def invalid_application? - unknown_application? || (application_name == Applications::ElasticStack.application_name && !Feature.enabled?(:enable_cluster_application_elastic_stack)) - end - def unknown_application? Clusters::Cluster::APPLICATIONS.keys.exclude?(application_name) end diff --git a/app/services/merge_requests/add_context_service.rb b/app/services/merge_requests/add_context_service.rb new file mode 100644 index 00000000000..bb82fa23468 --- /dev/null +++ b/app/services/merge_requests/add_context_service.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +module MergeRequests + class AddContextService < MergeRequests::BaseService + def execute + return error("You are not allowed to access the requested resource", 403) unless current_user&.can?(:update_merge_request, merge_request) + return error("Context commits: #{duplicates} are already created", 400) unless duplicates.empty? + return error("One or more context commits' sha is not valid.", 400) if commits.size != commit_ids.size + + context_commit_ids = [] + MergeRequestContextCommit.transaction do + context_commit_ids = MergeRequestContextCommit.bulk_insert(context_commit_rows, return_ids: true) + MergeRequestContextCommitDiffFile.bulk_insert(diff_rows(context_commit_ids)) + end + + commits + end + + private + + def raw_repository + project.repository.raw_repository + end + + def merge_request + params[:merge_request] + end + + def commit_ids + params[:commits] + end + + def commits + project.repository.commits_by(oids: commit_ids) + end + + def context_commit_rows + @context_commit_rows ||= build_context_commit_rows(merge_request.id, commits) + end + + def diff_rows(context_commit_ids) + @diff_rows ||= build_diff_rows(raw_repository, commits, context_commit_ids) + end + + def encode_in_base64?(diff_text) + (diff_text.encoding == Encoding::BINARY && !diff_text.ascii_only?) || + diff_text.include?("\0") + end + + def duplicates + existing_oids = merge_request.merge_request_context_commits.map { |commit| commit.sha.to_s } + duplicate_oids = existing_oids.select do |existing_oid| + commit_ids.select { |commit_id| existing_oid.start_with?(commit_id) }.count > 0 + end + + duplicate_oids + end + + def build_context_commit_rows(merge_request_id, commits) + commits.map.with_index do |commit, index| + # generate context commit information for given commit + commit_hash = commit.to_hash.except(:parent_ids) + sha = Gitlab::Database::ShaAttribute.serialize(commit_hash.delete(:id)) + commit_hash.merge( + merge_request_id: merge_request_id, + relative_order: index, + sha: sha, + authored_date: Gitlab::Database.sanitize_timestamp(commit_hash[:authored_date]), + committed_date: Gitlab::Database.sanitize_timestamp(commit_hash[:committed_date]) + ) + end + end + + def build_diff_rows(raw_repository, commits, context_commit_ids) + diff_rows = [] + diff_order = 0 + + commits.flat_map.with_index do |commit, index| + commit_hash = commit.to_hash.except(:parent_ids) + sha = Gitlab::Database::ShaAttribute.serialize(commit_hash.delete(:id)) + # generate context commit diff information for given commit + diffs = commit.diffs + + compare = Gitlab::Git::Compare.new( + raw_repository, + diffs.diff_refs.start_sha, + diffs.diff_refs.head_sha + ) + compare.diffs.map do |diff| + diff_hash = diff.to_hash.merge( + sha: sha, + binary: false, + merge_request_context_commit_id: context_commit_ids[index], + relative_order: diff_order + ) + + # Compatibility with old diffs created with Psych. + diff_hash.tap do |hash| + diff_text = hash[:diff] + + if encode_in_base64?(diff_text) + hash[:binary] = true + hash[:diff] = [diff_text].pack('m0') + end + end + + # Increase order for commit so when present the diffs we can use it to keep order + diff_order += 1 + diff_rows << diff_hash + end + end + + diff_rows + end + end +end diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 3096b2e43fe..e8dd467c7ff 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -209,7 +209,7 @@ %span = _('Artifacts') - - if !should_display_analytics_pages_in_sidebar && project_nav_tab?(:pipelines) + - if project_nav_tab?(:pipelines) = nav_link(controller: :pipeline_schedules) do = link_to pipeline_schedules_path(@project), title: _('Schedules'), class: 'shortcuts-builds' do %span diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index f4560404c03..18bdbd42d0d 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -46,7 +46,7 @@ - if job.try(:trigger_request) %span.badge.badge-info= _('triggered') - if job.try(:allow_failure) - %span.badge.badge-danger= _('allowed to fail') + %span.badge.badge-warning= _('allowed to fail') - if job.schedulable? %span.badge.badge-info= s_('DelayedJobs|delayed') - elsif job.action? |