F5Networks / f5-ansible

Imperative Ansible modules for F5 BIG-IP products
GNU General Public License v3.0
375 stars 229 forks source link

Add virtual-servers option "per_flow_request_access_policy" in bigip_virtual_server and bigip_device_info #2419

Closed oldmanhere closed 4 days ago

oldmanhere commented 1 month ago

Is your feature request related to a problem? Please describe.

There is no way to set per_flow_request_access_policy in bigip_virtual_server, or i haven't found how.

I do it for now with two tasks, one with bigip_virtual_server and after bigip_command like that : commands: "tmsh modify ltm virtual /{{ partition }}/{{ vs_name }} per-flow-request-access-policy {{ per_flow_request_access_policy }}"

But it's not idempotent. (this feature request could be a bug report with another point of view) Each time i run the playbook, the first task delete the option in the virtual_server with state "changed" and the second re-write the option.

Please, add a way to set directly the per_flow_request_access_policy option with bigip_virtual_server module, and the task will do nothing if it's already all set.

And, please again, add it in bigip_device_info to show all the options for the virtual_server in "virtual-servers" report.

Describe the solution you'd like

Sample:

      f5networks.f5_modules.bigip_virtual_server:
        state: present
        partition: "{{ item.partition }}"
        name: "{{ item.name }}"
        .../...
        irules: "{{ item.irules | default(omit) }}"
        per_flow_request_access_policy: {{ item.per_flow_request_access_policy | default(omit) }}"
        profiles: "{{ item.profiles | default(omit) }}"
        policies: "{{ item.policies | default(omit) }}"
        .../...
        provider: "{{ provider }}"
      loop: "{{ virtual_servers }}"
      delegate_to: localhost
oldmanhere commented 1 month ago

I have done my own patch yesterday and test it with success on virtual servers with and without the option. Idempotent work correctly. I have created the pull request #2420 for the community.

pgouband commented 1 month ago

Hi @oldmanhere,

Before we can work on your pull request, you need to fulfill and signed CLA form: https://clouddocs.f5.com/products/orchestration/ansible/devel/usage/contributor.html and send us back to Ansible_CLA@f5.com and to automation_ecosystem

oldmanhere commented 1 month ago

Hi @pgouband, I'm not sure i want to go that way. I don't need to be a regular contributor. This 7 lines' fix is easy to understand. This give one APM option in imperative module, which was hidden here and only available in the command-line tool. This fix isn't copyrighted, follow the coding style plus choices of original author and follow GPL license from module.

These lines solve my client's trouble faster than if you code it on your own. I just can't wait for your solution. My client need a correct report today for a more complex coding, and we need to solve the "two tasks not idempotents" situation when we use our playbook. AS3 and declarative module are out of our choices.

So, you are free to decline this code. I will wait for your solution to this issue. My client will probably open a case.

oldmanhere commented 1 month ago

Just for your information, the code from #2355 does not exist in my modified version tested. In the current PR #2420, this old patch is present. If i use the current PR, it break idempotency on our virtual servers that have a APM profile ( and a APM policy in the per_flow_request_access_policy option). I suspect that #2355 consider only LTM and ASM profiles.

pgouband commented 2 weeks ago

Hi,

Thanks for reporting. Added to the backlog and internal tracking ID for this request is: INFRAANO-1649.