From 2e392e5244e61452db133527a26a77f88e7d8f9a Mon Sep 17 00:00:00 2001 From: Anders Larsson Date: Fri, 15 Mar 2024 13:42:15 +0100 Subject: [PATCH] Return hash instead of array from API function --- REFERENCE.md | 2 +- lib/puppet/functions/vas/api_fetch.rb | 20 +++++++++++------- manifests/init.pp | 21 +++++++++---------- spec/classes/init_spec.rb | 22 ++++++++++++-------- spec/classes/parameter_spec.rb | 30 +++++++++++++++++++-------- spec/functions/api_fetch_spec.rb | 22 ++++++++------------ 6 files changed, 66 insertions(+), 51 deletions(-) diff --git a/REFERENCE.md b/REFERENCE.md index 6eb361f..fbf5f00 100644 --- 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` ##### Examples diff --git a/lib/puppet/functions/vas/api_fetch.rb b/lib/puppet/functions/vas/api_fetch.rb index 5089736..3f5ab74 100644 --- 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 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,6 +31,7 @@ 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) @@ -40,13 +39,18 @@ def api_fetch(url, token, ssl_verify = false) 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 index e833d04..13d63b1 100644 --- 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 index 231253e..9b52802 100644 --- 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 |user1@example.com |user2@example.com - | END it { @@ -353,9 +353,11 @@ end end - context 'with data' do + context 'with it returning "apiuser@example.com"' do let(:pre_condition) do - 'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { return [200, \'apiuser@example.com\'] }' + 'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { + return { content => ["apiuser@example.com"]} + }' 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 index c48ed5b..4a0f5b2 100644 --- 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, \'apiuser@test.ing\'] }' + 'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { + return { content => ["apiuser@test.ing"] } + }' 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 + "user1@test.ing\nuser2@test.ing\n\n") + is_expected.to contain_file('vas_users_allow').with_content(header + "user1@test.ing\nuser2@test.ing\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, \'apiuser@test.ing\'] }' + 'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { + return { content => ["apiuser@test.ing"] } + }' 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, \'apiuser@test.ing\'] }' + 'function vas::api_fetch($api_users_allow_url, $api_token, $api_ssl_verify) { + return { content => "apiuser@test.ing" } + }' end it do diff --git a/spec/functions/api_fetch_spec.rb b/spec/functions/api_fetch_spec.rb index cf978df..ba2ca44 100644 --- a/spec/functions/api_fetch_spec.rb +++ b/spec/functions/api_fetch_spec.rb @@ -55,7 +55,7 @@ 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 @@ -63,12 +63,11 @@ 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 @@ -76,34 +75,31 @@ 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