From b081ee8e25825c88ba974268cc24a488b461af67 Mon Sep 17 00:00:00 2001
From: Tobias Urdin <tobias.urdin@binero.se>
Date: Fri, 10 May 2019 17:11:07 +0200
Subject: [PATCH] Remove deprecated keystone::federation::openidc params

These params has been deprecated. Also removes the keystone_url
behavior where it doesn't fallback on keystone::public_endpoint
anymore.

Change-Id: I52b4c2a2911007b516a6ea20229a3579bc9dd945
---
 manifests/federation/openidc.pp               | 42 ++++---------------
 ...e-deprecated-openidc-845cea167976f90e.yaml |  8 ++++
 .../keystone_federation_openidc_spec.rb       |  5 +--
 templates/openidc.conf.erb                    |  4 +-
 4 files changed, 19 insertions(+), 40 deletions(-)
 create mode 100644 releasenotes/notes/remove-deprecated-openidc-845cea167976f90e.yaml

diff --git a/manifests/federation/openidc.pp b/manifests/federation/openidc.pp
index 87f2dfede..b65ff7f1a 100644
--- a/manifests/federation/openidc.pp
+++ b/manifests/federation/openidc.pp
@@ -2,6 +2,9 @@
 #
 # == Parameters
 #
+# [*keystone_url*]
+#  (Required) URL to keystone endpoint.
+#
 # [*methods*]
 #  A list of methods used for authentication separated by comma or an array.
 #  The allowed values are: 'external', 'password', 'token', 'oauth1', 'saml2',
@@ -89,26 +92,12 @@
 #  (Optional) Defaults to 331.
 #
 # [*package_ensure*]
-#   (Optional) Desired ensure state of packages.
-#   accepts latest or specific versions.
-#   Defaults to present.
-#
-# [*keystone_url*]
-#   (optional) URL to keystone endpoint.
-#
-# === DEPRECATED
-#
-# [*admin_port*]
-#  A boolean value to ensure that you want to configure openidc Federation
-#  using Keystone VirtualHost on port 35357.
-#  (Optional) Defaults to undef.
-#
-# [*main_port*]
-#  A boolean value to ensure that you want to configure openidc Federation
-#  using Keystone VirtualHost on port 5000.
-#  (Optional) Defaults to undef.
+#  (Optional) Desired ensure state of packages.
+#  accepts latest or specific versions.
+#  Defaults to present.
 #
 class keystone::federation::openidc (
+  $keystone_url,
   $methods,
   $idp_name,
   $openidc_provider_metadata_url,
@@ -129,29 +118,12 @@ class keystone::federation::openidc (
   $remote_id_attribute            = undef,
   $template_order                 = 331,
   $package_ensure                 = present,
-  $keystone_url                   = undef,
-  # DEPRECATED
-  $admin_port                     = undef,
-  $main_port                      = undef,
 ) {
 
   include ::apache
   include ::keystone::deps
   include ::keystone::params
 
-  # TODO(tobias-urdin): Make keystone_url required when keystone::public_endpoint is removed.
-  # Dont forget to change the keystone_url_real variable in the templates/openidc.conf.rb file.
-  # The fail statement below can also be removed since keystone_url will be a required parameter.
-  $keystone_url_real = pick($keystone_url, $::keystone::public_endpoint)
-
-  if $keystone_url_real == undef or is_service_default($keystone_url_real) {
-    fail('You must set either keystone_url or keystone::public_endpoint')
-  }
-
-  if $admin_port or $main_port {
-    warning('keystone::federation::openidc::admin_port and main_port are deprecated and have no effect')
-  }
-
   if $openidc_enable_oauth and !$openidc_introspection_endpoint {
     fail('You must set openidc_introspection_endpoint when enabling oauth support')
   }
diff --git a/releasenotes/notes/remove-deprecated-openidc-845cea167976f90e.yaml b/releasenotes/notes/remove-deprecated-openidc-845cea167976f90e.yaml
new file mode 100644
index 000000000..b06f3b80b
--- /dev/null
+++ b/releasenotes/notes/remove-deprecated-openidc-845cea167976f90e.yaml
@@ -0,0 +1,8 @@
+---
+upgrade:
+  - |
+    The deprecated parameters main_port and admin_port in keystone::federation::openidc
+    is now removed.
+  - |
+    The keystone::federation::openidc::keystone_url parameter is now mandatory and does
+    not fallback on the keystone::public_endpoint value.
diff --git a/spec/classes/keystone_federation_openidc_spec.rb b/spec/classes/keystone_federation_openidc_spec.rb
index dc3a127ca..c9d5e043f 100644
--- a/spec/classes/keystone_federation_openidc_spec.rb
+++ b/spec/classes/keystone_federation_openidc_spec.rb
@@ -10,8 +10,6 @@ describe 'keystone::federation::openidc' do
     <<-EOS
     class { 'keystone':
       admin_token => 'service_token',
-      public_endpoint => 'http://os.example.com:5000',
-      admin_endpoint => 'http://os.example.com:35357',
     }
 
     include keystone::wsgi::apache
@@ -19,7 +17,8 @@ describe 'keystone::federation::openidc' do
   end
 
   let :params do
-    { :methods => 'password, token, openid',
+    { :keystone_url => 'http://localhost:5000',
+      :methods => 'password, token, openid',
       :idp_name => 'myidp',
       :openidc_provider_metadata_url => 'https://accounts.google.com/.well-known/openid-configuration',
       :openidc_client_id => 'openid_client_id',
diff --git a/templates/openidc.conf.erb b/templates/openidc.conf.erb
index 39b957ed3..a0d344323 100644
--- a/templates/openidc.conf.erb
+++ b/templates/openidc.conf.erb
@@ -34,8 +34,8 @@
 
   # The following directives are necessary to support websso from Horizon
   # (Per https://docs.openstack.org/keystone/pike/advanced-topics/federation/websso.html)
-  OIDCRedirectURI "<%= @keystone_url_real -%>/v3/auth/OS-FEDERATION/identity_providers/<%= scope['keystone::federation::openidc::idp_name']-%>/protocols/openid/websso"
-  OIDCRedirectURI "<%= @keystone_url_real -%>/v3/auth/OS-FEDERATION/websso/openid"
+  OIDCRedirectURI "<%= @keystone_url -%>/v3/auth/OS-FEDERATION/identity_providers/<%= scope['keystone::federation::openidc::idp_name']-%>/protocols/openid/websso"
+  OIDCRedirectURI "<%= @keystone_url -%>/v3/auth/OS-FEDERATION/websso/openid"
 
   <LocationMatch "/v3/auth/OS-FEDERATION/websso/openid">
       AuthType "openid-connect"