Skip to content

Commit

Permalink
Fixes #29468 - Add default URL validator for plugins
Browse files Browse the repository at this point in the history
URL settings are common and this moves it from a custom validator in the
Puppet module to a default. Also adds tests and ensures the dhcp and dns
libvirt providers use it too.
  • Loading branch information
ekohl authored and mmoll committed Apr 11, 2020
1 parent 94a84f1 commit 8b27dab
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 19 deletions.
6 changes: 5 additions & 1 deletion lib/proxy/default_plugin_validators.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
class Proxy::DefaultPluginValidators
def self.validators
{:file_readable => ::Proxy::PluginValidators::FileReadable, :presence => ::Proxy::PluginValidators::Presence}
{
file_readable: ::Proxy::PluginValidators::FileReadable,
presence: ::Proxy::PluginValidators::Presence,
url: ::Proxy::PluginValidators::Url,
}
end
end
14 changes: 14 additions & 0 deletions lib/proxy/plugin_validators.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,18 @@ def validate!(settings)
true
end
end

class Url < Base
def validate!(settings)
setting_value = settings[@setting_name]
raise ::Proxy::Error::ConfigurationError, "Setting '#{@setting_name}' is expected to contain a url" if setting_value.to_s.empty?

parsed = URI.parse(setting_value)
raise ::Proxy::Error::ConfigurationError, "Setting '#{@setting_name}' is missing a scheme" if parsed.scheme.nil? || parsed.scheme.empty?

true
rescue URI::InvalidURIError
raise ::Proxy::Error::ConfigurationError.new("Setting '#{@setting_name}' contains an invalid url")
end
end
end
1 change: 1 addition & 0 deletions modules/dhcp_libvirt/dhcp_libvirt_plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class Plugin < ::Proxy::Provider

requires :dhcp, ::Proxy::VERSION
default_settings :url => "qemu:///system", :network => 'default'
validate :url, :url => true

load_classes ::Proxy::DHCP::Libvirt::PluginConfiguration
load_dependency_injection_wirings ::Proxy::DHCP::Libvirt::PluginConfiguration
Expand Down
1 change: 1 addition & 0 deletions modules/dns_libvirt/dns_libvirt_plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class Plugin < ::Proxy::Provider
requires :dns, ::Proxy::VERSION

default_settings :url => "qemu:///system", :network => 'default'
validate :url, :url => true

load_classes ::Proxy::Dns::Libvirt::PluginConfiguration
load_dependency_injection_wirings ::Proxy::Dns::Libvirt::PluginConfiguration
Expand Down
12 changes: 0 additions & 12 deletions modules/puppet_proxy_common/custom_validators.rb

This file was deleted.

1 change: 0 additions & 1 deletion modules/puppet_proxy_puppet_api/plugin_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ def load_programmable_settings(settings)
end

def load_classes
require 'puppet_proxy_common/custom_validators'
require 'puppet_proxy_common/errors'
require 'puppet_proxy_common/environments_retriever_base'
require 'puppet_proxy_common/environment'
Expand Down
1 change: 0 additions & 1 deletion modules/puppet_proxy_puppet_api/puppet_proxy_puppet_api.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
require 'puppet_proxy_common/custom_validators'
require 'puppet_proxy_puppet_api/plugin_configuration'
require 'puppet_proxy_puppet_api/puppet_proxy_puppet_api_plugin'
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ class Plugin < Proxy::Provider

plugin :puppet_proxy_puppet_api, ::Proxy::VERSION

load_validators :url => ::Proxy::Puppet::Validators::UrlValidator
load_programmable_settings ::Proxy::PuppetApi::PluginConfiguration
load_classes ::Proxy::PuppetApi::PluginConfiguration
load_dependency_injection_wirings ::Proxy::PuppetApi::PluginConfiguration
Expand Down
3 changes: 0 additions & 3 deletions modules/puppetca_http_api/puppetca_http_api_plugin.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
require 'puppet_proxy_common/custom_validators'

module ::Proxy
module PuppetCa
module PuppetcaHttpApi
Expand All @@ -8,7 +6,6 @@ class Plugin < ::Proxy::Provider

default_settings :puppet_ssl_ca => '/etc/puppetlabs/puppet/ssl/certs/ca.pem'

load_validators :url => ::Proxy::Puppet::Validators::UrlValidator
requires :puppetca, ::Proxy::VERSION

validate :puppet_url, :url => true
Expand Down
42 changes: 42 additions & 0 deletions test/plugins/validator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,45 @@ def test_optional_parameter_without_a_value_fails_validation
end
end
end

class UrlValidatorTest < Test::Unit::TestCase
class UrlValidatorTestPlugin < ::Proxy::Plugin
default_settings :a_setting => 'http://example.com'
end

def test_required_parameter_with_a_value_passes_validation
assert ::Proxy::PluginValidators::Url.new(UrlValidatorTestPlugin, 'a_setting', nil, nil).validate!(:a_setting => 'http://example.com')
end

def test_empty_string_treated_as_missing_value
error = assert_raises ::Proxy::Error::ConfigurationError do
::Proxy::PluginValidators::Url.new(UrlValidatorTestPlugin, 'a_setting', nil, nil).validate!(:a_setting => '')
end

assert_match(/expected to contain a url/, error.message)
end

def test_required_parameter_without_a_value_fails_validation
error = assert_raises ::Proxy::Error::ConfigurationError do
::Proxy::PluginValidators::Url.new(UrlValidatorTestPlugin, 'a_setting', nil, nil).validate!(:a_setting => nil)
end

assert_match(/expected to contain a url/, error.message)
end

def test_required_parameter_without_scheme_fails_validation
error = assert_raises ::Proxy::Error::ConfigurationError do
::Proxy::PluginValidators::Url.new(UrlValidatorTestPlugin, 'a_setting', nil, nil).validate!(:a_setting => 'example.com')
end

assert_match(/missing a scheme/, error.message)
end

def test_optional_parameter_without_a_value_fails_validation
error = assert_raises ::Proxy::Error::ConfigurationError do
::Proxy::PluginValidators::Url.new(UrlValidatorTestPlugin, 'optional_setting', nil, nil).validate!(:optional_setting => nil)
end

assert_match(/expected to contain a url/, error.message)
end
end

0 comments on commit 8b27dab

Please sign in to comment.