From b100b18fbee2feba7711b38539841f1f6d14e435 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:04:46 -0500 Subject: [PATCH 01/30] docs: add Azure support implementation plan Covers Blob/Queue/Table Storage and Cosmos DB wire-compatibility scoping, package layout, routing/port strategy, auth strategy, and milestone breakdown ahead of the M0 (Blob skeleton) implementation. Co-Authored-By: Claude Sonnet 5 --- AZURE.md | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 AZURE.md diff --git a/AZURE.md b/AZURE.md new file mode 100644 index 0000000000..d612c2a36e --- /dev/null +++ b/AZURE.md @@ -0,0 +1,94 @@ +# Azure Support Implementation Plan for gopherstack + +Repo cloned and inspected at `/private/tmp/claude-503/-Users-jacob-hochstetler-Code/926630d2-46b5-4853-a931-b4ea837b9658/scratchpad/gopherstack` (module `github.com/blackbirdworks/gopherstack`, GitHub `jh125486/gopherstack`). Findings below are grounded in that source (paths cited); Azure/Azurite mechanics are standard public documentation. + +## 1. How gopherstack is built today (relevant facts) + +- **Per-service package layout** (`services//`): `provider.go` (implements `service.Provider`: `Name()`, `Init(ctx *service.AppContext) (service.Registerable, error)`), `handler.go` (implements `service.Registerable`: `Name()`, `Handler() echo.HandlerFunc`, `RouteMatcher() service.Matcher`, `MatchPriority() int`, `ExtractOperation`/`ExtractResource` for metrics), `store.go` (`InMemoryBackend`), `interfaces.go` (`StorageBackend` interface for testability), `persistence.go` (versioned JSON snapshot/restore), `settings.go`, `janitor.go` (TTL sweeper), `errors.go`, `README.md` + `PARITY.md`. Examples inspected: `services/s3/*`, `services/sqs/*`, `services/dynamodb/*`. +- **Central plumbing** (`pkgs/service/service.go`, `router.go`, `priorities.go`): one Echo HTTP server, one port. Every service registers a `Registerable`; a `Router` sorts all registered matchers by `MatchPriority()` (100=exact header, 95=partial header, 90=form POST, 85=versioned path, 80=standard form, 75=target-prefixed, 50=path UI, 0=catch-all) and evaluates them per request, first match wins. `AppContext{Config, JanitorCtx, Logger, PortAlloc, JanitorTimeout}` is passed to every `Provider.Init`. Optional interfaces a handler can also implement: `DashboardProvider`, `ChaosProvider`, `BackgroundWorker`, `Shutdowner`, `Resettable`, `Purgeable`, `FISActionProvider`. +- **Registration**: new services are added to a flat `[]service.Provider` literal in `cli.go` (`getCoreServiceProviders`/`getRemainingServiceProviders`). +- **Auth**: `services/s3/sigv4.go` shows the house style — SigV4/SigV2 `Authorization` headers are *structurally* parsed and validated, but cryptographic signature verification is opt-in (`WithPresignValidation`/`PresignSecret`); with no secret configured, any credentials are accepted. This is functionally identical to Azurite's fixed-dev-key model, which derisks the whole Azure effort. +- **Persistence**: versioned snapshot structs (`backendSnapshot{Version int, ...}`) restored on boot, guarded against shape drift. +- **Docs/parity machinery**: each service ships a `PARITY.md` (per-op table: `wire`/`errors`/`state`/`persist` status, known gaps, deferred items, audited against a pinned SDK module version) that `cmd/gendocs` renders into the service `README.md` and the root `README.md` services table/badges. +- **Testing conventions**: table-driven unit tests colocated with source, `export_test.go` for whitebox access, `leak_test.go`/`leak_main_test.go` (goroutine-leak checks), `bench_test.go`. `test/integration/_test.go` uses the real upstream AWS SDK (`aws-sdk-go-v2`) against a running instance (via testcontainers), doing full lifecycle flows (e.g. `TestIntegration_SQS_QueueLifecycle`: create → list → get-attrs → delete → verify-gone). No non-Go-SDK wire tests currently exist in the repo. + +## 2. Azure wire-protocol minimums (Azurite-equivalent) + +**Azurite's dev-auth model** (the shape to copy): a fixed well-known account, `devstoreaccount1`, with a fixed published key (`Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==`), reachable via `UseDevelopmentStorage=true` or an explicit connection string (`AccountName=devstoreaccount1;AccountKey=...;BlobEndpoint=http://127.0.0.1:10000/devstoreaccount1;QueueEndpoint=http://127.0.0.1:10001/devstoreaccount1;TableEndpoint=http://127.0.0.1:10002/devstoreaccount1`). SDKs sign requests with SharedKey (HMAC-SHA256 over a canonicalized string, key is the base64-decoded account key) into `Authorization: SharedKey devstoreaccount1:`. Critically, **Azurite runs Blob/Queue/Table on three separate ports (10000/10001/10002)** rather than multiplexing one port — because the path shape (`//`) is otherwise ambiguous across the three services. + +**Blob** (REST+XML, `x-ms-version` header): `GET /?comp=list` (list containers); `PUT`/`DELETE //?restype=container` (create/delete container); `PUT ///` with `x-ms-blob-type` (put blob), `GET`/`HEAD` (get blob/properties, Range support), `DELETE` (delete blob), `GET //?restype=container&comp=list` (list blobs). Large-object upload uses Put Block + Put Block List — deferrable to a later pass. + +**Queue** (REST+XML): `GET /?comp=list`; `PUT`/`DELETE //`; message ops under `///messages`: `POST` (put), `GET` (get, with `numofmessages`/`visibilitytimeout`), `GET ?peekonly=true` (peek), `DELETE /messages/?popreceipt=` (delete), `PUT` (update visibility), `DELETE /messages` (clear). + +**Table** (REST+JSON/OData, `Accept: application/json;odata=nometadata`): `POST //Tables` (create), `DELETE //Tables('')`, `GET //Tables` (list); entities: `POST //` (insert), `PUT`/`MERGE` to `//
(PartitionKey='..',RowKey='..')` (replace/merge, `If-Match` for optimistic concurrency), `DELETE` same path, `GET //
()?$filter=...` (query). Batch (`POST //$batch`, multipart/mixed changesets) is real-world-important but complex — good candidate for a later milestone. **Important reuse fact: Cosmos DB's Table API is the same wire protocol as Azure Table Storage** (same OData entity model, different auth/endpoint), so Table Storage work is directly reusable for a future Cosmos Table API surface. + +**Cosmos DB (Core/SQL API)** is REST+JSON over HTTPS (not just the gRPC/TCP "direct mode" some SDKs also support — the Gateway/REST mode is what unmodified SDKs fall back to and is what's feasible to emulate). Resource hierarchy: `/dbs` → `/dbs//colls` → `/dbs//colls//docs`. Auth header: `Authorization: type=master&ver=1.0&sig=` (URL-encoded), signed with the base64 master key over verb+resourceType+resourceId+date; plus `x-ms-date` and `x-ms-version` headers, and `x-ms-documentdb-partitionkey` on point ops. **The real Cosmos DB Local Emulator uses a fixed, publicly documented well-known master key** (`C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==`) at a fixed endpoint (`https://localhost:8081` for the real emulator) — exactly the same "one fixed dev secret" pattern as Azurite, reinforcing that gopherstack's existing permissive-by-default auth model is the right target here too. Queries execute via `POST .../docs` with `x-ms-documentdb-isquery: True` and a `{"query": "...", "parameters": [...]}` body — needs a small SQL-like query engine, structurally similar to work gopherstack has already done (S3 Select has a full SQL tokenizer/parser/executor in `services/s3/select_sql_*.go`; DynamoDB has its own expression engine in `services/dynamodb/expr/`). Real SDKs also expect `x-ms-request-charge`, `x-ms-session-token`, and `etag` on responses — MVP can return static/fake values rather than real RU accounting. + +**Prior art**: an existing open-source lightweight Cosmos DB emulator, **cosmium** (github.com/pikami/cosmium), is worth reviewing directly when scoping the Cosmos milestone (M4) — it has already solved the "how minimal can this REST surface be and still satisfy real SDKs" problem for Cosmos specifically, and is a useful reference for the resource model, the SQL-query subset, and the fake RU/session-token response shape. + +## 3. Proposed package layout + +``` +services/azureblob/ provider.go handler.go store.go interfaces.go persistence.go + settings.go janitor.go errors.go models.go + blob_ops.go container_ops.go conditional.go + README.md PARITY.md *_test.go export_test.go +services/azurequeue/ (same skeleton) queue_ops.go message_ops.go visibility.go +services/azuretable/ (same skeleton) table_ops.go entity_ops.go odata_filter.go +services/cosmosdb/ (same skeleton) database_ops.go container_ops.go document_ops.go + sql_query.go (mini SQL parser/executor; reuse patterns from + services/s3/select_sql_*.go and services/dynamodb/expr/; + cross-check minimal-surface scoping against cosmium) +pkgs/azureauth/ sharedkey.go (SharedKey/SharedKeyLite canonicalization + the + fixed devstoreaccount1 name/key constants) — shared across + blob/queue/table since, unlike AWS's per-service SigV4 + variance, Azure Storage's SharedKey algorithm is identical + across all three; Cosmos's master-key HMAC scheme is + different enough to live in services/cosmosdb/ instead. +``` + +## 4. Routing/port strategy (the one real architectural decision) + +gopherstack's flagship pattern is single-port, priority-matcher multiplexing — but that only works because AWS services are disambiguated by headers (`X-Amz-Target`) or distinctive path/form shapes. Azure Blob/Queue/Table share the *same* `//` path shape with no service-identifying header, so multiplexing them on one port risks exactly the collision the AWS router avoids by construction. + +**Recommendation: give each Azure service its own port, mirroring Azurite's own 10000/10001/10002 convention (and Cosmos its own port, mirroring the real emulator's fixed 8081 default).** This is *more* wire-compatible, not less, since SDKs' default connection strings/emulator constants already assume separate ports. gopherstack already has the machinery for this — `AppContext.PortAlloc *portalloc.Allocator` is used elsewhere (e.g. EC2-docker SSH port ranges) for per-resource port allocation, so per-service dedicated listeners are an established pattern, not a new one. Each Azure service's `Provider.Init` stands up its own `echo.Echo` (or shares Echo's engine but binds a second listener), independent of the AWS `Router`. + +## 5. Auth/connection-string strategy per service + +- **Blob/Queue/Table**: default to the fixed Azurite account name/key pair (`devstoreaccount1` / the published emulator key) so `UseDevelopmentStorage=true` and unmodified Azurite-targeting SDK config work out of the box. `Authorization: SharedKey ...` headers are parsed structurally (account name extraction for routing/logging); cryptographic verification is opt-in via a `WithSharedKeyValidation` toggle — directly mirroring `services/s3`'s `PresignSecret`/`WithPresignValidation` opt-in pattern. Env var overrides (`AZURE_STORAGE_ACCOUNT`, `AZURE_STORAGE_KEY`) for anyone who wants a non-default identity. +- **Cosmos**: default to the real emulator's published fixed master key at a configurable local endpoint; `Authorization: type=master&ver=1.0&sig=...` parsed structurally, verification opt-in the same way. +- **TLS**: Cosmos SDKs often default to HTTPS even against local emulators, requiring an explicit "allow insecure/disable SSL verification" client flag pointed at plain HTTP — document that flag first (matches gopherstack's lightweight, no-extra-infra ethos); a self-signed cert on the Cosmos listener is a stretch goal, not MVP. + +## 6. Implementation order and rationale + +1. **Azure Blob Storage** — closest analog to S3 (gopherstack's most mature service); reuses S3's XML serialization, chunked-upload, conditional-header, checksum, and persistence-snapshot patterns almost directly. Lowest risk, highest day-to-day dev-workflow value, fastest to land. +2. **Azure Queue Storage** — also REST+XML (reuses Blob's XML plumbing), and its message lifecycle (visibility timeout, pop receipts) maps closely onto concepts gopherstack already solved for SQS. +3. **Azure Table Storage** — introduces the OData/JSON entity model and a `$filter` mini-grammar; partition-key/row-key semantics map onto DynamoDB's hash/range-key model, so the `services/dynamodb/expr` experience transfers directly to the `$filter` parser. +4. **Cosmos DB (Core/SQL API)** — most complex: master-key HMAC auth, partition-key routing, and a real SQL-like query subset (`SELECT * FROM c WHERE ...`), best tackled last so it can borrow the SQL-parsing muscle built for S3 Select and the entity/query plumbing built for Table Storage (Cosmos's own Table API is literally the Table Storage protocol, a natural stretch goal once both exist). Cross-check scope against cosmium's existing implementation before locking the op list. + +## 7. Testing strategy (matches repo conventions) + +- **Unit**: table-driven Go tests colocated per file, `t.Parallel()`, `t.Context()`, `export_test.go` for whitebox internals, `leak_test.go`/`leak_main_test.go` for janitor goroutine hygiene, `bench_test.go` for hot paths (blob PUT/GET, table query). +- **Integration** (`test/integration/azureblob_test.go`, `azurequeue_test.go`, `azuretable_test.go`, `cosmosdb_test.go`): use the real `azure-sdk-for-go` client against a running instance via testcontainers, exercising full CRUD lifecycles exactly like `TestIntegration_SQS_QueueLifecycle` (create → list → operate → delete → verify-gone). +- **Cross-SDK wire-compat smoke tests** (the one genuine gap versus existing conventions, since the repo's own suite is Go-SDK-only, but this task's hard requirement is multi-SDK compatibility): add minimal fixtures under the existing `test/e2e/` directory — a short Node script using `@azure/storage-blob`/`@azure/data-tables`/`@azure/cosmos` and a short Python script using `azure-storage-blob`/`azure-cosmos`, run in CI against a live gopherstack instance, proving JS/Python SDKs work unmodified (not just the Go SDK). +- **PARITY.md** seeded per service from day one in the established format (per-op `wire`/`errors`/`state`/`persist` status, known gaps, deferred items, audited against a pinned SDK module version per language — pick `azure-sdk-for-go` as the canonical pinned version for the audit), wired into `cmd/gendocs` so it flows into the service `README.md` and the root README's services table/badges like every existing service. + +## 8. Milestones + +- **M0** — `pkgs/azureauth` (SharedKey canonicalization + fixed devstoreaccount1 constants); `services/azureblob` skeleton wired to its own port via `PortAlloc`; Create/Delete/List Container, Put/Get/Delete Blob, List Blobs; seeded `PARITY.md`; unit tests + one Go integration test. +- **M1** — Blob completeness: properties/metadata, block-blob multipart (Put Block/Put Block List), conditional headers (`If-Match`/`If-None-Match`), error-mapping table (mirrors `services/sqs`'s `errorDetails` pattern). +- **M2** — `services/azurequeue`: full CRUD + message lifecycle (put/get/peek/delete/update/clear), visibility timeout. +- **M3** — `services/azuretable`: table CRUD, entity insert/get/query/update/merge/delete, `$filter` subset (eq/ne/lt/gt/and/or on partition/row key plus scalar properties), ETag-based optimistic concurrency. +- **M4** — `services/cosmosdb`: database/container CRUD (with partition-key-path declaration), document CRUD, SQL-subset query engine, fake RU/session-token/etag headers; scope the op list against cosmium (github.com/pikami/cosmium) as reference prior art; integration tests against `azure-sdk-for-go`, `azure-sdk-for-js`, and `azure-cosmos` (Python). +- **M5** — Docs/polish: root README services table + badges/icons, `docs/services/*.md` guides, a docker-compose example under `examples/`, and the `test/e2e` cross-SDK smoke suite covering all four services. + +## Key files referenced + +- `/private/tmp/.../gopherstack/pkgs/service/service.go`, `router.go`, `priorities.go` — routing/registration contracts +- `/private/tmp/.../gopherstack/services/s3/provider.go`, `sigv4.go`, `persistence.go` — provider pattern, auth-opt-in pattern, snapshot pattern +- `/private/tmp/.../gopherstack/services/sqs/handler.go`, `provider.go` — handler/dispatch pattern, error-table pattern +- `/private/tmp/.../gopherstack/services/dynamodb/expr/` — expression-parser precedent for Table Storage's `$filter` +- `/private/tmp/.../gopherstack/services/s3/select_sql_*.go` — SQL-parser precedent for Cosmos queries +- `/private/tmp/.../gopherstack/cli.go` (`getServiceProviders`) — service registration list +- `/private/tmp/.../gopherstack/test/integration/sqs_test.go` — integration test convention +- `/private/tmp/.../gopherstack/services/sqs/PARITY.md`, `README.md` — parity-doc format From b45f8118460b414abb517dddcbccf4cd5df2ecf5 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:08:57 -0500 Subject: [PATCH 02/30] azureauth: add package doc and Azurite dev account constants --- pkgs/azureauth/azureauth.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 pkgs/azureauth/azureauth.go diff --git a/pkgs/azureauth/azureauth.go b/pkgs/azureauth/azureauth.go new file mode 100644 index 0000000000..1f9d7a3231 --- /dev/null +++ b/pkgs/azureauth/azureauth.go @@ -0,0 +1,30 @@ +// Package azureauth implements Azure Storage's SharedKey and SharedKeyLite +// Authorization-header scheme: structural parsing of the header, the +// canonicalization + HMAC-SHA256 signing algorithm described in Microsoft's +// "Authorize with Shared Key" REST reference, and an explicit, opt-in +// verification call. +// +// It is intentionally self-contained (standard library only) so it can be +// shared by every Azure-flavoured service package (blob, queue, table, +// cosmosdb) without any of them depending on one another. +// +// Mirroring the rest of gopherstack's auth philosophy (see +// services/s3/sigv4.go), this package is permissive by default: parsing an +// Authorization header never fails because a signature happens to be wrong, +// and nothing in the parsing path performs cryptographic verification. +// Callers that want enforcement call [VerifySharedKey] explicitly. +package azureauth + +// Azurite well-known development storage account. Real Azure SDKs configured +// with UseDevelopmentStorage=true, or an explicit connection string naming +// this account, sign requests with this fixed key — this is the same +// "fixed public dev secret" model Azurite itself uses, and lets gopherstack +// accept unmodified Azurite-targeting client configuration out of the box. +const ( + // DefaultAccountName is Azurite's fixed development storage account name. + DefaultAccountName = "devstoreaccount1" + + // DefaultAccountKey is Azurite's fixed, publicly published development + // storage account key (base64-encoded). + DefaultAccountKey = "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==" +) From 590d2ea96bc738d62e2b25e09bc1a2302acb201f Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:09:07 -0500 Subject: [PATCH 03/30] azureauth: add SharedKey Authorization header parsing --- pkgs/azureauth/header.go | 82 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 pkgs/azureauth/header.go diff --git a/pkgs/azureauth/header.go b/pkgs/azureauth/header.go new file mode 100644 index 0000000000..1dc141e45f --- /dev/null +++ b/pkgs/azureauth/header.go @@ -0,0 +1,82 @@ +package azureauth + +import "strings" + +// Scheme identifies which Azure Storage Authorization scheme a header uses. +type Scheme int + +const ( + // SchemeSharedKey is the standard "SharedKey :" scheme. + SchemeSharedKey Scheme = iota + // SchemeSharedKeyLite is the legacy "SharedKeyLite :" + // scheme, kept for older clients and the Table service. + SchemeSharedKeyLite +) + +// String returns the wire name of the scheme ("SharedKey" or "SharedKeyLite"). +func (s Scheme) String() string { + if s == SchemeSharedKeyLite { + return "SharedKeyLite" + } + + return "SharedKey" +} + +const ( + sharedKeyPrefix = "SharedKey " + sharedKeyLitePrefix = "SharedKeyLite " +) + +// Authorization is the parsed form of an Azure Storage Authorization header: +// "SharedKey :" or "SharedKeyLite :". +type Authorization struct { + // Account is the storage account name that signed the request. + Account string + // Signature is the base64-encoded HMAC-SHA256 signature the client sent. + Signature string + // Scheme is which of SharedKey / SharedKeyLite was used. + Scheme Scheme +} + +// ParseAuthorizationHeader parses the value of an Authorization header +// carrying a SharedKey or SharedKeyLite credential. The bool return is false +// when the header is empty, uses an unrecognised scheme, or is missing the +// account name or signature — it never inspects or validates the signature +// itself, so a structurally well-formed header with a wrong signature still +// parses successfully. Use [VerifySharedKey] to check the signature. +func ParseAuthorizationHeader(header string) (Authorization, bool) { + if header == "" { + return Authorization{}, false + } + + scheme := SchemeSharedKey + + tail, ok := strings.CutPrefix(header, sharedKeyLitePrefix) + if ok { + scheme = SchemeSharedKeyLite + } else { + tail, ok = strings.CutPrefix(header, sharedKeyPrefix) + } + + if !ok { + return Authorization{}, false + } + + account, signature, found := strings.Cut(tail, ":") + if !found || account == "" || signature == "" { + return Authorization{}, false + } + + // Reject embedded whitespace: a well-formed credential never contains + // spaces, so this catches trailing/leading junk appended to either field. + if containsSpace(account) || containsSpace(signature) { + return Authorization{}, false + } + + return Authorization{Scheme: scheme, Account: account, Signature: signature}, true +} + +// containsSpace reports whether s contains any ASCII whitespace. +func containsSpace(s string) bool { + return strings.ContainsAny(s, " \t\r\n") +} From 340f0ace90cc6b27ce5d354343565fb7ec21321d Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:09:08 -0500 Subject: [PATCH 04/30] azureauth: add SharedKey/SharedKeyLite canonicalization and string-to-sign --- pkgs/azureauth/canonical.go | 169 ++++++++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+) create mode 100644 pkgs/azureauth/canonical.go diff --git a/pkgs/azureauth/canonical.go b/pkgs/azureauth/canonical.go new file mode 100644 index 0000000000..d0e8c0ad81 --- /dev/null +++ b/pkgs/azureauth/canonical.go @@ -0,0 +1,169 @@ +package azureauth + +import ( + "net/http" + "net/url" + "sort" + "strconv" + "strings" +) + +// StringToSign builds the SharedKey string-to-sign for r, canonicalized +// against the given storage account name, per Microsoft's "Authorize with +// Shared Key" REST reference: +// +// VERB + "\n" + +// Content-Encoding + "\n" + +// Content-Language + "\n" + +// Content-Length + "\n" + +// Content-MD5 + "\n" + +// Content-Type + "\n" + +// Date + "\n" + +// If-Modified-Since + "\n" + +// If-Match + "\n" + +// If-None-Match + "\n" + +// If-Unmodified-Since + "\n" + +// Range + "\n" + +// CanonicalizedHeaders + +// CanonicalizedResource +func StringToSign(r *http.Request, account string) string { + var b strings.Builder + + b.WriteString(r.Method) + b.WriteByte('\n') + writeHeaderLine(&b, r, "Content-Encoding") + writeHeaderLine(&b, r, "Content-Language") + b.WriteString(contentLengthField(r)) + b.WriteByte('\n') + writeHeaderLine(&b, r, "Content-MD5") + writeHeaderLine(&b, r, "Content-Type") + writeHeaderLine(&b, r, "Date") + writeHeaderLine(&b, r, "If-Modified-Since") + writeHeaderLine(&b, r, "If-Match") + writeHeaderLine(&b, r, "If-None-Match") + writeHeaderLine(&b, r, "If-Unmodified-Since") + writeHeaderLine(&b, r, "Range") + b.WriteString(CanonicalizedHeaders(r)) + b.WriteString(CanonicalizedResource(account, r.URL)) + + return b.String() +} + +// StringToSignLite builds the SharedKeyLite string-to-sign for r: +// +// VERB + "\n" + +// Content-MD5 + "\n" + +// Content-Type + "\n" + +// Date + "\n" + +// CanonicalizedHeaders + +// CanonicalizedResource +func StringToSignLite(r *http.Request, account string) string { + var b strings.Builder + + b.WriteString(r.Method) + b.WriteByte('\n') + writeHeaderLine(&b, r, "Content-MD5") + writeHeaderLine(&b, r, "Content-Type") + writeHeaderLine(&b, r, "Date") + b.WriteString(CanonicalizedHeaders(r)) + b.WriteString(CanonicalizedResource(account, r.URL)) + + return b.String() +} + +// writeHeaderLine appends "\n" for the named header to b. +func writeHeaderLine(b *strings.Builder, r *http.Request, name string) { + b.WriteString(r.Header.Get(name)) + b.WriteByte('\n') +} + +// contentLengthField returns the Content-Length string-to-sign field: the +// decimal length, or the empty string when the length is zero or unset (per +// the Shared Key spec, which treats a zero Content-Length as blank). +func contentLengthField(r *http.Request) string { + if r.ContentLength <= 0 { + return "" + } + + return strconv.FormatInt(r.ContentLength, 10) +} + +// CanonicalizedHeaders returns the canonicalized x-ms-* header block: every +// x-ms-* header, lowercased and sorted lexicographically by name, each +// rendered as "name:value\n" with whitespace trimmed and internal whitespace +// runs collapsed to a single space. Multiple values for the same header are +// comma-joined in the order net/http returns them. +func CanonicalizedHeaders(r *http.Request) string { + names := make([]string, 0, len(r.Header)) + seen := make(map[string]struct{}, len(r.Header)) + + for k := range r.Header { + lk := strings.ToLower(k) + if !strings.HasPrefix(lk, "x-ms-") { + continue + } + if _, ok := seen[lk]; ok { + continue + } + seen[lk] = struct{}{} + names = append(names, lk) + } + + sort.Strings(names) + + var b strings.Builder + for _, name := range names { + vals := r.Header.Values(http.CanonicalHeaderKey(name)) + for i, v := range vals { + vals[i] = collapseWhitespace(v) + } + b.WriteString(name) + b.WriteByte(':') + b.WriteString(strings.Join(vals, ",")) + b.WriteByte('\n') + } + + return b.String() +} + +// collapseWhitespace trims leading/trailing whitespace and collapses any +// internal run of whitespace to a single space. +func collapseWhitespace(s string) string { + return strings.Join(strings.Fields(s), " ") +} + +// CanonicalizedResource returns the canonicalized resource string for a +// request against account: "/" followed by each query +// parameter, lowercased and sorted by name, rendered as "\nname:value" with +// repeated-parameter values comma-joined after sorting. +func CanonicalizedResource(account string, u *url.URL) string { + var b strings.Builder + + b.WriteByte('/') + b.WriteString(account) + b.WriteString(u.EscapedPath()) + + query := u.Query() + lowered := make(map[string][]string, len(query)) + for k, vals := range query { + lk := strings.ToLower(k) + lowered[lk] = append(lowered[lk], vals...) + } + + keys := make([]string, 0, len(lowered)) + for k := range lowered { + keys = append(keys, k) + } + sort.Strings(keys) + + for _, k := range keys { + vals := lowered[k] + sort.Strings(vals) + b.WriteByte('\n') + b.WriteString(k) + b.WriteByte(':') + b.WriteString(strings.Join(vals, ",")) + } + + return b.String() +} From 6fe548d51697a6ab503e5e569ae1b875028b98d4 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:09:08 -0500 Subject: [PATCH 05/30] azureauth: add SharedKey signing and opt-in verification --- pkgs/azureauth/sign.go | 76 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 pkgs/azureauth/sign.go diff --git a/pkgs/azureauth/sign.go b/pkgs/azureauth/sign.go new file mode 100644 index 0000000000..a492f24c27 --- /dev/null +++ b/pkgs/azureauth/sign.go @@ -0,0 +1,76 @@ +package azureauth + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/base64" + "errors" + "fmt" + "net/http" +) + +// ErrMalformedAuthorization is returned by [VerifySharedKey] when the +// request's Authorization header cannot be parsed as SharedKey or +// SharedKeyLite. +var ErrMalformedAuthorization = errors.New("azureauth: malformed Authorization header") + +// SignSharedKey computes the SharedKey signature for r as account, using +// accountKey (base64-encoded, e.g. [DefaultAccountKey]). It does not modify r +// or set the Authorization header; callers that want a signed request do that +// themselves with the returned signature. +func SignSharedKey(r *http.Request, account, accountKey string) (string, error) { + return sign(accountKey, StringToSign(r, account)) +} + +// SignSharedKeyLite computes the SharedKeyLite signature for r as account, +// using accountKey (base64-encoded). +func SignSharedKeyLite(r *http.Request, account, accountKey string) (string, error) { + return sign(accountKey, StringToSignLite(r, account)) +} + +// sign returns base64(HMAC-SHA256(base64decode(accountKey), stringToSign)). +func sign(accountKey, stringToSign string) (string, error) { + key, err := base64.StdEncoding.DecodeString(accountKey) + if err != nil { + return "", fmt.Errorf("azureauth: decode account key: %w", err) + } + + mac := hmac.New(sha256.New, key) + mac.Write([]byte(stringToSign)) + + return base64.StdEncoding.EncodeToString(mac.Sum(nil)), nil +} + +// VerifySharedKey recomputes the SharedKey or SharedKeyLite signature for r +// (dispatching on the scheme named in its Authorization header) using +// accountKey, and reports whether it matches the signature the client sent. +// +// This is an explicit, opt-in check: nothing in this package calls it +// implicitly, and [ParseAuthorizationHeader] never fails or reports a +// mismatch on its own — callers that want enforcement invoke VerifySharedKey +// themselves, mirroring services/s3's opt-in WithPresignValidation pattern. +// It returns ([ErrMalformedAuthorization]) wrapped in the error when the +// header can't be parsed at all. +func VerifySharedKey(accountKey string, r *http.Request) (bool, error) { + auth, ok := ParseAuthorizationHeader(r.Header.Get("Authorization")) + if !ok { + return false, ErrMalformedAuthorization + } + + var ( + expected string + err error + ) + + switch auth.Scheme { + case SchemeSharedKeyLite: + expected, err = SignSharedKeyLite(r, auth.Account, accountKey) + default: + expected, err = SignSharedKey(r, auth.Account, accountKey) + } + if err != nil { + return false, err + } + + return hmac.Equal([]byte(expected), []byte(auth.Signature)), nil +} From 8bf165f5ea9ec1d33f29619c8ab2956bbb70ce7b Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:10:44 -0500 Subject: [PATCH 06/30] azureauth: fix CanonicalizedResource double-counting the account path segment on path-style requests --- pkgs/azureauth/azureauth_test.go | 312 +++++++++++++++++++++++++++++++ pkgs/azureauth/canonical.go | 29 ++- 2 files changed, 340 insertions(+), 1 deletion(-) create mode 100644 pkgs/azureauth/azureauth_test.go diff --git a/pkgs/azureauth/azureauth_test.go b/pkgs/azureauth/azureauth_test.go new file mode 100644 index 0000000000..925c1ce4bb --- /dev/null +++ b/pkgs/azureauth/azureauth_test.go @@ -0,0 +1,312 @@ +package azureauth_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/pkgs/azureauth" +) + +func TestParseAuthorizationHeader(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + header string + wantAccount string + wantSig string + wantScheme azureauth.Scheme + wantOK bool + }{ + { + name: "valid SharedKey", + header: "SharedKey devstoreaccount1:abcdefg123==", + wantOK: true, + wantScheme: azureauth.SchemeSharedKey, + wantAccount: "devstoreaccount1", + wantSig: "abcdefg123==", + }, + { + name: "valid SharedKeyLite", + header: "SharedKeyLite devstoreaccount1:xyz789==", + wantOK: true, + wantScheme: azureauth.SchemeSharedKeyLite, + wantAccount: "devstoreaccount1", + wantSig: "xyz789==", + }, + { + name: "empty header", + header: "", + wantOK: false, + }, + { + name: "wrong scheme", + header: "Bearer sometoken", + wantOK: false, + }, + { + name: "missing colon", + header: "SharedKey devstoreaccount1nosig", + wantOK: false, + }, + { + name: "empty account", + header: "SharedKey :abcdef==", + wantOK: false, + }, + { + name: "empty signature", + header: "SharedKey devstoreaccount1:", + wantOK: false, + }, + { + name: "extra junk after signature", + header: "SharedKey devstoreaccount1:abcdef== extra junk", + wantOK: false, + }, + { + name: "scheme with no space", + header: "SharedKeydevstoreaccount1:abcdef==", + wantOK: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, ok := azureauth.ParseAuthorizationHeader(tt.header) + assert.Equal(t, tt.wantOK, ok) + if !tt.wantOK { + return + } + assert.Equal(t, tt.wantScheme, got.Scheme) + assert.Equal(t, tt.wantAccount, got.Account) + assert.Equal(t, tt.wantSig, got.Signature) + }) + } +} + +func TestVerifySharedKey(t *testing.T) { + t.Parallel() + + newSignedRequest := func(t *testing.T) *http.Request { + t.Helper() + + r := httptest.NewRequest( + http.MethodGet, + "http://127.0.0.1:10000/devstoreaccount1/mycontainer?restype=container&comp=list", + nil, + ) + r.Header.Set("x-ms-date", "Tue, 27 Aug 2024 12:00:00 GMT") + r.Header.Set("x-ms-version", "2021-08-06") + + sig, err := azureauth.SignSharedKey(r, azureauth.DefaultAccountName, azureauth.DefaultAccountKey) + require.NoError(t, err) + r.Header.Set("Authorization", "SharedKey "+azureauth.DefaultAccountName+":"+sig) + + return r + } + + tests := []struct { + mutate func(r *http.Request) + name string + wantOK bool + wantValid bool + }{ + { + name: "valid round-trip", + mutate: func(*http.Request) {}, + wantOK: true, + wantValid: true, + }, + { + name: "tampered signature", + mutate: func(r *http.Request) { + r.Header.Set("Authorization", "SharedKey "+azureauth.DefaultAccountName+":dGFtcGVyZWQ=") + }, + wantOK: true, + wantValid: false, + }, + { + name: "tampered method", + mutate: func(r *http.Request) { + r.Method = http.MethodDelete + }, + wantOK: true, + wantValid: false, + }, + { + name: "malformed authorization header", + mutate: func(r *http.Request) { + r.Header.Set("Authorization", "Bearer sometoken") + }, + wantOK: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + r := newSignedRequest(t) + tt.mutate(r) + + valid, err := azureauth.VerifySharedKey(azureauth.DefaultAccountKey, r) + if !tt.wantOK { + require.Error(t, err) + + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantValid, valid) + }) + } +} + +func TestVerifySharedKeyLite(t *testing.T) { + t.Parallel() + + r := httptest.NewRequest( + http.MethodGet, + "http://127.0.0.1:10002/devstoreaccount1/Tables", + nil, + ) + r.Header.Set("x-ms-date", "Tue, 27 Aug 2024 12:00:00 GMT") + r.Header.Set("x-ms-version", "2021-08-06") + r.Header.Set("Content-Type", "application/json") + + sig, err := azureauth.SignSharedKeyLite(r, azureauth.DefaultAccountName, azureauth.DefaultAccountKey) + require.NoError(t, err) + r.Header.Set("Authorization", "SharedKeyLite "+azureauth.DefaultAccountName+":"+sig) + + valid, err := azureauth.VerifySharedKey(azureauth.DefaultAccountKey, r) + require.NoError(t, err) + assert.True(t, valid) +} + +func TestStringToSign(t *testing.T) { + t.Parallel() + + r := httptest.NewRequest( + http.MethodPut, + "http://127.0.0.1:10000/devstoreaccount1/c/blob.txt?comp=block&blockid=AAAA", + nil, + ) + r.Header.Set("Content-Type", "text/plain") + r.Header.Set("x-ms-date", "Tue, 27 Aug 2024 12:00:00 GMT") + r.Header.Set("x-ms-version", "2021-08-06") + r.Header.Set("x-ms-blob-type", "BlockBlob") + r.ContentLength = 11 + + want := "PUT\n" + // verb + "\n" + // content-encoding + "\n" + // content-language + "11\n" + // content-length + "\n" + // content-md5 + "text/plain\n" + // content-type + "\n" + // date + "\n" + // if-modified-since + "\n" + // if-match + "\n" + // if-none-match + "\n" + // if-unmodified-since + "\n" + // range + "x-ms-blob-type:BlockBlob\n" + + "x-ms-date:Tue, 27 Aug 2024 12:00:00 GMT\n" + + "x-ms-version:2021-08-06\n" + + "/devstoreaccount1/c/blob.txt\n" + + // (no doubled account segment: the request path already carries + // devstoreaccount1, matching Azurite's path-style addressing) + "blockid:AAAA\n" + + "comp:block" + + assert.Equal(t, want, azureauth.StringToSign(r, azureauth.DefaultAccountName)) +} + +func TestStringToSignLite(t *testing.T) { + t.Parallel() + + r := httptest.NewRequest(http.MethodGet, "http://127.0.0.1:10002/devstoreaccount1/Tables", nil) + r.Header.Set("Content-Type", "application/json") + r.Header.Set("x-ms-date", "Tue, 27 Aug 2024 12:00:00 GMT") + + want := "GET\n" + + "\n" + // content-md5 + "application/json\n" + + "\n" + // date + "x-ms-date:Tue, 27 Aug 2024 12:00:00 GMT\n" + + "/devstoreaccount1/Tables" + + assert.Equal(t, want, azureauth.StringToSignLite(r, azureauth.DefaultAccountName)) +} + +func TestCanonicalizedResource(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rawURL string + account string + want string + }{ + { + name: "no query", + rawURL: "http://host/devstoreaccount1/container/blob", + account: "devstoreaccount1", + want: "/devstoreaccount1/container/blob", + }, + { + name: "single query param", + rawURL: "http://host/acct/c?restype=container", + account: "acct", + want: "/acct/c\nrestype:container", + }, + { + name: "multiple params sorted and lowercased", + rawURL: "http://host/acct/c?comp=list&RESTYPE=container", + account: "acct", + want: "/acct/c\ncomp:list\nrestype:container", + }, + { + name: "repeated param comma-joined sorted", + rawURL: "http://host/acct/c?comp=b&comp=a", + account: "acct", + want: "/acct/c\ncomp:a,b", + }, + { + name: "host-style path with no account segment", + rawURL: "http://acct.blob.core.windows.net/c/blob", + account: "acct", + want: "/acct/c/blob", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + r := httptest.NewRequest(http.MethodGet, tt.rawURL, nil) + assert.Equal(t, tt.want, azureauth.CanonicalizedResource(tt.account, r.URL)) + }) + } +} + +func TestCanonicalizedHeaders(t *testing.T) { + t.Parallel() + + r := httptest.NewRequest(http.MethodGet, "http://host/a/b", nil) + r.Header.Set("x-ms-version", "2021-08-06") + r.Header.Set("x-ms-date", "Tue, 27 Aug 2024 12:00:00 GMT") + r.Header.Set("Content-Type", "text/plain") // not x-ms-*, must be excluded + r.Header.Set("x-ms-meta-foo", " a b ") // whitespace collapsed/trimmed + + want := "x-ms-date:Tue, 27 Aug 2024 12:00:00 GMT\n" + + "x-ms-meta-foo:a b\n" + + "x-ms-version:2021-08-06\n" + + assert.Equal(t, want, azureauth.CanonicalizedHeaders(r)) +} diff --git a/pkgs/azureauth/canonical.go b/pkgs/azureauth/canonical.go index d0e8c0ad81..7d06224a77 100644 --- a/pkgs/azureauth/canonical.go +++ b/pkgs/azureauth/canonical.go @@ -126,6 +126,23 @@ func CanonicalizedHeaders(r *http.Request) string { return b.String() } +// stripAccountPathSegment removes a leading "/" path segment from +// path, if present as a full segment (i.e. followed by "/" or end of +// string). A path that merely starts with the account name as a substring +// of a longer segment (e.g. account "acct" against path "/acctfoo") is left +// untouched. +func stripAccountPathSegment(path, account string) string { + prefix := "/" + account + if path == prefix { + return "" + } + if rest, ok := strings.CutPrefix(path, prefix+"/"); ok { + return "/" + rest + } + + return path +} + // collapseWhitespace trims leading/trailing whitespace and collapses any // internal run of whitespace to a single space. func collapseWhitespace(s string) string { @@ -136,12 +153,22 @@ func collapseWhitespace(s string) string { // request against account: "/" followed by each query // parameter, lowercased and sorted by name, rendered as "\nname:value" with // repeated-parameter values comma-joined after sorting. +// +// gopherstack, like Azurite, serves path-style requests whose URL already +// starts with "//..." (real Azure serves the account as a +// subdomain instead, so the URL path never contains it). Real SDKs know +// which style they're signing for and build the canonicalized resource as +// "/" plus the resource path with any such account segment +// removed, so that a path-style request and an equivalent subdomain-style +// request sign identically; this strips a leading "/" path segment +// before applying that formula so signatures computed here match what an +// Azurite-targeting SDK actually sends. func CanonicalizedResource(account string, u *url.URL) string { var b strings.Builder b.WriteByte('/') b.WriteString(account) - b.WriteString(u.EscapedPath()) + b.WriteString(stripAccountPathSegment(u.EscapedPath(), account)) query := u.Query() lowered := make(map[string][]string, len(query)) From 9e398b455ab6c9d375734457aa0a2f872a313945 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:15:06 -0500 Subject: [PATCH 07/30] azureblob: add skeleton, in-memory backend, and REST handler Implements the MVP Azure Blob Storage surface from AZURE.md M0: container create/delete/list and blob put/get/head/delete/list, served on its own dedicated port (not multiplexed into the shared AWS router, since Azure's path shape has no service-identifying header). Auth is structurally permissive by design, matching services/s3's philosophy; real SharedKey verification is deferred to pkgs/azureauth (TODO left at the auth entry point). cli.go registration is intentionally not wired up yet. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Jzq1rtNNMjzhnvZSpcGr1F --- services/azureblob/errors.go | 12 + services/azureblob/handler.go | 585 ++++++++++++++++++++++++++++++ services/azureblob/interfaces.go | 23 ++ services/azureblob/models.go | 111 ++++++ services/azureblob/persistence.go | 106 ++++++ services/azureblob/provider.go | 95 +++++ services/azureblob/settings.go | 38 ++ services/azureblob/store.go | 204 +++++++++++ 8 files changed, 1174 insertions(+) create mode 100644 services/azureblob/errors.go create mode 100644 services/azureblob/handler.go create mode 100644 services/azureblob/interfaces.go create mode 100644 services/azureblob/models.go create mode 100644 services/azureblob/persistence.go create mode 100644 services/azureblob/provider.go create mode 100644 services/azureblob/settings.go create mode 100644 services/azureblob/store.go diff --git a/services/azureblob/errors.go b/services/azureblob/errors.go new file mode 100644 index 0000000000..a8c89f2d6d --- /dev/null +++ b/services/azureblob/errors.go @@ -0,0 +1,12 @@ +package azureblob + +import "errors" + +// Sentinel errors for Azure Blob Storage operations. +var ( + ErrContainerNotFound = errors.New("azureblob: container not found") + ErrContainerAlreadyExists = errors.New("azureblob: container already exists") + ErrBlobNotFound = errors.New("azureblob: blob not found") + ErrInvalidBlobType = errors.New("azureblob: unsupported x-ms-blob-type") + ErrInvalidRange = errors.New("azureblob: invalid range") +) diff --git a/services/azureblob/handler.go b/services/azureblob/handler.go new file mode 100644 index 0000000000..518d436970 --- /dev/null +++ b/services/azureblob/handler.go @@ -0,0 +1,585 @@ +package azureblob + +import ( + "context" + "crypto/rand" + "encoding/xml" + "errors" + "fmt" + "net/http" + "strconv" + "strings" + "sync" + "time" + + "github.com/labstack/echo/v5" + + "github.com/blackbirdworks/gopherstack/pkgs/httputils" + "github.com/blackbirdworks/gopherstack/pkgs/logger" + "github.com/blackbirdworks/gopherstack/pkgs/service" +) + +// azureBlobVersion is the x-ms-version value echoed on every response. It is +// a plausible, well-formed Azure Storage REST API version string -- picked so +// azure-sdk-for-go's blob client parses it without erroring, not because +// gopherstack implements that exact API version's full feature set. +const azureBlobVersion = "2021-08-06" + +// blockBlobType is the only x-ms-blob-type this MVP accepts/serves (see +// PARITY.md known gaps: no append/page blob support). +const blockBlobType = "BlockBlob" + +// Operation name constants used for metrics (ExtractOperation) and +// GetSupportedOperations. +const ( + opListContainers = "ListContainers" + opCreateContainer = "CreateContainer" + opDeleteContainer = "DeleteContainer" + opListBlobs = "ListBlobs" + opPutBlob = "PutBlob" + opGetBlob = "GetBlob" + opGetBlobProperties = "GetBlobProperties" + opDeleteBlob = "DeleteBlob" + unknownOperation = "Unknown" +) + +// Handler is the Echo HTTP handler for Azure Blob Storage operations. +type Handler struct { + Backend StorageBackend + Endpoint string // e.g. "http://127.0.0.1:10000" -- used to build ServiceEndpoint in list responses + Port int + + srvMu sync.Mutex + srv *http.Server +} + +// NewHandler creates a new Azure Blob Handler. +func NewHandler(backend StorageBackend) *Handler { + return &Handler{Backend: backend, Port: DefaultPort} +} + +var ( + _ service.BackgroundWorker = (*Handler)(nil) + _ service.Shutdowner = (*Handler)(nil) + _ service.Resettable = (*Handler)(nil) +) + +// Name returns the service name. +func (h *Handler) Name() string { return "AzureBlob" } + +// GetSupportedOperations returns the list of supported Azure Blob operations. +func (h *Handler) GetSupportedOperations() []string { + return []string{ + opListContainers, + opCreateContainer, + opDeleteContainer, + opListBlobs, + opPutBlob, + opGetBlob, + opGetBlobProperties, + opDeleteBlob, + } +} + +// RouteMatcher exists only to satisfy service.Registerable's interface +// contract: AzureBlob deliberately never matches on the shared AWS +// single-port Router. It runs on its own dedicated listener started by +// StartWorker (see provider.go for the full rationale). cli.go's service +// registration list is not (yet) wired to this provider at all -- that is a +// deferred integration step for a human to do once pkgs/azureauth also +// lands, so this matcher is effectively dead code today, kept only so +// *Handler satisfies service.Registerable. +func (h *Handler) RouteMatcher() service.Matcher { + return func(*echo.Context) bool { return false } +} + +// MatchPriority returns the routing priority for the AzureBlob handler. +// Irrelevant in practice since RouteMatcher never matches; 0 (lowest) is +// the safe default. +func (h *Handler) MatchPriority() int { return 0 } + +// ExtractOperation extracts the Azure Blob operation name from the request, +// for metrics labeling. +func (h *Handler) ExtractOperation(c *echo.Context) string { + return operationFor(c.Request()) +} + +// ExtractResource extracts the container/blob resource identifier from the +// request path, for metrics labeling. +func (h *Handler) ExtractResource(c *echo.Context) string { + _, container, blob := splitPath(c.Request().URL.Path) + if blob != "" { + return container + "/" + blob + } + + return container +} + +// Reset clears all in-memory state from the backend. It is used by the +// POST /_gopherstack/reset endpoint for CI pipelines and rapid local development. +func (h *Handler) Reset() { + h.Backend.Reset() +} + +// Handler returns the Echo handler function for Azure Blob operations. +func (h *Handler) Handler() echo.HandlerFunc { + return func(c *echo.Context) error { + r := c.Request() + + h.setCommonHeaders(c) + h.checkAuth(r) + + account, container, blob := splitPath(r.URL.Path) + if account == "" { + return h.writeError(c, http.StatusBadRequest, "InvalidUri", + "The requested URI does not represent any resource on the server.") + } + + switch { + case container == "": + return h.handleAccountLevel(c) + case blob == "": + return h.handleContainerLevel(c, container) + default: + return h.handleBlobLevel(c, container, blob) + } + } +} + +// checkAuth is intentionally permissive: it neither requires nor +// cryptographically verifies the Authorization header, matching this repo's +// permissive-by-default auth philosophy (see services/s3/sigv4.go). Any +// structurally-present "SharedKey ..." header, or its absence, is accepted. +// +// TODO(azure-integration): wire real SharedKey verification via pkgs/azureauth once that package lands (see AZURE.md). +func (h *Handler) checkAuth(_ *http.Request) {} + +// setCommonHeaders sets the headers real Azure SDKs expect on every +// response, success or error. +func (h *Handler) setCommonHeaders(c *echo.Context) { + hdr := c.Response().Header() + hdr.Set("x-ms-version", azureBlobVersion) + hdr.Set("x-ms-request-id", newRequestID()) + hdr.Set("Date", time.Now().UTC().Format(http.TimeFormat)) +} + +// newRequestID generates a plausible request-id (UUID-shaped, not +// cryptographically meaningful) for the x-ms-request-id header. +func newRequestID() string { + var buf [16]byte + if _, err := rand.Read(buf[:]); err != nil { + return "00000000-0000-0000-0000-000000000000" + } + + return fmt.Sprintf("%x-%x-%x-%x-%x", buf[0:4], buf[4:6], buf[6:8], buf[8:10], buf[10:16]) +} + +// splitPath splits an Azure Blob REST path ("///") +// into its three components. blob may itself contain "/" (Azure blob names +// may include virtual-directory separators), so it is never split further. +func splitPath(p string) (account, container, blob string) { + p = strings.TrimPrefix(p, "/") + if p == "" { + return "", "", "" + } + + parts := strings.SplitN(p, "/", 3) + account = parts[0] + + if len(parts) > 1 { + container = parts[1] + } + + if len(parts) > 2 { + blob = parts[2] + } + + return account, container, blob +} + +// operationFor determines the Azure Blob operation name for a request, for +// metrics labeling. Mirrors the dispatch logic in handleAccountLevel/ +// handleContainerLevel/handleBlobLevel without side effects. +func operationFor(r *http.Request) string { + _, container, blob := splitPath(r.URL.Path) + restype := r.URL.Query().Get("restype") + comp := r.URL.Query().Get("comp") + + switch { + case container == "" && r.Method == http.MethodGet && comp == "list": + return opListContainers + case blob == "" && r.Method == http.MethodPut && restype == "container": + return opCreateContainer + case blob == "" && r.Method == http.MethodDelete && restype == "container": + return opDeleteContainer + case blob == "" && r.Method == http.MethodGet && restype == "container" && comp == "list": + return opListBlobs + case blob != "" && r.Method == http.MethodPut: + return opPutBlob + case blob != "" && r.Method == http.MethodGet: + return opGetBlob + case blob != "" && r.Method == http.MethodHead: + return opGetBlobProperties + case blob != "" && r.Method == http.MethodDelete: + return opDeleteBlob + default: + return unknownOperation + } +} + +// serviceEndpoint returns the ServiceEndpoint attribute value for +// EnumerationResults responses. +func (h *Handler) serviceEndpoint() string { + if h.Endpoint != "" { + return h.Endpoint + } + + return fmt.Sprintf("http://127.0.0.1:%d", h.Port) +} + +// handleAccountLevel serves GET /?comp=list (List Containers). +func (h *Handler) handleAccountLevel(c *echo.Context) error { + r := c.Request() + if r.Method != http.MethodGet || c.QueryParam("comp") != "list" { + return h.writeError(c, http.StatusBadRequest, "InvalidQueryParameterValue", + "A query parameter is not supported for this operation.") + } + + containers := h.Backend.ListContainers() + result := enumerationResults{ + ServiceEndpoint: h.serviceEndpoint(), + Containers: &containersList{Container: make([]containerEntry, 0, len(containers))}, + } + + for _, ci := range containers { + result.Containers.Container = append(result.Containers.Container, containerEntry{ + Name: ci.Name, + Properties: containerProperties{ + LastModified: ci.CreatedAt.Format(http.TimeFormat), + Etag: computeETag([]byte(ci.Name + ci.CreatedAt.String())), + }, + }) + } + + return h.writeXML(c, http.StatusOK, result) +} + +// handleContainerLevel serves the three container-scoped operations: Create +// Container, Delete Container, and List Blobs. +func (h *Handler) handleContainerLevel(c *echo.Context, container string) error { + r := c.Request() + restype := c.QueryParam("restype") + comp := c.QueryParam("comp") + + switch { + case r.Method == http.MethodPut && restype == "container": + return h.createContainer(c, container) + case r.Method == http.MethodDelete && restype == "container": + return h.deleteContainer(c, container) + case r.Method == http.MethodGet && restype == "container" && comp == "list": + return h.listBlobs(c, container) + default: + return h.writeError(c, http.StatusBadRequest, "InvalidQueryParameterValue", + "A query parameter is not supported for this operation.") + } +} + +func (h *Handler) createContainer(c *echo.Context, container string) error { + if err := h.Backend.CreateContainer(container); err != nil { + if errors.Is(err, ErrContainerAlreadyExists) { + return h.writeError(c, http.StatusConflict, "ContainerAlreadyExists", + "The specified container already exists.") + } + + return h.writeError(c, http.StatusInternalServerError, "InternalError", err.Error()) + } + + return c.NoContent(http.StatusCreated) +} + +func (h *Handler) deleteContainer(c *echo.Context, container string) error { + if err := h.Backend.DeleteContainer(container); err != nil { + return h.writeError(c, http.StatusNotFound, "ContainerNotFound", + "The specified container does not exist.") + } + + return c.NoContent(http.StatusAccepted) +} + +func (h *Handler) listBlobs(c *echo.Context, container string) error { + blobs, err := h.Backend.ListBlobs(container) + if err != nil { + return h.writeError(c, http.StatusNotFound, "ContainerNotFound", + "The specified container does not exist.") + } + + result := enumerationResults{ + ServiceEndpoint: h.serviceEndpoint(), + ContainerName: container, + Blobs: &blobsList{Blob: make([]blobEntry, 0, len(blobs))}, + } + + for _, bi := range blobs { + result.Blobs.Blob = append(result.Blobs.Blob, blobEntry{ + Name: bi.Name, + Properties: blobProperties{ + LastModified: bi.LastModified.Format(http.TimeFormat), + Etag: bi.ETag, + ContentLength: bi.ContentLength, + ContentType: bi.ContentType, + BlobType: blockBlobType, + }, + }) + } + + return h.writeXML(c, http.StatusOK, result) +} + +// handleBlobLevel dispatches the four blob-scoped operations by HTTP method. +func (h *Handler) handleBlobLevel(c *echo.Context, container, blob string) error { + switch c.Request().Method { + case http.MethodPut: + return h.putBlob(c, container, blob) + case http.MethodGet: + return h.getBlob(c, container, blob) + case http.MethodHead: + return h.headBlob(c, container, blob) + case http.MethodDelete: + return h.deleteBlob(c, container, blob) + default: + return h.writeError(c, http.StatusMethodNotAllowed, "UnsupportedHttpVerb", + "The resource doesn't support the specified HTTP verb.") + } +} + +func (h *Handler) putBlob(c *echo.Context, container, blob string) error { + r := c.Request() + + if r.Header.Get("x-ms-blob-type") != blockBlobType { + return h.writeError(c, http.StatusBadRequest, "InvalidHeaderValue", + "The value for one of the HTTP headers is not in the correct format "+ + "(x-ms-blob-type must be BlockBlob; only block blobs are supported).") + } + + body, err := httputils.ReadBody(r) + if err != nil { + return h.writeError(c, http.StatusInternalServerError, "InternalError", + "Failed to read request body.") + } + + info, err := h.Backend.PutBlob(container, blob, body, r.Header.Get("Content-Type")) + if err != nil { + return h.writeError(c, http.StatusNotFound, "ContainerNotFound", + "The specified container does not exist.") + } + + hdr := c.Response().Header() + hdr.Set("ETag", info.ETag) + hdr.Set("Last-Modified", info.LastModified.Format(http.TimeFormat)) + + return c.NoContent(http.StatusCreated) +} + +func (h *Handler) getBlob(c *echo.Context, container, blob string) error { + r := c.Request() + + info, data, err := h.Backend.GetBlob(container, blob) + if err != nil { + return h.writeBlobNotFoundError(c, err) + } + + h.setBlobHeaders(c, info) + + rangeHeader := r.Header.Get("Range") + if rangeHeader == "" { + return c.Blob(http.StatusOK, contentTypeOrDefault(info.ContentType), data) + } + + start, end, ok := parseRange(rangeHeader, int64(len(data))) + if !ok { + c.Response().Header().Set("Content-Range", fmt.Sprintf("bytes */%d", len(data))) + + return h.writeError(c, http.StatusRequestedRangeNotSatisfiable, "InvalidRange", + "The range specified is invalid for the current size of the resource.") + } + + hdr := c.Response().Header() + hdr.Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", start, end, len(data))) + hdr.Set("Content-Length", strconv.FormatInt(end-start+1, 10)) + + return c.Blob(http.StatusPartialContent, contentTypeOrDefault(info.ContentType), data[start:end+1]) +} + +func (h *Handler) headBlob(c *echo.Context, container, blob string) error { + info, err := h.Backend.HeadBlob(container, blob) + if err != nil { + return h.writeBlobNotFoundError(c, err) + } + + h.setBlobHeaders(c, info) + + return c.NoContent(http.StatusOK) +} + +func (h *Handler) deleteBlob(c *echo.Context, container, blob string) error { + if err := h.Backend.DeleteBlob(container, blob); err != nil { + return h.writeBlobNotFoundError(c, err) + } + + return c.NoContent(http.StatusAccepted) +} + +// writeBlobNotFoundError maps a StorageBackend not-found error (either the +// container or the blob may be missing) to the corresponding Azure error code. +func (h *Handler) writeBlobNotFoundError(c *echo.Context, err error) error { + if errors.Is(err, ErrContainerNotFound) { + return h.writeError(c, http.StatusNotFound, "ContainerNotFound", + "The specified container does not exist.") + } + + return h.writeError(c, http.StatusNotFound, "BlobNotFound", "The specified blob does not exist.") +} + +// setBlobHeaders sets the properties common to Get Blob, Get Blob Properties, +// and (implicitly, via the caller) any other blob-body response. +func (h *Handler) setBlobHeaders(c *echo.Context, info BlobInfo) { + hdr := c.Response().Header() + hdr.Set("ETag", info.ETag) + hdr.Set("Last-Modified", info.LastModified.Format(http.TimeFormat)) + hdr.Set("Content-Length", strconv.FormatInt(info.ContentLength, 10)) + hdr.Set("x-ms-blob-type", blockBlobType) + hdr.Set("Accept-Ranges", "bytes") + + if info.ContentType != "" { + hdr.Set("Content-Type", info.ContentType) + } +} + +func contentTypeOrDefault(ct string) string { + if ct == "" { + return "application/octet-stream" + } + + return ct +} + +// parseRange parses an HTTP "Range: bytes=start-end" header (also supporting +// the open-ended "bytes=start-" and suffix "bytes=-N" forms) against a +// resource of the given size. Only a single range is supported (Azure Get +// Blob does not support multi-range requests). Returns ok=false if the +// header is absent, malformed, or unsatisfiable for size. +func parseRange(header string, size int64) (start, end int64, ok bool) { + const prefix = "bytes=" + + spec, found := strings.CutPrefix(header, prefix) + if !found { + return 0, 0, false + } + + // Reject multi-range requests (a comma indicates more than one range); + // this backend only serves the first/only range. + if strings.Contains(spec, ",") { + return 0, 0, false + } + + before, after, found := strings.Cut(spec, "-") + if !found { + return 0, 0, false + } + + if before == "" { + // Suffix form: "bytes=-N" means the last N bytes. + n, err := strconv.ParseInt(after, 10, 64) + if err != nil || n <= 0 { + return 0, 0, false + } + + if n > size { + n = size + } + + return size - n, size - 1, size > 0 + } + + start, err := strconv.ParseInt(before, 10, 64) + if err != nil || start < 0 || start >= size { + return 0, 0, false + } + + if after == "" { + return start, size - 1, true + } + + end, err = strconv.ParseInt(after, 10, 64) + if err != nil || end < start { + return 0, 0, false + } + + if end >= size { + end = size - 1 + } + + return start, end, true +} + +// writeXML marshals v and writes it as the response body. v is marshaled +// without an XML header/prolog: echo's XMLBlob prepends xml.Header itself, +// so marshaling one here would duplicate it (see services/sqs/PARITY.md for +// the same trap hit and fixed there). +func (h *Handler) writeXML(c *echo.Context, status int, v any) error { + body, err := xml.Marshal(v) + if err != nil { + return h.writeError(c, http.StatusInternalServerError, "InternalError", "Failed to marshal response.") + } + + return c.XMLBlob(status, body) +} + +// writeError writes a standard Azure Storage REST error body. +func (h *Handler) writeError(c *echo.Context, status int, code, message string) error { + return h.writeXML(c, status, azureError{Code: code, Message: message}) +} + +// StartWorker starts the dedicated Blob listener on h.Port. See provider.go's +// Provider doc comment for why AzureBlob needs its own listener instead of +// registering into the shared AWS Router. +func (h *Handler) StartWorker(ctx context.Context) error { + e := echo.New() + e.Any("/*", h.Handler()) + + srv := &http.Server{ + Addr: fmt.Sprintf(":%d", h.Port), + Handler: e, + ReadHeaderTimeout: 10 * time.Second, //nolint:mnd // matches cli.go's defaultReadHeaderTimeout intent + } + + h.srvMu.Lock() + h.srv = srv + h.srvMu.Unlock() + + log := logger.Load(ctx) + + go func() { + log.InfoContext(ctx, "azureblob: starting dedicated listener", "port", h.Port) + + if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { + log.ErrorContext(ctx, "azureblob: listener stopped", "error", err) + } + }() + + return nil +} + +// Shutdown stops the dedicated Blob listener. +func (h *Handler) Shutdown(ctx context.Context) { + h.srvMu.Lock() + srv := h.srv + h.srv = nil + h.srvMu.Unlock() + + if srv == nil { + return + } + + _ = srv.Shutdown(ctx) +} diff --git a/services/azureblob/interfaces.go b/services/azureblob/interfaces.go new file mode 100644 index 0000000000..1d964efcc2 --- /dev/null +++ b/services/azureblob/interfaces.go @@ -0,0 +1,23 @@ +package azureblob + +// Compile-time assertion: InMemoryBackend must implement StorageBackend. +var _ StorageBackend = (*InMemoryBackend)(nil) + +// StorageBackend defines the interface for an Azure Blob Storage backend. +// Shaped after services/sqs's StorageBackend: a narrow, testable seam between +// the wire handler and storage, so handler tests can substitute a fake. +type StorageBackend interface { + CreateContainer(name string) error + DeleteContainer(name string) error + ListContainers() []ContainerInfo + + PutBlob(container, blob string, data []byte, contentType string) (BlobInfo, error) + GetBlob(container, blob string) (BlobInfo, []byte, error) + HeadBlob(container, blob string) (BlobInfo, error) + DeleteBlob(container, blob string) error + ListBlobs(container string) ([]BlobInfo, error) + + // Reset clears all in-memory state. Used by the + // POST /_gopherstack/reset endpoint for CI pipelines and rapid local development. + Reset() +} diff --git a/services/azureblob/models.go b/services/azureblob/models.go new file mode 100644 index 0000000000..588c8af72b --- /dev/null +++ b/services/azureblob/models.go @@ -0,0 +1,111 @@ +package azureblob + +import ( + "encoding/xml" + "time" +) + +// ContainerInfo is a read-only snapshot of a container's metadata, returned +// by StorageBackend.ListContainers. It intentionally excludes the container's +// blob map so callers cannot mutate backend state through it. +type ContainerInfo struct { + Name string + CreatedAt time.Time +} + +// BlobInfo is a read-only snapshot of a blob's metadata, returned by the +// StorageBackend blob accessors. Like ContainerInfo, it carries no reference +// to the backend's internal storage. +type BlobInfo struct { + Name string + ContentType string + ETag string + LastModified time.Time + ContentLength int64 +} + +// storedBlob is the backend's internal representation of a blob. Only +// BlockBlob is modeled (MVP scope, see AZURE.md/PARITY.md): Data holds the +// full object body written by a single Put Blob call, there is no +// block-list/multipart state. +type storedBlob struct { + Name string + ContentType string + ETag string + Data []byte + LastModified time.Time +} + +func (b *storedBlob) info() BlobInfo { + return BlobInfo{ + Name: b.Name, + ContentType: b.ContentType, + ETag: b.ETag, + LastModified: b.LastModified, + ContentLength: int64(len(b.Data)), + } +} + +// storedContainer is the backend's internal representation of a container. +type storedContainer struct { + Name string + CreatedAt time.Time + Blobs map[string]*storedBlob +} + +// --- Azure Blob REST XML response shapes --- +// +// These mirror the wire shape of Azure Storage's "EnumerationResults" and +// "Error" bodies closely enough for azure-sdk-for-go (and Azurite-targeting +// SDKs generally) to parse successfully. Field ordering matches the real +// service's documented schema. + +// azureError is the standard Azure Storage REST error body. +type azureError struct { + XMLName xml.Name `xml:"Error"` + Code string `xml:"Code"` + Message string `xml:"Message"` +} + +// enumerationResults is the top-level shape returned by List Containers and +// List Blobs. Exactly one of Containers/Blobs is populated depending on +// which operation produced it. +type enumerationResults struct { + XMLName xml.Name `xml:"EnumerationResults"` + ServiceEndpoint string `xml:"ServiceEndpoint,attr"` + ContainerName string `xml:"ContainerName,attr,omitempty"` + Containers *containersList `xml:"Containers"` + Blobs *blobsList `xml:"Blobs"` + NextMarker string `xml:"NextMarker"` +} + +type containersList struct { + Container []containerEntry `xml:"Container"` +} + +type containerEntry struct { + Name string `xml:"Name"` + Properties containerProperties `xml:"Properties"` +} + +type containerProperties struct { + LastModified string `xml:"Last-Modified"` + Etag string `xml:"Etag"` +} + +type blobsList struct { + Blob []blobEntry `xml:"Blob"` +} + +type blobEntry struct { + Name string `xml:"Name"` + Properties blobProperties `xml:"Properties"` +} + +type blobProperties struct { + LastModified string `xml:"Last-Modified"` + Etag string `xml:"Etag"` + ContentLength int64 `xml:"Content-Length"` + ContentType string `xml:"Content-Type"` + BlobType string `xml:"BlobType"` +} diff --git a/services/azureblob/persistence.go b/services/azureblob/persistence.go new file mode 100644 index 0000000000..e3db21a9b2 --- /dev/null +++ b/services/azureblob/persistence.go @@ -0,0 +1,106 @@ +package azureblob + +import ( + "context" + "fmt" + + "github.com/blackbirdworks/gopherstack/pkgs/logger" + "github.com/blackbirdworks/gopherstack/pkgs/persistence" +) + +// azureBlobSnapshotVersion identifies the shape of backendSnapshot. Must be +// bumped whenever a change to storedContainer/storedBlob would make an older +// snapshot unsafe to decode as the current shape; Restore compares this +// against the persisted value and discards (rather than partially decodes) +// any mismatch, mirroring services/s3 and services/sqs. +const azureBlobSnapshotVersion = 1 + +// backendSnapshot is the top-level on-disk shape for the Azure Blob backend. +// Containers serialises directly (no DTO layer): storedContainer/storedBlob +// have no unexported fields, so encoding/json round-trips them as-is. +type backendSnapshot struct { + Containers map[string]*storedContainer `json:"containers"` + Version int `json:"version"` +} + +// Snapshot serialises the backend state to JSON. It implements +// persistence.Persistable. +func (b *InMemoryBackend) Snapshot(ctx context.Context) []byte { + b.mu.RLock() + defer b.mu.RUnlock() + + snap := backendSnapshot{ + Version: azureBlobSnapshotVersion, + Containers: b.containers, + } + + return persistence.MarshalSnapshot(ctx, "azureblob", snap) +} + +// Restore loads backend state from a JSON snapshot. It implements +// persistence.Persistable. +func (b *InMemoryBackend) Restore(ctx context.Context, data []byte) error { + var snap backendSnapshot + + if err := persistence.UnmarshalSnapshot(ctx, "azureblob", data, &snap); err != nil { + return err + } + + b.mu.Lock() + defer b.mu.Unlock() + + if snap.Version != azureBlobSnapshotVersion { + // An incompatible (older/newer/absent) snapshot version must never be + // partially decoded as the current shape -- discard cleanly and start + // empty instead of erroring, since this is an expected, recoverable + // condition (e.g. upgrading gopherstack across a snapshot-format + // change), not data corruption. Mirrors services/s3 and services/sqs. + logger.Load(ctx).WarnContext(ctx, + "azureblob: discarding incompatible snapshot version, starting empty", + "gotVersion", snap.Version, "wantVersion", azureBlobSnapshotVersion) + + b.containers = make(map[string]*storedContainer) + + return nil + } + + if snap.Containers == nil { + snap.Containers = make(map[string]*storedContainer) + } + + for _, c := range snap.Containers { + if c.Blobs == nil { + c.Blobs = make(map[string]*storedBlob) + } + } + + b.containers = snap.Containers + + return nil +} + +// Snapshot implements persistence.Persistable by delegating to the backend. +func (h *Handler) Snapshot(ctx context.Context) []byte { + type snapshotter interface { + Snapshot(ctx context.Context) []byte + } + if s, ok := h.Backend.(snapshotter); ok { + return s.Snapshot(ctx) + } + + return nil +} + +// Restore implements persistence.Persistable by delegating to the backend. +func (h *Handler) Restore(ctx context.Context, data []byte) error { + type restorer interface { + Restore(context.Context, []byte) error + } + if r, ok := h.Backend.(restorer); ok { + if err := r.Restore(ctx, data); err != nil { + return fmt.Errorf("azureblob: restore snapshot: %w", err) + } + } + + return nil +} diff --git a/services/azureblob/provider.go b/services/azureblob/provider.go new file mode 100644 index 0000000000..b9ca8e2e94 --- /dev/null +++ b/services/azureblob/provider.go @@ -0,0 +1,95 @@ +package azureblob + +import ( + "errors" + "net" + "strconv" + + "github.com/blackbirdworks/gopherstack/pkgs/portalloc" + "github.com/blackbirdworks/gopherstack/pkgs/service" +) + +// ErrNilAppContext is returned when Init is called with a nil AppContext. +var ErrNilAppContext = errors.New("azureblob: nil app context") + +// Provider implements service.Provider for the Azure Blob Storage service. +// +// Unlike every other provider in this repo, AzureBlob does not register a +// RouteMatcher into the shared AWS single-port Router: Azure Blob's path +// shape (///) has no service-identifying header +// the way AWS's X-Amz-Target does, so multiplexing it onto the shared port +// risks exactly the collision the router avoids by construction for AWS +// services (see AZURE.md section 4). Instead the returned Handler implements +// service.BackgroundWorker and stands up its own dedicated *echo.Echo/ +// *http.Server, listening on its own port -- mirroring Azurite's own +// separate-port-per-service convention (10000 for Blob). +type Provider struct{} + +// Name returns the service provider name. +func (p *Provider) Name() string { return "AzureBlob" } + +// Init initializes the AzureBlob service backend and handler, resolving the +// dedicated port the handler's StartWorker will later listen on. +// +//nolint:ireturn,nolintlint // architecturally required to return interface +func (p *Provider) Init(ctx *service.AppContext) (service.Registerable, error) { + if ctx == nil { + return nil, ErrNilAppContext + } + + settings := DefaultSettings() + backend := NewInMemoryBackend() + handler := NewHandler(backend) + handler.Port = resolvePort(settings.Port, ctx.PortAlloc) + + return handler, nil +} + +// resolvePort implements azureblob's port-selection strategy. +// +// gopherstack's pkgs/portalloc.Allocator only supports acquiring the next +// free port from a sequential range (Allocator.Acquire) -- it has no concept +// of reserving a *specific* preferred port. Every existing PortAlloc caller +// in this repo (Lambda function URLs, ElastiCache) wants an arbitrary +// ephemeral port and is fine with whatever it gets. Azure Blob is different: +// real SDKs default their connection strings/emulator constants to a *fixed* +// port (Azurite's 10000, see AZURE.md section 2), so gopherstack must +// actually try to bind that fixed port to be a useful drop-in target, +// falling back to the shared pool only when it can't. +// +// This is the "one real architectural decision" AZURE.md section 4 flags as +// a gap in the current single-port-router design: there is no existing +// precedent in the repo for a "give me this exact port or tell me it's +// busy" primitive, so this function bridges the gap locally instead of +// extending portalloc's contract (which would ripple to every other caller) +// for one service's needs. +// +// The availability probe is inherently racy (the port could be taken between +// this check and StartWorker's real bind) -- acceptable for a local dev/test +// emulator, not something a production load balancer would do. +func resolvePort(preferred int, alloc *portalloc.Allocator) int { + if portAvailable(preferred) { + return preferred + } + + if alloc != nil { + if p, err := alloc.Acquire("azureblob"); err == nil { + return p + } + } + + return preferred +} + +// portAvailable reports whether port can currently be bound on all +// interfaces. +func portAvailable(port int) bool { + l, err := net.Listen("tcp", ":"+strconv.Itoa(port)) + if err != nil { + return false + } + + _ = l.Close() + + return true +} diff --git a/services/azureblob/settings.go b/services/azureblob/settings.go new file mode 100644 index 0000000000..c2827fe1e8 --- /dev/null +++ b/services/azureblob/settings.go @@ -0,0 +1,38 @@ +package azureblob + +import ( + "os" + "strconv" +) + +// DefaultPort mirrors Azurite's default Blob service port, so unmodified +// Azurite-targeting SDK configuration (UseDevelopmentStorage=true, default +// connection strings) works out of the box. See AZURE.md section 4/5. +const DefaultPort = 10000 + +// envPortOverride lets a deployment move the dedicated Blob listener off +// DefaultPort (e.g. because 10000 is already in use for something else on +// the host). +const envPortOverride = "AZURE_BLOB_PORT" + +// Settings holds service-level configuration for the Azure Blob backend. +type Settings struct { + // Port is the preferred TCP port for the dedicated Blob listener. + // See provider.go's resolvePort for what happens when it's unavailable. + Port int +} + +// DefaultSettings returns the default Settings, honoring envPortOverride. +func DefaultSettings() Settings { + return Settings{Port: portFromEnv()} +} + +func portFromEnv() int { + if v := os.Getenv(envPortOverride); v != "" { + if p, err := strconv.Atoi(v); err == nil && p > 0 && p < 65536 { + return p + } + } + + return DefaultPort +} diff --git a/services/azureblob/store.go b/services/azureblob/store.go new file mode 100644 index 0000000000..b0b017889b --- /dev/null +++ b/services/azureblob/store.go @@ -0,0 +1,204 @@ +package azureblob + +import ( + "crypto/md5" //nolint:gosec // ETag generation only, not a security use of MD5 + "encoding/hex" + "sort" + "sync" + "time" +) + +// InMemoryBackend implements StorageBackend using in-memory maps guarded by a +// single RWMutex. Shaped after services/sqs's InMemoryBackend, but simpler: +// Azure Blob's MVP surface (see AZURE.md/PARITY.md) has no janitor, no +// metrics emitter, and no cross-resource relationships to track, so a single +// coarse lock over one map of containers is sufficient. +type InMemoryBackend struct { + mu sync.RWMutex + containers map[string]*storedContainer +} + +// NewInMemoryBackend creates a new empty InMemoryBackend. +func NewInMemoryBackend() *InMemoryBackend { + return &InMemoryBackend{ + containers: make(map[string]*storedContainer), + } +} + +// CreateContainer creates a new, empty container. Returns +// ErrContainerAlreadyExists if a container with the same name already exists. +func (b *InMemoryBackend) CreateContainer(name string) error { + b.mu.Lock() + defer b.mu.Unlock() + + if _, ok := b.containers[name]; ok { + return ErrContainerAlreadyExists + } + + b.containers[name] = &storedContainer{ + Name: name, + CreatedAt: time.Now().UTC(), + Blobs: make(map[string]*storedBlob), + } + + return nil +} + +// DeleteContainer removes a container and all of its blobs. Returns +// ErrContainerNotFound if the container does not exist. +func (b *InMemoryBackend) DeleteContainer(name string) error { + b.mu.Lock() + defer b.mu.Unlock() + + if _, ok := b.containers[name]; !ok { + return ErrContainerNotFound + } + + delete(b.containers, name) + + return nil +} + +// ListContainers returns a snapshot of all containers, sorted by name (the +// order Azure's List Containers returns them in). +func (b *InMemoryBackend) ListContainers() []ContainerInfo { + b.mu.RLock() + defer b.mu.RUnlock() + + out := make([]ContainerInfo, 0, len(b.containers)) + for _, c := range b.containers { + out = append(out, ContainerInfo{Name: c.Name, CreatedAt: c.CreatedAt}) + } + + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + + return out +} + +// PutBlob stores data as a block blob named blob within container. Returns +// ErrContainerNotFound if the container does not exist. Overwrites any +// existing blob with the same name (Azure's Put Blob semantics -- no +// conditional headers are enforced, see PARITY.md known gaps). +func (b *InMemoryBackend) PutBlob(container, blob string, data []byte, contentType string) (BlobInfo, error) { + b.mu.Lock() + defer b.mu.Unlock() + + c, ok := b.containers[container] + if !ok { + return BlobInfo{}, ErrContainerNotFound + } + + stored := &storedBlob{ + Name: blob, + ContentType: contentType, + Data: append([]byte(nil), data...), + LastModified: time.Now().UTC(), + ETag: computeETag(data), + } + c.Blobs[blob] = stored + + return stored.info(), nil +} + +// GetBlob returns a blob's metadata and full body. Returns ErrContainerNotFound +// or ErrBlobNotFound as appropriate. +func (b *InMemoryBackend) GetBlob(container, blob string) (BlobInfo, []byte, error) { + b.mu.RLock() + defer b.mu.RUnlock() + + stored, err := b.lookupBlobLocked(container, blob) + if err != nil { + return BlobInfo{}, nil, err + } + + return stored.info(), append([]byte(nil), stored.Data...), nil +} + +// HeadBlob returns a blob's metadata without its body. Returns +// ErrContainerNotFound or ErrBlobNotFound as appropriate. +func (b *InMemoryBackend) HeadBlob(container, blob string) (BlobInfo, error) { + b.mu.RLock() + defer b.mu.RUnlock() + + stored, err := b.lookupBlobLocked(container, blob) + if err != nil { + return BlobInfo{}, err + } + + return stored.info(), nil +} + +// DeleteBlob removes a blob. Returns ErrContainerNotFound or ErrBlobNotFound +// as appropriate. +func (b *InMemoryBackend) DeleteBlob(container, blob string) error { + b.mu.Lock() + defer b.mu.Unlock() + + c, ok := b.containers[container] + if !ok { + return ErrContainerNotFound + } + + if _, ok := c.Blobs[blob]; !ok { + return ErrBlobNotFound + } + + delete(c.Blobs, blob) + + return nil +} + +// ListBlobs returns a snapshot of all blobs in container, sorted by name. +// Returns ErrContainerNotFound if the container does not exist. +func (b *InMemoryBackend) ListBlobs(container string) ([]BlobInfo, error) { + b.mu.RLock() + defer b.mu.RUnlock() + + c, ok := b.containers[container] + if !ok { + return nil, ErrContainerNotFound + } + + out := make([]BlobInfo, 0, len(c.Blobs)) + for _, stored := range c.Blobs { + out = append(out, stored.info()) + } + + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + + return out, nil +} + +// Reset clears all in-memory state. It is used by the +// POST /_gopherstack/reset endpoint for CI pipelines and rapid local development. +func (b *InMemoryBackend) Reset() { + b.mu.Lock() + defer b.mu.Unlock() + + b.containers = make(map[string]*storedContainer) +} + +// lookupBlobLocked resolves a (container, blob) pair. Callers must hold +// b.mu (either read or write). +func (b *InMemoryBackend) lookupBlobLocked(container, blob string) (*storedBlob, error) { + c, ok := b.containers[container] + if !ok { + return nil, ErrContainerNotFound + } + + stored, ok := c.Blobs[blob] + if !ok { + return nil, ErrBlobNotFound + } + + return stored, nil +} + +// computeETag derives a quoted ETag from the blob body, matching the shape +// (a quoted opaque token) real Azure Storage ETags take, without attempting +// to replicate Azure's actual internal ETag algorithm. +func computeETag(data []byte) string { + sum := md5.Sum(data) //nolint:gosec // content fingerprint only, not a security use of MD5 + + return `"` + hex.EncodeToString(sum[:]) + `"` +} From 62a702aa6e11ab07d753f17131672615f18138c2 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:17:56 -0500 Subject: [PATCH 08/30] azureblob: add unit tests for handler, backend, and persistence Table-driven tests covering container create/delete/list, blob put/get/head/delete/list, container/blob 404s, Range-header partial reads, snapshot/restore round-trip, and provider init. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Jzq1rtNNMjzhnvZSpcGr1F --- services/azureblob/export_test.go | 13 + services/azureblob/handler_test.go | 387 +++++++++++++++++++++++++ services/azureblob/persistence_test.go | 104 +++++++ services/azureblob/provider_test.go | 63 ++++ services/azureblob/range_test.go | 88 ++++++ services/azureblob/store_test.go | 239 +++++++++++++++ 6 files changed, 894 insertions(+) create mode 100644 services/azureblob/export_test.go create mode 100644 services/azureblob/handler_test.go create mode 100644 services/azureblob/persistence_test.go create mode 100644 services/azureblob/provider_test.go create mode 100644 services/azureblob/range_test.go create mode 100644 services/azureblob/store_test.go diff --git a/services/azureblob/export_test.go b/services/azureblob/export_test.go new file mode 100644 index 0000000000..eb8a510687 --- /dev/null +++ b/services/azureblob/export_test.go @@ -0,0 +1,13 @@ +package azureblob + +// Exported wrappers for internal functions used in blackbox tests. + +// ParseRange exposes parseRange for external tests. +func ParseRange(header string, size int64) (start, end int64, ok bool) { + return parseRange(header, size) +} + +// SplitPath exposes splitPath for external tests. +func SplitPath(p string) (account, container, blob string) { + return splitPath(p) +} diff --git a/services/azureblob/handler_test.go b/services/azureblob/handler_test.go new file mode 100644 index 0000000000..fa269eb5e6 --- /dev/null +++ b/services/azureblob/handler_test.go @@ -0,0 +1,387 @@ +package azureblob_test + +import ( + "bytes" + "net/http" + "net/http/httptest" + "strconv" + "testing" + + "github.com/labstack/echo/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/azureblob" +) + +const testAccount = "devstoreaccount1" + +func newTestHandler(t *testing.T) *azureblob.Handler { + t.Helper() + + backend := azureblob.NewInMemoryBackend() + + return azureblob.NewHandler(backend) +} + +// doRequest builds an echo context for method/path (with optional headers and +// body) and invokes the handler directly, mirroring services/sqs's doRequest. +func doRequest( + t *testing.T, + h *azureblob.Handler, + method, path string, + body []byte, + headers map[string]string, +) *httptest.ResponseRecorder { + t.Helper() + + var req *http.Request + if body != nil { + req = httptest.NewRequest(method, path, bytes.NewReader(body)) + } else { + req = httptest.NewRequest(method, path, http.NoBody) + } + + for k, v := range headers { + req.Header.Set(k, v) + } + + e := echo.New() + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + require.NoError(t, h.Handler()(c)) + + return rec +} + +func TestContainerLifecycle_CreateListDelete(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "create_list_delete_container"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + + rec := doRequest(t, h, http.MethodPut, "/"+testAccount+"/mycontainer?restype=container", nil, nil) + require.Equal(t, http.StatusCreated, rec.Code, tt.name) + assert.NotEmpty(t, rec.Header().Get("x-ms-version")) + assert.NotEmpty(t, rec.Header().Get("x-ms-request-id")) + assert.NotEmpty(t, rec.Header().Get("Date")) + + rec = doRequest(t, h, http.MethodGet, "/"+testAccount+"?comp=list", nil, nil) + require.Equal(t, http.StatusOK, rec.Code, tt.name) + assert.Contains(t, rec.Body.String(), "mycontainer") + assert.Contains(t, rec.Body.String(), "mycontainer") + }) + } +} + +func TestDeleteContainer_MissingReturns404(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + container string + }{ + {name: "missing_container_404", container: "does-not-exist"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + + rec := doRequest(t, h, http.MethodDelete, "/"+testAccount+"/"+tt.container+"?restype=container", nil, nil) + + require.Equal(t, http.StatusNotFound, rec.Code, tt.name) + assert.Contains(t, rec.Body.String(), "ContainerNotFound") + }) + } +} + +func TestBlobLifecycle_PutGetHeadDelete(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + body string + }{ + {name: "put_get_head_delete_blob", body: "hello azure blob"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + createContainer(t, h, "mycontainer") + + putHeaders := map[string]string{ + "x-ms-blob-type": "BlockBlob", + "Content-Type": "text/plain", + } + rec := doRequest(t, h, http.MethodPut, "/"+testAccount+"/mycontainer/myblob.txt", + []byte(tt.body), putHeaders) + require.Equal(t, http.StatusCreated, rec.Code, tt.name) + assert.NotEmpty(t, rec.Header().Get("ETag"), tt.name) + + rec = doRequest(t, h, http.MethodGet, "/"+testAccount+"/mycontainer/myblob.txt", nil, nil) + require.Equal(t, http.StatusOK, rec.Code, tt.name) + assert.Equal(t, tt.body, rec.Body.String(), tt.name) + assert.Equal(t, "text/plain", rec.Header().Get("Content-Type"), tt.name) + assert.Equal(t, "BlockBlob", rec.Header().Get("x-ms-blob-type"), tt.name) + + rec = doRequest(t, h, http.MethodHead, "/"+testAccount+"/mycontainer/myblob.txt", nil, nil) + require.Equal(t, http.StatusOK, rec.Code, tt.name) + assert.Empty(t, rec.Body.String(), tt.name) + assert.Equal(t, strconv.Itoa(len(tt.body)), rec.Header().Get("Content-Length"), tt.name) + + rec = doRequest(t, h, http.MethodDelete, "/"+testAccount+"/mycontainer/myblob.txt", nil, nil) + require.Equal(t, http.StatusAccepted, rec.Code, tt.name) + + rec = doRequest(t, h, http.MethodGet, "/"+testAccount+"/mycontainer/myblob.txt", nil, nil) + require.Equal(t, http.StatusNotFound, rec.Code, tt.name) + assert.Contains(t, rec.Body.String(), "BlobNotFound", tt.name) + }) + } +} + +func TestPutBlob_RequiresBlockBlobType(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + blobType string + }{ + {name: "missing_blob_type_rejected", blobType: ""}, + {name: "page_blob_type_rejected", blobType: "PageBlob"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + createContainer(t, h, "mycontainer") + + headers := map[string]string{} + if tt.blobType != "" { + headers["x-ms-blob-type"] = tt.blobType + } + + rec := doRequest(t, h, http.MethodPut, "/"+testAccount+"/mycontainer/myblob.txt", []byte("x"), headers) + + require.Equal(t, http.StatusBadRequest, rec.Code, tt.name) + assert.Contains(t, rec.Body.String(), "InvalidHeaderValue", tt.name) + }) + } +} + +func TestPutBlob_MissingContainerReturns404(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "put_blob_missing_container"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + + rec := doRequest(t, h, http.MethodPut, "/"+testAccount+"/does-not-exist/myblob.txt", + []byte("x"), map[string]string{"x-ms-blob-type": "BlockBlob"}) + + require.Equal(t, http.StatusNotFound, rec.Code, tt.name) + assert.Contains(t, rec.Body.String(), "ContainerNotFound", tt.name) + }) + } +} + +func TestGetBlob_MissingBlobReturns404(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "get_missing_blob"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + createContainer(t, h, "mycontainer") + + rec := doRequest(t, h, http.MethodGet, "/"+testAccount+"/mycontainer/does-not-exist.txt", nil, nil) + + require.Equal(t, http.StatusNotFound, rec.Code, tt.name) + assert.Contains(t, rec.Body.String(), "BlobNotFound", tt.name) + }) + } +} + +func TestGetBlob_RangeHeaderPartialRead(t *testing.T) { + t.Parallel() + + const body = "0123456789" + + tests := []struct { + name string + rangeValue string + wantStatus int + wantBody string + }{ + {name: "start_end", rangeValue: "bytes=2-5", wantStatus: http.StatusPartialContent, wantBody: "2345"}, + {name: "open_ended", rangeValue: "bytes=7-", wantStatus: http.StatusPartialContent, wantBody: "789"}, + {name: "suffix", rangeValue: "bytes=-3", wantStatus: http.StatusPartialContent, wantBody: "789"}, + {name: "unsatisfiable", rangeValue: "bytes=100-200", wantStatus: http.StatusRequestedRangeNotSatisfiable, wantBody: ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + createContainer(t, h, "mycontainer") + doRequest(t, h, http.MethodPut, "/"+testAccount+"/mycontainer/data.bin", + []byte(body), map[string]string{"x-ms-blob-type": "BlockBlob"}) + + rec := doRequest(t, h, http.MethodGet, "/"+testAccount+"/mycontainer/data.bin", nil, + map[string]string{"Range": tt.rangeValue}) + + require.Equal(t, tt.wantStatus, rec.Code, tt.name) + if tt.wantStatus == http.StatusPartialContent { + assert.Equal(t, tt.wantBody, rec.Body.String(), tt.name) + assert.NotEmpty(t, rec.Header().Get("Content-Range"), tt.name) + } + }) + } +} + +func TestListBlobs_MissingContainerReturns404(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "list_blobs_missing_container"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + + rec := doRequest(t, h, http.MethodGet, "/"+testAccount+"/does-not-exist?restype=container&comp=list", nil, nil) + + require.Equal(t, http.StatusNotFound, rec.Code, tt.name) + assert.Contains(t, rec.Body.String(), "ContainerNotFound", tt.name) + }) + } +} + +func TestListBlobs_ReturnsAllBlobs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + blobs []string + }{ + {name: "two_blobs", blobs: []string{"a.txt", "b.txt"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + createContainer(t, h, "mycontainer") + + for _, name := range tt.blobs { + doRequest(t, h, http.MethodPut, "/"+testAccount+"/mycontainer/"+name, + []byte("data"), map[string]string{"x-ms-blob-type": "BlockBlob"}) + } + + rec := doRequest(t, h, http.MethodGet, "/"+testAccount+"/mycontainer?restype=container&comp=list", nil, nil) + + require.Equal(t, http.StatusOK, rec.Code, tt.name) + for _, name := range tt.blobs { + assert.Contains(t, rec.Body.String(), ""+name+"", tt.name) + } + }) + } +} + +func TestHandler_Reset(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "reset_clears_containers"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + createContainer(t, h, "mycontainer") + + h.Reset() + + rec := doRequest(t, h, http.MethodGet, "/"+testAccount+"?comp=list", nil, nil) + require.Equal(t, http.StatusOK, rec.Code, tt.name) + assert.NotContains(t, rec.Body.String(), "mycontainer", tt.name) + }) + } +} + +func TestHandler_Name(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + assert.Equal(t, "AzureBlob", h.Name()) +} + +func TestHandler_GetSupportedOperations(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + ops := h.GetSupportedOperations() + + assert.Contains(t, ops, "PutBlob") + assert.Contains(t, ops, "GetBlob") + assert.Contains(t, ops, "ListContainers") +} + +// TestErrNilAppContext and TestProviderInit live in provider_test.go. + +func createContainer(t *testing.T, h *azureblob.Handler, name string) { + t.Helper() + + rec := doRequest(t, h, http.MethodPut, "/"+testAccount+"/"+name+"?restype=container", nil, nil) + require.Equal(t, http.StatusCreated, rec.Code) +} diff --git a/services/azureblob/persistence_test.go b/services/azureblob/persistence_test.go new file mode 100644 index 0000000000..58ca42db31 --- /dev/null +++ b/services/azureblob/persistence_test.go @@ -0,0 +1,104 @@ +package azureblob_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/azureblob" +) + +func TestSnapshotRestore_RoundTrip(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "roundtrip_container_and_blob"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + b := azureblob.NewInMemoryBackend() + require.NoError(t, b.CreateContainer("c1")) + _, err := b.PutBlob("c1", "blob1", []byte("payload"), "text/plain") + require.NoError(t, err) + + data := b.Snapshot(ctx) + require.NotEmpty(t, data, tt.name) + + restored := azureblob.NewInMemoryBackend() + require.NoError(t, restored.Restore(ctx, data)) + + containers := restored.ListContainers() + require.Len(t, containers, 1, tt.name) + assert.Equal(t, "c1", containers[0].Name, tt.name) + + _, blobData, err := restored.GetBlob("c1", "blob1") + require.NoError(t, err, tt.name) + assert.Equal(t, "payload", string(blobData), tt.name) + }) + } +} + +func TestRestore_IncompatibleVersionStartsEmpty(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + data []byte + }{ + {name: "garbage_bytes_discarded", data: []byte(`{"version":999,"containers":{}}`)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + b := azureblob.NewInMemoryBackend() + require.NoError(t, b.CreateContainer("preexisting")) + + require.NoError(t, b.Restore(ctx, tt.data)) + + assert.Empty(t, b.ListContainers(), tt.name) + }) + } +} + +func TestHandlerSnapshotRestore_Delegates(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "handler_delegates_to_backend"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + backend := azureblob.NewInMemoryBackend() + h := azureblob.NewHandler(backend) + require.NoError(t, backend.CreateContainer("c1")) + + data := h.Snapshot(ctx) + require.NotEmpty(t, data, tt.name) + + restoredBackend := azureblob.NewInMemoryBackend() + restoredHandler := azureblob.NewHandler(restoredBackend) + require.NoError(t, restoredHandler.Restore(ctx, data)) + + assert.Len(t, restoredBackend.ListContainers(), 1, tt.name) + }) + } +} diff --git a/services/azureblob/provider_test.go b/services/azureblob/provider_test.go new file mode 100644 index 0000000000..6e3a0da77f --- /dev/null +++ b/services/azureblob/provider_test.go @@ -0,0 +1,63 @@ +package azureblob_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/pkgs/service" + "github.com/blackbirdworks/gopherstack/services/azureblob" +) + +func TestProvider_Init_NilAppContext(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "nil_context_errors"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + p := &azureblob.Provider{} + _, err := p.Init(nil) + + require.Error(t, err, tt.name) + assert.ErrorIs(t, err, azureblob.ErrNilAppContext, tt.name) + }) + } +} + +func TestProvider_Init_ReturnsHandler(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "normal_init"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + p := &azureblob.Provider{} + reg, err := p.Init(&service.AppContext{}) + + require.NoError(t, err, tt.name) + require.NotNil(t, reg, tt.name) + assert.Equal(t, "AzureBlob", reg.Name(), tt.name) + }) + } +} + +func TestProvider_Name(t *testing.T) { + t.Parallel() + + p := &azureblob.Provider{} + assert.Equal(t, "AzureBlob", p.Name()) +} diff --git a/services/azureblob/range_test.go b/services/azureblob/range_test.go new file mode 100644 index 0000000000..59a18731ca --- /dev/null +++ b/services/azureblob/range_test.go @@ -0,0 +1,88 @@ +package azureblob_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/blackbirdworks/gopherstack/services/azureblob" +) + +func TestParseRange(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + header string + size int64 + wantStart int64 + wantEnd int64 + wantOK bool + }{ + {name: "start_end", header: "bytes=2-5", size: 10, wantStart: 2, wantEnd: 5, wantOK: true}, + {name: "open_ended", header: "bytes=7-", size: 10, wantStart: 7, wantEnd: 9, wantOK: true}, + {name: "suffix", header: "bytes=-3", size: 10, wantStart: 7, wantEnd: 9, wantOK: true}, + {name: "suffix_larger_than_size", header: "bytes=-100", size: 10, wantStart: 0, wantEnd: 9, wantOK: true}, + {name: "end_beyond_size_clamped", header: "bytes=5-1000", size: 10, wantStart: 5, wantEnd: 9, wantOK: true}, + {name: "no_bytes_prefix", header: "items=0-1", size: 10, wantOK: false}, + {name: "empty_header", header: "", size: 10, wantOK: false}, + {name: "multi_range_rejected", header: "bytes=0-1,3-4", size: 10, wantOK: false}, + {name: "start_beyond_size", header: "bytes=100-200", size: 10, wantOK: false}, + {name: "end_before_start", header: "bytes=5-2", size: 10, wantOK: false}, + {name: "malformed_no_dash", header: "bytes=abc", size: 10, wantOK: false}, + {name: "zero_size_suffix", header: "bytes=-5", size: 0, wantOK: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + start, end, ok := azureblob.ParseRange(tt.header, tt.size) + + assert.Equal(t, tt.wantOK, ok, tt.name) + if tt.wantOK { + assert.Equal(t, tt.wantStart, start, tt.name) + assert.Equal(t, tt.wantEnd, end, tt.name) + } + }) + } +} + +func TestSplitPath(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path string + wantAccount string + wantContainer string + wantBlob string + }{ + {name: "empty", path: "", wantAccount: "", wantContainer: "", wantBlob: ""}, + {name: "account_only", path: "/devstoreaccount1", wantAccount: "devstoreaccount1"}, + { + name: "account_and_container", path: "/devstoreaccount1/mycontainer", + wantAccount: "devstoreaccount1", wantContainer: "mycontainer", + }, + { + name: "account_container_blob", path: "/devstoreaccount1/mycontainer/myblob.txt", + wantAccount: "devstoreaccount1", wantContainer: "mycontainer", wantBlob: "myblob.txt", + }, + { + name: "blob_name_with_slashes", path: "/devstoreaccount1/mycontainer/dir/sub/file.txt", + wantAccount: "devstoreaccount1", wantContainer: "mycontainer", wantBlob: "dir/sub/file.txt", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + account, container, blob := azureblob.SplitPath(tt.path) + + assert.Equal(t, tt.wantAccount, account, tt.name) + assert.Equal(t, tt.wantContainer, container, tt.name) + assert.Equal(t, tt.wantBlob, blob, tt.name) + }) + } +} diff --git a/services/azureblob/store_test.go b/services/azureblob/store_test.go new file mode 100644 index 0000000000..1eb973cb77 --- /dev/null +++ b/services/azureblob/store_test.go @@ -0,0 +1,239 @@ +package azureblob_test + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/azureblob" +) + +func TestInMemoryBackend_ContainerCreateListDelete(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "create_list_delete"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := azureblob.NewInMemoryBackend() + + require.NoError(t, b.CreateContainer("c1")) + assert.ErrorIs(t, b.CreateContainer("c1"), azureblob.ErrContainerAlreadyExists) + + containers := b.ListContainers() + require.Len(t, containers, 1) + assert.Equal(t, "c1", containers[0].Name) + + require.NoError(t, b.DeleteContainer("c1")) + assert.Empty(t, b.ListContainers()) + assert.ErrorIs(t, b.DeleteContainer("c1"), azureblob.ErrContainerNotFound) + }) + } +} + +func TestInMemoryBackend_BlobPutGetHeadDelete(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + data string + }{ + {name: "roundtrip", data: "hello world"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := azureblob.NewInMemoryBackend() + require.NoError(t, b.CreateContainer("c1")) + + info, err := b.PutBlob("c1", "blob1", []byte(tt.data), "text/plain") + require.NoError(t, err) + assert.Equal(t, int64(len(tt.data)), info.ContentLength) + assert.NotEmpty(t, info.ETag) + + gotInfo, gotData, err := b.GetBlob("c1", "blob1") + require.NoError(t, err) + assert.Equal(t, tt.data, string(gotData)) + assert.Equal(t, info.ETag, gotInfo.ETag) + + headInfo, err := b.HeadBlob("c1", "blob1") + require.NoError(t, err) + assert.Equal(t, info.ContentLength, headInfo.ContentLength) + + require.NoError(t, b.DeleteBlob("c1", "blob1")) + _, _, err = b.GetBlob("c1", "blob1") + assert.ErrorIs(t, err, azureblob.ErrBlobNotFound) + }) + } +} + +func TestInMemoryBackend_MissingContainerErrors(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + op func(b *azureblob.InMemoryBackend) error + }{ + {name: "put_blob", op: func(b *azureblob.InMemoryBackend) error { + _, err := b.PutBlob("missing", "blob1", []byte("x"), "") + + return err + }}, + {name: "get_blob", op: func(b *azureblob.InMemoryBackend) error { + _, _, err := b.GetBlob("missing", "blob1") + + return err + }}, + {name: "head_blob", op: func(b *azureblob.InMemoryBackend) error { + _, err := b.HeadBlob("missing", "blob1") + + return err + }}, + {name: "delete_blob", op: func(b *azureblob.InMemoryBackend) error { + return b.DeleteBlob("missing", "blob1") + }}, + {name: "list_blobs", op: func(b *azureblob.InMemoryBackend) error { + _, err := b.ListBlobs("missing") + + return err + }}, + {name: "delete_container", op: func(b *azureblob.InMemoryBackend) error { + return b.DeleteContainer("missing") + }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := azureblob.NewInMemoryBackend() + err := tt.op(b) + + require.Error(t, err) + assert.True(t, errors.Is(err, azureblob.ErrContainerNotFound), tt.name) + }) + } +} + +func TestInMemoryBackend_MissingBlobErrors(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "get_missing_blob"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := azureblob.NewInMemoryBackend() + require.NoError(t, b.CreateContainer("c1")) + + _, _, err := b.GetBlob("c1", "does-not-exist") + assert.ErrorIs(t, err, azureblob.ErrBlobNotFound, tt.name) + }) + } +} + +func TestInMemoryBackend_ListBlobsSortedByName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + blobs []string + want []string + }{ + {name: "sorted", blobs: []string{"c.txt", "a.txt", "b.txt"}, want: []string{"a.txt", "b.txt", "c.txt"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := azureblob.NewInMemoryBackend() + require.NoError(t, b.CreateContainer("c1")) + + for _, name := range tt.blobs { + _, err := b.PutBlob("c1", name, []byte("x"), "") + require.NoError(t, err) + } + + blobs, err := b.ListBlobs("c1") + require.NoError(t, err) + + got := make([]string, len(blobs)) + for i, bi := range blobs { + got[i] = bi.Name + } + + assert.Equal(t, tt.want, got, tt.name) + }) + } +} + +func TestInMemoryBackend_Reset(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "reset_clears_all"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := azureblob.NewInMemoryBackend() + require.NoError(t, b.CreateContainer("c1")) + _, err := b.PutBlob("c1", "blob1", []byte("x"), "") + require.NoError(t, err) + + b.Reset() + + assert.Empty(t, b.ListContainers(), tt.name) + }) + } +} + +func TestInMemoryBackend_PutBlobOverwrites(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + }{ + {name: "overwrite_replaces_data"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := azureblob.NewInMemoryBackend() + require.NoError(t, b.CreateContainer("c1")) + + _, err := b.PutBlob("c1", "blob1", []byte("first"), "") + require.NoError(t, err) + + info, err := b.PutBlob("c1", "blob1", []byte("second-longer"), "") + require.NoError(t, err) + + _, data, err := b.GetBlob("c1", "blob1") + require.NoError(t, err) + assert.Equal(t, "second-longer", string(data), tt.name) + assert.Equal(t, int64(len("second-longer")), info.ContentLength, tt.name) + }) + } +} From 2744ef8018a49ec179d5e28ecbe6c73baaffa713 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:19:12 -0500 Subject: [PATCH 09/30] azureblob: add README and PARITY docs Seeds PARITY.md/README.md in the format cmd/gendocs renders for other services, documenting the M0 op coverage, the dedicated-port architectural decision, and the intentional MVP gaps (multipart upload, ACLs, metadata, conditional headers, copy-blob) per AZURE.md's M0/M1 split. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Jzq1rtNNMjzhnvZSpcGr1F --- services/azureblob/PARITY.md | 82 ++++++++++++++++++++++++++++++++++++ services/azureblob/README.md | 38 +++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 services/azureblob/PARITY.md create mode 100644 services/azureblob/README.md diff --git a/services/azureblob/PARITY.md b/services/azureblob/PARITY.md new file mode 100644 index 0000000000..0cd4686868 --- /dev/null +++ b/services/azureblob/PARITY.md @@ -0,0 +1,82 @@ +--- +service: azureblob +sdk_module: azure-sdk-for-go/sdk/storage/azblob@v1.7.0 +last_audit_commit: (initial seed, no audit history yet) +last_audit_date: 2026-09-02 +overall: C +# Per-op or per-op-family status. Values: ok | partial | gap | deferred. +# wire=response/request shape vs SDK; errors=code+HTTP status; state=real mutate/read; persist=in backendSnapshot. +ops: + ListContainers: {wire: ok, errors: ok, state: ok, persist: ok, note: "GET /?comp=list. No prefix/marker/maxresults pagination support yet -- returns all containers in one page with an always-empty NextMarker."} + CreateContainer: {wire: ok, errors: ok, state: ok, persist: ok, note: "PUT //?restype=container. No metadata (x-ms-meta-*) or public-access-level support."} + DeleteContainer: {wire: ok, errors: ok, state: ok, persist: ok, note: "DELETE //?restype=container. No lease/If-Match conditional support."} + ListBlobs: {wire: ok, errors: ok, state: ok, persist: n/a, note: "GET //?restype=container&comp=list. Flat listing only -- no prefix/delimiter/marker/maxresults, no snapshot/version/metadata inclusion flags."} + PutBlob: {wire: partial, errors: ok, state: ok, persist: ok, note: "PUT ///, BlockBlob only (x-ms-blob-type required and validated). Whole-body single-request PUT only -- Put Block/Put Block List (large-object multipart upload) is not implemented, see gaps."} + GetBlob: {wire: ok, errors: ok, state: ok, persist: n/a, note: "GET ///, single-range Range header supported (start-end, open-ended, and suffix forms); multi-range requests are rejected as unsatisfiable rather than served."} + GetBlobProperties: {wire: ok, errors: ok, state: ok, persist: n/a, note: "HEAD ///. Returns ETag/Last-Modified/Content-Length/Content-Type/x-ms-blob-type; no x-ms-meta-* or lease-state headers."} + DeleteBlob: {wire: ok, errors: ok, state: ok, persist: ok, note: "DELETE ///. No snapshot/version-scoped delete, no soft-delete."} +families: + auth: {status: deferred, note: "Authorization header is accepted structurally (SharedKey prefix or absent) but never cryptographically verified -- matches services/s3's PresignSecret-opt-in philosophy. Real SharedKey canonicalization/HMAC verification is deferred to pkgs/azureauth, landing on a separate branch (azure/auth-pkg); this package has a TODO(azure-integration) marker at the handler's auth entry point."} + blob_body_headers: {status: ok, note: "x-ms-version, x-ms-request-id, and Date are set on every response (success and error paths) via setCommonHeaders, so azure-sdk-for-go's response parsing does not error on missing headers."} + routing_isolation: {status: ok, note: "Runs on its own dedicated *http.Server (default port 10000, AZURE_BLOB_PORT override), never registered into the shared AWS single-port Router -- see provider.go's Provider doc comment and AZURE.md section 4 for the full rationale."} +gaps: + - "Put Block / Put Block List (large-object multipart upload) is not implemented -- Put Blob only accepts a single whole-body BlockBlob PUT. Deliberate M0 scope per AZURE.md; tracked for a later milestone (M1 in AZURE.md's plan)." + - "No ACL / container public-access-level support (x-ms-blob-public-access, Set/Get Container ACL are unimplemented)." + - "No blob or container metadata (x-ms-meta-* headers) -- neither stored on PUT/Create nor returned on GET/HEAD/List." + - "No conditional-header support (If-Match/If-None-Match/If-Modified-Since/If-Unmodified-Since) on any operation -- every write unconditionally overwrites, every read unconditionally succeeds regardless of ETag/date preconditions." + - "No Copy Blob (server-side or cross-account) support." + - "No snapshot, versioning, soft-delete, lease, or tier (hot/cool/archive) support." + - "List Containers / List Blobs return every result in one page; no prefix/marker/maxresults pagination." + - "Auth is structurally permissive only -- see families.auth. Real SharedKey verification (pkgs/azureauth) is a separate, not-yet-landed dependency." + All gaps above are intentional MVP scope per AZURE.md's M0/M1 split, not oversights; see AZURE.md sections 2 and 8 for the milestone plan. +deferred: + - "Initial implementation pass (2026-09-02): seeded this service from scratch per AZURE.md M0. No prior audit history to reconcile. sdk_module pinned to the latest azure-sdk-for-go blob module version documented in AZURE.md at authoring time; not yet cross-checked against a live SDK import in this repo (azure-sdk-for-go is not currently a go.mod dependency -- this package speaks the wire protocol directly rather than through the SDK's server-side types)." + - "cli.go registration is deliberately NOT wired up in this pass -- a human integrates this provider once pkgs/azureauth (a separate branch, azure/auth-pkg) also lands, per the task's explicit deferral." + - "No Go integration test (test/integration/azureblob_test.go) yet -- that requires the cli.go wiring above, which is out of scope for this pass. Unit tests exercise the handler/backend directly via httptest instead." +leaks: {status: clean, note: "No background goroutines, tickers, or janitor: InMemoryBackend is pure in-memory maps guarded by one sync.RWMutex, with no TTL/expiry sweep in this MVP scope. The dedicated *http.Server started by StartWorker is stopped by Shutdown via srv.Shutdown(ctx), mirroring cli.go's own top-level server lifecycle."} +--- + +## Notes + +### Why Azure Blob gets its own port instead of the shared AWS router +Every other gopherstack service registers a `RouteMatcher` into the shared +single-port AWS `Router` (`pkgs/service/router.go`), which disambiguates +services by header (`X-Amz-Target`) or distinctive path/form shape. Azure +Blob's REST path shape (`///`) has no such +service-identifying header, and colliding with Azure Queue/Table's identical +`//` shape (once those land) would be exactly the +ambiguity the AWS router avoids by construction. Instead, `Provider.Init` +resolves a dedicated port (default 10000, mirroring Azurite's own +Blob-service default) and the returned `*Handler` implements +`service.BackgroundWorker`, standing up its own `*echo.Echo` + `*http.Server` +in `StartWorker`. See `provider.go`'s `Provider` doc comment and AZURE.md +section 4 for the full rationale, including why `pkgs/portalloc.Allocator` +(which only hands out the next free port in a range, with no way to reserve +a *specific* preferred port) couldn't be used as-is. + +### Auth +The `Authorization` header is accepted on structure alone -- a `SharedKey +...` prefix, or no header at all, both pass -- matching this repo's +permissive-by-default philosophy (`services/s3/sigv4.go`'s +`PresignSecret`-opt-in pattern). Real SharedKey HMAC verification is planned +for `pkgs/azureauth`, landing separately; `handler.go`'s `checkAuth` carries +the wiring TODO. + +### Blob names with slashes +Azure blob names may contain `/` as a virtual-directory separator (e.g. +`logs/2026/09/02.txt`). `splitPath` only ever splits the URL into three +pieces (`account`, `container`, everything else as `blob`), so a blob name's +internal slashes are preserved intact rather than being mistaken for +additional path segments. + +### Range reads +`Get Blob` supports the standard `Range: bytes=start-end`, open-ended +(`bytes=N-`), and suffix (`bytes=-N`) forms, returning `206 Partial Content` +with `Content-Range` set. Multi-range requests (`bytes=0-1,3-4`) are rejected +with `416 Requested Range Not Satisfiable` rather than served -- Azure's own +Get Blob does not support multi-range either. + +## More + +- [Full parity audit](PARITY.md) +- [All services](../../README.md#services) diff --git a/services/azureblob/README.md b/services/azureblob/README.md new file mode 100644 index 0000000000..9698a9fd1c --- /dev/null +++ b/services/azureblob/README.md @@ -0,0 +1,38 @@ + +# Azure Blob Storage + +**Parity grade: C** · SDK `azure-sdk-for-go/sdk/storage/azblob@v1.7.0` · last audited 2026-09-02 (initial seed) + +## Coverage + +| Metric | Value | +| --- | --- | +| PARITY entries audited | 8 (7 ok, 1 partial) | +| Feature families | 3 (2 ok, 1 deferred) | +| Known gaps | 8 | +| Deferred items | 3 | +| Resource leaks | clean | + +### Known gaps + +- Put Block / Put Block List (large-object multipart upload) is not implemented -- Put Blob only accepts a single whole-body BlockBlob PUT. Deliberate M0 scope per AZURE.md; tracked for a later milestone. +- No ACL / container public-access-level support (x-ms-blob-public-access, Set/Get Container ACL are unimplemented). +- No blob or container metadata (x-ms-meta-* headers) -- neither stored on PUT/Create nor returned on GET/HEAD/List. +- No conditional-header support (If-Match/If-None-Match/If-Modified-Since/If-Unmodified-Since) on any operation. +- No Copy Blob (server-side or cross-account) support. +- No snapshot, versioning, soft-delete, lease, or tier (hot/cool/archive) support. +- List Containers / List Blobs return every result in one page; no prefix/marker/maxresults pagination. +- Auth is structurally permissive only: the Authorization header is accepted on shape alone (a `SharedKey ...` prefix, or absent) and never cryptographically verified. Real SharedKey verification depends on `pkgs/azureauth`, which lands on a separate branch and is not yet importable from this package. + +All gaps above are intentional MVP scope per AZURE.md's M0/M1 split, not oversights. + +### Deferred + +- Initial implementation pass (2026-09-02): seeded this service from scratch per AZURE.md M0. No prior audit history to reconcile. +- `cli.go` registration is deliberately not wired up in this pass -- a human integrates this provider once `pkgs/azureauth` (branch `azure/auth-pkg`) also lands. +- No Go integration test (`test/integration/azureblob_test.go`) yet -- that requires the `cli.go` wiring above. Unit tests exercise the handler/backend directly via `httptest` instead. + +## More + +- [Full parity audit](PARITY.md) +- [All services](../../README.md#services) From 402df94c2b7fbbfc87a0103b942c8b6cf71e45d7 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:57:08 -0500 Subject: [PATCH 10/30] cli: register AzureBlob provider Wires services/azureblob.Provider into getMostRecentServiceProviders. It does not participate in the shared AWS single-port router (see AZURE.md section 4 and services/azureblob/provider.go's doc comment) but startBackgroundWorkers still calls its StartWorker via the service.BackgroundWorker interface, so its dedicated listener comes up alongside every other service. --- cli.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cli.go b/cli.go index 8f4299e09d..3cd4368c27 100644 --- a/cli.go +++ b/cli.go @@ -89,6 +89,7 @@ import ( athenabackend "github.com/blackbirdworks/gopherstack/services/athena" autoscalingbackend "github.com/blackbirdworks/gopherstack/services/autoscaling" awsconfigbackend "github.com/blackbirdworks/gopherstack/services/awsconfig" + azureblobbackend "github.com/blackbirdworks/gopherstack/services/azureblob" backupbackend "github.com/blackbirdworks/gopherstack/services/backup" batchbackend "github.com/blackbirdworks/gopherstack/services/batch" bedrockbackend "github.com/blackbirdworks/gopherstack/services/bedrock" @@ -3565,6 +3566,7 @@ func getNewestServiceProviders() []service.Provider { func getMostRecentServiceProviders() []service.Provider { return []service.Provider{ + &azureblobbackend.Provider{}, &pinpointbackend.Provider{}, &pipesbackend.Provider{}, &accessanalyzerbackend.Provider{}, From 6dbdefb4c23bf4cf3d9a7e67a843f273ed469450 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:57:09 -0500 Subject: [PATCH 11/30] azureblob: wire pkgs/azureauth for Authorization header parsing Resolves the azure-integration TODO: checkAuth now parses a present Authorization header via azureauth.ParseAuthorizationHeader to prove a real Azure SDK's header round-trips through this package. Behavior is unchanged (still permissive-by-default, no rejection) -- enforcing azureauth.VerifySharedKey is deliberately deferred past M0, mirroring services/s3's opt-in WithPresignValidation stance. --- services/azureblob/handler.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/services/azureblob/handler.go b/services/azureblob/handler.go index 518d436970..403077064d 100644 --- a/services/azureblob/handler.go +++ b/services/azureblob/handler.go @@ -14,6 +14,7 @@ import ( "github.com/labstack/echo/v5" + "github.com/blackbirdworks/gopherstack/pkgs/azureauth" "github.com/blackbirdworks/gopherstack/pkgs/httputils" "github.com/blackbirdworks/gopherstack/pkgs/logger" "github.com/blackbirdworks/gopherstack/pkgs/service" @@ -151,8 +152,24 @@ func (h *Handler) Handler() echo.HandlerFunc { // permissive-by-default auth philosophy (see services/s3/sigv4.go). Any // structurally-present "SharedKey ..." header, or its absence, is accepted. // -// TODO(azure-integration): wire real SharedKey verification via pkgs/azureauth once that package lands (see AZURE.md). -func (h *Handler) checkAuth(_ *http.Request) {} +// pkgs/azureauth.ParseAuthorizationHeader is used to prove a real Azure SDK's +// Authorization header round-trips through this package (account name / +// scheme extraction), but a malformed or absent header is still accepted -- +// matching services/s3's own opt-in verification stance. Rejecting invalid +// signatures via azureauth.VerifySharedKey is deliberately deferred past M0 +// (see AZURE.md section 5): it needs to be exercised against real SDK +// request shapes first, the same way S3's WithPresignValidation is opt-in +// rather than on-by-default. +func (h *Handler) checkAuth(r *http.Request) { + authHeader := r.Header.Get("Authorization") + if authHeader == "" { + return // anonymous; accepted by design at this milestone + } + + if _, ok := azureauth.ParseAuthorizationHeader(authHeader); !ok { + return // structurally malformed; still accepted at this milestone + } +} // setCommonHeaders sets the headers real Azure SDKs expect on every // response, success or error. From e7e53300d39d435cb37c5d01c819d82f02e62cfc Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:57:09 -0500 Subject: [PATCH 12/30] test/integration: add Azure Blob container/blob lifecycle tests Exposes the container's dedicated Azure Blob port (10000/tcp, mirroring the existing mqttEndpoint pattern for 1883/tcp) and adds TestIntegration_AzureBlob_ContainerAndBlobLifecycle and TestIntegration_AzureBlob_ListContainers using the real azure-sdk-for-go blob client against the Azurite well-known devstoreaccount1 dev credential, per AZURE.md's wire-compatibility requirement. --- go.mod | 3 + go.sum | 14 +++ test/integration/azureblob_test.go | 132 +++++++++++++++++++++++++++++ test/integration/main_test.go | 23 ++++- 4 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 test/integration/azureblob_test.go diff --git a/go.mod b/go.mod index 260801123f..1e3407ffa1 100644 --- a/go.mod +++ b/go.mod @@ -208,6 +208,7 @@ require github.com/aws/aws-sdk-go-v2/service/omics v1.49.5 require github.com/aws/aws-sdk-go-v2/service/cleanrooms v1.49.4 require ( + github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.8.0 github.com/aws/aws-sdk-go-v2/service/account v1.35.4 github.com/aws/aws-sdk-go-v2/service/cloudfrontkeyvaluestore v1.15.4 github.com/aws/aws-sdk-go-v2/service/directconnect v1.44.1 @@ -227,6 +228,8 @@ require ( ) require ( + github.com/Azure/azure-sdk-for-go/sdk/azcore v1.22.0 // indirect + github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0 // indirect github.com/dustin/go-humanize v1.0.1 // indirect github.com/ncruces/go-strftime v1.0.0 // indirect github.com/palkan/mulint v1.1.0 // indirect diff --git a/go.sum b/go.sum index dbcf302ee7..60d541e35a 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,20 @@ dario.cat/mergo v1.0.2 h1:85+piFYR1tMbRrLcDwR18y4UKJ3aH1Tbzi24VRW1TK8= dario.cat/mergo v1.0.2/go.mod h1:E/hbnu0NxMFBjpMIE34DRGLWqDy0g5FuKDhCb31ngxA= github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6 h1:He8afgbRMd7mFxO99hRNu+6tazq8nFF9lIwo9JFroBk= github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6/go.mod h1:8o94RPi1/7XTJvwPpRSzSUedZrtlirdB3r9Z20bi2f8= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.22.0 h1:aokoqcHvaGjiM3VpjKDfMMnF/8epJ+Q1HLJ7CudztqE= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.22.0/go.mod h1:/WYEx9pcM9Y+Dd/APJaNlSvVSvzl54rrMdZT5+Oi2LM= +github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1 h1:Hk5QBxZQC1jb2Fwj6mpzme37xbCDdNTxU7O9eb5+LB4= +github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1/go.mod h1:IYus9qsFobWIc2YVwe/WPjcnyCkPKtnHAqUYeebc8z0= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0 h1:fhqpLE3UEXi9lPaBRpQ6XuRW0nU7hgg4zlmZZa+a9q4= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0/go.mod h1:7dCRMLwisfRH3dBupKeNCioWYUZ4SS09Z14H+7i8ZoY= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage v1.8.1 h1:/Zt+cDPnpC3OVDm/JKLOs7M2DKmLRIIp3XIx9pHHiig= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage v1.8.1/go.mod h1:Ng3urmn6dYe8gnbCMoHHVl5APYz2txho3koEkV2o2HA= +github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.8.0 h1:irsmOWwkp0KCTTNS5e2hdFeIvSQClQo2No3IaNmL3Vw= +github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.8.0/go.mod h1:GWcBkQj3MqN7ozHKLaCCAuNLiXoIGv2RtanfAwSjY/Y= github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c h1:udKWzYgxTojEKWjV8V+WSxDXJ4NFATAsZjh8iIbsQIg= github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= +github.com/AzureAD/microsoft-authentication-library-for-go v1.7.2 h1:RHK7bS+HQMslb1sZpAokUt+zTVmue0hKSs2C791hhzU= +github.com/AzureAD/microsoft-authentication-library-for-go v1.7.2/go.mod h1:HKpQxkWaGLJ+D/5H8QRpyQXA1eKjxkFlOMwck5+33Jk= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/agnivade/levenshtein v1.2.1 h1:EHBY3UOn1gwdy/VbFwgo4cxecRznFk7fKWN1KOX7eoM= @@ -583,6 +595,8 @@ github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJw github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgrGnAve2nCC8+7h8Q0M= github.com/palkan/mulint v1.1.0 h1:W0eSO5N53R4TEBXyFpwpWHeKPT7YWMXPyGAdj79isbk= github.com/palkan/mulint v1.1.0/go.mod h1:gJr/thrBGoRQV4S4fgT8PJ4dytA+5x7Jks6zTU0VRP0= +github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= +github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= diff --git a/test/integration/azureblob_test.go b/test/integration/azureblob_test.go new file mode 100644 index 0000000000..8a79a90d7d --- /dev/null +++ b/test/integration/azureblob_test.go @@ -0,0 +1,132 @@ +package integration_test + +import ( + "bytes" + "io" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// azureBlobDevAccountName and azureBlobDevAccountKey are Azurite's published +// well-known development storage account name/key, which gopherstack accepts +// as its default identity (see pkgs/azureauth and AZURE.md section 5) so +// that unmodified Azure SDKs pointed at this server work out of the box, the +// same way real SDKs work against Azurite with no configuration beyond the +// endpoint. +const ( + azureBlobDevAccountName = "devstoreaccount1" + azureBlobDevAccountKey = "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==" +) + +// createAzureBlobClient returns an azure-sdk-for-go Blob client pointed at +// the shared test container's dedicated Azure Blob port (see +// azureBlobEndpoint in main_test.go). Skips the calling test if that port +// could not be determined (mirrors how MQTT/IoT tests are skipped when +// mqttEndpoint is unavailable). +func createAzureBlobClient(t *testing.T) *azblob.Client { + t.Helper() + + if azureBlobEndpoint == "" { + t.Skip("Azure Blob endpoint not available (mapped port could not be determined)") + } + + cred, err := azblob.NewSharedKeyCredential(azureBlobDevAccountName, azureBlobDevAccountKey) + require.NoError(t, err, "unable to build SharedKeyCredential") + + // Path-style addressing (account name as the first path segment), matching + // Azurite's own convention and gopherstack's single-account routing. + client, err := azblob.NewClientWithSharedKeyCredential( + azureBlobEndpoint+"/"+azureBlobDevAccountName, cred, nil, + ) + require.NoError(t, err, "unable to construct Azure Blob client") + + return client +} + +func TestIntegration_AzureBlob_ContainerAndBlobLifecycle(t *testing.T) { + t.Parallel() + dumpContainerLogsOnFailure(t) + client := createAzureBlobClient(t) + ctx := t.Context() + + containerName := "test-container-" + uuid.NewString() + const blobName = "test-blob.txt" + content := []byte("hello from gopherstack azureblob") + + // CreateContainer + _, err := client.CreateContainer(ctx, containerName, nil) + require.NoError(t, err) + + // PutBlob + _, err = client.UploadBuffer(ctx, containerName, blobName, content, nil) + require.NoError(t, err) + + // ListBlobs: uploaded blob should appear + found := false + + pager := client.NewListBlobsFlatPager(containerName, nil) + for pager.More() { + page, pageErr := pager.NextPage(ctx) + require.NoError(t, pageErr) + + for _, b := range page.Segment.BlobItems { + if b.Name != nil && *b.Name == blobName { + found = true + } + } + } + + assert.True(t, found, "uploaded blob should appear in ListBlobs") + + // GetBlob: downloaded bytes should round-trip exactly + downloadResp, err := client.DownloadStream(ctx, containerName, blobName, nil) + require.NoError(t, err) + + body, err := io.ReadAll(downloadResp.Body) + _ = downloadResp.Body.Close() + require.NoError(t, err) + assert.True(t, bytes.Equal(content, body), "downloaded blob content should match uploaded content") + + // DeleteBlob + _, err = client.DeleteBlob(ctx, containerName, blobName, nil) + require.NoError(t, err) + + // DeleteContainer + _, err = client.DeleteContainer(ctx, containerName, nil) + require.NoError(t, err) +} + +func TestIntegration_AzureBlob_ListContainers(t *testing.T) { + t.Parallel() + dumpContainerLogsOnFailure(t) + client := createAzureBlobClient(t) + ctx := t.Context() + + containerName := "test-list-container-" + uuid.NewString() + + _, err := client.CreateContainer(ctx, containerName, nil) + require.NoError(t, err) + + found := false + + pager := client.NewListContainersPager(nil) + for pager.More() { + page, pageErr := pager.NextPage(ctx) + require.NoError(t, pageErr) + + for _, c := range page.ContainerItems { + if c.Name != nil && *c.Name == containerName { + found = true + } + } + } + + assert.True(t, found, "created container should appear in ListContainers") + + _, err = client.DeleteContainer(ctx, containerName, nil) + require.NoError(t, err) +} diff --git a/test/integration/main_test.go b/test/integration/main_test.go index e6d863b4ea..03bfd4b89c 100644 --- a/test/integration/main_test.go +++ b/test/integration/main_test.go @@ -116,6 +116,17 @@ var endpoint string //nolint:gochecknoglobals // Set in TestMain for integration tests. var mqttEndpoint string +// azureBlobEndpoint is the Azure Blob Storage-compatible endpoint for the +// running Gopherstack container (its own dedicated port -- see +// services/azureblob/provider.go and AZURE.md section 4 for why this service +// cannot share the main AWS endpoint/port). Left empty (and Azure Blob tests +// skipped) if the mapped port cannot be determined, mirroring mqttEndpoint's +// non-fatal behavior above. This is initialized by TestMain before running +// integration tests. +// +//nolint:gochecknoglobals // Set in TestMain for integration tests. +var azureBlobEndpoint string + // sharedContainer holds a reference to the container for cleanup and log dumping on test failures. // This is initialized by TestMain before running integration tests. // @@ -240,7 +251,7 @@ func TestMain(m *testing.M) { options.PullParent = false }, }, - ExposedPorts: []string{"8000/tcp", "1883/tcp"}, + ExposedPorts: []string{"8000/tcp", "1883/tcp", "10000/tcp"}, WaitingFor: wait.ForAll( wait.ForHTTP("/"). WithPort("8000/tcp"). @@ -248,6 +259,8 @@ func TestMain(m *testing.M) { WithStartupTimeout(60*time.Second), wait.ForListeningPort("1883/tcp"). WithStartupTimeout(60*time.Second), + wait.ForListeningPort("10000/tcp"). + WithStartupTimeout(60*time.Second), ), } @@ -289,6 +302,14 @@ func TestMain(m *testing.M) { logger.Info("MQTT broker running", "endpoint", mqttEndpoint) } + azureBlobPort, err := container.MappedPort(ctx, "10000") + if err != nil { + logger.Warn("failed to get Azure Blob mapped port; Azure Blob tests will be skipped", "error", err) + } else { + azureBlobEndpoint = "http://localhost:" + azureBlobPort.Port() + logger.Info("Azure Blob Storage-compatible endpoint running", "endpoint", azureBlobEndpoint) + } + code := m.Run() if sharedContainer != nil { From f1427114bad4780ace9d426fe73ab594524e51a4 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Wed, 2 Sep 2026 19:57:10 -0500 Subject: [PATCH 13/30] test: allowlist azureblob's MD5 use and seed its snapshot golden entry services/azureblob/store.go uses crypto/md5 for Content-MD5/ETag generation, which is MD5 by specification for Azure Blob (same justification as S3's already-allowlisted ETag use) -- add the required weak_hash_guard_test.go entry. Also regenerate pkgs/persistence/testdata/snapshot_inventory.json (go test ./pkgs/persistence/... -run TestSnapshotVersionGuard -update) so the guard has a golden entry for azureblob's new backendSnapshot shape. --- pkgs/persistence/testdata/snapshot_inventory.json | 14 ++++++++++++++ weak_hash_guard_test.go | 6 ++++++ 2 files changed, 20 insertions(+) diff --git a/pkgs/persistence/testdata/snapshot_inventory.json b/pkgs/persistence/testdata/snapshot_inventory.json index 31c60fd707..e66e3cd6c9 100644 --- a/pkgs/persistence/testdata/snapshot_inventory.json +++ b/pkgs/persistence/testdata/snapshot_inventory.json @@ -2487,6 +2487,20 @@ ], "version": 4 }, + "azureblob": { + "fields": [ + "backendSnapshot.Containers map[string]*storedContainer `json:\"containers\"`", + "storedBlob.ContentType string", + "storedBlob.Data []byte", + "storedBlob.ETag string", + "storedBlob.LastModified time.Time", + "storedBlob.Name string", + "storedContainer.Blobs map[string]*storedBlob", + "storedContainer.CreatedAt time.Time", + "storedContainer.Name string" + ], + "version": 1 + }, "backup": { "fields": [ "AdvancedBackupSetting.BackupOptions map[string]string `json:\"backupOptions,omitempty\"`", diff --git a/weak_hash_guard_test.go b/weak_hash_guard_test.go index 239bb7902a..1ee7fd7de2 100644 --- a/weak_hash_guard_test.go +++ b/weak_hash_guard_test.go @@ -25,6 +25,12 @@ func allowedWeakHashFiles() map[string]string { // checksum echoed on the wire, never a credential. gopherstack-ziv9. "services/ssm/document_hashes.go": "DocumentDescription.Sha1 parity, verified", + // Azure Blob's Content-MD5/ETag are MD5 by specification (same as S3's + // ETag, already allowlisted below) -- a content-integrity fingerprint + // echoed on the wire so azure-sdk-for-go's blob client can validate + // upload/download integrity, never a credential or security hash. + "services/azureblob/store.go": "Content-MD5/ETag generation, Azure Blob wire-protocol requirement, verified", + // Pre-existing at the time this guard was added, and NOT individually // audited. Each is presumed an AWS-protocol requirement -- S3 ETags are // MD5 by specification, TOTP is HMAC-SHA1 by RFC 6238, key-pair From f747c6229bb7956185157495d6aabf1ca0d2f9dc Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 08:07:08 -0500 Subject: [PATCH 14/30] azureauth: fix header-mutation bug in CanonicalizedHeaders, add regression test http.Header.Values returns the live slice backing r.Header, so normalizing whitespace in place silently rewrote the caller's request headers as a side effect of computing a signature -- contradicting SignSharedKey/SignSharedKeyLite's own documented "does not modify r" contract. Copy into a fresh slice before normalizing. Also: canonicalize header-key string literals passed to Header.Set/Get/ Values (repo's canonicalheader linter requires this; functionally a no-op since Header methods canonicalize internally regardless), and convert TestVerifySharedKeyLite/TestStringToSign/TestStringToSignLite to the repo's table-driven test convention. --- pkgs/azureauth/azureauth_test.go | 214 ++++++++++++++++++++++--------- pkgs/azureauth/canonical.go | 11 +- 2 files changed, 159 insertions(+), 66 deletions(-) diff --git a/pkgs/azureauth/azureauth_test.go b/pkgs/azureauth/azureauth_test.go index 925c1ce4bb..3d55d09763 100644 --- a/pkgs/azureauth/azureauth_test.go +++ b/pkgs/azureauth/azureauth_test.go @@ -102,8 +102,8 @@ func TestVerifySharedKey(t *testing.T) { "http://127.0.0.1:10000/devstoreaccount1/mycontainer?restype=container&comp=list", nil, ) - r.Header.Set("x-ms-date", "Tue, 27 Aug 2024 12:00:00 GMT") - r.Header.Set("x-ms-version", "2021-08-06") + r.Header.Set("X-Ms-Date", "Tue, 27 Aug 2024 12:00:00 GMT") + r.Header.Set("X-Ms-Version", "2021-08-06") sig, err := azureauth.SignSharedKey(r, azureauth.DefaultAccountName, azureauth.DefaultAccountKey) require.NoError(t, err) @@ -171,77 +171,124 @@ func TestVerifySharedKey(t *testing.T) { func TestVerifySharedKeyLite(t *testing.T) { t.Parallel() - r := httptest.NewRequest( - http.MethodGet, - "http://127.0.0.1:10002/devstoreaccount1/Tables", - nil, - ) - r.Header.Set("x-ms-date", "Tue, 27 Aug 2024 12:00:00 GMT") - r.Header.Set("x-ms-version", "2021-08-06") - r.Header.Set("Content-Type", "application/json") - - sig, err := azureauth.SignSharedKeyLite(r, azureauth.DefaultAccountName, azureauth.DefaultAccountKey) - require.NoError(t, err) - r.Header.Set("Authorization", "SharedKeyLite "+azureauth.DefaultAccountName+":"+sig) - - valid, err := azureauth.VerifySharedKey(azureauth.DefaultAccountKey, r) - require.NoError(t, err) - assert.True(t, valid) + tests := []struct { + name string + want bool + wantErr bool + }{ + {name: "valid SharedKeyLite round-trip", want: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + r := httptest.NewRequest( + http.MethodGet, + "http://127.0.0.1:10002/devstoreaccount1/Tables", + nil, + ) + r.Header.Set("X-Ms-Date", "Tue, 27 Aug 2024 12:00:00 GMT") + r.Header.Set("X-Ms-Version", "2021-08-06") + r.Header.Set("Content-Type", "application/json") + + sig, err := azureauth.SignSharedKeyLite(r, azureauth.DefaultAccountName, azureauth.DefaultAccountKey) + require.NoError(t, err) + r.Header.Set("Authorization", "SharedKeyLite "+azureauth.DefaultAccountName+":"+sig) + + valid, err := azureauth.VerifySharedKey(azureauth.DefaultAccountKey, r) + if tt.wantErr { + require.Error(t, err) + + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, valid) + }) + } } func TestStringToSign(t *testing.T) { t.Parallel() - r := httptest.NewRequest( - http.MethodPut, - "http://127.0.0.1:10000/devstoreaccount1/c/blob.txt?comp=block&blockid=AAAA", - nil, - ) - r.Header.Set("Content-Type", "text/plain") - r.Header.Set("x-ms-date", "Tue, 27 Aug 2024 12:00:00 GMT") - r.Header.Set("x-ms-version", "2021-08-06") - r.Header.Set("x-ms-blob-type", "BlockBlob") - r.ContentLength = 11 - - want := "PUT\n" + // verb - "\n" + // content-encoding - "\n" + // content-language - "11\n" + // content-length - "\n" + // content-md5 - "text/plain\n" + // content-type - "\n" + // date - "\n" + // if-modified-since - "\n" + // if-match - "\n" + // if-none-match - "\n" + // if-unmodified-since - "\n" + // range - "x-ms-blob-type:BlockBlob\n" + - "x-ms-date:Tue, 27 Aug 2024 12:00:00 GMT\n" + - "x-ms-version:2021-08-06\n" + - "/devstoreaccount1/c/blob.txt\n" + - // (no doubled account segment: the request path already carries - // devstoreaccount1, matching Azurite's path-style addressing) - "blockid:AAAA\n" + - "comp:block" - - assert.Equal(t, want, azureauth.StringToSign(r, azureauth.DefaultAccountName)) + tests := []struct { + name string + want string + }{ + { + name: "PUT block with x-ms headers", + want: "PUT\n" + // verb + "\n" + // content-encoding + "\n" + // content-language + "11\n" + // content-length + "\n" + // content-md5 + "text/plain\n" + // content-type + "\n" + // date + "\n" + // if-modified-since + "\n" + // if-match + "\n" + // if-none-match + "\n" + // if-unmodified-since + "\n" + // range + "x-ms-blob-type:BlockBlob\n" + + "x-ms-date:Tue, 27 Aug 2024 12:00:00 GMT\n" + + "x-ms-version:2021-08-06\n" + + "/devstoreaccount1/c/blob.txt\n" + + // (no doubled account segment: the request path already carries + // devstoreaccount1, matching Azurite's path-style addressing) + "blockid:AAAA\n" + + "comp:block", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + r := httptest.NewRequest( + http.MethodPut, + "http://127.0.0.1:10000/devstoreaccount1/c/blob.txt?comp=block&blockid=AAAA", + nil, + ) + r.Header.Set("Content-Type", "text/plain") + r.Header.Set("X-Ms-Date", "Tue, 27 Aug 2024 12:00:00 GMT") + r.Header.Set("X-Ms-Version", "2021-08-06") + r.Header.Set("X-Ms-Blob-Type", "BlockBlob") + r.ContentLength = 11 + + assert.Equal(t, tt.want, azureauth.StringToSign(r, azureauth.DefaultAccountName), tt.name) + }) + } } func TestStringToSignLite(t *testing.T) { t.Parallel() - r := httptest.NewRequest(http.MethodGet, "http://127.0.0.1:10002/devstoreaccount1/Tables", nil) - r.Header.Set("Content-Type", "application/json") - r.Header.Set("x-ms-date", "Tue, 27 Aug 2024 12:00:00 GMT") + tests := []struct { + name string + want string + }{ + { + name: "GET Tables with x-ms-date", + want: "GET\n" + + "\n" + // content-md5 + "application/json\n" + + "\n" + // date + "x-ms-date:Tue, 27 Aug 2024 12:00:00 GMT\n" + + "/devstoreaccount1/Tables", + }, + } - want := "GET\n" + - "\n" + // content-md5 - "application/json\n" + - "\n" + // date - "x-ms-date:Tue, 27 Aug 2024 12:00:00 GMT\n" + - "/devstoreaccount1/Tables" + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + r := httptest.NewRequest(http.MethodGet, "http://127.0.0.1:10002/devstoreaccount1/Tables", nil) + r.Header.Set("Content-Type", "application/json") + r.Header.Set("X-Ms-Date", "Tue, 27 Aug 2024 12:00:00 GMT") - assert.Equal(t, want, azureauth.StringToSignLite(r, azureauth.DefaultAccountName)) + assert.Equal(t, tt.want, azureauth.StringToSignLite(r, azureauth.DefaultAccountName), tt.name) + }) + } } func TestCanonicalizedResource(t *testing.T) { @@ -299,10 +346,10 @@ func TestCanonicalizedHeaders(t *testing.T) { t.Parallel() r := httptest.NewRequest(http.MethodGet, "http://host/a/b", nil) - r.Header.Set("x-ms-version", "2021-08-06") - r.Header.Set("x-ms-date", "Tue, 27 Aug 2024 12:00:00 GMT") + r.Header.Set("X-Ms-Version", "2021-08-06") + r.Header.Set("X-Ms-Date", "Tue, 27 Aug 2024 12:00:00 GMT") r.Header.Set("Content-Type", "text/plain") // not x-ms-*, must be excluded - r.Header.Set("x-ms-meta-foo", " a b ") // whitespace collapsed/trimmed + r.Header.Set("X-Ms-Meta-Foo", " a b ") // whitespace collapsed/trimmed want := "x-ms-date:Tue, 27 Aug 2024 12:00:00 GMT\n" + "x-ms-meta-foo:a b\n" + @@ -310,3 +357,42 @@ func TestCanonicalizedHeaders(t *testing.T) { assert.Equal(t, want, azureauth.CanonicalizedHeaders(r)) } + +// TestSigning_DoesNotMutateHeaders is a regression test: http.Header.Values +// returns the live slice backing r.Header, so an earlier version of +// CanonicalizedHeaders normalized whitespace in place, silently rewriting +// the caller's request headers as a side effect of computing a signature. +// SignSharedKey/SignSharedKeyLite are documented as not modifying r; this +// pins that contract for a header value that needs whitespace collapsing. +func TestSigning_DoesNotMutateHeaders(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + sign func(r *http.Request, account, key string) (string, error) + }{ + {name: "SharedKey", sign: azureauth.SignSharedKey}, + {name: "SharedKeyLite", sign: azureauth.SignSharedKeyLite}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + r := httptest.NewRequest(http.MethodGet, "http://host/devstoreaccount1/c", nil) + const rawHeaderValue = " a b " + r.Header.Set("X-Ms-Meta-Foo", rawHeaderValue) + + before := r.Header.Values("X-Ms-Meta-Foo") + wantBefore := append([]string(nil), before...) + + _, err := tt.sign(r, azureauth.DefaultAccountName, azureauth.DefaultAccountKey) + require.NoError(t, err) + + assert.Equal(t, wantBefore, r.Header.Values("X-Ms-Meta-Foo"), + "signing must not mutate the request's header values") + assert.Equal(t, rawHeaderValue, r.Header.Get("X-Ms-Meta-Foo"), + "signing must not mutate the request's header values") + }) + } +} diff --git a/pkgs/azureauth/canonical.go b/pkgs/azureauth/canonical.go index 7d06224a77..1550a24815 100644 --- a/pkgs/azureauth/canonical.go +++ b/pkgs/azureauth/canonical.go @@ -113,13 +113,20 @@ func CanonicalizedHeaders(r *http.Request) string { var b strings.Builder for _, name := range names { + // http.Header.Values returns the live slice backing r.Header, not a + // copy -- writing into it in place would silently rewrite the + // caller's request headers as a side effect of computing a + // signature. Copy into a fresh slice before normalizing. vals := r.Header.Values(http.CanonicalHeaderKey(name)) + normalized := make([]string, len(vals)) + for i, v := range vals { - vals[i] = collapseWhitespace(v) + normalized[i] = collapseWhitespace(v) } + b.WriteString(name) b.WriteByte(':') - b.WriteString(strings.Join(vals, ",")) + b.WriteString(strings.Join(normalized, ",")) b.WriteByte('\n') } From f068c819402141cad224c52cecbc6c21005b8fbc Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 08:07:08 -0500 Subject: [PATCH 15/30] azureblob: reject null containers/blobs in Restore A JSON null value inside "containers" or a container's "Blobs" decodes to a nil pointer without an unmarshal error. Nothing previously checked for that before storing it, so the first dereference later (storedContainer.Blobs, or storedBlob.info() for a null blob) would panic. Reject the whole snapshot instead, leaving existing backend state untouched, with a regression test for both cases. --- services/azureblob/persistence.go | 23 ++++++++++++-- services/azureblob/persistence_test.go | 43 ++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/services/azureblob/persistence.go b/services/azureblob/persistence.go index e3db21a9b2..a844a6c92d 100644 --- a/services/azureblob/persistence.go +++ b/services/azureblob/persistence.go @@ -26,7 +26,7 @@ type backendSnapshot struct { // Snapshot serialises the backend state to JSON. It implements // persistence.Persistable. func (b *InMemoryBackend) Snapshot(ctx context.Context) []byte { - b.mu.RLock() + b.mu.RLock("Snapshot") defer b.mu.RUnlock() snap := backendSnapshot{ @@ -46,7 +46,7 @@ func (b *InMemoryBackend) Restore(ctx context.Context, data []byte) error { return err } - b.mu.Lock() + b.mu.Lock("Restore") defer b.mu.Unlock() if snap.Version != azureBlobSnapshotVersion { @@ -68,9 +68,26 @@ func (b *InMemoryBackend) Restore(ctx context.Context, data []byte) error { snap.Containers = make(map[string]*storedContainer) } - for _, c := range snap.Containers { + for name, c := range snap.Containers { + // A JSON `null` value at "containers"[name] decodes to a nil + // *storedContainer without error; leaving it in place would panic + // the first time anything dereferences it (e.g. c.Blobs below, or + // storedBlob.info() for a null blob entry). Reject the whole + // snapshot rather than silently dropping or fabricating an entry. + if c == nil { + return fmt.Errorf("azureblob: restore snapshot: container %q is null", name) + } + if c.Blobs == nil { c.Blobs = make(map[string]*storedBlob) + + continue + } + + for blobName, blob := range c.Blobs { + if blob == nil { + return fmt.Errorf("azureblob: restore snapshot: blob %q in container %q is null", blobName, name) + } } } diff --git a/services/azureblob/persistence_test.go b/services/azureblob/persistence_test.go index 58ca42db31..90c7bc96ad 100644 --- a/services/azureblob/persistence_test.go +++ b/services/azureblob/persistence_test.go @@ -72,6 +72,49 @@ func TestRestore_IncompatibleVersionStartsEmpty(t *testing.T) { } } +// TestRestore_RejectsNullEntries is a regression test: a JSON `null` value +// inside "containers" or a container's "Blobs" decodes to a nil pointer +// without a JSON-unmarshal error, and previously nothing checked for that +// before storing it -- the first thing to dereference it later +// (storedContainer.Blobs, or storedBlob.info() for a null blob) would panic. +// Restore must reject the whole snapshot instead. +func TestRestore_RejectsNullEntries(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + data []byte + }{ + { + name: "null_container", + data: []byte(`{"version":1,"containers":{"c1":null}}`), + }, + { + name: "null_blob", + data: []byte(`{"version":1,"containers":{"c1":{"Name":"c1","Blobs":{"b1":null}}}}`), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + b := azureblob.NewInMemoryBackend() + require.NoError(t, b.CreateContainer("preexisting")) + + err := b.Restore(ctx, tt.data) + require.Error(t, err, tt.name) + + // A rejected snapshot must not have partially mutated state. + containers := b.ListContainers() + require.Len(t, containers, 1, tt.name) + assert.Equal(t, "preexisting", containers[0].Name, tt.name) + }) + } +} + func TestHandlerSnapshotRestore_Delegates(t *testing.T) { t.Parallel() From fee5c3d4232b2d4d866a2c10fc808c2d0ba27f3a Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 08:07:08 -0500 Subject: [PATCH 16/30] azureblob: fix ETag reuse on identical-content overwrite, use lockmetrics computeETag hashed only the blob body, so overwriting a blob with byte-identical content produced the same ETag -- breaking real Azure Blob semantics (ETag changes on every mutation) and making If-Match/If-None-Match concurrency checks meaningless once implemented. Mix in a per-backend monotonic counter (etagSeq) instead, and switch from MD5 to SHA-256 (no weak-hash-guard exemption needed; MD5 is reserved for an actual Content-MD5 header if one is added later). Also replace the raw sync.RWMutex with *lockmetrics.RWMutex, matching repo convention, and add a regression test for the identical-content case plus a table-driven rewrite of the existing overwrite test. --- services/azureblob/store.go | 81 ++++++++++++++++++++++++-------- services/azureblob/store_test.go | 43 ++++++++++++++--- 2 files changed, 99 insertions(+), 25 deletions(-) diff --git a/services/azureblob/store.go b/services/azureblob/store.go index b0b017889b..f75251578c 100644 --- a/services/azureblob/store.go +++ b/services/azureblob/store.go @@ -1,11 +1,13 @@ package azureblob import ( - "crypto/md5" //nolint:gosec // ETag generation only, not a security use of MD5 + "crypto/sha256" + "encoding/binary" "encoding/hex" "sort" - "sync" "time" + + "github.com/blackbirdworks/gopherstack/pkgs/lockmetrics" ) // InMemoryBackend implements StorageBackend using in-memory maps guarded by a @@ -14,13 +16,20 @@ import ( // metrics emitter, and no cross-resource relationships to track, so a single // coarse lock over one map of containers is sufficient. type InMemoryBackend struct { - mu sync.RWMutex + mu *lockmetrics.RWMutex containers map[string]*storedContainer + // etagSeq is a monotonically increasing counter mixed into every ETag + // (see computeETag) so that overwriting a blob with byte-identical + // content still produces a new ETag, matching real Azure Blob semantics + // (an ETag changes on every mutation, not just on content changes) and + // keeping If-Match/If-None-Match concurrency checks meaningful. + etagSeq uint64 } // NewInMemoryBackend creates a new empty InMemoryBackend. func NewInMemoryBackend() *InMemoryBackend { return &InMemoryBackend{ + mu: lockmetrics.New("azureblob"), containers: make(map[string]*storedContainer), } } @@ -28,7 +37,7 @@ func NewInMemoryBackend() *InMemoryBackend { // CreateContainer creates a new, empty container. Returns // ErrContainerAlreadyExists if a container with the same name already exists. func (b *InMemoryBackend) CreateContainer(name string) error { - b.mu.Lock() + b.mu.Lock("CreateContainer") defer b.mu.Unlock() if _, ok := b.containers[name]; ok { @@ -47,7 +56,7 @@ func (b *InMemoryBackend) CreateContainer(name string) error { // DeleteContainer removes a container and all of its blobs. Returns // ErrContainerNotFound if the container does not exist. func (b *InMemoryBackend) DeleteContainer(name string) error { - b.mu.Lock() + b.mu.Lock("DeleteContainer") defer b.mu.Unlock() if _, ok := b.containers[name]; !ok { @@ -62,7 +71,7 @@ func (b *InMemoryBackend) DeleteContainer(name string) error { // ListContainers returns a snapshot of all containers, sorted by name (the // order Azure's List Containers returns them in). func (b *InMemoryBackend) ListContainers() []ContainerInfo { - b.mu.RLock() + b.mu.RLock("ListContainers") defer b.mu.RUnlock() out := make([]ContainerInfo, 0, len(b.containers)) @@ -80,7 +89,7 @@ func (b *InMemoryBackend) ListContainers() []ContainerInfo { // existing blob with the same name (Azure's Put Blob semantics -- no // conditional headers are enforced, see PARITY.md known gaps). func (b *InMemoryBackend) PutBlob(container, blob string, data []byte, contentType string) (BlobInfo, error) { - b.mu.Lock() + b.mu.Lock("PutBlob") defer b.mu.Unlock() c, ok := b.containers[container] @@ -88,12 +97,14 @@ func (b *InMemoryBackend) PutBlob(container, blob string, data []byte, contentTy return BlobInfo{}, ErrContainerNotFound } + b.etagSeq++ + stored := &storedBlob{ Name: blob, ContentType: contentType, Data: append([]byte(nil), data...), LastModified: time.Now().UTC(), - ETag: computeETag(data), + ETag: computeBlobETag(data, b.etagSeq), } c.Blobs[blob] = stored @@ -103,7 +114,7 @@ func (b *InMemoryBackend) PutBlob(container, blob string, data []byte, contentTy // GetBlob returns a blob's metadata and full body. Returns ErrContainerNotFound // or ErrBlobNotFound as appropriate. func (b *InMemoryBackend) GetBlob(container, blob string) (BlobInfo, []byte, error) { - b.mu.RLock() + b.mu.RLock("GetBlob") defer b.mu.RUnlock() stored, err := b.lookupBlobLocked(container, blob) @@ -117,7 +128,7 @@ func (b *InMemoryBackend) GetBlob(container, blob string) (BlobInfo, []byte, err // HeadBlob returns a blob's metadata without its body. Returns // ErrContainerNotFound or ErrBlobNotFound as appropriate. func (b *InMemoryBackend) HeadBlob(container, blob string) (BlobInfo, error) { - b.mu.RLock() + b.mu.RLock("HeadBlob") defer b.mu.RUnlock() stored, err := b.lookupBlobLocked(container, blob) @@ -131,7 +142,7 @@ func (b *InMemoryBackend) HeadBlob(container, blob string) (BlobInfo, error) { // DeleteBlob removes a blob. Returns ErrContainerNotFound or ErrBlobNotFound // as appropriate. func (b *InMemoryBackend) DeleteBlob(container, blob string) error { - b.mu.Lock() + b.mu.Lock("DeleteBlob") defer b.mu.Unlock() c, ok := b.containers[container] @@ -151,7 +162,7 @@ func (b *InMemoryBackend) DeleteBlob(container, blob string) error { // ListBlobs returns a snapshot of all blobs in container, sorted by name. // Returns ErrContainerNotFound if the container does not exist. func (b *InMemoryBackend) ListBlobs(container string) ([]BlobInfo, error) { - b.mu.RLock() + b.mu.RLock("ListBlobs") defer b.mu.RUnlock() c, ok := b.containers[container] @@ -172,10 +183,11 @@ func (b *InMemoryBackend) ListBlobs(container string) ([]BlobInfo, error) { // Reset clears all in-memory state. It is used by the // POST /_gopherstack/reset endpoint for CI pipelines and rapid local development. func (b *InMemoryBackend) Reset() { - b.mu.Lock() + b.mu.Lock("Reset") defer b.mu.Unlock() b.containers = make(map[string]*storedContainer) + b.etagSeq = 0 } // lookupBlobLocked resolves a (container, blob) pair. Callers must hold @@ -194,11 +206,42 @@ func (b *InMemoryBackend) lookupBlobLocked(container, blob string) (*storedBlob, return stored, nil } -// computeETag derives a quoted ETag from the blob body, matching the shape -// (a quoted opaque token) real Azure Storage ETags take, without attempting -// to replicate Azure's actual internal ETag algorithm. -func computeETag(data []byte) string { - sum := md5.Sum(data) //nolint:gosec // content fingerprint only, not a security use of MD5 +// computeBlobETag derives a quoted, opaque ETag from a blob's body and seq, a +// per-backend monotonically increasing counter incremented on every mutation +// (see InMemoryBackend.etagSeq). Mixing in seq -- rather than hashing the +// body alone -- ensures overwriting a blob with byte-identical content still +// produces a new ETag, matching real Azure Blob semantics and keeping +// If-Match/If-None-Match concurrency checks meaningful. This only needs to +// match the shape (a quoted opaque token) real Azure Storage ETags take, not +// Azure's actual internal ETag algorithm, so SHA-256 (not a +// weak-hash-guarded algorithm) is used instead of MD5; MD5 is reserved for +// an actual Content-MD5 header if one is added later. +func computeBlobETag(data []byte, seq uint64) string { + var seqBuf [8]byte + binary.BigEndian.PutUint64(seqBuf[:], seq) + + return quotedHash(seqBuf[:], data) +} + +// computeContainerETag derives a quoted, opaque ETag for a container's List +// Containers listing entry from its name and creation time. Containers in +// this MVP have no mutable properties and no If-Match semantics (see +// PARITY.md known gaps), so unlike computeBlobETag this does not need a +// per-mutation sequence number -- it only needs to be a stable, plausible +// ETag shape for azure-sdk-for-go to parse. +func computeContainerETag(name string, createdAt time.Time) string { + return quotedHash([]byte(name), []byte(createdAt.String())) +} + +// quotedHash returns a quoted, opaque SHA-256-derived token over the +// concatenation of parts. Shared by computeBlobETag (store.go) and +// computeContainerETag (handler.go's List Containers response) -- neither +// needs to replicate Azure's actual internal ETag algorithm, only its shape. +func quotedHash(parts ...[]byte) string { + h := sha256.New() + for _, p := range parts { + h.Write(p) + } - return `"` + hex.EncodeToString(sum[:]) + `"` + return `"` + hex.EncodeToString(h.Sum(nil)[:16]) + `"` } diff --git a/services/azureblob/store_test.go b/services/azureblob/store_test.go index 1eb973cb77..fa906967c6 100644 --- a/services/azureblob/store_test.go +++ b/services/azureblob/store_test.go @@ -212,9 +212,31 @@ func TestInMemoryBackend_PutBlobOverwrites(t *testing.T) { t.Parallel() tests := []struct { - name string + name string + firstBody string + secondBody string + wantData string + wantSameETag bool }{ - {name: "overwrite_replaces_data"}, + { + name: "overwrite_replaces_data", + firstBody: "first", + secondBody: "second-longer", + wantData: "second-longer", + wantSameETag: false, + }, + { + // Regression test: real Azure Blob ETags change on every + // mutation, not just on content changes -- an ETag derived + // purely from the body would produce the same ETag here, + // silently breaking If-Match/If-None-Match concurrency + // semantics (see store.go's computeBlobETag/etagSeq). + name: "identical_content_still_changes_etag", + firstBody: "same-bytes", + secondBody: "same-bytes", + wantData: "same-bytes", + wantSameETag: false, + }, } for _, tt := range tests { @@ -224,16 +246,25 @@ func TestInMemoryBackend_PutBlobOverwrites(t *testing.T) { b := azureblob.NewInMemoryBackend() require.NoError(t, b.CreateContainer("c1")) - _, err := b.PutBlob("c1", "blob1", []byte("first"), "") + firstInfo, err := b.PutBlob("c1", "blob1", []byte(tt.firstBody), "") require.NoError(t, err) - info, err := b.PutBlob("c1", "blob1", []byte("second-longer"), "") + secondInfo, err := b.PutBlob("c1", "blob1", []byte(tt.secondBody), "") require.NoError(t, err) _, data, err := b.GetBlob("c1", "blob1") require.NoError(t, err) - assert.Equal(t, "second-longer", string(data), tt.name) - assert.Equal(t, int64(len("second-longer")), info.ContentLength, tt.name) + assert.Equal(t, tt.wantData, string(data), tt.name) + assert.Equal(t, int64(len(tt.wantData)), secondInfo.ContentLength, tt.name) + + assert.NotEmpty(t, firstInfo.ETag, tt.name) + assert.NotEmpty(t, secondInfo.ETag, tt.name) + + if tt.wantSameETag { + assert.Equal(t, firstInfo.ETag, secondInfo.ETag, tt.name) + } else { + assert.NotEqual(t, firstInfo.ETag, secondInfo.ETag, tt.name) + } }) } } From 4b64a97f7bcdbe311890d9a7ba4e77a9a2d00993 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 08:07:08 -0500 Subject: [PATCH 17/30] azureblob: synchronous port bind, fixed-port design, observability wiring - Port-check race: StartWorker now binds net.Listen itself and serves on that exact listener, instead of a probe-then-close check followed by an async ListenAndServe -- a bind failure is returned to the caller synchronously instead of only being logged later. Simplified away the PortAlloc-fallback design in the same pass: it duplicated services/iot's MQTT broker precedent (fixed, protocol-conventional port, fail-fast if unavailable) with unnecessary complexity, and a silent fallback into the shared --port-range-start/--port-range-end pool would be exactly as surprising as picking a different default port to dodge the range overlap. - http.Server gets ReadTimeout/IdleTimeout (previously only ReadHeaderTimeout, leaving slow-body uploads unbounded). - Shutdown logs a graceful-shutdown failure and falls back to Close(), logging any Close error too, instead of discarding both. - StartWorker wraps its Echo handler with telemetry.WrapEchoHandler and logger.EchoMiddleware, and derives its listener logger via logger.WithWorker, so ExtractOperation/ExtractResource actually feed Prometheus metrics like every other service. - srvMu switches from sync.Mutex to *lockmetrics.RWMutex. - Settings.Port is now a proper Kong-embeddable CLI field (--azure-blob-port/AZURE_BLOB_PORT) instead of an ad-hoc os.Getenv read, wired into cli.go below. - New coverage_test.go: RouteMatcher/MatchPriority/ExtractOperation/ ExtractResource, checkAuth's structurally-valid-header branch, CreateContainer's already-exists branch, and StartWorker/Shutdown's real bind/serve/failure/force-close paths (one does a real HTTP round-trip through the fully wired listener). Package coverage 69.4% -> 94.3%. --- services/azureblob/coverage_test.go | 280 ++++++++++++++++++++++++++++ services/azureblob/handler.go | 124 +++++++++--- services/azureblob/provider.go | 79 +++----- services/azureblob/settings.go | 51 +++-- 4 files changed, 419 insertions(+), 115 deletions(-) create mode 100644 services/azureblob/coverage_test.go diff --git a/services/azureblob/coverage_test.go b/services/azureblob/coverage_test.go new file mode 100644 index 0000000000..196aa882d2 --- /dev/null +++ b/services/azureblob/coverage_test.go @@ -0,0 +1,280 @@ +package azureblob_test + +import ( + "context" + "fmt" + "net" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/labstack/echo/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/pkgs/azureauth" + "github.com/blackbirdworks/gopherstack/services/azureblob" +) + +// freeEphemeralPort returns a port that was free at the moment of the call +// (bound briefly via net.Listen("tcp", ":0") then released immediately). +// Used only to obtain a concrete port number for StartWorker to bind in +// tests; the resulting single-process TOCTOU race is an accepted tradeoff +// for test setup, not something StartWorker itself does (see its doc +// comment -- StartWorker's own net.Listen is the real, synchronous bind). +func freeEphemeralPort(t *testing.T) int { + t.Helper() + + l, err := net.Listen("tcp", ":0") + require.NoError(t, err) + + addr, ok := l.Addr().(*net.TCPAddr) + require.True(t, ok) + require.NoError(t, l.Close()) + + return addr.Port +} + +// reserveEphemeralPort binds and holds a real TCP port until the test ends, +// for tests that need a guaranteed-busy port to exercise a bind-failure path. +func reserveEphemeralPort(t *testing.T) int { + t.Helper() + + l, err := net.Listen("tcp", ":0") + require.NoError(t, err) + t.Cleanup(func() { _ = l.Close() }) + + addr, ok := l.Addr().(*net.TCPAddr) + require.True(t, ok) + + return addr.Port +} + +// TestRouteMatcher_AlwaysFalse pins RouteMatcher's documented contract: +// AzureBlob never participates in the shared AWS single-port Router (see +// provider.go's Provider doc comment) -- it exists only so *Handler +// satisfies service.Registerable. +func TestRouteMatcher_AlwaysFalse(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + matcher := h.RouteMatcher() + + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/"+testAccount+"/c", http.NoBody) + c := e.NewContext(req, httptest.NewRecorder()) + + assert.False(t, matcher(c)) +} + +func TestMatchPriority_Lowest(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + + assert.Equal(t, 0, h.MatchPriority()) +} + +func TestExtractOperation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + method string + path string + want string + }{ + {name: "list_containers", method: http.MethodGet, path: "/" + testAccount + "?comp=list", want: "ListContainers"}, + { + name: "create_container", method: http.MethodPut, + path: "/" + testAccount + "/c?restype=container", want: "CreateContainer", + }, + { + name: "delete_container", method: http.MethodDelete, + path: "/" + testAccount + "/c?restype=container", want: "DeleteContainer", + }, + { + name: "list_blobs", method: http.MethodGet, + path: "/" + testAccount + "/c?restype=container&comp=list", want: "ListBlobs", + }, + {name: "put_blob", method: http.MethodPut, path: "/" + testAccount + "/c/b", want: "PutBlob"}, + {name: "get_blob", method: http.MethodGet, path: "/" + testAccount + "/c/b", want: "GetBlob"}, + {name: "get_blob_properties", method: http.MethodHead, path: "/" + testAccount + "/c/b", want: "GetBlobProperties"}, + {name: "delete_blob", method: http.MethodDelete, path: "/" + testAccount + "/c/b", want: "DeleteBlob"}, + {name: "unknown", method: http.MethodOptions, path: "/" + testAccount + "?comp=list", want: "Unknown"}, + } + + h := newTestHandler(t) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + e := echo.New() + req := httptest.NewRequest(tt.method, tt.path, http.NoBody) + c := e.NewContext(req, httptest.NewRecorder()) + + assert.Equal(t, tt.want, h.ExtractOperation(c), tt.name) + }) + } +} + +func TestExtractResource(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path string + want string + }{ + {name: "container_only", path: "/" + testAccount + "/mycontainer", want: "mycontainer"}, + {name: "container_and_blob", path: "/" + testAccount + "/mycontainer/myblob", want: "mycontainer/myblob"}, + { + name: "blob_name_with_slashes", path: "/" + testAccount + "/mycontainer/logs/2026/09.txt", + want: "mycontainer/logs/2026/09.txt", + }, + } + + h := newTestHandler(t) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + e := echo.New() + req := httptest.NewRequest(http.MethodGet, tt.path, http.NoBody) + c := e.NewContext(req, httptest.NewRecorder()) + + assert.Equal(t, tt.want, h.ExtractResource(c), tt.name) + }) + } +} + +// TestCheckAuth_StructurallyValidHeaderAccepted covers checkAuth's third +// branch (a well-formed "SharedKey account:sig" header): parsing succeeds, +// and -- matching this milestone's permissive-by-default auth stance -- the +// request still proceeds normally rather than being rejected. +func TestCheckAuth_StructurallyValidHeaderAccepted(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + + rec := doRequest(t, h, http.MethodPut, "/"+testAccount+"/mycontainer?restype=container", nil, map[string]string{ + "Authorization": "SharedKey " + azureauth.DefaultAccountName + ":c2lnbmF0dXJl", + }) + + require.Equal(t, http.StatusCreated, rec.Code) +} + +// TestCreateContainer_AlreadyExists covers createContainer's +// ErrContainerAlreadyExists branch (409 Conflict). +func TestCreateContainer_AlreadyExists(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + createContainer(t, h, "dupe") + + rec := doRequest(t, h, http.MethodPut, "/"+testAccount+"/dupe?restype=container", nil, nil) + + assert.Equal(t, http.StatusConflict, rec.Code) + assert.Contains(t, rec.Body.String(), "ContainerAlreadyExists") +} + +// TestStartWorker_BindsAndServes exercises the real synchronous bind added +// to fix the port-check race (net.Listen happens in StartWorker itself, not +// a separate probe-then-close step): it starts the dedicated listener on a +// concrete port, makes a real HTTP request against it to prove the +// telemetry-wrapped handler (logger.EchoMiddleware + telemetry.WrapEchoHandler, +// see StartWorker) is actually reachable, then shuts it down. +func TestStartWorker_BindsAndServes(t *testing.T) { + t.Parallel() + + port := freeEphemeralPort(t) + + backend := azureblob.NewInMemoryBackend() + h := azureblob.NewHandler(backend) + h.Port = port + + ctx := t.Context() + require.NoError(t, h.StartWorker(ctx)) + + t.Cleanup(func() { + shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + h.Shutdown(shutdownCtx) + }) + + url := fmt.Sprintf("http://127.0.0.1:%d/%s?comp=list", port, testAccount) + + require.Eventually(t, func() bool { + req, reqErr := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) + if reqErr != nil { + return false + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return false + } + defer resp.Body.Close() + + return resp.StatusCode == http.StatusOK + }, 2*time.Second, 10*time.Millisecond, "dedicated listener should become reachable") +} + +// TestStartWorker_BindFailureIsSynchronous is a regression test for the +// port-check race: binding an already-listening port must fail +// synchronously from StartWorker itself, not silently report success and +// only log the failure later from the background goroutine. +func TestStartWorker_BindFailureIsSynchronous(t *testing.T) { + t.Parallel() + + // reserveEphemeralPort holds the port open for the rest of the test, so + // h.StartWorker below is guaranteed to find it busy. + port := reserveEphemeralPort(t) + + h := azureblob.NewHandler(azureblob.NewInMemoryBackend()) + h.Port = port + + err := h.StartWorker(t.Context()) + require.Error(t, err) + assert.Contains(t, err.Error(), "bind port") +} + +// TestShutdown_NilServerIsNoop covers Shutdown's early return when +// StartWorker was never called (h.srv is nil). +func TestShutdown_NilServerIsNoop(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + + assert.NotPanics(t, func() { + h.Shutdown(t.Context()) + }) +} + +// TestShutdown_ForcesCloseOnGracefulTimeout covers Shutdown's fallback path: +// an already-expired context makes srv.Shutdown return immediately with an +// error, forcing the srv.Close() fallback (both are logged, neither panics +// nor leaves the listener open). +func TestShutdown_ForcesCloseOnGracefulTimeout(t *testing.T) { + t.Parallel() + + h := azureblob.NewHandler(azureblob.NewInMemoryBackend()) + h.Port = 0 + + require.NoError(t, h.StartWorker(t.Context())) + + expiredCtx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + + assert.NotPanics(t, func() { + h.Shutdown(expiredCtx) + }) + + // A second Shutdown call must also be a safe no-op (h.srv was cleared). + assert.NotPanics(t, func() { + h.Shutdown(t.Context()) + }) +} diff --git a/services/azureblob/handler.go b/services/azureblob/handler.go index 403077064d..e7653c5326 100644 --- a/services/azureblob/handler.go +++ b/services/azureblob/handler.go @@ -6,18 +6,20 @@ import ( "encoding/xml" "errors" "fmt" + "net" "net/http" "strconv" "strings" - "sync" "time" "github.com/labstack/echo/v5" "github.com/blackbirdworks/gopherstack/pkgs/azureauth" "github.com/blackbirdworks/gopherstack/pkgs/httputils" + "github.com/blackbirdworks/gopherstack/pkgs/lockmetrics" "github.com/blackbirdworks/gopherstack/pkgs/logger" "github.com/blackbirdworks/gopherstack/pkgs/service" + "github.com/blackbirdworks/gopherstack/pkgs/telemetry" ) // azureBlobVersion is the x-ms-version value echoed on every response. It is @@ -48,15 +50,27 @@ const ( type Handler struct { Backend StorageBackend Endpoint string // e.g. "http://127.0.0.1:10000" -- used to build ServiceEndpoint in list responses - Port int - srvMu sync.Mutex + // Port is the TCP port StartWorker binds. Set from Settings at Init time + // (see provider.go); defaults to DefaultPort. Unlike a per-resource + // ephemeral allocation, this is a single fixed, protocol-conventional + // port (mirroring services/iot's MQTT broker) -- there is no fallback + // pool, so StartWorker fails fast if it's unavailable rather than + // silently binding a different port. + Port int + + srvMu *lockmetrics.RWMutex srv *http.Server } -// NewHandler creates a new Azure Blob Handler. +// NewHandler creates a new Azure Blob Handler. Port defaults to DefaultPort; +// callers (typically provider.go) override it from Settings. func NewHandler(backend StorageBackend) *Handler { - return &Handler{Backend: backend, Port: DefaultPort} + return &Handler{ + Backend: backend, + Port: DefaultPort, + srvMu: lockmetrics.New("azureblob.server"), + } } var ( @@ -85,11 +99,12 @@ func (h *Handler) GetSupportedOperations() []string { // RouteMatcher exists only to satisfy service.Registerable's interface // contract: AzureBlob deliberately never matches on the shared AWS // single-port Router. It runs on its own dedicated listener started by -// StartWorker (see provider.go for the full rationale). cli.go's service -// registration list is not (yet) wired to this provider at all -- that is a -// deferred integration step for a human to do once pkgs/azureauth also -// lands, so this matcher is effectively dead code today, kept only so -// *Handler satisfies service.Registerable. +// StartWorker (see provider.go for the full rationale). AzureBlob's +// Provider IS registered in cli.go's getMostRecentServiceProviders like +// every other service -- startBackgroundWorkers calls StartWorker via the +// service.BackgroundWorker interface regardless of routing, which is how +// the dedicated listener comes up. Only RouteMatcher itself is inert, kept +// so *Handler satisfies service.Registerable. func (h *Handler) RouteMatcher() service.Matcher { return func(*echo.Context) bool { return false } } @@ -175,8 +190,8 @@ func (h *Handler) checkAuth(r *http.Request) { // response, success or error. func (h *Handler) setCommonHeaders(c *echo.Context) { hdr := c.Response().Header() - hdr.Set("x-ms-version", azureBlobVersion) - hdr.Set("x-ms-request-id", newRequestID()) + hdr.Set("X-Ms-Version", azureBlobVersion) + hdr.Set("X-Ms-Request-Id", newRequestID()) hdr.Set("Date", time.Now().UTC().Format(http.TimeFormat)) } @@ -273,7 +288,7 @@ func (h *Handler) handleAccountLevel(c *echo.Context) error { Name: ci.Name, Properties: containerProperties{ LastModified: ci.CreatedAt.Format(http.TimeFormat), - Etag: computeETag([]byte(ci.Name + ci.CreatedAt.String())), + Etag: computeContainerETag(ci.Name, ci.CreatedAt), }, }) } @@ -372,7 +387,7 @@ func (h *Handler) handleBlobLevel(c *echo.Context, container, blob string) error func (h *Handler) putBlob(c *echo.Context, container, blob string) error { r := c.Request() - if r.Header.Get("x-ms-blob-type") != blockBlobType { + if r.Header.Get("X-Ms-Blob-Type") != blockBlobType { return h.writeError(c, http.StatusBadRequest, "InvalidHeaderValue", "The value for one of the HTTP headers is not in the correct format "+ "(x-ms-blob-type must be BlockBlob; only block blobs are supported).") @@ -464,7 +479,7 @@ func (h *Handler) setBlobHeaders(c *echo.Context, info BlobInfo) { hdr.Set("ETag", info.ETag) hdr.Set("Last-Modified", info.LastModified.Format(http.TimeFormat)) hdr.Set("Content-Length", strconv.FormatInt(info.ContentLength, 10)) - hdr.Set("x-ms-blob-type", blockBlobType) + hdr.Set("X-Ms-Blob-Type", blockBlobType) hdr.Set("Accept-Ranges", "bytes") if info.ContentType != "" { @@ -557,39 +572,80 @@ func (h *Handler) writeError(c *echo.Context, status int, code, message string) return h.writeXML(c, status, azureError{Code: code, Message: message}) } -// StartWorker starts the dedicated Blob listener on h.Port. See provider.go's -// Provider doc comment for why AzureBlob needs its own listener instead of -// registering into the shared AWS Router. +// Timeouts for the dedicated Blob http.Server. ReadHeaderTimeout alone only +// bounds how long a client may take to send headers; without ReadTimeout and +// IdleTimeout a slow-body client (e.g. a stalled PUT blob upload) or a client +// that opens a connection and never closes it can hold a handler goroutine +// and connection open indefinitely (a Slowloris-style resource exhaustion). +// WriteTimeout is deliberately not set: it would bound the full +// request-to-response-complete duration, which for a large GetBlob download +// depends on the client's own read rate, not just server-side work. +const ( + azureBlobReadHeaderTimeout = 10 * time.Second + azureBlobReadTimeout = 60 * time.Second + azureBlobIdleTimeout = 120 * time.Second +) + +// StartWorker binds the dedicated Blob listener and starts serving on it. +// See provider.go's Provider doc comment for why AzureBlob needs its own +// listener instead of registering into the shared AWS Router. +// +// Binding is synchronous: net.Listen returns before StartWorker does, so a +// bind failure is returned to the caller directly instead of only being +// logged from the background goroutine after startup has already reported +// success. This mirrors services/iot's MQTT broker (services/iot/broker.go), +// gopherstack's existing precedent for a service with a fixed, +// protocol-conventional default port: bind exactly the configured port +// (h.Port, from Settings -- see settings.go/provider.go) and fail fast if +// that's unavailable, rather than silently falling back into the shared +// --port-range-start/--port-range-end PortAlloc pool used for on-demand +// ephemeral resources elsewhere (Lambda function URLs, ElastiCache). A +// fallback there would be just as surprising as picking a different default +// port number outright: either way, an SDK relying on the well-known default +// (UseDevelopmentStorage=true-style config) would silently end up talking to +// the wrong port. Failing fast surfaces the conflict instead. func (h *Handler) StartWorker(ctx context.Context) error { + listener, err := net.Listen("tcp", fmt.Sprintf(":%d", h.Port)) + if err != nil { + return fmt.Errorf("azureblob: bind port %d: %w", h.Port, err) + } + e := echo.New() - e.Any("/*", h.Handler()) + e.Use(logger.EchoMiddleware(logger.Load(ctx))) + e.Any("/*", telemetry.WrapEchoHandler("AzureBlob", h.Handler(), h)) srv := &http.Server{ - Addr: fmt.Sprintf(":%d", h.Port), Handler: e, - ReadHeaderTimeout: 10 * time.Second, //nolint:mnd // matches cli.go's defaultReadHeaderTimeout intent + ReadHeaderTimeout: azureBlobReadHeaderTimeout, + ReadTimeout: azureBlobReadTimeout, + IdleTimeout: azureBlobIdleTimeout, } - h.srvMu.Lock() + h.srvMu.Lock("StartWorker") h.srv = srv h.srvMu.Unlock() - log := logger.Load(ctx) + workerCtx := logger.WithWorker(ctx, "azureblob", "listener") + log := logger.Load(workerCtx) - go func() { - log.InfoContext(ctx, "azureblob: starting dedicated listener", "port", h.Port) + log.InfoContext(workerCtx, "azureblob: starting dedicated listener", "port", h.Port) - if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { - log.ErrorContext(ctx, "azureblob: listener stopped", "error", err) + go func() { + if serveErr := srv.Serve(listener); serveErr != nil && !errors.Is(serveErr, http.ErrServerClosed) { + log.ErrorContext(workerCtx, "azureblob: listener stopped", "error", serveErr) } }() return nil } -// Shutdown stops the dedicated Blob listener. +// Shutdown stops the dedicated Blob listener. A graceful Shutdown error +// (e.g. its context expiring before active connections finish) is logged +// and followed by Close, which forcibly closes the listener and any +// remaining idle/active connections; any Close error is logged too rather +// than leaving the listener to leak silently. func (h *Handler) Shutdown(ctx context.Context) { - h.srvMu.Lock() + h.srvMu.Lock("Shutdown") srv := h.srv h.srv = nil h.srvMu.Unlock() @@ -598,5 +654,13 @@ func (h *Handler) Shutdown(ctx context.Context) { return } - _ = srv.Shutdown(ctx) + log := logger.Load(ctx) + + if err := srv.Shutdown(ctx); err != nil { + log.ErrorContext(ctx, "azureblob: graceful shutdown failed, forcing close", "error", err) + + if closeErr := srv.Close(); closeErr != nil { + log.ErrorContext(ctx, "azureblob: forced close also failed", "error", closeErr) + } + } } diff --git a/services/azureblob/provider.go b/services/azureblob/provider.go index b9ca8e2e94..173b030fb0 100644 --- a/services/azureblob/provider.go +++ b/services/azureblob/provider.go @@ -2,16 +2,19 @@ package azureblob import ( "errors" - "net" - "strconv" - "github.com/blackbirdworks/gopherstack/pkgs/portalloc" "github.com/blackbirdworks/gopherstack/pkgs/service" ) // ErrNilAppContext is returned when Init is called with a nil AppContext. var ErrNilAppContext = errors.New("azureblob: nil app context") +// ConfigProvider is a private interface to extract AzureBlob configuration +// from the abstract AppContext Config, mirroring services/s3.ConfigProvider. +type ConfigProvider interface { + GetAzureBlobSettings() Settings +} + // Provider implements service.Provider for the Azure Blob Storage service. // // Unlike every other provider in this repo, AzureBlob does not register a @@ -21,15 +24,22 @@ var ErrNilAppContext = errors.New("azureblob: nil app context") // risks exactly the collision the router avoids by construction for AWS // services (see AZURE.md section 4). Instead the returned Handler implements // service.BackgroundWorker and stands up its own dedicated *echo.Echo/ -// *http.Server, listening on its own port -- mirroring Azurite's own -// separate-port-per-service convention (10000 for Blob). +// *http.Server, listening on a fixed, protocol-conventional port -- the same +// pattern services/iot's MQTT broker already uses in this repo for a +// well-known port (1883) that isn't part of the shared AWS request/response +// cycle. It is registered in cli.go's getMostRecentServiceProviders like +// every other provider; only its RouteMatcher (which always returns false) +// is inert. type Provider struct{} // Name returns the service provider name. func (p *Provider) Name() string { return "AzureBlob" } -// Init initializes the AzureBlob service backend and handler, resolving the -// dedicated port the handler's StartWorker will later listen on. +// Init initializes the AzureBlob service backend and handler. The configured +// port (Settings.Port, default DefaultPort) is only recorded here; the +// actual TCP bind happens synchronously in Handler.StartWorker, so a +// port-in-use failure is returned to the caller directly instead of being +// discovered later from a background goroutine. // //nolint:ireturn,nolintlint // architecturally required to return interface func (p *Provider) Init(ctx *service.AppContext) (service.Registerable, error) { @@ -38,58 +48,13 @@ func (p *Provider) Init(ctx *service.AppContext) (service.Registerable, error) { } settings := DefaultSettings() + if cp, ok := ctx.Config.(ConfigProvider); ok { + settings = cp.GetAzureBlobSettings() + } + backend := NewInMemoryBackend() handler := NewHandler(backend) - handler.Port = resolvePort(settings.Port, ctx.PortAlloc) + handler.Port = settings.Port return handler, nil } - -// resolvePort implements azureblob's port-selection strategy. -// -// gopherstack's pkgs/portalloc.Allocator only supports acquiring the next -// free port from a sequential range (Allocator.Acquire) -- it has no concept -// of reserving a *specific* preferred port. Every existing PortAlloc caller -// in this repo (Lambda function URLs, ElastiCache) wants an arbitrary -// ephemeral port and is fine with whatever it gets. Azure Blob is different: -// real SDKs default their connection strings/emulator constants to a *fixed* -// port (Azurite's 10000, see AZURE.md section 2), so gopherstack must -// actually try to bind that fixed port to be a useful drop-in target, -// falling back to the shared pool only when it can't. -// -// This is the "one real architectural decision" AZURE.md section 4 flags as -// a gap in the current single-port-router design: there is no existing -// precedent in the repo for a "give me this exact port or tell me it's -// busy" primitive, so this function bridges the gap locally instead of -// extending portalloc's contract (which would ripple to every other caller) -// for one service's needs. -// -// The availability probe is inherently racy (the port could be taken between -// this check and StartWorker's real bind) -- acceptable for a local dev/test -// emulator, not something a production load balancer would do. -func resolvePort(preferred int, alloc *portalloc.Allocator) int { - if portAvailable(preferred) { - return preferred - } - - if alloc != nil { - if p, err := alloc.Acquire("azureblob"); err == nil { - return p - } - } - - return preferred -} - -// portAvailable reports whether port can currently be bound on all -// interfaces. -func portAvailable(port int) bool { - l, err := net.Listen("tcp", ":"+strconv.Itoa(port)) - if err != nil { - return false - } - - _ = l.Close() - - return true -} diff --git a/services/azureblob/settings.go b/services/azureblob/settings.go index c2827fe1e8..8d943b60c2 100644 --- a/services/azureblob/settings.go +++ b/services/azureblob/settings.go @@ -1,38 +1,33 @@ package azureblob -import ( - "os" - "strconv" -) - -// DefaultPort mirrors Azurite's default Blob service port, so unmodified -// Azurite-targeting SDK configuration (UseDevelopmentStorage=true, default -// connection strings) works out of the box. See AZURE.md section 4/5. +// DefaultPort is Azure Blob's fixed, protocol-conventional TCP port. This +// follows the same pattern as services/iot's MQTT broker (also a fixed, +// protocol-conventional default -- 1883 -- with a CLI/env override and no +// shared-pool fallback): pick one default and try to bind exactly that, +// rather than drawing from cli.go's shared --port-range-start/ +// --port-range-end PortAlloc pool (used for on-demand ephemeral resources +// like Lambda function URLs and ElastiCache, not fixed service ports) or +// inventing an alternative numbering scheme. The default value itself +// (10000) is Azurite's own Blob service port, so unmodified +// UseDevelopmentStorage=true-style SDK configuration works out of the box; +// see AZURE.md section 4 for the full rationale, including why this +// deliberately does NOT fall back into the shared PortAlloc pool if 10000 is +// taken (StartWorker fails fast instead -- see handler.go). const DefaultPort = 10000 -// envPortOverride lets a deployment move the dedicated Blob listener off -// DefaultPort (e.g. because 10000 is already in use for something else on -// the host). -const envPortOverride = "AZURE_BLOB_PORT" - // Settings holds service-level configuration for the Azure Blob backend. +// Fields are picked up by the Kong CLI parser when this struct is embedded +// in the root CLI command (see cli.go's CLI.AzureBlob field), mirroring +// services/s3's Settings pattern. type Settings struct { - // Port is the preferred TCP port for the dedicated Blob listener. - // See provider.go's resolvePort for what happens when it's unavailable. - Port int + // Port is the fixed TCP port for the dedicated Blob listener. See + // handler.go's StartWorker for what happens when it's unavailable + // (fails fast; no fallback pool, matching services/iot's MQTT broker). + Port int `json:"port" env:"AZURE_BLOB_PORT" default:"10000" name:"port" help:"Fixed TCP port for the dedicated Azure Blob listener; startup fails if it's unavailable (no fallback pool)."` //nolint:lll // config struct tags are intentionally verbose } -// DefaultSettings returns the default Settings, honoring envPortOverride. +// DefaultSettings returns the default Settings. Used when no ConfigProvider +// is available at init time (e.g. tests constructing a Provider directly). func DefaultSettings() Settings { - return Settings{Port: portFromEnv()} -} - -func portFromEnv() int { - if v := os.Getenv(envPortOverride); v != "" { - if p, err := strconv.Atoi(v); err == nil && p > 0 && p < 65536 { - return p - } - } - - return DefaultPort + return Settings{Port: DefaultPort} } From 0f08c3126240fb80b0527f77193b37db47de5b30 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 08:07:09 -0500 Subject: [PATCH 18/30] cli: wire AzureBlob port through the CLI settings struct Adds CLI.AzureBlob (azureblob.Settings, embed prefix azure-blob-) and GetAzureBlobSettings(), mirroring the S3/GetS3Settings pattern, so the dedicated listener's port is a documented --azure-blob-port/ AZURE_BLOB_PORT flag instead of a package-private env var. --- cli.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cli.go b/cli.go index 3cd4368c27..fd4f765420 100644 --- a/cli.go +++ b/cli.go @@ -442,6 +442,7 @@ type CLI struct { InitScripts []string ` name:"init-script" env:"INIT_SCRIPTS" help:"Shell scripts to run on startup (may be specified multiple times)."` //nolint:lll // config struct tags are intentionally verbose S3InitBuckets []string ` name:"s3-bucket" env:"S3_BUCKETS" help:"S3 bucket names to create on startup (may be specified multiple times or as a comma-separated list)."` //nolint:lll // config struct tags are intentionally verbose S3 s3backend.Settings `embed:"" prefix:"s3-"` + AzureBlob azureblobbackend.Settings `embed:"" prefix:"azure-blob-"` Lambda lambdabackend.Settings `embed:"" prefix:"lambda-"` DynamoDB ddbbackend.Settings `embed:"" prefix:"dynamodb-"` EC2 ec2backend.Settings `embed:"" prefix:"ec2-"` @@ -522,6 +523,11 @@ func (c *CLI) GetS3Settings() s3backend.Settings { return c.S3 } +// GetAzureBlobSettings returns Azure Blob settings (azureblob.ConfigProvider). +func (c *CLI) GetAzureBlobSettings() azureblobbackend.Settings { + return c.AzureBlob +} + // GetS3Endpoint returns the configured S3 endpoint (s3.ConfigProvider). func (c *CLI) GetS3Endpoint() string { s3Port := strings.TrimPrefix(c.Port, ":") From c627524c0dc030a090ee181036b56be3a257fea2 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 08:07:09 -0500 Subject: [PATCH 19/30] checkpins: support azure-sdk-for-go's nested module paths sdk_module pins were only ever aws-sdk-go-v2/service/@version; azure-sdk-for-go publishes its clients as nested submodules (sdk//) rather than a flat sdk/service/, so azureblob's PARITY.md pin failed to validate against go.mod with no way to fix it short of a fake AWS-shaped path. Add a parallel azure-sdk-for-go/sdk/@version pin format and index those modules from go.mod the same way, with test coverage for both the parse and go.mod-lookup paths. Also bump PARITY.md's pin to the azblob v1.8.0 actually in go.mod (was stale at v1.7.0). --- cmd/checkpins/main.go | 46 +++++++++++++++++-------- cmd/checkpins/main_test.go | 20 +++++++++-- services/azureblob/PARITY.md | 65 +++++++++++++++++++++++------------- 3 files changed, 93 insertions(+), 38 deletions(-) diff --git a/cmd/checkpins/main.go b/cmd/checkpins/main.go index 753103092c..c519a2743e 100644 --- a/cmd/checkpins/main.go +++ b/cmd/checkpins/main.go @@ -1,6 +1,7 @@ // Command checkpins verifies that every services//PARITY.md sdk_module -// front-matter pin (aws-sdk-go-v2/service/@v) matches the -// version go.mod actually requires for that module. A stale pin silently +// front-matter pin (aws-sdk-go-v2/service/@v, or +// azure-sdk-for-go/sdk/@v for Azure-backed services) matches +// the version go.mod actually requires for that module. A stale pin silently // undermines every wire-shape claim in the file, since those claims were // checked against the pinned version, not whatever go.mod carries now. // @@ -35,6 +36,7 @@ const ( goModPath = "go.mod" sdkServiceModulePrefix = "github.com/aws/aws-sdk-go-v2/service/" + sdkAzureModulePrefix = "github.com/Azure/azure-sdk-for-go/sdk/" sdkModuleFieldPrefix = "sdk_module:" ) @@ -43,6 +45,14 @@ const ( // version including its leading "v". var pinRe = regexp.MustCompile(`^aws-sdk-go-v2/service/([A-Za-z0-9_-]+)@(v[0-9][0-9A-Za-z.\-+]*)$`) +// azurePinRe matches an sdk_module value shaped like +// "azure-sdk-for-go/sdk/storage/azblob@v1.8.0" -- azure-sdk-for-go publishes +// its service clients as nested submodules (sdk//) rather than +// aws-sdk-go-v2's flat sdk/service/, so the captured "module" group +// can itself contain a "/" and must be matched against the same nested path +// in go.mod (see loadGoModVersions). +var azurePinRe = regexp.MustCompile(`^azure-sdk-for-go/sdk/([A-Za-z0-9_/-]+)@(v[0-9][0-9A-Za-z.\-+]*)$`) + func main() { if err := run(); err != nil { fmt.Fprintln(os.Stderr, "checkpins:", err) @@ -105,9 +115,11 @@ func discoverServiceSlugs(dir string) ([]string, error) { } // loadGoModVersions parses go.mod and returns the pinned version (with its -// leading "v") of every aws-sdk-go-v2/service/ requirement, keyed by -// . Uses golang.org/x/mod/modfile rather than hand-parsing so both -// block-style and single-line `require` statements are covered correctly. +// leading "v") of every aws-sdk-go-v2/service/ and +// azure-sdk-for-go/sdk/ requirement, keyed by / +// respectively. Uses golang.org/x/mod/modfile rather than hand-parsing so +// both block-style and single-line `require` statements are covered +// correctly. func loadGoModVersions(path string) (map[string]string, error) { data, err := os.ReadFile(path) if err != nil { @@ -121,11 +133,15 @@ func loadGoModVersions(path string) (map[string]string, error) { versions := make(map[string]string, len(f.Require)) for _, req := range f.Require { - name, ok := strings.CutPrefix(req.Mod.Path, sdkServiceModulePrefix) - if !ok { + if name, ok := strings.CutPrefix(req.Mod.Path, sdkServiceModulePrefix); ok { + versions[name] = req.Mod.Version + continue } - versions[name] = req.Mod.Version + + if name, ok := strings.CutPrefix(req.Mod.Path, sdkAzureModulePrefix); ok { + versions[name] = req.Mod.Version + } } return versions, nil @@ -248,16 +264,20 @@ func splitValueComment(rest string) (string, string) { } // parsePinValue parses a raw sdk_module value (already stripped of its -// trailing comment) into a module name and version. +// trailing comment) into a module name and version. It tries the AWS shape +// first, then the Azure shape (see pinRe/azurePinRe). func parsePinValue(value string) (string, string, bool) { v := strings.Trim(strings.TrimSpace(value), `"'`) - m := pinRe.FindStringSubmatch(v) - if m == nil { - return "", "", false + if m := pinRe.FindStringSubmatch(v); m != nil { + return m[1], m[2], true + } + + if m := azurePinRe.FindStringSubmatch(v); m != nil { + return m[1], m[2], true } - return m[1], m[2], true + return "", "", false } // isModuleCacheOnly reports whether commentText documents the diff --git a/cmd/checkpins/main_test.go b/cmd/checkpins/main_test.go index d3de62a0ff..8d985d63e3 100644 --- a/cmd/checkpins/main_test.go +++ b/cmd/checkpins/main_test.go @@ -13,8 +13,9 @@ func TestEvaluatePin(t *testing.T) { t.Parallel() goModVersions := map[string]string{ - "dlm": "v1.39.4", - "opsworks": "v1.31.0", + "dlm": "v1.39.4", + "opsworks": "v1.31.0", + "storage/azblob": "v1.8.0", } tests := []struct { @@ -37,6 +38,19 @@ func TestEvaluatePin(t *testing.T) { wantKind: resultMismatch, wantMsg: "dlm: recorded v1.30.0, go.mod v1.39.4", }, + { + name: "matching azure pin", + slug: "azureblob", + content: "service: azureblob\nsdk_module: azure-sdk-for-go/sdk/storage/azblob@v1.8.0 # audited\n", + wantKind: resultOK, + }, + { + name: "mismatched azure pin", + slug: "azureblob", + content: "service: azureblob\nsdk_module: azure-sdk-for-go/sdk/storage/azblob@v1.7.0 # stale\n", + wantKind: resultMismatch, + wantMsg: "azureblob: recorded v1.7.0, go.mod v1.8.0", + }, { name: "module cache only documented", slug: "opsworks", @@ -108,6 +122,7 @@ go 1.26.5 require ( github.com/aws/aws-sdk-go-v2/service/dlm v1.39.4 + github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.8.0 github.com/other/pkg v0.1.0 ) @@ -120,5 +135,6 @@ require github.com/aws/aws-sdk-go-v2/service/opsworks v1.31.0 assert.Equal(t, "v1.39.4", versions["dlm"]) assert.Equal(t, "v1.31.0", versions["opsworks"]) + assert.Equal(t, "v1.8.0", versions["storage/azblob"]) assert.NotContains(t, versions, "pkg") } diff --git a/services/azureblob/PARITY.md b/services/azureblob/PARITY.md index 0cd4686868..5eba200d35 100644 --- a/services/azureblob/PARITY.md +++ b/services/azureblob/PARITY.md @@ -1,8 +1,8 @@ --- service: azureblob -sdk_module: azure-sdk-for-go/sdk/storage/azblob@v1.7.0 -last_audit_commit: (initial seed, no audit history yet) -last_audit_date: 2026-09-02 +sdk_module: azure-sdk-for-go/sdk/storage/azblob@v1.8.0 +last_audit_commit: f1427114b +last_audit_date: 2026-09-03 overall: C # Per-op or per-op-family status. Values: ok | partial | gap | deferred. # wire=response/request shape vs SDK; errors=code+HTTP status; state=real mutate/read; persist=in backendSnapshot. @@ -16,9 +16,10 @@ ops: GetBlobProperties: {wire: ok, errors: ok, state: ok, persist: n/a, note: "HEAD ///. Returns ETag/Last-Modified/Content-Length/Content-Type/x-ms-blob-type; no x-ms-meta-* or lease-state headers."} DeleteBlob: {wire: ok, errors: ok, state: ok, persist: ok, note: "DELETE ///. No snapshot/version-scoped delete, no soft-delete."} families: - auth: {status: deferred, note: "Authorization header is accepted structurally (SharedKey prefix or absent) but never cryptographically verified -- matches services/s3's PresignSecret-opt-in philosophy. Real SharedKey canonicalization/HMAC verification is deferred to pkgs/azureauth, landing on a separate branch (azure/auth-pkg); this package has a TODO(azure-integration) marker at the handler's auth entry point."} + auth: {status: partial, note: "pkgs/azureauth (SharedKey/SharedKeyLite header parsing + canonicalization + HMAC signing/verification) has landed and is wired in: checkAuth parses a present Authorization header via azureauth.ParseAuthorizationHeader. Verification (azureauth.VerifySharedKey) is implemented in pkgs/azureauth but not yet called from checkAuth -- enforcement is deliberately deferred past M0, matching services/s3's PresignSecret-opt-in philosophy. An absent or invalid header is still accepted."} blob_body_headers: {status: ok, note: "x-ms-version, x-ms-request-id, and Date are set on every response (success and error paths) via setCommonHeaders, so azure-sdk-for-go's response parsing does not error on missing headers."} - routing_isolation: {status: ok, note: "Runs on its own dedicated *http.Server (default port 10000, AZURE_BLOB_PORT override), never registered into the shared AWS single-port Router -- see provider.go's Provider doc comment and AZURE.md section 4 for the full rationale."} + routing_isolation: {status: ok, note: "Runs on its own dedicated *http.Server, bound synchronously in StartWorker to a fixed port (default 10000 via --azure-blob-port/AZURE_BLOB_PORT, no fallback pool -- fails fast if unavailable, mirroring services/iot's MQTT broker), never registered into the shared AWS single-port Router -- see provider.go's Provider doc comment and AZURE.md section 4 for the full rationale."} + observability: {status: ok, note: "StartWorker wraps its Echo handler with telemetry.WrapEchoHandler so ExtractOperation/ExtractResource feed Prometheus metrics, and derives its listener logger via logger.WithWorker(ctx, \"azureblob\", \"listener\"). InMemoryBackend and the server-lifecycle mutex both use *lockmetrics.RWMutex instead of raw sync.RWMutex/Mutex, matching repo convention."} gaps: - "Put Block / Put Block List (large-object multipart upload) is not implemented -- Put Blob only accepts a single whole-body BlockBlob PUT. Deliberate M0 scope per AZURE.md; tracked for a later milestone (M1 in AZURE.md's plan)." - "No ACL / container public-access-level support (x-ms-blob-public-access, Set/Get Container ACL are unimplemented)." @@ -27,13 +28,12 @@ gaps: - "No Copy Blob (server-side or cross-account) support." - "No snapshot, versioning, soft-delete, lease, or tier (hot/cool/archive) support." - "List Containers / List Blobs return every result in one page; no prefix/marker/maxresults pagination." - - "Auth is structurally permissive only -- see families.auth. Real SharedKey verification (pkgs/azureauth) is a separate, not-yet-landed dependency." + - "Auth verification is not enforced -- see families.auth. pkgs/azureauth.VerifySharedKey exists and is unit-tested but checkAuth does not call it yet." All gaps above are intentional MVP scope per AZURE.md's M0/M1 split, not oversights; see AZURE.md sections 2 and 8 for the milestone plan. deferred: - - "Initial implementation pass (2026-09-02): seeded this service from scratch per AZURE.md M0. No prior audit history to reconcile. sdk_module pinned to the latest azure-sdk-for-go blob module version documented in AZURE.md at authoring time; not yet cross-checked against a live SDK import in this repo (azure-sdk-for-go is not currently a go.mod dependency -- this package speaks the wire protocol directly rather than through the SDK's server-side types)." - - "cli.go registration is deliberately NOT wired up in this pass -- a human integrates this provider once pkgs/azureauth (a separate branch, azure/auth-pkg) also lands, per the task's explicit deferral." - - "No Go integration test (test/integration/azureblob_test.go) yet -- that requires the cli.go wiring above, which is out of scope for this pass. Unit tests exercise the handler/backend directly via httptest instead." -leaks: {status: clean, note: "No background goroutines, tickers, or janitor: InMemoryBackend is pure in-memory maps guarded by one sync.RWMutex, with no TTL/expiry sweep in this MVP scope. The dedicated *http.Server started by StartWorker is stopped by Shutdown via srv.Shutdown(ctx), mirroring cli.go's own top-level server lifecycle."} + - "Initial implementation pass (2026-09-02): seeded this service from scratch per AZURE.md M0. No prior audit history to reconcile." + - "M0 review pass (2026-09-03): pkgs/azureauth, cli.go registration, and test/integration/azureblob_test.go all landed in the same PR as this service (see AZURE.md's M0 entry) -- the file was previously drafted assuming a multi-PR sequence that did not happen. sdk_module bumped to the azblob v1.8.0 actually pinned in go.mod (used by the integration test)." +leaks: {status: clean, note: "No background goroutines, tickers, or janitor: InMemoryBackend is pure in-memory maps guarded by one *lockmetrics.RWMutex, with no TTL/expiry sweep in this MVP scope. The dedicated *http.Server started by StartWorker is stopped by Shutdown via srv.Shutdown(ctx) (falling back to srv.Close() on a graceful-shutdown error, both logged), mirroring cli.go's own top-level server lifecycle."} --- ## Notes @@ -45,22 +45,30 @@ services by header (`X-Amz-Target`) or distinctive path/form shape. Azure Blob's REST path shape (`///`) has no such service-identifying header, and colliding with Azure Queue/Table's identical `//` shape (once those land) would be exactly the -ambiguity the AWS router avoids by construction. Instead, `Provider.Init` -resolves a dedicated port (default 10000, mirroring Azurite's own -Blob-service default) and the returned `*Handler` implements -`service.BackgroundWorker`, standing up its own `*echo.Echo` + `*http.Server` -in `StartWorker`. See `provider.go`'s `Provider` doc comment and AZURE.md -section 4 for the full rationale, including why `pkgs/portalloc.Allocator` -(which only hands out the next free port in a range, with no way to reserve -a *specific* preferred port) couldn't be used as-is. +ambiguity the AWS router avoids by construction. Instead, the returned +`*Handler` implements `service.BackgroundWorker`, and `StartWorker` +synchronously binds a fixed port (default 10000, Azurite's own Blob-service +default, overridable via `--azure-blob-port`/`AZURE_BLOB_PORT`) before +standing up its own `*echo.Echo` + `*http.Server` to serve on that same +listener -- there is no probe-then-close window for another process to steal +the port between "we checked it was free" and "we're listening on it", and +no fallback into a different port if the bind fails (StartWorker returns the +bind error directly instead). This mirrors `services/iot`'s MQTT broker +(`services/iot/broker.go`), gopherstack's existing precedent for a +fixed-port service, rather than drawing from the shared `--port-range-start`/ +`--port-range-end` `PortAlloc` pool used for on-demand ephemeral resources. +See `provider.go`'s `Provider` doc comment and AZURE.md section 4 for the +full rationale. ### Auth -The `Authorization` header is accepted on structure alone -- a `SharedKey -...` prefix, or no header at all, both pass -- matching this repo's +The `Authorization` header is parsed via `pkgs/azureauth.ParseAuthorizationHeader` +(proving a real Azure SDK's header round-trips through this package), but a +malformed or absent header is still accepted -- matching this repo's permissive-by-default philosophy (`services/s3/sigv4.go`'s -`PresignSecret`-opt-in pattern). Real SharedKey HMAC verification is planned -for `pkgs/azureauth`, landing separately; `handler.go`'s `checkAuth` carries -the wiring TODO. +`PresignSecret`-opt-in pattern). `pkgs/azureauth.VerifySharedKey` implements +real SharedKey HMAC verification and is unit-tested, but `handler.go`'s +`checkAuth` does not call it yet; enforcing it is deliberately deferred past +this milestone. ### Blob names with slashes Azure blob names may contain `/` as a virtual-directory separator (e.g. @@ -76,6 +84,17 @@ with `Content-Range` set. Multi-range requests (`bytes=0-1,3-4`) are rejected with `416 Requested Range Not Satisfiable` rather than served -- Azure's own Get Blob does not support multi-range either. +### ETags +Blob ETags are derived from the body *and* a per-backend monotonically +increasing counter incremented on every `Put Blob` (see `store.go`'s +`computeBlobETag`/`etagSeq`), not from the body alone -- overwriting a blob +with byte-identical content still produces a new ETag, matching real Azure +Blob semantics and keeping `If-Match`/`If-None-Match` concurrency checks +meaningful (those headers are not yet enforced -- see gaps -- but the ETags +themselves are now correct). Container listing ETags (`List Containers`) use +a separate, simpler hash with no mutation-sequence component, since +containers have no mutable properties in this MVP. + ## More - [Full parity audit](PARITY.md) From 786dbb4260a579ff3a23c3337260e710065f996c Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 08:07:09 -0500 Subject: [PATCH 20/30] docs: fix stale claims, machine-specific paths, and a discarded Close error - AZURE.md: replace machine-specific /private/tmp/... paths with repo-relative ones; rewrite the port-selection rationale to follow services/iot's MQTT-broker precedent (fixed port, fail-fast) instead of citing Azurite's multi-port convention or an ad-hoc numbering scheme; mark M0 done. - services/azureblob/README.md: regenerated via `make docs` from PARITY.md (previously hand-authored, stale re: cli.go wiring and the azblob version). - test/integration/azureblob_test.go: capture and assert downloadResp.Body.Close()'s error instead of discarding it, still ahead of the read-result assertions. --- AZURE.md | 23 +++++++++++++---------- services/azureblob/README.md | 21 +++++++++------------ test/integration/azureblob_test.go | 3 ++- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/AZURE.md b/AZURE.md index d612c2a36e..9387ff546e 100644 --- a/AZURE.md +++ b/AZURE.md @@ -1,6 +1,6 @@ # Azure Support Implementation Plan for gopherstack -Repo cloned and inspected at `/private/tmp/claude-503/-Users-jacob-hochstetler-Code/926630d2-46b5-4853-a931-b4ea837b9658/scratchpad/gopherstack` (module `github.com/blackbirdworks/gopherstack`, GitHub `jh125486/gopherstack`). Findings below are grounded in that source (paths cited); Azure/Azurite mechanics are standard public documentation. +Findings below are grounded in this repository's own source (paths cited throughout are repo-relative); Azure/Azurite mechanics are standard public documentation. ## 1. How gopherstack is built today (relevant facts) @@ -53,6 +53,8 @@ gopherstack's flagship pattern is single-port, priority-matcher multiplexing — **Recommendation: give each Azure service its own port, mirroring Azurite's own 10000/10001/10002 convention (and Cosmos its own port, mirroring the real emulator's fixed 8081 default).** This is *more* wire-compatible, not less, since SDKs' default connection strings/emulator constants already assume separate ports. gopherstack already has the machinery for this — `AppContext.PortAlloc *portalloc.Allocator` is used elsewhere (e.g. EC2-docker SSH port ranges) for per-resource port allocation, so per-service dedicated listeners are an established pattern, not a new one. Each Azure service's `Provider.Init` stands up its own `echo.Echo` (or shares Echo's engine but binds a second listener), independent of the AWS `Router`. +**On the specific port-selection mechanism (raised in M0 review):** the right model to copy here isn't Azurite's own multi-port convention (10000/10001/10002) or an arbitrary "main port + 1" scheme — it's gopherstack's own existing precedent for a service with a fixed, protocol-conventional port: `services/iot`'s MQTT broker (`services/iot/broker.go`), which hardcodes MQTT's real default (`1883`) and, if that bind fails, simply fails fast rather than silently picking a different port. Azure Blob follows the same pattern: `Settings.Port` (default `10000`, Azurite's own Blob port, overridable via `--azure-blob-port`/`AZURE_BLOB_PORT`) is bound synchronously in `StartWorker`, with **no** fallback into the shared `--port-range-start`/`--port-range-end` `PortAlloc` pool that other resources (Lambda function URLs, ElastiCache) draw ephemeral ports from. A silent fallback would be just as surprising as inventing a different default number: either way, an SDK relying on the well-known default (`UseDevelopmentStorage=true`-style config, with zero further configuration) would end up silently talking to the wrong port. Failing fast with a clear "port already in use" error is simpler, matches the repo's own MQTT precedent, and — since `10000` is never drawn from the `PortAlloc` pool by this design — completely avoids the pool ever double-booking it, without needing any coordination between the two. + ## 5. Auth/connection-string strategy per service - **Blob/Queue/Table**: default to the fixed Azurite account name/key pair (`devstoreaccount1` / the published emulator key) so `UseDevelopmentStorage=true` and unmodified Azurite-targeting SDK config work out of the box. `Authorization: SharedKey ...` headers are parsed structurally (account name extraction for routing/logging); cryptographic verification is opt-in via a `WithSharedKeyValidation` toggle — directly mirroring `services/s3`'s `PresignSecret`/`WithPresignValidation` opt-in pattern. Env var overrides (`AZURE_STORAGE_ACCOUNT`, `AZURE_STORAGE_KEY`) for anyone who wants a non-default identity. @@ -75,7 +77,7 @@ gopherstack's flagship pattern is single-port, priority-matcher multiplexing — ## 8. Milestones -- **M0** — `pkgs/azureauth` (SharedKey canonicalization + fixed devstoreaccount1 constants); `services/azureblob` skeleton wired to its own port via `PortAlloc`; Create/Delete/List Container, Put/Get/Delete Blob, List Blobs; seeded `PARITY.md`; unit tests + one Go integration test. +- **M0 (done)** — `pkgs/azureauth` (SharedKey canonicalization + fixed devstoreaccount1 constants); `services/azureblob` wired into `cli.go` and bound to its own fixed port (synchronous bind, fail-fast, no `PortAlloc` fallback — see section 4's port-selection note); Create/Delete/List Container, Put/Get/Delete Blob, List Blobs; `PARITY.md`; unit tests + Go integration tests using `azure-sdk-for-go`. See `services/azureblob/README.md`/`PARITY.md` for current status and known gaps. - **M1** — Blob completeness: properties/metadata, block-blob multipart (Put Block/Put Block List), conditional headers (`If-Match`/`If-None-Match`), error-mapping table (mirrors `services/sqs`'s `errorDetails` pattern). - **M2** — `services/azurequeue`: full CRUD + message lifecycle (put/get/peek/delete/update/clear), visibility timeout. - **M3** — `services/azuretable`: table CRUD, entity insert/get/query/update/merge/delete, `$filter` subset (eq/ne/lt/gt/and/or on partition/row key plus scalar properties), ETag-based optimistic concurrency. @@ -84,11 +86,12 @@ gopherstack's flagship pattern is single-port, priority-matcher multiplexing — ## Key files referenced -- `/private/tmp/.../gopherstack/pkgs/service/service.go`, `router.go`, `priorities.go` — routing/registration contracts -- `/private/tmp/.../gopherstack/services/s3/provider.go`, `sigv4.go`, `persistence.go` — provider pattern, auth-opt-in pattern, snapshot pattern -- `/private/tmp/.../gopherstack/services/sqs/handler.go`, `provider.go` — handler/dispatch pattern, error-table pattern -- `/private/tmp/.../gopherstack/services/dynamodb/expr/` — expression-parser precedent for Table Storage's `$filter` -- `/private/tmp/.../gopherstack/services/s3/select_sql_*.go` — SQL-parser precedent for Cosmos queries -- `/private/tmp/.../gopherstack/cli.go` (`getServiceProviders`) — service registration list -- `/private/tmp/.../gopherstack/test/integration/sqs_test.go` — integration test convention -- `/private/tmp/.../gopherstack/services/sqs/PARITY.md`, `README.md` — parity-doc format +- `pkgs/service/service.go`, `router.go`, `priorities.go` — routing/registration contracts +- `services/s3/provider.go`, `sigv4.go`, `persistence.go` — provider pattern, auth-opt-in pattern, snapshot pattern +- `services/sqs/handler.go`, `provider.go` — handler/dispatch pattern, error-table pattern +- `services/dynamodb/expr/` — expression-parser precedent for Table Storage's `$filter` +- `services/s3/select_sql_*.go` — SQL-parser precedent for Cosmos queries +- `cli.go` (`getServiceProviders`) — service registration list +- `test/integration/sqs_test.go` — integration test convention +- `services/sqs/PARITY.md`, `README.md` — parity-doc format +- `services/azureblob/` (implemented, M0) — see section 8's M0 entry and `services/azureblob/PARITY.md` for current status diff --git a/services/azureblob/README.md b/services/azureblob/README.md index 9698a9fd1c..686d7ce918 100644 --- a/services/azureblob/README.md +++ b/services/azureblob/README.md @@ -1,36 +1,33 @@ - -# Azure Blob Storage + +# Azureblob -**Parity grade: C** · SDK `azure-sdk-for-go/sdk/storage/azblob@v1.7.0` · last audited 2026-09-02 (initial seed) +**Parity grade: C** · SDK `azure-sdk-for-go/sdk/storage/azblob@v1.8.0` · last audited 2026-09-03 (`f1427114b`) ## Coverage | Metric | Value | | --- | --- | | PARITY entries audited | 8 (7 ok, 1 partial) | -| Feature families | 3 (2 ok, 1 deferred) | +| Feature families | 4 (3 ok, 1 partial) | | Known gaps | 8 | -| Deferred items | 3 | +| Deferred items | 2 | | Resource leaks | clean | ### Known gaps -- Put Block / Put Block List (large-object multipart upload) is not implemented -- Put Blob only accepts a single whole-body BlockBlob PUT. Deliberate M0 scope per AZURE.md; tracked for a later milestone. +- Put Block / Put Block List (large-object multipart upload) is not implemented -- Put Blob only accepts a single whole-body BlockBlob PUT. Deliberate M0 scope per AZURE.md; tracked for a later milestone (M1 in AZURE.md's plan). - No ACL / container public-access-level support (x-ms-blob-public-access, Set/Get Container ACL are unimplemented). - No blob or container metadata (x-ms-meta-* headers) -- neither stored on PUT/Create nor returned on GET/HEAD/List. -- No conditional-header support (If-Match/If-None-Match/If-Modified-Since/If-Unmodified-Since) on any operation. +- No conditional-header support (If-Match/If-None-Match/If-Modified-Since/If-Unmodified-Since) on any operation -- every write unconditionally overwrites, every read unconditionally succeeds regardless of ETag/date preconditions. - No Copy Blob (server-side or cross-account) support. - No snapshot, versioning, soft-delete, lease, or tier (hot/cool/archive) support. - List Containers / List Blobs return every result in one page; no prefix/marker/maxresults pagination. -- Auth is structurally permissive only: the Authorization header is accepted on shape alone (a `SharedKey ...` prefix, or absent) and never cryptographically verified. Real SharedKey verification depends on `pkgs/azureauth`, which lands on a separate branch and is not yet importable from this package. - -All gaps above are intentional MVP scope per AZURE.md's M0/M1 split, not oversights. +- Auth verification is not enforced -- see families.auth. pkgs/azureauth.VerifySharedKey exists and is unit-tested but checkAuth does not call it yet. All gaps above are intentional MVP scope per AZURE.md's M0/M1 split, not oversights; see AZURE.md sections 2 and 8 for the milestone plan. ### Deferred - Initial implementation pass (2026-09-02): seeded this service from scratch per AZURE.md M0. No prior audit history to reconcile. -- `cli.go` registration is deliberately not wired up in this pass -- a human integrates this provider once `pkgs/azureauth` (branch `azure/auth-pkg`) also lands. -- No Go integration test (`test/integration/azureblob_test.go`) yet -- that requires the `cli.go` wiring above. Unit tests exercise the handler/backend directly via `httptest` instead. +- M0 review pass (2026-09-03): pkgs/azureauth, cli.go registration, and test/integration/azureblob_test.go all landed in the same PR as this service (see AZURE.md's M0 entry) -- the file was previously drafted assuming a multi-PR sequence that did not happen. sdk_module bumped to the azblob v1.8.0 actually pinned in go.mod (used by the integration test). ## More diff --git a/test/integration/azureblob_test.go b/test/integration/azureblob_test.go index 8a79a90d7d..9fa7a14991 100644 --- a/test/integration/azureblob_test.go +++ b/test/integration/azureblob_test.go @@ -87,7 +87,8 @@ func TestIntegration_AzureBlob_ContainerAndBlobLifecycle(t *testing.T) { require.NoError(t, err) body, err := io.ReadAll(downloadResp.Body) - _ = downloadResp.Body.Close() + closeErr := downloadResp.Body.Close() + require.NoError(t, closeErr, "closing the download stream should not error") require.NoError(t, err) assert.True(t, bytes.Equal(content, body), "downloaded blob content should match uploaded content") From c618b2b0ca2ca28fb70141227165fc1c01e2b8c1 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 08:07:16 -0500 Subject: [PATCH 21/30] test,docs: drop unneeded MD5 allowlist entry, canonicalize test headers, regen badges services/azureblob/store.go no longer imports crypto/md5 (see the ETag fix commit), so its weak_hash_guard_test.go allowlist entry is gone too. Canonicalize the remaining lowercase header-key literals in handler_test.go. Regenerate root README.md and .badges/*.svg via `make docs` to reflect azureblob's addition (163 services). --- .badges/operations.svg | 6 +++--- .badges/parity.svg | 12 ++++++------ .badges/services.svg | 6 +++--- README.md | 1 + services/azureblob/handler_test.go | 16 ++++++++-------- weak_hash_guard_test.go | 6 ------ 6 files changed, 21 insertions(+), 26 deletions(-) diff --git a/.badges/operations.svg b/.badges/operations.svg index 980e30b782..dc7cae144f 100644 --- a/.badges/operations.svg +++ b/.badges/operations.svg @@ -1,4 +1,4 @@ - + @@ -12,7 +12,7 @@ PARITY entries PARITY entries - 6371 - 6371 + 6379 + 6379 diff --git a/.badges/parity.svg b/.badges/parity.svg index a249485eea..3680cfa43f 100644 --- a/.badges/parity.svg +++ b/.badges/parity.svg @@ -1,18 +1,18 @@ - + - + - - + + parity parity - 158 A · 2 B - 158 A · 2 B + 158 A · 2 B · 1 C + 158 A · 2 B · 1 C diff --git a/.badges/services.svg b/.badges/services.svg index f2c24c76b0..8615f3de76 100644 --- a/.badges/services.svg +++ b/.badges/services.svg @@ -1,4 +1,4 @@ - + @@ -12,7 +12,7 @@ AWS services AWS services - 162 - 162 + 163 + 163 diff --git a/README.md b/README.md index f5c67c1dfd..e1b16da502 100644 --- a/README.md +++ b/README.md @@ -690,6 +690,7 @@ Every service links to its own page with a coverage breakdown — audited operat | Service | Parity | PARITY Entries | Notes | |---|---|---|---| | [AppStream 2.0](services/appstream/README.md) | A | 44 | clean | +| [Azureblob](services/azureblob/README.md) | C | 8 | 8 gaps; 2 deferred | | [Cloudfrontkeyvaluestore](services/cloudfrontkeyvaluestore/README.md) | B | 6 | 3 gaps; 1 structural gap | | [Directconnect](services/directconnect/README.md) | A | 64 | 3 gaps; 8 structural gaps; 1 deferred | | [Grafana](services/grafana/README.md) | A | 25 | 2 gaps; 1 structural gap | diff --git a/services/azureblob/handler_test.go b/services/azureblob/handler_test.go index fa269eb5e6..85cdb05a12 100644 --- a/services/azureblob/handler_test.go +++ b/services/azureblob/handler_test.go @@ -72,8 +72,8 @@ func TestContainerLifecycle_CreateListDelete(t *testing.T) { rec := doRequest(t, h, http.MethodPut, "/"+testAccount+"/mycontainer?restype=container", nil, nil) require.Equal(t, http.StatusCreated, rec.Code, tt.name) - assert.NotEmpty(t, rec.Header().Get("x-ms-version")) - assert.NotEmpty(t, rec.Header().Get("x-ms-request-id")) + assert.NotEmpty(t, rec.Header().Get("X-Ms-Version")) + assert.NotEmpty(t, rec.Header().Get("X-Ms-Request-Id")) assert.NotEmpty(t, rec.Header().Get("Date")) rec = doRequest(t, h, http.MethodGet, "/"+testAccount+"?comp=list", nil, nil) @@ -133,7 +133,7 @@ func TestBlobLifecycle_PutGetHeadDelete(t *testing.T) { createContainer(t, h, "mycontainer") putHeaders := map[string]string{ - "x-ms-blob-type": "BlockBlob", + "X-Ms-Blob-Type": "BlockBlob", "Content-Type": "text/plain", } rec := doRequest(t, h, http.MethodPut, "/"+testAccount+"/mycontainer/myblob.txt", @@ -145,7 +145,7 @@ func TestBlobLifecycle_PutGetHeadDelete(t *testing.T) { require.Equal(t, http.StatusOK, rec.Code, tt.name) assert.Equal(t, tt.body, rec.Body.String(), tt.name) assert.Equal(t, "text/plain", rec.Header().Get("Content-Type"), tt.name) - assert.Equal(t, "BlockBlob", rec.Header().Get("x-ms-blob-type"), tt.name) + assert.Equal(t, "BlockBlob", rec.Header().Get("X-Ms-Blob-Type"), tt.name) rec = doRequest(t, h, http.MethodHead, "/"+testAccount+"/mycontainer/myblob.txt", nil, nil) require.Equal(t, http.StatusOK, rec.Code, tt.name) @@ -182,7 +182,7 @@ func TestPutBlob_RequiresBlockBlobType(t *testing.T) { headers := map[string]string{} if tt.blobType != "" { - headers["x-ms-blob-type"] = tt.blobType + headers["X-Ms-Blob-Type"] = tt.blobType } rec := doRequest(t, h, http.MethodPut, "/"+testAccount+"/mycontainer/myblob.txt", []byte("x"), headers) @@ -209,7 +209,7 @@ func TestPutBlob_MissingContainerReturns404(t *testing.T) { h := newTestHandler(t) rec := doRequest(t, h, http.MethodPut, "/"+testAccount+"/does-not-exist/myblob.txt", - []byte("x"), map[string]string{"x-ms-blob-type": "BlockBlob"}) + []byte("x"), map[string]string{"X-Ms-Blob-Type": "BlockBlob"}) require.Equal(t, http.StatusNotFound, rec.Code, tt.name) assert.Contains(t, rec.Body.String(), "ContainerNotFound", tt.name) @@ -265,7 +265,7 @@ func TestGetBlob_RangeHeaderPartialRead(t *testing.T) { h := newTestHandler(t) createContainer(t, h, "mycontainer") doRequest(t, h, http.MethodPut, "/"+testAccount+"/mycontainer/data.bin", - []byte(body), map[string]string{"x-ms-blob-type": "BlockBlob"}) + []byte(body), map[string]string{"X-Ms-Blob-Type": "BlockBlob"}) rec := doRequest(t, h, http.MethodGet, "/"+testAccount+"/mycontainer/data.bin", nil, map[string]string{"Range": tt.rangeValue}) @@ -321,7 +321,7 @@ func TestListBlobs_ReturnsAllBlobs(t *testing.T) { for _, name := range tt.blobs { doRequest(t, h, http.MethodPut, "/"+testAccount+"/mycontainer/"+name, - []byte("data"), map[string]string{"x-ms-blob-type": "BlockBlob"}) + []byte("data"), map[string]string{"X-Ms-Blob-Type": "BlockBlob"}) } rec := doRequest(t, h, http.MethodGet, "/"+testAccount+"/mycontainer?restype=container&comp=list", nil, nil) diff --git a/weak_hash_guard_test.go b/weak_hash_guard_test.go index 1ee7fd7de2..239bb7902a 100644 --- a/weak_hash_guard_test.go +++ b/weak_hash_guard_test.go @@ -25,12 +25,6 @@ func allowedWeakHashFiles() map[string]string { // checksum echoed on the wire, never a credential. gopherstack-ziv9. "services/ssm/document_hashes.go": "DocumentDescription.Sha1 parity, verified", - // Azure Blob's Content-MD5/ETag are MD5 by specification (same as S3's - // ETag, already allowlisted below) -- a content-integrity fingerprint - // echoed on the wire so azure-sdk-for-go's blob client can validate - // upload/download integrity, never a credential or security hash. - "services/azureblob/store.go": "Content-MD5/ETag generation, Azure Blob wire-protocol requirement, verified", - // Pre-existing at the time this guard was added, and NOT individually // audited. Each is presumed an AWS-protocol requirement -- S3 ETags are // MD5 by specification, TOTP is HMAC-SHA1 by RFC 6238, key-pair From 92e3fd176b53bbb0a38f08dc2d842d4a94f9476e Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 09:58:12 -0500 Subject: [PATCH 22/30] portalloc,cli: reserve AzureBlob's fixed port in the shared pool services/azureblob binds its dedicated listener directly via net.Listen, not through PortAlloc -- but its default port (10000, Azurite's own Blob port) sits inside --port-range-start/--port-range-end's own default range (10000-10100). PortAlloc only tracks ports it has itself handed out via Acquire, so without this it could still hand 10000 to an unrelated caller (e.g. an ElastiCache instance), surfacing only later as a confusing address-in-use failure when that caller tried to actually bind it. services/iot's MQTT broker (the precedent the port-selection rework followed) avoids this by luck, not design: its fixed port (1883) simply falls outside the default range. Add Allocator.Reserve(port, label), which permanently marks a port unavailable to Acquire without binding anything (a no-op if the port is outside the pool's range, matching MQTT's case) -- and call it from a new cli.go helper, reserveFixedServicePorts, right after the pool is constructed. Update AZURE.md/PARITY.md's port-selection rationale to correct the previous claim that no coordination between the two was needed; there is, just one-directional and one-time at startup. --- AZURE.md | 4 +- cli.go | 30 ++++++++++ cli_azureblob_port_reservation_test.go | 69 ++++++++++++++++++++++ pkgs/portalloc/portalloc.go | 31 ++++++++++ pkgs/portalloc/portalloc_test.go | 79 ++++++++++++++++++++++++++ services/azureblob/PARITY.md | 2 +- 6 files changed, 213 insertions(+), 2 deletions(-) create mode 100644 cli_azureblob_port_reservation_test.go diff --git a/AZURE.md b/AZURE.md index 9387ff546e..e3468c8adf 100644 --- a/AZURE.md +++ b/AZURE.md @@ -53,7 +53,9 @@ gopherstack's flagship pattern is single-port, priority-matcher multiplexing — **Recommendation: give each Azure service its own port, mirroring Azurite's own 10000/10001/10002 convention (and Cosmos its own port, mirroring the real emulator's fixed 8081 default).** This is *more* wire-compatible, not less, since SDKs' default connection strings/emulator constants already assume separate ports. gopherstack already has the machinery for this — `AppContext.PortAlloc *portalloc.Allocator` is used elsewhere (e.g. EC2-docker SSH port ranges) for per-resource port allocation, so per-service dedicated listeners are an established pattern, not a new one. Each Azure service's `Provider.Init` stands up its own `echo.Echo` (or shares Echo's engine but binds a second listener), independent of the AWS `Router`. -**On the specific port-selection mechanism (raised in M0 review):** the right model to copy here isn't Azurite's own multi-port convention (10000/10001/10002) or an arbitrary "main port + 1" scheme — it's gopherstack's own existing precedent for a service with a fixed, protocol-conventional port: `services/iot`'s MQTT broker (`services/iot/broker.go`), which hardcodes MQTT's real default (`1883`) and, if that bind fails, simply fails fast rather than silently picking a different port. Azure Blob follows the same pattern: `Settings.Port` (default `10000`, Azurite's own Blob port, overridable via `--azure-blob-port`/`AZURE_BLOB_PORT`) is bound synchronously in `StartWorker`, with **no** fallback into the shared `--port-range-start`/`--port-range-end` `PortAlloc` pool that other resources (Lambda function URLs, ElastiCache) draw ephemeral ports from. A silent fallback would be just as surprising as inventing a different default number: either way, an SDK relying on the well-known default (`UseDevelopmentStorage=true`-style config, with zero further configuration) would end up silently talking to the wrong port. Failing fast with a clear "port already in use" error is simpler, matches the repo's own MQTT precedent, and — since `10000` is never drawn from the `PortAlloc` pool by this design — completely avoids the pool ever double-booking it, without needing any coordination between the two. +**On the specific port-selection mechanism (raised in M0 review):** the right model to copy here isn't Azurite's own multi-port convention (10000/10001/10002) or an arbitrary "main port + 1" scheme — it's gopherstack's own existing precedent for a service with a fixed, protocol-conventional port: `services/iot`'s MQTT broker (`services/iot/broker.go`), which hardcodes MQTT's real default (`1883`) and, if that bind fails, simply fails fast rather than silently picking a different port. Azure Blob follows the same pattern: `Settings.Port` (default `10000`, Azurite's own Blob port, overridable via `--azure-blob-port`/`AZURE_BLOB_PORT`) is bound synchronously in `StartWorker`, with **no** fallback into the shared `--port-range-start`/`--port-range-end` `PortAlloc` pool that other resources (Lambda function URLs, ElastiCache) draw ephemeral ports from. A silent fallback would be just as surprising as inventing a different default number: either way, an SDK relying on the well-known default (`UseDevelopmentStorage=true`-style config, with zero further configuration) would end up silently talking to the wrong port. Failing fast with a clear "port already in use" error is simpler and matches the repo's own MQTT precedent. + +That said, MQTT's fixed port (`1883`) happens to fall *outside* `--port-range-start`/`--port-range-end`'s own default range (`10000`-`10100`), so it never needed to coordinate with `PortAlloc` at all. Azure Blob's default (`10000`) does **not** have that luck — it sits at the very start of that range. `PortAlloc` only tracks ports it has itself handed out via `Acquire`; it has no way to know Azure Blob bound `10000` directly, and would happily hand that same number to an unrelated `Acquire` caller (e.g. an ElastiCache instance), which would only surface later as a confusing address-in-use failure when that caller tried to actually bind it. `cli.go`'s `reserveFixedServicePorts` closes this gap explicitly: right after the pool is constructed, it calls the new `portalloc.Allocator.Reserve(port, label)` for Azure Blob's configured port, permanently marking it unavailable to `Acquire` (a no-op if the port falls outside the pool's range, exactly MQTT's case). So there *is* coordination between the two, just one-directional and one-time at startup — Azure Blob doesn't ask the pool for anything, but the pool is told to leave Azure Blob's port alone. ## 5. Auth/connection-string strategy per service diff --git a/cli.go b/cli.go index fd4f765420..86e5d7c90a 100644 --- a/cli.go +++ b/cli.go @@ -1852,6 +1852,35 @@ func setupPortAllocator( return alloc } +// reserveFixedServicePorts marks ports bound directly by services outside +// the shared PortAlloc pool as unavailable within that pool, so Acquire +// never hands the same port number to a different caller. +// +// AzureBlob's dedicated listener (services/azureblob) binds a fixed, +// protocol-conventional default port (10000, matching Azurite's own Blob +// service port) via a raw net.Listen call, not through PortAlloc -- and that +// default sits squarely inside PortRangeStart/PortRangeEnd's own default +// range (10000-10100). Without this reservation, PortAlloc has no way to +// know AzureBlob already holds 10000 and could hand it to an unrelated +// caller (e.g. an ElastiCache instance), which would only surface later as +// a confusing address-in-use failure when that caller tries to actually +// bind it. See AZURE.md section 4 for the full rationale. +// +// A failed reservation is logged, not fatal: AzureBlob's own StartWorker +// bind is still synchronous and fails fast on a genuine conflict (see +// handler.go), so the worst outcome here is losing this early-warning +// cross-service protection, not an unrecoverable startup failure. +func reserveFixedServicePorts(ctx context.Context, log *slog.Logger, alloc *portalloc.Allocator, cli CLI) { + if alloc == nil { + return + } + + if err := alloc.Reserve(cli.AzureBlob.Port, "azureblob"); err != nil { + log.WarnContext(ctx, "failed to reserve AzureBlob's fixed port in the shared pool", + "port", cli.AzureBlob.Port, "error", err) + } +} + // run starts the server with the given CLI configuration. // It is separated from Run so it can be exercised in tests without [os.Exit]. func run(ctx context.Context, cli CLI) error { @@ -1876,6 +1905,7 @@ func run(ctx context.Context, cli CLI) error { // --- Port allocator --- cli.portAlloc = setupPortAllocator(ctx, log, cli.PortRangeStart, cli.PortRangeEnd) + reserveFixedServicePorts(ctx, log, cli.portAlloc, cli) // --- Embedded DNS server --- var dnsSrv *gopherDNS.Server diff --git a/cli_azureblob_port_reservation_test.go b/cli_azureblob_port_reservation_test.go new file mode 100644 index 0000000000..caf0ac8d71 --- /dev/null +++ b/cli_azureblob_port_reservation_test.go @@ -0,0 +1,69 @@ +package main + +import ( + "log/slog" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/pkgs/portalloc" + azureblobbackend "github.com/blackbirdworks/gopherstack/services/azureblob" +) + +// TestReserveFixedServicePorts is a regression test: services/azureblob binds +// its dedicated listener directly via net.Listen, not through PortAlloc, but +// its default port (10000) sits inside PortRangeStart/PortRangeEnd's own +// default range (10000-10100). Without reserving it, PortAlloc could still +// hand that same port number to an unrelated caller (e.g. ElastiCache), +// which would only surface later as a confusing address-in-use failure. See +// AZURE.md section 4 and pkgs/portalloc.Allocator.Reserve's doc comment. +func TestReserveFixedServicePorts(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + azurePort int + rangeStart int + rangeEnd int + wantBlockedFromPool bool + }{ + { + name: "default azure port collides with default pool range", + azurePort: azureblobbackend.DefaultPort, rangeStart: 10000, rangeEnd: 10100, + wantBlockedFromPool: true, + }, + { + name: "custom azure port outside a custom pool range", + azurePort: 9999, rangeStart: 10000, rangeEnd: 10100, + wantBlockedFromPool: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + alloc, err := portalloc.New(tt.rangeStart, tt.rangeEnd) + require.NoError(t, err) + + cli := CLI{AzureBlob: azureblobbackend.Settings{Port: tt.azurePort}} + reserveFixedServicePorts(t.Context(), slog.Default(), alloc, cli) + + assert.Equal(t, tt.wantBlockedFromPool, alloc.IsAllocated(tt.azurePort), tt.name) + }) + } +} + +// TestReserveFixedServicePorts_NilAllocatorIsNoop covers the disabled-pool +// path (setupPortAllocator returns nil for an invalid range): nothing to +// reserve against, must not panic. +func TestReserveFixedServicePorts_NilAllocatorIsNoop(t *testing.T) { + t.Parallel() + + cli := CLI{AzureBlob: azureblobbackend.Settings{Port: azureblobbackend.DefaultPort}} + + assert.NotPanics(t, func() { + reserveFixedServicePorts(t.Context(), slog.Default(), nil, cli) + }) +} diff --git a/pkgs/portalloc/portalloc.go b/pkgs/portalloc/portalloc.go index f75478851a..413ca59341 100644 --- a/pkgs/portalloc/portalloc.go +++ b/pkgs/portalloc/portalloc.go @@ -84,6 +84,37 @@ func (a *Allocator) Release(port int) error { return nil } +// ErrPortAlreadyReserved is returned by Reserve when port is already marked +// used (by a prior Reserve or Acquire call). +var ErrPortAlreadyReserved = errors.New("port already reserved or allocated") + +// Reserve permanently marks port as unavailable in the pool, associating it +// with label, without actually binding anything -- for services that bind a +// fixed port of their own outside this allocator entirely (e.g. a +// protocol-conventional default port), so Acquire never hands that same +// port to a different caller and causes a surprise address-in-use failure +// later. Intended to be called once at startup, before any Acquire calls. +// +// A port outside [start, end) is a no-op (nil, nil): Acquire never +// considers it anyway, so there is nothing to protect. Returns +// ErrPortAlreadyReserved if port is already marked used. +func (a *Allocator) Reserve(port int, label string) error { + a.mu.Lock("Reserve") + defer a.mu.Unlock() + + if port < a.start || port >= a.end { + return nil + } + + if _, taken := a.used[port]; taken { + return fmt.Errorf("%w: %d", ErrPortAlreadyReserved, port) + } + + a.used[port] = label + + return nil +} + func (a *Allocator) advanceNext(port int) { a.next = port + 1 if a.next >= a.end { diff --git a/pkgs/portalloc/portalloc_test.go b/pkgs/portalloc/portalloc_test.go index 6b9522d247..d6dd77e155 100644 --- a/pkgs/portalloc/portalloc_test.go +++ b/pkgs/portalloc/portalloc_test.go @@ -162,3 +162,82 @@ func TestConcurrentAcquire(t *testing.T) { } } } + +// TestReserve_InRangeBlocksAcquire is a regression test for the cross-service +// port collision this method exists to prevent: a service that binds a fixed +// port directly (outside this allocator) must be able to keep Acquire from +// ever handing that same port number to something else. +func TestReserve_InRangeBlocksAcquire(t *testing.T) { + t.Parallel() + + a, err := portalloc.New(10000, 10003) + require.NoError(t, err) + + require.NoError(t, a.Reserve(10000, "azureblob")) + assert.True(t, a.IsAllocated(10000)) + assert.Equal(t, 2, a.Available()) + + p1, err := a.Acquire("svc-a") + require.NoError(t, err) + assert.Equal(t, 10001, p1, "Acquire must skip the reserved port") + + p2, err := a.Acquire("svc-b") + require.NoError(t, err) + assert.Equal(t, 10002, p2) + + _, err = a.Acquire("svc-c") + assert.ErrorIs(t, err, portalloc.ErrNoPortsAvailable, "reserved port must never be handed out") +} + +func TestReserve_OutOfRangeIsNoop(t *testing.T) { + t.Parallel() + + a, err := portalloc.New(10000, 10003) + require.NoError(t, err) + + require.NoError(t, a.Reserve(1883, "iot-mqtt")) + assert.False(t, a.IsAllocated(1883), "a port outside the range is never tracked") + assert.Equal(t, 3, a.Available()) +} + +func TestReserve_AlreadyUsedErrors(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setup func(t *testing.T, a *portalloc.Allocator) + wantErr error + }{ + { + name: "already reserved", + setup: func(t *testing.T, a *portalloc.Allocator) { + t.Helper() + require.NoError(t, a.Reserve(10000, "first")) + }, + wantErr: portalloc.ErrPortAlreadyReserved, + }, + { + name: "already acquired", + setup: func(t *testing.T, a *portalloc.Allocator) { + t.Helper() + _, err := a.Acquire("svc-a") + require.NoError(t, err) + }, + wantErr: portalloc.ErrPortAlreadyReserved, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + a, err := portalloc.New(10000, 10003) + require.NoError(t, err) + + tt.setup(t, a) + + err = a.Reserve(10000, "second") + assert.ErrorIs(t, err, tt.wantErr) + }) + } +} diff --git a/services/azureblob/PARITY.md b/services/azureblob/PARITY.md index 5eba200d35..19f24d7c84 100644 --- a/services/azureblob/PARITY.md +++ b/services/azureblob/PARITY.md @@ -18,7 +18,7 @@ ops: families: auth: {status: partial, note: "pkgs/azureauth (SharedKey/SharedKeyLite header parsing + canonicalization + HMAC signing/verification) has landed and is wired in: checkAuth parses a present Authorization header via azureauth.ParseAuthorizationHeader. Verification (azureauth.VerifySharedKey) is implemented in pkgs/azureauth but not yet called from checkAuth -- enforcement is deliberately deferred past M0, matching services/s3's PresignSecret-opt-in philosophy. An absent or invalid header is still accepted."} blob_body_headers: {status: ok, note: "x-ms-version, x-ms-request-id, and Date are set on every response (success and error paths) via setCommonHeaders, so azure-sdk-for-go's response parsing does not error on missing headers."} - routing_isolation: {status: ok, note: "Runs on its own dedicated *http.Server, bound synchronously in StartWorker to a fixed port (default 10000 via --azure-blob-port/AZURE_BLOB_PORT, no fallback pool -- fails fast if unavailable, mirroring services/iot's MQTT broker), never registered into the shared AWS single-port Router -- see provider.go's Provider doc comment and AZURE.md section 4 for the full rationale."} + routing_isolation: {status: ok, note: "Runs on its own dedicated *http.Server, bound synchronously in StartWorker to a fixed port (default 10000 via --azure-blob-port/AZURE_BLOB_PORT, no fallback pool -- fails fast if unavailable, mirroring services/iot's MQTT broker), never registered into the shared AWS single-port Router -- see provider.go's Provider doc comment and AZURE.md section 4 for the full rationale. cli.go's reserveFixedServicePorts additionally reserves this port in the shared PortAlloc pool (pkgs/portalloc.Allocator.Reserve) at startup, since 10000 sits inside --port-range-start/--port-range-end's own default range and would otherwise be handed to an unrelated Acquire caller (fixed in M0 review)."} observability: {status: ok, note: "StartWorker wraps its Echo handler with telemetry.WrapEchoHandler so ExtractOperation/ExtractResource feed Prometheus metrics, and derives its listener logger via logger.WithWorker(ctx, \"azureblob\", \"listener\"). InMemoryBackend and the server-lifecycle mutex both use *lockmetrics.RWMutex instead of raw sync.RWMutex/Mutex, matching repo convention."} gaps: - "Put Block / Put Block List (large-object multipart upload) is not implemented -- Put Blob only accepts a single whole-body BlockBlob PUT. Deliberate M0 scope per AZURE.md; tracked for a later milestone (M1 in AZURE.md's plan)." From e8923218d46c227beef49aff32815e49a226b3b7 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 12:40:31 -0500 Subject: [PATCH 23/30] azureblob,portalloc: fieldalignment fixes Reorder struct fields (ContainerInfo, BlobInfo, storedBlob, storedContainer, blobProperties, Handler, and a table-test struct portalloc_test.go's TestReserve_AlreadyUsedErrors introduced) so larger and pointer-containing fields come before smaller/non-pointer ones, per govet's fieldalignment check. No behavior change: JSON snapshot encoding is unaffected (pkgs/persistence's golden-file inventory sorts field names alphabetically, independent of declaration order) and XML marshaling's one reordered field (blobProperties.ContentLength) is matched by tag name on unmarshal by every conformant client, not position. Left two already-optimally-ordered structs alone despite being named in the report (store_test.go's MissingContainerErrors table and azureauth_test.go's TestSigning_DoesNotMutateHeaders table both already put their larger field first) -- could not identify a real issue by hand and did not want to guess wrong. --- services/azureblob/errors.go | 7 +++++++ services/azureblob/persistence.go | 4 ++-- services/azureblob/persistence_test.go | 17 ++++++++++------- services/azureblob/store.go | 2 +- services/azureblob/store_test.go | 3 +-- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/services/azureblob/errors.go b/services/azureblob/errors.go index a8c89f2d6d..8e10f9529e 100644 --- a/services/azureblob/errors.go +++ b/services/azureblob/errors.go @@ -9,4 +9,11 @@ var ( ErrBlobNotFound = errors.New("azureblob: blob not found") ErrInvalidBlobType = errors.New("azureblob: unsupported x-ms-blob-type") ErrInvalidRange = errors.New("azureblob: invalid range") + + // ErrSnapshotContainerNull and ErrSnapshotBlobNull are returned by + // Restore when a snapshot's "containers" map (or a container's "Blobs" + // map) holds a JSON null entry, which decodes to a nil pointer that + // would panic on first dereference if stored as-is. See persistence.go. + ErrSnapshotContainerNull = errors.New("azureblob: restore snapshot: container is null") + ErrSnapshotBlobNull = errors.New("azureblob: restore snapshot: blob is null") ) diff --git a/services/azureblob/persistence.go b/services/azureblob/persistence.go index a844a6c92d..9e279ebd9d 100644 --- a/services/azureblob/persistence.go +++ b/services/azureblob/persistence.go @@ -75,7 +75,7 @@ func (b *InMemoryBackend) Restore(ctx context.Context, data []byte) error { // storedBlob.info() for a null blob entry). Reject the whole // snapshot rather than silently dropping or fabricating an entry. if c == nil { - return fmt.Errorf("azureblob: restore snapshot: container %q is null", name) + return fmt.Errorf("%w: %q", ErrSnapshotContainerNull, name) } if c.Blobs == nil { @@ -86,7 +86,7 @@ func (b *InMemoryBackend) Restore(ctx context.Context, data []byte) error { for blobName, blob := range c.Blobs { if blob == nil { - return fmt.Errorf("azureblob: restore snapshot: blob %q in container %q is null", blobName, name) + return fmt.Errorf("%w: %q in container %q", ErrSnapshotBlobNull, blobName, name) } } } diff --git a/services/azureblob/persistence_test.go b/services/azureblob/persistence_test.go index 90c7bc96ad..f4c6491875 100644 --- a/services/azureblob/persistence_test.go +++ b/services/azureblob/persistence_test.go @@ -82,16 +82,19 @@ func TestRestore_RejectsNullEntries(t *testing.T) { t.Parallel() tests := []struct { - name string - data []byte + name string + data []byte + wantErr error }{ { - name: "null_container", - data: []byte(`{"version":1,"containers":{"c1":null}}`), + name: "null_container", + data: []byte(`{"version":1,"containers":{"c1":null}}`), + wantErr: azureblob.ErrSnapshotContainerNull, }, { - name: "null_blob", - data: []byte(`{"version":1,"containers":{"c1":{"Name":"c1","Blobs":{"b1":null}}}}`), + name: "null_blob", + data: []byte(`{"version":1,"containers":{"c1":{"Name":"c1","Blobs":{"b1":null}}}}`), + wantErr: azureblob.ErrSnapshotBlobNull, }, } @@ -105,7 +108,7 @@ func TestRestore_RejectsNullEntries(t *testing.T) { require.NoError(t, b.CreateContainer("preexisting")) err := b.Restore(ctx, tt.data) - require.Error(t, err, tt.name) + require.ErrorIs(t, err, tt.wantErr, tt.name) // A rejected snapshot must not have partially mutated state. containers := b.ListContainers() diff --git a/services/azureblob/store.go b/services/azureblob/store.go index f75251578c..4916e0e259 100644 --- a/services/azureblob/store.go +++ b/services/azureblob/store.go @@ -150,7 +150,7 @@ func (b *InMemoryBackend) DeleteBlob(container, blob string) error { return ErrContainerNotFound } - if _, ok := c.Blobs[blob]; !ok { + if _, blobExists := c.Blobs[blob]; !blobExists { return ErrBlobNotFound } diff --git a/services/azureblob/store_test.go b/services/azureblob/store_test.go index fa906967c6..fb3e20d8ca 100644 --- a/services/azureblob/store_test.go +++ b/services/azureblob/store_test.go @@ -1,7 +1,6 @@ package azureblob_test import ( - "errors" "testing" "github.com/stretchr/testify/assert" @@ -120,7 +119,7 @@ func TestInMemoryBackend_MissingContainerErrors(t *testing.T) { err := tt.op(b) require.Error(t, err) - assert.True(t, errors.Is(err, azureblob.ErrContainerNotFound), tt.name) + assert.ErrorIs(t, err, azureblob.ErrContainerNotFound, tt.name) }) } } From 779230848f205cb124cebdffb259f600725822ab Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 12:40:42 -0500 Subject: [PATCH 24/30] azureblob: cyclop, goconst, mnd, nonamedreturns, noctx fixes - operationFor (cyclomatic complexity 23) split into accountOperationFor/ containerOperationFor/blobOperationFor, one per path level, mirroring handleAccountLevel/handleContainerLevel/handleBlobLevel's own dispatch shape. - Repeated "list"/"container" (and the "comp"/"restype" query keys) string literals extracted to named constants (compList, restypeContainer, queryComp, queryRestype), used consistently in operationFor's new helpers and the real handlers (handleAccountLevel/handleContainerLevel). - splitPath's magic numbers 3 (SplitN count) and 2 (blob-segment index) extracted to maxPathSegments/blobSegmentIndex. - splitPath and parseRange's named returns removed (nonamedreturns); export_test.go's wrapper signatures updated to match. - StartWorker's net.Listen("tcp", ...) replaced with (&net.ListenConfig{}).Listen(ctx, "tcp", ...) so bind failures respect ctx cancellation (noctx). --- services/azureblob/export_test.go | 4 +- services/azureblob/handler.go | 110 ++++++++++++++++++++++-------- 2 files changed, 84 insertions(+), 30 deletions(-) diff --git a/services/azureblob/export_test.go b/services/azureblob/export_test.go index eb8a510687..5dfb6d745d 100644 --- a/services/azureblob/export_test.go +++ b/services/azureblob/export_test.go @@ -3,11 +3,11 @@ package azureblob // Exported wrappers for internal functions used in blackbox tests. // ParseRange exposes parseRange for external tests. -func ParseRange(header string, size int64) (start, end int64, ok bool) { +func ParseRange(header string, size int64) (int64, int64, bool) { return parseRange(header, size) } // SplitPath exposes splitPath for external tests. -func SplitPath(p string) (account, container, blob string) { +func SplitPath(p string) (string, string, string) { return splitPath(p) } diff --git a/services/azureblob/handler.go b/services/azureblob/handler.go index e7653c5326..e5f160d3ff 100644 --- a/services/azureblob/handler.go +++ b/services/azureblob/handler.go @@ -51,6 +51,9 @@ type Handler struct { Backend StorageBackend Endpoint string // e.g. "http://127.0.0.1:10000" -- used to build ServiceEndpoint in list responses + srvMu *lockmetrics.RWMutex + srv *http.Server + // Port is the TCP port StartWorker binds. Set from Settings at Init time // (see provider.go); defaults to DefaultPort. Unlike a per-resource // ephemeral allocation, this is a single fixed, protocol-conventional @@ -58,9 +61,6 @@ type Handler struct { // pool, so StartWorker fails fast if it's unavailable rather than // silently binding a different port. Port int - - srvMu *lockmetrics.RWMutex - srv *http.Server } // NewHandler creates a new Azure Blob Handler. Port defaults to DefaultPort; @@ -206,53 +206,105 @@ func newRequestID() string { return fmt.Sprintf("%x-%x-%x-%x-%x", buf[0:4], buf[4:6], buf[6:8], buf[8:10], buf[10:16]) } +// maxPathSegments bounds splitPath's strings.SplitN call: account, container, +// and everything else as blob (blob names may contain "/" themselves). +const maxPathSegments = 3 + +// blobSegmentIndex is the index (and, as a length check, the minimum part +// count) at which a blob segment is present in splitPath's parts slice. +const blobSegmentIndex = 2 + // splitPath splits an Azure Blob REST path ("///") // into its three components. blob may itself contain "/" (Azure blob names // may include virtual-directory separators), so it is never split further. -func splitPath(p string) (account, container, blob string) { +func splitPath(p string) (string, string, string) { p = strings.TrimPrefix(p, "/") if p == "" { return "", "", "" } - parts := strings.SplitN(p, "/", 3) - account = parts[0] + parts := strings.SplitN(p, "/", maxPathSegments) + account := parts[0] + + var container, blob string if len(parts) > 1 { container = parts[1] } - if len(parts) > 2 { - blob = parts[2] + if len(parts) > blobSegmentIndex { + blob = parts[blobSegmentIndex] } return account, container, blob } +// Query-parameter names and values shared by operationFor and the actual +// request handlers (handleAccountLevel/handleContainerLevel). +const ( + queryRestype = "restype" + queryComp = "comp" + + restypeContainer = "container" + compList = "list" +) + // operationFor determines the Azure Blob operation name for a request, for // metrics labeling. Mirrors the dispatch logic in handleAccountLevel/ -// handleContainerLevel/handleBlobLevel without side effects. +// handleContainerLevel/handleBlobLevel without side effects. Split into one +// helper per path level (account/container/blob) to keep each branch small. func operationFor(r *http.Request) string { _, container, blob := splitPath(r.URL.Path) - restype := r.URL.Query().Get("restype") - comp := r.URL.Query().Get("comp") switch { - case container == "" && r.Method == http.MethodGet && comp == "list": + case blob != "": + return blobOperationFor(r.Method) + case container != "": + return containerOperationFor(r) + default: + return accountOperationFor(r) + } +} + +// accountOperationFor covers the one account-level operation, List +// Containers (GET /?comp=list). +func accountOperationFor(r *http.Request) string { + if r.Method == http.MethodGet && r.URL.Query().Get(queryComp) == compList { return opListContainers - case blob == "" && r.Method == http.MethodPut && restype == "container": + } + + return unknownOperation +} + +// containerOperationFor covers the three container-scoped operations: +// Create Container, Delete Container, and List Blobs. +func containerOperationFor(r *http.Request) string { + restype := r.URL.Query().Get(queryRestype) + comp := r.URL.Query().Get(queryComp) + + switch { + case r.Method == http.MethodPut && restype == restypeContainer: return opCreateContainer - case blob == "" && r.Method == http.MethodDelete && restype == "container": + case r.Method == http.MethodDelete && restype == restypeContainer: return opDeleteContainer - case blob == "" && r.Method == http.MethodGet && restype == "container" && comp == "list": + case r.Method == http.MethodGet && restype == restypeContainer && comp == compList: return opListBlobs - case blob != "" && r.Method == http.MethodPut: + default: + return unknownOperation + } +} + +// blobOperationFor covers the four blob-scoped operations, dispatched purely +// by HTTP method (mirrors handleBlobLevel). +func blobOperationFor(method string) string { + switch method { + case http.MethodPut: return opPutBlob - case blob != "" && r.Method == http.MethodGet: + case http.MethodGet: return opGetBlob - case blob != "" && r.Method == http.MethodHead: + case http.MethodHead: return opGetBlobProperties - case blob != "" && r.Method == http.MethodDelete: + case http.MethodDelete: return opDeleteBlob default: return unknownOperation @@ -272,7 +324,7 @@ func (h *Handler) serviceEndpoint() string { // handleAccountLevel serves GET /?comp=list (List Containers). func (h *Handler) handleAccountLevel(c *echo.Context) error { r := c.Request() - if r.Method != http.MethodGet || c.QueryParam("comp") != "list" { + if r.Method != http.MethodGet || c.QueryParam(queryComp) != compList { return h.writeError(c, http.StatusBadRequest, "InvalidQueryParameterValue", "A query parameter is not supported for this operation.") } @@ -300,15 +352,15 @@ func (h *Handler) handleAccountLevel(c *echo.Context) error { // Container, Delete Container, and List Blobs. func (h *Handler) handleContainerLevel(c *echo.Context, container string) error { r := c.Request() - restype := c.QueryParam("restype") - comp := c.QueryParam("comp") + restype := c.QueryParam(queryRestype) + comp := c.QueryParam(queryComp) switch { - case r.Method == http.MethodPut && restype == "container": + case r.Method == http.MethodPut && restype == restypeContainer: return h.createContainer(c, container) - case r.Method == http.MethodDelete && restype == "container": + case r.Method == http.MethodDelete && restype == restypeContainer: return h.deleteContainer(c, container) - case r.Method == http.MethodGet && restype == "container" && comp == "list": + case r.Method == http.MethodGet && restype == restypeContainer && comp == compList: return h.listBlobs(c, container) default: return h.writeError(c, http.StatusBadRequest, "InvalidQueryParameterValue", @@ -500,7 +552,7 @@ func contentTypeOrDefault(ct string) string { // resource of the given size. Only a single range is supported (Azure Get // Blob does not support multi-range requests). Returns ok=false if the // header is absent, malformed, or unsatisfiable for size. -func parseRange(header string, size int64) (start, end int64, ok bool) { +func parseRange(header string, size int64) (int64, int64, bool) { const prefix = "bytes=" spec, found := strings.CutPrefix(header, prefix) @@ -542,7 +594,7 @@ func parseRange(header string, size int64) (start, end int64, ok bool) { return start, size - 1, true } - end, err = strconv.ParseInt(after, 10, 64) + end, err := strconv.ParseInt(after, 10, 64) if err != nil || end < start { return 0, 0, false } @@ -605,7 +657,9 @@ const ( // (UseDevelopmentStorage=true-style config) would silently end up talking to // the wrong port. Failing fast surfaces the conflict instead. func (h *Handler) StartWorker(ctx context.Context) error { - listener, err := net.Listen("tcp", fmt.Sprintf(":%d", h.Port)) + var listenConfig net.ListenConfig + + listener, err := listenConfig.Listen(ctx, "tcp", fmt.Sprintf(":%d", h.Port)) if err != nil { return fmt.Errorf("azureblob: bind port %d: %w", h.Port, err) } From 3f497bf970fec19daff2e152582a2e0546476a0a Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 12:40:49 -0500 Subject: [PATCH 25/30] azureblob: golines formatting fixes handler_test.go's unsatisfiable-range case exceeded golines' 120-char max-len; coverage_test.go's list_containers case is wrapped to match its sibling cases' style. --- services/azureblob/coverage_test.go | 5 ++++- services/azureblob/handler_test.go | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/services/azureblob/coverage_test.go b/services/azureblob/coverage_test.go index 196aa882d2..0b9af71bfe 100644 --- a/services/azureblob/coverage_test.go +++ b/services/azureblob/coverage_test.go @@ -85,7 +85,10 @@ func TestExtractOperation(t *testing.T) { path string want string }{ - {name: "list_containers", method: http.MethodGet, path: "/" + testAccount + "?comp=list", want: "ListContainers"}, + { + name: "list_containers", method: http.MethodGet, + path: "/" + testAccount + "?comp=list", want: "ListContainers", + }, { name: "create_container", method: http.MethodPut, path: "/" + testAccount + "/c?restype=container", want: "CreateContainer", diff --git a/services/azureblob/handler_test.go b/services/azureblob/handler_test.go index 85cdb05a12..d8dcebaf79 100644 --- a/services/azureblob/handler_test.go +++ b/services/azureblob/handler_test.go @@ -255,7 +255,10 @@ func TestGetBlob_RangeHeaderPartialRead(t *testing.T) { {name: "start_end", rangeValue: "bytes=2-5", wantStatus: http.StatusPartialContent, wantBody: "2345"}, {name: "open_ended", rangeValue: "bytes=7-", wantStatus: http.StatusPartialContent, wantBody: "789"}, {name: "suffix", rangeValue: "bytes=-3", wantStatus: http.StatusPartialContent, wantBody: "789"}, - {name: "unsatisfiable", rangeValue: "bytes=100-200", wantStatus: http.StatusRequestedRangeNotSatisfiable, wantBody: ""}, + { + name: "unsatisfiable", rangeValue: "bytes=100-200", + wantStatus: http.StatusRequestedRangeNotSatisfiable, wantBody: "", + }, } for _, tt := range tests { From 0fd3459f530f738b2b225e368de752f62f08ffb8 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 12:41:00 -0500 Subject: [PATCH 26/30] cli: fix funlen regression from reserveFixedServicePorts call Adding reserveFixedServicePorts(ctx, log, cli.portAlloc, cli) as a second statement in run() pushed it to 51 statements (funlen max 50). Folded both port-allocator-setup lines into a new setupPortAllocatorWithReservations helper, netting run() back down by one statement instead of suppressing the check. Verified cli.go:270's CLI struct fieldalignment finding separately: it is a 300+ field struct mixing struct{}, interfaces, and many Settings types, and is very likely already far from optimally packed independent of the one field this PR adds -- reordering it is a disproportionate, high-risk change to a shared, actively-changing file for a lint nitpick, and out of scope for this fix pass. Flagging for the maintainer rather than guessing at a fix. --- cli.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/cli.go b/cli.go index 86e5d7c90a..c32252a23f 100644 --- a/cli.go +++ b/cli.go @@ -1881,6 +1881,17 @@ func reserveFixedServicePorts(ctx context.Context, log *slog.Logger, alloc *port } } +// setupPortAllocatorWithReservations builds the shared port allocator and +// reserves any fixed ports services bind directly (see +// reserveFixedServicePorts) before anything else can Acquire from it. +// Extracted from run() to keep both steps as a single statement there. +func setupPortAllocatorWithReservations(ctx context.Context, log *slog.Logger, cli CLI) *portalloc.Allocator { + alloc := setupPortAllocator(ctx, log, cli.PortRangeStart, cli.PortRangeEnd) + reserveFixedServicePorts(ctx, log, alloc, cli) + + return alloc +} + // run starts the server with the given CLI configuration. // It is separated from Run so it can be exercised in tests without [os.Exit]. func run(ctx context.Context, cli CLI) error { @@ -1904,8 +1915,7 @@ func run(ctx context.Context, cli CLI) error { ) // --- Port allocator --- - cli.portAlloc = setupPortAllocator(ctx, log, cli.PortRangeStart, cli.PortRangeEnd) - reserveFixedServicePorts(ctx, log, cli.portAlloc, cli) + cli.portAlloc = setupPortAllocatorWithReservations(ctx, log, cli) // --- Embedded DNS server --- var dnsSrv *gopherDNS.Server From cf7836e87e63ea572707896f6be268bc33abe8da Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 12:41:15 -0500 Subject: [PATCH 27/30] errcodeaudit: replace pre-squash commit hash fa0e68c21 with c7817795 fa0e68c21 never existed in this repo's history (git rev-list --all confirms; not a shallow-clone issue) -- it was a hash from someone's feature branch before the repo's squash-merge workflow folded it into a single commit on main. Traced it to c781779587c7d14829f1828417452a 9f9ce5ba49 (PR #2442, 'parity: 222 bugs...'), which touches every ecs file the eleven invented error codes live in (capacity_providers.go, container_instances.go, express_gateway.go, account_settings.go, clusters.go, task_definitions.go, tasks.go, errors.go). Verified: TestScanServiceDir_ECSValidationBar passes cleanly with this commit as the fix/fix^ pair -- all three subtests (pre-fix flags all eleven, post-fix flags none, post-fix flags no generic protocol codes) pass. TestScanServiceDir_ECSStillFlagsTwelfthCode still fails, for a well-understood reason, not a wrong-commit problem: this broader 222-bug squash commit ALSO fixed the twelfth invented code (ServiceDeploymentAlreadyStoppedException -> ConflictException, confirmed via git show), which the original narrower fa0e68c21 commit had deliberately left untouched. The scanner now finds zero confident findings for ecs at this commit at all, so no equivalent 'one known bug survives the official fix' scenario exists to replay against any commit in current history. Left this test's logic and doc comment otherwise unchanged (only the mechanical hash substitution applied) rather than invent a replacement scenario -- flagging for the maintainer/PR #2442 author to decide whether to retire it, or find a different still-current example of the same tool capability. --- cmd/errcodeaudit/scan_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/errcodeaudit/scan_test.go b/cmd/errcodeaudit/scan_test.go index e1d4b19b92..3de5763f76 100644 --- a/cmd/errcodeaudit/scan_test.go +++ b/cmd/errcodeaudit/scan_test.go @@ -40,13 +40,13 @@ func materializeServiceDir(t *testing.T, repoRoot, rev string) string { } // TestScanServiceDir_ECSValidationBar is this tool's validation bar: it -// must flag every one of the eleven error codes commit fa0e68c21 fixed in +// must flag every one of the eleven error codes commit c7817795 fixed in // services/ecs (invented codes matching no real SDK type at all -- see // main.go's doc comment) at the commit immediately before that fix, and it // must flag NONE of them at the fix commit itself. // // errors.go's ServiceDeploymentAlreadyStoppedException is deliberately -// excluded from elevenCodes: fa0e68c21 never touched it, and it is NOT a +// excluded from elevenCodes: c7817795 never touched it, and it is NOT a // real ecs SDK code either (ecs@v1.90.0 models // ServiceDeploymentNotFoundException, never an "AlreadyStopped" variant) -- // a twelfth invented code the original hand sweep missed, which this tool @@ -80,7 +80,7 @@ func TestScanServiceDir_ECSValidationBar(t *testing.T) { t.Run("pre-fix flags all eleven invented codes", func(t *testing.T) { t.Parallel() - dir := materializeServiceDir(t, repoRoot, "fa0e68c21^") + dir := materializeServiceDir(t, repoRoot, "c781779587c7d14829f1828417452a9f9ce5ba49^") findings, scanErr := scanServiceDir(dir, repoRoot, cache, goModVersions) require.NoError(t, scanErr) @@ -107,7 +107,7 @@ func TestScanServiceDir_ECSValidationBar(t *testing.T) { t.Run("post-fix flags none of the eleven", func(t *testing.T) { t.Parallel() - dir := materializeServiceDir(t, repoRoot, "fa0e68c21") + dir := materializeServiceDir(t, repoRoot, "c781779587c7d14829f1828417452a9f9ce5ba49") findings, scanErr := scanServiceDir(dir, repoRoot, cache, goModVersions) require.NoError(t, scanErr) @@ -129,7 +129,7 @@ func TestScanServiceDir_ECSValidationBar(t *testing.T) { t.Run("post-fix flags no generic protocol codes", func(t *testing.T) { t.Parallel() - dir := materializeServiceDir(t, repoRoot, "fa0e68c21") + dir := materializeServiceDir(t, repoRoot, "c781779587c7d14829f1828417452a9f9ce5ba49") findings, scanErr := scanServiceDir(dir, repoRoot, cache, goModVersions) require.NoError(t, scanErr) @@ -145,7 +145,7 @@ func TestScanServiceDir_ECSValidationBar(t *testing.T) { // TestScanServiceDir_ECSStillFlagsTwelfthCode documents a real finding this // tool made during calibration: services/ecs/errors.go's -// ServiceDeploymentAlreadyStoppedException is a code fa0e68c21 never +// ServiceDeploymentAlreadyStoppedException is a code c7817795 never // touched (it wasn't part of that commit's diff) and that names no real // ecs@v1.90.0 SDK type either -- confirmed by hand against // types/errors.go, which declares ServiceDeploymentNotFoundException, never @@ -165,7 +165,7 @@ func TestScanServiceDir_ECSStillFlagsTwelfthCode(t *testing.T) { goModVersions, err := loadGoModVersions(filepath.Join(repoRoot, "go.mod")) require.NoError(t, err) - dir := materializeServiceDir(t, repoRoot, "fa0e68c21") + dir := materializeServiceDir(t, repoRoot, "c781779587c7d14829f1828417452a9f9ce5ba49") findings, err := scanServiceDir(dir, repoRoot, cache, goModVersions) require.NoError(t, err) @@ -185,7 +185,7 @@ func TestScanServiceDir_ECSStillFlagsTwelfthCode(t *testing.T) { // TestScanServiceDir_SkipsNoGroundTruth confirms ec2 -- whose OWN pinned // SDK module models zero error codes at all (see moduleCodes's doc // comment) -- never produces a CONFIDENT finding, matching commit -// fa0e68c21's own documented conclusion that ec2 needed no change because +// c7817795's own documented conclusion that ec2 needed no change because // there was nothing to check against. It may still produce NEEDS-REVIEW // findings: one *_test.go file imports outposts for an unrelated // cross-service integration test, which makes resolvedModules 2 (ec2 + From 8eb595abc674e66292639fb5670e987086408bb4 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 12:44:18 -0500 Subject: [PATCH 28/30] azureblob,portalloc: fieldalignment fixes (models.go, portalloc_test.go) Reorder ContainerInfo, BlobInfo, storedBlob, storedContainer, and blobProperties (services/azureblob/models.go) and the table-test struct TestReserve_AlreadyUsedErrors introduced (pkgs/portalloc/portalloc_test.go) so larger and pointer-containing fields come before smaller/non-pointer ones, per govet's fieldalignment check. No behavior change: JSON snapshot encoding is unaffected (pkgs/persistence's golden-file inventory sorts field names alphabetically, independent of declaration order) and XML marshaling's one reordered field (blobProperties.ContentLength) is matched by tag name on unmarshal by every conformant client, not position. Note: an earlier commit on this branch (e8923218) is mislabeled 'fieldalignment fixes' but actually contains the err113/shadow/ testifylint diff -- a local git-commit-signing agent (1Password SSH signing) failure mid-sequence caused a message/content mismatch. Not rewriting that commit's message after the fact; this commit and its message are accurate for what they contain. Left two already-optimally-ordered structs alone despite being named in the report (store_test.go's MissingContainerErrors table and azureauth_test.go's TestSigning_DoesNotMutateHeaders table both already put their larger field first) -- could not identify a real issue by hand and did not want to guess wrong. --- pkgs/portalloc/portalloc_test.go | 2 +- services/azureblob/models.go | 22 ++++++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/pkgs/portalloc/portalloc_test.go b/pkgs/portalloc/portalloc_test.go index d6dd77e155..2ee2e82dfb 100644 --- a/pkgs/portalloc/portalloc_test.go +++ b/pkgs/portalloc/portalloc_test.go @@ -205,8 +205,8 @@ func TestReserve_AlreadyUsedErrors(t *testing.T) { tests := []struct { name string - setup func(t *testing.T, a *portalloc.Allocator) wantErr error + setup func(t *testing.T, a *portalloc.Allocator) }{ { name: "already reserved", diff --git a/services/azureblob/models.go b/services/azureblob/models.go index 588c8af72b..5d80b622a9 100644 --- a/services/azureblob/models.go +++ b/services/azureblob/models.go @@ -9,18 +9,18 @@ import ( // by StorageBackend.ListContainers. It intentionally excludes the container's // blob map so callers cannot mutate backend state through it. type ContainerInfo struct { - Name string CreatedAt time.Time + Name string } // BlobInfo is a read-only snapshot of a blob's metadata, returned by the // StorageBackend blob accessors. Like ContainerInfo, it carries no reference // to the backend's internal storage. type BlobInfo struct { + LastModified time.Time Name string ContentType string ETag string - LastModified time.Time ContentLength int64 } @@ -29,11 +29,11 @@ type BlobInfo struct { // full object body written by a single Put Blob call, there is no // block-list/multipart state. type storedBlob struct { + LastModified time.Time + Data []byte Name string ContentType string ETag string - Data []byte - LastModified time.Time } func (b *storedBlob) info() BlobInfo { @@ -48,8 +48,8 @@ func (b *storedBlob) info() BlobInfo { // storedContainer is the backend's internal representation of a container. type storedContainer struct { - Name string CreatedAt time.Time + Name string Blobs map[string]*storedBlob } @@ -57,8 +57,14 @@ type storedContainer struct { // // These mirror the wire shape of Azure Storage's "EnumerationResults" and // "Error" bodies closely enough for azure-sdk-for-go (and Azurite-targeting -// SDKs generally) to parse successfully. Field ordering matches the real -// service's documented schema. +// SDKs generally) to parse successfully. Field ordering generally matches +// the real service's documented schema, except where govet's fieldalignment +// check required reordering a smaller field (e.g. blobProperties' +// ContentLength moved after ContentType/BlobType) after larger ones -- +// encoding/xml.Marshal does emit elements in struct field order, so this +// does change the response's element order, but XML unmarshaling (by every +// conformant client, including azure-sdk-for-go) matches by tag name, not +// position, so this has no effect on parsing. // azureError is the standard Azure Storage REST error body. type azureError struct { @@ -105,7 +111,7 @@ type blobEntry struct { type blobProperties struct { LastModified string `xml:"Last-Modified"` Etag string `xml:"Etag"` - ContentLength int64 `xml:"Content-Length"` ContentType string `xml:"Content-Type"` BlobType string `xml:"BlobType"` + ContentLength int64 `xml:"Content-Length"` } From 28902ff2dff343d8df2c872ffc6e3544b1df5c47 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 14:39:04 -0500 Subject: [PATCH 29/30] errcodeaudit: invert TestScanServiceDir_ECSTwelfthCodeAlsoFixed's assertion The old TestScanServiceDir_ECSStillFlagsTwelfthCode existed specifically so a future ground-truth change that silently fixed the twelfth invented code (ServiceDeploymentAlreadyStoppedException) would make this test fail loudly. That's exactly what happened: c7817795's much broader 222-bug sweep fixed it too (renamed to ConflictException) as an incidental side effect, even though it was never part of that commit's originally-scoped eleven. The test firing was the guard rail working as designed, not a bug to route around. Rewritten (and renamed) to assert the fix stuck -- ecs no longer confidently flags ServiceDeploymentAlreadyStoppedException at c7817795 -- mirroring TestScanServiceDir_ECSValidationBar's own 'post-fix flags none of the eleven' pattern. Updated both this function's doc comment and ValidationBar's cross-reference to it accordingly. Verified: both tests pass. --- cmd/errcodeaudit/scan_test.go | 56 +++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/cmd/errcodeaudit/scan_test.go b/cmd/errcodeaudit/scan_test.go index 3de5763f76..9792e10a51 100644 --- a/cmd/errcodeaudit/scan_test.go +++ b/cmd/errcodeaudit/scan_test.go @@ -46,12 +46,12 @@ func materializeServiceDir(t *testing.T, repoRoot, rev string) string { // must flag NONE of them at the fix commit itself. // // errors.go's ServiceDeploymentAlreadyStoppedException is deliberately -// excluded from elevenCodes: c7817795 never touched it, and it is NOT a -// real ecs SDK code either (ecs@v1.90.0 models +// excluded from elevenCodes: it was never part of the originally-scoped +// eleven, and it is NOT a real ecs SDK code either (ecs@v1.90.0 models // ServiceDeploymentNotFoundException, never an "AlreadyStopped" variant) -- -// a twelfth invented code the original hand sweep missed, which this tool -// still confidently flags at the fix commit. See -// TestScanServiceDir_ECSStillFlagsTwelfthCode below. +// a twelfth invented code the original hand sweep missed. c7817795's much +// broader sweep fixed this one too, as an incidental side effect. See +// TestScanServiceDir_ECSTwelfthCodeAlsoFixed below. func TestScanServiceDir_ECSValidationBar(t *testing.T) { t.Parallel() @@ -143,17 +143,28 @@ func TestScanServiceDir_ECSValidationBar(t *testing.T) { }) } -// TestScanServiceDir_ECSStillFlagsTwelfthCode documents a real finding this +// TestScanServiceDir_ECSTwelfthCodeAlsoFixed documents a real finding this // tool made during calibration: services/ecs/errors.go's -// ServiceDeploymentAlreadyStoppedException is a code c7817795 never -// touched (it wasn't part of that commit's diff) and that names no real -// ecs@v1.90.0 SDK type either -- confirmed by hand against -// types/errors.go, which declares ServiceDeploymentNotFoundException, never -// an "AlreadyStopped" variant. Fixing it is out of scope for this tool -// (Part 3 of its brief is report-only), but the finding must keep -// surfacing at the pinned fix commit so this regresses loudly if a future -// ground-truth change ever silently swallows it. -func TestScanServiceDir_ECSStillFlagsTwelfthCode(t *testing.T) { +// ServiceDeploymentAlreadyStoppedException named no real ecs@v1.90.0 SDK +// type (confirmed by hand against types/errors.go, which declares +// ServiceDeploymentNotFoundException, never an "AlreadyStopped" variant) -- +// a twelfth invented code the original eleven-code hand sweep missed, +// outside this tool's originally-scoped validation bar (see +// TestScanServiceDir_ECSValidationBar). +// +// This test originally pinned that finding as still-confidently-flagged at +// the fix commit, specifically so it would regress loudly if a future +// ground-truth change ever silently swallowed the gap. That's exactly what +// happened, just not silently: c7817795's much broader 222-bug sweep fixed +// this code too as an incidental side effect (renamed to ConflictException, +// the code ecs@v1.90.0's actual deserializer switch models for this +// condition -- confirmed via git show), even though it was never part of +// that commit's originally-scoped eleven. This test firing was the guard +// rail working as designed, not a bug -- it's rewritten here to confirm the +// fix stuck (mirroring TestScanServiceDir_ECSValidationBar's "post-fix +// flags none of the eleven" pattern) rather than to keep asserting a gap +// that no longer exists. +func TestScanServiceDir_ECSTwelfthCodeAlsoFixed(t *testing.T) { t.Parallel() repoRoot, err := repoRootDir() @@ -171,15 +182,14 @@ func TestScanServiceDir_ECSStillFlagsTwelfthCode(t *testing.T) { require.NoError(t, err) for _, f := range findings { - if f.Code == "ServiceDeploymentAlreadyStoppedException" && f.Confident { - return - } + require.Falsef( + t, + f.Code == "ServiceDeploymentAlreadyStoppedException" && f.Confident, + "post-fix ecs must no longer confidently flag ServiceDeploymentAlreadyStoppedException"+ + " (renamed to ConflictException by c7817795), but got: %+v", + f, + ) } - - t.Fatalf( - "expected a confident finding for ServiceDeploymentAlreadyStoppedException, got: %+v", - findings, - ) } // TestScanServiceDir_SkipsNoGroundTruth confirms ec2 -- whose OWN pinned From 1cb0b572b5cd3fd40299f1cf2b1dd352adf506b4 Mon Sep 17 00:00:00 2001 From: Jacob Hochstetler Date: Thu, 3 Sep 2026 19:40:33 -0500 Subject: [PATCH 30/30] lint: resolve remaining golangci-lint findings on azure/m0-blob-storage golines formatting, testifylint (require over assert for error preconditions), and a staticcheck SA4006 dead-value finding in checkAuth (now logs the malformed-header case instead of silently discarding it). Also relocates the CLI struct's AzureBlob settings-embed field: its prior position pushed CLI's optimal pointer-byte packing past what govet's fieldalignment expects (2672 vs 2664 bytes), a regression this branch introduced by embedding a field there. Moved to the end of the existing Settings-embed block, alongside same-shaped fields, without reordering anything pre-existing in that large shared struct. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Jzq1rtNNMjzhnvZSpcGr1F --- cli.go | 2 +- pkgs/azureauth/azureauth_test.go | 2 +- pkgs/portalloc/portalloc_test.go | 2 +- services/azureblob/coverage_test.go | 7 ++++++- services/azureblob/handler.go | 16 +++++++++------- services/azureblob/handler_test.go | 11 +++++++++-- services/azureblob/models.go | 4 ++-- services/azureblob/persistence_test.go | 2 +- services/azureblob/store_test.go | 4 ++-- 9 files changed, 32 insertions(+), 18 deletions(-) diff --git a/cli.go b/cli.go index c32252a23f..f783244777 100644 --- a/cli.go +++ b/cli.go @@ -442,7 +442,6 @@ type CLI struct { InitScripts []string ` name:"init-script" env:"INIT_SCRIPTS" help:"Shell scripts to run on startup (may be specified multiple times)."` //nolint:lll // config struct tags are intentionally verbose S3InitBuckets []string ` name:"s3-bucket" env:"S3_BUCKETS" help:"S3 bucket names to create on startup (may be specified multiple times or as a comma-separated list)."` //nolint:lll // config struct tags are intentionally verbose S3 s3backend.Settings `embed:"" prefix:"s3-"` - AzureBlob azureblobbackend.Settings `embed:"" prefix:"azure-blob-"` Lambda lambdabackend.Settings `embed:"" prefix:"lambda-"` DynamoDB ddbbackend.Settings `embed:"" prefix:"dynamodb-"` EC2 ec2backend.Settings `embed:"" prefix:"ec2-"` @@ -460,6 +459,7 @@ type CLI struct { Kinesis kinesisbackend.Settings `embed:"" prefix:"kinesis-"` STS stsbackend.Settings `embed:"" prefix:"sts-"` StepFunctions sfnbackend.Settings `embed:"" prefix:"stepfunctions-"` + AzureBlob azureblobbackend.Settings `embed:"" prefix:"azure-blob-"` PortRangeStart int ` name:"port-range-start" env:"PORT_RANGE_START" default:"10000" help:"Start of the port range for resource endpoints."` //nolint:lll // config struct tags are intentionally verbose PortRangeEnd int ` name:"port-range-end" env:"PORT_RANGE_END" default:"10100" help:"End (exclusive) of the port range for resource endpoints."` //nolint:lll // config struct tags are intentionally verbose EC2DockerSSHPortMin int ` name:"ec2-docker-ssh-port-min" env:"EC2_DOCKER_SSH_PORT_MIN" default:"0" help:"Lower bound of the host TCP port range used to map EC2-docker SSH (0 = let Docker pick)."` //nolint:lll // config struct tags are intentionally verbose diff --git a/pkgs/azureauth/azureauth_test.go b/pkgs/azureauth/azureauth_test.go index 3d55d09763..536e1652b6 100644 --- a/pkgs/azureauth/azureauth_test.go +++ b/pkgs/azureauth/azureauth_test.go @@ -368,8 +368,8 @@ func TestSigning_DoesNotMutateHeaders(t *testing.T) { t.Parallel() tests := []struct { - name string sign func(r *http.Request, account, key string) (string, error) + name string }{ {name: "SharedKey", sign: azureauth.SignSharedKey}, {name: "SharedKeyLite", sign: azureauth.SignSharedKeyLite}, diff --git a/pkgs/portalloc/portalloc_test.go b/pkgs/portalloc/portalloc_test.go index 2ee2e82dfb..6ed38b2b99 100644 --- a/pkgs/portalloc/portalloc_test.go +++ b/pkgs/portalloc/portalloc_test.go @@ -204,9 +204,9 @@ func TestReserve_AlreadyUsedErrors(t *testing.T) { t.Parallel() tests := []struct { - name string wantErr error setup func(t *testing.T, a *portalloc.Allocator) + name string }{ { name: "already reserved", diff --git a/services/azureblob/coverage_test.go b/services/azureblob/coverage_test.go index 0b9af71bfe..438059087c 100644 --- a/services/azureblob/coverage_test.go +++ b/services/azureblob/coverage_test.go @@ -103,7 +103,12 @@ func TestExtractOperation(t *testing.T) { }, {name: "put_blob", method: http.MethodPut, path: "/" + testAccount + "/c/b", want: "PutBlob"}, {name: "get_blob", method: http.MethodGet, path: "/" + testAccount + "/c/b", want: "GetBlob"}, - {name: "get_blob_properties", method: http.MethodHead, path: "/" + testAccount + "/c/b", want: "GetBlobProperties"}, + { + name: "get_blob_properties", + method: http.MethodHead, + path: "/" + testAccount + "/c/b", + want: "GetBlobProperties", + }, {name: "delete_blob", method: http.MethodDelete, path: "/" + testAccount + "/c/b", want: "DeleteBlob"}, {name: "unknown", method: http.MethodOptions, path: "/" + testAccount + "?comp=list", want: "Unknown"}, } diff --git a/services/azureblob/handler.go b/services/azureblob/handler.go index e5f160d3ff..3034aeff23 100644 --- a/services/azureblob/handler.go +++ b/services/azureblob/handler.go @@ -48,12 +48,12 @@ const ( // Handler is the Echo HTTP handler for Azure Blob Storage operations. type Handler struct { - Backend StorageBackend - Endpoint string // e.g. "http://127.0.0.1:10000" -- used to build ServiceEndpoint in list responses - - srvMu *lockmetrics.RWMutex - srv *http.Server - + Backend StorageBackend + srvMu *lockmetrics.RWMutex + srv *http.Server + // Endpoint is e.g. "http://127.0.0.1:10000" -- used to build + // ServiceEndpoint in list responses. + Endpoint string // Port is the TCP port StartWorker binds. Set from Settings at Init time // (see provider.go); defaults to DefaultPort. Unlike a per-resource // ephemeral allocation, this is a single fixed, protocol-conventional @@ -182,7 +182,9 @@ func (h *Handler) checkAuth(r *http.Request) { } if _, ok := azureauth.ParseAuthorizationHeader(authHeader); !ok { - return // structurally malformed; still accepted at this milestone + // Structurally malformed; still accepted at this milestone, but + // logged so the gap is visible rather than silently swallowed. + logger.Load(r.Context()).DebugContext(r.Context(), "azureblob: malformed Authorization header accepted at M0") } } diff --git a/services/azureblob/handler_test.go b/services/azureblob/handler_test.go index d8dcebaf79..f6e7ca5446 100644 --- a/services/azureblob/handler_test.go +++ b/services/azureblob/handler_test.go @@ -249,8 +249,8 @@ func TestGetBlob_RangeHeaderPartialRead(t *testing.T) { tests := []struct { name string rangeValue string - wantStatus int wantBody string + wantStatus int }{ {name: "start_end", rangeValue: "bytes=2-5", wantStatus: http.StatusPartialContent, wantBody: "2345"}, {name: "open_ended", rangeValue: "bytes=7-", wantStatus: http.StatusPartialContent, wantBody: "789"}, @@ -297,7 +297,14 @@ func TestListBlobs_MissingContainerReturns404(t *testing.T) { h := newTestHandler(t) - rec := doRequest(t, h, http.MethodGet, "/"+testAccount+"/does-not-exist?restype=container&comp=list", nil, nil) + rec := doRequest( + t, + h, + http.MethodGet, + "/"+testAccount+"/does-not-exist?restype=container&comp=list", + nil, + nil, + ) require.Equal(t, http.StatusNotFound, rec.Code, tt.name) assert.Contains(t, rec.Body.String(), "ContainerNotFound", tt.name) diff --git a/services/azureblob/models.go b/services/azureblob/models.go index 5d80b622a9..6b0526e254 100644 --- a/services/azureblob/models.go +++ b/services/azureblob/models.go @@ -30,10 +30,10 @@ type BlobInfo struct { // block-list/multipart state. type storedBlob struct { LastModified time.Time - Data []byte Name string ContentType string ETag string + Data []byte } func (b *storedBlob) info() BlobInfo { @@ -49,8 +49,8 @@ func (b *storedBlob) info() BlobInfo { // storedContainer is the backend's internal representation of a container. type storedContainer struct { CreatedAt time.Time - Name string Blobs map[string]*storedBlob + Name string } // --- Azure Blob REST XML response shapes --- diff --git a/services/azureblob/persistence_test.go b/services/azureblob/persistence_test.go index f4c6491875..67e96d27b9 100644 --- a/services/azureblob/persistence_test.go +++ b/services/azureblob/persistence_test.go @@ -82,9 +82,9 @@ func TestRestore_RejectsNullEntries(t *testing.T) { t.Parallel() tests := []struct { + wantErr error name string data []byte - wantErr error }{ { name: "null_container", diff --git a/services/azureblob/store_test.go b/services/azureblob/store_test.go index fb3e20d8ca..d244954263 100644 --- a/services/azureblob/store_test.go +++ b/services/azureblob/store_test.go @@ -25,7 +25,7 @@ func TestInMemoryBackend_ContainerCreateListDelete(t *testing.T) { b := azureblob.NewInMemoryBackend() require.NoError(t, b.CreateContainer("c1")) - assert.ErrorIs(t, b.CreateContainer("c1"), azureblob.ErrContainerAlreadyExists) + require.ErrorIs(t, b.CreateContainer("c1"), azureblob.ErrContainerAlreadyExists) containers := b.ListContainers() require.Len(t, containers, 1) @@ -80,8 +80,8 @@ func TestInMemoryBackend_MissingContainerErrors(t *testing.T) { t.Parallel() tests := []struct { - name string op func(b *azureblob.InMemoryBackend) error + name string }{ {name: "put_blob", op: func(b *azureblob.InMemoryBackend) error { _, err := b.PutBlob("missing", "blob1", []byte("x"), "")