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

Custom sysctl #50

Closed
fitz123 opened this issue Oct 24, 2015 · 11 comments
Closed

Custom sysctl #50

fitz123 opened this issue Oct 24, 2015 · 11 comments

Comments

@fitz123
Copy link
Contributor

fitz123 commented Oct 24, 2015

Hi there!

Is there any way to rewrite some sysctl values? You're using /var, it's most prioritize variables.
I'm not sure how to make vars as "default", not mandatory, to be able change them in host/group vars

@rndmh3ro
Copy link
Member

The sysctl vars aren't meant to be overwritten. If you really want to, you could either change the vars in the vars-file or try putting the whole sysctl.yml content into the defaults/main.yml and overwrite it then. Make sure to empty the vars/sysctl.yml before, or remove the first tasks in the tasks/sysctl.yml.

Also what variable do you want to overwrite?

@fitz123
Copy link
Contributor Author

fitz123 commented Oct 25, 2015

Disable IPv4 traffic forwarding.

net.ipv4.ip_forward: 1

I'm using your role (and ssh also) for all my instances, including firewalls/gateways

@fitz123
Copy link
Contributor Author

fitz123 commented Oct 25, 2015

I'm not sure yet, but possible there is something else incompatible with ipvs, didn't finish testing yet

@rndmh3ro
Copy link
Member

I'll see if I can put the sysctl-variables in the defaults-file, so it's overwriteable.

@fitz123
Copy link
Contributor Author

fitz123 commented Oct 25, 2015

It would be great!
Thank you

@rndmh3ro
Copy link
Member

I hesitate to change this, because if you want to overwrite a single hash in the sysctl dict like this:

---
- name: wrapper playbook for kitchen testing "ansible-os-hardening" with custom vars for testing
  hosts: localhost
  roles:
   - ansible-os-hardening
  vars:
    sysctl_config:
    # Disable IPv4 traffic forwarding.
      net.ipv4.ip_forward: 1

you'll overwrite the whole dict, no only the single hash, meaning only the net.ipv4.ip_forward will actually be used. Unless you change ansibles hash-behaviour to merge. But this is not the default and ansible does not recommend it.

Now I see multiple options here:

  • put the sysctl-dict in the defaults and hope that the users know that they have to use merge or they read the documentation stating this.
  • wait for ansible 2.0 before changing this and maybe use the new hash combining feature, but I don't know if this will work.
  • rewrite the dict to normal variables, but this makes the output of the run ugly. See also issue Sysctl reloading #18 in this context.
  • use something like this:
ip_forward: 0
sysctl_config:
  # Disable IPv4 traffic forwarding.
  net.ipv4.ip_forward: {{ip_forward}}
  • But that can become ugly, too, when we have to define every user-changeable/overwritable variable twice. If we only let the user change certain settings, this would still be fine, but where do you draw the line then?
  • Keep it the way it currently is and let the user handle it by e.g. creating multiple roles

Right now I prefer the first or last option. What do you think @chris-rock? Let the user overwrite dict variables and then possibly break the whole sysctl-dict or keep it the way it currently is and be less modifiable?

@rndmh3ro
Copy link
Member

On alternate branch https://github.com/hardening-io/ansible-os-hardening/tree/mv_sysctl_to_defaults I moved the variables to the defaults and changed the docs. What do you guys think?

@fitz123
Copy link
Contributor Author

fitz123 commented Oct 29, 2015

Hi Sebastian,

Good CAUTION section

From one point of view having the dict with sysctl for all installations is
a good thing, but from another dict vs separate_values - what is the reason
for that except beauty?

If all the values is separated (like os_auth_*, it's not a dict by some
reason) then user is able to change them separately and drawback is only
more tasks in playbook.

Did I miss something?

On Thursday, October 29, 2015, Sebastian Gumprich notifications@github.com
wrote:

On alternate branch
https://github.com/hardening-io/ansible-os-hardening/tree/mv_sysctl_to_defaults
I moved the variables to the defaults and changed the docs. What do you
guys think?


Reply to this email directly or view it on GitHub
#50 (comment)
.

GOD! Save As!

@rndmh3ro
Copy link
Member

Hi @fitz123,

So I read #18 again and the reasons for doing it the way it currently is were:

  • beauty

and:

The first way only executes one task and is probably faster. Also only one sysctl-reload has to happen [for all changes]

I still think theses are good reasons and I'd like to keep the dict that way.
So in my opinion there are only two possibilities left:

  • keep it the way it currently is
  • move the dict to the defaults-vars and let the user overwrite them on playbook-level.

Since the work for no. 2 is already done I'm in favor of this.

@fitz123, @chris-rock what do you think?

PS: os_auth_* is no dict because these variables are used in different templates and contexts.

@conorsch
Copy link
Contributor

+1

Simply moving the nested dict into defaults is a good compromise here. Adding an explanatory comment above the dict in defaults could be helpful, really folks using Ansible should understand how dicts are handled.

Using the new Ansible filter to combine dicts is a great idea, but would force users of the role to start using Ansible 2—and it's a bit early to make it a hard requirement.

@conorsch
Copy link
Contributor

Resolved via #67; OK to close.

rndmh3ro added a commit that referenced this issue Nov 8, 2020
add modes to template and file tasks
divialth pushed a commit to divialth/ansible-collection-hardening that referenced this issue Aug 3, 2022
add modes to template and file tasks
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

No branches or pull requests

3 participants