diff --git a/README.md b/README.md index 582d0ca..c90d70e 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,12 @@ PATs are not supported. Details: [docs/install/github-app.md](docs/install/githu Env vars and Helm mapping: [docs/configuration/](docs/configuration/). +GitHub prominently exposes only `environment_url` and status `log_url`. Point +`config.logURLTemplate` at Grafana Loki Explore, a Grafana dashboard, or another +Flux/Kubernetes dashboard; enable `config.logURLTemplateEscape` for safe +placeholder substitution. Copy-paste presets and Kustomization, HelmRelease, +and monorepo examples are in [Log URL placeholders](docs/configuration/environment.md#log-url-placeholders). + PVC / SQLite at `/data/cache.db` deduplicates `(owner, repo, environment, commitSHA, deploymentName)` across restarts - see [docs/install/persistence.md](docs/install/persistence.md). diff --git a/charts/github-deployment-bridge/README.md b/charts/github-deployment-bridge/README.md index dcd26d3..2cec9f8 100644 --- a/charts/github-deployment-bridge/README.md +++ b/charts/github-deployment-bridge/README.md @@ -50,6 +50,7 @@ helm upgrade --install github-deployment-bridge \ | `config.environmentURL` | `""` | Optional URL attached to deployment statuses | | `config.description` | `""` | Optional default deployment description (empty → `Deployed by FluxCD`) | | `config.logURLTemplate` | `""` | Optional log URL template (`{sha}`, `{namespace}`, `{name}`, `{service}`, `{environment}`, `{cluster}`) | +| `config.logURLTemplateEscape` | `false` | Percent-encode substituted log URL values | | `github.existingSecret` | `""` | Secret with `app-id`, `installation-id`, `private-key` | | `persistence.enabled` | `true` | PVC for SQLite deduplication cache (keep on in production) | | `networkPolicy.enabled` | `false` | Opt-in NetworkPolicy | diff --git a/charts/github-deployment-bridge/templates/deployment.yaml b/charts/github-deployment-bridge/templates/deployment.yaml index 6a1b6e0..f961a6e 100644 --- a/charts/github-deployment-bridge/templates/deployment.yaml +++ b/charts/github-deployment-bridge/templates/deployment.yaml @@ -68,6 +68,8 @@ spec: value: {{ .Values.config.description | quote }} - name: LOG_URL_TEMPLATE value: {{ .Values.config.logURLTemplate | quote }} + - name: LOG_URL_TEMPLATE_ESCAPE + value: {{ .Values.config.logURLTemplateEscape | quote }} - name: LOG_LEVEL value: {{ .Values.config.logLevel | default "info" | quote }} - name: METRICS_ADDR diff --git a/charts/github-deployment-bridge/values.schema.json b/charts/github-deployment-bridge/values.schema.json index a3af301..1def261 100644 --- a/charts/github-deployment-bridge/values.schema.json +++ b/charts/github-deployment-bridge/values.schema.json @@ -184,6 +184,7 @@ "environmentURL": { "type": "string" }, "description": { "type": "string" }, "logURLTemplate": { "type": "string" }, + "logURLTemplateEscape": { "type": "boolean" }, "logLevel": { "type": "string", "enum": ["debug", "info", "warn", "error"] diff --git a/charts/github-deployment-bridge/values.yaml b/charts/github-deployment-bridge/values.yaml index c661447..bc95dc4 100644 --- a/charts/github-deployment-bridge/values.yaml +++ b/charts/github-deployment-bridge/values.yaml @@ -146,6 +146,9 @@ config: # {environment} {cluster}. Per-workload annotation log-url overrides this # and supports the same placeholders. logURLTemplate: "" + # Percent-encode every substituted placeholder value. The default false + # preserves the historical literal-substitution behavior. + logURLTemplateEscape: false # slog level: debug | info | warn | error logLevel: info leaderElection: true diff --git a/docs/configuration/environment.md b/docs/configuration/environment.md index c2b3ad4..a360464 100644 --- a/docs/configuration/environment.md +++ b/docs/configuration/environment.md @@ -15,6 +15,7 @@ SPDX-License-Identifier: Apache-2.0 | `ENVIRONMENT_URL` | no | | Default HTTPS URL on deployment statuses (overridable) | | `DESCRIPTION` | no | | Default GitHub Deployment description (overridable). Empty → `Deployed by FluxCD` | | `LOG_URL_TEMPLATE` | no | | Default log URL template (overridable). Placeholders: `{sha}`, `{namespace}`, `{name}`, `{service}`, `{environment}`, `{cluster}` | +| `LOG_URL_TEMPLATE_ESCAPE` | no | `false` | Percent-encode substituted values. Enable for templates containing path segments or query parameters; `false` preserves existing literal expansion. | | `LOG_LEVEL` | no | `info` | slog level: `debug`, `info`, `warn`, or `error` | | `METRICS_ADDR` | no | `:8080` | Prometheus metrics + convenience probes | | `PROBE_ADDR` | no | `:8081` | controller-runtime health probes | @@ -43,10 +44,72 @@ annotation both expand these tokens (annotation wins when set): | `{environment}` | Resolved GitHub Deployment environment | | `{cluster}` | Resolved cluster name | -Example (Grafana Loki Explore per service): +Only GitHub's `environment_url` and Deployment Status `log_url` are prominent +operational links. The bridge deliberately keeps the Deployment payload small; +use `log_url` to send operators to the system that already owns the rich data. + +Set `LOG_URL_TEMPLATE_ESCAPE=true` (Helm: `config.logURLTemplateEscape: true`) +for the presets below. Each substituted value is percent-encoded without +changing the template's URL syntax. The final result must be an absolute HTTPS +URL. An invalid result is omitted with a warning; deployment reporting continues. + +## Copy-paste presets + +Replace only the hostname, datasource/dashboard identifiers, and any fixed +dashboard path. + +### Grafana Loki Explore ```text https://grafana.example.com/a/grafana-lokiexplore-app/explore/service/{name}/logs?from=now-1h&to=now&var-ds=loki&var-filters=service_name%7C%3D%7C{name}&var-filters=namespace%7C%3D%7C{namespace} ``` +### Grafana dashboard + +```text +https://grafana.example.com/d/workload/logs?var-cluster={cluster}&var-namespace={namespace}&var-service={service}&var-environment={environment}&var-sha={sha} +``` + +### Generic Flux/Kubernetes dashboard + +```text +https://ops.example.com/clusters/{cluster}/namespaces/{namespace}/workloads/{name}?environment={environment}&service={service}&revision={sha} +``` + +## Workload overrides and monorepos + +The annotation remains a template and takes precedence over the global value. +This Kustomization-managed Deployment uses the global preset and gives one +monorepo workload its own deployment identity: + +```yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: storefront-api + namespace: storefront + annotations: + github-deployment-bridge.io/auto-report: "true" + github-deployment-bridge.io/deployment-name: "storefront-api" + github-deployment-bridge.io/service: "api" +``` + +A HelmRelease-managed StatefulSet can override the link (annotations belong on +the discovered workload, usually via the chart's pod/workload annotation values): + +```yaml +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: checkout-worker + namespace: checkout + annotations: + github-deployment-bridge.io/auto-report: "true" + github-deployment-bridge.io/deployment-name: "checkout-worker" + github-deployment-bridge.io/log-url: "https://grafana.example.com/d/workload/logs?var-cluster={cluster}&var-namespace={namespace}&var-service={service}&var-sha={sha}" +``` + +`deployment-name` separates cache/supersession identity for repositories with +multiple workloads; it does not add a new visible GitHub payload field. + See also: [Helm values map](./helm-values.md) diff --git a/docs/configuration/helm-values.md b/docs/configuration/helm-values.md index ff6d815..b007103 100644 --- a/docs/configuration/helm-values.md +++ b/docs/configuration/helm-values.md @@ -14,6 +14,7 @@ SPDX-License-Identifier: Apache-2.0 | `config.environmentURL` | `ENVIRONMENT_URL` | | `config.description` | `DESCRIPTION` | | `config.logURLTemplate` | `LOG_URL_TEMPLATE` | +| `config.logURLTemplateEscape` | `LOG_URL_TEMPLATE_ESCAPE` | | `config.logLevel` | `LOG_LEVEL` | | `config.leaderElection` | `LEADER_ELECTION` | | `config.githubBaseURL` | `GITHUB_BASE_URL` | diff --git a/docs/install/helm.md b/docs/install/helm.md index 5db8e95..1180414 100644 --- a/docs/install/helm.md +++ b/docs/install/helm.md @@ -22,7 +22,8 @@ helm upgrade --install github-deployment-bridge \ --set config.clusterName=production-eu \ --set config.environment=production \ --set config.environmentURL=https://app.example.com \ - --set config.logURLTemplate='https://grafana.example.com/explore?commit={sha}' + --set config.logURLTemplate='https://grafana.example.com/explore?commit={sha}' \ + --set config.logURLTemplateEscape=true ``` Per-service Loki Explore links (common for Grafana) can use workload placeholders @@ -54,6 +55,7 @@ config: environmentURL: https://app.example.com # Placeholders: {sha} {namespace} {name} {service} {environment} {cluster} logURLTemplate: https://grafana.example.com/explore?commit={sha} + logURLTemplateEscape: true logLevel: info github: @@ -150,6 +152,7 @@ reference (env vars, Helm map, metrics, registries). | `config.environmentURL` | `ENVIRONMENT_URL` | Optional URL on deployment statuses | | `config.description` | `DESCRIPTION` | Optional default deployment description (default `Deployed by FluxCD`) | | `config.logURLTemplate` | `LOG_URL_TEMPLATE` | Optional log URL template (`{sha}`, `{namespace}`, `{name}`, `{service}`, `{environment}`, `{cluster}`) | +| `config.logURLTemplateEscape` | `LOG_URL_TEMPLATE_ESCAPE` | Percent-encode substituted values (recommended) | | `config.logLevel` | `LOG_LEVEL` | `debug` / `info` / `warn` / `error` | | `config.githubBaseURL` | `GITHUB_BASE_URL` | GitHub Enterprise base URL | diff --git a/docs/operations/grafana.md b/docs/operations/grafana.md index d28194a..a6142b6 100644 --- a/docs/operations/grafana.md +++ b/docs/operations/grafana.md @@ -33,6 +33,15 @@ prometheusRule: release: kube-prometheus-stack ``` +## Link GitHub deployments to Grafana + +Use the bridge's `log_url` template to make GitHub's **View deployment logs** +link open the relevant Grafana view. Ready-to-use Loki Explore and dashboard +templates are in [Log URL placeholders](../configuration/environment.md#copy-paste-presets). +Enable `config.logURLTemplateEscape: true` so workload-derived values cannot +alter the URL query or path. This links to Grafana; it does not add Grafana data +or provider-specific fields to the GitHub Deployment payload. + ## Import ### Grafana UI diff --git a/docs/operations/runbook.md b/docs/operations/runbook.md index 557ae4b..a9189c8 100644 --- a/docs/operations/runbook.md +++ b/docs/operations/runbook.md @@ -108,3 +108,16 @@ When alerts are quiet but GitHub shows no Deployment: bridges share a repo. 4. Dedup cache: same `(owner, repo, environment, commit, deploymentName)` skips duplicates - expected after retries. + +## Deployment log link is missing (no alert) + +1. Confirm `LOG_URL_TEMPLATE` / `config.logURLTemplate`, or the workload's + `github-deployment-bridge.io/log-url` override, is non-empty. +2. Search bridge logs for `log URL template expanded to an invalid HTTPS URL`. + Invalid links are intentionally omitted without failing reconciliation or + GitHub deployment/status reporting. +3. The expanded URL must be absolute HTTPS. Enable + `LOG_URL_TEMPLATE_ESCAPE=true` / `config.logURLTemplateEscape: true` when + workload values appear in paths or query parameters. +4. Copy a known-good [preset](../configuration/environment.md#copy-paste-presets) + and replace its host and dashboard/datasource identifiers. diff --git a/internal/config/config.go b/internal/config/config.go index dc279bf..e4ee6b6 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -8,6 +8,7 @@ package config import ( "fmt" "log/slog" + "net/url" "strings" "time" @@ -23,6 +24,7 @@ type Config struct { EnvironmentURL string `envconfig:"ENVIRONMENT_URL"` Description string `envconfig:"DESCRIPTION"` LogURLTemplate string `envconfig:"LOG_URL_TEMPLATE"` + LogURLTemplateEscape bool `envconfig:"LOG_URL_TEMPLATE_ESCAPE" default:"false"` LogLevel string `envconfig:"LOG_LEVEL" default:"info"` MetricsAddr string `envconfig:"METRICS_ADDR" default:":8080"` ProbeAddr string `envconfig:"PROBE_ADDR" default:":8081"` @@ -105,7 +107,7 @@ type LogURLVars struct { // ExpandLogURL substitutes placeholders in LOG_URL_TEMPLATE. func (c Config) ExpandLogURL(v LogURLVars) string { - return ExpandLogURLTemplate(c.LogURLTemplate, v) + return ExpandLogURLTemplateEscaped(c.LogURLTemplate, v, c.LogURLTemplateEscape) } // ExpandLogURLTemplate substitutes log URL placeholders in tmpl. @@ -113,6 +115,12 @@ func (c Config) ExpandLogURL(v LogURLVars) string { // Supported: {sha}, {namespace}, {name}, {service}, {environment}, {cluster}. // {service} falls back to {name} when Service is empty. func ExpandLogURLTemplate(tmpl string, v LogURLVars) string { + return ExpandLogURLTemplateEscaped(tmpl, v, false) +} + +// ExpandLogURLTemplateEscaped substitutes placeholders, optionally percent-encoding +// each value so it is safe in both URL paths and query parameters. +func ExpandLogURLTemplateEscaped(tmpl string, v LogURLVars, escape bool) string { if strings.TrimSpace(tmpl) == "" { return "" } @@ -120,12 +128,20 @@ func ExpandLogURLTemplate(tmpl string, v LogURLVars) string { if service == "" { service = v.Name } + value := func(s string) string { + if escape { + // QueryEscape covers URL delimiters (including '/', '&', '?' and '#'). + // Use %20 rather than '+' so the result is also safe in path segments. + return strings.ReplaceAll(url.QueryEscape(s), "+", "%20") + } + return s + } return strings.NewReplacer( - "{sha}", v.SHA, - "{namespace}", v.Namespace, - "{name}", v.Name, - "{service}", service, - "{environment}", v.Environment, - "{cluster}", v.Cluster, + "{sha}", value(v.SHA), + "{namespace}", value(v.Namespace), + "{name}", value(v.Name), + "{service}", value(service), + "{environment}", value(v.Environment), + "{cluster}", value(v.Cluster), ).Replace(tmpl) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f333e07..43fbdcc 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -118,6 +118,18 @@ func TestExpandLogURLTemplate(t *testing.T) { vars: config.LogURLVars{Name: "backend"}, want: "https://logs.example.com/s/backend", }, + { + name: "grafana dashboard preset", + tmpl: "https://grafana.example.com/d/workload/logs?var-cluster={cluster}&var-namespace={namespace}&var-service={service}&var-environment={environment}&var-sha={sha}", + vars: config.LogURLVars{SHA: "deadbeef", Namespace: "apps", Name: "api", Cluster: "prod", Environment: "production"}, + want: "https://grafana.example.com/d/workload/logs?var-cluster=prod&var-namespace=apps&var-service=api&var-environment=production&var-sha=deadbeef", + }, + { + name: "generic flux kubernetes dashboard preset", + tmpl: "https://ops.example.com/clusters/{cluster}/namespaces/{namespace}/workloads/{name}?environment={environment}&service={service}&revision={sha}", + vars: config.LogURLVars{SHA: "abc", Namespace: "payments", Name: "worker", Service: "payments-worker", Cluster: "eu-1", Environment: "staging"}, + want: "https://ops.example.com/clusters/eu-1/namespaces/payments/workloads/worker?environment=staging&service=payments-worker&revision=abc", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -129,6 +141,28 @@ func TestExpandLogURLTemplate(t *testing.T) { } } +func TestExpandLogURLTemplateEscaped(t *testing.T) { + t.Parallel() + tests := []struct { + name string + tmpl string + vars config.LogURLVars + escape bool + want string + }{ + {"backwards compatible literal values", "https://logs.example/{namespace}?q={service}", config.LogURLVars{Namespace: "team one", Service: "api/core"}, false, "https://logs.example/team one?q=api/core"}, + {"escape path and query values", "https://logs.example/{namespace}?q={service}&env={environment}", config.LogURLVars{Namespace: "team one", Service: "api/core?x=1", Environment: "preview & qa"}, true, "https://logs.example/team%20one?q=api%2Fcore%3Fx%3D1&env=preview%20%26%20qa"}, + {"escape fallback service", "https://logs.example/?service={service}", config.LogURLVars{Name: "api/canary"}, true, "https://logs.example/?service=api%2Fcanary"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := config.ExpandLogURLTemplateEscaped(tt.tmpl, tt.vars, tt.escape); got != tt.want { + t.Fatalf("ExpandLogURLTemplateEscaped = %q, want %q", got, tt.want) + } + }) + } +} + func TestLogLevel(t *testing.T) { base := func(t *testing.T) { t.Helper() diff --git a/internal/deployment/reporter.go b/internal/deployment/reporter.go index c7debb1..f6320d4 100644 --- a/internal/deployment/reporter.go +++ b/internal/deployment/reporter.go @@ -434,15 +434,18 @@ func (r *Reporter) resolveImage(ctx context.Context, img WorkloadImage) (metadat if !fromAnnotation { tmpl = r.cfg.LogURLTemplate } - if logURL := config.ExpandLogURLTemplate(tmpl, vars); logURL != "" { + if logURL := config.ExpandLogURLTemplateEscaped(tmpl, vars, r.cfg.LogURLTemplateEscape); logURL != "" { if !metadata.ValidHTTPSURL(logURL) { src := "LOG_URL_TEMPLATE" if fromAnnotation { src = metadata.AnnotationLogURL } - return metadata.Resolved{}, "", retry.Permanent(fmt.Errorf("%s expanded to invalid HTTPS URL %q", src, logURL)) + r.log.Warn("log URL template expanded to an invalid HTTPS URL; omitting log_url", + "source", src, "url", logURL, "workload_kind", img.Kind, + "workload_namespace", img.Namespace, "workload_name", img.Name) + } else { + resolved.LogURL = logURL } - resolved.LogURL = logURL } return resolved, ociMeta.Digest, nil diff --git a/internal/deployment/reporter_test.go b/internal/deployment/reporter_test.go index 634dce5..28d1e6e 100644 --- a/internal/deployment/reporter_test.go +++ b/internal/deployment/reporter_test.go @@ -5,10 +5,12 @@ package deployment_test import ( + "bytes" "context" "fmt" "log/slog" "path/filepath" + "strings" "sync" "testing" "time" @@ -225,6 +227,31 @@ func TestReporterLogURLAnnotationTemplate(t *testing.T) { } } +func TestReporterInvalidExpandedLogURLIsOmitted(t *testing.T) { + t.Parallel() + store, err := cache.Open(filepath.Join(t.TempDir(), "cache.db")) + if err != nil { + t.Fatalf("open cache: %v", err) + } + t.Cleanup(func() { _ = store.Close() }) + + var logs bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logs, nil)) + g := &fakeGitHub{} + r := deployment.NewReporter(config.Config{ClusterName: "prod", Environment: "production", LogURLTemplate: "://bad/{name}"}, store, + &fakeRegistry{meta: ocilabels.Metadata{Source: "https://github.com/example/backend", Revision: "deadbeef", Digest: "sha256:abc"}}, g, nil, logger, "test") + + if err := r.Report(context.Background(), sampleInput(deployment.PhaseSuccess)); err != nil { + t.Fatalf("invalid log URL must not fail reporting: %v", err) + } + if len(g.statuses) != 1 || g.statuses[0].LogURL != "" { + t.Fatalf("statuses = %#v, want one status without log URL", g.statuses) + } + if !strings.Contains(logs.String(), "invalid HTTPS URL; omitting log_url") || !strings.Contains(logs.String(), "source=LOG_URL_TEMPLATE") { + t.Fatalf("warning log = %q", logs.String()) + } +} + func TestReporterPayloadOptionalAnnotations(t *testing.T) { t.Parallel() r, g := newTestReporter(t, nil) diff --git a/pkg/metadata/resolve.go b/pkg/metadata/resolve.go index 681547e..7ca9981 100644 --- a/pkg/metadata/resolve.go +++ b/pkg/metadata/resolve.go @@ -100,12 +100,11 @@ func Resolve(annotations map[string]string, oci ocilabels.Metadata, defaults Def return Resolved{}, err } - // Annotation log-url may still contain placeholders; the reporter expands them. - // defaults.LogURL is unused (template applied in the reporter after resolve). - logURL, err := resolveHTTPSURL(ann[AnnotationLogURL], defaults.LogURL, AnnotationLogURL) - if err != nil { - return Resolved{}, err - } + // log-url is a template, not a URL yet. The reporter expands and validates it + // non-fatally so a broken operational link cannot block deployment reporting. + // defaults.LogURL is retained for API compatibility; runtime templates are + // selected by the reporter. + logURL := firstNonEmpty(ann[AnnotationLogURL], defaults.LogURL) description := firstNonEmpty(ann[AnnotationDescription], defaults.Description) if description == "" {