Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
snapd: generate snap cookies on startup #3491
Conversation
stolowski
added some commits
Jun 19, 2017
Some checks were not successful
| +func (m *SnapManager) GenerateCookies(st *state.State) error { | ||
| + var snapNames map[string]*json.RawMessage | ||
| + err := st.Get("snaps", &snapNames) | ||
| + if err != nil && err != state.ErrNoState { |
mvo5
Jun 20, 2017
Collaborator
This could be written in a single line: if err := ...; err != nil && err != state.ErrNoState {
| + } | ||
| + | ||
| + var contexts map[string]string | ||
| + err = st.Get("snap-cookies", &contexts) |
| + c.Assert(contexts, HasLen, 2) | ||
| + | ||
| + var cookie, snapName string | ||
| + for cookie, snapName = range contexts { |
mvo5
Jun 20, 2017
Collaborator
contexts is a map, no? So instead of searching, could you just do cookie := contexts["some-snap"]?
stolowski
Jun 20, 2017
Contributor
I couldn't quite do that since the map is indexed by cookie value.. However you made me realize I'm reading the cookie file a few lines below, so I can just reorder these checks a little bit and get rid of this loop. Thanks :)
| + echo "Simulate upgrade from old snapd with no cookie support" | ||
| + systemctl stop snapd.{service,socket} | ||
| + rm -rf $COOKIE_FILE | ||
| + jq -c 'del(.data["snap-cookies"])' /var/lib/snapd/state.json > /var/lib/snapd/state.json.new |
stolowski
added some commits
Jun 20, 2017
Some checks were not successful
Some checks were not successful
codecov-io
commented
Jun 23, 2017
•
Edited 1 time
-
codecov-io
Jul 5, 2017
-
Jul 5, 2017
Codecov Report
@@ Coverage Diff @@
## master #3491 +/- ##
==========================================
- Coverage 76.78% 76.78% -0.01%
==========================================
Files 379 379
Lines 26285 26304 +19
==========================================
+ Hits 20184 20198 +14
- Misses 4307 4310 +3
- Partials 1794 1796 +2
Continue to review full report at Codecov.
|
| + for snap := range snapNames { | ||
| + if _, ok := contexts[snap]; !ok { | ||
| + err := m.createSnapCookie(st, snap) | ||
| + if err != nil { |
mvo5
Jun 23, 2017
Collaborator
(nitpick) this could be a single line: if err := m.createSnapCookie(...; err != nil {
All checks have passed
| +// before the feature of running snapctl outside of hooks was introduced, leading to a warning | ||
| +// from snap-confine). | ||
| +func (m *SnapManager) GenerateCookies(st *state.State) error { | ||
| + var snapNames map[string]*json.RawMessage |
zyga
Jul 6, 2017
Contributor
Do we need to lock the state here? In case that this is caller's responsibility could we perhaps indicate that in the documentation?
| + echo "Cookie file $COOKIE_FILE is missing" | ||
| + exit 1 | ||
| + fi | ||
| + if [ $(stat -c%a $COOKIE_FILE) != "600" ]; then |
stolowski
added some commits
Jul 6, 2017
stolowski commentedJun 19, 2017
•
Edited 1 time
-
stolowski
Jun 19, 2017
Generate snap cookies for snaps that miss them on snapd startup. Missing cookie files cause a confusing warning from snap-confine.
Normally we generate cookies when installing snaps, but cookies will be missing for snaps already present in the system when upgrading to the new snapd that adds support for running snapctl outside of hooks.
This branch also adjust the error message from snap-confine to clearly state it's a warning, although with the snapd fix from this branch we should never see the warning again under normal conditions.