diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-12-09 06:07:57 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-12-09 06:07:57 +0300 |
commit | 330eac18cef61a4f58b3601265f7b631d2311cd0 (patch) | |
tree | 87eec5d8c441581938ca908ce30199832e118b85 | |
parent | 3359a5a56337b93cd34b9914b6468395bfb6c514 (diff) |
Add latest changes from gitlab-org/gitlab@master
-rw-r--r-- | app/controllers/projects/environments/prometheus_api_controller.rb | 36 | ||||
-rw-r--r-- | app/services/prometheus/proxy_variable_substitution_service.rb | 51 | ||||
-rw-r--r-- | doc/api/README.md | 59 | ||||
-rw-r--r-- | doc/api/projects.md | 6 | ||||
-rw-r--r-- | doc/ci/enable_or_disable_ci.md | 16 | ||||
-rw-r--r-- | doc/user/clusters/applications.md | 20 | ||||
-rw-r--r-- | doc/user/project/pipelines/settings.md | 3 | ||||
-rw-r--r-- | lib/api/api.rb | 1 | ||||
-rw-r--r-- | lib/api/entities.rb | 12 | ||||
-rw-r--r-- | lib/api/remote_mirrors.rb | 30 | ||||
-rw-r--r-- | locale/gitlab.pot | 3 | ||||
-rw-r--r-- | qa/qa/specs/features/browser_ui/3_create/merge_request/squash_merge_request_spec.rb | 6 | ||||
-rw-r--r-- | spec/controllers/projects/environments/prometheus_api_controller_spec.rb | 4 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/remote_mirror.json | 26 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/remote_mirrors.json | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/prometheus/query_variables_spec.rb | 2 | ||||
-rw-r--r-- | spec/requests/api/remote_mirrors_spec.rb | 41 | ||||
-rw-r--r-- | spec/services/prometheus/proxy_variable_substitution_service_spec.rb | 143 |
18 files changed, 423 insertions, 40 deletions
diff --git a/app/controllers/projects/environments/prometheus_api_controller.rb b/app/controllers/projects/environments/prometheus_api_controller.rb index e902d218c75..98fcc594d6e 100644 --- a/app/controllers/projects/environments/prometheus_api_controller.rb +++ b/app/controllers/projects/environments/prometheus_api_controller.rb @@ -7,23 +7,34 @@ class Projects::Environments::PrometheusApiController < Projects::ApplicationCon before_action :environment def proxy - result = Prometheus::ProxyService.new( + variable_substitution_result = + variable_substitution_service.new(environment, permit_params).execute + + if variable_substitution_result[:status] == :error + return error_response(variable_substitution_result) + end + + prometheus_result = Prometheus::ProxyService.new( environment, proxy_method, proxy_path, - proxy_params + variable_substitution_result[:params] ).execute - return continue_polling_response if result.nil? - return error_response(result) if result[:status] == :error + return continue_polling_response if prometheus_result.nil? + return error_response(prometheus_result) if prometheus_result[:status] == :error - success_response(result) + success_response(prometheus_result) end private - def query_context - Gitlab::Prometheus::QueryVariables.call(environment) + def variable_substitution_service + Prometheus::ProxyVariableSubstitutionService + end + + def permit_params + params.permit! end def environment @@ -37,15 +48,4 @@ class Projects::Environments::PrometheusApiController < Projects::ApplicationCon def proxy_path params[:proxy_path] end - - def proxy_params - substitute_query_variables(params).permit! - end - - def substitute_query_variables(params) - query = params[:query] - return params unless query - - params.merge(query: query % query_context) - end end diff --git a/app/services/prometheus/proxy_variable_substitution_service.rb b/app/services/prometheus/proxy_variable_substitution_service.rb new file mode 100644 index 00000000000..d3d56987f07 --- /dev/null +++ b/app/services/prometheus/proxy_variable_substitution_service.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Prometheus + class ProxyVariableSubstitutionService < BaseService + include Stepable + + steps :add_params_to_result, :substitute_ruby_variables + + def initialize(environment, params = {}) + @environment, @params = environment, params.deep_dup + end + + def execute + execute_steps + end + + private + + def add_params_to_result(result) + result[:params] = params + + success(result) + end + + def substitute_ruby_variables(result) + return success(result) unless query + + # The % operator doesn't replace variables if the hash contains string + # keys. + result[:params][:query] = query % predefined_context.symbolize_keys + + success(result) + rescue TypeError, ArgumentError => exception + log_error(exception.message) + Gitlab::Sentry.track_acceptable_exception(exception, extra: { + template_string: query, + variables: predefined_context + }) + + error(_('Malformed string')) + end + + def predefined_context + @predefined_context ||= Gitlab::Prometheus::QueryVariables.call(@environment) + end + + def query + params[:query] + end + end +end diff --git a/doc/api/README.md b/doc/api/README.md index 6858e5b7d56..bcf238f9442 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -313,6 +313,17 @@ The following table shows the possible return codes for API requests. ## Pagination +We support two kinds of pagination methods: + +- Offset-based pagination. This is the default method and available on all endpoints. +- Keyset-based pagination. Added to selected endpoints but being + [progressively rolled out](https://gitlab.com/groups/gitlab-org/-/epics/2039). + +For large collections, we recommend keyset pagination (when available) over offset +pagination for performance reasons. + +### Offset-based pagination + Sometimes the returned result will span across many pages. When listing resources you can pass the following parameters: @@ -324,10 +335,10 @@ resources you can pass the following parameters: In the example below, we list 50 [namespaces](namespaces.md) per page. ```bash -curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/namespaces?per_page=50 +curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/namespaces?per_page=50" ``` -### Pagination Link header +#### Pagination Link header [Link headers](https://www.w3.org/wiki/LinkHeader) are sent back with each response. They have `rel` set to prev/next/first/last and contain the relevant @@ -362,7 +373,7 @@ X-Total: 8 X-Total-Pages: 3 ``` -### Other pagination headers +#### Other pagination headers Additional pagination headers are also sent back. @@ -383,6 +394,48 @@ and **behind the `api_kaminari_count_with_limit` more than 10,000, the `X-Total` and `X-Total-Pages` headers as well as the `rel="last"` `Link` are not present in the response headers. +### Keyset-based pagination + +Keyset-pagination allows for more efficient retrieval of pages and - in contrast to offset-based pagination - runtime +is independent of the size of the collection. + +This method is controlled by the following parameters: + +| Parameter | Description | +| ------------ | -------------------------------------- | +| `pagination` | `keyset` (to enable keyset pagination) | +| `per_page` | Number of items to list per page (default: `20`, max: `100`) | + +In the example below, we list 50 [projects](projects.md) per page, ordered by `id` ascending. + +```bash +curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects?pagination=keyset&per_page=50&order_by=id&sort=asc" +``` + +The response header includes a link to the next page. For example: + +``` +HTTP/1.1 200 OK +... +Link: <https://gitlab.example.com/api/v4/projects?pagination=keyset&per_page=50&order_by=id&sort=asc&id_after=42>; rel="next" +Status: 200 OK +... +``` + +The link to the next page contains an additional filter `id_after=42` which excludes records we have retrieved already. +Note the type of filter depends on the `order_by` option used and we may have more than one additional filter. + +The `Link` header is absent when the end of the collection has been reached and there are no additional records to retrieve. + +We recommend using only the given link to retrieve the next page instead of building your own URL. Apart from the headers shown, +we don't expose additional pagination headers. + +Keyset-based pagination is only supported for selected resources and ordering options: + +| Resource | Order | +| ------------------------- | -------------------------- | +| [Projects](projects.md) | `order_by=id` only | + ## Namespaced path encoding If using namespaced API calls, make sure that the `NAMESPACE/PROJECT_PATH` is diff --git a/doc/api/projects.md b/doc/api/projects.md index 222ab729810..ec3a081f5a3 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -61,6 +61,9 @@ GET /projects | `id_after` | integer | no | Limit results to projects with IDs greater than the specified ID | | `id_before` | integer | no | Limit results to projects with IDs less than the specified ID | +NOTE: **Note:** +This endpoint supports [keyset pagination](README.md#keyset-based-pagination) for selected `order_by` options. + When `simple=true` or the user is unauthenticated this returns something like: ```json @@ -309,6 +312,9 @@ GET /users/:user_id/projects | `id_after` | integer | no | Limit results to projects with IDs greater than the specified ID | | `id_before` | integer | no | Limit results to projects with IDs less than the specified ID | +NOTE: **Note:** +This endpoint supports [keyset pagination](README.md#keyset-based-pagination) for selected `order_by` options. + ```json [ { diff --git a/doc/ci/enable_or_disable_ci.md b/doc/ci/enable_or_disable_ci.md index dcf4d8dde2d..ff104d4e177 100644 --- a/doc/ci/enable_or_disable_ci.md +++ b/doc/ci/enable_or_disable_ci.md @@ -34,13 +34,17 @@ pipelines that are run from an [external integration](../user/project/integratio ## Per-project user setting -The setting to enable or disable GitLab CI/CD Pipelines can be found in your project in -**Settings > General > Visibility, project features, permissions**. If the project -visibility is set to: +To enable or disable GitLab CI/CD Pipelines in your project: -- **Private**, only project members can access pipelines. -- **Internal** or **Public**, pipelines can be made accessible to either - project members only or everyone with access. +1. Navigate to **Settings > General > Visibility, project features, permissions**. +1. Expand the **Repository** section +1. Enable or disable the **Pipelines** checkbox as required. + +**Project visibility** will also affect pipeline visibility. If set to: + +- **Private**: Only project members can access pipelines. +- **Internal** or **Public**: Pipelines can be set to either **Only Project Members** + or **Everyone With Access** via the drop-down box. Press **Save changes** for the settings to take effect. diff --git a/doc/user/clusters/applications.md b/doc/user/clusters/applications.md index 6498545a4f6..46b7eabaeea 100644 --- a/doc/user/clusters/applications.md +++ b/doc/user/clusters/applications.md @@ -267,13 +267,19 @@ This feature: kubectl -n gitlab-managed-apps exec -it $(kubectl get pods -n gitlab-managed-apps | grep 'ingress-controller' | awk '{print $1}') -- tail -f /var/log/modsec/audit.log ``` -There is a small performance overhead by enabling `modsecurity`. However, if this is -considered significant for your application, you can toggle the feature flag back to -false by running the following command within the Rails console: - -```ruby -Feature.disable(:ingress_modsecurity) -``` +There is a small performance overhead by enabling `modsecurity`. If this is +considered significant for your application, you can either: + +- Disable ModSecurity's rule engine for your deployed application by setting + [the deployment variable](../../topics/autodevops/index.md) + `AUTO_DEVOPS_MODSECURITY_SEC_RULE_ENGINE` to `Off`. This will prevent ModSecurity from + processing any requests for the given application or environment. +- Toggle the feature flag to false by running the following command within your + instance's Rails console: + + ```ruby + Feature.disable(:ingress_modsecurity) + ``` Once disabled, you must [uninstall](#uninstalling-applications) and reinstall your Ingress application for the changes to take effect. diff --git a/doc/user/project/pipelines/settings.md b/doc/user/project/pipelines/settings.md index a63da5c442f..671a55da5e3 100644 --- a/doc/user/project/pipelines/settings.md +++ b/doc/user/project/pipelines/settings.md @@ -149,6 +149,9 @@ Pipeline visibility is determined by: - Your current [user access level](../../permissions.md). - The **Public pipelines** project setting under your project's **Settings > CI/CD > General pipelines**. +NOTE: **Note:** +If the project visibility is set to **Private**, the [**Public pipelines** setting will have no effect](../../../ci/enable_or_disable_ci.md#per-project-user-setting). + This also determines the visibility of these related features: - Job output logs diff --git a/lib/api/api.rb b/lib/api/api.rb index a2bdb76b834..6949cfa8e49 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -156,6 +156,7 @@ module API mount ::API::ProtectedTags mount ::API::Releases mount ::API::Release::Links + mount ::API::RemoteMirrors mount ::API::Repositories mount ::API::Runner mount ::API::Runners diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 1297f8e87a2..f89070ea1cb 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -166,6 +166,18 @@ module API end end + class RemoteMirror < Grape::Entity + expose :id + expose :enabled + expose :safe_url, as: :url + expose :update_status + expose :last_update_at + expose :last_update_started_at + expose :last_successful_update_at + expose :last_error + expose :only_protected_branches + end + class ProjectImportStatus < ProjectIdentity expose :import_status diff --git a/lib/api/remote_mirrors.rb b/lib/api/remote_mirrors.rb new file mode 100644 index 00000000000..8a085517ce9 --- /dev/null +++ b/lib/api/remote_mirrors.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module API + class RemoteMirrors < Grape::API + include PaginationParams + + before do + # TODO: Remove flag: https://gitlab.com/gitlab-org/gitlab/issues/38121 + not_found! unless Feature.enabled?(:remote_mirrors_api, user_project) + end + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + desc "List the project's remote mirrors" do + success Entities::RemoteMirror + end + params do + use :pagination + end + get ':id/remote_mirrors' do + unauthorized! unless can?(current_user, :admin_remote_mirror, user_project) + + present paginate(user_project.remote_mirrors), + with: Entities::RemoteMirror + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6436a828df0..09071940131 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10614,6 +10614,9 @@ msgstr "" msgid "Makes this issue confidential." msgstr "" +msgid "Malformed string" +msgstr "" + msgid "Manage" msgstr "" diff --git a/qa/qa/specs/features/browser_ui/3_create/merge_request/squash_merge_request_spec.rb b/qa/qa/specs/features/browser_ui/3_create/merge_request/squash_merge_request_spec.rb index f1ba726ddec..89f0fc37f3f 100644 --- a/qa/qa/specs/features/browser_ui/3_create/merge_request/squash_merge_request_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/merge_request/squash_merge_request_spec.rb @@ -26,9 +26,11 @@ module QA merge_request.visit! - expect(page).to have_text('to be squashed') - Page::MergeRequest::Show.perform do |merge_request_page| + merge_request_page.retry_on_exception(reload: true) do + expect(merge_request_page).to have_text('to be squashed') + end + merge_request_page.mark_to_squash merge_request_page.merge! diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb index b12964f8d8b..b8cf47cb1f2 100644 --- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb +++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb @@ -63,9 +63,7 @@ describe Projects::Environments::PrometheusApiController do context 'with nil query' do let(:params_without_query) do - params = environment_params - params.delete(:query) - params + environment_params.except(:query) end before do diff --git a/spec/fixtures/api/schemas/remote_mirror.json b/spec/fixtures/api/schemas/remote_mirror.json new file mode 100644 index 00000000000..416b0f080d9 --- /dev/null +++ b/spec/fixtures/api/schemas/remote_mirror.json @@ -0,0 +1,26 @@ +{ + "type": "object", + "required": [ + "id", + "enabled", + "url", + "update_status", + "last_update_at", + "last_update_started_at", + "last_successful_update_at", + "last_error", + "only_protected_branches" + ], + "properties": { + "id": { "type": "integer" }, + "enabled": { "type": "boolean" }, + "url": { "type": "string" }, + "update_status": { "type": "string" }, + "last_update_at": { "type": ["string", "null"] }, + "last_update_started_at": { "type": ["string", "null"] }, + "last_successful_update_at": { "type": ["string", "null"] }, + "last_error": { "type": ["string", "null"] }, + "only_protected_branches": { "type": "boolean" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/remote_mirrors.json b/spec/fixtures/api/schemas/remote_mirrors.json new file mode 100644 index 00000000000..3c4600c6caa --- /dev/null +++ b/spec/fixtures/api/schemas/remote_mirrors.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "remote_mirror.json" } +} diff --git a/spec/lib/gitlab/prometheus/query_variables_spec.rb b/spec/lib/gitlab/prometheus/query_variables_spec.rb index 3f9b245a3fb..849265de513 100644 --- a/spec/lib/gitlab/prometheus/query_variables_spec.rb +++ b/spec/lib/gitlab/prometheus/query_variables_spec.rb @@ -14,7 +14,7 @@ describe Gitlab::Prometheus::QueryVariables do it do is_expected.to include(environment_filter: - %{container_name!="POD",environment="#{slug}"}) + %Q[container_name!="POD",environment="#{slug}"]) end context 'without deployment platform' do diff --git a/spec/requests/api/remote_mirrors_spec.rb b/spec/requests/api/remote_mirrors_spec.rb new file mode 100644 index 00000000000..c5ba9bd223e --- /dev/null +++ b/spec/requests/api/remote_mirrors_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::RemoteMirrors do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository, :remote_mirror) } + + describe 'GET /projects/:id/remote_mirrors' do + let(:route) { "/projects/#{project.id}/remote_mirrors" } + + it 'requires `admin_remote_mirror` permission' do + project.add_developer(user) + + get api(route, user) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + + it 'returns a list of remote mirrors' do + project.add_maintainer(user) + + get api(route, user) + + expect(response).to have_gitlab_http_status(:success) + expect(response).to match_response_schema('remote_mirrors') + end + + context 'with the `remote_mirrors_api` feature disabled' do + before do + stub_feature_flags(remote_mirrors_api: false) + end + + it 'responds with `not_found`' do + get api(route, user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/services/prometheus/proxy_variable_substitution_service_spec.rb b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb new file mode 100644 index 00000000000..b1cdb8fd3ae --- /dev/null +++ b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb @@ -0,0 +1,143 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Prometheus::ProxyVariableSubstitutionService do + describe '#execute' do + let_it_be(:environment) { create(:environment) } + + let(:params_keys) { { query: 'up{environment="%{ci_environment_slug}"}' } } + let(:params) { ActionController::Parameters.new(params_keys).permit! } + let(:result) { subject.execute } + + subject { described_class.new(environment, params) } + + shared_examples 'success' do + it 'replaces variables with values' do + expect(result[:status]).to eq(:success) + expect(result[:params][:query]).to eq(expected_query) + end + end + + shared_examples 'error' do |message| + it 'returns error' do + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq(message) + end + end + + context 'does not alter params passed to the service' do + it do + subject.execute + + expect(params).to eq( + ActionController::Parameters.new( + query: 'up{environment="%{ci_environment_slug}"}' + ).permit! + ) + end + end + + context 'with predefined variables' do + it_behaves_like 'success' do + let(:expected_query) { %Q[up{environment="#{environment.slug}"}] } + end + + context 'with nil query' do + let(:params_keys) { {} } + + it_behaves_like 'success' do + let(:expected_query) { nil } + end + end + end + + context 'ruby template rendering' do + let(:params_keys) do + { query: 'up{env=%{ci_environment_slug},%{environment_filter}}' } + end + + it_behaves_like 'success' do + let(:expected_query) do + "up{env=#{environment.slug},container_name!=\"POD\"," \ + "environment=\"#{environment.slug}\"}" + end + end + + context 'with multiple occurrences of variable in string' do + let(:params_keys) do + { query: 'up{env1=%{ci_environment_slug},env2=%{ci_environment_slug}}' } + end + + it_behaves_like 'success' do + let(:expected_query) { "up{env1=#{environment.slug},env2=#{environment.slug}}" } + end + end + + context 'with multiple variables in string' do + let(:params_keys) do + { query: 'up{env=%{ci_environment_slug},%{environment_filter}}' } + end + + it_behaves_like 'success' do + let(:expected_query) do + "up{env=#{environment.slug}," \ + "container_name!=\"POD\",environment=\"#{environment.slug}\"}" + end + end + end + + context 'with unknown variables in string' do + let(:params_keys) { { query: 'up{env=%{env_slug}}' } } + + it_behaves_like 'success' do + let(:expected_query) { 'up{env=%{env_slug}}' } + end + end + + # This spec is needed if there are multiple keys in the context provided + # by `Gitlab::Prometheus::QueryVariables.call(environment)` which is + # passed to the Ruby `%` operator. + # If the number of keys in the context is one, there is no need for + # this spec. + context 'with extra variables in context' do + let(:params_keys) { { query: 'up{env=%{ci_environment_slug}}' } } + + it_behaves_like 'success' do + let(:expected_query) { "up{env=#{environment.slug}}" } + end + + it 'has more than one variable in context' do + expect(Gitlab::Prometheus::QueryVariables.call(environment).size).to be > 1 + end + end + + # The ruby % operator will not replace known variables if there are unknown + # variables also in the string. It doesn't raise an error + # (though the `sprintf` and `format` methods do). + context 'with unknown and known variables in string' do + let(:params_keys) do + { query: 'up{env=%{ci_environment_slug},other_env=%{env_slug}}' } + end + + it_behaves_like 'success' do + let(:expected_query) { 'up{env=%{ci_environment_slug},other_env=%{env_slug}}' } + end + end + + context 'when rendering raises error' do + context 'when TypeError is raised' do + let(:params_keys) { { query: '{% a %}' } } + + it_behaves_like 'error', 'Malformed string' + end + + context 'when ArgumentError is raised' do + let(:params_keys) { { query: '%<' } } + + it_behaves_like 'error', 'Malformed string' + end + end + end + end +end |