Add support for force online metadata update #435

MVrachev · 2024-01-05T19:41:23Z

Add support for force online metadata update

Signed-off-by: Martin Vrachev [email protected]

MVrachev · 2024-01-05T19:41:50Z

It's best to test this together with repository-service-tuf/repository-service-tuf-api#513

codecov · 2024-01-05T19:44:48Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (714a29d) to head (0984391).
Report is 25 commits behind head on main.

❗ Current head 0984391 differs from pull request most recent head 5f42b8e. Consider uploading reports for the commit 5f42b8e to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##              main      #435    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           15        15            
  Lines         1071      1173   +102     
==========================================
+ Hits          1071      1173   +102

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

MVrachev · 2024-01-10T11:07:37Z

@kairoaraujo it's ready for a review.

MVrachev · 2024-01-12T12:59:52Z

I invited @fsavoia and @ivanayov for a review.

kairoaraujo

The feature is nice. Thank you @MVrachev.
I'm adding one specific comment.

kairoaraujo · 2024-01-12T14:00:56Z

repository_service_tuf_worker/repository.py

+                    "message": "No online metadata roles given",
+                },
+            )
+


In the API, we allow users to submit offline roles without checking it (example: Root)

In the Worker, we also don't verify it. It will return a task SUCCESS with status True.
I suggest we verify if the role

Note: We can verify Root and Targets now. We have in the settings/redis a setting if the Targets is offline or online key -- for future feature that allows offline targets.

I changed it and now we only allow online roles and specifically check if targets is an online role.

MVrachev · 2024-01-18T16:48:34Z

Ready for another review.

kairoaraujo · 2024-01-24T15:55:19Z

I'm reviewing it now

ivanayov · 2024-01-25T18:51:57Z

repository_service_tuf_worker/repository.py

+        settings_repository = get_worker_settings()
+        settings_repository.reload()
+        if not settings_repository.get_fresh("TARGETS_ONLINE_KEY", True):
+            online_roles = [Snapshot.type, Timestamp.type, "bins"]


Shouldn't bins be excluded as well?
I might be missing some details from the workflow - if so, please ignore to not waste time with the comment.
Otherwise, given that bins are delegates and TARGETS_ONLINE_KEY is False, it looks incorrect to have them in the online roles.

Bins could use the online key.
Even though targets could be offline in the future this doesn't mean we are required to use offline keys for all its delegations. The security properties coming from targets utilizing an offline key wouldn't be lost as when adding, removing or downloading artifact information all requires going through targets first which will be cryptographically signed by an offline key.

ivanayov · 2024-01-25T18:54:18Z

repository_service_tuf_worker/repository.py

+        bootstrap = self._settings.get_fresh("BOOTSTRAP")
+        if bootstrap is None or "pre-" in bootstrap or "signing-" in bootstrap:
+            logging.info(
+                "[automatic_version_bump] Bootstrap not completed, skipping..."


"No bootstrap or not completed, skipping..."?

I think Bootstrap not completed could be interpreted by both:

not started at all

not signed with threshold of keys

It's good that the user have GET /api/v1/bootstrap to understand that bootstrap is in the process of signing: https://github.com/repository-service-tuf/repository-service-tuf-api/blob/277d73b1610a5ec96c4af4d3ccbc8bdbfc69bcf9/repository_service_tuf_api/bootstrap.py#L128
and GET /api/v1/metadata to get the exact content of this bootstrap.

In conclusion, I don't we need to change the message to a longer now.

kairoaraujo

Is it better to merge custom delegated roles and rebase them?

Fail early when bootstrap is not completed instead of relly on _run_online_roles_bump to catch this. Signed-off-by: Martin Vrachev <[email protected]>

Signed-off-by: Martin Vrachev <[email protected]>

We handle this case in the API, so there is no need to handle when payload is empty for force_online_metadata_update(). Signed-off-by: Martin Vrachev <[email protected]>

Signed-off-by: Martin Vrachev <[email protected]>

Make sure the name it's clear it's only used for custom target delegation. Signed-off-by: Martin Vrachev <[email protected]>

Use the same method to call update_snapshot() with hash bin delegation as we do with custom target delegation. Signed-off-by: Martin Vrachev <[email protected]>

Signed-off-by: Martin Vrachev <[email protected]>

MVrachev · 2024-04-16T21:49:51Z

@kairoaraujo ready to test together with repository-service-tuf/repository-service-tuf-api#513
Support for pushing a new version of custom target delegation is there.

kairoaraujo · 2024-04-18T06:08:17Z

repository_service_tuf_worker/repository.py

@@ -1208,6 +1221,116 @@ def bump_online_roles(self, force: Optional[bool] = False) -> bool:

        return True

+    def _run_force_online_metadata_update(self, roles: List[str]) -> List[str]:


I'm wondering if the metadata_update is the best name here, as it confuses our process of updating the metadata.

Well it is actually metadata update...
Hard to get a name correctly...

MVrachev self-assigned this Jan 5, 2024

MVrachev requested a review from kairoaraujo January 10, 2024 11:07

kairoaraujo requested changes Jan 12, 2024

View reviewed changes

kairoaraujo mentioned this pull request Jan 12, 2024

Add support for PUT /api/v1/metadata/online repository-service-tuf/repository-service-tuf-api#513

Merged

6 tasks

MVrachev force-pushed the add-metadata-put branch 2 times, most recently from bf35eb3 to 6c80191 Compare January 18, 2024 16:48

MVrachev requested a review from kairoaraujo January 18, 2024 16:48

MVrachev force-pushed the add-metadata-put branch from f4d74a3 to 9597226 Compare January 19, 2024 15:57

ivanayov reviewed Jan 25, 2024

View reviewed changes

kairoaraujo reviewed Mar 12, 2024

View reviewed changes

MVrachev force-pushed the add-metadata-put branch from a5fc735 to 5848aa4 Compare April 11, 2024 13:19

MVrachev added 9 commits April 16, 2024 16:24

Bump online roles: fail on non final bootstrap

a78273c

Fail early when bootstrap is not completed instead of relly on _run_online_roles_bump to catch this. Signed-off-by: Martin Vrachev <[email protected]>

Add support for force online metadata update

6f6caf6

Signed-off-by: Martin Vrachev <[email protected]>

If payload empty => update all roles

574747e

Signed-off-by: Martin Vrachev <[email protected]>

Fix small nit in tests

bcd3885

Signed-off-by: Martin Vrachev <[email protected]>

Don't handle empty payload for force roles update

998dbfc

We handle this case in the API, so there is no need to handle when payload is empty for force_online_metadata_update(). Signed-off-by: Martin Vrachev <[email protected]>

Force new versions of custom target delegations

72621d8

Signed-off-by: Martin Vrachev <[email protected]>

Rename DELEGATED_ROLES -> CUSTOM_DELEGATED_ROLES

74b6df0

Make sure the name it's clear it's only used for custom target delegation. Signed-off-by: Martin Vrachev <[email protected]>

Use same method for bins as for custom delegation

8933c1c

Use the same method to call update_snapshot() with hash bin delegation as we do with custom target delegation. Signed-off-by: Martin Vrachev <[email protected]>

Fix linting

5f42b8e

Signed-off-by: Martin Vrachev <[email protected]>

MVrachev force-pushed the add-metadata-put branch from 0984391 to 5f42b8e Compare April 16, 2024 21:45

kairoaraujo approved these changes Apr 18, 2024

View reviewed changes

kairoaraujo merged commit b486903 into repository-service-tuf:main Apr 18, 2024
27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for force online metadata update #435

Add support for force online metadata update #435

MVrachev commented Jan 5, 2024

MVrachev commented Jan 5, 2024

codecov bot commented Jan 5, 2024 •

edited

Loading

MVrachev commented Jan 10, 2024

MVrachev commented Jan 12, 2024

kairoaraujo left a comment

kairoaraujo Jan 12, 2024

MVrachev Jan 18, 2024

MVrachev commented Jan 18, 2024

kairoaraujo commented Jan 24, 2024

ivanayov Jan 25, 2024

MVrachev Feb 5, 2024

ivanayov Jan 25, 2024 •

edited

Loading

MVrachev Feb 5, 2024

kairoaraujo left a comment

MVrachev commented Apr 16, 2024

kairoaraujo Apr 18, 2024

MVrachev Apr 19, 2024

		@@ -1208,6 +1221,116 @@ def bump_online_roles(self, force: Optional[bool] = False) -> bool:

		return True

		def _run_force_online_metadata_update(self, roles: List[str]) -> List[str]:

Add support for force online metadata update #435

Add support for force online metadata update #435

Conversation

MVrachev commented Jan 5, 2024

MVrachev commented Jan 5, 2024

codecov bot commented Jan 5, 2024 • edited Loading

Codecov Report

MVrachev commented Jan 10, 2024

MVrachev commented Jan 12, 2024

kairoaraujo left a comment

Choose a reason for hiding this comment

kairoaraujo Jan 12, 2024

Choose a reason for hiding this comment

MVrachev Jan 18, 2024

Choose a reason for hiding this comment

MVrachev commented Jan 18, 2024

kairoaraujo commented Jan 24, 2024

ivanayov Jan 25, 2024

Choose a reason for hiding this comment

MVrachev Feb 5, 2024

Choose a reason for hiding this comment

ivanayov Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

MVrachev Feb 5, 2024

Choose a reason for hiding this comment

kairoaraujo left a comment

Choose a reason for hiding this comment

MVrachev commented Apr 16, 2024

kairoaraujo Apr 18, 2024

Choose a reason for hiding this comment

MVrachev Apr 19, 2024

Choose a reason for hiding this comment

codecov bot commented Jan 5, 2024 •

edited

Loading

ivanayov Jan 25, 2024 •

edited

Loading