Merge pull request #2608 from lsst-sqre/tickets/DM-40947

DM-40947: Add documentation for shared charts

applications/livetap/Chart.yaml

@@ -9,4 +9,4 @@ sources:
 dependencies:
   - name: cadc-tap
     version: 1.0.0
-    repository: "file://../../charts/cadc-tap/"
+    repository: "file://../../charts/cadc-tap"

applications/production-tools/Chart.yaml

@@ -1,7 +1,6 @@
 apiVersion: v2
 name: production-tools
 version: 1.0.0
-dependencies:
 description: A collection of utility pages for monitoring data processing.
 sources:
   - https://github.com/lsst-dm/production_tools

applications/ssotap/Chart.yaml

@@ -9,4 +9,4 @@ sources:
 dependencies:
   - name: cadc-tap
     version: 1.0.0
-    repository: "file://../../charts/cadc-tap/"
+    repository: "file://../../charts/cadc-tap"

applications/tap/Chart.yaml

@@ -9,4 +9,4 @@ sources:
 dependencies:
   - name: cadc-tap
     version: 1.0.0
-    repository: "file://../../charts/cadc-tap/"
+    repository: "file://../../charts/cadc-tap"

charts/README.md

@@ -12,7 +12,7 @@ To use a chart in this directory, use a dependency stanza similar to the followi
 dependencies:
   - name: cadc-tap
     version: 1.0.0
-    repository: "file://../../charts/cadc-tap/"
+    repository: "file://../../charts/cadc-tap"
 ```
 
 If a Helm chart should be usable independently of Phalanx and warrants a separate existence with its own version number, that chart should instead go into the [charts](https://github.com/lsst-sqre/charts) repository.

docs/about/repository.rst

@@ -59,11 +59,26 @@ installer directory
 :bdg-link-primary-line:`Browse installer/ on GitHub <https://github.com/lsst-sqre/phalanx/tree/main/installer>`
 
 This directory contains a script named `install.sh <https://github.com/lsst-sqre/phalanx/blob/main/installer/install.sh>`__.
-The arguments to this are the name of the environment, the FQDN, and the read key for Vault (see :ref:`secrets` for more details on Vault).
+The arguments to this are the name of the environment, the Vault RoleID, and the Vault SecretID (see :ref:`secrets` for more details on Vault).
 This installer script is the entry point for setting up a new environment.
 It can also be run on an existing environment to update it.
 See the :ref:`environment bootstrapping documentation <bootstrapping-toc>` for details.
 
+charts directory
+----------------
+
+:bdg-link-primary-line:`Browse charts/ on GitHub <https://github.com/lsst-sqre/phalanx/tree/main/charts>`
+
+This directory contains Helm charts shared by multiple Phalanx applications that are not generally useful enough to warrant separate publication in a proper Helm chart repository.
+
+In some cases, several Phalanx applications should use common Helm templates to avoid duplication.
+The best way to do this within Helm is to use a subchart.
+This can be done by publishing a separate Helm chart using the `charts repository <https://github.com/lsst-sqre/charts>`__, but publication as a Helm chart implies that the chart may be useful outside of Phalanx.
+Sometimes these shared subcharts are merely artifacts of code organization and deduplication within Phalanx, and should not have an independent existence outside of Phalanx.
+In those cases, they're maintained in the :file:`charts` directory.
+
+See :doc:`/developers/shared-charts` for details.
+
 docs directory
 --------------
 

docs/developers/index.rst

@@ -24,6 +24,7 @@ Individual applications are documented in the :doc:`/applications/index` section
 
    write-a-helm-chart
    add-external-chart
+   shared-charts
    define-secrets
    add-application
 

docs/developers/shared-charts.rst

@@ -0,0 +1,65 @@
+######################################
+Sharing subcharts between applications
+######################################
+
+In some cases, you may want to instantiate multiple Phalanx applications from mostly the same Helm chart.
+For example, Phalanx contains multiple TAP server applications (:px-app:`tap`, :px-app:`ssotap`, and :px-app:`livetap`) that are all deployments of the CADC TAP server.
+The Helm template resources should be shared among those applications to avoid code duplication, unnecessary maintenance overhead, and unintentional inconsistencies.
+
+There are two options for how to handle cases like this:
+
+#. Publish a generic Helm chart for the underlying service using the `charts repository <https://github.com/lsst-sqre/charts>`__, and then use it like any other external chart.
+   See :doc:`add-external-chart` for more details on how to use an external chart within Phalanx.
+
+#. Use a shared chart within Phalanx.
+   This is more appropriate if the chart is only useful inside Phalanx and doesn't make sense to publish as a stand-alone Helm chart.
+   The shared chart is included as a subchart in each Phalanx application that needs roughly the same resources.
+
+This document describes the second choice.
+
+Writing the shared subchart
+===========================
+
+Shared subcharts go into the `charts directory <https://github.com/lsst-sqre/phalanx/tree/main/charts>`__.
+Each subdirectory of that directory is a Helm chart, similar to the structure of the :file:`applications` directory.
+Those Helm charts should follow our normal Phalanx chart conventions from :doc:`write-a-helm-chart`.
+For example, the ``version`` field of every chart should be set to ``1.0.0``, since these charts will not be published and don't need version tracking.
+
+Usually, the easiest way to create a shared subchart is to start by writing a regular application chart for one instance of the application following the instructions in :doc:`write-a-helm-chart`.
+Then, copy that application chart into a subdirectory in the :file:`charts` directory, remove all the parts that don't make sense to share between applications, and add any additional :file:`values.yaml` settings that will be required to customize the instantiation of this chart for different applications.
+
+Shared charts do not have :file:`values-{environment}.yaml` files and are not aware of Phalanx environments.
+Any per-environment settings must be handled in the parent charts that use this subchart and passed down as regular :file:`values.yaml` overrides.
+
+Shared charts do not have :file:`secrets.yaml` files.
+All application secrets must be defined by the application charts in the :file:`applications` directory.
+This may mean there is some duplication of secrets between applications.
+This is intentional; often, one application should be the owner of those secrets and other applications should use ``copy`` directives to use the same secret value.
+
+Any documentation URLs such as ``home``, ``sources``, and ``phalanx.lsst.io/docs`` annotations in the shared chart will be ignored.
+They can be included in the shared chart for reference, but each application will need to copy that information into its own :file:`Chart.yaml` file for it to show up in the generated Phalanx documentation.
+
+Using a shared subchart
+=======================
+
+To use a shared subchart, reference it as a dependency in :file:`Chart.yaml` the way that you would use any other Helm chart as a subchart, but use a ``file:`` URL to point to the shared chart directory.
+For example:
+
+.. code-block:: yaml
+   :caption: applications/tap/Chart.yaml
+
+   dependencies:
+     - name: cadc-tap
+       version: 1.0.0
+       repository: "file://../../charts/cadc-tap"
+
+Note the relative ``file:`` URL, which ensures the chart comes from the same checkout of Phalanx as the application chart.
+The ``version`` in the dependency must always be ``1.0.0``.
+
+Don't forget to copy any relevant ``home``, ``sources``, or ``annotations`` settings from the shared chart into the application :file:`Chart.yaml` so that it will be included in the generated Phalanx documentation.
+
+Next steps
+==========
+
+- Define the secrets needed by each application: :doc:`define-secrets`
+- Add the Argo CD applications to appropriate environments: :doc:`add-application`

docs/developers/write-a-helm-chart.rst

@@ -11,6 +11,9 @@ For first-party charts, the :file:`templates` directory is generally richly popu
 Here are instructions for writing a Helm chart for a newly-developed application.
 If you are using an external third-party chart to deploy part of the application, also see :doc:`add-external-chart`.
 
+In some cases where there is a lot of internal duplication between multiple Phalanx applications, those applications should share a subchart that encapsulates that duplication.
+See :doc:`shared-charts` if you think that may be the case for your application.
+
 .. _dev-chart-starters:
 
 Start from a template

src/phalanx/exceptions.py

@@ -13,6 +13,9 @@
     "InvalidApplicationConfigError",
     "InvalidEnvironmentConfigError",
     "InvalidSecretConfigError",
+    "MalformedOnepasswordSecretError",
+    "MissingOnepasswordSecretsError",
+    "NoOnepasswordConfigError",
     "NoOnepasswordCredentialsError",
     "UnknownEnvironmentError",
     "UnresolvedSecretsError",

tests/config_test.py

@@ -2,7 +2,10 @@
 
 from __future__ import annotations
 
+import re
+from collections.abc import Iterator
 from pathlib import Path
+from typing import Literal
 
 import yaml
 
@@ -15,22 +18,27 @@
 """Temporary whitelist of applications that haven't added secrets.yaml."""
 
 
+def all_charts(
+    parent: Literal["applications", "charts"],
+) -> Iterator[Path]:
+    """Iterate through all chart paths."""
+    root_path = Path(__file__).parent.parent / parent
+    for candidate in root_path.iterdir():
+        if not candidate.is_dir():
+            continue
+        yield candidate
+
+
 def test_application_version() -> None:
     """All application charts should have version 1.0.0."""
-    applications_path = Path(__file__).parent.parent / "applications"
-    for application in applications_path.iterdir():
-        if not application.is_dir():
-            continue
+    for application in all_charts("applications"):
         chart = yaml.safe_load((application / "Chart.yaml").read_text())
         assert (
             chart["version"] == "1.0.0"
         ), f"Chart for application {application.name} has incorrect version"
 
     # Check the same thing for shared charts.
-    Path(__file__).parent.parent / "charts"
-    for shared_chart in applications_path.iterdir():
-        if not shared_chart.is_dir():
-            continue
+    for shared_chart in all_charts("charts"):
         chart = yaml.safe_load((shared_chart / "Chart.yaml").read_text())
         assert (
             chart["version"] == "1.0.0"
@@ -39,9 +47,8 @@ def test_application_version() -> None:
 
 def test_secrets_defined() -> None:
     """Any application with a VaultSecret should have secrets.yaml."""
-    applications_path = Path(__file__).parent.parent / "applications"
-    for application in applications_path.iterdir():
-        if not application.is_dir() or application.name in _ALLOW_NO_SECRETS:
+    for application in all_charts("applications"):
+        if application.name in _ALLOW_NO_SECRETS:
             continue
         if list(application.glob("secrets*.yaml")):
             continue
@@ -62,3 +69,25 @@ def test_secrets_defined() -> None:
                     " resource but has no secrets.yaml configuration"
                 )
                 raise AssertionError(msg)
+
+
+def test_shared_subcharts() -> None:
+    """Check references to shared subcharts."""
+    available = [c.name for c in all_charts("charts")]
+    for application in all_charts("applications"):
+        chart = yaml.safe_load((application / "Chart.yaml").read_text())
+        chart.get("dependencies")
+        for dependency in chart.get("dependencies", []):
+            if not re.match("file:", dependency.get("repository", "")):
+                continue
+            name = application.name
+            version = dependency.get("version")
+            repository = dependency["repository"]
+            m = re.match(r"file://[.][.]/[.][.]/charts/([^/]+)$", repository)
+            assert m, f"Incorrect shared chart URL in {name}: {repository}"
+            assert (
+                m.group(1) in available
+            ), f"Missing shared chart dependency {m.group(1)} in {name}"
+            assert (
+                dependency["version"] == "1.0.0"
+            ), f"Incorrect shared chart version in {name}: {version} != 1.0.0"