Merge ~gabrielcocenza/juju-lint:bug/1999263 into juju-lint:master

[Gabriel Cocenza]

Added check for nova-compute ephemeral devices config

This alerts if the nova-compute config option "ephemeral-device" is set
to /dev/sdX or /dev/vgX, to prevent ephemeral devices on landing on a
root drive.

Closes-Bug: #1999263

I tested the change on ps5 (ephemeral-device is set to /dev/disk/..., so it passed without error) and did some local test with only a nova-compute charm:
https://pastebin.canonical.com/p/N6qPQ8KxCG/

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:d974ec7f398904747589e0fa1a3e6b96b5a406a9
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/59/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/2060/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1935/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/59//rebuild

review: Approve (continuous-integration)

[Erhan Sunar (esunar)]

review: Approve

[Gabriel Cocenza (gabrielcocenza)]

Hi Marcus.
Thanks for the patch. The change LGTM, but I think it would be beneficial to have a unit test checking this regex. You can take a look at the unit test code[0] to get some inspiration. Moreover, I think that a "custom-message[1]" would be beneficial for users understand the problem. A message like: "dev/sdX or/dev/vgX should not be used as ephemeral-devices because it can encrypt the root drive"

Thanks

[0]https://git.launchpad.net/juju-lint/tree/tests/unit/test_jujulint.py#n1009
[1] https://git.launchpad.net/juju-lint/tree/contrib/includes/openstack.yaml#n30

review: Needs Information

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:ac767336c90bdc045d991086e0a9246a0b70aae0
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/68/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/2170/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1977/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/68//rebuild

review: Approve (continuous-integration)

[Martin Kalcok (martin-kalcok)]

Please see inline comment. otherwise +1.

review: Needs Fixing

[Martin Kalcok (martin-kalcok)]

Fixed, thanks. +1

review: Approve

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:ea923140c98d70d2597d06543f644807fedb586b
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/69/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/2180/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1987/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/69//rebuild

review: Approve (continuous-integration)

[Tianqi Xiao (txiao)]

LGTM

review: Approve

[Erhan Sunar (esunar)]

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 3313998dfb8ea65a9f5d4e5043729beff558f598

diff --git a/contrib/includes/openstack.yaml b/contrib/includes/openstack.yaml
index 5814fac..4f8ee2e 100644
--- a/contrib/includes/openstack.yaml
+++ b/contrib/includes/openstack.yaml
@@ -29,6 +29,10 @@ openstack config base: &openstack-config-base
       log-level: warning
       custom-message:
         Default 4 workers is too low for production clouds.
+    ephemeral-device:
+      search: "^((?!/dev/vg|/dev/sd).)*$"
+      custom-message:
+        dev/sdX or/dev/vgX should not be used as ephemeral-devices. See lp#1999263
   rabbitmq-server:
     cluster-partition-handling:
       eq: "pause_minority"
diff --git a/jujulint/lint.py b/jujulint/lint.py
index 54267b2..d263feb 100755
--- a/jujulint/lint.py
+++ b/jujulint/lint.py
@@ -1397,7 +1397,6 @@ class Linter:
         input_file = input_handler(parsed_yaml, applications)
 
         if applications in parsed_yaml:
-
             # Build a list of deployed charms and mapping of charms <-> applications
             self.map_charms(parsed_yaml[applications])
 
diff --git a/tests/unit/test_jujulint.py b/tests/unit/test_jujulint.py
index 8970d29..c0f7c7a 100644
--- a/tests/unit/test_jujulint.py
+++ b/tests/unit/test_jujulint.py
@@ -1075,6 +1075,34 @@ applications:
                 errors[0]["actual_value"] == "[[/, queue1, 10, 20], [\\*, \\*, 10, 20]]"
             )
 
+    @pytest.mark.parametrize(
+        "block_device, show_error",
+        [("/dev/vga", True), ("/dev/sda", True), ("/dev/foo", False)],
+    )
+    def test_config_search_ephemeral_device(
+        self, linter, juju_status, block_device, show_error
+    ):
+        """Test the config ephemeral-device regex pattern."""
+        custom_msg = (
+            "dev/sdX or/dev/vgX should not be used as ephemeral-devices. See lp#1999263"
+        )
+        linter.lint_rules["config"] = {
+            "ubuntu": {
+                "fake_opt": {
+                    "search": "^((?!/dev/vg|/dev/sd).)*$",
+                    "custom-message": custom_msg,
+                }
+            }
+        }
+        juju_status["applications"]["ubuntu"]["options"] = {"fake_opt": block_device}
+        linter.do_lint(juju_status)
+        errors = linter.output_collector["errors"]
+        if show_error:
+            assert len(errors) == 1
+            assert errors[0]["message"] == custom_msg
+        else:
+            assert len(errors) == 0
+
     def test_config_search_missing(self, linter, mocker):
         """Test the config search method logs warning if the config option is missing."""
         app_name = "ubuntu"