Sync develop → main for v1.3.5 chart release

.github/workflows/helm-ci.yaml

@@ -5,11 +5,13 @@ on:
     branches: [main, openshift]
     paths:
       - 'client/**'
+      - 'ingestor/**'
       - '.github/workflows/helm-ci.yaml'
   pull_request:
     branches: [main, openshift]
     paths:
       - 'client/**'
+      - 'ingestor/**'
       - '.github/workflows/helm-ci.yaml'
 
 jobs:
@@ -24,13 +26,24 @@ jobs:
         with:
           version: v3.15.4
 
-      - name: Helm lint (strict) — all platforms
+      - name: Helm lint (strict) — parent client chart, all platforms
         run: |
           for f in client/ci/*-values.yaml; do
-            echo "=== Linting with $f ==="
+            echo "=== Linting client with $f ==="
             helm lint --strict ./client -f "$f"
           done
 
+      - name: Helm lint (strict) — ingestor subchart
+        # The ingestor subchart has no per-platform CI values files because
+        # its templates don't branch on platform — they only render a
+        # ConfigMap + Job + SA. The chart's one required value
+        # (`ingestConfig`) is supplied by customers at install time via
+        # `--set-file`; missing it during lint emits an INFO, not a FAIL,
+        # so plain `helm lint --strict ./ingestor` exercises the templates
+        # cleanly. See #135 for why we lint it here at all (publish workflow
+        # now packages both charts).
+        run: helm lint --strict ./ingestor
+
   template:
     name: Template render
     runs-on: ubuntu-latest

.github/workflows/release-helm-chart.yaml

@@ -6,10 +6,6 @@
 name: Release Helm Chart
 
 on:
-  push:
-    tags:
-      - 'client-v*'
-      - 'v*'
   release:
     types: [published]
 
@@ -26,37 +22,59 @@ jobs:
       id-token: write
     steps:
       - name: Checkout
+        # Pin ref to the release tag rather than letting checkout default to
+        # github.ref, which intermittently arrives empty on release-triggered
+        # runs (actions/runner#2788, still open). An empty default would cause
+        # checkout to fall back to the repo default branch and package the
+        # chart from the wrong commit.
         uses: actions/checkout@v4
         with:
+          ref: ${{ github.event.release.tag_name }}
           fetch-depth: 0
 
       - name: Set up Helm
         uses: azure/setup-helm@v4
         with:
           version: v3.15.4
 
-      - name: Lint chart
-        # Use a CI values file so `--strict` sees non-empty clientId /
-        # clientPassword (the defaults in values.yaml are deliberately empty
-        # to force operators to supply real credentials, and the schema
-        # enforces minLength:1). Exhaustive multi-platform linting happens in
-        # helm-ci.yaml on every PR — here we just need the chart to lint
-        # cleanly for packaging.
-        run: helm lint --strict ./client -f client/ci/eks-values.yaml
+      - name: Lint charts
+        # The parent `client` chart needs a CI values file so `--strict` sees
+        # non-empty clientId / clientPassword (the defaults in values.yaml are
+        # deliberately empty to force operators to supply real credentials, and
+        # the schema enforces minLength:1). The `ingestor` subchart needs no
+        # values file — its only required value (ingestConfig) emits an INFO
+        # level, not FAIL, when missing, and customers always supply it via
+        # --set-file at install time.
+        # Exhaustive multi-platform linting happens in helm-ci.yaml on every
+        # PR — here we just need both charts to lint cleanly for packaging.
+        run: |
+          helm lint --strict ./client -f client/ci/eks-values.yaml
+          helm lint --strict ./ingestor
 
-      - name: Package tracebloc chart
+      - name: Package tracebloc charts
         id: package
+        # Package BOTH the parent client chart AND the ingestor subchart.
+        # Customers run two flavors of install:
+        #   1. `helm install tracebloc/client ...` — bootstraps the cluster.
+        #   2. `helm install tracebloc/ingestor ...` — per-dataset ingestion.
+        # Both must resolve from the same helm repo, so both .tgz files need
+        # to live alongside each other in gh-pages and be referenced in a
+        # single shared index.yaml. Earlier versions packaged only `./client`
+        # and the second install path failed with "chart not found" — see #135.
         run: |
           helm package ./client
-          TGZ=$(ls -t client-*.tgz | head -1)
-          echo "chart_tgz=$TGZ" >> $GITHUB_OUTPUT
-          echo "Packaged $TGZ"
+          helm package ./ingestor
+          echo "Packaged:"
+          ls -la *.tgz
 
-      - name: Upload chart artifact
+      - name: Upload chart artifacts
         uses: actions/upload-artifact@v4
         with:
-          name: helm-chart
-          path: ${{ steps.package.outputs.chart_tgz }}
+          name: helm-charts
+          # Glob picks up both client-*.tgz and ingestor-*.tgz.
+          path: |
+            client-*.tgz
+            ingestor-*.tgz
 
       - name: Configure Git
         run: |
@@ -68,7 +86,8 @@ jobs:
         run: |
           # Remove any local chart packages before switching branches to avoid
           # "untracked working tree files would be overwritten" checkout errors.
-          rm -f client-*.tgz || true
+          # Cover both chart name prefixes — see Package step above.
+          rm -f client-*.tgz ingestor-*.tgz || true
           if git fetch origin gh-pages 2>/dev/null; then
             git checkout gh-pages
             echo "index_exists=true" >> $GITHUB_OUTPUT
@@ -78,34 +97,45 @@ jobs:
             echo "index_exists=false" >> $GITHUB_OUTPUT
           fi
 
-      - name: Download chart artifact
+      - name: Download chart artifacts
         uses: actions/download-artifact@v4
         with:
-          name: helm-chart
+          name: helm-charts
 
       - name: Update index and push to gh-pages
         env:
           REPO_URL: https://${{ github.repository_owner }}.github.io/${{ github.event.repository.name }}
         run: |
+          # `helm repo index .` indexes every .tgz in the working dir, so both
+          # client-*.tgz and ingestor-*.tgz get listed in the shared
+          # index.yaml. Customers then resolve `tracebloc/client` and
+          # `tracebloc/ingestor` from the same repo URL.
           if [ "${{ steps.fetch.outputs.index_exists }}" = "true" ] && [ -f index.yaml ]; then
             helm repo index . --url "$REPO_URL" --merge index.yaml
           else
             helm repo index . --url "$REPO_URL"
           fi
-          git add index.yaml client-*.tgz
+          git add index.yaml client-*.tgz ingestor-*.tgz
           git status
           if git diff --staged --quiet; then
             echo "No index/tgz changes to commit"
           else
-            git commit -m "Release helm chart(s): $(ls client-*.tgz 2>/dev/null | tr '\n' ' ')"
+            git commit -m "Release helm chart(s): $(ls client-*.tgz ingestor-*.tgz 2>/dev/null | tr '\n' ' ')"
             git push origin gh-pages
           fi
 
-      - name: Upload chart to GitHub Release (on tag)
-        if: startsWith(github.ref, 'refs/tags/')
+      - name: Upload charts to GitHub Release
+        # Pin tag_name from the release event payload rather than relying on
+        # github.ref / github.ref_name, which intermittently arrive empty on
+        # release-triggered runs (actions/runner#2788, still open).
+        # Attach BOTH chart tgzs so downloaders pinning a specific release
+        # tag can pull either chart's exact bytes from this release page.
         uses: softprops/action-gh-release@v2
         with:
-          files: client-*.tgz
+          tag_name: ${{ github.event.release.tag_name }}
+          files: |
+            client-*.tgz
+            ingestor-*.tgz
           generate_release_notes: true
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/wip-limit-check.yml

@@ -0,0 +1,10 @@
+name: WIP limit check
+
+on:
+  pull_request:
+    types: [opened, ready_for_review]
+
+jobs:
+  check:
+    uses: tracebloc/.github/.github/workflows/wip-limit-check.yml@main
+    secrets: inherit

.gitignore

@@ -118,6 +118,11 @@ venv.bak/
 /values*.yaml
 /*/values*.yaml
 !client/values*.yaml
+# The ingestor subchart's values.yaml is part of the chart shipped to
+# customers — must be tracked despite matching the `/*/values*.yaml`
+# anti-leak pattern above (which exists to keep operator-local values
+# files out of the repo).
+!ingestor/values.yaml
 secret-values.yaml
 test-template.yaml
 eks.diff

client/Chart.yaml

@@ -2,8 +2,8 @@ apiVersion: v2
 name: client
 description: A unified Helm chart for tracebloc on AKS, EKS, bare-metal, and OpenShift
 type: application
-version: 1.3.2
-appVersion: "1.3.2"
+version: 1.3.5
+appVersion: "1.3.5"
 keywords:
   - tracebloc
   - kubernetes

client/MIGRATION.md

@@ -2,6 +2,76 @@
 
 This guide explains how to migrate from the legacy per-platform charts (`aks/`, `bm/`, `eks/`, `oc/`) to the unified `client/` chart.
 
+## Upgrading to 1.3.4 — parent chart owns the shared ingestor ServiceAccount
+
+[#129](https://github.com/tracebloc/client/issues/129): the ingestor
+ServiceAccount has moved from the `tracebloc/ingestor` subchart into this
+parent chart. Background: the SA is shared by every ingestor subchart
+release in a namespace, but per-release Helm ownership meant two concurrent
+`helm install tracebloc/ingestor` calls collided with "cannot import into
+current release", and uninstalling the first release ripped the SA out
+from under all the others. With the SA in the parent chart, every
+ingestor release in the namespace shares it cleanly and `helm uninstall`
+of any individual ingestor release leaves it alone.
+
+> **The matching ingestor subchart change** ships as
+> `tracebloc/ingestor` **0.2.0** — `serviceAccount.create` default
+> flipped from `true` to `false`. Upgrade the subchart releases in
+> lockstep with the parent so they stop trying to own the SA.
+
+### When you need to adopt an existing SA
+
+If you already have a `tracebloc/ingestor` 0.1.0 release installed in the
+same namespace as this `tracebloc/client` release, `kubectl get sa
+ingestor -n <ns> -o jsonpath='{.metadata.annotations.meta\.helm\.sh/release-name}'`
+returns that subchart release's name. Plain `helm upgrade tracebloc/client`
+to 1.3.4 will fail with `Unable to continue with update: ServiceAccount
+"ingestor" ... exists and cannot be imported into the current release`.
+
+Transfer Helm ownership before upgrading:
+
+```bash
+# 1. Identify the values you need.
+NAMESPACE=<your-tracebloc-namespace>
+CLIENT_RELEASE=<your-client-release-name>          # e.g. "tracebloc"
+SA_NAME=ingestor                                    # or ingestionAuthz.serviceAccountName if overridden
+
+# 2. Re-annotate the SA so Helm sees the parent client release as its owner.
+kubectl annotate sa "$SA_NAME" -n "$NAMESPACE" \
+  meta.helm.sh/release-name="$CLIENT_RELEASE" \
+  meta.helm.sh/release-namespace="$NAMESPACE" \
+  --overwrite
+
+kubectl label sa "$SA_NAME" -n "$NAMESPACE" \
+  app.kubernetes.io/managed-by=Helm \
+  --overwrite
+
+# 3. Now run the upgrade — Helm adopts the SA on next reconcile.
+helm upgrade "$CLIENT_RELEASE" tracebloc/client \
+  -n "$NAMESPACE" --version 1.3.4 --reset-then-reuse-values
+
+# 4. Upgrade each ingestor subchart release to 0.2.0 so it stops trying
+#    to create the SA itself. The flipped default does this for you, but
+#    use --reset-then-reuse-values so pre-0.2.0 stored values don't
+#    re-apply serviceAccount.create=true.
+helm upgrade <ingestor-release> tracebloc/ingestor \
+  -n "$NAMESPACE" --version 0.2.0 --reset-then-reuse-values
+```
+
+If no ingestor 0.1.0 release exists in the namespace yet, you don't have
+to do anything — the parent chart creates the SA on first install of
+1.3.4 and subsequent ingestor 0.2.0 releases consume it.
+
+### `--reuse-values` upgrade path
+
+Operators using plain `--reuse-values` (or the auto-upgrade cronjob
+prior to 1.3.0, which used that flag) won't get the new
+`ingestionAuthz.serviceAccountName` default. The chart's template
+defaults the value to `"ingestor"` when absent, so the SA is created
+with the expected name and existing `allowed` entries keep matching.
+No template-level breakage; this is the same nil-guard pattern as
+[#124](https://github.com/tracebloc/client/pull/124).
+
 ## Upgrading to 1.3.0 — self-upgrade CronJob lands on by default
 
 Releases of 1.3.0+ install a `<release>-auto-upgrade` CronJob that polls

client/templates/_helpers.tpl

@@ -25,6 +25,20 @@ app.kubernetes.io/instance: {{ .Release.Name }}
 {{ .Release.Name }}-jobs-manager
 {{- end }}
 
+{{/*
+  Name of the shared ServiceAccount the parent chart creates for ingestor
+  subchart releases. Single source of truth — used by:
+    - templates/ingestor-serviceaccount.yaml (creates the SA)
+    - templates/ingestion-authz-configmap.yaml (default authz entry)
+  The ingestor subchart's post-install hook runs as this SA; jobs-manager
+  validates its token via TokenReview against `ingestionAuthz.allowed`.
+  Nil-guarded: pre-#129 stored values from `--reuse-values` upgrades won't
+  have `ingestionAuthz.serviceAccountName`, so default to "ingestor".
+*/}}
+{{- define "tracebloc.ingestorServiceAccountName" -}}
+{{- (default dict .Values.ingestionAuthz).serviceAccountName | default "ingestor" -}}
+{{- end }}
+
 {{/*
   Release-scoped name for the resource-monitor DaemonSet, ServiceAccount,
   ClusterRoleBinding subject, and selector/pod labels. Multiple releases

client/templates/ingestion-authz-configmap.yaml

@@ -0,0 +1,43 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  # client-runtime#21: defines which ServiceAccount(s) may call
+  # POST /internal/submit-ingestion-run on jobs-manager, scoped to which
+  # tables. Mounted into jobs-manager at /etc/tracebloc/ingestion-authz.yaml
+  # and read at startup (see submit_ingestion_run.load_authz_policy).
+  #
+  # Each entry in `ingestionAuthz.allowed` maps a (namespace, service_account)
+  # to a list of table-name prefixes. Omit `namespace` to default to the
+  # release's namespace — the common case where the ingestor subchart is
+  # installed alongside the tracebloc client.
+  name: {{ .Release.Name }}-ingestion-authz
+  namespace: {{ .Release.Namespace }}
+  labels:
+    {{- include "tracebloc.labels" . | nindent 4 }}
+data:
+  ingestion-authz.yaml: |
+    allowed:
+    {{- /*
+        Nil-guarded chain: an upgrade with `--reuse-values` from a
+        pre-#123 release won't have `.Values.ingestionAuthz` in its
+        stored values, and an unguarded `.Values.ingestionAuthz.allowed`
+        crashes with "nil pointer evaluating interface {}.allowed".
+        `default dict` + `default list` collapse the missing parent /
+        missing child to an empty list, which renders as `allowed: []`
+        — fail-safe (the authz policy then denies every caller, which
+        is correct: there's no policy until the operator sets one).
+
+        Per-entry `service_account` is nil-guarded to fall back to
+        `ingestionAuthz.serviceAccountName` (#129) so the default
+        values.yaml entry doesn't need to repeat the SA name — change
+        the name in one place and both the SA template + this policy
+        pick it up.
+    */ -}}
+    {{- range default list (default dict .Values.ingestionAuthz).allowed }}
+      - service_account: {{ .service_account | default (include "tracebloc.ingestorServiceAccountName" $) | quote }}
+        namespace: {{ .namespace | default $.Release.Namespace | quote }}
+        table_prefixes:
+        {{- range .table_prefixes }}
+          - {{ . | quote }}
+        {{- end }}
+    {{- end }}

client/templates/ingestor-serviceaccount.yaml

@@ -0,0 +1,18 @@
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  # Shared ServiceAccount that every ingestor subchart release in this
+  # namespace runs its post-install hook as. Owned by the parent chart
+  # (not the subchart) because:
+  #   - multiple concurrent ingestor releases need to share it — per-release
+  #     ownership made the second `helm install tracebloc/ingestor` fail
+  #     with Helm's "cannot import into current release" error (#129);
+  #   - the matching `ingestionAuthz` ConfigMap is already owned here, so
+  #     the SA and the policy referencing it have the same lifecycle.
+  #
+  # The subchart's `serviceAccount.create` defaults to `false` as of
+  # ingestor 0.2.0 — it consumes this SA rather than creating its own.
+  name: {{ include "tracebloc.ingestorServiceAccountName" . }}
+  namespace: {{ .Release.Namespace }}
+  labels:
+    {{- include "tracebloc.labels" . | nindent 4 }}