cmd/snap: return empty document if snap has no configuration

cmd/snap/cmd_get.go

@@ -163,42 +163,48 @@ func (x *cmdGet) Execute(args []string) error {
 	}
 
 	isTerminal := terminal.IsTerminal(int(os.Stdin.Fd()))
-
+	rootRequested := len(confKeys) == 0
 	var confToPrint interface{} = conf
-	if !x.Document && !x.List && len(confKeys) == 1 {
-		// if single key was requested, then just output the value unless it's a map,
-		// in which case it will be printed as a list below.
-		if _, ok := conf[confKeys[0]].(map[string]interface{}); !ok {
-			confToPrint = conf[confKeys[0]]
+
+	if rootRequested && len(conf) == 0 {
+		if !x.Document {
+			return fmt.Errorf("snap %q has no configuration", snapName)
 		}
-	}
+	} else {
+		if !x.Document && !x.List && len(confKeys) == 1 {
+			// if single key was requested, then just output the value unless it's a map,
+			// in which case it will be printed as a list below.
+			if _, ok := conf[confKeys[0]].(map[string]interface{}); !ok {
+				confToPrint = conf[confKeys[0]]
+			}
+		}
+
+		if cfg, ok := confToPrint.(map[string]interface{}); ok && !x.Document {
+			// TODO: remove this conditional and the warning below after a transition period.
+			if isTerminal || x.List {
+				w := tabWriter()
+				defer w.Flush()
 
-	if cfg, ok := confToPrint.(map[string]interface{}); ok && !x.Document {
-		// TODO: remove this conditional and the warning below after a transition period.
-		if isTerminal || x.List {
-			w := tabWriter()
-			defer w.Flush()
-
-			rootRequested := len(confKeys) == 0
-			fmt.Fprintf(w, "Key\tValue\n")
-			values := flattenConfig(cfg, rootRequested)
-			for _, v := range values {
-				fmt.Fprintf(w, "%s\t%v\n", v.Path, v.Value)
+				fmt.Fprintf(w, "Key\tValue\n")
+				values := flattenConfig(cfg, rootRequested)
+				for _, v := range values {
+					fmt.Fprintf(w, "%s\t%v\n", v.Path, v.Value)
+				}
+				return nil
+			} else {
+				fmt.Fprintf(Stderr, i18n.G(`WARNING: The output of "snap get" will become a list with columns - use -d or -l to force the output format.\n`))
 			}
-			return nil
-		} else {
-			fmt.Fprintf(Stderr, i18n.G(`WARNING: The output of "snap get" will become a list with columns - use -d or -l to force the output format.\n`))
 		}
-	}
 
-	if x.Typed && confToPrint == nil {
-		fmt.Fprintln(Stdout, "null")
-		return nil
-	}
+		if x.Typed && confToPrint == nil {
+			fmt.Fprintln(Stdout, "null")
+			return nil
+		}
 
-	if s, ok := confToPrint.(string); ok && !x.Typed {
-		fmt.Fprintln(Stdout, s)
-		return nil
+		if s, ok := confToPrint.(string); ok && !x.Typed {
+			fmt.Fprintln(Stdout, s)
+			return nil
+		}
 	}
 
 	var bytes []byte

cmd/snap/cmd_get_test.go

@@ -29,9 +29,11 @@ import (
 	snapset "github.com/snapcore/snapd/cmd/snap"
 )
 
-var getTests = []struct {
+type getCmdArgs struct {
 	args, stdout, stderr, error string
-}{{
+}
+
+var getTests = []getCmdArgs{{
 	args:  "get snap-name --foo",
 	error: ".*unknown flag.*foo.*",
 }, {
@@ -61,6 +63,10 @@ var getTests = []struct {
 }, {
 	args:   "get snapname -l test-key1 test-key2",
 	stdout: "Key        Value\ntest-key1  test-value1\ntest-key2  2\n",
+}, {
+	args:   "get snapname document",
+	stderr: `WARNING: The output of "snap get" will become a list with columns - use -d or -l to force the output format.\n`,
+	stdout: "{\n\t\"document\": {\n\t\t\"key1\": \"value1\",\n\t\t\"key2\": \"value2\"\n\t}\n}\n",
 }, {
 	args:   "get snapname -d test-key1 test-key2",
 	stdout: "{\n\t\"test-key1\": \"test-value1\",\n\t\"test-key2\": 2\n}\n",
@@ -81,10 +87,8 @@ var getTests = []struct {
 	stdout: "{\n\t\"bar\": 100,\n\t\"foo\": {\n\t\t\"key1\": \"value1\",\n\t\t\"key2\": \"value2\"\n\t}\n}\n",
 }}
 
-func (s *SnapSuite) TestSnapGetTests(c *C) {
-	s.mockGetConfigServer(c)
-
-	for _, test := range getTests {
+func (s *SnapSuite) runTests(cmds []getCmdArgs, c *C) {
+	for _, test := range cmds {
 		s.stdout.Truncate(0)
 		s.stderr.Truncate(0)
 
@@ -101,6 +105,27 @@ func (s *SnapSuite) TestSnapGetTests(c *C) {
 	}
 }
 
+func (s *SnapSuite) TestSnapGetTests(c *C) {
+	s.mockGetConfigServer(c)
+	s.runTests(getTests, c)
+}
+
+var getNoConfigTests = []getCmdArgs{{
+	args:  "get -l snapname",
+	error: `snap "snapname" has no configuration`,
+}, {
+	args:  "get snapname",
+	error: `snap "snapname" has no configuration`,
+}, {
+	args:   "get -d snapname",
+	stdout: "{}\n",
+}}
+
+func (s *SnapSuite) TestSnapGetNoConfiguration(c *C) {
+	s.mockGetEmptyConfigServer(c)
+	s.runTests(getNoConfigTests, c)
+}
+
 func (s *SnapSuite) TestSortByPath(c *C) {
 	values := []snapset.ConfigValue{
 		{Path: "test-key3.b"},
@@ -168,3 +193,16 @@ func (s *SnapSuite) mockGetConfigServer(c *C) {
 		}
 	})
 }
+
+func (s *SnapSuite) mockGetEmptyConfigServer(c *C) {
+	s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/v2/snaps/snapname/conf" {
+			c.Errorf("unexpected path %q", r.URL.Path)
+			return
+		}
+
+		c.Check(r.Method, Equals, "GET")
+
+		fmt.Fprintln(w, `{"type":"sync", "status-code": 200, "result": {}}`)
+	})
+}

daemon/api.go

@@ -1526,6 +1526,11 @@ func getSnapConf(c *Command, r *http.Request, user *auth.UserState) Response {
 		var value interface{}
 		if err := tr.Get(snapName, key, &value); err != nil {
 			if config.IsNoOption(err) {
+				if key == "" {
+					// no configuration - return empty document
+					currentConfValues = make(map[string]interface{})
+					break
+				}
 				return BadRequest("%v", err)
 			} else {
 				return InternalError("%v", err)

overlord/configstate/config/transaction_test.go

@@ -306,3 +306,15 @@ func (s *transactionSuite) TestGetUnmarshalError(c *C) {
 	err = tr.Get("test-snap", "foo", &broken)
 	c.Assert(err, ErrorMatches, ".*BAM!.*")
 }
+
+func (s *transactionSuite) TestNoConfiguration(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	var res interface{}
+	tr := config.NewTransaction(s.state)
+	err := tr.Get("some-snap", "", &res)
+	c.Assert(err, NotNil)
+	c.Assert(config.IsNoOption(err), Equals, true)
+	c.Assert(err, ErrorMatches, `snap "some-snap" has no configuration`)
+}

tests/main/snap-get/task.yaml

@@ -22,7 +22,7 @@ execute: |
         exit 1
     fi
 
-    echo "Test that getting root document without any configuration produces an error"
+    echo "Test that getting root document without any configuration produces an error with list format"
     if output=$(snap get snapctl-hooks 2>&1); then
         echo "snap get didn't fail as expected"
         exit 1
@@ -33,6 +33,9 @@ execute: |
         exit 1
     fi
 
+    echo "Test that getting root document without any configuration works for json output"
+    snap get snapctl-hooks -d | MATCH "^{}$"
+
     echo "Test that values set via snapctl can be obtained via snap get"
     if ! snap set snapctl-hooks command=test-snapctl-set-foo; then
         echo "snap set unexpectedly failed"