Merge ~teknoraver/snappy-hwe-snaps/+git/wifi-ap:wizard into ~snappy-hwe-team/snappy-hwe-snaps/+git/wifi-ap:master

Proposed by Matteo Croce
Status: Merged
Approved by: Matteo Croce
Approved revision: f77adfa785a7725decf7ad373b9cbffbbef7f2b2
Merged at revision: 91a519eda8cc4c17fa76ce34040692332ab0060f
Proposed branch: ~teknoraver/snappy-hwe-snaps/+git/wifi-ap:wizard
Merge into: ~snappy-hwe-team/snappy-hwe-snaps/+git/wifi-ap:master
Diff against target: 452 lines (+244/-32)
6 files modified
cmd/client/cmd_wizard.go (+158/-31)
run-tests.sh (+1/-1)
tests/main/conf-wizard-auto-nodefaultip/task.yaml (+25/-0)
tests/main/conf-wizard-auto-noip/task.yaml (+21/-0)
tests/main/conf-wizard-auto-nowifi/task.yaml (+8/-0)
tests/main/conf-wizard-auto/task.yaml (+31/-0)
Reviewer Review Type Date Requested Status
System Enablement Bot continuous-integration Approve
Simon Fels Approve
Matteo Croce (community) Needs Fixing
Jim Hodapp (community) code Needs Fixing
Review via email: mp+311026@code.launchpad.net

Description of the change

automatic configuration for wizard

To post a comment you must log in.
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Jim Hodapp (jhodapp) wrote :

Please expand the commit message to say a little bit more about what this MR is about.

Some comments inline below.

review: Needs Fixing (code)
Revision history for this message
Simon Fels (morphis) wrote :

This misses unit-tests and spread tests to verify things are working correct.

review: Needs Fixing
Revision history for this message
Matteo Croce (teknoraver) :
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Matteo Croce (teknoraver) wrote :

spread tests are coming

review: Needs Fixing
Revision history for this message
Simon Fels (morphis) :
review: Needs Fixing
Revision history for this message
Matteo Croce (teknoraver) :
Revision history for this message
Matteo Croce (teknoraver) :
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Simon Fels (morphis) wrote :

Looks really good already but a few comments about the spread tests.

review: Needs Fixing
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Simon Fels (morphis) :
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Simon Fels (morphis) wrote :

A few last comments in line

review: Needs Fixing
Revision history for this message
Simon Fels (morphis) wrote :

Once CI approves we can merge this one.

review: Approve
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cmd/client/cmd_wizard.go b/cmd/client/cmd_wizard.go
2index 9a9ebd5..a368c24 100644
3--- a/cmd/client/cmd_wizard.go
4+++ b/cmd/client/cmd_wizard.go
5@@ -20,15 +20,24 @@ import (
6 "bytes"
7 "encoding/json"
8 "fmt"
9- "io/ioutil"
10 "net"
11 "net/http"
12 "os"
13+ "path/filepath"
14 "regexp"
15 "strconv"
16 "strings"
17 )
18
19+type wizardStep func(map[string]string, *bufio.Reader, bool) error
20+
21+// Go stores both IPv4 and IPv6 as [16]byte
22+// with IPv4 addresses stored in the end of the buffer
23+// in bytes 13..16
24+const ipv4Offset = net.IPv6len - net.IPv4len
25+
26+var defaultIp = net.IPv4(10, 0, 60, 1)
27+
28 // Utility function to read input and strip the trailing \n
29 func readUserInput(reader *bufio.Reader) string {
30 ret, err := reader.ReadString('\n')
31@@ -41,15 +50,15 @@ func readUserInput(reader *bufio.Reader) string {
32 // Helper function to list network interfaces
33 func findExistingInterfaces(wifi bool) (wifis []string) {
34 // Use sysfs to get interfaces list
35- const sysNetPath = "/sys/class/net/"
36- ifaces, err := ioutil.ReadDir(sysNetPath)
37+ const sysNetPath = "/sys/class/net"
38+ ifaces, err := net.Interfaces()
39 wifis = []string{}
40 if err == nil {
41 for _, iface := range ifaces {
42- if iface.Name() != "lo" {
43+ if iface.Flags&net.FlagLoopback == 0 {
44 // The "wireless" subdirectory exists only for wireless interfaces
45- if _, err := os.Stat(sysNetPath + iface.Name() + "/wireless"); os.IsNotExist(err) != wifi {
46- wifis = append(wifis, iface.Name())
47+ if _, err := os.Stat(filepath.Join(sysNetPath, iface.Name, "wireless")); os.IsNotExist(err) != wifi {
48+ wifis = append(wifis, iface.Name)
49 }
50 }
51 }
52@@ -58,11 +67,39 @@ func findExistingInterfaces(wifi bool) (wifis []string) {
53 return
54 }
55
56-type wizardStep func(map[string]string, *bufio.Reader) error
57+func findFreeSubnet(startIp net.IP) (net.IP, error) {
58+ curIp := startIp
59+
60+ addrs, err := net.InterfaceAddrs()
61+ if err != nil {
62+ return nil, err
63+ }
64+
65+ // Start from startIp and increment the third octect every time
66+ // until we found an IP which isn't assigned to any interface
67+ for found := false; !found; {
68+ // Cycle through all the assigned addresses
69+ for _, addr := range addrs {
70+ found = true
71+ _, subnet, _ := net.ParseCIDR(addr.String())
72+ // If busy, increment the third octect and retry
73+ if subnet.Contains(curIp) {
74+ found = false
75+ if curIp[ipv4Offset+2] == 255 {
76+ return nil, fmt.Errorf("No free netmask found")
77+ }
78+ curIp[ipv4Offset+2]++
79+ break
80+ }
81+ }
82+ }
83+
84+ return curIp, nil
85+}
86
87 var allSteps = [...]wizardStep{
88 // determine the WiFi interface
89- func(configuration map[string]string, reader *bufio.Reader) error {
90+ func(configuration map[string]string, reader *bufio.Reader, nonInteractive bool) error {
91 ifaces := findExistingInterfaces(true)
92 if len(ifaces) == 0 {
93 return fmt.Errorf("There are no valid wireless network interfaces available")
94@@ -70,12 +107,19 @@ var allSteps = [...]wizardStep{
95 fmt.Println("Automatically selected only available wireless network interface " + ifaces[0])
96 return nil
97 }
98+
99+ if nonInteractive {
100+ fmt.Println("Selecting interface", ifaces[0])
101+ configuration["wifi.interface"] = ifaces[0]
102+ return nil
103+ }
104+
105 fmt.Print("Which wireless interface you want to use? ")
106 ifacesVerb := "are"
107 if len(ifaces) == 1 {
108 ifacesVerb = "is"
109 }
110- fmt.Printf("Available %s %s:", ifacesVerb, strings.Join(ifaces, ", "))
111+ fmt.Printf("Available %s %s: ", ifacesVerb, strings.Join(ifaces, ", "))
112 iface := readUserInput(reader)
113 if re := regexp.MustCompile("^[[:alnum:]]+$"); !re.MatchString(iface) {
114 return fmt.Errorf("Invalid interface name '%s' given", iface)
115@@ -86,7 +130,12 @@ var allSteps = [...]wizardStep{
116 },
117
118 // Ask for WiFi ESSID
119- func(configuration map[string]string, reader *bufio.Reader) error {
120+ func(configuration map[string]string, reader *bufio.Reader, nonInteractive bool) error {
121+ if nonInteractive {
122+ configuration["wifi.ssid"] = "Ubuntu"
123+ return nil
124+ }
125+
126 fmt.Print("Which SSID you want to use for the access point: ")
127 iface := readUserInput(reader)
128 if len(iface) == 0 || len(iface) > 31 {
129@@ -98,7 +147,12 @@ var allSteps = [...]wizardStep{
130 },
131
132 // Select WiFi encryption type
133- func(configuration map[string]string, reader *bufio.Reader) error {
134+ func(configuration map[string]string, reader *bufio.Reader, nonInteractive bool) error {
135+ if nonInteractive {
136+ configuration["wifi.security"] = "open"
137+ return nil
138+ }
139+
140 fmt.Print("Do you want to protect your network with a WPA2 password instead of staying open for everyone? (y/n) ")
141 switch resp := strings.ToLower(readUserInput(reader)); resp {
142 case "y":
143@@ -113,7 +167,7 @@ var allSteps = [...]wizardStep{
144 },
145
146 // If WPA2 is set, ask for valid password
147- func(configuration map[string]string, reader *bufio.Reader) error {
148+ func(configuration map[string]string, reader *bufio.Reader, nonInteractive bool) error {
149 if configuration["wifi.security"] == "open" {
150 return nil
151 }
152@@ -128,7 +182,20 @@ var allSteps = [...]wizardStep{
153 },
154
155 // Configure WiFi AP IP address
156- func(configuration map[string]string, reader *bufio.Reader) error {
157+ func(configuration map[string]string, reader *bufio.Reader, nonInteractive bool) error {
158+ if nonInteractive {
159+ wifiIp, err := findFreeSubnet(defaultIp)
160+ if err != nil {
161+ return err
162+ }
163+
164+ fmt.Println("AccessPoint IP set to", wifiIp.String())
165+
166+ configuration["wifi.address"] = wifiIp.String()
167+ configuration["wifi.netmask"] = "255.255.255.0"
168+ return nil
169+ }
170+
171 fmt.Print("Insert the Access Point IP address: ")
172 inputIp := readUserInput(reader)
173 ipv4 := net.ParseIP(inputIp)
174@@ -143,21 +210,33 @@ var allSteps = [...]wizardStep{
175 }
176
177 configuration["wifi.address"] = inputIp
178-
179- nmask := ipv4.DefaultMask()
180- configuration["wifi.netmask"] = fmt.Sprintf("%d.%d.%d.%d", nmask[0], nmask[1], nmask[2], nmask[3])
181+ configuration["wifi.netmask"] = ipv4.DefaultMask().String()
182
183 return nil
184 },
185
186 // Configure the DHCP pool
187- func(configuration map[string]string, reader *bufio.Reader) error {
188+ func(configuration map[string]string, reader *bufio.Reader, nonInteractive bool) error {
189+ if nonInteractive {
190+ wifiIp := net.ParseIP(configuration["wifi.address"])
191+
192+ // Set the DCHP in the range 2..199 with 198 total hosts
193+ // leave about 50 hosts outside the pool for static addresses
194+ // wifiIp[ipv4Offset + 3] is the last octect of the IP address
195+ wifiIp[ipv4Offset+3] = 2
196+ configuration["dhcp.range-start"] = wifiIp.String()
197+ wifiIp[ipv4Offset+3] = 199
198+ configuration["dhcp.range-stop"] = wifiIp.String()
199+
200+ return nil
201+ }
202+
203 var maxpoolsize byte
204 ipv4 := net.ParseIP(configuration["wifi.address"])
205- if ipv4[15] <= 128 {
206- maxpoolsize = 254 - ipv4[15]
207+ if ipv4[ipv4Offset+3] <= 128 {
208+ maxpoolsize = 254 - ipv4[ipv4Offset+3]
209 } else {
210- maxpoolsize = ipv4[15] - 1
211+ maxpoolsize = ipv4[ipv4Offset+3] - 1
212 }
213
214 fmt.Printf("How many host do you want your DHCP pool to hold to? (1-%d) ", maxpoolsize)
215@@ -172,19 +251,28 @@ var allSteps = [...]wizardStep{
216
217 nhosts := byte(inputhost)
218 // Allocate the pool in the bigger half, trying to avoid overlap with access point IP
219- if ipv4[15] <= 128 {
220- configuration["dhcp.range-start"] = fmt.Sprintf("%d.%d.%d.%d", ipv4[12], ipv4[13], ipv4[14], ipv4[15]+1)
221- configuration["dhcp.range-stop"] = fmt.Sprintf("%d.%d.%d.%d", ipv4[12], ipv4[13], ipv4[14], ipv4[15]+nhosts)
222+ if octect3 := ipv4[ipv4Offset+3]; octect3 <= 128 {
223+ ipv4[ipv4Offset+3] = octect3 + 1
224+ configuration["dhcp.range-start"] = ipv4.String()
225+ ipv4[ipv4Offset+3] = octect3 + nhosts
226+ configuration["dhcp.range-stop"] = ipv4.String()
227 } else {
228- configuration["dhcp.range-start"] = fmt.Sprintf("%d.%d.%d.%d", ipv4[12], ipv4[13], ipv4[14], ipv4[15]-nhosts)
229- configuration["dhcp.range-stop"] = fmt.Sprintf("%d.%d.%d.%d", ipv4[12], ipv4[13], ipv4[14], ipv4[15]-1)
230+ ipv4[ipv4Offset+3] = octect3 - nhosts
231+ configuration["dhcp.range-start"] = ipv4.String()
232+ ipv4[ipv4Offset+3] = octect3 - 1
233+ configuration["dhcp.range-stop"] = ipv4.String()
234 }
235
236 return nil
237 },
238
239 // Enable or disable connection sharing
240- func(configuration map[string]string, reader *bufio.Reader) error {
241+ func(configuration map[string]string, reader *bufio.Reader, nonInteractive bool) error {
242+ if nonInteractive {
243+ configuration["share.disabled"] = "0"
244+ return nil
245+ }
246+
247 fmt.Print("Do you want to enable connection sharing? (y/n) ")
248 switch resp := strings.ToLower(readUserInput(reader)); resp {
249 case "y":
250@@ -199,10 +287,39 @@ var allSteps = [...]wizardStep{
251 },
252
253 // Select the wired interface to share
254- func(configuration map[string]string, reader *bufio.Reader) error {
255+ func(configuration map[string]string, reader *bufio.Reader, nonInteractive bool) error {
256+ if nonInteractive {
257+ configuration["share.disabled"] = "1"
258+
259+ procNetRoute, err := os.Open("/proc/net/route")
260+ if err != nil {
261+ return err
262+ }
263+ defer procNetRoute.Close()
264+
265+ scanner := bufio.NewScanner(procNetRoute)
266+ // Skip the first line with table header
267+ scanner.Text()
268+ for scanner.Scan() {
269+ route := strings.Fields(scanner.Text())
270+ // A /proc/net/route line is in the form: iface destination gateway ...
271+ // eg.
272+ // eth1 00000000 0155A8C0 ...
273+ // look for a 0 destination (0.0.0.0) which is our default route
274+ if len(route) > 2 && route[1] == "00000000" {
275+ fmt.Println("Selecting", route[0], "for connection sharing")
276+ configuration["share.disabled"] = "0"
277+ configuration["share.network-interface"] = route[0]
278+ }
279+ }
280+
281+ return nil
282+ }
283+
284 if configuration["share.disabled"] == "1" {
285 return nil
286 }
287+
288 ifaces := findExistingInterfaces(false)
289 if len(ifaces) == 0 {
290 fmt.Println("No network interface available which's connection can be shared. Disabling connection sharing.")
291@@ -224,7 +341,12 @@ var allSteps = [...]wizardStep{
292 return nil
293 },
294
295- func (configuration map[string]string, reader *bufio.Reader) error {
296+ func(configuration map[string]string, reader *bufio.Reader, nonInteractive bool) error {
297+ if nonInteractive {
298+ configuration["disabled"] = "0"
299+ return nil
300+ }
301+
302 fmt.Print("Do you want to enable the AP now? (y/n) ")
303 switch resp := strings.ToLower(readUserInput(reader)); resp {
304 case "y":
305@@ -262,7 +384,9 @@ func applyConfiguration(configuration map[string]string) error {
306 }
307 }
308
309-type wizardCommand struct{}
310+type wizardCommand struct {
311+ Auto bool `long:"auto" description:"Automatically configure the AP"`
312+}
313
314 func (cmd *wizardCommand) Execute(args []string) error {
315 // Setup some sane default values, we don't ask the user for everything
316@@ -272,9 +396,12 @@ func (cmd *wizardCommand) Execute(args []string) error {
317
318 for _, step := range allSteps {
319 for {
320- if err := step(configuration, reader); err != nil {
321+ if err := step(configuration, reader, cmd.Auto); err != nil {
322+ if cmd.Auto {
323+ return err
324+ }
325 fmt.Println("Error: ", err)
326- fmt.Print("You want to try again? (y/n) ")
327+ fmt.Print("Do you want to try again? (y/n) ")
328 answer := readUserInput(reader)
329 if strings.ToLower(answer) != "y" {
330 return err
331diff --git a/run-tests.sh b/run-tests.sh
332index eaa2fca..e234fc2 100755
333--- a/run-tests.sh
334+++ b/run-tests.sh
335@@ -73,7 +73,7 @@ if [ ! -e $SPREAD_QEMU_PATH/$image_name ] || [ $force_new_image -eq 1 ] ; then
336 echo "INFO: Creating new qemu test image ..."
337 (cd tests/image ; sudo ./create-image.sh $channel)
338 mkdir -p $SPREAD_QEMU_PATH
339- mv tests/image/ubuntu-core-16.img $SPREAD_QEMU_PATH/$image_name
340+ mv -f tests/image/ubuntu-core-16.img $SPREAD_QEMU_PATH/$image_name
341 fi
342
343 # We currently only run spread tests but we could do other things
344diff --git a/tests/main/conf-wizard-auto-nodefaultip/task.yaml b/tests/main/conf-wizard-auto-nodefaultip/task.yaml
345new file mode 100644
346index 0000000..e6d6e06
347--- /dev/null
348+++ b/tests/main/conf-wizard-auto-nodefaultip/task.yaml
349@@ -0,0 +1,25 @@
350+summary: Verify that the wizard is able to find an unused IP
351+
352+prepare: |
353+ # Simulate a radio network interfaces
354+ modprobe mac80211_hwsim radios=1
355+
356+ # Dummy interface to assign an IP to
357+ modprobe dummy
358+
359+ # Use 10.0.60.0/20 to keep 10.0.48.1-10.0.63.254 busy
360+ # and force the wizard to use the 10.0.64.0/24
361+ ifconfig dummy0 10.0.48.2/20
362+
363+restore: |
364+ rmmod mac80211_hwsim dummy
365+
366+execute: |
367+ # Start the automatic wizard, it must fail
368+ /snap/bin/wifi-ap.setup-wizard wizard --auto
369+
370+ # Check for assigned IP on subnet 10.0.64.0/24
371+ test "$(/snap/bin/wifi-ap.config get wifi.address)" = 10.0.64.1
372+ test "$(/snap/bin/wifi-ap.config get wifi.netmask)" = 255.255.255.0
373+ test "$(/snap/bin/wifi-ap.config get dhcp.range-start)" = 10.0.64.2
374+ test "$(/snap/bin/wifi-ap.config get dhcp.range-stop)" = 10.0.64.199
375diff --git a/tests/main/conf-wizard-auto-noip/task.yaml b/tests/main/conf-wizard-auto-noip/task.yaml
376new file mode 100644
377index 0000000..5a0885c
378--- /dev/null
379+++ b/tests/main/conf-wizard-auto-noip/task.yaml
380@@ -0,0 +1,21 @@
381+summary: Verify that wizard fails when all private subnets are busy
382+
383+prepare: |
384+ # Simulate a radio network interfaces
385+ modprobe mac80211_hwsim radios=1
386+
387+ # Dummy interface to assign an IP to
388+ modprobe dummy
389+
390+ # Set an IP with a /8 mask which will waste all usable IP
391+ ifconfig dummy0 10.0.0.2/8
392+
393+restore: |
394+ rmmod mac80211_hwsim dummy
395+
396+execute: |
397+ # Start the automatic wizard, it must fail
398+ ! /snap/bin/wifi-ap.setup-wizard wizard --auto
399+
400+ # Check for a descriptive error message
401+ /snap/bin/wifi-ap.setup-wizard wizard --auto 2>&1 |grep 'No free netmask found'
402diff --git a/tests/main/conf-wizard-auto-nowifi/task.yaml b/tests/main/conf-wizard-auto-nowifi/task.yaml
403new file mode 100644
404index 0000000..18301f1
405--- /dev/null
406+++ b/tests/main/conf-wizard-auto-nowifi/task.yaml
407@@ -0,0 +1,8 @@
408+summary: Verify that wizard fails when there are no WiFi devices
409+
410+execute: |
411+ # Start the automatic wizard, it must fail
412+ ! /snap/bin/wifi-ap.setup-wizard wizard --auto
413+
414+ # Check for a descriptive error message
415+ /snap/bin/wifi-ap.setup-wizard wizard --auto 2>&1 |grep 'There are no valid wireless network interfaces available'
416diff --git a/tests/main/conf-wizard-auto/task.yaml b/tests/main/conf-wizard-auto/task.yaml
417new file mode 100644
418index 0000000..3d1a2c7
419--- /dev/null
420+++ b/tests/main/conf-wizard-auto/task.yaml
421@@ -0,0 +1,31 @@
422+summary: Verify that the automatic wizard works
423+
424+prepare: |
425+ # Simulate two WiFi radio network interfaces
426+ modprobe mac80211_hwsim radios=2
427+
428+restore: |
429+ rmmod mac80211_hwsim
430+
431+execute: |
432+ # Start the automatic wizard
433+ /snap/bin/wifi-ap.setup-wizard wizard --auto
434+
435+ # Check that we get good default values
436+ test "$(/snap/bin/wifi-ap.config get disabled)" = 0
437+
438+ test "$(/snap/bin/wifi-ap.config get wifi.interface)" = wlan0
439+ test "$(/snap/bin/wifi-ap.config get wifi.security)" = open
440+ test "$(/snap/bin/wifi-ap.config get wifi.ssid)" = Ubuntu
441+ test "$(/snap/bin/wifi-ap.config get wifi.address)" = 10.0.60.1
442+
443+ test "$(/snap/bin/wifi-ap.config get dhcp.range-start)" = 10.0.60.2
444+ test "$(/snap/bin/wifi-ap.config get dhcp.range-stop)" = 10.0.60.199
445+
446+ default_route=$(ip route |awk '/default/{print$5}')
447+ if [ -n "$default_route" ]; then
448+ test "$(/snap/bin/wifi-ap.config get share.disabled)" = 0
449+ test "$(/snap/bin/wifi-ap.config get share.network-interface)" = "$default_route"
450+ else
451+ test "$(/snap/bin/wifi-ap.config get share.disabled)" = 1
452+ fi

Subscribers

People subscribed via source and target branches

to all changes: