From 9f6b031b95a233d87c48880f46a4d602616aa4ce Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Thu, 18 Jul 2019 20:50:31 -0500 Subject: [PATCH 1/3] [ansible_runner] Add NetworkCredential With network credentials, awx favor's the MachineCredetial info over the NetworkCredential data in the env/password file, so that file is loaded if it exists already, and the data is appended to it only if there isn't data already. --- .../runner/credential/network_credential.rb | 48 +++++ .../credential/network_credential_spec.rb | 186 ++++++++++++++++++ 2 files changed, 234 insertions(+) create mode 100644 lib/ansible/runner/credential/network_credential.rb create mode 100644 spec/lib/ansible/runner/credential/network_credential_spec.rb diff --git a/lib/ansible/runner/credential/network_credential.rb b/lib/ansible/runner/credential/network_credential.rb new file mode 100644 index 00000000000..d0f4313586c --- /dev/null +++ b/lib/ansible/runner/credential/network_credential.rb @@ -0,0 +1,48 @@ +module Ansible + class Runner + class NetworkCredential < Credential + def self.auth_type + "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential" + end + + # Modeled off of awx codebase: + # + # https://github.com/ansible/awx/blob/1242ee2b/awx/main/tasks.py#L1432-L1443 + # + def env_vars + env = { + "ANSIBLE_NET_USERNAME" => auth.userid || "", + "ANSIBLE_NET_PASSWORD" => auth.password || "", + "ANSIBLE_NET_AUTHORIZE" => auth.authorize ? "1" : "0" + } + + env["ANSIBLE_NET_AUTH_PASS"] = auth.become_password || "" if auth.authorize + env["ANSIBLE_NET_SSH_KEYFILE"] = network_ssh_key_file if auth.auth_key + env + end + + def write_config_files + write_password_file + write_network_ssh_key_file if auth.auth_key + end + + private + + SSH_UNLOCK_KEY = "^Enter passphrase for [a-zA-Z0-9\-\/]+\/ssh_key_data:".freeze + def write_password_file + password_data = initialize_password_data + password_data[SSH_UNLOCK_KEY] ||= auth.ssh_key_unlock || "" + File.write(password_file, password_data.to_yaml) + end + + def write_network_ssh_key_file + File.write(network_ssh_key_file, auth.auth_key) + File.chmod(0o0400, network_ssh_key_file) + end + + def network_ssh_key_file + File.join(env_dir, "network_ssh_key") + end + end + end +end diff --git a/spec/lib/ansible/runner/credential/network_credential_spec.rb b/spec/lib/ansible/runner/credential/network_credential_spec.rb new file mode 100644 index 00000000000..e595382abaf --- /dev/null +++ b/spec/lib/ansible/runner/credential/network_credential_spec.rb @@ -0,0 +1,186 @@ +require 'ansible/runner' +require 'ansible/runner/credential' + +RSpec.describe Ansible::Runner::NetworkCredential do + it ".auth_type is the correct Authentication sub-class" do + expect(described_class.auth_type).to eq("ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential") + end + + context "with a credential object" do + around do |example| + Dir.mktmpdir("ansible-runner-credential-test") do |dir| + @base_dir = dir + example.run + end + end + + let(:auth) { FactoryBot.create(:embedded_ansible_network_credential, auth_attributes) } + let(:cred) { described_class.new(auth.id, @base_dir) } + let(:key_file) { File.join(@base_dir, "env", "network_ssh_key") } + let(:auth_attributes) do + { + :userid => "manageiq-network", + :password => "network_secret" + } + end + + describe "#command_line" do + it "returns an empty hash" do + expect(cred.command_line).to eq({}) + end + end + + # Modeled off of awx codebase: + # + # https://github.com/ansible/awx/blob/1242ee2b/awx/main/tasks.py#L1432-L1443 + # + describe "#env_vars" do + it "sets ANSIBLE_NET_USERNAME, ANSIBLE_NET_PASSWORD, and ANSIBLE_NET_AUTHORIZE" do + expected = { + "ANSIBLE_NET_USERNAME" => "manageiq-network", + "ANSIBLE_NET_PASSWORD" => "network_secret", + "ANSIBLE_NET_AUTHORIZE" => "0", + } + expect(cred.env_vars).to eq(expected) + end + + context "with an auth_key" do + let(:auth_attributes) do + { + :userid => "", + :password => "", + :auth_key => "key_data" + } + end + + it "sets ANSIBLE_NET_SSH_KEYFILE to the network_ssh_key_file location" do + expected = { + "ANSIBLE_NET_USERNAME" => "", + "ANSIBLE_NET_PASSWORD" => "", + "ANSIBLE_NET_AUTHORIZE" => "0", + "ANSIBLE_NET_SSH_KEYFILE" => key_file + } + expect(cred.env_vars).to eq(expected) + end + end + + context "with authorize set" do + let(:auth_attributes) do + { + :userid => "user", + :password => "pass", + :options => { :authorize => true } + } + end + + it "sets ANSIBLE_NET_AUTHORIZE to '1'" do + expected = { + "ANSIBLE_NET_USERNAME" => "user", + "ANSIBLE_NET_PASSWORD" => "pass", + "ANSIBLE_NET_AUTHORIZE" => "1", + "ANSIBLE_NET_AUTH_PASS" => "" + } + expect(cred.env_vars).to eq(expected) + end + + it "defines ANSIBLE_NET_AUTH_PASS if it is present" do + auth.update!(:become_password => "auth_pass") + expected = { + "ANSIBLE_NET_USERNAME" => "user", + "ANSIBLE_NET_PASSWORD" => "pass", + "ANSIBLE_NET_AUTHORIZE" => "1", + "ANSIBLE_NET_AUTH_PASS" => "auth_pass" + } + expect(cred.env_vars).to eq(expected) + end + end + end + + describe "#extra_vars" do + it "returns an empty hash" do + expect(cred.extra_vars).to eq({}) + end + end + + describe "#write_config_files" do + let(:password_file) { File.join(@base_dir, "env", "passwords") } + + def password_hash + YAML.load_file(password_file) + end + + context "with an auth_key" do + let(:auth_attributes) { { :auth_key => "key_data" } } + + it "writes the network_ssh_key_file" do + cred.write_config_files + expect(File.read(key_file)).to eq("key_data") + expect(File.stat(key_file).mode).to eq(0o100400) + end + end + + context "without an auth_key" do + it "writes the network_ssh_key_file" do + cred.write_config_files + expect(File.exist?(key_file)).to be_falsey + end + end + + context "with authorize set" do + let(:ssh_unlock_key) { "^Enter passphrase for [a-zA-Z0-9\-\/]+\/ssh_key_data:" } + let(:auth_attributes) do + { + :userid => "user", + :password => "pass", + :auth_key_password => "key_pass", + :options => { :authorize => true } + } + end + + it "writes the password file" do + cred.write_config_files + + expect(password_hash).to eq(ssh_unlock_key => "key_pass") + end + + it "defaults auth_key_password to ''" do + auth.update!(:auth_key_password => nil) + cred.write_config_files + + expect(password_hash).to eq(ssh_unlock_key => "") + end + + context "and an existing password file" do + def existing_env_password_file(data) + cred # initialize the dir + File.write(password_file, data.to_yaml) + end + + it "without the existing ssh unlock key adds the password to the file" do + existing_data = { + "^SSH [pP]assword:" => "hunter2", + "^BECOME [pP]assword:" => "hunter3" + } + expected_data = existing_data.merge(ssh_unlock_key => "key_pass") + existing_env_password_file(existing_data) + cred.write_config_files + + expect(password_hash).to eq(expected_data) + end + + it "with the existing data including the ssh unlock does nothing" do + existing_data = { + "^SSH [pP]assword:" => "hunter2", + "^BECOME [pP]assword:" => "hunter3", + ssh_unlock_key => "hunter4...really?" + } + existing_env_password_file(existing_data) + cred.write_config_files + + expect(password_hash).to eq(existing_data) + end + end + end + end + end +end From 5e66c85c3041ad4547fabe3c575eb22ff509a1f6 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Thu, 18 Jul 2019 20:52:56 -0500 Subject: [PATCH 2/3] [ansible_runner] AutomationManager::NetworkCredential fixes - Adds missing closing paren in comment - Re-aligns assignments in .params_to_attributes --- .../automation_manager/network_credential.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/manageiq/providers/embedded_ansible/automation_manager/network_credential.rb b/app/models/manageiq/providers/embedded_ansible/automation_manager/network_credential.rb index faa3b574adf..4238de66139 100644 --- a/app/models/manageiq/providers/embedded_ansible/automation_manager/network_credential.rb +++ b/app/models/manageiq/providers/embedded_ansible/automation_manager/network_credential.rb @@ -16,7 +16,7 @@ class ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential # rubocop:disable Layout/AlignHash # # looks better to align the nested keys to the same distance, instead of - # scope just for the hash in question (which is what rubocop does. + # scope just for the hash in question (which is what rubocop does). EXTRA_ATTRIBUTES = { :authorize => { :type => :boolean, @@ -62,9 +62,9 @@ def self.display_name(number = 1) def self.params_to_attributes(params) attrs = params.dup - attrs[:auth_key] = attrs.delete(:ssh_key_data) - attrs[:auth_key_password] = attrs.delete(:ssh_key_unlock) - attrs[:become_password] = attrs.delete(:authorize_password) + attrs[:auth_key] = attrs.delete(:ssh_key_data) + attrs[:auth_key_password] = attrs.delete(:ssh_key_unlock) + attrs[:become_password] = attrs.delete(:authorize_password) if attrs[:authorize] attrs[:options] = { :authorize => attrs.delete(:authorize) } From bcdd800845a4e701f34a18d941a7a3e7988cbd40 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Thu, 18 Jul 2019 21:14:56 -0500 Subject: [PATCH 3/3] [ansible_runner][MachineCredential] Clobber existing only When creating a env/password file in MachineCredential, only inject new values into the file if it already exists, but don't overwrite data that wouldn't otherwise be re-written. Example: if an key for unlocking an encrypted SSH key exists, don't delete that data while adding the other existing data. A helper method, `.initialize_password_data` was added to the top level `Ansible::Runner::Credential` class to share loading the existing data or creating a new hash. --- lib/ansible/runner/credential.rb | 4 +++ .../runner/credential/machine_credential.rb | 12 ++++--- .../credential/machine_credential_spec.rb | 34 +++++++++++++++++++ 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/lib/ansible/runner/credential.rb b/lib/ansible/runner/credential.rb index 30813a97f88..b62784a07c9 100644 --- a/lib/ansible/runner/credential.rb +++ b/lib/ansible/runner/credential.rb @@ -36,6 +36,10 @@ def write_config_files private + def initialize_password_data + File.exist?(password_file) ? YAML.load_file(password_file) : {} + end + def password_file File.join(env_dir, "passwords") end diff --git a/lib/ansible/runner/credential/machine_credential.rb b/lib/ansible/runner/credential/machine_credential.rb index 35d36948332..c131d44ab87 100644 --- a/lib/ansible/runner/credential/machine_credential.rb +++ b/lib/ansible/runner/credential/machine_credential.rb @@ -29,12 +29,14 @@ def become_args } end + SSH_KEY = "^SSH [pP]assword:".freeze + BECOME_KEY = "^BECOME [pP]assword:".freeze + SSH_UNLOCK_KEY = "^Enter passphrase for [a-zA-Z0-9\-\/]+\/ssh_key_data:".freeze def write_password_file - password_hash = { - "^SSH [pP]assword:" => auth.password, - "^BECOME [pP]assword:" => auth.become_password, - "^Enter passphrase for [a-zA-Z0-9\-\/]+\/ssh_key_data:" => auth.ssh_key_unlock - }.delete_blanks + password_hash = initialize_password_data + password_hash[SSH_KEY] = auth.password if auth.password + password_hash[BECOME_KEY] = auth.become_password if auth.become_password + password_hash[SSH_UNLOCK_KEY] = auth.ssh_key_unlock if auth.ssh_key_unlock File.write(password_file, password_hash.to_yaml) if password_hash.present? end diff --git a/spec/lib/ansible/runner/credential/machine_credential_spec.rb b/spec/lib/ansible/runner/credential/machine_credential_spec.rb index 22be280497f..8108dcaedc1 100644 --- a/spec/lib/ansible/runner/credential/machine_credential_spec.rb +++ b/spec/lib/ansible/runner/credential/machine_credential_spec.rb @@ -101,6 +101,40 @@ def password_hash expect(password_hash["^SSH [pP]assword:"]).to eq(password) end + + context "with an existing password_file" do + let(:ssh_unlock_key) { "^Enter passphrase for [a-zA-Z0-9\-\/]+\/ssh_key_data:" } + def existing_env_password_file(data) + cred # initialize the dir + File.write(password_file, data.to_yaml) + end + + it "clobbers existing ssh key unlock keys" do + existing_data = { ssh_unlock_key => "hunter2" } + expected_data = { + "^SSH [pP]assword:" => "secret", + "^BECOME [pP]assword:" => "othersecret", + ssh_unlock_key => "keypass" + } + existing_env_password_file(existing_data) + cred.write_config_files + + expect(password_hash).to eq(expected_data) + end + + it "appends data if not setting ssh_unlock_key" do + auth.update!(:auth_key_password => nil) + existing_data = { ssh_unlock_key => "hunter2" } + added_data = { + "^SSH [pP]assword:" => "secret", + "^BECOME [pP]assword:" => "othersecret" + } + existing_env_password_file(existing_data) + cred.write_config_files + + expect(password_hash).to eq(existing_data.merge(added_data)) + end + end end end end