diff options
23 files changed, 259 insertions, 83 deletions
diff --git a/app/models/clusters/applications/knative.rb b/app/models/clusters/applications/knative.rb index c15d9edf7b2..89ffb72a22f 100644 --- a/app/models/clusters/applications/knative.rb +++ b/app/models/clusters/applications/knative.rb @@ -11,6 +11,8 @@ module Clusters self.table_name = 'clusters_applications_knative' + has_one :serverless_domain_cluster, class_name: 'Serverless::DomainCluster', foreign_key: 'clusters_applications_knative_id', inverse_of: :knative + include ::Clusters::Concerns::ApplicationCore include ::Clusters::Concerns::ApplicationStatus include ::Clusters::Concerns::ApplicationVersion diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 65f5cbf69c6..994e69912b6 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -217,6 +217,23 @@ class Deployment < ApplicationRecord SQL end + # Changes the status of a deployment and triggers the correspinding state + # machine events. + def update_status(status) + case status + when 'running' + run + when 'success' + succeed + when 'failed' + drop + when 'canceled' + cancel + else + raise ArgumentError, "The status #{status.inspect} is invalid" + end + end + private def ref_path diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb index 3869d86b667..dd2cafd9a35 100644 --- a/app/models/pages_domain.rb +++ b/app/models/pages_domain.rb @@ -6,6 +6,7 @@ class PagesDomain < ApplicationRecord SSL_RENEWAL_THRESHOLD = 30.days.freeze enum certificate_source: { user_provided: 0, gitlab_provided: 1 }, _prefix: :certificate + enum domain_type: { instance: 0, group: 1, project: 2 }, _prefix: :domain_type belongs_to :project has_many :acme_orders, class_name: "PagesDomainAcmeOrder" @@ -25,6 +26,8 @@ class PagesDomain < ApplicationRecord validate :validate_intermediates, if: ->(domain) { domain.certificate.present? && domain.certificate_changed? } default_value_for(:auto_ssl_enabled, allow_nil: false) { ::Gitlab::LetsEncrypt.enabled? } + default_value_for :domain_type, allow_nil: false, value: :project + default_value_for :wildcard, allow_nil: false, value: false attr_encrypted :key, mode: :per_attribute_iv_and_salt, @@ -217,6 +220,8 @@ class PagesDomain < ApplicationRecord # rubocop: disable CodeReuse/ServiceClass def update_daemon + return if domain_type_instance? + ::Projects::UpdatePagesConfigurationService.new(project).execute end # rubocop: enable CodeReuse/ServiceClass diff --git a/app/models/serverless/domain_cluster.rb b/app/models/serverless/domain_cluster.rb new file mode 100644 index 00000000000..a8365649dd1 --- /dev/null +++ b/app/models/serverless/domain_cluster.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Serverless + class DomainCluster < ApplicationRecord + self.table_name = 'serverless_domain_cluster' + + belongs_to :pages_domain + belongs_to :knative, class_name: 'Clusters::Applications::Knative', foreign_key: 'clusters_applications_knative_id' + belongs_to :creator, class_name: 'User', optional: true + + validates :pages_domain, :knative, :uuid, presence: true + validates :uuid, uniqueness: true, length: { is: 14 } + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 698848c5b16..b6ed550e441 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -110,6 +110,10 @@ class User < ApplicationRecord through: :group_members, source: :group alias_attribute :masters_groups, :maintainers_groups + has_many :reporter_developer_maintainer_owned_groups, + -> { where(members: { access_level: [Gitlab::Access::REPORTER, Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER, Gitlab::Access::OWNER] }) }, + through: :group_members, + source: :group # Projects has_many :groups_projects, through: :groups, source: :projects diff --git a/app/services/deployments/create_service.rb b/app/services/deployments/create_service.rb index 89e3f7c8b83..7355747d778 100644 --- a/app/services/deployments/create_service.rb +++ b/app/services/deployments/create_service.rb @@ -11,15 +11,17 @@ module Deployments end def execute - create_deployment.tap do |deployment| - AfterCreateService.new(deployment).execute if deployment.persisted? + environment.deployments.build(deployment_attributes).tap do |deployment| + # Deployment#change_status already saves the model, so we only need to + # call #save ourselves if no status is provided. + if (status = params[:status]) + deployment.update_status(status) + else + deployment.save + end end end - def create_deployment - environment.deployments.create(deployment_attributes) - end - def deployment_attributes # We use explicit parameters here so we never by accident allow parameters # to be set that one should not be able to set (e.g. the row ID). @@ -31,8 +33,7 @@ module Deployments tag: params[:tag], sha: params[:sha], user: current_user, - on_stop: params[:on_stop], - status: params[:status] + on_stop: params[:on_stop] } end end diff --git a/app/services/deployments/update_service.rb b/app/services/deployments/update_service.rb index 97b233f16a7..b8f8740c9b9 100644 --- a/app/services/deployments/update_service.rb +++ b/app/services/deployments/update_service.rb @@ -10,22 +10,7 @@ module Deployments end def execute - # A regular update() does not trigger the state machine transitions, which - # we need to ensure merge requests are linked when changing the status to - # success. To work around this we use this case statment, using the right - # event methods to trigger the transition hooks. - case params[:status] - when 'running' - deployment.run - when 'success' - deployment.succeed - when 'failed' - deployment.drop - when 'canceled' - deployment.cancel - else - false - end + deployment.update_status(params[:status]) end end end diff --git a/changelogs/unreleased/35591-instance-and-project-level-ssl-and-custom-domain-support.yml b/changelogs/unreleased/35591-instance-and-project-level-ssl-and-custom-domain-support.yml new file mode 100644 index 00000000000..9242a97ee81 --- /dev/null +++ b/changelogs/unreleased/35591-instance-and-project-level-ssl-and-custom-domain-support.yml @@ -0,0 +1,5 @@ +--- +title: Create data model for serverless domains +merge_request: 19835 +author: +type: added diff --git a/changelogs/unreleased/deployment-finished-at.yml b/changelogs/unreleased/deployment-finished-at.yml new file mode 100644 index 00000000000..214ea2eb765 --- /dev/null +++ b/changelogs/unreleased/deployment-finished-at.yml @@ -0,0 +1,5 @@ +--- +title: Refactor the Deployment model so state machine events are used by both CI and the API +merge_request: 20474 +author: +type: fixed diff --git a/db/migrate/20191127030005_create_serverless_domain_cluster.rb b/db/migrate/20191127030005_create_serverless_domain_cluster.rb new file mode 100644 index 00000000000..7fb24400b0d --- /dev/null +++ b/db/migrate/20191127030005_create_serverless_domain_cluster.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CreateServerlessDomainCluster < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def change + create_table :serverless_domain_cluster, id: false, primary_key: :uuid do |t| + t.references :pages_domain, null: false, foreign_key: { on_delete: :cascade } + t.references :clusters_applications_knative, null: false, + foreign_key: { to_table: :clusters_applications_knative, on_delete: :cascade }, + index: { name: :idx_serverless_domain_cluster_on_clusters_applications_knative, unique: true } + t.references :creator, name: :created_by, foreign_key: { to_table: :users, on_delete: :nullify } + t.timestamps_with_timezone null: false + t.string :uuid, null: false, limit: 14, primary_key: true + end + end +end diff --git a/db/migrate/20191127221608_add_wildcard_and_domain_type_to_pages_domains.rb b/db/migrate/20191127221608_add_wildcard_and_domain_type_to_pages_domains.rb new file mode 100644 index 00000000000..6893a02bcad --- /dev/null +++ b/db/migrate/20191127221608_add_wildcard_and_domain_type_to_pages_domains.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddWildcardAndDomainTypeToPagesDomains < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + PROJECT_TYPE = 2 + + disable_ddl_transaction! + + def up + add_column_with_default :pages_domains, :wildcard, :boolean, default: false + add_column_with_default :pages_domains, :domain_type, :integer, limit: 2, default: PROJECT_TYPE + end + + def down + remove_column :pages_domains, :wildcard + remove_column :pages_domains, :domain_type + end +end diff --git a/db/migrate/20191206022133_add_indexes_to_pages_domains_on_wildcard_and_domain_type.rb b/db/migrate/20191206022133_add_indexes_to_pages_domains_on_wildcard_and_domain_type.rb new file mode 100644 index 00000000000..3c1704a3377 --- /dev/null +++ b/db/migrate/20191206022133_add_indexes_to_pages_domains_on_wildcard_and_domain_type.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddIndexesToPagesDomainsOnWildcardAndDomainType < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :pages_domains, :wildcard + add_concurrent_index :pages_domains, :domain_type + end + + def down + remove_concurrent_index :pages_domains, :wildcard + remove_concurrent_index :pages_domains, :domain_type + end +end diff --git a/db/schema.rb b/db/schema.rb index ac2372a63d0..9ba5fcc41c6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2951,13 +2951,17 @@ ActiveRecord::Schema.define(version: 2019_12_08_071112) do t.datetime_with_timezone "certificate_valid_not_before" t.datetime_with_timezone "certificate_valid_not_after" t.integer "certificate_source", limit: 2, default: 0, null: false + t.boolean "wildcard", default: false, null: false + t.integer "domain_type", limit: 2, default: 2, null: false t.index ["certificate_source", "certificate_valid_not_after"], name: "index_pages_domains_need_auto_ssl_renewal", where: "(auto_ssl_enabled = true)" t.index ["domain"], name: "index_pages_domains_on_domain", unique: true + t.index ["domain_type"], name: "index_pages_domains_on_domain_type" t.index ["project_id", "enabled_until"], name: "index_pages_domains_on_project_id_and_enabled_until" t.index ["project_id"], name: "index_pages_domains_on_project_id" t.index ["remove_at"], name: "index_pages_domains_on_remove_at" t.index ["verified_at", "enabled_until"], name: "index_pages_domains_on_verified_at_and_enabled_until" t.index ["verified_at"], name: "index_pages_domains_on_verified_at" + t.index ["wildcard"], name: "index_pages_domains_on_wildcard" end create_table "path_locks", id: :serial, force: :cascade do |t| @@ -3654,6 +3658,17 @@ ActiveRecord::Schema.define(version: 2019_12_08_071112) do t.index ["issue_id"], name: "index_sentry_issues_on_issue_id", unique: true end + create_table "serverless_domain_cluster", primary_key: "uuid", id: :string, limit: 14, force: :cascade do |t| + t.bigint "pages_domain_id", null: false + t.bigint "clusters_applications_knative_id", null: false + t.bigint "creator_id" + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.index ["clusters_applications_knative_id"], name: "idx_serverless_domain_cluster_on_clusters_applications_knative", unique: true + t.index ["creator_id"], name: "index_serverless_domain_cluster_on_creator_id" + t.index ["pages_domain_id"], name: "index_serverless_domain_cluster_on_pages_domain_id" + end + create_table "service_desk_settings", primary_key: "project_id", id: :bigint, default: nil, force: :cascade do |t| t.string "issue_template_key", limit: 255 end @@ -4714,6 +4729,9 @@ ActiveRecord::Schema.define(version: 2019_12_08_071112) do add_foreign_key "self_managed_prometheus_alert_events", "environments", on_delete: :cascade add_foreign_key "self_managed_prometheus_alert_events", "projects", on_delete: :cascade add_foreign_key "sentry_issues", "issues", on_delete: :cascade + add_foreign_key "serverless_domain_cluster", "clusters_applications_knative", on_delete: :cascade + add_foreign_key "serverless_domain_cluster", "pages_domains", on_delete: :cascade + add_foreign_key "serverless_domain_cluster", "users", column: "creator_id", on_delete: :nullify add_foreign_key "service_desk_settings", "projects", on_delete: :cascade add_foreign_key "services", "projects", name: "fk_71cce407f9", on_delete: :cascade add_foreign_key "slack_integrations", "services", on_delete: :cascade diff --git a/lib/api/users.rb b/lib/api/users.rb index 3b54d95504f..b8c60f1969c 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -446,7 +446,7 @@ module API end # rubocop: disable CodeReuse/ActiveRecord delete ":id" do - Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42279') + Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab/issues/20757') authenticated_as_admin! diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 3e0a894e72e..edef24f6595 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -5,7 +5,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do include ApiHelpers include HttpIOHelpers - let(:project) { create(:project, :public) } + let(:project) { create(:project, :public, :repository) } let(:pipeline) { create(:ci_pipeline, project: project) } let(:user) { create(:user) } @@ -511,7 +511,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do def get_show_json expect { get_show(id: job.id, format: :json) } - .to change { Gitlab::GitalyClient.get_request_count }.by(1) # ListCommitsByOid + .to change { Gitlab::GitalyClient.get_request_count }.by_at_most(2) end def get_show(**extra_params) diff --git a/spec/factories/serverless/domain_cluster.rb b/spec/factories/serverless/domain_cluster.rb new file mode 100644 index 00000000000..290d3fc152e --- /dev/null +++ b/spec/factories/serverless/domain_cluster.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :serverless_domain_cluster, class: Serverless::DomainCluster do + pages_domain { create(:pages_domain) } + knative { create(:clusters_applications_knative) } + creator { create(:user) } + uuid { SecureRandom.hex(7) } + end +end diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index f1ce447a0d8..5b45dc078de 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -16,6 +16,10 @@ describe Clusters::Applications::Knative do allow(ClusterWaitForIngressIpAddressWorker).to receive(:perform_async) end + describe 'associations' do + it { is_expected.to have_one(:serverless_domain_cluster).class_name('Serverless::DomainCluster').with_foreign_key('clusters_applications_knative_id').inverse_of(:knative) } + end + describe 'when cloud run is enabled' do let(:cluster) { create(:cluster, :provided_by_gcp, :cloud_run_enabled) } let(:knative_cloud_run) { create(:clusters_applications_knative, cluster: cluster) } diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 522a27954e2..33e4cd34aa5 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -474,4 +474,29 @@ describe Deployment do end end end + + context '#update_status' do + let(:deploy) { create(:deployment, status: :running) } + + it 'changes the status' do + deploy.update_status('success') + + expect(deploy).to be_success + end + + it 'schedules SuccessWorker and FinishedWorker when finishing a deploy' do + expect(Deployments::SuccessWorker).to receive(:perform_async) + expect(Deployments::FinishedWorker).to receive(:perform_async) + + deploy.update_status('success') + end + + it 'updates finished_at when transitioning to a finished status' do + Timecop.freeze do + deploy.update_status('success') + + expect(deploy.read_attribute(:finished_at)).to eq(Time.now) + end + end + end end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 4b65bf032d1..b1df13e8c2a 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -175,6 +175,16 @@ describe PagesDomain do it { is_expected.to validate_presence_of(:verification_code) } end + describe 'default values' do + it 'defaults wildcard to false' do + expect(subject.wildcard).to eq(false) + end + + it 'defaults domain_type to project' do + expect(subject.domain_type).to eq('project') + end + end + describe '#verification_code' do subject { pages_domain.verification_code } @@ -305,6 +315,14 @@ describe PagesDomain do end describe '#update_daemon' do + context 'when domain_type is instance' do + it 'does nothing' do + expect(Projects::UpdatePagesConfigurationService).not_to receive(:new) + + create(:pages_domain, domain_type: :instance) + end + end + it 'runs when the domain is created' do domain = build(:pages_domain) diff --git a/spec/models/serverless/domain_cluster_spec.rb b/spec/models/serverless/domain_cluster_spec.rb new file mode 100644 index 00000000000..73d7d64d35e --- /dev/null +++ b/spec/models/serverless/domain_cluster_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Serverless::DomainCluster do + subject { create(:serverless_domain_cluster) } + + describe 'validations' do + it { is_expected.to validate_presence_of(:pages_domain) } + it { is_expected.to validate_presence_of(:knative) } + it { is_expected.to validate_presence_of(:uuid) } + + it { is_expected.to validate_uniqueness_of(:uuid) } + it { is_expected.to validate_length_of(:uuid).is_equal_to(14) } + end + + describe 'associations' do + it { is_expected.to belong_to(:pages_domain) } + it { is_expected.to belong_to(:knative) } + it { is_expected.to belong_to(:creator).optional } + end +end diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb index 6e78b747a8c..3dc8e5749d4 100644 --- a/spec/requests/api/deployments_spec.rb +++ b/spec/requests/api/deployments_spec.rb @@ -147,7 +147,7 @@ describe API::Deployments do expect(response).to have_gitlab_http_status(500) end - it 'links any merged merge requests to the deployment' do + it 'links any merged merge requests to the deployment', :sidekiq_inline do mr = create( :merge_request, :merged, @@ -199,7 +199,7 @@ describe API::Deployments do expect(json_response['ref']).to eq('master') end - it 'links any merged merge requests to the deployment' do + it 'links any merged merge requests to the deployment', :sidekiq_inline do mr = create( :merge_request, :merged, diff --git a/spec/services/deployments/create_service_spec.rb b/spec/services/deployments/create_service_spec.rb index e41c8259ea9..6ab1f8635f7 100644 --- a/spec/services/deployments/create_service_spec.rb +++ b/spec/services/deployments/create_service_spec.rb @@ -3,67 +3,54 @@ require 'spec_helper' describe Deployments::CreateService do - let(:environment) do - double( - :environment, - deployment_platform: double(:platform, cluster_id: 1), - project_id: 2, - id: 3 - ) - end - - let(:user) { double(:user) } + let(:user) { create(:user) } describe '#execute' do - let(:service) { described_class.new(environment, user, {}) } - - it 'does not run the AfterCreateService service if the deployment is not persisted' do - deploy = double(:deployment, persisted?: false) + let(:project) { create(:project, :repository) } + let(:environment) { create(:environment, project: project) } - expect(service) - .to receive(:create_deployment) - .and_return(deploy) + it 'creates a deployment' do + service = described_class.new( + environment, + user, + sha: 'b83d6e391c22777fca1ed3012fce84f633d7fed0', + ref: 'master', + tag: false, + status: 'success' + ) - expect(Deployments::AfterCreateService) - .not_to receive(:new) + expect(Deployments::SuccessWorker).to receive(:perform_async) + expect(Deployments::FinishedWorker).to receive(:perform_async) - expect(service.execute).to eq(deploy) + expect(service.execute).to be_persisted end - it 'runs the AfterCreateService service if the deployment is persisted' do - deploy = double(:deployment, persisted?: true) - after_service = double(:after_create_service) - - expect(service) - .to receive(:create_deployment) - .and_return(deploy) - - expect(Deployments::AfterCreateService) - .to receive(:new) - .with(deploy) - .and_return(after_service) + it 'does not change the status if no status is given' do + service = described_class.new( + environment, + user, + sha: 'b83d6e391c22777fca1ed3012fce84f633d7fed0', + ref: 'master', + tag: false + ) - expect(after_service) - .to receive(:execute) + expect(Deployments::SuccessWorker).not_to receive(:perform_async) + expect(Deployments::FinishedWorker).not_to receive(:perform_async) - expect(service.execute).to eq(deploy) + expect(service.execute).to be_persisted end end - describe '#create_deployment' do - it 'creates a deployment' do - environment = build(:environment) - service = described_class.new(environment, user, {}) - - expect(environment.deployments) - .to receive(:create) - .with(an_instance_of(Hash)) - - service.create_deployment + describe '#deployment_attributes' do + let(:environment) do + double( + :environment, + deployment_platform: double(:platform, cluster_id: 1), + project_id: 2, + id: 3 + ) end - end - describe '#deployment_attributes' do it 'only includes attributes that we want to persist' do service = described_class.new( environment, @@ -72,8 +59,7 @@ describe Deployments::CreateService do tag: true, sha: '123', foo: 'bar', - on_stop: 'stop', - status: 'running' + on_stop: 'stop' ) expect(service.deployment_attributes).to eq( @@ -84,8 +70,7 @@ describe Deployments::CreateService do tag: true, sha: '123', user: user, - on_stop: 'stop', - status: 'running' + on_stop: 'stop' ) end end diff --git a/spec/services/deployments/update_service_spec.rb b/spec/services/deployments/update_service_spec.rb index 8a918d28ffd..471e90de467 100644 --- a/spec/services/deployments/update_service_spec.rb +++ b/spec/services/deployments/update_service_spec.rb @@ -34,9 +34,9 @@ describe Deployments::UpdateService do expect(deploy).to be_canceled end - it 'returns false when the status is not supported' do - expect(described_class.new(deploy, status: 'kittens').execute) - .to be_falsey + it 'raises ArgumentError if the status is invalid' do + expect { described_class.new(deploy, status: 'kittens').execute } + .to raise_error(ArgumentError) end it 'links merge requests when changing the status to success', :sidekiq_inline do |