From e48f1db9eef50391d4ec700a319abc5986303bd5 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Tue, 26 Jan 2021 11:28:31 -0500 Subject: [PATCH 01/21] Added ability to manage IPA groups and IPA group memberships --- lib/puppet/provider/ipa.rb | 50 ++++++- lib/puppet/provider/ipa_group/default.rb | 124 +++++++++++++++++ .../provider/ipa_group_membership/default.rb | 93 +++++++++++++ lib/puppet/provider/ipa_kinit/default.rb | 2 +- lib/puppet/provider/ipa_user/default.rb | 66 ++++----- lib/puppet/type/ipa_group.rb | 99 ++++++++++++++ lib/puppet/type/ipa_group_membership.rb | 127 ++++++++++++++++++ lib/puppet/type/ipa_kinit.rb | 18 +-- lib/puppet/type/ipa_user.rb | 74 ++++------ lib/puppet_x/encore/ipa.rb | 8 ++ lib/puppet_x/encore/ipa/cache.rb | 8 +- lib/puppet_x/encore/ipa/http_client.rb | 5 +- lib/puppet_x/encore/ipa/list_property.rb | 46 +++++++ lib/puppet_x/encore/ipa/type_utils.rb | 28 ++++ manifests/group.pp | 18 +++ manifests/group_membership.pp | 22 +++ manifests/user.pp | 25 +++- 17 files changed, 710 insertions(+), 103 deletions(-) create mode 100644 lib/puppet/provider/ipa_group/default.rb create mode 100644 lib/puppet/provider/ipa_group_membership/default.rb create mode 100644 lib/puppet/type/ipa_group.rb create mode 100644 lib/puppet/type/ipa_group_membership.rb create mode 100644 lib/puppet_x/encore/ipa.rb create mode 100644 lib/puppet_x/encore/ipa/list_property.rb create mode 100644 lib/puppet_x/encore/ipa/type_utils.rb create mode 100644 manifests/group.pp create mode 100644 manifests/group_membership.pp diff --git a/lib/puppet/provider/ipa.rb b/lib/puppet/provider/ipa.rb index 06694da..5d45160 100644 --- a/lib/puppet/provider/ipa.rb +++ b/lib/puppet/provider/ipa.rb @@ -1,7 +1,5 @@ -require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', - 'puppet_x', 'encore', 'ipa', 'http_client')) -require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', - 'puppet_x', 'encore', 'ipa', 'cache')) +require 'puppet_x/encore/ipa/cache' +require 'puppet_x/encore/ipa/http_client' require 'cgi' # This class is a "base" provider to use to implement your own custom providers here @@ -97,8 +95,24 @@ def cached_instance # note: we explicitly do NOT cache within this method because we want to be # able to call it both in initialize() and in flush() and return the current # state of the resource from the API each time - def read_instance - raise NotImplementedError, 'read_instance needs to be implemented by child providers' + def read_instance(use_cache: true) + instances_hash = use_cache ? cached_all_instances : read_all_instances + if instances_hash.key?(resource[:name]) + instances_hash[resource[:name]] + else + { ensure: :absent, name: resource[:name] } + end + end + + # Read all instances of this type from the API, this will then be stored in a cache + # We do it like this so that the first resource of this type takes the burden of + # reading all of the data, but the following resources are all super fast because + # they can use the global cache + # + # This should be a hash where the key is the namevar of the resource and the value + # is a hash with all of the properties of the resource set + def read_all_instances + raise NotImplementedError, 'read_all_instances needs to be implemented by child providers' end # this method should check resource[:ensure] @@ -112,6 +126,18 @@ def flush_instance raise NotImplementedError, 'flush_instance needs to be implemented by child providers' end + # global cached instances, so we only have to read in the groups list once + def cached_all_instances + # return cache if it has been created, this means that this function will only need + # to be loaded once, returning all instances that exist of this resource in vsphere + # then, we can lookup our version by name/id/whatever. This saves a TON of processing + cached_instances = PuppetX::Encore::Ipa::Cache.instance.cached_instances[resource.type] + return cached_instances unless cached_instances.nil? + + # read all instances from the API and save them in the cache + PuppetX::Encore::Ipa::Cache.instance.cached_instances[resource.type] = read_all_instances + end + def api_client # create an HTTP client with this username/password and cache it # be sure to use resource[:xxx] here so that we can use the parameters @@ -174,4 +200,16 @@ def api_post(endpoint, body: nil, json_parse: true) response end end + + def get_ldap_attribute(obj, attr) + return :absent if obj[attr].nil? + # special handling for arrays + if obj[attr].is_a?(Array) + return [] if obj[attr].empty? + # de-array-ify the thing if there is only one element + return obj[attr][0] if obj[attr].size == 1 + end + # either return the full array if >1 element, or return the non-array value + obj[attr] + end end diff --git a/lib/puppet/provider/ipa_group/default.rb b/lib/puppet/provider/ipa_group/default.rb new file mode 100644 index 0000000..4977ed1 --- /dev/null +++ b/lib/puppet/provider/ipa_group/default.rb @@ -0,0 +1,124 @@ +require 'puppet/provider/ipa' + +Puppet::Type.type(:ipa_group).provide(:default, parent: Puppet::Provider::Ipa) do + defaultfor kernel: 'Linux' + + commands ipa: 'ipa' + + # always need to define this in our implementation classes + mk_resource_methods + + ########################## + # private methods that we need to implement because we inherit from Puppet::Provider::Synapse + + # Read all instances of this type from the API, this will then be stored in a cache + # We do it like this so that the first resource of this type takes the burden of + # reading all of the data, but the following resources are all super fast because + # they can use the global cache + def read_all_instances + # read all of the groups, once + body = { + 'id' => 0, + 'method' => 'group_find/1', + 'params' => [ + # args (positional arguments) + [], + # options (CLI flags / options) + { + 'all' => true, + }, + ], + } + response_body = api_post('/session/json', body: body, json_parse: true) + group_list = response_body['result']['result'] + Puppet.debug("Got group list: #{group_list}") + + instance_hash = {} + group_list.each do |group| + instance = { + ensure: :present, + name: get_ldap_attribute(group, 'cn'), + description: get_ldap_attribute(group, 'description'), + gid: get_ldap_attribute(group, 'gidnumber'), + group_type: :non_posix, + } + + # there nothing special on a group that determines if they are non_posix other + # than the absence of the following two objectclass attributes that denote + # either posix or external group types. + # posix groups are denoted by an objectclass="posixgroup" attribute + # external groups are denoted by an objectclass="ipaexternalgroup" attribute + objectclasses = group['objectclass'] + objectclasses.each do |oc| + if oc == 'posixgroup' + instance[:group_type] = :posix + break + elsif oc == 'ipaexternalgroup' + instance[:group_type] = :external + break + end + end + instance_hash[instance[:name]] = instance + end + Puppet.debug("Returning group instances: #{instance_hash}") + instance_hash + end + + # this method should check resource[:ensure] + # if it is :present this method should create/update the instance using the values + # in resource[:xxx] (these are the desired state values) + # else if it is :absent this method should delete the instance + # + # if you want to have access to the values before they were changed you can use + # cached_instance[:xxx] to compare against (that's why it exists) + def flush_instance + # write a single instance at a time + # we can't bulk write because instances may be written in different orders depending + # on their relationships defined in PuppetDSL + case resource[:ensure] + when :absent + body = { + 'id' => 0, + 'method' => 'group_del/1', + 'params' => [ + # args (positional arguments) + [resource[:name]], + # options (CLI flags / options) + {}, + ], + } + api_post('/session/json', body: body) + when :present + method = if cached_instance[:ensure] == :absent + # if the group was absent, we need to add + 'group_add/1' + else + # if the group was present then we need to modify + 'group_mod/1' + end + body = { + 'id' => 0, + 'method' => method, + 'params' => [ + # args (positional arguments) + [resource[:name]], + # options (CLI flags / options) + {}, + ], + } + body['params'][1]['description'] = resource[:description] if resource[:description] + body['params'][1]['gidnumber'] = resource[:gid] if resource[:gid] + # can only set group "type" when adding the group + if cached_instance[:ensure] == :absent + if resource[:group_type] == :non_posix + body['params'][1]['nonposix'] = true + elsif resource[:group_type] == :external + body['params'][1]['external'] = true + end + # default is posix type, no flags need to be set + end + + api_post('/session/json', body: body) + end + end +end diff --git a/lib/puppet/provider/ipa_group_membership/default.rb b/lib/puppet/provider/ipa_group_membership/default.rb new file mode 100644 index 0000000..d82469c --- /dev/null +++ b/lib/puppet/provider/ipa_group_membership/default.rb @@ -0,0 +1,93 @@ +require 'puppet/provider/ipa' + +Puppet::Type.type(:ipa_group_membership).provide(:default, parent: Puppet::Provider::Ipa) do + defaultfor kernel: 'Linux' + + commands ipa: 'ipa' + + # always need to define this in our implementation classes + mk_resource_methods + + ########################## + # private methods that we need to implement because we inherit from Puppet::Provider::Synapse + + # In this case we need to be pretty different about how we handle group memberships + # because they are special, so we aren't using a cache here + def read_instance(use_cache: true) + # TODO read from a "groups cache" + body = { + 'id' => 0, + 'method' => 'group_find/1', + 'params' => [ + # args (positional arguments) + [resource[:group]], + # options (CLI flags / options) + { + 'all' => true, + }, + ], + } + response_body = api_post('/session/json', body: body, json_parse: true) + group_list = response_body['result']['result'] + group = group_list.find { |g| get_ldap_attribute(g, 'cn') == resource[:group] } + Puppet.debug("Got group: #{group}") + + instance = nil + unless group.nil? + instance = { + ensure: :present, + name: resource[:name], + group: get_ldap_attribute(group, 'cn'), + } + instance[:groups] = get_ldap_attribute(group, 'member_group') if resource[:groups] + instance[:users] = get_ldap_attribute(group, 'member_user') if resource[:users] + instance[:services] = get_ldap_attribute(group, 'member_service') if resource[:services] + + # if we are trying to delete the instance and all of the memberships are gone, then tell + # Puppet that its gone + if (resource[:ensure] == :absent && + (instance[:groups].nil? || instance[:groups] == :absent) && + (instance[:users].nil? || instance[:users] == :absent) && + (instance[:services].nil? || instance[:services] == :absent)) + instance = nil + end + end + instance = { ensure: :absent, name: resource[:name] } if instance.nil? + Puppet.debug("Returning group membership instance #{instance}") + instance + end + + + # this method should check resource[:ensure] + # if it is :present this method should create/update the instance using the values + # in resource[:xxx] (these are the desired state values) + # else if it is :absent this method should delete the instance + # + # if you want to have access to the values before they were changed you can use + # cached_instance[:xxx] to compare against (that's why it exists) + def flush_instance + # write a single instance at a time + # we can't bulk write because instances may be written in different orders depending + # on their relationships defined in PuppetDSL + method = if resource[:ensure] == :absent + 'group_remove_member/1' + else + 'group_add_member/1' + end + body = { + 'id' => 0, + 'method' => method, + 'params' => [ + # args (positional arguments) + [resource[:group]], + # options (CLI flags / options) + {}, + ], + } + body['params'][1]['group'] = resource[:groups] if resource[:groups] + body['params'][1]['user'] = resource[:users] if resource[:users] + body['params'][1]['service'] = resource[:services] if resource[:services] + + api_post('/session/json', body: body) + end +end diff --git a/lib/puppet/provider/ipa_kinit/default.rb b/lib/puppet/provider/ipa_kinit/default.rb index 6ee680e..760e02d 100644 --- a/lib/puppet/provider/ipa_kinit/default.rb +++ b/lib/puppet/provider/ipa_kinit/default.rb @@ -1,4 +1,4 @@ -require File.expand_path(File.join(File.dirname(__FILE__), '..', 'ipa')) +require 'puppet/provider/ipa' Puppet::Type.type(:ipa_kinit).provide(:default, parent: Puppet::Provider::Ipa) do defaultfor kernel: 'Linux' diff --git a/lib/puppet/provider/ipa_user/default.rb b/lib/puppet/provider/ipa_user/default.rb index 0105e77..53a022f 100644 --- a/lib/puppet/provider/ipa_user/default.rb +++ b/lib/puppet/provider/ipa_user/default.rb @@ -1,4 +1,4 @@ -require File.expand_path(File.join(File.dirname(__FILE__), '..', 'ipa')) +require 'puppet/provider/ipa' Puppet::Type.type(:ipa_user).provide(:default, parent: Puppet::Provider::Ipa) do defaultfor kernel: 'Linux' @@ -11,17 +11,18 @@ ########################## # private methods that we need to implement because we inherit from Puppet::Provider::Synapse - # this method should retrieve an instance and return it as a hash - # note: we explicitly do NOT cache within this method because we want to be - # able to call it both in initialize() and in flush() and return the current - # state of the resource from the API each time - def read_instance + # Read all instances of this type from the API, this will then be stored in a cache + # We do it like this so that the first resource of this type takes the burden of + # reading all of the data, but the following resources are all super fast because + # they can use the global cache + def read_all_instances + # read all of the instances, once body = { 'id' => 0, 'method' => 'user_find/1', 'params' => [ # args (positional arguments) - [resource[:name]], + [], # options (CLI flags / options) { 'all' => true, @@ -30,13 +31,10 @@ def read_instance } response_body = api_post('/session/json', body: body, json_parse: true) user_list = response_body['result']['result'] - user = user_list.find { |u| u['uid'][0] == resource[:name] } - Puppet.debug("Got user: #{user}") + Puppet.debug("Got user list: #{user_list}") - instance = nil - if user.nil? - instance = { ensure: :absent, name: resource[:name] } - else + instance_hash = {} + user_list.each do |user| instance = { ensure: :present, name: get_ldap_attribute(user, 'uid'), @@ -46,17 +44,35 @@ def read_instance instance[:sshpubkeys] = get_ldap_attribute(user, 'ipasshpubkey') if user['ipasshpubkey'] instance[:mail] = get_ldap_attribute(user, 'mail') if user['mail'] - # fill out additional LDAP attributes that the user is asking to sync + # save all LDAP attributes and we'll filter later + instance[:ldap_attributes] = {} + user.each do |attr_key, _attr_value| + instance[:ldap_attributes][attr_key] = get_ldap_attribute(user, attr_key) + end + instance_hash[instance[:name]] = instance + end + Puppet.debug("Returning user instances: #{instance_hash}") + instance_hash + end + + def read_instance(use_cache: true) + instances_hash = use_cache ? cached_all_instances : read_all_instances + if instances_hash.key?(resource[:name]) + instance = instances_hash[resource[:name]] + # special handling for custom ldap attributes if resource[:ldap_attributes] - instance[:ldap_attributes] = {} - resource[:ldap_attributes].each do |attr_key, _attr_value| - next if user[attr_key].nil? - instance[:ldap_attributes][attr_key] = get_ldap_attribute(user, attr_key) + # only keep the LDAP attributes in the instance that are specified on the resource + instance[:ldap_attributes].select! do |attr_key, attr_value| + resource[:ldap_attributes].key?(attr_key) end + else + # resource didn't have ldap_attributes, so delete it + instance[:ldap_attributes] = {} end + instance + else + { ensure: :absent, name: resource[:name] } end - Puppet.debug("Returning user instance: #{instance}") - instance end # this method should check resource[:ensure] @@ -120,14 +136,4 @@ def flush_instance api_post('/session/json', body: body) end end - - def get_ldap_attribute(obj, attr) - if obj[attr].empty? - [] - elsif obj[attr].size == 1 - obj[attr][0] - else - obj[attr] - end - end end diff --git a/lib/puppet/type/ipa_group.rb b/lib/puppet/type/ipa_group.rb new file mode 100644 index 0000000..3d9000c --- /dev/null +++ b/lib/puppet/type/ipa_group.rb @@ -0,0 +1,99 @@ +require 'puppet_x/encore/ipa/type_utils' + +Puppet::Type.newtype(:ipa_group) do + desc 'Manages a user group in IPA' + + ensurable do + newvalue(:present) do + provider.create + end + + newvalue(:absent) do + provider.destroy + end + + defaultto :present + end + + # namevar is always a parameter + newparam(:name, namevar: true) do + desc 'Name of the group' + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newproperty(:description) do + desc 'Description of the group' + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newproperty(:group_type) do + desc <<-EOS + Type of the IPA group to create. When creating POSIX groups, you can specify the gid. + Once a groups is created, it is not possible to change its group type. Attempting + to do so will result in an error from the API. + EOS + + newvalue(:posix) + newvalue(:non_posix) + newvalue(:external) + + defaultto :posix + end + + newproperty(:gid) do + desc 'Group ID of the group, (only for POSIX groups)' + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_type(name, value, Integer) + unless @resource[:group_type] == :posix + raise ArgumentError, "gid is only allowed to be specified when creating POSIX groups." + end + end + end + + newparam(:api_url) do + desc 'URL of the IPA API. Note: we will append endpoints to the end of this. Default: https:///ipa' + + isrequired + + defaultto do + "https://#{Facter.value(:fqdn)}/ipa" + end + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newparam(:api_username) do + desc 'Username for authentication to the API. This user must be an admin user.' + + isrequired + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newparam(:api_password) do + desc 'Password for authentication to the API.' + + isrequired + + sensitive true + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + validate do + PuppetX::Encore::Ipa::TypeUtils.validate_required_attributes(self) + end +end diff --git a/lib/puppet/type/ipa_group_membership.rb b/lib/puppet/type/ipa_group_membership.rb new file mode 100644 index 0000000..e2a7808 --- /dev/null +++ b/lib/puppet/type/ipa_group_membership.rb @@ -0,0 +1,127 @@ +require 'puppet_x/encore/ipa/list_property' +require 'puppet_x/encore/ipa/type_utils' + +Puppet::Type.newtype(:ipa_group_membership) do + desc 'Manages a membership of a group. Group membership can be user->group, group->group, external->group (TODO), idoverride->group (TODO), service->group.' + + ensurable do + newvalue(:present) do + provider.create + end + + newvalue(:absent) do + provider.destroy + end + + defaultto :present + end + + # namevar is always a parameter + newparam(:name, namevar: true) do + desc <<-EOS + Unique name of the membership, by default we use this for the group name + We support this NOT being the group name in case you want to manage membership + in a unique way. + EOS + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newproperty(:group) do + desc 'Name of the group who members will be managed' + + defaultto do + @resource[:name] + end + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newparam(:membership) do + desc <<-EOS + Whether specified members should be considered the **complete list** + (`inclusive`) or the **minimum list** (`minimum`) of members the group has. + EOS + + newvalues(:inclusive, :minimum) + + defaultto :minimum + end + + newproperty(:groups, array_patching: :all, parent: PuppetX::Encore::Ipa::ListProperty) do + desc 'Group members of this group' + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newproperty(:users, array_patching: :all, parent: PuppetX::Encore::Ipa::ListProperty) do + desc 'User members of this group' + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newproperty(:services, array_patching: :all, parent: PuppetX::Encore::Ipa::ListProperty) do + desc 'Service members of this group' + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newparam(:api_url) do + desc 'URL of the IPA API. Note: we will append endpoints to the end of this. Default: https:///ipa' + + isrequired + + defaultto do + "https://#{Facter.value(:fqdn)}/ipa" + end + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newparam(:api_username) do + desc 'Username for authentication to the API. This user must be an admin user.' + + isrequired + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newparam(:api_password) do + desc 'Password for authentication to the API.' + + isrequired + + sensitive true + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + validate do + PuppetX::Encore::Ipa::TypeUtils.validate_required_attributes(self) + end + + autorequire(:ipa_group) do + grp = [@parameters[:group].should] + grp += @parameters[:groups].should if @parameters[:groups] && @parameters[:groups].should + end + + autorequire(:ipa_user) do + @parameters[:users].nil? ? [] : @parameters[:users].should + end +end diff --git a/lib/puppet/type/ipa_kinit.rb b/lib/puppet/type/ipa_kinit.rb index e00f8f9..e285548 100644 --- a/lib/puppet/type/ipa_kinit.rb +++ b/lib/puppet/type/ipa_kinit.rb @@ -1,3 +1,5 @@ +require 'puppet_x/encore/ipa/type_utils' + Puppet::Type.newtype(:ipa_kinit) do desc 'Ensures a kereberos ticket is obtained for a given user' @@ -18,9 +20,7 @@ desc 'Username to obtain a kerberose ticket for' validate do |value| - unless value.is_a?(String) - raise ArgumentError, "name is expected to be an String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end end @@ -28,9 +28,7 @@ desc 'Optional realm to help with user matching when running klist.' validate do |value| - unless value.is_a?(String) - raise ArgumentError, "api_password is expected to be an String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end munge do |value| @@ -46,9 +44,11 @@ sensitive true validate do |value| - unless value.is_a?(String) - raise ArgumentError, "api_password is expected to be an String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end end + + validate do + PuppetX::Encore::Ipa::TypeUtils.validate_required_attributes(self) + end end diff --git a/lib/puppet/type/ipa_user.rb b/lib/puppet/type/ipa_user.rb index 83bce04..19e21f1 100644 --- a/lib/puppet/type/ipa_user.rb +++ b/lib/puppet/type/ipa_user.rb @@ -1,3 +1,6 @@ +require 'puppet_x/encore/ipa/list_property' +require 'puppet_x/encore/ipa/type_utils' + Puppet::Type.newtype(:ipa_user) do desc 'Manages a user in IPA' @@ -18,9 +21,7 @@ desc 'Username of the user' validate do |value| - unless value.is_a?(String) - raise ArgumentError, "name is expected to be an String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end end @@ -32,9 +33,7 @@ sensitive true validate do |value| - unless value.is_a?(String) - raise ArgumentError, "initial_password is expected to be an String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end end @@ -46,9 +45,7 @@ end validate do |value| - unless value.is_a?(String) - raise ArgumentError, "first_name is expected to be an String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end end @@ -60,50 +57,37 @@ end validate do |value| - unless value.is_a?(String) - raise ArgumentError, "last_name is expected to be an String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end end - newproperty(:sshpubkeys, array_matching: :all) do + newproperty(:sshpubkeys, array_matching: :all, parent: PuppetX::Encore::Ipa::ListProperty) do validate do |value| # note: Puppet automatically detects if the value is an array and calls this validate() # on each item/value within the array - unless value.is_a?(String) - raise ArgumentError, "sshpubkeys are expected to be String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end - # sort the array so we can compute the difference correct and order doesn't matter - def sort_array(a) - if a.nil? - [] - else - a.sort - end + def membership + :sshpubkey_membership end + end - def should - sort_array(super) - end + newparam(:sshpubkey_membership) do + desc "Whether specified SSH public keys should be considered the **complete list** + (`inclusive`) or the **minimum list** (`minimum`) of roles the user + has." - def should=(values) - super(sort_array(values)) - end + newvalues(:inclusive, :minimum) - def insync?(is) - sort_array(is) == should - end + defaultto :minimum end newproperty(:mail) do desc 'Email address of the user' validate do |value| - unless value.is_a?(String) - raise ArgumentError, "mail is expected to be an String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end end @@ -111,9 +95,7 @@ def insync?(is) desc 'Hash of additional IPA attributes to set on the user' validate do |value| - unless value.is_a?(Hash) - raise ArgumentError, "ldap_attributes is expected to be a Hash, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_type(name, value, Hash) end munge do |value| @@ -132,9 +114,7 @@ def insync?(is) end validate do |value| - unless value.is_a?(String) - raise ArgumentError, "api_url is expected to be an String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end end @@ -144,9 +124,7 @@ def insync?(is) isrequired validate do |value| - unless value.is_a?(String) - raise ArgumentError, "api_username is expected to be an String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end end @@ -158,9 +136,11 @@ def insync?(is) sensitive true validate do |value| - unless value.is_a?(String) - raise ArgumentError, "api_password is expected to be an String, given: #{value.class.name}" - end + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) end end + + validate do + PuppetX::Encore::Ipa::TypeUtils.validate_required_attributes(self) + end end diff --git a/lib/puppet_x/encore/ipa.rb b/lib/puppet_x/encore/ipa.rb new file mode 100644 index 0000000..2744d74 --- /dev/null +++ b/lib/puppet_x/encore/ipa.rb @@ -0,0 +1,8 @@ +require 'puppet_x' + +module PuppetX + module Encore + module Ipa + end + end +end diff --git a/lib/puppet_x/encore/ipa/cache.rb b/lib/puppet_x/encore/ipa/cache.rb index ab38802..c256ed0 100644 --- a/lib/puppet_x/encore/ipa/cache.rb +++ b/lib/puppet_x/encore/ipa/cache.rb @@ -1,18 +1,16 @@ -require 'puppet_x' +require 'puppet_x/encore/ipa' require 'singleton' -# Encore module -module PuppetX::Encore -end - module PuppetX::Encore::Ipa # Class for caching HTTP clients class Cache include Singleton attr_accessor :cached_clients + attr_accessor :cached_instances def initialize @cached_clients = {} + @cached_instances = {} end end end diff --git a/lib/puppet_x/encore/ipa/http_client.rb b/lib/puppet_x/encore/ipa/http_client.rb index 0f84e33..14bedc6 100644 --- a/lib/puppet_x/encore/ipa/http_client.rb +++ b/lib/puppet_x/encore/ipa/http_client.rb @@ -1,12 +1,9 @@ require 'net/https' require 'ipaddr' require 'json' -require 'puppet_x' +require 'puppet_x/encore/ipa' require 'puppet' -# Encore module -module PuppetX::Encore -end module PuppetX::Encore::Ipa # Client class for HTTP calls diff --git a/lib/puppet_x/encore/ipa/list_property.rb b/lib/puppet_x/encore/ipa/list_property.rb new file mode 100644 index 0000000..23fc502 --- /dev/null +++ b/lib/puppet_x/encore/ipa/list_property.rb @@ -0,0 +1,46 @@ +require 'puppet/property' +require 'puppet_x/encore/ipa' + +module PuppetX::Encore::Ipa + class ListProperty < Puppet::Property + + def membership + :membership + end + + def inclusive? + @resource[membership] == :inclusive + end + + # sort the array so we can compute the difference correct and order doesn't matter + def sort_array(a) + (a.nil?) ? [] : make_array(a).sort + end + + def make_array(a) + a.is_a?(Array) ? a : [a] + end + + def should + members = make_array(super) + + # inclusive means we are managing everything so if it isn't in should, its gone + unless inclusive? + current = retrieve + members += make_array(current) if current && current != :absent + members.uniq! + end + + sort_array(members) + end + + def should=(values) + super(sort_array(values)) + end + + def insync?(is) + return true unless is + sort_array(is) == should + end + end +end diff --git a/lib/puppet_x/encore/ipa/type_utils.rb b/lib/puppet_x/encore/ipa/type_utils.rb new file mode 100644 index 0000000..e180ada --- /dev/null +++ b/lib/puppet_x/encore/ipa/type_utils.rb @@ -0,0 +1,28 @@ +require 'puppet_x/encore/ipa' +require 'singleton' + +# Encore module +module PuppetX::Encore::Ipa::TypeUtils + def validate_type(attr, value, type) + unless value.is_a?(type) + raise ArgumentError, "#{attr} is expected to be an #{type.name}, given: #{value.class.name}" + end + end + module_function :validate_type + + def validate_string(attr, value) + validate_type(attr, value, String) + end + module_function :validate_string + + def validate_required_attributes(type) + # validate all required parameters, properties and metaparams are set + type.class.allattrs.each do |attr| + attrclass = type.class.attrclass(attr) + if attrclass.required? && !type[attr] + raise ArgumentError, "#{type.class.attrtype(attr)} '#{attr}' is required" + end + end + end + module_function :validate_required_attributes +end diff --git a/manifests/group.pp b/manifests/group.pp new file mode 100644 index 0000000..5138508 --- /dev/null +++ b/manifests/group.pp @@ -0,0 +1,18 @@ +# @summar Manages an IPA group +define ipa::group ( + String $ensure = 'present', + Optional[String] $description = undef, + Enum['posix', 'non_posix', 'external'] $group_type = 'posix', + Optional[Integer] $gid = undef, + String $api_username = $ipa::admin_user, + String $api_password = $ipa::admin_password, +) { + ipa_group { $title: + ensure => $ensure, + name => $name, + description => $description, + group_type => $group_type, + api_username => $api_username, + api_password => $api_password, + } +} diff --git a/manifests/group_membership.pp b/manifests/group_membership.pp new file mode 100644 index 0000000..898ceb8 --- /dev/null +++ b/manifests/group_membership.pp @@ -0,0 +1,22 @@ +# @summar Manages an IPA group membership +define ipa::group_membership ( + String $ensure = 'present', + String $group = $name, + Enum['inclusive', 'minimum'] $membership = 'minimum', + Optional[Array[String]] $groups = undef, + Optional[Array[String]] $users = undef, + Optional[Array[String]] $services = undef, + String $api_username = $ipa::admin_user, + String $api_password = $ipa::admin_password, +) { + ipa_group_membership { $title: + ensure => $ensure, + name => $name, + group => $group, + groups => $groups, + users => $users, + services => $services, + api_username => $api_username, + api_password => $api_password, + } +} diff --git a/manifests/user.pp b/manifests/user.pp index 36dab64..fccfc4b 100644 --- a/manifests/user.pp +++ b/manifests/user.pp @@ -64,6 +64,14 @@ # ] # } # +# @example Adding a user to groups, this auto-creates ipa::group_membership instances +# ipa::user { 'testuser': +# ensure => present, +# initial_password => 'abc123', +# home_dir_base => '/srv/nfs/home', +# groups => +# } +# # @example Deleting a user # ipa::user { 'testuser': # ensure => absent, @@ -82,11 +90,13 @@ Boolean $manage_etc_skel = true, Boolean $manage_dot_ssh = true, Optional[Array[String]] $sshpubkeys = undef, + Optional[Array[String]] $groups = undef, String $api_username = $ipa::admin_user, String $api_password = $ipa::admin_password, ) { - ipa_user { $name: + ipa_user { $title: ensure => $ensure, + name => $name, initial_password => $initial_password, first_name => $first_name, last_name => $last_name, @@ -133,4 +143,17 @@ require => Ipa_user[$name], } } + + # create group memberships for this user + if $groups { + $groups.each |$grp| { + ipa::group_membership { "${grp}:${title}": + ensure => $ensure, + group => $grp, + users => [$name], + api_username => $api_username, + api_password => $api_password, + } + } + } } From 620109e564f1ffec983262de2a6d057337a4fcdd Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Tue, 26 Jan 2021 11:51:05 -0500 Subject: [PATCH 02/21] Linting fixes --- .../provider/ipa_group_membership/default.rb | 14 +++++------ lib/puppet/provider/ipa_user/default.rb | 2 +- lib/puppet/type/ipa_group.rb | 2 +- lib/puppet/type/ipa_group_membership.rb | 1 + lib/puppet_x/encore/ipa.rb | 10 ++++---- lib/puppet_x/encore/ipa/http_client.rb | 1 - lib/puppet_x/encore/ipa/list_property.rb | 24 ++++++++++++++----- lib/puppet_x/encore/ipa/type_utils.rb | 6 ++--- .../puppet/provider/ipa_kinit/default_spec.rb | 2 ++ spec/unit/puppet/type/ipa_kinit_spec.rb | 3 ++- 10 files changed, 39 insertions(+), 26 deletions(-) diff --git a/lib/puppet/provider/ipa_group_membership/default.rb b/lib/puppet/provider/ipa_group_membership/default.rb index d82469c..660727c 100644 --- a/lib/puppet/provider/ipa_group_membership/default.rb +++ b/lib/puppet/provider/ipa_group_membership/default.rb @@ -13,8 +13,8 @@ # In this case we need to be pretty different about how we handle group memberships # because they are special, so we aren't using a cache here - def read_instance(use_cache: true) - # TODO read from a "groups cache" + def read_instance(*) + # TODO: read from a "groups cache" body = { 'id' => 0, 'method' => 'group_find/1', @@ -45,10 +45,10 @@ def read_instance(use_cache: true) # if we are trying to delete the instance and all of the memberships are gone, then tell # Puppet that its gone - if (resource[:ensure] == :absent && - (instance[:groups].nil? || instance[:groups] == :absent) && - (instance[:users].nil? || instance[:users] == :absent) && - (instance[:services].nil? || instance[:services] == :absent)) + if resource[:ensure] == :absent && + (instance[:groups].nil? || instance[:groups] == :absent) && + (instance[:users].nil? || instance[:users] == :absent) && + (instance[:services].nil? || instance[:services] == :absent) instance = nil end end @@ -57,7 +57,6 @@ def read_instance(use_cache: true) instance end - # this method should check resource[:ensure] # if it is :present this method should create/update the instance using the values # in resource[:xxx] (these are the desired state values) @@ -74,6 +73,7 @@ def flush_instance else 'group_add_member/1' end + body = { 'id' => 0, 'method' => method, diff --git a/lib/puppet/provider/ipa_user/default.rb b/lib/puppet/provider/ipa_user/default.rb index 53a022f..5106a63 100644 --- a/lib/puppet/provider/ipa_user/default.rb +++ b/lib/puppet/provider/ipa_user/default.rb @@ -62,7 +62,7 @@ def read_instance(use_cache: true) # special handling for custom ldap attributes if resource[:ldap_attributes] # only keep the LDAP attributes in the instance that are specified on the resource - instance[:ldap_attributes].select! do |attr_key, attr_value| + instance[:ldap_attributes].select! do |attr_key, _attr_value| resource[:ldap_attributes].key?(attr_key) end else diff --git a/lib/puppet/type/ipa_group.rb b/lib/puppet/type/ipa_group.rb index 3d9000c..a156efe 100644 --- a/lib/puppet/type/ipa_group.rb +++ b/lib/puppet/type/ipa_group.rb @@ -52,7 +52,7 @@ validate do |value| PuppetX::Encore::Ipa::TypeUtils.validate_type(name, value, Integer) unless @resource[:group_type] == :posix - raise ArgumentError, "gid is only allowed to be specified when creating POSIX groups." + raise ArgumentError, 'gid is only allowed to be specified when creating POSIX groups.' end end end diff --git a/lib/puppet/type/ipa_group_membership.rb b/lib/puppet/type/ipa_group_membership.rb index e2a7808..36cec01 100644 --- a/lib/puppet/type/ipa_group_membership.rb +++ b/lib/puppet/type/ipa_group_membership.rb @@ -119,6 +119,7 @@ autorequire(:ipa_group) do grp = [@parameters[:group].should] grp += @parameters[:groups].should if @parameters[:groups] && @parameters[:groups].should + grp end autorequire(:ipa_user) do diff --git a/lib/puppet_x/encore/ipa.rb b/lib/puppet_x/encore/ipa.rb index 2744d74..93de8e1 100644 --- a/lib/puppet_x/encore/ipa.rb +++ b/lib/puppet_x/encore/ipa.rb @@ -1,8 +1,8 @@ require 'puppet_x' -module PuppetX - module Encore - module Ipa - end - end +# module for encore +module PuppetX::Encore +end +# module for ipa +module PuppetX::Encore::Ipa end diff --git a/lib/puppet_x/encore/ipa/http_client.rb b/lib/puppet_x/encore/ipa/http_client.rb index 14bedc6..a9c65ae 100644 --- a/lib/puppet_x/encore/ipa/http_client.rb +++ b/lib/puppet_x/encore/ipa/http_client.rb @@ -4,7 +4,6 @@ require 'puppet_x/encore/ipa' require 'puppet' - module PuppetX::Encore::Ipa # Client class for HTTP calls class HTTPClient diff --git a/lib/puppet_x/encore/ipa/list_property.rb b/lib/puppet_x/encore/ipa/list_property.rb index 23fc502..bbb2c9d 100644 --- a/lib/puppet_x/encore/ipa/list_property.rb +++ b/lib/puppet_x/encore/ipa/list_property.rb @@ -2,25 +2,37 @@ require 'puppet_x/encore/ipa' module PuppetX::Encore::Ipa + # This class implements an array/list property and handles things like sorting + # for you so we can properly compute the difference between the requested list in + # PuppetDSL and the "actual" list read in from the provider. + # + # This class also helps by implementing the "membership" concept meaning + # whether specified members should be considered the **complete list**. + # :inclusive = the list given in PuppetDSL should be the complete list + # :minimum = the list given in PuppetDSL should be contained in the list, but is allowed to have extras + # + # To determine what type of "membership" we read from another type's property + # defined by the membership() method, default is :membership. If you want to use + # a different property name for your membership determination just override this + # method in your newproperty() definition block. class ListProperty < Puppet::Property - def membership :membership end - + def inclusive? @resource[membership] == :inclusive end - - # sort the array so we can compute the difference correct and order doesn't matter + def sort_array(a) - (a.nil?) ? [] : make_array(a).sort + # sort the array so we can compute the difference correct and order doesn't matter + a.nil? ? [] : make_array(a).sort end def make_array(a) a.is_a?(Array) ? a : [a] end - + def should members = make_array(super) diff --git a/lib/puppet_x/encore/ipa/type_utils.rb b/lib/puppet_x/encore/ipa/type_utils.rb index e180ada..a7f885f 100644 --- a/lib/puppet_x/encore/ipa/type_utils.rb +++ b/lib/puppet_x/encore/ipa/type_utils.rb @@ -4,12 +4,10 @@ # Encore module module PuppetX::Encore::Ipa::TypeUtils def validate_type(attr, value, type) - unless value.is_a?(type) - raise ArgumentError, "#{attr} is expected to be an #{type.name}, given: #{value.class.name}" - end + raise ArgumentError, "#{attr} is expected to be an #{type.name}, given: #{value.class.name}" unless value.is_a?(type) end module_function :validate_type - + def validate_string(attr, value) validate_type(attr, value, String) end diff --git a/spec/unit/puppet/provider/ipa_kinit/default_spec.rb b/spec/unit/puppet/provider/ipa_kinit/default_spec.rb index 52288c2..0e88c99 100644 --- a/spec/unit/puppet/provider/ipa_kinit/default_spec.rb +++ b/spec/unit/puppet/provider/ipa_kinit/default_spec.rb @@ -62,6 +62,7 @@ provider: described_class.name, name: name, realm: 'EXPECTED.DOMAIN.TLD', + password: 'AdminPassword123', } end @@ -98,6 +99,7 @@ provider: described_class.name, name: name, ensure: :absent, + password: 'AdminPassword123', } end diff --git a/spec/unit/puppet/type/ipa_kinit_spec.rb b/spec/unit/puppet/type/ipa_kinit_spec.rb index 0575b46..7a7bce6 100644 --- a/spec/unit/puppet/type/ipa_kinit_spec.rb +++ b/spec/unit/puppet/type/ipa_kinit_spec.rb @@ -83,7 +83,8 @@ expect(type_instance[:ensure]).to eq(:present) end it 'default to :present' do - expect(Puppet::Type.type(:ipa_kinit).new(name: name)[:ensure]).to eq(:present) + type = Puppet::Type.type(:ipa_kinit).new(name: name, password: 'Password123') + expect(type[:ensure]).to eq(:present) end end end From f16ffd21bc47a95e001e86ab7e98d5f239904663 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Fri, 29 Jan 2021 09:34:37 -0500 Subject: [PATCH 03/21] Fixing ipa_user autorequire list for group memberships --- lib/puppet/type/ipa_group_membership.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppet/type/ipa_group_membership.rb b/lib/puppet/type/ipa_group_membership.rb index 36cec01..19497d9 100644 --- a/lib/puppet/type/ipa_group_membership.rb +++ b/lib/puppet/type/ipa_group_membership.rb @@ -123,6 +123,6 @@ end autorequire(:ipa_user) do - @parameters[:users].nil? ? [] : @parameters[:users].should + @parameters[:users] ? @parameters[:users].should : [] end end From cd7858a2ed1cbdae5d4ef7a66bf5d5620d1368b9 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Fri, 29 Jan 2021 20:20:45 -0500 Subject: [PATCH 04/21] Fixing params to be able to work with puppet apply --- manifests/params.pp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/manifests/params.pp b/manifests/params.pp index 3c826b1..435fc7b 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -142,7 +142,10 @@ # https://en.wikipedia.org/wiki/User_identifier#Reserved_ranges $uid_gid_min = 65536 # allows for the fact to be empty/undef - $uid_gid_max = max(pick(dig($facts, 'ipa_login_defs', 'UID_MAX'), $uid_gid_min), - pick(dig($facts, 'ipa_login_defs', 'GID_MAX'), $uid_gid_min)) + $uid_gid_max = $facts['ipa_login_defs'] ? { + Hash => max(pick($facts['ipa_login_defs']['UID_MAX'], $uid_gid_min), + pick($facts['ipa_login_defs']['UID_MAX'], $uid_gid_min)), + default => $uid_gid_min, + } $idstart = (fqdn_rand('10737') + max($uid_gid_max, $uid_gid_min)) } From fef8c90b420ccce03634a6dccb1afba056a211c3 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Mon, 8 Feb 2021 11:16:14 -0500 Subject: [PATCH 05/21] properly remove user files when ensure absent --- manifests/user.pp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/manifests/user.pp b/manifests/user.pp index fccfc4b..392cf18 100644 --- a/manifests/user.pp +++ b/manifests/user.pp @@ -107,10 +107,14 @@ api_password => $ipa::admin_password, } + $_file_ensure = $ensure ? { + 'absent' => 'absent', + default => undef, + } if $manage_home_dir { $_home_dir = "${home_dir_base}/${name}" file { $_home_dir: - ensure => directory, + ensure => pick($_file_ensure, 'directory'), owner => $name, group => $name, mode => $home_dir_mode, @@ -122,7 +126,7 @@ if $manage_etc_skel { $facts['ipa_etc_skel_files'].each |$file_path, $file_props| { file { "${_home_dir}/${file_props['local_path']}": - ensure => $file_props['ensure'], + ensure => pick($_file_ensure, $file_props['ensure']), owner => $name, group => $name, source => $file_path, @@ -136,7 +140,7 @@ # create user's ~/.ssh directory and set proper permissions if $manage_dot_ssh { file { "${_home_dir}/.ssh": - ensure => directory, + ensure => pick($_file_ensure, 'directory'), owner => $name, group => $name, mode => '0700', From 53f3f1968b4e183dd606f6f674d7616f571d1873 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Mon, 8 Feb 2021 11:20:44 -0500 Subject: [PATCH 06/21] properly remove user files when ensure absent --- manifests/user.pp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/manifests/user.pp b/manifests/user.pp index 392cf18..bdd1e10 100644 --- a/manifests/user.pp +++ b/manifests/user.pp @@ -118,6 +118,7 @@ owner => $name, group => $name, mode => $home_dir_mode, + force => true, require => Ipa_user[$name], } } @@ -144,6 +145,7 @@ owner => $name, group => $name, mode => '0700', + force => true, require => Ipa_user[$name], } } From adbed58f5ba60d56b70f2720e5744a510fec434d Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Thu, 18 Feb 2021 08:49:36 -0500 Subject: [PATCH 07/21] Fixing ipa_user and ipa_group issues. Also fixing install where Apache is already managed --- lib/puppet/provider/ipa.rb | 6 ++++++ lib/puppet/provider/ipa_group/default.rb | 2 -- lib/puppet/provider/ipa_group_membership/default.rb | 2 -- lib/puppet/provider/ipa_user/default.rb | 2 -- manifests/install/server.pp | 8 +++++--- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/puppet/provider/ipa.rb b/lib/puppet/provider/ipa.rb index 5d45160..642d640 100644 --- a/lib/puppet/provider/ipa.rb +++ b/lib/puppet/provider/ipa.rb @@ -212,4 +212,10 @@ def get_ldap_attribute(obj, attr) # either return the full array if >1 element, or return the non-array value obj[attr] end + + def get_ldap_attribute_boolean(obj, attr) + # values can be: "TRUE", "True", or "true" + # casecmp does case insensitive comparison and returns 0 if equal + get_ldap_attribute(obj, attr).casecmp('true') == 0 + end end diff --git a/lib/puppet/provider/ipa_group/default.rb b/lib/puppet/provider/ipa_group/default.rb index 4977ed1..172c1fa 100644 --- a/lib/puppet/provider/ipa_group/default.rb +++ b/lib/puppet/provider/ipa_group/default.rb @@ -3,8 +3,6 @@ Puppet::Type.type(:ipa_group).provide(:default, parent: Puppet::Provider::Ipa) do defaultfor kernel: 'Linux' - commands ipa: 'ipa' - # always need to define this in our implementation classes mk_resource_methods diff --git a/lib/puppet/provider/ipa_group_membership/default.rb b/lib/puppet/provider/ipa_group_membership/default.rb index 660727c..2c7ddcb 100644 --- a/lib/puppet/provider/ipa_group_membership/default.rb +++ b/lib/puppet/provider/ipa_group_membership/default.rb @@ -3,8 +3,6 @@ Puppet::Type.type(:ipa_group_membership).provide(:default, parent: Puppet::Provider::Ipa) do defaultfor kernel: 'Linux' - commands ipa: 'ipa' - # always need to define this in our implementation classes mk_resource_methods diff --git a/lib/puppet/provider/ipa_user/default.rb b/lib/puppet/provider/ipa_user/default.rb index 5106a63..1fea5d4 100644 --- a/lib/puppet/provider/ipa_user/default.rb +++ b/lib/puppet/provider/ipa_user/default.rb @@ -3,8 +3,6 @@ Puppet::Type.type(:ipa_user).provide(:default, parent: Puppet::Provider::Ipa) do defaultfor kernel: 'Linux' - commands ipa: 'ipa' - # always need to define this in our implementation classes mk_resource_methods diff --git a/manifests/install/server.pp b/manifests/install/server.pp index 02dcf25..bd6e1ca 100644 --- a/manifests/install/server.pp +++ b/manifests/install/server.pp @@ -105,9 +105,11 @@ } # This will set the SSL protocols to use. - service { 'httpd': - ensure => running, - enable => true, + if !defined(Service[ 'httpd']) { + service { 'httpd': + ensure => running, + enable => true, + } } # harden the SSL ciphers and protocols for Apache using the NSS module From 921bfd01ab431401cc73b22f9fa1308ee51edca1 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Thu, 18 Feb 2021 08:50:07 -0500 Subject: [PATCH 08/21] Added DNS zone management type --- lib/puppet/provider/ipa_dns_zone/default.rb | 106 ++++++++++++++++++++ lib/puppet/type/ipa_dns_zone.rb | 79 +++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 lib/puppet/provider/ipa_dns_zone/default.rb create mode 100644 lib/puppet/type/ipa_dns_zone.rb diff --git a/lib/puppet/provider/ipa_dns_zone/default.rb b/lib/puppet/provider/ipa_dns_zone/default.rb new file mode 100644 index 0000000..3e886b1 --- /dev/null +++ b/lib/puppet/provider/ipa_dns_zone/default.rb @@ -0,0 +1,106 @@ +require 'puppet/provider/ipa' + +Puppet::Type.type(:ipa_dns_zone).provide(:default, parent: Puppet::Provider::Ipa) do + defaultfor kernel: 'Linux' + + # always need to define this in our implementation classes + mk_resource_methods + + ########################## + # private methods that we need to implement because we inherit from Puppet::Provider::Synapse + + # Read all instances of this type from the API, this will then be stored in a cache + # We do it like this so that the first resource of this type takes the burden of + # reading all of the data, but the following resources are all super fast because + # they can use the global cache + def read_all_instances + # read all of the groups, once + body = { + 'id' => 0, + 'method' => 'dnszone_find/1', + 'params' => [ + # args (positional arguments) + [], + # options (CLI flags / options) + { + 'all' => true, + # use raw here because otherwise some of the properties return these weird nested dictionaries + # only downside is that some of the keys come back with weird cases + # we'll fix this later + 'raw' => true, + }, + ], + } + response_body = api_post('/session/json', body: body, json_parse: true) + dnszone_list = response_body['result']['result'] + Puppet.debug("Got DNS Zone list: #{dnszone_list}") + + instance_hash = {} + dnszone_list.each do |dnszone| + # --raw returns stuff in weird case formats + # to prevent forward compatability problems we downcase everything before we pull it out + dnszone.transform_keys!(&:downcase) + instance = { + ensure: :present, + name: get_ldap_attribute(dnszone, 'idnsname'), + allow_dynamic_update: get_ldap_attribute_boolean(dnszone, 'idnsallowdynupdate'), + allow_sync_ptr: get_ldap_attribute_boolean(dnszone, 'idnsallowsyncptr'), + } + instance_hash[instance[:name]] = instance + end + Puppet.debug("Returning group instances: #{instance_hash}") + instance_hash + end + + # this method should check resource[:ensure] + # if it is :present this method should create/update the instance using the values + # in resource[:xxx] (these are the desired state values) + # else if it is :absent this method should delete the instance + # + # if you want to have access to the values before they were changed you can use + # cached_instance[:xxx] to compare against (that's why it exists) + def flush_instance + # write a single instance at a time + # we can't bulk write because instances may be written in different orders depending + # on their relationships defined in PuppetDSL + case resource[:ensure] + when :absent + body = { + 'id' => 0, + 'method' => 'dnszone_del/1', + 'params' => [ + # args (positional arguments) + [resource[:name]], + # options (CLI flags / options) + {}, + ], + } + api_post('/session/json', body: body) + when :present + method = if cached_instance[:ensure] == :absent + # if the group was absent, we need to add + 'dnszone_add/1' + else + # if the group was present then we need to modify + 'dnszone_mod/1' + end + body = { + 'id' => 0, + 'method' => method, + 'params' => [ + # args (positional arguments) + [resource[:name]], + # options (CLI flags / options) + {}, + ], + } + unless resource[:allow_dynamic_update].nil? + body['params'][1]['idnsallowdynupdate'] = resource[:allow_dynamic_update] ? 'TRUE' : 'FALSE' + end + unless resource[:allow_sync_ptr].nil? + body['params'][1]['idnsallowsyncptr'] = resource[:allow_sync_ptr] ? 'TRUE' : 'FALSE' + end + api_post('/session/json', body: body) + end + end +end diff --git a/lib/puppet/type/ipa_dns_zone.rb b/lib/puppet/type/ipa_dns_zone.rb new file mode 100644 index 0000000..69728ee --- /dev/null +++ b/lib/puppet/type/ipa_dns_zone.rb @@ -0,0 +1,79 @@ +require 'puppet/property/boolean' +require 'puppet_x/encore/ipa/type_utils' + +Puppet::Type.newtype(:ipa_dns_zone) do + desc 'Manages a DNS Zone in IPA' + + ensurable do + newvalue(:present) do + provider.create + end + + newvalue(:absent) do + provider.destroy + end + + defaultto :present + end + + # namevar is always a parameter + newparam(:name, namevar: true) do + desc 'Name of the DNS zone' + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newproperty(:allow_dynamic_update, boolean: true, parent: Puppet::Property::Boolean) do + desc 'Allow dynamic updates or not' + + defaultto false + end + + newproperty(:allow_sync_ptr, boolean: true, parent: Puppet::Property::Boolean) do + desc 'Allow syncing PTR records' + + defaultto false + end + + newparam(:api_url) do + desc 'URL of the IPA API. Note: we will append endpoints to the end of this. Default: https:///ipa' + + isrequired + + defaultto do + "https://#{Facter.value(:fqdn)}/ipa" + end + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newparam(:api_username) do + desc 'Username for authentication to the API. This user must be an admin user.' + + isrequired + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newparam(:api_password) do + desc 'Password for authentication to the API.' + + isrequired + + sensitive true + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + validate do + PuppetX::Encore::Ipa::TypeUtils.validate_required_attributes(self) + end +end From 4d7e9b524651d69bcfeaf13da078445729853b1e Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Fri, 19 Feb 2021 09:46:08 -0500 Subject: [PATCH 09/21] Ability to create and manage kerberos services and keytabs --- lib/puppet/provider/ipa_service/default.rb | 91 ++++++++++++++++++++++ lib/puppet/type/ipa_service.rb | 66 ++++++++++++++++ manifests/init.pp | 1 + manifests/keytab.pp | 44 +++++++++++ manifests/params.pp | 1 + 5 files changed, 203 insertions(+) create mode 100644 lib/puppet/provider/ipa_service/default.rb create mode 100644 lib/puppet/type/ipa_service.rb create mode 100644 manifests/keytab.pp diff --git a/lib/puppet/provider/ipa_service/default.rb b/lib/puppet/provider/ipa_service/default.rb new file mode 100644 index 0000000..58a7375 --- /dev/null +++ b/lib/puppet/provider/ipa_service/default.rb @@ -0,0 +1,91 @@ +require 'puppet/provider/ipa' + +Puppet::Type.type(:ipa_service).provide(:default, parent: Puppet::Provider::Ipa) do + defaultfor kernel: 'Linux' + + # always need to define this in our implementation classes + mk_resource_methods + + ########################## + # private methods that we need to implement because we inherit from Puppet::Provider::Synapse + + # Read all instances of this type from the API, this will then be stored in a cache + # We do it like this so that the first resource of this type takes the burden of + # reading all of the data, but the following resources are all super fast because + # they can use the global cache + def read_all_instances + # read all of the groups, once + body = { + 'id' => 0, + 'method' => 'service_find/1', + 'params' => [ + # args (positional arguments) + [], + # options (CLI flags / options) + { + 'all' => true, + }, + ], + } + response_body = api_post('/session/json', body: body, json_parse: true) + service_list = response_body['result']['result'] + Puppet.debug("Got Service list: #{service_list}") + + instance_hash = {} + service_list.each do |service| + instance = { + ensure: :present, + name: get_ldap_attribute(service, 'krbprincipalname'), + } + instance_hash[instance[:name]] = instance + end + Puppet.debug("Returning Service instances: #{instance_hash}") + instance_hash + end + + # this method should check resource[:ensure] + # if it is :present this method should create/update the instance using the values + # in resource[:xxx] (these are the desired state values) + # else if it is :absent this method should delete the instance + # + # if you want to have access to the values before they were changed you can use + # cached_instance[:xxx] to compare against (that's why it exists) + def flush_instance + # write a single instance at a time + # we can't bulk write because instances may be written in different orders depending + # on their relationships defined in PuppetDSL + case resource[:ensure] + when :absent + body = { + 'id' => 0, + 'method' => 'service_del/1', + 'params' => [ + # args (positional arguments) + [resource[:name]], + # options (CLI flags / options) + {}, + ], + } + api_post('/session/json', body: body) + when :present + method = if cached_instance[:ensure] == :absent + # if the group was absent, we need to add + 'service_add/1' + else + # if the group was present then we need to modify + 'service_mod/1' + end + body = { + 'id' => 0, + 'method' => method, + 'params' => [ + # args (positional arguments) + [resource[:name]], + # options (CLI flags / options) + {}, + ], + } + api_post('/session/json', body: body) + end + end +end diff --git a/lib/puppet/type/ipa_service.rb b/lib/puppet/type/ipa_service.rb new file mode 100644 index 0000000..83ae3dd --- /dev/null +++ b/lib/puppet/type/ipa_service.rb @@ -0,0 +1,66 @@ +require 'puppet_x/encore/ipa/type_utils' + +Puppet::Type.newtype(:ipa_service) do + desc 'Manages a Kerberos Service principal in IPA' + + ensurable do + newvalue(:present) do + provider.create + end + + newvalue(:absent) do + provider.destroy + end + + defaultto :present + end + + # namevar is always a parameter + newparam(:name, namevar: true) do + desc 'Name of the service' + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newparam(:api_url) do + desc 'URL of the IPA API. Note: we will append endpoints to the end of this. Default: https:///ipa' + + isrequired + + defaultto do + "https://#{Facter.value(:fqdn)}/ipa" + end + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newparam(:api_username) do + desc 'Username for authentication to the API. This user must be an admin user.' + + isrequired + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + newparam(:api_password) do + desc 'Password for authentication to the API.' + + isrequired + + sensitive true + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + + validate do + PuppetX::Encore::Ipa::TypeUtils.validate_required_attributes(self) + end +end diff --git a/manifests/init.pp b/manifests/init.pp index da7f82d..6208d17 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -214,6 +214,7 @@ String $ipa_master_fqdn = '', String $ipa_role = 'client', String $ipa_server_fqdn = $facts['fqdn'], + String $keytab_default = $ipa::params::keytab_default, Boolean $manage_host_entry = true, Boolean $manage = true, Boolean $mkhomedir = false, diff --git a/manifests/keytab.pp b/manifests/keytab.pp new file mode 100644 index 0000000..e8aecdf --- /dev/null +++ b/manifests/keytab.pp @@ -0,0 +1,44 @@ +# Retrieves a Kerberos Service keytab and stores it in a local file. +define ipa::keytab ( + String $principal = $name, + Optional[String] $file = undef, + Optional[String] $server = undef, + Optional[String] $username = undef, + Optional[String] $password = undef, + Boolean $file_manage = true, + String $owner = 'root', + String $group = 'root', + String $mode = '0600', +) { + include ipa + $_file = pick($file, $ipa::keytab_default) + $_server = pick($server, $ipa::ipa_master_fqdn) + if $ipa::ipa_role == 'client' { + $_username = pick($username, $ipa::domain_join_principal) + $_password = pick($password, $ipa::domain_join_password) + } + else { + $_username = pick($username, $ipa::admin_user) + $_password = pick($password, $ipa::admin_password) + } + + $_ldap_domain = $ipa::domain.split(/\./).map |$item| { "dc=${item}" }.join(',') + $_username_dn = "uid=${_username},cn=users,cn=accounts,${_ldap_domain}" + + exec { "ipa-getkeytab -p ${principal} -k ${_file}": + command => "ipa-getkeytab -s ${_server} -p ${principal} -k ${_file} -D ${_username_dn} -w \"\$IPA_PASSWORD\"", + unless => "klist -k ${_file} | grep -q '${principal}'", + environment => [ "IPA_PASSWORD=${_password}" ], + path => ['/usr/bin', '/bin', '/usr/sbin', '/sbin'], + } + + if $file_manage { + file { $_file: + ensure => file, + owner => $owner, + group => $group, + mode => $mode, + subscribe => Exec["ipa-getkeytab -p ${principal} -k ${_file}"], + } + } +} diff --git a/manifests/params.pp b/manifests/params.pp index 435fc7b..4ec6d7f 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -10,6 +10,7 @@ $autofs_service = 'autofs' $sssd_service = 'sssd' + $keytab_default = '/etc/krb5.keytab' $ds_ssl_min_version_tls12 = 'TLS1.2' $ds_ssl_ciphers_tls12 = [ From 07dc92c743524b652edf21610a67e918cf9f3979 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Fri, 19 Feb 2021 10:20:35 -0500 Subject: [PATCH 10/21] Added ability to specify other config values --- manifests/init.pp | 23 +++++++++++++++++++ manifests/install/client.pp | 2 ++ manifests/install/server/master.pp | 2 ++ templates/sssd.conf.epp | 37 ++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+) diff --git a/manifests/init.pp b/manifests/init.pp index 6208d17..f12217c 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -171,6 +171,28 @@ # # (integer) The HTTPS port to use for the reverse proxy. Cannot be 443. # +# @param [Hash[String, Hash[String, Any]]] sssd_config_entries +# Hash of extra configuration entries to include in the sssd config. +# The hash should be in the format of +# { +# 'section' => { +# 'key' => 'value', +# }, +# 'other_section' => { +# 'key2' => 'value2', +# }, +# } +# Example: +# { +# 'domain/ipa.domain.tld' => { +# 'krb5_renewable_lifetime' => '50d', +# 'krb5_renew_interval' => 3600, +# }, +# 'sudo' => { +# 'sudo_threshold' => 100, +# }, +# } +# class ipa ( String $admin_user = 'admin', String $admin_password = '', @@ -229,6 +251,7 @@ Boolean $server_install_ldaputils = true, Array[String] $sssd_services = ['nss','sudo','pam','ssh','autofs'], Boolean $trust_dns = true, + Hash[String, Hash[String, Any]] $sssd_config_entries = {}, ) inherits ipa::params { if $manage { diff --git a/manifests/install/client.pp b/manifests/install/client.pp index abf129f..6a6ea6e 100644 --- a/manifests/install/client.pp +++ b/manifests/install/client.pp @@ -23,6 +23,7 @@ String $sssd_debug_level = $ipa::sssd_debug_level, String $sssd_package_name = $ipa::params::sssd_package_name, Array[String] $sssd_services = $ipa::sssd_services, + Hash[String, Hash[String, Any]] $sssd_config_entries = $ipa::sssd_config_entries, ) inherits ipa { package{ 'ipa-client': ensure => $client_ensure, @@ -90,6 +91,7 @@ override_homedir => $override_homedir, sssd_debug_level => $sssd_debug_level, sssd_services => $sssd_services, + config_entries => $sssd_config_entries, }), mode => '0600', require => [ diff --git a/manifests/install/server/master.pp b/manifests/install/server/master.pp index b152334..4244d7a 100644 --- a/manifests/install/server/master.pp +++ b/manifests/install/server/master.pp @@ -25,6 +25,7 @@ Optional[String] $override_homedir = $ipa::override_homedir, String $sssd_debug_level = $ipa::sssd_debug_level, Array[String] $sssd_services = $ipa::sssd_services, + Hash[String, Hash[String, Any]] $sssd_config_entries = $ipa::sssd_config_entries, ) { # Build server-install command @@ -92,6 +93,7 @@ override_homedir => $override_homedir, sssd_debug_level => $sssd_debug_level, sssd_services => $sssd_services, + config_entries => $sssd_config_entries, }), mode => '0600', require => Exec["server_install_${$facts['fqdn']}"], diff --git a/templates/sssd.conf.epp b/templates/sssd.conf.epp index fbb5c96..1e9d1a1 100644 --- a/templates/sssd.conf.epp +++ b/templates/sssd.conf.epp @@ -12,6 +12,7 @@ String $ad_site = '', String $ad_ldap_search_base = '', Optional[String] $override_homedir = undef, + Hash[String, Hash[String, Any]] $config_entries = {}, |-%> [domain/<%= $domain %>] debug_level = <%= $sssd_debug_level %> @@ -42,6 +43,9 @@ subdomain_inherit = ldap_user_principal, ignore_group_members subdomain_inherit = ldap_user_principal <% } -%> ldap_user_principal = nosuchattr +<% pick($config_entries["domain/$domain"], {}).each |$cfg_key, $cfg_value| { -%> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> <% if $ad_domain != '' { -%> [domain/<%= $domain -%>/<%= $ad_domain %>] @@ -51,12 +55,18 @@ ad_site = <%= $ad_site %> <%- if $ad_ldap_search_base != '' { -%> ldap_search_base = <%= $ad_ldap_search_base %> <%- } -%> +<% pick($config_entries["domain/${domain}/${ad_domain}"], {}).each |$cfg_key, $cfg_value| { %> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> <% } -%> [sssd] debug_level = <%= $sssd_debug_level %> services = <%= $sssd_services.join(',') %> domains = <%= $domain %> +<% pick($config_entries["sssd"], {}).each |$cfg_key, $cfg_value| { %> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> [nss] debug_level = <%= $sssd_debug_level %> @@ -68,28 +78,55 @@ default_shell = /bin/bash <% if ($ipa_role == 'master') or ($ipa_role == 'replica') { -%> memcache_timeout = 300 <% }-%> +<% pick($config_entries["nss"], {}).each |$cfg_key, $cfg_value| { %> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> [pam] debug_level = <%= $sssd_debug_level %> +<% pick($config_entries["pam"], {}).each |$cfg_key, $cfg_value| { %> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> [sudo] debug_level = <%= $sssd_debug_level %> +<% pick($config_entries["sudo"], {}).each |$cfg_key, $cfg_value| { %> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> [autofs] debug_level = <%= $sssd_debug_level %> +<% pick($config_entries["autofs"], {}).each |$cfg_key, $cfg_value| { %> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> [ssh] debug_level = <%= $sssd_debug_level %> +<% pick($config_entries["ssh"], {}).each |$cfg_key, $cfg_value| { %> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> [pac] debug_level = <%= $sssd_debug_level %> +<% pick($config_entries["pac"], {}).each |$cfg_key, $cfg_value| { %> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> [ifp] debug_level = <%= $sssd_debug_level %> +<% pick($config_entries["ipf"], {}).each |$cfg_key, $cfg_value| { %> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> <% if ($ipa_role == 'master') or ($ipa_role == 'replica') { -%> [secrets] debug_level = <%= $sssd_debug_level %> +<% pick($config_entries["secrets"], {}).each |$cfg_key, $cfg_value| { %> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> [session_recording] +<% pick($config_entries["session_recording"], {}).each |$cfg_key, $cfg_value| { %> +<%= $cfg_key %> = <%= $cfg_value %> +<% } -%> <% } -%> From fab6d4537830201e7ea980ed22569934a89e1db8 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Sat, 20 Feb 2021 12:18:56 -0500 Subject: [PATCH 11/21] Fixed spacing in server install --- manifests/install/server.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/install/server.pp b/manifests/install/server.pp index bd6e1ca..73db7de 100644 --- a/manifests/install/server.pp +++ b/manifests/install/server.pp @@ -105,7 +105,7 @@ } # This will set the SSL protocols to use. - if !defined(Service[ 'httpd']) { + if !defined(Service['httpd']) { service { 'httpd': ensure => running, enable => true, From 881b858065f03edd2c6f7e33a5b787ce106914ba Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Sat, 20 Feb 2021 12:55:03 -0500 Subject: [PATCH 12/21] Fix linting --- lib/puppet/provider/ipa.rb | 2 +- lib/puppet/type/ipa_group_membership.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/puppet/provider/ipa.rb b/lib/puppet/provider/ipa.rb index 642d640..3f5a2ab 100644 --- a/lib/puppet/provider/ipa.rb +++ b/lib/puppet/provider/ipa.rb @@ -216,6 +216,6 @@ def get_ldap_attribute(obj, attr) def get_ldap_attribute_boolean(obj, attr) # values can be: "TRUE", "True", or "true" # casecmp does case insensitive comparison and returns 0 if equal - get_ldap_attribute(obj, attr).casecmp('true') == 0 + get_ldap_attribute(obj, attr).casecmp('true').zero? end end diff --git a/lib/puppet/type/ipa_group_membership.rb b/lib/puppet/type/ipa_group_membership.rb index 19497d9..29fe815 100644 --- a/lib/puppet/type/ipa_group_membership.rb +++ b/lib/puppet/type/ipa_group_membership.rb @@ -123,6 +123,6 @@ end autorequire(:ipa_user) do - @parameters[:users] ? @parameters[:users].should : [] + (@parameters[:users]) ? @parameters[:users].should : [] end end From 6774c38a06400cd8bc020ab5774f545cdc9fb950 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Sat, 20 Feb 2021 22:00:41 -0500 Subject: [PATCH 13/21] Fixing replica installs --- manifests/install/server/replica.pp | 58 ++++++++++++++++++----------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/manifests/install/server/replica.pp b/manifests/install/server/replica.pp index f6b6e2c..c9810f9 100644 --- a/manifests/install/server/replica.pp +++ b/manifests/install/server/replica.pp @@ -2,7 +2,14 @@ class ipa::install::server::replica ( String $admin_pass = $ipa::admin_password, String $admin_user = $ipa::admin_user, - String $install_opts = $ipa::install::server::server_install_cmd_opts_setup_ca, + String $cmd_opts_setup_ca = $ipa::install::server::server_install_cmd_opts_setup_ca, + String $cmd_opts_dns = $ipa::install::server::server_install_cmd_opts_setup_dns, + String $cmd_opts_dnssec = $ipa::install::server::server_install_cmd_opts_dnssec_validation, + String $cmd_opts_forwarders = $ipa::install::server::server_install_cmd_opts_forwarders, + String $cmd_opts_hostname = $ipa::install::server::server_install_cmd_opts_hostname, + String $cmd_opts_ntp = $ipa::install::server::server_install_cmd_opts_no_ntp, + String $cmd_opts_ui = $ipa::install::server::server_install_cmd_opts_no_ui_redirect, + String $cmd_opts_zones = $ipa::install::server::server_install_cmd_opts_zone_overlap, String $ipa_role = $ipa::ipa_role, Sensitive[String] $principal_pass = $ipa::final_domain_join_password, String $principal_user = $ipa::final_domain_join_principal, @@ -14,7 +21,13 @@ ipa-replica-install \ --principal=${principal_user} \ --admin-password=\$IPA_ADMIN_PASS \ - ${install_opts} \ + ${cmd_opts_setup_ca} \ + ${cmd_opts_dnssec} \ + ${cmd_opts_forwarders} \ + ${cmd_opts_ntp} \ + ${cmd_opts_ui} \ + ${cmd_opts_dns} \ + ${cmd_opts_zones} \ --unattended | EOC @@ -25,27 +38,28 @@ contain ipa::helpers::firewalld - if str2bool($facts['ipa_installed']) != true { - include ipa::install::server::kinit + include ipa::install::server::kinit - # Needed to ensure ipa-replica-install succeeds if new client is installed. - exec { 'replica_restart_sssd': - command => inline_epp($service_restart, {'service' => $sssd_service}), - path => ['/sbin', '/bin', '/usr/bin'], - tag => 'ipa::install::replica', - refreshonly => true, - } - ~> Ipa_kinit[$admin_user] - -> exec { 'replica_server_install': - command => $replica_install_cmd, - environment => [ "IPA_ADMIN_PASS=${principal_pass.unwrap}" ], - path => ['/sbin', '/usr/sbin'], - timeout => 0, - unless => '/usr/sbin/ipactl status >/dev/null 2>&1', - logoutput => 'on_failure', - require => Class['ipa::helpers::firewalld'], - notify => Ipa::Helpers::Flushcache["server_${$facts['fqdn']}"], - } + # Needed to ensure ipa-replica-install succeeds if new client is installed. + exec { 'replica_restart_sssd': + command => inline_epp($service_restart, {'service' => $sssd_service}), + environment => [ "IPA_ADMIN_PASS=${principal_pass.unwrap}" ], + # try to kinit and restart sssd if kinit fails + unless => "echo \$IPA_ADMIN_PASS | kinit ${principal_user}", + path => ['/sbin', '/bin', '/usr/bin', '/usr/sbin'], + tag => 'ipa::install::replica', + } + -> Ipa_kinit[$admin_user] + -> exec { 'replica_server_install': + command => $replica_install_cmd, + environment => [ "IPA_ADMIN_PASS=${principal_pass.unwrap}" ], + path => ['/bin', '/sbin', '/usr/bin', '/usr/sbin'], + timeout => 0, + # only attempt to reinstall if IPA is not configured + onlyif => '/usr/sbin/ipactl status | grep -i "IPA is not configured"', + logoutput => 'on_failure', + require => Class['ipa::helpers::firewalld'], + notify => Ipa::Helpers::Flushcache["server_${$facts['fqdn']}"], } facter::fact { 'ipa_installed': From 91408ea722f99f4b872b58316bbca5744752b6f7 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Sat, 20 Feb 2021 22:17:40 -0500 Subject: [PATCH 14/21] Fixing replica installs --- manifests/install/server/replica.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/install/server/replica.pp b/manifests/install/server/replica.pp index c9810f9..bc87e2d 100644 --- a/manifests/install/server/replica.pp +++ b/manifests/install/server/replica.pp @@ -56,7 +56,7 @@ path => ['/bin', '/sbin', '/usr/bin', '/usr/sbin'], timeout => 0, # only attempt to reinstall if IPA is not configured - onlyif => '/usr/sbin/ipactl status | grep -i "IPA is not configured"', + onlyif => 'ipactl status 2>&1 | grep -i "IPA is not configured"', logoutput => 'on_failure', require => Class['ipa::helpers::firewalld'], notify => Ipa::Helpers::Flushcache["server_${$facts['fqdn']}"], From f321fa027d5245a39d45ff52c996394336b68a29 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Fri, 30 Jul 2021 13:29:40 -0400 Subject: [PATCH 15/21] Fixing keytab create --- manifests/keytab.pp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/manifests/keytab.pp b/manifests/keytab.pp index e8aecdf..9665a3b 100644 --- a/manifests/keytab.pp +++ b/manifests/keytab.pp @@ -5,6 +5,8 @@ Optional[String] $server = undef, Optional[String] $username = undef, Optional[String] $password = undef, + # default is retrieve, otherwise keytab will overwrite everyone else + Enum['retrieve', 'create'] $action = 'retrieve', Boolean $file_manage = true, String $owner = 'root', String $group = 'root', @@ -25,8 +27,15 @@ $_ldap_domain = $ipa::domain.split(/\./).map |$item| { "dc=${item}" }.join(',') $_username_dn = "uid=${_username},cn=users,cn=accounts,${_ldap_domain}" + case $action { + # when retrieving keytab, need to pass in -r + 'retrieve': { $action_args = '-r' } + 'create': { $action_args = '' } + default: { $action_args = '' } + } + exec { "ipa-getkeytab -p ${principal} -k ${_file}": - command => "ipa-getkeytab -s ${_server} -p ${principal} -k ${_file} -D ${_username_dn} -w \"\$IPA_PASSWORD\"", + command => "ipa-getkeytab ${action_args} -s ${_server} -p ${principal} -k ${_file} -D ${_username_dn} -w \"\$IPA_PASSWORD\"", unless => "klist -k ${_file} | grep -q '${principal}'", environment => [ "IPA_PASSWORD=${_password}" ], path => ['/usr/bin', '/bin', '/usr/sbin', '/sbin'], From b36a1c9cac256a4192b0697f8a061d6f754c9f96 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Tue, 7 Sep 2021 08:22:52 -0400 Subject: [PATCH 16/21] Added more features to ipa_user resource --- lib/puppet/provider/ipa.rb | 17 ++++-- lib/puppet/provider/ipa_user/default.rb | 9 ++++ lib/puppet/type/ipa_user.rb | 24 +++++++++ lib/puppet_x/encore/ipa/boolean_property.rb | 57 +++++++++++++++++++++ manifests/user.pp | 6 +++ 5 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 lib/puppet_x/encore/ipa/boolean_property.rb diff --git a/lib/puppet/provider/ipa.rb b/lib/puppet/provider/ipa.rb index 3f5a2ab..0ec1141 100644 --- a/lib/puppet/provider/ipa.rb +++ b/lib/puppet/provider/ipa.rb @@ -214,8 +214,19 @@ def get_ldap_attribute(obj, attr) end def get_ldap_attribute_boolean(obj, attr) - # values can be: "TRUE", "True", or "true" - # casecmp does case insensitive comparison and returns 0 if equal - get_ldap_attribute(obj, attr).casecmp('true').zero? + ldap_attr = get_ldap_attribute(obj, attr) + # FreeIPA is inconsistent + # sometimes values can be straight booleans + # sometimes they can be strings + case ldap_attr + when true, false # yes, this is how you do it in Ruby because there is no Boolean type + return ldap_attr + when String + # values can be: "TRUE", "True", or "true" + # casecmp does case insensitive comparison and returns 0 if equal + return ldap_attr.casecmp('true').zero? + else + raise Puppet::Error, "Unable to convert LDAP Attribute #{attr} from #{ldap_attr.class.name} to Boolean" + end end end diff --git a/lib/puppet/provider/ipa_user/default.rb b/lib/puppet/provider/ipa_user/default.rb index 1fea5d4..82899c7 100644 --- a/lib/puppet/provider/ipa_user/default.rb +++ b/lib/puppet/provider/ipa_user/default.rb @@ -36,11 +36,15 @@ def read_all_instances instance = { ensure: :present, name: get_ldap_attribute(user, 'uid'), + # negate lock = enabled, yes use a symbol here + enable: get_ldap_attribute_boolean(user, 'nsaccountlock') ? :false : :true, first_name: get_ldap_attribute(user, 'givenname'), last_name: get_ldap_attribute(user, 'sn'), # surname } instance[:sshpubkeys] = get_ldap_attribute(user, 'ipasshpubkey') if user['ipasshpubkey'] + instance[:login_shell] = get_ldap_attribute(user, 'loginshell') if user['loginshell'] instance[:mail] = get_ldap_attribute(user, 'mail') if user['mail'] + instance[:job_title] = get_ldap_attribute(user, 'title') if user['title'] # save all LDAP attributes and we'll filter later instance[:ldap_attributes] = {} @@ -121,8 +125,13 @@ def flush_instance body['params'][1]['userpassword'] = resource[:initial_password] end + # negate enable = lock + # yes, use a symbol here + body['params'][1]['nsaccountlock'] = resource[:enable] == :false unless resource[:enable].nil? body['params'][1]['ipasshpubkey'] = resource[:sshpubkeys] if resource[:sshpubkeys] + body['params'][1]['loginshell'] = resource[:login_shell] if resource[:login_shell] body['params'][1]['mail'] = resource[:mail] if resource[:mail] + body['params'][1]['title'] = resource[:job_title] if resource[:job_title] # fill out additional LDAP attributes that the user is asking to sync if resource[:ldap_attributes] diff --git a/lib/puppet/type/ipa_user.rb b/lib/puppet/type/ipa_user.rb index 19e21f1..c7667ae 100644 --- a/lib/puppet/type/ipa_user.rb +++ b/lib/puppet/type/ipa_user.rb @@ -1,3 +1,5 @@ +require 'puppet/property/boolean' +require 'puppet_x/encore/ipa/boolean_property' require 'puppet_x/encore/ipa/list_property' require 'puppet_x/encore/ipa/type_utils' @@ -37,6 +39,12 @@ end end + newproperty(:enable, boolean: true, parent: PuppetX::Encore::Ipa::BooleanProperty) do + desc 'Account is enabled or not' + + defaultto :true # yes, use a symbol here + end + newproperty(:first_name) do desc 'First name for the user. This will be the "givenname" LDAP parameter.' @@ -83,6 +91,14 @@ def membership defaultto :minimum end + newproperty(:login_shell) do + desc 'Login shell for the user' + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + newproperty(:mail) do desc 'Email address of the user' @@ -91,6 +107,14 @@ def membership end end + newproperty(:job_title) do + desc 'Job title of the user' + + validate do |value| + PuppetX::Encore::Ipa::TypeUtils.validate_string(name, value) + end + end + newproperty(:ldap_attributes) do desc 'Hash of additional IPA attributes to set on the user' diff --git a/lib/puppet_x/encore/ipa/boolean_property.rb b/lib/puppet_x/encore/ipa/boolean_property.rb new file mode 100644 index 0000000..1968826 --- /dev/null +++ b/lib/puppet_x/encore/ipa/boolean_property.rb @@ -0,0 +1,57 @@ +require 'puppet_x/encore/ipa' + +module PuppetX::Encore::Ipa + # By default the Puppet Boolean property returns true/false values which is good for + # enabling things, but not good for disabling. The reason is that inside the Puppet + # code there is a lot of "if should" statements. This causes issues because if we + # return a "real" boolean, it won't get evaluated. Instead we map our boolean into + # a symbol :true or :false and compare against the symbol. + # + # Note: when using this property type, a symbol :true or :false will be returned + # NOT a boolean value + class BooleanProperty < Puppet::Property + # All values that are considered 'true' by Puppet internals + def true_values + [true, 'true', :true, :yes, 'yes'] + end + + # All values that are considered 'false' by Puppet internals + def false_values + [false, 'false', :false, :no, 'no', :undef, nil, :absent] + end + + def munge(v) + if true_values.include?(v) + :true + elsif false_values.include?(v) + :false + else + raise ArgumentError, "Value '#{v}':#{v.class} cannot be determined as a boolean value" + end + end + + def insync?(is) + munge(is) == munge(should) + end + + def self.defaultvalues + newvalue(:true) + newvalue(:false) + aliasvalue(true, :true) + aliasvalue(false, :false) + + aliasvalue('true', :true) + aliasvalue('false', :false) + + aliasvalue(:yes, :true) + aliasvalue(:no, :false) + + aliasvalue('yes', :true) + aliasvalue('no', :false) + + validate do |value| + munge(value) + end + end + end +end diff --git a/manifests/user.pp b/manifests/user.pp index bdd1e10..0bf85c4 100644 --- a/manifests/user.pp +++ b/manifests/user.pp @@ -82,7 +82,10 @@ String $ensure = 'present', String $first_name = $name, String $last_name = $name, + Boolean $enable = true, + Optional[String] $login_shell = undef, Optional[String] $mail = undef, + Optional[String] $job_title = undef, Optional[Hash] $ldap_attributes = undef, Boolean $manage_home_dir = true, String $home_dir_base = '', @@ -96,12 +99,15 @@ ) { ipa_user { $title: ensure => $ensure, + enable => $enable, name => $name, initial_password => $initial_password, first_name => $first_name, last_name => $last_name, sshpubkeys => $sshpubkeys, + login_shell => $login_shell, mail => $mail, + job_title => $job_title, ldap_attributes => $ldap_attributes, api_username => $ipa::admin_user, api_password => $ipa::admin_password, From 8dd6797783ab8cd1708f3ede04eac8281dfc4ced Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Wed, 2 Feb 2022 10:55:18 -0500 Subject: [PATCH 17/21] Fixed optional properites on dns zone --- lib/puppet/provider/ipa_dns_zone/default.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/puppet/provider/ipa_dns_zone/default.rb b/lib/puppet/provider/ipa_dns_zone/default.rb index 3e886b1..fe2f9a8 100644 --- a/lib/puppet/provider/ipa_dns_zone/default.rb +++ b/lib/puppet/provider/ipa_dns_zone/default.rb @@ -43,9 +43,14 @@ def read_all_instances instance = { ensure: :present, name: get_ldap_attribute(dnszone, 'idnsname'), - allow_dynamic_update: get_ldap_attribute_boolean(dnszone, 'idnsallowdynupdate'), - allow_sync_ptr: get_ldap_attribute_boolean(dnszone, 'idnsallowsyncptr'), } + # optional properties + dynamic_update = get_ldap_attribute_boolean(dnszone, 'idnsallowdynupdate'), + instance[:allow_dynamic_update] = dynamic_update if dynamic_update != :absent + + sync_ptr = get_ldap_attribute_boolean(dnszone, 'idnsallowsyncptr') + instance[:allow_sync_ptr] = sync_ptr if sync_ptr != :absent + instance_hash[instance[:name]] = instance end Puppet.debug("Returning group instances: #{instance_hash}") From 0eee490bcf90b75319f585e664da96314a83388e Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Wed, 2 Feb 2022 12:16:13 -0500 Subject: [PATCH 18/21] Fixed optional properites on dns zone --- lib/puppet/provider/ipa.rb | 2 ++ lib/puppet/provider/ipa_dns_zone/default.rb | 7 ++----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/puppet/provider/ipa.rb b/lib/puppet/provider/ipa.rb index 0ec1141..ef488ae 100644 --- a/lib/puppet/provider/ipa.rb +++ b/lib/puppet/provider/ipa.rb @@ -219,6 +219,8 @@ def get_ldap_attribute_boolean(obj, attr) # sometimes values can be straight booleans # sometimes they can be strings case ldap_attr + when :absent + return nil when true, false # yes, this is how you do it in Ruby because there is no Boolean type return ldap_attr when String diff --git a/lib/puppet/provider/ipa_dns_zone/default.rb b/lib/puppet/provider/ipa_dns_zone/default.rb index fe2f9a8..c9b6805 100644 --- a/lib/puppet/provider/ipa_dns_zone/default.rb +++ b/lib/puppet/provider/ipa_dns_zone/default.rb @@ -45,11 +45,8 @@ def read_all_instances name: get_ldap_attribute(dnszone, 'idnsname'), } # optional properties - dynamic_update = get_ldap_attribute_boolean(dnszone, 'idnsallowdynupdate'), - instance[:allow_dynamic_update] = dynamic_update if dynamic_update != :absent - - sync_ptr = get_ldap_attribute_boolean(dnszone, 'idnsallowsyncptr') - instance[:allow_sync_ptr] = sync_ptr if sync_ptr != :absent + instance[:allow_dynamic_update] = get_ldap_attribute_boolean(dnszone, 'idnsallowdynupdate') if dnszone['idnsallowdynupdate'] + instance[:allow_sync_ptr] = get_ldap_attribute_boolean(dnszone, 'idnsallowsyncptr') if dnszone['idnsallowsyncptr'] instance_hash[instance[:name]] = instance end From 9a7fc83e49f44770601f8736ecd259bfacc2b1d1 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Mon, 14 Mar 2022 15:38:59 -0400 Subject: [PATCH 19/21] Fixed issue with ipa-client-install missing sssd-ipa package dependency --- manifests/install/client.pp | 2 ++ manifests/install/sssd.pp | 3 ++- manifests/params.pp | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/manifests/install/client.pp b/manifests/install/client.pp index 6a6ea6e..9614d19 100644 --- a/manifests/install/client.pp +++ b/manifests/install/client.pp @@ -22,6 +22,7 @@ String $principal_user = $ipa::final_domain_join_principal, String $sssd_debug_level = $ipa::sssd_debug_level, String $sssd_package_name = $ipa::params::sssd_package_name, + String $sssd_ipa_package_name = $ipa::params::sssd_ipa_package_name, Array[String] $sssd_services = $ipa::sssd_services, Hash[String, Hash[String, Any]] $sssd_config_entries = $ipa::sssd_config_entries, ) inherits ipa { @@ -96,6 +97,7 @@ mode => '0600', require => [ Package[$sssd_package_name], + Package[$sssd_ipa_package_name], Exec["client_install_${$facts['fqdn']}"], ], notify => Ipa::Helpers::Flushcache["server_${$facts['fqdn']}"], diff --git a/manifests/install/sssd.pp b/manifests/install/sssd.pp index 99735bf..caf13c9 100644 --- a/manifests/install/sssd.pp +++ b/manifests/install/sssd.pp @@ -1,8 +1,9 @@ # class ipa::install::sssd ( String $sssd_package_name = $ipa::params::sssd_package_name, + String $sssd_ipa_package_name = $ipa::params::sssd_ipa_package_name, ) inherits ipa::params { - package { $sssd_package_name: + package { [$sssd_package_name, $sssd_ipa_package_name]: ensure => present, } diff --git a/manifests/params.pp b/manifests/params.pp index 4ec6d7f..a47ce91 100644 --- a/manifests/params.pp +++ b/manifests/params.pp @@ -134,6 +134,7 @@ $ipa_server_package_name = 'ipa-server' $kstart_package_name = 'kstart' $sssd_package_name = 'sssd-common' + $sssd_ipa_package_name = 'sssd-ipa' $sssdtools_package_name = 'sssd-tools' # In order to avoid this error: From 0f634b92fb8e43ec33d9c0ee24b9f17f1e2efb14 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Mon, 9 May 2022 12:57:59 -0400 Subject: [PATCH 20/21] Fixing facter for latest release --- manifests/install/server/master.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/install/server/master.pp b/manifests/install/server/master.pp index 4244d7a..70c449f 100644 --- a/manifests/install/server/master.pp +++ b/manifests/install/server/master.pp @@ -73,7 +73,7 @@ } facter::fact { 'ipa_installed': - value => true, + value => 'true', } # Updated master sssd.conf file after IPA is installed. From 83a08e7ec808c16f95d01e1ac696915bc7d2e4f2 Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Thu, 7 Jul 2022 15:43:41 -0400 Subject: [PATCH 21/21] Fix IPA replica fact --- manifests/install/server/replica.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/install/server/replica.pp b/manifests/install/server/replica.pp index bc87e2d..d7eb74a 100644 --- a/manifests/install/server/replica.pp +++ b/manifests/install/server/replica.pp @@ -63,7 +63,7 @@ } facter::fact { 'ipa_installed': - value => true, + value => 'true', } }