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

vars / defaults - possible naming conflict in code #757

Open
hhenkel opened this issue Aug 13, 2024 · 3 comments
Open

vars / defaults - possible naming conflict in code #757

hhenkel opened this issue Aug 13, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@hhenkel
Copy link

hhenkel commented Aug 13, 2024

Hi,

thank you for your software! I was toying around with your tool today and was also looking into other tools that are trying to achieve similar things. One thing I'm currently missing is the possibility to add files under vars/ to the readme.

I had a quick glance into the source code to get an understanding how things work and how time consuming it would be to add the functionality.

So the first question would be, if you're open for adding the files under vars/ to the readme? If so I see a naming conflict because you decided to store the data from defaults/ in the dict and everywhere else with the name var - see for example.

I could try to do a PR for renaming all the stuff related to defaults/ if you are considering changing the internal variable names.

@xoxys xoxys added the enhancement New feature or request label Aug 15, 2024
@xoxys
Copy link
Member

xoxys commented Aug 15, 2024

Hi, thanks for your feedback.

So the first question would be, if you're open for adding the files under vars/ to the readme?

Sure would be a nice enhancement to support annotations in vars files as well.

If you are considering changing the internal variable

I'm not sure if that's really required. There should be no duplicate names for Ansible variables in vars and defaults anyway. So we should just add parsing for files in the vars directory and store them in the var key of the internal data dict as well. Or am I missing something?

@hhenkel
Copy link
Author

hhenkel commented Aug 15, 2024

Sure would be a nice enhancement to support annotations in vars files as well.

Okay - I will look into that then - might take a while as I will be on vacation.

I'm not sure if that's really required. There should be no duplicate names for Ansible variables in vars and defaults anyway. So we should just add parsing for files in the vars directory and store them in the var key of the internal data dict as well. Or am I missing something?

My thought was, that it might be good to separate them, so it is possible to list them independently from the defaults. In our case, when importing/including a role, the variables that are defined under defaults/ are usually over-writable by providing variables to the include_task. Variables under vars/ are normally platform / os specific in our case, but we might have files defining different values for different versions of an os.

I thought about using the key "defaults" and "vars" to make it more obvious in the code. Another topic could be to store the actual filename that contain the variables, maybe like this:

self._data["defaults"]["main.yml"]
self._data["vars"]["redhat-redhat-8.9.yaml"]

Here is an example for my specific use case:

defaults/main.yaml => The "sane" defaults for a role that are independent from the OS. This also includes a variable <role_name>_cis_hardened: false so that the role uses the system defaults. If the target system needs CIS hardening, this is set to true.

As a default we follow the following schema to include var files if they exist via ansible.builtin.first_found:

vars/redhat-redhat-8.9.yaml => OS_Family-Distribution-Distribution_Version
vars/redhat-redhat-8.yaml => OS_Family-Distribution-Distribution_Major_Version
vars/redhat-redhat.yaml => OS_Family-Distribution
vars/redhat.yaml => OS_Family

If CIS hardening is used we read additional var files to overwrite the os default config following this schema:

vars/cis-redhat-redhat-8.9.yaml
vars/cis-redhat-redhat-8.yaml
vars/cis-redhat-redhat.yaml
vars/cis-redhat.yaml

It might be nice to see what different files exist under vars/ and what variables are defined in them.

ansible-mdgen, which creates a lot of pages, even adds information to the documentation where the variable is referenced.

@xoxys
Copy link
Member

xoxys commented Aug 15, 2024

Okay - I will look into that then - might take a while as I will be on vacation.

No worries, enjoy your vacation 🏖️

My thought was, that it might be good to separate them, so it is possible to list them independently from the defaults.

Thanks for the detailed information. Makes sense to me, and I'm also fine to separate vars and defaults in the data dict.

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

No branches or pull requests

2 participants