diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-12-27 11:45:16 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-12-27 11:45:16 +0000 |
commit | f444f745544528accf5811bb9a8fb0a252775d3b (patch) | |
tree | c248e283ea2825b68f405e738faea0e56ed57bba | |
parent | 58fa510acd34ab3c8b36be95f5e7b58dd9b0f5f4 (diff) | |
download | gitlab-ce-f444f745544528accf5811bb9a8fb0a252775d3b.tar.gz |
Add latest changes from gitlab-org/gitlab@12-6-stable-ee
17 files changed, 190 insertions, 24 deletions
diff --git a/app/finders/clusters/knative_serving_namespace_finder.rb b/app/finders/clusters/knative_serving_namespace_finder.rb index d3db5be558c..b6cf84beb79 100644 --- a/app/finders/clusters/knative_serving_namespace_finder.rb +++ b/app/finders/clusters/knative_serving_namespace_finder.rb @@ -12,6 +12,14 @@ module Clusters cluster.kubeclient&.get_namespace(Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE) rescue Kubeclient::ResourceNotFoundError nil + rescue Kubeclient::HttpError => e + # If the kubernetes auth engine is enabled, it will return 403 + if e.error_code == 403 + Gitlab::ErrorTracking.track_exception(e) + nil + else + raise + end end end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 29ec41ef1a1..3710a0b914e 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -32,6 +32,7 @@ module Ci has_many :stages, -> { order(position: :asc) }, inverse_of: :pipeline has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline + has_many :latest_statuses_ordered_by_stage, -> { latest.order(:stage_idx, :stage) }, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline has_many :processables, -> { processables }, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline @@ -45,6 +46,7 @@ module Ci has_many :merge_requests_as_head_pipeline, foreign_key: "head_pipeline_id", class_name: 'MergeRequest' has_many :pending_builds, -> { pending }, foreign_key: :commit_id, class_name: 'Ci::Build' + has_many :failed_builds, -> { latest.failed }, foreign_key: :commit_id, class_name: 'Ci::Build', inverse_of: :pipeline has_many :retryable_builds, -> { latest.failed_or_canceled.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build' has_many :cancelable_statuses, -> { cancelable }, foreign_key: :commit_id, class_name: 'CommitStatus' has_many :manual_actions, -> { latest.manual_actions.includes(:project) }, foreign_key: :commit_id, class_name: 'Ci::Build' @@ -377,9 +379,7 @@ module Ci end def legacy_stages_using_composite_status - stages = statuses.latest - .order(:stage_idx, :stage) - .group_by(&:stage) + stages = latest_statuses_ordered_by_stage.group_by(&:stage) stages.map do |stage_name, jobs| composite_status = Gitlab::Ci::Status::Composite diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb index cddb894fd64..6b2a1bfe666 100644 --- a/app/serializers/pipeline_entity.rb +++ b/app/serializers/pipeline_entity.rb @@ -78,7 +78,7 @@ class PipelineEntity < Grape::Entity end expose :failed_builds, if: -> (*) { can_retry? }, using: JobEntity do |pipeline| - pipeline.builds.failed + pipeline.failed_builds end private diff --git a/app/serializers/pipeline_serializer.rb b/app/serializers/pipeline_serializer.rb index fc3160e3c69..b25a1ea9209 100644 --- a/app/serializers/pipeline_serializer.rb +++ b/app/serializers/pipeline_serializer.rb @@ -40,7 +40,11 @@ class PipelineSerializer < BaseSerializer def preloaded_relations [ + :latest_statuses_ordered_by_stage, :stages, + { + failed_builds: %i(project metadata) + }, :retryable_builds, :cancelable_statuses, :trigger_requests, @@ -48,6 +52,7 @@ class PipelineSerializer < BaseSerializer :scheduled_actions, :artifacts, :merge_request, + :user, { pending_builds: :project, project: [:route, { namespace: :route }], diff --git a/changelogs/unreleased/121714-fix-sentry-stack-trace-highlight-for-php.yml b/changelogs/unreleased/121714-fix-sentry-stack-trace-highlight-for-php.yml new file mode 100644 index 00000000000..4f3b80dfcaa --- /dev/null +++ b/changelogs/unreleased/121714-fix-sentry-stack-trace-highlight-for-php.yml @@ -0,0 +1,5 @@ +--- +title: Fix stack trace highlight for PHP +merge_request: 22258 +author: +type: fixed diff --git a/changelogs/unreleased/alexives-119379-handle_http_error_instead_of_not_found.yml b/changelogs/unreleased/alexives-119379-handle_http_error_instead_of_not_found.yml new file mode 100644 index 00000000000..6f5eea45660 --- /dev/null +++ b/changelogs/unreleased/alexives-119379-handle_http_error_instead_of_not_found.yml @@ -0,0 +1,5 @@ +--- +title: Handle forbidden error when checking for knative +merge_request: 22170 +author: +type: fixed diff --git a/changelogs/unreleased/sh-optimize-pipeline-sql.yml b/changelogs/unreleased/sh-optimize-pipeline-sql.yml new file mode 100644 index 00000000000..f40fff99454 --- /dev/null +++ b/changelogs/unreleased/sh-optimize-pipeline-sql.yml @@ -0,0 +1,5 @@ +--- +title: Eliminate N+1 queries in PipelinesController#index +merge_request: 22189 +author: +type: performance diff --git a/doc/api/issue_links.md b/doc/api/issue_links.md index b7e21310a19..9351b3e4dd5 100644 --- a/doc/api/issue_links.md +++ b/doc/api/issue_links.md @@ -57,7 +57,7 @@ Parameters: Creates a two-way relation between two issues. User must be allowed to update both issues in order to succeed. ``` -POST /projects/:id/issues/:issue_iid/links/:target_project_id/:target_issue_iid +POST /projects/:id/issues/:issue_iid/links ``` | Attribute | Type | Required | Description | @@ -67,6 +67,12 @@ POST /projects/:id/issues/:issue_iid/links/:target_project_id/:target_issue_iid | `target_project_id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) of a target project | | `target_issue_iid` | integer/string | yes | The internal ID of a target project's issue | +```bash +curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/4/issues/1/links?target_project_id=5&target_issue_iid=1" +``` + +Example response: + ```json { "source_issue" : { diff --git a/doc/user/project/repository/index.md b/doc/user/project/repository/index.md index fc422bb5aba..fad8cbe81cb 100644 --- a/doc/user/project/repository/index.md +++ b/doc/user/project/repository/index.md @@ -116,6 +116,35 @@ rendered to HTML when viewed. Interactive features, including JavaScript plots, will not work when viewed in GitLab. +### OpenAPI viewer + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/19515) in GitLab 12.6. + +GitLab can render OpenAPI specification files with its file viewer, provided +their filenames include `openapi` or `swagger` and their extension is `yaml`, +`yml`, or `json`. The following examples are all correct: + +- `openapi.yml` +- `openapi.yaml` +- `openapi.json` +- `swagger.yml` +- `swagger.yaml` +- `swagger.json` +- `gitlab_swagger.yml` +- `openapi_gitlab.yml` +- `OpenAPI.YML` +- `openapi.Yaml` +- `openapi.JSON` +- `openapi.gitlab.yml` +- `gitlab.openapi.yml` + +Then, to render them: + +1. Navigate to the OpenAPI file in your repository in GitLab's UI. +1. Click the "Display OpenAPI" button which is located between the "Display source" + and "Edit" buttons (when an OpenAPI file is found, it replaces the + "Display rendered file" button). + ## Branches For details, see [Branches](branches/index.md). diff --git a/lib/gitlab/error_tracking/stack_trace_highlight_decorator.rb b/lib/gitlab/error_tracking/stack_trace_highlight_decorator.rb index a403275fd4e..1e490e52c43 100644 --- a/lib/gitlab/error_tracking/stack_trace_highlight_decorator.rb +++ b/lib/gitlab/error_tracking/stack_trace_highlight_decorator.rb @@ -28,7 +28,7 @@ module Gitlab end def highlight_entry_context(filename, context) - language = Rouge::Lexer.guess_by_filename(filename).tag + language = guess_language_by_filename(filename) context.map do |line_number, line_of_code| [ @@ -38,6 +38,12 @@ module Gitlab ] end end + + def guess_language_by_filename(filename) + Rouge::Lexer.guess_by_filename(filename).tag + rescue Rouge::Guesser::Ambiguous => e + e.alternatives.min_by(&:tag)&.tag + end end end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index b4549e4e635..902a84a843b 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -6,7 +6,7 @@ describe Projects::PipelinesController do include ApiHelpers let_it_be(:user) { create(:user) } - let(:project) { create(:project, :public, :repository) } + let_it_be(:project) { create(:project, :public, :repository) } let(:feature) { ProjectFeature::ENABLED } before do @@ -19,12 +19,12 @@ describe Projects::PipelinesController do describe 'GET index.json' do before do - %w(pending running success failed canceled).each_with_index do |status, index| - create_pipeline(status, project.commit("HEAD~#{index}")) - end + create_all_pipeline_types end context 'when using persisted stages', :request_store do + render_views + before do stub_feature_flags(ci_pipeline_persisted_stages: true) end @@ -32,9 +32,7 @@ describe Projects::PipelinesController do it 'returns serialized pipelines', :request_store do expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original - queries = ActiveRecord::QueryRecorder.new do - get_pipelines_index_json - end + get_pipelines_index_json expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('pipeline') @@ -49,8 +47,22 @@ describe Projects::PipelinesController do json_response.dig('pipelines', 0, 'details', 'stages').tap do |stages| expect(stages.count).to eq 3 end + end + + it 'does not execute N+1 queries' do + get_pipelines_index_json + + control_count = ActiveRecord::QueryRecorder.new do + get_pipelines_index_json + end.count - expect(queries.count).to be + create_all_pipeline_types + + # There appears to be one extra query for Pipelines#has_warnings? for some reason + expect { get_pipelines_index_json }.not_to exceed_query_limit(control_count + 1) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['pipelines'].count).to eq 10 end end @@ -133,19 +145,27 @@ describe Projects::PipelinesController do format: :json end + def create_all_pipeline_types + %w(pending running success failed canceled).each_with_index do |status, index| + create_pipeline(status, project.commit("HEAD~#{index}")) + end + end + def create_pipeline(status, sha) + user = create(:user) pipeline = create(:ci_empty_pipeline, status: status, project: project, - sha: sha) + sha: sha, + user: user) - create_build(pipeline, 'build', 1, 'build') - create_build(pipeline, 'test', 2, 'test') - create_build(pipeline, 'deploy', 3, 'deploy') + create_build(pipeline, 'build', 1, 'build', user) + create_build(pipeline, 'test', 2, 'test', user) + create_build(pipeline, 'deploy', 3, 'deploy', user) end - def create_build(pipeline, stage, stage_idx, name) + def create_build(pipeline, stage, stage_idx, name, user = nil) status = %w[created running pending success failed canceled].sample - create(:ci_build, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name, status: status) + create(:ci_build, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name, status: status, user: user) end end diff --git a/spec/factories/error_tracking/error_event.rb b/spec/factories/error_tracking/error_event.rb index c4dcd67bc9f..1590095f1bd 100644 --- a/spec/factories/error_tracking/error_event.rb +++ b/spec/factories/error_tracking/error_event.rb @@ -36,6 +36,16 @@ FactoryBot.define do ] }, { + 'function' => 'print', + 'lineNo' => 3, + 'filename' => 'hello_world.php', + 'context' => [ + [1, "// PHP/Hack example\n"], + [2, "<?php\n"], + [3, "echo 'Hello, World!';\n"] + ] + }, + { 'filename' => 'blank.txt' } ] diff --git a/spec/finders/clusters/knative_serving_namespace_finder_spec.rb b/spec/finders/clusters/knative_serving_namespace_finder_spec.rb new file mode 100644 index 00000000000..f3587df680a --- /dev/null +++ b/spec/finders/clusters/knative_serving_namespace_finder_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Clusters::KnativeServingNamespaceFinder do + include KubernetesHelpers + let(:cluster) { create(:cluster, :project, :provided_by_gcp) } + let(:service) { environment.deployment_platform } + let(:project) { cluster.cluster_project.project } + let(:environment) { create(:environment, project: project) } + + subject { described_class.new(cluster) } + + before do + stub_kubeclient_discover(service.api_url) + end + + it 'finds the namespace in a cluster where it exists' do + stub_kubeclient_get_namespace(service.api_url, namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE) + expect(subject.execute).to be_a Kubeclient::Resource + end + + it 'returns nil in a cluster where it does not' do + stub_kubeclient_get_namespace( + service.api_url, + namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE, + response: { + status: [404, "Resource Not Found"] + } + ) + expect(subject.execute).to be nil + end + + it 'returns nil in a cluster where the lookup results in a 403 as it will in some versions of kubernetes' do + stub_kubeclient_get_namespace( + service.api_url, + namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE, + response: { + status: [403, "Resource Not Found"] + } + ) + expect(subject.execute).to be nil + end + + it 'raises an error if error code is not 404 or 403' do + stub_kubeclient_get_namespace( + service.api_url, + namespace: Clusters::Kubernetes::KNATIVE_SERVING_NAMESPACE, + response: { + status: [500, "Internal Server Error"] + } + ) + expect { subject.execute }.to raise_error(Kubeclient::HttpError) + end +end diff --git a/spec/lib/gitlab/error_tracking/stack_trace_highlight_decorator_spec.rb b/spec/lib/gitlab/error_tracking/stack_trace_highlight_decorator_spec.rb index 04ef5ba516e..d553fb4848b 100644 --- a/spec/lib/gitlab/error_tracking/stack_trace_highlight_decorator_spec.rb +++ b/spec/lib/gitlab/error_tracking/stack_trace_highlight_decorator_spec.rb @@ -49,6 +49,16 @@ describe Gitlab::ErrorTracking::StackTraceHighlightDecorator do ] }, { + 'function' => 'print', + 'lineNo' => 3, + 'filename' => 'hello_world.php', + 'context' => [ + [1, '<span id="LC1" class="line" lang="hack"><span class="c1">// PHP/Hack example</span></span>'], + [2, '<span id="LC1" class="line" lang="hack"><span class="cp"><?php</span></span>'], + [3, '<span id="LC1" class="line" lang="hack"><span class="k">echo</span> <span class="s1">\'Hello, World!\'</span><span class="p">;</span></span>'] + ] + }, + { 'filename' => 'blank.txt' } ] diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 2ea563c50b6..817aedc19b0 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -160,6 +160,7 @@ ci_pipelines: - user - stages - statuses +- latest_statuses_ordered_by_stage - builds - processables - trigger_requests @@ -168,6 +169,7 @@ ci_pipelines: - auto_canceled_pipelines - auto_canceled_jobs - pending_builds +- failed_builds - retryable_builds - cancelable_statuses - manual_actions diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index f1f761a6fd0..8158277ffbc 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -159,7 +159,7 @@ describe PipelineSerializer do it 'verifies number of queries', :request_store do recorded = ActiveRecord::QueryRecorder.new { subject } - expected_queries = Gitlab.ee? ? 38 : 35 + expected_queries = Gitlab.ee? ? 42 : 39 expect(recorded.count).to be_within(2).of(expected_queries) expect(recorded.cached_count).to eq(0) @@ -180,7 +180,7 @@ describe PipelineSerializer do # pipeline. With the same ref this check is cached but if refs are # different then there is an extra query per ref # https://gitlab.com/gitlab-org/gitlab-foss/issues/46368 - expected_queries = Gitlab.ee? ? 41 : 38 + expected_queries = Gitlab.ee? ? 45 : 42 expect(recorded.count).to be_within(2).of(expected_queries) expect(recorded.cached_count).to eq(0) diff --git a/spec/support/helpers/kubernetes_helpers.rb b/spec/support/helpers/kubernetes_helpers.rb index ad4ae93a027..b2145ca729f 100644 --- a/spec/support/helpers/kubernetes_helpers.rb +++ b/spec/support/helpers/kubernetes_helpers.rb @@ -229,9 +229,9 @@ module KubernetesHelpers .to_return(kube_response(kube_v1_namespace_list_body)) end - def stub_kubeclient_get_namespace(api_url, namespace: 'default') + def stub_kubeclient_get_namespace(api_url, namespace: 'default', response: kube_response({})) WebMock.stub_request(:get, api_url + "/api/v1/namespaces/#{namespace}") - .to_return(kube_response({})) + .to_return(response) end def stub_kubeclient_put_cluster_role(api_url, name) |