RSTUF initial implementation #4167

simi · 2023-10-31T01:24:52Z

The RSTUF idea is simple, call add endpoint when package is added to index and call remove endpoint when package is removed.

RSTUF API is async, by calling add/remove endpoints, background job is enqueued on the RSTUF side. That's the reason for CheckJob. It is used later to ensure remote job has finished successfully.

RSTUF is enabled only when RSTUF_API_URL ENV variable is set (=> disabled by default). Plan is to deploy RSTUF API just to be accessible by the RubyGems.org app, but not from the outside of K8s cluster. That's RSUF recommendation for now, it is considered internal API and is not meant to be exposed publicly into internet. Next step is to prepare deploy of RSTUF onto staging environment using helm chart.

Open questions

How to set retry mechanism for waiting unless remote job finishes?
How to handle failed remote jobs?

codecov · 2023-10-31T01:35:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (150005e) 97.13% compared to head (135b9e9) 97.15%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4167      +/-   ##
==========================================
+ Coverage   97.13%   97.15%   +0.02%     
==========================================
  Files         385      391       +6     
  Lines        8200     8259      +59     
==========================================
+ Hits         7965     8024      +59     
  Misses        235      235

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

app/jobs/rstuf/check_job.rb

test/jobs/rstuf/check_job_test.rb

segiddins · 2023-10-31T21:54:44Z

The RSTUF idea is simple, call add endpoint when package is added to index and call remove endpoint when package is removed.

Could you please add (either here or in a doc somewhere) a short description of what RSTUF is, why we/our users should care about it, and how this PR fits into further work? It feels like I'm missing some context here on what the project/big idea is

simi · 2023-10-31T22:10:40Z

The RSTUF idea is simple, call add endpoint when package is added to index and call remove endpoint when package is removed.

Could you please add (either here or in a doc somewhere) a short description of what RSTUF is, why we/our users should care about it, and how this PR fits into further work? It feels like I'm missing some context here on what the project/big idea is

Good idea, I'll add also RSTUF.md with short info and also steps how to run with RubyGems.org locally. In short, this is replacement for #626, but instead of implementing whole TUF using RSTUF as self-hosted API based service.

And what TUF in context of RubyGems.org is? TUF is additional layer of security to ensure that the gems you download are verified and haven't been tampered with, using cryptographic signatures even if the gem server is compromised. In super short - it is isolated crypto-safe repository of package signatures. It is also initial step for (hopefully following) sigstore implementation. More info at https://blog.sigstore.dev/sigstore-bring-your-own-stuf-with-tuf-40febfd2badd/.

martinemde

Overall I wonder why we are checking the status if we don't store success or failure anywhere. We are either relying on our exception reporting as part of our application data or we are checking the status unnecessarily. If we want to know when it's broken, can we store that in our database somewhere in addition to error reporting.

app/jobs/rstuf/application_job.rb

martinemde · 2023-11-01T04:25:16Z

app/jobs/rstuf/check_job.rb

+    when "SUCCESS"
+      # no-op, all good
+    when "FAILURE"
+      raise FailureException, "RSTUF job failed"


How much do we care about this failing? Jobs fail often enough that these might be missed, right? Should we build a different way to record error percentage?

For example, should it make the request and ignore the result, then periodically check for consistency?

I'm not 100% sure how much to "infect" RubyGems.org with RSTUF. Yes, it is possible to add rstuf_indexed to Version model and update in here.

Currently the main idea is to raise exception on problems. If RSTUF is not able to finish the task or task is failing, it would be great to get maintainers notified since it usually means something is wrong with RSTUF.

Do we maintain the server side? If so, could we just error on the server?

I'm OK with using persistent job failures as a signal for ONCALL to investigate, it's a pattern I've used before

@martinemde plan is to maintain server side (deploy to RubyGems.org cluster). I'll see during deployment how simple that is to track errors in there. I'll definitelly work on improvements in there if needed. But I would like to keep RubyGems.org isolated and raise error if 3rd party app doesn't work. Maybe in the future RSTUF will be maintained by different maintainers.

app/models/deletion.rb

lib/rstuf.rb

lib/rstuf/client.rb

app/jobs/rstuf/add_job.rb

app/jobs/rstuf/check_job.rb

test/models/deletion_test.rb

test/models/pusher_test.rb

segiddins · 2023-11-01T18:06:30Z

app/jobs/rstuf/check_job.rb

+    when "SUCCESS"
+      # no-op, all good
+    when "FAILURE"
+      raise FailureException, "RSTUF job failed"


I'm OK with using persistent job failures as a signal for ONCALL to investigate, it's a pattern I've used before

config/initializers/rstuf.rb

simi · 2023-11-13T23:26:52Z

Overall I wonder why we are checking the status if we don't store success or failure anywhere. We are either relying on our exception reporting as part of our application data or we are checking the status unnecessarily. If we want to know when it's broken, can we store that in our database somewhere in addition to error reporting.

This implements only server side of TUF, there is no client support yet. Fail on RSTUF means no problem for gem install for now. We will need to find out meanwhile how to safely operate this before rolling out default TUF check on gem/bundle install, but that is still relatively far away. Consider this PR as initial work for staging or even production testing without affecting gem/bundler clients.

kairoaraujo

Hi @simi,

I comment about Tasks state and status result.
I still want to discuss how we can improve the RSTUF documentation about it.

lib/rstuf/client.rb

app/jobs/rstuf/check_job.rb

simi · 2024-01-31T08:19:45Z

ℹ️ This needs changes from repository-service-tuf/repository-service-tuf-api#518 applied. I'm on it.

segiddins

👍🏻 with some of the existing comments addressed

lib/rstuf/client.rb

indirect

I am happy to move forward with this and see how it is to run in production, as we (over time) expand towards signing more gem data, and reading and verifying the TUF data in the client. 👍🏻

simi · 2024-01-31T21:06:20Z

@kairoaraujo would you mind to do last check for state handling?
@segiddins logger updated.
@indirect indeed this is just beginning, we will explore over time what and how to store in RSTUF.

kairoaraujo · 2024-02-01T06:56:49Z

@kairoaraujo would you mind to do last check for state handling?

Yes, LGTM @simi

kairoaraujo

The RSUF task state handling LGTM! 🎉

simi · 2024-02-18T02:18:14Z

@martinemde @indirect @segiddins I have added short info in CONTRIBUTING.md. From my side this is ready for merge.

martinemde

I think it's ready too. One minor quibble about the placement of teardown. Looking forward to seeing this!

test/unit/rstuf/client_test.rb

simi force-pushed the rstuf-init branch from 6270516 to 4fcbd90 Compare October 31, 2023 01:26

simi requested review from segiddins, martinemde and indirect and removed request for segiddins and martinemde October 31, 2023 10:54

mensfeld requested changes Oct 31, 2023

View reviewed changes

app/jobs/rstuf/check_job.rb Outdated Show resolved Hide resolved

app/jobs/rstuf/check_job.rb Outdated Show resolved Hide resolved

mensfeld reviewed Oct 31, 2023

View reviewed changes

test/jobs/rstuf/check_job_test.rb Outdated Show resolved Hide resolved

simi force-pushed the rstuf-init branch from 4fcbd90 to f9fea24 Compare October 31, 2023 20:01

simi requested a review from mensfeld October 31, 2023 20:02

martinemde reviewed Nov 1, 2023

View reviewed changes

mensfeld approved these changes Nov 1, 2023

View reviewed changes

olleolleolle reviewed Nov 1, 2023

View reviewed changes

app/models/deletion.rb Outdated Show resolved Hide resolved

olleolleolle reviewed Nov 1, 2023

View reviewed changes

app/models/deletion.rb Outdated Show resolved Hide resolved

segiddins reviewed Nov 1, 2023

View reviewed changes

simi force-pushed the rstuf-init branch from f9fea24 to 5821d27 Compare November 13, 2023 23:19

simi requested review from olleolleolle, segiddins and martinemde November 13, 2023 23:47

olleolleolle approved these changes Nov 14, 2023

View reviewed changes

martinemde approved these changes Nov 16, 2023

View reviewed changes

kairoaraujo reviewed Nov 17, 2023

View reviewed changes

lib/rstuf/client.rb Outdated Show resolved Hide resolved

app/jobs/rstuf/check_job.rb Outdated Show resolved Hide resolved

app/jobs/rstuf/check_job.rb Show resolved Hide resolved

simi force-pushed the rstuf-init branch from 5821d27 to 5fb7deb Compare January 31, 2024 00:07

segiddins approved these changes Jan 31, 2024

View reviewed changes

lib/rstuf/client.rb Outdated Show resolved Hide resolved

indirect approved these changes Jan 31, 2024

View reviewed changes

simi force-pushed the rstuf-init branch from 5fb7deb to dd9d256 Compare January 31, 2024 20:54

simi requested review from kairoaraujo and segiddins January 31, 2024 21:06

kairoaraujo reviewed Feb 1, 2024

View reviewed changes

simi force-pushed the rstuf-init branch from dd9d256 to 98b895b Compare February 15, 2024 20:27

martinemde approved these changes Feb 18, 2024

View reviewed changes

test/unit/rstuf/client_test.rb Outdated Show resolved Hide resolved

simi added 9 commits February 18, 2024 04:40

Initial RSTUF implementation.

dcd75b6

Upgrade to latest RSTUF API.

226ff8b

Test RSTUF API implementation.

2a73796

Update with review suggestions.

b289acf

Rename Rstuf::Client.task_status to task_state.

82f26ff

Use semantic logger in Rstuf::Client.

2213849

Handle update RSTUF API states.

25036ea

Add short RSTUF dev docs.

92cc1a5

Move teardown up.

135b9e9

simi force-pushed the rstuf-init branch from 42e7612 to 135b9e9 Compare February 18, 2024 03:40

simi merged commit b2b787c into master Feb 18, 2024
17 checks passed

simi deleted the rstuf-init branch February 18, 2024 22:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RSTUF initial implementation #4167

RSTUF initial implementation #4167

simi commented Oct 31, 2023

codecov bot commented Oct 31, 2023 •

edited

Loading

segiddins commented Oct 31, 2023

simi commented Oct 31, 2023

martinemde left a comment

martinemde Nov 1, 2023

simi Nov 1, 2023

martinemde Nov 1, 2023

segiddins Nov 1, 2023

simi Nov 13, 2023

segiddins Nov 1, 2023

simi commented Nov 13, 2023 •

edited

Loading

kairoaraujo left a comment

simi commented Jan 31, 2024

segiddins left a comment

indirect left a comment

simi commented Jan 31, 2024

kairoaraujo commented Feb 1, 2024 •

edited

Loading

kairoaraujo left a comment

simi commented Feb 18, 2024

martinemde left a comment

RSTUF initial implementation #4167

RSTUF initial implementation #4167

Conversation

simi commented Oct 31, 2023

Open questions

codecov bot commented Oct 31, 2023 • edited Loading

Codecov Report

segiddins commented Oct 31, 2023

simi commented Oct 31, 2023

martinemde left a comment

Choose a reason for hiding this comment

martinemde Nov 1, 2023

Choose a reason for hiding this comment

simi Nov 1, 2023

Choose a reason for hiding this comment

martinemde Nov 1, 2023

Choose a reason for hiding this comment

segiddins Nov 1, 2023

Choose a reason for hiding this comment

simi Nov 13, 2023

Choose a reason for hiding this comment

segiddins Nov 1, 2023

Choose a reason for hiding this comment

simi commented Nov 13, 2023 • edited Loading

kairoaraujo left a comment

Choose a reason for hiding this comment

simi commented Jan 31, 2024

segiddins left a comment

Choose a reason for hiding this comment

indirect left a comment

Choose a reason for hiding this comment

simi commented Jan 31, 2024

kairoaraujo commented Feb 1, 2024 • edited Loading

kairoaraujo left a comment

Choose a reason for hiding this comment

simi commented Feb 18, 2024

martinemde left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 31, 2023 •

edited

Loading

simi commented Nov 13, 2023 •

edited

Loading

kairoaraujo commented Feb 1, 2024 •

edited

Loading