Support multiple API versions #889

ansasaki · 2024-12-18T19:52:45Z

This introduces a new configuration option api_versions which allows the user to select a subset of the supported API versions to enable.

The agent creates the endpoints under each enable API version, allowing it to receive requests from older versions of the other components (verifier and tenant).

During registration, the agent will try to query the version of the API supported by the registrar making a GET request to the /version endpoint. If the registrar supports it and replies with a version, the agent will check if the version is enabled and will use it for the following requests.

In case the registrar does not support the /version endpoint, then the agent will try all the enabled versions. If the registration is successful, the agent will keep the successful API version to use in the following requests.

This is part of the implementation for the enhancement proposal 114: keylime/enhancements#115

codecov · 2024-12-18T21:22:00Z

Codecov Report

Attention: Patch coverage is 74.82877% with 147 lines in your changes missing coverage. Please review.

Project coverage is 62.18%. Comparing base (2f7b3ad) to head (949af61).
Report is 88 commits behind head on master.

Files with missing lines	Patch %	Lines
keylime-agent/src/main.rs	7.84%	47 Missing ⚠️
keylime/src/version.rs	38.96%	47 Missing ⚠️
keylime/src/registrar_client.rs	92.73%	22 Missing ⚠️
keylime-agent/src/config.rs	79.62%	11 Missing ⚠️
keylime-agent/src/errors_handler.rs	26.66%	11 Missing ⚠️
keylime-agent/src/error.rs	0.00%	6 Missing ⚠️
keylime/src/serialization.rs	94.82%	3 Missing ⚠️

Additional details and impacted files

Flag	Coverage Δ
e2e-testsuite	`62.18% <74.82%> (+4.59%)`	⬆️
upstream-unit-tests	`62.18% <74.82%> (+11.17%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
keylime-agent/src/agent_handler.rs	`58.69% <ø> (ø)`
keylime-agent/src/api.rs	`77.38% <100.00%> (ø)`
keylime-agent/src/common.rs	`69.33% <ø> (+1.38%)`	⬆️
keylime-agent/src/keys_handler.rs	`65.71% <ø> (+0.27%)`	⬆️
keylime-agent/src/notifications_handler.rs	`64.70% <ø> (-1.97%)`	⬇️
keylime-agent/src/payloads.rs	`83.90% <ø> (+1.05%)`	⬆️
keylime-agent/src/quotes_handler.rs	`52.12% <ø> (+1.26%)`	⬆️
keylime/src/serialization.rs	`94.82% <94.82%> (ø)`
keylime-agent/src/error.rs	`9.90% <0.00%> (-5.35%)`	⬇️
keylime-agent/src/config.rs	`77.96% <79.62%> (-9.54%)`	⬇️
... and 4 more

... and 5 files with indirect coverage changes

ansasaki · 2024-12-19T17:12:53Z

The failing test needs an update proposed in: RedHat-SP-Security/keylime-tests#688

ansasaki · 2024-12-20T09:59:34Z

I just realized the registrar already supports the /version API in an undocumented API change.
We need to be more diligent updating the documentation. This is also a reminder for myself.

EDIT: Actually, with the refactoring to use a web framework for the registrar, the /version endpoint is no longer exposed. I'll create a PR adding it

This is to account for the addition of the options: - idevid_password - idevid_handle - iak_password - iak_handle Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>

In many places the dependency are unnecessary and used only for testing. Replace the usage of common::API_VERSION with a static string. Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>

Move the serialization module from keylime-agent to the keylime library Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>

Make the agent to provide the endpoints under multiple API versions (currently only under versions 2.1 and 2.2). A new configuration option is introduced, 'api_versions', which allows the user to set the API versions to enable. Only a subset of the versions defined in api::SUPPORTED_API_VERSIONS can be enabled. If a unsupported version is set in the configuration, it will be ignored with a warning. The agent will fail to start if no valid API versions list is configured. The 'api_versions' option supports 2 keywords that can be used instead of the explicit list of versions: - "default": Enables all the supported API versions - "latest": Enables only the latest supported API version This is part of the implementation of the enhancement proposal 114: keylime/enhancements#115 Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>

The registrar_client module implements the builder pattern to allow setting the optional parameters as needed. This also implements the mechanism to allow the agent to communicate with the registrar that support different API versions: - The client will make a GET request to the '/version' endpoint of the registrar. If the request is successful, the client will use the provided API version if it is enabled. - If the registrar does not support the '/version' endpoint, the client will try to register using each of the enabled API versions, starting from the latest. If none of the enabled versions is supported by the registrar, the registration fails. This is part of the implementation of the enhancement proposal 114: keylime/enhancements#115 Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>

Use the keylime::registrar_client module instead of the registrar_agent, which is deleted. This enables the agent to communicate with a registrar using an older API version, restoring the backwards compatibility. This also removes the unnecessary `API_VERSION` from `common.rs`. This is part of the implementation of the enhancement proposal 114: keylime/enhancements#115 Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>

Validate the values set in the `api_versions` configuration option, and filter only the supported versions. The configured versions are also sorted so that the agent can try the enabled versions from the newest to the oldest. If none of the configured options are supported, fallback to use all the supported API versions instead. This is part of the implementation of the enhancement proposal 114: keylime/enhancements#115 Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>

ansasaki · 2025-01-13T10:54:37Z

@keylime/developer Please consider prioritizing reviewing this PR as it is blocking the new agent version release.

sarroutbi · 2025-01-13T11:02:00Z

keylime-agent.conf

@@ -11,7 +11,17 @@
 # The configuration file version
 #
 # To override, set KEYLIME_AGENT_VERSION environment variable.
-version = "2.2"
+version = "2.4"


What happened to 2.3 version? Is this 2.2 -> 2.4 jump expected?

Look at the first commit in the series. 2.3 takes into account some idevid options.

as @sergio-correia commented, the first commit in the series is the bump to the 2.3 version to account for the IDevID/IAK related configuration options.

I recommend reviewing the commits one by one instead of the whole diff from master as it makes it easier to follow the changes.

sarroutbi · 2025-01-13T11:08:23Z

keylime/src/version.rs

+        let result = Version::from_str("22");
+        assert!(result.is_err());
+        let result = Version::from_str(".12");
+        assert!(result.is_err());


Does it make sense to add a version like 1.2.3? And if so ... should this be treated as error?

It was agreed a long time ago that the configuration and API versions will be in the MAJOR.MINOR format.

sarroutbi

Minor comments. Change LGTM

ansasaki marked this pull request as draft December 18, 2024 19:52

ansasaki force-pushed the multiple_api branch from 743f8ea to 2dde804 Compare December 18, 2024 20:00

ansasaki force-pushed the multiple_api branch from 2dde804 to 5627a10 Compare December 19, 2024 13:09

ansasaki marked this pull request as ready for review December 19, 2024 17:13

ansasaki force-pushed the multiple_api branch 3 times, most recently from ede6c9a to a9aec72 Compare December 23, 2024 17:15

ansasaki requested review from THS-on and sergio-correia January 3, 2025 08:28

ansasaki force-pushed the multiple_api branch from a9aec72 to 2725d29 Compare January 3, 2025 08:29

keylime-bot assigned sergio-correia and THS-on Jan 3, 2025

ansasaki requested review from ueno, lkatalin and Isaac-Matthews January 8, 2025 13:20

keylime-bot assigned lkatalin, Isaac-Matthews and ueno Jan 8, 2025

ansasaki requested a review from aplanas January 8, 2025 15:36

keylime-bot assigned aplanas Jan 8, 2025

ansasaki mentioned this pull request Jan 10, 2025

API version configuration keylime/keylime#1555

Open

ansasaki added 6 commits January 13, 2025 10:19

keylime-agent.conf: Bump version to 2.3

146b67f

This is to account for the addition of the options: - idevid_password - idevid_handle - iak_password - iak_handle Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>

Drop unnecessary dependency on common::API_VERSION

41618d5

In many places the dependency are unnecessary and used only for testing. Replace the usage of common::API_VERSION with a static string. Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>

Move 'serialization' module to the keylime library

7f93352

Move the serialization module from keylime-agent to the keylime library Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>

ansasaki force-pushed the multiple_api branch 2 times, most recently from c289e48 to 949af61 Compare January 13, 2025 09:27

sarroutbi reviewed Jan 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support multiple API versions #889

Support multiple API versions #889

ansasaki commented Dec 18, 2024 •

edited

Loading

codecov bot commented Dec 18, 2024 •

edited

Loading

ansasaki commented Dec 19, 2024

ansasaki commented Dec 20, 2024 •

edited

Loading

ansasaki commented Jan 13, 2025

sarroutbi Jan 13, 2025

sergio-correia Jan 13, 2025

ansasaki Jan 13, 2025

sarroutbi Jan 13, 2025

ansasaki Jan 13, 2025

sarroutbi left a comment

Support multiple API versions #889

Are you sure you want to change the base?

Support multiple API versions #889

Conversation

ansasaki commented Dec 18, 2024 • edited Loading

codecov bot commented Dec 18, 2024 • edited Loading

Codecov Report

ansasaki commented Dec 19, 2024

ansasaki commented Dec 20, 2024 • edited Loading

ansasaki commented Jan 13, 2025

sarroutbi Jan 13, 2025

Choose a reason for hiding this comment

sergio-correia Jan 13, 2025

Choose a reason for hiding this comment

ansasaki Jan 13, 2025

Choose a reason for hiding this comment

sarroutbi Jan 13, 2025

Choose a reason for hiding this comment

ansasaki Jan 13, 2025

Choose a reason for hiding this comment

sarroutbi left a comment

Choose a reason for hiding this comment

ansasaki commented Dec 18, 2024 •

edited

Loading

codecov bot commented Dec 18, 2024 •

edited

Loading

ansasaki commented Dec 20, 2024 •

edited

Loading