Commit e90fbe3e authored by Georges Racinet's avatar Georges Racinet 🦑
Browse files

multi-vcs: validate Project vcs_type against Application Settings

Now that we have the `vcs_types` Application Settings field,
we use it to enforce upon Project creation that the selected
`vcs_type` is among them.

This validation has to be done by a separate method in order
to access the `CurrentSettings` object.

Also, we've considered making the default value for `vcs_type` in
`CreateService` not to be the library-level default, so that
explicitely passing a value is never required, even in the case where
the library default is not allowed by Application Settings, but
finally decided that failing with an explicit message in that case
was better than such magic. Note also that the Web UI will always
pass a value, so this case is in practice for API calls only.
parent e6e8952549c8
......@@ -405,8 +405,6 @@
# Validations
validates :creator, presence: true, on: :create
validates :vcs_type, presence: false,
inclusion: { in: Gitlab::VCS::SUPPORTED_TYPES }
validates :description, length: { maximum: 2000 }, allow_blank: true
validates :ci_config_path,
format: { without: %r{(\.{2}|\A/)},
......@@ -431,6 +429,7 @@
ports: ->(project) { project.persisted? ? VALID_MIRROR_PORTS : VALID_IMPORT_PORTS },
enforce_user: true }, if: [:external_import?, :import_url_changed?]
validates :star_count, numericality: { greater_than_or_equal_to: 0 }
validate :check_vcs_type, on: :create
validate :check_personal_projects_limit, on: :create
validate :check_repository_path_availability, on: :update, if: ->(project) { project.renamed? }
validate :visibility_level_allowed_by_group, if: :should_validate_visibility_level?
......@@ -1141,6 +1140,12 @@
::Gitlab::CurrentSettings.mirror_available
end
def check_vcs_type
unless ::Gitlab::CurrentSettings.current_application_settings.vcs_types.include?(vcs_type)
self.errors.add(:vcs_type, "VCS type '#{vcs_type}' not allowed by Application Settings")
end
end
def check_personal_projects_limit
# Since this method is called as validation hook, `creator` might not be
# present. Since the validation for that will fail, we can just return
......
......@@ -701,4 +701,5 @@
expect(project.valid?).to be(true)
expect(project.vcs_type).to eq('git')
end
end
......@@ -704,4 +705,35 @@
context 'with no vcs_type and VCS::DEFAULT_TYPE not allowed in application settings' do
before do
stub_application_setting(vcs_types: ['not_the_default'])
end
it 'does not create the project' do
project = create_project(user, opts)
expect(project).to respond_to(:errors)
expect(project.errors.any?).to be(true)
expect(project.saved?).to be_falsey
expect(project.valid?).to be_falsey
expect(project.errors.messages).to have_key(:vcs_type)
end
end
context 'with vcs_type not allowed in application settings' do
before do
stub_application_setting(vcs_types: ['hg'])
opts.merge!(
vcs_type: 'git'
)
end
it 'does not create the project' do
project = create_project(user, opts)
expect(project).to respond_to(:errors)
expect(project.errors.any?).to be(true)
expect(project.saved?).to be_falsey
expect(project.valid?).to be_falsey
expect(project.errors.messages).to have_key(:vcs_type)
end
end
context 'with Heptapod rules for external users' do
......
Supports Markdown
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