The Road to Making Puppet Optional

TimoGoebel · March 30, 2020, 1:02pm

Isn’t this already the case?
I’m asking because we might want to gather facts or reports from Compute Resources, e.g. for a VM import and it would be good to do the heavy lifting in core as the proxy doesn’t know anything about compute resources.

tbrisker · March 30, 2020, 2:10pm

Good point. Gathering facts from CRs is definitely a great idea! The intention is that the heavy lifting (i.e. processing) is still done in core (or perhaps in a background task), the proxy is there to handle communications between foreman and external services. I know right now CRs all communicate directly to Foreman but perhaps we should ask ourselves if that is correct.
One way I can see this is a “Facts and Reports” module in a proxy that defines what sources it accepts and passes them along to the Foreman server, so you don’t have to have your external service authenticated directly with foreman for it to work.

ekohl · April 2, 2020, 1:25pm

Currently the Puppet integration scripts (ENC, reports) call directly to Foreman. My idea was to provide a Proxy module where Puppet can call. So currently we have:

Foreman -> Proxy -> Puppet
Puppet -> Foreman

The Proxy already has a setting where the Foreman server is located. It has the credentials to call back. Then we only need to ensure Puppet can talk to the Proxy. Another argument is that I thought it’d be easier to use separate certificates between Proxy <-> Puppet. Perhaps set up a minimal Proxy instance using the Puppet certificates that knows how to securely call back to Foreman.

I haven’t thought it through too much so I’d like some feedback on this.

TimoGoebel · April 2, 2020, 1:33pm

I think this actually makes sense. The enc/report scripts currently hijack the credentials from the smart-proxy. It would be cleaner if they sent their data to smart-proxy.
How would you handle the case if the smart-proxy is up and running and is receiving data but it cannot reach Foreman. Would you make this a synchronous action? Would Foreman poll the smart-proxy for data?

ekohl · April 2, 2020, 3:52pm

That was why I was thinking about a very small service that can actually be multi process/threaded. A blocking API is much easier to deal with. If it runs as the foreman-proxy user it should still have the credentials and config.

ehelms · April 2, 2020, 8:50pm

Do I read this concept correctly that client puppet agents would talk to the smart-proxy instead of Foreman? If yes, is this effectively using the smart-proxy as a reverse proxy?

ekohl · April 2, 2020, 9:09pm

In a way yes. However, I’m not entirely sure how to do the auth. In the Katello setup you would have Foreman and Foreman Proxy both using the Katello default CA. Then if you have a Puppetserver, you’d ideally use the Puppet CA certificates because it allows using the built in HTTP report processor instead of our custom one.

One possible way is to let Foreman Proxy bind on an additional port and serve the Puppet CA. Another is to use Server Name Indication and use separate DNS names. A third I’m not sure that will work is make sure the Puppet CA itself is signed by the Katello root so it’s the same hierarchy.

TimoGoebel · April 3, 2020, 7:26am

Why is that? Isn’t the Katello CA part of the trusted roots of a host? The proxy would then only need to trust the PuppetCA (client auth) and could forward the data to Foreman with whatever CA.

I believe that depends on your point of view. Currently the smart-proxy has to be installed on the same host as the puppetserver so the scripts can use the credentials of the smart-proxy to send data to Foreman. If we send the data via smart-proxy we could decouple this.
If we don’t need to do any processing or normalization, it’s a reverse proxy. Correct. Would we want to do any post-processing on the proxy?

Marek_Hulan · April 6, 2020, 6:55am

The current issue is, the interface defined in FactParser doesn’t well define, what values are expected and how they create objects. E.g. operating systems - it’s not clear what values parsers should derive from facts, in the past, some facters created RHEL as RedHat, some as RHEL. The same aplies to values format e.g. ram. We have a comment we expect the value in MB but I think it would be good to have a proper documentation for the full parser interface.

lzap · April 8, 2020, 1:24pm

This is what I would like to tackle in the fact parser rewrite - to find a common model. Basically to follow facter3 with some changes (facter does some things incorrectly - e.g. reporting boot uptime instead of boot time which does not change every single fact upload). The CFM would not only define which facts should be reported but also contents of these facts, I’d definitely kill all those “123 MiB” crazy formats which are maybe human readable but that’s totally wrong. It should be reported in bytes, machine readable, it’s our UI job to format them for human beings!

ehelms · April 8, 2020, 3:24pm

I wanted to see if I understood the outcome of this correctly. I’ll phrase things as a series of questions:

If I have Foreman without the Puppet plugin, I can “register” hosts to Foreman via Puppet still?
- If yes, does this assume I have a smart proxy present with the Puppet CA feature?
- If yes, is the same true for other host sources (or does it require their plugins)?
- Ansible
- subscription-manager
- Chef
- Salt

ekohl · April 8, 2020, 3:39pm

This has never been true. You only need the Puppet CA feature if you want to provision and deprovision.

Registration happens via fact upload. Currently we use the Puppet feature as authentication and authorization. The Puppetmaster does a POST to the facts endpoint using its certificate. In the certificate is the common name of the smart-proxy. Foreman checks whether this smart proxy exists and has the Puppet feature and then allows uploading facts. Reports happen using the same authentication.

I believe foreman_ansible uploads facts and reports using the same way and abuse the Puppet feature being present to authenticate.

In theory you don’t even need the Puppet feature for that since you can use username/password with sufficient credentials instead.

github.com

theforeman/foreman/blob/develop/app/controllers/concerns/foreman/controller/smart_proxy_auth.rb

require 'resolv'
require 'uri'

module Foreman::Controller::SmartProxyAuth
  extend ActiveSupport::Concern

  module ClassMethods
    def add_smart_proxy_filters(actions, options = {})
      skip_before_action :require_login, :only => actions, :raise => false
      skip_before_action :authorize, :only => actions
      skip_before_action :verify_authenticity_token, :only => actions
      skip_before_action :set_taxonomy, :only => actions, :raise => false
      skip_before_action :session_expiry, :update_activity_time, :only => actions
      before_action(:only => actions) { require_smart_proxy_or_login(options[:features]) }
      attr_reader :detected_proxy

      cattr_accessor :smart_proxy_filter_actions
      self.smart_proxy_filter_actions ||= []
      self.smart_proxy_filter_actions.push(*actions)

This file has been truncated. show original

TimoGoebel · April 8, 2020, 4:11pm

No abuse is needed here. We already have the concept of FactImporters. They can register a smart proxy feature that is allowed to send facts.

ehelms · April 8, 2020, 4:12pm

Does this still work with the Puppet master and smart-proxy are on separate hosts?

Do I have these “registration” workflows correct?

So for Puppet (requires smart-proxy with Puppet feature):

Puppet agent -> Puppet master (impersonating smart proxy) -> Foreman

For Ansible (requires foreman_ansible + smart-proxy with Puppet feature):

Ansible on host -> Smart Proxy -> Foreman

For subscription-manager (requires Katello):

sub-man on host -> Foreman

ekohl · April 8, 2020, 5:19pm

This depends on how you configure it. The installer sets it up for REX:

github.com

theforeman/puppet-foreman_proxy/blob/b2279e386fb5a53254a1ced6967442ade92affb2/manifests/plugin/ansible.pp#L50-L60


file {"${foreman_proxy::config_dir}/ansible.cfg":
  ensure  => file,
  content => template('foreman_proxy/plugin/ansible.cfg.erb'),
  owner   => 'root',
  group   => $::foreman_proxy::user,
  mode    => '0640',
}
~> file { "${::foreman_proxy::dir}/.ansible.cfg":
  ensure => link,
  target => "${foreman_proxy::config_dir}/ansible.cfg",
}

github.com

theforeman/puppet-foreman_proxy/blob/b2279e386fb5a53254a1ced6967442ade92affb2/templates/plugin/ansible.cfg.erb#L8-L12


[callback_foreman]
url = <%= @foreman_url %>
ssl_cert = <%= @foreman_ssl_cert %>
ssl_key = <%= @foreman_ssl_key %>
verify_certs = <%= @foreman_ssl_ca %>

I don’t know how many other users have set up the callback on non-smart proxies.

ekohl · April 8, 2020, 5:19pm

Correct, but I’m not sure if it’s implemented today and must be verified.

TimoGoebel · April 9, 2020, 7:08am

Not with our current implementation. This could be fixed (if it needs fixing) by proxying the requests via smart-proxy as we’ve already discussed.

It works as I’ve used it for foreman_omaha. If you want to verify it yourself:

github.com

theforeman/foreman/blob/bad5f878b6fb275dbc7267930bcd0cb589d9aa66/app/controllers/api/v2/hosts_controller.rb#L18


include ParameterAttributes


wrap_parameters :host, :include => host_params_filter.accessible_attributes(parameter_filter_context) + ['compute_attributes']
include HostsControllerExtension


before_action :find_optional_nested_object, :except => [:facts]
before_action :find_resource, :except => [:index, :create, :facts]
check_permissions_for %w{power boot}
before_action :process_parameter_attributes, :only => %w{update}


add_smart_proxy_filters :facts, :features => proc { Foreman::Plugin.fact_importer_registry.fact_features }


add_scope_for(:index) do |base_scope|
  base_scope.preload([:host_statuses, :compute_resource, :hostgroup, :operatingsystem,
                      :interfaces, :token, :owner, :model, :environment, :location,
                      :organization, :image, :compute_profile, :realm, :architecture,
                      :ptable, :medium, :puppet_proxy, :puppet_ca_proxy])
end


api :GET, "/hosts/", N_("List all hosts")
api :GET, "/hostgroups/:hostgroup_id/hosts", N_("List all hosts for a host group")

github.com

theforeman/foreman/blob/bad5f878b6fb275dbc7267930bcd0cb589d9aa66/app/registries/foreman/plugin/fact_importer_registry.rb#L20


      def register(key, klass, default = false)
        @importers.default = klass.to_s if default
        @importers[key.to_sym] = klass.to_s
      end


      def get(key)
        @importers[key].constantize
      end


      def fact_features
        @importers.values.map { |importer| importer.constantize.authorized_smart_proxy_features }.compact.flatten.uniq
      end
    end
  end
end

github.com

theforeman/foreman_ansible/blob/19d755b9404f8b44c903a3d75cee2f7acf30c070/app/services/foreman_ansible/structured_fact_importer.rb#L11-L13


def self.authorized_smart_proxy_features
  'Ansible'
end

nadjaheitmann · May 28, 2021, 1:09pm

Hi everyone,
I have been working on Hammer and the V2 API of the new Foreman Puppet plugin, and I am looking for a more concrete direction to go:

Users of Puppet should not see a deterioration in their workflows - ideally, some workflows would even see an improvement.

Taking this quote from @tbrisker, I would assume that the main objective is to keep the new Puppet Plugin API as similar to the old one as possible. Ideally, it has exactly the same interface and the user will never notice a difference.

Advantages:

No changes for Hammer-related interface
No changes for FAM
No changes for Foreman users that use the API directly.

Disadvantages:

Implementation is a little bit messy for Foreman 2.X as functionality from Foreman core needs to be overwritten by the plugin.
Changes to the API might bring advantages for the new hammer-cli-foreman-puppet plugin.
We might miss a chance to clean up the API (in case this is necessary).

As of now, the implementation aims to go somewhere in between by prepending the current API commands with foreman_puppet, e.g. foreman_puppet/api/.... This is done in other Foreman plugins as well. As consequence, people using the API directly are the ones eventually “suffering”.

Please correct me if anything I wrote is not correct.

So the remaining question is:

Continue with the current implementation?
Move towards a complete copy of the current Foreman core API?
Add any other changes to the new API?

Thank you for your thoughts!

tbrisker · June 1, 2021, 11:56am

Thanks for raising the concern @nadjaheitmann !
Currently, the new plugin still supports the old API endpoints as well, only logging a deprecation warning when users use them instead of the namespaced ones:

github.com

theforeman/foreman_puppet/blob/master/app/controllers/foreman_puppet/api/v2/base_controller.rb#L10-L23


  before_action :show_deprecation_for_core_routes
  protected
  def show_deprecation_for_core_routes
    return if request.path.starts_with?('/foreman_puppet')
    Foreman::Deprecation.api_deprecation_warning(
      format(
        '/api/v2/%{controller} API endpoints are deprecated, please use /foreman_puppet/api/v2/%{controller} instead',
        controller: controller_name
      )
    )
  end
end

This means that users will have time to update any scripts or tools to use the new endpoints instead of the old ones and will not have a breaking change until we actually drop support for the non-namespaced endpoints.
We can certainly consider making changes to the API in the future if we feel it is needed, but I would suggest to avoid any breaking changes right now as the move to the plugin is a big enough change already.

cuky · August 27, 2021, 1:01pm

I was already hitting issues with these smart varibles and them over riding anything in hiera. Is the hostgroup way of selecting classes staying?

We have started to move away from setting hostgroup varibles via foreman and redesigned the Hiera a little have an extra lyer for hostgroups

extract here , this is getting hostgroup fact, will this still be around in the newer versions.

" - name: “Hostgroup level settings/overrides YAML”
data_hash: yaml_data
paths:
- “environment/%{::environment}/roles/%{::hostgroup}.yaml”

We put this just before any node over ride and after env , default

I test and build foreman a lot with my dev platform, we also use it in prod rather extensivily.

steve