Merge ~jfguedez/juju-lint:bug/1896551 into juju-lint:master

[Jose Guedez]

Add support for includes, as well as several reference rules files

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

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

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

Merge proposal is approved, but source revision has changed, setting status to needs review.

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

Change successfully merged at revision 737b1c02c3a8cbfe2100d9f7d8f275e53c34d9e5

diff --git a/README.md b/README.md
index 00d737d..d7ef4c7 100644
--- a/README.md
+++ b/README.md
@@ -2,7 +2,7 @@
 
 == Introduction ==
 
-This is intended to be run against a yaml dump of Juju status, a YAML dump of 
+This is intended to be run against a yaml dump of Juju status, a YAML dump of
 a juju bundle or a remote cloud or clouds via SSH.
 
 To generate a status if you just want to audit placement:
@@ -49,7 +49,8 @@ Supported top-level options for your rules file:
  3. `operations [mandatory|optional|subordinate]`
  4. `openstack [mandatory|optional|subordinate]`
  5. `config` - application configuration auditing
- 5. `[openstack|kubernetes] config` - config auditing for specific cloud types.
+ 6. `[openstack|kubernetes] config` - config auditing for specific cloud types.
+ 7. `!include <relative path>` - Extension to yaml to include files.
 
 == License ==
 
diff --git a/contrib/canonical-rules.yaml b/contrib/canonical-rules.yaml
index 6c1a101..b655f34 100644
--- a/contrib/canonical-rules.yaml
+++ b/contrib/canonical-rules.yaml
@@ -1,4 +1,5 @@
 ---
+# legacy rules file, there are also more specific configurations available
 kubernetes config:
  kubernetes-master:
   authorization-mode:
diff --git a/contrib/includes/aggregator-openstack.yaml b/contrib/includes/aggregator-openstack.yaml
new file mode 100644
index 0000000..6982a91
--- /dev/null
+++ b/contrib/includes/aggregator-openstack.yaml
@@ -0,0 +1,20 @@
+operations charms: &operations-charms
+ - *operations-mandatory-charms
+ - *operations-mandatory-deps
+ - *operations-mandatory-subs
+ - *operations-optional-charms
+ - *operations-optional-subs
+ - *operations-openstack-mandatory-charms
+
+openstack charms: &openstack-charms
+ - *openstack-mandatory-charms
+ - *openstack-mandatory-deps
+ - *openstack-mandatory-subs
+ - *openstack-optional-charms
+ - *cisco-aci-charms
+ - *trilio-charms
+
+known charms:
+ - ubuntu
+ - *openstack-charms
+ - *operations-charms
diff --git a/contrib/includes/base.yaml b/contrib/includes/base.yaml
new file mode 100644
index 0000000..1815ee2
--- /dev/null
+++ b/contrib/includes/base.yaml
@@ -0,0 +1,40 @@
+config:
+ hacluster:
+  cluster_count:
+   gte: 3
+ ntp:
+  auto_peers:
+   eq: false
+ nrpe:
+  lacp_bonds:
+   neq: ""
+  netlinks:
+   neq: ""
+ landscape-client:
+  disable-unattended-upgrades:
+   eq: true
+
+subordinates:
+ telegraf:
+  where: all except prometheus
+  # and prometheus-ceph-exporter and prometheus-openstack-exporter
+ landscape-client:
+  where: all except landscape-server
+ filebeat:
+  where: all except graylog
+ canonical-livepatch:
+  where: host only
+ nrpe:
+  where: container aware
+  host-suffixes: [host, physical, guest]
+  container-suffixes: [lxd, container]
+  exceptions: [nagios]
+ ntp:
+  # You don't want NTP in a container dueling with ntp in the host
+  where: host only
+ thruk-agent:
+  where: on nagios
+ hw-health:
+  where: host only
+ logrotated:
+  where: all
diff --git a/contrib/includes/database/mysql.yaml b/contrib/includes/database/mysql.yaml
new file mode 100644
index 0000000..ef827e3
--- /dev/null
+++ b/contrib/includes/database/mysql.yaml
@@ -0,0 +1,10 @@
+openstack database config: &openstack-config-database
+ mysql-innodb-cluster:
+  innodb-buffer-pool-size:
+   gte: 6G
+  max-connections:
+   gte: 2000
+
+openstack mandatory deps database: &openstack-mandatory-deps-database
+ - mysql-innodb-cluster
+ - mysql-router
diff --git a/contrib/includes/database/percona-cluster.yaml b/contrib/includes/database/percona-cluster.yaml
new file mode 100644
index 0000000..7102f52
--- /dev/null
+++ b/contrib/includes/database/percona-cluster.yaml
@@ -0,0 +1,9 @@
+openstack database config: &openstack-config-database
+ percona-cluster:
+  innodb-buffer-pool-size:
+   gte: 6G
+  max-connections:
+   gte: 2000
+
+openstack mandatory deps database: &openstack-mandatory-deps-database
+ - percona-cluster
diff --git a/contrib/includes/kubernetes.yaml b/contrib/includes/kubernetes.yaml
new file mode 100644
index 0000000..02d5fa0
--- /dev/null
+++ b/contrib/includes/kubernetes.yaml
@@ -0,0 +1,24 @@
+kubernetes config:
+ kubernetes-master:
+  authorization-mode:
+   eq: "RBAC,Node"
+ canal:
+  cidr:
+   isset: false
+
+kubernetes mandatory: &kubernetes-mandatory-charms
+ - containerd
+ - kubeapi-load-balancer
+ - kubernetes-master
+ - kubernetes-worker
+
+kubernetes optional charms: &kubernetes-optional-charms
+ - calico
+ - canal
+ - coredns
+ - easyrsa
+ - etcd
+ - flannel
+ - kubernetes-dashboard
+ - openstack-integrator
+ - vsphere-integrator
diff --git a/contrib/includes/networking/ovn.yaml b/contrib/includes/networking/ovn.yaml
new file mode 100644
index 0000000..b4f4140
--- /dev/null
+++ b/contrib/includes/networking/ovn.yaml
@@ -0,0 +1,9 @@
+openstack config networking: &openstack-config-networking
+ ovn-central:
+  ovsdb-server-election-timer:
+   gte: 4
+
+openstack mandatory charms networking: &openstack-mandatory-charms-networking
+ - ovn-central
+ - ovn-chassis
+ - neutron-api-plugin-ovn
diff --git a/contrib/includes/networking/ovs.yaml b/contrib/includes/networking/ovs.yaml
new file mode 100644
index 0000000..493195e
--- /dev/null
+++ b/contrib/includes/networking/ovs.yaml
@@ -0,0 +1,4 @@
+openstack config networking: &openstack-config-networking {}
+
+openstack mandatory charms networking: &openstack-mandatory-charms-networking
+ - neutron-openvswitch
diff --git a/contrib/includes/openstack.yaml b/contrib/includes/openstack.yaml
new file mode 100644
index 0000000..5235aea
--- /dev/null
+++ b/contrib/includes/openstack.yaml
@@ -0,0 +1,105 @@
+openstack config base: &openstack-config-base
+ ceph-radosgw:
+  ceph-osd-replication-count:
+   eq: 3
+ cinder-ceph:
+  ceph-osd-replication-count:
+   eq: 3
+ glance:
+  ceph-osd-replication-count:
+   eq: 3
+ neutron-api:
+  path-mtu:
+   eq: 9000
+  global-physnet-mtu:
+   eq: 9000
+ nova-compute:
+  cpu-model:
+   neq: ""
+  ceph-osd-replication-count:
+   eq: 3
+ rabbitmq-server:
+  cluster-partition-handling:
+   eq: "pause_minority"
+ keystone:
+  token-expiration:
+   gte: 86400
+ sysconfig:
+  governor:
+   eq: "performance"
+
+openstack config:
+ << : [ *openstack-config-base, *openstack-config-networking, *openstack-config-database ]
+
+openstack mandatory: &openstack-mandatory-charms
+ - ceilometer
+ - ceilometer-agent
+ - ceph-mon
+ - ceph-osd
+ - cinder
+ - cinder-ceph
+ - glance
+ - heat
+ - keystone
+ - neutron-api
+ - nova-cloud-controller
+ - nova-compute
+ - openstack-dashboard
+ - *openstack-mandatory-charms-networking
+
+openstack mandatory deps base: &openstack-mandatory-deps-base
+ - haproxy
+ - memcached
+ - rabbitmq-server
+
+openstack mandatory deps: &openstack-mandatory-deps
+ - *openstack-mandatory-deps-base
+ - *openstack-mandatory-deps-database
+
+openstack mandatory subordinates: &openstack-mandatory-subs
+ - hacluster
+
+openstack optional charms: &openstack-optional-charms
+ - aodh
+ - barbican
+ - barbican-vault
+ - ceph-fs
+ - ceph-radosgw
+ - cinder-backup
+ - designate
+ - designate-bind
+ - easyrsa
+ - etcd
+ - glance-simplestreams-sync
+ - glance-sync-slave
+ - gnocchi
+ - ironic-api
+ - ironic-conductor
+ - keystone-ldap
+ - manila
+ - manila-dashboard
+ - manila-ganesha
+ - masakari
+ - masakari-monitors
+ - mongodb  # Optional since Gnocchi
+ - neutron-gateway
+ - neutron-api-plugin-ironic
+ - octavia
+ - octavia-dashboard
+ - octavia-diskimage-retrofit
+ - pacemaker-remote
+ - placement
+ - swift-proxy
+ - swift-storage
+ - vault
+ - cinder-lvm
+
+cisco-aci-charms: &cisco-aci-charms
+ - neutron-api-plugin-aci
+ - openstack-dashboard-plugin-gbp
+
+trilio-charms: &trilio-charms
+ - trilio-dm-api
+ - trilio-horizon-plugin
+ - trilio-data-mover
+ - trilio-wlm
diff --git a/contrib/includes/operations.yaml b/contrib/includes/operations.yaml
new file mode 100644
index 0000000..40b628b
--- /dev/null
+++ b/contrib/includes/operations.yaml
@@ -0,0 +1,49 @@
+operations mandatory: &operations-mandatory-charms
+ - elasticsearch
+ - grafana
+ - graylog
+ - landscape-server
+ - nagios
+ - prometheus2
+
+operations optional: &operations-optional-charms
+ - infra-node
+ - cloudstats
+ - juju-lint
+
+operations openstack mandatory base: &operations-openstack-mandatory-base
+ - openstack-service-checks
+ - prometheus-libvirt-exporter
+ - prometheus-openstack-exporter
+ - prometheus-grok-exporter
+
+operations openstack mandatory: &operations-openstack-mandatory-charms
+ - *operations-openstack-mandatory-base
+ - *operations-openstack-mandatory-series
+
+operations mandatory dependencies: &operations-mandatory-deps
+ - postgresql
+
+operations subordinates: &operations-mandatory-subs
+ - canonical-livepatch
+ - filebeat
+ - ksplice
+ - landscape-client
+ - lldpd
+ - nrpe
+ - ntp
+ - telegraf
+ - thruk-agent
+ - hw-health
+ - logrotated
+
+operations optional subordinates: &operations-optional-subs
+ - policy-routing
+ - bcache-tuning
+ - sysconfig
+ - logrotate-charm
+ - advanced-routing
+ - rsyslog-forwarder-ha
+
+operations kubernetes mandatory: &operations-kubernetes-mandatory-charms
+ - kubernetes-service-checks
diff --git a/contrib/includes/operations/bionic.yaml b/contrib/includes/operations/bionic.yaml
new file mode 100644
index 0000000..a6baf9e
--- /dev/null
+++ b/contrib/includes/operations/bionic.yaml
@@ -0,0 +1,2 @@
+operations openstack mandatory series: &operations-openstack-mandatory-series
+ - prometheus-ceph-exporter
diff --git a/contrib/includes/operations/focal.yaml b/contrib/includes/operations/focal.yaml
new file mode 100644
index 0000000..d658661
--- /dev/null
+++ b/contrib/includes/operations/focal.yaml
@@ -0,0 +1 @@
+operations openstack mandatory series: &operations-openstack-mandatory-series []
diff --git a/contrib/includes/saas.yaml b/contrib/includes/saas.yaml
new file mode 100644
index 0000000..a3ed08f
--- /dev/null
+++ b/contrib/includes/saas.yaml
@@ -0,0 +1,7 @@
+saas:
+ - elasticsearch
+ - grafana
+ - graylog
+ - landscape-server
+ - nagios
+ - prometheus2
diff --git a/contrib/kubernetes.yaml b/contrib/kubernetes.yaml
new file mode 100644
index 0000000..c7460b1
--- /dev/null
+++ b/contrib/kubernetes.yaml
@@ -0,0 +1,22 @@
+---
+!include includes/base.yaml
+!include includes/operations.yaml
+!include includes/kubernetes.yaml
+!include includes/saas.yaml
+
+operations charms: &operations-charms
+ - *operations-kubernetes-mandatory-charms
+ - *operations-mandatory-charms
+ - *operations-mandatory-deps
+ - *operations-mandatory-subs
+ - *operations-optional-charms
+ - *operations-optional-subs
+
+kubernetes charms: &kubernetes-charms
+ - *kubernetes-mandatory-charms
+ - *kubernetes-optional-charms
+
+known charms:
+ - ubuntu
+ - *operations-charms
+ - *kubernetes-charms
diff --git a/contrib/openstack-bionic-ovn.yaml b/contrib/openstack-bionic-ovn.yaml
new file mode 100644
index 0000000..d1a600c
--- /dev/null
+++ b/contrib/openstack-bionic-ovn.yaml
@@ -0,0 +1,13 @@
+---
+# Openstack rule set for a Bionic OVN Cloud
+# * Use OVN, not OVS
+# * Uses percona-cluster, not mysql-innodb-cluster/router
+# * Includes prometheus-ceph-exporter
+!include includes/base.yaml
+!include includes/networking/ovn.yaml
+!include includes/database/percona-cluster.yaml
+!include includes/operations/bionic.yaml
+!include includes/openstack.yaml
+!include includes/operations.yaml
+!include includes/saas.yaml
+!include includes/aggregator-openstack.yaml
diff --git a/contrib/openstack-bionic-ovs.yaml b/contrib/openstack-bionic-ovs.yaml
new file mode 100644
index 0000000..60304e3
--- /dev/null
+++ b/contrib/openstack-bionic-ovs.yaml
@@ -0,0 +1,13 @@
+---
+# Openstack rule set for a Bionic OVS Cloud
+# * Use OVS, not OVN
+# * Uses percona-cluster, not mysql-innodb-cluster/router
+# * Includes prometheus-ceph-exporter
+!include includes/base.yaml
+!include includes/networking/ovs.yaml
+!include includes/database/percona-cluster.yaml
+!include includes/operations/bionic.yaml
+!include includes/openstack.yaml
+!include includes/operations.yaml
+!include includes/saas.yaml
+!include includes/aggregator-openstack.yaml
diff --git a/contrib/openstack-focal-ovn.yaml b/contrib/openstack-focal-ovn.yaml
new file mode 100644
index 0000000..3c0a562
--- /dev/null
+++ b/contrib/openstack-focal-ovn.yaml
@@ -0,0 +1,13 @@
+---
+# Openstack rule set for a Focal OVN Cloud
+# * Use OVN, not OVS
+# * Uses mysql-innodb-cluster/router, not percona-cluster
+# * No prometheus-ceph-exporter
+!include includes/base.yaml
+!include includes/networking/ovn.yaml
+!include includes/database/mysql.yaml
+!include includes/operations/focal.yaml
+!include includes/openstack.yaml
+!include includes/operations.yaml
+!include includes/saas.yaml
+!include includes/aggregator-openstack.yaml
diff --git a/contrib/openstack-focal-ovs.yaml b/contrib/openstack-focal-ovs.yaml
new file mode 100644
index 0000000..e1039cb
--- /dev/null
+++ b/contrib/openstack-focal-ovs.yaml
@@ -0,0 +1,13 @@
+---
+# Openstack rule set for a Focal OVS Cloud
+# * Use OVS, not OVN
+# * Uses mysql-innodb-cluster/router, not percona-cluster
+# * No prometheus-ceph-exporter
+!include includes/base.yaml
+!include includes/networking/ovs.yaml
+!include includes/database/mysql.yaml
+!include includes/operations/focal.yaml
+!include includes/openstack.yaml
+!include includes/operations.yaml
+!include includes/saas.yaml
+!include includes/aggregator-openstack.yaml
diff --git a/jujulint/lint.py b/jujulint/lint.py
index 3add8bf..38dfc00 100755
--- a/jujulint/lint.py
+++ b/jujulint/lint.py
@@ -118,8 +118,11 @@ class Linter:
     def read_rules(self):
         """Read and parse rules from YAML, optionally processing provided overrides."""
         if os.path.isfile(self.filename):
-            with open(self.filename, "r") as yaml_file:
-                self.lint_rules = yaml.safe_load(yaml_file)
+            with open(self.filename, "r") as rules_file:
+                raw_rules_txt = rules_file.read()
+
+            self.lint_rules = self._process_includes_in_rules(raw_rules_txt)
+
             if self.overrides:
                 for override in self.overrides.split("#"):
                     (name, where) = override.split(":")
@@ -133,9 +136,10 @@ class Linter:
                         )
                     )
                     self.lint_rules["subordinates"][name] = dict(where=where)
-            self.lint_rules["known charms"] = flatten_list(
-                self.lint_rules["known charms"]
-            )
+
+            # Flatten all entries (to account for nesting due to YAML anchors (templating)
+            self.lint_rules = {k: flatten_list(v) for k, v in self.lint_rules.items()}
+
             self.logger.debug(
                 "[{}] [{}/{}] Lint Rules: {}".format(
                     self.cloud_name,
@@ -1163,3 +1167,32 @@ class Linter:
         )
         if self.collect_errors:
             self.collect(error)
+
+    def _process_includes_in_rules(self, yaml_txt):
+        """
+        Process any includes in the rules file.
+
+        Only top level includes are supported (without recursion), with relative paths.
+
+        Example syntax:
+
+        !include foo.yaml
+        """
+        collector = []
+        for line in yaml_txt.splitlines():
+            if line.startswith("!include"):
+                try:
+                    _, rel_path = line.split()
+                except ValueError:
+                    self.logger.warn("invalid include in rules, ignored: '{}'".format(line))
+                    continue
+
+                include_path = os.path.join(os.path.dirname(self.filename), rel_path)
+
+                if os.path.isfile(include_path):
+                    with open(include_path, "r") as f:
+                        collector.append(f.read())
+            else:
+                collector.append(line)
+
+        return yaml.safe_load("\n".join(collector))
diff --git a/jujulint/util.py b/jujulint/util.py
index b8c1359..c6eb80e 100644
--- a/jujulint/util.py
+++ b/jujulint/util.py
@@ -29,6 +29,10 @@ class InvalidCharmNameError(Exception):
 
 def flatten_list(lumpy_list):
     """Flatten a list potentially containing other lists."""
+    # Ensure we only operate on lists, otherwise will affect other iterables
+    if not isinstance(lumpy_list, list):
+        return lumpy_list
+
     flat_list = []
     for item in lumpy_list:
         if not isinstance(item, list):
diff --git a/tests/test_jujulint.py b/tests/test_jujulint.py
index 1e200d2..ce123ee 100644
--- a/tests/test_jujulint.py
+++ b/tests/test_jujulint.py
@@ -16,6 +16,12 @@ def test_flatten_list(utils):
     assert flattened_list == utils.flatten_list(unflattened_list)
 
 
+def test_flatten_list_non_list_iterable(utils):
+    """Test the utils flatten_list function."""
+    iterable = {1: 2}
+    assert iterable == utils.flatten_list(iterable)
+
+
 def test_map_charms(linter, utils):
     """Test the charm name validation code."""
     applications = {
@@ -434,3 +440,24 @@ class TestLinter:
         assert error is not None
         assert error["id"] == "ops-charm-missing"
         assert error["charm"] == "grafana"
+
+    def test_read_rules_plain_yaml(self, linter, tmp_path):
+        """Test that a simple rules YAML is imported as expected."""
+        rules_path = tmp_path / "rules.yaml"
+        rules_path.write_text('---\nkey:\n "value"')
+
+        linter.filename = str(rules_path)
+        linter.read_rules()
+        assert linter.lint_rules == {"key": "value"}
+
+    def test_read_rules_include(self, linter, tmp_path):
+        """Test that rules YAML with an include is imported as expected."""
+        include_path = tmp_path / "include.yaml"
+        include_path.write_text('key-inc:\n "value2"')
+
+        rules_path = tmp_path / "rules.yaml"
+        rules_path.write_text('---\n!include include.yaml\nkey:\n "value"')
+
+        linter.filename = str(rules_path)
+        linter.read_rules()
+        assert linter.lint_rules == {"key": "value", "key-inc": "value2"}