* notices: Ensure that two notices don't have the same lastDateTime
When a client wants to receive the notices using the /v2/notices
interface, it usually will first receive all the old
notifications, store the date and time of the last notification,
and in the next calls use that date and time plus 1 nanosecond
to avoid geting the same notices over and over again.
Unfortunately, if, due to chance, two notifications have the
same date and time, this scheme will fail, because after
receiving the first one, the call will return it, and when the
client asks for the next, it will filter from the date/time plus
one microsecond, so the second notice won't be sent.
This patch fixes this by ensuring that no notice has the same
date/time value for lastReceived, by adding one or more
nanoseconds when required.
* Implement time.Time.Compare()
time.Time.Compare() was added in Go 1.20, but snapd uses Go 1.18
so this method isn't available.
* Added tests for CompareDate()
* Simplified time.Time.Compare() function
* Update overlord/state/notices.go
Co-authored-by: Zeyad Yasser <zeyady98@gmail.com>
* Changes requested by Zeyad
* Fix tests
* Remove unneeded MockGetTimeNow
* Changed timestamp name, and fixed test
* Replace all notice-last-date with notice-last-timestamp
* fix typo
* Remove unneeded GetLastOccurred function
* Change noticeLastTimestamp with lastNoticeTimestamp
* Fix tests
* Add a comment explaining why this is important
* Add missing space
---------
Co-authored-by: Zeyad Yasser <zeyady98@gmail.com>
* boot: added function to set EFI variables
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: renamed trustedShimFallbackBinary to seedShimPath
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: refactored setting EFI boot variables at install
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: adjusted variable names and fixed variable initialization
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: improve setting Boot#### EFI variable
Notably, splits off the process of reading a Boot#### variable and
extracting its DevicePath into its own function `readBootVariable` which
can be mocked and otherwise simplifies the `setBootNumberVariable`
function.
Also, fixes behavior around the final BootFFFF variable. Previously, it
was not possible to select the BootFFFF variable if it was unused, due
to overflow concerns on uint16. Now, the behavior around BootFFFF is
identical to that of any other boot variable, by using an int internally
instead of uint16, which also allows a more robust check for whether
there were no matching variables.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: added unit tests for setting EFI Boot#### variable
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: refactored setting EFI boot variables
Rewrote EFI boot variable functions to more closely match the behavior
of shim fallback: https://github.com/rhboot/shim/blob/main/fallback.c
In particular, the following have changed:
1. Existing Boot#### variables must fully match the new load option to
be considered a match. In particular, the load option attributes,
label, and device path must all be byte-for-byte identical.
Previously, only the device paths were compared.
2. Matching Boot#### variables are no longer overwritten. Since the
variable data must now byte-for-byte match the new load option, there
is no need to overwrite the existing variable.
3. Since existing Boot#### variables are no longer overwritten, the
variable attributes are no longer checked for those variables.
Instead, it is assumed that the Boot#### variable attributes are
viable for it to be used as a boot option. This matches the behavior
of `rhboot/shim/fallback.c`, for better or for worse.
4. When modifying the BootOrder variable, boot option numbers are no
longer pruned if there is no matching Boot#### variable.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot,bootloader: introduce UefiBootloader to build EFI load options
Previously, the path of the shim binary relative to the EFI partition
was passed into `SetEfiBootVariables`. However, different bootloaders
may wish to set up `OptionalData` in the load option.
Additionally, not all `TrustedAssetBootloaders` will attempt to set
EFI boot variables, and not all bootloaders which should set EFI boot
variables necessarily support secure boot. Thus, these should be
decoupled.
This commit adds a new `UefiBootloader` interface with the
`ConstructShimEfiLoadOption` method, which builds an EFI load option
from the shim path for the given bootloader.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot,bootloader: fixed linting errors and improved EFI boot variable test clarity
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
bootloader: improved unit test for grub EFI load option creation
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: set EFI boot variables in `MakeRunnableSystem`
Previously, attempted to set boot variables in
`MakeRecoverySystemBootable`, which is called by `MakeBootableImage`,
which is called when building the image file, rather than during install
mode.
`MakeRunnableSystem` is called on first boot during install mode, and
thus should be responsible for setting EFI boot variables.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: use seed bootloader when setting EFI variables
In install mode, the bootloader located in ubuntu-seed should be used
when setting the EFI boot variables. Previously, the bootloader in
ubuntu-boot was accidentally re-used.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
tests: added simple test to execute setefibootvar.go code
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
tests: fixed standalone set EFI vars code test to work with different layouts
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
tests: moved simple setefibootvar.go check to nested test
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
tests: added check for idempotence when setting EFI boot variables
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
bootloader: adjust comments, organization, and add TODO
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot,bootloader: fix setting EFI boot variables
Make function to search for EFI asset device path and construct load
option common so each UefiBootloader does not have to re-implement it.
Instead, the bootloader returns the description, asset file path, and
optional data, which can then be used to create the EFI load option.
Also, in `makeRunnableSystem`, the bootloader in ubuntu-seed must have
`NoSlashBoot` in order to correctly find the grub.cfg file and thus the
grub bootloader. This commit fixes this bug, and refactors a bit to
account for the changes in responsibilities between the bootloader and
the setefibootvars.go code.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
bootloader: fixed grub EFI load option test with tmp rootdir
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
go.mod: move golang.org/x/text import next to other golang.org/x/ imports
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: adjust opts to look for recovery bootloader when setting EFI variables
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: do not overwrite BootOrder if unchanged, and unexport EFI variable helper functions
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: unexport `setEfiBootOrderVariable`
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: move code to detect bootloader and set EFI variables accordingly into dedicated function
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: unexport `setUbuntuSeedEfiBootVariables` and accompanying error
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot,bootloader: ensure nil optionalData for EFI variable is equivalent to 0-length slice
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: handle empty boot order and other boot var improvements
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
boot: make setefibootvars functions linux-only
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* tests: add nested spread test for setting EFI vars
The test checks that EFI boot variables exist for the following:
1. A Boot#### variable pointing to the shim file path.
2. A BootOrder variable with the #### from the above Boot#### as first.
Since the layout of EFI assets is dependent on the gadget snap, the test
downloads and unpacks the gadget, then modifies the contents so that one
variant has the shim and grub binaries in `EFI/boot/` and another
variant has the shim and grub binaries in `EFI/ubuntu/` and the fallback
binary in `EFI/boot/`.
After building a core image around that modified gadget, the VM is
booted and the test checks that the EFI variables are set correctly.
Then, the test modifies the gadget to match the other variant's initial
layout, and then installs the newly modified gadget. This should trigger
re-setting EFI boot variables as well.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
tests: fix problems in spread test for setting EFI boot variables
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
tests: disabled TPM on EFI boot vars test and separated gadget script
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
tests: fixed EFI vars test to use correct toolbox and include all EFI assets
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
tests: modify-gadget.sh re-use existing gadget so edition is incremented
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
tests: fix mangled EFI var search string and other improvements
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
tests: polish tests for setting EFI boot variables
Notably, allow tests/nested/core/core20-set-efi-boot-variables to run on
arm64 as well as amd64, simplify setefivars.go to search for multiple
assets on multiple architectures, and allow
tests/nested/manual/core20-set-efi-boot-vars to run on any ubuntu-2*.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* bootloader/grub.go: only consider new shim asset in boot entry for now
* tests/nested/core/core20-set-efi-boot-variables: fix details
* boot: update uefi variables on gadget update
* tests/nested/manual/core20-set-efi-boot-vars: work-around file not deleted
* tests/nested/manual/core20-set-efi-boot-vars: use fb.efi like other tests
* tests/nested/manual/core20-set-efi-boot-vars: drop use of toolbox snap
* tests/nested/manual/core20-set-efi-boot-vars: drop work-around for not deleted files
* tests/nested/manual/core20-set-efi-boot-vars: verify install does add a boot entry
* tests/nested/manual/core20-set-efi-boot-vars: run only on version that have UC
* tests/nested/manual/core20-set-efi-boot-vars: obey GADGET_CHANNEL
* tests/nested/manual/core20-set-efi-boot-vars: move get_boot_entry.py to libs
* tests/nested/manual/core20-set-efi-boot-vars: factorize copy of variables
... so we can reuse the script in other tests
* tests/nested/core/core20-set-efi-boot-variables: stop using toolbox snap
* tests/nested/core/core20-set-efi-boot-variables: only run on versions with UC available
* overlord/devicestate: test using EfiLoadOptionParameters
* boot: test that variables are set
* boot: test observers' UpdateBootEntry
* tests/nested/manual/core20-set-efi-boot-vars: also test without secure boot
* many: use trusted install observer when UEFI variables are supported
* boot/makebootable.go: rename sealer to observer
* boot/grub.go: fix function name in doc
* cmd/snap-bootstrap: verify that ObserveExistingTrustedRecoveryAssets is called
* boot: add tests for SetEfiBootVariables
* many: comment on calls to ObserveExistingTrustedRecoveryAssets
---------
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Co-authored-by: Oliver Calder <oliver.calder@canonical.com>
Build drivers tree on installation so kernel modules can be found on first boot after installation (otherwise the modules would be available too late, after initial installation of the kernel has happened while seeding).
Also, make sure that the tree is not re-created when seeding, and add a spread test for kernel-modules components.
* dirs: add function to get root of drivers trees
* kernel: do not recreate drivers tree if it already exists
On installation, a drivers tree has already been copied to the data
partition. Do not re-create if found when seeding happens (note though
that on preseeding we will not find the tree and we will still create
it).
* gadget/install: create device tree on installations
Create a device tree on installation so we have it on first boot.
* o/snapstate: fix task message
* tests/nested/manual: add kernel-modules-components test
This test installs a kernel-modules component and verifies that the
shipped kernel module is installed and can be loaded.
* tests/lib: do not error out if cloud-init finished with
'recoverable error' status.
* kernel: have destination as argument when building drivers tree
We need to tackle the generic installation case as opposed to creating
the drivers tree when seeding the system. In that case the destination
tree can vary depending on installation type.
* overlord: pass kernel information to install package
so the drivers tree can be created.
* cmd/snap-bootstrap: build drivers tree on installation from initramfs
* tests: adapt to additional parameters in install.Run
* snap/naming: add ancillary method to split a full component name
into snap and component names.
* gadget/install: fix nosecboot tests
go test -race complains about data race, which isn't entirely accurate as the
goroutine which may modify the data has exited by the time checks are done.
However, should we want to enable -race as part of the test suite, this fixes a
blocker.
```
==================
WARNING: DATA RACE
Read at 0x00c0000123a8 by goroutine 16:
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch_test.(*triggerwatchSuite).TestNoDevsWaitKeyTimeout()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch_test.go:118 +0x3e5
runtime.call16()
/usr/lib/go/src/runtime/asm_amd64.s:770 +0x42
reflect.Value.Call()
/usr/lib/go/src/reflect/value.go:380 +0xb5
gopkg.in/check%2ev1.(*suiteRunner).forkTest.func1()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:775 +0x9c5
gopkg.in/check%2ev1.(*suiteRunner).forkCall.func1()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:669 +0xe9
Previous write at 0x00c0000123a8 by goroutine 18:
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch_test.(*mockTriggerDevice).WaitForTrigger()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch_test.go:48 +0x44
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch.Wait.gowrap2()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch.go:112 +0x50
Goroutine 16 (running) created at:
gopkg.in/check%2ev1.(*suiteRunner).forkCall()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:666 +0x5ba
gopkg.in/check%2ev1.(*suiteRunner).forkTest()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:757 +0x155
gopkg.in/check%2ev1.(*suiteRunner).runTest()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:812 +0x419
gopkg.in/check%2ev1.(*suiteRunner).run()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:618 +0x3c6
gopkg.in/check%2ev1.Run()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/run.go:92 +0x44
gopkg.in/check%2ev1.RunAll()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/run.go:84 +0x124
gopkg.in/check%2ev1.TestingT()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/run.go:72 +0x5d3
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch_test.Test()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch_test.go:35 +0x26
testing.tRunner()
/usr/lib/go/src/testing/testing.go:1689 +0x21e
testing.(*T).Run.gowrap1()
/usr/lib/go/src/testing/testing.go:1742 +0x44
Goroutine 18 (finished) created at:
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch.Wait()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch.go:112 +0x94b
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch_test.(*triggerwatchSuite).TestNoDevsWaitKeyTimeout()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch_test.go:115 +0x21b
runtime.call16()
/usr/lib/go/src/runtime/asm_amd64.s:770 +0x42
reflect.Value.Call()
/usr/lib/go/src/reflect/value.go:380 +0xb5
gopkg.in/check%2ev1.(*suiteRunner).forkTest.func1()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:775 +0x9c5
gopkg.in/check%2ev1.(*suiteRunner).forkCall.func1()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:669 +0xe9
==================
```
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* c/snap-bootstrap: lift requirement for fde-setup hook for single-boot-install
* tests/nested/manual/uc20-install-in-initrd: test various systems that support single-boot install
* c/snap-bootstrap: add test for single-boot install without a fde-setup hook
* tests/nested/manual/uc20-install-in-initrd: use "not" rather than "!" in spread test
Some invocations to external programs used exec.CombinedOutput, that
combines stdout and strerr into a single byte array. This can be an
issue if this output is parsed, as many programs print debug output or
warnings to stderr and that data is unexpected by the parsers. This
patch changes to using osutil.RunSplitOutput or osutil.RunCmd (that
return separately stdout and stderr) when we need to parse stdout, and
also in some other cases when printing separately both streams could
be helpful. Fixes LP #1885597.
Replace ioutil.WriteFile with os.WriteFile since the former has been
deprecated since go1.16 and simply calls the latter.
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
We got asked to print the version information of snap-bootstrap
during the boot to help the enablement/field team to know if
they are using the right version of snap-bootstrap.
This is a small commit that archives this. It uses the logger
so quiet boot will also be honored.
Go 1.19 includes some changes to gofmt which intend to make lists and
heading clearer when rendered (https://go.dev/doc/go1.19). This commit
is the result of running the new gofmt and manually fixing some of it.
This was necessary because the new gofmt assumed lines beginning w/ tabs
to start lists or examples. While this is often true in our codebase,
we occasionally also use tabs to indent the lines after a TODO or FIXME
prefix or in yaml (e.g., excerpts of a snap.yaml). This meant that a lot of the
reformatted comments were broken and had to be fixed manually.
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
In case a bad try kernel was in the disk (i.e. by just a simple
dangling symlink try-kernel.efi), we entered a boot loop. Avoid that.
* tests: add test that checks that there is no boot loop
when we have a dangling try-kernel.efi symlink.
* boot: return status when asking for kernel revisions
Return status when asking for kernel revisions even if there is no try
kernel. It will still be useful. Also, add some clarifying comments to
genericInitramfsSelectSnap.
* boot: clarify comments and traces
* boot: add test for bad try status with no try kernel
* cmd/snap-bootstrap: adapt error string in tests
Due to changes in bootstate.
