Add support for `PUT /api/v1/metadata/online` #513

MVrachev · 2024-01-05T19:36:03Z

Description

Add support for PUT /api/v1/metadata which will force a new metadata version
of a given set of online roles.

Fixes #399

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality not to work as expected)

Additional requirements

Tests have been added for the bug fix or new feature
Docs have been added for the bug fix or new feature

Code of Conduct

By submitting this PR, you agree to follow our Code of Conduct.

I agree to follow this project's Code of Conduct

codecov · 2024-01-05T19:39:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.09%. Comparing base (25928b7) to head (61e05da).
Report is 32 commits behind head on main.

❗ Current head 61e05da differs from pull request most recent head 5153aaa. Consider uploading reports for the commit 5153aaa to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #513      +/-   ##
==========================================
+ Coverage   98.97%   99.09%   +0.11%     
==========================================
  Files          14       14              
  Lines         588      666      +78     
==========================================
+ Hits          582      660      +78     
  Misses          6        6

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

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

It's best to test this together with #513

MVrachev · 2024-01-08T13:58:24Z

Before merging this pr, lets merge: #514.

MVrachev · 2024-01-10T11:06:31Z

@kairoaraujo it's ready for a review.

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

I invited @fsavoia and @ivanayov for a review.

repository_service_tuf_api/metadata.py

kairoaraujo · 2024-01-12T14:18:07Z

repository_service_tuf_api/api/metadata.py

@@ -31,6 +31,25 @@ def post(payload: metadata.MetadataPostPayload):
    return metadata.post_metadata(payload)


+@router.put(
+    "/",


I think it should be a specific endpoint "/api/v1/metadata/"
Also, I would use the following behavior.
No rolename = bump all online roles.

It is already using api/v1/metadata:

I will address the second comment though.

Sorry, what I mean is to use a more specific endpoint /api/v1/metadata/<choose a name>
I see the /api/v1/metadata for more generic actions like Update Metadata.

As this description says, "Force a new version of online metadata role(s)." it should have a more specific endpoint
i.e.: /api/v1/metadata/online. Following our past changes, it is an asynchronous task that could be a POST.

After a discussion we agreed it will be POST /api/v1/metadata/online/

MVrachev · 2024-01-18T15:59:34Z

@kairoaraujo I did as you suggested if payload is empty I made sure force update on all roles.
I also added a check in Roles.online_roles to whether Targets is an online role.
If it's not we don't return it in this list.

It's ready for review.

ivanayov · 2024-01-19T18:53:44Z

docs/swagger.json

+                    "Metadata"
+                ],
+                "summary": "Force a new version of online metadata role(s).",
+                "description": "Force a new version of online metadata role(s). If the roles list is empty all roles will be updated. The new metadata version(s) will have extended expiration which will equal to: today + ROLE_NAME_EXPIRATION number of days, where ROLE_NAME_EXPIRATION is a tuf repository setting. Note: depending on which metadata role you want to update other online roles will likely be updated as well otherwise consistency will be lost.",


What will happen if datetime.now() + <ROLE_NAME_EXPIRATION number of days> is less than the current expiration? I think we should either make it impossible or define it more strictly here (like extended or redused - practically it can also be the same, so we can just say "updated").

What I mean is that this description creates expectations that don't strictly match the functionality.

The whole idea is to be able to redefine current expiration.
For context: the idea of this feature came to life after I implemented PUT /api/v1/config.
With that API endpoint, you can change the expiration policy for each role.
Still, this policy will be enforced when an online role expires and the automatic job to bump that role is executed.
See: https://github.com/repository-service-tuf/repository-service-tuf-worker/blob/486e98cdbc16dd41c62c0ec8b473e2f94312ee9c/app.py#L139

The idea behind this new endpoint is that after you have called PUT /api/v1/config you can call this new endpoint PUT /api/v1/metadata and request a new metadata version of part or all online metadata roles.

For the future I have thought about adding a flag force_new_version to the PUT /api/v1/config endpoint which will do the same thing as this new endpoint but for all roles.

As suggested by Ivana I added a commit that makes sure we cannot push negative expiration days.
Here is the commit: 8a00f5b

ivanayov · 2024-01-19T18:56:24Z

repository_service_tuf_api/api/metadata.py

+    description=(
+        "Force a new version of online metadata role(s). If the roles list is "
+        "empty all roles will be updated. The new metadata version(s) will "
+        "have extended expiration which will equal to: "


Same about "extended" here.

Check my comment to your questions above.

MVrachev · 2024-01-24T11:56:02Z

I had to remove my latest changes where I added more requirements to our attributes as there was an issue.
I have added a new issue about this: #520.

repository_service_tuf_api/metadata.py

repository_service_tuf_api/config.py

MVrachev · 2024-03-12T14:21:43Z

@kairoaraujo I updated the pr by:

changing the endpoint to PUT /api/v1/metadata/online
the roles that could be forced bumped are not predefined literals, but any string
added a check if bins are used, then only predefined roles (a.k.a roles defined in Roles) can be force bumped.
removed comments about Fast API issue.

I suggest first merging custom target delegation prs and rebasing those on top of them.
That will allow us to check if we can bump custom target delegated roles.

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

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

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

The only remaining question that comes to my mind is whether we should keep the API call to POST /api/v1/metadata/online as we update our current online roles instead of adding a new one?
The other possibility is to instead use PUT /api/v1/metadat/online which to me sounds more precise.

MVrachev mentioned this pull request Jan 5, 2024

Add support for force online metadata update repository-service-tuf/repository-service-tuf-worker#435

Merged

MVrachev self-assigned this Jan 5, 2024

MVrachev force-pushed the add-metadata-put branch from 0127ee8 to 74c8ddd Compare January 8, 2024 13:57

MVrachev requested a review from kairoaraujo January 10, 2024 10:55

kairoaraujo requested changes Jan 12, 2024

View reviewed changes

MVrachev force-pushed the add-metadata-put branch 2 times, most recently from c89d9f9 to 9d226cb Compare January 18, 2024 15:57

ivanayov reviewed Jan 19, 2024

View reviewed changes

MVrachev force-pushed the add-metadata-put branch 2 times, most recently from 8a00f5b to 865d89c Compare January 24, 2024 11:46

MVrachev commented Mar 11, 2024

View reviewed changes

repository_service_tuf_api/metadata.py Outdated Show resolved Hide resolved

MVrachev commented Mar 11, 2024

View reviewed changes

repository_service_tuf_api/config.py Outdated Show resolved Hide resolved

MVrachev force-pushed the add-metadata-put branch from 62cd823 to 4fc3dcc Compare March 12, 2024 14:17

MVrachev changed the title ~~Add support for PUT /api/v1/metadata~~ Add support for PUT /api/v1/metadata/online Mar 12, 2024

MVrachev added 7 commits April 11, 2024 15:46

Add support for PUT /api/v1/metadata

29b16f1

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

Add tests

1f6a678

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

Fix: allow only online roles

66f5aae

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

Check if targets is an online role or not

11c2a9e

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

Is targets is offline in Roles.online_roles_values

e40ce85

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

If payload["roles"] empty => update all roles

839f34b

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

Add todo notes for future attribute requirements

1af9080

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

MVrachev added 14 commits April 11, 2024 16:23

Combine response data types

4350b28

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

Change force online metadata endpoint

54c9989

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

Use new pydantic format

4946987

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

Allow enforcing a new md version of custom target

9247cb9

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

Fix linting issue

a14a334

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

Fix docs

7aafb3e

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

Fix import

efa9536

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

Make PUT config payload to accept custom targets

dc46938

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

Add explicit check that "targets" inside payload

85bf83b

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

Change how we check for bins or custom delegation

53825fc

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

Check custom delegation used => not allow "bins"

77b5aa3

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

Fix Roles.online_roles_values()

3f5c3b1

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

Simplify hash bin delegation check and add comment

5f5945e

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

Fix linting

8677271

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

MVrachev force-pushed the add-metadata-put branch from 4fc3dcc to 8677271 Compare April 16, 2024 21:40

Tests: simplify mocking paths

4d1261c

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

MVrachev force-pushed the add-metadata-put branch from 61e05da to 4d1261c Compare April 16, 2024 21:48

MVrachev requested a review from kairoaraujo April 16, 2024 21:48

Merge branch 'main' into add-metadata-put

5153aaa

kairoaraujo approved these changes Apr 18, 2024

View reviewed changes

kairoaraujo merged commit 7631efa into repository-service-tuf:main Apr 18, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for `PUT /api/v1/metadata/online` #513

Add support for `PUT /api/v1/metadata/online` #513

MVrachev commented Jan 5, 2024 •

edited

Loading

codecov bot commented Jan 5, 2024 •

edited

Loading

MVrachev commented Jan 5, 2024

MVrachev commented Jan 8, 2024

MVrachev commented Jan 10, 2024

MVrachev commented Jan 12, 2024

kairoaraujo Jan 12, 2024

MVrachev Jan 18, 2024

kairoaraujo Jan 27, 2024

MVrachev Mar 11, 2024

MVrachev commented Jan 18, 2024 •

edited

Loading

ivanayov Jan 19, 2024

ivanayov Jan 19, 2024

MVrachev Jan 22, 2024 •

edited

Loading

MVrachev Jan 23, 2024

ivanayov Jan 19, 2024

MVrachev Jan 22, 2024

MVrachev commented Jan 24, 2024

MVrachev commented Mar 12, 2024

MVrachev commented Apr 16, 2024 •

edited

Loading

Add support for PUT /api/v1/metadata/online #513

Add support for PUT /api/v1/metadata/online #513

Conversation

MVrachev commented Jan 5, 2024 • edited Loading

Description

Types of changes

Additional requirements

Code of Conduct

codecov bot commented Jan 5, 2024 • edited Loading

Codecov Report

MVrachev commented Jan 5, 2024

MVrachev commented Jan 8, 2024

MVrachev commented Jan 10, 2024

MVrachev commented Jan 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MVrachev commented Jan 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MVrachev Jan 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MVrachev commented Jan 24, 2024

MVrachev commented Mar 12, 2024

MVrachev commented Apr 16, 2024 • edited Loading

Add support for `PUT /api/v1/metadata/online` #513

Add support for `PUT /api/v1/metadata/online` #513

MVrachev commented Jan 5, 2024 •

edited

Loading

codecov bot commented Jan 5, 2024 •

edited

Loading

MVrachev commented Jan 18, 2024 •

edited

Loading

MVrachev Jan 22, 2024 •

edited

Loading

MVrachev commented Apr 16, 2024 •

edited

Loading