Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SVCPLAN-6686: Add support for AD createhost functionality #25

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

billglick
Copy link
Member

This has been tested on cc-test[01-03].

For more details on this, see: https://wiki.ncsa.illinois.edu/display/ICI/AD%2C+Automated+Linux+Computer+Add+and+Join+Domain

@billglick billglick requested review from bsper2 and a team December 4, 2024 21:17
@billglick billglick self-assigned this Dec 4, 2024
@billglick billglick force-pushed the wglick/SVCPLAN-6686/ad_createhost_prinicpal branch 2 times, most recently from 5dadc46 to 6726ca7 Compare December 4, 2024 22:30
manifests/kerberos.pp Outdated Show resolved Hide resolved
@billglick billglick force-pushed the wglick/SVCPLAN-6686/ad_createhost_prinicpal branch 2 times, most recently from 87d98d4 to a98606b Compare December 6, 2024 18:40
@billglick billglick force-pushed the wglick/SVCPLAN-6686/ad_createhost_prinicpal branch from c4946c1 to f02d3cc Compare December 6, 2024 18:46
}

# Step 3: Ensure the temporary script file is deleted once it has been run
exec { 'delete_ad_create_host_keytab_script':
Copy link
Member

Choose a reason for hiding this comment

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

Why delete the /root/ad_createhostkeytab.sh script? Unless I'm missing something it looks like all the sensitive data is in the arguments for the script and not in the script itself.

Copy link
Member Author

@billglick billglick Dec 6, 2024

Choose a reason for hiding this comment

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

Yes, that was a late pivot to remove all the sensitive data from the script. I wrote the logic for removing the script before I ended up removing the sensitive data from the script.

It could be left on the host, but there really isn't much use for it to stay there. But the removal of the script was inspired from the current #19 issue where it was requested that the older createhostkeytab.sh be removed.

Are you suggesting that there is a reason to keep the script around?

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha, no real reason to keep it, but knowing there isn't anything sensitive in the script I think the logic can be simplified.

Make a custom fact with the command "klist -kte 2>&1 | grep 'host/${facts['networking']['fqdn']}' | grep -i '${ad_domain}'" to be something like is_joined_to_ad_domain

Since that is now a fact and there is no sensitive info in the script itself, you should be safe to go back to using the File resource and the template to create the /root/ad_createhostkeytab.sh script.

So something like this:

if ( $ad_createhostkeytab and $ad_createhostuser and $ad_computers_ou and $ad_domain ) {

   if $facts['is_joined_ad_domain'] {
      $ensure_parm = 'absent'
   } else {
      $ensure_parm = 'present'

      exec { 'run_ad_create_host_keytab_script':
         path        => ['/usr/bin', '/usr/sbin', '/usr/lib/mit/bin'],
         command     => Sensitive(
           "/root/ad_createhostkeytab.sh '${ad_domain}' '${ad_computers_ou}' '${ad_createhostuser}' '${ad_createhostkeytab}' "
        ),
        refreshonly => true,
        require => File['/root/ad_createhostkeytab.sh'],        
      }

   }

   file { '/root/ad_createhostkeytab.sh':
      ensure    => $ensure_parm,
      content => #grab content from your template
      ... rest of standard stuff here ...
   }

}

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!!! That's great advice!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants