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

Import build uki command from enki #92

Merged
merged 29 commits into from
Nov 11, 2024

Conversation

jimmykarily
Copy link
Contributor

@jimmykarily jimmykarily commented Nov 6, 2024

This is just a copy-over from the enki code. Some minor changes were made so that no function expects a global viper object. They now take everything they need as function parameters.

Steps to deprecate the enki command:

  • Port the code to Auroraboot and make it work the same
  • Migrate whatever tests can be migrated
  • Merge
  • Refactor the code to make it cleaner and re-usable
  • Move any leftovers from enki to Auroraboot (because this PR still uses things from enki, like constants and functions).

When the rest of the enki commands have been migrated (genkey and sysext remaining), we can archive the enki repo to make sure no fixes are made there instead of Auroraboot.

@jimmykarily jimmykarily self-assigned this Nov 6, 2024
@jimmykarily
Copy link
Contributor Author

This is driving me crazy. I create an ISO with:

CGO_ENABLED=0 go build -ldflags "-X main.version=myversion" -o build/auroraboot && docker run --rm --privileged -v $PWD/unpacked:/unpacked -v $PWD/build/auroraboot:/bin/auroraboot -v $PWD/build:/result -v /home/dimitris/workspace/kairos/AuroraBoot/e2e/assets/keys:/home/dimitris/workspace/kairos/AuroraBoot/e2e/assets/keys quay.io/kairos/osbuilder-tools build-uki --output-dir /result -k /home/dimitris/workspace/kairos/AuroraBoot/e2e/assets/keys --output-type iso --single-efi-cmdline "My Entry: someoption=somevalue" --single-efi-cmdline "My Other Entry: someoption2=somevalue2" --single-efi-cmdline "Something Here Is Not Right: foo=bar" dir:/unpacked

(notice the --single-efi-cmdline arguments)

and in the ISO I find:

root@localhost:/home/kairos# find / -iname *.efi
/run/rootfsbase/EFI/BOOT/BOOTX64.EFI
/run/rootfsbase/EFI/kairos/norole.efi
/run/rootfsbase/EFI/kairos/my_entry.efi
/run/rootfsbase/EFI/kairos/My_Other_Entry.efi
/run/rootfsbase/EFI/kairos/Something_Here_Is_Not_Right.efi

I can't explain how my_entry ends up being lower cased. The others works just fine.

@jimmykarily
Copy link
Contributor Author

I printed the files as they are being copied with mcopy :

2024-11-07T09:29:12Z INF Copying files in the img file
************************* f = /tmp/enki-build-uki-3252635559/BOOTX64.EFI
************************* f = /tmp/enki-build-uki-3252635559/norole.efi
************************* f = /tmp/enki-build-uki-3252635559/My_Entry.efi
************************* f = /tmp/enki-build-uki-3252635559/My_Other_Entry.efi
************************* f = /tmp/enki-build-uki-3252635559/Something_Here_Is_Not_Right.efi
************************* f = /tmp/enki-build-uki-3252635559/loader.conf
************************* f = /tmp/enki-build-uki-3252635559/norole.conf
************************* f = /tmp/enki-build-uki-3252635559/My_Entry.conf
************************* f = /tmp/enki-build-uki-3252635559/My_Other_Entry.conf
************************* f = /tmp/enki-build-uki-3252635559/Something_Here_Is_Not_Right.conf
************************* f = /home/dimitris/workspace/kairos/AuroraBoot/e2e/assets/keys/PK.der
************************* f = /home/dimitris/workspace/kairos/AuroraBoot/e2e/assets/keys/KEK.der
************************* f = /home/dimitris/workspace/kairos/AuroraBoot/e2e/assets/keys/db.der
************************* f = /home/dimitris/workspace/kairos/AuroraBoot/e2e/assets/keys/PK.auth
************************* f = /home/dimitris/workspace/kairos/AuroraBoot/e2e/assets/keys/KEK.auth
************************* f = /home/dimitris/workspace/kairos/AuroraBoot/e2e/assets/keys/db.auth

The file name looks correct until the last minute. I wonder if it's the ISO filesystem somehow lying to me (not having filename case information on the the files or something).

@jimmykarily
Copy link
Contributor Author

Never mind, we decided to go lowercase with all files. Fat can't be trusted to maintain the case.

@jimmykarily jimmykarily force-pushed the import-build-uki-command-from-enki branch 3 times, most recently from 4b36937 to 820cbcd Compare November 8, 2024 08:46
e2e/genkey_test.go Outdated Show resolved Hide resolved
Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
instead of trying to re-use it. It was never meant to be re-used. Deep
functions where accessing top-level cli args using viper. We will:

- port the code
- make it work
- refactor it to be clean and re-usable
- deprecate the enki counterpart

Signed-off-by: Dimitris Karakasilis <[email protected]>
by having a default, the condition was never false

Signed-off-by: Dimitris Karakasilis <[email protected]>
because that's how urfave/cli works (positional args come last)

Signed-off-by: Dimitris Karakasilis <[email protected]>
because FAT can't be trusted with mixed case (some files are copied as
lowercase although they were upper case)

Also handle errors in place (instead of at the end)

Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
to remove the additional dependency to mkfs

Signed-off-by: Dimitris Karakasilis <[email protected]>
to run the bootable test

Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
because it's not set for some reason

Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
Signed-off-by: Dimitris Karakasilis <[email protected]>
@jimmykarily jimmykarily force-pushed the import-build-uki-command-from-enki branch from b966199 to 3901d83 Compare November 11, 2024 08:04
@jimmykarily jimmykarily requested a review from a team November 11, 2024 08:12
@jimmykarily jimmykarily marked this pull request as ready for review November 11, 2024 08:12
Copy link
Member

@Itxaka Itxaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. I guess the refactor of things wil come later? Seeing some things in the build-uki iso that may be better in a separate utils package or something similar as having a command being 1000 lines its a bit terrible xD

@jimmykarily
Copy link
Contributor Author

looks good. I guess the refactor of things wil come later? Seeing some things in the build-uki iso that may be better in a separate utils package or something similar as having a command being 1000 lines its a bit terrible xD

Yup, it's as terrible as it gets :D I didn't want to do the refactoring together with the porting because it would make the review of the refactoring part impossible. Let's first migrate the commands.

@jimmykarily jimmykarily merged commit 188b34f into main Nov 11, 2024
8 checks passed
@jimmykarily jimmykarily deleted the import-build-uki-command-from-enki branch November 11, 2024 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants