diff --git a/AGENTS.md b/AGENTS.md index 80b362c..96c1230 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,10 +51,11 @@ and `sprue` (the upload service; mirror its patterns where relevant). - Build / vet / test: `go build ./... && go vet ./... && go test ./...`. Run all three after changes — this is the standard loop. -- Run locally: `go run ./cmd serve --revocation-service-id= - --revocation-service-url=` (both required, no default; add - `--storage=memory --vault=memory` to avoid external deps; see `cmd/main.go` / - `pkg/config`). +- Run locally: `go run ./cmd serve --upload-service-id= + --upload-service-url= --upload-product-id= + --revocation-service-id= --revocation-service-url=` (all required, + no default; add `--storage=memory --vault=memory` to avoid external deps; see + `cmd/main.go` / `pkg/config`). - Postgres and OpenBao-backed tests use testcontainers and **skip when Docker is unavailable** (`internal/testutil`). `go test ./...` passes without Docker but only exercises the memory backends; run with Docker for full coverage. diff --git a/README.md b/README.md index be150b1..d947e84 100644 --- a/README.md +++ b/README.md @@ -102,21 +102,23 @@ Pre-shared bearer token required on Tenant API requests. ### Upload service (Sprue) -| Key | Flag | Env var | Default | +| Key | Flag | Env var | Example | | --- | --- | --- | --- | | `upload.service_id` | `--upload-service-id` | `HILT_UPLOAD_SERVICE_ID` | `did:web:upload.fil-forge.com` | | `upload.service_url` | `--upload-service-url` | `HILT_UPLOAD_SERVICE_URL` | `https://upload.fil-forge.com` | | `upload.product_id` | `--upload-product-id` | `HILT_UPLOAD_PRODUCT_ID` | `did:web:hilt.fil-forge.com` | The Sprue service DID + HTTP endpoint Hilt calls to provision bucket space, and -the product/plan DID tenants are registered under. +the product/plan DID tenants are registered under. All three are required, and +`hilt serve` exits at startup when any of them is unset. Each stage points at +its own Sprue. ### Revocation service (Swarf) -| Key | Flag | Env var | Default | +| Key | Flag | Env var | Example | | --- | --- | --- | --- | -| `revocation.service_id` | `--revocation-service-id` | `HILT_REVOCATION_SERVICE_ID` | required | -| `revocation.service_url` | `--revocation-service-url` | `HILT_REVOCATION_SERVICE_URL` | required | +| `revocation.service_id` | `--revocation-service-id` | `HILT_REVOCATION_SERVICE_ID` | `did:web:revoke.fil-forge.com` | +| `revocation.service_url` | `--revocation-service-url` | `HILT_REVOCATION_SERVICE_URL` | `https://revoke.fil-forge.com` | The Swarf service DID + HTTP endpoint Hilt publishes UCAN revocations to when a delegation it issued is withdrawn (e.g. when an access key is deleted). Both diff --git a/cmd/main.go b/cmd/main.go index 7f612c8..e59f85f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -63,9 +63,9 @@ func main() { serveCmd.Flags().String("partner-key", "", "CSV partner bearer key(s) required on Tenant API requests (prefer HILT_AUTH_PARTNER_KEY env var or config file to avoid exposing via process args)") // upload service config - serveCmd.Flags().String("upload-service-id", "did:web:upload.fil-forge.com", "Upload service DID") - serveCmd.Flags().String("upload-service-url", "https://upload.fil-forge.com", "Upload service HTTP endpoint") - serveCmd.Flags().String("upload-product-id", "did:web:hilt.fil-forge.com", "Upload service product/plan DID that tenants are registered under") + serveCmd.Flags().String("upload-service-id", "", "Upload service DID (required)") + serveCmd.Flags().String("upload-service-url", "", "Upload service HTTP endpoint (required)") + serveCmd.Flags().String("upload-product-id", "", "Upload service product/plan DID that tenants are registered under (required)") serveCmd.Flags().String("upload-proofs", "", "Upload service proofs: an encoded UCAN container or a path to a file containing one") // revocation service config diff --git a/pkg/config/config.go b/pkg/config/config.go index 6ae6974..b1abd37 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -52,7 +52,8 @@ type RevocationConfig struct { } // UploadConfig holds settings for the Sprue upload service, which Hilt calls to -// provision a bucket's storage space. +// provision a bucket's storage space. ServiceID, ServiceURL and ProductID are +// required by `hilt serve`. type UploadConfig struct { // ServiceID is the Sprue service's DID (e.g. "did:web:sprue.example.com"). ServiceID string `mapstructure:"service_id"` @@ -181,9 +182,9 @@ func SetDefaults(v *viper.Viper) { v.SetDefault("plc.directory", "https://plc.directory") - v.SetDefault("upload.service_id", "did:web:upload.fil-forge.com") - v.SetDefault("upload.service_url", "https://upload.fil-forge.com") - v.SetDefault("upload.product_id", "did:web:hilt.fil-forge.com") + // upload.service_id, upload.service_url and upload.product_id have no + // default: every deployment names its own stage's Sprue and product, and + // `serve` refuses to start without them (see fx.NewUploadClient). // revocation.service_id and revocation.service_url have no default: every // deployment names its own stage's Swarf, and `serve` refuses to start diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 29aa97e..757ccd6 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -26,6 +26,20 @@ func TestLoadEnvOnly(t *testing.T) { require.Equal(t, "https://revocation.example.com", cfg.Revocation.ServiceURL) } +// TestLoadUploadHasNoDefault verifies the upload service settings stay empty +// when nothing configures them, so `hilt serve` refuses to start instead of +// provisioning tenants and buckets on another stage's Sprue. +func TestLoadUploadHasNoDefault(t *testing.T) { + t.Setenv("HILT_UPLOAD_SERVICE_ID", "") + t.Setenv("HILT_UPLOAD_SERVICE_URL", "") + t.Setenv("HILT_UPLOAD_PRODUCT_ID", "") + t.Setenv("HILT_UPLOAD_PROOFS", "") + + cfg, err := config.Load("") + require.NoError(t, err) + require.Equal(t, config.UploadConfig{}, cfg.Upload) +} + // TestLoadRevocationHasNoDefault verifies the revocation service settings stay // empty when nothing configures them, so `hilt serve` refuses to start instead // of publishing revocations to another stage's Swarf. diff --git a/pkg/fx/rpc_test.go b/pkg/fx/rpc_test.go index c6ed173..f2332f9 100644 --- a/pkg/fx/rpc_test.go +++ b/pkg/fx/rpc_test.go @@ -45,7 +45,7 @@ func newRPCApp(t *testing.T) (routes []server.Route, admin []server.Route, srv * cfg := &config.Config{ Storage: config.StorageConfig{Type: config.StorageTypeMemory}, Vault: config.VaultConfig{Type: config.VaultTypeMemory}, - Upload: config.UploadConfig{ServiceID: testutil.RandomDID(t).String(), ServiceURL: "http://sprue.test"}, + Upload: config.UploadConfig{ServiceID: testutil.RandomDID(t).String(), ServiceURL: "http://sprue.test", ProductID: testutil.RandomDID(t).String()}, Revocation: config.RevocationConfig{ServiceID: testutil.RandomDID(t).String(), ServiceURL: "http://swarf.test"}, } app := fx.New( diff --git a/pkg/fx/upload.go b/pkg/fx/upload.go index c8ce6c1..2fb5d47 100644 --- a/pkg/fx/upload.go +++ b/pkg/fx/upload.go @@ -1,6 +1,7 @@ package fx import ( + "errors" "fmt" "net/url" "os" @@ -18,7 +19,20 @@ import ( // NewUploadClient builds the Sprue upload-service client from configuration. Its // proof store — the delegations it presents to Sprue — is loaded from // upload.proofs (see [uploadProofs]). +// +// The service DID, URL and product DID are required. None has a default, so a +// deployment that forgets one fails at startup rather than provisioning on +// another stage's Sprue. func NewUploadClient(id identity.Identity, cfg config.UploadConfig, logger *zap.Logger) (*upload.Client, error) { + if cfg.ServiceID == "" { + return nil, errors.New("upload.service_id is required: set HILT_UPLOAD_SERVICE_ID or --upload-service-id") + } + if cfg.ServiceURL == "" { + return nil, errors.New("upload.service_url is required: set HILT_UPLOAD_SERVICE_URL or --upload-service-url") + } + if cfg.ProductID == "" { + return nil, errors.New("upload.product_id is required: set HILT_UPLOAD_PRODUCT_ID or --upload-product-id") + } serviceID, err := did.Parse(cfg.ServiceID) if err != nil { return nil, fmt.Errorf("parsing upload.service_id %q: %w", cfg.ServiceID, err) @@ -27,15 +41,9 @@ func NewUploadClient(id identity.Identity, cfg config.UploadConfig, logger *zap. if err != nil { return nil, fmt.Errorf("parsing upload.service_url %q: %w", cfg.ServiceURL, err) } - // The product DID is optional here: it is only consumed when registering - // tenants as customers. Leave it undefined when unset so the client remains - // usable for the bucket-provisioning flows that do not need it. - var product did.DID - if cfg.ProductID != "" { - product, err = did.Parse(cfg.ProductID) - if err != nil { - return nil, fmt.Errorf("parsing upload.product_id %q: %w", cfg.ProductID, err) - } + product, err := did.Parse(cfg.ProductID) + if err != nil { + return nil, fmt.Errorf("parsing upload.product_id %q: %w", cfg.ProductID, err) } proofs, err := uploadProofs(cfg.Proofs) if err != nil { diff --git a/pkg/fx/upload_test.go b/pkg/fx/upload_test.go index c2ea411..25e91d4 100644 --- a/pkg/fx/upload_test.go +++ b/pkg/fx/upload_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "testing" + "github.com/fil-forge/hilt/pkg/client/upload" "github.com/fil-forge/hilt/pkg/config" appfx "github.com/fil-forge/hilt/pkg/fx" "github.com/fil-forge/libforge/testutil" @@ -12,6 +13,7 @@ import ( "github.com/fil-forge/ucantone/ucan/container" "github.com/fil-forge/ucantone/ucan/delegation" "github.com/stretchr/testify/require" + "go.uber.org/fx" "go.uber.org/zap" ) @@ -23,6 +25,7 @@ func TestNewUploadClient(t *testing.T) { return config.UploadConfig{ ServiceID: testutil.RandomDID(t).String(), ServiceURL: "http://sprue.test", + ProductID: testutil.RandomDID(t).String(), Proofs: proofs, } } @@ -66,8 +69,57 @@ func TestNewUploadClient(t *testing.T) { require.NotNil(t, c) }) + missingCases := map[string]struct { + unset func(*config.UploadConfig) + wantMessage string + }{ + "service DID is unset": { + unset: func(c *config.UploadConfig) { c.ServiceID = "" }, + wantMessage: "upload.service_id is required: set HILT_UPLOAD_SERVICE_ID or --upload-service-id", + }, + "service URL is unset": { + unset: func(c *config.UploadConfig) { c.ServiceURL = "" }, + wantMessage: "upload.service_url is required: set HILT_UPLOAD_SERVICE_URL or --upload-service-url", + }, + "product DID is unset": { + unset: func(c *config.UploadConfig) { c.ProductID = "" }, + wantMessage: "upload.product_id is required: set HILT_UPLOAD_PRODUCT_ID or --upload-product-id", + }, + } + for desc, tc := range missingCases { + t.Run("errors when "+desc, func(t *testing.T) { + cfg := baseCfg("") + tc.unset(&cfg) + _, err := appfx.NewUploadClient(id, cfg, zap.NewNop()) + require.EqualError(t, err, tc.wantMessage) + }) + } + + t.Run("invalid product DID errors", func(t *testing.T) { + cfg := baseCfg("") + cfg.ProductID = "not-a-did" + _, err := appfx.NewUploadClient(id, cfg, zap.NewNop()) + require.ErrorContains(t, err, "upload.product_id") + }) + t.Run("invalid proofs (neither container nor file) errors", func(t *testing.T) { _, err := appfx.NewUploadClient(id, baseCfg("not-a-container-and-not-a-file"), zap.NewNop()) require.Error(t, err) }) } + +// TestUploadClientRequiresConfig checks that an app built without the upload +// settings fails while it is being constructed, which is when `hilt serve` +// exits, rather than when it first calls Sprue. +func TestUploadClientRequiresConfig(t *testing.T) { + app := fx.New( + fx.Supply(&config.Config{}), + appfx.ConfigModule, + appfx.LoggerModule, + appfx.IdentityModule, + fx.Provide(appfx.NewUploadClient), + fx.NopLogger, + fx.Invoke(func(*upload.Client) {}), + ) + require.ErrorContains(t, app.Err(), "upload.service_id is required") +}