Merge ~smoser/cloud-init:bug/1764264-schema-runcmd-is-not-unique into cloud-init:master

Proposed by Scott Moser on 2018-04-19
Status: Merged
Approved by: Chad Smith on 2018-04-19
Approved revision: 65ad8752dc214d3d7fba31a7f7a8e7fbcc48118a
Merge reported by: Chad Smith
Merged at revision: 6811926fdb991ad02ad9c0134c1d4bbe82ef87e1
Proposed branch: ~smoser/cloud-init:bug/1764264-schema-runcmd-is-not-unique
Merge into: cloud-init:master
Diff against target: 177 lines (+82/-7)
7 files modified
cloudinit/config/cc_bootcmd.py (+0/-1)
cloudinit/config/cc_runcmd.py (+0/-1)
cloudinit/config/cc_snap.py (+0/-1)
cloudinit/config/cc_ubuntu_advantage.py (+0/-1)
cloudinit/config/tests/test_snap.py (+36/-0)
tests/unittests/test_handler/test_handler_bootcmd.py (+23/-1)
tests/unittests/test_handler/test_handler_runcmd.py (+23/-2)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve on 2018-04-19
Chad Smith 2018-04-19 Approve on 2018-04-19
Review via email: mp+343575@code.launchpad.net

Commit message

Schema: do not warn on duplicate items in commands.

runcmd, bootcmd, snap/commands, ubuntu-advantage/commands would
log warning (and fail if strict) on duplicate values in the commands.
But those should be allowed. Example, it is perfectly valid to do:
   runcmd: ['sleep 1', 'sleep 1']

LP: #1764264

Description of the change

see commit message

To post a comment you must log in.
Chad Smith (chad.smith) wrote :

This is good, better to be flexible in the event users need to run the same command multiple times. As you mentioned in irc apt-get update may frequently show up multiple times.

Chad Smith (chad.smith) :
review: Approve

PASSED: Continuous integration, rev:65ad8752dc214d3d7fba31a7f7a8e7fbcc48118a
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1028/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1028/rebuild

review: Approve (continuous-integration)
Chad Smith (chad.smith) wrote :

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=6811926f

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/config/cc_bootcmd.py b/cloudinit/config/cc_bootcmd.py
2index 233da1e..db64f0a 100644
3--- a/cloudinit/config/cc_bootcmd.py
4+++ b/cloudinit/config/cc_bootcmd.py
5@@ -63,7 +63,6 @@ schema = {
6 'additionalProperties': False,
7 'minItems': 1,
8 'required': [],
9- 'uniqueItems': True
10 }
11 }
12 }
13diff --git a/cloudinit/config/cc_runcmd.py b/cloudinit/config/cc_runcmd.py
14index 539cbd5..b6f6c80 100644
15--- a/cloudinit/config/cc_runcmd.py
16+++ b/cloudinit/config/cc_runcmd.py
17@@ -66,7 +66,6 @@ schema = {
18 'additionalProperties': False,
19 'minItems': 1,
20 'required': [],
21- 'uniqueItems': True
22 }
23 }
24 }
25diff --git a/cloudinit/config/cc_snap.py b/cloudinit/config/cc_snap.py
26index 34a53fd..a7a0321 100644
27--- a/cloudinit/config/cc_snap.py
28+++ b/cloudinit/config/cc_snap.py
29@@ -110,7 +110,6 @@ schema = {
30 'additionalItems': False, # Reject non-string & non-list
31 'minItems': 1,
32 'minProperties': 1,
33- 'uniqueItems': True
34 },
35 'squashfuse_in_container': {
36 'type': 'boolean'
37diff --git a/cloudinit/config/cc_ubuntu_advantage.py b/cloudinit/config/cc_ubuntu_advantage.py
38index 16b1868..29d18c9 100644
39--- a/cloudinit/config/cc_ubuntu_advantage.py
40+++ b/cloudinit/config/cc_ubuntu_advantage.py
41@@ -87,7 +87,6 @@ schema = {
42 'additionalItems': False, # Reject non-string & non-list
43 'minItems': 1,
44 'minProperties': 1,
45- 'uniqueItems': True
46 }
47 },
48 'additionalProperties': False, # Reject keys not in schema
49diff --git a/cloudinit/config/tests/test_snap.py b/cloudinit/config/tests/test_snap.py
50index c5b4a9d..492d2d4 100644
51--- a/cloudinit/config/tests/test_snap.py
52+++ b/cloudinit/config/tests/test_snap.py
53@@ -340,6 +340,42 @@ class TestSchema(CiTestCase):
54 {'snap': {'assertions': {'01': 'also valid'}}}, schema)
55 self.assertEqual('', self.logs.getvalue())
56
57+ def test_duplicates_are_fine_array_array(self):
58+ """Duplicated commands array/array entries are allowed."""
59+ byebye = ["echo", "bye"]
60+ try:
61+ cfg = {'snap': {'commands': [byebye, byebye]}}
62+ validate_cloudconfig_schema(cfg, schema, strict=True)
63+ except schema.SchemaValidationError as e:
64+ self.fail("command entries can be duplicate.")
65+
66+ def test_duplicates_are_fine_array_string(self):
67+ """Duplicated commands array/string entries are allowed."""
68+ byebye = "echo bye"
69+ try:
70+ cfg = {'snap': {'commands': [byebye, byebye]}}
71+ validate_cloudconfig_schema(cfg, schema, strict=True)
72+ except schema.SchemaValidationError as e:
73+ self.fail("command entries can be duplicate.")
74+
75+ def test_duplicates_are_fine_dict_array(self):
76+ """Duplicated commands dict/array entries are allowed."""
77+ byebye = ["echo", "bye"]
78+ try:
79+ cfg = {'snap': {'commands': {'00': byebye, '01': byebye}}}
80+ validate_cloudconfig_schema(cfg, schema, strict=True)
81+ except schema.SchemaValidationError as e:
82+ self.fail("command entries can be duplicate.")
83+
84+ def test_duplicates_are_fine_dict_string(self):
85+ """Duplicated commands dict/string entries are allowed."""
86+ byebye = "echo bye"
87+ try:
88+ cfg = {'snap': {'commands': {'00': byebye, '01': byebye}}}
89+ validate_cloudconfig_schema(cfg, schema, strict=True)
90+ except schema.SchemaValidationError as e:
91+ self.fail("command entries can be duplicate.")
92+
93
94 class TestHandle(CiTestCase):
95
96diff --git a/tests/unittests/test_handler/test_handler_bootcmd.py b/tests/unittests/test_handler/test_handler_bootcmd.py
97index 29fc25e..c3abedd 100644
98--- a/tests/unittests/test_handler/test_handler_bootcmd.py
99+++ b/tests/unittests/test_handler/test_handler_bootcmd.py
100@@ -1,6 +1,6 @@
101 # This file is part of cloud-init. See LICENSE file for license information.
102
103-from cloudinit.config import cc_bootcmd
104+from cloudinit.config import cc_bootcmd, schema
105 from cloudinit.sources import DataSourceNone
106 from cloudinit import (distros, helpers, cloud, util)
107 from cloudinit.tests.helpers import CiTestCase, mock, skipUnlessJsonSchema
108@@ -138,4 +138,26 @@ class TestBootcmd(CiTestCase):
109 self.logs.getvalue())
110
111
112+class TestSchema(CiTestCase):
113+ """Directly test schema rather than through handle."""
114+
115+ def test_duplicates_are_fine_array_array(self):
116+ """Duplicated array entries are allowed."""
117+ try:
118+ byebye = ["echo", "bye"]
119+ schema.validate_cloudconfig_schema(
120+ {'bootcmd': [byebye, byebye]}, cc_bootcmd.schema, strict=True)
121+ except schema.SchemaValidationError as e:
122+ self.fail("runcmd entries as list are allowed to be duplicates.")
123+
124+ def test_duplicates_are_fine_array_string(self):
125+ """Duplicated array entries entries are allowed in values of array."""
126+ try:
127+ byebye = "echo bye"
128+ schema.validate_cloudconfig_schema(
129+ {'bootcmd': [byebye, byebye]}, cc_bootcmd.schema, strict=True)
130+ except schema.SchemaValidationError as e:
131+ self.fail("runcmd entries are allowed to be duplicates.")
132+
133+
134 # vi: ts=4 expandtab
135diff --git a/tests/unittests/test_handler/test_handler_runcmd.py b/tests/unittests/test_handler/test_handler_runcmd.py
136index dbbb271..ee1981d 100644
137--- a/tests/unittests/test_handler/test_handler_runcmd.py
138+++ b/tests/unittests/test_handler/test_handler_runcmd.py
139@@ -1,10 +1,10 @@
140 # This file is part of cloud-init. See LICENSE file for license information.
141
142-from cloudinit.config import cc_runcmd
143+from cloudinit.config import cc_runcmd, schema
144 from cloudinit.sources import DataSourceNone
145 from cloudinit import (distros, helpers, cloud, util)
146 from cloudinit.tests.helpers import (
147- FilesystemMockingTestCase, skipUnlessJsonSchema)
148+ CiTestCase, FilesystemMockingTestCase, skipUnlessJsonSchema)
149
150 import logging
151 import os
152@@ -99,4 +99,25 @@ class TestRuncmd(FilesystemMockingTestCase):
153 self.assertEqual(0o700, stat.S_IMODE(file_stat.st_mode))
154
155
156+class TestSchema(CiTestCase):
157+ """Directly test schema rather than through handle."""
158+
159+ def test_duplicates_are_fine_array(self):
160+ """Duplicated array entries are allowed."""
161+ try:
162+ byebye = ["echo", "bye"]
163+ schema.validate_cloudconfig_schema(
164+ {'runcmd': [byebye, byebye]}, cc_runcmd.schema, strict=True)
165+ except schema.SchemaValidationError as e:
166+ self.fail("runcmd entries as list are allowed to be duplicates.")
167+
168+ def test_duplicates_are_fine_string(self):
169+ """Duplicated string entries are allowed."""
170+ try:
171+ byebye = "echo bye"
172+ schema.validate_cloudconfig_schema(
173+ {'runcmd': [byebye, byebye]}, cc_runcmd.schema, strict=True)
174+ except schema.SchemaValidationError as e:
175+ self.fail("runcmd entries are allowed to be duplicates.")
176+
177 # vi: ts=4 expandtab

Subscribers

People subscribed via source and target branches