Skip to content
Snippets Groups Projects
Unverified Commit 14420903 authored by Yorick Peterse's avatar Yorick Peterse
Browse files

Validate deployment SHAs and refs

This prevents users from creating deployments using the API while
providing an invalid SHA or ref name. Providing invalid data for these
fields will break certain parts of GitLab, such as the UI used for
displaying the deployments of an environment.

These changes require that we correct a variety of tests that were using
deployments without a valid repository, or were even testing behaviour
that would never run due to the lack of a repository.

This fixes https://gitlab.com/gitlab-org/gitlab/issues/36967
parent a55eed83
No related branches found
No related tags found
No related merge requests found
Showing
with 74 additions and 43 deletions
Loading
Loading
@@ -23,6 +23,8 @@ class Deployment < ApplicationRecord
 
validates :sha, presence: true
validates :ref, presence: true
validate :valid_sha, on: :create
validate :valid_ref, on: :create
 
delegate :name, to: :environment, prefix: true
 
Loading
Loading
@@ -234,6 +236,18 @@ def update_status(status)
end
end
 
def valid_sha
return if project&.commit(sha)
errors.add(:sha, _('The commit does not exist'))
end
def valid_ref
return if project&.commit(ref)
errors.add(:ref, _('The branch or tag does not exist'))
end
private
 
def ref_path
Loading
Loading
---
title: Validate deployment SHAs and refs
merge_request:
author:
type: fixed
Loading
Loading
@@ -105,7 +105,7 @@
let(:project) { create(:project, :repository) }
let(:commit) { project.commit }
let!(:environment) { create(:environment, name: 'production', project: project) }
let!(:deployment) { create(:deployment, :success, environment: environment, sha: commit.id, created_at: now) }
let!(:deployment) { create(:deployment, :success, environment: environment, sha: commit.id, created_at: now, project: project) }
 
it_behaves_like 'unlicensed', :get, :list
 
Loading
Loading
@@ -276,7 +276,7 @@
end
 
context 'with a project in the dashboard' do
let(:project) { create(:project, :with_avatar) }
let(:project) { create(:project, :with_avatar, :repository) }
 
before do
project.add_developer(user)
Loading
Loading
Loading
Loading
@@ -5,7 +5,7 @@
describe Environment, :use_clean_rails_memory_store_caching do
include ReactiveCachingHelpers
 
let(:project) { create(:project, :stubbed_repository) }
let(:project) { create(:project, :repository) }
let(:environment) { create(:environment, project: project) }
 
describe '.deployed_to_cluster' do
Loading
Loading
@@ -211,10 +211,9 @@
end
 
describe '#rollout_status' do
let(:cluster) { create(:cluster, :project, :provided_by_user) }
let(:project) { cluster.project }
let(:environment) { create(:environment, project: project) }
let!(:deployment) { create(:deployment, :success, environment: environment) }
let!(:cluster) { create(:cluster, :project, :provided_by_user, projects: [project]) }
let!(:environment) { create(:environment, project: project) }
let!(:deployment) { create(:deployment, :success, environment: environment, project: project) }
 
subject { environment.rollout_status }
 
Loading
Loading
Loading
Loading
@@ -6,7 +6,7 @@
 
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:pipeline) { create(:ci_empty_pipeline, project: project) }
let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: 'b83d6e391c22777fca1ed3012fce84f633d7fed0') }
 
describe '#update_build?' do
let(:environment) { create(:environment, project: project, name: 'production') }
Loading
Loading
Loading
Loading
@@ -104,7 +104,6 @@
end
 
describe 'PUT /projects/:id/deployments/:deployment_id' do
let(:project) { create(:project) }
let(:deploy) do
create(
:deployment,
Loading
Loading
Loading
Loading
@@ -6,7 +6,7 @@
describe '#execute' do
def setup
user = create(:user)
project = create(:project)
project = create(:project, :repository)
project.add_developer(user)
user.update!(ops_dashboard_projects: [project])
 
Loading
Loading
Loading
Loading
@@ -14,7 +14,7 @@
 
let(:client) { double('prometheus_client') }
let(:query_result) { described_class.new(client).query(*query_params) }
let(:project) { create(:project) }
let(:project) { create(:project, :repository) }
let(:environment) { create(:environment, slug: 'environment-slug', project: project) }
 
before do
Loading
Loading
Loading
Loading
@@ -17852,6 +17852,9 @@ msgstr ""
msgid "The branch for this project has no active pipeline configuration."
msgstr ""
 
msgid "The branch or tag does not exist"
msgstr ""
msgid "The character highlighter helps you keep the subject line to %{titleLength} characters and wrap the body at %{bodyLength} so they are readable in git."
msgstr ""
 
Loading
Loading
@@ -17861,6 +17864,9 @@ msgstr ""
msgid "The collection of events added to the data gathered for that stage."
msgstr ""
 
msgid "The commit does not exist"
msgstr ""
msgid "The configuration status of the table below only applies to the default branch and is based on the %{linkStart}latest pipeline%{linkEnd}. Once you've configured a scan for the default branch, any subsequent feature branch you create will include the scan."
msgstr ""
 
Loading
Loading
Loading
Loading
@@ -6,7 +6,7 @@
include ApiHelpers
 
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:project) { create(:project, :repository) }
let(:environment) { create(:environment, name: 'production', project: project) }
 
before do
Loading
Loading
Loading
Loading
@@ -7,9 +7,9 @@
include ReactiveCachingHelpers
 
let(:user) { create(:user) }
let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
let(:project) { create(:project, :repository) }
let(:cluster) { create(:cluster, :project, :provided_by_gcp, projects: [project]) }
let(:service) { cluster.platform_kubernetes }
let(:project) { cluster.project }
let(:environment) { create(:environment, project: project) }
let!(:deployment) { create(:deployment, :success, environment: environment, cluster: cluster) }
let(:knative_services_finder) { environment.knative_services_finder }
Loading
Loading
Loading
Loading
@@ -33,10 +33,10 @@
end
 
context 'when a user created a new merge request with the same SHA' do
let(:pipeline2) { create(:ci_pipeline, sha: sha, project: project, ref: 'new-patch-1') }
let(:pipeline2) { create(:ci_pipeline, sha: sha, project: project, ref: 'video') }
let(:build2) { create(:ci_build, :success, pipeline: pipeline2) }
let(:environment2) { create(:environment, project: project) }
let!(:deployment2) { create(:deployment, environment: environment2, sha: sha, ref: 'new-patch-1', deployable: build2) }
let!(:deployment2) { create(:deployment, environment: environment2, sha: sha, ref: 'video', deployable: build2) }
 
it 'displays one environment which is related to the pipeline' do
visit project_merge_request_path(project, merge_request)
Loading
Loading
Loading
Loading
@@ -6,7 +6,7 @@
include PrometheusHelpers
 
let(:user) { create(:user) }
let(:project) { create(:prometheus_project) }
let(:project) { create(:prometheus_project, :repository) }
let(:pipeline) { create(:ci_pipeline, project: project) }
let(:build) { create(:ci_build, pipeline: pipeline) }
let(:environment) { create(:environment, project: project) }
Loading
Loading
Loading
Loading
@@ -3,7 +3,7 @@
require 'spec_helper'
 
describe 'Environment' do
let(:project) { create(:project) }
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:role) { :developer }
 
Loading
Loading
@@ -17,6 +17,7 @@
let!(:permissions) { }
let!(:deployment) { }
let!(:action) { }
let!(:cluster) { }
 
before do
visit_environment(environment)
Loading
Loading
@@ -94,19 +95,10 @@
 
it 'does show build name' do
expect(page).to have_link("#{build.name} (##{build.id})")
expect(page).not_to have_link('Re-deploy')
expect(page).not_to have_terminal_button
end
 
context 'when user has ability to re-deploy' do
let(:permissions) do
create(:protected_branch, :developers_can_merge,
name: build.ref, project: project)
end
it 'does show re-deploy' do
expect(page).to have_link('Re-deploy')
end
it 'shows the re-deploy button' do
expect(page).to have_button('Re-deploy to environment')
end
 
context 'with manual action' do
Loading
Loading
@@ -141,6 +133,11 @@
end
 
context 'when user has no ability to trigger a deployment' do
let(:permissions) do
create(:protected_branch, :no_one_can_merge,
name: action.ref, project: project)
end
it 'does not show a play button' do
expect(page).not_to have_link(action.name)
end
Loading
Loading
@@ -158,8 +155,9 @@
 
context 'with terminal' do
context 'when user configured kubernetes from CI/CD > Clusters' do
let!(:cluster) { create(:cluster, :project, :provided_by_gcp) }
let(:project) { cluster.project }
let!(:cluster) do
create(:cluster, :project, :provided_by_gcp, projects: [project])
end
 
context 'for project maintainer' do
let(:role) { :maintainer }
Loading
Loading
@@ -228,6 +226,11 @@
end
 
context 'when user has no ability to stop environment' do
let(:permissions) do
create(:protected_branch, :no_one_can_merge,
name: action.ref, project: project)
end
it 'does not allow to stop environment' do
expect(page).not_to have_button('Stop')
end
Loading
Loading
Loading
Loading
@@ -6,7 +6,7 @@
include KubernetesHelpers
include ReactiveCachingHelpers
 
let(:project) { create(:project) }
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
 
before do
Loading
Loading
@@ -36,9 +36,8 @@
end
 
context 'when the user has a cluster and knative installed and visits the serverless page' do
let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
let(:cluster) { create(:cluster, :project, :provided_by_gcp, projects: [project]) }
let(:service) { cluster.platform_kubernetes }
let(:project) { cluster.project }
let(:environment) { create(:environment, project: project) }
let!(:deployment) { create(:deployment, :success, cluster: cluster, environment: environment) }
let(:knative_services_finder) { environment.knative_services_finder }
Loading
Loading
Loading
Loading
@@ -6,9 +6,9 @@
include KubernetesHelpers
include ReactiveCachingHelpers
 
let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
let(:project) { create(:project, :repository) }
let(:cluster) { create(:cluster, :project, :provided_by_gcp, projects: [project]) }
let(:service) { environment.deployment_platform }
let(:project) { cluster.cluster_project.project }
let(:environment) { create(:environment, project: project) }
let!(:deployment) { create(:deployment, :success, environment: environment, cluster: cluster) }
let(:namespace) do
Loading
Loading
Loading
Loading
@@ -5,7 +5,7 @@
describe DeploymentsFinder do
subject { described_class.new(project, params).execute }
 
let(:project) { create(:project, :public, :repository) }
let(:project) { create(:project, :public, :test_repo) }
let(:params) { {} }
 
describe "#execute" do
Loading
Loading
@@ -34,7 +34,7 @@
 
let!(:deployment_1) { create(:deployment, :success, project: project, iid: 11, ref: 'master', created_at: 2.days.ago, updated_at: Time.now) }
let!(:deployment_2) { create(:deployment, :success, project: project, iid: 12, ref: 'feature', created_at: 1.day.ago, updated_at: 2.hours.ago) }
let!(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'patch', created_at: Time.now, updated_at: 1.hour.ago) }
let!(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'video', created_at: Time.now, updated_at: 1.hour.ago) }
 
where(:order_by, :sort, :ordered_deployments) do
'created_at' | 'asc' | [:deployment_1, :deployment_2, :deployment_3]
Loading
Loading
Loading
Loading
@@ -8,9 +8,9 @@
include ReactiveCachingHelpers
 
let(:user) { create(:user) }
let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
let(:project) { create(:project, :repository) }
let(:cluster) { create(:cluster, :project, :provided_by_gcp, projects: [project]) }
let(:service) { cluster.platform_kubernetes }
let(:project) { cluster.project }
let(:environment) { create(:environment, project: project) }
let!(:deployment) { create(:deployment, :success, environment: environment, cluster: cluster) }
let(:knative_services_finder) { environment.knative_services_finder }
Loading
Loading
Loading
Loading
@@ -3,8 +3,13 @@
require 'spec_helper'
 
describe Gitlab::Ci::Pipeline::Seed::Deployment do
let_it_be(:project) { create(:project) }
let(:job) { build(:ci_build, project: project) }
let_it_be(:project) { create(:project, :repository) }
let(:pipeline) do
create(:ci_pipeline, project: project,
sha: 'b83d6e391c22777fca1ed3012fce84f633d7fed0')
end
let(:job) { build(:ci_build, project: project, pipeline: pipeline) }
let(:seed) { described_class.new(job) }
let(:attributes) { {} }
 
Loading
Loading
Loading
Loading
@@ -8,7 +8,8 @@
end
 
include_examples 'additional metrics query' do
let(:deployment) { create(:deployment, environment: environment) }
let(:project) { create(:project, :repository) }
let(:deployment) { create(:deployment, environment: environment, project: project) }
let(:query_params) { [deployment.id] }
 
it 'queries using specific time' do
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment