firewall icon indicating copy to clipboard operation
firewall copied to clipboard

An easy way to explicit deactivate previous: replaced for easier operation

Open Markus- opened this issue 2 years ago • 10 comments

What would you like to be added: I could'nt come up with an easy solution for dynamically overwrite or not overwrite the whole firewall config depending on a boolean.

Why is the variable "previous: replaced" and not "overwrite_config: true|false" (or something like this). Why is this needed: I need an easy way to trigger the overwriting of the configuration exernally, without have to manipulate the firewall-list-of-dict.

If I understood the code, at the moment there is just filtered for "replaced" and I could not find the reason for this.

Regards Markus

Markus- avatar Aug 25 '23 11:08 Markus-

What would you like to be added: I could'nt come up with an easy solution for dynamically overwrite or not overwrite the whole firewall config depending on a boolean.

tl;dr in Ansible, the semantic is to specify the desired state of the system, not to list a bunch of actions to take - the implementation is supposed to take care of the "how" - the user is supposed to state the "what".

https://linux-system-roles.github.io/documentation/incremental_settings.html

Why is the variable "previous: replaced" and not "overwrite_config: true|false" (or something like this). Why is this needed: I need an easy way to trigger the overwriting of the configuration exernally, without have to manipulate the firewall-list-of-dict.

I don't understand the issue - why is manipulating a list of dict harder than specifying a boolean value? Can you give me an example of the problem you have, and how using a boolean would solve it?

If I understood the code, at the moment there is just filtered for "replaced" and I could not find the reason for this.

Regards Markus

richm avatar Aug 27 '23 16:08 richm

I have a multistage environment, where I can overwrite the firewalll in staging or test environments without any problems. So here in my inventory I want to set the boolean to True, while in a production environment I would need to take additional actions before overwriting all firewall-rules.

I tried to dynamically change the dict-value based on a boolean (by using default, omit ...), but there was always some strange behaviour based on the templating engine of ansible and the fact that in this case most of the time a string used.

So right now, I have a separate task which removes previous: replaced when a boolean is not set. I thought it would be easier to just reference an external variable.

The action overwriting the firewall or not is more a boolean operation than any other. Or are there any additional use cases the previous-option is used in the future?

Markus- avatar Aug 28 '23 05:08 Markus-

Ok - I think you want something like this?

Inventory:

firewall:
  - overwrite_config: "{{ false if is_production_environment else true }}"
  - ports: .....

does this work?

firewall:
  - previous: "{{ 'kept' if is_production_environment else 'replaced' }}"
  - ports: ....

?

richm avatar Aug 28 '23 17:08 richm

If 'kept' does not result in an error?

I scanned through the code quickly and thought everything beside 'replaced' results in an error.

But both ideas are good, because this way you could decide to overwrite the firewall or not without the need to manipulate the dict.

Regards Markus

Markus- avatar Aug 28 '23 17:08 Markus-

If 'kept' does not result in an error?

Well, if it works, then it does not result in an error.

I scanned through the code quickly and thought everything beside 'replaced' results in an error.

Have you tried it?

But both ideas are good, because this way you could decide to overwrite the firewall or not without the need to manipulate the dict.

Regards Markus

richm avatar Aug 28 '23 17:08 richm

Anything beside "previous: replaced" results in this error

failed: [oa89696pbmx001] (item={'previous': 'kept'}) => {"ansible_loop_var": "item", "changed": false, "item": {"previous": "kept"}, "msg": "One of service, port, source_port, forward_port, masquerade, rich_rule, source, interface, icmp_block, icmp_block_inversion, target, zone, set_default_zone, ipset or firewalld_conf needs to be set"}

Regards Markus

Markus- avatar Aug 28 '23 17:08 Markus-

ok - then how about this?

firewall_previous: "{{ [] if is_production_environment else [{'previous': 'replaced'}] }}"
firewall_settings:
  - ports: ....
firewall: "{{ firewall_previous + firewall_settings }}"

that should at least eliminate the need for an additional task (with the cost of adding two variables).

richm avatar Aug 28 '23 17:08 richm

This should work. Not as easy as a separate variable, but good enough.

Should I close this issue, as the "problem" is solveable without additional code or is this a "feature" which could be implemented in the future?

Markus- avatar Aug 28 '23 18:08 Markus-

This should work. Not as easy as a separate variable, but good enough.

Should I close this issue, as the "problem" is solveable without additional code or is this a "feature" which could be implemented in the future?

I think you have pointed out a good use case. I would prefer to support an additional value for previous: - previous: kept (or something like that - not enthusiastic about the word "kept" - hopefully we can come up with something better).

richm avatar Aug 28 '23 18:08 richm

What about something like append (like in the network role)

  • route_append_only

The route_append_only option allows only to add new routes to the existing routes on the system.

If the route_append_only boolean option is set to true, the specified routes are appended to the existing routes. If route_append_only is set to false (default), the current routes are replaced. Note that setting route_append_only to truewithout setting route has the effect of preserving the current static routes.

I think you have pointed out a good use case. I would prefer to support an additional value for previous: - previous: kept (or something like that - not enthusiastic about the word "kept" - hopefully we can come up with something better).

Markus- avatar Aug 28 '23 19:08 Markus-