Skip to content

Commit

Permalink
Fixes #23210 - Handle PuppetCA tokens
Browse files Browse the repository at this point in the history
In a new SmartProxy PuppetCA autosigning variant
tokens get returned that need to be provisioned on
the host.
  • Loading branch information
Julian Todt committed Jul 9, 2018
1 parent d666c6c commit 7075e12
Show file tree
Hide file tree
Showing 12 changed files with 172 additions and 63 deletions.
9 changes: 9 additions & 0 deletions app/models/concerns/hostext/puppetca.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Hostext
module Puppetca
extend ActiveSupport::Concern

included do
has_one :puppetca_token, :foreign_key => :host_id, :dependent => :destroy, :inverse_of => :host, :class_name => 'Token::Puppetca'
end
end
end
2 changes: 1 addition & 1 deletion app/models/concerns/hostext/token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module Token
extend ActiveSupport::Concern

included do
has_one :token, :foreign_key => :host_id, :dependent => :destroy
has_one :token, :foreign_key => :host_id, :dependent => :destroy, :inverse_of => :host, :class_name => 'Token::Build'

scope :for_token, ->(token) { joins(:token).where(:tokens => { :value => token }).where("expires >= ?", Time.now.utc.to_s(:db)).select('hosts.*') }
scope :for_token_when_built, ->(token) { joins(:token).where(:tokens => { :value => token }).select('hosts.*') }
Expand Down
30 changes: 19 additions & 11 deletions app/models/concerns/orchestration/puppetca.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,20 @@ def resetCertname
# Adds the host's name to the autosign.conf file
def setAutosign
logger.info "Adding autosign entry for #{name}"
puppetca.set_autosign certname
response = puppetca.set_autosign certname
# return if puppetca is using basic autosigning
return response if response.in? [true, false]
unless response.is_a?(Hash) && response['generated_token'].present?
logger.warn "Received an unexpected smart proxy response: #{response}"
return false
end
self.create_puppetca_token value: response['generated_token']
end

# Removes the host's name from the autosign.conf file
def delAutosign
logger.info "Delete the autosign entry for #{name}"
self.puppetca_token.destroy! if self.puppetca_token.present?
puppetca.del_autosign certname
end

Expand All @@ -57,15 +65,15 @@ def queue_puppetca
end

def queue_puppetca_certname_reset
queue.create(:name => _("Reset PuppetCA certname for %s") % self, :priority => 49,
:action => [self, :resetCertname])
post_queue.create(:name => _("Reset PuppetCA certname for %s") % self, :priority => 49,
:action => [self, :resetCertname])
end

def queue_puppetca_create
queue.create(:name => _("Cleanup PuppetCA certificates for %s") % self, :priority => 51,
:action => [self, :delCertificate])
queue.create(:name => _("Enable PuppetCA autosigning for %s") % self, :priority => 55,
:action => [self, :setAutosign])
post_queue.create(:name => _("Cleanup PuppetCA certificates for %s") % self, :priority => 51,
:action => [self, :delCertificate])
post_queue.create(:name => _("Enable PuppetCA autosigning for %s") % self, :priority => 55,
:action => [self, :setAutosign])
end

def queue_puppetca_update
Expand All @@ -88,14 +96,14 @@ def queue_puppetca_update
def queue_puppetca_destroy
return unless puppetca? && errors.empty?
return unless Setting[:manage_puppetca]
queue.create(:name => _("Delete PuppetCA certificates for %s") % self, :priority => 59,
:action => [self, :delCertificate])
post_queue.create(:name => _("Delete PuppetCA certificates for %s") % self, :priority => 59,
:action => [self, :delCertificate])
queue_puppetca_autosign_destroy
true
end

def queue_puppetca_autosign_destroy
queue.create(:name => _("Disable PuppetCA autosigning for %s") % self, :priority => 50,
:action => [self, :delAutosign])
post_queue.create(:name => _("Disable PuppetCA autosigning for %s") % self, :priority => 50,
:action => [self, :delAutosign])
end
end
3 changes: 2 additions & 1 deletion app/models/host/managed.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class Host::Managed < Host::Base
include Hostext::SmartProxy
include Hostext::Token
include Hostext::OperatingSystem
include Hostext::Puppetca
include SelectiveClone
include HostInfoExtensions
include HostParams
Expand Down Expand Up @@ -88,7 +89,7 @@ class Jail < ::Safemode::Jail
:provision_interface, :interfaces, :bond_interfaces, :bridge_interfaces, :interfaces_with_identifier,
:managed_interfaces, :facts, :facts_hash, :root_pass, :sp_name, :sp_ip, :sp_mac, :sp_subnet, :use_image,
:multiboot, :jumpstart_path, :install_path, :miniroot, :medium, :bmc_nic, :templates_used, :owner, :owner_type,
:ssh_authorized_keys, :pxe_loader
:ssh_authorized_keys, :pxe_loader, :puppetca_token
end

scope :recent, lambda { |interval = Setting[:outofsync_interval]|
Expand Down
2 changes: 1 addition & 1 deletion app/models/token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class Token < ApplicationRecord
validates_lengths_from_database
belongs_to_host :foreign_key => :host_id

validates :value, :host_id, :expires, :presence => true
validates :value, :host_id, :presence => true

class Jail < ::Safemode::Jail
allow :host, :value, :expires, :nil?
Expand Down
3 changes: 3 additions & 0 deletions app/models/token/build.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Token::Build < ::Token
validates :expires, presence: true
end
3 changes: 3 additions & 0 deletions app/models/token/puppetca.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Token::Puppetca < ::Token
validates :value, uniqueness: true
end
19 changes: 19 additions & 0 deletions db/migrate/20180613100703_add_type_to_token.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
class AddTypeToToken < ActiveRecord::Migration[5.1]
def up
remove_foreign_key :tokens, :column => :host_id if foreign_key_exists?(:tokens, { :name => "tokens_host_id_fk" })
remove_index :tokens, :host_id if index_exists? :tokens, :host_id # was unique
add_index :tokens, :host_id
add_foreign_key :tokens, :hosts, :name => "tokens_host_id_fk" unless foreign_key_exists?(:tokens, { :name => "tokens_host_id_fk" })
add_column :tokens, :type, :string, default: 'Token::Build', null: false, index: true
change_column :tokens, :value, :string, limit: 900
end

def down
change_column :tokens, :value, :string, limit: 255
remove_column :tokens, :type
remove_foreign_key :tokens, :column => :host_id if foreign_key_exists?(:tokens, { :name => "tokens_host_id_fk" })
remove_index :tokens, :host_id if index_exists? :tokens, :host_id
add_index :tokens, :host_id, :unique => true
add_foreign_key :tokens, :hosts, :name => "tokens_host_id_fk" unless foreign_key_exists?(:tokens, { :name => "tokens_host_id_fk" })
end
end
68 changes: 55 additions & 13 deletions test/models/orchestration/puppetca_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def teardown

test 'should queue puppetca autosigning' do
assert_valid host
tasks = host.queue.all.sort.map(&:name)
tasks = host.post_queue.all.sort.map(&:name)
assert_equal tasks[0], "Cleanup PuppetCA certificates for #{host}"
assert_equal tasks[1], "Enable PuppetCA autosigning for #{host}"
assert_equal 2, tasks.size
Expand Down Expand Up @@ -53,13 +53,13 @@ def teardown

setup do
@host = host
@host.queue.clear
@host.post_queue.clear
@host.build = true
@host.save!
end

test 'should queue puppetca autosigning' do
tasks = @host.queue.all.sort.map(&:name)
tasks = @host.post_queue.all.sort.map(&:name)
assert_equal tasks[0], "Disable PuppetCA autosigning for #{host}"
assert_equal tasks[1], "Cleanup PuppetCA certificates for #{host}"
assert_equal tasks[2], "Enable PuppetCA autosigning for #{host}"
Expand All @@ -72,11 +72,11 @@ def teardown

test 'should reset certname when changing from hostname to uuid' do
assert_valid host
host.queue.clear
host.post_queue.clear
Setting[:use_uuid_for_certificates] = true
host.build = true
host.save!
tasks = host.queue.all.sort.map(&:name)
tasks = host.post_queue.all.sort.map(&:name)
assert_equal tasks[0], "Disable PuppetCA autosigning for #{host}"
assert_equal tasks[1], "Cleanup PuppetCA certificates for #{host}"
assert_equal tasks[2], "Enable PuppetCA autosigning for #{host}"
Expand All @@ -88,11 +88,11 @@ def teardown
test 'should reset certname when changing from uuid to hostname' do
Setting[:use_uuid_for_certificates] = true
assert_valid host
host.queue.clear
host.post_queue.clear
Setting[:use_uuid_for_certificates] = false
host.build = true
host.save!
tasks = host.queue.all.sort.map(&:name)
tasks = host.post_queue.all.sort.map(&:name)
assert_equal tasks[0], "Reset PuppetCA certname for #{host}"
assert_equal tasks[1], "Disable PuppetCA autosigning for #{host}"
assert_equal tasks[2], "Cleanup PuppetCA certificates for #{host}"
Expand All @@ -106,13 +106,13 @@ def teardown

setup do
@host = host
@host.queue.clear
@host.post_queue.clear
@host.build = false
@host.save!
end

test 'should remove autosign entry for host' do
tasks = @host.queue.all.sort.map(&:name)
tasks = @host.post_queue.all.sort.map(&:name)
assert_equal tasks[0], "Disable PuppetCA autosigning for #{host}"
assert_equal 1, tasks.size
end
Expand All @@ -123,10 +123,10 @@ def teardown

test 'should not queue anything if build mode is not changed' do
assert_valid host
host.queue.clear
host.post_queue.clear
host.comment = "updated"
host.save!
assert_equal 0, host.queue.all.size
assert_equal 0, host.post_queue.all.size
end
end

Expand All @@ -135,13 +135,55 @@ def teardown

test 'should queue puppetca destroy' do
assert_valid host
host.queue.clear
host.post_queue.clear
host.send(:queue_puppetca_destroy)
tasks = host.queue.all.sort.map(&:name)
tasks = host.post_queue.all.sort.map(&:name)
assert_equal tasks[0], "Disable PuppetCA autosigning for #{host}"
assert_equal tasks[1], "Delete PuppetCA certificates for #{host}"
assert_equal 2, tasks.size
end
end

context 'handles smart proxy responses correctly' do
let(:host) { FactoryBot.create(:host, :managed, :with_puppet_ca, :build => true) }

setup do
@host = host
@host.send(:initialize_puppetca)
end

test 'when it uses basic autosigning' do
@host.puppetca.stubs(:set_autosign).with(@host.certname).returns(true)
assert @host.send(:setAutosign)
assert_nil @host.puppetca_token
end

test 'when autosigning fails' do
@host.puppetca.stubs(:set_autosign).with(@host.certname).returns(false)
refute @host.send(:setAutosign)
assert_nil @host.puppetca_token
end

test 'when using token based autosigning' do
spresponse = { 'generated_token' => 'foo42' }
@host.puppetca.stubs(:set_autosign).with(@host.certname).returns(spresponse)
assert @host.send(:setAutosign)
assert_valid @host.puppetca_token
assert_equal @host.puppetca_token.value, 'foo42'
end

test 'when it gets an invalid hash response' do
spresponse = { 'not_a_token' => '' }
@host.puppetca.stubs(:set_autosign).with(@host.certname).returns(spresponse)
refute @host.send(:setAutosign)
assert_nil @host.puppetca_token
end

test 'when it gets an invalid nil response' do
@host.puppetca.stubs(:set_autosign).with(@host.certname).returns(nil)
refute @host.send(:setAutosign)
assert_nil @host.puppetca_token
end
end
end
end
38 changes: 38 additions & 0 deletions test/models/token/build_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
require 'test_helper'

class Token::BuildTest < ActiveSupport::TestCase
should validate_presence_of(:expires)

let(:host) { FactoryBot.create(:host) }

test "a host can create a token" do
host.create_token(:value => "aaaaaa", :expires => Time.now.utc)
assert_equal Token.first.value, "aaaaaa"
assert_equal Token.first.host_id, host.id
end

test "a host can delete its token" do
host.create_token(:value => 'aaaaaa', :expires => Time.now.utc + 1.minute)
assert_instance_of Token::Build, host.token
host.token = nil
assert Token.where(:value => 'aaaaaa', :host_id => host.id).empty?
end

test "a host cannot delete tokens for other hosts" do
host2 = FactoryBot.create(:host)
host.create_token(:value => 'aaaaaa', :expires => Time.now.utc + 1.minute)
host2.create_token(:value => 'bbbbbb', :expires => Time.now.utc + 1.minute)
assert_equal Token.all.size, 2
host.token = nil
assert_equal Token.all.size, 1
end

test "not all expired tokens should be removed" do
host2 = FactoryBot.create(:host)
host.create_token(:value => 'aaaaaa', :expires => Time.now.utc + 1.minute)
host2.create_token(:value => 'bbbbbb', :expires => Time.now.utc - 1.minute)
assert_equal 2, Token.count
host.expire_token
assert_equal 1, Token.count
end
end
22 changes: 22 additions & 0 deletions test/models/token/puppetca_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
require 'test_helper'

class Token::PuppetcaTest < ActiveSupport::TestCase
should validate_uniqueness_of(:value)

let(:host) { FactoryBot.create(:host) }

test "a host can create a puppetca-token" do
host.create_puppetca_token value: 'foo.bar.baz'
assert_instance_of Token::Puppetca, host.puppetca_token
assert_equal Token::Puppetca.first.host_id, host.id
assert_equal 'foo.bar.baz', host.puppetca_token.value
end

test "a host can delete its puppetca-token" do
host.create_puppetca_token value: 'aaaa'
assert_equal host.puppetca_token.value, 'aaaa'
host.puppetca_token = nil
assert_nil host.puppetca_token
assert_equal Token::Puppetca.all, []
end
end
36 changes: 0 additions & 36 deletions test/models/token_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,44 +2,8 @@

class TokenTest < ActiveSupport::TestCase
should validate_presence_of(:value)
should validate_presence_of(:expires)
should validate_presence_of(:host_id)

test "a host can create a token" do
h = FactoryBot.create(:host)
h.create_token(:value => "aaaaaa", :expires => Time.now.utc)
assert_equal Token.first.value, "aaaaaa"
assert_equal Token.first.host_id, h.id
end

test "a host can delete its token" do
h = FactoryBot.create(:host)
h.create_token(:value => "aaaaaa", :expires => Time.now.utc + 1.minute)
assert_instance_of Token, h.token
h.token = nil
assert Token.where(:value => "aaaaaa", :host_id => h.id).empty?
end

test "a host cannot delete tokens for other hosts" do
h1 = FactoryBot.create(:host)
h2 = FactoryBot.create(:host)
h1.create_token(:value => "aaaaaa", :expires => Time.now.utc + 1.minute)
h2.create_token(:value => "bbbbbb", :expires => Time.now.utc + 1.minute)
assert_equal Token.all.size, 2
h1.token = nil
assert_equal Token.all.size, 1
end

test "not all expired tokens should be removed" do
h1 = FactoryBot.create(:host)
h2 = FactoryBot.create(:host)
h1.create_token(:value => "aaaaaa", :expires => Time.now.utc + 1.minute)
h2.create_token(:value => "bbbbbb", :expires => Time.now.utc - 1.minute)
assert_equal 2, Token.count
h1.expire_token
assert_equal 1, Token.count
end

test "token jail test" do
allowed = [:host, :value, :expires, :nil?]
allowed.each do |m|
Expand Down

0 comments on commit 7075e12

Please sign in to comment.