Merge ~mertkirpici/juju-lint:lp/1987359 into juju-lint:master

[Mert Kirpici]

Close LP #1987359

lint: add support for fetching rules over url

This patch extends the functionality of juju-lint in several ways:

- juju-lint now accepts a comma separated rules file list to be given through the command line and processes them in the order that they are given to update the rules. i.e.

    $ juju-lint -c ./rules-1.yaml,./rules-2.yaml ...

if the rules-2.yaml file overrides an option, in the resulting rules
dictionary, it will take precedence. Note that this is a deep update
operation, which means, it will follow through the nested dictionaries.
For implementation details, see jujulint.util.deep_update().

- juju-lint now accepts urls as rules file command line arguments. i.e.

    $ juju-lint -c https://git.launchpad.net/myrepo/plain/rules.yaml ..

the underlying implementation utilizes urllib which is part of the
python stdlib. Therefore what schemas it supports, we should be able to
support given that the resource is accessible to us.

[🤖 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:70782cd290f2a446d147432d39570244ff8ec101
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/62/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/2104/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1943/

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

review: Approve (continuous-integration)

[Robert Gildein (rgildein)]

2 not-blocking comments and one small issue.

[Gabriel Cocenza (gabrielcocenza)]

Thanks for implementing this feature.
I've left some comments and suggestions in the code

review: Needs Information

[Andrea Ieri (aieri)]

Please make sure this commit is merged to both master (principal implementation), and whichever branch MP#436414 gets merged to (subordinate implementation). Any future development should go to the subordinate implementation.

I'll also link this MP to the LP bug, the link is currently missing.

[Gabriel Cocenza (gabrielcocenza)]

@aieri I think this change it's not suppose to affect MP#436414. This change it's on the snap package itself and not in the charm. However, the cham-juju-lint should be prepared to receive multiple rules file in the "lint-config".

[Mert Kirpici (mertkirpici)]

Hi all,
I tried to address everything in the update(s), which I will push in a sec.

Also with Gabriel's suggestion to implement this feature as a comma separated list rather than specifying the "-c" command line option multiple times; i.e.

  $ juju-lint -c "file1.yaml,https://file2.yaml" ...

I think we can avoid code change in the charm-juju-lint completely. Just a description change in the config.yaml suggestion should be enough.

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

PASSED: Continuous integration, rev:d7316ceeb18a25b0bf506bf835707902ff249605
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/63/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/2117/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1957/

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

review: Approve (continuous-integration)

[Erhan Sunar (esunar)]

LGTM

review: Approve

[Gabriel Cocenza (gabrielcocenza)]

Thanks for the update. I've left couple more points that I think we can improve.

review: Needs Information

[Mert Kirpici (mertkirpici)]

Hi Gabriel, thanks for the review!
I tried to address/answer the issues you raised in the update.

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

PASSED: Continuous integration, rev:8e08adef109d39d9f30bd339126b5ad49a12550b
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/64/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/2152/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1960/

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

review: Approve (continuous-integration)

[Gabriel Cocenza (gabrielcocenza)]

Just one more little thing regarding the urlopen and we will be ready to merge.

[Mert Kirpici (mertkirpici)]

Applied the update. We should be good to go after the last CI run

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

PASSED: Continuous integration, rev:30a938c9789e6877499dad75d3b4df97f09b072c
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/65/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/2153/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1961/

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:e741ba4c72865aafb020f59f30609550c90e192e
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/66/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/2157/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1964/

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:4c85ec66e9f542ce2c0492779043768fa27d9d80
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/67/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/2159/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1966/

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

review: Approve (continuous-integration)

[Gabriel Cocenza (gabrielcocenza)]

LGTM

review: Approve

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

Change successfully merged at revision 07d72e116d6e70ea3a7d6d13c7d405fdf6db3b35

diff --git a/jujulint/cli.py b/jujulint/cli.py
index ac85b2b..0c3c711 100755
--- a/jujulint/cli.py
+++ b/jujulint/cli.py
@@ -30,6 +30,7 @@ from jujulint.config import Config
 from jujulint.lint import Linter
 from jujulint.logging import Logger
 from jujulint.openstack import OpenStack
+from jujulint.util import is_url
 
 
 class Cli:
@@ -55,16 +56,7 @@ class Cli:
         except pkg_resources.DistributionNotFound:
             self.version = "unknown"
 
-        rules_file = self.config["rules"]["file"].get()
-        # handle absolute path provided
-        if os.path.isfile(rules_file):
-            self.lint_rules = rules_file
-        elif os.path.isfile("{}/{}".format(self.config.config_dir(), rules_file)):
-            # default to relative path
-            self.lint_rules = "{}/{}".format(self.config.config_dir(), rules_file)
-        else:
-            self.logger.error("Cloud not locate rules file {}".format(rules_file))
-            sys.exit(1)
+        self.rules_files = self.validate_rules_file_args()
 
     @property
     def cloud_type(self):
@@ -90,6 +82,42 @@ class Cli:
             manual_file = self.config["manual-file"].get()
         return manual_file
 
+    def validate_rules_file_args(self):
+        """Validate the given rules file arguments.
+
+        :return: a list of validated and slightly adjusted
+                 paths/urls to the rules files.
+        :rtype: list
+        """
+        rules_file_args = [
+            rules_file.strip()
+            for rules_file in self.config["rules"]["file"].get().split(",")
+            if rules_file.strip()
+        ]
+        validated_rules_file_args = []
+
+        for arg in rules_file_args:
+            # does not say anything about accessibility of the resource
+            # pointed to by the url. we are just checking if the url
+            # is well formed.
+            if is_url(arg):
+                validated_rules_file_args.append(arg)
+
+            # absolute path provided
+            elif os.path.isfile(arg):
+                validated_rules_file_args.append(arg)
+
+            # default to relative path
+            elif os.path.isfile("{}/{}".format(self.config.config_dir(), arg)):
+                validated_rules_file_args.append(
+                    "{}/{}".format(self.config.config_dir(), arg)
+                )
+            else:
+                self.logger.error("Cloud not locate rules file {}".format(arg))
+                sys.exit(1)
+
+        return validated_rules_file_args
+
     def startup_message(self):
         """Print startup message to log."""
         self.logger.info(
@@ -98,14 +126,14 @@ class Cli:
                 "\t* Config directory: {}\n"
                 "\t* Cloud type: {}\n"
                 "\t* Manual file: {}\n"
-                "\t* Rules file: {}\n"
+                "\t* Rules files: {}\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.rules_files,
                 self.config["logging"]["loglevel"].get(),
             )
         )
@@ -119,11 +147,12 @@ class Cli:
         self.logger.debug("Starting audit of file {}".format(filename))
         linter = Linter(
             filename,
-            self.lint_rules,
+            self.rules_files,
             cloud_type=cloud_type,
             output_format=self.output_format,
         )
-        linter.read_rules()
+        if not linter.read_rules():
+            Logger.fubar("Error while reading the rules. Exiting...")
         self.logger.info("[{}] Linting manual file...".format(filename))
         linter.lint_yaml_file(filename)
 
@@ -160,7 +189,7 @@ class Cli:
                 access_method=access_method,
                 ssh_host=ssh_host,
                 sudo_user=sudo_user,
-                lint_rules=self.lint_rules,
+                lint_rules=self.rules_files,
             )
         # refresh information
         result = cloud_instance.refresh()
diff --git a/jujulint/config.py b/jujulint/config.py
index be2453e..16bb33d 100644
--- a/jujulint/config.py
+++ b/jujulint/config.py
@@ -65,7 +65,12 @@ class Config(Configuration):
         self.parser.add_argument(
             "-c",
             "--config",
-            help="File to read lint rules from. Defaults to `lint-rules.yaml`",
+            help=(
+                "File to read lint rules from. Defaults to `lint-rules.yaml`. "
+                "Also supports urls and comma separated multiple files. "
+                "Note that if multiple files given, rules will be merged and existing "
+                "rules will be overriden."
+            ),
             dest="rules.file",
         )
         self.parser.add_argument(
diff --git a/jujulint/lint.py b/jujulint/lint.py
index 4219b2f..54267b2 100755
--- a/jujulint/lint.py
+++ b/jujulint/lint.py
@@ -26,6 +26,8 @@ import pprint
 import re
 import traceback
 from datetime import datetime, timezone
+from urllib.error import HTTPError, URLError
+from urllib.request import urlopen
 
 import attr
 import dateutil.parser
@@ -106,7 +108,7 @@ class Linter:
     def __init__(
         self,
         name,
-        filename,
+        rules_files,
         controller_name="manual",
         model_name="manual",
         overrides=None,
@@ -117,7 +119,7 @@ class Linter:
         self.logger = Logger()
         self.lint_rules = {}
         self.model = ModelInfo()
-        self.filename = filename
+        self.rules_files = rules_files
         self.overrides = overrides
         self.cloud_name = name
         self.cloud_type = cloud_type
@@ -130,7 +132,7 @@ class Linter:
             "name": name,
             "controller": controller_name,
             "model": model_name,
-            "rules": filename,
+            "rules": rules_files,
             "errors": [],
         }
 
@@ -144,31 +146,50 @@ 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 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(":")
-                    self._log_with_header(
-                        "Overriding {} with {}".format(name, where), level=logging.INFO
+        for rules_file in self.rules_files:
+            if utils.is_url(rules_file):
+                try:
+                    with urlopen(rules_file, timeout=10) as response:
+                        raw_rules_txt = response.read().decode()
+                except (HTTPError, URLError) as error:
+                    self.logger.error(
+                        "Failed to fetch url: {} reason: {}".format(
+                            rules_file, error.reason
+                        )
                     )
-                    self.lint_rules["subordinates"][name] = dict(where=where)
+                    return False
+                except TimeoutError:
+                    self.logger.error(
+                        "Failed to fetch url: {} reason: TimeoutError".format(
+                            rules_file
+                        )
+                    )
+                    return False
+            elif os.path.isfile(rules_file):
+                with open(rules_file, "r") as f:
+                    raw_rules_txt = f.read()
+            else:
+                self.logger.error("Rules file {} does not exist.".format(rules_file))
+                return False
+
+            lint_rules = self._process_includes_in_rules(raw_rules_txt, rules_file)
 
             # Flatten all entries (to account for nesting due to YAML anchors (templating)
-            self.lint_rules = {
-                k: utils.flatten_list(v) for k, v in self.lint_rules.items()
-            }
+            lint_rules = {k: utils.flatten_list(v) for k, v in lint_rules.items()}
 
-            self._log_with_header(
-                "Lint Rules: {}".format(pprint.pformat(self.lint_rules))
-            )
-            return True
-        self.logger.error("Rules file {} does not exist.".format(self.filename))
-        return False
+            # Update the current rules with the new ones
+            self.lint_rules = utils.deep_update(self.lint_rules, lint_rules)
+
+        if self.overrides:
+            for override in self.overrides.split("#"):
+                (name, where) = override.split(":")
+                self._log_with_header(
+                    "Overriding {} with {}".format(name, where), level=logging.INFO
+                )
+                self.lint_rules["subordinates"][name] = dict(where=where)
+
+        self._log_with_header("Lint Rules: {}".format(pprint.pformat(self.lint_rules)))
+        return True
 
     def process_subordinates(self, app_d, app_name):
         """Iterate over subordinates and run subordinate checks."""
@@ -1454,7 +1475,7 @@ class Linter:
         if self.collect_errors and log_level == logging.ERROR:
             self.collect(message)
 
-    def _process_includes_in_rules(self, yaml_txt):
+    def _process_includes_in_rules(self, yaml_txt, filename):
         """
         Process any includes in the rules file.
 
@@ -1475,7 +1496,7 @@ class Linter:
                     )
                     continue
 
-                include_path = os.path.join(os.path.dirname(self.filename), rel_path)
+                include_path = os.path.join(os.path.dirname(filename), rel_path)
 
                 if os.path.isfile(include_path):
                     with open(include_path, "r") as f:
diff --git a/jujulint/util.py b/jujulint/util.py
index 3b24419..0224fff 100644
--- a/jujulint/util.py
+++ b/jujulint/util.py
@@ -19,7 +19,10 @@
 """Utility library for all helpful functions this project uses."""
 
 import argparse
+import collections
 import re
+from copy import deepcopy
+from urllib.parse import urlparse
 
 from jujulint.logging import Logger
 
@@ -45,6 +48,28 @@ def flatten_list(lumpy_list):
     return flat_list
 
 
+def deep_update(existing, new):
+    """Deep update an existing dictionary with new dictionary."""
+    result = deepcopy(existing)
+
+    def _deep_update_inplace(_existing, _new):
+        """Perform an in-place recursive deep update of two dictionaries."""
+        for key, val in _new.items():
+            if isinstance(val, collections.abc.Mapping):
+                _existing[key] = _deep_update_inplace(_existing.get(key, {}), val)
+            else:
+                _existing[key] = val
+        return _existing
+
+    return _deep_update_inplace(result, new)
+
+
+def is_url(string):
+    """Determine if a string is a url."""
+    result = urlparse(string)
+    return result.scheme and result.netloc
+
+
 def is_container(machine):
     """Check if a provided machine is a container."""
     if "lxd/" in machine:
diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py
index 6620b31..c1fae66 100644
--- a/tests/functional/conftest.py
+++ b/tests/functional/conftest.py
@@ -82,6 +82,40 @@ def rules_file(basedir):
 
 
 @pytest.fixture
+def rules_file_url(httpserver):
+    """Return a http url for a rules file.
+
+    This fixture will make use of the pytest-httpserver
+    plugin and fire up a HTTP server locally for the juju-lint
+    application to call urlopen() against. It also temporarily
+    makes necessary changes to bypass the proxy if necessary.
+    """
+    saved_no_proxy = os.environ.get("no_proxy", "")
+    if "localhost" not in saved_no_proxy.split(","):
+        os.environ["no_proxy"] = saved_no_proxy + ",localhost"
+
+    endpoint = "/rules.yaml"
+    rules_file_content = dedent(
+        """
+        openstack config:
+            mysql-innodb-cluster:
+                max-connections:
+                    gte: 99999
+        """
+    )
+    httpserver.expect_request(endpoint).respond_with_data(
+        response_data=rules_file_content
+    )
+
+    yield httpserver.url_for(endpoint)
+
+    if saved_no_proxy:
+        os.environ["no_proxy"] = saved_no_proxy
+    else:
+        del os.environ["no_proxy"]
+
+
[e-mail]
 def manual_file():
     """Return the bundle file for testing."""
     return os.path.join(
diff --git a/tests/functional/requirements.txt b/tests/functional/requirements.txt
index f7fed82..6a1cad4 100644
--- a/tests/functional/requirements.txt
+++ b/tests/functional/requirements.txt
@@ -1,2 +1,3 @@
 pytest
 pytest-operator
+pytest-httpserver
diff --git a/tests/functional/test_jujulint.py b/tests/functional/test_jujulint.py
index 9d908f1..24e382a 100644
--- a/tests/functional/test_jujulint.py
+++ b/tests/functional/test_jujulint.py
@@ -1,7 +1,7 @@
 """Functional tests for juju-lint."""
 import json
 import socket
-from subprocess import check_call, check_output
+from subprocess import PIPE, check_call, check_output, run
 
 import pytest
 
@@ -34,6 +34,48 @@ def test_json_output(rules_file, manual_file):
     )
 
 
[e-mail]
[e-mail]("arg", ["", "rules_file"])
+def test_bad_url_among_the_rules_file_args_causes_crash(arg, manual_file, request):
+    """Test that a bad url in the arguments causes a crash."""
+    bad_url = "foo://bad.url"
+    cmdline_arg = bad_url
+    if arg:
+        rules_file = request.getfixturevalue(arg)
+        cmdline_arg += f",{rules_file}"
+
+    process = run(
+        f"juju-lint -c {cmdline_arg} {manual_file}".split(),
+        universal_newlines=True,
+        stderr=PIPE,
+    )
+    assert not process.returncode == 0
+    assert "Failed to fetch url" in process.stderr
+    assert "Error while reading the rules. Exiting..." in process.stderr
+    assert "Linting manual file" not in process.stderr
+
+
[e-mail]
+def test_multiple_rules_files_update_rules(rules_file, rules_file_url, manual_file):
+    """Test that multiple rules files actually update the rules."""
+    error_string = "Application mysql-innodb-cluster has config for 'max-connections' which is less than"
+    process = run(
+        f"juju-lint -c {rules_file} {manual_file}".split(),
+        universal_newlines=True,
+        stderr=PIPE,
+    )
+    assert process.returncode == 0
+    assert error_string not in process.stderr
+
+    process = run(
+        f"juju-lint -c {rules_file},{rules_file_url} {manual_file}".split(),
+        universal_newlines=True,
+        stderr=PIPE,
+    )
+    assert process.returncode == 0
+    assert error_string in process.stderr
+
+
 @pytest.mark.cloud
 async def test_audit_local_cloud(ops_test, local_cloud, rules_file):
     """Test running juju-lint against a live local cloud."""
diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py
index 81d8934..3870185 100644
--- a/tests/unit/test_cli.py
+++ b/tests/unit/test_cli.py
@@ -48,7 +48,7 @@ def test_cli_init(output_format_value, mocker):
 
     assert cli_instance.logger.logger.level == WARN
     assert cli_instance.output_format == output_format_value
-    assert cli_instance.lint_rules == rules_file_value
+    assert cli_instance.rules_files == [rules_file_value]
 
     if output_format_value != "text":
         logging_mock.disable.assert_called_once_with(level=logging_mock.CRITICAL)
@@ -80,7 +80,7 @@ def test_cli_ini_version(version, mocker):
     assert cli_instance.version == expected_version
 
 
[e-mail]("rules_path", ["absolute", "relative", None])
[e-mail]("rules_path", ["absolute", "relative", "url", None])
 def test_cli_init_rules_path(rules_path, mocker):
     """Test different methods of loading rules file on Cli init.
 
@@ -109,21 +109,52 @@ def test_cli_init_rules_path(rules_path, mocker):
         mocker.patch.object(cli.os.path, "isfile", return_value=True)
     elif rules_path == "relative":
         mocker.patch.object(cli.os.path, "isfile", side_effect=[False, True])
+    elif rules_path == "url":
+        mocker.patch.object(cli, "is_url", return_value=True)
     else:
         mocker.patch.object(cli.os.path, "isfile", return_value=False)
 
     cli_instance = cli.Cli()
 
-    if rules_path == "absolute":
-        assert cli_instance.lint_rules == file_path
+    if rules_path == "absolute" or rules_path == "url":
+        assert cli_instance.rules_files == [file_path]
         exit_mock.assert_not_called()
     elif rules_path == "relative":
-        assert cli_instance.lint_rules == "{}/{}".format(config_dir, file_path)
+        assert cli_instance.rules_files == ["{}/{}".format(config_dir, file_path)]
         exit_mock.assert_not_called()
     else:
         exit_mock.assert_called_once_with(1)
 
 
[e-mail](
+    "rules_file_value",
+    (
+        "/rule1.yaml, /rule2.yaml",
+        ",,,/rule1.yaml,/rule2.yaml",
+        "/rule1.yaml,/rule2.yaml,,,",
+        " /rule1.yaml,, /rule2.yaml ",
+        "/rule1.yaml,,,,,/rule2.yaml",
+    ),
+)
+def test_cli_init_rules_file_comma_separated_values(rules_file_value, mocker):
+    """Test that comma separated rules file values handle commas and spaces well."""
+    rules_file = MagicMock()
+    rules_file.get.return_value = rules_file_value
+
+    config = {
+        "logging": {"loglevel": MagicMock()},
+        "format": MagicMock(),
+        "rules": {"file": rules_file},
+    }
+
+    mocker.patch.object(cli, "Config", return_value=config)
+    mocker.patch.object(cli.os.path, "isfile", return_value=True)
+
+    cli_instance = cli.Cli()
+
+    assert cli_instance.rules_files == ["/rule1.yaml", "/rule2.yaml"]
+
+
 @pytest.mark.parametrize("is_cloud_set", [True, False])
 def test_cli_cloud_type(cli_instance, is_cloud_set):
     """Test cloud_type() property of Cli class."""
@@ -167,7 +198,7 @@ def test_cli_startup_message(cli_instance, cloud_type_value, manual_file_value, 
     """Test output of a startup message."""
     version = "1.0"
     config_dir = "/tmp/"
-    lint_rules = "some rules"
+    rules_files = ["rules.yaml"]
 
     log_level_value = "debug"
     log_level = MagicMock()
@@ -192,20 +223,20 @@ def test_cli_startup_message(cli_instance, cloud_type_value, manual_file_value, 
 
     expected_msg = (
         "juju-lint version {} starting...\n\t* Config directory: {}\n"
-        "\t* Cloud type: {}\n\t* Manual file: {}\n\t* Rules file: {}\n"
+        "\t* Cloud type: {}\n\t* Manual file: {}\n\t* Rules files: {}\n"
         "\t* Log level: {}\n"
     ).format(
         version,
         config_dir,
         cloud_type_value or "Unknown",
         manual_file_value or False,
-        lint_rules,
+        rules_files,
         log_level_value,
     )
 
     cli_instance.version = version
     cli_instance.config = config
-    cli_instance.lint_rules = lint_rules
+    cli_instance.rules_files = rules_files
     log_mock = mocker.patch.object(cli_instance, "logger")
 
     assert cli_instance.cloud_type == cloud_type_value
@@ -227,13 +258,13 @@ def test_cli_usage(cli_instance):
 def test_cli_audit_file(cli_instance, mocker):
     """Test method audit_file() from Cli class."""
     filename = "/tmp/bundle.yaml"
-    rules = "/tmp/rules.yaml"
+    rules = ["/tmp/rules.yaml"]
     cloud_type = "openstack"
     output_format = "text"
     linter_object = MagicMock()
 
     mock_linter = mocker.patch.object(cli, "Linter", return_value=linter_object)
-    cli_instance.lint_rules = rules
+    cli_instance.rules_files = rules
     cli_instance.output_format = output_format
 
     cli_instance.audit_file(filename, cloud_type)
@@ -245,6 +276,30 @@ def test_cli_audit_file(cli_instance, mocker):
     linter_object.lint_yaml_file.assert_called_once_with(filename)
 
 
+def test_cli_audit_file_exits_when_read_rule_fails(cli_instance, mocker):
+    """Test method audit_file() from Cli class terminates when cannot read rules."""
+    filename = "/tmp/bundle.yaml"
+    rules = ["/tmp/rules.yaml"]
+    cloud_type = "openstack"
+    output_format = "text"
+    linter_object = MagicMock()
+    linter_object.read_rules.return_value = False
+
+    mock_linter = mocker.patch.object(cli, "Linter", return_value=linter_object)
+    cli_instance.rules_files = rules
+    cli_instance.output_format = output_format
+
+    with pytest.raises(SystemExit) as mock_exception:
+        cli_instance.audit_file(filename, cloud_type)
+
+    assert mock_exception.value.code == 1
+    mock_linter.assert_called_once_with(
+        filename, rules, cloud_type=cloud_type, output_format=output_format
+    )
+    linter_object.read_rules.assert_called_once()
+    linter_object.lint_yaml_file.assert_not_called()
+
+
 def test_cli_audit_all(cli_instance, mocker):
     """Test audit_all() method from Cli class."""
     audit_mock = mocker.patch.object(cli_instance, "audit")
@@ -276,7 +331,7 @@ def test_cli_audit_all(cli_instance, mocker):
 def test_cli_audit(cli_instance, success, mocker):
     """Test audit() method from Cli class."""
     cloud_name = "test cloud"
-    lint_rules = "rules.yaml"
+    rules_files = ["rules.yaml"]
     cloud_data = {
         "access": "ssh",
         "sudo": "root",
@@ -303,7 +358,7 @@ def test_cli_audit(cli_instance, success, mocker):
     cli_instance.logger = mock_logger
 
     cli_instance.config = config_data
-    cli_instance.lint_rules = lint_rules
+    cli_instance.rules_files = rules_files
 
     # assert cli_instance.config["clouds"]
     cli_instance.audit(cloud_name=cloud_name)
@@ -313,7 +368,7 @@ def test_cli_audit(cli_instance, success, mocker):
         access_method=cloud_data["access"],
         ssh_host=cloud_data["host"],
         sudo_user=cloud_data["sudo"],
-        lint_rules=lint_rules,
+        lint_rules=rules_files,
     )
 
     if success:
diff --git a/tests/unit/test_jujulint.py b/tests/unit/test_jujulint.py
index 8f548d6..8970d29 100644
--- a/tests/unit/test_jujulint.py
+++ b/tests/unit/test_jujulint.py
@@ -2,6 +2,7 @@
 """Tests for jujulint."""
 import logging
 from datetime import datetime, timezone
+from textwrap import dedent
 from unittest import mock
 
 import pytest
@@ -1217,24 +1218,83 @@ applications:
         rules_path = tmp_path / "rules.yaml"
         rules_path.write_text('---\nkey:\n "value"')
 
-        linter.filename = str(rules_path)
+        linter.lint_rules = {}
+        linter.rules_files = [str(rules_path)]
         result = linter.read_rules()
         assert linter.lint_rules == {"key": "value"}
         assert result
 
+    def test_read_rules_url(self, linter, mocker):
+        """Test that rules YAML coming from urlopen works as expected."""
+        rules_content = b'---\nkey:\n "value"'
+        mock_urlopen = mocker.patch.object(lint, "urlopen")
+        mock_urlopen.return_value.__enter__.return_value.read.return_value = (
+            rules_content
+        )
+        linter.lint_rules = {}
+        linter.rules_files = ["https://rules.yaml"]
+        result = linter.read_rules()
+        assert linter.lint_rules == {"key": "value"}
+        assert result
+
+    def test_read_rules_multiple_files_update(self, tmp_path, linter, mocker):
+        """Test if successive rules file contents update the repeated fields."""
+        rules_one_content = dedent(
+            """
+                key_one: "foo"
+                key_two: "bar"
+            """
+        )
+        rules_two_content = dedent(
+            """
+                key_one: "baz"
+            """
+        ).encode()  # urlopen will eventually yield a bytes object
+        rules_one_path = tmp_path / "rules.yaml"
+        rules_one_path.write_text(rules_one_content)
+        mock_urlopen = mocker.patch.object(lint, "urlopen")
+        mock_urlopen.return_value.__enter__.return_value.read.return_value = (
+            rules_two_content
+        )
+        linter.lint_rules = {}
+        linter.rules_files = [str(rules_one_path), "https://rules.yaml"]
+        result = linter.read_rules()
+        assert linter.lint_rules == {"key_one": "baz", "key_two": "bar"}
+        assert result
+
+    def test_read_rules_url_exception(self, linter, mocker):
+        """Test read_rules() handles url exceptions."""
+        mock_urlopen = mocker.patch.object(lint, "urlopen")
+        mock_urlopen.side_effect = lint.URLError("")
+        linter.lint_rules = {}
+        linter.rules_files = ["https://rules.yaml"]
+        result = linter.read_rules()
+        assert linter.lint_rules == {}
+        assert result is False
+
+    def test_read_rules_timeout_exception(self, linter, mocker):
+        """Test read_rules() handles timeout exceptions."""
+        mock_urlopen = mocker.patch.object(lint, "urlopen")
+        mock_urlopen.side_effect = TimeoutError("")
+        linter.lint_rules = {}
+        linter.rules_files = ["https://rules.yaml"]
+        result = linter.read_rules()
+        assert linter.lint_rules == {}
+        assert result is False
+
     def test_snap_rules_files(self, rules_files, linter):
         """Ensure that all standard rules in the snap is loading correctly."""
-        for rule_file in rules_files:
+        for rules_file in rules_files:
             try:
-                linter.filename = rule_file
+                linter.rules_files = [rules_file]
                 linter.read_rules()
             except yaml.YAMLError:
-                pytest.fail(f"File: {rule_file} not loading")
+                pytest.fail(f"File: {rules_file} not loading")
 
     def test_wrong_rule_file_raise_error(self, rules_files, linter, mocker):
         """Test that bad formatted rules raise YAMLError."""
         rules_file = rules_files[0]
-        linter.filename = rules_file
+        linter.rules_files = [rules_file]
         mocker.patch(
             "jujulint.lint.Linter._process_includes_in_rules",
             side_effect=yaml.YAMLError,
@@ -1250,7 +1310,8 @@ applications:
         rules_path = tmp_path / "rules.yaml"
         rules_path.write_text('---\n!include include.yaml\nkey:\n "value"')
 
-        linter.filename = str(rules_path)
+        linter.lint_rules = {}
+        linter.rules_files = [str(rules_path)]
         result = linter.read_rules()
         assert linter.lint_rules == {"key": "value", "key-inc": "value2"}
         assert result
@@ -1262,7 +1323,8 @@ applications:
 
         linter.overrides = "override_1:value_1#override_2:value_2"
 
-        linter.filename = str(rules_path)
+        linter.lint_rules = {}
+        linter.rules_files = [str(rules_path)]
         linter.read_rules()
         assert linter.lint_rules == {
             "key": "value",
@@ -1274,17 +1336,17 @@ applications:
 
     def test_read_rules_fail(self, linter, mocker):
         """Test handling of a read_rules() failure."""
-        rule_file = "rules.yaml"
+        rules_files = ["rules.yaml"]
         mocker.patch.object(lint.os.path, "isfile", return_value=False)
         logger_mock = mock.MagicMock()
         linter.logger = logger_mock
-        linter.filename = rule_file
+        linter.rules_files = rules_files
 
         result = linter.read_rules()
 
         assert not result
         logger_mock.error.assert_called_once_with(
-            "Rules file {} does not exist.".format(rule_file)
+            "Rules file {} does not exist.".format(rules_files[0])
         )
 
     check_spaces_example_bundle = {
@@ -1706,7 +1768,8 @@ applications:
     def test_path_mtu_for_ovs_ovn_rules(self, linter, rules_files):
         """Test that ovs and ovn rules set right value of path-mtu."""
         for rule in [rules_file for rules_file in rules_files if "fcb" in rules_file]:
-            linter.filename = rule
+            linter.lint_rules = {}
+            linter.rules_files = [rule]
             linter.read_rules()
             path_mtu = linter.lint_rules["openstack config"]["neutron-api"]["path-mtu"]
             if "ovs" in rule:
@@ -1731,7 +1794,8 @@ applications:
         ]
         for rule in [rules_file for rules_file in rules_files if "fcb" in rules_file]:
             for charm in charms:
-                linter.filename = rule
+                linter.lint_rules = {}
+                linter.rules_files = [rule]
                 linter.read_rules()
                 worker_multiplier = linter.lint_rules["openstack config"][charm][
                     "worker-multiplier"