Merge pull request #1673 from bwitt/geo-network-rework

Reverse geo `networks` param hash

Author: bastelfreak

REFERENCE.md

@@ -1778,28 +1778,30 @@ nginx::resource::geo { 'client_network':
   proxy_recursive => false,
   proxies         => [ '192.168.99.99' ],
   networks        => {
-    '10.0.0.0/8'     => 'intra',
-    '172.16.0.0/12'  => 'intra',
-    '192.168.0.0/16' => 'intra',
+    'intra' => ['10.0.0.0/8', '172.16.0.0/12', '192.168.0.0/16'],
   }
 }
 ```
 
 ##### Hiera usage
 
 ```puppet
+# Define network lists that can be reused
+my_internal_networks: &internal_nets
+  - '10.0.0.0/8'
+  - '172.16.0.0/12'
+  - '192.168.0.0/16'
+
 nginx::geo_mappings:
   client_network:
     ensure: present
     ranges: false
     default: 'extra'
     proxy_recursive: false
     proxies:
-       - 192.168.99.99
+      - 192.168.99.99
     networks:
-      '10.0.0.0/8': 'intra'
-      '172.16.0.0/12': 'intra'
-      '192.168.0.0/16': 'intra'
+      intra: *internal_nets
 ```
 
 #### Parameters
@@ -1817,9 +1819,9 @@ The following parameters are available in the `nginx::resource::geo` defined typ
 
 ##### <a name="-nginx--resource--geo--networks"></a>`networks`
 
-Data type: `Hash`
+Data type: `Hash[String[1], Array[String[1]]]`
 
-Hash of geo lookup keys and resultant values
+Hash where keys are geo result values and values are network CIDR arrays.
 
 ##### <a name="-nginx--resource--geo--default"></a>`default`
 

manifests/resource/geo.pp

@@ -1,7 +1,7 @@
 # @summary Create a new geo mapping entry for NGINX
 #
 # @param networks
-#    Hash of geo lookup keys and resultant values
+#    Hash where keys are geo result values and values are network CIDR arrays.
 #
 # @param default
 #    Sets the resulting value if the source value fails to match any of the
@@ -35,27 +35,29 @@
 #     proxy_recursive => false,
 #     proxies         => [ '192.168.99.99' ],
 #     networks        => {
-#       '10.0.0.0/8'     => 'intra',
-#       '172.16.0.0/12'  => 'intra',
-#       '192.168.0.0/16' => 'intra',
+#       'intra' => ['10.0.0.0/8', '172.16.0.0/12', '192.168.0.0/16'],
 #     }
 #   }
 #
 # @example Hiera usage
+#   # Define network lists that can be reused
+#   my_internal_networks: &internal_nets
+#     - '10.0.0.0/8'
+#     - '172.16.0.0/12'
+#     - '192.168.0.0/16'
+#
 #   nginx::geo_mappings:
 #     client_network:
 #       ensure: present
 #       ranges: false
 #       default: 'extra'
 #       proxy_recursive: false
 #       proxies:
-#          - 192.168.99.99
+#         - 192.168.99.99
 #       networks:
-#         '10.0.0.0/8': 'intra'
-#         '172.16.0.0/12': 'intra'
-#         '192.168.0.0/16': 'intra'
+#         intra: *internal_nets
 define nginx::resource::geo (
-  Hash $networks,
+  Hash[String[1], Array[String[1]]] $networks,
   Optional[String] $default           = undef,
   Enum['present', 'absent'] $ensure   = 'present',
   Boolean $ranges                     = false,

spec/defines/resource_geo_spec.rb

@@ -22,9 +22,7 @@
         {
           default: 'extra',
           networks: {
-            '172.16.0.0/12'  => 'intra',
-            '192.168.0.0/16' => 'intra',
-            '10.0.0.0/8'     => 'intra',
+            'intra' => ['10.0.0.0/8', '172.16.0.0/12', '192.168.0.0/16'],
           },
           proxies: ['1.2.3.4', '4.3.2.1'],
         }
@@ -71,9 +69,7 @@
               title: 'should contain ordered network directives',
               attr: 'networks',
               value: {
-                '192.168.0.0/16' => 'intra',
-                '172.16.0.0/12'  => 'intra',
-                '10.0.0.0/8'     => 'intra',
+                'intra' => ['10.0.0.0/8', '172.16.0.0/12', '192.168.0.0/16'],
               },
               match: [
                 '  10.0.0.0/8     intra;',
@@ -129,6 +125,59 @@
             it { is_expected.to contain_file("/etc/nginx/conf.d/#{title}-geo.conf").with_ensure('absent') }
           end
         end
+
+        describe 'networks parameter with multiple values' do
+          context 'with multiple geo values' do
+            let :params do
+              {
+                default: 'extra',
+                networks: {
+                  'intra' => ['10.0.0.0/8', '172.16.0.0/12', '192.168.0.0/16'],
+                  'external' => ['8.8.8.0/24'],
+                },
+                proxies: ['1.2.3.4'],
+              }
+            end
+
+            it { is_expected.to contain_file("/etc/nginx/conf.d/#{title}-geo.conf").with_mode('0644') }
+
+            it 'contains network directives for all values' do
+              is_expected.to contain_file("/etc/nginx/conf.d/#{title}-geo.conf").with_content(%r{10\.0\.0\.0/8\s+intra;})
+              is_expected.to contain_file("/etc/nginx/conf.d/#{title}-geo.conf").with_content(%r{172\.16\.0\.0/12\s+intra;})
+              is_expected.to contain_file("/etc/nginx/conf.d/#{title}-geo.conf").with_content(%r{192\.168\.0\.0/16\s+intra;})
+              is_expected.to contain_file("/etc/nginx/conf.d/#{title}-geo.conf").with_content(%r{8\.8\.8\.0/24\s+external;})
+            end
+          end
+
+          context 'with empty networks hash' do
+            let :params do
+              {
+                networks: {},
+              }
+            end
+
+            it { is_expected.to compile.with_all_deps }
+            it { is_expected.to contain_file("/etc/nginx/conf.d/#{title}-geo.conf") }
+          end
+
+          context 'networks are sorted by IP address' do
+            let :params do
+              {
+                networks: {
+                  'external' => ['8.8.8.0/24'],
+                  'intra' => ['10.0.0.0/8', '192.168.0.0/16'],
+                },
+              }
+            end
+
+            it 'outputs networks in ascending IP order' do
+              # 8.8.8.0 < 10.0.0.0 < 192.168.0.0 numerically
+              is_expected.to contain_file("/etc/nginx/conf.d/#{title}-geo.conf").with_content(
+                %r{8\.8\.8\.0/24\s+external;.*10\.0\.0\.0/8\s+intra;.*192\.168\.0\.0/16\s+intra;}m,
+              )
+            end
+          end
+        end
       end
     end
   end

templates/conf.d/geo.erb

@@ -25,8 +25,13 @@ geo <%= @address ? "#{@address} " : '' %>$<%= @name %> {
 <% end -%>
 <% if @networks -%>
 
-  <%- field_width = @networks.inject(0) { |l,(k,v)| k.size > l ? k.size : l } -%>
-  <%- @networks.sort_by{|k,v| IPAddr.new(k.split('-').first).to_i }.each do |key,value| -%>
+  <%-
+    # Expand { 'value' => ['net1', 'net2'] } to { 'net1' => 'value', 'net2' => 'value' }
+    expanded = {}
+    @networks.each { |value, nets| nets.each { |net| expanded[net] = value } }
+    field_width = expanded.inject(0) { |l,(k,v)| k.size > l ? k.size : l }
+  -%>
+  <%- expanded.sort_by{|k,v| IPAddr.new(k.split('-').first).to_i }.each do |key,value| -%>
   <%= sprintf("%-*s", field_width, key) %> <%= value %>;
   <%- end -%>
 <% end -%>