ixs / napalm-procurve

HP ProCurve Driver for NAPALM automation frontend
Apache License 2.0
21 stars 15 forks source link

napalm-prcurve requires manager level permissions #9

Closed 0x5a17ed closed 4 years ago

0x5a17ed commented 5 years ago

The current implementation automatically calls the enable command on the prompt after the connection has been sucessfully established. This requires the given ssh key to have the manager permission level though. I would rather prefer to use the principle of least privileges on the netbox user and have the netbox ssh key only on the operator permission level. At this level the enable command can't be used though.

Is there a specific need for the enable command to be executed at the start of the connection? I would otherwise recommend to either patch it out in a PR or hide it behind a configuration to explicitly require it instead of assuming the enable command to be available.

ixs commented 5 years ago

Hmm. If you only use napalm for monitoring - which napalm-procurve works well for, enable mode is not needed.

If you use napalm for configuration management - which napalm-procurve sucks at due to limitations of the ProcurveOS - enable mode is needed.

As napalm is often used for cfgmgmt, enable mode has been used by default...

But I am sure one could put that behind a flag or so...

0x5a17ed commented 5 years ago

As napalm is often used for cfgmgmt, enable mode has been used by default...

While napalm can also be used for configuring a given switch, as far as I can tell the functionality provided by this library so far is all about retrieving information from supported switches. Not configuring them. So I can't see the point in entering the enable mode by default. The enable mode can always be entered at a later point in time if there is need for it.

ixs commented 4 years ago

https://github.com/ixs/napalm-procurve/commit/a622f90ac6b2dd305e1e4613e4d84ddeef27966a reworks the connection method which means it has gotten much easier to use.

As a side benefit, there's now a new optional_arg called force_no_enable that skips the enable step when set to true. That should take care of your issue I believe.

I'll probably tag a 0.6.1 release in the next days or so that should encompass that change. If you wanna test it out beforehand to see if the above commit works for your use-case, that'd be splendid.

ixs commented 4 years ago

This should be resolved in 0.7.0. Closing the ticket. If you find that the reworked connection handler is posing problems for you, please feel free to reopen the ticket.