feat(flux-dispatch): add flux-dispatch service, CI/release wiring, and Kubernetes manifests - #3925
Draft
arealmaas wants to merge 29 commits into
Draft
feat(flux-dispatch): add flux-dispatch service, CI/release wiring, and Kubernetes manifests#3925arealmaas wants to merge 29 commits into
arealmaas wants to merge 29 commits into
Conversation
Load the service configuration from the environment with defaults for every optional value, failing with the offending variable name when a required one is missing or an optional one cannot be parsed. Co-Authored-By: Claude <noreply@anthropic.com>
Model the notification-controller webhook body and derive the values the service routes on: commit SHA from originRevision (last "/" segment, so branch names containing "/" still work), the artifact revision, and its sha256 digest for the dedup key. Co-Authored-By: Claude <noreply@anthropic.com>
Constant-time HMAC-SHA256 verification of the raw request body against the shared token, accepting the "sha256=" prefixed form Flux's generic-hmac provider sends as well as a bare hex digest. Co-Authored-By: Claude <noreply@anthropic.com>
Accept only the kustomize-controller v1 reconciliation reasons from RFC 0010, enforce the strict owner/repo form plus Altinn org prefix on dispatch_repo, and route reasons by dispatch_event suffix so an eventSeverity: info Alert never forwards a failure through a success event type. Co-Authored-By: Claude <noreply@anthropic.com>
Bounded, TTL'd in-memory tracker keyed by product/env/reason/digest/repo. New keys evict the oldest entry at capacity, a background sweep drops entries past the TTL, and the entry count is published to the dedup_entries gauge. Co-Authored-By: Claude <noreply@anthropic.com>
Sign an RS256 App JWT with the mounted PEM and exchange it for an installation access token, caching the token until five minutes before it expires so a busy service does not re-authenticate on every dispatch. Co-Authored-By: Claude <noreply@anthropic.com>
POST repository_dispatch to GitHub with the client_payload from RFC 0010, truncating the Flux message to 1024 runes. Failures are classified as retryable (5xx, timeout, transport, auth) or non-retryable (4xx, bad target repo) and carry the error_code metric label. The target path is built from a split owner/repo through url.JoinPath so a product-supplied value cannot steer the request elsewhere. Co-Authored-By: Claude <noreply@anthropic.com>
Define exactly the seven collectors from RFC 0010 on a dedicated registry so the metrics port exposes only the service's own signals. Behaviour is asserted through the server integration test. Co-Authored-By: Claude <noreply@anthropic.com>
Wire RFC 0010's request flow into one handler on POST /flux-events: 64 KB MaxBytesReader before any read, HMAC verify, parse, reason and dispatch_repo validation, reason routing, dedup, then the GitHub dispatch. Health endpoints and the hardened timeouts sit on 8080, the metrics registry on 9090, and every decision is logged as JSON with product, env, repo, reason and outcome. The integration test walks the RFC corner-case table end to end against a fake GitHub, including the metrics each path must move. Co-Authored-By: Claude <noreply@anthropic.com>
…auth errors
I1: repoRe permits "." inside a segment, so "Altinn/.." and "Altinn/." were
well-formed and passed the org prefix check. url.JoinPath *cleans* traversal
segments instead of rejecting them, so such a value silently rewrote the
outbound path while carrying a valid installation token. Reject all-dot
segments in validate.RepoAllowed and again in dispatch.dispatchURL; leading
and trailing dots (".github", "repo.") stay legal. The four new test rows
fail against the previous regex.
I2/M3: auth failures returned before the ErrRetryable branch, so
dispatch_errors_total never moved — a rotated App key would fail 100% of
dispatches with a flat-zero error-rate alert. Also increment it with
error_code="auth" (which makes ErrorCode's "auth" return value live), while
still skipping the duration histogram since no API call was made.
M1: malformed JSON now answers 200 with the existing outcome=invalid_payload
warning. Flux collapses all non-2xx into one error class, so the 400 bought no
visibility and added a third exception to the binding return-code contract.
M2: drop Parse's field-presence checks. An empty reason is an unrecognised
reason, which RFC step 3 answers 200 for; the shape check pushed RFC-200 cases
behind an error response.
Co-Authored-By: Claude <noreply@anthropic.com>
Dockerfile and .dockerignore/.trivyignore copy lakmus's multi-stage
golang:1.26.5-alpine -> gcr.io/distroless/static:nonroot pattern verbatim,
adjusted for flux-dispatch's actual tree (cmd/, internal/ only - no pkg/,
no test/).
flux-dispatch-lint-test.yml mirrors dis-vault-operator's lint-test shape
(separate lint/test jobs, golangci-lint-action v2.12.2 default config,
go mod tidy + make verify) rather than lakmus's literally, since lakmus's
cdk8s/Node manifest-generation steps depend on services/flux-dispatch/manifests,
which is Task 6's deliverable and doesn't exist yet. golangci-lint 2.12.2
verified clean against flux-dispatch under default rules (0 issues); the
repo's stricter operator-style .golangci.yml flags 32 pre-existing goconst
hits in test files, so no .golangci.yml was added here (matches lakmus,
which also ships none).
flux-dispatch-release.yml is lakmus-release.yml with only names/paths/tags
swapped (diff confirms), publishing:
- GHCR image ghcr.io/altinn/altinn-platform/flux-dispatch:v<version> (and
:latest from main) via reusable-image-scan-and-release-ghcr.yml, tag
trigger flux-dispatch-v*.
- Flux OCI kustomize artifact dis/kustomize/flux-dispatch (:latest from
main, :<version> from release tags) from services/flux-dispatch/config,
which Task 6 populates.
Registers services/flux-dispatch in release-please-config.json (mirrors the
lakmus entry: release-type simple, component flux-dispatch) and seeds
.release-please-manifest.json at 0.0.0, matching the seed used for every
other currently-active service that had zero prior releases (dis-apim-operator,
dis-console, dis-vault-operator).
Also adds services/flux-dispatch to release-please-post-config.json's
post-release-hooks (dispatch-pipelines -> flux-dispatch-release.yml). This
third file isn't in the task brief's file list, but release-please.yml's
trigger-post-release-workflows job reads exactly this file to
workflow_dispatch each service's release workflow after a release is cut -
every existing service has an entry. Without it the release PR would still
open and merge, but the image would never actually build/publish.
Makefile: removed the local `tidy` target, which duplicated Makefile.common's
identical target and produced a `make: overriding commands for target tidy`
warning; kept everything else Task 4 wired up (verify, docker-build via
Makefile.common, CONTAINER_TOOL convention).
Co-Authored-By: Claude <noreply@anthropic.com>
Bootstraps services/flux-dispatch/manifests/ from services/lakmus/manifests/ (cdk8s Go), following RFC 0010 SS"Kubernetes deployment" and SSNetworkPolicy. - Deployment: replicas pinned to 1 (in-memory dedup forbids horizontal scaling; documented in a source comment and a scaling-note annotation, since cdk8s's JSON-patch synth pipeline has no YAML comment support), ports 8080/9090, /healthz liveness, /readyz readiness, env vars matching internal/config/config.go exactly, volumes mounting the two Secrets, 50m/64Mi resource requests. - ClusterIP Service flux-dispatch on 8080. - Three NetworkPolicies copied verbatim from RFC 0010 SSNetworkPolicy (ingress 8080 from flux-system, ingress 9090 from monitoring, egress for GitHub API 443 + DNS). The other dis-* operators referenced by the brief have no egress convention of their own (unmodified kubebuilder ingress-only boilerplate), so the RFC block is authoritative as-is. - PodMonitor on azmonitoring.coreos.com/v1, matching lakmus. - SecretStore + two ExternalSecrets for the GitHub App key and HMAC token. The HMAC secret's data key is "token", matching what Flux's generic-hmac Provider requires. Secret provisioning mechanism is an explicitly flagged assumption (see task-6-report.md) mirroring gitops-manifests' otel-collector and dis-tls-cert SecretStore/ExternalSecret pattern — no dis-platform SecretStore exists yet anywhere, and this is the closest real precedent. Also vendors imports/k8s (generated cdk8s Go bindings for the k8s core API) copied from lakmus, with IntOrString/Quantity reimplemented as plain Go values instead of jsii-kernel proxies: the jsii kernel needs a locally-built k8s-0.0.0.tgz assembly that lakmus's own tooling generates and gitignores, which this environment cannot regenerate offline (confirmed lakmus itself does not build here for the same reason). go.mod gains the three cdk8s toolchain deps lakmus already carries (aws/constructs-go, aws/jsii-runtime-go, cdk8s-core-go); no other dependency or cmd/internal file changed. Co-Authored-By: Claude <noreply@anthropic.com>
…Secret naming The azure.workload.identity/use pod label triggered the workload-identity mutating webhook to inject an unused projected token volume and env vars: flux-dispatch never calls Azure, external-secrets exchanges the ServiceAccount token itself via the SecretStore's serviceAccountRef, and the ServiceAccount's azure.workload.identity/client-id annotation (which that exchange actually needs) is independent of this pod label. Removing it drops one line from the generated Deployment. newExternalSecret also took an independent id argument that duplicated the targetSecretName + "-external-secret" convention it was meant to describe, inviting the two to drift. Derive both the construct id and metadata.name from targetSecretName instead; rendered output is byte-identical.
Step 3 said parsing failure returns 400; the handler has returned 200 (outcomeInvalidPayload) since the malformed-JSON handling change, and the README's own return-code table and rationale already say so. Only this one line had drifted.
config/ is exactly what the release workflow packages into the Flux OCI artifact, but nothing regenerated it in CI, so a future manifests/main.go edit merged without re-running the generator would ship stale YAML with no signal. Mirrors services/lakmus's manifests + manifests-verify pattern (Makefile:24-38, lakmus-lint-test.yml:55-56). Named cdk8s-manifests / cdk8s-manifests-verify rather than lakmus's bare manifests / manifests-verify: unlike lakmus, this service's Makefile includes ../../Makefile.common, which already defines `manifests: controller-gen` for the kubebuilder operators. GNU Make unions prerequisites across same-named rules even when the recipe is overridden, so the bare name would silently pull in a controller-gen install on every run — confirmed with a standalone Makefile before picking the new name. Verified with a negative test: an edit to manifests/main.go's appName const makes cdk8s-manifests-verify fail with a diff and non-zero exit; reverting it makes the target pass again.
flux-dispatch's Makefile includes ../../Makefile.common for its shared Go tasks, but the path filters didn't list it, so a shared lint/vet/tidy or GOLANGCI_LINT_VERSION change would silently skip this service's CI. Every other service that includes Makefile.common (dis-apim, dis-console, dis-identity, dis-pgsql, dis-vault) already filters on it in both push and pull_request; flux-dispatch was the one gap.
No args meant golangci-lint ran over ./..., including the 111 generated files under imports/ (cdk8s's Go k8s API bindings). None carry a "Code generated ... DO NOT EDIT." header, so golangci-lint's generated-file exclusion doesn't skip them. Mirrors services/lakmus's scoping (lakmus-lint-test.yml:53), adapted to flux-dispatch's own package layout (no pkg/ or test/azfakes/ here). Pre-emptive alignment with the established precedent, not a confirmed-red build.
Access control moves entirely to the NetworkPolicy that already restricts webhook ingress to flux-system: the product namespace never sent the request, so a shared HMAC token proved cluster-insider status against an endpoint already restricted to cluster insiders, without providing any per-product authorization. See .superpowers/sdd/FLUX-DISPATCH-IMPLEMENTATION-PLAN/DECISION-drop-hmac.md. - Delete internal/hmacsig and its test. - internal/config: drop HMAC_TOKEN_PATH and its required-var check. - internal/server: drop the signature-verification step and the 401 path. http.MaxBytesReader stays exactly where it was, first, before any body read. - cmd/main.go: drop the HMAC token file load and its newline trim. - internal/server/server_test.go: drop the bad/missing-signature 401 cases; every other corner case is unchanged. - README.md: drop HMAC from the request flow and return-code table; note that access control is now the NetworkPolicy.
Companion to the Go service change dropping HMAC verification. The GitHub App private key ExternalSecret, volume, and mount are unaffected and remain required. - manifests/main.go: remove the HMAC ExternalSecret, its volume, its volumeMount, the KV_SECRET_NAME_HMAC_TOKEN placeholder, and the HMAC_TOKEN_PATH env var. - config/: regenerated via `go run ./manifests`.
…mment newDeployment's doc comment still said "the four required variables" after the prior commit dropped HMAC_TOKEN_PATH, leaving three.
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| env := e.Meta(event.MetaEnv) | ||
| repo := e.Meta(event.MetaDispatchRepo) | ||
| log := s.log.With( | ||
| "product", product, |
|
|
||
| // Step 5: the target must be a well-formed repo in the Altinn org. | ||
| if err := validate.RepoAllowed(repo); err != nil { | ||
| log.Warn("rejecting dispatch target", "outcome", outcomeInvalidRepo, "error", err) |
| if eventType == "" { | ||
| eventType = s.opts.DefaultDispatchEvent | ||
| } | ||
| log = log.With("event_type", eventType) |
| s.opts.Metrics.Dispatches.WithLabelValues(repo, eventType, e.Reason).Inc() | ||
| log.Info("dispatched repository_dispatch", | ||
| "outcome", outcomeDispatched, | ||
| "commit_sha", payload.CommitSHA, |
| log.Info("dispatched repository_dispatch", | ||
| "outcome", outcomeDispatched, | ||
| "commit_sha", payload.CommitSHA, | ||
| "revision", payload.Revision, |
DRY_RUN (default false) makes the three GitHub App variables optional and, when true, skips the startup check that the private key file exists and is readable. When DRY_RUN is false that check now runs at Load time so a bad mount fails the pod at startup instead of on the first webhook delivery. See DECISION-dry-run.md "Config".
Gives the server's upcoming DRY_RUN logging path (which builds a Payload but never calls Send) access to the same message-truncation Send applies, so a dry-run log line matches what a real dispatch would have carried.
The handler runs every step exactly as normal — body limit, parse, KnownReason, dispatch_repo, RepoAllowed, -failed routing, dedup — and, when Options.DryRun is true, replaces the GitHub App auth and repository_dispatch call with a structured info log and a dedicated flux_dispatch_dryrun_dispatches_total counter. The dedup key is still recorded so dedup behaviour stays observable, and flux_dispatch_dispatches_total does not move. cmd/main.go now only reads the GitHub App private key file when DryRun is false, so the binary starts cleanly with no key mounted — config.Load already guarantees the file exists and is readable whenever it will actually be needed. See DECISION-dry-run.md "Behaviour" and "Metrics".
Deployment now sources DRY_RUN from a ${DRY_RUN} postBuild placeholder,
and the github-app-key Secret volume is marked optional so the pod can
start before the ExternalSecret has materialized it — e.g. during the
DRY_RUN rollout, before the GitHub App and its Key Vault entry exist.
Production still fails fast via config.Load's startup readability
check. SecretStore and ExternalSecret are unchanged. config/ is
regenerated via `make cdk8s-manifests`.
See DECISION-dry-run.md "Manifests".
Adds the DRY_RUN row and conditional-required notes to the configuration table, the new flux_dispatch_dryrun_dispatches_total metric, and a one-line description of what the mode is for.
| s.opts.Metrics.DryRunDispatches.WithLabelValues(repo, eventType, e.Reason).Inc() | ||
| log.Info("dry run: would have dispatched repository_dispatch", | ||
| "outcome", outcomeDryRun, | ||
| "commit_sha", payload.CommitSHA, |
| log.Info("dry run: would have dispatched repository_dispatch", | ||
| "outcome", outcomeDryRun, | ||
| "commit_sha", payload.CommitSHA, | ||
| "revision", payload.Revision, |
| "outcome", outcomeDryRun, | ||
| "commit_sha", payload.CommitSHA, | ||
| "revision", payload.Revision, | ||
| "kustomization_name", payload.KustomizationName, |
Collaborator
|
I think it's a good idea but the PR is massive and not really sure if we need everything that claude generated for such a small service. Need to re-read the RFC first and then I can come back to this PR. |
…entralize DRY_RUN rationale in README
17 comments cited DECISION-dry-run.md and task-6-report.md, both gitignored
scratch files in a different repository's working directory that were never
committed anywhere. Strip the citations and keep code comments terse and
local; the DRY_RUN design rationale now lives once in README.md ("DRY_RUN
mode" and "Secret management" sections) instead of being duplicated across
seven files.
…nd label cardinality Six defects found in review, all in paths that fail silently. Retryable/permanent was inverted at both ends. GitHub signals rate limits with 403 as well as 429, and both were classified permanent, so a throttled dispatch answered 200 and Flux never redelivered it -- the deploy signal was lost with only a warning log. In the other direction every token-exchange failure was classified retryable, so a malformed key or a wrong App ID answered 502 forever and Flux retried until it gave up and dropped the event anyway. internal/githubapi is now the single source of truth for which responses are transient, shared by the token exchange and the dispatch, and githubauth.TokenError reports whether a retry could ever help. A revoked installation token was never invalidated. GitHub revokes tokens the moment the App key rotates, but the cache only refreshed on wall-clock expiry, so every dispatch presented a dead token for up to 55 minutes -- and, via the misclassification above, answered 200 each time while github_auth_errors_total stayed flat. A 401 on the dispatch now drops the cached token and retries once with a fresh one. An event with no artifact digest collapsed the dedup key. Without the revision metadata the key degraded to product/env/reason//repo, identical for every later event, so the first delivery suppressed the rest for the full 24h TTL. Such events are now dispatched without deduplication and logged with outcome=no_digest: they have no identity to deduplicate on. Deduplication was check-then-act. Seen() and Record() sat on either side of the outbound call, leaving that whole window open for a concurrent duplicate. A test that holds the dispatch open shows the old code dispatching 16 times for 16 concurrent deliveries of one event. Tracker.Claim now reserves the key atomically; the handler confirms it on success and releases it otherwise. Metric labels were unbounded. events_received_total counted the raw reason before the known-reason check, and dispatch_event was never validated at all, so one malformed Alert could pin arbitrary series for the process lifetime. Reasons are bucketed through validate.ReasonLabel, and dispatch_event and dispatch_repo are now length- and charset-bounded. The outbound budget could exceed the write timeout. A token exchange plus a dispatch is 2x githubTimeout = 30s, exactly WriteTimeout, so the response was dropped rather than answered. The handler now bounds both under one 25s budget, and concurrent callers share a single in-flight token exchange instead of queueing their own behind the mutex. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
7 of the 9 services carry an AGENTS.md and 6 carry a .golangci.yml; flux-dispatch had neither. Without a config the lint step in CI fell back to golangci-lint's bare default set, silently dropping revive, unparam, prealloc, gocyclo, dupl, goconst, lll, misspell and the formatter checks that the sibling services enforce. The config matches services/dis-console/.golangci.yml, plus exclusions for the cdk8s-generated imports/k8s bindings and for manifests/, where the strings goconst flags are Kubernetes API field names and hoisting them out would hide the shape of the manifest being generated. AGENTS.md records the invariants that the compiler and the linter cannot check: the return-code contract, dedup as a claim rather than a lookup, why an empty digest disables dedup, that metric labels come from the request body, and that replicas: 1 is load-bearing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ation The 401-refresh path and the permanent-versus-transient auth split were only exercised end-to-end through the handler. Cover them at the dispatch layer as well: that a rejected token is invalidated and the retry carries a fresh one, that a second rejection is not retried again, and that a rejected credential wraps ErrNonRetryable while an outage wraps ErrRetryable. Raises internal/dispatch coverage from 78.8% to 87.1%. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| repo := e.Meta(event.MetaDispatchRepo) | ||
| log := s.log.With( | ||
| "product", product, | ||
| "env", env, |
| log := s.log.With( | ||
| "product", product, | ||
| "env", env, | ||
| "repo", repo, |
| "product", product, | ||
| "env", env, | ||
| "repo", repo, | ||
| "reason", e.Reason, |
| "env", env, | ||
| "repo", repo, | ||
| "reason", e.Reason, | ||
| "kustomization", e.InvolvedObject.Name, |
| // The dispatch_event reaches GitHub and three Prometheus labels, so it is | ||
| // bounded here the same way the dispatch_repo is. | ||
| if err := validate.DispatchEvent(eventType); err != nil { | ||
| log.Warn("rejecting dispatch_event", "outcome", outcomeInvalidEvent, "error", err) |
| // delivery of the same event also sees "unseen" and dispatches too. | ||
| if !s.opts.Tracker.Claim(dedupKey) { | ||
| s.opts.Metrics.DedupHits.WithLabelValues(e.Reason).Inc() | ||
| log.Info("skipping duplicate event", "outcome", outcomeDuplicate, "revision", e.Revision()) |
| "commit_sha", payload.CommitSHA, | ||
| "revision", payload.Revision, | ||
| "kustomization_name", payload.KustomizationName, | ||
| "message", dispatch.TruncateMessage(payload.Message)) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the
flux-dispatchplatform service specified by RFC 0010 — a webhook receiver that turns Flux reconciliation events into GitHubrepository_dispatchevents, so product teams get a "deploy finished" (or "deploy failed") signal without running a receiver themselves.Covers Tasks 4–6 of the implementation plan. Draft until the RFC lands — see Dependencies.
What's here
services/flux-dispatch/— Go service, stdlib-first (golang-jwt/jwt/v5+prometheus/client_golangonly, no GitHub SDK). Built TDD, one commit per package: config, event parsing, validation, dedup tracker, GitHub App auth, dispatcher, metrics, server wiring.lakmuslayout. Includes release-please registration inrelease-please-config.json,.release-please-manifest.json, andrelease-please-post-config.json.services/flux-dispatch/manifests/— cdk8s (Go) manifests: single-replica Deployment, ClusterIP Service, three NetworkPolicies, PodMonitor, and the ExternalSecret for the GitHub App private key.Design notes worth reviewing
replicas: 1is load-bearing. Dedup is in-memory; a second replica has a disjoint dedup map and would produce duplicate workflow runs — the exact thing the service exists to prevent. Commented in the manifest.flux-system. Rationale and the accepted trust model are recorded in RFC 0010 §NetworkPolicy.{product}/{env}/{reason}/{sha256-digest}/{dispatch_repo}, recorded only after a successful dispatch so a transient failure is never suppressed on retry.Verification
gofmt -l .clean,go vet ./...clean,go test -race ./...green across 7 packagesmake cdk8s-manifests-verifypasses, and fails on genuine drift (negative-tested)internal/config/config.go; Secret mount paths verified to land where the config reads themDependencies
README.mdandmanifests/main.gociterfcs/0010-flux-reconcile-webhooks.md, which does not exist onmainyet.dis-way/gitops-manifests.dis-flux-dispatchGitHub App (App ID, installation ID, private key in Key Vault) plus workload identity — not yet provisioned.🤖 Generated with Claude Code