panda-re / panda

Platform for Architecture-Neutral Dynamic Analysis
https://panda.re
Other
2.45k stars 475 forks source link

Andrew Q pipeline run #1509

Closed coolkingcole closed 2 weeks ago

coolkingcole commented 1 month ago

testing pipeline for Andrew Quijano.

AndrewFasano commented 1 month ago

I'm fairly reluctant to merge changes to our CI as we can't really test CI in advance and it typically requires a series of PRs to fix the things each change breaks. There are a lot of different things going on here. I like some of them, but definitely don't think they should all be in a single PR. There are still lots of things mixed in/across commits in ways that make it hard to understand your changes.

Here's my best attempt to identify what's happening here and provide feedback. I'm very interested in merging the docs updates and action version increments. I'm worried the other changes will break things.

AndrewQuijano commented 1 month ago

So as for my change to setup.sh, currently for deployment we:

1- create a panda_installer image up to installer target 2- create a panda image up to panda target

As this script is only used for deployment, I propose just creating one panda image, build up to installer, extract the wheel file, and continue to build until panda target to package the Debian package. This should speed up deployment quite a bit as we are running make for panda only once instead of twice, at least that is what I assume the "CACHED" means on the screenshots provided.

image image image

Here is the full run log https://productionresultssa7.blob.core.windows.net/actions-results/ccb307b3-5aa8-49f9-88f2-63447c0a8652/workflow-job-run-23cbbf46-0090-5639-7e11-ae079f190a1b/logs/job/job-logs.txt?rsct=text%2Fplain&se=2024-07-15T04%3A26%3A12Z&sig=E0aXzZEK6NmDb2r0dxEPnaIWOFOtOt7M4Q%2B0anbSMEw%3D&ske=2024-07-15T14%3A38%3A20Z&skoid=ca7593d4-ee42-46cd-af88-8b886a2f84eb&sks=b&skt=2024-07-15T02%3A38%3A20Z&sktid=398a6654-997b-47e9-b12b-9515b896b4de&skv=2023-11-03&sp=r&spr=https&sr=b&st=2024-07-15T04%3A16%3A07Z&sv=2023-11-03

About creating pandare/panda_test, the issue is that external users can't push to ghcr.io/pandare, so I propose we cache on DockerHub, since sharing a secret is easier than creating a shared account. Based on what I read, it seems the solution then is to call a Workflow dispatch button, so that then the external PR workflow can access the dockerhub secret and cache the image in DockerHub. It seems like the caching worked just fine. Unfortunately, it seems like there is no way to know if this will work until it is merged and when an external party submits a PR. But I'd say this meets both security needs as dispatch workflow can only occur manually, but would allow external users to use the cached dockerhub API token to cache a test panda container.

https://docs.github.com/en/actions/using-workflows/manually-running-a-workflow

P. S. For libcapstone, I just followed what you did on the Dockerfile https://github.com/panda-re/panda/commit/f861fb3772a9e9150487c38a9adba1a8d48b23a6

AndrewQuijano commented 3 weeks ago

Everything should be addressed on this branch: https://github.com/AndrewQuijano/panda/tree/cole-updates

AndrewQuijano commented 3 weeks ago

And I raised the issue to get a Debian package for Capstone, seems like whenever v6 comes out, we will finally get a package

https://github.com/capstone-engine/capstone/issues/2398