many: remove dm-verity support from snap pack #14869

sespiros · 2024-12-17T23:38:58Z

Under the new design, generating dm-verity data via snap pack is not needed as support for integrity data was simplified and there is no extra logic or separate header anymore.

Jira: https://warthogs.atlassian.net/browse/FR-9880

Under the new design, generating dm-verity data via `snap pack` is not needed as support for integrity data was simplified and there is no extra logic or separate header anymore.

codecov · 2024-12-17T23:57:04Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (master@e7a49b9). Learn more about missing BASE report.
Report is 15 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master   #14869   +/-   ##
=========================================
  Coverage          ?   78.28%           
=========================================
  Files             ?     1156           
  Lines             ?   153498           
  Branches          ?        0           
=========================================
  Hits              ?   120173           
  Misses            ?    25920           
  Partials          ?     7405

Flag	Coverage Δ
unittests	`78.28% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

alfonsosanchezbeato

Shouldn't also the files in snap/integrity be removed?

sespiros · 2025-01-08T10:56:57Z

Shouldn't also the files in snap/integrity be removed?

I didn't remove it in this one since that's the path I still use for the integrity API (see upcoming PRs such as #14872 which changes it almost completely). The snap integrity API (under snap/integrity) will be called by other various parts of snapd (overlord during snap installation, snap-bootstrap etc.).

pedronis

looks good, but one question

pedronis · 2025-01-08T15:22:38Z

snap/pack/pack.go

@@ -283,13 +280,6 @@ func mksquashfs(sourceDir, fName, snapType string, opts *Options) error {
 		return err
 	}

-	if opts.Integrity {
-		err := integrity.GenerateAndAppend(fName)


shouldn't at least GenerateAndAppend also go away? or is it modified in a follow up?

It will go away, see https://github.com/canonical/snapd/pull/14872/files#diff-f2b95ee6cb86d0d94e54d356707f39c2e6014beef108da27d124daf04d6eb5daL115

Instead of removing everything under snap/integrity in this one and reintroduce it in the follow-up, I left it as is but I drastically change it in the follow-ups.

pedronis

thanks

alfonsosanchezbeato

Thank you

Under the new design, generating dm-verity data via `snap pack` is not needed as support for integrity data was simplified and there is no extra logic or separate header anymore.

github-actions · 2025-01-23T11:35:04Z

Thu Jan 23 15:52:37 UTC 2025
The following results are from: https://github.com/canonical/snapd/actions/runs/12928893684

Failures:

Executing:

google-distro-1:debian-11-64:tests/main/cgroup-devices-v2

Restoring:

openstack:fedora-41-64:tests/main/core18-configure-hook
openstack:fedora-41-64:tests/main/refresh-mode-ignore-running
openstack:fedora-41-64:tests/main/
openstack:fedora-41-64
openstack:fedora-41-64:tests/main/
openstack:fedora-41-64
openstack:fedora-41-64:tests/main/snap-user-service-upgrade-failure
openstack:fedora-41-64:tests/main/
openstack:fedora-41-64

pedronis · 2025-01-23T11:46:21Z

@sespiros this spread test need changing or removing:

google:ubuntu-24.10-64:tests/main/snap-pack-integrity

…to snap-integrity-remove-pack * sespiros/snap-integrity-remove-pack: (86 commits) multiple: remove dm-verity support from snap pack asserts: snap integrity assertion (canonical#14870) overlord: cleanup some old edges i/builtin: make auditd-support grant seccomp setpriority (canonical#14940) tests: use quotation marks to support directories with spaces (canonical#14948) t/m/snap-service-install-mode: fix line being longer than expected interfaces/opengl: Enable parsing of nvidia driver information files (canonical#14893) i/b/fwupd: allow access to dell bios recovery (canonical#14920) tests: divide spread into fundamental/non-fundamental (canonical#14785) c/snap-bootstrap: refactor systemd-mount dm-verity/overlayfs options API (canonical#14790) o/snapstate: do not restart again for snapd along the undo path inside undoUnlinkCurrentSnap (canonical#14917) release: 2.67.1 tests: fix missing spread failures in PR comments (canonical#14931) i/prompting{,requestrules}: merge rules which have identical lifespans (canonical#14757) tests: skip apparmor-prompting-integration-tests in armhf (canonical#14919) cmd/snap-bootstrap: mount drivers tree if present (canonical#14522) i/p/patterns: disallow /./ and /../ in path patterns (canonical#14774) osutil/user: look up getent executable in known host directories (canonical#14792) overlord: wait for snapd restart after requesting by undo of 'link-snap' (canonical#14850) interfaces: update template with new syscalls (canonical#14861) ...

sespiros · 2025-01-23T12:14:52Z

@pedronis --append-integrity-data doesn't exist anymore so I removed the test.

@Meulengracht gh UI is confusing, I assume you merged master in my branch right? Somehow this was forced pushed in my tree, not sure how that happened, I thought only myself could write my tree :D Anyway what I did was fetch/merge my branch, added the new commit on top and pushed my branch. I didn't use --force so I assume it did the right thing.

Meulengracht · 2025-01-23T12:27:57Z

@pedronis --append-integrity-data doesn't exist anymore so I removed the test.

@Meulengracht gh UI is confusing, I assume you merged master in my branch right? Somehow this was forced pushed in my tree, not sure how that happened, I thought only myself could write my tree :D Anyway what I did was fetch/merge my branch, added the new commit on top and pushed my branch. I didn't use --force so I assume it did the right thing.

Correct, i updated your branch due to test changes, so I want them rerun with newest master, same for your other PR

olivercalder · 2025-01-24T21:50:21Z

This appears to have broken formatting in cmd/snap/cmd_pack.go, not sure how the static checks passed.

diff --git a/cmd/snap/cmd_pack.go b/cmd/snap/cmd_pack.go
index 8b093ad70e..bed9b1afe8 100644
--- a/cmd/snap/cmd_pack.go
+++ b/cmd/snap/cmd_pack.go
@@ -36,11 +36,10 @@ import (
 )
 
 type packCmd struct {
-       CheckSkeleton bool   `long:"check-skeleton"`
-       AppendVerity  bool   `long:"append-integrity-data" hidden:"yes"`
-       Filename      string `long:"filename"`
-       Compression   string `long:"compression"`
-       Positional    struct {
+       CheckSkeleton    bool   `long:"check-skeleton"`
+       Filename         string `long:"filename"`
+       Compression      string `long:"compression"`
+       Positional       struct {
                SnapDir   string `positional-arg-name:"<snap-dir>"`
                TargetDir string `positional-arg-name:"<target-dir>"`
        } `positional-args:"yes"`

multiple: remove dm-verity support from snap pack

a64d219

Under the new design, generating dm-verity data via `snap pack` is not needed as support for integrity data was simplified and there is no extra logic or separate header anymore.

sespiros mentioned this pull request Dec 17, 2024

many: Snap integrity discovery #14873

Draft

ndyer requested review from pedronis and alfonsosanchezbeato January 7, 2025 16:47

alfonsosanchezbeato reviewed Jan 7, 2025

View reviewed changes

pedronis reviewed Jan 8, 2025

View reviewed changes

pedronis changed the title ~~multiple: remove dm-verity support from snap pack~~ many: remove dm-verity support from snap pack Jan 8, 2025

pedronis approved these changes Jan 8, 2025

View reviewed changes

alfonsosanchezbeato approved these changes Jan 8, 2025

View reviewed changes

multiple: remove dm-verity support from snap pack

759bc4b

Under the new design, generating dm-verity data via `snap pack` is not needed as support for integrity data was simplified and there is no extra logic or separate header anymore.

Meulengracht force-pushed the snap-integrity-remove-pack branch from a64d219 to 759bc4b Compare January 23, 2025 09:37

sespiros added 2 commits January 23, 2025 14:11

t/main/snap-pack-integrity: remove snap-pack-integrity spread test

da50b55

Meulengracht merged commit 78f68f0 into canonical:master Jan 24, 2025
44 of 46 checks passed

olivercalder mentioned this pull request Jan 24, 2025

cmd/snap: fix formatting in packCmd struct #14981

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

many: remove dm-verity support from snap pack #14869

many: remove dm-verity support from snap pack #14869

sespiros commented Dec 17, 2024

codecov bot commented Dec 17, 2024 •

edited

Loading

alfonsosanchezbeato left a comment

sespiros commented Jan 8, 2025

pedronis left a comment

pedronis Jan 8, 2025

sespiros Jan 8, 2025

pedronis left a comment

alfonsosanchezbeato left a comment

github-actions bot commented Jan 23, 2025 •

edited

Loading

pedronis commented Jan 23, 2025

sespiros commented Jan 23, 2025

Meulengracht commented Jan 23, 2025

olivercalder commented Jan 24, 2025 •

edited

Loading

many: remove dm-verity support from snap pack #14869

many: remove dm-verity support from snap pack #14869

Conversation

sespiros commented Dec 17, 2024

codecov bot commented Dec 17, 2024 • edited Loading

Codecov Report

alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

sespiros commented Jan 8, 2025

pedronis left a comment

Choose a reason for hiding this comment

pedronis Jan 8, 2025

Choose a reason for hiding this comment

sespiros Jan 8, 2025

Choose a reason for hiding this comment

pedronis left a comment

Choose a reason for hiding this comment

alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 23, 2025 • edited Loading

Failures:

Executing:

Restoring:

pedronis commented Jan 23, 2025

sespiros commented Jan 23, 2025

Meulengracht commented Jan 23, 2025

olivercalder commented Jan 24, 2025 • edited Loading

codecov bot commented Dec 17, 2024 •

edited

Loading

github-actions bot commented Jan 23, 2025 •

edited

Loading

olivercalder commented Jan 24, 2025 •

edited

Loading