From 9c6424df5d9d0cb41ec78cbdddb520f1c1ec604b Mon Sep 17 00:00:00 2001 From: Ben Nemec Date: Mon, 21 Nov 2016 18:53:53 +0000 Subject: [PATCH] Validate vips when generating certificate too When generate_service_certificate is True, undercloud_service_certificate will not necessarily be set when it is passed to validation. We need to check if either value is set when deciding whether to validate vips. Unit tests for this behavior were missing as well, so those have been added. Another consideration for this change is that we have started passing non-IP values to these vip parameters when configuring ssl. This is counterintuitive, but apparently works as intended so let's just rename the parameters and handle both IPs and DNS names for those values. Change-Id: I53151d4f555d5d161a3e53ce5f022e3bf3b2ffbd Closes-Bug: 1643655 --- .../puppet-stack-config.pp | 4 ++-- .../puppet-stack-config.yaml.template | 14 +++++------ .../os-apply-config/root/stackrc | 2 +- instack_undercloud/tests/test_validator.py | 21 ++++++++++++++++ instack_undercloud/undercloud.py | 24 ++++++++++--------- instack_undercloud/validator.py | 24 ++++++++++++------- templates/config.json.template | 2 +- undercloud.conf.sample | 16 +++++++------ 8 files changed, 70 insertions(+), 37 deletions(-) diff --git a/elements/puppet-stack-config/puppet-stack-config.pp b/elements/puppet-stack-config/puppet-stack-config.pp index acc39e507..ab31d048b 100644 --- a/elements/puppet-stack-config/puppet-stack-config.pp +++ b/elements/puppet-stack-config/puppet-stack-config.pp @@ -264,7 +264,7 @@ class { '::ironic::inspector::db::mysql': include ::swift if hiera('tripleo::haproxy::service_certificate', undef) { - $keystone_public_endpoint = join(['https://', hiera('controller_public_vip'), ':13000']) + $keystone_public_endpoint = join(['https://', hiera('controller_public_host'), ':13000']) $enable_proxy_headers_parsing = true } else { $keystone_public_endpoint = undef @@ -511,7 +511,7 @@ if str2bool(hiera('enable_docker_registry', true)) { line => join ([ 'INSECURE_REGISTRY="', '--insecure-registry ', hiera('controller_host'), ':8787 ', - '--insecure-registry ', hiera('controller_admin_vip'), ':8787"']), + '--insecure-registry ', hiera('controller_admin_host'), ':8787"']), match => 'INSECURE_REGISTRY=', notify => Service['docker'], } diff --git a/elements/puppet-stack-config/puppet-stack-config.yaml.template b/elements/puppet-stack-config/puppet-stack-config.yaml.template index d18eca806..d4966edd8 100644 --- a/elements/puppet-stack-config/puppet-stack-config.yaml.template +++ b/elements/puppet-stack-config/puppet-stack-config.yaml.template @@ -5,8 +5,8 @@ keystone_region: 'regionOne' debug: {{UNDERCLOUD_DEBUG}} controller_host: {{LOCAL_IP}} #local-ipv4 -controller_admin_vip: {{UNDERCLOUD_ADMIN_VIP}} -controller_public_vip: {{UNDERCLOUD_PUBLIC_VIP}} +controller_admin_host: {{UNDERCLOUD_ADMIN_HOST}} +controller_public_host: {{UNDERCLOUD_PUBLIC_HOST}} ntp::servers: - @@ -24,7 +24,7 @@ tripleo::profile::base::haproxy::certificates_specs: service_pem: {{UNDERCLOUD_SERVICE_CERTIFICATE}} service_certificate: '/etc/pki/tls/certs/undercloud-front.crt' service_key: '/etc/pki/tls/private/undercloud-front.key' - hostname: "%{hiera('controller_public_vip')}" + hostname: "%{hiera('controller_public_host')}" postsave_cmd: "/usr/bin/instack-haproxy-cert-update '/etc/pki/tls/certs/undercloud-front.crt' '/etc/pki/tls/private/undercloud-front.key' {{UNDERCLOUD_SERVICE_CERTIFICATE}}" principal: {{SERVICE_PRINCIPAL}} @@ -645,9 +645,9 @@ cinder::wsgi::apache::workers: "%{::os_workers}" # HAproxy tripleo::profile::base::haproxy::step: 1 tripleo::haproxy::haproxy_stats_password: {{UNDERCLOUD_HAPROXY_STATS_PASSWORD}} -tripleo::haproxy::controller_virtual_ip: "%{hiera('controller_admin_vip')}" +tripleo::haproxy::controller_virtual_ip: "%{hiera('controller_admin_host')}" tripleo::haproxy::controller_hosts: "%{hiera('controller_host')}" -tripleo::haproxy::public_virtual_ip: "%{hiera('controller_public_vip')}" +tripleo::haproxy::public_virtual_ip: "%{hiera('controller_public_host')}" tripleo::haproxy::public_virtual_interface: 'br-ctlplane' tripleo::haproxy::keystone_admin: true tripleo::haproxy::keystone_public: true @@ -672,9 +672,9 @@ tripleo::haproxy::zaqar_ws: {{ENABLE_ZAQAR}} tripleo::haproxy::docker_registry: true # Keepalived -tripleo::keepalived::controller_virtual_ip: "%{hiera('controller_admin_vip')}" +tripleo::keepalived::controller_virtual_ip: "%{hiera('controller_admin_host')}" tripleo::keepalived::control_virtual_interface: 'br-ctlplane' -tripleo::keepalived::public_virtual_ip: "%{hiera('controller_public_vip')}" +tripleo::keepalived::public_virtual_ip: "%{hiera('controller_public_host')}" tripleo::keepalived::public_virtual_interface: 'br-ctlplane' # UI diff --git a/elements/undercloud-install/os-apply-config/root/stackrc b/elements/undercloud-install/os-apply-config/root/stackrc index 735bc9391..7465d2f40 100644 --- a/elements/undercloud-install/os-apply-config/root/stackrc +++ b/elements/undercloud-install/os-apply-config/root/stackrc @@ -3,7 +3,7 @@ export NOVA_VERSION OS_PASSWORD=$(sudo hiera admin_password) export OS_PASSWORD {{#service_certificate}} -OS_AUTH_URL=https://{{public_vip}}:13000/v2.0 +OS_AUTH_URL=https://{{public_host}}:13000/v2.0 PYTHONWARNINGS="ignore:Certificate has no, ignore:A true SSLContext object is not available" export OS_AUTH_URL export PYTHONWARNINGS diff --git a/instack_undercloud/tests/test_validator.py b/instack_undercloud/tests/test_validator.py index 7a5c6e123..64bab062a 100644 --- a/instack_undercloud/tests/test_validator.py +++ b/instack_undercloud/tests/test_validator.py @@ -129,5 +129,26 @@ class TestValidator(base.BaseTestCase): def test_invalid_undercloud_nameserver_fails(self): self.conf.config(undercloud_nameservers=['Iamthewalrus']) + + def test_fail_on_invalid_public_host(self): + self.conf.config(undercloud_public_host='192.0.3.2', + undercloud_service_certificate='foo.pem') + self.assertRaises(validator.FailedValidation, + undercloud._validate_network) + + def test_fail_on_invalid_admin_host(self): + self.conf.config(undercloud_admin_host='192.0.3.3', + generate_service_certificate=True) + self.assertRaises(validator.FailedValidation, + undercloud._validate_network) + + def test_ssl_hosts_allowed(self): + self.conf.config(undercloud_public_host='public.domain', + undercloud_admin_host='admin.domain', + undercloud_service_certificate='foo.pem') + undercloud._validate_network() + + def test_fail_on_invalid_ip(self): + self.conf.config(dhcp_start='foo.bar') self.assertRaises(validator.FailedValidation, undercloud._validate_network) diff --git a/instack_undercloud/undercloud.py b/instack_undercloud/undercloud.py index d1981f22c..f1fc59b43 100644 --- a/instack_undercloud/undercloud.py +++ b/instack_undercloud/undercloud.py @@ -136,15 +136,17 @@ _opts = [ 'Overcloud instances. This should match the local_ip ' 'above when using masquerading.') ), - cfg.StrOpt('undercloud_public_vip', + cfg.StrOpt('undercloud_public_host', + deprecated_name='undercloud_public_vip', default='192.168.24.2', - help=('Virtual IP address to use for the public endpoints of ' - 'Undercloud services. Only used with SSL.') + help=('Virtual IP or DNS address to use for the public ' + 'endpoints of Undercloud services. Only used with SSL.') ), - cfg.StrOpt('undercloud_admin_vip', + cfg.StrOpt('undercloud_admin_host', + deprecated_name='undercloud_admin_vip', default='192.168.24.3', - help=('Virtual IP address to use for the admin endpoints of ' - 'Undercloud services. Only used with SSL.') + help=('Virtual IP or DNS address to use for the admin ' + 'endpoints of Undercloud services. Only used with SSL.') ), cfg.ListOpt('undercloud_nameservers', default=[], @@ -163,7 +165,7 @@ _opts = [ 'will be used in place of the value for ' 'undercloud_service_certificate. The resulting ' 'certificate will be written to ' - '/etc/pki/tls/certs/undercloud-[undercloud_public_vip].' + '/etc/pki/tls/certs/undercloud-[undercloud_public_host].' 'pem. This certificate is signed by CA selected by the ' '"certificate_generation_ca" option.') ), @@ -692,8 +694,8 @@ def _generate_endpoints(instack_env): if (CONF.undercloud_service_certificate or CONF.generate_service_certificate): - public_host = CONF.undercloud_public_vip - internal_host = CONF.undercloud_admin_vip + public_host = CONF.undercloud_public_host + internal_host = CONF.undercloud_admin_host public_proto = 'https' zaqar_ws_public_proto = 'wss' @@ -953,9 +955,9 @@ def _generate_environment(instack_root): _write_password_file(instack_env) if CONF.generate_service_certificate: - public_vip = CONF.undercloud_public_vip + public_host = CONF.undercloud_public_host instack_env['UNDERCLOUD_SERVICE_CERTIFICATE'] = ( - '/etc/pki/tls/certs/undercloud-%s.pem' % public_vip) + '/etc/pki/tls/certs/undercloud-%s.pem' % public_host) _member_role_exists(instack_env) diff --git a/instack_undercloud/validator.py b/instack_undercloud/validator.py index cdfb8f802..558441ce9 100644 --- a/instack_undercloud/validator.py +++ b/instack_undercloud/validator.py @@ -73,11 +73,16 @@ def _validate_value_formats(params, error_callback): def _validate_in_cidr(params, error_callback): cidr = netaddr.IPNetwork(params['network_cidr']) - def validate_addr_in_cidr(params, name, pretty_name=None): - if netaddr.IPAddress(params[name]) not in cidr: - message = ('%s "%s" not in defined CIDR "%s"' % - (pretty_name or name, params[name], cidr)) - error_callback(message) + def validate_addr_in_cidr(params, name, pretty_name=None, require_ip=True): + try: + if netaddr.IPAddress(params[name]) not in cidr: + message = ('%s "%s" not in defined CIDR "%s"' % + (pretty_name or name, params[name], cidr)) + error_callback(message) + except netaddr.core.AddrFormatError: + if require_ip: + message = 'Invalid IP address: %s' % params[name] + error_callback(message) params['just_local_ip'] = params['local_ip'].split('/')[0] # undercloud.conf uses inspection_iprange, the configuration wizard @@ -88,9 +93,12 @@ def _validate_in_cidr(params, error_callback): params['inspection_end'] = inspection_iprange[1] validate_addr_in_cidr(params, 'just_local_ip', 'local_ip') validate_addr_in_cidr(params, 'network_gateway') - if params['undercloud_service_certificate']: - validate_addr_in_cidr(params, 'undercloud_public_vip') - validate_addr_in_cidr(params, 'undercloud_admin_vip') + if (params['undercloud_service_certificate'] or + params['generate_service_certificate']): + validate_addr_in_cidr(params, 'undercloud_public_host', + require_ip=False) + validate_addr_in_cidr(params, 'undercloud_admin_host', + require_ip=False) validate_addr_in_cidr(params, 'dhcp_start') validate_addr_in_cidr(params, 'dhcp_end') validate_addr_in_cidr(params, 'inspection_start', 'Inspection range start') diff --git a/templates/config.json.template b/templates/config.json.template index 16fa2b69b..9ab6f649d 100644 --- a/templates/config.json.template +++ b/templates/config.json.template @@ -11,7 +11,7 @@ "local-ip": "{{LOCAL_IP}}", "masquerade_networks": ["{{MASQUERADE_NETWORK}}"], "service_certificate": "{{UNDERCLOUD_SERVICE_CERTIFICATE}}", - "public_vip": "{{UNDERCLOUD_PUBLIC_VIP}}", + "public_host": "{{UNDERCLOUD_PUBLIC_HOST}}", "neutron": { "dhcp_start": "{{DHCP_START}}", "dhcp_end": "{{DHCP_END}}", diff --git a/undercloud.conf.sample b/undercloud.conf.sample index 50796083a..d46871236 100644 --- a/undercloud.conf.sample +++ b/undercloud.conf.sample @@ -28,13 +28,15 @@ # masquerading. (string value) #network_gateway = 192.168.24.1 -# Virtual IP address to use for the public endpoints of Undercloud -# services. Only used with SSL. (string value) -#undercloud_public_vip = 192.168.24.2 +# Virtual IP or DNS address to use for the public endpoints of +# Undercloud services. Only used with SSL. (string value) +# Deprecated group/name - [DEFAULT]/undercloud_public_vip +#undercloud_public_host = 192.168.24.2 -# Virtual IP address to use for the admin endpoints of Undercloud -# services. Only used with SSL. (string value) -#undercloud_admin_vip = 192.168.24.3 +# Virtual IP or DNS address to use for the admin endpoints of +# Undercloud services. Only used with SSL. (string value) +# Deprecated group/name - [DEFAULT]/undercloud_admin_vip +#undercloud_admin_host = 192.168.24.3 # DNS nameserver(s) to use for the undercloud node. (list value) #undercloud_nameservers = @@ -48,7 +50,7 @@ # the undercloud install and this certificate will be used in place of # the value for undercloud_service_certificate. The resulting # certificate will be written to -# /etc/pki/tls/certs/undercloud-[undercloud_public_vip].pem. This +# /etc/pki/tls/certs/undercloud-[undercloud_public_host].pem. This # certificate is signed by CA selected by the # "certificate_generation_ca" option. (boolean value) #generate_service_certificate = false