saltstack-formulas / bareos-formula

A Saltstack formula to install and configure Bareos
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
5 stars 12 forks source link

Various small fixes in bareos formula #5

Closed pkleanthous-zz closed 6 years ago

pkleanthous-zz commented 6 years ago

Use pillar file instead of populating the kitchen.yml

This change is necessary because:
* It's easier and cleaner to have pillar data in a separate file

The issue is resolved in this commit by:
* Adding a new pillar file

Clean up default configuration from all services

This change is necessary because:
* Bareos 16.x comes with a new configuration structure,
they introduced several directories and split the configuration into different files.
As we would like to have a central config file for every service,
it's good to clean up the default config and structure.

The issue is resolved in this commit by:
* When we install a new package, we automatically clean up those directories.

Fixed jinja template IF statements

This change is necessary as the template wasn't adding
the name & password <key:value> elements when the config object  
had a <key> that partially contains those strings.

Example
-------
yaml object

director:
  config:
    Catalog:
      MyCatalog:
        DbName: bareos

The 'Name = MyCatalog' was missing from the generated config 
as the object has DbName <key>

Change the object format that we pass to Jinja template

As pillar is in yaml format, it's better to pass the same format to Jinja template

Fix map.jinja file

Merge the file contents with pillar data

Clean up repo.sls code

Use the same syntax when calling grains

Security hardening

Change file permissions and owner/group

Remove Debian-9 as supported platform

pkleanthous-zz commented 6 years ago

Hi @javierbertoli

Thanks for the review. I agree with your comments:

My only concern it's for the jinja template. As is, it causes problems when the words 'name' & 'password' exist in the pillar data.

Examples:

DbName: bareos
Description: File device. A connecting Director must have the same Name and MediaType.
dbpassword: "whatever"

I will try to find a solution on this, any suggestions are welcomed 😄

javierbertoli commented 6 years ago

Thanks @pkleanthous. Yes, I kept thinking of a possible solution to this. I'm wondering if we can use some of the new regex searches, like:

{%- if type|lower in require_password and
         config | regex_match('^password$', ignorecase=True)  %}

perhaps that works as we need it?

pkleanthous-zz commented 6 years ago

Hi @javierbertoli

I found a way to fix the issue inside jinja template

            {% if (type|lower) in requires_name and ( (config.name is not defined) and (config.Name is not defined) )  %}
    Name = "{{ name }}"
            {%- endif %}

I will write a simple python code to test the template so we will be able to render and verify it without running salt every time.

javierbertoli commented 6 years ago

@pkleanthous, but that will still not consider NAME, nAME, etc, that bareos accepts (the doc says it does not care for case, not even whitespaces, in names for the lvalues).

That's why I tried, originally, converting all to lowercase and comparing: that way, no matter how you named your vars, it would match. The only case I left aside was the one with whitespaces in the lvalue, like N a m e, but I considered it a edge case.

That's why I proposed the config | regex_match('^password$', ignorecase=True) option (thought I didn't try it yet and I'm not quite sure it will work as I expect it to): it compares whole string, case insensitive

pkleanthous-zz commented 6 years ago

@javierbertoli I tried using regex_match but it's saltstack's function not a built-in function of jinja2 http://jinja.pocoo.org/docs/2.10/templates/

pkleanthous-zz commented 6 years ago

Hi @javierbertoli

Can't find a way to capture all the edge cases so on whatever lower or upper case the user adds the key to capture it, either at SaltStack or jinja level.

The guys in #pocoo irc channel and they suggested that modifying a dict in a template sounds wrong...

I suggest going with the 2 most common cases

Let me know your thoughts. Also, do you have any additional comments?

javierbertoli commented 6 years ago

@pkleanthous let's capture those cases, as they are a bug right now, and I'll see if I can play with the formula over the weekend to see if we can find a more general rule to capture those MiXeD cases.

Other than that, I think you made a great work so I'd be happy to merge when you pushe these changes you're working on.

javierbertoli commented 6 years ago

Thanks for all this great work, @pkleanthous !