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

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

Open
Markus- opened this issue Aug 25, 2023 · 10 comments
Open

Comments

@Markus-
Copy link

Markus- commented Aug 25, 2023

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

@richm
Copy link
Contributor

richm commented Aug 27, 2023

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

@Markus-
Copy link
Author

Markus- commented Aug 28, 2023

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?

@richm
Copy link
Contributor

richm commented Aug 28, 2023

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: ....

?

@Markus-
Copy link
Author

Markus- commented Aug 28, 2023

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

@richm
Copy link
Contributor

richm commented Aug 28, 2023

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

@Markus-
Copy link
Author

Markus- commented Aug 28, 2023

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

@richm
Copy link
Contributor

richm commented Aug 28, 2023

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).

@Markus-
Copy link
Author

Markus- commented Aug 28, 2023

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?

@richm
Copy link
Contributor

richm commented Aug 28, 2023

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).

@Markus-
Copy link
Author

Markus- commented Aug 28, 2023 via email

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

2 participants