From 56f0f60bbfcd2f68d97285a6a1d1a336e2e85cdd Mon Sep 17 00:00:00 2001 From: Hunar Khanna Date: Thu, 22 Jun 2023 12:40:55 +0800 Subject: [PATCH 1/4] Validate DNS zone when saving config --- .../remote_development_agent_config.rb | 1 + .../agent_config/update_processor_spec.rb | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/ee/app/models/remote_development/remote_development_agent_config.rb b/ee/app/models/remote_development/remote_development_agent_config.rb index 6bf3718aefdfa6..e84984f66b0446 100644 --- a/ee/app/models/remote_development/remote_development_agent_config.rb +++ b/ee/app/models/remote_development/remote_development_agent_config.rb @@ -17,6 +17,7 @@ class RemoteDevelopmentAgentConfig < ApplicationRecord validates :enabled, inclusion: { in: [true], message: 'is currently immutable, and must be set to true' } # noinspection RubyResolve before_validation :prevent_dns_zone_update, if: ->(record) { record.persisted? && record.dns_zone_changed? } + validates :dns_zone, hostname: true private diff --git a/ee/spec/lib/remote_development/agent_config/update_processor_spec.rb b/ee/spec/lib/remote_development/agent_config/update_processor_spec.rb index f5ad206b3f9401..a77e5f2ba37ae1 100644 --- a/ee/spec/lib/remote_development/agent_config/update_processor_spec.rb +++ b/ee/spec/lib/remote_development/agent_config/update_processor_spec.rb @@ -67,6 +67,21 @@ expect(config_instance).to be_nil end end + + context 'when dns_zone is invalid' do + let(:dns_zone) { "invalid dns zone" } + + it 'does not create the record and returns error' do + result = subject + + expect(result[0]).to be_nil + expect(result[1].message).to match(/Error\(s\) updating .*Dns zone contains invalid characters.*/) + expect(result[1].reason).to eq(:bad_request) + + config_instance = agent.reload.remote_development_agent_config + expect(config_instance).to be_nil + end + end end end end -- GitLab From 7ee1d5cf23c8cb24ad685962ad8a7160c3224515 Mon Sep 17 00:00:00 2001 From: Hunar Khanna Date: Mon, 26 Jun 2023 11:29:46 +0800 Subject: [PATCH 2/4] Place validation in the correct group --- .../remote_development/remote_development_agent_config.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/models/remote_development/remote_development_agent_config.rb b/ee/app/models/remote_development/remote_development_agent_config.rb index e84984f66b0446..6233570b0b0458 100644 --- a/ee/app/models/remote_development/remote_development_agent_config.rb +++ b/ee/app/models/remote_development/remote_development_agent_config.rb @@ -12,12 +12,12 @@ class RemoteDevelopmentAgentConfig < ApplicationRecord validates :enabled, presence: true validates :agent, presence: true + validates :dns_zone, hostname: true # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409772 - Make this a type:enum validates :enabled, inclusion: { in: [true], message: 'is currently immutable, and must be set to true' } # noinspection RubyResolve before_validation :prevent_dns_zone_update, if: ->(record) { record.persisted? && record.dns_zone_changed? } - validates :dns_zone, hostname: true private -- GitLab From eedb474136ac5687392eaf3853f6e18721c2252e Mon Sep 17 00:00:00 2001 From: Hunar Khanna Date: Mon, 26 Jun 2023 13:02:58 +0800 Subject: [PATCH 3/4] Add new spec/update existing spec --- .../agent_config/update_processor_spec.rb | 2 +- .../remote_development_agent_config_spec.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/ee/spec/lib/remote_development/agent_config/update_processor_spec.rb b/ee/spec/lib/remote_development/agent_config/update_processor_spec.rb index a77e5f2ba37ae1..29f010317d1326 100644 --- a/ee/spec/lib/remote_development/agent_config/update_processor_spec.rb +++ b/ee/spec/lib/remote_development/agent_config/update_processor_spec.rb @@ -75,7 +75,7 @@ result = subject expect(result[0]).to be_nil - expect(result[1].message).to match(/Error\(s\) updating .*Dns zone contains invalid characters.*/) + expect(result[1].message).to match(/Error.*Dns zone.*/) expect(result[1].reason).to eq(:bad_request) config_instance = agent.reload.remote_development_agent_config diff --git a/ee/spec/models/remote_development/remote_development_agent_config_spec.rb b/ee/spec/models/remote_development/remote_development_agent_config_spec.rb index 24aac01450ad87..2c91539faa465a 100644 --- a/ee/spec/models/remote_development/remote_development_agent_config_spec.rb +++ b/ee/spec/models/remote_development/remote_development_agent_config_spec.rb @@ -29,4 +29,16 @@ .to match_array(['Dns zone is currently immutable, and cannot be updated. Create a new agent instead.']) end end + + describe 'validations' do + let_it_be(:invalid_config) { build(:remote_development_agent_config, dns_zone: "invalid dns zone") } + + subject { invalid_config } + + it 'prevents creation of config with invalid dns zon' do + subject.save # rubocop:disable Rails/SaveBang + expect(subject.errors.full_messages) + .to match_array(['Dns zone contains invalid characters (valid characters: [a-z0-9\\-])']) + end + end end -- GitLab From 2cde4402e830899af741ace0ee7a8c6bd52ba40c Mon Sep 17 00:00:00 2001 From: Hunar Khanna Date: Mon, 26 Jun 2023 13:15:27 +0800 Subject: [PATCH 4/4] Refactor spec for flexibility --- .../remote_development_agent_config_spec.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ee/spec/models/remote_development/remote_development_agent_config_spec.rb b/ee/spec/models/remote_development/remote_development_agent_config_spec.rb index 2c91539faa465a..7e1ff9c4ceb3a9 100644 --- a/ee/spec/models/remote_development/remote_development_agent_config_spec.rb +++ b/ee/spec/models/remote_development/remote_development_agent_config_spec.rb @@ -31,14 +31,16 @@ end describe 'validations' do - let_it_be(:invalid_config) { build(:remote_development_agent_config, dns_zone: "invalid dns zone") } + context 'when config has an invalid dns_zone' do + let_it_be(:config) { build(:remote_development_agent_config, dns_zone: "invalid dns zone") } - subject { invalid_config } + subject { config } - it 'prevents creation of config with invalid dns zon' do - subject.save # rubocop:disable Rails/SaveBang - expect(subject.errors.full_messages) - .to match_array(['Dns zone contains invalid characters (valid characters: [a-z0-9\\-])']) + it 'prevents config from being created' do + subject.save # rubocop:disable Rails/SaveBang + expect(subject.errors.full_messages) + .to match_array(['Dns zone contains invalid characters (valid characters: [a-z0-9\\-])']) + end end end end -- GitLab