Merge remote-tracking branch 'upstream/master' into HEADfirsboot-early-basecheck

9 files changed, 130 insertions, 104 deletions

diff --git a/cmd/snap/cmd_debug_validate_seed_test.go b/cmd/snap/cmd_debug_validate_seed_test.go
index 951c72f21c..1effde85c7 100644
--- a/cmd/snap/cmd_debug_validate_seed_test.go
+++ b/cmd/snap/cmd_debug_validate_seed_test.go
@@ -47,3 +47,18 @@ snaps:
	_, err = snap.Parser(snap.Client()).ParseArgs([]string{"debug", "validate-seed", tmpf})
	c.Assert(err, ErrorMatches, "cannot read seed yaml: empty element in seed")
 }
+
+func (s *SnapSuite) TestDebugValidateSeedDuplicatedSnap(c *C) {
+	tmpf := filepath.Join(c.MkDir(), "seed.yaml")
+	err := ioutil.WriteFile(tmpf, []byte(`
+snaps:
+ - name: foo
+ file: foo.snap
+ - name: foo
+ file: bar.snap
+`), 0644)
+	c.Assert(err, IsNil)
+
+	_, err = snap.Parser(snap.Client()).ParseArgs([]string{"debug", "validate-seed", tmpf})
+	c.Assert(err, ErrorMatches, `cannot read seed yaml: snap name "foo" must be unique`)
+}
diff --git a/httputil/retry.go b/httputil/retry.go
index 1dde63b415..ae0227c33a 100644
--- a/httputil/retry.go
+++ b/httputil/retry.go
@@ -30,8 +30,6 @@ import (
	"syscall"
	"time"
 
-	"golang.org/x/net/http2"
-
	"gopkg.in/retry.v1"
 
	"github.com/snapcore/snapd/logger"
@@ -71,22 +69,33 @@ func ShouldRetryHttpResponse(attempt *retry.Attempt, resp *http.Response) bool {
	return resp.StatusCode >= 500
 }
 
+// isHttp2ProtocolError returns true if the given error is a http2
+// stream error with code 0x1 (PROTOCOL_ERROR).
+//
+// Unfortunately it seems this is not easy to detect. In e3be142 this
+// code tried to be smart and detect this via http2.StreamError but it
+// seems like with the h2_bundle.go in the go distro this does not
+// work, i.e. in https://travis-ci.org/snapcore/snapd/jobs/575471665
+// we still got protocol errors even with this detection code.
+//
+// So this code falls back to simple and naive detection.
+func isHttp2ProtocolError(err error) bool {
+	if strings.Contains(err.Error(), "PROTOCOL_ERROR") {
+	return true
+	}
+	// here is what a protocol error may look like:
+	// "DEBUG: Not retrying: http.http2StreamError{StreamID:0x1, Code:0x1, Cause:error(nil)}"
+	if strings.Contains(err.Error(), "http2StreamError") && strings.Contains(err.Error(), "Code:0x1,") {
+	return true
+	}
+	return false
+}
+
 func ShouldRetryError(attempt *retry.Attempt, err error) bool {
	if !attempt.More() {
	return false
	}
	if urlErr, ok := err.(*url.Error); ok {
-	// we see this from http2 downloads sometimes - it is unclear what
-	// is causing it but https://github.com/golang/go/issues/29125
-	// indicates a retry might be enough. Note that we get the
-	// PROTOCOL_ERROR *from* the remote side (fastly it seems)
-	netErr := urlErr.Err
-	if http2StreamErr, ok := netErr.(http2.StreamError); ok {
-	if http2StreamErr.Code == http2.ErrCodeProtocol {
-	logger.Debugf("Retrying because of: %s", err)
-	return true
-	}
-	}
	err = urlErr.Err
	}
	if netErr, ok := err.(net.Error); ok {
@@ -140,11 +149,20 @@ func ShouldRetryError(attempt *retry.Attempt, err error) bool {
	logger.Debugf("Encountered non temporary net.OpError: %#v", opErr)
	}
 
-	if err == io.ErrUnexpectedEOF || err == io.EOF {
+	// we see this from http2 downloads sometimes - it is unclear what
+	// is causing it but https://github.com/golang/go/issues/29125
+	// indicates a retry might be enough. Note that we get the
+	// PROTOCOL_ERROR *from* the remote side (fastly it seems)
+	if isHttp2ProtocolError(err) {
	logger.Debugf("Retrying because of: %s", err)
	return true
	}
 
+	if err == io.ErrUnexpectedEOF || err == io.EOF {
+	logger.Debugf("Retrying because of: %s (%s)", err, err)
+	return true
+	}
+
	if osutil.GetenvBool("SNAPD_DEBUG") {
	logger.Debugf("Not retrying: %#v", err)
	}
diff --git a/httputil/retry_test.go b/httputil/retry_test.go
index 51d729f64f..ec30de10a3 100644
--- a/httputil/retry_test.go
+++ b/httputil/retry_test.go
@@ -30,8 +30,6 @@ import (
	"sync"
	"time"
 
-	"golang.org/x/net/http2"
-
	. "gopkg.in/check.v1"
	"gopkg.in/retry.v1"
 
@@ -495,7 +493,7 @@ func (s *retrySuite) TestRetryOnHttp2ProtocolErrors(c *C) {
	return nil, &url.Error{
	Op: "Get",
	URL: "http://...",
-	Err: http2.StreamError{Code: http2.ErrCodeProtocol},
+	Err: fmt.Errorf("http.http2StreamError{StreamID:0x1, Code:0x1, Cause:error(nil)}"),
	}
	}
	readResponseBody := func(resp *http.Response) error {
diff --git a/packaging/fedora/snapd.spec b/packaging/fedora/snapd.spec
index 3a6ff2e380..6dbb124204 100644
--- a/packaging/fedora/snapd.spec
+++ b/packaging/fedora/snapd.spec
@@ -162,7 +162,6 @@ BuildRequires: golang(golang.org/x/crypto/openpgp/armor)
 BuildRequires: golang(golang.org/x/crypto/openpgp/packet)
 BuildRequires: golang(golang.org/x/crypto/sha3)
 BuildRequires: golang(golang.org/x/crypto/ssh/terminal)
-BuildRequires: golang(golang.org/x/net/http2)
 BuildRequires: golang(gopkg.in/check.v1)
 BuildRequires: golang(gopkg.in/macaroon.v1)
 BuildRequires: golang(gopkg.in/mgo.v2/bson)
diff --git a/snap/seed_yaml.go b/snap/seed_yaml.go
index cde82af5a0..3e95ba9bad 100644
--- a/snap/seed_yaml.go
+++ b/snap/seed_yaml.go
@@ -71,6 +71,7 @@ func ReadSeedYaml(fn string) (*Seed, error) {
	return nil, fmt.Errorf("%s: cannot unmarshal %q: %s", errPrefix, yamlData, err)
	}
 
+	seenNames := make(map[string]bool, len(seed.Snaps))
	// validate
	for _, sn := range seed.Snaps {
	if sn == nil {
@@ -90,6 +91,12 @@ func ReadSeedYaml(fn string) (*Seed, error) {
	if strings.Contains(sn.File, "/") {
	return nil, fmt.Errorf("%s: %q must be a filename, not a path", errPrefix, sn.File)
	}
+
+	// make sure names and file names are unique
+	if seenNames[sn.Name] {
+	return nil, fmt.Errorf("%s: snap name %q must be unique", errPrefix, sn.Name)
+	}
+	seenNames[sn.Name] = true
	}
 
	return &seed, nil
diff --git a/snap/seed_yaml_test.go b/snap/seed_yaml_test.go
index b067663357..e9651e476a 100644
--- a/snap/seed_yaml_test.go
+++ b/snap/seed_yaml_test.go
@@ -82,6 +82,23 @@ func (s *seedYamlTestSuite) TestNoPathAllowed(c *C) {
	c.Assert(err, ErrorMatches, `cannot read seed yaml: "foo/bar.snap" must be a filename, not a path`)
 }
 
+func (s *seedYamlTestSuite) TestDuplicatedSnapName(c *C) {
+	fn := filepath.Join(c.MkDir(), "seed.yaml")
+	err := ioutil.WriteFile(fn, []byte(`
+snaps:
+ - name: foo
+ channel: stable
+ file: foo_1.0_all.snap
+ - name: foo
+ channel: edge
+ file: bar_1.0_all.snap
+`), 0644)
+	c.Assert(err, IsNil)
+
+	_, err = snap.ReadSeedYaml(fn)
+	c.Assert(err, ErrorMatches, `cannot read seed yaml: snap name "foo" must be unique`)
+}
+
 func (s *seedYamlTestSuite) TestValidateChannelUnhappy(c *C) {
	fn := filepath.Join(c.MkDir(), "seed.yaml")
	err := ioutil.WriteFile(fn, []byte(`
diff --git a/tests/lib/bin/user-tool b/tests/lib/bin/user-tool
index a2b1b98f88..c291f8af70 100755
--- a/tests/lib/bin/user-tool
+++ b/tests/lib/bin/user-tool
@@ -1,42 +1,55 @@
-#!/bin/bash
+#!/usr/bin/env any-python
+from __future__ import print_function, absolute_import, unicode_literals
 
-set -e
+import argparse
+import os
+import subprocess
 
-# remove_user_with_group remove the given username from the system and also
-# cleans the group
-remove_user_with_group() {
- REMOVE_USER="$1"
+# Define MYPY as False and use it as a conditional for typing import. Despite
+# this declaration mypy will really treat MYPY as True when type-checking.
+# This is required so that we can import typing on Python 2.x without the
+# typing module installed. For more details see:
+# https://mypy.readthedocs.io/en/latest/common_issues.html#import-cycles
+MYPY = False
+if MYPY:
+ from typing import Text
 
- if [ -z "$REMOVE_USER" ]; then
- echo "please provide a username to remove"
- return 1
- fi
 
- if [ -e /var/lib/extrausers/passwd ]; then
- userdel --extrausers --force "${REMOVE_USER}" || true
- else
- userdel --force --remove "${REMOVE_USER}" || true
- # some systems do not set "USERGROUPS_ENAB yes" so we need to cleanup
+def remove_user_with_group(user_name):
+ # type: (Text) -> None
+ """remove the user and group with the same name, if present."""
+ if os.path.exists("/var/lib/extrausers/passwd"):
+ subprocess.call(["userdel", "--extrausers", "--force", user_name])
+ else:
+ subprocess.call(["userdel", "--force", "--remove", user_name])
+ # Some systems do not set "USERGROUPS_ENAB yes" so we need to cleanup
 # the group manually. Use "-f" (force) when available, older versions
 # do not have it.
- if groupdel -h | grep force; then
- groupdel -f "${REMOVE_USER}" || true
- else
- groupdel "${REMOVE_USER}" || true
- fi
- fi
- # ensure the ${REMOVE_USER} user really got deleted 
- not getent passwd "${REMOVE_USER}"
- not getent group "${REMOVE_USER}"
-}
-
-# main()
-case "$1" in
- remove-with-group)
- remove_user_with_group "$2"
- ;;
- *)
- echo "unknown argument $1"
- echo "look at the source of $0 to see what is supported (sry!)"
- exit 1
-esac
+ proc = subprocess.Popen(["groupdel", "-h"], stdout=subprocess.PIPE)
+ out, _ = proc.communicate()
+ if b"force" in out:
+ subprocess.call(["groupdel", "-f", user_name])
+ else:
+ subprocess.call(["groupdel", user_name])
+ # Ensure the user user really got deleted
+ if subprocess.call(["getent", "passwd", user_name]) == 0:
+ raise SystemExit("user exists after removal?")
+ if subprocess.call(["getent", "group", user_name]) == 0:
+ raise SystemExit("group exists after removal?")
+
+
+def main():
+ # type: () -> None
+ parser = argparse.ArgumentParser()
+ sub = parser.add_subparsers()
+ cmd = sub.add_parser(
+ "remove-with-group", description="Remove system user and group, if present"
+ )
+ cmd.set_defaults(func=lambda ns: remove_user_with_group(ns.user))
+ cmd.add_argument("user", help="name of the user and group to remove")
+ ns = parser.parse_args()
+ ns.func(ns)
+
+
+if __name__ == "__main__":
+ main()
diff --git a/tests/main/interfaces-fuse_support/task.yaml b/tests/main/interfaces-fuse_support/task.yaml
index 527d02a150..f7b8d18926 100644
--- a/tests/main/interfaces-fuse_support/task.yaml
+++ b/tests/main/interfaces-fuse_support/task.yaml
@@ -24,6 +24,10 @@ environment:
 NAME/parallel: test-snapd-fuse-consumer_foo
 
 prepare: |
+ if not mountinfo-tool /sys/fs/fuse/connections .fs_type=fusectl; then
+ touch please-unmount-fuse-connections
+ fi
+
 if [[ "$SPREAD_VARIANT" == "parallel" ]]; then
 snap set system experimental.parallel-instances=true
 fi
@@ -45,8 +49,11 @@ restore: |
 if [[ "$SPREAD_VARIANT" == "parallel" ]]; then
 snap set system experimental.parallel-instances=null
 fi
- if mountinfo-tool /sys/fs/fuse/connections .fs_type=fusectl; then
- umount /sys/fs/fuse/connections
+ if [ -e please-unmount-fuse-connections ]; then
+ if mountinfo-tool /sys/fs/fuse/connections .fs_type=fusectl; then
+ umount /sys/fs/fuse/connections
+ fi
+ rm -f please-unmount-fuse-connections
 fi
 
 execute: |
diff --git a/vendor/vendor.json b/vendor/vendor.json
index 214d138be1..03f51c14d4 100644
--- a/vendor/vendor.json
+++ b/vendor/vendor.json
@@ -165,54 +165,6 @@
	"revisionTime": "2017-06-28T23:42:41Z"
	},
	{
-	"checksumSHA1": "pCY4YtdNKVBYRbNvODjx8hj0hIs=",
-	"path": "golang.org/x/net/http/httpguts",
-	"revision": "74dc4d7220e7acc4e100824340f3e66577424772",
-	"revisionTime": "2019-08-11T06:12:18Z"
-	},
-	{
-	"checksumSHA1": "35XpM1XtbNTQ3qYTaxBOOmR9Y6Q=",
-	"path": "golang.org/x/net/http2",
-	"revision": "74dc4d7220e7acc4e100824340f3e66577424772",
-	"revisionTime": "2019-08-11T06:12:18Z"
-	},
-	{
-	"checksumSHA1": "VJwSx33rjMC7O6K2O50Jw6o1vw4=",
-	"path": "golang.org/x/net/http2/hpack",
-	"revision": "74dc4d7220e7acc4e100824340f3e66577424772",
-	"revisionTime": "2019-08-11T06:12:18Z"
-	},
-	{
-	"checksumSHA1": "vL6l4FZWitsxht0uqA/GpDNkNNc=",
-	"path": "golang.org/x/net/idna",
-	"revision": "74dc4d7220e7acc4e100824340f3e66577424772",
-	"revisionTime": "2019-08-11T06:12:18Z"
-	},
-	{
-	"checksumSHA1": "CbpjEkkOeh0fdM/V8xKDdI0AA88=",
-	"path": "golang.org/x/text/secure/bidirule",
-	"revision": "342b2e1fbaa52c93f31447ad2c6abc048c63e475",
-	"revisionTime": "2018-12-15T17:52:45Z"
-	},
-	{
-	"checksumSHA1": "R9iBDY+aPnT+8pyRcqGjXq5QixA=",
-	"path": "golang.org/x/text/transform",
-	"revision": "342b2e1fbaa52c93f31447ad2c6abc048c63e475",
-	"revisionTime": "2018-12-15T17:52:45Z"
-	},
-	{
-	"checksumSHA1": "vv9EDuekZgHxFbh+0jJhB7jLZXY=",
-	"path": "golang.org/x/text/unicode/bidi",
-	"revision": "342b2e1fbaa52c93f31447ad2c6abc048c63e475",
-	"revisionTime": "2018-12-15T17:52:45Z"
-	},
-	{
-	"checksumSHA1": "8fzd6fnxrstdNSVg+Srhf1nKgek=",
-	"path": "golang.org/x/text/unicode/norm",
-	"revision": "342b2e1fbaa52c93f31447ad2c6abc048c63e475",
-	"revisionTime": "2018-12-15T17:52:45Z"
-	},
-	{
	"checksumSHA1": "9gypWnVZJEaH3jMK9KqOp4xgQD4=",
	"path": "gopkg.in/check.v1",
	"revision": "788fd78401277ebd861206a03c884797c6ec5541",