Merge ~gabrielcocenza/juju-lint:bug/1980019-B into juju-lint:master

[Gabriel Cocenza]

Added check for cloud_type.

- check if cloud_type passed is known.
- in case cloud_type it's not passed in the cli, it tries to detect
  by the charms in the deployment.

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

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

[Eric Chen (eric-chen)]

review: Needs Fixing

[Gabriel Cocenza (gabrielcocenza)]

Thanks for your suggestions Eric.

[Eric Chen (eric-chen)]

review: Approve

[Martin Kalcok (martin-kalcok)]

+1

review: Approve

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

Change successfully merged at revision 428f0c6f11ff1092806d1d09f7b11fa2c53faac4

diff --git a/jujulint/cli.py b/jujulint/cli.py
index 2d76832..97e5a34 100755
--- a/jujulint/cli.py
+++ b/jujulint/cli.py
@@ -64,17 +64,45 @@ class Cli:
             self.logger.error("Cloud not locate rules file {}".format(rules_file))
             sys.exit(1)
 
+    @property
+    def cloud_type(self):
+        """Get the cloud type passed in the CLI.
+
+        :return: cloud-type of the deployment or None.
+        :rtype: str
+        """
+        cloud_type = None
+        if "cloud-type" in self.config:
+            cloud_type = self.config["cloud-type"].get()
+        return cloud_type
+
+    @property
+    def manual_file(self):
+        """Get the manual file passed in the CLI.
+
+        :return: path to manual file to lint or None.
+        :rtype: str
+        """
+        manual_file = None
+        if "manual-file" in self.config:
+            manual_file = self.config["manual-file"].get()
+        return manual_file
+
     def startup_message(self):
         """Print startup message to log."""
         self.logger.info(
             (
                 "juju-lint version {} starting...\n"
                 "\t* Config directory: {}\n"
+                "\t* Cloud type: {}\n"
+                "\t* Manual file: {}\n"
                 "\t* Rules file: {}\n"
                 "\t* Log level: {}\n"
             ).format(
                 self.version,
                 self.config.config_dir(),
+                self.cloud_type or "Unknown",
+                self.manual_file or False,
                 self.lint_rules,
                 self.config["logging"]["loglevel"].get(),
             )
@@ -161,13 +189,8 @@ def main():
     """Program entry point."""
     cli = Cli()
     cli.startup_message()
-    if "manual-file" in cli.config:
-        manual_file = cli.config["manual-file"].get()
-        if "manual-type" in cli.config:
-            manual_type = cli.config["manual-type"].get()
-            cli.audit_file(manual_file, cloud_type=manual_type)
-        else:
-            cli.audit_file(manual_file)
+    if cli.manual_file:
+        cli.audit_file(cli.manual_file, cloud_type=cli.cloud_type)
     elif "clouds" in cli.config:
         cli.audit_all()
     else:
diff --git a/jujulint/config.py b/jujulint/config.py
index d87677a..a4822ee 100644
--- a/jujulint/config.py
+++ b/jujulint/config.py
@@ -80,9 +80,9 @@ class Config(Configuration):
             "-t",
             "--cloud-type",
             help=(
-                "Sets the cloud type when specifying a YAML file to audit with -f or --cloud-file."
+                "Sets the cloud type when specifying a YAML file to audit with -t or --cloud-type."
             ),
-            dest="manual-type",
+            dest="cloud-type",
         )
         self.parser.add_argument(
             "-o",
diff --git a/jujulint/lint.py b/jujulint/lint.py
index 4fb14bb..8f25617 100755
--- a/jujulint/lint.py
+++ b/jujulint/lint.py
@@ -720,6 +720,51 @@ class Linter:
                         }
                     )
 
+    def check_cloud_type(self, deployment_charms):
+        """Check cloud_type or detect depending on the deployed charms.
+
+        :param deployment_charms: charms present in the bundle and/or status.
+        :type deployment_charms: Set
+        """
+        typical_cloud_charms = {
+            "openstack": {
+                "keystone",
+                "nova-compute",
+                "nova-cloud-controller",
+                "glance",
+                "openstack-dashboard",
+                "neutron-api",
+            },
+            "kubernetes": {
+                "kubernetes-worker",
+                "kubernetes-control-plane",
+                "containerd",
+                "calico",
+                "canal",
+                "etcd",
+            },
+        }
+        if self.cloud_type:
+            if self.cloud_type not in typical_cloud_charms.keys():
+                self._log_with_header(
+                    "Cloud type {} is unknown".format(self.cloud_type),
+                    level=logging.WARN,
+                )
+            return
+
+        for cloud_type, charms in typical_cloud_charms.items():
+            match = deployment_charms.intersection(charms)
+            if len(match) >= 2:
+                self._log_with_header(
+                    (
+                        "Setting cloud-type to '{}'. "
+                        "Deployment has these charms: {} that are typically from {}."
+                    ).format(cloud_type, match, cloud_type),
+                    level=logging.WARN,
+                )
+                self.cloud_type = cloud_type
+                return
+
     def check_spaces(self, parsed_yaml):
         """Check that relations end with the same endpoint."""
         space_checks = self.lint_rules.get("space checks", {})
@@ -1133,6 +1178,9 @@ class Linter:
             # Build a list of deployed charms and mapping of charms <-> applications
             self.map_charms(parsed_yaml[applications])
 
+            # Automatically detects cloud type if it's not passed as argument
+            self.check_cloud_type(self.model.charms)
+
             # Parse SAAS / remote-applications
             self.parse_cmr_apps(parsed_yaml)
 
diff --git a/tests/test_jujulint.py b/tests/test_jujulint.py
index a66c174..e99401a 100644
--- a/tests/test_jujulint.py
+++ b/tests/test_jujulint.py
@@ -119,6 +119,42 @@ class TestLinter:
         with pytest.raises(utils.InvalidCharmNameError):
             linter.map_charms(applications)
 
+    def test_check_cloud_type(self, linter, mocker):
+        """Test cloud_type detection on different scenarios."""
+        mock_log = mocker.patch("jujulint.lint.Linter._log_with_header")
+        #  test models with more or equal than two matches
+        model_charms = {
+            "openstack": {"keystone", "nova-compute", "glance", "foo"},
+            "kubernetes": {"kubernetes-worker", "kubernetes-control-plane", "bar"},
+        }
+        # detect cloud_type depending on the deployment charms and
+        # if it's not passe as an argument in the cli.
+        for cloud_type, charms in model_charms.items():
+            linter.cloud_type = None
+            linter.check_cloud_type(charms)
+            assert linter.cloud_type == cloud_type
+
+        # in case cloud_type is passed doesn't change the value
+        linter.cloud_type = "openstack"
+        for cloud_type, charms in model_charms.items():
+            linter.check_cloud_type(charms)
+            assert linter.cloud_type == "openstack"
+
+        # if it's an invalid cloud_type log warn to user
+        linter.cloud_type = "foo-bar"
+        linter.check_cloud_type({"foo", "bar"})
+        mock_log.assert_called_with("Cloud type foo-bar is unknown", level=logging.WARN)
+
+        # test models with less than 2 matches without passing cloud_type in the cli
+        model_charms = {
+            "openstack": {"keystone", "foo", "bar"},
+            "kubernetes": {"foo", "bar"},
+        }
+        for cloud_type, charms in model_charms.items():
+            linter.cloud_type = None
+            linter.check_cloud_type(charms)
+            assert linter.cloud_type is None
+
     def test_juju_status_unexpected(self, linter, juju_status):
         """Test that juju and workload status is expected."""
         # inject invalid status to the application