potiuk commented on a change in pull request #19981:
URL: https://github.com/apache/airflow/pull/19981#discussion_r762444304
##########
File path: airflow/providers/ssh/hooks/ssh.py
##########
@@ -257,58 +259,74 @@ def get_conn(self) -> paramiko.SSHClient:
:rtype: paramiko.client.SSHClient
"""
self.log.debug('Creating SSH client for conn_id: %s', self.ssh_conn_id)
- client = paramiko.SSHClient()
- if not self.allow_host_key_change:
- self.log.warning(
- "Remote Identification Change is not verified. "
- "This won't protect against Man-In-The-Middle attacks"
- )
- client.load_system_host_keys()
+ max_time_to_wait = 10
+ for time_to_wait in self._exponential_sleep_generator(initial=1,
maximum=max_time_to_wait):
+ try:
+ client = paramiko.SSHClient()
- if self.no_host_key_check:
- self.log.warning("No Host Key Verification. This won't protect
against Man-In-The-Middle attacks")
- # Default is RejectPolicy
- client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
- else:
- if self.host_key is not None:
- client_host_keys = client.get_host_keys()
- if self.port == SSH_PORT:
- client_host_keys.add(self.remote_host,
self.host_key.get_name(), self.host_key)
- else:
- client_host_keys.add(
- f"[{self.remote_host}]:{self.port}",
self.host_key.get_name(), self.host_key
+ if not self.allow_host_key_change:
Review comment:
I think we shoudl move it up to before the loop. Otherwise it will keep
on repeating in the logs when ssh fails.
##########
File path: airflow/providers/ssh/hooks/ssh.py
##########
@@ -257,58 +259,74 @@ def get_conn(self) -> paramiko.SSHClient:
:rtype: paramiko.client.SSHClient
"""
self.log.debug('Creating SSH client for conn_id: %s', self.ssh_conn_id)
- client = paramiko.SSHClient()
- if not self.allow_host_key_change:
- self.log.warning(
- "Remote Identification Change is not verified. "
- "This won't protect against Man-In-The-Middle attacks"
- )
- client.load_system_host_keys()
+ max_time_to_wait = 10
+ for time_to_wait in self._exponential_sleep_generator(initial=1,
maximum=max_time_to_wait):
+ try:
+ client = paramiko.SSHClient()
- if self.no_host_key_check:
- self.log.warning("No Host Key Verification. This won't protect
against Man-In-The-Middle attacks")
- # Default is RejectPolicy
- client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
- else:
- if self.host_key is not None:
- client_host_keys = client.get_host_keys()
- if self.port == SSH_PORT:
- client_host_keys.add(self.remote_host,
self.host_key.get_name(), self.host_key)
- else:
- client_host_keys.add(
- f"[{self.remote_host}]:{self.port}",
self.host_key.get_name(), self.host_key
+ if not self.allow_host_key_change:
+ self.log.warning(
+ "Remote Identification Change is not verified. "
+ "This won't protect against Man-In-The-Middle attacks"
)
- else:
- pass # will fallback to system host keys if none explicitly
specified in conn extra
-
- connect_kwargs = dict(
- hostname=self.remote_host,
- username=self.username,
- timeout=self.conn_timeout,
- compress=self.compress,
- port=self.port,
- sock=self.host_proxy,
- look_for_keys=self.look_for_keys,
- )
-
- if self.password:
- password = self.password.strip()
- connect_kwargs.update(password=password)
-
- if self.pkey:
- connect_kwargs.update(pkey=self.pkey)
-
- if self.key_file:
- connect_kwargs.update(key_filename=self.key_file)
-
- client.connect(**connect_kwargs)
+ client.load_system_host_keys()
- if self.keepalive_interval:
- client.get_transport().set_keepalive(self.keepalive_interval)
-
- self.client = client
- return client
+ if self.no_host_key_check:
Review comment:
Same here.
##########
File path: airflow/providers/ssh/hooks/ssh.py
##########
@@ -257,58 +259,74 @@ def get_conn(self) -> paramiko.SSHClient:
:rtype: paramiko.client.SSHClient
"""
self.log.debug('Creating SSH client for conn_id: %s', self.ssh_conn_id)
- client = paramiko.SSHClient()
- if not self.allow_host_key_change:
- self.log.warning(
- "Remote Identification Change is not verified. "
- "This won't protect against Man-In-The-Middle attacks"
- )
- client.load_system_host_keys()
+ max_time_to_wait = 10
+ for time_to_wait in self._exponential_sleep_generator(initial=1,
maximum=max_time_to_wait):
Review comment:
We should use tenacity here (we already use it elsewhere).
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
