[
https://issues.apache.org/jira/browse/AIRFLOW-3112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16667278#comment-16667278
]
ASF GitHub Bot commented on AIRFLOW-3112:
-----------------------------------------
Fokko closed pull request #4085: [AIRFLOW-3112] Fix SFTPHook not validating
hosts by default
URL: https://github.com/apache/incubator-airflow/pull/4085
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/airflow/contrib/hooks/sftp_hook.py
b/airflow/contrib/hooks/sftp_hook.py
index 3cb32ab8ba..c23bd3012b 100644
--- a/airflow/contrib/hooks/sftp_hook.py
+++ b/airflow/contrib/hooks/sftp_hook.py
@@ -49,6 +49,9 @@ def __init__(self, ftp_conn_id='sftp_default', *args,
**kwargs):
self.conn = None
self.private_key_pass = None
+ # Fail for unverified hosts, unless this is explicitly allowed
+ self.no_host_key_check = False
+
if self.ssh_conn_id is not None:
conn = self.get_connection(self.ssh_conn_id)
if conn.extra is not None:
@@ -59,9 +62,7 @@ def __init__(self, ftp_conn_id='sftp_default', *args,
**kwargs):
# For backward compatibility
# TODO: remove in Airflow 2.1
import warnings
- if 'ignore_hostkey_verification' in extra_options \
- and str(extra_options["ignore_hostkey_verification"])\
- .lower() == 'false':
+ if 'ignore_hostkey_verification' in extra_options:
warnings.warn(
'Extra option `ignore_hostkey_verification` is
deprecated.'
'Please use `no_host_key_check` instead.'
@@ -69,7 +70,14 @@ def __init__(self, ftp_conn_id='sftp_default', *args,
**kwargs):
DeprecationWarning,
stacklevel=2,
)
- self.no_host_key_check = False
+ self.no_host_key_check = str(
+ extra_options['ignore_hostkey_verification']
+ ).lower() == 'true'
+
+ if 'no_host_key_check' in extra_options:
+ self.no_host_key_check = str(
+ extra_options['no_host_key_check']).lower() == 'true'
+
if 'private_key' in extra_options:
warnings.warn(
'Extra option `private_key` is deprecated.'
diff --git a/airflow/utils/db.py b/airflow/utils/db.py
index 30dd84417a..ffc41115a1 100644
--- a/airflow/utils/db.py
+++ b/airflow/utils/db.py
@@ -183,7 +183,7 @@ def initdb(rbac=False):
conn_id='sftp_default', conn_type='sftp',
host='localhost', port=22, login='airflow',
extra='''
- {"private_key": "~/.ssh/id_rsa",
"ignore_hostkey_verification": true}
+ {"key_file": "~/.ssh/id_rsa", "no_host_key_check": true}
'''))
merge_conn(
models.Connection(
diff --git a/tests/contrib/hooks/test_sftp_hook.py
b/tests/contrib/hooks/test_sftp_hook.py
index 322b5bbbe1..ac4d78e9b1 100644
--- a/tests/contrib/hooks/test_sftp_hook.py
+++ b/tests/contrib/hooks/test_sftp_hook.py
@@ -19,12 +19,13 @@
from __future__ import print_function
+import mock
import unittest
import shutil
import os
import pysftp
-from airflow import configuration
+from airflow import configuration, models
from airflow.contrib.hooks.sftp_hook import SFTPHook
TMP_PATH = '/tmp'
@@ -105,6 +106,63 @@ def test_get_mod_time(self):
TMP_PATH, TMP_DIR_FOR_TESTS, TMP_FILE_FOR_TESTS))
self.assertEqual(len(output), 14)
+ @mock.patch('airflow.contrib.hooks.sftp_hook.SFTPHook.get_connection')
+ def test_no_host_key_check_default(self, get_connection):
+ connection = models.Connection(login='login', host='host')
+ get_connection.return_value = connection
+ hook = SFTPHook()
+ self.assertEqual(hook.no_host_key_check, False)
+
+ @mock.patch('airflow.contrib.hooks.sftp_hook.SFTPHook.get_connection')
+ def test_no_host_key_check_enabled(self, get_connection):
+ connection = models.Connection(
+ login='login', host='host',
+ extra='{"no_host_key_check": true}')
+
+ get_connection.return_value = connection
+ hook = SFTPHook()
+ self.assertEqual(hook.no_host_key_check, True)
+
+ @mock.patch('airflow.contrib.hooks.sftp_hook.SFTPHook.get_connection')
+ def test_no_host_key_check_disabled(self, get_connection):
+ connection = models.Connection(
+ login='login', host='host',
+ extra='{"no_host_key_check": false}')
+
+ get_connection.return_value = connection
+ hook = SFTPHook()
+ self.assertEqual(hook.no_host_key_check, False)
+
+ @mock.patch('airflow.contrib.hooks.sftp_hook.SFTPHook.get_connection')
+ def test_no_host_key_check_disabled_for_all_but_true(self, get_connection):
+ connection = models.Connection(
+ login='login', host='host',
+ extra='{"no_host_key_check": "foo"}')
+
+ get_connection.return_value = connection
+ hook = SFTPHook()
+ self.assertEqual(hook.no_host_key_check, False)
+
+ @mock.patch('airflow.contrib.hooks.sftp_hook.SFTPHook.get_connection')
+ def test_no_host_key_check_ignore(self, get_connection):
+ connection = models.Connection(
+ login='login', host='host',
+ extra='{"ignore_hostkey_verification": true}')
+
+ get_connection.return_value = connection
+ hook = SFTPHook()
+ self.assertEqual(hook.no_host_key_check, True)
+
+ @mock.patch('airflow.contrib.hooks.sftp_hook.SFTPHook.get_connection')
+ def test_no_host_key_check_no_ignore(self, get_connection):
+ connection = models.Connection(
+ login='login', host='host',
+ extra='{"ignore_hostkey_verification": false}')
+
+ get_connection.return_value = connection
+ hook = SFTPHook()
+ self.assertEqual(hook.no_host_key_check, False)
+
def tearDown(self):
shutil.rmtree(os.path.join(TMP_PATH, TMP_DIR_FOR_TESTS))
os.remove(os.path.join(TMP_PATH, TMP_FILE_FOR_TESTS))
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Align SFTP hook with SSH hook
> -----------------------------
>
> Key: AIRFLOW-3112
> URL: https://issues.apache.org/jira/browse/AIRFLOW-3112
> Project: Apache Airflow
> Issue Type: Improvement
> Components: hooks
> Affects Versions: 1.10.0
> Reporter: John Cheng
> Assignee: John Cheng
> Priority: Minor
> Fix For: 2.0.0
>
>
> SFTP hook input arguments should be alined with SSH hook so that it can use
> SSH connections.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)