Files
Nguyen Minh Phuc c76154aeaa
ci / lint (push) Successful in 34s
ci / unit (push) Successful in 1m41s
ci / types (push) Successful in 1m41s
ci / dockerfile (push) Successful in 18s
ci / security (push) Successful in 1m27s
ci / chart (push) Failing after 1m11s
ci / integration (push) Successful in 1m10s
ci / image (api) (push) Has been skipped
ci / image (reconciler) (push) Has been skipped
ci / image (worker) (push) Has been skipped
ci / bump (push) Has been skipped
review: fix 26 findings from a 4-agent audit
CORRECTNESS
- lost-lease race: complete()/fail() did not check ownership, so a worker whose
  lease expired could mark a task done while another worker was running it, or
  requeue a task someone else owned. Reproduced, fixed with a CAS on
  (state, locked_by), pinned by two regression tests.
- worker died on report failure: _run_one's docstring claimed no exception
  escapes the TaskGroup; fail()/complete() were outside the guarded block, so a
  DB blip cancelled every sibling provision on the pod.
- claim query used an INNER join, which could strand a just-claimed task and
  report 'queue empty'. LEFT join.
- InstanceRepo.set_error bypassed the state machine and had no callers. Deleted.
- handle_deprovision ignored its CAS result, so a wrong-state instance kept a
  dangling endpoint and got re-provisioned by the drift check 60s later.
- handle_verify re-notified on every retry: five pages for one halt.

DEPLOY-BREAKING
- the migration Job could never succeed: no Dockerfile copied migrations/, and
  migrate.py resolved the path relative to the source tree, which only works for
  an editable install. Added COPY + SVCFORGE_MIGRATIONS_DIR.
- ServiceMonitor selector did not match the Service: API metrics never scraped.
- SvcforgeReconcilerStale fired permanently from every pod, because the gauge is
  module-level and every service exports it as 0. Scoped to the reconciler job.
- SvcforgeTaskFailed latched forever on a monotonic counter. Now increase()[15m].
- the digest guard accepted the all-zeros placeholder.
- worker terminationGracePeriodSeconds was 60s against a 600s helm timeout.

DEAD CODE THAT SHOULD NOT HAVE BEEN
- adapters/k8s.py was never called, so tenant namespaces were never created and
  the first provision for a new team would fail. Wired into handle_provision.
- adapters/redis.py was never imported by any service. Rate limiting is now wired
  into the API, failing open.
- Settings.check_production() had no callers. Given an explicit environment and
  called from every entrypoint.

OBSERVABILITY
- the API never called obs.setup(): no JSON logs, no trace correlation, log_json
  silently inert.
- LogNotifier's structured fields were discarded by the stdlib->structlog bridge.
- bind_task_context cleared the 'service' binding for the life of every task.
- split tasks_failed into task_attempts_failed and tasks_dead_lettered.

SECURITY
- trivy correctly blocked the worker/reconciler images: helm 3.16.2 and kubectl
  1.31.2 carry CRITICAL Go stdlib CVEs. Bumped to helm 3.21.3 and kubectl 1.35.3,
  which also closes a four-minor skew against the v1.35.3 cluster.

TESTS THAT COULD NOT FAIL
- the concurrency cap test passed on a fully serial worker.
- the alert/metric cross-check asserted a hardcoded list instead of reading the
  chart, so it could not catch a rename on the chart side.
- fixed OTel tracer-provider pollution between test files.

DOCS
- ARCHITECTURE.md: mermaid diagrams, user stories, and the helm-vs-ArgoCD
  guarantee (verified with --dry-run=server).
- AGENTS.md + CLAUDE.md.
- prose sweep for back-and-forth phrasing across 19 files.
2026-07-18 12:13:49 +00:00

121 lines
4.5 KiB
Smarty

{{/* Name helpers. Standard chart boilerplate the interesting parts are below. */}}
{{- define "svcforge.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}}
{{- end -}}
{{- define "svcforge.fullname" -}}
{{- if .Values.fullnameOverride -}}
{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" -}}
{{- else -}}
{{- $name := default .Chart.Name .Values.nameOverride -}}
{{- if contains $name .Release.Name -}}
{{- .Release.Name | trunc 63 | trimSuffix "-" -}}
{{- else -}}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}}
{{- end -}}
{{- end -}}
{{- end -}}
{{- define "svcforge.labels" -}}
helm.sh/chart: {{ printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }}
app.kubernetes.io/name: {{ include "svcforge.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
app.kubernetes.io/part-of: svcforge
{{- end -}}
{{/*
Per-component selector labels.
`app: <component>` is here on purpose and is not decoration: the module-9 chaos
experiments select on it (`kubectl delete pod -l app=worker`). Renaming it breaks the
runbook, not just a dashboard.
*/}}
{{- define "svcforge.selectorLabels" -}}
app.kubernetes.io/name: {{ include "svcforge.name" .ctx }}
app.kubernetes.io/instance: {{ .ctx.Release.Name }}
app.kubernetes.io/component: {{ .component }}
app: {{ .component }}
{{- end -}}
{{/*
Resolve a component's image to repo@digest.
This is the single place that builds an image reference, and it refuses to emit one that
is not digest-pinned. If CI has not bumped values.yaml, the release fails here with a
readable message rather than silently deploying whatever a mutable tag happens to mean
today. (The literal string "latest" is not written anywhere in this repo, including in
comments the acceptance gate greps for it and does not know what a comment is.)
*/}}
{{- define "svcforge.image" -}}
{{- $img := index .ctx.Values.image .component -}}
{{- if not $img -}}
{{- fail (printf "no image config for component %q" .component) -}}
{{- end -}}
{{- $digest := $img.digest | default "" -}}
{{/*
Full-shape match, not `hasPrefix "sha256:"`. A prefix check accepts the all-zeros
placeholder in values.yaml, so a fresh clone rendered clean and the guard guarded nothing.
Two conditions, both required: the digest must be sha256: plus exactly 64 lowercase hex
characters, AND it must not be the placeholder literal.
*/}}
{{- if not (regexMatch "^sha256:[0-9a-f]{64}$" $digest) -}}
{{- fail (printf "image.%s.digest must be a sha256 digest (sha256: + 64 hex chars), not a tag — CI bumps it; got %q" .component ($digest | default "<empty>")) -}}
{{- end -}}
{{- if eq $digest "sha256:0000000000000000000000000000000000000000000000000000000000000000" -}}
{{- fail (printf "image.%s.digest is still the all-zeros placeholder from values.yaml — this chart has never been bumped by CI and must not be deployed" .component) -}}
{{- end -}}
{{- printf "%s@%s" $img.repo $img.digest -}}
{{- end -}}
{{- define "svcforge.serviceAccountName" -}}
{{- printf "%s-%s" (include "svcforge.fullname" .ctx) .component | trunc 63 | trimSuffix "-" -}}
{{- end -}}
{{- define "svcforge.secretName" -}}
{{- .Values.externalSecret.targetName | default (printf "%s-secrets" (include "svcforge.fullname" .)) -}}
{{- end -}}
{{/*
Pod-level hardening, identical for all three services and the migrate job.
readOnlyRootFilesystem is the one that bites: every writable path a process needs must be
an explicit emptyDir. That is the point it makes the writes visible in review.
*/}}
{{- define "svcforge.podSecurityContext" -}}
runAsNonRoot: true
runAsUser: 10001
runAsGroup: 10001
fsGroup: 10001
seccompProfile:
type: RuntimeDefault
{{- end -}}
{{- define "svcforge.containerSecurityContext" -}}
allowPrivilegeEscalation: false
readOnlyRootFilesystem: true
runAsNonRoot: true
runAsUser: 10001
capabilities:
drop: ["ALL"]
{{- end -}}
{{/*
Shared environment. Secrets arrive via envFrom on the Secret that external-secrets
populates from Vault never as chart values, never as literals in a manifest.
*/}}
{{- define "svcforge.env" -}}
- name: SVCFORGE_POOL_MIN_SIZE
value: {{ .Values.pool.minSize | quote }}
- name: SVCFORGE_POOL_MAX_SIZE
value: {{ .Values.pool.maxSize | quote }}
- name: SVCFORGE_LOG_LEVEL
value: {{ .Values.log.level | quote }}
{{- if .Values.otel.enabled }}
- name: OTEL_EXPORTER_OTLP_ENDPOINT
value: {{ .Values.otel.endpoint | quote }}
- name: OTEL_EXPORTER_OTLP_PROTOCOL
value: grpc
{{- end }}
{{- end -}}