Merge pull request #22 from canonical/deployment-additions

Add a launchpad secret option
Refactor directory creation
Add additional charmcraft.yaml links
Only provide worker nodes to secondary units

Author: lvoytek

charmcraft.yaml

@@ -48,7 +48,14 @@ peers:
     interface: git_ubuntu_primary_info
 
 links:
-  documentation: https://discourse.charmhub.io/t/git-ubuntu-operator-documentation/19098
+  documentation: |
+    https://discourse.charmhub.io/t/git-ubuntu-operator-documentation/19098
+  website: |
+    https://charmhub.io/git-ubuntu
+  issues: |
+    https://github.com/canonical/git-ubuntu-operator/issues
+  source: |
+    https://github.com/canonical/git-ubuntu-operator
 
 config:
   options:
@@ -74,6 +81,11 @@ config:
         An ssh private key that matches with a public key associated with the
         lpuser account on Launchpad.
       type: secret
+    lpuser_lp_key:
+      description: |
+        A Launchpad keyring entry that allows launchpadlib access, associated
+        with the lpuser account on Launchpad.
+      type: secret
     publish:
       description: |
         If updates should be pushed to Launchpad. Set to False for local
@@ -82,6 +94,6 @@ config:
       type: boolean
     workers:
       description: |
-        The number of git-ubuntu worker processes to maintain.
+        The number of git-ubuntu worker processes to maintain per secondary node.
       default: 2
       type: int

src/charm.py

@@ -112,6 +112,21 @@ def _lpuser_ssh_key(self) -> str | None:
 
         return None
 
+    @property
+    def _lpuser_lp_key(self) -> str | None:
+        try:
+            secret_id = str(self.config["lpuser_lp_key"])
+            lp_key_secret = self.model.get_secret(id=secret_id)
+            lp_key_data = lp_key_secret.get_content().get("lpkey")
+
+            if lp_key_data is not None:
+                return str(lp_key_data)
+
+        except (KeyError, ops.SecretNotFoundError, ops.model.ModelError):
+            pass
+
+        return None
+
     @property
     def _git_ubuntu_primary_relation(self) -> ops.Relation | None:
         """Get the peer relation that contains the primary node IP.
@@ -193,6 +208,7 @@ def _refresh_importer_node(self) -> None:
 
         will_publish = self._is_publishing_active
         ssh_key_data = self._lpuser_ssh_key
+        lp_key_data = self._lpuser_lp_key
 
         if will_publish:
             if ssh_key_data is None:
@@ -205,13 +221,19 @@ def _refresh_importer_node(self) -> None:
                     GIT_UBUNTU_SYSTEM_USER_USERNAME, GIT_UBUNTU_USER_HOME_DIR, ssh_key_data
                 )
 
+        if lp_key_data is None:
+            logger.warning(
+                "Launchpad keyring entry unavailable, unable to gather package updates."
+            )
+        else:
+            usr.update_launchpad_keyring_secret(
+                GIT_UBUNTU_SYSTEM_USER_USERNAME, GIT_UBUNTU_USER_HOME_DIR, lp_key_data
+            )
+
         if self._is_primary:
             if not node.setup_primary_node(
                 GIT_UBUNTU_USER_HOME_DIR,
-                self._node_id,
-                self._num_workers,
                 GIT_UBUNTU_SYSTEM_USER_USERNAME,
-                will_publish,
                 self._controller_port,
             ):
                 self.unit.status = ops.BlockedStatus("Failed to install git-ubuntu services.")

src/importer_node.py

@@ -57,38 +57,21 @@ def setup_secondary_node(
 
 def setup_primary_node(
     git_ubuntu_user_home: str,
-    node_id: int,
-    num_workers: int,
     system_user: str,
-    push_to_lp: bool,
     primary_port: int,
 ) -> bool:
-    """Set up necessary services for a primary git-ubuntu importer node.
+    """Set up poller and broker services to create a primary git-ubuntu importer node.
 
     Args:
         git_ubuntu_user_home: The home directory of the git-ubuntu user.
-        node_id: The unique ID of this node.
-        num_workers: The number of worker instances to set up.
         system_user: The user + group to run the services as.
-        push_to_lp: True if publishing repositories to Launchpad.
         primary_port: The network port used for worker assignments.
 
     Returns:
         True if installation succeeded, False otherwise.
     """
     services_folder = pathops.LocalPath(git_ubuntu_user_home, "services")
 
-    if not setup_secondary_node(
-        git_ubuntu_user_home,
-        node_id,
-        num_workers,
-        system_user,
-        push_to_lp,
-        primary_port,
-        "127.0.0.1",
-    ):
-        return False
-
     # Setup broker service.
     if not git_ubuntu.setup_broker_service(
         services_folder.as_posix(),

src/user_management.py

@@ -30,6 +30,42 @@ def _run_command_as_user(user: str, command: str) -> bool:
     return True
 
 
+def _mkdir_for_user_with_error_checking(
+    directory: pathops.LocalPath, user: str, mode: int = 0o755
+) -> bool:
+    """Create a directory and handle possible mkdir errors.
+
+    Args:
+        directory: The directory to create, skipping if it exists.
+        user: The user who should own this directory.
+        mode: The permissions mode for the folder, defaults to standard rwxr-xr-x.
+
+    Returns:
+        True if the folder was created, False otherwise.
+    """
+    try:
+        directory.mkdir(parents=True, user=user, group=user, mode=mode)
+        return True
+    except FileExistsError:
+        logger.info("Directory %s already exists.", directory.as_posix())
+        return True
+    except NotADirectoryError:
+        logger.error("Directory location %s already exists as a file.", directory.as_posix())
+    except PermissionError:
+        logger.error(
+            "Unable to create new directory %s: permission denied.",
+            directory.as_posix(),
+        )
+    except LookupError:
+        logger.error(
+            "Unable to create directory %s: unknown user/group %s",
+            directory.as_posix(),
+            user,
+        )
+
+    return False
+
+
 def _clone_git_ubuntu_source(cloning_user: str, parent_directory: str, source_url: str) -> bool:
     """Clone the git-ubuntu git repo to a given directory.
 
@@ -71,31 +107,8 @@ def _write_python_keyring_config_file(user: str, home_dir: str) -> bool:
     python_keyring_config = pathops.LocalPath(home_dir, ".config/python_keyring/keyringrc.cfg")
 
     parent_dir = python_keyring_config.parent
-    config_dir_success = False
-
-    try:
-        parent_dir.mkdir(parents=True, user=user, group=user)
-        config_dir_success = True
-    except FileExistsError:
-        logger.info("User config directory %s already exists.", parent_dir.as_posix())
-        config_dir_success = True
-    except NotADirectoryError:
-        logger.error(
-            "User config directory location %s already exists as a file.", parent_dir.as_posix()
-        )
-    except PermissionError:
-        logger.error(
-            "Unable to create new user config directory %s: permission denied.",
-            parent_dir.as_posix(),
-        )
-    except LookupError:
-        logger.error(
-            "Unable to create config directory %s: unknown user/group %s",
-            parent_dir.as_posix(),
-            user,
-        )
 
-    if not config_dir_success:
+    if not _mkdir_for_user_with_error_checking(parent_dir, user):
         return False
 
     keyring_config_success = False
@@ -151,27 +164,8 @@ def setup_git_ubuntu_user_files(user: str, home_dir: str, git_ubuntu_source_url:
 
     # Create the services folder if it does not yet exist
     services_dir = pathops.LocalPath(home_dir, "services")
-    services_dir_success = False
-
-    try:
-        services_dir.mkdir(parents=True, user=user, group=user)
-        logger.info("Created services directory %s.", services_dir)
-        services_dir_success = True
-    except FileExistsError:
-        logger.info("Services directory %s already exists.", services_dir)
-        services_dir_success = True
-    except NotADirectoryError:
-        logger.error("Service directory location %s already exists as a file.", services_dir)
-    except PermissionError:
-        logger.error("Unable to create new service directory %s: permission denied.", services_dir)
-    except LookupError:
-        logger.error(
-            "Unable to create service directory %s: unknown user/group %s",
-            services_dir,
-            user,
-        )
 
-    if not services_dir_success:
+    if not _mkdir_for_user_with_error_checking(services_dir, user):
         return False
 
     return _write_python_keyring_config_file(user, home_dir)
@@ -191,31 +185,8 @@ def update_ssh_private_key(user: str, home_dir: str, ssh_key_data: str) -> bool:
     ssh_key_file = pathops.LocalPath(home_dir, ".ssh/id")
 
     parent_dir = ssh_key_file.parent
-    ssh_dir_success = False
 
-    try:
-        parent_dir.mkdir(mode=0o700, parents=True, user=user, group=user)
-        ssh_dir_success = True
-    except FileExistsError:
-        logger.info("ssh directory %s already exists.", parent_dir.as_posix())
-        ssh_dir_success = True
-    except NotADirectoryError:
-        logger.error(
-            "User ssh directory location %s already exists as a file.", parent_dir.as_posix()
-        )
-    except PermissionError:
-        logger.error(
-            "Unable to create user ssh directory %s: permission denied.",
-            parent_dir.as_posix(),
-        )
-    except LookupError:
-        logger.error(
-            "Unable to create user ssh directory %s: unknown user/group %s",
-            parent_dir.as_posix(),
-            user,
-        )
-
-    if not ssh_dir_success:
+    if not _mkdir_for_user_with_error_checking(parent_dir, user, 0o700):
         return False
 
     key_success = False
@@ -238,6 +209,44 @@ def update_ssh_private_key(user: str, home_dir: str, ssh_key_data: str) -> bool:
     return key_success
 
 
+def update_launchpad_keyring_secret(user: str, home_dir: str, lp_key_data: str) -> bool:
+    """Create or refresh the python keyring file for launchpad access.
+
+    Args:
+        user: The git-ubuntu user.
+        home_dir: The home directory for the user.
+        lp_key_data: The private keyring data.
+
+    Returns:
+        True if directory and file creation succeeded, False otherwise.
+    """
+    lp_key_file = pathops.LocalPath(home_dir, ".local/share/python_keyring/keyring_pass.cfg")
+
+    parent_dir = lp_key_file.parent
+
+    if not _mkdir_for_user_with_error_checking(parent_dir, user):
+        return False
+
+    key_success = False
+
+    try:
+        lp_key_file.write_text(
+            lp_key_data,
+            mode=0o600,
+            user=user,
+            group=user,
+        )
+        key_success = True
+    except (FileNotFoundError, NotADirectoryError) as e:
+        logger.error("Failed to create lp key entry due to directory issues: %s", str(e))
+    except LookupError as e:
+        logger.error("Failed to create lp key entry due to issues with root user: %s", str(e))
+    except PermissionError as e:
+        logger.error("Failed to create lp key entry due to permission issues: %s", str(e))
+
+    return key_success
+
+
 def set_snap_homedirs(home_dir: str) -> bool:
     """Allow snaps to run for a user with a given home directory.
 

tests/integration/test_charm.py

@@ -67,18 +67,19 @@ def get_services_dict(unit_name: str, juju: jubilant.Juju) -> dict[str, dict[str
                 == "git-ubuntu importer service poller"
             )
 
-        node_id = int(unit_name.split("/")[-1])
+        else:
+            node_id = int(unit_name.split("/")[-1])
 
-        assert services[f"git-ubuntu-importer-service-worker{node_id}_0.service"]["active"]
-        assert (
-            services[f"git-ubuntu-importer-service-worker{node_id}_0.service"]["description"]
-            == "git-ubuntu importer service worker"
-        )
-        assert services[f"git-ubuntu-importer-service-worker{node_id}_1.service"]["active"]
-        assert (
-            services[f"git-ubuntu-importer-service-worker{node_id}_1.service"]["description"]
-            == "git-ubuntu importer service worker"
-        )
+            assert services[f"git-ubuntu-importer-service-worker{node_id}_0.service"]["active"]
+            assert (
+                services[f"git-ubuntu-importer-service-worker{node_id}_0.service"]["description"]
+                == "git-ubuntu importer service worker"
+            )
+            assert services[f"git-ubuntu-importer-service-worker{node_id}_1.service"]["active"]
+            assert (
+                services[f"git-ubuntu-importer-service-worker{node_id}_1.service"]["description"]
+                == "git-ubuntu importer service worker"
+            )
 
 
 def test_installed_apps(app: str, juju: jubilant.Juju):

tests/unit/test_importer_node.py

@@ -37,19 +37,14 @@ def test_setup_secondary_node_failure(mock_setup_worker):
 
 @patch("importer_node.git_ubuntu.setup_poller_service")
 @patch("importer_node.git_ubuntu.setup_broker_service")
-@patch("importer_node.setup_secondary_node")
-def test_setup_primary_node_success(mock_secondary, mock_broker, mock_poller):
+def test_setup_primary_node_success(mock_broker, mock_poller):
     """Test successful primary node setup."""
-    mock_secondary.return_value = True
     mock_broker.return_value = True
     mock_poller.return_value = True
 
-    result = importer_node.setup_primary_node(
-        "/var/local/git-ubuntu", 1, 2, "git-ubuntu", True, 1692
-    )
+    result = importer_node.setup_primary_node("/var/local/git-ubuntu", "git-ubuntu", 1692)
 
     assert result is True
-    mock_secondary.assert_called_once()
     mock_broker.assert_called_once()
     mock_poller.assert_called_once()
 
@@ -59,9 +54,7 @@ def test_setup_primary_node_secondary_failure(mock_secondary):
     """Test primary node setup with secondary failure."""
     mock_secondary.return_value = False
 
-    result = importer_node.setup_primary_node(
-        "/var/local/git-ubuntu", 1, 2, "git-ubuntu", True, 1692
-    )
+    result = importer_node.setup_primary_node("/var/local/git-ubuntu", "git-ubuntu", 1692)
 
     assert result is False
 
@@ -73,9 +66,7 @@ def test_setup_primary_node_broker_failure(mock_secondary, mock_broker):
     mock_secondary.return_value = True
     mock_broker.return_value = False
 
-    result = importer_node.setup_primary_node(
-        "/var/local/git-ubuntu", 1, 2, "git-ubuntu", True, 1692
-    )
+    result = importer_node.setup_primary_node("/var/local/git-ubuntu", "git-ubuntu", 1692)
 
     assert result is False
 
@@ -89,9 +80,7 @@ def test_setup_primary_node_poller_failure(mock_secondary, mock_broker, mock_pol
     mock_broker.return_value = True
     mock_poller.return_value = False
 
-    result = importer_node.setup_primary_node(
-        "/var/local/git-ubuntu", 1, 2, "git-ubuntu", True, 1692
-    )
+    result = importer_node.setup_primary_node("/var/local/git-ubuntu", "git-ubuntu", 1692)
 
     assert result is False