From 6673ef24c1863fb1af79e5a72dcc0541f23da205 Mon Sep 17 00:00:00 2001 From: Varun Valada Date: Wed, 18 Sep 2024 17:15:22 -0500 Subject: [PATCH 1/9] Add authentication to CLI when submitting jobs with priority --- cli/testflinger_cli/__init__.py | 63 ++++++++++++++++++++++++++++++--- cli/testflinger_cli/client.py | 33 +++++++++++++---- 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/cli/testflinger_cli/__init__.py b/cli/testflinger_cli/__init__.py index c7b9a3d6..4caa7a07 100644 --- a/cli/testflinger_cli/__init__.py +++ b/cli/testflinger_cli/__init__.py @@ -150,6 +150,17 @@ def __init__(self): or os.environ.get("TESTFLINGER_SERVER") or "https://testflinger.canonical.com" ) + self.client_id = ( + self.args.client_id + or self.config.get("client_id") + or os.environ.get("TESTFLINGER_CLIENT_ID") + ) + self.secret_key = ( + self.args.secret_key + or self.config.get("secret_key") + or os.environ.get("TESTFLINGER_SECRET_KEY") + ) + # Allow config subcommand without worrying about server or client if ( hasattr(self.args, "func") @@ -185,6 +196,16 @@ def get_args(self): parser.add_argument( "--server", default=None, help="Testflinger server to use" ) + parser.add_argument( + "--client_id", + default=None, + help="Client ID to authenticate with Testflinger server", + ) + parser.add_argument( + "--secret_key", + default=None, + help="Secret key to be used with client id for authentication", + ) sub = parser.add_subparsers() arg_artifacts = sub.add_parser( "artifacts", @@ -373,11 +394,16 @@ def submit(self): except FileNotFoundError: sys.exit(f"File not found: {self.args.filename}") job_dict = yaml.safe_load(data) + if "job_priority" in job_dict: + jwt = self.authenticate_with_server() + auth_headers = {"Authorization": jwt} + else: + auth_headers = None attachments_data = self.extract_attachment_data(job_dict) if attachments_data is None: # submit job, no attachments - job_id = self.submit_job_data(job_dict) + job_id = self.submit_job_data(job_dict, headers=auth_headers) else: with tempfile.NamedTemporaryFile(suffix="tar.gz") as archive: archive_path = Path(archive.name) @@ -385,7 +411,7 @@ def submit(self): logger.info("Packing attachments into %s", archive_path) self.pack_attachments(archive_path, attachments_data) # submit job, followed by the submission of the archive - job_id = self.submit_job_data(job_dict) + job_id = self.submit_job_data(job_dict, headers=auth_headers) try: logger.info("Submitting attachments for %s", job_id) self.submit_job_attachments(job_id, path=archive_path) @@ -406,10 +432,10 @@ def submit(self): if self.args.poll: self.do_poll(job_id) - def submit_job_data(self, data: dict): + def submit_job_data(self, data: dict, headers: dict = None): """Submit data that was generated or read from a file as a test job""" try: - job_id = self.client.submit_job(data) + job_id = self.client.submit_job(data, headers=headers) except client.HTTPError as exc: if exc.status == 400: sys.exit( @@ -482,6 +508,35 @@ def submit_job_attachments(self, job_id: str, path: Path): f"failed after {tries} tries" ) + def authenticate_with_server(self): + """ + Authenticate client id and secret key with server + and return JWT with permissions + """ + if self.client_id is None or self.secret_key is None: + sys.exit("Must provide client id and secret key for priority jobs") + + try: + jwt = self.client.authenticate(self.client_id, self.secret_key) + except client.HTTPError as exc: + if exc.status == 401: + sys.exit( + "Authentication with Testflinger server failed. " + "Check your client id and secret key" + ) + if exc.status == 404: + sys.exit( + "Received 404 error from server. Are you " + "sure this is a testflinger server?" + ) + # This shouldn't happen, so let's get more information + logger.error( + "Unexpected error status from testflinger server: %s", + exc.status, + ) + sys.exit(1) + return jwt + def show(self): """Show the requested job JSON for a specified JOB_ID""" try: diff --git a/cli/testflinger_cli/client.py b/cli/testflinger_cli/client.py index a5e55426..b0f2a55d 100644 --- a/cli/testflinger_cli/client.py +++ b/cli/testflinger_cli/client.py @@ -23,6 +23,7 @@ from pathlib import Path import sys import urllib.parse +import base64 import requests @@ -45,7 +46,7 @@ class Client: def __init__(self, server): self.server = server - def get(self, uri_frag, timeout=15): + def get(self, uri_frag, timeout=15, headers=None): """Submit a GET request to the server :param uri_frag: endpoint for the GET request @@ -54,7 +55,7 @@ def get(self, uri_frag, timeout=15): """ uri = urllib.parse.urljoin(self.server, uri_frag) try: - req = requests.get(uri, timeout=timeout) + req = requests.get(uri, timeout=timeout, headers=headers) except requests.exceptions.ConnectionError: logger.error("Unable to communicate with specified server.") raise @@ -68,7 +69,7 @@ def get(self, uri_frag, timeout=15): raise HTTPError(req.status_code) return req.text - def put(self, uri_frag, data, timeout=15): + def put(self, uri_frag, data, timeout=15, headers=None): """Submit a POST request to the server :param uri_frag: endpoint for the POST request @@ -77,7 +78,9 @@ def put(self, uri_frag, data, timeout=15): """ uri = urllib.parse.urljoin(self.server, uri_frag) try: - req = requests.post(uri, json=data, timeout=timeout) + req = requests.post( + uri, json=data, timeout=timeout, headers=headers + ) except requests.exceptions.ConnectTimeout: logger.error( "Timeout while trying to communicate with the server." @@ -147,7 +150,7 @@ def post_job_state(self, job_id, state): data = {"job_state": state} self.put(endpoint, data) - def submit_job(self, data: dict) -> str: + def submit_job(self, data: dict, headers: dict = None) -> str: """Submit a test job to the testflinger server :param job_data: @@ -156,9 +159,27 @@ def submit_job(self, data: dict) -> str: ID for the test job """ endpoint = "/v1/job" - response = self.put(endpoint, data) + response = self.put(endpoint, data, headers=headers) return json.loads(response).get("job_id") + def authenticate(self, client_id: str, secret_key: str) -> dict: + """Authenticates client id and secret key with the server + and returns JWT with allowed permissions + + :param job_data: + Dictionary containing data for the job to submit + :return: + ID for the test job + """ + endpoint = "/v1/oauth2/token" + id_key_pair = f"{client_id}:{secret_key}" + encoded_id_key_pair = base64.b64encode( + id_key_pair.encode("utf-8") + ).decode("utf-8") + headers = {"Authorization": f"Basic {encoded_id_key_pair}"} + response = self.put(endpoint, {}, headers=headers) + return response + def post_attachment(self, job_id: str, path: Path, timeout: int): """Send a test job attachment to the testflinger server From ab9682fe2d87dde619216f8416654ba1bb61f511 Mon Sep 17 00:00:00 2001 From: Varun Valada Date: Tue, 8 Oct 2024 16:08:18 -0500 Subject: [PATCH 2/9] Add lint exception for too many methods --- cli/testflinger_cli/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/testflinger_cli/__init__.py b/cli/testflinger_cli/__init__.py index 4caa7a07..02bebd3a 100644 --- a/cli/testflinger_cli/__init__.py +++ b/cli/testflinger_cli/__init__.py @@ -138,6 +138,7 @@ class AttachmentError(Exception): """Exception thrown when attachments fail to be submitted""" +# pylint: disable=R0904 class TestflingerCli: """Class for handling the Testflinger CLI""" From 3df30ea7520c24c41ce803e66ac8d68c74fd3b90 Mon Sep 17 00:00:00 2001 From: Varun Valada Date: Mon, 28 Oct 2024 05:37:35 -0500 Subject: [PATCH 3/9] Added documentation for authentication in the CLI --- cli/README.rst | 5 +++++ docs/how-to/submit-job.rst | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/cli/README.rst b/cli/README.rst index 5f8d0304..0296b84c 100644 --- a/cli/README.rst +++ b/cli/README.rst @@ -48,6 +48,11 @@ You may also set the environment variable 'TESTFLINGER_SERVER' to the URI of your server, and it will prefer that over the default or the string specified by --server. +To specify Testflinger authentication parameters, like client_id +and secret_key, you can use '--client_id' and '--secret_key' respectively. +You can also specify these parameters as environment variables, +'TESTFLINGER_CLIENT_ID' and 'TESTFLINGER_SECRET_KEY'. + To submit a new test job, first create a yaml or json file containing the job definition. Then run: .. code-block:: console diff --git a/docs/how-to/submit-job.rst b/docs/how-to/submit-job.rst index ddb04db1..5633b250 100644 --- a/docs/how-to/submit-job.rst +++ b/docs/how-to/submit-job.rst @@ -19,3 +19,12 @@ If the job is successful submitted, you will see a ``job_id`` returned by Testfl You can use the ``job_id`` to further monitor and manager the submitted job. + +If you specify a job_priority in the YAML file, the CLI will attempt to authenticate with the server first. You can specify authentication parameters using command line options: + +.. code-block:: shell + + $ testflinger-cli submit example-job.yaml --client_id "my_client_id" --secret_key "my_secret_key" + +You can also specify these as environment variables, 'TESTFLINGER_CLIENT_ID' and +'TESTFLINGER_SECRET_KEY'. From 2211f73223ab1e52f263bf76c77933a08a8cf5f8 Mon Sep 17 00:00:00 2001 From: Varun Valada Date: Mon, 28 Oct 2024 11:16:43 -0500 Subject: [PATCH 4/9] Added unit tests for authorization functionality within job submission --- cli/testflinger_cli/tests/test_cli.py | 42 +++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/cli/testflinger_cli/tests/test_cli.py b/cli/testflinger_cli/tests/test_cli.py index 95ed21fc..7d9c6d8c 100644 --- a/cli/testflinger_cli/tests/test_cli.py +++ b/cli/testflinger_cli/tests/test_cli.py @@ -316,6 +316,48 @@ def test_submit_attachments_timeout(tmp_path): assert history[3].path == f"/v1/job/{job_id}/action" +def test_submit_with_priority(tmp_path, requests_mock): + """Tests authorization of jobs submitted with priority""" + job_id = str(uuid.uuid1()) + job_data = { + "queue": "fake", + "job_priority": 100, + } + job_file = tmp_path / "test.json" + job_file.write_text(json.dumps(job_data)) + + sys.argv = ["", "submit", str(job_file)] + tfcli = testflinger_cli.TestflingerCli() + tfcli.client_id = "my_client_id" + tfcli.secret_key = "my_secret_key" + + fake_jwt = "my_jwt" + requests_mock.post(f"{URL}/v1/oauth2/token", text=fake_jwt) + mock_response = {"job_id": job_id} + requests_mock.post(f"{URL}/v1/job", json=mock_response) + tfcli.submit() + assert requests_mock.last_request.headers.get("Authorization") == fake_jwt + + +def test_submit_priority_no_credentials(tmp_path): + """Tests priority jobs rejected with no specified credentials""" + job_data = { + "queue": "fake", + "job_priority": 100, + } + job_file = tmp_path / "test.json" + job_file.write_text(json.dumps(job_data)) + + sys.argv = ["", "submit", str(job_file)] + tfcli = testflinger_cli.TestflingerCli() + with pytest.raises(SystemExit) as exc_info: + tfcli.submit() + assert ( + "Must provide client id and secret key for priority jobs" + in exc_info.value + ) + + def test_show(capsys, requests_mock): """Exercise show command""" jobid = str(uuid.uuid1()) From ae353c27f1846a492af05f5024ce6ee801779a41 Mon Sep 17 00:00:00 2001 From: Varun Valada Date: Wed, 30 Oct 2024 05:48:40 -0500 Subject: [PATCH 5/9] Added more documentation describing job priority and authentication --- docs/explanation/authentication.rst | 9 ++++++++ docs/explanation/index.rst | 2 ++ docs/explanation/job-priority.rst | 9 ++++++++ docs/how-to/authentication.rst | 36 +++++++++++++++++++++++++++++ docs/how-to/index.rst | 2 ++ docs/how-to/job-priority.rst | 15 ++++++++++++ docs/how-to/submit-job.rst | 9 -------- 7 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 docs/explanation/authentication.rst create mode 100644 docs/explanation/job-priority.rst create mode 100644 docs/how-to/authentication.rst create mode 100644 docs/how-to/job-priority.rst diff --git a/docs/explanation/authentication.rst b/docs/explanation/authentication.rst new file mode 100644 index 00000000..90681001 --- /dev/null +++ b/docs/explanation/authentication.rst @@ -0,0 +1,9 @@ +Authentication and Authorization +-------------------------------- + +Authentication requires a client_id and a secret_key. These credentials can be +obtained by contacting the server administrator with the queues you want priority +access for as well as the maximum priority level to set for each queue. The +expecation is that these credentials are shared between users on a team. + +These credentials can be :doc:`set using the Testflinger CLI <../how-to/authentication>`. diff --git a/docs/explanation/index.rst b/docs/explanation/index.rst index 4d4bd890..c4cc421a 100644 --- a/docs/explanation/index.rst +++ b/docs/explanation/index.rst @@ -8,3 +8,5 @@ This section covers conceptual questions about Testflinger. agents queues + job-priority + authentication diff --git a/docs/explanation/job-priority.rst b/docs/explanation/job-priority.rst new file mode 100644 index 00000000..ce2c8ade --- /dev/null +++ b/docs/explanation/job-priority.rst @@ -0,0 +1,9 @@ +Job Priority +============ + +Adding job priority to your jobs gives them the ability to be selected before +other jobs. Job priority can be specified by adding the job_priority field to +your job YAML. This field takes an integer value with a default value of 0. Jobs +with a higher job_priority value will be selected over jobs with lower value. +Using this feature requires :doc:`authenticating <./authentication>` with +Testflinger server. diff --git a/docs/how-to/authentication.rst b/docs/how-to/authentication.rst new file mode 100644 index 00000000..f95aa6d2 --- /dev/null +++ b/docs/how-to/authentication.rst @@ -0,0 +1,36 @@ +Authentication using Testflinger CLI +==================================== + +:doc:`Authentication <../explanation/authentication>` is only required for submitting jobs with priority. + +Authenticating with Testflinger server requires a client id and a secret key. +These credentials can be provided to the CLI using the environment variables +``TESTFLINGER_CLIENT_ID`` and ``TESTFLINGER_SECRET_KEY``. You can put these +variables in a .env file: + +.. code-block:: shell + + TESTFLINGER_CLIENT_ID=my_client_id + TESTFLINGER_SECRET_KEY=my_secret_key + + +You can then export these variables in your shell: + +.. code-block:: shell + + set -a + source .env + set +a + + +With these variables set, you can ``testflinger_cli submit`` your jobs normally, and the authentication will be done by the CLI +automatically. + +Alternatively, you can set the client id and secret key using +command line arguments: + +.. code-block:: shell + + $ testflinger-cli submit example-job.yaml --client_id "my_client_id" --secret_key "my_secret_key" + +However, this is not recommended for security purposes. diff --git a/docs/how-to/index.rst b/docs/how-to/index.rst index 78b2bb80..7c50ad12 100644 --- a/docs/how-to/index.rst +++ b/docs/how-to/index.rst @@ -14,3 +14,5 @@ Work with jobs via Testflinger CLI submit-job cancel-job search-job + job-priority + authentication diff --git a/docs/how-to/job-priority.rst b/docs/how-to/job-priority.rst new file mode 100644 index 00000000..8391a19f --- /dev/null +++ b/docs/how-to/job-priority.rst @@ -0,0 +1,15 @@ +Submit a test job with priority +=============================== + +You can add the :doc:`job_priority <../explanation/job-priority>` field to your +job YAML like this: + +.. code-block:: yaml + + job_priority: 100 + +This field requires an integer value with a default value of 0. The maximum +priority you can set depends on the permissions that you have for the queue +you are submtting to. + +In order to use this field, you need to be :doc:`authenticated <./authentication>` with the server. diff --git a/docs/how-to/submit-job.rst b/docs/how-to/submit-job.rst index 5633b250..ddb04db1 100644 --- a/docs/how-to/submit-job.rst +++ b/docs/how-to/submit-job.rst @@ -19,12 +19,3 @@ If the job is successful submitted, you will see a ``job_id`` returned by Testfl You can use the ``job_id`` to further monitor and manager the submitted job. - -If you specify a job_priority in the YAML file, the CLI will attempt to authenticate with the server first. You can specify authentication parameters using command line options: - -.. code-block:: shell - - $ testflinger-cli submit example-job.yaml --client_id "my_client_id" --secret_key "my_secret_key" - -You can also specify these as environment variables, 'TESTFLINGER_CLIENT_ID' and -'TESTFLINGER_SECRET_KEY'. From 34dab97cbf49731b87ef9d2a642d9bd5174df3fd Mon Sep 17 00:00:00 2001 From: Varun Valada Date: Thu, 31 Oct 2024 05:43:20 -0500 Subject: [PATCH 6/9] fix spelling errors in docs --- docs/explanation/authentication.rst | 4 ++-- docs/how-to/job-priority.rst | 2 +- docs/reference/job-schema.rst | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/explanation/authentication.rst b/docs/explanation/authentication.rst index 90681001..80308384 100644 --- a/docs/explanation/authentication.rst +++ b/docs/explanation/authentication.rst @@ -1,9 +1,9 @@ -Authentication and Authorization +Authentication and Authorisation -------------------------------- Authentication requires a client_id and a secret_key. These credentials can be obtained by contacting the server administrator with the queues you want priority access for as well as the maximum priority level to set for each queue. The -expecation is that these credentials are shared between users on a team. +expectation is that these credentials are shared between users on a team. These credentials can be :doc:`set using the Testflinger CLI <../how-to/authentication>`. diff --git a/docs/how-to/job-priority.rst b/docs/how-to/job-priority.rst index 8391a19f..b3bceb59 100644 --- a/docs/how-to/job-priority.rst +++ b/docs/how-to/job-priority.rst @@ -10,6 +10,6 @@ job YAML like this: This field requires an integer value with a default value of 0. The maximum priority you can set depends on the permissions that you have for the queue -you are submtting to. +you are submitting to. In order to use this field, you need to be :doc:`authenticated <./authentication>` with the server. diff --git a/docs/reference/job-schema.rst b/docs/reference/job-schema.rst index f02a2ddd..d133e52c 100644 --- a/docs/reference/job-schema.rst +++ b/docs/reference/job-schema.rst @@ -54,7 +54,7 @@ The following table lists the key elements that a job definition file should con - | (Optional) URL to send job status updates to. These updates originate from the agent and get posted to the server which then posts the update to the webhook. If no webhook is specified, these updates will not be generated. * - ``job_priority`` - integer - - | (Optional) Integer specifying how much priority this job has. Jobs with higher job_priority will be selected by agents before other jobs. Specifying a job priority requires authorization in the form of a JWT obtained by sending a POST request to /v1/oauth2/token with a client id and client key specified in an Authorization header. + - | (Optional) Integer specifying how much priority this job has. Jobs with higher job_priority will be selected by agents before other jobs. Specifying a job priority requires authorisation in the form of a JWT obtained by sending a POST request to /v1/oauth2/token with a client id and client key specified in an Authorisation header. Example jobs in YAML ---------------------------- From 5449f89ae3966c4ba4a575f611670c4de36e3439 Mon Sep 17 00:00:00 2001 From: Varun Valada Date: Thu, 31 Oct 2024 05:47:13 -0500 Subject: [PATCH 7/9] Add env to spell check wordlist --- docs/.wordlist.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/.wordlist.txt b/docs/.wordlist.txt index 1916326e..61409b52 100644 --- a/docs/.wordlist.txt +++ b/docs/.wordlist.txt @@ -23,6 +23,7 @@ EBS EFI EKS EMMC +env favicon Git GitHub From 057105bab487fa797ece943f1f90a11c493b9860 Mon Sep 17 00:00:00 2001 From: Varun Valada Date: Thu, 31 Oct 2024 10:51:43 -0500 Subject: [PATCH 8/9] More spelling rules and fixes --- docs/.wordlist.txt | 3 +++ docs/reference/job-schema.rst | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/.wordlist.txt b/docs/.wordlist.txt index 61409b52..3385bc3d 100644 --- a/docs/.wordlist.txt +++ b/docs/.wordlist.txt @@ -4,6 +4,7 @@ APIs authorized artifact artifacts +Authorization autoinstall balancer BG @@ -39,6 +40,7 @@ IoT Jira JSON Juju +JWT Kubeflow Kubernetes KVM @@ -64,6 +66,7 @@ netboot NodePort noprovision NVidia +oauth observability OEM oem diff --git a/docs/reference/job-schema.rst b/docs/reference/job-schema.rst index d133e52c..c4488dcc 100644 --- a/docs/reference/job-schema.rst +++ b/docs/reference/job-schema.rst @@ -54,7 +54,8 @@ The following table lists the key elements that a job definition file should con - | (Optional) URL to send job status updates to. These updates originate from the agent and get posted to the server which then posts the update to the webhook. If no webhook is specified, these updates will not be generated. * - ``job_priority`` - integer - - | (Optional) Integer specifying how much priority this job has. Jobs with higher job_priority will be selected by agents before other jobs. Specifying a job priority requires authorisation in the form of a JWT obtained by sending a POST request to /v1/oauth2/token with a client id and client key specified in an Authorisation header. + - 0 + - | (Optional) Integer specifying how much priority this job has. Jobs with higher job_priority will be selected by agents before other jobs. Specifying a job priority requires authorisation in the form of a JWT obtained by sending a POST request to /v1/oauth2/token with a client id and client key specified in an `Authorization` header. Example jobs in YAML ---------------------------- From 39388a244f10a3ba5ee557abee449ccb2702e8cb Mon Sep 17 00:00:00 2001 From: Varun Valada Date: Thu, 31 Oct 2024 10:59:38 -0500 Subject: [PATCH 9/9] Minor documentation changes --- docs/how-to/job-priority.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/how-to/job-priority.rst b/docs/how-to/job-priority.rst index b3bceb59..5107fc6b 100644 --- a/docs/how-to/job-priority.rst +++ b/docs/how-to/job-priority.rst @@ -12,4 +12,4 @@ This field requires an integer value with a default value of 0. The maximum priority you can set depends on the permissions that you have for the queue you are submitting to. -In order to use this field, you need to be :doc:`authenticated <./authentication>` with the server. +In order to use this field, you need an :doc:`authorisation token <./authentication>` from the server.