diff --git a/applications/livetap/Chart.yaml b/applications/livetap/Chart.yaml index 8d2668f59b..650616a8d9 100644 --- a/applications/livetap/Chart.yaml +++ b/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" diff --git a/applications/production-tools/Chart.yaml b/applications/production-tools/Chart.yaml index 95add46d37..3df45ea6a5 100644 --- a/applications/production-tools/Chart.yaml +++ b/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 diff --git a/applications/ssotap/Chart.yaml b/applications/ssotap/Chart.yaml index ce8558f9e4..232e7f780d 100644 --- a/applications/ssotap/Chart.yaml +++ b/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" diff --git a/applications/tap/Chart.yaml b/applications/tap/Chart.yaml index a5f99cd97a..fa6f85e365 100644 --- a/applications/tap/Chart.yaml +++ b/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" diff --git a/charts/README.md b/charts/README.md index 3a868d1d92..af2a8f66c6 100644 --- a/charts/README.md +++ b/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. diff --git a/docs/about/repository.rst b/docs/about/repository.rst index 251aafc696..5546459183 100644 --- a/docs/about/repository.rst +++ b/docs/about/repository.rst @@ -59,11 +59,26 @@ installer directory :bdg-link-primary-line:`Browse installer/ on GitHub ` This directory contains a script named `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 ` for details. +charts directory +---------------- + +:bdg-link-primary-line:`Browse charts/ on GitHub ` + +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 `__, 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 -------------- diff --git a/docs/developers/index.rst b/docs/developers/index.rst index 991327cce5..bb8d0bbf7e 100644 --- a/docs/developers/index.rst +++ b/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 diff --git a/docs/developers/shared-charts.rst b/docs/developers/shared-charts.rst new file mode 100644 index 0000000000..a0f4edd0f9 --- /dev/null +++ b/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 `__, 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 `__. +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` diff --git a/docs/developers/write-a-helm-chart.rst b/docs/developers/write-a-helm-chart.rst index ebef7d96cf..3e1ead63de 100644 --- a/docs/developers/write-a-helm-chart.rst +++ b/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 diff --git a/src/phalanx/exceptions.py b/src/phalanx/exceptions.py index f349c59259..fc7102d2a0 100644 --- a/src/phalanx/exceptions.py +++ b/src/phalanx/exceptions.py @@ -13,6 +13,9 @@ "InvalidApplicationConfigError", "InvalidEnvironmentConfigError", "InvalidSecretConfigError", + "MalformedOnepasswordSecretError", + "MissingOnepasswordSecretsError", + "NoOnepasswordConfigError", "NoOnepasswordCredentialsError", "UnknownEnvironmentError", "UnresolvedSecretsError", diff --git a/tests/config_test.py b/tests/config_test.py index d17f3c6d74..26a6936304 100644 --- a/tests/config_test.py +++ b/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"