Fixes #23210 - Handle PuppetCA tokens #5730

juliantodt · 2018-06-21T13:42:48Z

In a new SmartProxy PuppetCA autosigning variant tokens get returned that need to be provisioned on the host.
This adds a Token::PuppetCa model; moves the PuppetCa-orchestration form queue to post-queue (we need the host's id to save the token); and handles the response from the SmartProxy correctly (save the token if we get one, don't do anything if we don't).
This is completely backwards-compatible - old SmartProxy versions and the basic autosigning variant will also work with this.

theforeman-bot · 2018-06-21T13:42:56Z

Issues: #23210

houndci-bot · 2018-06-21T13:43:06Z

app/models/concerns/orchestration/puppetca.rb

+    # 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.to_s}"


Lint/StringConversionInInterpolation: Redundant use of Object#to_s in interpolation.

houndci-bot · 2018-06-27T08:48:45Z

test/models/token/build_test.rb

+    assert_equal 1, Token.count
+  end
+end
+


Layout/TrailingBlankLines: 1 trailing blank lines detected.

timogoebel

@juliantodt: Thanks, some comments inline.

timogoebel · 2018-06-25T11:41:59Z

app/models/concerns/orchestration/puppetca.rb

  end

  # Removes the host's name from the autosign.conf file
  def delAutosign
    logger.info "Delete the autosign entry for #{name}"
+    self.puppetca_token.delete if self.puppetca_token.present?


Please use .destroy so that callbacks run.

timogoebel · 2018-06-25T11:42:28Z

app/models/concerns/orchestration/puppetca.rb

@@ -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,


Why do we need to take a look at the post_queue?

To save tokens, we need the host's id which is only available after the host is saved, therefore we need these actions to happen in the post_queue

juliantodt · 2018-06-28T12:19:31Z

Updated, thanks @timogoebel. Tests are now green as well.

timogoebel

@juliantodt: Thanks. Looks good in general.
Could you please add puppetca_token to Host::Managed::Jail so it's allowed in safe mode template rendering? Thanks.

juliantodt · 2018-07-09T19:43:37Z

@timogoebel Thanks for the review, is done.

timogoebel

Looks good to me. Ready to merge pending these PRs:

SmartProxy:
theforeman/smart-proxy#592

Puppet Foreman Proxy:
theforeman/puppet-foreman_proxy#433
theforeman/puppet-foreman_proxy#425

timogoebel · 2018-09-14T08:28:36Z

app/models/token.rb

@@ -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?


This also needs present? for the community templates PR to work.

Good catch. Fixed.

In a new SmartProxy PuppetCA autosigning variant tokens get returned that need to be provisioned on the host.

timogoebel · 2018-09-19T10:17:37Z

Thanks, @juliantodt.

theforeman-bot added Needs testing Not yet reviewed labels Jun 21, 2018

houndci-bot reviewed Jun 21, 2018

View reviewed changes

juliantodt force-pushed the 23210_handlesptokens branch from 37d8ece to b4c8ffe Compare June 21, 2018 13:45

juliantodt mentioned this pull request Jun 21, 2018

Fixes #23211 - Add PuppetCa TokenWhitelisting provider theforeman/smart-proxy#592

Merged

juliantodt force-pushed the 23210_handlesptokens branch from b4c8ffe to 1b31cd1 Compare June 27, 2018 08:48

houndci-bot reviewed Jun 27, 2018

View reviewed changes

test/models/token/build_test.rb Outdated

assert_equal 1, Token.count

end

end

Copy link

houndci-bot Jun 27, 2018

Choose a reason for hiding this comment

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

Layout/TrailingBlankLines: 1 trailing blank lines detected.

juliantodt force-pushed the 23210_handlesptokens branch from 1b31cd1 to 28fb52a Compare June 27, 2018 08:50

timogoebel reviewed Jun 27, 2018

View reviewed changes

juliantodt force-pushed the 23210_handlesptokens branch from 28fb52a to 8ad5c2d Compare June 28, 2018 10:41

juliantodt mentioned this pull request Jul 5, 2018

Refs #23210 - Provision PuppetCA-Token on Host theforeman/community-templates#475

Merged

timogoebel requested changes Jul 9, 2018

View reviewed changes

theforeman-bot added Waiting on contributor and removed Not yet reviewed labels Jul 9, 2018

juliantodt force-pushed the 23210_handlesptokens branch from 8ad5c2d to 7075e12 Compare July 9, 2018 19:42

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Jul 9, 2018

juliantodt force-pushed the 23210_handlesptokens branch from 7075e12 to a18186d Compare July 27, 2018 06:58

juliantodt force-pushed the 23210_handlesptokens branch from a18186d to 2ddb60b Compare August 24, 2018 08:03

timogoebel approved these changes Aug 24, 2018

View reviewed changes

theforeman-bot removed the Needs re-review label Aug 24, 2018

timogoebel added Ready to merge and removed Needs testing labels Aug 27, 2018

timogoebel requested changes Sep 14, 2018

View reviewed changes

theforeman-bot added the Waiting on contributor label Sep 14, 2018

timogoebel removed the Ready to merge label Sep 14, 2018

Fixes #23210 - Handle PuppetCA tokens

29916f1

In a new SmartProxy PuppetCA autosigning variant tokens get returned that need to be provisioned on the host.

juliantodt force-pushed the 23210_handlesptokens branch from 2ddb60b to 29916f1 Compare September 19, 2018 07:43

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Sep 19, 2018

timogoebel approved these changes Sep 19, 2018

View reviewed changes

theforeman-bot removed the Needs re-review label Sep 19, 2018

timogoebel added Ready to merge and removed Needs testing labels Sep 19, 2018

timogoebel merged commit 30face9 into theforeman:develop Sep 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #23210 - Handle PuppetCA tokens #5730

Fixes #23210 - Handle PuppetCA tokens #5730

juliantodt commented Jun 21, 2018

theforeman-bot commented Jun 21, 2018

houndci-bot Jun 21, 2018

houndci-bot Jun 27, 2018

timogoebel left a comment

timogoebel Jun 25, 2018

timogoebel Jun 25, 2018

juliantodt Jun 28, 2018

juliantodt commented Jun 28, 2018

timogoebel left a comment

juliantodt commented Jul 9, 2018

timogoebel left a comment

timogoebel Sep 14, 2018

juliantodt Sep 19, 2018

timogoebel commented Sep 19, 2018

Fixes #23210 - Handle PuppetCA tokens #5730

Fixes #23210 - Handle PuppetCA tokens #5730

Conversation

juliantodt commented Jun 21, 2018

theforeman-bot commented Jun 21, 2018

houndci-bot Jun 21, 2018

Choose a reason for hiding this comment

houndci-bot Jun 27, 2018

Choose a reason for hiding this comment

timogoebel left a comment

Choose a reason for hiding this comment

timogoebel Jun 25, 2018

Choose a reason for hiding this comment

timogoebel Jun 25, 2018

Choose a reason for hiding this comment

juliantodt Jun 28, 2018

Choose a reason for hiding this comment

juliantodt commented Jun 28, 2018

timogoebel left a comment

Choose a reason for hiding this comment

juliantodt commented Jul 9, 2018

timogoebel left a comment

Choose a reason for hiding this comment

timogoebel Sep 14, 2018

Choose a reason for hiding this comment

juliantodt Sep 19, 2018

Choose a reason for hiding this comment

timogoebel commented Sep 19, 2018