From a7fdbc8cd04d8a9868700fb569043743c9d1691e Mon Sep 17 00:00:00 2001
From: Carlos Nihelton <carlos.santanadeoliveira@canonical.com>
Date: Fri, 28 Jun 2024 11:43:52 -0300
Subject: [PATCH 01/10] [wsl] Landscape computer tags are special

UP4W business logic is so that its data overrides user at a key (module)
level.
That means the entire Landscape config is overriden if both agent data
and user data contains config for that module.
Yet, for better usability, computer tags must be assignable per instance.
That's not possible with agent.yaml, because it's meant to be global.
Its config data affects all Ubuntu WSL instances.

Thus this aims to make a special case for landscape.client.tags,
if present in user provided data (either Landscape or local user -
  whatever is picked up before merging with agent.yaml)
its value overwrites any tags set by agent.yaml.

Only landscape.client.tags are treated specially.
The pre-existing merge rules still apply for any other value present in
both agent.yaml and user provided data.
---
 cloudinit/sources/DataSourceWSL.py  |  13 ++++
 tests/unittests/sources/test_wsl.py | 102 ++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+)

diff --git a/cloudinit/sources/DataSourceWSL.py b/cloudinit/sources/DataSourceWSL.py
index b81298927a0..634800eb569 100644
--- a/cloudinit/sources/DataSourceWSL.py
+++ b/cloudinit/sources/DataSourceWSL.py
@@ -329,8 +329,12 @@ def _get_data(self) -> bool:
         # That's the reason for not using util.mergemanydict().
         merged: dict = {}
         overridden_keys: typing.List[str] = []
+        user_tags = None
         if user_data:
             merged = user_data
+            user_tags = (
+                merged.get("landscape", {}).get("client", {}).get("tags")
+            )
         if agent_data:
             if user_data:
                 LOG.debug("Merging both user_data and agent.yaml configs.")
@@ -345,6 +349,15 @@ def _get_data(self) -> bool:
                         ", ".join(overridden_keys)
                     )
                 )
+            if user_tags:
+                LOG.debug(
+                    " Landscape client conf updated with user-data tags: %s",
+                    user_tags,
+                )
+                # Does nothing if there is no "landscape"."client" key.
+                merged.get("landscape", {}).get("client", {}).update(
+                    {"tags": user_tags}
+                )
 
         self.userdata_raw = "#cloud-config\n%s" % yaml.dump(merged)
         return True
diff --git a/tests/unittests/sources/test_wsl.py b/tests/unittests/sources/test_wsl.py
index 31c5c897ed5..d9821da9d5a 100644
--- a/tests/unittests/sources/test_wsl.py
+++ b/tests/unittests/sources/test_wsl.py
@@ -412,6 +412,7 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
 landscape:
     client:
       account_name: agenttest
+      tags: wsl
 ubuntu_advantage:
     token: testtoken"""
         )
@@ -447,6 +448,7 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
 landscape:
   client:
     account_name: landscapetest
+    tags: tag_aiml, tag_dev
 package_update: true"""
         )
 
@@ -480,3 +482,103 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
         assert (
             "landscapetest" not in userdata and "agenttest" in userdata
         ), "Landscape account name should have been overriden by agent data"
+        assert (
+            "tag_aiml" in userdata and "tag_dev" in userdata
+        ), "User-data should override agent data's Landscape computer tags"
+
+        # Set up some Landscape provided user data without tags
+        landscape_file.write(
+            """#cloud-config
+landscape:
+  client:
+    account_name: landscapetest
+package_update: true"""
+        )
+
+        # Run the datasource
+        ds = wsl.DataSourceWSL(
+            sys_cfg=SAMPLE_CFG,
+            distro=_get_distro("ubuntu"),
+            paths=paths,
+        )
+
+        assert ds.get_data() is True
+        ud = ds.get_userdata()
+
+        assert ud is not None
+        userdata = cast(
+            str,
+            join_payloads_from_content_type(
+                cast(MIMEMultipart, ud), "text/cloud-config"
+            ),
+        )
+
+        assert "landscape" in userdata
+        assert (
+            "landscapetest" not in userdata and "agenttest" in userdata
+        ), "Landscape account name should have been overriden by agent data"
+        assert (
+            "tags: wsl" in userdata
+        ), "Landscape computer tags should match UP4W agent's data defaults"
+
+        # Make sure we don't crash if there are no tags anywhere.
+        agent_file.write(
+            """#cloud-config
+ubuntu_advantage:
+    token: up4w_token"""
+        )
+        # Run the datasource
+        ds = wsl.DataSourceWSL(
+            sys_cfg=SAMPLE_CFG,
+            distro=_get_distro("ubuntu"),
+            paths=paths,
+        )
+
+        assert ds.get_data() is True
+        ud = ds.get_userdata()
+
+        assert ud is not None
+        userdata = cast(
+            str,
+            join_payloads_from_content_type(
+                cast(MIMEMultipart, ud), "text/cloud-config"
+            ),
+        )
+        assert "landscapetest" in userdata
+        assert "up4w_token" in userdata
+
+        # Make sure we don't crash if there is no client subkey.
+        # (That would be a bug in the agent as there is no other config
+        # value for landscape outside of landscape.client, so I'm making up
+        # some non-sense keys just to make sure we won't crash)
+        agent_file.write(
+            """#cloud-config
+landscape:
+    server:
+        port: 6554
+ubuntu_advantage:
+    token: up4w_token"""
+        )
+        # Run the datasource
+        ds = wsl.DataSourceWSL(
+            sys_cfg=SAMPLE_CFG,
+            distro=_get_distro("ubuntu"),
+            paths=paths,
+        )
+
+        assert ds.get_data() is True
+        ud = ds.get_userdata()
+
+        assert ud is not None
+        userdata = cast(
+            str,
+            join_payloads_from_content_type(
+                cast(MIMEMultipart, ud), "text/cloud-config"
+            ),
+        )
+        assert "landscapetest" not in userdata
+        assert (
+            "port: 6554" in userdata
+        ), "agent data should override the entire landscape config."
+
+        assert "up4w_token" in userdata

From e9906e89f45c524262463cbfaeb67c6c9c625702 Mon Sep 17 00:00:00 2001
From: Carlos Nihelton <carlos.santanadeoliveira@canonical.com>
Date: Fri, 28 Jun 2024 11:56:20 -0300
Subject: [PATCH 02/10] [wsl] s/ubuntu_advantage/ubuntu_pro

Seems a cosmetic "fix", but consistency is good.
`ubuntu_advantage` is deprecated, so we'd better not risk
our test's future-proofing.
---
 tests/unittests/sources/test_wsl.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/unittests/sources/test_wsl.py b/tests/unittests/sources/test_wsl.py
index d9821da9d5a..fe72186accf 100644
--- a/tests/unittests/sources/test_wsl.py
+++ b/tests/unittests/sources/test_wsl.py
@@ -413,7 +413,7 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
     client:
       account_name: agenttest
       tags: wsl
-ubuntu_advantage:
+ubuntu_pro:
     token: testtoken"""
         )
 
@@ -437,7 +437,7 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
         )
         assert "wsl.conf" in userdata
         assert "packages" not in userdata
-        assert "ubuntu_advantage" in userdata
+        assert "ubuntu_pro" in userdata
         assert "landscape" in userdata
         assert "agenttest" in userdata
 
@@ -473,7 +473,7 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
 
         assert "wsl.conf" not in userdata
         assert "packages" not in userdata
-        assert "ubuntu_advantage" in userdata
+        assert "ubuntu_pro" in userdata
         assert "package_update" in userdata, (
             "package_update entry should not be overriden by agent data"
             " nor ignored"
@@ -524,7 +524,7 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
         # Make sure we don't crash if there are no tags anywhere.
         agent_file.write(
             """#cloud-config
-ubuntu_advantage:
+ubuntu_pro:
     token: up4w_token"""
         )
         # Run the datasource
@@ -556,7 +556,7 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
 landscape:
     server:
         port: 6554
-ubuntu_advantage:
+ubuntu_pro:
     token: up4w_token"""
         )
         # Run the datasource

From 975ffbea635dadc285f7a2dce6588d40eed33111 Mon Sep 17 00:00:00 2001
From: Carlos Nihelton <carlos.santanadeoliveira@canonical.com>
Date: Fri, 28 Jun 2024 12:05:52 -0300
Subject: [PATCH 03/10] [wsl] Update docs about Landscape tags

---
 doc/rtd/reference/datasources/wsl.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/rtd/reference/datasources/wsl.rst b/doc/rtd/reference/datasources/wsl.rst
index ab96f9490c4..0e2a126e174 100644
--- a/doc/rtd/reference/datasources/wsl.rst
+++ b/doc/rtd/reference/datasources/wsl.rst
@@ -66,7 +66,9 @@ following paths:
    the Ubuntu Pro for WSL agent. If this file is present, its modules will be
    merged with (1), overriding any conflicting modules. If (1) is not provided,
    then this file will be merged with any valid user-provided configuration
-   instead.
+   instead. Exception is made for Landscape client config computer tags. If
+   user provided data contains a value for that subkey it will be used instead
+   of the one provided by the ``agent.yaml``, which is treated as a default.
 
 Then, if a file from (1) is not found, a user-provided configuration will be
 looked for instead in the following order:

From e2920342bce29ac8ec03e628ffdd76e7f387067e Mon Sep 17 00:00:00 2001
From: Carlos Nihelton <carlosnsoliveira@gmail.com>
Date: Wed, 10 Jul 2024 23:11:39 -0300
Subject: [PATCH 04/10] Apply readability suggestions from code review

Co-authored-by: Chad Smith <chad.smith@canonical.com>
---
 cloudinit/sources/DataSourceWSL.py    | 13 +++++++------
 doc/rtd/reference/datasources/wsl.rst |  5 +++--
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/cloudinit/sources/DataSourceWSL.py b/cloudinit/sources/DataSourceWSL.py
index 634800eb569..f6e9096e6d3 100644
--- a/cloudinit/sources/DataSourceWSL.py
+++ b/cloudinit/sources/DataSourceWSL.py
@@ -329,7 +329,9 @@ def _get_data(self) -> bool:
         # That's the reason for not using util.mergemanydict().
         merged: dict = {}
         overridden_keys: typing.List[str] = []
-        user_tags = None
+        user_tags = (
+            user_data.get("landscape", {}).get("client", {}).get("tags")
+        )
         if user_data:
             merged = user_data
             user_tags = (
@@ -351,13 +353,12 @@ def _get_data(self) -> bool:
                 )
             if user_tags:
                 LOG.debug(
-                    " Landscape client conf updated with user-data tags: %s",
+                    " Landscape client conf updated with user-data"
+                    " landscape.client.tags: %s",
                     user_tags,
                 )
-                # Does nothing if there is no "landscape"."client" key.
-                merged.get("landscape", {}).get("client", {}).update(
-                    {"tags": user_tags}
-                )
+                if merged.get("landscape", {}).get("client"):
+                    merged["landscape"]["client"]["tags"] = user_tags
 
         self.userdata_raw = "#cloud-config\n%s" % yaml.dump(merged)
         return True
diff --git a/doc/rtd/reference/datasources/wsl.rst b/doc/rtd/reference/datasources/wsl.rst
index 0e2a126e174..c6970448b5c 100644
--- a/doc/rtd/reference/datasources/wsl.rst
+++ b/doc/rtd/reference/datasources/wsl.rst
@@ -67,8 +67,9 @@ following paths:
    merged with (1), overriding any conflicting modules. If (1) is not provided,
    then this file will be merged with any valid user-provided configuration
    instead. Exception is made for Landscape client config computer tags. If
-   user provided data contains a value for that subkey it will be used instead
-   of the one provided by the ``agent.yaml``, which is treated as a default.
+   user provided data contains a value for ``landscape.client.tags`` it will be
+   used instead of the one provided by the ``agent.yaml``, which is treated as
+   a default.
 
 Then, if a file from (1) is not found, a user-provided configuration will be
 looked for instead in the following order:

From 5e626d7e270dcc8ea7b7dce6aaa97d71197ce546 Mon Sep 17 00:00:00 2001
From: Carlos Nihelton <carlos.santanadeoliveira@canonical.com>
Date: Wed, 10 Jul 2024 23:47:35 -0300
Subject: [PATCH 05/10] Splits the long test in smaller more scoped units

---
 tests/unittests/sources/test_wsl.py | 95 ++++++++++++++++++++++++++++-
 1 file changed, 92 insertions(+), 3 deletions(-)

diff --git a/tests/unittests/sources/test_wsl.py b/tests/unittests/sources/test_wsl.py
index fe72186accf..79b3ed45ac7 100644
--- a/tests/unittests/sources/test_wsl.py
+++ b/tests/unittests/sources/test_wsl.py
@@ -355,6 +355,8 @@ def test_get_data_sh(self, m_lsb_release, tmpdir, paths):
 
     @mock.patch("cloudinit.util.get_linux_distro")
     def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
+        """Validates the precedence of user-data files."""
+
         m_get_linux_dist.return_value = SAMPLE_LINUX_DISTRO
 
         # Set up basic user data:
@@ -400,9 +402,17 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
 
         assert "" == shell_script
 
-        # Additionally set up some UP4W agent data:
+    @mock.patch("cloudinit.util.get_linux_distro")
+    def test_interaction_with_pro(self, m_get_linux_dist, tmpdir, paths):
+        """Validates the interaction of user-data and Pro For WSL agent data"""
+
+        m_get_linux_dist.return_value = SAMPLE_LINUX_DISTRO
+
+        user_file = tmpdir.join(".cloud-init", "ubuntu-24.04.user-data")
+        user_file.dirpath().mkdir()
+        user_file.write("#cloud-config\nwrite_files:\n- path: /etc/wsl.conf")
 
-        # Now the winner should be the merge of the agent and Landscape data.
+        # The winner should be the merge of the agent and user provided data.
         ubuntu_pro_tmp = tmpdir.join(".ubuntupro", ".cloud-init")
         os.makedirs(ubuntu_pro_tmp, exist_ok=True)
 
@@ -441,7 +451,37 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
         assert "landscape" in userdata
         assert "agenttest" in userdata
 
-        # Additionally set up some Landscape provided user data
+    @mock.patch("cloudinit.util.get_linux_distro")
+    def test_landscape_provided_data(self, m_get_linux_dist, tmpdir, paths):
+        """Validates the interaction of Pro For WSL agent and Landscape data"""
+
+        m_get_linux_dist.return_value = SAMPLE_LINUX_DISTRO
+
+        user_file = tmpdir.join(".cloud-init", "ubuntu-24.04.user-data")
+        user_file.dirpath().mkdir()
+        user_file.write(
+            """#cloud-config
+landscape:
+  client:
+    account_name: user
+    tags: usertags
+package_update: true"""
+        )
+
+        ubuntu_pro_tmp = tmpdir.join(".ubuntupro", ".cloud-init")
+        os.makedirs(ubuntu_pro_tmp, exist_ok=True)
+
+        agent_file = ubuntu_pro_tmp.join("agent.yaml")
+        agent_file.write(
+            """#cloud-config
+landscape:
+    client:
+      account_name: agenttest
+      tags: wsl
+ubuntu_pro:
+    token: testtoken"""
+        )
+
         landscape_file = ubuntu_pro_tmp.join("%s.user-data" % INSTANCE_NAME)
         landscape_file.write(
             """#cloud-config
@@ -482,11 +522,36 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
         assert (
             "landscapetest" not in userdata and "agenttest" in userdata
         ), "Landscape account name should have been overriden by agent data"
+        # Make sure we have tags from Landscape data, not agent's
         assert (
             "tag_aiml" in userdata and "tag_dev" in userdata
         ), "User-data should override agent data's Landscape computer tags"
+        # and also not from the user provided data.
+        assert (
+            "usertags" not in userdata
+        ), "User provided data should have been overriden"
+
+    @mock.patch("cloudinit.util.get_linux_distro")
+    def test_with_landscape_no_tags(self, m_get_linux_dist, tmpdir, paths):
+        """Validates the Pro For WSL default Landscape tags are applied"""
 
+        m_get_linux_dist.return_value = SAMPLE_LINUX_DISTRO
+
+        ubuntu_pro_tmp = tmpdir.join(".ubuntupro", ".cloud-init")
+        os.makedirs(ubuntu_pro_tmp, exist_ok=True)
+
+        agent_file = ubuntu_pro_tmp.join("agent.yaml")
+        agent_file.write(
+            """#cloud-config
+landscape:
+    client:
+      account_name: agenttest
+      tags: wsl
+ubuntu_pro:
+    token: testtoken"""
+        )
         # Set up some Landscape provided user data without tags
+        landscape_file = ubuntu_pro_tmp.join("%s.user-data" % INSTANCE_NAME)
         landscape_file.write(
             """#cloud-config
 landscape:
@@ -521,12 +586,36 @@ def test_data_precedence(self, m_get_linux_dist, tmpdir, paths):
             "tags: wsl" in userdata
         ), "Landscape computer tags should match UP4W agent's data defaults"
 
+    @mock.patch("cloudinit.util.get_linux_distro")
+    def test_with_no_tags_at_all(self, m_get_linux_dist, tmpdir, paths):
+        """Asserts the DS still works if there are no Landscape tags at all"""
+
+        m_get_linux_dist.return_value = SAMPLE_LINUX_DISTRO
+
+        user_file = tmpdir.join(".cloud-init", "ubuntu-24.04.user-data")
+        user_file.dirpath().mkdir()
+        user_file.write("#cloud-config\nwrite_files:\n- path: /etc/wsl.conf")
+
+        ubuntu_pro_tmp = tmpdir.join(".ubuntupro", ".cloud-init")
+        os.makedirs(ubuntu_pro_tmp, exist_ok=True)
+
+        agent_file = ubuntu_pro_tmp.join("agent.yaml")
         # Make sure we don't crash if there are no tags anywhere.
         agent_file.write(
             """#cloud-config
 ubuntu_pro:
     token: up4w_token"""
         )
+        # Set up some Landscape provided user data without tags
+        landscape_file = ubuntu_pro_tmp.join("%s.user-data" % INSTANCE_NAME)
+        landscape_file.write(
+            """#cloud-config
+landscape:
+  client:
+    account_name: landscapetest
+package_update: true"""
+        )
+
         # Run the datasource
         ds = wsl.DataSourceWSL(
             sys_cfg=SAMPLE_CFG,

From 51a4b736ba03c78aa494f91ff98862d3ffdc8db3 Mon Sep 17 00:00:00 2001
From: Carlos Nihelton <carlos.santanadeoliveira@canonical.com>
Date: Wed, 10 Jul 2024 23:53:54 -0300
Subject: [PATCH 06/10] Please mypy about user_data possibly being None

---
 cloudinit/sources/DataSourceWSL.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cloudinit/sources/DataSourceWSL.py b/cloudinit/sources/DataSourceWSL.py
index f6e9096e6d3..947e4cc3a71 100644
--- a/cloudinit/sources/DataSourceWSL.py
+++ b/cloudinit/sources/DataSourceWSL.py
@@ -329,6 +329,8 @@ def _get_data(self) -> bool:
         # That's the reason for not using util.mergemanydict().
         merged: dict = {}
         overridden_keys: typing.List[str] = []
+        # We've already checked, this is just to please mypy.
+        assert user_data is not None
         user_tags = (
             user_data.get("landscape", {}).get("client", {}).get("tags")
         )

From 51c9ea04419683dffc1c04ef8563a5ce47170c3b Mon Sep 17 00:00:00 2001
From: Carlos Nihelton <carlos.santanadeoliveira@canonical.com>
Date: Fri, 19 Jul 2024 10:25:49 -0300
Subject: [PATCH 07/10] Move to the right scope log about merging tags

And trim spaces
---
 cloudinit/sources/DataSourceWSL.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/cloudinit/sources/DataSourceWSL.py b/cloudinit/sources/DataSourceWSL.py
index 947e4cc3a71..bd71cd4310f 100644
--- a/cloudinit/sources/DataSourceWSL.py
+++ b/cloudinit/sources/DataSourceWSL.py
@@ -353,14 +353,13 @@ def _get_data(self) -> bool:
                         ", ".join(overridden_keys)
                     )
                 )
-            if user_tags:
+            if user_tags and merged.get("landscape", {}).get("client"):
                 LOG.debug(
-                    " Landscape client conf updated with user-data"
+                    "Landscape client conf updated with user-data"
                     " landscape.client.tags: %s",
                     user_tags,
                 )
-                if merged.get("landscape", {}).get("client"):
-                    merged["landscape"]["client"]["tags"] = user_tags
+                merged["landscape"]["client"]["tags"] = user_tags
 
         self.userdata_raw = "#cloud-config\n%s" % yaml.dump(merged)
         return True

From ced6cc5054c74f36f980c6e7ea25498aa92a589a Mon Sep 17 00:00:00 2001
From: Carlos Nihelton <carlos.santanadeoliveira@canonical.com>
Date: Fri, 19 Jul 2024 14:10:23 -0300
Subject: [PATCH 08/10] Makes Landscape configs in tests more realistic

Adding common fields we expect to see in real world usage.
Computer title is certainly expected on local user setups,
but in the case of UP4W it's handled internally, transparently from
cloud-init.
---
 tests/unittests/sources/test_wsl.py | 36 +++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/tests/unittests/sources/test_wsl.py b/tests/unittests/sources/test_wsl.py
index 79b3ed45ac7..ee844868b25 100644
--- a/tests/unittests/sources/test_wsl.py
+++ b/tests/unittests/sources/test_wsl.py
@@ -420,9 +420,13 @@ def test_interaction_with_pro(self, m_get_linux_dist, tmpdir, paths):
         agent_file.write(
             """#cloud-config
 landscape:
+    host:
+        url: landscape.canonical.com:6554
     client:
-      account_name: agenttest
-      tags: wsl
+        account_name: agenttest
+        url: https://landscape.canonical.com/message-system
+        ping_url: https://landscape.canonical.com/ping
+        tags: wsl
 ubuntu_pro:
     token: testtoken"""
         )
@@ -462,9 +466,14 @@ def test_landscape_provided_data(self, m_get_linux_dist, tmpdir, paths):
         user_file.write(
             """#cloud-config
 landscape:
-  client:
-    account_name: user
-    tags: usertags
+    host:
+        url: landscape.canonical.com:6554
+    client:
+        computer_title: MLMachine
+        account_name: user
+        url: https://landscape.canonical.com/message-system
+        ping_url: https://landscape.canonical.com/ping
+        tags: usertags
 package_update: true"""
         )
 
@@ -475,9 +484,14 @@ def test_landscape_provided_data(self, m_get_linux_dist, tmpdir, paths):
         agent_file.write(
             """#cloud-config
 landscape:
+    host:
+        url: hosted.com:6554
     client:
-      account_name: agenttest
-      tags: wsl
+        account_name: agenttest
+        url: https://hosted.com/message-system
+        ping_url: https://hosted.com/ping
+        ssl_public_key: C:\\Users\\User\\server.pem
+        tags: wsl
 ubuntu_pro:
     token: testtoken"""
         )
@@ -544,9 +558,13 @@ def test_with_landscape_no_tags(self, m_get_linux_dist, tmpdir, paths):
         agent_file.write(
             """#cloud-config
 landscape:
+    host:
+        url: landscape.canonical.com:6554
     client:
-      account_name: agenttest
-      tags: wsl
+        account_name: agenttest
+        url: https://landscape.canonical.com/message-system
+        ping_url: https://landscape.canonical.com/ping
+        tags: wsl
 ubuntu_pro:
     token: testtoken"""
         )

From 7c1a3a5e04822abc7ba259d4060887b16a9546ab Mon Sep 17 00:00:00 2001
From: Carlos Nihelton <carlos.santanadeoliveira@canonical.com>
Date: Fri, 19 Jul 2024 14:47:02 -0300
Subject: [PATCH 09/10] More focused datasource tests

Splitting the longer tests in more specific test cases.
Split "missing tags" from "missing landscape.client subkey" tests.
Remove some no-longer relevant assertions.
---
 tests/unittests/sources/test_wsl.py | 89 +++++++++++++++++++++--------
 1 file changed, 66 insertions(+), 23 deletions(-)

diff --git a/tests/unittests/sources/test_wsl.py b/tests/unittests/sources/test_wsl.py
index ee844868b25..9e8e030f3bf 100644
--- a/tests/unittests/sources/test_wsl.py
+++ b/tests/unittests/sources/test_wsl.py
@@ -456,8 +456,8 @@ def test_interaction_with_pro(self, m_get_linux_dist, tmpdir, paths):
         assert "agenttest" in userdata
 
     @mock.patch("cloudinit.util.get_linux_distro")
-    def test_landscape_provided_data(self, m_get_linux_dist, tmpdir, paths):
-        """Validates the interaction of Pro For WSL agent and Landscape data"""
+    def test_landscape_vs_local_user(self, m_get_linux_dist, tmpdir, paths):
+        """Validates the precendence of Landscape-provided over local data"""
 
         m_get_linux_dist.return_value = SAMPLE_LINUX_DISTRO
 
@@ -465,18 +465,53 @@ def test_landscape_provided_data(self, m_get_linux_dist, tmpdir, paths):
         user_file.dirpath().mkdir()
         user_file.write(
             """#cloud-config
-landscape:
-    host:
-        url: landscape.canonical.com:6554
-    client:
-        computer_title: MLMachine
-        account_name: user
-        url: https://landscape.canonical.com/message-system
-        ping_url: https://landscape.canonical.com/ping
-        tags: usertags
+ubuntu_pro:
+    token: usertoken
 package_update: true"""
         )
 
+        ubuntu_pro_tmp = tmpdir.join(".ubuntupro", ".cloud-init")
+        os.makedirs(ubuntu_pro_tmp, exist_ok=True)
+        landscape_file = ubuntu_pro_tmp.join("%s.user-data" % INSTANCE_NAME)
+        landscape_file.write(
+            """#cloud-config
+landscape:
+  client:
+    account_name: landscapetest
+    tags: tag_aiml, tag_dev
+locale: en_GB.UTF-8"""
+        )
+
+        # Run the datasource
+        ds = wsl.DataSourceWSL(
+            sys_cfg=SAMPLE_CFG,
+            distro=_get_distro("ubuntu"),
+            paths=paths,
+        )
+
+        assert ds.get_data() is True
+        ud = ds.get_userdata()
+        assert ud is not None
+        userdata = cast(
+            str,
+            join_payloads_from_content_type(
+                cast(MIMEMultipart, ud), "text/cloud-config"
+            ),
+        )
+
+        assert (
+            "locale" in userdata
+            and "landscapetest" in userdata
+            and "ubuntu_pro" not in userdata
+            and "package_update" not in userdata
+        ), "Landscape data should have overriden user provided data"
+
+    @mock.patch("cloudinit.util.get_linux_distro")
+    def test_landscape_provided_data(self, m_get_linux_dist, tmpdir, paths):
+        """Validates the interaction of Pro For WSL agent and Landscape data"""
+
+        m_get_linux_dist.return_value = SAMPLE_LINUX_DISTRO
+
         ubuntu_pro_tmp = tmpdir.join(".ubuntupro", ".cloud-init")
         os.makedirs(ubuntu_pro_tmp, exist_ok=True)
 
@@ -525,14 +560,11 @@ def test_landscape_provided_data(self, m_get_linux_dist, tmpdir, paths):
             ),
         )
 
-        assert "wsl.conf" not in userdata
-        assert "packages" not in userdata
-        assert "ubuntu_pro" in userdata
+        assert "ubuntu_pro" in userdata, "Agent data should be present"
         assert "package_update" in userdata, (
             "package_update entry should not be overriden by agent data"
             " nor ignored"
         )
-        assert "landscape" in userdata
         assert (
             "landscapetest" not in userdata and "agenttest" in userdata
         ), "Landscape account name should have been overriden by agent data"
@@ -540,10 +572,6 @@ def test_landscape_provided_data(self, m_get_linux_dist, tmpdir, paths):
         assert (
             "tag_aiml" in userdata and "tag_dev" in userdata
         ), "User-data should override agent data's Landscape computer tags"
-        # and also not from the user provided data.
-        assert (
-            "usertags" not in userdata
-        ), "User provided data should have been overriden"
 
     @mock.patch("cloudinit.util.get_linux_distro")
     def test_with_landscape_no_tags(self, m_get_linux_dist, tmpdir, paths):
@@ -596,10 +624,6 @@ def test_with_landscape_no_tags(self, m_get_linux_dist, tmpdir, paths):
             ),
         )
 
-        assert "landscape" in userdata
-        assert (
-            "landscapetest" not in userdata and "agenttest" in userdata
-        ), "Landscape account name should have been overriden by agent data"
         assert (
             "tags: wsl" in userdata
         ), "Landscape computer tags should match UP4W agent's data defaults"
@@ -653,7 +677,17 @@ def test_with_no_tags_at_all(self, m_get_linux_dist, tmpdir, paths):
         )
         assert "landscapetest" in userdata
         assert "up4w_token" in userdata
+        assert "tags" not in userdata
+
+    @mock.patch("cloudinit.util.get_linux_distro")
+    def test_with_no_client_subkey(self, m_get_linux_dist, tmpdir, paths):
+        """Validates the DS works without the landscape.client subkey"""
 
+        m_get_linux_dist.return_value = SAMPLE_LINUX_DISTRO
+        ubuntu_pro_tmp = tmpdir.join(".ubuntupro", ".cloud-init")
+        os.makedirs(ubuntu_pro_tmp, exist_ok=True)
+
+        agent_file = ubuntu_pro_tmp.join("agent.yaml")
         # Make sure we don't crash if there is no client subkey.
         # (That would be a bug in the agent as there is no other config
         # value for landscape outside of landscape.client, so I'm making up
@@ -666,6 +700,15 @@ def test_with_no_tags_at_all(self, m_get_linux_dist, tmpdir, paths):
 ubuntu_pro:
     token: up4w_token"""
         )
+
+        landscape_file = ubuntu_pro_tmp.join("%s.user-data" % INSTANCE_NAME)
+        landscape_file.write(
+            """#cloud-config
+landscape:
+  client:
+    account_name: landscapetest
+package_update: true"""
+        )
         # Run the datasource
         ds = wsl.DataSourceWSL(
             sys_cfg=SAMPLE_CFG,

From 7c955278e6214280ade44b19e6ded68447c121a9 Mon Sep 17 00:00:00 2001
From: Chad Smith <chad.smith@canonical.com>
Date: Fri, 19 Jul 2024 13:15:44 -0600
Subject: [PATCH 10/10] mypy: init user_tags to empty string, only set it when
 present in userdata

- tests: lanscape.client.tags schema disallows spaces between tags.
  Update tests to assert comma-delimited values and the exclusion of
  the wsl tag from agent.yaml
---
 cloudinit/sources/DataSourceWSL.py  | 8 ++------
 tests/unittests/sources/test_wsl.py | 5 +++--
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/cloudinit/sources/DataSourceWSL.py b/cloudinit/sources/DataSourceWSL.py
index bd71cd4310f..7a75ff4e691 100644
--- a/cloudinit/sources/DataSourceWSL.py
+++ b/cloudinit/sources/DataSourceWSL.py
@@ -328,16 +328,12 @@ def _get_data(self) -> bool:
         # provides them instead.
         # That's the reason for not using util.mergemanydict().
         merged: dict = {}
+        user_tags: str = ""
         overridden_keys: typing.List[str] = []
-        # We've already checked, this is just to please mypy.
-        assert user_data is not None
-        user_tags = (
-            user_data.get("landscape", {}).get("client", {}).get("tags")
-        )
         if user_data:
             merged = user_data
             user_tags = (
-                merged.get("landscape", {}).get("client", {}).get("tags")
+                merged.get("landscape", {}).get("client", {}).get("tags", "")
             )
         if agent_data:
             if user_data:
diff --git a/tests/unittests/sources/test_wsl.py b/tests/unittests/sources/test_wsl.py
index 9e8e030f3bf..2f26d7fd565 100644
--- a/tests/unittests/sources/test_wsl.py
+++ b/tests/unittests/sources/test_wsl.py
@@ -478,7 +478,7 @@ def test_landscape_vs_local_user(self, m_get_linux_dist, tmpdir, paths):
 landscape:
   client:
     account_name: landscapetest
-    tags: tag_aiml, tag_dev
+    tags: tag_aiml,tag_dev
 locale: en_GB.UTF-8"""
         )
 
@@ -537,7 +537,7 @@ def test_landscape_provided_data(self, m_get_linux_dist, tmpdir, paths):
 landscape:
   client:
     account_name: landscapetest
-    tags: tag_aiml, tag_dev
+    tags: tag_aiml,tag_dev
 package_update: true"""
         )
 
@@ -572,6 +572,7 @@ def test_landscape_provided_data(self, m_get_linux_dist, tmpdir, paths):
         assert (
             "tag_aiml" in userdata and "tag_dev" in userdata
         ), "User-data should override agent data's Landscape computer tags"
+        assert "wsl" not in userdata
 
     @mock.patch("cloudinit.util.get_linux_distro")
     def test_with_landscape_no_tags(self, m_get_linux_dist, tmpdir, paths):