cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:bug/1764264-schema-runcmd-is-not-unique into cloud-init:master.

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

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1764264 in cloud-init: "bionic cloud-init 18.2 WARNING Juju's 'runcmd' stanza"
  https://bugs.launchpad.net/cloud-init/+bug/1764264

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/343575

see commit message
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:bug/1764264-schema-runcmd-is-not-unique into cloud-init:master.

diff --git a/cloudinit/config/cc_bootcmd.py b/cloudinit/config/cc_bootcmd.py
index 233da1e..db64f0a 100644
--- a/cloudinit/config/cc_bootcmd.py
+++ b/cloudinit/config/cc_bootcmd.py
@@ -63,7 +63,6 @@ schema = {
             'additionalProperties': False,
             'minItems': 1,
             'required': [],
-            'uniqueItems': True
         }
     }
 }
diff --git a/cloudinit/config/cc_runcmd.py b/cloudinit/config/cc_runcmd.py
index 539cbd5..b6f6c80 100644
--- a/cloudinit/config/cc_runcmd.py
+++ b/cloudinit/config/cc_runcmd.py
@@ -66,7 +66,6 @@ schema = {
             'additionalProperties': False,
             'minItems': 1,
             'required': [],
-            'uniqueItems': True
         }
     }
 }
diff --git a/cloudinit/config/cc_snap.py b/cloudinit/config/cc_snap.py
index 34a53fd..a7a0321 100644
--- a/cloudinit/config/cc_snap.py
+++ b/cloudinit/config/cc_snap.py
@@ -110,7 +110,6 @@ schema = {
                     'additionalItems': False,  # Reject non-string & non-list
                     'minItems': 1,
                     'minProperties': 1,
-                    'uniqueItems': True
                 },
                 'squashfuse_in_container': {
                     'type': 'boolean'
diff --git a/cloudinit/config/cc_ubuntu_advantage.py b/cloudinit/config/cc_ubuntu_advantage.py
index 16b1868..29d18c9 100644
--- a/cloudinit/config/cc_ubuntu_advantage.py
+++ b/cloudinit/config/cc_ubuntu_advantage.py
@@ -87,7 +87,6 @@ schema = {
                     'additionalItems': False,  # Reject non-string & non-list
                     'minItems': 1,
                     'minProperties': 1,
-                    'uniqueItems': True
                 }
             },
             'additionalProperties': False,  # Reject keys not in schema
diff --git a/cloudinit/config/tests/test_snap.py b/cloudinit/config/tests/test_snap.py
index c5b4a9d..492d2d4 100644
--- a/cloudinit/config/tests/test_snap.py
+++ b/cloudinit/config/tests/test_snap.py
@@ -340,6 +340,42 @@ class TestSchema(CiTestCase):
             {'snap': {'assertions': {'01': 'also valid'}}}, schema)
         self.assertEqual('', self.logs.getvalue())
 
+    def test_duplicates_are_fine_array_array(self):
+        """Duplicated commands array/array entries are allowed."""
+        byebye = ["echo", "bye"]
+        try:
+            cfg = {'snap': {'commands': [byebye, byebye]}}
+            validate_cloudconfig_schema(cfg, schema, strict=True)
+        except schema.SchemaValidationError as e:
+            self.fail("command entries can be duplicate.")
+
+    def test_duplicates_are_fine_array_string(self):
+        """Duplicated commands array/string entries are allowed."""
+        byebye = "echo bye"
+        try:
+            cfg = {'snap': {'commands': [byebye, byebye]}}
+            validate_cloudconfig_schema(cfg, schema, strict=True)
+        except schema.SchemaValidationError as e:
+            self.fail("command entries can be duplicate.")
+
+    def test_duplicates_are_fine_dict_array(self):
+        """Duplicated commands dict/array entries are allowed."""
+        byebye = ["echo", "bye"]
+        try:
+            cfg = {'snap': {'commands': {'00': byebye, '01': byebye}}}
+            validate_cloudconfig_schema(cfg, schema, strict=True)
+        except schema.SchemaValidationError as e:
+            self.fail("command entries can be duplicate.")
+
+    def test_duplicates_are_fine_dict_string(self):
+        """Duplicated commands dict/string entries are allowed."""
+        byebye = "echo bye"
+        try:
+            cfg = {'snap': {'commands': {'00': byebye, '01': byebye}}}
+            validate_cloudconfig_schema(cfg, schema, strict=True)
+        except schema.SchemaValidationError as e:
+            self.fail("command entries can be duplicate.")
+
 
 class TestHandle(CiTestCase):
 
diff --git a/tests/unittests/test_handler/test_handler_bootcmd.py b/tests/unittests/test_handler/test_handler_bootcmd.py
index 29fc25e..c3abedd 100644
--- a/tests/unittests/test_handler/test_handler_bootcmd.py
+++ b/tests/unittests/test_handler/test_handler_bootcmd.py
@@ -1,6 +1,6 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
-from cloudinit.config import cc_bootcmd
+from cloudinit.config import cc_bootcmd, schema
 from cloudinit.sources import DataSourceNone
 from cloudinit import (distros, helpers, cloud, util)
 from cloudinit.tests.helpers import CiTestCase, mock, skipUnlessJsonSchema
@@ -138,4 +138,26 @@ class TestBootcmd(CiTestCase):
             self.logs.getvalue())
 
 
+class TestSchema(CiTestCase):
+    """Directly test schema rather than through handle."""
+
+    def test_duplicates_are_fine_array_array(self):
+        """Duplicated array entries are allowed."""
+        try:
+            byebye = ["echo", "bye"]
+            schema.validate_cloudconfig_schema(
+                {'bootcmd': [byebye, byebye]}, cc_bootcmd.schema, strict=True)
+        except schema.SchemaValidationError as e:
+            self.fail("runcmd entries as list are allowed to be duplicates.")
+
+    def test_duplicates_are_fine_array_string(self):
+        """Duplicated array entries entries are allowed in values of array."""
+        try:
+            byebye = "echo bye"
+            schema.validate_cloudconfig_schema(
+                {'bootcmd': [byebye, byebye]}, cc_bootcmd.schema, strict=True)
+        except schema.SchemaValidationError as e:
+            self.fail("runcmd entries are allowed to be duplicates.")
+
+
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_handler/test_handler_runcmd.py b/tests/unittests/test_handler/test_handler_runcmd.py
index dbbb271..ee1981d 100644
--- a/tests/unittests/test_handler/test_handler_runcmd.py
+++ b/tests/unittests/test_handler/test_handler_runcmd.py
@@ -1,10 +1,10 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
-from cloudinit.config import cc_runcmd
+from cloudinit.config import cc_runcmd, schema
 from cloudinit.sources import DataSourceNone
 from cloudinit import (distros, helpers, cloud, util)
 from cloudinit.tests.helpers import (
-    FilesystemMockingTestCase, skipUnlessJsonSchema)
+    CiTestCase, FilesystemMockingTestCase, skipUnlessJsonSchema)
 
 import logging
 import os
@@ -99,4 +99,25 @@ class TestRuncmd(FilesystemMockingTestCase):
         self.assertEqual(0o700, stat.S_IMODE(file_stat.st_mode))
 
 
+class TestSchema(CiTestCase):
+    """Directly test schema rather than through handle."""
+
+    def test_duplicates_are_fine_array(self):
+        """Duplicated array entries are allowed."""
+        try:
+            byebye = ["echo", "bye"]
+            schema.validate_cloudconfig_schema(
+                {'runcmd': [byebye, byebye]}, cc_runcmd.schema, strict=True)
+        except schema.SchemaValidationError as e:
+            self.fail("runcmd entries as list are allowed to be duplicates.")
+
+    def test_duplicates_are_fine_string(self):
+        """Duplicated string entries are allowed."""
+        try:
+            byebye = "echo bye"
+            schema.validate_cloudconfig_schema(
+                {'runcmd': [byebye, byebye]}, cc_runcmd.schema, strict=True)
+        except schema.SchemaValidationError as e:
+            self.fail("runcmd entries are allowed to be duplicates.")
+
 # vi: ts=4 expandtab