Skip to content

G
gitlab-ce
Commit 3b7dd732 authored by Miguel Rincon's avatar Miguel Rincon Committed by James Lopez
Browse files
Options

Extract dashboard clone service 

Numerb of responsibilities
which has to be handled while cloning
dashboard is increasing, to avoid
fat controller, we need to extract
this process into separate service.
parent 5a5d928d
Hide whitespace changes
Inline Side-by-side
Showing with 512 additions and 441 deletions
app/controllers/projects/performance_monitoring/dashboards_controller.rb
View file @ 3b7dd732
...@@ -7,90 +7,53 @@ module Projects ...@@ -7,90 +7,53 @@ module Projects
before_action :check_repository_available! before_action :check_repository_available!
before_action :validate_required_params! before_action :validate_required_params!
before_action :validate_dashboard_template!
before_action :authorize_push!
USER_DASHBOARDS_DIR = ::Metrics::Dashboard::ProjectDashboardService::DASHBOARD_ROOT rescue_from ActionController::ParameterMissing do |exception|
DASHBOARD_TEMPLATES = { respond_error(http_status: :bad_request, message: _('Request parameter %{param} is missing.') % { param: exception.param })
::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH => ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH end
}.freeze
def create def create
result = ::Files::CreateService.new(project, current_user, dashboard_attrs).execute result = ::Metrics::Dashboard::CloneDashboardService.new(project, current_user, dashboard_params).execute
if result[:status] == :success if result[:status] == :success
respond_success respond_success(result)
else else
respond_error(result[:message]) respond_error(result)
end end
end end
private private
def respond_success def respond_success(result)
set_web_ide_link_notice(result.dig(:dashboard, :path))
respond_to do |format| respond_to do |format|
format.html { redirect_to ide_edit_path(project, redirect_safe_branch_name, new_dashboard_path) } format.json { render status: result.delete(:http_status), json: result }
format.json { render json: { redirect_to: ide_edit_path(project, redirect_safe_branch_name, new_dashboard_path) }, status: :created }
end end
end end
def respond_error(message) def respond_error(result)
flash[:alert] = message
respond_to do |format| respond_to do |format|
format.html { redirect_back_or_default(default: namespace_project_environments_path) } format.json { render json: { error: result[:message] }, status: result[:http_status] }
format.json { render json: { error: message }, status: :bad_request }
end end
end end
def authorize_push! def set_web_ide_link_notice(new_dashboard_path)
access_denied!(%q(You can't commit to this project)) unless user_access(project).can_push_to_branch?(params[:branch]) web_ide_link_start = "<a href=\"#{ide_edit_path(project, redirect_safe_branch_name, new_dashboard_path)}\">"
message = _("Your dashboard has been copied. You can %{web_ide_link_start}edit it here%{web_ide_link_end}.") % { web_ide_link_start: web_ide_link_start, web_ide_link_end: "</a>" }
flash[:notice] = message.html_safe
end end
def validate_required_params! def validate_required_params!
params.require(%i(branch file_name dashboard)) params.require(%i(branch file_name dashboard commit_message))
end
def validate_dashboard_template!
access_denied! unless dashboard_template
end
def dashboard_attrs
{
commit_message: commit_message,
file_path: new_dashboard_path,
file_content: new_dashboard_content,
encoding: 'text',
branch_name: params[:branch],
start_branch: repository.branch_exists?(params[:branch]) ? params[:branch] : project.default_branch
}
end
def commit_message
params[:commit_message] || "Create custom dashboard #{params[:file_name]}"
end
def new_dashboard_path
File.join(USER_DASHBOARDS_DIR, params[:file_name])
end
def new_dashboard_content
File.read(Rails.root.join(dashboard_template))
end
def dashboard_template
dashboard_templates[params[:dashboard]]
end
def dashboard_templates
DASHBOARD_TEMPLATES
end end
def redirect_safe_branch_name def redirect_safe_branch_name
repository.find_branch(params[:branch]).name repository.find_branch(params[:branch]).name
end end
def dashboard_params
params.permit(%i(branch file_name dashboard commit_message)).to_h
end
end end
end end
end end
Projects::PerformanceMonitoring::DashboardsController.prepend_if_ee('EE::Projects::PerformanceMonitoring::DashboardsController')
app/helpers/environments_helper.rb
View file @ 3b7dd732
...@@ -31,6 +31,7 @@ module EnvironmentsHelper ...@@ -31,6 +31,7 @@ module EnvironmentsHelper
"metrics-endpoint" => additional_metrics_project_environment_path(project, environment, format: :json), "metrics-endpoint" => additional_metrics_project_environment_path(project, environment, format: :json),
"dashboard-endpoint" => metrics_dashboard_project_environment_path(project, environment, format: :json), "dashboard-endpoint" => metrics_dashboard_project_environment_path(project, environment, format: :json),
"deployments-endpoint" => project_environment_deployments_path(project, environment, format: :json), "deployments-endpoint" => project_environment_deployments_path(project, environment, format: :json),
"default-branch" => project.default_branch,
"environments-endpoint": project_environments_path(project, format: :json), "environments-endpoint": project_environments_path(project, format: :json),
"project-path" => project_path(project), "project-path" => project_path(project),
"tags-path" => project_tags_path(project), "tags-path" => project_tags_path(project),
......
app/services/metrics/dashboard/clone_dashboard_service.rb 0 → 100644
View file @ 3b7dd732
# frozen_string_literal: true
# Copies system dashboard definition in .yml file into designated
# .yml file inside `.gitlab/dashboards`
module Metrics
module Dashboard
class CloneDashboardService < ::BaseService
ALLOWED_FILE_TYPE = '.yml'
USER_DASHBOARDS_DIR = ::Metrics::Dashboard::ProjectDashboardService::DASHBOARD_ROOT
def self.allowed_dashboard_templates
@allowed_dashboard_templates ||= Set[::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH].freeze
end
def execute
catch(:error) do
throw(:error, error(_(%q(You can't commit to this project)), :forbidden)) unless push_authorized?
result = ::Files::CreateService.new(project, current_user, dashboard_attrs).execute
throw(:error, wrap_error(result)) unless result[:status] == :success
repository.refresh_method_caches([:metrics_dashboard])
success(result.merge(http_status: :created, dashboard: dashboard_details))
end
end
private
def dashboard_attrs
{
commit_message: params[:commit_message],
file_path: new_dashboard_path,
file_content: new_dashboard_content,
encoding: 'text',
branch_name: branch,
start_branch: repository.branch_exists?(branch) ? branch : project.default_branch
}
end
def dashboard_details
{
path: new_dashboard_path,
display_name: ::Metrics::Dashboard::ProjectDashboardService.name_for_path(new_dashboard_path),
default: false,
system_dashboard: false
}
end
def push_authorized?
Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch)
end
def dashboard_template
@dashboard_template ||= begin
throw(:error, error(_('Not found.'), :not_found)) unless self.class.allowed_dashboard_templates.include?(params[:dashboard])
params[:dashboard]
end
end
def branch
@branch ||= begin
throw(:error, error(_('There was an error creating the dashboard, branch name is invalid.'), :bad_request)) unless valid_branch_name?
throw(:error, error(_('There was an error creating the dashboard, branch named: %{branch} already exists.') % { branch: params[:branch] }, :bad_request)) unless new_or_default_branch? # temporary validation for first UI iteration
params[:branch]
end
end
def new_or_default_branch?
!repository.branch_exists?(params[:branch]) || project.default_branch == params[:branch]
end
def valid_branch_name?
Gitlab::GitRefValidator.validate(params[:branch])
end
def new_dashboard_path
@new_dashboard_path ||= File.join(USER_DASHBOARDS_DIR, file_name)
end
def file_name
@file_name ||= begin
throw(:error, error(_('The file name should have a .yml extension'), :bad_request)) unless target_file_type_valid?
File.basename(params[:file_name])
end
end
def target_file_type_valid?
File.extname(params[:file_name]) == ALLOWED_FILE_TYPE
end
def new_dashboard_content
File.read(Rails.root.join(dashboard_template))
end
def repository
@repository ||= project.repository
end
def wrap_error(result)
if result[:message] == 'A file with this name already exists'
error(_("A file with '%{file_name}' already exists in %{branch} branch") % { file_name: file_name, branch: branch }, :bad_request)
else
result
end
end
end
end
end
Metrics::Dashboard::CloneDashboardService.prepend_if_ee('EE::Metrics::Dashboard::CloneDashboardService')
ee/app/controllers/ee/projects/performance_monitoring/dashboards_controller.rb deleted 100644 → 0
View file @ 5a5d928d
# frozen_string_literal: true
module EE
module Projects
module PerformanceMonitoring
module DashboardsController
extend ::Gitlab::Utils::Override
DASHBOARD_TEMPLATES = {
::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH => ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH,
::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH => ::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH
}.freeze
private
override :dashboard_templates
def dashboard_templates
DASHBOARD_TEMPLATES
end
end
end
end
end
ee/app/services/ee/metrics/dashboard/clone_dashboard_service.rb 0 → 100644
View file @ 3b7dd732
# frozen_string_literal: true
# Copies system dashboard definition in .yml file into designated
# .yml file inside `.gitlab/dashboards`
module EE
module Metrics
module Dashboard
module CloneDashboardService
extend ActiveSupport::Concern
class_methods do
extend ::Gitlab::Utils::Override
override :allowed_dashboard_templates
def allowed_dashboard_templates
@allowed_dashboard_templates ||= (Set[::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH] + super).freeze
end
end
end
end
end
end
ee/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb deleted 100644 → 0
View file @ 5a5d928d
# frozen_string_literal: true
require 'spec_helper'
describe Projects::PerformanceMonitoring::DashboardsController do
let_it_be(:user) { create(:user) }
let_it_be(:namespace) { create(:namespace) }
let!(:project) { create(:project, :repository, name: 'dashboard-project', namespace: namespace) }
let(:repository) { project.repository }
let(:branch) { double(name: branch_name) }
let(:commit_message) { 'test' }
let(:branch_name) { "#{Time.current.to_i}_dashboard_new_branch" }
let(:dashboard) { 'config/prometheus/common_metrics.yml' }
let(:file_name) { 'custom_dashboard.yml' }
let(:params) do
{
namespace_id: namespace,
project_id: project,
dashboard: dashboard,
file_name: file_name,
commit_message: commit_message,
branch: branch_name,
format: :json
}
end
describe 'POST #create' do
context 'authenticated user' do
before do
sign_in(user)
end
context 'project with repository feature' do
context 'with rights to push to the repository' do
before do
project.add_maintainer(user)
end
context 'valid parameters' do
['config/prometheus/common_metrics.yml', 'ee/config/prometheus/cluster_metrics.yml'].each do |dashboard_template|
context "dashboard template #{dashboard_template}" do
let(:dashboard) { dashboard_template }
it 'delegates commit creation to service' do
allow(controller).to receive(:repository).and_return(repository)
allow(repository).to receive(:find_branch).and_return(branch)
dashboard_attrs = {
commit_message: commit_message,
branch_name: branch_name,
start_branch: 'master',
encoding: 'text',
file_path: '.gitlab/dashboards/custom_dashboard.yml',
file_content: File.read(dashboard)
}
service_instance = instance_double(::Files::CreateService)
expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
expect(service_instance).to receive(:execute).and_return(status: :success)
post :create, params: params
end
end
end
it 'extends dashboard template path to absolute url' do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success }))
allow(controller).to receive(:repository).and_return(repository)
allow(repository).to receive(:find_branch).and_return(branch)
expect(File).to receive(:read).with(Rails.root.join('config/prometheus/common_metrics.yml')).and_return('')
post :create, params: params
end
context 'selected branch already exists' do
it 'responds with :created status code', :aggregate_failures do
repository.add_branch(user, branch_name, 'master')
post :create, params: params
expect(response).to have_gitlab_http_status :created
end
end
context 'request format json' do
it 'returns path to new file' do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success }))
allow(controller).to receive(:repository).and_return(repository)
expect(repository).to receive(:find_branch).with(branch_name).and_return(branch)
post :create, params: params
expect(response).to have_gitlab_http_status :created
expect(json_response).to eq('redirect_to' => "/-/ide/project/#{namespace.path}/#{project.name}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}")
end
context 'files create service failure' do
it 'returns json with failure message' do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: false, message: 'something went wrong' }))
post :create, params: params
expect(response).to have_gitlab_http_status :bad_request
expect(response).to set_flash[:alert].to eq('something went wrong')
expect(json_response).to eq('error' => 'something went wrong')
end
end
end
context 'request format html' do
before do
params.delete(:format)
end
it 'redirects to ide with new file' do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success }))
allow(controller).to receive(:repository).and_return(repository)
expect(repository).to receive(:find_branch).with(branch_name).and_return(branch)
post :create, params: params
expect(response).to redirect_to "/-/ide/project/#{namespace.path}/#{project.name}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}"
end
context 'files create service failure' do
it 'redirects back and sets alert' do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: false, message: 'something went wrong' }))
post :create, params: params
expect(response).to set_flash[:alert].to eq('something went wrong')
expect(response).to redirect_to namespace_project_environments_path
end
end
end
end
context 'invalid dashboard template' do
let(:dashboard) { 'config/database.yml' }
it 'responds 404 not found' do
post :create, params: params
expect(response).to have_gitlab_http_status :not_found
end
end
context 'missing commit message' do
before do
params.delete(:commit_message)
end
it 'use default commit message' do
allow(controller).to receive(:repository).and_return(repository)
allow(repository).to receive(:find_branch).and_return(branch)
dashboard_attrs = {
commit_message: 'Create custom dashboard custom_dashboard.yml',
branch_name: branch_name,
start_branch: 'master',
encoding: 'text',
file_path: ".gitlab/dashboards/custom_dashboard.yml",
file_content: File.read('config/prometheus/common_metrics.yml')
}
service_instance = instance_double(::Files::CreateService)
expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
expect(service_instance).to receive(:execute).and_return(status: :success)
post :create, params: params
end
end
context 'missing branch' do
let(:branch_name) { nil }
it 'raises ActionController::ParameterMissing' do
expect { post :create, params: params }.to raise_error ActionController::ParameterMissing
end
end
end
context 'without rights to push to repository' do
before do
project.add_guest(user)
end
it 'responds with :forbidden status code' do
post :create, params: params
expect(response).to have_gitlab_http_status :forbidden
end
end
end
context 'project without repository feature' do
let!(:project) { create(:project, name: 'dashboard-project', namespace: namespace) }
it 'responds with :not_found status code' do
post :create, params: params
expect(response).to have_gitlab_http_status :not_found
end
end
end
end
end
ee/spec/services/metrics/dashboard/clone_dashboard_service_spec.rb 0 → 100644
View file @ 3b7dd732
# frozen_string_literal: true
require 'spec_helper'
describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memory_store_caching do
set(:user) { create(:user) }
set(:project) { create(:project, :repository) }
set(:environment) { create(:environment, project: project) }
describe '#execute' do
context 'with rights to push to the repository' do
before do
project.add_maintainer(user)
end
context 'valid parameters' do
let(:commit_message) { 'test' }
let(:branch) { "#{Time.current.to_i}_dashboard_new_branch" }
let(:file_name) { 'custom_dashboard.yml' }
[::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH, ::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH].each do |dashboard_template|
context "dashboard template #{dashboard_template}" do
let(:dashboard) { dashboard_template }
let(:params) do
{
dashboard: dashboard,
file_name: file_name,
commit_message: commit_message,
branch: branch
}
end
it 'delegates commit creation to Files::CreateService', :aggregate_failures do
dashboard_attrs = {
commit_message: commit_message,
branch_name: branch,
start_branch: 'master',
encoding: 'text',
file_path: '.gitlab/dashboards/custom_dashboard.yml',
file_content: File.read(dashboard)
}
service_instance = instance_double(::Files::CreateService)
expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
expect(service_instance).to receive(:execute).and_return(status: :success)
described_class.new(project, user, params).execute
end
end
end
end
end
end
end
locale/gitlab.pot
View file @ 3b7dd732
...@@ -742,6 +742,9 @@ msgstr "" ...@@ -742,6 +742,9 @@ msgstr ""
msgid "A deleted user" msgid "A deleted user"
msgstr "" msgstr ""
msgid "A file with '%{file_name}' already exists in %{branch} branch"
msgstr ""
msgid "A fork is a copy of a project.<br />Forking a repository allows you to make changes without affecting the original project." msgid "A fork is a copy of a project.<br />Forking a repository allows you to make changes without affecting the original project."
msgstr "" msgstr ""
...@@ -15542,6 +15545,9 @@ msgstr "" ...@@ -15542,6 +15545,9 @@ msgstr ""
msgid "Request Access" msgid "Request Access"
msgstr "" msgstr ""
msgid "Request parameter %{param} is missing."
msgstr ""
msgid "Request to link SAML account must be authorized" msgid "Request to link SAML account must be authorized"
msgstr "" msgstr ""
...@@ -18289,6 +18295,9 @@ msgstr "" ...@@ -18289,6 +18295,9 @@ msgstr ""
msgid "The file has been successfully deleted." msgid "The file has been successfully deleted."
msgstr "" msgstr ""
msgid "The file name should have a .yml extension"
msgstr ""
msgid "The following items will NOT be exported:" msgid "The following items will NOT be exported:"
msgstr "" msgstr ""
...@@ -18600,6 +18609,12 @@ msgstr "" ...@@ -18600,6 +18609,12 @@ msgstr ""
msgid "There was an error adding a To Do." msgid "There was an error adding a To Do."
msgstr "" msgstr ""
msgid "There was an error creating the dashboard, branch name is invalid."
msgstr ""
msgid "There was an error creating the dashboard, branch named: %{branch} already exists."
msgstr ""
msgid "There was an error creating the issue" msgid "There was an error creating the issue"
msgstr "" msgstr ""
...@@ -21167,6 +21182,9 @@ msgstr "" ...@@ -21167,6 +21182,9 @@ msgstr ""
msgid "You can try again using %{begin_link}basic search%{end_link}" msgid "You can try again using %{begin_link}basic search%{end_link}"
msgstr "" msgstr ""
msgid "You can't commit to this project"
msgstr ""
msgid "You cannot access the raw file. Please wait a minute." msgid "You cannot access the raw file. Please wait a minute."
msgstr "" msgstr ""
...@@ -21494,6 +21512,9 @@ msgstr "" ...@@ -21494,6 +21512,9 @@ msgstr ""
msgid "Your comment could not be updated! Please check your network connection and try again." msgid "Your comment could not be updated! Please check your network connection and try again."
msgstr "" msgstr ""
msgid "Your dashboard has been copied. You can %{web_ide_link_start}edit it here%{web_ide_link_end}."
msgstr ""
msgid "Your deployment services will be broken, you will need to manually fix the services after renaming." msgid "Your deployment services will be broken, you will need to manually fix the services after renaming."
msgstr "" msgstr ""
......
spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb
View file @ 3b7dd732
...@@ -37,144 +37,72 @@ describe Projects::PerformanceMonitoring::DashboardsController do ...@@ -37,144 +37,72 @@ describe Projects::PerformanceMonitoring::DashboardsController do
end end
context 'valid parameters' do context 'valid parameters' do
it 'delegates commit creation to service' do it 'delegates cloning to ::Metrics::Dashboard::CloneDashboardService' do
allow(controller).to receive(:repository).and_return(repository) allow(controller).to receive(:repository).and_return(repository)
allow(repository).to receive(:find_branch).and_return(branch) allow(repository).to receive(:find_branch).and_return(branch)
dashboard_attrs = { dashboard_attrs = {
dashboard: dashboard,
file_name: file_name,
commit_message: commit_message, commit_message: commit_message,
branch_name: branch_name, branch: branch_name
start_branch: 'master',
encoding: 'text',
file_path: '.gitlab/dashboards/custom_dashboard.yml',
file_content: File.read('config/prometheus/common_metrics.yml')
} }
service_instance = instance_double(::Files::CreateService) service_instance = instance_double(::Metrics::Dashboard::CloneDashboardService)
expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance) expect(::Metrics::Dashboard::CloneDashboardService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
expect(service_instance).to receive(:execute).and_return(status: :success) expect(service_instance).to receive(:execute).and_return(status: :success, http_status: :created, dashboard: { path: 'dashboard/path' })
post :create, params: params post :create, params: params
end end
it 'extends dashboard template path to absolute url' do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success }))
allow(controller).to receive(:repository).and_return(repository)
allow(repository).to receive(:find_branch).and_return(branch)
expect(File).to receive(:read).with(Rails.root.join('config/prometheus/common_metrics.yml')).and_return('')
post :create, params: params
end
context 'selected branch already exists' do
it 'responds with :created status code', :aggregate_failures do
repository.add_branch(user, branch_name, 'master')
post :create, params: params
expect(response).to have_gitlab_http_status :created
end
end
context 'request format json' do context 'request format json' do
it 'returns path to new file' do it 'returns services response' do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success })) allow(::Metrics::Dashboard::CloneDashboardService).to receive(:new).and_return(double(execute: { status: :success, dashboard: { path: ".gitlab/dashboards/#{file_name}" }, http_status: :created }))
allow(controller).to receive(:repository).and_return(repository) allow(controller).to receive(:repository).and_return(repository)
allow(repository).to receive(:find_branch).and_return(branch)
expect(repository).to receive(:find_branch).with(branch_name).and_return(branch)
post :create, params: params post :create, params: params
expect(response).to have_gitlab_http_status :created expect(response).to have_gitlab_http_status :created
expect(json_response).to eq('redirect_to' => "/-/ide/project/#{namespace.path}/#{project.name}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}") expect(response).to set_flash[:notice].to eq("Your dashboard has been copied. You can <a href=\"/-/ide/project/#{namespace.path}/#{project.name}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}\">edit it here</a>.")
expect(json_response).to eq('status' => 'success', 'dashboard' => { 'path' => ".gitlab/dashboards/#{file_name}" })
end end
context 'files create service failure' do context 'Metrics::Dashboard::CloneDashboardService failure' do
it 'returns json with failure message' do it 'returns json with failure message', :aggregate_failures do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: false, message: 'something went wrong' })) allow(::Metrics::Dashboard::CloneDashboardService).to receive(:new).and_return(double(execute: { status: :error, message: 'something went wrong', http_status: :bad_request }))
post :create, params: params post :create, params: params
expect(response).to have_gitlab_http_status :bad_request expect(response).to have_gitlab_http_status :bad_request
expect(response).to set_flash[:alert].to eq('something went wrong')
expect(json_response).to eq('error' => 'something went wrong') expect(json_response).to eq('error' => 'something went wrong')
end end
end end
end
context 'request format html' do %w(commit_message file_name dashboard).each do |param|
before do context "param #{param} is missing" do
params.delete(:format) let(param.to_s) { nil }
end
it 'redirects to ide with new file' do it 'responds with bad request status and error message', :aggregate_failures do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success })) post :create, params: params
allow(controller).to receive(:repository).and_return(repository)
expect(repository).to receive(:find_branch).with(branch_name).and_return(branch) expect(response).to have_gitlab_http_status :bad_request
expect(json_response).to eq('error' => "Request parameter #{param} is missing.")
post :create, params: params end
end
expect(response).to redirect_to "/-/ide/project/#{namespace.path}/#{project.name}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}"
end end
context 'files create service failure' do context "param branch_name is missing" do
it 'redirects back and sets alert' do let(:branch_name) { nil }
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: false, message: 'something went wrong' }))
allow(controller).to receive(:repository).and_return(repository)
allow(repository).to receive(:find_branch).and_return(branch)
it 'responds with bad request status and error message', :aggregate_failures do
post :create, params: params post :create, params: params
expect(response).to set_flash[:alert].to eq('something went wrong') expect(response).to have_gitlab_http_status :bad_request
expect(response).to redirect_to namespace_project_environments_path expect(json_response).to eq('error' => "Request parameter branch is missing.")
end end
end end
end end
end end
context 'invalid dashboard template' do
let(:dashboard) { 'config/database.yml' }
it 'responds 404 not found' do
post :create, params: params
expect(response).to have_gitlab_http_status :not_found
end
end
context 'missing commit message' do
before do
params.delete(:commit_message)
end
it 'use default commit message' do
allow(controller).to receive(:repository).and_return(repository)
allow(repository).to receive(:find_branch).and_return(branch)
dashboard_attrs = {
commit_message: 'Create custom dashboard custom_dashboard.yml',
branch_name: branch_name,
start_branch: 'master',
encoding: 'text',
file_path: ".gitlab/dashboards/custom_dashboard.yml",
file_content: File.read('config/prometheus/common_metrics.yml')
}
service_instance = instance_double(::Files::CreateService)
expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
expect(service_instance).to receive(:execute).and_return(status: :success)
post :create, params: params
end
end
context 'missing branch' do
let(:branch_name) { nil }
it 'raises ActionController::ParameterMissing' do
expect { post :create, params: params }.to raise_error ActionController::ParameterMissing
end
end
end end
context 'without rights to push to repository' do context 'without rights to push to repository' do
......
spec/helpers/environments_helper_spec.rb
View file @ 3b7dd732
...@@ -3,9 +3,9 @@ ...@@ -3,9 +3,9 @@
require 'spec_helper' require 'spec_helper'
describe EnvironmentsHelper do describe EnvironmentsHelper do
set(:environment) { create(:environment) }
set(:project) { environment.project }
set(:user) { create(:user) } set(:user) { create(:user) }
set(:project) { create(:project, :repository) }
set(:environment) { create(:environment, project: project) }
describe '#metrics_data' do describe '#metrics_data' do
before do before do
...@@ -28,6 +28,7 @@ describe EnvironmentsHelper do ...@@ -28,6 +28,7 @@ describe EnvironmentsHelper do
'empty-unable-to-connect-svg-path' => match_asset_path('/assets/illustrations/monitoring/unable_to_connect.svg'), 'empty-unable-to-connect-svg-path' => match_asset_path('/assets/illustrations/monitoring/unable_to_connect.svg'),
'metrics-endpoint' => additional_metrics_project_environment_path(project, environment, format: :json), 'metrics-endpoint' => additional_metrics_project_environment_path(project, environment, format: :json),
'deployments-endpoint' => project_environment_deployments_path(project, environment, format: :json), 'deployments-endpoint' => project_environment_deployments_path(project, environment, format: :json),
'default-branch' => 'master',
'environments-endpoint': project_environments_path(project, format: :json), 'environments-endpoint': project_environments_path(project, format: :json),
'project-path' => project_path(project), 'project-path' => project_path(project),
'tags-path' => project_tags_path(project), 'tags-path' => project_tags_path(project),
......
spec/services/metrics/dashboard/clone_dashboard_service_spec.rb 0 → 100644
View file @ 3b7dd732
# frozen_string_literal: true
require 'spec_helper'
describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memory_store_caching do
include MetricsDashboardHelpers
set(:user) { create(:user) }
set(:project) { create(:project, :repository) }
set(:environment) { create(:environment, project: project) }
describe '#execute' do
subject(:service_call) { described_class.new(project, user, params).execute }
let(:commit_message) { 'test' }
let(:branch) { "dashboard_new_branch" }
let(:dashboard) { 'config/prometheus/common_metrics.yml' }
let(:file_name) { 'custom_dashboard.yml' }
let(:params) do
{
dashboard: dashboard,
file_name: file_name,
commit_message: commit_message,
branch: branch
}
end
let(:dashboard_attrs) do
{
commit_message: commit_message,
branch_name: branch,
start_branch: project.default_branch,
encoding: 'text',
file_path: ".gitlab/dashboards/#{file_name}",
file_content: File.read(dashboard)
}
end
context 'user does not have push right to repository' do
it_behaves_like 'misconfigured dashboard service response', :forbidden, %q(You can't commit to this project)
end
context 'with rights to push to the repository' do
before do
project.add_maintainer(user)
end
context 'wrong target file extension' do
let(:file_name) { 'custom_dashboard.txt' }
it_behaves_like 'misconfigured dashboard service response', :bad_request, 'The file name should have a .yml extension'
end
context 'wrong source dashboard file' do
let(:dashboard) { 'config/prometheus/common_metrics_123.yml' }
it_behaves_like 'misconfigured dashboard service response', :not_found, 'Not found.'
end
context 'path traversal attack attempt' do
let(:dashboard) { 'config/prometheus/../database.yml' }
it_behaves_like 'misconfigured dashboard service response', :not_found, 'Not found.'
end
context 'path traversal attack attempt on target file' do
let(:file_name) { '../../custom_dashboard.yml' }
let(:dashboard_attrs) do
{
commit_message: commit_message,
branch_name: branch,
start_branch: project.default_branch,
encoding: 'text',
file_path: ".gitlab/dashboards/custom_dashboard.yml",
file_content: File.read(dashboard)
}
end
it 'strips target file name to safe value', :aggregate_failures do
service_instance = instance_double(::Files::CreateService)
expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
expect(service_instance).to receive(:execute).and_return(status: :success)
service_call
end
end
context 'valid parameters' do
it 'delegates commit creation to Files::CreateService', :aggregate_failures do
service_instance = instance_double(::Files::CreateService)
expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
expect(service_instance).to receive(:execute).and_return(status: :success)
service_call
end
context 'selected branch already exists' do
let(:branch) { 'existing_branch' }
before do
project.repository.add_branch(user, branch, 'master')
end
it_behaves_like 'misconfigured dashboard service response', :bad_request, "There was an error creating the dashboard, branch named: existing_branch already exists."
# temporary not available function for first iteration
# follow up issue https://gitlab.com/gitlab-org/gitlab/issues/196237 which
# require this feature
# it 'pass correct params to Files::CreateService', :aggregate_failures do
# project.repository.add_branch(user, branch, 'master')
#
# service_instance = instance_double(::Files::CreateService)
# expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance)
# expect(service_instance).to receive(:execute).and_return(status: :success)
#
# service_call
# end
end
context 'blank branch name' do
let(:branch) { '' }
it_behaves_like 'misconfigured dashboard service response', :bad_request, 'There was an error creating the dashboard, branch name is invalid.'
end
context 'dashboard file already exists' do
let(:branch) { 'custom_dashboard' }
before do
Files::CreateService.new(
project,
user,
commit_message: 'Create custom dashboard custom_dashboard.yml',
branch_name: 'master',
start_branch: 'master',
file_path: ".gitlab/dashboards/custom_dashboard.yml",
file_content: File.read('config/prometheus/common_metrics.yml')
).execute
end
it_behaves_like 'misconfigured dashboard service response', :bad_request, "A file with 'custom_dashboard.yml' already exists in custom_dashboard branch"
end
it 'extends dashboard template path to absolute url' do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success }))
expect(File).to receive(:read).with(Rails.root.join('config/prometheus/common_metrics.yml')).and_return('')
service_call
end
context 'Files::CreateService success' do
before do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success }))
end
it 'clears dashboards cache' do
expect(project.repository).to receive(:refresh_method_caches).with([:metrics_dashboard])
service_call
end
it 'returns success', :aggregate_failures do
result = service_call
dashboard_details = {
path: '.gitlab/dashboards/custom_dashboard.yml',
display_name: 'custom_dashboard.yml',
default: false,
system_dashboard: false
}
expect(result[:status]).to be :success
expect(result[:http_status]).to be :created
expect(result[:dashboard]).to match dashboard_details
end
end
context 'Files::CreateService fails' do
before do
allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :error }))
end
it 'does NOT clear dashboards cache' do
expect(project.repository).not_to receive(:refresh_method_caches)
service_call
end
it 'returns error' do
result = service_call
expect(result[:status]).to be :error
end
end
end
end
end
end
spec/support/helpers/metrics_dashboard_helpers.rb
View file @ 3b7dd732
...@@ -29,54 +29,4 @@ module MetricsDashboardHelpers ...@@ -29,54 +29,4 @@ module MetricsDashboardHelpers
def business_metric_title def business_metric_title
PrometheusMetricEnums.group_details[:business][:group_title] PrometheusMetricEnums.group_details[:business][:group_title]
end end
shared_examples_for 'misconfigured dashboard service response' do |status_code|
it 'returns an appropriate message and status code' do
result = service_call
expect(result.keys).to contain_exactly(:message, :http_status, :status)
expect(result[:status]).to eq(:error)
expect(result[:http_status]).to eq(status_code)
end
end
shared_examples_for 'valid dashboard service response for schema' do
it 'returns a json representation of the dashboard' do
result = service_call
expect(result.keys).to contain_exactly(:dashboard, :status)
expect(result[:status]).to eq(:success)
expect(JSON::Validator.fully_validate(dashboard_schema, result[:dashboard])).to be_empty
end
end
shared_examples_for 'valid dashboard service response' do
let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/dashboard.json')) }
it_behaves_like 'valid dashboard service response for schema'
end
shared_examples_for 'caches the unprocessed dashboard for subsequent calls' do
it do
expect(YAML).to receive(:safe_load).once.and_call_original
described_class.new(*service_params).get_dashboard
described_class.new(*service_params).get_dashboard
end
end
shared_examples_for 'valid embedded dashboard service response' do
let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/embedded_dashboard.json')) }
it_behaves_like 'valid dashboard service response for schema'
end
shared_examples_for 'raises error for users with insufficient permissions' do
context 'when the user does not have sufficient access' do
let(:user) { build(:user) }
it_behaves_like 'misconfigured dashboard service response', :unauthorized
end
end
end end
spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb 0 → 100644
View file @ 3b7dd732
# frozen_string_literal: true
shared_examples_for 'misconfigured dashboard service response' do |status_code, message = nil|
it 'returns an appropriate message and status code', :aggregate_failures do
result = service_call
expect(result.keys).to contain_exactly(:message, :http_status, :status)
expect(result[:status]).to eq(:error)
expect(result[:http_status]).to eq(status_code)
expect(result[:message]).to eq(message) if message
end
end
shared_examples_for 'valid dashboard service response for schema' do
it 'returns a json representation of the dashboard' do
result = service_call
expect(result.keys).to contain_exactly(:dashboard, :status)
expect(result[:status]).to eq(:success)
expect(JSON::Validator.fully_validate(dashboard_schema, result[:dashboard])).to be_empty
end
end
shared_examples_for 'valid dashboard service response' do
let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/dashboard.json')) }
it_behaves_like 'valid dashboard service response for schema'
end
shared_examples_for 'caches the unprocessed dashboard for subsequent calls' do
it do
expect(YAML).to receive(:safe_load).once.and_call_original
described_class.new(*service_params).get_dashboard
described_class.new(*service_params).get_dashboard
end
end
shared_examples_for 'valid embedded dashboard service response' do
let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/embedded_dashboard.json')) }
it_behaves_like 'valid dashboard service response for schema'
end
shared_examples_for 'raises error for users with insufficient permissions' do
context 'when the user does not have sufficient access' do
let(:user) { build(:user) }
it_behaves_like 'misconfigured dashboard service response', :unauthorized
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7