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

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

Closed
oldmanhere opened this issue Aug 2, 2024 · 5 comments · Fixed by #2425
Labels
enhancement PRs or Issues for basic feature requests for an existing module.

Comments

@oldmanhere
Copy link

oldmanhere commented Aug 2, 2024

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 oldmanhere added enhancement PRs or Issues for basic feature requests for an existing module. untriaged issue that needs an initial response from the developers labels Aug 2, 2024
@oldmanhere oldmanhere changed the title Add virtual-servers option "per-flow-request-access-policy" in bigip_virtual_server and bigip_device_info Add virtual-servers option "per_flow_request_access_policy" in bigip_virtual_server and bigip_device_info Aug 6, 2024
@oldmanhere
Copy link
Author

oldmanhere commented Aug 6, 2024

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
Copy link
Contributor

pgouband commented Aug 6, 2024

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 [email protected] and to automation_ecosystem

@oldmanhere
Copy link
Author

oldmanhere commented Aug 6, 2024

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
Copy link
Author

oldmanhere commented Aug 9, 2024

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 pgouband removed the untriaged issue that needs an initial response from the developers label Aug 23, 2024
@pgouband
Copy link
Contributor

Hi,

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement PRs or Issues for basic feature requests for an existing module.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants