Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP Bots #1

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

WIP Bots #1

wants to merge 6 commits into from

Conversation

Gankra
Copy link

@Gankra Gankra commented Sep 27, 2022

Workflow Design Board

Upstream Issue

Most of this is just a bunch of churn to make the json schema more explicitly typed so that other things can more easily consume it.

TODO:

  • Change cargo-vet to be bottable
    • emit extra context in the json output
    • ? add a --with-metadata flag to be able to pass a cached cargo-metadata on stdin(?)/file(?) to operate "headless"
  • create a Github App/Action that runs cargo vet --output-format=json-full
    • have the bot send the json blob to our webapp and get back some kind of transcation id
    • have the bot post a comment on the repo with the issue / link to webapp with that id
    • have the bot(?) able to run cargo-vet certify in CI at the webapp's request?
  • create a webapp
    • have a simple database (redis?)
    • render the json report
    • have a form for the values in certify
    • have the webapp auth the user with github to be able to do things on their behalf
    • have the webapp command the bot(?) to do a certify on submit

This makes it more plausible for this format to be made available in a subcrate to allow other
libraries/tools to easily deserialize it. Also makes it more obvious the places where we're
being weirdly inconsistent or exposing random internals because I was lazy/hasty.

A weird side-effect of this is that the order of the fields got shuffled around. It seems
the json macro will sort fields by name, while proper derives use the decl order. The latter
seems preferred anyway, but unfortunately we take a huge obfuscation churn.

This also contains part of the introduction of the concept of --output-format=json-cli
because I realized I wanted to do this half way through adding that. WHOOPS.
@Gankra
Copy link
Author

Gankra commented Oct 13, 2022

From chat:

ok so I got a public cargo-dist repo setup at: https://github.com/axodotdev/cargo-dist

it's basically the template but with things tweaked to be cargo-* friendly. binary does nothing.

I also setup cargo-vet ci as per the book (wasmtime uses this task): https://github.com/axodotdev/cargo-dist/blob/main/.github/workflows/ci.yml#L95-L97

but pointing at my PR branch (and using --output-format=json-full): #1

(As a side-effect the cargo-vet binary caching is broken so the tasks take extra long to fetch+build it from scratch every time.)

Here is a failing dependabot PR because it updates a dep to a version not audited in cargo-vet: axodotdev/cargo-dist#7

Here is a the failing CI task and the JSON it vomits out: https://github.com/axodotdev/cargo-dist/actions/runs/3245691708/jobs/5323548466

@Gankra
Copy link
Author

Gankra commented Oct 13, 2022

The biggest hurdle to overcome at this point is creating the GH app/bot and hooking this failing CI into that, as per the Design Board

@Gankra
Copy link
Author

Gankra commented Oct 25, 2022

I've been prototyping out some actions stuff over at https://github.com/Gankra/gh-action-tests/ just so I'm the only one suffering under 10 million failed CI runs.

Current Status

  • cargo-vet-check

    • ✅ runs cargo vet --locked --output-format=json-full, and on failure (for PRs)...
    • ✅ adds fields to the json about the repo/pr
      • "repo": "user/repo"
      • "pr": 5
    • ✅ curls the json to cargo-vet.axo.dev
    • ✅ posts a comment on the PR linking the appropriate page on cargo-vet.axo.dev
    • ❌ does this under a generic "github ci" account and not "cargo-vet" (fine, whatever for MVP)
    • 🚧 on-push trigger currently disabled because I'm using to develop the stuff below...
  • 🚧 cargo-vet-certify

Future Work

  • We need to trigger the cargo-vet-certify workflow remotely (Nikkita)

    • Apparently the API we have available can be passed inputs so we can maybe remove the get-audits job altogether and just invoke the cargo-vet-certify job directly? Possibly also remove the base64 stuff which is largely hacking around terrible bash/conf-file crap for sending data between tasks!
    • cargo-vet-check may need to send extra branch information for the REST API?
  • Need to make a commit and push to the PR branch! (Gankra)

    • Need to do this "as" the bot? Maybe naturally happens based on the way we trigger the job?
    • Do we need to use octokit to make the commit or can we legit run git commit -am ...?
    • Need to figure out the push API

@Gankra
Copy link
Author

Gankra commented Oct 27, 2022

🎊 the mvp is working!

Changes

  • cargo-vet-check now does not submit comments, but instead bundles up the failure in an artifact zip

  • the new cargo-vet-check-submit-failure task responds to cargo-vet-check failing, unpacks the zip, and submits the problem/comment

    • this is necessary to have the PR sandboxed (readonly) when doing things based on its code while still letting us do things like post comments (write)
  • a fuck ton of shell-scripty stuff is replaced with "github-script" which is just node.js programs inline in the yaml with some implicit imports

  • cargo-vet-certify now commits and pushes

  • and works cross-repo by triggering the action on the repo the PR is FROM

    • this has some weird permissions implications

TODO

  • We need to work out exactly what to do in the cross-repo commit+push situation.

  • We need to work out the UX for when two people submit audits at the same time (or one person submits, then resubmits).

    • Currently the two tasks will race and the loser will get a "please pull" error.
    • Thankfully no data is "lost" because we save the form state
    • We could rebase and retry?
      • May result in duplicate audits being committed
    • We could run the checkout action later to minimize the time when racing can happen
    • ??? something smarter ???
  • There isn't any feedback on the PR that the background tasks are running.

    • cargo-vet-check-submit-failure
    • cargo-vet-certify
  • Have the comments+commits be done by our bot and not the generic github actions account?

  • Retrigger CI in response to audits

  • Iterate on UI/UX/Strings

@Gankra
Copy link
Author

Gankra commented Oct 28, 2022

filed a bunch of these on https://github.com/axodotdev/cargo-vet-webapp/issues/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant