enable TLS by default; remove command-line option `--mqtt-enable-tls`

CHANGELOG.md

@@ -9,10 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - declare compatibility with `python3.11`
 
 ### Changed
+- TLS now enabled by default (disable via `--mqtt-disable-tls`)
 - replaced [paho-mqtt](https://github.com/eclipse/paho.mqtt.python)
   with its async wrapper [aiomqtt](https://github.com/sbtinstruments/aiomqtt)
 
 ### Removed
+- command-line option `--mqtt-enable-tls` (TLS now enabled by default)
 - compatibility with `python3.7`
 
 ## [3.3.1] - 2022-08-31

README.md

@@ -22,7 +22,7 @@ $ pip3 install --user --upgrade switchbot-mqtt
 ## Usage
 
 ```sh
-$ switchbot-mqtt --mqtt-host HOSTNAME_OR_IP_ADDRESS --mqtt-enable-tls
+$ switchbot-mqtt --mqtt-host HOSTNAME_OR_IP_ADDRESS
 # or
 $ switchbot-mqtt --mqtt-host HOSTNAME_OR_IP_ADDRESS --mqtt-disable-tls
 ```

switchbot_mqtt/_cli.py

@@ -22,7 +22,6 @@ import json
 import logging
 import os
 import pathlib
-import warnings
 
 import switchbot
 
@@ -44,17 +43,9 @@ def _main() -> None:
     argparser.add_argument(
         "--mqtt-port",
         type=int,
-        help=f"default {_MQTT_DEFAULT_PORT} ({_MQTT_DEFAULT_TLS_PORT} with --mqtt-enable-tls)",
-    )
-    mqtt_tls_argument_group = argparser.add_mutually_exclusive_group()
-    mqtt_tls_argument_group.add_argument(
-        "--mqtt-enable-tls",
-        action="store_true",
-        help="TLS will be enabled by default in the next major release",
-    )
-    mqtt_tls_argument_group.add_argument(  # for upward compatibility
-        "--mqtt-disable-tls", action="store_true", help="Currently enabled by default"
+        help=f"default {_MQTT_DEFAULT_TLS_PORT} ({_MQTT_DEFAULT_PORT} with --mqtt-disable-tls)",
     )
+    argparser.add_argument("--mqtt-disable-tls", action="store_true")
     argparser.add_argument("--mqtt-username", type=str)
     password_argument_group = argparser.add_mutually_exclusive_group()
     password_argument_group.add_argument("--mqtt-password", type=str)
@@ -129,17 +120,10 @@ def _main() -> None:
     _LOGGER.debug("args=%r", args)
     if args.mqtt_port:
         mqtt_port = args.mqtt_port
-    elif args.mqtt_enable_tls:
-        mqtt_port = _MQTT_DEFAULT_TLS_PORT
-    else:
+    elif args.mqtt_disable_tls:
         mqtt_port = _MQTT_DEFAULT_PORT
-    if not args.mqtt_enable_tls and not args.mqtt_disable_tls:
-        warnings.warn(
-            "In switchbot-mqtt's next major release, TLS will be enabled by default"
-            " (--mqtt-enable-tls)."
-            " Please add --mqtt-disable-tls to your command for upward compatibility.",
-            UserWarning,  # DeprecationWarning ignored by default
-        )
+    else:
+        mqtt_port = _MQTT_DEFAULT_TLS_PORT
     if args.mqtt_password_path:
         # .read_text() replaces \r\n with \n
         mqtt_password = args.mqtt_password_path.read_bytes().decode()
@@ -159,7 +143,7 @@ def _main() -> None:
         switchbot_mqtt._run(  # pylint: disable=protected-access; internal
             mqtt_host=args.mqtt_host,
             mqtt_port=mqtt_port,
-            mqtt_disable_tls=not args.mqtt_enable_tls,
+            mqtt_disable_tls=args.mqtt_disable_tls,
             mqtt_username=args.mqtt_username,
             mqtt_password=mqtt_password,
             mqtt_topic_prefix=args.mqtt_topic_prefix,

tests/test_cli.py

@@ -52,15 +52,15 @@ def test_console_entry_point() -> None:
         (
             ["", "--mqtt-host", "mqtt-broker.local"],
             "mqtt-broker.local",
-            1883,
+            8883,
             None,
             None,
             3,
         ),
         (
-            ["", "--mqtt-host", "mqtt-broker.local", "--mqtt-port", "8883"],
+            ["", "--mqtt-host", "mqtt-broker.local", "--mqtt-port", "8884"],
             "mqtt-broker.local",
-            8883,
+            8884,
             None,
             None,
             3,
@@ -68,7 +68,7 @@ def test_console_entry_point() -> None:
         (
             ["", "--mqtt-host", "mqtt-broker.local", "--mqtt-username", "me"],
             "mqtt-broker.local",
-            1883,
+            8883,
             "me",
             None,
             3,
@@ -86,7 +86,7 @@ def test_console_entry_point() -> None:
                 "21",
             ],
             "mqtt-broker.local",
-            1883,
+            8883,
             "me",
             "secret",
             21,
@@ -103,12 +103,12 @@ def test__main(
 ) -> None:
     with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
         "sys.argv", argv
-    ), pytest.warns(UserWarning, match=r"Please add --mqtt-disable-tls\b"):
+    ):
         switchbot_mqtt._cli._main()
     run_mock.assert_called_once_with(
         mqtt_host=expected_mqtt_host,
         mqtt_port=expected_mqtt_port,
-        mqtt_disable_tls=True,
+        mqtt_disable_tls=False,
         mqtt_username=expected_username,
         mqtt_password=expected_password,
         mqtt_topic_prefix="homeassistant/",
@@ -152,8 +152,8 @@ def test__main_mqtt_password_file(
         switchbot_mqtt._cli._main()
     run_mock.assert_called_once_with(
         mqtt_host="localhost",
-        mqtt_port=1883,
-        mqtt_disable_tls=True,
+        mqtt_port=8883,
+        mqtt_disable_tls=False,
         mqtt_username="me",
         mqtt_password=expected_password,
         mqtt_topic_prefix="homeassistant/",
@@ -215,8 +215,8 @@ def test__main_device_password_file(
         switchbot_mqtt._cli._main()
     run_mock.assert_called_once_with(
         mqtt_host="localhost",
-        mqtt_port=1883,
-        mqtt_disable_tls=True,
+        mqtt_port=8883,
+        mqtt_disable_tls=False,
         mqtt_username=None,
         mqtt_password=None,
         mqtt_topic_prefix="homeassistant/",
@@ -228,8 +228,8 @@ def test__main_device_password_file(
 
 _RUN_DEFAULT_KWARGS: typing.Dict[str, typing.Any] = {
     "mqtt_host": "localhost",
-    "mqtt_port": 1883,
-    "mqtt_disable_tls": True,
+    "mqtt_port": 8883,
+    "mqtt_disable_tls": False,
     "mqtt_username": None,
     "mqtt_password": None,
     "mqtt_topic_prefix": "homeassistant/",
@@ -239,10 +239,10 @@ _RUN_DEFAULT_KWARGS: typing.Dict[str, typing.Any] = {
 }
 
 
-def test__main_mqtt_disable_tls_implicit() -> None:
+def test__main_mqtt_disable_tls() -> None:
     with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
-        "sys.argv", ["", "--mqtt-host", "mqtt.local"]
-    ), pytest.warns(UserWarning, match=r"Please add --mqtt-disable-tls\b"):
+        "sys.argv", ["", "--mqtt-host", "mqtt.local", "--mqtt-disable-tls"]
+    ):
         switchbot_mqtt._cli._main()
     run_mock.assert_called_once_with(
         **{
@@ -254,50 +254,22 @@ def test__main_mqtt_disable_tls_implicit() -> None:
     )
 
 
-def test__main_mqtt_enable_tls() -> None:
-    with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
-        "sys.argv", ["", "--mqtt-host", "mqtt.local", "--mqtt-enable-tls"]
-    ):
-        switchbot_mqtt._cli._main()
-    run_mock.assert_called_once_with(
-        **{
-            **_RUN_DEFAULT_KWARGS,
-            "mqtt_host": "mqtt.local",
-            "mqtt_disable_tls": False,
-            "mqtt_port": 8883,
-        }
-    )
-
-
-def test__main_mqtt_enable_tls_overwrite_port() -> None:
+def test__main_mqtt_disable_tls_overwrite_port() -> None:
     with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
         "sys.argv",
-        ["", "--mqtt-host", "mqtt.local", "--mqtt-port", "1883", "--mqtt-enable-tls"],
+        ["", "--mqtt-host", "mqtt.local", "--mqtt-port", "1884", "--mqtt-disable-tls"],
     ):
         switchbot_mqtt._cli._main()
     run_mock.assert_called_once_with(
         **{
             **_RUN_DEFAULT_KWARGS,
             "mqtt_host": "mqtt.local",
-            "mqtt_disable_tls": False,
-            "mqtt_port": 1883,
+            "mqtt_disable_tls": True,
+            "mqtt_port": 1884,
         }
     )
 
 
-def test__main_mqtt_tls_collision(capsys: _pytest.capture.CaptureFixture) -> None:
-    with unittest.mock.patch("switchbot_mqtt._run") as run_mock, unittest.mock.patch(
-        "sys.argv",
-        ["", "--mqtt-host", "mqtt.local", "--mqtt-enable-tls", "--mqtt-disable-tls"],
-    ), pytest.raises(SystemExit):
-        switchbot_mqtt._cli._main()
-    run_mock.assert_not_called()
-    assert (
-        "error: argument --mqtt-disable-tls: not allowed with argument --mqtt-enable-tls\n"
-        in capsys.readouterr()[1]
-    )
-
-
 @pytest.mark.parametrize(
     ("additional_argv", "expected_topic_prefix"),
     [([], "homeassistant/"), (["--mqtt-topic-prefix", ""], "")],