From 6483f604d10bfdad2ad43ff8129bcc2274930dc4 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Sun, 21 Jun 2026 00:26:26 -0400 Subject: [PATCH] feat: reserve telemetry webhook and job-summary fields Signed-off-by: Joshua Temple --- docs/public/manifest.schema.json | 13 +- docs/src/content/docs/versioning.md | 9 + e2e/harness/scenario.go | 6 + e2e/scenarios/25-telemetry-sink-reserved.yaml | 45 ++++ internal/config/schema_v1.go | 30 +++ internal/config/validate_telemetry_test.go | 192 ++++++++++++++++++ internal/config/validate_v1.go | 44 ++++ internal/generate/telemetry_reserved_test.go | 112 ++++++++++ internal/schema/manifest.schema.json | 13 +- schema/manifest.schema.json | 13 +- 10 files changed, 474 insertions(+), 3 deletions(-) create mode 100644 e2e/scenarios/25-telemetry-sink-reserved.yaml create mode 100644 internal/config/validate_telemetry_test.go create mode 100644 internal/generate/telemetry_reserved_test.go diff --git a/docs/public/manifest.schema.json b/docs/public/manifest.schema.json index aedc578c..66417a55 100644 --- a/docs/public/manifest.schema.json +++ b/docs/public/manifest.schema.json @@ -631,7 +631,18 @@ "description": "Vendor-neutral metrics seam.", "properties": { "enabled": { "type": "boolean" }, - "adapter": { "type": "string", "description": "Metrics adapter (for example none, datadog)." } + "adapter": { "type": "string", "description": "Metrics adapter (for example none, datadog)." }, + "webhook": { "$ref": "#/definitions/telemetryWebhook" }, + "job_summary": { "type": "boolean", "description": "Toggle the run-UI summary table. Reserved." } + } + }, + "telemetryWebhook": { + "type": "object", + "additionalProperties": false, + "description": "Generic vendor-neutral JSON-POST telemetry sink. Reserved.", + "properties": { + "url": { "type": "string", "description": "Destination the run posts telemetry to. Reserved." }, + "secret_name": { "type": "string", "description": "Name of a GitHub Actions secret holding the auth token (a reference, never an inline token). Reserved." } } }, "environmentConfig": { diff --git a/docs/src/content/docs/versioning.md b/docs/src/content/docs/versioning.md index c6321706..79e49186 100644 --- a/docs/src/content/docs/versioning.md +++ b/docs/src/content/docs/versioning.md @@ -105,6 +105,15 @@ A matching per-env deploy state slot, `target_sha`, reserves room to record the `branch` and `track_sha` are meaningful only when `mode` is `gitops`. These fields parse and pass structural validation today, but carry no generator behavior. A manifest declaring them produces byte-identical generated workflows, so the reserved shape is safe to adopt now. Attaching behavior to these fields later is additive and does not bump `schema_version`. +## Reserved shape: telemetry sink + +The manifest reserves a vendor-neutral telemetry seam under `config.telemetry`. The seam carries `enabled` and an `adapter` value (for example `none` or `datadog`); the vendor stays a value behind the adapter, so no vendor client is baked into cascade. The reserved shape adds two enriched fields: + +- `webhook`, a generic JSON-POST sink with a `url` (the destination the run posts telemetry to) and a `secret_name` (the name of a GitHub Actions secret holding the auth token). `secret_name` is a reference to a secret, never an inline token value. +- `job_summary`, a boolean that toggles the run-UI summary table. It is omitted when unset, so an unset value stays distinct from an explicit `false`; default-on behavior arrives in a later release. + +These fields parse and pass structural validation today, but carry no generator or emit behavior. A manifest declaring them produces byte-identical generated workflows, so the reserved shape is safe to adopt now. Attaching behavior to these fields later is additive and does not bump `schema_version`. + ## Migrations Each `schema_version` bump is recorded with a `Migration` section in [CHANGELOG.md](https://github.com/stablekernel/cascade/blob/main/CHANGELOG.md) describing exactly what changed and the steps to update a manifest from the previous version. There are no migrations yet: the current schema version is the first. diff --git a/e2e/harness/scenario.go b/e2e/harness/scenario.go index 8cc2a181..a0531d13 100644 --- a/e2e/harness/scenario.go +++ b/e2e/harness/scenario.go @@ -66,6 +66,12 @@ type Config struct { PRPreview map[string]any `yaml:"pr_preview,omitempty"` Notify map[string]any `yaml:"notify,omitempty"` External []map[string]any `yaml:"external,omitempty"` + // Telemetry carries the reserved vendor-neutral telemetry block (enabled, + // adapter, webhook, job_summary) through to the generated manifest untouched. + // A generic map keeps the harness decoupled from the generator's + // TelemetryConfig shape, so a scenario can declare any reserved telemetry + // field without the harness needing to know its structure. + Telemetry map[string]any `yaml:"telemetry,omitempty"` } // EnvEnvironmentConfig mirrors internal/config.EnvironmentConfig's gha_environment diff --git a/e2e/scenarios/25-telemetry-sink-reserved.yaml b/e2e/scenarios/25-telemetry-sink-reserved.yaml new file mode 100644 index 00000000..3b258ef0 --- /dev/null +++ b/e2e/scenarios/25-telemetry-sink-reserved.yaml @@ -0,0 +1,45 @@ +name: "Telemetry Sink Reserved Shape" +description: | + Exercises the reserved vendor-neutral telemetry seam (config.telemetry) with + the enriched reserved fields webhook (url, secret_name) and job_summary. This + block is reserved and shape-only today: it parses and passes structural + validation, but carries no generator or emit behavior. secret_name is a + reference to a GitHub Actions secret, never an inline token. The scenario + declares the telemetry shape, generates the workflows, then regenerates and + proves the output is byte-identical with no drift. + +config: + trunk_branch: main + environments: [dev, prod] + builds: + - name: app + workflow: build.yaml + triggers: ["src/**"] + deploys: + - name: app + workflow: deploy.yaml + triggers: ["src/**"] + telemetry: + enabled: true + adapter: none + webhook: + url: https://metrics.example.com/ingest + secret_name: TELEMETRY_TOKEN + job_summary: true + +steps: + - name: "Seed a minimal source tree" + action: commit + commit: + message: "seed source" + files: + src/main.go: | + package main + + func main() {} + + - name: "Regenerate and confirm no drift" + action: verify + verify: + regenerate: true + expect_exit: 0 diff --git a/internal/config/schema_v1.go b/internal/config/schema_v1.go index 3b337ada..c41252ab 100644 --- a/internal/config/schema_v1.go +++ b/internal/config/schema_v1.go @@ -273,6 +273,36 @@ const ( type TelemetryConfig struct { Enabled bool `yaml:"enabled,omitempty" json:"enabled,omitempty"` Adapter string `yaml:"adapter,omitempty" json:"adapter,omitempty"` // none | datadog | + // Webhook is the reserved generic vendor-neutral JSON-POST sink. RESERVED - + // not yet wired to generation/emit. Carries the destination shape behind the + // adapter seam so no vendor client is baked into core. + Webhook *TelemetryWebhook `yaml:"webhook,omitempty" json:"webhook,omitempty"` + // JobSummary toggles the run-UI summary table. A pointer so unset differs + // from an explicit false (mirroring the FailFast "unset != false" precedent). + // RESERVED - not yet wired to generation/emit; default-on behavior lands in a + // later minor. + JobSummary *bool `yaml:"job_summary,omitempty" json:"job_summary,omitempty"` +} + +// Telemetry adapter constants. The vendor stays an Adapter value behind the +// seam; no vendor client is wired into core. These name the known adapter +// values for documentation and future use and are not enforced as an enum (an +// arbitrary adapter string parses today and must keep parsing). +const ( + TelemetryAdapterNone = "none" + TelemetryAdapterDatadog = "datadog" +) + +// TelemetryWebhook is the reserved generic JSON-POST telemetry sink. RESERVED - +// not yet wired to generation/emit. +type TelemetryWebhook struct { + // URL is the destination the run posts telemetry to. + URL string `yaml:"url,omitempty" json:"url,omitempty"` + // SecretName is the name of a GitHub Actions secret holding the auth token. + // This is a secret REFERENCE, never an inline token value: the resolver + // reads the named secret at run time rather than carrying a raw credential + // in the manifest. RESERVED - not yet wired to generation/emit. + SecretName string `yaml:"secret_name,omitempty" json:"secret_name,omitempty"` } // EnvironmentConfig is the reserved per-environment settings block, keyed by env diff --git a/internal/config/validate_telemetry_test.go b/internal/config/validate_telemetry_test.go new file mode 100644 index 00000000..30d6247b --- /dev/null +++ b/internal/config/validate_telemetry_test.go @@ -0,0 +1,192 @@ +package config + +import "testing" + +// TestValidateTelemetryReservedFields exercises the newly reserved telemetry +// fields (webhook.url, webhook.secret_name). Validation is intentionally lenient +// and applies only when webhook is present; adapter is never enum-checked so an +// arbitrary adapter string keeps parsing and validating. +func TestValidateTelemetryReservedFields(t *testing.T) { + t.Parallel() + + boolPtr := func(b bool) *bool { return &b } + + tests := []struct { + name string + telemetry *TelemetryConfig + wantErr bool + errContains string + }{ + { + name: "nil telemetry is valid", + telemetry: nil, + wantErr: false, + }, + { + name: "enabled with no webhook is valid", + telemetry: &TelemetryConfig{Enabled: true, Adapter: TelemetryAdapterNone}, + wantErr: false, + }, + { + name: "arbitrary adapter still validates (no enum enforcement)", + telemetry: &TelemetryConfig{ + Enabled: true, + Adapter: "some-future-vendor", + }, + wantErr: false, + }, + { + name: "full webhook with https url and safe secret name is valid", + telemetry: &TelemetryConfig{ + Enabled: true, + Adapter: TelemetryAdapterNone, + Webhook: &TelemetryWebhook{ + URL: "https://metrics.example.com/ingest", + SecretName: "TELEMETRY_TOKEN", + }, + JobSummary: boolPtr(true), + }, + wantErr: false, + }, + { + name: "http url is accepted", + telemetry: &TelemetryConfig{ + Webhook: &TelemetryWebhook{URL: "http://localhost:9000/ingest"}, + }, + wantErr: false, + }, + { + name: "non-http url is rejected", + telemetry: &TelemetryConfig{ + Webhook: &TelemetryWebhook{URL: "ftp://metrics.example.com"}, + }, + wantErr: true, + errContains: "telemetry.webhook.url must be an http(s) URL", + }, + { + name: "secret name with dollar interpolation is rejected", + telemetry: &TelemetryConfig{ + Webhook: &TelemetryWebhook{SecretName: "${{ secrets.X }}"}, + }, + wantErr: true, + errContains: "telemetry.webhook.secret_name must be a valid GitHub Actions secret name", + }, + { + name: "secret name starting with a digit is rejected", + telemetry: &TelemetryConfig{ + Webhook: &TelemetryWebhook{SecretName: "1TOKEN"}, + }, + wantErr: true, + errContains: "secret_name must be a valid GitHub Actions secret name", + }, + { + name: "empty webhook fields are valid (shape only)", + telemetry: &TelemetryConfig{ + Webhook: &TelemetryWebhook{}, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + errs := validateTelemetry(tt.telemetry) + if tt.wantErr { + if len(errs) == 0 { + t.Fatalf("expected an error, got none") + } + if !hasErrContaining(errs, tt.errContains) { + t.Fatalf("expected error containing %q, got %v", tt.errContains, errs) + } + return + } + if len(errs) != 0 { + t.Fatalf("expected no errors, got %v", errs) + } + }) + } +} + +// TestParseTelemetryReservedFields asserts a manifest carrying the enriched +// telemetry shape (webhook, job_summary) parses into the typed fields, validates +// at CurrentSchemaVersion, and does not bump schema_version. +func TestParseTelemetryReservedFields(t *testing.T) { + t.Parallel() + + cfg := parseInline(t, ` +environments: [dev, prod] +deploys: + - name: app + workflow: .github/workflows/deploy.yaml +telemetry: + enabled: true + adapter: none + webhook: + url: https://metrics.example.com/ingest + secret_name: TELEMETRY_TOKEN + job_summary: true +`) + if cfg.Telemetry == nil { + t.Fatalf("telemetry block did not parse") + } + if !cfg.Telemetry.Enabled || cfg.Telemetry.Adapter != TelemetryAdapterNone { + t.Fatalf("telemetry enabled/adapter: %#v", cfg.Telemetry) + } + if cfg.Telemetry.Webhook == nil { + t.Fatalf("telemetry.webhook did not parse") + } + if cfg.Telemetry.Webhook.URL != "https://metrics.example.com/ingest" { + t.Fatalf("telemetry.webhook.url: %q", cfg.Telemetry.Webhook.URL) + } + if cfg.Telemetry.Webhook.SecretName != "TELEMETRY_TOKEN" { + t.Fatalf("telemetry.webhook.secret_name: %q", cfg.Telemetry.Webhook.SecretName) + } + if cfg.Telemetry.JobSummary == nil || !*cfg.Telemetry.JobSummary { + t.Fatalf("telemetry.job_summary: %#v", cfg.Telemetry.JobSummary) + } + if errs := Validate(cfg); len(errs) != 0 { + t.Fatalf("expected no errors, got %v", errs) + } + if got := cfg.GetSchemaVersion(); got != CurrentSchemaVersion { + t.Fatalf("schema_version = %d, want %d (reserved shape must not bump)", got, CurrentSchemaVersion) + } + if CurrentSchemaVersion != 1 { + t.Fatalf("CurrentSchemaVersion = %d, want 1 (reserved shape must not bump)", CurrentSchemaVersion) + } +} + +// TestParseTelemetryJobSummaryUnsetVsFalse confirms the *bool pointer +// distinguishes an unset job_summary from an explicit false. +func TestParseTelemetryJobSummaryUnsetVsFalse(t *testing.T) { + t.Parallel() + + unset := parseInline(t, ` +environments: [dev] +deploys: + - name: app + workflow: .github/workflows/deploy.yaml +telemetry: + enabled: true +`) + if unset.Telemetry == nil || unset.Telemetry.JobSummary != nil { + t.Fatalf("unset job_summary should be nil, got %#v", unset.Telemetry) + } + + explicitFalse := parseInline(t, ` +environments: [dev] +deploys: + - name: app + workflow: .github/workflows/deploy.yaml +telemetry: + enabled: true + job_summary: false +`) + if explicitFalse.Telemetry == nil || explicitFalse.Telemetry.JobSummary == nil { + t.Fatalf("explicit job_summary: false should parse to a non-nil pointer, got %#v", explicitFalse.Telemetry) + } + if *explicitFalse.Telemetry.JobSummary { + t.Fatalf("explicit job_summary: false should be false") + } +} diff --git a/internal/config/validate_v1.go b/internal/config/validate_v1.go index efb7a119..d6cf8904 100644 --- a/internal/config/validate_v1.go +++ b/internal/config/validate_v1.go @@ -310,9 +310,53 @@ func validateConfigLevel(cfg *TrunkConfig) []string { } } + errs = append(errs, validateTelemetry(cfg.Telemetry)...) + return errs } +// validateTelemetry checks only the newly reserved telemetry.webhook fields. +// adapter is left unchecked on purpose: an arbitrary adapter string parses and +// validates today, and the seam must stay additive, so no enum is enforced. The +// checks here are lenient and apply only when webhook is present, so they never +// reject a manifest that is valid without these new fields. secret_name is a +// reference to a GitHub Actions secret, never an inline token, so it is checked +// for a safe secret-name shape rather than treated as a credential. +func validateTelemetry(t *TelemetryConfig) []string { + if t == nil || t.Webhook == nil { + return nil + } + var errs []string + w := t.Webhook + if w.URL != "" { + if !strings.HasPrefix(w.URL, "https://") && !strings.HasPrefix(w.URL, "http://") { + errs = append(errs, "telemetry.webhook.url must be an http(s) URL") + } + } + if w.SecretName != "" && !safeSecretName(w.SecretName) { + errs = append(errs, "telemetry.webhook.secret_name must be a valid GitHub Actions secret name (letters, digits, underscores; not starting with a digit)") + } + return errs +} + +// safeSecretName reports whether name is a syntactically valid GitHub Actions +// secret name: ASCII letters, digits, and underscores only, and not starting +// with a digit. This guards a reference, not a credential value. +func safeSecretName(name string) bool { + for i, r := range name { + isLetter := (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') + isDigit := r >= '0' && r <= '9' + isUnderscore := r == '_' + if i == 0 && isDigit { + return false + } + if !isLetter && !isDigit && !isUnderscore { + return false + } + } + return true +} + // validateComponents validates the reserved top-level components map (#176). // Rules frozen at v1: component names must be job-ID-safe (so a future // generator can key job IDs on the name without breakage), and any configured diff --git a/internal/generate/telemetry_reserved_test.go b/internal/generate/telemetry_reserved_test.go new file mode 100644 index 00000000..b3cde684 --- /dev/null +++ b/internal/generate/telemetry_reserved_test.go @@ -0,0 +1,112 @@ +package generate + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stablekernel/cascade/internal/config" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +// writeTelemetryPlanManifest writes the determinism manifest, optionally enriched +// with the reserved telemetry block (webhook + job_summary), into a temp repo and +// returns the repo root. It reuses the determinism workflow stubs so the plan set +// is substantial and order-sensitive emission paths are exercised. +func writeTelemetryPlanManifest(t *testing.T, telemetry *config.TelemetryConfig) string { + t.Helper() + dir := writeDeterminismWorkflows(t) + + cfg := determinismConfig() + cfg.Telemetry = telemetry + + manifest := map[string]any{ + config.DefaultManifestKey: config.CICDFile{Config: cfg}, + } + body, err := yaml.Marshal(manifest) + require.NoError(t, err) + + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".github"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ".github", "manifest.yaml"), body, 0o644)) + return dir +} + +// telemetryPlan runs Plan rooted at dir and returns the emitted set keyed by the +// path relative to dir. Plan mixes relative (workflow) and absolute (composite +// action under baseDir) paths, so paths are normalized against the resolved dir +// to make two runs in different temp dirs comparable. +func telemetryPlan(t *testing.T, dir string) map[string]string { + t.Helper() + chdir(t, dir) + + resolved, err := filepath.EvalSymlinks(dir) + require.NoError(t, err) + + planned, err := Plan(PlanOptions{ + ConfigPath: ".github/manifest.yaml", + ManifestKey: config.DefaultManifestKey, + ActionFolder: "manage-release", + OutputPath: ".github/workflows/orchestrate.yaml", + PromoteOutputPath: ".github/workflows/promote.yaml", + }) + require.NoError(t, err) + require.NotEmpty(t, planned) + + out := make(map[string]string, len(planned)) + for _, p := range planned { + abs := p.Path + if !filepath.IsAbs(abs) { + abs = filepath.Join(resolved, abs) + } + rel, rerr := filepath.Rel(resolved, abs) + require.NoError(t, rerr) + out[rel] = p.Content + } + return out +} + +// TestTelemetryReservedFieldsAreByteIdentical asserts that populating the reserved +// telemetry fields (webhook.url, webhook.secret_name, job_summary) produces a +// byte-identical generated plan set to a manifest that omits the telemetry block. +// These fields are reserved shape but not yet wired to generation/emit. The test +// drives the full Plan path (orchestrate, promote, composite action, and the rest) +// because telemetry is a config-level field that could affect any emitted file. +func TestTelemetryReservedFieldsAreByteIdentical(t *testing.T) { + jobSummary := true + + // Case A: reserved telemetry fields populated. + withTelemetry := writeTelemetryPlanManifest(t, &config.TelemetryConfig{ + Enabled: true, + Adapter: config.TelemetryAdapterNone, + Webhook: &config.TelemetryWebhook{ + URL: "https://metrics.example.com/ingest", + SecretName: "TELEMETRY_TOKEN", + }, + JobSummary: &jobSummary, + }) + plannedA := telemetryPlan(t, withTelemetry) + + // Case B: no telemetry block at all. + withoutTelemetry := writeTelemetryPlanManifest(t, nil) + plannedB := telemetryPlan(t, withoutTelemetry) + + // Guard against a vacuous comparison: the plan must be substantial for the + // equality to be meaningful. Sum every emitted file's length and require the + // total to clear a real-pipeline floor. + var totalA int + for _, content := range plannedA { + totalA += len(content) + } + require.NotEmpty(t, plannedA, "plan must emit at least one file") + require.Greater(t, totalA, 1024, "generated plan should be substantial") + + require.Equal(t, len(plannedB), len(plannedA), + "reserved telemetry fields changed the set of emitted files") + for path, want := range plannedB { + got, ok := plannedA[path] + require.Truef(t, ok, "telemetry plan omitted %s that the empty plan emitted", path) + require.Equalf(t, want, got, + "reserved telemetry fields must not affect generated output for %s", path) + } +} diff --git a/internal/schema/manifest.schema.json b/internal/schema/manifest.schema.json index aedc578c..66417a55 100644 --- a/internal/schema/manifest.schema.json +++ b/internal/schema/manifest.schema.json @@ -631,7 +631,18 @@ "description": "Vendor-neutral metrics seam.", "properties": { "enabled": { "type": "boolean" }, - "adapter": { "type": "string", "description": "Metrics adapter (for example none, datadog)." } + "adapter": { "type": "string", "description": "Metrics adapter (for example none, datadog)." }, + "webhook": { "$ref": "#/definitions/telemetryWebhook" }, + "job_summary": { "type": "boolean", "description": "Toggle the run-UI summary table. Reserved." } + } + }, + "telemetryWebhook": { + "type": "object", + "additionalProperties": false, + "description": "Generic vendor-neutral JSON-POST telemetry sink. Reserved.", + "properties": { + "url": { "type": "string", "description": "Destination the run posts telemetry to. Reserved." }, + "secret_name": { "type": "string", "description": "Name of a GitHub Actions secret holding the auth token (a reference, never an inline token). Reserved." } } }, "environmentConfig": { diff --git a/schema/manifest.schema.json b/schema/manifest.schema.json index aedc578c..66417a55 100644 --- a/schema/manifest.schema.json +++ b/schema/manifest.schema.json @@ -631,7 +631,18 @@ "description": "Vendor-neutral metrics seam.", "properties": { "enabled": { "type": "boolean" }, - "adapter": { "type": "string", "description": "Metrics adapter (for example none, datadog)." } + "adapter": { "type": "string", "description": "Metrics adapter (for example none, datadog)." }, + "webhook": { "$ref": "#/definitions/telemetryWebhook" }, + "job_summary": { "type": "boolean", "description": "Toggle the run-UI summary table. Reserved." } + } + }, + "telemetryWebhook": { + "type": "object", + "additionalProperties": false, + "description": "Generic vendor-neutral JSON-POST telemetry sink. Reserved.", + "properties": { + "url": { "type": "string", "description": "Destination the run posts telemetry to. Reserved." }, + "secret_name": { "type": "string", "description": "Name of a GitHub Actions secret holding the auth token (a reference, never an inline token). Reserved." } } }, "environmentConfig": {