From 07572a93ac5f111b04a191636adcb1e2a5958457 Mon Sep 17 00:00:00 2001 From: Tiger Date: Fri, 26 Jul 2019 15:40:36 +1000 Subject: Use stubbed certificate and key in Helm factory --- spec/factories/clusters/applications/helm.rb | 14 ++++++++++++++ spec/fixtures/clusters/sample_key.key | 9 +++++++++ 2 files changed, 23 insertions(+) create mode 100644 spec/fixtures/clusters/sample_key.key diff --git a/spec/factories/clusters/applications/helm.rb b/spec/factories/clusters/applications/helm.rb index 24c22ef3928..89f7bc15217 100644 --- a/spec/factories/clusters/applications/helm.rb +++ b/spec/factories/clusters/applications/helm.rb @@ -4,6 +4,20 @@ FactoryBot.define do factory :clusters_applications_helm, class: Clusters::Applications::Helm do cluster factory: %i(cluster provided_by_gcp) + before(:create) do + allow(Gitlab::Kubernetes::Helm::Certificate).to receive(:generate_root) + .and_return( + double( + key_string: File.read(Rails.root.join('spec/fixtures/clusters/sample_key.key')), + cert_string: File.read(Rails.root.join('spec/fixtures/clusters/sample_cert.pem')) + ) + ) + end + + after(:create) do + allow(Gitlab::Kubernetes::Helm::Certificate).to receive(:generate_root).and_call_original + end + trait :not_installable do status(-2) end diff --git a/spec/fixtures/clusters/sample_key.key b/spec/fixtures/clusters/sample_key.key new file mode 100644 index 00000000000..4ddb20b0922 --- /dev/null +++ b/spec/fixtures/clusters/sample_key.key @@ -0,0 +1,9 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIBOgIBAAJBAMA5sXIBE0HwgIB40iNidN4PGWzOyLQK0bsdOBNgpEXkDlZBvnak +OUgAPF+rME4PB0Yl415DabUI40T5UNmlwxcCAwEAAQJAZtY2pSwIFm3JAXIh0cZZ +iXcAfiJ+YzuqinUOS+eW2sBCAEzjcARlU/o6sFQgtsOi4FOMczAd1Yx8UDMXMmrw +2QIhAPBgVhJiTF09pdmeFWutCvTJDlFFAQNbrbo2X2x/9WF9AiEAzLgqMKeStSRu +H9N16TuDrUoO8R+DPqriCwkKrSHaWyMCIFzMhE4inuKcSywBaLmiG4m3GQzs++Al +A6PRG/PSTpQtAiBxtBg6zdf+JC3GH3zt/dA0/10tL4OF2wORfYQghRzyYQIhAL2l +0ZQW+yLIZAGrdBFWYEAa52GZosncmzBNlsoTgwE4 +-----END RSA PRIVATE KEY----- \ No newline at end of file -- cgit v1.2.1 From 0f91eadc8742054823ec5ea2767f843449fdee93 Mon Sep 17 00:00:00 2001 From: Ben Kochie Date: Fri, 2 Aug 2019 09:58:08 +0200 Subject: Update Unicorn Worker recommendation Update the Unicorn worker documentation based on the new scaling formula. https://gitlab.com/gitlab-org/omnibus-gitlab/issues/4535 --- doc/install/requirements.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/install/requirements.md b/doc/install/requirements.md index 25ab608de3a..2e3eedad992 100644 --- a/doc/install/requirements.md +++ b/doc/install/requirements.md @@ -146,8 +146,8 @@ CREATE EXTENSION postgres_fdw; ## Unicorn Workers -For most instances we recommend using: CPU cores + 1 = unicorn workers. -So for a machine with 2 cores, 3 unicorn workers is ideal. +For most instances we recommend using: (CPU cores * 1.5) + 1 = unicorn workers. +For example a node with 4 cores would have 7 unicorn workers. For all machines that have 2GB and up we recommend a minimum of three unicorn workers. If you have a 1GB machine we recommend to configure only two Unicorn workers to prevent excessive swapping. -- cgit v1.2.1 From 8f653f098394d3f048c0abdecda2a0de54b48e08 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Fri, 2 Aug 2019 11:54:40 +0200 Subject: Again run quarantine specs if tag provided In f59438c0a9da7bb4d98291d2adedfc5a13a50798 we changed how quarantined specs are excluded. But that made it impossible to run them by providing `--tag quarantine`, this changes that. --- spec/spec_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6994b6687fc..bcc133790d1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -148,9 +148,9 @@ RSpec.configure do |config| Gitlab::ThreadMemoryCache.cache_backend.clear end - config.around(:example, :quarantine) do + config.around(:example, :quarantine) do |example| # Skip tests in quarantine unless we explicitly focus on them. - skip('In quarantine') unless config.inclusion_filter[:quarantine] + example.run if config.inclusion_filter[:quarantine] end config.before(:example, :request_store) do -- cgit v1.2.1 From e5e1c907c01b53194f77e8d8de53554ba1824e7c Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Fri, 26 Jul 2019 11:21:52 +0100 Subject: Add outbound requests setting for system hooks This MR adds new application setting to network section `allow_local_requests_from_system_hooks`. Prior to this change system hooks were allowed to do local network requests by default and we are adding an ability for admins to control it. --- app/helpers/application_settings_helper.rb | 3 +- app/models/application_setting_implementation.rb | 3 +- app/models/hooks/system_hook.rb | 5 ++- app/models/hooks/web_hook.rb | 2 +- app/services/web_hook_service.rb | 6 ++- app/validators/addressable_url_validator.rb | 2 +- app/validators/system_hook_url_validator.rb | 30 +++++++++++++++ .../admin/application_settings/_outbound.html.haml | 10 +++-- ..._from_hooks_and_services_application_setting.rb | 15 ++++++++ ...ts_from_system_hooks_to_application_settings.rb | 18 +++++++++ db/schema.rb | 3 +- doc/api/settings.md | 3 +- lib/gitlab/http_connection_adapter.rb | 4 +- lib/gitlab/kubernetes/kube_client.rb | 2 +- locale/gitlab.pot | 5 ++- spec/features/admin/admin_settings_spec.rb | 7 +++- spec/lib/gitlab/http_spec.rb | 6 +-- spec/lib/gitlab/kubernetes/kube_client_spec.rb | 2 +- spec/models/clusters/platforms/kubernetes_spec.rb | 2 +- spec/models/lfs_download_object_spec.rb | 2 +- .../lfs_pointers/lfs_download_service_spec.rb | 2 +- .../self_monitoring/project/create_service_spec.rb | 2 +- spec/services/web_hook_service_spec.rb | 44 ++++++++++++++++++---- 23 files changed, 144 insertions(+), 34 deletions(-) create mode 100644 app/validators/system_hook_url_validator.rb create mode 100644 db/migrate/20190726101050_rename_allow_local_requests_from_hooks_and_services_application_setting.rb create mode 100644 db/migrate/20190726101133_add_allow_local_requests_from_system_hooks_to_application_settings.rb diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 3847a35fbab..a646b62027a 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -159,7 +159,8 @@ module ApplicationSettingsHelper :after_sign_up_text, :akismet_api_key, :akismet_enabled, - :allow_local_requests_from_hooks_and_services, + :allow_local_requests_from_web_hooks_and_services, + :allow_local_requests_from_system_hooks, :dns_rebinding_protection_enabled, :archive_builds_in_human_readable, :authorized_keys_enabled, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 4bb09bf3b53..b7a4d7aa803 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -21,7 +21,8 @@ module ApplicationSettingImplementation { after_sign_up_text: nil, akismet_enabled: false, - allow_local_requests_from_hooks_and_services: false, + allow_local_requests_from_web_hooks_and_services: false, + allow_local_requests_from_system_hooks: true, dns_rebinding_protection_enabled: true, authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand container_registry_token_expire_delay: 5, diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb index 90b4588a325..b83913c845f 100644 --- a/app/models/hooks/system_hook.rb +++ b/app/models/hooks/system_hook.rb @@ -14,8 +14,11 @@ class SystemHook < WebHook default_value_for :repository_update_events, true default_value_for :merge_requests_events, false + validates :url, presence: true, public_url: false, system_hook_url: { allow_localhost: lambda(&:allow_local_requests?), + allow_local_network: lambda(&:allow_local_requests?) } + # Allow urls pointing localhost and the local network def allow_local_requests? - true + Gitlab::CurrentSettings.allow_local_requests_from_system_hooks? end end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index daf7ff4b771..3203bb024ab 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -35,6 +35,6 @@ class WebHook < ApplicationRecord # Allow urls pointing localhost and the local network def allow_local_requests? - false + Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? end end diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 6d675c026bb..8c294218708 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -17,8 +17,10 @@ class WebHookService @hook = hook @data = data @hook_name = hook_name.to_s - @request_options = { timeout: Gitlab.config.gitlab.webhook_timeout } - @request_options.merge!(allow_local_requests: true) if @hook.is_a?(SystemHook) + @request_options = { + timeout: Gitlab.config.gitlab.webhook_timeout, + allow_local_requests: hook.allow_local_requests? + } end def execute diff --git a/app/validators/addressable_url_validator.rb b/app/validators/addressable_url_validator.rb index 273e15ef925..bb445499cee 100644 --- a/app/validators/addressable_url_validator.rb +++ b/app/validators/addressable_url_validator.rb @@ -107,6 +107,6 @@ class AddressableUrlValidator < ActiveModel::EachValidator # calls this validator. # # See https://gitlab.com/gitlab-org/gitlab-ee/issues/9833 - ApplicationSetting.current&.allow_local_requests_from_hooks_and_services? + ApplicationSetting.current&.allow_local_requests_from_web_hooks_and_services? end end diff --git a/app/validators/system_hook_url_validator.rb b/app/validators/system_hook_url_validator.rb new file mode 100644 index 00000000000..c8c0007e35b --- /dev/null +++ b/app/validators/system_hook_url_validator.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +# SystemHookUrlValidator +# +# Custom validator specifically for SystemHook URLs. This validator works like AddressableUrlValidator but +# it blocks urls pointing to localhost or the local network depending on +# ApplicationSetting.allow_local_requests_from_system_hooks +# +# Example: +# +# class SystemHook < WebHook +# validates :url, system_hook_url: { allow_localhost: true, allow_local_network: true } +# end +# +class SystemHookUrlValidator < AddressableUrlValidator + DEFAULT_OPTIONS = { + allow_localhost: true, + allow_local_network: true + }.freeze + + def initialize(options) + options.reverse_merge!(DEFAULT_OPTIONS) + + super(options) + end + + def self.allow_setting_local_requests? + ApplicationSetting.current&.allow_local_requests_from_system_hooks? + end +end diff --git a/app/views/admin/application_settings/_outbound.html.haml b/app/views/admin/application_settings/_outbound.html.haml index 4fecdb59e1d..d39e192d604 100644 --- a/app/views/admin/application_settings/_outbound.html.haml +++ b/app/views/admin/application_settings/_outbound.html.haml @@ -4,9 +4,13 @@ %fieldset .form-group .form-check - = f.check_box :allow_local_requests_from_hooks_and_services, class: 'form-check-input' - = f.label :allow_local_requests_from_hooks_and_services, class: 'form-check-label' do - Allow requests to the local network from hooks and services + = f.check_box :allow_local_requests_from_web_hooks_and_services, class: 'form-check-input' + = f.label :allow_local_requests_from_web_hooks_and_services, class: 'form-check-label' do + Allow requests to the local network from web hooks and services + .form-check + = f.check_box :allow_local_requests_from_system_hooks, class: 'form-check-input' + = f.label :allow_local_requests_from_system_hooks, class: 'form-check-label' do + Allow requests to the local network from system hooks .form-group = f.label :outbound_local_requests_whitelist_raw, class: 'label-bold' do diff --git a/db/migrate/20190726101050_rename_allow_local_requests_from_hooks_and_services_application_setting.rb b/db/migrate/20190726101050_rename_allow_local_requests_from_hooks_and_services_application_setting.rb new file mode 100644 index 00000000000..f1ba7da9fc7 --- /dev/null +++ b/db/migrate/20190726101050_rename_allow_local_requests_from_hooks_and_services_application_setting.rb @@ -0,0 +1,15 @@ +class RenameAllowLocalRequestsFromHooksAndServicesApplicationSetting < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + rename_column :application_settings, :allow_local_requests_from_hooks_and_services, :allow_local_requests_from_web_hooks_and_services + end + + def down + rename_column :application_settings, :allow_local_requests_from_web_hooks_and_services, :allow_local_requests_from_hooks_and_services + end +end diff --git a/db/migrate/20190726101133_add_allow_local_requests_from_system_hooks_to_application_settings.rb b/db/migrate/20190726101133_add_allow_local_requests_from_system_hooks_to_application_settings.rb new file mode 100644 index 00000000000..ed58d4e57fc --- /dev/null +++ b/db/migrate/20190726101133_add_allow_local_requests_from_system_hooks_to_application_settings.rb @@ -0,0 +1,18 @@ +class AddAllowLocalRequestsFromSystemHooksToApplicationSettings < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:application_settings, :allow_local_requests_from_system_hooks, + :boolean, + default: true, + allow_null: false) + end + + def down + remove_column(:application_settings, :allow_local_requests_from_system_hooks) + end +end diff --git a/db/schema.rb b/db/schema.rb index 709f9ce2541..e2ae13174cd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -183,7 +183,7 @@ ActiveRecord::Schema.define(version: 2019_07_31_084415) do t.string "external_authorization_service_default_label" t.boolean "pages_domain_verification_enabled", default: true, null: false t.string "user_default_internal_regex" - t.boolean "allow_local_requests_from_hooks_and_services", default: false, null: false + t.boolean "allow_local_requests_from_web_hooks_and_services", default: false, null: false t.float "external_authorization_service_timeout", default: 0.5 t.text "external_auth_client_cert" t.text "encrypted_external_auth_client_key" @@ -230,6 +230,7 @@ ActiveRecord::Schema.define(version: 2019_07_31_084415) do t.string "grafana_url", default: "/-/grafana", null: false t.string "outbound_local_requests_whitelist", limit: 255, default: [], null: false, array: true t.integer "raw_blob_request_limit", default: 300, null: false + t.boolean "allow_local_requests_from_system_hooks", default: true, null: false t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id" t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id" t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id" diff --git a/doc/api/settings.md b/doc/api/settings.md index c3ac70f0579..f17a49cfc89 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -177,7 +177,8 @@ are listed in the descriptions of the relevant settings. | `akismet_api_key` | string | required by: `akismet_enabled` | API key for akismet spam protection. | | `akismet_enabled` | boolean | no | (**If enabled, requires:** `akismet_api_key`) Enable or disable akismet spam protection. | | `allow_group_owners_to_manage_ldap` | boolean | no | **(PREMIUM)** Set to `true` to allow group owners to manage LDAP | -| `allow_local_requests_from_hooks_and_services` | boolean | no | Allow requests to the local network from hooks and services. | +| `allow_local_requests_from_web_hooks_and_services` | boolean | no | Allow requests to the local network from web hooks and services. | +| `allow_local_requests_from_system_hooks` | boolean | no | Allow requests to the local network from system hooks. | | `authorized_keys_enabled` | boolean | no | By default, we write to the `authorized_keys` file to support Git over SSH without additional configuration. GitLab can be optimized to authenticate SSH keys via the database file. Only disable this if you have configured your OpenSSH server to use the AuthorizedKeysCommand. | | `auto_devops_domain` | string | no | Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages. | | `auto_devops_enabled` | boolean | no | Enable Auto DevOps for projects by default. It will automatically build, test, and deploy applications based on a predefined CI/CD configuration. | diff --git a/lib/gitlab/http_connection_adapter.rb b/lib/gitlab/http_connection_adapter.rb index 41eab3658bc..84eb60f3a5d 100644 --- a/lib/gitlab/http_connection_adapter.rb +++ b/lib/gitlab/http_connection_adapter.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # This class is part of the Gitlab::HTTP wrapper. Depending on the value -# of the global setting allow_local_requests_from_hooks_and_services this adapter +# of the global setting allow_local_requests_from_web_hooks_and_services this adapter # will allow/block connection to internal IPs and/or urls. # # This functionality can be overridden by providing the setting the option @@ -38,7 +38,7 @@ module Gitlab end def allow_settings_local_requests? - Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services? + Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? end end end diff --git a/lib/gitlab/kubernetes/kube_client.rb b/lib/gitlab/kubernetes/kube_client.rb index 1350924cd76..64317225ec6 100644 --- a/lib/gitlab/kubernetes/kube_client.rb +++ b/lib/gitlab/kubernetes/kube_client.rb @@ -128,7 +128,7 @@ module Gitlab private def validate_url! - return if Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services? + return if Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services? Gitlab::UrlBlocker.validate!(api_prefix, allow_local_network: false) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5e9e371a5fc..cca55aeb8bd 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -945,7 +945,10 @@ msgstr "" msgid "Allow rendering of PlantUML diagrams in Asciidoc documents." msgstr "" -msgid "Allow requests to the local network from hooks and services." +msgid "Allow requests to the local network from web hooks and services." +msgstr "" + +msgid "Allow requests to the local network from system hooks." msgstr "" msgid "Allow this key to push to repository as well? (Default only allows pull access.)" diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index c77605f3869..ddd87404003 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -338,14 +338,17 @@ describe 'Admin updates settings' do visit network_admin_application_settings_path page.within('.as-outbound') do - check 'Allow requests to the local network from hooks and services' + check 'Allow requests to the local network from web hooks and services' + # Enabled by default + uncheck 'Allow requests to the local network from system hooks' # Enabled by default uncheck 'Enforce DNS rebinding attack protection' click_button 'Save changes' end expect(page).to have_content "Application settings saved successfully" - expect(current_settings.allow_local_requests_from_hooks_and_services).to be true + expect(current_settings.allow_local_requests_from_web_hooks_and_services).to be true + expect(current_settings.allow_local_requests_from_system_hooks).to be false expect(current_settings.dns_rebinding_protection_enabled).to be false end end diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb index 158f77cab2c..d3f9be845dd 100644 --- a/spec/lib/gitlab/http_spec.rb +++ b/spec/lib/gitlab/http_spec.rb @@ -23,14 +23,14 @@ describe Gitlab::HTTP do end end - describe 'allow_local_requests_from_hooks_and_services is' do + describe 'allow_local_requests_from_web_hooks_and_services is' do before do WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success') end context 'disabled' do before do - allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(false) + allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(false) end it 'deny requests to localhost' do @@ -52,7 +52,7 @@ describe Gitlab::HTTP do context 'enabled' do before do - allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(true) + allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(true) end it 'allow requests to localhost' do diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb index 97ebb5f1554..f49d4e23e39 100644 --- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb +++ b/spec/lib/gitlab/kubernetes/kube_client_spec.rb @@ -58,7 +58,7 @@ describe Gitlab::Kubernetes::KubeClient do context 'when local requests are allowed' do before do - stub_application_setting(allow_local_requests_from_hooks_and_services: true) + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) end it 'allows local addresses' do diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index 471769e4aab..5811016ea4d 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -106,7 +106,7 @@ describe Clusters::Platforms::Kubernetes do before do allow(ApplicationSetting) .to receive(:current) - .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true)) + .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: true)) end it { expect(kubernetes.save).to be_truthy } diff --git a/spec/models/lfs_download_object_spec.rb b/spec/models/lfs_download_object_spec.rb index effd8b08124..8b53effe98f 100644 --- a/spec/models/lfs_download_object_spec.rb +++ b/spec/models/lfs_download_object_spec.rb @@ -50,7 +50,7 @@ describe LfsDownloadObject do before do allow(ApplicationSetting) .to receive(:current) - .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: setting)) + .and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: setting)) end context 'are allowed' do diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb index 75d534c59bf..970e82e7107 100644 --- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -17,7 +17,7 @@ describe Projects::LfsPointers::LfsDownloadService do before do ApplicationSetting.create_from_defaults - stub_application_setting(allow_local_requests_from_hooks_and_services: local_request_setting) + stub_application_setting(allow_local_requests_from_web_hooks_and_services: local_request_setting) allow(project).to receive(:lfs_enabled?).and_return(true) end diff --git a/spec/services/self_monitoring/project/create_service_spec.rb b/spec/services/self_monitoring/project/create_service_spec.rb index a1e7aaf45f2..87d7d776a69 100644 --- a/spec/services/self_monitoring/project/create_service_spec.rb +++ b/spec/services/self_monitoring/project/create_service_spec.rb @@ -37,7 +37,7 @@ describe SelfMonitoring::Project::CreateService do allow(ApplicationSetting) .to receive(:current) .and_return( - ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true) + ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: true) ) end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 37bafc0c002..8353fade5ea 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -19,15 +19,43 @@ describe WebHookService do let(:service_instance) { described_class.new(project_hook, data, :push_hooks) } describe '#initialize' do - it 'allow_local_requests is true if hook is a SystemHook' do - instance = described_class.new(build(:system_hook), data, :system_hook) - expect(instance.request_options[:allow_local_requests]).to be_truthy - end + context 'when SystemHook' do + context 'when allow_local_requests_from_system_hooks application setting is true' do + it 'allows local requests' do + stub_application_setting(allow_local_requests_from_system_hooks: true) + instance = described_class.new(build(:system_hook), data, :system_hook) + + expect(instance.request_options[:allow_local_requests]).to be_truthy + end + end - it 'allow_local_requests is false if hook is not a SystemHook' do - %i(project_hook service_hook web_hook_log).each do |hook| - instance = described_class.new(build(hook), data, hook) - expect(instance.request_options[:allow_local_requests]).to be_falsey + context 'when allow_local_requests_from_system_hooks application setting is false' do + it 'denies local requests' do + stub_application_setting(allow_local_requests_from_system_hooks: false) + instance = described_class.new(build(:system_hook), data, :system_hook) + + expect(instance.request_options[:allow_local_requests]).to be_falsey + end + end + + context 'when ProjectHook' do + context 'when allow_local_requests_from_web_hooks_and_services application setting is true' do + it 'allows local requests' do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) + instance = described_class.new(build(:project_hook), data, :project_hook) + + expect(instance.request_options[:allow_local_requests]).to be_truthy + end + end + + context 'when allow_local_requests_from_system_hooks application setting is false' do + it 'denies local requests' do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) + instance = described_class.new(build(:project_hook), data, :project_hook) + + expect(instance.request_options[:allow_local_requests]).to be_falsey + end + end end end end -- cgit v1.2.1 From 6c42088fe2fb1970550d9b418dab785af26fcbf5 Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Fri, 26 Jul 2019 11:59:05 +0100 Subject: Add changelog entry --- .../unreleased/georgekoltsov-55474-outbound-setting-system-hooks.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/georgekoltsov-55474-outbound-setting-system-hooks.yml diff --git a/changelogs/unreleased/georgekoltsov-55474-outbound-setting-system-hooks.yml b/changelogs/unreleased/georgekoltsov-55474-outbound-setting-system-hooks.yml new file mode 100644 index 00000000000..46b4a408a27 --- /dev/null +++ b/changelogs/unreleased/georgekoltsov-55474-outbound-setting-system-hooks.yml @@ -0,0 +1,5 @@ +--- +title: Add new outbound network requests application setting for system hooks +merge_request: 31177 +author: +type: changed -- cgit v1.2.1 From 7fe145b1b5854a0918d0c04e8f79d8aacd81561e Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Fri, 26 Jul 2019 12:23:38 +0100 Subject: Add SystemHookUrlValidator spec --- spec/validators/system_hook_url_validator_spec.rb | 51 +++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 spec/validators/system_hook_url_validator_spec.rb diff --git a/spec/validators/system_hook_url_validator_spec.rb b/spec/validators/system_hook_url_validator_spec.rb new file mode 100644 index 00000000000..0862a67df9f --- /dev/null +++ b/spec/validators/system_hook_url_validator_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe SystemHookUrlValidator do + include_examples 'url validator examples', AddressableUrlValidator::DEFAULT_OPTIONS[:schemes] + + context 'by default' do + let(:validator) { described_class.new(attributes: [:link_url]) } + let!(:badge) { build(:badge, link_url: 'http://www.example.com') } + + subject { validator.validate(badge) } + + it 'does not block urls pointing to localhost' do + badge.link_url = 'https://127.0.0.1' + + subject + + expect(badge.errors).not_to be_present + end + + it 'does not block urls pointing to the local network' do + badge.link_url = 'https://192.168.1.1' + + subject + + expect(badge.errors).not_to be_present + end + end + + context 'when local requests are not allowed' do + let(:validator) { described_class.new(attributes: [:link_url], allow_localhost: false, allow_local_network: false) } + let!(:badge) { build(:badge, link_url: 'http://www.example.com') } + + subject { validator.validate(badge) } + + it 'blocks urls pointing to localhost' do + badge.link_url = 'https://127.0.0.1' + + subject + + expect(badge.errors).to be_present + end + + it 'blocks urls pointing to the local network' do + badge.link_url = 'https://192.168.1.1' + + subject + + expect(badge.errors).to be_present + end + end +end -- cgit v1.2.1 From 5a19a43a13031de83af2d241498465a882421270 Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Fri, 26 Jul 2019 13:08:05 +0100 Subject: Update translations in gitlab.pot --- app/views/admin/application_settings/_outbound.html.haml | 4 ++-- ...w_local_requests_from_hooks_and_services_application_setting.rb | 2 ++ ...low_local_requests_from_system_hooks_to_application_settings.rb | 2 ++ locale/gitlab.pot | 7 +++++-- spec/validators/system_hook_url_validator_spec.rb | 2 ++ 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/views/admin/application_settings/_outbound.html.haml b/app/views/admin/application_settings/_outbound.html.haml index d39e192d604..ad26f52aea7 100644 --- a/app/views/admin/application_settings/_outbound.html.haml +++ b/app/views/admin/application_settings/_outbound.html.haml @@ -6,11 +6,11 @@ .form-check = f.check_box :allow_local_requests_from_web_hooks_and_services, class: 'form-check-input' = f.label :allow_local_requests_from_web_hooks_and_services, class: 'form-check-label' do - Allow requests to the local network from web hooks and services + = _('Allow requests to the local network from web hooks and services') .form-check = f.check_box :allow_local_requests_from_system_hooks, class: 'form-check-input' = f.label :allow_local_requests_from_system_hooks, class: 'form-check-label' do - Allow requests to the local network from system hooks + = _('Allow requests to the local network from system hooks') .form-group = f.label :outbound_local_requests_whitelist_raw, class: 'label-bold' do diff --git a/db/migrate/20190726101050_rename_allow_local_requests_from_hooks_and_services_application_setting.rb b/db/migrate/20190726101050_rename_allow_local_requests_from_hooks_and_services_application_setting.rb index f1ba7da9fc7..e3e8514427c 100644 --- a/db/migrate/20190726101050_rename_allow_local_requests_from_hooks_and_services_application_setting.rb +++ b/db/migrate/20190726101050_rename_allow_local_requests_from_hooks_and_services_application_setting.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class RenameAllowLocalRequestsFromHooksAndServicesApplicationSetting < ActiveRecord::Migration[5.2] include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20190726101133_add_allow_local_requests_from_system_hooks_to_application_settings.rb b/db/migrate/20190726101133_add_allow_local_requests_from_system_hooks_to_application_settings.rb index ed58d4e57fc..c0ecdbf4e37 100644 --- a/db/migrate/20190726101133_add_allow_local_requests_from_system_hooks_to_application_settings.rb +++ b/db/migrate/20190726101133_add_allow_local_requests_from_system_hooks_to_application_settings.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AddAllowLocalRequestsFromSystemHooksToApplicationSettings < ActiveRecord::Migration[5.2] include Gitlab::Database::MigrationHelpers diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cca55aeb8bd..c4ee722c4a2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -945,10 +945,13 @@ msgstr "" msgid "Allow rendering of PlantUML diagrams in Asciidoc documents." msgstr "" -msgid "Allow requests to the local network from web hooks and services." +msgid "Allow requests to the local network from hooks and services." msgstr "" -msgid "Allow requests to the local network from system hooks." +msgid "Allow requests to the local network from system hooks" +msgstr "" + +msgid "Allow requests to the local network from web hooks and services" msgstr "" msgid "Allow this key to push to repository as well? (Default only allows pull access.)" diff --git a/spec/validators/system_hook_url_validator_spec.rb b/spec/validators/system_hook_url_validator_spec.rb index 0862a67df9f..fc4261666e7 100644 --- a/spec/validators/system_hook_url_validator_spec.rb +++ b/spec/validators/system_hook_url_validator_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe SystemHookUrlValidator do -- cgit v1.2.1 From ac7661924eebd6eb0fa72848e2b4bf4391ebf113 Mon Sep 17 00:00:00 2001 From: George Koltsov Date: Fri, 26 Jul 2019 14:03:06 +0100 Subject: Update security/webhooks.md doc page & specs Updating security/webhooks.md to match new behaviour as well as drying up few specs to extract shared examples --- app/validators/system_hook_url_validator.rb | 6 +-- doc/security/img/outbound_requests_section_v2.png | Bin 0 -> 21108 bytes doc/security/webhooks.md | 8 ++-- spec/services/web_hook_service_spec.rb | 50 +++++++++------------- spec/validators/system_hook_url_validator_spec.rb | 25 ++++++----- 5 files changed, 43 insertions(+), 46 deletions(-) create mode 100644 doc/security/img/outbound_requests_section_v2.png diff --git a/app/validators/system_hook_url_validator.rb b/app/validators/system_hook_url_validator.rb index c8c0007e35b..e482828685d 100644 --- a/app/validators/system_hook_url_validator.rb +++ b/app/validators/system_hook_url_validator.rb @@ -2,7 +2,7 @@ # SystemHookUrlValidator # -# Custom validator specifically for SystemHook URLs. This validator works like AddressableUrlValidator but +# Custom validator specific to SystemHook URLs. This validator works like AddressableUrlValidator but # it blocks urls pointing to localhost or the local network depending on # ApplicationSetting.allow_local_requests_from_system_hooks # @@ -14,8 +14,8 @@ # class SystemHookUrlValidator < AddressableUrlValidator DEFAULT_OPTIONS = { - allow_localhost: true, - allow_local_network: true + allow_localhost: false, + allow_local_network: false }.freeze def initialize(options) diff --git a/doc/security/img/outbound_requests_section_v2.png b/doc/security/img/outbound_requests_section_v2.png new file mode 100644 index 00000000000..4fd3c7d9fce Binary files /dev/null and b/doc/security/img/outbound_requests_section_v2.png differ diff --git a/doc/security/webhooks.md b/doc/security/webhooks.md index 1194234a295..401df02e5c3 100644 --- a/doc/security/webhooks.md +++ b/doc/security/webhooks.md @@ -34,15 +34,15 @@ to 127.0.0.1, ::1 and 0.0.0.0, as well as IPv4 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16 and IPv6 site-local (ffc0::/10) addresses won't be allowed. This behavior can be overridden by enabling the option *"Allow requests to the -local network from hooks and services"* in the *"Outbound requests"* section +local network from web hooks and services"* in the *"Outbound requests"* section inside the Admin area under **Settings** (`/admin/application_settings/network`): -![Outbound requests admin settings](img/outbound_requests_section.png) +![Outbound requests admin settings](img/outbound_requests_section_v2.png) >**Note:** -*System hooks* are exempt from this protection because they are set up by -admins. +*System hooks* are enabled to make requests to local network by default since they are set up by admins. +However, it can be turned off by disabling *"Allow requests to the local network from system hooks"* option. ## Releases tasks @@ -12,9 +12,9 @@ Set the title to: `Security Release: 11.4.X, 11.3.X, and 11.2.X` ## Version issues: -* 11.4.X: {release task link} -* 11.3.X: {release task link} -* 11.2.X: {release task link} +* 12.2.X: {release task link} +* 12.1.X: {release task link} +* 12.0.X: {release task link} ## Security Issues: @@ -34,9 +34,9 @@ Set the title to: `Security Release: 11.4.X, 11.3.X, and 11.2.X` | Version | MR | |---------|----| -| 11.4 | {https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/ link} | -| 11.3 | {https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/ link} | -| 11.2 | {https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/ link} | +| 12.2 | {https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/ link} | +| 12.1 | {https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/ link} | +| 12.0 | {https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/ link} | | master | {https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/ link} | @@ -48,9 +48,9 @@ Set the title to: `Security Release: 11.4.X, 11.3.X, and 11.2.X` | Version | MR | |---------|----| -| 11.4| {https://dev.gitlab.org/gitlab/gitlab-ee/merge_requests/ link} | -| 11.3 | {https://dev.gitlab.org/gitlab/gitlab-ee/merge_requests/ link} | -| 11.2 | {https://dev.gitlab.org/gitlab/gitlab-ee/merge_requests/ link} | +| 12.2 | {https://dev.gitlab.org/gitlab/gitlab-ee/merge_requests/ link} | +| 12.1 | {https://dev.gitlab.org/gitlab/gitlab-ee/merge_requests/ link} | +| 12.0 | {https://dev.gitlab.org/gitlab/gitlab-ee/merge_requests/ link} | | master | {https://dev.gitlab.org/gitlab/gitlab-ee/merge_requests/ link} | diff --git a/.gitlab/issue_templates/Security developer workflow.md b/.gitlab/issue_templates/Security developer workflow.md index 7857afb66c2..3e634de4f0c 100644 --- a/.gitlab/issue_templates/Security developer workflow.md +++ b/.gitlab/issue_templates/Security developer workflow.md @@ -17,7 +17,7 @@ Set the title to: `Description of the original issue` #### Backports -- [ ] Once the MR is ready to be merged, create MRs targeting the last 3 releases, plus the current RC if between the 7th and 22nd of the month. +- [ ] Once the MR is ready to be merged, create MRs targeting the latest 3 stable branches - [ ] At this point, it might be easy to squash the commits from the MR into one - You can use the script `bin/secpick` instead of the following steps, to help you cherry-picking. See the [secpick documentation] - [ ] Create each MR targeting the stable branch `X-Y-stable`, using the "Security Release" merge request template. -- cgit v1.2.1 From 2849715a0e559465ae95b4d040a298aeadde0b69 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Mon, 5 Aug 2019 17:58:00 +0000 Subject: Document when quotes aren't needed for quick actions --- doc/user/project/quick_actions.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/user/project/quick_actions.md b/doc/user/project/quick_actions.md index b8e0ef8d12f..437899dce1e 100644 --- a/doc/user/project/quick_actions.md +++ b/doc/user/project/quick_actions.md @@ -70,6 +70,19 @@ Many quick actions require a parameter, for example: username, milestone, and label. [Autocomplete characters](autocomplete_characters.md) can make it easier to enter a parameter, compared to selecting items from a list. +## Quick actions parameters + +The easiest way to set parameters for quick actions is to use autocomplete. If +you manually enter a parameter, it must be enclosed in double quotation marks +(`"`), unless it contains only: + +1. ASCII letters. +2. Numerals. +3. Underscore, hyphen, question mark, dot, and ampersand. + +Parameters are also case-sensitive. Autocomplete handles this, and the insertion +of quotation marks, automatically. + ## Quick actions for commit messages The following quick actions are applicable for commit messages: -- cgit v1.2.1 From dd2b87241d3caccb240e901cbb2e6892d758f588 Mon Sep 17 00:00:00 2001 From: Marcel Amirault Date: Mon, 5 Aug 2019 18:02:23 +0000 Subject: Bring diagnostics_tools.md from debug project to docs --- doc/administration/index.md | 2 ++ .../troubleshooting/diagnostics_tools.md | 27 ++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 doc/administration/troubleshooting/diagnostics_tools.md diff --git a/doc/administration/index.md b/doc/administration/index.md index 00c8863f200..febee3e5af8 100644 --- a/doc/administration/index.md +++ b/doc/administration/index.md @@ -185,3 +185,5 @@ Learn how to install, configure, update, and maintain your GitLab instance. - [Debugging tips](troubleshooting/debug.md): Tips to debug problems when things go wrong - [Log system](logs.md): Where to look for logs. - [Sidekiq Troubleshooting](troubleshooting/sidekiq.md): Debug when Sidekiq appears hung and is not processing jobs. +- Useful [diagnostics tools](troubleshooting/diagnostics_tools.md) that are sometimes used by the GitLab + Support team. diff --git a/doc/administration/troubleshooting/diagnostics_tools.md b/doc/administration/troubleshooting/diagnostics_tools.md new file mode 100644 index 00000000000..ab3b25f0e97 --- /dev/null +++ b/doc/administration/troubleshooting/diagnostics_tools.md @@ -0,0 +1,27 @@ +--- +type: reference +--- + +# Diagnostics tools + +These are some of the diagnostics tools the GitLab Support team uses during troubleshooting. +They are listed here for transparency, and they may be useful for users with experience +with troubleshooting GitLab. If you are currently having an issue with GitLab, you +may want to check your [support options](https://about.gitlab.com/support/) first, +before attempting to use these tools. + +## gitlabsos + +The [gitlabsos](https://gitlab.com/gitlab-com/support/toolbox/gitlabsos/) utility +provides a unified method of gathering info and logs from GitLab and the system it's +running on. + +## strace-parser + +[strace-parser](https://gitlab.com/wchandler/strace-parser) is a small tool to analyze +and summarize raw strace data. + +## Pritaly + +[Pritaly](https://gitlab.com/wchandler/pritaly) takes Gitaly logs and colorizes output +or converts the logs to JSON. -- cgit v1.2.1 From 1c8ed27ac7ea312b1370b1dee751b8539e76b3a9 Mon Sep 17 00:00:00 2001 From: Lukas Eipert Date: Mon, 5 Aug 2019 18:24:52 +0000 Subject: Update dependency @gitlab/ui to v5.14.0 --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index bf6000dc53d..4ff99f29e24 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "@babel/preset-env": "^7.4.4", "@gitlab/csslab": "^1.9.0", "@gitlab/svgs": "^1.67.0", - "@gitlab/ui": "5.12.1", + "@gitlab/ui": "5.14.0", "apollo-cache-inmemory": "^1.5.1", "apollo-client": "^2.5.1", "apollo-link": "^1.2.11", diff --git a/yarn.lock b/yarn.lock index d8193af1310..acf1a4d8652 100644 --- a/yarn.lock +++ b/yarn.lock @@ -996,10 +996,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.67.0.tgz#c7b94eca13b99fd3aaa737fb6dcc0abc41d3c579" integrity sha512-hJOmWEs6RkjzyKkb1vc9wwKGZIBIP0coHkxu/KgOoxhBVudpGk4CH7xJ6UuB2TKpb0SEh5CC1CzRZfBYaFhsaA== -"@gitlab/ui@5.12.1": - version "5.12.1" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.12.1.tgz#70035747cec96a729e012924ab2d3e3b6067a558" - integrity sha512-W4rvZj2Fab1UpXR0Wyi7wSvj+5Ko+TWHibC/q/FSRHMsbeSLq77lljd7rQWeXXNMBvEKwr4NqSmckWsjaSOLfw== +"@gitlab/ui@5.14.0": + version "5.14.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.14.0.tgz#850214cfc6bb57f7ce672dc1cc60ea3d39ad41f3" + integrity sha512-JXUmk+hT4Rj2GBh0xAF43dYeloBEDX22rgeaDU6/RzD3JEA353yEm2+HOsBjPkQFDAh6Zp7OZSBuhDFrQe8sbg== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.2.1" -- cgit v1.2.1 From 7fd512d84c7fd77a0e8778b6641c82826d3c0cad Mon Sep 17 00:00:00 2001 From: Olena Horal-Koretska Date: Mon, 5 Aug 2019 18:53:39 +0000 Subject: Removed external dashboard legend border --- .../javascripts/operation_settings/components/external_dashboard.vue | 2 ++ changelogs/unreleased/64675-Dashboard-URL-legend-border.yml | 5 +++++ 2 files changed, 7 insertions(+) create mode 100644 changelogs/unreleased/64675-Dashboard-URL-legend-border.yml diff --git a/app/assets/javascripts/operation_settings/components/external_dashboard.vue b/app/assets/javascripts/operation_settings/components/external_dashboard.vue index ed518611d0b..3c5de189d51 100644 --- a/app/assets/javascripts/operation_settings/components/external_dashboard.vue +++ b/app/assets/javascripts/operation_settings/components/external_dashboard.vue @@ -50,9 +50,11 @@ export default {
Date: Mon, 5 Aug 2019 12:45:04 -0700 Subject: Move migration from migrate to post_migrate This is just in case the creation of this index takes a while to create. --- ..._add_index_on_id_and_type_and_public_to_keys.rb | 23 ---------------------- ..._add_index_on_id_and_type_and_public_to_keys.rb | 23 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 23 deletions(-) delete mode 100644 db/migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb create mode 100644 db/post_migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb diff --git a/db/migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb b/db/migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb deleted file mode 100644 index 9b4d74b4bea..00000000000 --- a/db/migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -class AddIndexOnIdAndTypeAndPublicToKeys < ActiveRecord::Migration[5.2] - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - disable_ddl_transaction! - - INDEX_NAME = "index_on_deploy_keys_id_and_type_and_public" - - def up - add_concurrent_index(:keys, - [:id, :type], - where: "public = 't'", - unique: true, - name: INDEX_NAME) - end - - def down - remove_concurrent_index_by_name(:keys, INDEX_NAME) - end -end diff --git a/db/post_migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb b/db/post_migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb new file mode 100644 index 00000000000..9b4d74b4bea --- /dev/null +++ b/db/post_migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddIndexOnIdAndTypeAndPublicToKeys < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + INDEX_NAME = "index_on_deploy_keys_id_and_type_and_public" + + def up + add_concurrent_index(:keys, + [:id, :type], + where: "public = 't'", + unique: true, + name: INDEX_NAME) + end + + def down + remove_concurrent_index_by_name(:keys, INDEX_NAME) + end +end -- cgit v1.2.1 From 3dbf3997bbf51eca8a313c4e152c77c1b038fd5d Mon Sep 17 00:00:00 2001 From: Steve Abrams Date: Mon, 5 Aug 2019 20:00:50 +0000 Subject: Add group level container repository endpoints API endpoints for requesting container repositories and container repositories with their tag information are enabled for users that want to specify the group containing the repository rather than the specific project. --- app/finders/container_repositories_finder.rb | 34 +++ app/models/group.rb | 2 + app/policies/group_policy.rb | 1 + ...t-to-list-the-docker-images-tags-of-a-group.yml | 6 + doc/api/container_registry.md | 77 ++++++- lib/api/api.rb | 3 +- lib/api/container_registry.rb | 149 ------------- lib/api/entities/container_registry.rb | 10 +- lib/api/group_container_repositories.rb | 39 ++++ lib/api/project_container_repositories.rb | 152 +++++++++++++ spec/finders/container_repositories_finder_spec.rb | 44 ++++ spec/fixtures/api/schemas/registry/repository.json | 6 +- spec/models/group_spec.rb | 1 + spec/requests/api/container_registry_spec.rb | 245 --------------------- .../api/group_container_repositories_spec.rb | 57 +++++ .../api/project_container_repositories_spec.rb | 228 +++++++++++++++++++ .../policies/group_policy_shared_context.rb | 2 +- .../container_repositories_shared_examples.rb | 58 +++++ 18 files changed, 710 insertions(+), 404 deletions(-) create mode 100644 app/finders/container_repositories_finder.rb create mode 100644 changelogs/unreleased/26866-api-endpoint-to-list-the-docker-images-tags-of-a-group.yml delete mode 100644 lib/api/container_registry.rb create mode 100644 lib/api/group_container_repositories.rb create mode 100644 lib/api/project_container_repositories.rb create mode 100644 spec/finders/container_repositories_finder_spec.rb delete mode 100644 spec/requests/api/container_registry_spec.rb create mode 100644 spec/requests/api/group_container_repositories_spec.rb create mode 100644 spec/requests/api/project_container_repositories_spec.rb create mode 100644 spec/support/shared_examples/container_repositories_shared_examples.rb diff --git a/app/finders/container_repositories_finder.rb b/app/finders/container_repositories_finder.rb new file mode 100644 index 00000000000..eb91d7f825b --- /dev/null +++ b/app/finders/container_repositories_finder.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class ContainerRepositoriesFinder + # id: group or project id + # container_type: :group or :project + def initialize(id:, container_type:) + @id = id + @type = container_type.to_sym + end + + def execute + if project_type? + project.container_repositories + else + group.container_repositories + end + end + + private + + attr_reader :id, :type + + def project_type? + type == :project + end + + def project + Project.find(id) + end + + def group + Group.find(id) + end +end diff --git a/app/models/group.rb b/app/models/group.rb index 74eb556b1b5..6c868b1d1f0 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -44,6 +44,8 @@ class Group < Namespace has_many :cluster_groups, class_name: 'Clusters::Group' has_many :clusters, through: :cluster_groups, class_name: 'Clusters::Cluster' + has_many :container_repositories, through: :projects + has_many :todos accepts_nested_attributes_for :variables, allow_destroy: true diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 84b1873c05d..52c944491bf 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -68,6 +68,7 @@ class GroupPolicy < BasePolicy rule { developer }.enable :admin_milestone rule { reporter }.policy do + enable :read_container_image enable :admin_label enable :admin_list enable :admin_issue diff --git a/changelogs/unreleased/26866-api-endpoint-to-list-the-docker-images-tags-of-a-group.yml b/changelogs/unreleased/26866-api-endpoint-to-list-the-docker-images-tags-of-a-group.yml new file mode 100644 index 00000000000..adbd7971a14 --- /dev/null +++ b/changelogs/unreleased/26866-api-endpoint-to-list-the-docker-images-tags-of-a-group.yml @@ -0,0 +1,6 @@ +--- +title: Add API endpoints to return container repositories and tags from the group + level +merge_request: 30817 +author: +type: added diff --git a/doc/api/container_registry.md b/doc/api/container_registry.md index 174b93a4f7a..bf544f64178 100644 --- a/doc/api/container_registry.md +++ b/doc/api/container_registry.md @@ -6,6 +6,8 @@ This is the API docs of the [GitLab Container Registry](../user/project/containe ## List registry repositories +### Within a project + Get a list of registry repositories in a project. ``` @@ -14,7 +16,8 @@ GET /projects/:id/registry/repositories | 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. | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) accessible by the authenticated user. | +| `tags` | boolean | no | If the param is included as true, each repository will include an array of `"tags"` in the response. | ```bash curl --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/projects/5/registry/repositories" @@ -28,6 +31,7 @@ Example response: "id": 1, "name": "", "path": "group/project", + "project_id": 9, "location": "gitlab.example.com:5000/group/project", "created_at": "2019-01-10T13:38:57.391Z" }, @@ -35,12 +39,77 @@ Example response: "id": 2, "name": "releases", "path": "group/project/releases", + "project_id": 9, "location": "gitlab.example.com:5000/group/project/releases", "created_at": "2019-01-10T13:39:08.229Z" } ] ``` +### Within a group + +Get a list of registry repositories in a group. + +``` +GET /groups/:id/registry/repositories +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) accessible by the authenticated user. | +| `tags` | boolean | no | If the param is included as true, each repository will include an array of `"tags"` in the response. | + +```bash +curl --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/groups/2/registry/repositories?tags=1" +``` + +Example response: + +```json +[ + { + "id": 1, + "name": "", + "path": "group/project", + "project_id": 9, + "location": "gitlab.example.com:5000/group/project", + "created_at": "2019-01-10T13:38:57.391Z", + "tags": [ + { + "name": "0.0.1", + "path": "group/project:0.0.1", + "location": "gitlab.example.com:5000/group/project:0.0.1" + } + ] + }, + { + "id": 2, + "name": "", + "path": "group/other_project", + "project_id": 11, + "location": "gitlab.example.com:5000/group/other_project", + "created_at": "2019-01-10T13:39:08.229Z", + "tags": [ + { + "name": "0.0.1", + "path": "group/other_project:0.0.1", + "location": "gitlab.example.com:5000/group/other_project:0.0.1" + }, + { + "name": "0.0.2", + "path": "group/other_project:0.0.2", + "location": "gitlab.example.com:5000/group/other_project:0.0.2" + }, + { + "name": "latest", + "path": "group/other_project:latest", + "location": "gitlab.example.com:5000/group/other_project:latest" + } + ] + } +] +``` + ## Delete registry repository Delete a repository in registry. @@ -62,6 +131,8 @@ curl --request DELETE --header "PRIVATE-TOKEN: " "https://git ## List repository tags +### Within a project + Get a list of tags for given registry repository. ``` @@ -70,7 +141,7 @@ GET /projects/:id/registry/repositories/:repository_id/tags | 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. | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) accessible by the authenticated user. | | `repository_id` | integer | yes | The ID of registry repository. | ```bash @@ -104,7 +175,7 @@ GET /projects/:id/registry/repositories/:repository_id/tags/:tag_name | 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. | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) accessible by the authenticated user. | | `repository_id` | integer | yes | The ID of registry repository. | | `tag_name` | string | yes | The name of tag. | diff --git a/lib/api/api.rb b/lib/api/api.rb index 223ae13bd2d..e500a93b31e 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -104,7 +104,6 @@ module API mount ::API::BroadcastMessages mount ::API::Commits mount ::API::CommitStatuses - mount ::API::ContainerRegistry mount ::API::DeployKeys mount ::API::Deployments mount ::API::Environments @@ -116,6 +115,7 @@ module API mount ::API::GroupLabels mount ::API::GroupMilestones mount ::API::Groups + mount ::API::GroupContainerRepositories mount ::API::GroupVariables mount ::API::ImportGithub mount ::API::Internal @@ -138,6 +138,7 @@ module API mount ::API::Pipelines mount ::API::PipelineSchedules mount ::API::ProjectClusters + mount ::API::ProjectContainerRepositories mount ::API::ProjectEvents mount ::API::ProjectExport mount ::API::ProjectImport diff --git a/lib/api/container_registry.rb b/lib/api/container_registry.rb deleted file mode 100644 index 7dad20a822a..00000000000 --- a/lib/api/container_registry.rb +++ /dev/null @@ -1,149 +0,0 @@ -# frozen_string_literal: true - -module API - class ContainerRegistry < Grape::API - include PaginationParams - - REGISTRY_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge( - tag_name: API::NO_SLASH_URL_PART_REGEX) - - before { error!('404 Not Found', 404) unless Feature.enabled?(:container_registry_api, user_project, default_enabled: true) } - before { authorize_read_container_images! } - - params do - requires :id, type: String, desc: 'The ID of a project' - end - resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do - desc 'Get a project container repositories' do - detail 'This feature was introduced in GitLab 11.8.' - success Entities::ContainerRegistry::Repository - end - params do - use :pagination - end - get ':id/registry/repositories' do - repositories = user_project.container_repositories.ordered - - present paginate(repositories), with: Entities::ContainerRegistry::Repository - end - - desc 'Delete repository' do - detail 'This feature was introduced in GitLab 11.8.' - end - params do - requires :repository_id, type: Integer, desc: 'The ID of the repository' - end - delete ':id/registry/repositories/:repository_id', requirements: REGISTRY_ENDPOINT_REQUIREMENTS do - authorize_admin_container_image! - - DeleteContainerRepositoryWorker.perform_async(current_user.id, repository.id) - - status :accepted - end - - desc 'Get a list of repositories tags' do - detail 'This feature was introduced in GitLab 11.8.' - success Entities::ContainerRegistry::Tag - end - params do - requires :repository_id, type: Integer, desc: 'The ID of the repository' - use :pagination - end - get ':id/registry/repositories/:repository_id/tags', requirements: REGISTRY_ENDPOINT_REQUIREMENTS do - authorize_read_container_image! - - tags = Kaminari.paginate_array(repository.tags) - present paginate(tags), with: Entities::ContainerRegistry::Tag - end - - desc 'Delete repository tags (in bulk)' do - detail 'This feature was introduced in GitLab 11.8.' - end - params do - requires :repository_id, type: Integer, desc: 'The ID of the repository' - requires :name_regex, type: String, desc: 'The tag name regexp to delete, specify .* to delete all' - optional :keep_n, type: Integer, desc: 'Keep n of latest tags with matching name' - optional :older_than, type: String, desc: 'Delete older than: 1h, 1d, 1month' - end - delete ':id/registry/repositories/:repository_id/tags', requirements: REGISTRY_ENDPOINT_REQUIREMENTS do - authorize_admin_container_image! - - message = 'This request has already been made. You can run this at most once an hour for a given container repository' - render_api_error!(message, 400) unless obtain_new_cleanup_container_lease - - CleanupContainerRepositoryWorker.perform_async(current_user.id, repository.id, - declared_params.except(:repository_id)) # rubocop: disable CodeReuse/ActiveRecord - - status :accepted - end - - desc 'Get a details about repository tag' do - detail 'This feature was introduced in GitLab 11.8.' - success Entities::ContainerRegistry::TagDetails - end - params do - requires :repository_id, type: Integer, desc: 'The ID of the repository' - requires :tag_name, type: String, desc: 'The name of the tag' - end - get ':id/registry/repositories/:repository_id/tags/:tag_name', requirements: REGISTRY_ENDPOINT_REQUIREMENTS do - authorize_read_container_image! - validate_tag! - - present tag, with: Entities::ContainerRegistry::TagDetails - end - - desc 'Delete repository tag' do - detail 'This feature was introduced in GitLab 11.8.' - end - params do - requires :repository_id, type: Integer, desc: 'The ID of the repository' - requires :tag_name, type: String, desc: 'The name of the tag' - end - delete ':id/registry/repositories/:repository_id/tags/:tag_name', requirements: REGISTRY_ENDPOINT_REQUIREMENTS do - authorize_destroy_container_image! - validate_tag! - - tag.delete - - status :ok - end - end - - helpers do - def authorize_read_container_images! - authorize! :read_container_image, user_project - end - - def authorize_read_container_image! - authorize! :read_container_image, repository - end - - def authorize_destroy_container_image! - authorize! :destroy_container_image, repository - end - - def authorize_admin_container_image! - authorize! :admin_container_image, repository - end - - def obtain_new_cleanup_container_lease - Gitlab::ExclusiveLease - .new("container_repository:cleanup_tags:#{repository.id}", - timeout: 1.hour) - .try_obtain - end - - def repository - @repository ||= user_project.container_repositories.find(params[:repository_id]) - end - - def tag - @tag ||= repository.tag(params[:tag_name]) - end - - def validate_tag! - not_found!('Tag') unless tag.valid? - end - end - end -end diff --git a/lib/api/entities/container_registry.rb b/lib/api/entities/container_registry.rb index 00833ca7480..6250f35c7cb 100644 --- a/lib/api/entities/container_registry.rb +++ b/lib/api/entities/container_registry.rb @@ -3,18 +3,20 @@ module API module Entities module ContainerRegistry - class Repository < Grape::Entity - expose :id + class Tag < Grape::Entity expose :name expose :path expose :location - expose :created_at end - class Tag < Grape::Entity + class Repository < Grape::Entity + expose :id expose :name expose :path + expose :project_id expose :location + expose :created_at + expose :tags, using: Tag, if: -> (_, options) { options[:tags] } end class TagDetails < Tag diff --git a/lib/api/group_container_repositories.rb b/lib/api/group_container_repositories.rb new file mode 100644 index 00000000000..fd24662cc9a --- /dev/null +++ b/lib/api/group_container_repositories.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module API + class GroupContainerRepositories < Grape::API + include PaginationParams + + before { authorize_read_group_container_images! } + + REPOSITORY_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge( + tag_name: API::NO_SLASH_URL_PART_REGEX) + + params do + requires :id, type: String, desc: "Group's ID or path" + end + resource :groups, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + desc 'Get a list of all repositories within a group' do + detail 'This feature was introduced in GitLab 12.2.' + success Entities::ContainerRegistry::Repository + end + params do + use :pagination + optional :tags, type: Boolean, default: false, desc: 'Determines if tags should be included' + end + get ':id/registry/repositories' do + repositories = ContainerRepositoriesFinder.new( + id: user_group.id, container_type: :group + ).execute + + present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags] + end + end + + helpers do + def authorize_read_group_container_images! + authorize! :read_container_image, user_group + end + end + end +end diff --git a/lib/api/project_container_repositories.rb b/lib/api/project_container_repositories.rb new file mode 100644 index 00000000000..6d53abcc500 --- /dev/null +++ b/lib/api/project_container_repositories.rb @@ -0,0 +1,152 @@ +# frozen_string_literal: true + +module API + class ProjectContainerRepositories < Grape::API + include PaginationParams + + REPOSITORY_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge( + tag_name: API::NO_SLASH_URL_PART_REGEX) + + before { error!('404 Not Found', 404) unless Feature.enabled?(:container_registry_api, user_project, default_enabled: true) } + before { authorize_read_container_images! } + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + desc 'Get a project container repositories' do + detail 'This feature was introduced in GitLab 11.8.' + success Entities::ContainerRegistry::Repository + end + params do + use :pagination + optional :tags, type: Boolean, default: false, desc: 'Determines if tags should be included' + end + get ':id/registry/repositories' do + repositories = ContainerRepositoriesFinder.new( + id: user_project.id, container_type: :project + ).execute + + present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags] + end + + desc 'Delete repository' do + detail 'This feature was introduced in GitLab 11.8.' + end + params do + requires :repository_id, type: Integer, desc: 'The ID of the repository' + end + delete ':id/registry/repositories/:repository_id', requirements: REPOSITORY_ENDPOINT_REQUIREMENTS do + authorize_admin_container_image! + + DeleteContainerRepositoryWorker.perform_async(current_user.id, repository.id) + + status :accepted + end + + desc 'Get a list of repositories tags' do + detail 'This feature was introduced in GitLab 11.8.' + success Entities::ContainerRegistry::Tag + end + params do + requires :repository_id, type: Integer, desc: 'The ID of the repository' + use :pagination + end + get ':id/registry/repositories/:repository_id/tags', requirements: REPOSITORY_ENDPOINT_REQUIREMENTS do + authorize_read_container_image! + + tags = Kaminari.paginate_array(repository.tags) + present paginate(tags), with: Entities::ContainerRegistry::Tag + end + + desc 'Delete repository tags (in bulk)' do + detail 'This feature was introduced in GitLab 11.8.' + end + params do + requires :repository_id, type: Integer, desc: 'The ID of the repository' + requires :name_regex, type: String, desc: 'The tag name regexp to delete, specify .* to delete all' + optional :keep_n, type: Integer, desc: 'Keep n of latest tags with matching name' + optional :older_than, type: String, desc: 'Delete older than: 1h, 1d, 1month' + end + delete ':id/registry/repositories/:repository_id/tags', requirements: REPOSITORY_ENDPOINT_REQUIREMENTS do + authorize_admin_container_image! + + message = 'This request has already been made. You can run this at most once an hour for a given container repository' + render_api_error!(message, 400) unless obtain_new_cleanup_container_lease + + CleanupContainerRepositoryWorker.perform_async(current_user.id, repository.id, + declared_params.except(:repository_id)) # rubocop: disable CodeReuse/ActiveRecord + + status :accepted + end + + desc 'Get a details about repository tag' do + detail 'This feature was introduced in GitLab 11.8.' + success Entities::ContainerRegistry::TagDetails + end + params do + requires :repository_id, type: Integer, desc: 'The ID of the repository' + requires :tag_name, type: String, desc: 'The name of the tag' + end + get ':id/registry/repositories/:repository_id/tags/:tag_name', requirements: REPOSITORY_ENDPOINT_REQUIREMENTS do + authorize_read_container_image! + validate_tag! + + present tag, with: Entities::ContainerRegistry::TagDetails + end + + desc 'Delete repository tag' do + detail 'This feature was introduced in GitLab 11.8.' + end + params do + requires :repository_id, type: Integer, desc: 'The ID of the repository' + requires :tag_name, type: String, desc: 'The name of the tag' + end + delete ':id/registry/repositories/:repository_id/tags/:tag_name', requirements: REPOSITORY_ENDPOINT_REQUIREMENTS do + authorize_destroy_container_image! + validate_tag! + + tag.delete + + status :ok + end + end + + helpers do + def authorize_read_container_images! + authorize! :read_container_image, user_project + end + + def authorize_read_container_image! + authorize! :read_container_image, repository + end + + def authorize_destroy_container_image! + authorize! :destroy_container_image, repository + end + + def authorize_admin_container_image! + authorize! :admin_container_image, repository + end + + def obtain_new_cleanup_container_lease + Gitlab::ExclusiveLease + .new("container_repository:cleanup_tags:#{repository.id}", + timeout: 1.hour) + .try_obtain + end + + def repository + @repository ||= user_project.container_repositories.find(params[:repository_id]) + end + + def tag + @tag ||= repository.tag(params[:tag_name]) + end + + def validate_tag! + not_found!('Tag') unless tag.valid? + end + end + end +end diff --git a/spec/finders/container_repositories_finder_spec.rb b/spec/finders/container_repositories_finder_spec.rb new file mode 100644 index 00000000000..deec62d6598 --- /dev/null +++ b/spec/finders/container_repositories_finder_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ContainerRepositoriesFinder do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + let(:project_repository) { create(:container_repository, project: project) } + + describe '#execute' do + let(:id) { nil } + + subject { described_class.new(id: id, container_type: container_type).execute } + + context 'when container_type is group' do + let(:other_project) { create(:project, group: group) } + + let(:other_repository) do + create(:container_repository, name: 'test_repository2', project: other_project) + end + + let(:container_type) { :group } + let(:id) { group.id } + + it { is_expected.to match_array([project_repository, other_repository]) } + end + + context 'when container_type is project' do + let(:container_type) { :project } + let(:id) { project.id } + + it { is_expected.to match_array([project_repository]) } + end + + context 'with invalid id' do + let(:container_type) { :project } + let(:id) { 123456789 } + + it 'raises an error' do + expect { subject.execute }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end +end diff --git a/spec/fixtures/api/schemas/registry/repository.json b/spec/fixtures/api/schemas/registry/repository.json index e0fd4620c43..d0a068b65a7 100644 --- a/spec/fixtures/api/schemas/registry/repository.json +++ b/spec/fixtures/api/schemas/registry/repository.json @@ -17,6 +17,9 @@ "path": { "type": "string" }, + "project_id": { + "type": "integer" + }, "location": { "type": "string" }, @@ -28,7 +31,8 @@ }, "destroy_path": { "type": "string" - } + }, + "tags": { "$ref": "tags.json" } }, "additionalProperties": false } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 7e9bbf5a407..1c41ceb7deb 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -23,6 +23,7 @@ describe Group do it { is_expected.to have_many(:badges).class_name('GroupBadge') } it { is_expected.to have_many(:cluster_groups).class_name('Clusters::Group') } it { is_expected.to have_many(:clusters).class_name('Clusters::Cluster') } + it { is_expected.to have_many(:container_repositories) } describe '#members & #requesters' do let(:requester) { create(:user) } diff --git a/spec/requests/api/container_registry_spec.rb b/spec/requests/api/container_registry_spec.rb deleted file mode 100644 index b64f3ea1081..00000000000 --- a/spec/requests/api/container_registry_spec.rb +++ /dev/null @@ -1,245 +0,0 @@ -require 'spec_helper' - -describe API::ContainerRegistry do - include ExclusiveLeaseHelpers - - set(:project) { create(:project, :private) } - set(:maintainer) { create(:user) } - set(:developer) { create(:user) } - set(:reporter) { create(:user) } - set(:guest) { create(:user) } - - let(:root_repository) { create(:container_repository, :root, project: project) } - let(:test_repository) { create(:container_repository, project: project) } - - let(:api_user) { maintainer } - - before do - project.add_maintainer(maintainer) - project.add_developer(developer) - project.add_reporter(reporter) - project.add_guest(guest) - - stub_feature_flags(container_registry_api: true) - stub_container_registry_config(enabled: true) - - root_repository - test_repository - end - - shared_examples 'being disallowed' do |param| - context "for #{param}" do - let(:api_user) { public_send(param) } - - it 'returns access denied' do - subject - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context "for anonymous" do - let(:api_user) { nil } - - it 'returns not found' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - describe 'GET /projects/:id/registry/repositories' do - subject { get api("/projects/#{project.id}/registry/repositories", api_user) } - - it_behaves_like 'being disallowed', :guest - - context 'for reporter' do - let(:api_user) { reporter } - - it 'returns a list of repositories' do - subject - - expect(json_response.length).to eq(2) - expect(json_response.map { |repository| repository['id'] }).to contain_exactly( - root_repository.id, test_repository.id) - end - - it 'returns a matching schema' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('registry/repositories') - end - end - end - - describe 'DELETE /projects/:id/registry/repositories/:repository_id' do - subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}", api_user) } - - it_behaves_like 'being disallowed', :developer - - context 'for maintainer' do - let(:api_user) { maintainer } - - it 'schedules removal of repository' do - expect(DeleteContainerRepositoryWorker).to receive(:perform_async) - .with(maintainer.id, root_repository.id) - - subject - - expect(response).to have_gitlab_http_status(:accepted) - end - end - end - - describe 'GET /projects/:id/registry/repositories/:repository_id/tags' do - subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user) } - - it_behaves_like 'being disallowed', :guest - - context 'for reporter' do - let(:api_user) { reporter } - - before do - stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA latest)) - end - - it 'returns a list of tags' do - subject - - expect(json_response.length).to eq(2) - expect(json_response.map { |repository| repository['name'] }).to eq %w(latest rootA) - end - - it 'returns a matching schema' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('registry/tags') - end - end - end - - describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags' do - subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user), params: params } - - it_behaves_like 'being disallowed', :developer do - let(:params) do - { name_regex: 'v10.*' } - end - end - - context 'for maintainer' do - let(:api_user) { maintainer } - - context 'without required parameters' do - let(:params) { } - - it 'returns bad request' do - subject - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - - context 'passes all declared parameters' do - let(:params) do - { name_regex: 'v10.*', - keep_n: 100, - older_than: '1 day', - other: 'some value' } - end - - let(:worker_params) do - { name_regex: 'v10.*', - keep_n: 100, - older_than: '1 day' } - end - - let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" } - - it 'schedules cleanup of tags repository' do - stub_exclusive_lease(lease_key, timeout: 1.hour) - expect(CleanupContainerRepositoryWorker).to receive(:perform_async) - .with(maintainer.id, root_repository.id, worker_params) - - subject - - expect(response).to have_gitlab_http_status(:accepted) - end - - context 'called multiple times in one hour' do - it 'returns 400 with an error message' do - stub_exclusive_lease_taken(lease_key, timeout: 1.hour) - subject - - expect(response).to have_gitlab_http_status(400) - expect(response.body).to include('This request has already been made.') - end - - it 'executes service only for the first time' do - expect(CleanupContainerRepositoryWorker).to receive(:perform_async).once - - 2.times { subject } - end - end - end - end - end - - describe 'GET /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do - subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) } - - it_behaves_like 'being disallowed', :guest - - context 'for reporter' do - let(:api_user) { reporter } - - before do - stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true) - end - - it 'returns a details of tag' do - subject - - expect(json_response).to include( - 'name' => 'rootA', - 'digest' => 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15', - 'revision' => 'd7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac', - 'total_size' => 2319870) - end - - it 'returns a matching schema' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('registry/tag') - end - end - end - - describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do - subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) } - - it_behaves_like 'being disallowed', :reporter - - context 'for developer' do - let(:api_user) { developer } - - before do - stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true) - end - - it 'properly removes tag' do - expect_any_instance_of(ContainerRegistry::Client) - .to receive(:delete_repository_tag).with(root_repository.path, - 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15') - - subject - - expect(response).to have_gitlab_http_status(:ok) - end - end - end -end diff --git a/spec/requests/api/group_container_repositories_spec.rb b/spec/requests/api/group_container_repositories_spec.rb new file mode 100644 index 00000000000..0a41e455d01 --- /dev/null +++ b/spec/requests/api/group_container_repositories_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::GroupContainerRepositories do + set(:group) { create(:group, :private) } + set(:project) { create(:project, :private, group: group) } + let(:reporter) { create(:user) } + let(:guest) { create(:user) } + + let(:root_repository) { create(:container_repository, :root, project: project) } + let(:test_repository) { create(:container_repository, project: project) } + + let(:users) do + { + anonymous: nil, + guest: guest, + reporter: reporter + } + end + + let(:api_user) { reporter } + + before do + group.add_reporter(reporter) + group.add_guest(guest) + + stub_feature_flags(container_registry_api: true) + stub_container_registry_config(enabled: true) + + root_repository + test_repository + end + + describe 'GET /groups/:id/registry/repositories' do + let(:url) { "/groups/#{group.id}/registry/repositories" } + + subject { get api(url, api_user) } + + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + it_behaves_like 'returns repositories for allowed users', :reporter, 'group' do + let(:object) { group } + end + + context 'with invalid group id' do + let(:url) { '/groups/123412341234/registry/repositories' } + + it 'returns not found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/requests/api/project_container_repositories_spec.rb b/spec/requests/api/project_container_repositories_spec.rb new file mode 100644 index 00000000000..f1dc4e6f0b2 --- /dev/null +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -0,0 +1,228 @@ +require 'spec_helper' + +describe API::ProjectContainerRepositories do + include ExclusiveLeaseHelpers + + set(:project) { create(:project, :private) } + set(:maintainer) { create(:user) } + set(:developer) { create(:user) } + set(:reporter) { create(:user) } + set(:guest) { create(:user) } + + let(:root_repository) { create(:container_repository, :root, project: project) } + let(:test_repository) { create(:container_repository, project: project) } + + let(:users) do + { + anonymous: nil, + developer: developer, + guest: guest, + maintainer: maintainer, + reporter: reporter + } + end + + let(:api_user) { maintainer } + + before do + project.add_maintainer(maintainer) + project.add_developer(developer) + project.add_reporter(reporter) + project.add_guest(guest) + + stub_feature_flags(container_registry_api: true) + stub_container_registry_config(enabled: true) + + root_repository + test_repository + end + + describe 'GET /projects/:id/registry/repositories' do + let(:url) { "/projects/#{project.id}/registry/repositories" } + + subject { get api(url, api_user) } + + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + it_behaves_like 'returns repositories for allowed users', :reporter, 'project' do + let(:object) { project } + end + end + + describe 'DELETE /projects/:id/registry/repositories/:repository_id' do + subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}", api_user) } + + it_behaves_like 'rejected container repository access', :developer, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + context 'for maintainer' do + let(:api_user) { maintainer } + + it 'schedules removal of repository' do + expect(DeleteContainerRepositoryWorker).to receive(:perform_async) + .with(maintainer.id, root_repository.id) + + subject + + expect(response).to have_gitlab_http_status(:accepted) + end + end + end + + describe 'GET /projects/:id/registry/repositories/:repository_id/tags' do + subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user) } + + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + context 'for reporter' do + let(:api_user) { reporter } + + before do + stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA latest)) + end + + it 'returns a list of tags' do + subject + + expect(json_response.length).to eq(2) + expect(json_response.map { |repository| repository['name'] }).to eq %w(latest rootA) + end + + it 'returns a matching schema' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('registry/tags') + end + end + end + + describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags' do + subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags", api_user), params: params } + + context 'disallowed' do + let(:params) do + { name_regex: 'v10.*' } + end + + it_behaves_like 'rejected container repository access', :developer, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + end + + context 'for maintainer' do + let(:api_user) { maintainer } + + context 'without required parameters' do + let(:params) { } + + it 'returns bad request' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'passes all declared parameters' do + let(:params) do + { name_regex: 'v10.*', + keep_n: 100, + older_than: '1 day', + other: 'some value' } + end + + let(:worker_params) do + { name_regex: 'v10.*', + keep_n: 100, + older_than: '1 day' } + end + + let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" } + + it 'schedules cleanup of tags repository' do + stub_exclusive_lease(lease_key, timeout: 1.hour) + expect(CleanupContainerRepositoryWorker).to receive(:perform_async) + .with(maintainer.id, root_repository.id, worker_params) + + subject + + expect(response).to have_gitlab_http_status(:accepted) + end + + context 'called multiple times in one hour' do + it 'returns 400 with an error message' do + stub_exclusive_lease_taken(lease_key, timeout: 1.hour) + subject + + expect(response).to have_gitlab_http_status(400) + expect(response.body).to include('This request has already been made.') + end + + it 'executes service only for the first time' do + expect(CleanupContainerRepositoryWorker).to receive(:perform_async).once + + 2.times { subject } + end + end + end + end + end + + describe 'GET /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do + subject { get api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) } + + it_behaves_like 'rejected container repository access', :guest, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + context 'for reporter' do + let(:api_user) { reporter } + + before do + stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true) + end + + it 'returns a details of tag' do + subject + + expect(json_response).to include( + 'name' => 'rootA', + 'digest' => 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15', + 'revision' => 'd7a513a663c1a6dcdba9ed832ca53c02ac2af0c333322cd6ca92936d1d9917ac', + 'total_size' => 2319870) + end + + it 'returns a matching schema' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('registry/tag') + end + end + end + + describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do + subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) } + + it_behaves_like 'rejected container repository access', :reporter, :forbidden + it_behaves_like 'rejected container repository access', :anonymous, :not_found + + context 'for developer' do + let(:api_user) { developer } + + before do + stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true) + end + + it 'properly removes tag' do + expect_any_instance_of(ContainerRegistry::Client) + .to receive(:delete_repository_tag).with(root_repository.path, + 'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15') + + subject + + expect(response).to have_gitlab_http_status(:ok) + end + end + end +end diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index c11725c63d2..fd24c443288 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -16,7 +16,7 @@ RSpec.shared_context 'GroupPolicy context' do read_group_merge_requests ] end - let(:reporter_permissions) { [:admin_label] } + let(:reporter_permissions) { %i[admin_label read_container_image] } let(:developer_permissions) { [:admin_milestone] } let(:maintainer_permissions) do %i[ diff --git a/spec/support/shared_examples/container_repositories_shared_examples.rb b/spec/support/shared_examples/container_repositories_shared_examples.rb new file mode 100644 index 00000000000..946b130fca2 --- /dev/null +++ b/spec/support/shared_examples/container_repositories_shared_examples.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +shared_examples 'rejected container repository access' do |user_type, status| + context "for #{user_type}" do + let(:api_user) { users[user_type] } + + it "returns #{status}" do + subject + + expect(response).to have_gitlab_http_status(status) + end + end +end + +shared_examples 'returns repositories for allowed users' do |user_type, scope| + context "for #{user_type}" do + it 'returns a list of repositories' do + subject + + expect(json_response.length).to eq(2) + expect(json_response.map { |repository| repository['id'] }).to contain_exactly( + root_repository.id, test_repository.id) + expect(response.body).not_to include('tags') + end + + it 'returns a matching schema' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('registry/repositories') + end + + context 'with tags param' do + let(:url) { "/#{scope}s/#{object.id}/registry/repositories?tags=true" } + + before do + stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA latest), with_manifest: true) + stub_container_registry_tags(repository: test_repository.path, tags: %w(rootA latest), with_manifest: true) + end + + it 'returns a list of repositories and their tags' do + subject + + expect(json_response.length).to eq(2) + expect(json_response.map { |repository| repository['id'] }).to contain_exactly( + root_repository.id, test_repository.id) + expect(response.body).to include('tags') + end + + it 'returns a matching schema' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('registry/repositories') + end + end + end +end -- cgit v1.2.1 From 1dfbb27f6e8d01023564eededff2a0ba1a04badc Mon Sep 17 00:00:00 2001 From: GitLab Release Tools Bot Date: Mon, 5 Aug 2019 20:17:57 +0000 Subject: Update CHANGELOG.md for 12.1.4 [ci skip] --- CHANGELOG.md | 13 +++++++++++++ changelogs/unreleased/fix-i18n-updated-projects.yml | 5 ----- changelogs/unreleased/leipert-improve-ansi2html.yml | 5 ----- .../unreleased/osw-avoid-errors-due-to-concurrent-calls.yml | 5 ----- changelogs/unreleased/patch-72.yml | 5 ----- 5 files changed, 13 insertions(+), 20 deletions(-) delete mode 100644 changelogs/unreleased/fix-i18n-updated-projects.yml delete mode 100644 changelogs/unreleased/leipert-improve-ansi2html.yml delete mode 100644 changelogs/unreleased/osw-avoid-errors-due-to-concurrent-calls.yml delete mode 100644 changelogs/unreleased/patch-72.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 15b55f01c32..0752708d5e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 12.1.4 + +### Fixed (3 changes, 1 of them is from the community) + +- Properly translate term in projects list. !30958 +- Add exclusive lease to mergeability check process. !31082 +- Fix Docker in Docker (DIND) listen port behavior change by adding DOCKER_TLS_CERTDIR in CI job templates. !31201 (Cameron Boulton) + +### Performance (1 change) + +- Improve job log rendering performance. !31262 + + ## 12.1.3 ### Fixed (11 changes) diff --git a/changelogs/unreleased/fix-i18n-updated-projects.yml b/changelogs/unreleased/fix-i18n-updated-projects.yml deleted file mode 100644 index 408ee438480..00000000000 --- a/changelogs/unreleased/fix-i18n-updated-projects.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Properly translate term in projects list -merge_request: 30958 -author: -type: fixed diff --git a/changelogs/unreleased/leipert-improve-ansi2html.yml b/changelogs/unreleased/leipert-improve-ansi2html.yml deleted file mode 100644 index dd3582b3434..00000000000 --- a/changelogs/unreleased/leipert-improve-ansi2html.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Improve job log rendering performance -merge_request: 31262 -author: -type: performance diff --git a/changelogs/unreleased/osw-avoid-errors-due-to-concurrent-calls.yml b/changelogs/unreleased/osw-avoid-errors-due-to-concurrent-calls.yml deleted file mode 100644 index 17ff1b012cf..00000000000 --- a/changelogs/unreleased/osw-avoid-errors-due-to-concurrent-calls.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Add exclusive lease to mergeability check process -merge_request: 31082 -author: -type: fixed diff --git a/changelogs/unreleased/patch-72.yml b/changelogs/unreleased/patch-72.yml deleted file mode 100644 index ff2bac2fc29..00000000000 --- a/changelogs/unreleased/patch-72.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix Docker in Docker (DIND) listen port behavior change by adding DOCKER_TLS_CERTDIR in CI job templates. -merge_request: 31201 -author: Cameron Boulton -type: fixed -- cgit v1.2.1 From 6c5e9480130308abf9458f1ac58e25695c5ce2ea Mon Sep 17 00:00:00 2001 From: Gosia Ksionek Date: Mon, 5 Aug 2019 21:15:00 +0000 Subject: Fix error on project name Add project path to sql query to build proper path --- app/serializers/analytics_issue_entity.rb | 2 +- .../fix-name-vs-path-problem-for-cycle-analytics.yml | 5 +++++ lib/gitlab/cycle_analytics/base_query.rb | 13 ++++++++++--- lib/gitlab/cycle_analytics/code_event_fetcher.rb | 4 +--- lib/gitlab/cycle_analytics/issue_event_fetcher.rb | 4 +--- lib/gitlab/cycle_analytics/issue_helper.rb | 13 ++++++++++--- lib/gitlab/cycle_analytics/plan_event_fetcher.rb | 4 +--- lib/gitlab/cycle_analytics/plan_helper.rb | 6 ++++-- lib/gitlab/cycle_analytics/production_event_fetcher.rb | 1 - lib/gitlab/cycle_analytics/review_event_fetcher.rb | 4 +--- spec/serializers/analytics_issue_entity_spec.rb | 6 +++--- spec/serializers/analytics_issue_serializer_spec.rb | 6 +++--- spec/serializers/analytics_merge_request_serializer_spec.rb | 6 +++--- 13 files changed, 43 insertions(+), 31 deletions(-) create mode 100644 changelogs/unreleased/fix-name-vs-path-problem-for-cycle-analytics.yml diff --git a/app/serializers/analytics_issue_entity.rb b/app/serializers/analytics_issue_entity.rb index 29d4a6ae1d0..307ce14a921 100644 --- a/app/serializers/analytics_issue_entity.rb +++ b/app/serializers/analytics_issue_entity.rb @@ -26,6 +26,6 @@ class AnalyticsIssueEntity < Grape::Entity private def url_to(route, object) - public_send("#{route}_url", object[:path], object[:name], object[:iid].to_s) # rubocop:disable GitlabSecurity/PublicSend + public_send("#{route}_url", object[:namespace_path], object[:project_path], object[:iid].to_s) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/changelogs/unreleased/fix-name-vs-path-problem-for-cycle-analytics.yml b/changelogs/unreleased/fix-name-vs-path-problem-for-cycle-analytics.yml new file mode 100644 index 00000000000..7d171c2cf5b --- /dev/null +++ b/changelogs/unreleased/fix-name-vs-path-problem-for-cycle-analytics.yml @@ -0,0 +1,5 @@ +--- +title: Fix broken issue links and possible 500 error on cycle analytics page when project name and path are different +merge_request: 31471 +author: +type: fixed diff --git a/lib/gitlab/cycle_analytics/base_query.rb b/lib/gitlab/cycle_analytics/base_query.rb index 9c98c0bfbf2..459bb5177b5 100644 --- a/lib/gitlab/cycle_analytics/base_query.rb +++ b/lib/gitlab/cycle_analytics/base_query.rb @@ -19,9 +19,10 @@ module Gitlab .join(projects_table).on(issue_table[:project_id].eq(projects_table[:id])) .join(routes_table).on(projects_table[:namespace_id].eq(routes_table[:source_id])) .project(issue_table[:project_id].as("project_id")) - .where(issue_table[:project_id].in(project_ids)) - .where(routes_table[:source_type].eq('Namespace')) - .where(issue_table[:created_at].gteq(options[:from])) + .project(projects_table[:path].as("project_path")) + .project(routes_table[:path].as("namespace_path")) + + query = limit_query(query, project_ids) # Load merge_requests @@ -30,6 +31,12 @@ module Gitlab query end + def limit_query(query, project_ids) + query.where(issue_table[:project_id].in(project_ids)) + .where(routes_table[:source_type].eq('Namespace')) + .where(issue_table[:created_at].gteq(options[:from])) + end + def load_merge_requests(query) query.join(mr_table, Arel::Nodes::OuterJoin) .on(mr_table[:id].eq(mr_closing_issues_table[:merge_request_id])) diff --git a/lib/gitlab/cycle_analytics/code_event_fetcher.rb b/lib/gitlab/cycle_analytics/code_event_fetcher.rb index 1e4e9b9e02c..fcc282bf7a6 100644 --- a/lib/gitlab/cycle_analytics/code_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/code_event_fetcher.rb @@ -11,9 +11,7 @@ module Gitlab mr_table[:id], mr_table[:created_at], mr_table[:state], - mr_table[:author_id], - projects_table[:name], - routes_table[:path]] + mr_table[:author_id]] @order = mr_table[:created_at] super(*args) diff --git a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb index 2d03e425a6a..6914cf24c19 100644 --- a/lib/gitlab/cycle_analytics/issue_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/issue_event_fetcher.rb @@ -10,9 +10,7 @@ module Gitlab issue_table[:iid], issue_table[:id], issue_table[:created_at], - issue_table[:author_id], - projects_table[:name], - routes_table[:path]] + issue_table[:author_id]] super(*args) end diff --git a/lib/gitlab/cycle_analytics/issue_helper.rb b/lib/gitlab/cycle_analytics/issue_helper.rb index 0fc4f1dd41a..295eca5edca 100644 --- a/lib/gitlab/cycle_analytics/issue_helper.rb +++ b/lib/gitlab/cycle_analytics/issue_helper.rb @@ -8,12 +8,19 @@ module Gitlab .join(projects_table).on(issue_table[:project_id].eq(projects_table[:id])) .join(routes_table).on(projects_table[:namespace_id].eq(routes_table[:source_id])) .project(issue_table[:project_id].as("project_id")) - .where(issue_table[:project_id].in(project_ids)) + .project(projects_table[:path].as("project_path")) + .project(routes_table[:path].as("namespace_path")) + + query = limit_query(query, project_ids) + + query + end + + def limit_query(query, project_ids) + query.where(issue_table[:project_id].in(project_ids)) .where(routes_table[:source_type].eq('Namespace')) .where(issue_table[:created_at].gteq(options[:from])) .where(issue_metrics_table[:first_added_to_board_at].not_eq(nil).or(issue_metrics_table[:first_associated_with_milestone_at].not_eq(nil))) - - query end end end diff --git a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb index 77cc358daa9..bad02e00a13 100644 --- a/lib/gitlab/cycle_analytics/plan_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/plan_event_fetcher.rb @@ -10,9 +10,7 @@ module Gitlab issue_table[:iid], issue_table[:id], issue_table[:created_at], - issue_table[:author_id], - projects_table[:name], - routes_table[:path]] + issue_table[:author_id]] super(*args) end diff --git a/lib/gitlab/cycle_analytics/plan_helper.rb b/lib/gitlab/cycle_analytics/plan_helper.rb index c3f742503a9..a63ae58ad21 100644 --- a/lib/gitlab/cycle_analytics/plan_helper.rb +++ b/lib/gitlab/cycle_analytics/plan_helper.rb @@ -8,14 +8,16 @@ module Gitlab .join(projects_table).on(issue_table[:project_id].eq(projects_table[:id])) .join(routes_table).on(projects_table[:namespace_id].eq(routes_table[:source_id])) .project(issue_table[:project_id].as("project_id")) + .project(projects_table[:path].as("project_path")) + .project(routes_table[:path].as("namespace_path")) .where(issue_table[:project_id].in(project_ids)) .where(routes_table[:source_type].eq('Namespace')) - query = add_conditions_to_query(query) + query = limit_query(query) query end - def add_conditions_to_query(query) + def limit_query(query) query.where(issue_table[:created_at].gteq(options[:from])) .where(issue_metrics_table[:first_added_to_board_at].not_eq(nil).or(issue_metrics_table[:first_associated_with_milestone_at].not_eq(nil))) .where(issue_metrics_table[:first_mentioned_in_commit_at].not_eq(nil)) diff --git a/lib/gitlab/cycle_analytics/production_event_fetcher.rb b/lib/gitlab/cycle_analytics/production_event_fetcher.rb index 404b2460814..8843ab2bcb9 100644 --- a/lib/gitlab/cycle_analytics/production_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/production_event_fetcher.rb @@ -11,7 +11,6 @@ module Gitlab issue_table[:id], issue_table[:created_at], issue_table[:author_id], - projects_table[:name], routes_table[:path]] super(*args) diff --git a/lib/gitlab/cycle_analytics/review_event_fetcher.rb b/lib/gitlab/cycle_analytics/review_event_fetcher.rb index 6acd12517fa..4b5d79097b7 100644 --- a/lib/gitlab/cycle_analytics/review_event_fetcher.rb +++ b/lib/gitlab/cycle_analytics/review_event_fetcher.rb @@ -11,9 +11,7 @@ module Gitlab mr_table[:id], mr_table[:created_at], mr_table[:state], - mr_table[:author_id], - projects_table[:name], - routes_table[:path]] + mr_table[:author_id]] super(*args) end diff --git a/spec/serializers/analytics_issue_entity_spec.rb b/spec/serializers/analytics_issue_entity_spec.rb index dd5e43a4b62..c5b03bdd8c1 100644 --- a/spec/serializers/analytics_issue_entity_spec.rb +++ b/spec/serializers/analytics_issue_entity_spec.rb @@ -10,12 +10,12 @@ describe AnalyticsIssueEntity do id: "1", created_at: "2016-11-12 15:04:02.948604", author: user, - name: project.name, - path: project.namespace + project_path: project.path, + namespace_path: project.namespace.route.path } end - let(:project) { create(:project) } + let(:project) { create(:project, name: 'my project') } let(:request) { EntityRequest.new(entity: :merge_request) } let(:entity) do diff --git a/spec/serializers/analytics_issue_serializer_spec.rb b/spec/serializers/analytics_issue_serializer_spec.rb index c9ffe1c5dad..9cb2ce13d12 100644 --- a/spec/serializers/analytics_issue_serializer_spec.rb +++ b/spec/serializers/analytics_issue_serializer_spec.rb @@ -8,7 +8,7 @@ describe AnalyticsIssueSerializer do end let(:user) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, name: 'my project') } let(:resource) do { total_time: "172802.724419", @@ -17,8 +17,8 @@ describe AnalyticsIssueSerializer do id: "1", created_at: "2016-11-12 15:04:02.948604", author: user, - name: project.name, - path: project.namespace + project_path: project.path, + namespace_path: project.namespace.route.path } end diff --git a/spec/serializers/analytics_merge_request_serializer_spec.rb b/spec/serializers/analytics_merge_request_serializer_spec.rb index 123d7d795ce..a864051b2a3 100644 --- a/spec/serializers/analytics_merge_request_serializer_spec.rb +++ b/spec/serializers/analytics_merge_request_serializer_spec.rb @@ -8,7 +8,7 @@ describe AnalyticsMergeRequestSerializer do end let(:user) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, name: 'my project') } let(:resource) do { total_time: "172802.724419", @@ -18,8 +18,8 @@ describe AnalyticsMergeRequestSerializer do state: 'open', created_at: "2016-11-12 15:04:02.948604", author: user, - name: project.name, - path: project.namespace + project_path: project.path, + namespace_path: project.namespace.route.path } end -- cgit v1.2.1 From fd2862fdddf6c836d3bae5f78574fa07401b034c Mon Sep 17 00:00:00 2001 From: Alexander Tanayno Date: Mon, 5 Aug 2019 21:22:20 +0000 Subject: Add description how to set custom CI file --- doc/user/project/pipelines/settings.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/user/project/pipelines/settings.md b/doc/user/project/pipelines/settings.md index 45f27b3c811..456209c2180 100644 --- a/doc/user/project/pipelines/settings.md +++ b/doc/user/project/pipelines/settings.md @@ -76,6 +76,13 @@ Here are some valid examples: - `my/path/.gitlab-ci.yml` - `my/path/.my-custom-file.yml` +The path can be customized at a project level. To customize the path: + +1. Go to the project's **Settings > CI / CD**. +1. Expand the **General pipelines** section. +1. Provide a value in the **Custom CI config path** field. +1. Click **Save changes**. + ## Test coverage parsing If you use test coverage in your code, GitLab can capture its output in the -- cgit v1.2.1 From 79a39a5c5543331cdd54f95fdfd323006fa9fcd2 Mon Sep 17 00:00:00 2001 From: Ronald van Zon Date: Mon, 5 Aug 2019 21:24:32 +0000 Subject: Add example to plugins file --- doc/administration/plugins.md | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/doc/administration/plugins.md b/doc/administration/plugins.md index 4302667caf5..4cf3c607dae 100644 --- a/doc/administration/plugins.md +++ b/doc/administration/plugins.md @@ -52,7 +52,37 @@ as appropriate. The plugins file list is updated for each event, there is no need to restart GitLab to apply a new plugin. If a plugin executes with non-zero exit code or GitLab fails to execute it, a -message will be logged to `plugin.log`. +message will be logged to: + +- `gitlab-rails/plugin.log` in an Omnibus installation. +- `log/plugin.log` in a source installation. + +## Creating plugins + +Below is an example that will only response on the event `project_create` and +will inform the admins from the GitLab instance that a new project has been created. + +```ruby +# By using the embedded ruby version we eliminate the possibility that our chosen language +# would be unavailable from +#!/opt/gitlab/embedded/bin/ruby +require 'json' +require 'mail' + +# The incoming variables are in JSON format so we need to parse it first. +ARGS = JSON.parse(STDIN.read) + +# We only want to trigger this plugin on the event project_create +return unless ARGS['event_name'] == 'project_create' + +# We will inform our admins of our gitlab instance that a new project is created +Mail.deliver do + from 'info@gitlab_instance.com' + to 'admin@gitlab_instance.com' + subject "new project " + ARGS['name'] + body ARGS['owner_name'] + 'created project ' + ARGS['name'] +end +``` ## Validation -- cgit v1.2.1 From 96d10213714c6d463b02b768525d7411ae6bd3d0 Mon Sep 17 00:00:00 2001 From: Marcel van Remmerden Date: Mon, 5 Aug 2019 21:59:20 +0000 Subject: Revert "Make status icon in merge widget borderless" This reverts commit c3751046d217008404a0bd371e59d6ffd6734923. --- .../javascripts/reports/components/report_section.vue | 2 +- .../components/mr_widget_status_icon.vue | 11 +++++++---- app/assets/stylesheets/pages/merge_requests.scss | 16 ++-------------- .../unreleased/64831-add-padding-to-merged-by-widget.yml | 5 +++++ 4 files changed, 15 insertions(+), 19 deletions(-) create mode 100644 changelogs/unreleased/64831-add-padding-to-merged-by-widget.yml diff --git a/app/assets/javascripts/reports/components/report_section.vue b/app/assets/javascripts/reports/components/report_section.vue index 9bc3e6388e3..24612c8681a 100644 --- a/app/assets/javascripts/reports/components/report_section.vue +++ b/app/assets/javascripts/reports/components/report_section.vue @@ -166,7 +166,7 @@ export default {
-
+
{{ headerText }} diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue index 8dbd9e52cfe..13e4b061fda 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue @@ -32,10 +32,13 @@ export default { }; - {{ shortSha }} + {{ shortSha }} -
- +
+ - - {{ title }} - - + + {{ title }} + + {{ __("Can't find HEAD commit for this branch") }}
diff --git a/app/assets/stylesheets/framework/responsive_tables.scss b/app/assets/stylesheets/framework/responsive_tables.scss index 6bd44ee19bd..fd6f80e26cb 100644 --- a/app/assets/stylesheets/framework/responsive_tables.scss +++ b/app/assets/stylesheets/framework/responsive_tables.scss @@ -155,7 +155,7 @@ text-overflow: ellipsis; @include media-breakpoint-up(md) { - flex: 0 0 90%; + flex: 0 0 85%; } .avatar { diff --git a/changelogs/unreleased/64608-double-tooltips.yml b/changelogs/unreleased/64608-double-tooltips.yml new file mode 100644 index 00000000000..f6cb1944d26 --- /dev/null +++ b/changelogs/unreleased/64608-double-tooltips.yml @@ -0,0 +1,5 @@ +--- +title: Prevents showing 2 tooltips in pipelines table +merge_request: +author: +type: fixed -- cgit v1.2.1 From 18d221326ef7838a0b8391a986678ccbe763b3ec Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Tue, 6 Aug 2019 10:10:21 +0000 Subject: Add note about refactoring to board_service.js --- app/assets/javascripts/boards/services/board_service.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/javascripts/boards/services/board_service.js b/app/assets/javascripts/boards/services/board_service.js index 5202620057c..56a6cab6c73 100644 --- a/app/assets/javascripts/boards/services/board_service.js +++ b/app/assets/javascripts/boards/services/board_service.js @@ -1,4 +1,9 @@ /* eslint-disable class-methods-use-this */ +/** + * This file is intended to be deleted. + * The existing functions will removed one by one in favor of using the board store directly. + * see https://gitlab.com/gitlab-org/gitlab-ce/issues/61621 + */ import boardsStore from '~/boards/stores/boards_store'; -- cgit v1.2.1 From 48b93f7aee2e00c8ab6263591f8c4ba90436bdf9 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 5 Aug 2019 14:51:08 +0100 Subject: Hides loading spinner after request In the pipeline's actions, moves the request to the component to allow to manage the inner state properly --- .../pipelines/components/pipelines_actions.vue | 25 +++++++++++-- .../javascripts/pipelines/mixins/pipelines.js | 2 ++ changelogs/unreleased/65263-manual-action.yml | 5 +++ .../pipelines/pipelines_actions_spec.js | 42 +++++++++++++++++----- 4 files changed, 62 insertions(+), 12 deletions(-) create mode 100644 changelogs/unreleased/65263-manual-action.yml diff --git a/app/assets/javascripts/pipelines/components/pipelines_actions.vue b/app/assets/javascripts/pipelines/components/pipelines_actions.vue index 244d332f38f..4b2d816c6a0 100644 --- a/app/assets/javascripts/pipelines/components/pipelines_actions.vue +++ b/app/assets/javascripts/pipelines/components/pipelines_actions.vue @@ -1,9 +1,11 @@