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

Add support for epp templates #300

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

patemery
Copy link
Contributor

@patemery patemery commented Mar 30, 2024

Add the capability to use epp templates in hiera data to create sudoers.d files. There are two solutions for this. If the epp template is a simple template, then it can simply be used with the template meta-parameter without any other changes as in the following example.

sudo::configs:
    'os_admins':
        'template': "mytemplates/osadmins.epp"
    'dba_admins':
        'template': "mytemplates/dbaadmins.erb"

If the epp template is more complex and requires input parameters, there is a new meta-parameter template_epp. This meta-parameter requires two input parameters; filename (string) and params (hash). There are examples of this given in the README.

Additionally, I've added two new epp templates that allow for some more elaborate sudoers.d allocations.

@saz
Copy link
Owner

saz commented May 13, 2024

@patemery Please rebase this MR, fix issues and squash commits

@@ -40,6 +40,7 @@
$content = undef,
$source = undef,
$template = undef,
$template_epp = undef,
Copy link
Owner

Choose a reason for hiding this comment

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

Add a type:

Optional[Hash[String[1], Hash]] $template = undef,

This will also make validation easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried your type suggestion but couldn't get it to work. It all looks right to me. But I just can't get it.

image
image
image

I'd welcome any suggestions you might have

Copy link
Owner

Choose a reason for hiding this comment

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

Try it with the following, please.

Optional[Struct[{ filename => String, params => Hash }]]

Copy link
Owner

Choose a reason for hiding this comment

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

btw: it would be good to have some tests for this functionality

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried your new type and it worked perfectly. Thanks for that. I definitely would not have ever come up with that.

@patemery
Copy link
Contributor Author

patemery commented May 14, 2024

Wow. I don't have a lot of git experience. It took me about 3 hours to get this all cleaned up. I think I have it the way you wanted it. Thank you so much for considering my enhancement. Let me know if I am still missing anything

@saz
Copy link
Owner

saz commented May 14, 2024

@patemery I've just merged a PR, which adds data types to all parameters in sudo::conf. It would be great to rebase your branch against the master branch of this module

@patemery
Copy link
Contributor Author

@saz I merged all of your latest commits and tried your latest type suggestion. It worked great. Thank you for that. I would have never come up with that.

I looked at the existing tests. I'm sad to admit I've never written any rspec tests. I think I can create a few following the existing code as examples. Can you tell me how to execute the tests on my local system? I downloaded and installed pdk and I tried doing pdk test unit and got the error This module is not PDK compatible.

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