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

many: run component install hooks #13775

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

andrewphelpsj
Copy link
Collaborator

@andrewphelpsj andrewphelpsj commented Apr 1, 2024

This PR is the final piece that allows us to run install hooks for components. This change contains the snap-confine changes. The majority of changes in snap-confine deal with parsing security tags that are generated for component hooks.

An example of a security tag from a component hook would be: snap.name+comp.hook.install
And one with an instance key: snap.name_instance+comp.hook.install

Something important to note is how these are encoded as udev tags. Currently, when converting a security tag to a udev tag, we replace all . characters in the tag with _ characters because systemd limits udev tags to having only alphanumeric characters, with the addition of the characters - and _. Since security tags can now contain +
characters, those will be encoded as two consecutive _ characters.

For example:

"snap.name+comp.hook.install" -> "snap_name__comp_hook_install"
"snap.name_instance+comp.hook.install" -> "snap_name_instance__comp_hook_install"

This allows the conversion to maintain its reversibility.

@github-actions github-actions bot added the Needs Documentation -auto- Label automatically added which indicates the change needs documentation label Apr 1, 2024
@bboozzoo
Copy link
Collaborator

bboozzoo commented Apr 2, 2024

shellcheck appears to be unhappy:


In tests/main/component-hooks/comp-with-install-hook/meta/hooks/install line 5:
printf "GET /ip HTTP/1.0\r\n\r\n" | nc httpbin.org 80 2>&1 > "${SNAP_COMMON}/install-logs" 
                                                      ^--^ SC2069 (warning): To redirect stdout+stderr, 2>&1 must be last (or use '{ cmd > file; } 2>&1' to clarify).

@andrewphelpsj andrewphelpsj force-pushed the component-hooks-run branch 2 times, most recently from 5e4f87d to 236f2ea Compare April 2, 2024 19:59
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 66.99411% with 168 lines in your changes are missing coverage. Please review.

Project coverage is 78.73%. Comparing base (1bad859) to head (9678627).
Report is 3 commits behind head on master.

Files Patch % Lines
interfaces/ifacetest/backendtest.go 0.00% 64 Missing ⚠️
interfaces/ifacetest/app_set.go 28.57% 48 Missing and 2 partials ⚠️
interfaces/repo.go 61.70% 12 Missing and 6 partials ⚠️
cmd/snap/cmd_run.go 85.41% 9 Missing and 5 partials ⚠️
interfaces/connection.go 33.33% 8 Missing ⚠️
interfaces/snap_app_set.go 93.24% 4 Missing and 1 partial ⚠️
cmd/snap-exec/main.go 90.00% 2 Missing and 1 partial ⚠️
interfaces/kmod/backend.go 71.42% 0 Missing and 2 partials ⚠️
daemon/api_sideload_n_try.go 0.00% 1 Missing ⚠️
interfaces/builtin/modem_manager.go 50.00% 1 Missing ⚠️
... and 2 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13775      +/-   ##
==========================================
- Coverage   78.83%   78.73%   -0.11%     
==========================================
  Files        1043     1044       +1     
  Lines      134470   135082     +612     
==========================================
+ Hits       106012   106357     +345     
- Misses      21847    22076     +229     
- Partials     6611     6649      +38     
Flag Coverage Δ
unittests 78.73% <66.99%> (-0.11%) ⬇️

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.

@andrewphelpsj andrewphelpsj force-pushed the component-hooks-run branch 3 times, most recently from c43b233 to 429cad9 Compare April 8, 2024 17:50
@andrewphelpsj andrewphelpsj force-pushed the component-hooks-run branch 4 times, most recently from 035b839 to c5d6971 Compare April 17, 2024 15:22
@andrewphelpsj andrewphelpsj force-pushed the component-hooks-run branch 3 times, most recently from eb91528 to c9a9f92 Compare May 7, 2024 14:42
@andrewphelpsj andrewphelpsj force-pushed the component-hooks-run branch 2 times, most recently from 73fa729 to 6cd4807 Compare May 15, 2024 07:44
…urrent revision of a component for a snap revision
…resent snap hooks, component hooks, and apps

This commit doesn't need to be here, and things will work without it.
But things were getting a bit complicated in runSnapConfine with
arguments that represented different things based on what we were
running.
…er: update snap-confine to be able to handle security tags that come from component hooks

An example of a security tag from a component hook would be:
"snap.name+comp.hook.install"

And one with an instance key:
"snap.name_instance+comp.hook.install"

Something important to note is how these are encoded as udev tags.
Currently, when converting a security tag to a udev tag, we replace all
'.' characters in the tag with '_' characters because systemd limits
udev tags to having only alphanumeric characters, with the addition of
the characters '-' and '_'. Since security tags can now contain '+'
characters, those will be encoded as two consecutive '_' characters.

For example:
"snap.name+comp.hook.install" -> "snap_name__comp_hook_install"
"snap.name_instance+comp.hook.install" -> "snap_name_instance__comp_hook_install"

This allows the conversion to maintain its reversibility.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Documentation -auto- Label automatically added which indicates the change needs documentation
Projects
None yet
3 participants