From 21069c358e8a4064c8b3fcf6d63009b710defc28 Mon Sep 17 00:00:00 2001 From: Charles Cowart <42684307+charles-cowart@users.noreply.github.com> Date: Tue, 23 Jul 2024 10:18:08 -0700 Subject: [PATCH] update_job_status() now has option to ignore runtime errors (#51) * update_job_status() now has option to ignore runtime errors * Updates based on CI failures * Updated CI config * Migrating CI off Python 2.x Migrating CI off Python 2.x to support latest change. Since latest change is straightforward, current operation using Python 2.7 should not be disrupted. * Updated CI configuration based on qp-klp plugin config. * bugfix * Recently renamed certs * Remove 3.11 from test matrix * Added script to add qiita root-ca to environment. Added script to add the root-ca used to sign Qiita's server.crt to qiita_client's environment. The functionality in this small script may be more useful if it becomes part of qiita_client itself. * Update PluginTestCase base class Update PluginTestCase base class to rely on standard environment to verify server credentials. Rely on new script to update standard environment. * Updated CI config to avoid installing extra plugins * Updates from testing * Add testing for 3.11 * Removed matrix testing for python 3.11 * test removal of pip updates * Test removal of some qiita installation commands * Updated based on discussions * test change * Removed unused config setting * Updates based on feedback --- .github/workflows/qiita-ci.yml | 12 +++---- README.md | 4 +-- qiita_client/plugin.py | 8 +++-- qiita_client/qiita_client.py | 31 ++++++++++------ qiita_client/testing.py | 9 ++--- qiita_client/tests/test_plugin.py | 9 ++--- qiita_client/tests/test_qiita_client.py | 47 ++++++++++++++++++++----- 7 files changed, 80 insertions(+), 40 deletions(-) diff --git a/.github/workflows/qiita-ci.yml b/.github/workflows/qiita-ci.yml index 50cda0d..ebfd92c 100644 --- a/.github/workflows/qiita-ci.yml +++ b/.github/workflows/qiita-ci.yml @@ -11,7 +11,7 @@ jobs: strategy: matrix: - python-version: ["2.7", "3.9"] + python-version: ["3.9"] services: postgres: @@ -79,14 +79,13 @@ jobs: run: | conda create --yes -n qiita_client python=${{ matrix.python-version }} pip nose flake8 coverage conda activate qiita_client - pip --quiet install . - name: Starting Main Services shell: bash -l {0} run: | conda activate qiita - export QIITA_SERVER_CERT=`pwd`/qiita-dev/qiita_core/support_files/server.crt + export QIITA_ROOTCA_CERT=`pwd`/qiita-dev/qiita_core/support_files/ci_rootca.crt export QIITA_CONFIG_FP=`pwd`/qiita-dev/qiita_core/support_files/config_test_local.cfg sed "s#/home/runner/work/qiita/qiita#${PWD}/qiita-dev/#g" `pwd`/qiita-dev/qiita_core/support_files/config_test.cfg > ${QIITA_CONFIG_FP} @@ -118,12 +117,9 @@ jobs: COVER_PACKAGE: ${{ matrix.cover_package }} run: | conda activate qiita_client - export QIITA_SERVER_CERT=`pwd`/qiita-dev/qiita_core/support_files/server.crt + export QIITA_ROOTCA_CERT=`pwd`/qiita-dev/qiita_core/support_files/ci_rootca.crt export QIITA_CONFIG_FP=`pwd`/qiita-dev/qiita_core/support_files/config_test_local.cfg - - nosetests --with-doctest --with-coverage --cover-package=qiita_client - - - uses: codecov/codecov-action@v1 + - uses: codecov/codecov-action@v3 with: token: ${{ secrets.CODECOV_TOKEN }} file: codecov.yml diff --git a/README.md b/README.md index c4677ba..7dfeedf 100644 --- a/README.md +++ b/README.md @@ -10,9 +10,9 @@ This package includes the Qiita Client utility library, a library to simplify th How to test this package? ------------------------- In order to test the Qiita Client package, a local installation of Qiita should be running in test mode on the address `https://localhost:21174`, with the default test database created in Qiita's test suite. -Also, if Qiita is running with the default server SSL certificate, you need to export the variable `QIITA_SERVER_CERT` in your environment, so the Qiita Client can perform secure connections against the Qiita server: +Also, if Qiita is running with the default server SSL certificate, you need to export the variable `QIITA_ROOTCA_CERT` in your environment, so the Qiita Client can perform secure connections against the Qiita server: ```bash -export QIITA_SERVER_CERT=/qiita_core/support_files/server.crt +export QIITA_ROOT_CA=/qiita_core/support_files/ci_rootca.crt ``` diff --git a/qiita_client/plugin.py b/qiita_client/plugin.py index a674eb8..a0fdc53 100644 --- a/qiita_client/plugin.py +++ b/qiita_client/plugin.py @@ -245,7 +245,12 @@ def __call__(self, server_url, job_id, output_dir): qclient = QiitaClient(server_url, config.get('oauth2', 'CLIENT_ID'), config.get('oauth2', 'CLIENT_SECRET'), - server_cert=config.get('oauth2', 'SERVER_CERT')) + # for this group of tests, confirm optional + # ca_cert parameter works as intended. Setting + # this value will prevent underlying libraries + # from validating the server's cert using + # certifi's pem cache. + ca_cert=environ['QIITA_ROOTCA_CERT']) if job_id == 'register': self._register(qclient) @@ -374,6 +379,5 @@ def register_command(self, command): PUBLICATIONS = %s [oauth2] -SERVER_CERT = %s CLIENT_ID = %s CLIENT_SECRET = %s""" diff --git a/qiita_client/qiita_client.py b/qiita_client/qiita_client.py index 881cf5a..610097f 100644 --- a/qiita_client/qiita_client.py +++ b/qiita_client/qiita_client.py @@ -57,7 +57,7 @@ def __init__(self, output_name, artifact_type, files, archive=None): def __eq__(self, other): logger.debug('Entered ArtifactInfo.__eq__()') - if type(self) != type(other): + if type(self) is not type(other): return False if self.output_name != other.output_name or \ self.artifact_type != other.artifact_type or \ @@ -166,8 +166,8 @@ class QiitaClient(object): The client id to connect to the Qiita server client_secret : str The client secret id to connect to the Qiita server - server_cert : str, optional - The server certificate, in case that it is not verified + ca_cert : str, optional + CA cert used to sign and verify cert@server_url Methods @@ -175,7 +175,7 @@ class QiitaClient(object): get post """ - def __init__(self, server_url, client_id, client_secret, server_cert=None): + def __init__(self, server_url, client_id, client_secret, ca_cert=None): self._server_url = server_url self._session = requests.Session() @@ -186,16 +186,21 @@ def __init__(self, server_url, client_id, client_secret, server_cert=None): # if certificate verification should be performed or not, or a # string with the path to the certificate file that needs to be used # to verify the identity of the server. - # We are setting this attribute at __init__ time so we can avoid - # executing this if statement for each request issued. - if not server_cert: + # We are setting this attribute at __init__ time to avoid executing + # this if statement for each request issued. + + # As self-signed server certs are no longer allowed in one or more of + # our dependencies, ca_cert (if provided) must now reference a file + # that can be used to verify the certificate used by the server + # referenced by server_url, rather than the server's own certificate. + if not ca_cert: # The server certificate is not provided, use standard certificate # verification methods self._verify = True else: # The server certificate is provided, use it to verify the identity # of the server - self._verify = server_cert + self._verify = ca_cert # Set up oauth2 self._client_id = client_id @@ -527,7 +532,7 @@ def get_job_info(self, job_id): logger.debug('Entered QiitaClient.get_job_info()') return self.get("/qiita_db/jobs/%s" % job_id) - def update_job_step(self, job_id, new_step): + def update_job_step(self, job_id, new_step, ignore_error=True): """Updates the current step of the job in the server Parameters @@ -536,10 +541,16 @@ def update_job_step(self, job_id, new_step): The job id new_step : str The new step + ignore_error : bool + Problems communicating w/Qiita will not raise an Error. """ logger.debug('Entered QiitaClient.update_job_step()') json_payload = dumps({'step': new_step}) - self.post("/qiita_db/jobs/%s/step/" % job_id, data=json_payload) + try: + self.post("/qiita_db/jobs/%s/step/" % job_id, data=json_payload) + except BaseException as e: + if ignore_error is False: + raise e def complete_job(self, job_id, success, error_msg=None, artifacts_info=None): diff --git a/qiita_client/testing.py b/qiita_client/testing.py index ef94989..89e7a25 100644 --- a/qiita_client/testing.py +++ b/qiita_client/testing.py @@ -24,11 +24,12 @@ def setUpClass(cls): cls.client_id = '19ndkO3oMKsoChjVVWluF7QkxHRfYhTKSFbAVt8IhK7gZgDaO4' cls.client_secret = ('J7FfQ7CQdOxuKhQAf1eoGgBAE81Ns8Gu3EKaWFm3IO2JKh' 'AmmCWZuabe0O5Mp28s1') - cls.server_cert = environ.get('QIITA_SERVER_CERT', None) qiita_port = int(environ.get('QIITA_PORT', 21174)) - cls.qclient = QiitaClient( - "https://localhost:%d" % qiita_port, cls.client_id, - cls.client_secret, server_cert=cls.server_cert) + + # do not rely on defining ca_cert for these tests. Instead append + # the appropriate CA cert to certifi's pem file. + cls.qclient = QiitaClient("https://localhost:%d" % qiita_port, + cls.client_id, cls.client_secret) logger.debug( 'PluginTestCase.setUpClass() token %s' % cls.qclient._token) cls.qclient.post('/apitest/reload_plugins/') diff --git a/qiita_client/tests/test_plugin.py b/qiita_client/tests/test_plugin.py index 05cba7f..4981cfa 100644 --- a/qiita_client/tests/test_plugin.py +++ b/qiita_client/tests/test_plugin.py @@ -146,8 +146,7 @@ def html_generator_func(a, b, c, d): 'PLUGIN_TYPE = artifact definition\n', 'PUBLICATIONS = \n', '\n', - '[oauth2]\n', - 'SERVER_CERT = \n'] + '[oauth2]\n'] # We will test the last 2 lines independently since they're variable # in each test run self.assertEqual(conf[:-2], exp_lines) @@ -168,8 +167,7 @@ def html_generator_func(a, b, c, d): validate_func, html_generator_func, atypes) # Generate the config file for the new plugin - tester.generate_config('ls', 'echo', - server_cert=self.server_cert) + tester.generate_config('ls', 'echo') # Ask Qiita to reload the plugins self.qclient.post('/apitest/reload_plugins/') @@ -213,8 +211,7 @@ def func(qclient, job_id, job_params, working_dir): {'out1': 'Demultiplexed'}) tester.register_command(a_cmd) - tester.generate_config('ls', 'echo', - server_cert=self.server_cert) + tester.generate_config('ls', 'echo') self.qclient.post('/apitest/reload_plugins/') tester("https://localhost:21174", 'register', 'ignored') diff --git a/qiita_client/tests/test_qiita_client.py b/qiita_client/tests/test_qiita_client.py index cfc6ec2..17184e1 100644 --- a/qiita_client/tests/test_qiita_client.py +++ b/qiita_client/tests/test_qiita_client.py @@ -7,7 +7,7 @@ # ----------------------------------------------------------------------------- from unittest import TestCase, main -from os import environ, remove, close +from os import remove, close, environ from os.path import basename, exists from tempfile import mkstemp from json import dumps @@ -19,6 +19,7 @@ from qiita_client.exceptions import BadRequestError CLIENT_ID = '19ndkO3oMKsoChjVVWluF7QkxHRfYhTKSFbAVt8IhK7gZgDaO4' +BAD_CLIENT_ID = 'NOT_A_CLIENT_ID' CLIENT_SECRET = ('J7FfQ7CQdOxuKhQAf1eoGgBAE81Ns8Gu3EKaWFm3IO2JKh' 'AmmCWZuabe0O5Mp28s1') @@ -96,9 +97,12 @@ def test_format_payload_error(self): class QiitaClientTests(PluginTestCase): def setUp(self): - self.server_cert = environ.get('QIITA_SERVER_CERT', None) - self.tester = QiitaClient("https://localhost:21174", CLIENT_ID, - CLIENT_SECRET, server_cert=self.server_cert) + self.tester = QiitaClient("https://localhost:21174", + CLIENT_ID, + CLIENT_SECRET) + self.bad_tester = QiitaClient("https://localhost:21174", + BAD_CLIENT_ID, + CLIENT_SECRET) self.clean_up_files = [] # making assertRaisesRegex compatible with Python 2.7 and 3.9 @@ -111,12 +115,14 @@ def tearDown(self): remove(fp) def test_init(self): - obs = QiitaClient("https://localhost:21174", CLIENT_ID, - CLIENT_SECRET, server_cert=self.server_cert) + obs = QiitaClient("https://localhost:21174", + CLIENT_ID, + CLIENT_SECRET, + ca_cert=environ['QIITA_ROOT_CA']) self.assertEqual(obs._server_url, "https://localhost:21174") self.assertEqual(obs._client_id, CLIENT_ID) self.assertEqual(obs._client_secret, CLIENT_SECRET) - self.assertEqual(obs._verify, self.server_cert) + self.assertEqual(obs._verify, environ['QIITA_ROOT_CA']) def test_get(self): obs = self.tester.get("/qiita_db/artifacts/1/") @@ -232,11 +238,36 @@ def test_update_job_step(self): obs = self.tester.update_job_step(job_id, new_step) self.assertIsNone(obs) + def test_update_job_step_ignore_failure(self): + job_id = "bcc7ebcd-39c1-43e4-af2d-822e3589f14d" + new_step = "some new step" + + # confirm that update_job_step behaves as before when ignore_error + # parameter absent or set to False. + + with self.assertRaises(BaseException): + self.bad_tester.update_job_step(job_id, new_step) + + with self.assertRaises(BaseException): + self.bad_tester.update_job_step(job_id, + new_step, + ignore_error=False) + + # confirm that when ignore_error is set to True, an Error is NOT + # raised. + try: + self.bad_tester.update_job_step(job_id, + new_step, + ignore_error=True) + except BaseException as e: + self.fail("update_job_step() raised an error: %s" % str(e)) + def test_complete_job(self): # Create a new job data = { 'user': 'demo@microbio.me', - 'command': dumps(['QIIME', '1.9.1', 'Pick closed-reference OTUs']), + 'command': dumps(['QIIMEq2', '1.9.1', + 'Pick closed-reference OTUs']), 'status': 'running', 'parameters': dumps({"reference": 1, "sortmerna_e_value": 1,