diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 850a028..e0e9654 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -34,6 +34,12 @@ jobs: - name: go vet run: go vet ./... + - name: go vet (build-tagged code) + run: | + set -euo pipefail + go vet -tags=load ./... + go vet -tags=e2e ./... + - name: go test (race + cover) run: go test -race -coverprofile=coverage.out ./... diff --git a/cmd/artemis/bootrun_test.go b/cmd/artemis/bootrun_test.go index 76f7d99..c53a254 100644 --- a/cmd/artemis/bootrun_test.go +++ b/cmd/artemis/bootrun_test.go @@ -20,6 +20,7 @@ import ( "github.com/testcontainers/testcontainers-go/wait" "github.com/freeCodeCamp/artemis/internal/config" + "github.com/freeCodeCamp/artemis/internal/config/configtest" "github.com/freeCodeCamp/artemis/internal/pg" vkstore "github.com/freeCodeCamp/artemis/internal/registry/valkey" ) @@ -322,22 +323,24 @@ func TestRun_BootsFromEnvAndExitsOnSigterm(t *testing.T) { dsn, valkeyAddr := startDeps(t) port := freePort(t) - t.Setenv("PORT", strconv.Itoa(port)) - t.Setenv("DATABASE_URL", dsn) - t.Setenv("VALKEY_ADDR", valkeyAddr) - t.Setenv("R2_ENDPOINT", "http://127.0.0.1:1") - t.Setenv("R2_ACCESS_KEY_ID", "k") - t.Setenv("R2_SECRET_ACCESS_KEY", "s") - t.Setenv("R2_BUCKET", "b") - t.Setenv("GH_CLIENT_ID", "cid") - t.Setenv("JWT_SIGNING_KEY", "0123456789abcdef0123456789abcdef") - t.Setenv("DEPLOY_PREFIX_FORMAT", ".example.test/deploys/-/") - t.Setenv("ALIAS_PRODUCTION_KEY_FORMAT", ".example.test/production") - t.Setenv("ALIAS_PREVIEW_KEY_FORMAT", ".example.test/preview") - t.Setenv("LOG_LEVEL", "error") - t.Setenv("SENTRY_DSN", "https://publickey@o0.ingest.sentry.io/0") - t.Setenv("ENVIRONMENT", "test") - t.Setenv("SENTRY_TRACES_SAMPLE_RATE", "0") + configtest.Hermetic(t, config.EnvKeys(), map[string]string{ + "PORT": strconv.Itoa(port), + "DATABASE_URL": dsn, + "VALKEY_ADDR": valkeyAddr, + "R2_ENDPOINT": "http://127.0.0.1:1", + "R2_ACCESS_KEY_ID": "k", + "R2_SECRET_ACCESS_KEY": "s", + "R2_BUCKET": "b", + "GH_CLIENT_ID": "cid", + "JWT_SIGNING_KEY": "0123456789abcdef0123456789abcdef", + "DEPLOY_PREFIX_FORMAT": ".example.test/deploys/-/", + "ALIAS_PRODUCTION_KEY_FORMAT": ".example.test/production", + "ALIAS_PREVIEW_KEY_FORMAT": ".example.test/preview", + "LOG_LEVEL": "error", + "SENTRY_DSN": "https://publickey@o0.ingest.sentry.io/0", + "ENVIRONMENT": "test", + "SENTRY_TRACES_SAMPLE_RATE": "0", + }) done := make(chan error, 1) go func() { done <- run() }() diff --git a/cmd/artemis/driftreport.go b/cmd/artemis/driftreport.go index 687a578..9613a67 100644 --- a/cmd/artemis/driftreport.go +++ b/cmd/artemis/driftreport.go @@ -245,7 +245,7 @@ func driftReportSites(ctx context.Context, repo siteDirnameReader, reg registryS if err != nil { return nil, err } - slugs := make([]string, 0, len(sites)) + slugs := make([]sitekey.Slug, 0, len(sites)) for _, s := range sites { slugs = append(slugs, s.Slug) } diff --git a/cmd/artemis/driftreport_e2e_test.go b/cmd/artemis/driftreport_e2e_test.go index 679bc03..0ecd721 100644 --- a/cmd/artemis/driftreport_e2e_test.go +++ b/cmd/artemis/driftreport_e2e_test.go @@ -18,8 +18,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/freeCodeCamp/artemis/internal/config" + "github.com/freeCodeCamp/artemis/internal/config/configtest" "github.com/freeCodeCamp/artemis/internal/pg" - "github.com/freeCodeCamp/artemis/internal/sitekey" ) @@ -112,15 +113,17 @@ func (f *fakeBucket) listV2(w http.ResponseWriter, prefix string) { func driftReportEnv(t *testing.T, dsn, endpoint string) { t.Helper() - t.Setenv("DATABASE_URL", dsn) - t.Setenv("R2_ENDPOINT", endpoint) - t.Setenv("R2_BUCKET", "artemis-test") - t.Setenv("R2_ACCESS_KEY_ID", "k") - t.Setenv("R2_SECRET_ACCESS_KEY", "s") - t.Setenv("DEPLOY_PREFIX_FORMAT", "/deploys/-/") - t.Setenv("GH_CLIENT_ID", "cid") - t.Setenv("JWT_SIGNING_KEY", "0123456789abcdef0123456789abcdef") - t.Setenv("VALKEY_ADDR", "127.0.0.1:1") + configtest.Hermetic(t, config.EnvKeys(), map[string]string{ + "DATABASE_URL": dsn, + "R2_ENDPOINT": endpoint, + "R2_BUCKET": "artemis-test", + "R2_ACCESS_KEY_ID": "k", + "R2_SECRET_ACCESS_KEY": "s", + "DEPLOY_PREFIX_FORMAT": "/deploys/-/", + "GH_CLIENT_ID": "cid", + "JWT_SIGNING_KEY": "0123456789abcdef0123456789abcdef", + "VALKEY_ADDR": "127.0.0.1:1", + }) } func seedDriftFixture(t *testing.T, dsn string, site sitekey.Dirname, deployID string) { diff --git a/cmd/artemis/driftreport_test.go b/cmd/artemis/driftreport_test.go index 5bb729a..8176fc4 100644 --- a/cmd/artemis/driftreport_test.go +++ b/cmd/artemis/driftreport_test.go @@ -119,7 +119,7 @@ func (r fakeDirnameReader) KnownSiteDirnames(context.Context) ([]sitekey.Dirname return r.sites, nil } -type fakeRegistryReader struct{ slugs []string } +type fakeRegistryReader struct{ slugs []sitekey.Slug } func (r fakeRegistryReader) Sites(context.Context) ([]registry.Site, error) { out := make([]registry.Site, 0, len(r.slugs)) @@ -137,7 +137,7 @@ func TestDriftReportSites_CoversSitesTheSchedulerCannotSee(t *testing.T) { sites, err := driftReportSites(context.Background(), fakeDirnameReader{sites: []sitekey.Dirname{"orphan.freecode.camp", "www.freecode.camp"}}, - fakeRegistryReader{slugs: []string{"www", "quiet"}}, + fakeRegistryReader{slugs: []sitekey.Slug{"www", "quiet"}}, tmpl) require.NoError(t, err) diff --git a/cmd/artemis/gcworkflows.go b/cmd/artemis/gcworkflows.go index d2e0fdd..ae031bf 100644 --- a/cmd/artemis/gcworkflows.go +++ b/cmd/artemis/gcworkflows.go @@ -178,13 +178,13 @@ func registerGCWorkflows(rt workflowRegistrar, gcw *gcWiring, dryRun bool, sweep return nil } -func storageSiteNames(slugs []string, tmpl handler.DeployPrefixTemplate) []sitekey.Dirname { +func storageSiteNames(slugs []sitekey.Slug, tmpl handler.DeployPrefixTemplate) []sitekey.Dirname { if len(slugs) == 0 { return nil } names := make([]sitekey.Dirname, 0, len(slugs)) for _, s := range slugs { - names = append(names, tmpl.SiteDirname(sitekey.Slug(s))) + names = append(names, tmpl.SiteDirname(s)) } return names } diff --git a/cmd/artemis/reconcile_keyspace_test.go b/cmd/artemis/reconcile_keyspace_test.go index ac8018a..a1da170 100644 --- a/cmd/artemis/reconcile_keyspace_test.go +++ b/cmd/artemis/reconcile_keyspace_test.go @@ -20,12 +20,12 @@ func TestStorageSiteNames_ProduceThePrefixTheWritePathUsed(t *testing.T) { layout, err := newGCLayout(domainFormat, "_trash/") require.NoError(t, err) - slugs := []string{"test", "hello-universe", "flag-frenzy"} + slugs := []sitekey.Slug{"test", "hello-universe", "flag-frenzy"} names := storageSiteNames(slugs, tmpl) require.Len(t, names, len(slugs)) for i, slug := range slugs { - require.Equal(t, tmpl.SitePrefix(sitekey.Slug(slug)), layout.sitePrefix(names[i]), + require.Equal(t, tmpl.SitePrefix(slug), layout.sitePrefix(names[i]), "slug %q: reconcile would list a prefix no deploy is stored under", slug) } } @@ -38,7 +38,7 @@ func TestStorageSiteNames_MatchTheOutboxSiteChangedForm(t *testing.T) { require.Equal(t, []sitekey.Dirname{tmpl.SiteDirname("test")}, - storageSiteNames([]string{"test"}, tmpl)) + storageSiteNames([]sitekey.Slug{"test"}, tmpl)) } func TestStorageSiteNames_EmptyRegistryYieldsNoNames(t *testing.T) { @@ -57,7 +57,7 @@ func TestStorageSiteNames_BareFormatIsIdentity(t *testing.T) { layout, err := newGCLayout("/deploys/-/", "_trash/") require.NoError(t, err) - names := storageSiteNames([]string{"test", "www"}, tmpl) + names := storageSiteNames([]sitekey.Slug{"test", "www"}, tmpl) require.Equal(t, []sitekey.Dirname{"test", "www"}, names) require.Equal(t, tmpl.SitePrefix("test"), layout.sitePrefix(names[0])) } diff --git a/cmd/loadgen/main.go b/cmd/loadgen/main.go index c26fa70..0ff3b0d 100644 --- a/cmd/loadgen/main.go +++ b/cmd/loadgen/main.go @@ -15,6 +15,7 @@ import ( "github.com/freeCodeCamp/artemis/internal/gc" "github.com/freeCodeCamp/artemis/internal/pg" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/freeCodeCamp/artemis/internal/worker" ) @@ -119,7 +120,7 @@ func runDeploys(ctx context.Context, cfg config, repo *pg.Repo) stageResult { total := cfg.sites * cfg.deploysPerSite base := time.Now().Add(-90 * 24 * time.Hour) return drive("deploy_upsert", total, cfg.concurrency, func(i int) error { - site := siteSlug(i % cfg.sites) + site := siteDirname(i % cfg.sites) seq := i / cfg.sites id := fmt.Sprintf("%d-%08x", base.Add(time.Duration(seq)*time.Hour).Unix(), i) return repo.UpsertDeploy(ctx, site, id, base.Add(time.Duration(seq)*time.Hour), 1<<20, true, "active") @@ -128,7 +129,7 @@ func runDeploys(ctx context.Context, cfg config, repo *pg.Repo) stageResult { func runOutboxEnqueue(ctx context.Context, cfg config, repo *pg.Repo) stageResult { return drive("outbox_enqueue", cfg.sites, cfg.concurrency, func(i int) error { - return repo.EnqueueSiteChanged(ctx, siteSlug(i)) + return repo.EnqueueSiteChanged(ctx, siteDirname(i)) }) } @@ -160,12 +161,12 @@ func runGCPlan(ctx context.Context, cfg config, repo *pg.Repo) stageResult { Mover: nopMover{}, Policy: gc.Policy{RecentKeep: 10, Grace: 24 * time.Hour, Retention: 30 * 24 * time.Hour, ServeCacheTTL: time.Hour}, BlastCap: 1000, - DeployPrefix: func(site, id string) string { return site + "/deploys/" + id + "/" }, - TrashPrefix: func(site, id string) string { return "_trash/" + site + "/" + id + "/" }, + DeployPrefix: func(site sitekey.Dirname, id string) string { return string(site) + "/deploys/" + id + "/" }, + TrashPrefix: func(site sitekey.Dirname, id string) string { return "_trash/" + string(site) + "/" + id + "/" }, Now: time.Now, } return drive("gc_plan_dryrun", cfg.sites, cfg.concurrency, func(i int) error { - _, err := g.Run(ctx, siteSlug(i), true) + _, err := g.Run(ctx, siteDirname(i), true) return err }) } @@ -236,7 +237,11 @@ func truncate(ctx context.Context, db *pg.DB) error { return err } -func siteSlug(i int) string { return fmt.Sprintf("loadgen-site-%06d.freecode.camp", i) } +func siteSlug(i int) sitekey.Slug { return sitekey.Slug(fmt.Sprintf("loadgen-site-%06d", i)) } + +func siteDirname(i int) sitekey.Dirname { + return sitekey.Dirname(string(siteSlug(i)) + ".freecode.camp") +} type nopPublisher struct{} diff --git a/docs/design/0003-postgres-durability.md b/docs/design/0003-postgres-durability.md index 04ad600..1c8c331 100644 --- a/docs/design/0003-postgres-durability.md +++ b/docs/design/0003-postgres-durability.md @@ -21,7 +21,7 @@ Loss is NOT a serving outage: the serve plane (Caddy `r2_alias` → R2) never to | Disk loss on the node | no — same as node loss | | R2 bucket loss (backup target) | out of scope here — R2 is the platform's own durability domain | -RPO today: up to 24 h. RTO today: manual — new PVC + `psql < dump` + repoint; unrehearsed (unverified — no restore drill is recorded anywhere in this repo or the infra runbooks). +RPO today: up to 24 h. RTO today: manual — new PVC + `psql < dump` + repoint — against a stated floor of \<= 60 min. The restore leg **is** rehearsed: `infra:docs/runbooks/08-artemis-pg-restore-drill.md` records the R8 drill PASSED on 2026-06-05, restoring the newest R2 dump into a scratch Postgres with both tenants back and 6/6 artemis tables present. What is not rehearsed is the StatefulSet re-provision that precedes it; that is the remaining wall-time inside the 60 min (runbook 08 §F). ## 3. Options scored diff --git a/docs/design/0005-drift-at-source.md b/docs/design/0005-drift-at-source.md index 1a79b2b..e1ba32c 100644 --- a/docs/design/0005-drift-at-source.md +++ b/docs/design/0005-drift-at-source.md @@ -193,7 +193,7 @@ Everything verified during this audit, with a decision against each. "Accept, do | 8 | Postgres stores dirnames, registry stores slugs | **Out of scope** — migration; P1's types make it survivable | | 9 | `runDriftReport` is called with no argv (`cmd/artemis/main.go:49`) so `driftreport ` silently ignores it; and any unrecognised subcommand falls through to `run()` (`:62`), i.e. **a mistyped subcommand starts the server** | **P0-adjacent** — operators started running these subcommands against production *this week*. A typo that boots a server, and a report that ignores the argument an operator typed, are both how a run gets misread as authoritative. Fix with P0: reject unknown subcommands, reject unexpected args. | | 10 | A finalized deploy remains writable for the JWT's remaining TTL (up to 15 min) | **Accept, documented.** Real, but requires an authorized token holder; tightening it means invalidating the JWT at finalize, which is its own design. Record in ONBOARDING traps. | -| 11 | `outbox` has no retention — unbounded growth | **Backlog.** Small table, slow growth, no correctness impact. Needs a purge job eventually; not part of this wave. | +| 11 | `outbox` has no retention — unbounded growth | **Shipped 1.9.0.** 30-day window on the nightly tombstone-purge; first run 2026-08-20 retired 285 rows. See Open decisions. | | 12 | Dead worker code paths | **Backlog**, cosmetic. | | 13 | `RequireScope` / latched rate limiter behaviours | **Accept, documented.** Both behave as designed; the surprise is documentation, not code. Already captured in ONBOARDING §10. | | 16 | `PlanSite` appended `in.Expired` (mtime ASC, `internal/pg/pending.go:29`) onto `Retain`'s output (mtime DESC, `internal/gc/retain.go:33-37`) without re-sorting, while the blast cap truncates from the tail (`internal/gc/plan.go`). Over-cap runs therefore reaped the **newest** abandoned sessions and starved retention entirely, while the reason string claimed "reaping oldest". Introduced by this sprint; found by the adversarial review, which reproduced it with a probe. | **Fixed** — merged set sorted newest-first before the cap; the test now asserts *which* deploys survive, not how many. | @@ -228,14 +228,24 @@ Everything below waits on an operator call. The evidence is cited so the decisio HTTP writers record the registry slug; the GC writers record the storage dirname (`cmd/artemis/gcwire.go:49-77` pass through the site value gc hands them, which is a dirname). The only reader that joins on the column, `DeployActors` (`internal/pg/audit.go:91`), receives the URL slug (`internal/handler/site.go:311`) — so **slug is the correct keyspace** and the GC writers are the ones to fix. No backfill of existing rows: `0006_audit_log.sql` installs BEFORE UPDATE/DELETE/TRUNCATE triggers that raise, so the table is append-only by design and rewriting history means dropping triggers on production. Recommended: convert the GC writers to slugs and record the cutover date here. -### outbox retention +### outbox retention — CLOSED 2026-08-20 -`Enqueue` only inserts (`internal/pg/outbox.go:31`); published rows are never deleted, so the table grows without bound. Small and slow, no correctness impact. Needs a retention-window decision; the purge can ride the nightly tombstone-purge workflow once a window is chosen. +Was: `Enqueue` only inserts (`internal/pg/outbox.go:31`); published rows were never deleted, so the table grew without bound. + +Shipped in 1.9.0. The window is 30 days and the purge rides the nightly tombstone-purge workflow, as proposed. First production run 2026-08-20T03:00:00Z: `outbox.purged rows=285 before=2026-07-21T03:00:00Z dryRun=false`, taking the table from 344 rows to 59. Zero unpublished rows were touched, before or after. No `outbox.purge.capped` line — 285 is well under the 5000 batch limit. ### Slug/Dirname type split (P1, still deferred) Its own wave. First deliverable is the compiler-produced coercion-site list — change the type, read every resulting error — BEFORE any behaviour change, so the refactor is provably zero-runtime-effect. +**Wave 2 outcome (2026-08-19).** `sitekey.Slug` now runs from every ingress to the render boundary: URL params (`chi.URLParam`), request/response bodies (`SiteRegisterRequest.Slug`, `SiteRow.Slug`), the deploy-session JWT claim (`auth.DeploySessionClaims.Site`), the registry contract (`registry.Site.Slug`, `Writer`, `Snapshot`) and both backends (`internal/pg/registry.go`, `internal/registry/valkey`). Wire and storage bytes are unchanged: the valkey store converts to `string` at every command argument, and `TestStore_Subscribe_DeliversInOrder` (`internal/registry/valkey/store_test.go:395`) still asserts a plain-`string` pub-sub payload. Signatures are pinned against silent reversion in `internal/auth/sitekey_pin_test.go`, `internal/handler/sitekey_pin_test.go` and `internal/registry/sitekey_pin_test.go` — method expressions and field selectors, not string literals, because Go's untyped-constant rule means a literal-only pin proves nothing. + +The split **stops at the audit boundary on purpose.** `pg.AuditEvent.Site`, `pg.AuditFilter.Site` and `telemetry.Scope.SetResource` keep plain `string`, because `auditSite` (`cmd/artemis/gcwire.go:56`) falls back to writing the dirname with a `site_unmapped` detail flag when a dirname renders from no slug. Typing that column `Slug` would assert a guarantee the fallback path breaks. The audit boundary converts explicitly with `string(site)` until the keyspace decision above lands. + ### Orphan reclaim (operator run, time-sensitive) -The live drift report against production proposed 37 repairs (32 failed-upload prefixes, 5 lost index rows) across 9 sites. `drift.reclaimable` alerts at threshold 25, so the first nightly sweep after 1.8.0 deploys will fire until the backlog is reclaimed: `artemis reconcile --apply` per site, and the blast cap of 10 means any site holding more than 10 items needs repeat runs. +The prediction held. The first nightly sweep on 1.9.0 fired: 2026-08-20T04:01:20Z, Sentry issue `ARTEMIS-D`, tag `op=drift.reclaimable`, naming all nine sites. Its check-in stayed green, which is the designed shape — a reclaimable verdict leaves `Fails` unset. + +The count is **35**, not the 37 first reported: 5 reindex + 30 tombstone, re-probed twice on 2026-08-20 (`/app/artemis driftreport` at 03:26Z and the cron sweep at 04:00Z, agreeing exactly). Per site, reindex + tombstone: `the-story-of-the-moon` 0+9, `teleprompter` 4+4, `latex` 0+6, `omotenashi-training` 0+4, `palette-contrast-checker` 1+1, `svg-draw` 0+2, `wave-forge` 0+2, `hexnova` 0+1, `plumb-select` 0+1. + +Still unrun. Every site is under the blast cap of 10, so one `artemis reconcile --apply` each is enough — no repeat runs. The alert re-fires nightly until the backlog is reclaimed. diff --git a/internal/auth/jwt.go b/internal/auth/jwt.go index c89f33e..097332b 100644 --- a/internal/auth/jwt.go +++ b/internal/auth/jwt.go @@ -16,6 +16,8 @@ import ( "time" "github.com/golang-jwt/jwt/v5" + + "github.com/freeCodeCamp/artemis/internal/sitekey" ) const ( @@ -32,8 +34,8 @@ const ( // had outer Login/Issuer fields that shadowed the embedded ones at // marshal time and silently dropped the embedded values on the wire. type DeploySessionClaims struct { - Site string `json:"site"` - DeployID string `json:"deployId"` + Site sitekey.Slug `json:"site"` + DeployID string `json:"deployId"` jwt.RegisteredClaims } @@ -71,7 +73,7 @@ func NewDeploySessionSigner(secret string, ttl time.Duration) (*DeploySessionSig // Sign issues a JWT scoped to (login, site, deployId). Returns the token // string and its absolute expiry time. -func (s *DeploySessionSigner) Sign(login, site, deployID string) (string, time.Time, error) { +func (s *DeploySessionSigner) Sign(login string, site sitekey.Slug, deployID string) (string, time.Time, error) { now := time.Now() exp := now.Add(s.ttl) claims := DeploySessionClaims{ diff --git a/internal/auth/jwt_test.go b/internal/auth/jwt_test.go index abb9eb9..2911107 100644 --- a/internal/auth/jwt_test.go +++ b/internal/auth/jwt_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -31,7 +32,7 @@ func TestSignAndVerify_Roundtrip(t *testing.T) { claims, err := s.Verify(tok) require.NoError(t, err) assert.Equal(t, "alice", claims.Subject) - assert.Equal(t, "www", claims.Site) + assert.Equal(t, sitekey.Slug("www"), claims.Site) assert.Equal(t, "20260420-141522-abc1234", claims.DeployID) assert.Equal(t, "artemis", claims.Issuer) } diff --git a/internal/auth/sitekey_pin_test.go b/internal/auth/sitekey_pin_test.go new file mode 100644 index 0000000..42790b5 --- /dev/null +++ b/internal/auth/sitekey_pin_test.go @@ -0,0 +1,34 @@ +package auth + +import ( + "encoding/base64" + "encoding/json" + "strings" + "testing" + "time" + + "github.com/freeCodeCamp/artemis/internal/sitekey" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var ( + _ func(*DeploySessionSigner, string, sitekey.Slug, string) (string, time.Time, error) = (*DeploySessionSigner).Sign + _ sitekey.Slug = DeploySessionClaims{}.Site +) + +func TestSign_SiteWireEncodingIsPlainString(t *testing.T) { + s := newSigner(t) + + tok, _, err := s.Sign("alice", sitekey.Slug("www"), "d-1") + require.NoError(t, err) + + parts := strings.Split(tok, ".") + require.Len(t, parts, 3) + raw, err := base64.RawURLEncoding.DecodeString(parts[1]) + require.NoError(t, err) + + var payload map[string]any + require.NoError(t, json.Unmarshal(raw, &payload)) + assert.Equal(t, "www", payload["site"]) +} diff --git a/internal/config/config_repo_test.go b/internal/config/config_repo_test.go index cd5fff5..5de3529 100644 --- a/internal/config/config_repo_test.go +++ b/internal/config/config_repo_test.go @@ -1,6 +1,7 @@ package config import ( + "github.com/freeCodeCamp/artemis/internal/config/configtest" "testing" "github.com/stretchr/testify/assert" @@ -8,9 +9,7 @@ import ( ) func TestLoad_RepoDefaults(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) cfg, err := Load() require.NoError(t, err) @@ -22,9 +21,7 @@ func TestLoad_RepoDefaults(t *testing.T) { } func TestLoad_RepoOverridesAndAppCreds(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("GH_REPO_ORG", "ExampleUniverse") t.Setenv("REPO_CREATE_AUTHZ_TEAM", "contributors") t.Setenv("REPO_APPROVE_AUTHZ_TEAM", "maintainers") @@ -44,9 +41,7 @@ func TestLoad_RepoOverridesAndAppCreds(t *testing.T) { } func TestLoad_RepoPartialAppConfigFails(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) // App id set but installation id + key missing → partial → error. t.Setenv("GH_APP_ID", "123456") @@ -59,9 +54,7 @@ func TestLoad_RepoEmptyTeamOverrideFails(t *testing.T) { // An explicit empty override is ignored (defaults retained), so the // guard against empty teams only trips on a programmatic zero value; // assert the happy default holds when the env var is blank. - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("REPO_APPROVE_AUTHZ_TEAM", "") cfg, err := Load() diff --git a/internal/config/config_sentry_test.go b/internal/config/config_sentry_test.go index e481ff6..7fd183d 100644 --- a/internal/config/config_sentry_test.go +++ b/internal/config/config_sentry_test.go @@ -1,27 +1,23 @@ package config_test import ( - "os" "testing" "github.com/freeCodeCamp/artemis/internal/config" + "github.com/freeCodeCamp/artemis/internal/config/configtest" "github.com/stretchr/testify/require" ) -// setRequiredForSentry sets the minimum required env for Load() to -// succeed and clears every SENTRY_* var, so each test starts from a -// known baseline regardless of the developer's shell. func setRequiredForSentry(t *testing.T) { t.Helper() - for _, k := range []string{"SENTRY_DSN", "ENVIRONMENT", "SENTRY_TRACES_SAMPLE_RATE", "SENTRY_DEBUG"} { - _ = os.Unsetenv(k) - } - t.Setenv("R2_ENDPOINT", "https://acct.r2.cloudflarestorage.com") - t.Setenv("R2_ACCESS_KEY_ID", "ak") - t.Setenv("R2_SECRET_ACCESS_KEY", "sk") - t.Setenv("GH_CLIENT_ID", "cid") - t.Setenv("JWT_SIGNING_KEY", "0123456789abcdef0123456789abcdef") - t.Setenv("VALKEY_ADDR", "localhost:6379") + configtest.Hermetic(t, config.EnvKeys(), map[string]string{ + "R2_ENDPOINT": "https://acct.r2.cloudflarestorage.com", + "R2_ACCESS_KEY_ID": "ak", + "R2_SECRET_ACCESS_KEY": "sk", + "GH_CLIENT_ID": "cid", + "JWT_SIGNING_KEY": "0123456789abcdef0123456789abcdef", + "VALKEY_ADDR": "localhost:6379", + }) } func TestLoad_SentryDefaultsOff(t *testing.T) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 0078e4e..83fc901 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -2,6 +2,7 @@ package config import ( "bytes" + "github.com/freeCodeCamp/artemis/internal/config/configtest" "log/slog" "os" "strings" @@ -25,9 +26,7 @@ func requiredEnv() map[string]string { } func TestLoad_AllDefaults(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) cfg, err := Load() require.NoError(t, err) @@ -75,9 +74,7 @@ func TestLoad_ValkeyConnectRetryWindow(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("VALKEY_CONNECT_RETRY_WINDOW", tc.value) cfg, err := Load() @@ -106,9 +103,7 @@ func TestLoad_PGConnectRetryWindow(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("PG_CONNECT_RETRY_WINDOW", tc.value) cfg, err := Load() @@ -134,9 +129,7 @@ func TestLoad_GitHubAPIBaseValidation(t *testing.T) { } for _, base := range valid { t.Run("valid/"+base, func(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("GH_API_BASE", base) _, err := Load() require.NoError(t, err) @@ -153,9 +146,7 @@ func TestLoad_GitHubAPIBaseValidation(t *testing.T) { } for _, base := range invalid { t.Run("invalid/"+base, func(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("GH_API_BASE", base) _, err := Load() require.Error(t, err, "GH_API_BASE %q must be rejected", base) @@ -164,9 +155,7 @@ func TestLoad_GitHubAPIBaseValidation(t *testing.T) { } func TestLoad_OverridesViaEnv(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("PORT", "9090") t.Setenv("R2_BUCKET", "test-bucket") t.Setenv("GH_ORG", "ExampleOrg") @@ -200,9 +189,7 @@ func TestLoad_OverridesViaEnv(t *testing.T) { } func TestConfigLoad(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) cfg, err := Load() require.NoError(t, err) @@ -218,9 +205,7 @@ func TestConfigLoad(t *testing.T) { } func TestConfigLoad_Overrides(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("DATABASE_URL", "postgres://artemis@pg/artemis") t.Setenv("HATCHET_CLIENT_TOKEN", "ht-token") t.Setenv("HATCHET_ADDR", "hatchet.svc:7077") @@ -249,9 +234,7 @@ func TestConfigLoad_Overrides(t *testing.T) { } func TestConfigLoad_GraceBelowJWTTTLFails(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("JWT_TTL_SECONDS", "3600") t.Setenv("CLEANUP_GRACE", "30m") @@ -261,9 +244,7 @@ func TestConfigLoad_GraceBelowJWTTTLFails(t *testing.T) { } func TestConfigLoad_GraceBelowServeCacheTTLFails(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("JWT_TTL_SECONDS", "5") t.Setenv("CLEANUP_GRACE", "10s") @@ -275,9 +256,7 @@ func TestConfigLoad_GraceBelowServeCacheTTLFails(t *testing.T) { func TestLoad_UploadMaxBytes_RejectsNonPositive(t *testing.T) { for _, bad := range []string{"0", "-1", "not-a-number", ""} { t.Run("v="+bad, func(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("UPLOAD_MAX_BYTES", bad) _, err := Load() require.Error(t, err) @@ -297,9 +276,7 @@ func TestLoad_MissingRequiredFails(t *testing.T) { } for _, omitted := range cases { t.Run("missing "+omitted, func(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) require.NoError(t, os.Unsetenv(omitted)) _, err := Load() require.Error(t, err) @@ -309,9 +286,7 @@ func TestLoad_MissingRequiredFails(t *testing.T) { } func TestLoad_RejectsInvalidNumeric(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("PORT", "not-a-port") _, err := Load() require.Error(t, err) @@ -319,9 +294,7 @@ func TestLoad_RejectsInvalidNumeric(t *testing.T) { } func TestLoad_RejectsShortSigningKey(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("JWT_SIGNING_KEY", "tooshort") _, err := Load() require.Error(t, err) @@ -329,9 +302,7 @@ func TestLoad_RejectsShortSigningKey(t *testing.T) { } func TestLoad_LogLevelValidation(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("LOG_LEVEL", "absurd") _, err := Load() require.Error(t, err) @@ -355,9 +326,7 @@ func TestLoad_RejectsMalformedDeployPrefix(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("DEPLOY_PREFIX_FORMAT", tc.fmt) _, err := Load() require.Error(t, err) @@ -369,9 +338,7 @@ func TestLoad_RejectsMalformedDeployPrefix(t *testing.T) { } func TestLoad_AcceptsValidDeployPrefix(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("DEPLOY_PREFIX_FORMAT", "/custom/-/sub/") cfg, err := Load() require.NoError(t, err) @@ -379,9 +346,7 @@ func TestLoad_AcceptsValidDeployPrefix(t *testing.T) { } func TestLoad_RegistryAuthzTeamRejectsWhitespace(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("REGISTRY_AUTHZ_TEAM", " ") _, err := Load() require.Error(t, err) @@ -389,9 +354,7 @@ func TestLoad_RegistryAuthzTeamRejectsWhitespace(t *testing.T) { } func TestValidate_RegistryAuthzTeamRejectsBlank(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) cfg, err := Load() require.NoError(t, err) require.Equal(t, "staff", cfg.Registry.AuthzTeam) @@ -418,9 +381,7 @@ func captureSlog(t *testing.T) *bytes.Buffer { // refactor could fire the warn unconditionally and bury real overrides // in the noise. func TestLoad_GHAPIBaseDefaultNoWarn(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) logs := captureSlog(t) _, err := Load() require.NoError(t, err) @@ -432,9 +393,7 @@ func TestLoad_GHAPIBaseDefaultNoWarn(t *testing.T) { // the canonical default. The warn is the operator's only visible // signal that GitHub probes are routing through a non-canonical host. func TestLoad_GHAPIBaseOverrideWarn(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) const override = "https://evil.example.com" t.Setenv("GH_API_BASE", override) @@ -452,9 +411,7 @@ func TestLoad_GHAPIBaseOverrideWarn(t *testing.T) { } func TestLoad_RejectsNonNumericAppIDs(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("GH_APP_ID", "3.287718e+06") t.Setenv("GH_APP_INSTALLATION_ID", "121700722") t.Setenv("GH_APP_PRIVATE_KEY", "-----BEGIN RSA PRIVATE KEY-----\nx\n-----END RSA PRIVATE KEY-----") @@ -465,9 +422,7 @@ func TestLoad_RejectsNonNumericAppIDs(t *testing.T) { } func TestLoad_RejectsNonNumericInstallationID(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("GH_APP_ID", "3287718") t.Setenv("GH_APP_INSTALLATION_ID", "1.21700722e+08") t.Setenv("GH_APP_PRIVATE_KEY", "-----BEGIN RSA PRIVATE KEY-----\nx\n-----END RSA PRIVATE KEY-----") @@ -478,9 +433,7 @@ func TestLoad_RejectsNonNumericInstallationID(t *testing.T) { } func TestLoad_AcceptsNumericAppIDs(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) t.Setenv("GH_APP_ID", "3287718") t.Setenv("GH_APP_INSTALLATION_ID", "121700722") t.Setenv("GH_APP_PRIVATE_KEY", "-----BEGIN RSA PRIVATE KEY-----\nx\n-----END RSA PRIVATE KEY-----") @@ -492,9 +445,7 @@ func TestLoad_AcceptsNumericAppIDs(t *testing.T) { } func TestLoad_SeedsTheOutboxRetentionDefault(t *testing.T) { - for k, v := range requiredEnv() { - t.Setenv(k, v) - } + configtest.Hermetic(t, EnvKeys(), requiredEnv()) cfg, err := Load() require.NoError(t, err) assert.Equal(t, 30, cfg.Cleanup.OutboxRetentionDays, diff --git a/internal/config/configtest/configtest.go b/internal/config/configtest/configtest.go new file mode 100644 index 0000000..51a289d --- /dev/null +++ b/internal/config/configtest/configtest.go @@ -0,0 +1,36 @@ +package configtest + +import ( + "os" + "slices" + "testing" + + "github.com/stretchr/testify/require" +) + +func UnreadableKeys(known []string, want map[string]string) []string { + var bad []string + for k := range want { + if !slices.Contains(known, k) { + bad = append(bad, k) + } + } + slices.Sort(bad) + return bad +} + +func Hermetic(t *testing.T, known []string, want map[string]string) { + t.Helper() + require.Empty(t, UnreadableKeys(known, want), + "config.Load never reads these, so setting them asserts nothing") + for _, k := range known { + if _, present := os.LookupEnv(k); !present { + continue + } + t.Setenv(k, "") + require.NoError(t, os.Unsetenv(k)) + } + for k, v := range want { + t.Setenv(k, v) + } +} diff --git a/internal/config/configtest/configtest_test.go b/internal/config/configtest/configtest_test.go new file mode 100644 index 0000000..8d9f66d --- /dev/null +++ b/internal/config/configtest/configtest_test.go @@ -0,0 +1,80 @@ +package configtest_test + +import ( + "os" + "testing" + + "github.com/freeCodeCamp/artemis/internal/config" + "github.com/freeCodeCamp/artemis/internal/config/configtest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func requiredEnv() map[string]string { + return map[string]string{ + "R2_ENDPOINT": "https://acct.r2.cloudflarestorage.com", + "R2_ACCESS_KEY_ID": "ak", + "R2_SECRET_ACCESS_KEY": "sk", + "GH_CLIENT_ID": "Iv1.deadbeef", + "JWT_SIGNING_KEY": "0123456789abcdef0123456789abcdef", + "VALKEY_ADDR": "valkey.artemis.svc:6379", + } +} + +func TestHermetic_LeavesUndeclaredVariablesAbsentNotEmpty(t *testing.T) { + t.Setenv("PORT", "9999") + + configtest.Hermetic(t, config.EnvKeys(), requiredEnv()) + + _, present := os.LookupEnv("PORT") + require.False(t, present, + "PORT is read without an empty-string guard, so an empty value would reach strconv and fail Load") + + cfg, err := config.Load() + require.NoError(t, err) + assert.Equal(t, 8080, cfg.Port, "an undeclared variable must fall back to the default") +} + +func TestHermetic_OverridesAnAmbientValueItDeclares(t *testing.T) { + t.Setenv("R2_BUCKET", "leaked-from-the-shell") + + want := requiredEnv() + want["R2_BUCKET"] = "declared-by-the-test" + configtest.Hermetic(t, config.EnvKeys(), want) + + cfg, err := config.Load() + require.NoError(t, err) + assert.Equal(t, "declared-by-the-test", cfg.R2.Bucket) +} + +func TestHermetic_RestoresEveryClearedVariableWhenTheTestEnds(t *testing.T) { + const ambient = "ambient-value-set-outside" + t.Setenv("R2_BUCKET", ambient) + + t.Run("inner", func(t *testing.T) { + configtest.Hermetic(t, config.EnvKeys(), requiredEnv()) + _, present := os.LookupEnv("R2_BUCKET") + require.False(t, present, "the inner test must not see the ambient value") + }) + + assert.Equal(t, ambient, os.Getenv("R2_BUCKET"), + "clearing must be scoped to the test; leaking it breaks every later test in the binary") +} + +func TestUnreadableKeys_NamesAKeyLoadNeverReads(t *testing.T) { + want := requiredEnv() + want["DEPLOY_PREFIX_FORMATT"] = "typo" + + assert.Equal(t, []string{"DEPLOY_PREFIX_FORMATT"}, + configtest.UnreadableKeys(config.EnvKeys(), want), + "a typo sets a variable nobody reads, so the test passes while asserting nothing") +} + +func TestUnreadableKeys_AcceptsEveryKeyLoadReads(t *testing.T) { + all := map[string]string{} + for _, k := range config.EnvKeys() { + all[k] = "x" + } + + assert.Empty(t, configtest.UnreadableKeys(config.EnvKeys(), all)) +} diff --git a/internal/config/envkeys.go b/internal/config/envkeys.go new file mode 100644 index 0000000..33ef1a6 --- /dev/null +++ b/internal/config/envkeys.go @@ -0,0 +1,54 @@ +package config + +import "slices" + +var envKeys = []string{ + "ALIAS_PREVIEW_KEY_FORMAT", + "ALIAS_PRODUCTION_KEY_FORMAT", + "AUDIT_READ_AUTHZ_TEAM", + "BACKFILL_ON_BOOT", + "CLEANUP_BLAST_CAP", + "CLEANUP_DRY_RUN", + "CLEANUP_GRACE", + "CLEANUP_OUTBOX_RETENTION_DAYS", + "CLEANUP_RECENT_KEEP", + "CLEANUP_RECOVERY_DAYS", + "CLEANUP_RETENTION_DAYS", + "CLEANUP_TRASH_PREFIX", + "DATABASE_URL", + "DEPLOY_PREFIX_FORMAT", + "ENVIRONMENT", + "GH_API_BASE", + "GH_APP_ID", + "GH_APP_INSTALLATION_ID", + "GH_APP_PRIVATE_KEY", + "GH_CLIENT_ID", + "GH_MEMBERSHIP_CACHE_TTL", + "GH_ORG", + "GH_REPO_ORG", + "HATCHET_ADDR", + "HATCHET_CLIENT_TOKEN", + "JWT_SIGNING_KEY", + "JWT_TTL_SECONDS", + "LOG_LEVEL", + "PG_CONNECT_RETRY_WINDOW", + "PORT", + "PUBLIC_URL_PREVIEW_FORMAT", + "PUBLIC_URL_PRODUCTION_FORMAT", + "R2_ACCESS_KEY_ID", + "R2_BUCKET", + "R2_ENDPOINT", + "R2_SECRET_ACCESS_KEY", + "REGISTRY_AUTHZ_TEAM", + "REPO_APPROVE_AUTHZ_TEAM", + "REPO_CREATE_AUTHZ_TEAM", + "SENTRY_DEBUG", + "SENTRY_DSN", + "SENTRY_TRACES_SAMPLE_RATE", + "UPLOAD_MAX_BYTES", + "VALKEY_ADDR", + "VALKEY_CONNECT_RETRY_WINDOW", + "VALKEY_PASSWORD", +} + +func EnvKeys() []string { return slices.Clone(envKeys) } diff --git a/internal/config/envkeys_test.go b/internal/config/envkeys_test.go new file mode 100644 index 0000000..93227a3 --- /dev/null +++ b/internal/config/envkeys_test.go @@ -0,0 +1,103 @@ +package config + +import ( + "go/ast" + "go/parser" + "go/token" + "io/fs" + "sort" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func readsEnv(name string) bool { + return name == "LookupEnv" || name == "Getenv" || strings.HasPrefix(name, "getEnv") +} + +func calleeName(fn ast.Expr) string { + switch f := fn.(type) { + case *ast.Ident: + return f.Name + case *ast.SelectorExpr: + return f.Sel.Name + } + return "" +} + +func envKeysReadInPackageSource(t *testing.T) map[string]bool { + t.Helper() + fset := token.NewFileSet() + pkgs, err := parser.ParseDir(fset, ".", func(fi fs.FileInfo) bool { + return !strings.HasSuffix(fi.Name(), "_test.go") + }, 0) + require.NoError(t, err) + + read := map[string]bool{} + files := 0 + for _, pkg := range pkgs { + for range pkg.Files { + files++ + } + ast.Inspect(pkg, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok || !readsEnv(calleeName(call.Fun)) { + return true + } + for _, arg := range call.Args { + lit, ok := arg.(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + continue + } + if v, err := strconv.Unquote(lit.Value); err == nil && v != "" { + read[v] = true + } + } + return true + }) + } + require.NotZero(t, files, "parsed no package source at all") + require.NotEmpty(t, read, "found no environment reads, so the package changed shape") + return read +} + +func TestEnvKeys_ListsExactlyTheVariablesThePackageReads(t *testing.T) { + listed := map[string]bool{} + for _, k := range EnvKeys() { + listed[k] = true + } + + assert.Equal(t, sortedKeys(envKeysReadInPackageSource(t)), sortedKeys(listed), + "a stale list leaves the newest variable leaking in from the developer shell, "+ + "which is the failure it exists to prevent; every non-test file in the package is parsed "+ + "and every string argument to an env-reading call is collected, so neither a sibling file "+ + "nor a multi-argument helper can hide a key") +} + +func TestEnvKeys_HasNoDuplicates(t *testing.T) { + seen := map[string]bool{} + for _, k := range EnvKeys() { + assert.False(t, seen[k], "%s is listed twice", k) + seen[k] = true + } +} + +func TestEnvKeys_ReturnsACopyCallersCannotCorrupt(t *testing.T) { + first := EnvKeys() + require.NotEmpty(t, first) + first[0] = "MUTATED_BY_A_CALLER" + + assert.NotEqual(t, "MUTATED_BY_A_CALLER", EnvKeys()[0]) +} + +func sortedKeys(m map[string]bool) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return out +} diff --git a/internal/config/publicurl_test.go b/internal/config/publicurl_test.go index 5a0b2b2..86ee442 100644 --- a/internal/config/publicurl_test.go +++ b/internal/config/publicurl_test.go @@ -1,18 +1,25 @@ package config import ( + "github.com/freeCodeCamp/artemis/internal/config/configtest" "testing" "github.com/stretchr/testify/require" ) +func publicURLEnv() map[string]string { + return map[string]string{ + "GH_CLIENT_ID": "cid", + "JWT_SIGNING_KEY": "0123456789abcdef0123456789abcdef", + "R2_ENDPOINT": "http://127.0.0.1:1", + "R2_ACCESS_KEY_ID": "k", + "R2_SECRET_ACCESS_KEY": "s", + "VALKEY_ADDR": "127.0.0.1:6379", + } +} + func TestLoad_PublicURLFormatsDefaultToTheServedHostShapes(t *testing.T) { - t.Setenv("GH_CLIENT_ID", "cid") - t.Setenv("JWT_SIGNING_KEY", "0123456789abcdef0123456789abcdef") - t.Setenv("R2_ENDPOINT", "http://127.0.0.1:1") - t.Setenv("R2_ACCESS_KEY_ID", "k") - t.Setenv("R2_SECRET_ACCESS_KEY", "s") - t.Setenv("VALKEY_ADDR", "127.0.0.1:6379") + configtest.Hermetic(t, EnvKeys(), publicURLEnv()) c, err := Load() require.NoError(t, err) @@ -21,12 +28,7 @@ func TestLoad_PublicURLFormatsDefaultToTheServedHostShapes(t *testing.T) { } func TestLoad_PublicURLFormatsAreOverridable(t *testing.T) { - t.Setenv("GH_CLIENT_ID", "cid") - t.Setenv("JWT_SIGNING_KEY", "0123456789abcdef0123456789abcdef") - t.Setenv("R2_ENDPOINT", "http://127.0.0.1:1") - t.Setenv("R2_ACCESS_KEY_ID", "k") - t.Setenv("R2_SECRET_ACCESS_KEY", "s") - t.Setenv("VALKEY_ADDR", "127.0.0.1:6379") + configtest.Hermetic(t, EnvKeys(), publicURLEnv()) t.Setenv("PUBLIC_URL_PRODUCTION_FORMAT", "https://.example.test") t.Setenv("PUBLIC_URL_PREVIEW_FORMAT", "https://.pre.example.test") @@ -37,12 +39,7 @@ func TestLoad_PublicURLFormatsAreOverridable(t *testing.T) { } func TestLoad_RejectsAPublicURLFormatWithoutSiteToken(t *testing.T) { - t.Setenv("GH_CLIENT_ID", "cid") - t.Setenv("JWT_SIGNING_KEY", "0123456789abcdef0123456789abcdef") - t.Setenv("R2_ENDPOINT", "http://127.0.0.1:1") - t.Setenv("R2_ACCESS_KEY_ID", "k") - t.Setenv("R2_SECRET_ACCESS_KEY", "s") - t.Setenv("VALKEY_ADDR", "127.0.0.1:6379") + configtest.Hermetic(t, EnvKeys(), publicURLEnv()) t.Setenv("PUBLIC_URL_PRODUCTION_FORMAT", "https://freecode.camp") _, err := Load() @@ -51,12 +48,7 @@ func TestLoad_RejectsAPublicURLFormatWithoutSiteToken(t *testing.T) { } func TestLoad_BlastCapDefaultsToARealCeiling(t *testing.T) { - t.Setenv("GH_CLIENT_ID", "cid") - t.Setenv("JWT_SIGNING_KEY", "0123456789abcdef0123456789abcdef") - t.Setenv("R2_ENDPOINT", "http://127.0.0.1:1") - t.Setenv("R2_ACCESS_KEY_ID", "k") - t.Setenv("R2_SECRET_ACCESS_KEY", "s") - t.Setenv("VALKEY_ADDR", "127.0.0.1:6379") + configtest.Hermetic(t, EnvKeys(), publicURLEnv()) c, err := Load() require.NoError(t, err) diff --git a/internal/handler/accesslog_actor_test.go b/internal/handler/accesslog_actor_test.go index 9c04134..18e37d3 100644 --- a/internal/handler/accesslog_actor_test.go +++ b/internal/handler/accesslog_actor_test.go @@ -8,6 +8,7 @@ import ( "sync" "testing" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/freeCodeCamp/artemis/internal/telemetry" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -151,7 +152,7 @@ func TestAccessLog_GitHubBearer_ActorPopulated(t *testing.T) { cap := captureAccessLog(t) h, _ := newTestHandlers(t, &fakeGH{tokenLogins: map[string]string{"good": "alice"}}, - &fakeSites{bySite: map[string][]string{}}, + &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) final := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) @@ -170,7 +171,7 @@ func TestAccessLog_NoDuplicateKeys(t *testing.T) { cap := captureAccessLog(t) h, _ := newTestHandlers(t, &fakeGH{tokenLogins: map[string]string{"good": "alice"}}, - &fakeSites{bySite: map[string][]string{}}, + &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) final := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) @@ -190,7 +191,7 @@ func TestActionLog_ScopeSuppliesSingleActor(t *testing.T) { cap := captureAccessLog(t) h, _ := newTestHandlers(t, &fakeGH{tokenLogins: map[string]string{"good": "alice"}}, - &fakeSites{bySite: map[string][]string{}}, + &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) final := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -215,7 +216,7 @@ func TestAccessLog_DeployJWT_ActorPopulated(t *testing.T) { cap := captureAccessLog(t) h, jwt := newTestHandlers(t, &fakeGH{}, - &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, + &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, newFakeR2()) tok, _, err := jwt.Sign("alice", "www", "d-1") diff --git a/internal/handler/alias.go b/internal/handler/alias.go index 35157a9..7b5ab1a 100644 --- a/internal/handler/alias.go +++ b/internal/handler/alias.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/freeCodeCamp/artemis/internal/r2" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/go-chi/chi/v5" ) @@ -19,7 +20,7 @@ import ( // unknown mode, 404 when the alias key has not been written yet // (fresh site, never finalized), 502 on R2 transport errors. func (h *Handlers) AliasGet(w http.ResponseWriter, r *http.Request) { - site := chi.URLParam(r, "site") + site := sitekey.Slug(chi.URLParam(r, "site")) mode := strings.ToLower(strings.TrimSpace(chi.URLParam(r, "mode"))) switch mode { diff --git a/internal/handler/audit_wiring_destructive_test.go b/internal/handler/audit_wiring_destructive_test.go index 69c736c..af7c24d 100644 --- a/internal/handler/audit_wiring_destructive_test.go +++ b/internal/handler/audit_wiring_destructive_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/freeCodeCamp/artemis/internal/registry" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -15,7 +16,7 @@ func bearerTok() map[string]string { return map[string]string{"Authorization": " func TestSiteDelete_RecordsExactlyOneAudit(t *testing.T) { h, _ := newTestHandlers(t, staffCallerGH(), - &fakeSites{bySite: map[string][]string{"example": {"team-eng"}}}, newFakeR2()) + &fakeSites{bySite: map[sitekey.Slug][]string{"example": {"team-eng"}}}, newFakeR2()) fa := &fakeAudit{} h.Audit = fa @@ -39,7 +40,7 @@ func TestSitePurge_RecordsExactlyOneAudit(t *testing.T) { store.objects["example/production"] = []byte("20260420-141522-abc1234") h, _ := newTestHandlers(t, staffCallerGH(), - &fakeSites{bySite: map[string][]string{"example": {"team-eng"}}}, store) + &fakeSites{bySite: map[sitekey.Slug][]string{"example": {"team-eng"}}}, store) h.Tombstones = &fakeTombstones{} fa := &fakeAudit{} h.Audit = fa diff --git a/internal/handler/audit_wiring_test.go b/internal/handler/audit_wiring_test.go index 98c0e2c..0b2a078 100644 --- a/internal/handler/audit_wiring_test.go +++ b/internal/handler/audit_wiring_test.go @@ -5,6 +5,7 @@ import ( "net/http" "testing" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -19,7 +20,7 @@ func TestSitePromote_RecordsExactlyOneAudit(t *testing.T) { tokenLogins: map[string]string{"good": "alice"}, userTeams: map[string]map[string]bool{"alice": {"team-a": true}}, }, - &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, + &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, store) h.Audit = fa @@ -44,7 +45,7 @@ func TestSiteRegister_RecordsAuditWithCreatedBy(t *testing.T) { tokenLogins: map[string]string{"good": "alice"}, userTeams: map[string]map[string]bool{"alice": {"staff": true}}, }, - &fakeSites{bySite: map[string][]string{}}, + &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) h.Audit = fa diff --git a/internal/handler/breadcrumb_test.go b/internal/handler/breadcrumb_test.go index a3f951d..8921c8a 100644 --- a/internal/handler/breadcrumb_test.go +++ b/internal/handler/breadcrumb_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/getsentry/sentry-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -21,7 +22,7 @@ func TestSitePromote_AddsBreadcrumbs(t *testing.T) { tokenLogins: map[string]string{"good": "alice"}, userTeams: map[string]map[string]bool{"alice": {"team-a": true}}, }, - &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, + &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, store) ctx := sentry.SetHubOnContext(context.Background(), hub) diff --git a/internal/handler/deploy.go b/internal/handler/deploy.go index 029d457..3f31836 100644 --- a/internal/handler/deploy.go +++ b/internal/handler/deploy.go @@ -22,9 +22,9 @@ import ( // DeployInitRequest is the body of POST /api/deploy/init. type DeployInitRequest struct { - Site string `json:"site"` - SHA string `json:"sha"` - Files []string `json:"files,omitempty"` // optional manifest used by /finalize + Site sitekey.Slug `json:"site"` + SHA string `json:"sha"` + Files []string `json:"files,omitempty"` // optional manifest used by /finalize } // DeployInitResponse is the success payload of /api/deploy/init. @@ -57,7 +57,7 @@ func (h *Handlers) DeployInit(w http.ResponseWriter, r *http.Request) { return } - telemetry.FromContext(r.Context()).SetResource(req.Site, "") + telemetry.FromContext(r.Context()).SetResource(string(req.Site), "") h.logAction(r.Context(), "deploy.init", "start", slog.String("sha", req.SHA)) teams := h.Sites.Snapshot().TeamsForSite(req.Site) @@ -87,8 +87,8 @@ func (h *Handlers) DeployInit(w http.ResponseWriter, r *http.Request) { return } - telemetry.FromContext(r.Context()).SetResource(req.Site, deployID) - h.beginPendingDeploy(r.Context(), h.DeployPrefix.SiteDirname(sitekey.Slug(req.Site)), deployID) + telemetry.FromContext(r.Context()).SetResource(string(req.Site), deployID) + h.beginPendingDeploy(r.Context(), h.DeployPrefix.SiteDirname(req.Site), deployID) h.logAction(r.Context(), "deploy.init", "success") h.auditFromScope(r.Context(), "deploy.init", "success", map[string]any{"sha": req.SHA}) @@ -164,7 +164,7 @@ func (h *Handlers) DeployUpload(w http.ResponseWriter, r *http.Request) { return } - telemetry.FromContext(r.Context()).SetResource(claims.Site, deployID) + telemetry.FromContext(r.Context()).SetResource(string(claims.Site), deployID) h.logAction(r.Context(), "deploy.upload", "success", slog.String("path", relPath), slog.Int64("bytes", contentLength)) h.auditFromScope(r.Context(), "deploy.upload", "success", map[string]any{"path": relPath, "bytes": contentLength}) @@ -244,7 +244,7 @@ func (h *Handlers) DeployFinalize(w http.ResponseWriter, r *http.Request) { markerKey := prefix + gc.MarkerObjectName meta := fmt.Sprintf(`{"site":%q,"deployId":%q,"mode":%q,"finalizedAt":%q}`, - claims.Site, deployID, mode, time.Now().UTC().Format(time.RFC3339)) + string(claims.Site), deployID, mode, time.Now().UTC().Format(time.RFC3339)) if err := telemetry.WithSpan(r.Context(), "r2.put.marker.finalize", func(ctx context.Context) error { return h.R2.PutObject(ctx, markerKey, strings.NewReader(meta), "application/json", int64(len(meta))) }); err != nil { @@ -267,7 +267,7 @@ func (h *Handlers) DeployFinalize(w http.ResponseWriter, r *http.Request) { aliasKey := h.aliasKey(claims.Site, mode) commitCtx, cancelCommit := context.WithTimeout(context.WithoutCancel(r.Context()), aliasCommitTimeout) defer cancelCommit() - lockErr := h.withSiteLock(commitCtx, h.DeployPrefix.SiteDirname(sitekey.Slug(claims.Site)), func() error { + lockErr := h.withSiteLock(commitCtx, h.DeployPrefix.SiteDirname(claims.Site), func() error { telemetry.Breadcrumb(commitCtx, "lock", "site lock acquired") if _, err := h.Registry.GetSite(commitCtx, claims.Site); err != nil { if errors.Is(err, registry.ErrNotFound) { @@ -285,7 +285,7 @@ func (h *Handlers) DeployFinalize(w http.ResponseWriter, r *http.Request) { } if h.Index != nil { if err := telemetry.WithSpan(commitCtx, "pg.finalize.index", func(ctx context.Context) error { - return h.Index.FinalizeAtomic(ctx, h.DeployPrefix.SiteDirname(sitekey.Slug(claims.Site)), deployID, mode, time.Now().UTC(), deployBytes) + return h.Index.FinalizeAtomic(ctx, h.DeployPrefix.SiteDirname(claims.Site), deployID, mode, time.Now().UTC(), deployBytes) }); err != nil { writeUpstreamError(w, r, http.StatusBadGateway, "pg_write_failed", "pg.finalize.index", err) return errAliasWriteHandled @@ -299,7 +299,7 @@ func (h *Handlers) DeployFinalize(w http.ResponseWriter, r *http.Request) { } return } - telemetry.FromContext(r.Context()).SetResource(claims.Site, deployID) + telemetry.FromContext(r.Context()).SetResource(string(claims.Site), deployID) h.logAction(r.Context(), "deploy.finalize", "success", slog.String("mode", mode), slog.Int64("bytes", deployBytes)) h.auditFromScope(r.Context(), "deploy.finalize", "success", map[string]any{"mode": mode, "bytes": deployBytes}) @@ -357,26 +357,26 @@ func frameworkBuildHint(files []string) string { // deployPrefix returns the R2 key prefix for one deploy, e.g. // "www/deploys/20260420-141522-abc1234/". -func (h *Handlers) deployPrefix(site, deployID string) string { - return h.DeployPrefix.DeployPrefix(sitekey.Slug(site), deployID) +func (h *Handlers) deployPrefix(site sitekey.Slug, deployID string) string { + return h.DeployPrefix.DeployPrefix(site, deployID) } // aliasKey returns the R2 alias key for `mode` ("preview"/"production"). -func (h *Handlers) aliasKey(site, mode string) string { +func (h *Handlers) aliasKey(site sitekey.Slug, mode string) string { switch mode { case "production": - return strings.ReplaceAll(h.AliasProductionFmt, "", site) + return strings.ReplaceAll(h.AliasProductionFmt, "", string(site)) default: - return strings.ReplaceAll(h.AliasPreviewFmt, "", site) + return strings.ReplaceAll(h.AliasPreviewFmt, "", string(site)) } } // publicURL returns the user-visible URL for a finalized deploy. -func (h *Handlers) publicURL(site, mode string) string { +func (h *Handlers) publicURL(site sitekey.Slug, mode string) string { if mode == "production" { - return strings.ReplaceAll(h.PublicProductionURLFmt, "", site) + return strings.ReplaceAll(h.PublicProductionURLFmt, "", string(site)) } - return strings.ReplaceAll(h.PublicPreviewURLFmt, "", site) + return strings.ReplaceAll(h.PublicPreviewURLFmt, "", string(site)) } // normalizeMode validates and normalizes finalize/promote/rollback `mode` arg. diff --git a/internal/handler/deploy_delete.go b/internal/handler/deploy_delete.go index 262620e..4f7fbab 100644 --- a/internal/handler/deploy_delete.go +++ b/internal/handler/deploy_delete.go @@ -19,7 +19,7 @@ const destructiveMoveTimeout = 10 * time.Minute const aliasCommitTimeout = 60 * time.Second func (h *Handlers) SiteDeployDelete(w http.ResponseWriter, r *http.Request) { - site := chi.URLParam(r, "site") + site := sitekey.Slug(chi.URLParam(r, "site")) if err := h.requireSiteAuthz(w, r, site); err != nil { return } @@ -40,7 +40,7 @@ func (h *Handlers) SiteDeployDelete(w http.ResponseWriter, r *http.Request) { moved int success bool ) - lockErr := h.withSiteLock(opCtx, h.DeployPrefix.SiteDirname(sitekey.Slug(site)), func() error { + lockErr := h.withSiteLock(opCtx, h.DeployPrefix.SiteDirname(site), func() error { for _, mode := range []string{"production", "preview"} { cur, err := h.R2.GetAlias(opCtx, h.aliasKey(site, mode)) if err != nil && !r2.IsNotFound(err) { @@ -65,7 +65,7 @@ func (h *Handlers) SiteDeployDelete(w http.ResponseWriter, r *http.Request) { if bytesErr != nil { deployBytes = 0 } - if err := h.Tombstones.RecordTombstone(opCtx, h.DeployPrefix.SiteDirname(sitekey.Slug(site)), deployID, deployBytes); err != nil { + if err := h.Tombstones.RecordTombstone(opCtx, h.DeployPrefix.SiteDirname(site), deployID, deployBytes); err != nil { writeUpstreamError(w, r, http.StatusBadGateway, "tombstone_record_failed", "pg.tombstone.record", err) return nil } @@ -87,7 +87,7 @@ func (h *Handlers) SiteDeployDelete(w http.ResponseWriter, r *http.Request) { return } - telemetry.FromContext(r.Context()).SetResource(site, deployID) + telemetry.FromContext(r.Context()).SetResource(string(site), deployID) h.logAction(r.Context(), "site.deploy.delete", "success", slog.Int("moved", moved)) h.auditFromScope(r.Context(), "site.deploy.delete", "success", map[string]any{"moved": moved}) writeJSON(w, http.StatusOK, map[string]any{ @@ -98,10 +98,10 @@ func (h *Handlers) SiteDeployDelete(w http.ResponseWriter, r *http.Request) { }) } -func (h *Handlers) trashPrefix(site, id string) string { +func (h *Handlers) trashPrefix(site sitekey.Slug, id string) string { base := h.TrashPrefixBase if base == "" { base = "_trash/" } - return base + string(h.DeployPrefix.SiteDirname(sitekey.Slug(site))) + "/" + id + "/" + return base + string(h.DeployPrefix.SiteDirname(site)) + "/" + id + "/" } diff --git a/internal/handler/deploy_logaction_test.go b/internal/handler/deploy_logaction_test.go index a17545c..04064d6 100644 --- a/internal/handler/deploy_logaction_test.go +++ b/internal/handler/deploy_logaction_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -20,7 +21,7 @@ func TestDeployInit_LogsActionWithActor(t *testing.T) { tokenLogins: map[string]string{"good": "alice"}, userTeams: map[string]map[string]bool{"alice": {"team-a": true}}, }, - &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, + &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, newFakeR2()) chain := RequestID(h.RequireGitHubBearer(http.HandlerFunc(h.DeployInit))) @@ -51,7 +52,7 @@ func TestDeployInit_LogsDeniedWithActor(t *testing.T) { tokenLogins: map[string]string{"good": "mallory"}, userTeams: map[string]map[string]bool{"mallory": {"other": true}}, }, - &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, + &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, newFakeR2()) chain := RequestID(h.RequireGitHubBearer(http.HandlerFunc(h.DeployInit))) @@ -70,7 +71,7 @@ func TestDeployInit_LogsDeniedWithActor(t *testing.T) { func TestDeployUpload_LogsSuccessWithActor(t *testing.T) { cap := captureAccessLog(t) store := newFakeR2() - h, jwt := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, store) + h, jwt := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, store) deployID := "20260420-141522-abc1234" tok, _, err := jwt.Sign("alice", "www", deployID) @@ -95,7 +96,7 @@ func TestDeployUpload_LogsSuccessWithActor(t *testing.T) { func TestDeployFinalize_LogsSuccessWithActorAndBytes(t *testing.T) { cap := captureAccessLog(t) store := newFakeR2() - h, jwt := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, store) + h, jwt := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, store) deployID := "20260420-141522-abc1234" prefix := "www/deploys/" + deployID + "/" diff --git a/internal/handler/deploy_restore.go b/internal/handler/deploy_restore.go index 3c8c419..80fa6dd 100644 --- a/internal/handler/deploy_restore.go +++ b/internal/handler/deploy_restore.go @@ -17,7 +17,7 @@ import ( const defaultTrashRecovery = 7 * 24 * time.Hour func (h *Handlers) SiteDeployRestore(w http.ResponseWriter, r *http.Request) { - site := chi.URLParam(r, "site") + site := sitekey.Slug(chi.URLParam(r, "site")) if err := h.requireSiteAuthz(w, r, site); err != nil { return } @@ -39,7 +39,7 @@ func (h *Handlers) SiteDeployRestore(w http.ResponseWriter, r *http.Request) { liveBytes int64 outcome string ) - lockErr := h.withSiteLock(opCtx, h.DeployPrefix.SiteDirname(sitekey.Slug(site)), func() error { + lockErr := h.withSiteLock(opCtx, h.DeployPrefix.SiteDirname(site), func() error { if _, err := h.Registry.GetSite(opCtx, site); err != nil { if errors.Is(err, registry.ErrNotFound) { writeError(w, http.StatusGone, "site_gone", "site was deleted; deploy cannot be restored") @@ -66,7 +66,7 @@ func (h *Handlers) SiteDeployRestore(w http.ResponseWriter, r *http.Request) { liveBytes = 0 } - restoreErr := h.Trash.RestoreDeploy(opCtx, h.DeployPrefix.SiteDirname(sitekey.Slug(site)), deployID, h.Now().UTC(), liveBytes) + restoreErr := h.Trash.RestoreDeploy(opCtx, h.DeployPrefix.SiteDirname(site), deployID, h.Now().UTC(), liveBytes) if restoreErr != nil { if !errors.Is(restoreErr, registry.ErrNotFound) { writeUpstreamError(w, r, http.StatusBadGateway, "restore_failed", "pg.restore.deploy", restoreErr) @@ -96,7 +96,7 @@ func (h *Handlers) SiteDeployRestore(w http.ResponseWriter, r *http.Request) { return } - telemetry.FromContext(r.Context()).SetResource(site, deployID) + telemetry.FromContext(r.Context()).SetResource(string(site), deployID) attrs := []slog.Attr{slog.Int("moved", moved)} detail := map[string]any{"moved": moved} if outcome == "success" { @@ -115,7 +115,7 @@ func (h *Handlers) SiteDeployRestore(w http.ResponseWriter, r *http.Request) { } func (h *Handlers) SiteTrashList(w http.ResponseWriter, r *http.Request) { - site := chi.URLParam(r, "site") + site := sitekey.Slug(chi.URLParam(r, "site")) if err := h.requireSiteAuthz(w, r, site); err != nil { return } @@ -125,7 +125,7 @@ func (h *Handlers) SiteTrashList(w http.ResponseWriter, r *http.Request) { return } - tombstones, err := h.Trash.TombstonesForSite(r.Context(), h.DeployPrefix.SiteDirname(sitekey.Slug(site))) + tombstones, err := h.Trash.TombstonesForSite(r.Context(), h.DeployPrefix.SiteDirname(site)) if err != nil { writeUpstreamError(w, r, http.StatusBadGateway, "pg_read_failed", "pg.tombstones.list", err) return diff --git a/internal/handler/deploy_test.go b/internal/handler/deploy_test.go index 1ddbbeb..c57bf57 100644 --- a/internal/handler/deploy_test.go +++ b/internal/handler/deploy_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/freeCodeCamp/artemis/internal/gc" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/go-chi/chi/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -38,7 +39,7 @@ func withChiRoute(method, pattern, target string, body []byte, headers map[strin } func standardSites() *fakeSites { - return &fakeSites{bySite: map[string][]string{ + return &fakeSites{bySite: map[sitekey.Slug][]string{ "www": {"team-eng", "team-platform"}, "learn": {"team-eng"}, }} diff --git a/internal/handler/destructive_span_test.go b/internal/handler/destructive_span_test.go index cc68217..1307ea5 100644 --- a/internal/handler/destructive_span_test.go +++ b/internal/handler/destructive_span_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/getsentry/sentry-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -37,7 +38,7 @@ func TestDestructiveFlow_BreadcrumbsAndSpans(t *testing.T) { store.objects["www/deploys/20260420-141522-abc1234/index.html"] = []byte("hi") h, _ := newTestHandlers(t, &fakeGH{tokenLogins: map[string]string{"good": "alice"}, userTeams: map[string]map[string]bool{"alice": {"team-a": true}}}, - &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, + &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, store) w := withChiRoute(http.MethodPost, "/api/site/{site}/promote", diff --git a/internal/handler/github_ratelimit_test.go b/internal/handler/github_ratelimit_test.go index cddc5a4..e23b223 100644 --- a/internal/handler/github_ratelimit_test.go +++ b/internal/handler/github_ratelimit_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/freeCodeCamp/artemis/internal/auth" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -32,7 +33,7 @@ func TestDeployInit_RateLimitedProbe_Returns429(t *testing.T) { tokenLogins: map[string]string{"good": "alice"}, authorizeErr: auth.ErrGitHubRateLimited, }, - &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, + &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, newFakeR2()) chain := RequestID(h.RequireGitHubBearer(http.HandlerFunc(h.DeployInit))) diff --git a/internal/handler/handler.go b/internal/handler/handler.go index f9d3701..3df3cad 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -37,7 +37,7 @@ type GitHubAuthenticator interface { // DeployJWTSigner is the subset of *auth.DeploySessionSigner used by the // handler layer. type DeployJWTSigner interface { - Sign(login, site, deployID string) (string, time.Time, error) + Sign(login string, site sitekey.Slug, deployID string) (string, time.Time, error) Verify(token string) (auth.DeploySessionClaims, error) } diff --git a/internal/handler/middleware_sentry_user_test.go b/internal/handler/middleware_sentry_user_test.go index a541063..033e421 100644 --- a/internal/handler/middleware_sentry_user_test.go +++ b/internal/handler/middleware_sentry_user_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/getsentry/sentry-go" "github.com/stretchr/testify/require" ) @@ -14,7 +15,7 @@ import ( func TestRequireDeployJWT_SetsSentryUser(t *testing.T) { hub, ft := newHubWithTransport(t) h, jwt := newTestHandlers(t, &fakeGH{}, - &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, newFakeR2()) + &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, newFakeR2()) tok, _, err := jwt.Sign("alice", "www", "d-1") require.NoError(t, err) diff --git a/internal/handler/middleware_test.go b/internal/handler/middleware_test.go index 1bc57ae..dd08e96 100644 --- a/internal/handler/middleware_test.go +++ b/internal/handler/middleware_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/freeCodeCamp/artemis/internal/auth" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/freeCodeCamp/artemis/internal/telemetry" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -14,7 +15,7 @@ import ( func TestRequireGitHubBearer_MissingHeader(t *testing.T) { h, _ := newTestHandlers(t, &fakeGH{tokenLogins: map[string]string{}}, - &fakeSites{bySite: map[string][]string{}}, + &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) r := httptest.NewRequest(http.MethodGet, "/api/whoami", nil) @@ -29,7 +30,7 @@ func TestRequireGitHubBearer_MissingHeader(t *testing.T) { func TestRequireGitHubBearer_RateLimited_Returns429(t *testing.T) { h, _ := newTestHandlers(t, &fakeGH{upstreamErr: auth.ErrGitHubRateLimited}, - &fakeSites{bySite: map[string][]string{}}, + &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) r := httptest.NewRequest(http.MethodGet, "/api/whoami", nil) @@ -45,7 +46,7 @@ func TestRequireGitHubBearer_RateLimited_Returns429(t *testing.T) { func TestRequireGitHubBearer_5xx_Returns503(t *testing.T) { h, _ := newTestHandlers(t, &fakeGH{upstreamErr: auth.ErrGitHubUnavailable}, - &fakeSites{bySite: map[string][]string{}}, + &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) r := httptest.NewRequest(http.MethodGet, "/api/whoami", nil) @@ -59,7 +60,7 @@ func TestRequireGitHubBearer_5xx_Returns503(t *testing.T) { func TestRequireGitHubBearer_OK_AttachesLoginToContext(t *testing.T) { h, _ := newTestHandlers(t, &fakeGH{tokenLogins: map[string]string{"good": "alice"}}, - &fakeSites{bySite: map[string][]string{}}, + &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) var seen string @@ -75,7 +76,7 @@ func TestRequireGitHubBearer_OK_AttachesLoginToContext(t *testing.T) { } func TestRequireDeployJWT_MissingHeader(t *testing.T) { - h, _ := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[string][]string{}}, newFakeR2()) + h, _ := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) r := httptest.NewRequest(http.MethodPut, "/api/deploy/d1/upload", nil) w := httptest.NewRecorder() @@ -87,7 +88,7 @@ func TestRequireDeployJWT_MissingHeader(t *testing.T) { } func TestRequireDeployJWT_BadToken_Returns403(t *testing.T) { - h, _ := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[string][]string{}}, newFakeR2()) + h, _ := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) r := httptest.NewRequest(http.MethodPut, "/api/deploy/d1/upload", nil) r.Header.Set("Authorization", "Bearer not-a-jwt") @@ -100,7 +101,7 @@ func TestRequireDeployJWT_BadToken_Returns403(t *testing.T) { } func TestRequireDeployJWT_OK_AttachesClaims(t *testing.T) { - h, jwt := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, newFakeR2()) + h, jwt := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, newFakeR2()) tok, _, err := jwt.Sign("alice", "www", "d-1") require.NoError(t, err) @@ -119,7 +120,7 @@ func TestRequireDeployJWT_OK_AttachesClaims(t *testing.T) { } func TestRequireDeployJWT_RejectsUnregisteredSite(t *testing.T) { - h, jwt := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[string][]string{}}, newFakeR2()) + h, jwt := newTestHandlers(t, &fakeGH{}, &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) tok, _, err := jwt.Sign("alice", "purged", "d-9") require.NoError(t, err) @@ -199,7 +200,7 @@ func TestAccessLog_SkipsProbePaths(t *testing.T) { func TestRequireGitHubBearer_BadToken_Returns401(t *testing.T) { h, _ := newTestHandlers(t, &fakeGH{tokenLogins: map[string]string{}}, - &fakeSites{bySite: map[string][]string{}}, + &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) r := httptest.NewRequest(http.MethodGet, "/api/whoami", nil) diff --git a/internal/handler/repo_test.go b/internal/handler/repo_test.go index 6c01790..3fa0c47 100644 --- a/internal/handler/repo_test.go +++ b/internal/handler/repo_test.go @@ -19,6 +19,7 @@ import ( "github.com/freeCodeCamp/artemis/internal/githubapp" "github.com/freeCodeCamp/artemis/internal/reporequest" + "github.com/freeCodeCamp/artemis/internal/sitekey" ) // fakeRepoStore is an in-memory RepoStore with the same status semantics @@ -206,7 +207,7 @@ func (f *fakeRepoCreator) ListTemplates(_ context.Context) ([]string, error) { // Universe-org membership prober used by repo authz. func repoHandlers(t *testing.T, repoGH *fakeGH, store RepoStore, creator RepoCreator) *Handlers { t.Helper() - h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[string][]string{}}, newFakeR2()) + h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) h.RepoGH = repoGH h.Repos = store h.GitHubApp = creator diff --git a/internal/handler/site.go b/internal/handler/site.go index 4901618..9dd1670 100644 --- a/internal/handler/site.go +++ b/internal/handler/site.go @@ -57,7 +57,7 @@ type SitePromoteRequest struct { // // Authz: unchanged — staff-team gate enforced by requireSiteAuthz. func (h *Handlers) SitePromote(w http.ResponseWriter, r *http.Request) { - site := chi.URLParam(r, "site") + site := sitekey.Slug(chi.URLParam(r, "site")) if err := h.requireSiteAuthz(w, r, site); err != nil { return // already wrote response } @@ -88,7 +88,7 @@ func (h *Handlers) SitePromote(w http.ResponseWriter, r *http.Request) { var deployID string commitCtx, cancelCommit := context.WithTimeout(context.WithoutCancel(r.Context()), aliasCommitTimeout) defer cancelCommit() - lockErr := h.withSiteLock(commitCtx, h.DeployPrefix.SiteDirname(sitekey.Slug(site)), func() error { + lockErr := h.withSiteLock(commitCtx, h.DeployPrefix.SiteDirname(site), func() error { telemetry.Breadcrumb(commitCtx, "lock", "site lock acquired") // CAS guard: read current production alias and bail on mismatch. // Treat missing-alias as the empty string so callers can use CAS @@ -153,7 +153,7 @@ func (h *Handlers) SitePromote(w http.ResponseWriter, r *http.Request) { return errAliasWriteHandled } if h.Index != nil { - if err := h.Index.AliasAtomic(commitCtx, h.DeployPrefix.SiteDirname(sitekey.Slug(site)), "production", deployID, time.Now().UTC()); err != nil { + if err := h.Index.AliasAtomic(commitCtx, h.DeployPrefix.SiteDirname(site), "production", deployID, time.Now().UTC()); err != nil { writeUpstreamError(w, r, http.StatusBadGateway, "pg_write_failed", "pg.alias.promote", err) return errAliasWriteHandled } @@ -166,7 +166,7 @@ func (h *Handlers) SitePromote(w http.ResponseWriter, r *http.Request) { } return } - telemetry.FromContext(r.Context()).SetResource(site, deployID) + telemetry.FromContext(r.Context()).SetResource(string(site), deployID) h.logAction(r.Context(), "site.promote", "success") h.auditFromScope(r.Context(), "site.promote", "success", nil) writeJSON(w, http.StatusOK, map[string]any{ @@ -193,7 +193,7 @@ type SiteRollbackRequest struct { // cron) or if ExpectedCurrent is set and disagrees with the current // production alias body. func (h *Handlers) SiteRollback(w http.ResponseWriter, r *http.Request) { - site := chi.URLParam(r, "site") + site := sitekey.Slug(chi.URLParam(r, "site")) if err := h.requireSiteAuthz(w, r, site); err != nil { return } @@ -217,7 +217,7 @@ func (h *Handlers) SiteRollback(w http.ResponseWriter, r *http.Request) { commitCtx, cancelCommit := context.WithTimeout(context.WithoutCancel(r.Context()), aliasCommitTimeout) defer cancelCommit() - lockErr := h.withSiteLock(commitCtx, h.DeployPrefix.SiteDirname(sitekey.Slug(site)), func() error { + lockErr := h.withSiteLock(commitCtx, h.DeployPrefix.SiteDirname(site), func() error { prefix := h.deployPrefix(site, req.To) exists, err := h.R2.HasPrefix(commitCtx, prefix) if err != nil { @@ -270,7 +270,7 @@ func (h *Handlers) SiteRollback(w http.ResponseWriter, r *http.Request) { return errAliasWriteHandled } if h.Index != nil { - if err := h.Index.AliasAtomic(commitCtx, h.DeployPrefix.SiteDirname(sitekey.Slug(site)), "production", req.To, time.Now().UTC()); err != nil { + if err := h.Index.AliasAtomic(commitCtx, h.DeployPrefix.SiteDirname(site), "production", req.To, time.Now().UTC()); err != nil { writeUpstreamError(w, r, http.StatusBadGateway, "pg_write_failed", "pg.alias.rollback", err) return errAliasWriteHandled } @@ -283,7 +283,7 @@ func (h *Handlers) SiteRollback(w http.ResponseWriter, r *http.Request) { } return } - telemetry.FromContext(r.Context()).SetResource(site, req.To) + telemetry.FromContext(r.Context()).SetResource(string(site), req.To) h.logAction(r.Context(), "site.rollback", "success", slog.String("to", req.To)) h.auditFromScope(r.Context(), "site.rollback", "success", map[string]any{"to": req.To}) writeJSON(w, http.StatusOK, map[string]any{ @@ -296,12 +296,12 @@ func (h *Handlers) SiteRollback(w http.ResponseWriter, r *http.Request) { // deploys under /deploys/. Each deploy is identified by the prefix // segment "-". func (h *Handlers) SiteDeploys(w http.ResponseWriter, r *http.Request) { - site := chi.URLParam(r, "site") + site := sitekey.Slug(chi.URLParam(r, "site")) if err := h.requireSiteAuthz(w, r, site); err != nil { return } - deploysPrefix := h.DeployPrefix.SitePrefix(sitekey.Slug(site)) + deploysPrefix := h.DeployPrefix.SitePrefix(site) keys, err := h.R2.ListPrefix(r.Context(), deploysPrefix) if err != nil { writeUpstreamError(w, r, http.StatusBadGateway, "r2_list_failed", "r2.list.deploys", err) @@ -310,7 +310,7 @@ func (h *Handlers) SiteDeploys(w http.ResponseWriter, r *http.Request) { actors := map[string]string{} if h.Audit != nil { - if a, aErr := h.Audit.DeployActors(r.Context(), site); aErr != nil { + if a, aErr := h.Audit.DeployActors(r.Context(), string(site)); aErr != nil { slog.WarnContext(r.Context(), "site.deploys.actor_join_failed", "site", site, "err", aErr) } else { actors = a @@ -344,7 +344,7 @@ func (h *Handlers) SiteDeploys(w http.ResponseWriter, r *http.Request) { // authenticated GitHub user is on at least one of the site's authorized // teams. Writes the response on failure and returns a non-nil error so // the caller can early-return without further work. -func (h *Handlers) requireSiteAuthz(w http.ResponseWriter, r *http.Request, site string) error { +func (h *Handlers) requireSiteAuthz(w http.ResponseWriter, r *http.Request, site sitekey.Slug) error { teams := h.Sites.Snapshot().TeamsForSite(site) if len(teams) == 0 { writeError(w, http.StatusForbidden, "site_unauthorized", "site is not registered or has no authorized teams") diff --git a/internal/handler/site_delete_test.go b/internal/handler/site_delete_test.go index 487b50a..e91ea78 100644 --- a/internal/handler/site_delete_test.go +++ b/internal/handler/site_delete_test.go @@ -8,6 +8,7 @@ import ( "net/http/httptest" "testing" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/go-chi/chi/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -28,7 +29,7 @@ func callDelete(h *Handlers, slug, login, token string) *httptest.ResponseRecord } func TestSiteDelete_HappyPath(t *testing.T) { - h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[string][]string{}}, newFakeR2()) + h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) regBody, _ := json.Marshal(SiteRegisterRequest{Slug: "example", Teams: []string{"staff"}}) require.Equal(t, http.StatusCreated, callRegister(h, regBody, "alice", "tok").Code) diff --git a/internal/handler/site_logaction_test.go b/internal/handler/site_logaction_test.go index 650af23..ab4e172 100644 --- a/internal/handler/site_logaction_test.go +++ b/internal/handler/site_logaction_test.go @@ -5,6 +5,7 @@ import ( "net/http" "testing" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -19,7 +20,7 @@ func TestSitePromote_LogsActionWithActor(t *testing.T) { tokenLogins: map[string]string{"good": "alice"}, userTeams: map[string]map[string]bool{"alice": {"team-a": true}}, }, - &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, + &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, store) w := withChiRoute(http.MethodPost, "/api/site/{site}/promote", @@ -46,7 +47,7 @@ func TestSiteUpdate_LogsBeforeAfterTeams(t *testing.T) { tokenLogins: map[string]string{"good": "alice"}, userTeams: map[string]map[string]bool{"alice": {"staff": true}}, }, - &fakeSites{bySite: map[string][]string{"www": {"team-a"}}}, + &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-a"}}}, newFakeR2()) w := withChiRoute(http.MethodPatch, "/api/site/{slug}", diff --git a/internal/handler/site_purge_test.go b/internal/handler/site_purge_test.go index f01f3e0..7c100ed 100644 --- a/internal/handler/site_purge_test.go +++ b/internal/handler/site_purge_test.go @@ -9,6 +9,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/freeCodeCamp/artemis/internal/sitekey" ) type flakyMoveR2 struct { @@ -112,11 +114,11 @@ func TestSitePurge_FailedMoveKeepsSiteRetryable(t *testing.T) { require.Equal(t, http.StatusOK, listW.Code) var rows []SiteRow require.NoError(t, json.Unmarshal(listW.Body.Bytes(), &rows)) - slugs := make([]string, len(rows)) + slugs := make([]sitekey.Slug, len(rows)) for i, r := range rows { slugs[i] = r.Slug } - assert.Contains(t, slugs, "example", "failed purge must not deregister the site (still retryable)") + assert.Contains(t, slugs, sitekey.Slug("example"), "failed purge must not deregister the site (still retryable)") assert.Equal(t, []string{"example"}, tomb.purged, "the site tombstone lands before the move, so a failed move leaves the row naming the bytes still "+ "in place; the retry re-records it, restarting the recovery clock exactly as "+ @@ -144,9 +146,12 @@ func TestSitePurge_FailedMoveKeepsSiteRetryable(t *testing.T) { require.Equal(t, http.StatusOK, gone.Code) var after []SiteRow require.NoError(t, json.Unmarshal(gone.Body.Bytes(), &after)) - for _, r := range after { - assert.NotEqual(t, "example", r.Slug, "successful purge deregisters the site") + remaining := make([]sitekey.Slug, len(after)) + for i, r := range after { + remaining[i] = r.Slug } + assert.NotContains(t, remaining, sitekey.Slug("example"), + "successful purge deregisters the site") } func TestSiteDelete_NoPurge_LeavesBytes(t *testing.T) { diff --git a/internal/handler/site_register.go b/internal/handler/site_register.go index 2f1590b..b788e96 100644 --- a/internal/handler/site_register.go +++ b/internal/handler/site_register.go @@ -20,11 +20,11 @@ import ( // register / list / update endpoints. The shape is stable so // universe-cli can decode the same struct from any of them. type SiteRow struct { - Slug string `json:"slug"` - Teams []string `json:"teams"` - CreatedAt time.Time `json:"createdAt"` - UpdatedAt time.Time `json:"updatedAt"` - CreatedBy string `json:"createdBy"` + Slug sitekey.Slug `json:"slug"` + Teams []string `json:"teams"` + CreatedAt time.Time `json:"createdAt"` + UpdatedAt time.Time `json:"updatedAt"` + CreatedBy string `json:"createdBy"` } func toSiteRow(s registry.Site) SiteRow { @@ -39,8 +39,8 @@ func toSiteRow(s registry.Site) SiteRow { // SiteRegisterRequest is the body of POST /api/site/register. type SiteRegisterRequest struct { - Slug string `json:"slug"` - Teams []string `json:"teams,omitempty"` + Slug sitekey.Slug `json:"slug"` + Teams []string `json:"teams,omitempty"` } // SiteRegisterResponse is the 201 body returned on successful @@ -79,7 +79,7 @@ func (h *Handlers) SiteRegister(w http.ResponseWriter, r *http.Request) { if !decodeJSON(w, r, &req, maxJSONBodyBytes) { return } - if !slugRe.MatchString(req.Slug) { + if !slugRe.MatchString(string(req.Slug)) { writeError(w, http.StatusBadRequest, "invalid_slug", "slug must be 1-63 chars, lowercase letter first, then [a-z0-9-]") return @@ -110,7 +110,7 @@ func (h *Handlers) SiteRegister(w http.ResponseWriter, r *http.Request) { } slog.InfoContext(r.Context(), "site.register", "site", req.Slug, "teams", teams) - telemetry.FromContext(r.Context()).SetResource(req.Slug, "") + telemetry.FromContext(r.Context()).SetResource(string(req.Slug), "") h.auditFromScope(r.Context(), "site.register", "success", map[string]any{"teams": teams, "createdBy": login}) writeJSON(w, http.StatusCreated, toSiteRow(site)) } @@ -135,8 +135,8 @@ func (h *Handlers) SiteUpdate(w http.ResponseWriter, r *http.Request) { if err := h.requireRegistryAuthz(w, r); err != nil { return } - slug := chi.URLParam(r, "slug") - if !slugRe.MatchString(slug) { + slug := sitekey.Slug(chi.URLParam(r, "slug")) + if !slugRe.MatchString(string(slug)) { writeError(w, http.StatusBadRequest, "invalid_slug", "slug must be 1-63 chars, lowercase letter first, then [a-z0-9-]") return @@ -165,7 +165,7 @@ func (h *Handlers) SiteUpdate(w http.ResponseWriter, r *http.Request) { site registry.Site wrote bool ) - lockErr := h.withSiteLock(r.Context(), h.DeployPrefix.SiteDirname(sitekey.Slug(slug)), func() error { + lockErr := h.withSiteLock(r.Context(), h.DeployPrefix.SiteDirname(slug), func() error { before, beforeErr = h.Registry.GetSite(r.Context(), slug) var err error site, err = h.Registry.UpdateTeams(r.Context(), slug, req.Teams) @@ -191,7 +191,7 @@ func (h *Handlers) SiteUpdate(w http.ResponseWriter, r *http.Request) { if beforeErr != nil { beforeTeams = "unknown" } - telemetry.FromContext(r.Context()).SetResource(slug, "") + telemetry.FromContext(r.Context()).SetResource(string(slug), "") h.logAction(r.Context(), "site.update", "success", slog.Any("before", beforeTeams), slog.Any("after", site.Teams)) h.auditFromScope(r.Context(), "site.update", "success", map[string]any{"before": beforeTeams, "after": site.Teams}) @@ -219,8 +219,8 @@ func (h *Handlers) SiteDelete(w http.ResponseWriter, r *http.Request) { if err := h.requireRegistryAuthz(w, r); err != nil { return } - slug := chi.URLParam(r, "slug") - if !slugRe.MatchString(slug) { + slug := sitekey.Slug(chi.URLParam(r, "slug")) + if !slugRe.MatchString(string(slug)) { writeError(w, http.StatusBadRequest, "invalid_slug", "slug must be 1-63 chars, lowercase letter first, then [a-z0-9-]") return @@ -231,7 +231,7 @@ func (h *Handlers) SiteDelete(w http.ResponseWriter, r *http.Request) { writeRegistryDeleteError(w, r, err) return } - telemetry.FromContext(r.Context()).SetResource(slug, "") + telemetry.FromContext(r.Context()).SetResource(string(slug), "") h.logAction(r.Context(), "site.delete", "success") h.auditFromScope(r.Context(), "site.delete", "success", nil) w.WriteHeader(http.StatusNoContent) @@ -246,7 +246,7 @@ func (h *Handlers) SiteDelete(w http.ResponseWriter, r *http.Request) { if base == "" { base = "_trash/" } - dirname := h.DeployPrefix.SiteDirname(sitekey.Slug(slug)) + dirname := h.DeployPrefix.SiteDirname(slug) opCtx, cancel := context.WithTimeout(context.WithoutCancel(r.Context()), destructiveMoveTimeout) defer cancel() var ( @@ -279,7 +279,7 @@ func (h *Handlers) SiteDelete(w http.ResponseWriter, r *http.Request) { return } - telemetry.FromContext(r.Context()).SetResource(slug, "") + telemetry.FromContext(r.Context()).SetResource(string(slug), "") h.logAction(r.Context(), "site.purge", "success", slog.Int("moved", moved)) h.auditFromScope(r.Context(), "site.purge", "success", map[string]any{"moved": moved}) writeJSON(w, http.StatusOK, map[string]any{"slug": slug, "status": "purged", "moved": moved}) diff --git a/internal/handler/site_register_test.go b/internal/handler/site_register_test.go index da8aec9..0186bbc 100644 --- a/internal/handler/site_register_test.go +++ b/internal/handler/site_register_test.go @@ -12,6 +12,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/freeCodeCamp/artemis/internal/sitekey" ) // callRegister POSTs the given body to SiteRegister with the test @@ -46,7 +48,7 @@ func TestSiteRegister_HappyPath(t *testing.T) { var got SiteRegisterResponse require.NoError(t, json.Unmarshal(w.Body.Bytes(), &got)) - assert.Equal(t, "example", got.Slug) + assert.Equal(t, sitekey.Slug("example"), got.Slug) assert.Equal(t, []string{"staff", "platform"}, got.Teams) assert.Equal(t, "alice", got.CreatedBy) assert.False(t, got.CreatedAt.IsZero()) @@ -115,7 +117,7 @@ func TestSiteRegister_409OnDuplicateSlug(t *testing.T) { func TestSiteRegister_400OnInvalidSlug(t *testing.T) { cases := []struct { name string - slug string + slug sitekey.Slug }{ {"empty", ""}, {"uppercase", "Example"}, diff --git a/internal/handler/sitekey_pin_test.go b/internal/handler/sitekey_pin_test.go new file mode 100644 index 0000000..c60b19c --- /dev/null +++ b/internal/handler/sitekey_pin_test.go @@ -0,0 +1,41 @@ +package handler + +import ( + "encoding/json" + "testing" + "time" + + "github.com/freeCodeCamp/artemis/internal/sitekey" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var ( + _ sitekey.Slug = SiteRow{}.Slug + _ sitekey.Slug = SiteRegisterRequest{}.Slug + _ sitekey.Slug = DeployInitRequest{}.Site + + _ func(*Handlers, sitekey.Slug, string) string = (*Handlers).deployPrefix + _ func(*Handlers, sitekey.Slug, string) string = (*Handlers).aliasKey + _ func(*Handlers, sitekey.Slug, string) string = (*Handlers).publicURL + _ func(*Handlers, sitekey.Slug, string) string = (*Handlers).trashPrefix +) + +func TestSiteRow_SlugMarshalsAsAPlainJSONString(t *testing.T) { + t.Parallel() + + raw, err := json.Marshal(SiteRow{Slug: "example", CreatedAt: time.Unix(0, 0).UTC()}) + require.NoError(t, err) + + var wire map[string]any + require.NoError(t, json.Unmarshal(raw, &wire)) + assert.Equal(t, "example", wire["slug"]) +} + +func TestSiteRegisterRequest_SlugUnmarshalsFromAPlainJSONString(t *testing.T) { + t.Parallel() + + var req SiteRegisterRequest + require.NoError(t, json.Unmarshal([]byte(`{"slug":"example","teams":["staff"]}`), &req)) + assert.Equal(t, sitekey.Slug("example"), req.Slug) +} diff --git a/internal/handler/sites_list_test.go b/internal/handler/sites_list_test.go index c261868..71c105d 100644 --- a/internal/handler/sites_list_test.go +++ b/internal/handler/sites_list_test.go @@ -8,6 +8,7 @@ import ( "net/http/httptest" "testing" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -21,7 +22,7 @@ func callSitesList(h *Handlers, login, token string) *httptest.ResponseRecorder } func TestSitesList_EmptyRegistry(t *testing.T) { - h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[string][]string{}}, newFakeR2()) + h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) w := callSitesList(h, "alice", "tok") require.Equal(t, http.StatusOK, w.Code, w.Body.String()) @@ -32,7 +33,7 @@ func TestSitesList_EmptyRegistry(t *testing.T) { } func TestSitesList_PopulatedReturnsRowsSorted(t *testing.T) { - h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[string][]string{}}, newFakeR2()) + h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) for _, slug := range []string{"charlie", "alpha", "bravo"} { body := []byte(`{"slug":"` + slug + `","teams":["staff"]}`) @@ -49,9 +50,9 @@ func TestSitesList_PopulatedReturnsRowsSorted(t *testing.T) { var got []SiteRow require.NoError(t, json.Unmarshal(w.Body.Bytes(), &got)) require.Len(t, got, 3) - assert.Equal(t, "alpha", got[0].Slug) - assert.Equal(t, "bravo", got[1].Slug) - assert.Equal(t, "charlie", got[2].Slug) + assert.Equal(t, sitekey.Slug("alpha"), got[0].Slug) + assert.Equal(t, sitekey.Slug("bravo"), got[1].Slug) + assert.Equal(t, sitekey.Slug("charlie"), got[2].Slug) assert.Equal(t, []string{"staff"}, got[0].Teams) assert.Equal(t, "alice", got[0].CreatedBy) assert.False(t, got[0].CreatedAt.IsZero()) @@ -65,7 +66,7 @@ func nonStaffGH() *fakeGH { } func TestSitesList_RedactsCreatedByForNonStaff(t *testing.T) { - h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[string][]string{}}, newFakeR2()) + h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) body := []byte(`{"slug":"alpha","teams":["staff"]}`) require.Equal(t, http.StatusCreated, callRegister(h, body, "alice", "tok").Code) @@ -80,12 +81,12 @@ func TestSitesList_RedactsCreatedByForNonStaff(t *testing.T) { require.NoError(t, json.Unmarshal(w.Body.Bytes(), &got)) require.Len(t, got, 1) assert.Empty(t, got[0].CreatedBy, "non-staff caller must not see actor identity") - assert.Equal(t, "alpha", got[0].Slug, "non-actor fields stay visible") + assert.Equal(t, sitekey.Slug("alpha"), got[0].Slug, "non-actor fields stay visible") assert.Equal(t, []string{"staff"}, got[0].Teams) } func TestSitesList_RedactsWhenAuthzProbeErrors(t *testing.T) { - h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[string][]string{}}, newFakeR2()) + h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) body := []byte(`{"slug":"alpha","teams":["staff"]}`) require.Equal(t, http.StatusCreated, callRegister(h, body, "alice", "tok").Code) @@ -120,7 +121,7 @@ func TestSitesList_502OnRegistryReadError(t *testing.T) { } func TestSitesList_ActorGateIndependentOfRepoFeature(t *testing.T) { - h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[string][]string{}}, newFakeR2()) + h, _ := newTestHandlers(t, staffCallerGH(), &fakeSites{bySite: map[sitekey.Slug][]string{}}, newFakeR2()) h.RepoGH = staffCallerGH() h.AuditReadAuthzTeam = "staff" require.False(t, h.RepoEnabled(), "repo-create feature off (Repos/GitHubApp nil) — actor/audit gating must not depend on it") diff --git a/internal/handler/test_helpers_test.go b/internal/handler/test_helpers_test.go index 41828fe..0020e20 100644 --- a/internal/handler/test_helpers_test.go +++ b/internal/handler/test_helpers_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "slices" "sort" "github.com/freeCodeCamp/artemis/internal/auth" @@ -22,7 +23,7 @@ import ( // ErrAlreadyExists on duplicate. The injected clock keeps timestamps // deterministic. type fakeRegistry struct { - bySite map[string]registry.Site + bySite map[sitekey.Slug]registry.Site // fixedNow drives created_at / updated_at; if zero, time.Now() is used. fixedNow time.Time @@ -32,10 +33,10 @@ type fakeRegistry struct { } func newFakeRegistry() *fakeRegistry { - return &fakeRegistry{bySite: map[string]registry.Site{}} + return &fakeRegistry{bySite: map[sitekey.Slug]registry.Site{}} } -func (f *fakeRegistry) Register(_ context.Context, slug string, teams []string, createdBy string) (registry.Site, error) { +func (f *fakeRegistry) Register(_ context.Context, slug sitekey.Slug, teams []string, createdBy string) (registry.Site, error) { if f.registerErr != nil { return registry.Site{}, f.registerErr } @@ -59,7 +60,7 @@ func (f *fakeRegistry) Register(_ context.Context, slug string, teams []string, return site, nil } -func (f *fakeRegistry) UpdateTeams(_ context.Context, slug string, teams []string) (registry.Site, error) { +func (f *fakeRegistry) UpdateTeams(_ context.Context, slug sitekey.Slug, teams []string) (registry.Site, error) { if f.registerErr != nil { return registry.Site{}, f.registerErr } @@ -84,7 +85,7 @@ func (f *fakeRegistry) UpdateTeams(_ context.Context, slug string, teams []strin return updated, nil } -func (f *fakeRegistry) Delete(_ context.Context, slug string) error { +func (f *fakeRegistry) Delete(_ context.Context, slug sitekey.Slug) error { if f.registerErr != nil { return f.registerErr } @@ -95,7 +96,7 @@ func (f *fakeRegistry) Delete(_ context.Context, slug string) error { return nil } -func (f *fakeRegistry) GetSite(_ context.Context, slug string) (registry.Site, error) { +func (f *fakeRegistry) GetSite(_ context.Context, slug sitekey.Slug) (registry.Site, error) { if f.getErr != nil { return registry.Site{}, f.getErr } @@ -126,19 +127,19 @@ func (f *fakeRegistry) Sites(_ context.Context) ([]registry.Site, error) { // tests that need to assert the handler's error envelope mapping. type erroringRegistry struct{ err error } -func (e *erroringRegistry) Register(_ context.Context, _ string, _ []string, _ string) (registry.Site, error) { +func (e *erroringRegistry) Register(_ context.Context, _ sitekey.Slug, _ []string, _ string) (registry.Site, error) { return registry.Site{}, e.err } -func (e *erroringRegistry) UpdateTeams(_ context.Context, _ string, _ []string) (registry.Site, error) { +func (e *erroringRegistry) UpdateTeams(_ context.Context, _ sitekey.Slug, _ []string) (registry.Site, error) { return registry.Site{}, e.err } -func (e *erroringRegistry) Delete(_ context.Context, _ string) error { +func (e *erroringRegistry) Delete(_ context.Context, _ sitekey.Slug) error { return e.err } func (e *erroringRegistry) Sites(_ context.Context) ([]registry.Site, error) { return nil, e.err } -func (e *erroringRegistry) GetSite(_ context.Context, _ string) (registry.Site, error) { +func (e *erroringRegistry) GetSite(_ context.Context, _ sitekey.Slug) (registry.Site, error) { return registry.Site{}, e.err } @@ -243,7 +244,7 @@ func sleepUntilExpired() { time.Sleep(20 * time.Millisecond) } -func (f *fakeJWT) Sign(login, site, deployID string) (string, time.Time, error) { +func (f *fakeJWT) Sign(login string, site sitekey.Slug, deployID string) (string, time.Time, error) { return f.signer.Sign(login, site, deployID) } @@ -253,11 +254,11 @@ func (f *fakeJWT) Verify(token string) (auth.DeploySessionClaims, error) { // fakeSites implements SitesProvider over an in-memory map. type fakeSites struct { - bySite map[string][]string + bySite map[sitekey.Slug][]string } func (f *fakeSites) Snapshot() registry.Snapshot { - cp := make(map[string][]string, len(f.bySite)) + cp := make(map[sitekey.Slug][]string, len(f.bySite)) for k, v := range f.bySite { dup := make([]string, len(v)) copy(dup, v) @@ -269,19 +270,19 @@ func (f *fakeSites) Snapshot() registry.Snapshot { // staticSnapshot is a registry.Snapshot impl backed by an in-memory // map. Test-only — production reads come from valkey.Reader. type staticSnapshot struct { - bySite map[string][]string + bySite map[sitekey.Slug][]string } -func (s staticSnapshot) Sites() []string { - out := make([]string, 0, len(s.bySite)) +func (s staticSnapshot) Sites() []sitekey.Slug { + out := make([]sitekey.Slug, 0, len(s.bySite)) for k := range s.bySite { out = append(out, k) } - sort.Strings(out) + slices.Sort(out) return out } -func (s staticSnapshot) TeamsForSite(slug string) []string { +func (s staticSnapshot) TeamsForSite(slug sitekey.Slug) []string { teams, ok := s.bySite[slug] if !ok { return nil diff --git a/internal/handler/whoami.go b/internal/handler/whoami.go index bbbcd45..db4b99e 100644 --- a/internal/handler/whoami.go +++ b/internal/handler/whoami.go @@ -2,7 +2,9 @@ package handler import ( "net/http" - "sort" + "slices" + + "github.com/freeCodeCamp/artemis/internal/sitekey" ) // WhoAmI implements GET /api/whoami. Returns the resolved login plus the @@ -31,7 +33,7 @@ func (h *Handlers) WhoAmI(w http.ResponseWriter, r *http.Request) { userTeams[t] = struct{}{} } - authorized := []string{} + authorized := []sitekey.Slug{} snap := h.Sites.Snapshot() for _, site := range snap.Sites() { siteTeams := snap.TeamsForSite(site) @@ -45,7 +47,7 @@ func (h *Handlers) WhoAmI(w http.ResponseWriter, r *http.Request) { } } } - sort.Strings(authorized) + slices.Sort(authorized) writeJSON(w, http.StatusOK, map[string]any{ "login": login, "authorizedSites": authorized, diff --git a/internal/handler/whoami_test.go b/internal/handler/whoami_test.go index 0777d3d..af8e4d4 100644 --- a/internal/handler/whoami_test.go +++ b/internal/handler/whoami_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "testing" + "github.com/freeCodeCamp/artemis/internal/sitekey" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -18,7 +19,7 @@ func TestWhoAmI_ReturnsLoginAndAuthorizedSites(t *testing.T) { "alice": {"team-eng": true}, }, } - st := &fakeSites{bySite: map[string][]string{ + st := &fakeSites{bySite: map[sitekey.Slug][]string{ "www": {"team-eng", "team-platform"}, "learn": {"team-eng"}, "news": {"team-content"}, @@ -46,7 +47,7 @@ func TestWhoAmI_UpstreamErrorReturns503(t *testing.T) { tokenLogins: map[string]string{"good": "alice"}, upstreamErr: assert.AnError, } - st := &fakeSites{bySite: map[string][]string{"www": {"team-eng"}}} + st := &fakeSites{bySite: map[sitekey.Slug][]string{"www": {"team-eng"}}} h, _ := newTestHandlers(t, gh, st, newFakeR2()) r := httptest.NewRequest(http.MethodGet, "/api/whoami", nil). @@ -65,7 +66,7 @@ func TestWhoAmI_SkipsSitesWithNoTeams(t *testing.T) { }, } // Site with no teams should be skipped (cannot grant via empty team list). - st := &fakeSites{bySite: map[string][]string{ + st := &fakeSites{bySite: map[sitekey.Slug][]string{ "www": {"team-eng"}, "empty": {}, }} @@ -89,7 +90,7 @@ func TestWhoAmI_NoAuthorizedSites(t *testing.T) { tokenLogins: map[string]string{"g": "bob"}, userTeams: map[string]map[string]bool{}, } - st := &fakeSites{bySite: map[string][]string{ + st := &fakeSites{bySite: map[sitekey.Slug][]string{ "www": {"team-eng"}, }} h, _ := newTestHandlers(t, gh, st, newFakeR2()) @@ -119,7 +120,7 @@ func TestWhoAmI_OneGitHubCallPerCold(t *testing.T) { "alice": {"team-eng": true, "team-platform": true}, }, } - st := &fakeSites{bySite: map[string][]string{ + st := &fakeSites{bySite: map[sitekey.Slug][]string{ "www": {"team-eng", "team-platform", "team-content"}, "learn": {"team-eng", "team-research", "team-platform"}, "news": {"team-content", "team-platform", "team-eng"}, diff --git a/internal/pg/registry.go b/internal/pg/registry.go index bf040aa..7191557 100644 --- a/internal/pg/registry.go +++ b/internal/pg/registry.go @@ -10,12 +10,13 @@ import ( "github.com/jackc/pgx/v5/pgxpool" "github.com/freeCodeCamp/artemis/internal/registry" + "github.com/freeCodeCamp/artemis/internal/sitekey" ) type RegistryStore struct { pool *pgxpool.Pool now func() time.Time - onChange func(slug string) + onChange func(slug sitekey.Slug) } func NewRegistryStore(db *DB) *RegistryStore { @@ -27,18 +28,18 @@ func (s *RegistryStore) WithClock(now func() time.Time) *RegistryStore { return s } -func (s *RegistryStore) WithOnChange(fn func(slug string)) *RegistryStore { +func (s *RegistryStore) WithOnChange(fn func(slug sitekey.Slug)) *RegistryStore { s.onChange = fn return s } -func (s *RegistryStore) changed(slug string) { +func (s *RegistryStore) changed(slug sitekey.Slug) { if s.onChange != nil { s.onChange(slug) } } -func (s *RegistryStore) Register(ctx context.Context, slug string, teams []string, createdBy string) (registry.Site, error) { +func (s *RegistryStore) Register(ctx context.Context, slug sitekey.Slug, teams []string, createdBy string) (registry.Site, error) { now := s.now().UTC() teams = append([]string(nil), teams...) tag, err := s.pool.Exec(ctx, @@ -56,7 +57,7 @@ func (s *RegistryStore) Register(ctx context.Context, slug string, teams []strin return registry.Site{Slug: slug, Teams: teams, CreatedAt: now, UpdatedAt: now, CreatedBy: createdBy}, nil } -func (s *RegistryStore) UpdateTeams(ctx context.Context, slug string, teams []string) (registry.Site, error) { +func (s *RegistryStore) UpdateTeams(ctx context.Context, slug sitekey.Slug, teams []string) (registry.Site, error) { now := s.now().UTC() teams = append([]string(nil), teams...) var site registry.Site @@ -74,7 +75,7 @@ func (s *RegistryStore) UpdateTeams(ctx context.Context, slug string, teams []st return site, nil } -func (s *RegistryStore) Delete(ctx context.Context, slug string) error { +func (s *RegistryStore) Delete(ctx context.Context, slug sitekey.Slug) error { tag, err := s.pool.Exec(ctx, `DELETE FROM sites WHERE slug = $1`, slug) if err != nil { return fmt.Errorf("pg registry delete %s: %w", slug, err) @@ -133,7 +134,7 @@ func (s *RegistryStore) Import(ctx context.Context, src SitesSource) (int, error return imported, nil } -func (s *RegistryStore) GetSite(ctx context.Context, slug string) (registry.Site, error) { +func (s *RegistryStore) GetSite(ctx context.Context, slug sitekey.Slug) (registry.Site, error) { var site registry.Site err := s.pool.QueryRow(ctx, `SELECT slug, teams, created_at, updated_at, created_by FROM sites WHERE slug = $1`, diff --git a/internal/pg/registry_import_test.go b/internal/pg/registry_import_test.go index 909a7fb..db1a077 100644 --- a/internal/pg/registry_import_test.go +++ b/internal/pg/registry_import_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "github.com/freeCodeCamp/artemis/internal/registry/valkey" + "github.com/freeCodeCamp/artemis/internal/sitekey" ) func seededValkey(t *testing.T) *valkey.Store { @@ -39,9 +40,9 @@ func TestRegistryImportOnBoot(t *testing.T) { sites, err := pgStore.Sites(ctx) require.NoError(t, err) require.Len(t, sites, 2) - assert.Equal(t, "learn", sites[0].Slug) + assert.Equal(t, sitekey.Slug("learn"), sites[0].Slug) assert.Equal(t, []string{"team-eng"}, sites[0].Teams) - assert.Equal(t, "www", sites[1].Slug) + assert.Equal(t, sitekey.Slug("www"), sites[1].Slug) assert.ElementsMatch(t, []string{"team-eng", "team-platform"}, sites[1].Teams) n2, err := pgStore.Import(ctx, src) @@ -68,7 +69,7 @@ func TestRegistryImportOnBoot_NoClobberWhenPGNonEmpty(t *testing.T) { sites, err := pgStore.Sites(ctx) require.NoError(t, err) require.Len(t, sites, 1, "Valkey rows do not clobber existing PG data") - assert.Equal(t, "www", sites[0].Slug) + assert.Equal(t, sitekey.Slug("www"), sites[0].Slug) assert.Equal(t, []string{"newer-team"}, sites[0].Teams) } diff --git a/internal/pg/registry_test.go b/internal/pg/registry_test.go index 116da7d..5fd4ff6 100644 --- a/internal/pg/registry_test.go +++ b/internal/pg/registry_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "github.com/freeCodeCamp/artemis/internal/registry" + "github.com/freeCodeCamp/artemis/internal/sitekey" ) func newTestRegistry(t *testing.T) *RegistryStore { @@ -18,12 +19,12 @@ func newTestRegistry(t *testing.T) *RegistryStore { func TestRegistryPG(t *testing.T) { ctx := context.Background() - var changed []string - store := newTestRegistry(t).WithOnChange(func(slug string) { changed = append(changed, slug) }) + var changed []sitekey.Slug + store := newTestRegistry(t).WithOnChange(func(slug sitekey.Slug) { changed = append(changed, slug) }) site, err := store.Register(ctx, "www", []string{"team-eng", "team-platform"}, "alice") require.NoError(t, err) - assert.Equal(t, "www", site.Slug) + assert.Equal(t, sitekey.Slug("www"), site.Slug) assert.ElementsMatch(t, []string{"team-eng", "team-platform"}, site.Teams) _, err = store.Register(ctx, "www", []string{"x"}, "bob") @@ -40,7 +41,7 @@ func TestRegistryPG(t *testing.T) { got, err := store.GetSite(ctx, "www") require.NoError(t, err) - assert.Equal(t, "www", got.Slug) + assert.Equal(t, sitekey.Slug("www"), got.Slug) assert.Equal(t, []string{"team-platform"}, got.Teams, "GetSite reflects the latest authoritative row") _, err = store.GetSite(ctx, "absent") assert.ErrorIs(t, err, registry.ErrNotFound) @@ -50,8 +51,8 @@ func TestRegistryPG(t *testing.T) { sites, err := store.Sites(ctx) require.NoError(t, err) require.Len(t, sites, 2) - assert.Equal(t, "learn", sites[0].Slug, "sorted by slug ascending") - assert.Equal(t, "www", sites[1].Slug) + assert.Equal(t, sitekey.Slug("learn"), sites[0].Slug, "sorted by slug ascending") + assert.Equal(t, sitekey.Slug("www"), sites[1].Slug) require.NoError(t, store.Delete(ctx, "www")) assert.ErrorIs(t, store.Delete(ctx, "www"), registry.ErrNotFound, "double delete -> not found") @@ -59,6 +60,6 @@ func TestRegistryPG(t *testing.T) { _, err = store.GetSite(ctx, "www") assert.ErrorIs(t, err, registry.ErrNotFound, "GetSite after delete -> not found") - assert.Equal(t, []string{"www", "www", "learn", "www"}, changed, + assert.Equal(t, []sitekey.Slug{"www", "www", "learn", "www"}, changed, "registry.changed fires on register/update/register/delete for Valkey cache invalidation") } diff --git a/internal/registry/errors_test.go b/internal/registry/errors_test.go index f263f5a..a5108e6 100644 --- a/internal/registry/errors_test.go +++ b/internal/registry/errors_test.go @@ -3,26 +3,28 @@ package registry import ( "context" "fmt" - "sort" + "slices" "testing" "github.com/stretchr/testify/require" + + "github.com/freeCodeCamp/artemis/internal/sitekey" ) type fakeSnapshot struct { - bySite map[string][]string + bySite map[sitekey.Slug][]string } -func (f fakeSnapshot) Sites() []string { - out := make([]string, 0, len(f.bySite)) +func (f fakeSnapshot) Sites() []sitekey.Slug { + out := make([]sitekey.Slug, 0, len(f.bySite)) for k := range f.bySite { out = append(out, k) } - sort.Strings(out) + slices.Sort(out) return out } -func (f fakeSnapshot) TeamsForSite(slug string) []string { +func (f fakeSnapshot) TeamsForSite(slug sitekey.Slug) []string { teams, ok := f.bySite[slug] if !ok { return nil @@ -40,17 +42,17 @@ type fakeWriter struct{} func (fakeWriter) Sites(context.Context) ([]Site, error) { return nil, nil } -func (fakeWriter) Register(context.Context, string, []string, string) (Site, error) { +func (fakeWriter) Register(context.Context, sitekey.Slug, []string, string) (Site, error) { return Site{}, nil } -func (fakeWriter) UpdateTeams(context.Context, string, []string) (Site, error) { +func (fakeWriter) UpdateTeams(context.Context, sitekey.Slug, []string) (Site, error) { return Site{}, nil } -func (fakeWriter) Delete(context.Context, string) error { return nil } +func (fakeWriter) Delete(context.Context, sitekey.Slug) error { return nil } -func (fakeWriter) GetSite(context.Context, string) (Site, error) { return Site{}, nil } +func (fakeWriter) GetSite(context.Context, sitekey.Slug) (Site, error) { return Site{}, nil } var ( _ Snapshot = fakeSnapshot{} @@ -112,14 +114,14 @@ func TestSentinelErrors_WrapPreservesErrorsIs(t *testing.T) { func TestSnapshotContract_TeamsForSiteGatesOnRegistration(t *testing.T) { t.Parallel() - snap := fakeSnapshot{bySite: map[string][]string{ + snap := fakeSnapshot{bySite: map[sitekey.Slug][]string{ "blog": {"news-editors", "platform"}, "internal": {}, }} tests := []struct { name string - slug string + slug sitekey.Slug want []string }{ {"registered site returns its teams", "blog", []string{"news-editors", "platform"}}, @@ -140,11 +142,11 @@ func TestSnapshotContract_TeamsForSiteGatesOnRegistration(t *testing.T) { func TestSnapshotContract_SitesReturnsSortedSlugs(t *testing.T) { t.Parallel() - snap := fakeSnapshot{bySite: map[string][]string{ + snap := fakeSnapshot{bySite: map[sitekey.Slug][]string{ "charlie": {"staff"}, "alpha": {"staff"}, "bravo": {"staff"}, }} - require.Equal(t, []string{"alpha", "bravo", "charlie"}, snap.Sites()) + require.Equal(t, []sitekey.Slug{"alpha", "bravo", "charlie"}, snap.Sites()) } diff --git a/internal/registry/reader.go b/internal/registry/reader.go index 7dff4e1..bc5f06b 100644 --- a/internal/registry/reader.go +++ b/internal/registry/reader.go @@ -5,6 +5,8 @@ // The single implementation lives at internal/registry/valkey. package registry +import "github.com/freeCodeCamp/artemis/internal/sitekey" + // Snapshot is a point-in-time view of the registry. Each call to // Reader.Snapshot returns a freshly captured Snapshot — callers // holding the returned value see a stable view across multiple @@ -13,12 +15,12 @@ type Snapshot interface { // Sites returns the registered slugs in stable (typically sorted) // order. The returned slice is safe to mutate; callers do not // need to copy defensively before iteration. - Sites() []string + Sites() []sitekey.Slug // TeamsForSite returns the GitHub team slugs authorized for the // given site, or nil when the site is not in the registry. The // returned slice is safe to mutate. - TeamsForSite(slug string) []string + TeamsForSite(slug sitekey.Slug) []string } // Reader is the read-side handler-facing contract. Writers use the diff --git a/internal/registry/sitekey_pin_test.go b/internal/registry/sitekey_pin_test.go new file mode 100644 index 0000000..5ef01b7 --- /dev/null +++ b/internal/registry/sitekey_pin_test.go @@ -0,0 +1,17 @@ +package registry + +import ( + "context" + + "github.com/freeCodeCamp/artemis/internal/sitekey" +) + +var ( + _ func(Snapshot) []sitekey.Slug = Snapshot.Sites + _ func(Snapshot, sitekey.Slug) []string = Snapshot.TeamsForSite + _ func(Writer, context.Context, sitekey.Slug) (Site, error) = Writer.GetSite + _ func(Writer, context.Context, sitekey.Slug, []string, string) (Site, error) = Writer.Register + _ func(Writer, context.Context, sitekey.Slug, []string) (Site, error) = Writer.UpdateTeams + _ func(Writer, context.Context, sitekey.Slug) error = Writer.Delete + _ sitekey.Slug = Site{}.Slug +) diff --git a/internal/registry/types.go b/internal/registry/types.go index c43e335..d5a3aba 100644 --- a/internal/registry/types.go +++ b/internal/registry/types.go @@ -4,6 +4,8 @@ import ( "context" "errors" "time" + + "github.com/freeCodeCamp/artemis/internal/sitekey" ) // Site is the in-memory representation of one registry row. It is the @@ -11,7 +13,7 @@ import ( // handlers; backends are responsible for marshalling to/from their // wire encodings (e.g. Valkey hash fields). type Site struct { - Slug string + Slug sitekey.Slug Teams []string CreatedAt time.Time UpdatedAt time.Time @@ -45,17 +47,17 @@ type Writer interface { // reflects the source-of-truth at call time. Sites(ctx context.Context) ([]Site, error) - GetSite(ctx context.Context, slug string) (Site, error) + GetSite(ctx context.Context, slug sitekey.Slug) (Site, error) // Register creates a new site row and publishes a // registry.changed event on success. Returns ErrAlreadyExists // when slug is already registered. - Register(ctx context.Context, slug string, teams []string, createdBy string) (Site, error) + Register(ctx context.Context, slug sitekey.Slug, teams []string, createdBy string) (Site, error) // UpdateTeams replaces the teams list for an existing slug, // stamps updated_at, and publishes a registry.changed event. // Returns ErrNotFound if the slug is absent. - UpdateTeams(ctx context.Context, slug string, teams []string) (Site, error) + UpdateTeams(ctx context.Context, slug sitekey.Slug, teams []string) (Site, error) // Delete removes a slug from the registry (hash row + index set // member) and publishes a registry.changed event. Returns @@ -65,5 +67,5 @@ type Writer interface { // gc-site never fires for the site again. The bytes and index // rows stay until an operator runs `artemis reconcile` or the // slug is re-registered (which resumes normal retention). - Delete(ctx context.Context, slug string) error + Delete(ctx context.Context, slug sitekey.Slug) error } diff --git a/internal/registry/valkey/cutover_test.go b/internal/registry/valkey/cutover_test.go index cf78969..74275f3 100644 --- a/internal/registry/valkey/cutover_test.go +++ b/internal/registry/valkey/cutover_test.go @@ -10,18 +10,19 @@ import ( "github.com/freeCodeCamp/artemis/internal/registry" "github.com/freeCodeCamp/artemis/internal/registry/valkey" + "github.com/freeCodeCamp/artemis/internal/sitekey" ) type stubSource struct { mu sync.Mutex - bySite map[string][]string + bySite map[sitekey.Slug][]string } func newStubSource() *stubSource { - return &stubSource{bySite: map[string][]string{}} + return &stubSource{bySite: map[sitekey.Slug][]string{}} } -func (s *stubSource) set(slug string, teams []string) { +func (s *stubSource) set(slug sitekey.Slug, teams []string) { s.mu.Lock() defer s.mu.Unlock() s.bySite[slug] = append([]string(nil), teams...) @@ -51,7 +52,7 @@ func TestRegistryCutover(t *testing.T) { require.NoError(t, err) snap := reader.Snapshot() - require.Equal(t, []string{"preexisting"}, snap.Sites(), + require.Equal(t, []sitekey.Slug{"preexisting"}, snap.Sites(), "initial read served from PG source via cache-front") require.Equal(t, []string{"staff"}, snap.TeamsForSite("preexisting")) diff --git a/internal/registry/valkey/reader.go b/internal/registry/valkey/reader.go index b182cf4..56d14c4 100644 --- a/internal/registry/valkey/reader.go +++ b/internal/registry/valkey/reader.go @@ -4,12 +4,13 @@ import ( "context" "fmt" "log/slog" - "sort" + "slices" "sync" "sync/atomic" "time" "github.com/freeCodeCamp/artemis/internal/registry" + "github.com/freeCodeCamp/artemis/internal/sitekey" ) // onRefreshErrorFn names the OnRefreshError callback type so it can @@ -67,23 +68,23 @@ func (r *Reader) SetOnRefreshError(f func(error)) { // returned from Sites/TeamsForSite — the snapshot returns fresh // copies on every call. type snapshot struct { - bySite map[string][]string + bySite map[sitekey.Slug][]string } // Sites returns the registered slugs sorted ascending. The returned // slice is a fresh copy; callers may mutate freely. -func (s snapshot) Sites() []string { - out := make([]string, 0, len(s.bySite)) +func (s snapshot) Sites() []sitekey.Slug { + out := make([]sitekey.Slug, 0, len(s.bySite)) for k := range s.bySite { out = append(out, k) } - sort.Strings(out) + slices.Sort(out) return out } // TeamsForSite returns the team slugs authorized for the given site, // or nil when the slug is absent. The returned slice is a fresh copy. -func (s snapshot) TeamsForSite(slug string) []string { +func (s snapshot) TeamsForSite(slug sitekey.Slug) []string { teams, ok := s.bySite[slug] if !ok { return nil @@ -136,7 +137,7 @@ func (r *Reader) Refresh(ctx context.Context) error { if err != nil { return err } - bySite := make(map[string][]string, len(sites)) + bySite := make(map[sitekey.Slug][]string, len(sites)) for _, s := range sites { teams := make([]string, len(s.Teams)) copy(teams, s.Teams) diff --git a/internal/registry/valkey/reader_test.go b/internal/registry/valkey/reader_test.go index 31050cc..c349dfc 100644 --- a/internal/registry/valkey/reader_test.go +++ b/internal/registry/valkey/reader_test.go @@ -10,6 +10,7 @@ import ( "github.com/freeCodeCamp/artemis/internal/registry" "github.com/freeCodeCamp/artemis/internal/registry/valkey" + "github.com/freeCodeCamp/artemis/internal/sitekey" ) // eventually polls fn every 10ms until it returns true or timeout @@ -49,7 +50,7 @@ func TestReader_InitialSnapshotPreloadsState(t *testing.T) { require.NoError(t, err) snap := r.Snapshot() - require.Equal(t, []string{"preexisting"}, snap.Sites()) + require.Equal(t, []sitekey.Slug{"preexisting"}, snap.Sites()) require.Equal(t, []string{"staff"}, snap.TeamsForSite("preexisting")) } diff --git a/internal/registry/valkey/store.go b/internal/registry/valkey/store.go index 1befc4f..210208e 100644 --- a/internal/registry/valkey/store.go +++ b/internal/registry/valkey/store.go @@ -24,6 +24,7 @@ import ( "github.com/redis/go-redis/v9" "github.com/freeCodeCamp/artemis/internal/registry" + "github.com/freeCodeCamp/artemis/internal/sitekey" ) // ChannelRegistryChanged is the pub-sub channel emitted on every @@ -204,12 +205,12 @@ func (s *Store) Subscribe(ctx context.Context) (<-chan string, error) { return out, nil } -func (s *Store) Publish(ctx context.Context, slug string) error { - return s.client.Publish(ctx, ChannelRegistryChanged, slug).Err() +func (s *Store) Publish(ctx context.Context, slug sitekey.Slug) error { + return s.client.Publish(ctx, ChannelRegistryChanged, string(slug)).Err() } -func PublishOnChange(ctx context.Context, store *Store) func(slug string) { - return func(slug string) { +func PublishOnChange(ctx context.Context, store *Store) func(slug sitekey.Slug) { + return func(slug sitekey.Slug) { if err := store.Publish(ctx, slug); err != nil { slog.Warn("registry.publish.failed", "site", slug, "err", err) } @@ -218,8 +219,8 @@ func PublishOnChange(ctx context.Context, store *Store) func(slug string) { // siteKey returns the hash key for a given slug. Defined in one place // so the wire format (`site:`) cannot drift between methods. -func siteKey(slug string) string { - return "site:" + slug +func siteKey(slug sitekey.Slug) string { + return "site:" + string(slug) } // Register writes a new site row atomically and publishes a @@ -227,7 +228,7 @@ func siteKey(slug string) string { // slug is already in the index set; the existing row is left // untouched. All concurrent Register calls for the same slug are // serialized — exactly one succeeds, the rest return ErrAlreadyExists. -func (s *Store) Register(ctx context.Context, slug string, teams []string, createdBy string) (Site, error) { +func (s *Store) Register(ctx context.Context, slug sitekey.Slug, teams []string, createdBy string) (Site, error) { if slug == "" { return Site{}, errors.New("registry: empty slug") } @@ -245,7 +246,7 @@ func (s *Store) Register(ctx context.Context, slug string, teams []string, creat // (first one) or trip the WATCH (rest) and re-read the index; // re-read sees the slug present and returns ErrAlreadyExists. txf := func(tx *redis.Tx) error { - exists, err := tx.SIsMember(ctx, keyAllSites, slug).Result() + exists, err := tx.SIsMember(ctx, keyAllSites, string(slug)).Result() if err != nil { return err } @@ -263,8 +264,8 @@ func (s *Store) Register(ctx context.Context, slug string, teams []string, creat fieldUpdatedAt, site.UpdatedAt.Format(time.RFC3339Nano), fieldCreatedBy, site.CreatedBy, ) - pipe.SAdd(ctx, keyAllSites, slug) - pipe.Publish(ctx, ChannelRegistryChanged, slug) + pipe.SAdd(ctx, keyAllSites, string(slug)) + pipe.Publish(ctx, ChannelRegistryChanged, string(slug)) return nil }) return err @@ -292,7 +293,7 @@ func (s *Store) Register(ctx context.Context, slug string, teams []string, creat // event. Returns ErrNotFound if the slug is not in the index set. // Concurrent updates are serialized via WATCH+MULTI/EXEC on the row // key; the loser of an optimistic-lock race retries and re-reads. -func (s *Store) UpdateTeams(ctx context.Context, slug string, teams []string) (Site, error) { +func (s *Store) UpdateTeams(ctx context.Context, slug sitekey.Slug, teams []string) (Site, error) { if slug == "" { return Site{}, errors.New("registry: empty slug") } @@ -301,7 +302,7 @@ func (s *Store) UpdateTeams(ctx context.Context, slug string, teams []string) (S var resolved Site txf := func(tx *redis.Tx) error { - exists, err := tx.SIsMember(ctx, keyAllSites, slug).Result() + exists, err := tx.SIsMember(ctx, keyAllSites, string(slug)).Result() if err != nil { return err } @@ -327,7 +328,7 @@ func (s *Store) UpdateTeams(ctx context.Context, slug string, teams []string) (S fieldTeams, string(teamsJSON), fieldUpdatedAt, now.Format(time.RFC3339Nano), ) - pipe.Publish(ctx, ChannelRegistryChanged, slug) + pipe.Publish(ctx, ChannelRegistryChanged, string(slug)) return nil }) if err != nil { @@ -361,12 +362,12 @@ func (s *Store) UpdateTeams(ctx context.Context, slug string, teams []string) (S // slug is absent. R2 deploy bytes are NOT touched, and no job // collects them afterwards — see registry.Writer.Delete for the // retention consequences. -func (s *Store) Delete(ctx context.Context, slug string) error { +func (s *Store) Delete(ctx context.Context, slug sitekey.Slug) error { if slug == "" { return errors.New("registry: empty slug") } txf := func(tx *redis.Tx) error { - exists, err := tx.SIsMember(ctx, keyAllSites, slug).Result() + exists, err := tx.SIsMember(ctx, keyAllSites, string(slug)).Result() if err != nil { return err } @@ -375,8 +376,8 @@ func (s *Store) Delete(ctx context.Context, slug string) error { } _, err = tx.TxPipelined(ctx, func(pipe redis.Pipeliner) error { pipe.Del(ctx, siteKey(slug)) - pipe.SRem(ctx, keyAllSites, slug) - pipe.Publish(ctx, ChannelRegistryChanged, slug) + pipe.SRem(ctx, keyAllSites, string(slug)) + pipe.Publish(ctx, ChannelRegistryChanged, string(slug)) return nil }) return err @@ -398,7 +399,7 @@ func (s *Store) Delete(ctx context.Context, slug string) error { // TeamsForSite returns the authorized teams for a slug or // ErrNotFound when the slug is absent. Callers MUST treat the slice // as read-only; the package returns a fresh copy per call. -func (s *Store) TeamsForSite(ctx context.Context, slug string) ([]string, error) { +func (s *Store) TeamsForSite(ctx context.Context, slug sitekey.Slug) ([]string, error) { site, err := s.GetSite(ctx, slug) if err != nil { return nil, err @@ -409,7 +410,7 @@ func (s *Store) TeamsForSite(ctx context.Context, slug string) ([]string, error) // GetSite returns the full Site row or ErrNotFound. Used by the // list endpoint to enumerate metadata; callers that only need the // teams list should use TeamsForSite. -func (s *Store) GetSite(ctx context.Context, slug string) (Site, error) { +func (s *Store) GetSite(ctx context.Context, slug sitekey.Slug) (Site, error) { values, err := s.client.HGetAll(ctx, siteKey(slug)).Result() if err != nil { return Site{}, err @@ -434,7 +435,8 @@ func (s *Store) Sites(ctx context.Context) ([]Site, error) { } sort.Strings(slugs) out := make([]Site, 0, len(slugs)) - for _, slug := range slugs { + for _, raw := range slugs { + slug := sitekey.Slug(raw) values, err := s.client.HGetAll(ctx, siteKey(slug)).Result() if err != nil { return nil, err @@ -455,7 +457,7 @@ func (s *Store) Sites(ctx context.Context) ([]Site, error) { // decodeSite parses the raw hash fields back into a Site. Wire // format (JSON teams, RFC3339Nano timestamps) is enforced here. -func decodeSite(slug string, values map[string]string) (Site, error) { +func decodeSite(slug sitekey.Slug, values map[string]string) (Site, error) { site := Site{Slug: slug, CreatedBy: values[fieldCreatedBy]} if raw, ok := values[fieldTeams]; ok && raw != "" { if err := json.Unmarshal([]byte(raw), &site.Teams); err != nil { diff --git a/internal/registry/valkey/store_test.go b/internal/registry/valkey/store_test.go index b40a9a9..c00f115 100644 --- a/internal/registry/valkey/store_test.go +++ b/internal/registry/valkey/store_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" "github.com/freeCodeCamp/artemis/internal/registry/valkey" + "github.com/freeCodeCamp/artemis/internal/sitekey" ) // newMiniredis returns a miniredis server seeded with the given @@ -114,7 +115,7 @@ func TestStore_Register_HappyPath(t *testing.T) { got, err := s.Register(ctx, "blog", []string{"news-editors", "platform"}, "alice") require.NoError(t, err) - require.Equal(t, "blog", got.Slug) + require.Equal(t, sitekey.Slug("blog"), got.Slug) require.Equal(t, []string{"news-editors", "platform"}, got.Teams) require.Equal(t, "alice", got.CreatedBy) require.False(t, got.CreatedAt.IsZero()) @@ -253,7 +254,7 @@ func TestStore_Sites_EnumeratesSorted(t *testing.T) { s, _, _ := newStore(t) ctx := context.Background() - for _, slug := range []string{"charlie", "alpha", "bravo"} { + for _, slug := range []sitekey.Slug{"charlie", "alpha", "bravo"} { _, err := s.Register(ctx, slug, []string{"staff"}, "alice") require.NoError(t, err) } @@ -261,9 +262,9 @@ func TestStore_Sites_EnumeratesSorted(t *testing.T) { all, err := s.Sites(ctx) require.NoError(t, err) require.Len(t, all, 3) - require.Equal(t, "alpha", all[0].Slug) - require.Equal(t, "bravo", all[1].Slug) - require.Equal(t, "charlie", all[2].Slug) + require.Equal(t, sitekey.Slug("alpha"), all[0].Slug) + require.Equal(t, sitekey.Slug("bravo"), all[1].Slug) + require.Equal(t, sitekey.Slug("charlie"), all[2].Slug) } func TestStore_Sites_EmptyWhenUnregistered(t *testing.T) { @@ -290,7 +291,7 @@ func TestStore_UpdateTeams_HappyPath(t *testing.T) { updated, err := s.UpdateTeams(ctx, "blog", []string{"news-editors", "platform"}) require.NoError(t, err) - require.Equal(t, "blog", updated.Slug) + require.Equal(t, sitekey.Slug("blog"), updated.Slug) require.Equal(t, []string{"news-editors", "platform"}, updated.Teams) require.Equal(t, "alice", updated.CreatedBy, "created_by must round-trip") require.True(t, updated.CreatedAt.Equal(original.CreatedAt), "created_at frozen") @@ -403,7 +404,7 @@ func TestStore_Subscribe_DeliversInOrder(t *testing.T) { want := []string{"alpha", "bravo", "charlie"} for _, slug := range want { - _, err := s.Register(ctx, slug, []string{"staff"}, "alice") + _, err := s.Register(ctx, sitekey.Slug(slug), []string{"staff"}, "alice") require.NoError(t, err) } diff --git a/internal/server/timeout_test.go b/internal/server/timeout_test.go index 28e7979..7cc290f 100644 --- a/internal/server/timeout_test.go +++ b/internal/server/timeout_test.go @@ -13,12 +13,14 @@ import ( "github.com/freeCodeCamp/artemis/internal/registry" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/freeCodeCamp/artemis/internal/sitekey" ) type stubSnapshot struct{} -func (stubSnapshot) Sites() []string { return []string{"www"} } -func (stubSnapshot) TeamsForSite(string) []string { return []string{"team-eng"} } +func (stubSnapshot) Sites() []sitekey.Slug { return []sitekey.Slug{"www"} } +func (stubSnapshot) TeamsForSite(sitekey.Slug) []string { return []string{"team-eng"} } type stubSites struct{}