diff --git a/app/assets/javascripts/monitoring/components/dashboard_header.vue b/app/assets/javascripts/monitoring/components/dashboard_header.vue index 4e9f00c6d6e..7b68c5f1680 100644 --- a/app/assets/javascripts/monitoring/components/dashboard_header.vue +++ b/app/assets/javascripts/monitoring/components/dashboard_header.vue @@ -136,7 +136,7 @@ export default { ]), selectDashboard(dashboard) { const params = { - dashboard: dashboard.path, + dashboard: encodeURIComponent(dashboard.path), }; redirectTo(mergeUrlParams(params, window.location.href)); }, diff --git a/app/assets/javascripts/monitoring/monitoring_app.js b/app/assets/javascripts/monitoring/monitoring_app.js index 1991a9ce733..6e7df9efbb1 100644 --- a/app/assets/javascripts/monitoring/monitoring_app.js +++ b/app/assets/javascripts/monitoring/monitoring_app.js @@ -11,7 +11,8 @@ export default (props = {}) => { const el = document.getElementById('prometheus-graphs'); if (el && el.dataset) { - const [currentDashboard] = getParameterValues('dashboard'); + const [encodedDashboard] = getParameterValues('dashboard'); + const currentDashboard = encodedDashboard ? decodeURIComponent(encodedDashboard) : null; const { metricsDashboardBasePath, ...dataset } = el.dataset; const { initState, dataProps } = stateAndPropsFromDataset({ currentDashboard, ...dataset }); diff --git a/app/controllers/concerns/metrics_dashboard.rb b/app/controllers/concerns/metrics_dashboard.rb index 106cf9f13fe..28d0692d748 100644 --- a/app/controllers/concerns/metrics_dashboard.rb +++ b/app/controllers/concerns/metrics_dashboard.rb @@ -13,7 +13,7 @@ module MetricsDashboard result = dashboard_finder.find( project_for_dashboard, current_user, - metrics_dashboard_params.to_h.symbolize_keys + decoded_params ) if result @@ -114,4 +114,14 @@ module MetricsDashboard json: result.slice(:all_dashboards, :message, :status) } end + + def decoded_params + params = metrics_dashboard_params + + if params[:dashboard_path] + params[:dashboard_path] = CGI.unescape(params[:dashboard_path]) + end + + params + end end diff --git a/changelogs/unreleased/rc-escape_dashboard_paths.yml b/changelogs/unreleased/rc-escape_dashboard_paths.yml new file mode 100644 index 00000000000..1751c592f4b --- /dev/null +++ b/changelogs/unreleased/rc-escape_dashboard_paths.yml @@ -0,0 +1,5 @@ +--- +title: Allow special characters in dashboard path +merge_request: 32714 +author: +type: fixed diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index c09157fd7ff..2d29c61ba29 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -4187,7 +4187,8 @@ need to be used to merge arrays. YAML has a handy feature called 'anchors', which lets you easily duplicate content across your document. Anchors can be used to duplicate/inherit properties, and is a perfect example to be used with [hidden jobs](#hide-jobs) -to provide templates for your jobs. +to provide templates for your jobs. When there is duplicate keys, GitLab will +perform a reverse deep merge based on the keys. The following example uses anchors and map merging. It will create two jobs, `test1` and `test2`, that will inherit the parameters of `.job_template`, each @@ -4248,6 +4249,8 @@ directive defined in `.postgres_services` and `.mysql_services` respectively: .job_template: &job_definition script: - test project + tags: + - dev .postgres_services: services: &postgres_definition @@ -4262,6 +4265,8 @@ directive defined in `.postgres_services` and `.mysql_services` respectively: test:postgres: <<: *job_definition services: *postgres_definition + tags: + - postgres test:mysql: <<: *job_definition @@ -4274,6 +4279,8 @@ The expanded version looks like this: .job_template: script: - test project + tags: + - dev .postgres_services: services: @@ -4291,6 +4298,8 @@ test:postgres: services: - postgres - ruby + tags: + - postgres test:mysql: script: @@ -4298,10 +4307,15 @@ test:mysql: services: - mysql - ruby + tags: + - dev ``` You can see that the hidden jobs are conveniently used as templates. +NOTE: **Note:** +Note that `tags: [dev]` has been overwritten by `tags: [postgres]`. + NOTE: **Note:** You can't use YAML anchors across multiple files when leveraging the [`include`](#include) feature. Anchors are only valid within the file they were defined in. Instead diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index a7083d799c7..833693e896d 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -814,7 +814,7 @@ When using a `JSONB` column, use the [JsonSchemaValidator](https://gitlab.com/gi ```ruby class BuildMetadata - validates: :config_options, json_schema: { filename: 'build_metadata_config_option' } + validates :config_options, json_schema: { filename: 'build_metadata_config_option' } end ``` diff --git a/spec/controllers/concerns/metrics_dashboard_spec.rb b/spec/controllers/concerns/metrics_dashboard_spec.rb index 9da28a25b71..f0c9874965e 100644 --- a/spec/controllers/concerns/metrics_dashboard_spec.rb +++ b/spec/controllers/concerns/metrics_dashboard_spec.rb @@ -76,6 +76,22 @@ RSpec.describe MetricsDashboard do end end + context 'when dashboard path includes encoded characters' do + let(:params) { { dashboard_path: 'dashboard%26copy.yml' } } + + before do + allow(controller) + .to receive(:metrics_dashboard_params) + .and_return(params) + end + + it 'decodes dashboard path' do + expect(::Gitlab::Metrics::Dashboard::Finder).to receive(:find).with(anything, anything, hash_including(dashboard_path: 'dashboard©.yml')) + + json_response + end + end + context 'when parameters are provided and the list of all dashboards is required' do before do allow(controller).to receive(:include_all_dashboards?).and_return(true) diff --git a/spec/frontend/monitoring/components/dashboard_spec.js b/spec/frontend/monitoring/components/dashboard_spec.js index 8fed77e5643..f894b2af5cf 100644 --- a/spec/frontend/monitoring/components/dashboard_spec.js +++ b/spec/frontend/monitoring/components/dashboard_spec.js @@ -426,6 +426,32 @@ describe('Dashboard', () => { ); }); }); + + describe('when custom dashboard is selected', () => { + const windowLocation = window.location; + const findDashboardDropdown = () => wrapper.find(DashboardHeader).find(DashboardsDropdown); + + beforeEach(() => { + delete window.location; + window.location = { ...windowLocation, assign: jest.fn() }; + createMountedWrapper(); + + return wrapper.vm.$nextTick(); + }); + + afterEach(() => { + window.location = windowLocation; + }); + + it('encodes dashboard param', () => { + findDashboardDropdown().vm.$emit('selectDashboard', { + path: 'dashboard©.yml', + }); + expect(window.location.assign).toHaveBeenCalledWith( + 'http://localhost/?dashboard=dashboard%2526copy.yml', + ); + }); + }); }); describe('when all requests have been commited by the store', () => { diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index bc4a4fd53e8..41c01c2cabe 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -87,7 +87,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '#data' do + describe '.data' do let!(:ud) { build(:usage_data) } before do @@ -267,7 +267,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '#usage_data_counters' do + describe '.usage_data_counters' do subject { described_class.usage_data_counters } it { is_expected.to all(respond_to :totals) } @@ -294,7 +294,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '#license_usage_data' do + describe '.license_usage_data' do subject { described_class.license_usage_data } it 'gathers license data' do @@ -307,7 +307,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end context 'when not relying on database records' do - describe '#features_usage_data_ce' do + describe '.features_usage_data_ce' do subject { described_class.features_usage_data_ce } it 'gathers feature usage data', :aggregate_failures do @@ -340,7 +340,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '#components_usage_data' do + describe '.components_usage_data' do subject { described_class.components_usage_data } it 'gathers basic components usage data' do @@ -364,7 +364,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '#app_server_type' do + describe '.app_server_type' do subject { described_class.app_server_type } it 'successfully identifies runtime and returns the identifier' do @@ -386,7 +386,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '#object_store_config' do + describe '.object_store_config' do let(:component) { 'lfs' } subject { described_class.object_store_config(component) } @@ -427,7 +427,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '#object_store_usage_data' do + describe '.object_store_usage_data' do subject { described_class.object_store_usage_data } it 'fetches object store config of five components' do @@ -446,7 +446,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '#cycle_analytics_usage_data' do + describe '.cycle_analytics_usage_data' do subject { described_class.cycle_analytics_usage_data } it 'works when queries time out in new' do @@ -464,7 +464,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '#ingress_modsecurity_usage' do + describe '.ingress_modsecurity_usage' do subject { described_class.ingress_modsecurity_usage } let(:environment) { create(:environment) } @@ -596,7 +596,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '#grafana_embed_usage_data' do + describe '.grafana_embed_usage_data' do subject { described_class.grafana_embed_usage_data } let(:project) { create(:project) } @@ -662,7 +662,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '#merge_requests_usage' do + describe '.merge_requests_usage' do let(:time_period) { { created_at: 2.days.ago..Time.current } } let(:merge_request) { create(:merge_request) } let(:other_user) { create(:user) }