From 645461f62980d26a7116c0dd60821d56e64863d7 Mon Sep 17 00:00:00 2001
From: syasonik <syasonik@gitlab.com>
Date: Wed, 1 Apr 2020 19:20:31 -0500
Subject: [PATCH] Add support for DB-less embeds
In order to support metrics embeds for alerts from external
alerting sources, we need a way to store that content,
because the alert does not yet live in the database.
This adds a class which allows post-processing for a
url-provided embed json object. As dashboard content is
already user-provided, this poses no further security concerns.
---
.../projects/environments_controller.rb | 2 +-
.../dashboard/transient_embed_service.rb | 35 +++++++++
changelogs/unreleased/sy-transient-embeds.yml | 5 ++
.../metrics/dashboard/service_selector.rb | 1 +
spec/features/markdown/metrics_spec.rb | 30 ++++++++
.../dashboard/service_selector_spec.rb | 11 +++
.../dashboard/transient_embed_service_spec.rb | 72 +++++++++++++++++++
7 files changed, 155 insertions(+), 1 deletion(-)
create mode 100644 app/services/metrics/dashboard/transient_embed_service.rb
create mode 100644 changelogs/unreleased/sy-transient-embeds.yml
create mode 100644 spec/services/metrics/dashboard/transient_embed_service_spec.rb
diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb
index 5c49fa842a4fda59..e51a5c7b84d12db8 100644
--- a/app/controllers/projects/environments_controller.rb
+++ b/app/controllers/projects/environments_controller.rb
@@ -222,7 +222,7 @@ def metrics_params
def metrics_dashboard_params
params
- .permit(:embedded, :group, :title, :y_label, :dashboard_path, :environment, :sample_metrics)
+ .permit(:embedded, :group, :title, :y_label, :dashboard_path, :environment, :sample_metrics, :embed_json)
.merge(dashboard_path: params[:dashboard], environment: environment)
end
diff --git a/app/services/metrics/dashboard/transient_embed_service.rb b/app/services/metrics/dashboard/transient_embed_service.rb
new file mode 100644
index 0000000000000000..035707dceb9fab94
--- /dev/null
+++ b/app/services/metrics/dashboard/transient_embed_service.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+# Acts as a pass-through to allow embeddable dashboards to be
+# generated based on external data, but still processed with the
+# required attributes that allow the FE to render them appropriately.
+#
+# Use Gitlab::Metrics::Dashboard::Finder to retrive dashboards.
+module Metrics
+ module Dashboard
+ class TransientEmbedService < ::Metrics::Dashboard::BaseEmbedService
+ extend ::Gitlab::Utils::Override
+
+ class << self
+ def valid_params?(params)
+ [
+ embedded?(params[:embedded]),
+ params[:embed_json]
+ ].all?
+ end
+ end
+
+ private
+
+ override :get_raw_dashboard
+ def get_raw_dashboard
+ JSON.parse(params[:embed_json])
+ end
+
+ override :sequence
+ def sequence
+ [STAGES::EndpointInserter]
+ end
+ end
+ end
+end
diff --git a/changelogs/unreleased/sy-transient-embeds.yml b/changelogs/unreleased/sy-transient-embeds.yml
new file mode 100644
index 0000000000000000..2e2189827bc9f232
--- /dev/null
+++ b/changelogs/unreleased/sy-transient-embeds.yml
@@ -0,0 +1,5 @@
+---
+title: Add support for database-independent embedded metric charts
+merge_request: 28618
+author:
+type: added
diff --git a/lib/gitlab/metrics/dashboard/service_selector.rb b/lib/gitlab/metrics/dashboard/service_selector.rb
index b455f95537726f7b..49682da320cfa9b9 100644
--- a/lib/gitlab/metrics/dashboard/service_selector.rb
+++ b/lib/gitlab/metrics/dashboard/service_selector.rb
@@ -16,6 +16,7 @@ class << self
::Metrics::Dashboard::GitlabAlertEmbedService,
::Metrics::Dashboard::CustomMetricEmbedService,
::Metrics::Dashboard::GrafanaMetricEmbedService,
+ ::Metrics::Dashboard::TransientEmbedService,
::Metrics::Dashboard::DynamicEmbedService,
::Metrics::Dashboard::DefaultEmbedService,
::Metrics::Dashboard::SystemDashboardService,
diff --git a/spec/features/markdown/metrics_spec.rb b/spec/features/markdown/metrics_spec.rb
index 0d8858a7afd9e5ab..dadb9571c545ace3 100644
--- a/spec/features/markdown/metrics_spec.rb
+++ b/spec/features/markdown/metrics_spec.rb
@@ -136,6 +136,36 @@
end
end
+ context 'transient metrics embeds' do
+ let(:metrics_url) { urls.metrics_project_environment_url(project, environment, embed_json: embed_json) }
+ let(:title) { 'Important Metrics' }
+ let(:embed_json) do
+ {
+ panel_groups: [{
+ panels: [{
+ type: "line-graph",
+ title: title,
+ y_label: "metric",
+ metrics: [{
+ query_range: "metric * 0.5 < 1"
+ }]
+ }]
+ }]
+ }.to_json
+ end
+
+ before do
+ stub_any_prometheus_request_with_response
+ end
+
+ it 'shows embedded metrics' do
+ visit project_issue_path(project, issue)
+
+ expect(page).to have_css('div.prometheus-graph')
+ expect(page).to have_text(title)
+ end
+ end
+
def import_common_metrics
::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute
end
diff --git a/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb b/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb
index 387baf1ee5364679..245c98cdd00cb496 100644
--- a/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb
+++ b/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb
@@ -98,6 +98,17 @@
it { is_expected.to be Metrics::Dashboard::GrafanaMetricEmbedService }
end
+
+ context 'with the embed defined in the arguments' do
+ let(:arguments) do
+ {
+ embedded: true,
+ embed_json: '{}'
+ }
+ end
+
+ it { is_expected.to be Metrics::Dashboard::TransientEmbedService }
+ end
end
end
end
diff --git a/spec/services/metrics/dashboard/transient_embed_service_spec.rb b/spec/services/metrics/dashboard/transient_embed_service_spec.rb
new file mode 100644
index 0000000000000000..fddfbe1528118f5e
--- /dev/null
+++ b/spec/services/metrics/dashboard/transient_embed_service_spec.rb
@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Metrics::Dashboard::TransientEmbedService, :use_clean_rails_memory_store_caching do
+ let_it_be(:project) { build(:project) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:environment) { create(:environment, project: project) }
+
+ before do
+ project.add_maintainer(user)
+ end
+
+ describe '.valid_params?' do
+ let(:params) { { embedded: 'true', embed_json: '{}' } }
+
+ subject { described_class.valid_params?(params) }
+
+ it { is_expected.to be_truthy }
+
+ context 'missing embedded' do
+ let(:params) { { embed_json: '{}' } }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'not embedded' do
+ let(:params) { { embedded: 'false', embed_json: '{}' } }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'missing embed_json' do
+ let(:params) { { embedded: 'true' } }
+
+ it { is_expected.to be_falsey }
+ end
+ end
+
+ describe '#get_dashboard' do
+ let(:embed_json) do
+ {
+ panel_groups: [{
+ panels: [{
+ type: 'line-graph',
+ title: 'title',
+ y_label: 'y_label',
+ metrics: [{
+ query_range: 'up',
+ label: 'y_label'
+ }]
+ }]
+ }]
+ }.to_json
+ end
+ let(:service_params) { [project, user, { environment: environment, embedded: 'true', embed_json: embed_json }] }
+ let(:service_call) { described_class.new(*service_params).get_dashboard }
+
+ it_behaves_like 'valid embedded dashboard service response'
+ it_behaves_like 'raises error for users with insufficient permissions'
+
+ it 'caches the unprocessed dashboard for subsequent calls' do
+ expect_any_instance_of(described_class)
+ .to receive(:get_raw_dashboard)
+ .once
+ .and_call_original
+
+ described_class.new(*service_params).get_dashboard
+ described_class.new(*service_params).get_dashboard
+ end
+ end
+end
--
GitLab