Return hash instead of array from API function

Ericsson · Mar 27, 2024 · d57711e · d57711e
1 parent 2b20fab
commit d57711e
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 51 deletions.
diff --git a/REFERENCE.md b/REFERENCE.md
@@ -1225,7 +1225,7 @@ vas::api_fetch("https://host.domain.tld/api/${facts['trusted.certname']}")
 
 Query a remote HTTP-based service for entries to be added to users_allow.
 
-Returns: `Stdlib::Http::Status` If a valid response and contains entries
+Returns: `Hash` Key 'content' with [Array] if API responds. Key 'errors' with [Array[String]] if errors happens.
 
 ##### Examples
 

diff --git a/lib/puppet/functions/vas/api_fetch.rb b/lib/puppet/functions/vas/api_fetch.rb
@@ -6,16 +6,14 @@
   # @param url URL to connect to
   # @param token Token used for authentication
   # @param ssl_verify Whether TLS connections should be verified or not
-  # @return [Stdlib::Http::Status, Array[String]] If a valid response and contains entries
-  # @return [Stdlib::Http::Status, Array[nil]] If a valid response, but no entries
-  # @return [Stdlib::Http::Status, nil] If response is not of SUCCESS status code
-  # @return [0, String] If the query is unable to reach server or other error
+  # @return [Hash] Key 'content' with [Array] if API responds. Key 'errors' with [Array[String]] if errors happens.
   # @example Calling the function
   #   vas::api_fetch("https://host.domain.tld/api/${facts['trusted.certname']}")
   dispatch :api_fetch do
     param 'Stdlib::HTTPUrl', :url
     param 'String[1]', :token
     optional_param 'Boolean', :ssl_verify
+    return_type 'Hash'
   end
 
   def api_fetch(url, token, ssl_verify = false)
@@ -33,20 +31,26 @@ def api_fetch(url, token, ssl_verify = false)
     https.open_timeout = 2
     https.read_timeout = 2
 
+    data = {}
     begin
       response = https.start do |cx|
         cx.request(req)
       end
 
       case response
       when Net::HTTPSuccess
-        return response.code, response.body.split("\n") unless response.body.to_s.empty?
-        [response.code, []]
+        data['content'] = if response.body.empty?
+                            []
+                          else
+                            response.body.split("\n")
+                          end
       else
-        [response.code, nil]
+        (data['errors'] ||= []) << "#{url} returns HTTP code: #{response.code}"
       end
     rescue => error
-      [0, error.message]
+      (data['errors'] ||= []) << "#{url} connection failed: #{error.message}"
     end
+
+    data
   end
 end
diff --git a/manifests/init.pp b/manifests/init.pp
@@ -678,17 +678,16 @@
   } elsif $api_enable == true {
     $api_users_allow_data = vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify)
 
-    case $api_users_allow_data[0] {
-      200,'200': { # api_fetch() returns integer in Puppet 3 and string in Puppet 6
-        # VAS API is configured and responding
-        $manage_users_allow = true
-        $users_allow_entries_real = concat($users_allow_entries, $api_users_allow_data[1])
-      }
-      default: {
-        # VAS API is configured but down. Don't manage users_allow to prevent removal of entries.
-        $manage_users_allow = false
-        warning("VAS API Error. Code: ${api_users_allow_data[0]}, Error: ${api_users_allow_data[1]}")
-      }
+    if $api_users_allow_data['content'] {
+      $manage_users_allow = true
+      $users_allow_entries_real = concat($users_allow_entries, $api_users_allow_data['content'])
+    } else {
+      $manage_users_allow = false
+    }
+
+    if $api_users_allow_data['errors'] {
+      $api_errors = join($api_users_allow_data['errors'], ', ')
+      warning("API Error: ${api_errors}")
     }
   } else {
     $manage_users_allow = true

diff --git a/spec/classes/init_spec.rb b/spec/classes/init_spec.rb
@@ -314,16 +314,17 @@
               )
             end
 
-            context 'and returns 200' do
-              context 'without data' do
+            context 'and queries successfully' do
+              context 'with no return entries' do
                 let(:pre_condition) do
-                  'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { return [200, undef] }'
+                  'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) {
+                    return { content => [] }
+                  }'
                 end
 
                 users_allow_api_nodata_content = <<-END.gsub(%r{^\s+\|}, '')
                   |# This file is being maintained by Puppet.
                   |# DO NOT EDIT
-                  |
                 END
 
                 it {
@@ -344,7 +345,6 @@
                     |# DO NOT EDIT
                     |[email protected]
                     |[email protected]
-                    |
                   END
 
                   it {
@@ -353,9 +353,11 @@
                 end
               end
 
-              context 'with data' do
+              context 'with it returning "[email protected]"' do
                 let(:pre_condition) do
-                  'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { return [200, \'[email protected]\'] }'
+                  'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) {
+                    return { content => ["[email protected]"]}
+                  }'
                 end
 
                 users_allow_api_data_content = <<-END.gsub(%r{^\s+\|}, '')
@@ -392,9 +394,11 @@
               end
             end
 
-            context 'and return non-200 code' do
+            context 'and queries fails' do
               let(:pre_condition) do
-                'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { return [0, undef] }'
+                'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) {
+                  return { error => ["https://host.domain.tld returns HTTP code: 502"] }
+                }'
               end
 
               it {

diff --git a/spec/classes/parameter_spec.rb b/spec/classes/parameter_spec.rb
@@ -918,11 +918,13 @@
           }
         end
         let(:pre_condition) do
-          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { return [200, undef] }'
+          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) {
+            return { content => [] }
+          }'
         end
 
         it do
-          is_expected.to contain_file('vas_users_allow').with_content(header + "\n")
+          is_expected.to contain_file('vas_users_allow').with_content(header)
         end
       end
 
@@ -935,7 +937,9 @@
           }
         end
         let(:pre_condition) do
-          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { return [200, \'[email protected]\'] }'
+          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) {
+            return { content => ["[email protected]"] }
+          }'
         end
 
         it do
@@ -953,11 +957,13 @@
           }
         end
         let(:pre_condition) do
-          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { return [200, undef] }'
+          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) {
+            return { content => [] }
+          }'
         end
 
         it do
-          is_expected.to contain_file('vas_users_allow').with_content(header + "[email protected]\n[email protected]\n\n")
+          is_expected.to contain_file('vas_users_allow').with_content(header + "[email protected]\n[email protected]\n")
         end
       end
 
@@ -971,7 +977,9 @@
           }
         end
         let(:pre_condition) do
-          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { return [200, \'[email protected]\'] }'
+          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) {
+            return { content => ["[email protected]"] }
+          }'
         end
 
         it do
@@ -1007,11 +1015,13 @@
           }
         end
         let(:pre_condition) do
-          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { return [200, undef] }'
+          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) {
+            return { content => [] }
+          }'
         end
 
         it do
-          is_expected.to contain_file('vas_users_allow').with_content(header + "\n")
+          is_expected.to contain_file('vas_users_allow').with_content(header)
         end
       end
 
@@ -1024,7 +1034,9 @@
           }
         end
         let(:pre_condition) do
-          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { return [200, \'[email protected]\'] }'
+          'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) {
+            return { content => "[email protected]" }
+          }'
         end
 
         it do

diff --git a/spec/functions/api_fetch_spec.rb b/spec/functions/api_fetch_spec.rb
@@ -55,55 +55,51 @@
 
       is_expected.to run
         .with_params(url, 'somesecret')
-        .and_return([0, 'execution expired'])
+        .and_return({ 'errors' => ['https://api.example.local/ connection failed: execution expired'] })
     end
 
     it 'returns an array containing http response code and body' do
       response_body = "line1\nline2"
 
       stub_request(:get, url).with(
         headers: headers,
-      )
-                             .to_return(body: response_body, status: 200)
+      ).to_return(body: response_body, status: 200)
 
       is_expected.to run
         .with_params(url, 'somesecret')
-        .and_return(['200', ['line1', 'line2']])
+        .and_return({ 'content' => ['line1', 'line2'] })
     end
 
     it 'returns an array containing http response code and an empty array when response body is empty' do
       response_body = ''
 
       stub_request(:get, url).with(
               headers: headers,
-            )
-                             .to_return(body: response_body, status: 200)
+            ).to_return(body: response_body, status: 200)
 
       is_expected.to run
         .with_params(url, 'somesecret')
-        .and_return(['200', []])
+        .and_return({ 'content' => [] })
     end
 
     it 'returns nil when http response code is not success' do
       stub_request(:get, url).with(
               headers: headers,
-            )
-                             .to_return(body: nil, status: 404)
+            ).to_return(body: nil, status: 404)
 
       is_expected.to run
         .with_params(url, 'somesecret')
-        .and_return(['404', nil])
+        .and_return({ 'errors' => ['https://api.example.local/ returns HTTP code: 404'] })
     end
 
     it 'returns an array containing 0 and error when error occurs' do
       stub_request(:get, url).with(
               headers: headers,
-            )
-                             .and_raise(StandardError.new('error'))
+            ).and_raise(StandardError.new('error'))
 
       is_expected.to run
         .with_params(url, 'somesecret')
-        .and_return([0, 'error'])
+        .and_return({ 'errors' => ['https://api.example.local/ connection failed: error'] })
     end
   end
 end