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

Run integration using uroot #302

Merged
merged 2 commits into from
Apr 27, 2024

Conversation

jimmykarily
Copy link
Contributor

This makes it possible to run the tests both locally and in CI with: make integration (at least when the host is based on Arch linux because it still relies on /usr/share/edk2-ovmf/x64/OVMF_VARS.fd and /usr/share/edk2-ovmf/x64/OVMF_CODE.secboot.fd to be around. But we can fix that later by either running the tests in a container locally or by adding these 2 files in git (TBD).

Note: One test is still failing but I'm trying to understand how the original one worked in order to fix it.

@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:12 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:12 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:12 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:18 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:18 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:18 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:21 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:21 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:21 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:35 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:35 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:35 — with GitHub Actions Inactive
@jimmykarily jimmykarily force-pushed the run-integration-locally-uroot branch from 7de3420 to 48fdd49 Compare April 24, 2024 13:40
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:40 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:40 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:40 — with GitHub Actions Inactive
@jimmykarily
Copy link
Contributor Author

Currently having trouble getting KVM to work on github runners (with container: archlinux:latest possibly complicating things)

@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:55 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:55 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 13:55 — with GitHub Actions Inactive
@jimmykarily jimmykarily force-pushed the run-integration-locally-uroot branch from 30403ba to 4abd2b1 Compare April 24, 2024 14:00
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:05 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:05 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:05 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:08 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:08 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:08 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:15 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:15 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:15 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:34 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:34 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 14:34 — with GitHub Actions Inactive
@jimmykarily
Copy link
Contributor Author

jimmykarily commented Apr 24, 2024

Currently having trouble getting KVM to work on github runners (with container: archlinux:latest possibly complicating things)

ok KVM problem finally solved (#4a71d2c). Now tests fail the same way locally and in CI.

The test that fails is this one: https://github.com/Foxboron/sbctl/pull/302/files#diff-ce25e955d527dcf8278d30d19452c97b5ff2b62824e2b60c02e0a90ba88e4596R20

and it expects the machine to be booted in secure boot mode. For this to work, we need to have the efirvars populated (at least PK enrolled). I don't fully understand how vmtest works yet but although we start with empty vars, I see this test succeeds which means something enrolled keys. Is it because of the order of tests here?

In any case, for secure boot to be enabled, we would need to reboot the VM. I'm not sure if that's possible with vmtest. @Foxboron maybe you can chime in at this point since you've used these libraries in go-uefi too. In the meantime I'll keep trying things.

@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 15:39 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 15:39 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 15:39 — with GitHub Actions Inactive
@jimmykarily
Copy link
Contributor Author

Currently having trouble getting KVM to work on github runners (with container: archlinux:latest possibly complicating things)

ok KVM problem finally solved (#4a71d2c). Now tests fail the same way locally and in CI.

The test that fails is this one: https://github.com/Foxboron/sbctl/pull/302/files#diff-ce25e955d527dcf8278d30d19452c97b5ff2b62824e2b60c02e0a90ba88e4596R20

and it expects the machine to be booted in secure boot mode. For this to work, we need to have the efirvars populated (at least PK enrolled). I don't fully understand how vmtest works yet but although we start with empty vars, I see this test succeeds which means something enrolled keys. Is it because of the order of tests here?

In any case, for secure boot to be enabled, we would need to reboot the VM. I'm not sure if that's possible with vmtest. @Foxboron maybe you can chime in at this point since you've used these libraries in go-uefi too. In the meantime I'll keep trying things.

It was the order of things. I just made sure the test re-uses the same vars from the previous test (the one that enrolled the keys) and it now works.

@Foxboron
Copy link
Owner

and it expects the machine to be booted in secure boot mode. For this to work, we need to have the efirvars populated (at least PK enrolled). I don't fully understand how vmtest works yet but although we start with empty vars, I see this test succeeds which means something enrolled keys. Is it because of the order of tests here?

The abstractions I wound up with in go-uefi isn't necessarily great. WithVM() is suppose to ensure we are using the same OVMF variables between each of the tests, and each t.Run() inside the test boots each of their own VM to ensure we have a clean slate.

@jimmykarily
Copy link
Contributor Author

I think it's fine for now. Small steps towards perfection :).

Also, I think the whole tests/utils directory can now be deleted, no? Maybe the make_image.sh and the /tests/kernel dir too?

@Foxboron
Copy link
Owner

Also, I think the whole tests/utils directory can now be deleted, no? Maybe the make_image.sh and the /tests/kernel dir too?

I think so yes, there shouldn't be anything important there now.

@jimmykarily jimmykarily marked this pull request as ready for review April 24, 2024 16:50
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 16:50 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 16:50 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 24, 2024 16:50 — with GitHub Actions Inactive
@Foxboron
Copy link
Owner

Cool, thanks for working on this.

It would be nice to squash together some of these commits into logical ones :)

I'll also try and fix the "deploy" thing on github so it doesn't spam the PRs.

Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
@jimmykarily jimmykarily force-pushed the run-integration-locally-uroot branch from e7d5257 to de2bd3c Compare April 25, 2024 08:12
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 25, 2024 08:12 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 25, 2024 08:12 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 25, 2024 08:12 — with GitHub Actions Inactive
@jimmykarily
Copy link
Contributor Author

Commits squashed to just 2

@Foxboron Foxboron merged commit ea6fa41 into Foxboron:master Apr 27, 2024
6 checks passed
@jimmykarily jimmykarily deleted the run-integration-locally-uroot branch April 29, 2024 06:33
@kerneis-anssi
Copy link

@Foxboron This PR broke my build, I think you need to run go mod tidy.

@jimmykarily
Copy link
Contributor Author

Strange, go mod tidy indeed makes some changes but otoh the building pipeline didn't fail:

https://github.com/Foxboron/sbctl/actions/runs/8829203645/job/24239720668

Building locally also doesn't fail. Sometimes I don't understand go :D .

@kerneis-anssi
Copy link

I also don't understand go :-D My build broke only because I did peculiar things, to be honest (I run go mod vendor before building offline, and it's the former step that fails).

@Foxboron
Copy link
Owner

Mm, I'll push a fix and maybe try and do a release soon'ish.

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.

3 participants