[GitHub] [airflow] NBardelot commented on pull request #12467: Added support for DSS, ECDSA, and Ed25519 private keys in SSHHook

[GitBox <gi...@apache.org>]

NBardelot commented on pull request #12467:
URL: https://github.com/apache/airflow/pull/12467#issuecomment-730255164


   @brighton1101 this looks good. One suggestion though: the list of key types `allowed_pkey_types` might be better configured than hardcoded.
   
   The issue is that Paramiko uses a bad design pattern, since each subclass of PKey implements the `from_private_key` but there is no utility function or collection that provides the list of algorithms... So you'll have to hardcode the list anyway in Airflow (and also hardcode a transcoding name -> Paramiko's PKey subclass). 
   
   Proposition: 
   
     * a section `[ssh]` in Airflow configuration
     * with an option `private_keys_algorithm_support`
     * which contains a list of coma separated algorithm names (I'd choose the same names as `ssh-keygen` uses for its option `-t`)
     * and in the `hooks/ssh.py` Airflow code a map that transcodes from those name to the corresponding Paramiko class, and on which you can iterate
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org