Puppet-Finland / puppet-ipa

Puppet module that can manage an IPA master, replicas and clients.
5 stars 11 forks source link

ipa_force_join custom fact not present in module, but referenced #58

Closed canihavethisone closed 7 months ago

canihavethisone commented 7 months ago

Line 50 of /install/client.pp references a custom fact ipa_force_join, however the fact is not present and therefore the option to invoke --force-join cannot be enabled. Can you please advise what the fact content should/could be, and if required I will create a PR for it? Please don't just remove the logic as I would very much like to be able to invoke the force-join option.

If the fact option is no longer desired, perhaps the option could be changed to a param if set to true?

Thank you, and cheers for an awesome module!

Edit: GPT suggests this condition check, but I have not yet tested it:

Facter.add('ipa_force_join') do
  confine :osfamily => 'RedHat'
  setcode do
    if File.exist?('/etc/ipa/default.conf') && !File.exist?('/etc/ipa/default.conf.bak')
      # The IPA client configuration exists but doesn't have a backup file, indicating it's not enrolled
      false
    elsif File.exist?('/etc/ipa/default.conf.bak')
      # The IPA client configuration has a backup file, indicating it's enrolled but may have been unenrolled later
      true
    else
      # No evidence of IPA client installation
      false
    end
  end
end
mattock commented 7 months ago

The $ipa_force_join fact is supposed to be set by you when you need it. A simple structured data fact will do it. The main use-case for us has been reprovisioning nodes - we know that ipa-client-install will fail in those cases because the old machine entry etc. already exists in FreeIPA, so --force-join is required. At the end of reprovisioning we automatically remove the structured fact so the next Puppet run will not do a --force-join again.

What ChatGPT suggests look interesting, but I have no idea if it actually works. LLM output tends to look good but could be complete garbage when you test it :smile:. I also think that the force-join decision should be left to the admin and should not rely on automatic logic which may have side-effects.

canihavethisone commented 7 months ago

Thanks for your super fast reply! So is the tl:dr that the implementer should construct and provide a custom fact themselves if they want to leverage the class logic? If so, do you have a working example that you use?

Another thing I observed is the using the --force-join option on first join has no side effect, so I wonder if it should be in the command line string by default?

Edit: I'm guessing you just stub the fact as described with something like ipa_force_join=true in a txt file?

canihavethisone commented 7 months ago

Accepting advice from module owner, thank you