fortinet-solutions-cse / fortiosapi

Python library aimed to be used by configuration management system using Fortigate/Fortios devices (REST API)
https://fndn.fortinet.com
Apache License 2.0
116 stars 42 forks source link

fortiosapi ssh implementation #76

Open mdraevich opened 3 years ago

mdraevich commented 3 years ago

Good day!

Let's discuss the destiny of the following method: https://github.com/fortinet-solutions-cse/fortiosapi/blob/de593924f2b1018f1bfc85d4487858bdb676ebfc/fortiosapi/fortiosapi.py#L663-L667

Could you clarify me the purpose for Deprecated comment in this method? Does it state the method is no longer supported? The reason for the clarification is the bug in this method, so I really do not know what to do...

thomnico commented 3 years ago

Intent is to remove this call entirely, it remains to not break previous usage. Direct access to ssh using pexect or paramiko should be prefered.

You can see an example of pexpect with fortios in the tests folder of this project.

Is there any usage that absolutely require this call in an api library ??

mdraevich commented 3 years ago

Thank you for reply!

Sounds sadly if the method is no longer maintained. Could you give me a piece of advice on how do you do diagnostics on your FortiGate using automation tools such as Ansible? Do you really use expect for these tasks?

To my mind the tool such as expect is powerful tool but in the very noose way. It's the last tool I would use for automation because it's difficult to say that expect is scalable tool. Just consider the examples below to upload new firmware (using SSH because of old FortiOS version).

I think there could be two ways if automation is really matters for Fortinet devices. Two ways to let your partners do complicated diagnostics in order to obtain a state of the device... The first way -- SSH method should be alive and should be developed and fixed for known bugs. The second way -- new HTTP API calls in order to execute get, diagnose, show, execute (just diagnostics commands such as execute ping, get router info routing-table database) commands in order to understand the state of the device.

Could you say what actually do you think about it?

SSH script

fortiosconfig:
  action: "ssh"
  host:  "{{ host }}"
  username: "{{ username }}"
  password: "{{ password }}"
  commands: |
    config global
    exec restore image tftp {{ images }}/{{ firmware_filename }} {{ tftp_server }}

Expect script

  shell: |
     set timeout 10
     spawn ssh -o StrictHostKeyChecking=no {{ login }}@{{ ip }}
     sleep 2
     expect "password:"
     send "{{ password }}\r"
     expect "#"
     sleep 10
     expect "#"
     send "\r"
     expect "#"
     send "config global\r"
     expect "#"
     send "exec restore image tftp {{ images }}/{{ firmware_filename }} {{ tftp_server }}\r"
     expect "(y/n)" {
        send "y"
     }
     set timeout 200
     expect { 
       "(y/n)" { 
         send "y" 
       }
       "closed." { 
         exit(0) 
       }
     }
     expect "closed." {
       exit(0)
     }
     exit(1)
thomnico commented 3 years ago

The recommended method by far is the API calls only, you should not use cli in a programatic way it is a receipt for troubles when calls fails.

You should reach to your Fortinet representative. If you want to enhance and maintain this code by my guest, I no longer work with fortigate devices

mdraevich commented 3 years ago

I couldn't agree more about the fact the API calls is preferable way to do automation tasks, butt.... I think you understand the reason why engineers try to use SSH for automation -- just because most vendors do not develop API calls so fast and good as they develop Shell management. That's really problem to my mind.

But either way as I see you have no desire for fixing bugs at least in SSH method... Maybe that's the reason you want to remove this method from the project. I as well as my team think the method should exist until Fortinet plucks up the courage and does implement API in better way.

So I have an implementation of SSH method using Fabric pip module. This implementation is easy enough from the code perspective and it do the job well enough: a bit slower than paramiko, but works without bugs I see in paramiko implementation.

I think there is a sense to make a pull request to allow engineers do SSH automation. What do you think?

The implementation looks in the following way:

from fabric import Connection  # the replacement for paramiko
...

    @staticmethod
    def ssh(cmds, host, user, password=None, port=22):
        """
        Send a multi line string via ssh to the fortigate

        :param cmds: multi line string with the Fortigate config cli
        :param host: ip/hostname of the fortigate interface
        :param user/password: fortigate admin user and password
        :param port: port 22 if not set or a port on which fortigate listen for ssh commands.
        :return:
            The output of the console commands and raise exception if failed
        """
        ssh_connection = Connection(host=host, port=port, user=user, 
                                    connect_kwargs={'password': password})
        #
        ssh_result = ssh_connection.run(cmds)
        error_list = [ 
            "Unknown action 0", 
            "Command fail", 
            "Return code 1"
        ]

        search_errors = [ error in ssh_result.stdout for error in error_list ]
        ssh_connection.close()

        # var['meta']['err'] is True if there were errors during execution 
        return (ssh_result.stdout, any(search_errors))