From f772e96e12500a8312c84275094010b51c886ee2 Mon Sep 17 00:00:00 2001
From: Takashi Kajinami <kajinamit@oss.nttdata.com>
Date: Sun, 26 Nov 2023 20:27:54 +0900
Subject: [PATCH] Add parameter type validations

Some parameters accept only certain parameter types/values. This adds
validations in parameter input to return sensible errors to users.

Change-Id: Ie6c0cf7956c228fead74dcb59e94111402303a56
---
 manifests/containerreconciler.pp | 36 ++++++++++---------
 manifests/objectexpirer.pp       | 10 ++++--
 manifests/proxy.pp               | 60 ++++++++++++++++----------------
 manifests/service.pp             | 18 +++++-----
 manifests/storage/account.pp     | 10 +++---
 manifests/storage/container.pp   | 12 +++----
 manifests/storage/drive_audit.pp |  2 +-
 manifests/storage/generic.pp     | 12 +++----
 manifests/storage/object.pp      | 10 +++---
 types/serviceprovider.pp         |  1 +
 10 files changed, 90 insertions(+), 81 deletions(-)
 create mode 100644 types/serviceprovider.pp

diff --git a/manifests/containerreconciler.pp b/manifests/containerreconciler.pp
index 7738f088..be2c3ccb 100644
--- a/manifests/containerreconciler.pp
+++ b/manifests/containerreconciler.pp
@@ -89,27 +89,31 @@
 #   Defaults to $facts['os_service_default']
 #
 class swift::containerreconciler(
-  Boolean $manage_service   = true,
-  Boolean $enabled          = true,
-  $package_ensure           = 'present',
-  Swift::Pipeline $pipeline = ['catch_errors', 'proxy-logging', 'cache', 'proxy-server'],
-  $interval                 = $facts['os_service_default'],
-  $concurrency              = $facts['os_service_default'],
-  $process                  = $facts['os_service_default'],
-  $processes                = $facts['os_service_default'],
-  $reclaim_age              = $facts['os_service_default'],
-  $request_tries            = $facts['os_service_default'],
-  $service_provider         = $::swift::params::service_provider,
-  $memcache_servers         = ['127.0.0.1:11211'],
-  $cache_tls_enabled        = $facts['os_service_default'],
-  $cache_tls_cafile         = $facts['os_service_default'],
-  $cache_tls_certfile       = $facts['os_service_default'],
-  $cache_tls_keyfile        = $facts['os_service_default'],
+  Boolean $manage_service                  = true,
+  Boolean $enabled                         = true,
+  $package_ensure                          = 'present',
+  Swift::Pipeline $pipeline                = ['catch_errors', 'proxy-logging', 'cache', 'proxy-server'],
+  $interval                                = $facts['os_service_default'],
+  $concurrency                             = $facts['os_service_default'],
+  $process                                 = $facts['os_service_default'],
+  $processes                               = $facts['os_service_default'],
+  $reclaim_age                             = $facts['os_service_default'],
+  $request_tries                           = $facts['os_service_default'],
+  Swift::ServiceProvider $service_provider = $::swift::params::service_provider,
+  $memcache_servers                        = ['127.0.0.1:11211'],
+  $cache_tls_enabled                       = $facts['os_service_default'],
+  $cache_tls_cafile                        = $facts['os_service_default'],
+  $cache_tls_certfile                      = $facts['os_service_default'],
+  $cache_tls_keyfile                       = $facts['os_service_default'],
 ) inherits swift::params {
 
   include swift::deps
   Swift_container_reconciler_config<||> ~> Service['swift-container-reconciler']
 
+  if $pipeline[-1] != 'proxy-server' {
+    fail('proxy-server must be the last element in pipeline')
+  }
+
   # only add memcache servers if 'cache' is included in the pipeline
   if !empty(grep(any2array($pipeline), 'cache')) {
 
diff --git a/manifests/objectexpirer.pp b/manifests/objectexpirer.pp
index 8128b8a7..823b6e71 100644
--- a/manifests/objectexpirer.pp
+++ b/manifests/objectexpirer.pp
@@ -102,10 +102,10 @@
 #    Defaults to 'LOG_LOCAL2'.
 #
 class swift::objectexpirer(
-  $manage_service                = true,
-  $enabled                       = true,
+  Boolean $manage_service        = true,
+  Boolean $enabled               = true,
   $package_ensure                = 'present',
-  $pipeline                      = ['catch_errors', 'proxy-logging', 'cache', 'proxy-server'],
+  Swift::Pipeline $pipeline      = ['catch_errors', 'proxy-logging', 'cache', 'proxy-server'],
   $concurrency                   = $facts['os_service_default'],
   $expiring_objects_account_name = $facts['os_service_default'],
   $interval                      = $facts['os_service_default'],
@@ -137,6 +137,10 @@ class swift::objectexpirer(
     }
   }
 
+  if $pipeline[-1] != 'proxy-server' {
+    fail('proxy-server must be the last element in pipeline')
+  }
+
   # only add memcache servers if 'cache' is included in the pipeline
   if !empty(grep(any2array($pipeline), 'cache')) {
 
diff --git a/manifests/proxy.pp b/manifests/proxy.pp
index f7e5a2d5..05cde4ee 100644
--- a/manifests/proxy.pp
+++ b/manifests/proxy.pp
@@ -152,38 +152,38 @@
 #
 class swift::proxy(
   $proxy_local_net_ip,
-  $port                             = '8080',
-  Swift::Pipeline $pipeline         = [
+  $port                                    = '8080',
+  Swift::Pipeline $pipeline                = [
     'catch_errors', 'gatekeeper', 'healthcheck', 'proxy-logging', 'cache',
     'listing_formats', 'tempauth', 'copy', 'proxy-logging', 'proxy-server'],
-  $workers                          = $facts['os_workers'],
-  Boolean $allow_account_management = true,
-  Boolean $account_autocreate       = true,
-  $log_headers                      = $facts['os_service_default'],
-  $log_udp_host                     = $facts['os_service_default'],
-  $log_udp_port                     = $facts['os_service_default'],
-  $log_address                      = '/dev/log',
-  $log_level                        = 'INFO',
-  $log_facility                     = 'LOG_LOCAL2',
-  $log_handoffs                     = $facts['os_service_default'],
-  $log_name                         = 'proxy-server',
-  $cors_allow_origin                = undef,
-  $strict_cors_mode                 = true,
-  $object_chunk_size                = $facts['os_service_default'],
-  $client_chunk_size                = $facts['os_service_default'],
-  $max_containers_per_account       = $facts['os_service_default'],
-  $max_containers_whitelist         = $facts['os_service_default'],
-  $read_affinity                    = undef,
-  $write_affinity                   = undef,
-  $write_affinity_node_count        = $facts['os_service_default'],
-  $client_timeout                   = $facts['os_service_default'],
-  $node_timeout                     = $facts['os_service_default'],
-  $recoverable_node_timeout         = $facts['os_service_default'],
-  Boolean $manage_service           = true,
-  Boolean $enabled                  = true,
-  $package_ensure                   = 'present',
-  $service_provider                 = $::swift::params::service_provider,
-  $purge_config                     = false,
+  $workers                                 = $facts['os_workers'],
+  Boolean $allow_account_management        = true,
+  Boolean $account_autocreate              = true,
+  $log_headers                             = $facts['os_service_default'],
+  $log_udp_host                            = $facts['os_service_default'],
+  $log_udp_port                            = $facts['os_service_default'],
+  $log_address                             = '/dev/log',
+  $log_level                               = 'INFO',
+  $log_facility                            = 'LOG_LOCAL2',
+  $log_handoffs                            = $facts['os_service_default'],
+  $log_name                                = 'proxy-server',
+  $cors_allow_origin                       = undef,
+  $strict_cors_mode                        = true,
+  $object_chunk_size                       = $facts['os_service_default'],
+  $client_chunk_size                       = $facts['os_service_default'],
+  $max_containers_per_account              = $facts['os_service_default'],
+  $max_containers_whitelist                = $facts['os_service_default'],
+  $read_affinity                           = undef,
+  $write_affinity                          = undef,
+  $write_affinity_node_count               = $facts['os_service_default'],
+  $client_timeout                          = $facts['os_service_default'],
+  $node_timeout                            = $facts['os_service_default'],
+  $recoverable_node_timeout                = $facts['os_service_default'],
+  Boolean $manage_service                  = true,
+  Boolean $enabled                         = true,
+  $package_ensure                          = 'present',
+  Swift::ServiceProvider $service_provider = $::swift::params::service_provider,
+  Boolean $purge_config                    = false,
 ) inherits swift::params {
 
   include swift::deps
diff --git a/manifests/service.pp b/manifests/service.pp
index 6a051633..50f03734 100644
--- a/manifests/service.pp
+++ b/manifests/service.pp
@@ -42,14 +42,14 @@
 # (optional) Additional tag to be added to the service resource
 #
 define swift::service(
-  $os_family_service_name,
-  $config_file_name,
-  $service_ensure    = undef,
-  $enabled           = true,
-  $service_provider  = $::swift::params::service_provider,
-  $service_subscribe = undef,
-  $service_require   = undef,
-  $service_tag       = undef,
+  String[1] $os_family_service_name,
+  String[1] $config_file_name,
+  $service_ensure                          = undef,
+  Boolean $enabled                         = true,
+  Swift::ServiceProvider $service_provider = $::swift::params::service_provider,
+  $service_subscribe                       = undef,
+  $service_require                         = undef,
+  Optional[String[1]] $service_tag         = undef,
 ) {
 
   include swift::deps
@@ -72,7 +72,7 @@ define swift::service(
       subscribe => $service_subscribe,
       require   => $service_require,
     }
-  } elsif $service_provider == 'swiftinit' {
+  } else {
     service { $name:
       ensure     => $service_ensure,
       enable     => $enabled,
diff --git a/manifests/storage/account.pp b/manifests/storage/account.pp
index c5ed5c6c..400993ae 100644
--- a/manifests/storage/account.pp
+++ b/manifests/storage/account.pp
@@ -27,11 +27,11 @@
 #    Defaults to $::swift::params::service_provider.
 #
 class swift::storage::account(
-  $manage_service   = true,
-  $enabled          = true,
-  $package_ensure   = 'present',
-  $config_file_name = 'account-server.conf',
-  $service_provider = $::swift::params::service_provider
+  Boolean $manage_service                  = true,
+  Boolean $enabled                         = true,
+  $package_ensure                          = 'present',
+  String[1] $config_file_name              = 'account-server.conf',
+  Swift::ServiceProvider $service_provider = $::swift::params::service_provider
 ) inherits swift::params {
 
   include swift::deps
diff --git a/manifests/storage/container.pp b/manifests/storage/container.pp
index fb6318a5..a925380d 100644
--- a/manifests/storage/container.pp
+++ b/manifests/storage/container.pp
@@ -32,12 +32,12 @@
 #    Defaults to $::swift::params::service_provider.
 #
 class swift::storage::container(
-  $manage_service     = true,
-  $enabled            = true,
-  $package_ensure     = 'present',
-  $allowed_sync_hosts = ['127.0.0.1'],
-  $config_file_name   = 'container-server.conf',
-  $service_provider   = $::swift::params::service_provider
+  Boolean $manage_service                  = true,
+  Boolean $enabled                         = true,
+  $package_ensure                          = 'present',
+  Array[String[1]] $allowed_sync_hosts     = ['127.0.0.1'],
+  String[1] $config_file_name              = 'container-server.conf',
+  Swift::ServiceProvider $service_provider = $::swift::params::service_provider
 ) inherits swift::params {
 
   include swift::deps
diff --git a/manifests/storage/drive_audit.pp b/manifests/storage/drive_audit.pp
index f84267b0..1577a639 100644
--- a/manifests/storage/drive_audit.pp
+++ b/manifests/storage/drive_audit.pp
@@ -121,7 +121,7 @@ class swift::storage::drive_audit(
   $log_to_console                           = $facts['os_service_default'],
   $unmount_failed_device                    = $facts['os_service_default'],
   Hash[String[1], String[1]] $regex_pattern = {},
-  $purge_config                             = false,
+  Boolean $purge_config                     = false,
 ) inherits swift::params {
 
   include swift::deps
diff --git a/manifests/storage/generic.pp b/manifests/storage/generic.pp
index 97382fdc..c7a7c6d2 100644
--- a/manifests/storage/generic.pp
+++ b/manifests/storage/generic.pp
@@ -34,12 +34,12 @@
 #  Requires Class[swift::storage]
 #
 define swift::storage::generic(
-  Swift::StorageServerType $type = $name,
-  Boolean $manage_service        = true,
-  Boolean $enabled               = true,
-  $package_ensure                = 'present',
-  $config_file_name              = "${name}-server.conf",
-  $service_provider              = $::swift::params::service_provider
+  Swift::StorageServerType $type           = $name,
+  Boolean $manage_service                  = true,
+  Boolean $enabled                         = true,
+  $package_ensure                          = 'present',
+  String[1] $config_file_name              = "${name}-server.conf",
+  Swift::ServiceProvider $service_provider = $::swift::params::service_provider
 ) {
 
   include swift::deps
diff --git a/manifests/storage/object.pp b/manifests/storage/object.pp
index 2133aeab..aaf130cf 100644
--- a/manifests/storage/object.pp
+++ b/manifests/storage/object.pp
@@ -27,11 +27,11 @@
 #    Defaults to $::swift::params::service_provider.
 #
 class swift::storage::object(
-  $manage_service   = true,
-  $enabled          = true,
-  $package_ensure   = 'present',
-  $config_file_name = 'object-server.conf',
-  $service_provider = $::swift::params::service_provider
+  Boolean $manage_service                  = true,
+  Boolean $enabled                         = true,
+  $package_ensure                          = 'present',
+  String[1] $config_file_name              = 'object-server.conf',
+  Swift::ServiceProvider $service_provider = $::swift::params::service_provider
 ) inherits swift::params {
 
   include swift::deps
diff --git a/types/serviceprovider.pp b/types/serviceprovider.pp
new file mode 100644
index 00000000..d43163fc
--- /dev/null
+++ b/types/serviceprovider.pp
@@ -0,0 +1 @@
+type Swift::ServiceProvider = Optional[Enum['swiftinit', 'systemd']]