Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Disable legacy upload #4160

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

chore: Disable legacy upload #4160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion insights/client/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ def _core_collect_default():
'legacy_upload': {
# True: upload to insights classic API
# False: upload to insights platform API
'default': True
'default': False
},
'payload': {
'default': None,
Expand Down Expand Up @@ -663,6 +663,11 @@ def _load_config_file(self, fname=None):
'ERROR: {0}.\nCould not read configuration file, '
'using defaults\n'.format(e))
return

if d.get("legacy_upload", False):
logger.debug("'legacy_upload' is set in the configuration file, but is no longer supported. Ignoring.")
del d["legacy_upload"]

self._update_dict(d)

def load_all(self):
Expand Down Expand Up @@ -763,6 +768,10 @@ def _validate_options(self):
raise ValueError("Unable to find app: %s\nList of available apps: %s"
% (self.app, ', '.join(sorted(manifests.keys()))))

if self.legacy_upload:
logger.debug("Legacy upload is no longer supported. Using current APIs.")
self.legacy_upload = False

def _imply_options(self):
'''
Some options enable others automatically
Expand Down
14 changes: 0 additions & 14 deletions insights/tests/client/test_ansiblehost.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,3 @@ def test_ansible_host_no_reg_forces_legacy_false():
conf._cli_opts = ["ansible_host"]
conf._imply_options()
assert not conf.legacy_upload


def test_ansible_host_reg_legacy_no_change():
'''
When specifying --register, using --ansible-host on the CLI does not affect legacy_upload
'''
conf = InsightsConfig(register=True, ansible_host="test", legacy_upload=True)
conf._cli_opts = ["ansible_host"]
conf._imply_options()
assert conf.legacy_upload
conf = InsightsConfig(register=True, ansible_host="test", legacy_upload=False)
conf._cli_opts = ["ansible_host"]
conf._imply_options()
assert not conf.legacy_upload
196 changes: 4 additions & 192 deletions insights/tests/client/test_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,33 +146,6 @@ def test_get_log_handler_by_client_version(get_version_info, mock_path_dirname):
pass


@patch('insights.client.client.generate_machine_id')
@patch('insights.client.utilities.delete_unregistered_file')
@patch('insights.client.utilities.write_to_disk')
@patch('insights.client.utilities.constants.registered_files',
[TEMP_TEST_REG_DIR + '/.registered',
TEMP_TEST_REG_DIR2 + '/.registered'])
@patch('insights.client.utilities.constants.unregistered_files',
[TEMP_TEST_REG_DIR + '/.unregistered',
TEMP_TEST_REG_DIR2 + '/.unregistered'])
@patch('insights.client.utilities.constants.machine_id_file',
TEMP_TEST_REG_DIR + '/machine-id')
def test_register_legacy(utilities_write, delete_unregistered_file, generate_machine_id):
config = InsightsConfig(register=True, legacy_upload=True)
client = InsightsClient(config)
client.connection = _mock_InsightsConnection(registered=None)
client.connection.config = config
client.session = True
with _mock_no_register_files():
client.register() is True
delete_unregistered_file.assert_called_once()
generate_machine_id.assert_called_once_with()
utilities_write.assert_has_calls((
call(constants.registered_files[0]),
call(constants.registered_files[1])
))


@patch('insights.client.client.generate_machine_id')
@patch('insights.client.utilities.delete_unregistered_file')
@patch('insights.client.utilities.write_to_disk')
Expand Down Expand Up @@ -304,32 +277,8 @@ def test_reg_check_unregistered():

# test function and integration in .register()
with _mock_no_register_files():
assert client.get_registration_status()['status'] is False
for r in constants.registered_files:
assert os.path.isfile(r) is False
for u in constants.unregistered_files:
assert os.path.isfile(u) is True


@patch('insights.client.utilities.constants.registered_files',
[TEMP_TEST_REG_DIR + '/.registered',
TEMP_TEST_REG_DIR2 + '/.registered'])
@patch('insights.client.utilities.constants.unregistered_files',
[TEMP_TEST_REG_DIR + '/.unregistered',
TEMP_TEST_REG_DIR2 + '/.unregistered'])
@patch('insights.client.utilities.constants.machine_id_file',
TEMP_TEST_REG_DIR + '/machine-id')
def test_reg_check_registered_res_unreg_legacy():
# system is registered but receives the unregistration status
config = InsightsConfig(legacy_upload=True)
client = InsightsClient(config)
client.connection = _mock_InsightsConnection(registered=None)
client.connection.config = config
client.session = True

# test function and integration in .register()
with _mock_registered_files():
assert client.get_registration_status()['status'] is False
status = client.get_registration_status()
assert status is False
for r in constants.registered_files:
assert os.path.isfile(r) is False
for u in constants.unregistered_files:
Expand Down Expand Up @@ -361,29 +310,6 @@ def test_reg_check_registered_res_unreg():
assert os.path.isfile(u) is True


@patch('insights.client.utilities.constants.registered_files',
[TEMP_TEST_REG_DIR + '/.registered',
TEMP_TEST_REG_DIR2 + '/.registered'])
@patch('insights.client.utilities.constants.unregistered_files',
[TEMP_TEST_REG_DIR + '/.unregistered',
TEMP_TEST_REG_DIR2 + '/.unregistered'])
@patch('insights.client.utilities.constants.machine_id_file',
TEMP_TEST_REG_DIR + '/machine-id')
def test_reg_check_registered_unreachable_legacy():
config = InsightsConfig(legacy_upload=True)
client = InsightsClient(config)
client.connection = _mock_InsightsConnection(registered=False)
client.connection.config = config
client.session = True

with _mock_registered_files():
assert client.get_registration_status()['unreachable'] is True
for r in constants.registered_files:
assert os.path.isfile(r) is True
for u in constants.unregistered_files:
assert os.path.isfile(u) is False


@patch('insights.client.utilities.constants.registered_files',
[TEMP_TEST_REG_DIR + '/.registered',
TEMP_TEST_REG_DIR2 + '/.registered'])
Expand Down Expand Up @@ -431,29 +357,6 @@ def test_reg_check_unregistered_unreachable():
assert os.path.isfile(u) is True


@patch('insights.client.utilities.constants.registered_files',
[TEMP_TEST_REG_DIR + '/.registered',
TEMP_TEST_REG_DIR2 + '/.registered'])
@patch('insights.client.utilities.constants.unregistered_files',
[TEMP_TEST_REG_DIR + '/.unregistered',
TEMP_TEST_REG_DIR2 + '/.unregistered'])
@patch('insights.client.utilities.constants.machine_id_file',
TEMP_TEST_REG_DIR + '/machine-id')
def test_reg_check_unregistered_unreachable_legacy():
config = InsightsConfig(legacy_upload=True)
client = InsightsClient(config)
client.connection = _mock_InsightsConnection(registered=False)
client.connection.config = config
client.session = True

with _mock_no_register_files():
assert client.get_registration_status()['unreachable'] is True
for r in constants.registered_files:
assert os.path.isfile(r) is False
for u in constants.unregistered_files:
assert os.path.isfile(u) is True


@patch('insights.client.client.constants.sleep_time', 0)
@patch('insights.client.client.InsightsConnection._init_session', return_value=None)
@patch('insights.client.client.InsightsConnection.upload_archive', return_value=Mock(status_code=500))
Expand Down Expand Up @@ -486,7 +389,7 @@ def test_upload_500_retry(logger, _, upload_archive, __):
@patch('insights.client.client.InsightsConnection.upload_archive')
@patch("insights.client.client.logger")
def test_upload_exception_retry(logger, upload_archive, _):
from requests.exceptions import ConnectionError, ProxyError, Timeout, HTTPError, SSLError
from requests.exceptions import ConnectionError, ProxyError, Timeout
upload_archive.side_effect = [ConnectionError("Connection Error"),
ProxyError("Proxy Error"),
Timeout("Timeout Error")]
Expand All @@ -504,23 +407,6 @@ def test_upload_exception_retry(logger, upload_archive, _):
logger.error.assert_any_call("Upload attempt %d of %d failed! Reason: %s", 3, config.retries, "Timeout Error")
logger.error.assert_called_with("All attempts to upload have failed!")

# Test legacy uploads
logger.reset_mock()
upload_archive.reset_mock()
upload_archive.side_effect = [HTTPError("HTTP Error"),
SSLError("SSL Error")]
retries = 2
config = InsightsConfig(legacy_upload=True, logging_file='/tmp/insights.log', retries=retries)
client = InsightsClient(config)
with patch('insights.client.os.path.exists', return_value=True):
with pytest.raises(RuntimeError):
client.upload('/tmp/insights.tar.gz')
assert upload_archive.call_count == retries
logger.debug.assert_any_call("Legacy upload attempt %d of %d ...", 1, config.retries)
logger.error.assert_any_call("Upload attempt %d of %d failed! Reason: %s", 1, config.retries, "HTTP Error")
logger.error.assert_any_call("Upload attempt %d of %d failed! Reason: %s", 2, config.retries, "SSL Error")
logger.error.assert_called_with("All attempts to upload have failed!")


@patch('insights.client.client.InsightsConnection._init_session', return_value=None)
@patch('insights.client.client.InsightsConnection.handle_fail_rcs')
Expand All @@ -533,7 +419,7 @@ def test_upload_412_no_retry(_, upload_archive, handle_fail_rcs, __):
sys.argv = []

try:
config = InsightsConfig(logging_file='/tmp/insights.log', retries=3)
config = InsightsConfig(logging_file='/tmp/insights.log')
client = InsightsClient(config)
with pytest.raises(RuntimeError):
client.upload('/tmp/insights.tar.gz')
Expand All @@ -543,30 +429,6 @@ def test_upload_412_no_retry(_, upload_archive, handle_fail_rcs, __):
sys.argv = tmp


@patch('insights.client.client.InsightsConnection._init_session', return_value=None)
@patch('insights.client.connection.write_unregistered_file')
@patch('insights.client.client.InsightsConnection.upload_archive',
return_value=Mock(**{"status_code": 412,
"json.return_value": {"unregistered_at": "now", "message": "msg"}}))
@patch('insights.client.os.path.exists', return_value=True)
def test_upload_412_write_unregistered_file(_, upload_archive, write_unregistered_file, __):

# Hack to prevent client from parsing args to py.test
tmp = sys.argv
sys.argv = []

try:
config = InsightsConfig(logging_file='/tmp/insights.log', retries=3)
client = InsightsClient(config)
with pytest.raises(RuntimeError):
client.upload('/tmp/insights.tar.gz')

unregistered_at = upload_archive.return_value.json()["unregistered_at"]
write_unregistered_file.assert_called_once_with(unregistered_at)
finally:
sys.argv = tmp


@patch('insights.client.archive.InsightsArchive.storing_archive')
@patch('insights.client.archive.tempfile.mkdtemp', Mock())
@patch('insights.client.archive.os.makedirs', Mock())
Expand Down Expand Up @@ -607,28 +469,6 @@ def test_cleanup_tmp_obfuscation(storing_archive):
storing_archive.assert_called_once()


@patch('insights.client.client._legacy_handle_registration')
def test_legacy_register(_legacy_handle_registration):
'''
_legacy_handle_registration called when legacy upload
'''
config = InsightsConfig(legacy_upload=True)
client = InsightsClient(config)
client.register()
_legacy_handle_registration.assert_called_once()


@patch('insights.client.client._legacy_handle_unregistration')
def test_legacy_unregister(_legacy_handle_unregistration):
'''
_legacy_handle_unregistration called when legacy upload
'''
config = InsightsConfig(legacy_upload=True)
client = InsightsClient(config)
client.unregister()
_legacy_handle_unregistration.assert_called_once()


@patch('insights.client.client.handle_registration')
def test_register_upload(handle_registration):
'''
Expand All @@ -651,20 +491,6 @@ def test_unregister_upload(handle_unregistration):
handle_unregistration.assert_called_once()


@patch('insights.client.os.path.exists', return_value=True)
@patch('insights.client.connection.InsightsConnection.upload_archive', Mock(return_value=Mock(status_code=200)))
@patch('insights.client.client._legacy_upload')
@patch('insights.client.client.write_to_disk', Mock())
def test_legacy_upload(_legacy_upload, path_exists):
'''
_legacy_upload called when legacy upload
'''
config = InsightsConfig(legacy_upload=True)
client = InsightsClient(config)
client.upload('test.gar.gz', 'test.content.type')
_legacy_upload.assert_called_once()


@patch('insights.client.os.path.exists', return_value=True)
@patch('insights.client.connection.InsightsConnection.upload_archive', Mock(return_value=Mock(status_code=200)))
@patch('insights.client.client._legacy_upload')
Expand All @@ -679,20 +505,6 @@ def test_platform_upload(_legacy_upload, path_exists):
_legacy_upload.assert_not_called()


@patch('insights.client.os.path.exists', return_value=True)
@patch('insights.client.connection.InsightsConnection.upload_archive', return_value=Mock(status_code=200))
@patch('insights.client.client._legacy_upload')
def test_platform_upload_with_no_log_path(_legacy_upload, _, path_exists):
'''
testing logger when no path is given
'''
config = InsightsConfig(legacy_upload=True, logging_file='tmp.log')
client = InsightsClient(config)
response = client.upload('test.gar.gz', 'test.content.type')
_legacy_upload.assert_called_once()
assert response is not None


@patch('insights.client.shutil')
def test_copy_to_output_dir(shutil_):
'''
Expand Down
35 changes: 0 additions & 35 deletions insights/tests/client/test_platform.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,6 @@ def test_upload_urls():
'''
Ensure upload urls are defined correctly
'''
# legacy default
conf = InsightsConfig()
c = InsightsConnection(conf)
assert c.upload_url == c.base_url + '/uploads'

# plaform implied
conf = InsightsConfig(legacy_upload=False)
c = InsightsConnection(conf)
Expand Down Expand Up @@ -108,33 +103,3 @@ def test_payload_upload(op, post, c, _legacy_upload_archive, rm_conf):
})},
headers={})
_legacy_upload_archive.assert_not_called()


@patch('insights.contrib.magic.open', MockMagic)
@patch('insights.client.connection.generate_machine_id', mock_machine_id)
@patch("insights.client.connection.get_canonical_facts", return_value={'test': 'facts'})
@patch('insights.client.connection.InsightsConnection.post')
@patch("insights.client.connection.open", new_callable=mock_open)
def test_legacy_upload(op, post, c):
'''
Ensure an Insights collected tar upload to legacy occurs with the right URL and params
'''
conf = InsightsConfig()
c = InsightsConnection(conf)
c.upload_archive('testp', 'testct', None)
post.assert_called_with(
c.base_url + '/uploads/XXXXXXXX',
files={
'file': ('testp', ANY, 'application/gzip')}, # ANY = return call from mocked open(), acts as filepointer here
headers={'x-rh-collection-time': 'None'})


@patch("insights.client.connection.InsightsConnection._legacy_upload_archive")
def test_legacy_upload_func(_legacy_upload_archive):
'''
Ensure the _legacy_upload_archive path is used
'''
conf = InsightsConfig()
c = InsightsConnection(conf)
c.upload_archive('testp', 'testct', None)
_legacy_upload_archive.assert_called_with('testp', None)
10 changes: 0 additions & 10 deletions insights/tests/client/test_upload_archive_cfacts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,6 @@ def mock_init_session(obj):
return MockSession()


@patch("insights.client.connection.InsightsConnection._legacy_upload_archive", return_value=None)
@patch("insights.client.connection.logger")
def test_cfacts_no_cleaning_1(logger, legacy_upload):
# 1. No need to cleaning as legacy_upload=True
config = InsightsConfig(legacy_upload=True, obfuscate=True, obfuscate_hostname=True)
conn = InsightsConnection(config)
conn.upload_archive('data_collected', 'content_type')
logger.debug.assert_not_called()


@patch('insights.client.connection.InsightsConnection._init_session', mock_init_session)
@patch('insights.client.connection.InsightsConnection.post', return_value=MockSession())
@patch(builtin_open, new_callable=mock_open, read_data='')
Expand Down
Loading