Merge lp:~chipaca/snappy/dont-panic into lp:snappy/15.04

Proposed by John Lenton
Status: Merged
Approved by: Michael Vogt
Approved revision: 712
Merged at revision: 710
Proposed branch: lp:~chipaca/snappy/dont-panic
Merge into: lp:snappy/15.04
Diff against target: 324 lines (+249/-4)
7 files modified
_integration-tests/testutils/image/image.go (+1/-1)
cmd/snappy/cmd_console.go (+165/-0)
cmd/snappy/cmd_console_test.go (+57/-0)
coreconfig/config.go (+12/-3)
coreconfig/config_test.go (+12/-0)
debian/control (+1/-0)
dependencies.tsv (+1/-0)
To merge this branch: bzr merge lp:~chipaca/snappy/dont-panic
Reviewer Review Type Date Requested Status
Michael Vogt Approve
Review via email: mp+271833@code.launchpad.net

This proposal supersedes a proposal from 2015-09-21.

Commit message

Avoid a panic when coreconfig is given invalid config.

To post a comment you must log in.
Revision history for this message
Michael Vogt (mvo) wrote : Posted in a previous version of this proposal

Thanks!

review: Approve
Revision history for this message
Michael Vogt (mvo) wrote : Posted in a previous version of this proposal

We should probably target this to 15.04, ok?

Revision history for this message
Michael Vogt (mvo) wrote :

\o/

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file '_integration-tests/testutils/image/image.go'
2--- _integration-tests/testutils/image/image.go 2015-07-28 04:03:52 +0000
3+++ _integration-tests/testutils/image/image.go 2015-09-21 15:47:26 +0000
4@@ -51,7 +51,7 @@
5 udfCommand := []string{"sudo", "ubuntu-device-flash", "--verbose"}
6
7 if img.revision != "" {
8- udfCommand = append(udfCommand, "--revision", img.revision)
9+ udfCommand = append(udfCommand, "--revision="+img.revision)
10 }
11
12 imagePath := img.imagePath(imageDir)
13
14=== added file 'cmd/snappy/cmd_console.go'
15--- cmd/snappy/cmd_console.go 1970-01-01 00:00:00 +0000
16+++ cmd/snappy/cmd_console.go 2015-09-21 15:47:26 +0000
17@@ -0,0 +1,165 @@
18+// -*- Mode: Go; indent-tabs-mode: t -*-
19+
20+/*
21+ * Copyright (C) 2014-2015 Canonical Ltd
22+ *
23+ * This program is free software: you can redistribute it and/or modify
24+ * it under the terms of the GNU General Public License version 3 as
25+ * published by the Free Software Foundation.
26+ *
27+ * This program is distributed in the hope that it will be useful,
28+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
29+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
30+ * GNU General Public License for more details.
31+ *
32+ * You should have received a copy of the GNU General Public License
33+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
34+ *
35+ */
36+
37+package main
38+
39+import (
40+ "fmt"
41+ "io"
42+ "os"
43+ "os/exec"
44+ "strings"
45+
46+ "github.com/peterh/liner"
47+
48+ "launchpad.net/snappy/logger"
49+)
50+
51+// for testing
52+var stdout io.Writer = os.Stdout
53+
54+type cmdConsole struct {
55+ repl *liner.State
56+ extraCommands []consoleCommand
57+}
58+
59+func init() {
60+ _, err := parser.AddCommand("console",
61+ "Run snappy console interface",
62+ "Run snappy console interface",
63+ &cmdConsole{})
64+ if err != nil {
65+ logger.Panicf("Unable to console: %v", err)
66+ }
67+}
68+
69+func (x *cmdConsole) Execute(args []string) error {
70+ return x.doConsole()
71+}
72+
73+type consoleCommand struct {
74+ name string
75+ fn func(line string) error
76+}
77+
78+func (x *cmdConsole) snappyCompleter(line string) (c []string) {
79+ // FIXME: add smartz and also complete arguments of
80+ // commands
81+ for _, cmd := range parser.Commands() {
82+ if strings.HasPrefix(cmd.Name, strings.ToLower(line)) {
83+ c = append(c, cmd.Name)
84+ }
85+ }
86+ for _, cmd := range x.extraCommands {
87+ if strings.HasPrefix(cmd.name, line) {
88+ c = append(c, cmd.name)
89+ }
90+ }
91+
92+ return c
93+}
94+
95+func (x *cmdConsole) initConsole() error {
96+ // FIXME: add history (ReadHistory/WriteHistory)
97+
98+ x.extraCommands = []consoleCommand{
99+ {"help", x.doHelp},
100+ {"shell", x.doShell},
101+ }
102+
103+ x.repl = liner.NewLiner()
104+ x.repl.SetCompleter(x.snappyCompleter)
105+
106+ return nil
107+}
108+
109+func (x *cmdConsole) CloseConsole() {
110+ x.repl.Close()
111+}
112+
113+func (x *cmdConsole) PrintWelcomeMessage() {
114+ fmt.Println("Welcome to the snappy console")
115+ fmt.Println("Type 'help' for help")
116+ fmt.Println("Type 'shell' for entering a shell")
117+}
118+
119+func (x *cmdConsole) doShell(line string) error {
120+ // restore terminal for the shell
121+ x.CloseConsole()
122+ defer x.initConsole()
123+
124+ sh := os.Getenv("SHELL")
125+ if sh == "" {
126+ sh = "/bin/sh"
127+ }
128+ cmd := exec.Command(sh)
129+ cmd.Stdin = os.Stdin
130+ cmd.Stdout = os.Stdout
131+ cmd.Stderr = os.Stderr
132+ if err := cmd.Run(); err != nil {
133+ return err
134+ }
135+
136+ return nil
137+}
138+
139+func (x *cmdConsole) doHelp(line string) error {
140+ line = strings.TrimPrefix(line, "help")
141+ line = strings.TrimSpace(line)
142+ parser.Active = nil
143+ // find subcmd
144+ for _, cmd := range parser.Commands() {
145+ if strings.HasPrefix(line, cmd.Name) {
146+ parser.Active = cmd
147+ break
148+ }
149+ }
150+ parser.WriteHelp(stdout)
151+
152+ return nil
153+}
154+
155+func (x *cmdConsole) doConsole() error {
156+ x.initConsole()
157+ defer x.CloseConsole()
158+ x.PrintWelcomeMessage()
159+
160+outer:
161+ for {
162+ line, err := x.repl.Prompt("> ")
163+ if err != nil {
164+ return err
165+ }
166+ x.repl.AppendHistory(line)
167+
168+ for _, cmd := range x.extraCommands {
169+ if strings.HasPrefix(line, cmd.name) {
170+ if err := cmd.fn(line); err != nil {
171+ fmt.Println(err)
172+ }
173+ continue outer
174+ }
175+ }
176+
177+ if _, err = parser.ParseArgs(strings.Fields(line)); err != nil {
178+ fmt.Println(err)
179+ }
180+
181+ }
182+}
183
184=== added file 'cmd/snappy/cmd_console_test.go'
185--- cmd/snappy/cmd_console_test.go 1970-01-01 00:00:00 +0000
186+++ cmd/snappy/cmd_console_test.go 2015-09-21 15:47:26 +0000
187@@ -0,0 +1,57 @@
188+// -*- Mode: Go; indent-tabs-mode: t -*-
189+
190+/*
191+ * Copyright (C) 2014-2015 Canonical Ltd
192+ *
193+ * This program is free software: you can redistribute it and/or modify
194+ * it under the terms of the GNU General Public License version 3 as
195+ * published by the Free Software Foundation.
196+ *
197+ * This program is distributed in the hope that it will be useful,
198+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
199+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
200+ * GNU General Public License for more details.
201+ *
202+ * You should have received a copy of the GNU General Public License
203+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
204+ *
205+ */
206+
207+package main
208+
209+import (
210+ "bytes"
211+
212+ . "gopkg.in/check.v1"
213+)
214+
215+func (s *CmdTestSuite) TestCmdConsoleCompleter(c *C) {
216+ // setup
217+ x := cmdConsole{}
218+
219+ err := x.initConsole()
220+ c.Assert(err, IsNil)
221+
222+ // from cmdline parser
223+ c.Check(x.snappyCompleter("hw-"), DeepEquals, []string{"hw-assign", "hw-info", "hw-unassign"})
224+
225+ // extra consoleCommand
226+ c.Check(x.snappyCompleter("he"), DeepEquals, []string{"help"})
227+ c.Check(x.snappyCompleter("help"), DeepEquals, []string{"help"})
228+}
229+
230+func (s *CmdTestSuite) TestDoHelpGeneric(c *C) {
231+ stdout = bytes.NewBuffer(nil)
232+
233+ x := cmdConsole{}
234+ x.doHelp("")
235+ c.Assert(stdout.(*bytes.Buffer).String(), Matches, `(?sm).*Available commands:`)
236+}
237+
238+func (s *CmdTestSuite) TestDoHelpSet(c *C) {
239+ stdout = bytes.NewBuffer(nil)
240+
241+ x := cmdConsole{}
242+ x.doHelp("set")
243+ c.Assert(stdout.(*bytes.Buffer).String(), Matches, `(?sm).*Set properties of system or package`)
244+}
245
246=== modified file 'coreconfig/config.go'
247--- coreconfig/config.go 2015-09-15 21:35:38 +0000
248+++ coreconfig/config.go 2015-09-21 15:47:26 +0000
249@@ -58,9 +58,15 @@
250 watchdogStartupPath = "/etc/default/watchdog"
251 )
252
253-// ErrInvalidUnitStatus signals that a unit is not returning a status
254-// of "enabled" or "disabled".
255-var ErrInvalidUnitStatus = errors.New("invalid unit status")
256+var (
257+ // ErrInvalidUnitStatus signals that a unit is not returning a status
258+ // of "enabled" or "disabled".
259+ ErrInvalidUnitStatus = errors.New("invalid unit status")
260+
261+ // ErrInvalidConfig is returned from Set when the value
262+ // provided is not a valid configuration string.
263+ ErrInvalidConfig = errors.New("invalid ubuntu-core configuration")
264+)
265
266 type systemConfig struct {
267 Autopilot *bool `yaml:"autopilot,omitempty"`
268@@ -194,6 +200,9 @@
269 return "", err
270 }
271 newConfig := configWrap.Config.UbuntuCore
272+ if newConfig == nil {
273+ return "", ErrInvalidConfig
274+ }
275
276 rNewConfig := reflect.ValueOf(newConfig).Elem()
277 rType := rNewConfig.Type()
278
279=== modified file 'coreconfig/config_test.go'
280--- coreconfig/config_test.go 2015-09-15 21:36:22 +0000
281+++ coreconfig/config_test.go 2015-09-21 15:47:26 +0000
282@@ -148,6 +148,18 @@
283 c.Assert(rawConfig, Equals, expected)
284 }
285
286+func (cts *ConfigTestSuite) TestSetBadValueDoesNotPanic(c *C) {
287+ for _, s := range []string{
288+ "",
289+ "\n",
290+ "config:\n",
291+ "config:\n ubuntu-core:\n",
292+ } {
293+ _, err := Set(s)
294+ c.Assert(err, Equals, ErrInvalidConfig)
295+ }
296+}
297+
298 // TestSetTimezone is a broad test, close enough to be an integration test.
299 func (cts *ConfigTestSuite) TestSetTimezone(c *C) {
300 // TODO figure out if we care about exact output or just want valid yaml.
301
302=== modified file 'debian/control'
303--- debian/control 2015-09-14 19:04:47 +0000
304+++ debian/control 2015-09-21 15:47:26 +0000
305@@ -15,6 +15,7 @@
306 golang-go-flags-dev,
307 golang-go.crypto-dev,
308 golang-goconfigparser-dev,
309+ golang-go-liner-dev,
310 golang-pb-dev,
311 golang-uboot-go-dev,
312 golang-yaml.v2-dev,
313
314=== modified file 'dependencies.tsv'
315--- dependencies.tsv 2015-09-16 12:04:05 +0000
316+++ dependencies.tsv 2015-09-21 15:47:26 +0000
317@@ -6,6 +6,7 @@
318 github.com/gosexy/gettext git 98b7b91596d20b96909e6b60d57411547dd9959c 2013-02-21T11:21:43Z
319 github.com/jessevdk/go-flags git 1acbbaff2f347c412a0c7884873bd72cc9c1f5b4 2015-08-16T10:05:21Z
320 github.com/mvo5/goconfigparser git 26426272dda20cc76aa1fa44286dc743d2972fe8 2015-02-12T09:37:50Z
321+github.com/peterh/liner git 1bb0d1c1a25ed393d8feb09bab039b2b1b1fbced 2015-04-02T04:04:07Z
322 github.com/mvo5/uboot-go git 361f6ebcbb54f389d15dc9faefa000e996ba3e37 2015-07-22T06:53:46Z
323 golang.org/x/crypto git 60052bd85f2d91293457e8811b0cf26b773de469 2015-06-22T23:34:07Z
324 gopkg.in/check.v1 git 64131543e7896d5bcc6bd5a76287eb75ea96c673 2014-10-24T13:38:53Z

Subscribers

People subscribed via source and target branches

to all changes: