From f5f5214bf3f586f0bececa9d8600f1404b944b90 Mon Sep 17 00:00:00 2001 From: AdamSharon Date: Tue, 16 Mar 2021 16:35:53 +0200 Subject: [PATCH 1/4] working - no tests. --- .gitignore | 3 + drivers/ansible_shell/drivermetadata.xml | 2 +- drivers/ansible_shell/requirements.txt | 2 +- drivers/version.txt | 2 +- .../cloudshell/cm/ansible/ansible_shell.py | 5 +- .../ansible/domain/ansible_configuration.py | 2 + .../cm/ansible/domain/http_request_service.py | 5 +- .../cm/ansible/domain/playbook_downloader.py | 58 +++++++++++++++++-- package/version.txt | 2 +- version.txt | 2 +- 10 files changed, 71 insertions(+), 12 deletions(-) diff --git a/.gitignore b/.gitignore index 377b022..d65ed87 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,6 @@ cloudshell_cm_ansible.egg-info/ package/cloudshell_cm_ansible.egg-info/ drivers/ansible_shell.zip *.zip +.vscode/ +venv +dist/ \ No newline at end of file diff --git a/drivers/ansible_shell/drivermetadata.xml b/drivers/ansible_shell/drivermetadata.xml index 548c696..246bd31 100644 --- a/drivers/ansible_shell/drivermetadata.xml +++ b/drivers/ansible_shell/drivermetadata.xml @@ -1,4 +1,4 @@ - + diff --git a/drivers/ansible_shell/requirements.txt b/drivers/ansible_shell/requirements.txt index 7eab21e..b885e3b 100644 --- a/drivers/ansible_shell/requirements.txt +++ b/drivers/ansible_shell/requirements.txt @@ -1,2 +1,2 @@ cloudshell-shell-core>=3.1.0,<3.2.0 -cloudshell-cm-ansible>=1.5.1,<1.6.0 \ No newline at end of file +cloudshell-cm-ansible>=1.6.1,<1.7.0 \ No newline at end of file diff --git a/drivers/version.txt b/drivers/version.txt index 3e1ad72..ce6a70b 100644 --- a/drivers/version.txt +++ b/drivers/version.txt @@ -1 +1 @@ -1.5.0 \ No newline at end of file +1.6.0 \ No newline at end of file diff --git a/package/cloudshell/cm/ansible/ansible_shell.py b/package/cloudshell/cm/ansible/ansible_shell.py index 6d7e5ab..12fd40a 100644 --- a/package/cloudshell/cm/ansible/ansible_shell.py +++ b/package/cloudshell/cm/ansible/ansible_shell.py @@ -118,8 +118,11 @@ def _download_playbook(self, ansi_conf, cancellation_sampler, logger): :rtype str """ repo = ansi_conf.playbook_repo - auth = HttpAuth(repo.username, repo.password) if repo.username else None + auth = None + if ansi_conf.playbook_repo.username: + auth = HttpAuth(repo.username, repo.password, repo.token) playbook_name = self.downloader.get(ansi_conf.playbook_repo.url, auth, logger, cancellation_sampler) + logger.info('download playbook file' + playbook_name) return playbook_name def _run_playbook(self, ansi_conf, playbook_name, output_writer, cancellation_sampler, logger): diff --git a/package/cloudshell/cm/ansible/domain/ansible_configuration.py b/package/cloudshell/cm/ansible/domain/ansible_configuration.py index a23dd71..35842a3 100644 --- a/package/cloudshell/cm/ansible/domain/ansible_configuration.py +++ b/package/cloudshell/cm/ansible/domain/ansible_configuration.py @@ -22,6 +22,7 @@ def __init__(self): self.url = None self.username = None self.password = None + self.token = None class HostConfiguration(object): @@ -61,6 +62,7 @@ def json_to_object(self, json_str): ansi_conf.playbook_repo.url = json_obj['repositoryDetails'].get('url') ansi_conf.playbook_repo.username = json_obj['repositoryDetails'].get('username') ansi_conf.playbook_repo.password = json_obj['repositoryDetails'].get('password') + ansi_conf.playbook_repo.token = json_obj['repositoryDetails'].get('token') for json_host in json_obj.get('hostsDetails',[]): host_conf = HostConfiguration() diff --git a/package/cloudshell/cm/ansible/domain/http_request_service.py b/package/cloudshell/cm/ansible/domain/http_request_service.py index fb22061..d4a3ffd 100644 --- a/package/cloudshell/cm/ansible/domain/http_request_service.py +++ b/package/cloudshell/cm/ansible/domain/http_request_service.py @@ -3,4 +3,7 @@ class HttpRequestService(object): def get_response(self, url, auth): - return requests.get(url, auth=(auth.username, auth.password) if auth else None, stream=True) \ No newline at end of file + return requests.get(url, auth=(auth.username, auth.password) if auth else None, stream=True) + + def get_response_with_headers(self, url, headers): + return requests.get(url, headers=headers, stream=True) diff --git a/package/cloudshell/cm/ansible/domain/playbook_downloader.py b/package/cloudshell/cm/ansible/domain/playbook_downloader.py index 84af1da..e1661fb 100644 --- a/package/cloudshell/cm/ansible/domain/playbook_downloader.py +++ b/package/cloudshell/cm/ansible/domain/playbook_downloader.py @@ -1,15 +1,15 @@ import os from cloudshell.cm.ansible.domain.cancellation_sampler import CancellationSampler -from file_system_service import FileSystemService +from cloudshell.cm.ansible.domain.file_system_service import FileSystemService from logging import Logger class HttpAuth(object): - def __init__(self, username, password): + def __init__(self, username, password, token): self.username = username self.password = password - + self.token = token class PlaybookDownloader(object): CHUNK_SIZE = 1024 * 1024 @@ -50,9 +50,40 @@ def _download(self, url, auth, logger, cancel_sampler): :rtype [str,int] :return The downloaded file name """ - logger.info('Downloading file from \'%s\' ...'%url) + + response_valid = False + + # assume repo is public, try to download without credentials + logger.info('Starting download script as public... from \'%s\' ...'%url) response = self.http_request_service.get_response(url, auth) - file_name = self.filename_extractor.get_filename(response) + response_valid = self._is_response_valid(logger ,response, "public") + + if response_valid: + file_name = self.filename_extractor.get_filename(response) + + # repo is private and token provided + if not response_valid and auth.token is not None: + logger.info("Token provided. Starting download script with Token...") + headers = {"Authorization": "Bearer %s" % auth.token } + response = self.http_request_service.get_response_with_headers(url, headers) + + response_valid = self._is_response_valid(logger, response, "Token") + + if response_valid: + file_name = self.filename_extractor.get_filename(response) + + # repo is private and credentials provided, and Token did not provided or did not work. this will NOT work for github. github require Token + if not response_valid and (auth.username is not None and auth.password is not None): + logger.info("username\password provided, Starting download script with username\password...") + response = self.http_request_service.get_response(url, auth) + + response_valid = self._is_response_valid(logger, response, "username\password") + + if response_valid: + file_name = self.filename_extractor.get_filename(response) + + if not response_valid: + raise Exception('Failed to download script file. please check the logs for more details.') with self.file_system.create_file(file_name) as file: for chunk in response.iter_content(PlaybookDownloader.CHUNK_SIZE): @@ -87,3 +118,20 @@ def _unzip(self, file_name, logger): logger.info('Found playbook: \'%s\' in zip file' % (playbook_name)) return playbook_name + + def _is_response_valid(self, logger, response, request_method): + try: + self._validate_response(response) + response_valid = True + except Exception as ex: + failure_message = "failed to Authorize repository with %s" % request_method + logger.error(failure_message + " :" + str(ex)) + response_valid = False + + return response_valid + + + def _validate_response(self, response): + if response.status_code < 200 or response.status_code > 300: + raise Exception('Failed to download script file: '+str(response.status_code)+' '+response.reason+ + '. Please make sure the URL is valid, and the credentials are correct and necessary.') diff --git a/package/version.txt b/package/version.txt index 3e1ad72..ce6a70b 100644 --- a/package/version.txt +++ b/package/version.txt @@ -1 +1 @@ -1.5.0 \ No newline at end of file +1.6.0 \ No newline at end of file diff --git a/version.txt b/version.txt index 8e03717..ce6a70b 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -1.5.1 \ No newline at end of file +1.6.0 \ No newline at end of file From 9117af9e20719aa74e1b3972dbefe966e6c0c0fc Mon Sep 17 00:00:00 2001 From: AdamSharon Date: Tue, 16 Mar 2021 18:23:28 +0200 Subject: [PATCH 2/4] small fix and refactor\add tests --- .../cloudshell/cm/ansible/ansible_shell.py | 2 +- package/tests/test_playbook_downloader.py | 62 +++++++++++++++++-- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/package/cloudshell/cm/ansible/ansible_shell.py b/package/cloudshell/cm/ansible/ansible_shell.py index 12fd40a..22fbf9d 100644 --- a/package/cloudshell/cm/ansible/ansible_shell.py +++ b/package/cloudshell/cm/ansible/ansible_shell.py @@ -122,7 +122,7 @@ def _download_playbook(self, ansi_conf, cancellation_sampler, logger): if ansi_conf.playbook_repo.username: auth = HttpAuth(repo.username, repo.password, repo.token) playbook_name = self.downloader.get(ansi_conf.playbook_repo.url, auth, logger, cancellation_sampler) - logger.info('download playbook file' + playbook_name) + logger.info('download playbook file' + str(playbook_name)) return playbook_name def _run_playbook(self, ansi_conf, playbook_name, output_writer, cancellation_sampler, logger): diff --git a/package/tests/test_playbook_downloader.py b/package/tests/test_playbook_downloader.py index e165830..682e4a8 100644 --- a/package/tests/test_playbook_downloader.py +++ b/package/tests/test_playbook_downloader.py @@ -26,59 +26,111 @@ def _set_extract_all_zip(self, files_to_create): def test_playbook_downloader_zip_file_one_yaml(self): self.zip_service.extract_all = lambda zip_file_name: self._set_extract_all_zip(["lie.yaml"]) - auth = HttpAuth("user", "pass") + auth = HttpAuth("user", "pass", "token") self.reqeust.url = "blabla/lie.zip" dic = dict([('content-disposition', 'lie.zip')]) self.reqeust.headers = dic self.reqeust.iter_content.return_value = '' self.http_request_serivce.get_response=Mock(return_value=self.reqeust) + self.http_request_serivce.get_response_with_headers = Mock(return_value=self.reqeust) + self.playbook_downloader._is_response_valid = Mock(return_value=True) + file_name = self.playbook_downloader.get("", auth, self.logger, Mock()) self.assertEquals(file_name, "lie.yaml") def test_playbook_downloader_zip_file_two_yaml_correct(self): self.zip_service.extract_all = lambda zip_file_name: self._set_extract_all_zip(["lie.yaml", "site.yaml"]) - auth = HttpAuth("user", "pass") + auth = HttpAuth("user", "pass", "token") self.reqeust.url = "blabla/lie.zip" dic = dict([('content-disposition', 'lie.zip')]) self.reqeust.headers = dic self.reqeust.iter_content.return_value = '' self.http_request_serivce.get_response = Mock(return_value=self.reqeust) + self.http_request_serivce.get_response_with_headers = Mock(return_value=self.reqeust) + self.playbook_downloader._is_response_valid = Mock(return_value=True) + file_name = self.playbook_downloader.get("", auth, self.logger, Mock()) + self.assertEquals(file_name, "site.yaml") def test_playbook_downloader_zip_file_two_yaml_incorrect(self): self.zip_service.extract_all = lambda zip_file_name: self._set_extract_all_zip(["lie.yaml", "lie2.yaml"]) - auth = HttpAuth("user", "pass") + auth = HttpAuth("user", "pass", "token") self.reqeust.url = "blabla/lie.zip" dic = dict([('content-disposition', 'lie.zip')]) self.reqeust.headers = dic self.reqeust.iter_content.return_value = '' self.http_request_serivce.get_response = Mock(return_value=self.reqeust) + self.http_request_serivce.get_response_with_headers = Mock(return_value=self.reqeust) + self.playbook_downloader._is_response_valid = Mock(return_value=True) with self.assertRaises(Exception) as e: self.playbook_downloader.get("", auth, self.logger, Mock()) self.assertEqual(e.exception.message,"Playbook file name was not found in zip file") def test_playbook_downloader_with_one_yaml(self): - auth = HttpAuth("user", "pass") + auth = HttpAuth("user", "pass", "token") self.reqeust.url = "blabla/lie.yaml" dic = dict([('content-disposition', 'lie.yaml')]) self.reqeust.headers = dic self.reqeust.iter_content.return_value = 'hello' self.http_request_serivce.get_response = Mock(return_value=self.reqeust) + self.http_request_serivce.get_response_with_headers = Mock(return_value=self.reqeust) + self.playbook_downloader._is_response_valid = Mock(return_value=True) file_name = self.playbook_downloader.get("", auth, self.logger, Mock()) self.assertEquals(file_name, "lie.yaml") def test_playbook_downloader_no_parsing_from_rfc(self): - auth = HttpAuth("user", "pass") + auth = HttpAuth("user", "pass", "token") self.reqeust.url = "blabla/lie.yaml" dic = dict([('content-disposition', 'lie.yaml')]) self.reqeust.headers = dic self.reqeust.iter_content.return_value = '' self.http_request_serivce.get_response = Mock(return_value=self.reqeust) + self.http_request_serivce.get_response_with_headers = Mock(return_value=self.reqeust) + self.playbook_downloader._is_response_valid = Mock(return_value=True) + + file_name = self.playbook_downloader.get("", auth, self.logger, Mock()) + + self.assertEquals(file_name, "lie.yaml") + + def test_playbook_downloader_with_one_yaml_only_credentials(self): + auth = HttpAuth("user", "pass", None) + self.reqeust.url = "blabla/lie.yaml" + dic = dict([('content-disposition', 'lie.yaml')]) + self.reqeust.headers = dic + self.reqeust.iter_content.return_value = 'hello' + self.http_request_serivce.get_response = Mock(return_value=self.reqeust) + self.http_request_serivce.get_response_with_headers = Mock(return_value=self.reqeust) + self.playbook_downloader._is_response_valid = Mock(side_effect=self.mock_response_valid_for_credentials) file_name = self.playbook_downloader.get("", auth, self.logger, Mock()) self.assertEquals(file_name, "lie.yaml") + + def test_playbook_downloader_with_one_yaml_only_token(self): + auth = HttpAuth(None, None, "Token") + self.reqeust.url = "blabla/lie.yaml" + dic = dict([('content-disposition', 'lie.yaml')]) + self.reqeust.headers = dic + self.reqeust.iter_content.return_value = 'hello' + self.http_request_serivce.get_response = Mock(return_value=self.reqeust) + self.http_request_serivce.get_response_with_headers = Mock(return_value=self.reqeust) + self.playbook_downloader._is_response_valid = Mock(side_effect=self.mock_response_valid_for_not_public) + + file_name = self.playbook_downloader.get("", auth, self.logger, Mock()) + + self.assertEquals(file_name, "lie.yaml") + + + + + # helpers method to mock the request according the request in order to test the right flow for Token\Cred + def mock_response_valid_for_not_public(self, logger, response, request_method): + return request_method != "Public" + + + def mock_response_valid_for_credentials(self, logger, response, request_method): + return request_method == "username\password" \ No newline at end of file From 4467aad120f3352acb5a6dd04c897c6c46e8e1c0 Mon Sep 17 00:00:00 2001 From: AdamSharon Date: Tue, 16 Mar 2021 18:26:25 +0200 Subject: [PATCH 3/4] fix requirements for driver --- drivers/ansible_shell/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ansible_shell/requirements.txt b/drivers/ansible_shell/requirements.txt index b885e3b..43c6cf4 100644 --- a/drivers/ansible_shell/requirements.txt +++ b/drivers/ansible_shell/requirements.txt @@ -1,2 +1,2 @@ cloudshell-shell-core>=3.1.0,<3.2.0 -cloudshell-cm-ansible>=1.6.1,<1.7.0 \ No newline at end of file +cloudshell-cm-ansible>=1.6.0,<1.7.0 \ No newline at end of file From 055d66827f3e13e7c070d418fee1ce5bc75b9a00 Mon Sep 17 00:00:00 2001 From: AdamSharon Date: Wed, 17 Mar 2021 09:34:51 +0200 Subject: [PATCH 4/4] tiny change to imporve test --- package/tests/test_playbook_downloader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/tests/test_playbook_downloader.py b/package/tests/test_playbook_downloader.py index 682e4a8..7b475fc 100644 --- a/package/tests/test_playbook_downloader.py +++ b/package/tests/test_playbook_downloader.py @@ -129,7 +129,7 @@ def test_playbook_downloader_with_one_yaml_only_token(self): # helpers method to mock the request according the request in order to test the right flow for Token\Cred def mock_response_valid_for_not_public(self, logger, response, request_method): - return request_method != "Public" + return request_method != "public" def mock_response_valid_for_credentials(self, logger, response, request_method):