Skip to content

catalogpublisher: decentralized-catalog publisher plugin + catalog/publish handler - #893

Open
nirmay wants to merge 14 commits into
mainfrom
catalog-publisher
Open

catalogpublisher: decentralized-catalog publisher plugin + catalog/publish handler#893
nirmay wants to merge 14 commits into
mainfrom
catalog-publisher

Conversation

@nirmay

@nirmay nirmay commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • New catalogpublisher core plugin (pkg/plugin/implementation/catalogpublisher) implementing definition.CatalogPublisher: given a publisher's catalog submissions, produces a signed manifest and a catalog index matching the Decentralized Catalog file spec, with incremental diffing (baseline vs. change file vs. no-op) against caller-supplied prior state.
  • New catalogPublish HTTP handler (core/module/handler/catalogPublishHandler.go), a DS-internal, unsigned trigger mirroring catalogPullHandler's conventions -- no validateSign/addRoute/signAck pipeline, since the caller is the operator's own tooling.
  • Shared localstore package for reading/writing the manifest, catalog index, and catalog files to a local output root, used by both the handler and a demo CLI (cmd/catalogpublisherctl).
  • Request body matches beckn.yaml's real CatalogPublishAction envelope (context/message.catalogs[]/message.publishDirectives[]) rather than a synthesized wrapper, so schema validation runs against the real request.
  • Optional, reused-as-is plugin wiring for the handler: schemaValidator (schemav2validator) and policyChecker (opapolicychecker) validate/police the raw request; manifestLoader (manifestloader) checks read-only whether this node's manifest already declares its catalog index, staging a proposed update locally (never writing to the real manifest) and warning if not.
  • subscriberId is derived automatically from keyManager.config.subscriberId (no duplicate declaration); a distinct manifestSubscriberId config covers DeDi's namespace/registry/recordName identity format for the manifest self-lookup, kept separate since it's a different shape than the plain Beckn subscriberId used for signing.
  • config/local-beckn-one-bpp.yaml now also wires a catalogPublish module (it previously only existed in local-beckn-one-bap.yaml), matching how starter-kit's generic-bpp.yaml wires it -- schemaValidator/manifestLoader active by default, not just commented examples.

Fixes from self-review

  • A catalog submission (or change-file upsert list) containing two items with the same id no longer produces a duplicated upsert entry in the emitted baseline/change file or when reapplying one (catalogpublisher.go's itemsByIDOrdered, catalogfile.go's applyDiffBlock); last write wins, cleanly.
  • NewCatalogPublishHandler now loads manifestLoader before policyChecker and threads it through, so a manifest-backed policy type can actually resolve under this handler instead of always failing with "requires ManifestLoader plugin to be configured".
  • A PublishError.Fatal per-catalog error now flips the handler's overall response status to FAILED instead of being silently reported identically to a routine rejection.
  • Retired-catalog tombstone ordering in the published index is now deterministic (iterates the request's retire list instead of a Go map), and a catalogId that's both submitted and retired in the same call -- where Publish's own rule is "submission wins" -- no longer gets a spurious extra "retired" result in the HTTP response alongside its real outcome.
  • diffCatalogs/diffArrayField/diffCatalogAttributes now parse each submission's prior/next catalog JSON once instead of 3-6 times per submission.
  • Removed dead commented-out manifest-write code in localstore.Write; fixed a stale README cross-reference that only mentioned the bap config example, not the newer bpp one.

Test plan

  • go build ./... and go test ./... clean.
  • Exercised end-to-end against the starter-kit generic devkit (see companion PR generic-devkit: catalog/publish support for the generic BPP starter-kit#17): publish, incremental change file, retire, schema-validation rejection, manifest-link warning.
  • Added regression tests for the duplicate-id dedup fix, the manifestLoader/policyChecker threading fix, the Fatal-status wiring, and the submitted-and-retired-same-id response fix.
  • Reviewer sanity check of the plugin's signing/diffing logic and the handler's error/response shape.

🤖 Generated with Claude Code

nirmay added 10 commits July 29, 2026 14:47
… plus demo CLI

Implements definition.CatalogPublisher: given a publisher's catalog
submissions, produces a signed DeDi manifest and signed catalog index
whose wire shape matches exactly what catalogcrawler consumes -- the
producing side of the same three-level chain the crawler verifies.

Publish diffs each submission against caller-supplied prior state (added/
updated/removed items by id): no prior state or ForceBaseline emits a
fresh baseline; an empty diff is a no-op; a non-empty diff produces a
versioned change file. Publish holds no storage-backed state of its own --
callers reconstruct and pass in whatever was last published. Signing uses
the real detached-JWS scheme (artifactsigner), the counterpart to
catalogcrawler's fixed verifyDediProof.

Adds cmd/catalogpublisherctl, a demo CLI exercising Publish end to end
against a local output directory (manifest/index/baseline/change files,
demo-only local signing key), and a round-trip test proving
catalogpublisher's output verifies correctly through the real
catalogcrawler over HTTP.

Minor: gofmt cleanup on catalogcrawler.go, README note on the earlier
detached-JWS fix.
… file spec

Rewrites catalogpublisher's output to match "Decentralized Catalog file
spec.md" exactly, superseding the earlier DeDi-wrapper-shaped index this
package originally produced:

- Catalog index is now the plain {participantId, version, next_update,
  catalogs[]} shape -- no DeDi wrapper, no whole-index proof. Each
  catalog's baseline/changes entries carry their own signature (a plain
  Ed25519 signature over the JCS-canonicalized tuple
  {catalogId, version, url, digest, validUntil}), replacing the single
  whole-document detached JWS the index used to carry.
- Manifest file entries now use "name" (not "registry"), gained
  "networkIds", dropped "digest"/"state" -- matching the spec's three
  Beckn extensions to DeDi's manifest format.
- Adds retirement (tombstone entries), carry-forward of catalogs untouched
  by a given Publish call, an index-level version distinct from any one
  catalog's file versions, resources/offers diffing with upserts/removals,
  versioned immutable filenames, and compaction via ForceBaseline.
- catalogpublisherctl rewritten to match: new output layout
  (.well-known/dedi.index.json, dedi/becknCatalogs.index.json,
  flat catalogs/<name>.v<version>[.changes].json), prior-state
  reconstruction now reads the previous index to carry forward every
  other catalog, -retire flag, dropped the now-superseded
  -subscriberFileURL default.

catalogcrawler has not been updated for this shape yet -- its round-trip
test is temporarily skipped with an explicit reason rather than left
silently broken; rewriting the crawler to match is the next step.
…ler round-trip tests

catalogpublisher defaulted an unconfigured file signature's validUntil to
now+0 when neither FileValidityIn nor NextUpdateIn was set, i.e. already
expired the instant a crawler checked it -- now falls back to a 24h
default. Found while writing round-trip tests against the rewritten
catalogcrawler (catalog-crawler branch).

Replaces the previously-skipped round-trip test with four real ones now
that catalogcrawler has been rewritten to match: baseline, incremental
change-file composition, retirement (tombstone), and rejection of a
tampered/re-digested file. Also adds -publicBaseURL to
catalogpublisherctl, for serving its output over real HTTP instead of
file:// -- used to verify the whole chain live (published, served via
`python3 -m http.server`, fetched back with curl) alongside the in-process
httptest-based tests.
…e, config simplification

Adds core/module/handler/catalogPublishHandler.go: a DS-internal, unsigned
catalog/publish trigger mirroring catalogPullHandler exactly (no
validateSign/addRoute/signAck). Request body is plain {catalogs:[...],
retire:[...], forceBaseline} with no context/message envelope -- each
catalog's own top-level "id" is used verbatim as its catalogId. Response
borrows beckn.yaml's CatalogPublishAction/CatalogProcessingResult
vocabulary (ACCEPTED/REJECTED per catalog) as one synchronous call rather
than an Ack-now/on_publish-later pair. Public catalogs only in this phase.

Wires PluginManager.CatalogPublisher (interface + pkg/plugin.Manager
implementation), a new outputRoot handler-level config field, and
registers catalogPublish as a handler type. Updates the three mock
PluginManager test implementations to satisfy the extended interface.

Extracts pkg/plugin/implementation/catalogpublisher/localstore: the
shared "write a Publish result to a local directory, read it back as
prior state" logic that used to live only inside catalogpublisherctl --
now used by both the CLI and the new handler instead of two copies. Reads
the catalog index once per call and returns prior state for a batch of
catalogIds, a capability the single-catalog-at-a-time CLI logic didn't
have.

Simplifies catalogpublisher.Config: removes KeyID/Domain/IndexSchemaURL.
KeyID and Domain were redundant with (and had to exactly match) the
KeyManager's own SubscriberID/UniqueKeyID -- a real gotcha hit while
wiring this against simplekeymanager. Now Config only takes SubscriberID
(the KeyManager.Keyset lookup key), and the manifest's kid/domain are
read directly off the resolved Keyset. IndexSchemaURL becomes a package
constant pending a shared becknconstants home for canonical schema URLs.

Wires a working example into config/local-beckn-one-bap.yaml.
…unknown catalog fields

- catalogPublishHandler now derives subscriberId from keyManager's own
  config instead of requiring it declared twice, fixing a runtime
  "subscriberId is required" error caused by stale config.
- catalogfile.Apply no longer uses a fixed struct that silently dropped
  any catalog field beyond id/descriptor/provider/resources/offers on
  every reconstruction; it now preserves unknown top-level fields.
- diffCatalogAttributes generalized to report changes on any catalog
  field (not just descriptor/provider), matching the file spec's
  "catalog-level attribute changes (name, validity window)" wording.
… PublicBaseURL

Both URLs were always meant to point under the same publicly-served
root as localstore's on-disk layout, so there was no reason for two
independent config values. Publisher now derives the index and catalog
part URLs from a single PublicBaseURL (config key catalogBaseURL),
matching what catalogpublisherctl's -publicBaseURL flag already did
internally. localstore/localstore_test.go moved to an external test
package to avoid an import cycle now that catalogpublisher imports
localstore for its path constants.
…or incoming catalogs

Reuses schemav2validator and opapolicychecker as-is, no new plugin or
interface: the handler wraps submitted catalogs in a minimal
{context:{action:"catalog/publish"}, message:{catalogs:[...]}} envelope
and runs both against it before Publish is called. Both plugins already
work off a bare context.action / raw body, respectively, and catalog/
publish's message.catalogs[]:Catalog[] schema already exists in
beckn.yaml, so this validates for real, not just a rubber-stamped
envelope. Both plugins are optional and skipped when unconfigured; a
retire-only request (no catalogs submitted) skips validation entirely.
The catalogPublish handler's own request body is unchanged.
…files under catalogs/changes/, stop writing the manifest

- localstore.IndexDirName: "dedi" -> "index" (was a confusing collision
  with the manifest's own dedi.index.json filename).
- Change files now live under catalogs/changes/ instead of flat in
  catalogs/ alongside baselines; catalogPartURL/CatalogFilePath route
  by suffix accordingly.
- localstore no longer writes .well-known/dedi.index.json at all --
  that write is commented out with an explanation, and EnsureDirs no
  longer creates .well-known/. Publish still computes the manifest as
  part of PublishResult; localstore just doesn't persist it for now,
  pending a real ArtifactStore/manifest-ownership story.
…shDirectives, and beckn.yaml error shape

- catalogPublishHandler now optionally checks (read-only, via
  ManifestLoader/dediregistry) whether this node's manifest already
  declares this publisher's catalog index under
  catalog.catalogIndexes. If not, it stages a proposed updated
  manifest locally (never writing to the real one) and returns a
  warning; publish itself still succeeds either way.
- Adds a distinct manifestSubscriberId config, separate from
  subscriberId: ManifestLoader.GetBySubscriberID requires DeDi's
  namespace/registry/recordName path format, which is a different
  identifier than the plain Beckn subscriberId KeyManager.Keyset is
  indexed by -- reusing subscriberId for both would break signing.
- A manifest fetch/parse failure is now surfaced as a real error
  instead of being silently treated as "no manifest published yet",
  which previously produced a false "not declared" warning whenever
  the lookup failed for any reason (including a bad subscriberId
  format).
- Request body now adopts beckn.yaml's real CatalogPublishAction
  envelope (context/message.catalogs[]/message.publishDirectives[])
  instead of a synthesized wrapper, so schemaValidator/policyChecker
  validate the raw request directly. publishDirectives[].visibleTo
  maps onto CatalogSubmission.NetworkIds; catalogType maps onto
  CatalogSubmission.CatalogType.
- Error responses now nest under message.error, matching beckn.yaml's
  own NACK envelope convention instead of a flat top-level error field.
- Adds debug logging throughout catalogPublishHandler.go and
  catalogpublisher.go, matching the rest of the codebase's house style
  (both were previously under-instrumented at debug level).
…ter-kit's generic-bpp.yaml

Mirrors local-beckn-one-bap.yaml's catalogPublish wiring, adapted for
the bpp role/port, with schemaValidator and manifestLoader active
(not just commented examples) to match how starter-kit's
generic-bpp.yaml wires this handler.
@nirmay
nirmay marked this pull request as ready for review July 29, 2026 11:16
@nirmay
nirmay requested a review from nirmalnr July 29, 2026 11:17
…o policyChecker, honor Fatal errors

Self-review fixes across two rounds:
- itemsByIDOrdered/applyDiffBlock no longer emit a duplicated upsert when
  a submitted catalog or change file lists the same item id twice.
- NewCatalogPublishHandler loads manifestLoader before policyChecker and
  threads it through (was hard-coded nil), so a manifest-backed policy
  type can actually resolve.
- A PublishError.Fatal now flips the handler's overall response status to
  FAILED instead of being silently reported like a routine rejection.
- Retired-catalog tombstone ordering is now deterministic (iterates
  req.Retire instead of a map), and a catalogId that's both submitted and
  retired in one call no longer gets a spurious extra "retired" result
  alongside its real outcome.
- diffCatalogs/diffArrayField/diffCatalogAttributes parse each
  submission's prior/next catalog JSON once instead of 3-6 times.
- Removed dead commented-out manifest-write code in localstore.Write;
  fixed a stale README reference to only the bap config example.

Adds regression tests for each of the above.
@nirmalnr

Copy link
Copy Markdown
Contributor

localstore.Write is not atomic/ordered safely (localstore.go:150)
The index file is written first, unconditionally, before any per-catalog content file. If a later os.WriteFile for a baseline/change file fails (disk full, permission error, process killed), the index on disk already references a baseline/changes entry — complete with digest and signature — for a file that was never written. Any later localstore.Load for that catalog will fail reading it (reconstructState), and any external crawler fetching that URL gets a 404 despite the (correctly signed) index claiming it exists.

nirmalnr and others added 3 commits July 30, 2026 11:44
e2b03b8 removed the manifestSubscriberId config field in favor of a
synthetic subscriberID/subscribers.beckn.one/keyID path built from
values already known (keyManager's subscriberId + a resolved keyID),
but had three correctness gaps a self-review turned up:

- keyID was never validated non-empty before being spliced into the
  synthetic path -- an empty keyID silently produced a malformed path
  that failed the same generic "must be in namespace/registry/
  recordName format" error on every check forever, with no startup
  signal.
- subscriberID/keyID were never checked for embedded "/" characters,
  which would corrupt the synthetic path's 3-part shape the same way.
- keyID was resolved once at construction and cached, so a signing-key
  rotation after startup left the manifest self-lookup silently
  querying a stale keyID that no longer matched what
  catalogpublisher.Publish was actually signing with.

Now: subscriberID is validated for embedded "/" once at construction
(it can't change afterward, so this fails loudly at startup);
keyManager is stored instead of a cached keyID, and loadOwnNodeManifest
resolves the keyset fresh on every check, validating the resulting
keyID for emptiness and embedded "/" before building the synthetic
path. Matches catalogpublisher.Publish's own per-request Keyset
resolution, so a rotation is picked up immediately.

Adds regression tests for all three gaps.
…cord meta, drop manifest indirection

Collapses the three-level indirection (DeDi record -> node manifest ->
catalog index) to two levels (DeDi record -> catalog index directly):

- catalogPublishHandler's link check now reads meta.catalog_index_url
  directly off the subscriber's own DeDi registry record via
  RegistryMetadataLookup.LookupNode, instead of fetching, signature-
  verifying, and caching a whole node-manifest document just to read one
  nested field. ManifestLoader is no longer built or used by this
  handler at all -- registryMetadata reuses the already-loaded Registry
  plugin type-asserted to RegistryMetadataLookup, the same pattern
  stdHandler.go already uses.
- A missing/mismatched link is reported as a warning naming the meta key
  and URL to set -- there's nothing to stage locally anymore, since
  dediregistry has no write path either way and getting a value onto a
  DeDi record's meta was always an external, manual operator action.
  Removed localstore.WriteStagedNodeManifest/StagedNodeManifestPath and
  model.NodeManifestCatalog/CatalogIndexEntry accordingly.
- catalogpublisher.Publish no longer builds or signs a DeDi manifest at
  all: PublishResult.Manifest was already dead output (nothing read or
  persisted it -- confirmed by grepping every caller), so the whole
  dediManifest/dediKey/dediProof/dediFile/signManifest code path, plus
  Config.IndexNetworkIds/IndexAuthMethods/ExtraManifestFiles (the
  catalog-index-level auth/network restriction that only existed to
  populate that manifest), are deleted as dead weight.
- Replaced the misleading cfg.Plugins.ManifestLoader on/off toggle
  (which no longer built an actual ManifestLoader instance) with an
  honest, explicit catalogPublisher.config.checkCatalogIndexLink
  boolean.

Per-catalog AuthMethods/NetworkIds (CatalogSubmission's own restricted-
catalog support) are untouched -- only the whole-index-level manifest
restriction is gone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants