Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add local mode and remove deprecated http_instance call #16

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions lib/puppet/functions/vault_lookup/lookup.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
Puppet::Functions.create_function(:'vault_lookup::lookup') do
dispatch :lookup do
param 'String', :path
required_param 'String', :path
optional_param 'String', :vault_url
optional_param 'Variant[Boolean, String]', :local_token
optional_param 'String', :local_token_file
end

def lookup(path, vault_url = nil)
def lookup(path, vault_url = nil, local_token = false, local_token_file = '/etc/vault.token')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tuxmaster5000 have you considered in adding in support for Hashicorp environment variables?

I'm not sure of your use case, but I think that adding support similar to what is already present for VAULT_ADDR might be a better approach than expanding the argument list here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far I understood the link, the variable store the token itself. But my idea was to add an option the load the token from an file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggested following support for some of the Hashicorp environment variables because it seems like a good idea to follow some of the standard ways Hashicorp supports reading in the token. If we are going to add support for it as a method argument, it might be a good idea to add support reading it from the environment as well? There is already a pattern for reading in the vault_url from the ENV, and I think it would be good to extend that pattern.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this implementation, the code is setting a default location for the token file to be in /etc/vault.token, but the standard place is often ~/.vault-token. Is this the reason for splitting local_token and local_token_file, so we can set that default location? I'm not sure we need to provide that default here, but I'd like to hear why the code is written that way.

if vault_url.nil?
Puppet.debug 'No Vault address was set on function, defaulting to value from VAULT_ADDR env value'
vault_url = ENV['VAULT_ADDR']
Expand All @@ -19,9 +21,14 @@ def lookup(path, vault_url = nil)
raise Puppet::Error, "Unable to parse a hostname from #{vault_url}" unless uri.hostname

use_ssl = uri.scheme == 'https'
connection = Puppet::Network::HttpPool.http_instance(uri.host, uri.port, use_ssl)
context = if use_ssl
Puppet::SSL::SSLContext.new
else
nil
end
connection = Puppet::Network::HttpPool.connection(uri.host, uri.port, use_ssl: use_ssl, ssl_context: context)
tuxmaster5000 marked this conversation as resolved.
Show resolved Hide resolved

token = get_auth_token(connection)
token = get_auth_token(connection, local_token, local_token_file)

secret_response = connection.get("/v1/#{path}", 'X-Vault-Token' => token)
unless secret_response.is_a?(Net::HTTPOK)
Expand All @@ -40,19 +47,27 @@ def lookup(path, vault_url = nil)

private

def get_auth_token(connection)
response = connection.post('/v1/auth/cert/login', '')
unless response.is_a?(Net::HTTPOK)
message = "Received #{response.code} response code from vault at #{connection.address} for authentication"
raise Puppet::Error, append_api_errors(message, response)
end
def get_auth_token(connection, local_token, local_token_file)
if local_token
begin
token = Puppet::FileSystem.read(local_token_file)
rescue
raise Puppet::Error, "Unable to read #{local_token_file}"
end

begin
token = JSON.parse(response.body)['auth']['client_token']
rescue StandardError
raise Puppet::Error, 'Unable to parse client_token from vault response'
end
else
response = connection.post('/v1/auth/cert/login', '')
unless response.is_a?(Net::HTTPOK)
message = "Received #{response.code} response code from vault at #{connection.address} for authentication"
raise Puppet::Error, append_api_errors(message, response)
end

begin
token = JSON.parse(response.body)['auth']['client_token']
rescue StandardError
raise Puppet::Error, 'Unable to parse client_token from vault response'
end
end
raise Puppet::Error, 'No client_token found' if token.nil?

token
Expand Down