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

New user device module for Fortinet's FortiOS and HTTPAPI plugin #56447

Open

Conversation

Projects
None yet
3 participants
@mamunozgonzalez
Copy link
Contributor

commented May 15, 2019

SUMMARY

Fortinet is adding Ansible support for FortiOS and FortiGate products. This module follows the same structure, guidelines and ideas given in previous approved module for a parallel feature of FortiGate (webfiltering): #37196
In this case we are providing a different functionality: "Fortios User Device".

Please note that this will be part of other modules to come for FortiGate, including different functionalities: system, wireless-controller, firewall, webfilter, ips, web-proxy, wanopt, application, dlp spamfilter, log, vpn, certificate, user, dnsfilter, antivirus, report, waf, authentication, switch controller, endpoint-control and router. We plan to follow the same style, structure and usage as in the previous module in order to make it easier to comply with Ansible guidelines.

Also, this PR includes a new HTTPAPI plugin for FortiOS, as suggested by Ansible guidelines.

This PR has been updated according to suggestions and code reviews given in previous submissions of Fortinet modules.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

fortios_user_device

ANSIBLE VERSION
ansible 2.8.0.dev0 (new_module ddbbe5dfa5) last updated 2018/09/24 14:54:57 (GMT +200)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/magonzalez/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/magonzalez/ansible/lib/ansible
  executable location = /home/magonzalez/ansible/bin/ansible
  python version = 2.7.15rc1 (default, Apr 15 2018, 21:51:34) [GCC 7.3.0]
@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 15, 2019

@mamunozgonzalez, just so you are aware we have a dedicated Working Group for network.
You can find other people interested in this in #ansible-network on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

mamunozgonzalez added some commits May 15, 2019

@ansibot ansibot added core_review and removed needs_revision labels May 15, 2019

@trishnaguha

This comment has been minimized.

Copy link
Member

commented May 15, 2019

@mamunozgonzalez Will you split the module and httpapi plugin in two different PRs?

@trishnaguha trishnaguha requested a review from NilashishC May 15, 2019

@ansibot ansibot added needs_revision and removed core_review labels May 15, 2019

@mamunozgonzalez

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

@mamunozgonzalez Will you split the module and httpapi plugin in two different PRs?

Sure, I can split into two PRs. However I had the impression from the previous long discussions we had and due to the amount of PRs already submitted that you wanted to minimize the number of PRs for Fortinet. That's the main reason why I put this two together. Are you sure you want to have this separated? Can you confirm with @gundalow ?

@mamunozgonzalez

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

Btw, it seems that the last commit made Shippable to fail, but apparently I can't see a logical reason that ties the changes with the failures. Could it be due to a temporary fail in Shippable? (I have seen similar weird and random errors other times).
If so could you trigger again the Shippable checks?
Thanks!

@trishnaguha

This comment has been minimized.

Copy link
Member

commented May 16, 2019

@mamunozgonzalez If PRs are related, then you can have as one PR.
Since this PR has one new module and http plugin, we would prefer to have those as two separate PRs so it would be easy to review.
Thanks.

@trishnaguha

This comment has been minimized.

Copy link
Member

commented May 16, 2019

Retriggered Shippable.

@mamunozgonzalez

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

Shippable failed again, due to strange errors.
One of the errors is saying:
17:55 ERROR: lib/ansible/modules/cloud/vmware/vsphere_copy.py:0:0: E309 version_added for new option (host) should be '2.9'. Currently None (0%)

However this does not have anything to do with the changes in this PR. 'vsphere_copy.py' has not been modified at all.
Actually the failures that appear now didn't appear before the last commit (and the last commit only does 'imports urllib' and calls 'urllib.parse.quote' method).
Apparently it seems to be caused by something external to this PR.
@trishnaguha , @gundalow , Can you help me investigate this?
Thanks!

@trishnaguha

This comment has been minimized.

Copy link
Member

commented May 17, 2019

@mamunozgonzalez you can ignore this CI failure. This won't be a blocker for merging this PR.
Please keep your module commit in this PR and open a separate PR for httpapi plugin so that the PRs can be reviewed.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.