diff --git a/tools/chart-service-edge/.gitignore b/tools/chart-service-edge/.gitignore new file mode 100644 index 000000000..d914e38f8 --- /dev/null +++ b/tools/chart-service-edge/.gitignore @@ -0,0 +1,2 @@ +# go build ./... drops the binary here; it must never be committed. +/chart-service-edge diff --git a/tools/chart-service-edge/go.mod b/tools/chart-service-edge/go.mod new file mode 100644 index 000000000..07d9aa7bc --- /dev/null +++ b/tools/chart-service-edge/go.mod @@ -0,0 +1,3 @@ +module chart-service-edge + +go 1.26 diff --git a/tools/chart-service-edge/main.go b/tools/chart-service-edge/main.go new file mode 100644 index 000000000..0ab39f7c6 --- /dev/null +++ b/tools/chart-service-edge/main.go @@ -0,0 +1,152 @@ +// SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Command chart-service-edge reports which charts declare the service they +// deploy. +// +// chart-service-edge --audit report, exit 0 +// chart-service-edge --audit --strict report, exit 1 if any chart is undeclared +// +// A chart release moves a stack pin, and stack-pin-resolver handles that +// because a released tag names its own chart. The other direction does not work +// that way: when a service releases, nothing in the repository says which chart +// deploys it, so nothing can bump that chart's appVersion or image pin. +// +// That edge has to be declared, because it cannot be derived. Three attempts, +// all of which fail: +// +// Image repository. Charts deliberately leave registry and repository empty so +// an operator supplies them, so most charts name no image at all. +// +// Directory name. Only 12 of 22 chart directories share a name with a service, +// and two of those matches are wrong: deploy/helm/cassandra and +// deploy/helm/openbao match the migrations service rather than the service +// itself, because infra/cassandra and migrations/cassandra share a leaf name. +// A derivation that is wrong is worse than one that is missing. +// +// Chart name. The chart at deploy/helm/icms publishes as helm-nvcf-sis and +// carries the instance-cluster-management service. Directory, published name, +// and service are three different strings; the chart was never renamed after +// the service was. +// +// So a chart entry in tools/ci/github-release-subprojects.json may carry: +// +// "deploys": ["", ...] +// +// listing the release-metadata ids of the services whose images it ships. A +// chart that ships no first-party image (an upstream dependency, or resources +// only) declares "deploys": [] to say so deliberately. +// +// This starts in report mode on purpose. Turning it strict before the +// declarations exist would fail every build and teach people to route around +// it. Land the declarations, then add --strict to CI. +package main + +import ( + "flag" + "fmt" + "io" + "os" + "path/filepath" + "sort" + "strings" +) + +func main() { + audit := flag.Bool("audit", false, "report the declared and undeclared chart to service edges") + strict := flag.Bool("strict", false, "exit non-zero when a chart has not declared its edge") + root := flag.String("root", ".", "repository root") + flag.Parse() + + if !*audit { + flag.Usage() + os.Exit(1) + } + code, err := run(*root, *strict, os.Stdout, os.Stderr) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(code) +} + +func run(root string, strict bool, out, errOut io.Writer) (int, error) { + meta, err := LoadMetadata(filepath.Join(root, MetadataPath)) + if err != nil { + return 1, err + } + return Audit(meta, strict, out, errOut), nil +} + +// Audit prints the edge report and returns the process exit code. +func Audit(meta *Metadata, strict bool, out, errOut io.Writer) int { + charts := meta.Charts() + serviceIDs := meta.ServiceIDs() + + var declared, undeclared []Entry + type badEntry struct { + chart Entry + unknown []string + } + var bad []badEntry + + sort.Slice(charts, func(i, j int) bool { return charts[i].ID < charts[j].ID }) + for _, c := range charts { + // Absent and empty must stay distinguishable. An undeclared chart is + // outstanding work; an empty list is a decision that the chart ships no + // first-party image. Both look like "no service" if you only test + // whether the list is empty. + if c.Deploys == nil { + undeclared = append(undeclared, c) + continue + } + var unknown []string + for _, s := range c.Deploys { + if !serviceIDs[s] { + unknown = append(unknown, s) + } + } + if len(unknown) > 0 { + bad = append(bad, badEntry{c, unknown}) + } else { + declared = append(declared, c) + } + } + + for _, c := range declared { + target := "(no first-party image)" + if len(c.Deploys) > 0 { + target = strings.Join(c.Deploys, ", ") + } + fmt.Fprintf(out, " %-26s -> %s\n", c.ID, target) + } + for _, b := range bad { + fmt.Fprintf(out, " %-26s -> UNKNOWN SERVICE ID: %s\n", b.chart.ID, strings.Join(b.unknown, ", ")) + } + for _, c := range undeclared { + fmt.Fprintf(out, " %-26s -> undeclared\n", c.ID) + } + + fmt.Fprintf(out, "\n%d charts: %d declared, %d undeclared, %d naming an unknown service\n", + len(charts), len(declared), len(undeclared), len(bad)) + + // An id that does not resolve is always an error: it means a service was + // renamed or removed and this edge was left pointing at nothing. + if len(bad) > 0 { + fmt.Fprintln(errOut, "\nThese charts name a service id that does not exist:") + for _, b := range bad { + fmt.Fprintf(errOut, " %s: %s\n", b.chart.ID, strings.Join(b.unknown, ", ")) + } + return 1 + } + + if len(undeclared) > 0 && strict { + fmt.Fprintln(errOut, "\nThese charts do not declare the service they deploy, so a service release cannot reach them:") + for _, c := range undeclared { + fmt.Fprintf(errOut, " %s (%s)\n", c.ID, c.Path) + } + return 1 + } + + return 0 +} diff --git a/tools/chart-service-edge/main_test.go b/tools/chart-service-edge/main_test.go new file mode 100644 index 000000000..8d9bf188c --- /dev/null +++ b/tools/chart-service-edge/main_test.go @@ -0,0 +1,203 @@ +// SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// The distinction this whole tool rests on is absent versus empty. A chart that +// has not declared its edge is outstanding work; one that declares an empty +// list has stated it ships no first-party image. Every test below exists to +// keep those two apart, or to keep a dangling id from passing as declared. + +func decode(t *testing.T, body string) *Metadata { + t.Helper() + var m Metadata + if err := json.Unmarshal([]byte(body), &m); err != nil { + t.Fatalf("fixture does not parse: %v", err) + } + return &m +} + +func audit(t *testing.T, body string, strict bool) (int, string, string) { + t.Helper() + var out, errOut bytes.Buffer + code := Audit(decode(t, body), strict, &out, &errOut) + return code, out.String(), errOut.String() +} + +const fixture = `{"services":[ + {"id":"svc", "path":"src/svc"}, + {"id":"other", "path":"src/other"}, + {"id":"declared", "path":"deploy/helm/declared", "deploys":["svc"]}, + {"id":"noimage", "path":"deploy/helm/noimage", "deploys":[]}, + {"id":"gap", "path":"deploy/helm/gap"} +]}` + +func TestUndeclaredIsNotCountedAsDeclared(t *testing.T) { + code, out, _ := audit(t, fixture, false) + if code != 0 { + t.Fatalf("report mode must not fail, got %d", code) + } + // Asserted on the counts, not on the word "undeclared" appearing somewhere: + // the report uses that word itself, so a substring check would pass with the + // behaviour removed entirely. + if !strings.Contains(out, "3 charts: 2 declared, 1 undeclared, 0 naming an unknown service") { + t.Fatalf("counts wrong:\n%s", out) + } + if !strings.Contains(out, "gap -> undeclared") { + t.Fatalf("the undeclared chart must appear as undeclared:\n%s", out) + } +} + +func TestEmptyDeploysIsADeclarationNotAGap(t *testing.T) { + // The case a len()-only test would get wrong: an empty list is a decision, + // so strict mode must not fail on it. + code, out, _ := audit(t, fixture, true) + if !strings.Contains(out, "noimage") || !strings.Contains(out, "(no first-party image)") { + t.Fatalf("empty deploys should report as a deliberate declaration:\n%s", out) + } + // strict still fails, but for the undeclared chart, not this one. + if code != 1 { + t.Fatalf("strict must fail while a chart is undeclared, got %d", code) + } +} + +func TestStrictFailsOnUndeclaredAndNamesIt(t *testing.T) { + code, _, errOut := audit(t, fixture, true) + if code != 1 { + t.Fatalf("strict must fail on an undeclared chart, got %d", code) + } + if !strings.Contains(errOut, "gap (deploy/helm/gap)") { + t.Fatalf("strict failure must name the chart and its path:\n%s", errOut) + } +} + +func TestStrictPassesOnceEveryChartDeclares(t *testing.T) { + body := `{"services":[ + {"id":"svc","path":"src/svc"}, + {"id":"a","path":"deploy/helm/a","deploys":["svc"]}, + {"id":"b","path":"deploy/helm/b","deploys":[]} + ]}` + if code, _, _ := audit(t, body, true); code != 0 { + t.Fatalf("strict must pass when nothing is undeclared, got %d", code) + } +} + +func TestUnknownServiceIDFailsEvenInReportMode(t *testing.T) { + // A dangling id means a service was renamed and the edge left pointing at + // nothing. That is an error whether or not strict is on, because unlike an + // undeclared chart it is not outstanding work anyone planned. + body := `{"services":[ + {"id":"svc","path":"src/svc"}, + {"id":"stale","path":"deploy/helm/stale","deploys":["renamed-away"]} + ]}` + code, out, errOut := audit(t, body, false) + if code != 1 { + t.Fatalf("an unknown service id must fail even in report mode, got %d", code) + } + if !strings.Contains(out, "UNKNOWN SERVICE ID") { + t.Fatalf("report should flag the unknown id:\n%s", out) + } + if !strings.Contains(errOut, "renamed-away") { + t.Fatalf("stderr should name the missing id:\n%s", errOut) + } +} + +func TestAChartMayNotPointAtAnotherChart(t *testing.T) { + // Chart entries are excluded from the service id set, so naming one is a + // dangling edge. Without that exclusion a chart could satisfy the audit by + // pointing at a sibling chart, which moves no image. + body := `{"services":[ + {"id":"svc","path":"src/svc"}, + {"id":"target","path":"deploy/helm/target","deploys":[]}, + {"id":"pointer","path":"deploy/helm/pointer","deploys":["target"]} + ]}` + if code, _, _ := audit(t, body, false); code != 1 { + t.Fatalf("pointing at a chart must not count as a declared edge, got %d", code) + } +} + +func TestNullDeploysReadsAsUndeclared(t *testing.T) { + // An explicit null is not a declaration of "no image": nobody writing null + // means that, and treating it as one would silently exempt the chart from + // strict mode forever. + body := `{"services":[ + {"id":"svc","path":"src/svc"}, + {"id":"nulled","path":"deploy/helm/nulled","deploys":null} + ]}` + code, out, _ := audit(t, body, true) + if code != 1 { + t.Fatalf("null deploys must be treated as undeclared, got %d", code) + } + if !strings.Contains(out, "-> undeclared") { + t.Fatalf("null deploys should report as undeclared:\n%s", out) + } +} + +func TestServicesAreNotAudited(t *testing.T) { + // Only chart entries carry the edge. A service with no deploys key is not a + // gap, and counting it as one would make strict mode unreachable. + body := `{"services":[ + {"id":"svc","path":"src/svc"}, + {"id":"a","path":"deploy/helm/a","deploys":["svc"]} + ]}` + code, out, _ := audit(t, body, true) + if code != 0 { + t.Fatalf("services must not be audited as charts, got %d", code) + } + if !strings.Contains(out, "1 charts") { + t.Fatalf("only the chart should be counted:\n%s", out) + } +} + +func TestReportListsEveryChartExactlyOnce(t *testing.T) { + // The report is how someone finds the work; a chart silently missing from + // it reads as "nothing to do". + _, out, _ := audit(t, fixture, false) + for _, id := range []string{"declared", "noimage", "gap"} { + if n := strings.Count(out, " "+id+" "); n != 1 { + t.Fatalf("chart %s appears %d times in the report, want 1:\n%s", id, n, out) + } + } +} + +func TestRealMetadataParsesAndAudits(t *testing.T) { + // The checked-in metadata must stay loadable and free of dangling ids. This + // is the case that catches a service renamed without updating its edges. + root := repoRoot(t) + meta, err := LoadMetadata(filepath.Join(root, MetadataPath)) + if err != nil { + t.Fatalf("checked-in release metadata does not load: %v", err) + } + var out, errOut bytes.Buffer + if code := Audit(meta, false, &out, &errOut); code != 0 { + t.Fatalf("checked-in metadata has a dangling chart to service edge:\n%s", errOut.String()) + } + if len(meta.Charts()) == 0 { + t.Fatal("no charts found in the checked-in metadata; the path or prefix is wrong") + } +} + +func repoRoot(t *testing.T) string { + t.Helper() + dir, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + for i := 0; i < 6; i++ { + if _, err := os.Stat(filepath.Join(dir, MetadataPath)); err == nil { + return dir + } + dir = filepath.Dir(dir) + } + t.Fatalf("could not find %s above the test directory", MetadataPath) + return "" +} diff --git a/tools/chart-service-edge/metadata.go b/tools/chart-service-edge/metadata.go new file mode 100644 index 000000000..49d0755f1 --- /dev/null +++ b/tools/chart-service-edge/metadata.go @@ -0,0 +1,70 @@ +// SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "encoding/json" + "fmt" + "os" + "strings" +) + +// MetadataPath is the release metadata that github-release already owns. The +// chart to service edges live there rather than in a file of their own so a +// service rename has one place to update, not two. +const MetadataPath = "tools/ci/github-release-subprojects.json" + +// ChartPrefix marks an entry as a chart rather than a service. +const ChartPrefix = "deploy/helm/" + +// Entry is one subproject in the release metadata. +type Entry struct { + ID string `json:"id"` + Path string `json:"path"` + // Deploys is nil when the key is absent and non-nil empty when the chart + // declares it ships no first-party image. That distinction is the whole + // point of the audit, so it must survive decoding: a []string does exactly + // that, where a map lookup or a len() test would flatten the two together. + Deploys []string `json:"deploys"` +} + +// Metadata is the decoded release metadata file. +type Metadata struct { + Services []Entry `json:"services"` +} + +// LoadMetadata reads and decodes the release metadata. +func LoadMetadata(path string) (*Metadata, error) { + b, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("read release metadata: %w", err) + } + var m Metadata + if err := json.Unmarshal(b, &m); err != nil { + return nil, fmt.Errorf("parse %s: %w", path, err) + } + return &m, nil +} + +// Charts returns the entries that are charts. +func (m *Metadata) Charts() []Entry { + var out []Entry + for _, e := range m.Services { + if strings.HasPrefix(e.Path, ChartPrefix) { + out = append(out, e) + } + } + return out +} + +// ServiceIDs returns the ids of the entries that are not charts. +func (m *Metadata) ServiceIDs() map[string]bool { + out := map[string]bool{} + for _, e := range m.Services { + if !strings.HasPrefix(e.Path, ChartPrefix) { + out[e.ID] = true + } + } + return out +} diff --git a/tools/ci/chart-service-edge b/tools/ci/chart-service-edge new file mode 100755 index 000000000..4d801e7d0 --- /dev/null +++ b/tools/ci/chart-service-edge @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Stable CI entrypoint for the Go tool in tools/chart-service-edge. +# +# The wrapper exists for two reasons. +# +# The repository root. `go run -C ` leaves the process running with that +# directory as its working directory, so the tool cannot find the release +# metadata on its own. Resolving the root from this script's own location means +# callers do not have to pass it. +# +# The exit code. `go run` does NOT propagate the program's status: it prints +# "exit status N" and exits 1, collapsing every non-zero code into one. This +# tool only uses 0 and 1 today, so nothing is lost yet, but building the binary +# keeps that from becoming a trap the first time a distinct code is added. +# +# Run the tests with: go test -C tools/chart-service-edge ./... +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +bin_dir="$(mktemp -d)" +trap 'rm -rf "${bin_dir}"' EXIT + +go build -C "${repo_root}/tools/chart-service-edge" -o "${bin_dir}/chart-service-edge" . + +# Not exec, so the trap above still runs, and not under errexit, so the exit +# code reaches the caller rather than aborting the shell first. +set +e +"${bin_dir}/chart-service-edge" --root "${repo_root}" "$@" +status=$? +set -e +exit "${status}" diff --git a/tools/ci/github-release-subprojects.json b/tools/ci/github-release-subprojects.json index 37fd945ca..77364a125 100644 --- a/tools/ci/github-release-subprojects.json +++ b/tools/ci/github-release-subprojects.json @@ -63,7 +63,10 @@ "id": "grpc-proxy-helm", "path": "deploy/helm/grpc-proxy", "service_name": "helm-nvcf-grpc-proxy", - "legacy_tag_prefix": "helm-nvcf-grpc-proxy-v" + "legacy_tag_prefix": "helm-nvcf-grpc-proxy-v", + "deploys": [ + "grpc-proxy" + ] }, { "id": "ratelimiter", @@ -75,7 +78,10 @@ "id": "ratelimiter-helm", "path": "deploy/helm/ratelimiter", "service_name": "helm-nvcf-rate-limiter", - "legacy_tag_prefix": "helm-nvcf-rate-limiter-v" + "legacy_tag_prefix": "helm-nvcf-rate-limiter-v", + "deploys": [ + "ratelimiter" + ] }, { "id": "http-invocation", @@ -99,7 +105,10 @@ "id": "llm-api-gateway-helm", "path": "deploy/helm/llm-api-gateway", "service_name": "helm-nvcf-llm-api-gateway", - "legacy_tag_prefix": "helm-nvcf-llm-api-gateway-v" + "legacy_tag_prefix": "helm-nvcf-llm-api-gateway-v", + "deploys": [ + "llm-api-gateway" + ] }, { "id": "vanity-gateway", @@ -116,7 +125,10 @@ { "id": "function-autoscaler-helm", "path": "deploy/helm/function-autoscaler", - "service_name": "helm-nvcf-function-autoscaler" + "service_name": "helm-nvcf-function-autoscaler", + "deploys": [ + "function-autoscaler" + ] }, { "id": "nats-auth-callout", @@ -175,7 +187,8 @@ "id": "nats", "path": "deploy/helm/nats", "service_name": "helm-nvcf-nats", - "legacy_tag_prefix": "helm-nvcf-nats-v" + "legacy_tag_prefix": "helm-nvcf-nats-v", + "deploys": [] }, { "id": "admin-token-issuer-proxy", @@ -205,12 +218,14 @@ "id": "cert-manager", "path": "deploy/helm/cert-manager", "service_name": "helm-nvcf-cert-manager", - "legacy_tag_prefix": "helm-nvcf-cert-manager-v" + "legacy_tag_prefix": "helm-nvcf-cert-manager-v", + "deploys": [] }, { "id": "nvcf-pki", "path": "deploy/helm/nvcf-pki", - "service_name": "helm-nvcf-pki" + "service_name": "helm-nvcf-pki", + "deploys": [] }, { "id": "llm-request-router", @@ -222,19 +237,26 @@ "id": "gateway-routes", "path": "deploy/helm/gateway-routes", "service_name": "nvcf-gateway-routes", - "legacy_tag_prefix": "nvcf-gateway-routes-v" + "legacy_tag_prefix": "nvcf-gateway-routes-v", + "deploys": [] }, { "id": "vanity-gateway-helm", "path": "deploy/helm/vanity-gateway", "service_name": "helm-nvcf-vanity-gateway", - "initial_version": "0.2.0" + "initial_version": "0.2.0", + "deploys": [ + "vanity-gateway" + ] }, { "id": "nvca-operator", "path": "deploy/helm/nvca-operator", "service_name": "helm-nvca-operator", - "legacy_tag_prefix": "helm-nvca-operator-v" + "legacy_tag_prefix": "helm-nvca-operator-v", + "deploys": [ + "nvca" + ] }, { "id": "cassandra-migrations", @@ -288,7 +310,10 @@ { "id": "event-ledger-helm", "path": "deploy/helm/event-ledger", - "service_name": "helm-nvcf-event-ledger" + "service_name": "helm-nvcf-event-ledger", + "deploys": [ + "event-ledger" + ] }, { "id": "ess", @@ -300,25 +325,37 @@ "id": "ess-helm", "path": "deploy/helm/encrypted-secret-store", "service_name": "helm-nvcf-ess-api", - "legacy_tag_prefix": "deploy/helm/ess/v" + "legacy_tag_prefix": "deploy/helm/ess/v", + "deploys": [ + "ess" + ] }, { "id": "cloud-tasks-helm", "path": "deploy/helm/cloud-tasks", "service_name": "helm-nvcf-nvct-api", - "initial_version": "1.4.4" + "initial_version": "1.4.4", + "deploys": [ + "cloud-tasks" + ] }, { "id": "notary-helm", "path": "deploy/helm/notary", "service_name": "helm-nvcf-notary-service", - "initial_version": "1.4.2" + "initial_version": "1.4.2", + "deploys": [ + "notary" + ] }, { "id": "cloud-functions-helm", "path": "deploy/helm/cloud-functions", "service_name": "helm-nvcf-api", - "initial_version": "1.23.11" + "initial_version": "1.23.11", + "deploys": [ + "cloud-functions" + ] }, { "id": "instance-cluster-management",