snapd icon indicating copy to clipboard operation
snapd copied to clipboard

s/a/notify: implement protocol v5 with metadata tagging parser

Open olivercalder opened this issue 10 months ago • 2 comments

Implement version 5 of the apparmor notification protocol, as defined here: https://docs.google.com/document/d/1_WvEM9Qi2Je2Vwulzv5TzHwLJ8ld0W8gTNESZZd9VsY/edit?tab=t.0#heading=h.q2d8zdyargyy

This work is tracked internally by https://warthogs.atlassian.net/browse/SNAPDENG-32517

olivercalder avatar Feb 14 '25 23:02 olivercalder

Codecov Report

Attention: Patch coverage is 90.90909% with 14 lines in your changes missing coverage. Please review.

Project coverage is 78.10%. Comparing base (d6d95f0) to head (ba5b0cd). Report is 16 commits behind head on master.

Files with missing lines Patch % Lines
sandbox/apparmor/notify/message.go 80.28% 8 Missing and 6 partials :warning:
Additional details and impacted files
@@           Coverage Diff            @@
##           master   #15089    +/-   ##
========================================
  Coverage   78.09%   78.10%            
========================================
  Files        1190     1191     +1     
  Lines      158458   158867   +409     
========================================
+ Hits       123746   124079   +333     
- Misses      27017    27072    +55     
- Partials     7695     7716    +21     
Flag Coverage Δ
unittests 78.10% <90.90%> (+<0.01%) :arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Feb 15 '25 00:02 codecov[bot]

Mon Mar 17 22:15:40 UTC 2025 The following results are from: https://github.com/canonical/snapd/actions/runs/13866996541

Failures:

Preparing:

  • openstack:debian-sid-64:tests/main/
  • openstack:opensuse-tumbleweed-64
  • openstack:opensuse-tumbleweed-64
  • openstack:opensuse-tumbleweed-64
  • openstack:opensuse-tumbleweed-64

Executing:

  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-pre-download:close
  • openstack:opensuse-tumbleweed-64:tests/main/snap-refresh-hold
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-pre-download:restart
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-retry
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-pre-download:ignore
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-pre-download:close_mid_restart
  • openstack:opensuse-tumbleweed-64:tests/main/refresh-app-awareness-notify
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-gating
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh:parallel
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-backoff
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh:regular
  • openstack:opensuse-tumbleweed-64:tests/main/auto-refresh-gating-from-snap
  • google:ubuntu-25.04-64:tests/main/snap-ns-forward-compat
  • google:ubuntu-25.04-64:tests/main/snap-user-service-socket-activation
  • google:ubuntu-25.04-64:tests/main/microk8s-smoke:edge

Restoring:

  • openstack:debian-sid-64:tests/main/
  • openstack:opensuse-tumbleweed-64
  • openstack:opensuse-tumbleweed-64
  • openstack:opensuse-tumbleweed-64
  • openstack:opensuse-tumbleweed-64
  • openstack:opensuse-tumbleweed-64:tests/main/refresh-app-awareness-notify

github-actions[bot] avatar Feb 15 '25 01:02 github-actions[bot]

@olivercalder can you rebase it once the prerequisite lands please.

zyga avatar Feb 26 '25 09:02 zyga

@olivercalder shall I try to review without rebasing?

zyga avatar Mar 05 '25 12:03 zyga

Should methods on the stringPacker be public or private? The type is private and only used in the s/a/notify package, but the existing methods are all public, seems weird to have just packTagsets be private.

olivercalder avatar Mar 11 '25 23:03 olivercalder

Since JJ is planning to roll the restarts changes (formerly v7) into v5 as well, I'm going to mark v5 as manually disabled for now, in case we cut snapd 2.69 before the rest of the v7->v5 changes land, so snapd doesn't incorrectly report that it supports the full v5 with restarts. Then I can open new PRs for the restarts work, based on this PR.

olivercalder avatar Mar 14 '25 22:03 olivercalder

I've manually squashed all the commits except the last one which manually disables protocol v5. Please DO NOT SQUASH, just rebase merge, so that the v5 disable commit can be easily reverted.

olivercalder avatar Mar 14 '25 23:03 olivercalder