release: the provider plugins, per-provider auth and the CI pipeline - #26
Merged
Conversation
Adds a run-tests/security/build-and-push flow: gated lint, gotestsum with diff-scoped coverage, dependency+image Trivy scans gated on HIGH/CRITICAL, multi-registry multi-arch build-and-push on release tags. Both Dockerfiles switch to dhi.io base images (with public overrides for a local build without a DHI subscription), so the security and build-and-push jobs authenticate to dhi.io first.
… dependency [#5] security.yml's Trivy gate failed on unpatched CVEs in dhi.io/debian-base's bundled perl-base, ncurses, util-linux and zlib1g -- none of which the adapter uses. Neither can move to the fully static dhi.io/static image though: the adapter loads plugin .so files at runtime via Go's `plugin` package, which needs cgo and a real glibc dynamic linker to work at all. dhi.io/static's glibc variant is the middle ground -- keeps libc6 so plugin.Open still works, drops everything else the scan was flagging. That variant has no shell, so CMD's `sh -c "./server --config=$CONFIG_FILE"` can no longer expand the env var. Switched to an exec-form ENTRYPOINT and taught main.go to read CONFIG_FILE itself as the --config default.
…wn CVEs [#5] security.yml's trivy-deps scan flagged CVE-2026-56854 (x/crypto, CRITICAL) and CVE-2026-84304 (grpc, HIGH), both with fixed versions already published upstream.
Ran on every push including this branch and always failed here -- missing Gerrit credentials for the private Terraform-CICD repo it clones. workflow_dispatch stays, so it's still runnable by hand; kept, not deleted, pending a decision on whether it's folded into build-and-push.yml or removed outright.
…ersions [#5] This is CI setup, not a dependency-bump PR -- the two CVEs trivy-deps flags (CVE-2026-56854, CVE-2026-84304) are real and the scan catching them is working as intended; the actual bump is deferred to its own change.
Dockerfile.adapter goes back to the original public base images -- Dockerfile.adapter-with-plugins is the one actually deployed, so that's the one that needs the CVE fix. Keeps dhi.io/static's glibc-only variant (clears the unpatched perl-base/ncurses/util-linux/zlib1g CVEs, keeps libc6 so plugin.Open still works) but drops the main.go change entirely: Deployment/deployment.yaml now passes --config=$(CONFIG_FILE) via args, letting the kubelet do the substitution instead of a shell inside the container.
…dapter [#5] security.yml and build-and-push.yml still pointed at Dockerfile.adapter, which we just reverted back to the original, unpatched base images -- so the security gate was scanning the wrong Dockerfile and the CVE fix never took effect. Dockerfile.adapter-with-plugins is the one actually deployed and the one carrying the DHI glibc-only runtime fix.
…ce+coverage run [#5] pkg/plugin and benchmarks/e2e each build a real .so via a subprocess `go build -buildmode=plugin`, then load it with plugin.Open in the same test run. Instrumenting the whole module for coverage/race in one ./... build gives a shared package a different build identity than the plain subprocess build produces, so plugin.Open rejects the .so. Split them into their own go test invocation and merge the coverage output.
None of these six can run or need to run on this repo as it stands:
beckn_ci.yml, beckn_ci_test.yml trigger on PRs to
beckn-onix-v1.0-develop, a branch that
does not exist here — they have never
fired and cannot.
deploy-to-gke.yml,
deploy-to-gke-BS.yml,
build-and-deploy-plugins.yml,
onix-gcp-terraform-deploy.yml deployment, which is out of scope for
now. Keeping them means keeping the
GCP/registry secret surface and the
maintenance cost of workflows nobody
runs.
What is left is CI, Security and sign-beckn-constants.yml.
build-and-push.yml is release/deploy, which is out of scope for now, so it
goes with the other deploy workflows. That also retires the review findings
that only existed inside it (the tag globs, id-token: write with no consumer,
:latest on a -rc tag, the duplicated registry-login block, and the multi-arch
manifest step).
The Docker Hardened Images switch went with it, and it had to:
Dockerfile.adapter was migrated to dhi.io in a PR whose stated
scope is CI workflows. dhi.io is
subscription-gated, so the image is not
pullable without the DHI entitlement.
Dockerfile.adapter-with-plugins same switch, plus trailing-slash churn. Back
to golang:1.26.1-bookworm building and
cgr.dev/chainguard/wolfi-base as the runtime
— which has a shell and glibc, so
plugin.Open works, and is publicly pullable,
so a fork PR's image scan still builds.
Deployment/deployment.yaml the added args: ["--config=$(CONFIG_FILE)"]
block does not match the adapter's flags and
would CrashLoopBackOff on rollout.
.github/actions/dhi-login only existed to authenticate to dhi.io.
All three files are now byte-identical to development.
…es [#5] The Security workflow this PR adds is only worth adding if it can be green, and on development it is not: trivy fs flags three HIGH/CRITICAL advisories in the dependency graph. golang.org/x/crypto v0.54.0 -> v0.55.0 google.golang.org/grpc v1.82.1 -> v1.83.1 github.com/rabbitmq/amqp091-go v1.11.0 -> v1.13.0 `go mod tidy` carried the transitive minimums along with them (otel 1.43 -> 1.44, x/net 0.56 -> 0.57, x/text 0.40 -> 0.41, genproto). No source change was needed: `go build -trimpath ./cmd/adapter/...` and `go vet ./...` are both clean, and the full suite passes.
Review feedback was that the workflows carried the logic and the Makefile
carried aliases — 132 lines of inline shell across the three workflows, none
of it runnable locally. That is inverted here: the Makefile is the single
source of truth and every CI step becomes a one-line `make <target>` call, so
a red check reproduces by running the command the log shows.
Fixes, not just relocation:
test / cover both failed on development. pkg/plugin and benchmarks/e2e
build a real .so and plugin.Open it in the same run, and
both -race and whole-module coverage instrumentation make
that .so unloadable. They now run as their own invocation.
The carve-out is named once (PLUGIN_PKGS/MAIN_PKGS) and
shared by test, cover and test-ci, so the three cannot
disagree about it.
-coverpkg answered in a comment rather than added: benchmarks/e2e is
the only test-only package in the module and it is in the
carve-out precisely because whole-module instrumentation is
what breaks plugin.Open. It cannot be added without
reintroducing the failure.
cover-diff no longer swallows a broken base. An unresolvable BASE_REF
and a failed `git diff` are both hard errors with an
::error:: annotation — an unknown changed-file set is not an
empty one. It writes coverage-report.md on every exit path,
including those two, so the workflow can read the file
unconditionally. BASE_REF defaults to origin/development,
the branch PRs actually target.
merge-coverage `;` not `&&`, and always removes coverage-plugin.out, so a
failed merge cannot leave a half-profile behind to be
misread. coverage-plugin.out is gitignored as well.
coverage.out a file rule, so cover-diff has a buildable prerequisite: it
reuses the profile test-ci already wrote in CI, and runs
the suites once on a clean local checkout.
trivy-gate a missing or unparsable SARIF report is now a failure. It
used to pass vacuously when a scan wrote nothing, which is
the one case a security gate must not be green for.
docker builds Dockerfile.adapter-with-plugins — the image that
ships and that security.yml scans — not Dockerfile.adapter.
Version build args come from install/scripts/version-vars.sh
so they are spelled out in one place.
trivy-deps --skip-dirs bin, so the scanner stops reporting on the
trivy binary the cache restored next to it.
help one line per target from the `## ` docstrings, instead of a
wall of text.
tools/trivy-comment.jq is the 16-line jq program that was pasted into four
places. As a file it is lintable (`jq -n -f`), diffable, and free of Makefile
`$$`/backslash escaping, and one copy means the dependency and image reports
cannot drift apart.
Removed as dead: trivy-release-gate (its only caller was build-and-push.yml,
now deleted) and the `tools` alias (nothing invoked it — every tool is already
built on demand as a prerequisite).
Verified locally: build, test, cover, cover-diff (pass, no-changed-files,
below-threshold and unresolvable-BASE_REF paths), trivy-deps, trivy-image,
trivy-comment, all five trivy-gate states, and help.
run-tests.yml becomes ci.yml, replacing the dead ci.yml that triggered on
`beck-onix-v1.0-develop` (a typo of a branch that does not exist, so it never
fired). Names now read as what they gate: the workflow is `CI` and the jobs
are `Lint` and `Test`, so branch protection matches `CI / Lint` and
`CI / Test` rather than the old `Run Tests / lint`.
The job bodies are four `make` calls plus GitHub plumbing:
make lint advisory, continue-on-error. There is no .golangci.yml in
this repo yet, so `run ./...` uses tool defaults against a
codebase that has never been linted — enough pre-existing
findings to block every PR on day one. This surfaces them.
(The comment this replaced cited a CLAUDE.md and a
pre-commit hook, neither of which exists here.)
make build
make test-ci
make cover-diff
push: added `development`, the branch PRs target and merge into, so a merge
landing there runs the same gates the PR did. It previously only ran on main
and PRs, leaving merges into development ungated.
BASE_REF goes through `env:`, not string-interpolated into the run script —
`?=` in the Makefile means the environment wins, so nothing untrusted ends
up inside a shell command.
permissions are least-privilege per job: the workflow grants contents: read
and only the Test job adds pull-requests: write for the coverage comment.
the coverage comment steps are continue-on-error — a fork PR's default
GITHUB_TOKEN is read-only, so create-or-update-comment 403s there. Coverage
is still gated by the last step; a comment that cannot post must not fail
the job over it.
find-comment v3 -> v4 and create-or-update-comment v4 -> v5.
The final gate step is `exit 1`, nothing more: cover-diff already emitted the
::error:: annotation and the per-file breakdown.
…y-gate [#5] The single `security` job scanned dependencies, then built the image, then scanned that, then gated — serially, in one job, so a dependency finding waited on an image build that has nothing to do with it, and the PR could not say which scan found what without opening a log. Three jobs now, named for what they do: `Dependency scan` and `Image scan` run in parallel (the dependency scan needs nothing but a checkout), and `Security gate` fans in with needs + if: always(). Branch protection matches `Security / Security gate`. Every step in the file is a one-line `make` call or GitHub plumbing — zero lines of inline shell, down from 132 across the three original workflows: make trivy-deps make trivy-comment SARIF=... TITLE=... OUT=... make docker IMAGE=network-adapter:<sha> make trivy-image IMAGE=network-adapter:<sha> make trivy-gate Also fixed: the gate now reads the SARIF the scan jobs produced, handed over as artifacts, rather than scanning a third and fourth time. if-no-files-found is deliberately `ignore`: a scan that produced nothing surfaces as the gate reporting MISSING — which now fails — with both verdicts visible in one place, rather than as a second red check on the scan job. push: added `development`, so a merge landing there runs the same scans the PR did. permissions are least-privilege per job: contents: read at the workflow level, with pull-requests: write and security-events: write only on the two scan jobs. The gate stays read-only. both upload-sarif steps are continue-on-error — a fork PR's GITHUB_TOKEN has no security-events scope, so they 403 there. The gate decides pass/fail; an upload that cannot happen must not fail the scan over it. a `category` per scan, so the image upload no longer replaces the dependency analysis (GitHub keys an analysis on ref + category). find-comment v3 -> v4, create-or-update-comment v4 -> v5. .github/actions/trivy-cache is the `Trivy cache date` + actions/cache pair, extracted now that two jobs need it — both jobs stay a one-line `uses:`. It keeps the trivy binary (~30MB) and the vulnerability DB (~100MB+) out of every run, which the trivy-action it replaced did for us.
] Both workflows already grouped on github.ref with cancel-in-progress, so a new commit on a PR did supersede the previous one's runs — but the intent was undocumented and the flag was a bare `true`, which also cancelled runs on main and development. cancel-in-progress is now scoped to pull_request. On a PR, pushing a new commit cancels the CI and Security runs still in flight for the commit before it: github.ref is refs/pull/<n>/merge for the whole life of the PR, so every push lands in the same group. On main and development nothing is cancelled — each commit there keeps its own recorded verdict, instead of being left with a grey check and no way to tell afterwards whether it was ever green. The two workflows keep separate groups, so cancelling a superseded CI run does not take that commit's Security run down with it.
] Nothing in this repo validated the workflow files. A bad ${{ }} expression, a mistyped `uses:` ref or a `needs:` pointing at a job that does not exist was only discoverable by running the workflow — or by running actionlint by hand. Deliberately not a CI step. The gate is a pre-commit hook, so the feedback lands before the commit exists rather than after a push: lint-actions actionlint (pinned v1.7.12, go-installable like the other tools) over the whole .github tree. Whole-repo rather than per-file because actionlint resolves `needs:` across a workflow's jobs and checks `uses: ./.github/actions/...` against the action on disk. lint-staged what the hook runs. Fires actionlint only when a workflow or composite action is staged, and gofmt only over the staged .go files. hooks sets core.hooksPath to .githooks. Run once per clone. .githooks/pre-commit is a one-line `exec make lint-staged`: the logic stays in the Makefile, so `make lint-staged` reproduces exactly what blocked a commit, and there is a single place to change what runs. core.hooksPath rather than copying into .git/hooks means the hook is versioned and reviewable, and a change to it reaches everyone on their next pull. Scope is what can pass today, so the hook never becomes something people reflexively --no-verify past: - actionlint reports nothing on the current tree, so it blocks from day one. - 22 files in the repo are not gofmt-clean. Staged-only means that history stays someone else's problem until you touch one of those files. - `golangci-lint run` is excluded: with no .golangci.yml it uses tool defaults against a codebase that has never been linted, so it would reject every commit. - the test suite is excluded: a pre-commit hook has to stay in seconds, and `make test-ci` in CI is where that belongs. Verified all six paths: nothing staged, staged Go/workflow with a clean tree, actionlint catching a bad steps.<id> reference, gofmt rejecting and then accepting a staged file, a real `git commit` blocked by the installed hook, and --no-verify still getting through.
Two checks on a PR, two on a tag, one workflow.
CI / End to End verification build, test, changed-line coverage, result
posted to the PR and the job summary
CI / Security Scan trivy on the dependency graph and on the
shipped image, one comment, one gate
CI / Build Artifact (amd64) push the image to ghcr.io by digest,
CI / Build Artifact (arm64) natively on a runner of that arch
CI / Publish Artifact bind the version tag (and :latest for a
plain vX.Y.Z) to both digests
Dropped the Lint check. It was continue-on-error, so it gated nothing: there
is no .golangci.yml yet and `golangci-lint run` against a never-linted
codebase reports enough to block every PR, which is why it was advisory in
the first place. `make lint` still exists for local use, and the .githooks
pre-commit hook is what actually enforces gofmt on every commit.
Folded security.yml into ci.yml. Dependency scan, image scan and the gate
were three jobs and three checks to answer one question; they are one job
now, which also removes the SARIF artifact handoff between runners and turns
the two per-scan PR comments into one. trivy-comment's SARIF/TITLE/OUT
parameters existed only to serve two jobs and are gone with them —
trivy-report renders both reports from the SARIF_REPORTS list that
trivy-gate reads, so the comment and the gate cannot disagree.
Tag triggers accept 1.0.0 and 1.0.0-RC1 as well as the v-prefixed spelling
every existing tag in this repo uses. Each arch builds on a runner of that
arch rather than under QEMU, because the image compiles every plugin with
`go build -buildmode=plugin` and a plugin .so has to match the adapter
binary's GOARCH. Build Artifact pushes by digest with no tag, so only
Publish Artifact creates something visible, and a tag can never resolve to
one architecture.
Job conditions test startsWith(github.ref, 'refs/tags/') rather than
github.ref_type: on a pull_request the ref is refs/pull/<n>/merge and
ref_type is not documented to be 'branch' there, so a ref_type test risked
skipping both PR checks and reporting green for a run that did nothing.
] Gating Build Artifact and Publish Artifact with `if: startsWith(github.ref, 'refs/tags/')` inside ci.yml put two permanently skipped checks on every PR: CI / Build Artifact (${{ matrix.arch }}) (pull_request) Skipped CI / Publish Artifact (pull_request) Skipped A job whose `if:` is false is still reported as a check, and its `name:` comes through with `${{ matrix.arch }}` unexpanded because the matrix leg never materialised — so the skipped-check noise and the raw expression in the name were the same bug. A workflow whose trigger does not match the event does not exist on that event at all, which is the only way to get zero checks. So: ci.yml is PR/trunk only (push to main and development, plus pull_request) and carries no `if:` on either job, and ci-release.yml is tag-only. Both are `name: CI`, so the check names still read "CI / Build Artifact (amd64)" and "CI / Publish Artifact". The one cost is two entries called "CI" in the Actions sidebar. A PR now shows exactly two checks, and a tag exactly three.
… the gate [#5] A PR reported three checks, not two: CI / End to End verification success CI / Security Scan failure Code scanning results / Trivy success — "No new alerts in code change" The third is not a job. github/codeql-action/upload-sarif registers its own check named after the SARIF tool driver, so it cannot be renamed, given an `if:`, or suppressed while the upload happens. And on that run it was green while `make trivy-gate` failed on the same SARIF: 20 HIGH stdlib CVEs, none of them *new to the diff*, which is the only thing the code-scanning check measures. Two checks disagreeing about one scan is worse than one check, and the gate is the one that is right. Every finding is already in the PR comment with its severity, installed version, fixed version and advisory link, and `make trivy-gate` decides pass/fail. `security-events: write` goes with the upload. The cost, stated: no Security-tab history or diff annotations for Trivy findings. The comment and the gate are the surface now.
The image scan the Security Scan job runs found 20 HIGH findings, all of them the same package: `stdlib` at v1.26.1. Zero came from application dependencies — `trivy-deps` is clean. Trivy reads the Go build info embedded in the binary, which is why these show up in the image scan and nowhere the dependency scan can see. The advisories' fixed versions top out at 1.26.6; 1.26.8 is the current 1.26 patch, so it clears all 20 with room before the next one lands. Bumped in three places that have to agree: go.mod's `go` directive (which is also what setup-go resolves through `go-version-file`, so the test toolchain follows) and both Dockerfiles' builder base. Leaving Dockerfile.adapter at 1.26.1 would have kept the two images on different toolchains for no reason. This re-touches the Dockerfiles, which this PR had otherwise reverted to byte-identical with development as out of scope. Different case: a one-token patch bump to the base image tag, made because the Security gate this PR introduces cannot go green without it — not the base-image migration that was reverted. Verified: `make build` and the full `make test` (every package, including pkg/plugin and benchmarks/e2e, which build and plugin.Open a real .so) pass on 1.26.8 with zero failures.
The gate scanned at HIGH,CRITICAL only, so "0 findings" in the check meant "0 in that band" rather than "clean". Unfiltered, the tree has three findings in golang.org/x/crypto v0.55.0 that the gate never saw. UNKNOWN stays out of the band deliberately. All three of those findings carry no vendor severity, and one (GO-2026-5932, x/crypto/openpgp is unmaintained) has no fix at all, so including UNKNOWN would pin the gate red with no action available. None is reachable here: the only x/crypto import in the tree is blake2b, and the build closure of the adapter plus every plugin contains no x/crypto/ssh or x/crypto/openpgp. SEVERITY stays the single source of truth. The gate already counted rows in the SARIF rather than re-filtering, so widening the scan band was enough; the three places that restated "HIGH or CRITICAL" in prose now derive it, including the jq renderer, which takes the band via --arg so the "nothing found" line cannot drift from the scan that produced it. Verified with trivy v0.74.0, the version CI pins: deps 0 at the new band (the 3 UNKNOWN correctly excluded), wolfi-base runtime layer 0, shipped binary 0. The band change has teeth — the pre-bump go1.26.1 binary goes from 20 to 28 findings, the 8 extra all MEDIUM. Gate exit codes: 2 on findings, 2 on a missing report, 0 only when clean.
…er [#5] image-publish joined its steps with `;`, so the recipe exit status came from `imagetools inspect` alone. A `create` that failed against an already-existing tag left inspect reporting the previous index and the job green, publishing a tag bound to the wrong digests with nothing red to say so. Every step is now `&&`-chained. The trivy installer was fetched from the repo default branch while only the version it installs was pinned, so a job holding the runner GITHUB_TOKEN piped an unreviewed remote script into sh on every cache miss. It now comes from the $(TRIVY_VERSION) tag. .gitignore also drifted from the target set: trivy-report.md, image-metadata-*.json and digest-*.txt are produced but were not ignored, while trivy-deps-comment.md and trivy-image-comment.md outlived the trivy-comment target that wrote them.
chore: add CI and Security workflows, drop the dead and deploy-only ones [#5]
image-build pushes by digest and image-publish binds the tag; nothing in between looked at what was being published. The PR-time Security Scan does not cover it — it grades an image built from the PR's tree on the day the PR ran, while a tag cut weeks later is a fresh build off a rolling wolfi-base and a freshly resolved module graph. trivy-release-gate was removed earlier in this branch as dead code, correctly at the time: its only caller was build-and-push.yml. Adding ci-release.yml put a release path back without putting the gate back with it. Reinstated against the pushed digest rather than a local tag, since push-by-digest leaves no local reference. The digest is unreachable by name until image-publish binds a tag, and the gate runs before that, so a finding means the version tag is never created. --exit-code 1 with a table: there is no PR to comment on, so the findings belong in the log the red check points at. Scanned per arch on its own native runner. Also adds timeout-minutes to all four jobs. The default is 360, and the new gate pulls from a registry — a stalled pull would otherwise hold a runner for six hours, and on a release leg it blocks Publish Artifact behind `needs`, so the tag simply never appears with no failure to point at.
] cover-diff has two paths that compute no percentage — no changed non-test .go files, so there is no denominator, and changed files carrying no coverable statements, which would be 0/0 — and both reported "Test Coverage: ✅ Passed — not applicable". Read from the PR that is a green pass, so a CI-only or docs-only PR looked indistinguishable from a well-tested one, and there was no number to reconcile the claim against. Both now report "➖ Not applicable" and say which case applies and why there is no percentage. Still exit 0 — a PR that changes no Go code must not be blocked on Go coverage — so the check verdict is unchanged; only the claim the comment makes about it is. Also corrects "no changed Go files" to "no non-test Go files changed": the changed-file set already filters _test.go, so a test-only PR took this path while the message said no Go files had changed at all.
fix: scan the release image before its version tag is bound [#5]
Resolves two different things from the OAN Registry, a SunbirdRC deployment, and keeps them apart because they answer different questions about different parties. RegistryLookup answers "who sent this": given the subscriber and key named in an inbound Authorization header, it returns that sender's signing key so the signature can be verified. This runs inside signature validation on every inbound message, so its timeout and retry budget are deliberately tighter than the sibling registry plugins' -- timeout x (retry_max + 1) is time a request spends waiting before it can even be rejected. ProviderRecordLookup answers "who do I call next" [beckn#63]: given a capability binding taken from a request body, it reads the binding and the participant that owns it, and joins them into one call plan -- where the provider is, and per Beckn action, how to reach it. Every way of saying "this capability cannot be served" returns one sentinel, because a caller does the same thing with all of them; a registry that could not be CONSULTED returns its own error, since an outage is not an answer. Several decisions here were forced by the deployed registry rather than chosen: - records are read from the nested shape the registry actually serves, with keys under node.keys[] rather than flat on the record - a key is matched by its osid, which is what an Authorization header carries; the friendly keyId identifies nothing the registry indexes - the "base64:" label is stripped from key material, because model.Subscription carries the bare value signvalidator feeds straight to base64.StdEncoding.DecodeString - status is checked at both levels, since a participant stays active while one of its keys is retired - actions are read as an array: the registry treats every nested object as an entity and injects osid into it, which a map cannot carry Status is an allow-list throughout, not a deny-list. model.IsKeyStatusUsable treats anything it does not recognise as usable, so passing the registry's own vocabulary through unchanged would let a suspended participant's signature verify. Verified against a live registry and the recorded response it serves.
Providers do not speak Beckn. The old provider backend answered that with one
hand-written service per provider -- around 6,900 lines across eight of them,
most of it building catalog JSON field by field. This makes the translation
configuration instead: a new provider ships mapping files, not another
transformation routine.
The plugin is domain-free by design. It knows nothing about who is calling,
nothing about what a mapping says, and nothing about the payloads passing
through: it is handed a reference and an input, and it fetches, compiles,
caches and runs whatever is there. Anything specific to a network or a provider
belongs in the caller, which is what lets one mapper serve all of them.
A mapping file carries every action one capability serves, keyed by action name.
Request files are keyed by the action they translate, response files by the one
they produce -- so a select mapping sits under "select" and its answer under
"on_select", and each file names the Beckn actions it actually deals in. One
file per direction rather than per action means a transaction walking select
then confirm pays one fetch, not one per step.
An action may be declared with an empty value. That is a statement rather than
an omission: this action needs no document built, because the caller supplies
the request itself -- a provider taking two query parameters is the ordinary
case, and passing already-resolved values through a fetch and a compile to
arrive at the same two fields buys nothing. Declared-but-empty and absent are
deliberately different, and reported differently: the first says "I serve this,
build it yourself", the second says "I do not serve this at all". Collapsing
them would send an empty request where a refusal was owed, answered with a 200
and the wrong data.
Two things the race detector settled rather than the design:
- jsonata.Expression.Evaluate MUTATES the expression it is called on, binding
into its own frame, so a cached compiled expression cannot serve two
requests at once. Evaluation takes a per-mapping lock rather than
recompiling: measured, evaluation is ~22us against ~184us to compile, and
both are dwarfed by the upstream call that follows
- a compile failure is held against its own action, so a typo in confirm is no
reason for select to stop being served
References arrive from the registry, which makes them external input: anything
that is not an http(s) URL with a host is refused, and reads are capped in both
time and size.
The last piece: a step that recognises its own capability, resolves what the
provider needs beyond the Beckn payload, calls it, and lets the mapper translate
both ways. With the registry supplying the call plan and the mapper the
translation, adding a provider is now a plugin with two short methods, two
mapping files and a registry row.
Dispatch turned out to need no mechanism at all. A provider step handed a
request for a capability it does not serve does nothing and returns nil, so
several sit in one pipeline and each recognises its own work. There is no
routing table to keep in step with the registry, and no filename convention --
which matters because a binding key contains | and :, and a plugin id is its .so
basename. Keying on the binding key rather than the participant is deliberate:
one provider can serve several capabilities with different logic, as
gfr-crop-registry and gfr-crop-recommendation did in the old backend.
Three supporting pieces:
- definition.ProviderStepProvider, because a provider step needs a registry
and a mapper handed to it, which the plain StepProvider contract cannot do.
Same shape as PolicyCheckerProvider taking a ManifestLoader
- internal/oanbinding derives the binding from a payload. Shared, because the
binding is a property of OAN's payloads and not of any provider. A payload
naming more than one distinct provider or type is refused rather than
resolved to its first: one binding key describes one upstream call, so
guessing would silently serve part of the request
- model.StepContext.ResponseBody, so a step that has already obtained an
answer has somewhere to put it. Without it the no-route path writes a fixed
ACK and ignores the body entirely, so the answer was discarded and the
caller got an ACK for data it asked for synchronously
That last one has four call sites, every one gated on the field being
non-empty, so no existing module changes behaviour by a byte. The gate that
matters least visibly is in the step instrumentor: it shallow-copies the context
in but copies only named fields out, so without one line there an answer written
by an instrumented step vanishes -- and instrumentation is the default path,
meaning it would work unwrapped and fail wrapped.
signAck signs whichever body will actually be written. Signing the generated ACK
while sending an answer would put a valid signature over the wrong bytes, which
is the one failure here that looks fine in testing and is rejected by every
peer.
Mausamgram itself is small: its prerequisite reads a point from the request, and
coordinates are GeoJSON order -- [lon, lat] -- which read the other way round
yields a valid request for the wrong hemisphere, so there is a test for exactly
that. Auth is configured by scheme naming the ENVIRONMENT VARIABLE to read,
never the credential: the secret reaches the process through its environment and
nothing else, and never through the registry. A configured credential that is
absent fails the request rather than calling the provider unauthenticated.
Verified end to end against a live registry, mappings served over HTTP and a
mock provider: a signed select in, a valid on_select out.
upstream.go had grown to 1,532 lines and 58 declarations, from config
parsing to URL escaping. Split into seven files along the call chain, all in
the same package:
constant.go 87 the 16 constants, and nothing else
upstream.go 225 Config, Step, New, applyDefaults, bindingPaths
serve.go 219 Run, serve, resolve, buildRequest -- the pipeline
http.go 436 the call, its retry budget, the endpoint, the body
auth.go 281 AuthProfile, ParseProviderAuth, authenticate
token.go 163 the oauth2 exchange and the token each provider holds
redact.go 196 redact, secretForms, longestFirst
A PURE MOVE. Nothing renamed, nothing exported, no test touched. Verified
three ways rather than by reading:
- all 58 declarations present exactly once; the splitter refuses to run if
one is placed twice or left behind
- every declaration byte-for-byte identical, doc comment included, compared
as an unordered set so file boundaries and ordering do not matter
- 66 packages green with zero test edits, no races, vet clean
http.go is the largest at 436 because it absorbs what could have been three
files -- the retry loop, the endpoint construction and the method and body
helpers. They are one subject: everything about talking HTTP to a provider.
Imports were pruned per file with goimports; constant.go needs only time.
authFields stays in auth.go rather than constant.go. It is a var -- Go has no
constant maps -- and it belongs beside the parser that reads it.
One block was headed "Auth schemes this step can present upstream" while also holding RetryBackoffBase, RetryBackoffMax and redactedMarker. Three subjects under one heading, which the split carried across unchanged rather than fixing mid-move. Now three: the retry waits, the redaction marker on its own, and the auth schemes under the comment that was always about them. Comments only -- all 16 constants present with identical values, verified name by name against the file before the split. 66 packages green, no races. The file doc also says why authFields is not here: it is a var, Go has no constant maps, and it belongs beside the parser in auth.go that reads it.
The package holds what all three capability plugins share, and the name now says so: WeatherObservation, MandiPrice and KnowledgeAdvisory are each a name, a set of binding keys and a credential profile over this machinery. internal/upstream -> internal/common package upstream -> package common upstream.go -> common.go upstream_test.go -> common_test.go 17 files and 45 qualified references. Four of them had to ALIAS the import as upstreamstep, because `upstream` was already the name of the httptest server in those tests -- that alias is gone, since nothing collides with `common`. THE WORD "UPSTREAM" STAYS where it means the API being called. It is the registry's own -- a Participant of type upstream, as against a node that speaks Beckn -- so it remains in the 51 error and log prefixes, in upstreamRequest and upstreamResponse, and in the comments. An operator reading "upstream: provider did not answer" is being told which leg of the call failed; "common:" would tell them nothing. The package doc now records that split explicitly. The doc also records what the name must not become. `common` is a name that attracts unrelated code, so it says: everything here is one subject, calling an upstream on a capability's behalf, and something shared by fewer than all three plugins belongs in its own package rather than here on the strength of the name. 66 packages green, vet clean, no races. One thing the compiler caught rather than review: the first pass rewrote `upstream.URL` and `upstream.Close` on the httptest server, because the regex matched a capitalised field the same way it matched an exported identifier. 82 of those, reverted -- the four legitimate reference shapes are Config, New, AuthProfile and ParseProviderAuth.
capabilitybinding had exactly ONE consumer -- internal/common, in three files -- so it was a package boundary with nothing on the other side of it. Its contents are now two files in common: capabilitybinding/capabilitybinding.go -> common/binding.go capabilitybinding/paths.go -> common/paths.go capabilitybinding_test.go -> common/binding_test.go The move forces two renames, because a name that reads well qualified does not read well bare: capabilitybinding.From -> bindingFrom From(paths, body) alone says nothing capabilitybinding.ErrNoBinding -> errNoBinding nothing outside needs it Error prefixes move from "capabilitybinding: " to "upstream: ", which is what the rest of the package already uses and what tells an operator which leg of the call failed. Two of the path messages gained the word "binding path" so they still say what was wrong: "upstream: binding path providerIdAt is empty". No test asserted on the old prefix. The package doc is no longer a package doc -- it now heads binding.go as a section comment, keeping the reason the binding lives here rather than in one capability: it is a property of the network's payloads, not of any provider. 65 packages green rather than 66, and the missing one is capabilitybinding itself -- verified by diffing the tested package lists, nothing else dropped out. Vet clean, no races.
serves said the config order is preserved in "the log line above". That line is New's "Upstream step created for ...", which the split moved to common.go, so the reference pointed at nothing in serve.go. It now names New instead of a position. Found by auditing the rearrangement for stale references rather than by reading past it. Three other positional comments were checked and are correct: two are about a paragraph or a mapping file rather than code position, and common_test.go's "budget() above" still holds because the test file was not split.
renderScalar said how it worked, not what it was for, and its doc comment only restated the name. It renders one mapped field as a query parameter value, so: renderScalar -> asQueryValue Matching asQuery directly above it, and the asX convention the package already uses -- asQuery, asString, asNestedBlock. The comment now says what the bool means and why only three types are carried: an object or an array has no single obvious encoding, so choosing one here would put a convention in Go that belongs in the mapping, where the upstream's shape is known. It also documents null, which was undocumented and untested. null is refused rather than sent as empty, because a parameter present but empty and a parameter absent mean different things to some upstreams -- a mapping says which it wants by omitting the field or setting "". Two tests now pin that: objects, arrays and null are refused with the field named, and an empty string IS carried as token=. Negative control: making null render as empty fails the null case, so the test is not vacuous.
58 error and log messages carried it. The structured log already records module_id, message_id and the step, so the prefix repeated what the fields say and cost a reader eight characters on every line: upstream: provider did not answer after 3 attempts provider did not answer after 3 attempts Consistent with sunbirdRegistry and router, which carry no prefix. jsonmapper still does; not touched here. One test mentioned the prefix and it was building its own wrapper string, not asserting on ours -- checked before changing, and it still passes. The package doc went from 25 lines to 14. It had a paragraph justifying the prefix, which no longer exists, and the blanket replace had mangled that paragraph's own quoted example into a sentence contradicting itself. "Upstream" remains the word for the API being called -- it is the registry's, for a Participant of type upstream as against a node that speaks Beckn. 65 packages green, vet clean.
105 comment lines to 48, 196 total to 139. Comments only -- code verified identical by stripping every comment and blank line from both versions and diffing: 82 lines each, byte for byte. A DUPLICATE FOUND WHILE READING. The nineteen-line per-scheme block appeared on BOTH secretForms methods, copied there when the file was split. It belongs only on the one that switches on the scheme; Step.secretForms now says what it does in three lines. What went, and why it was safe: the incident detail on redactedErr -- that isPermanent tests before redaction, so nothing failed visibly. Pinned by TestRedactKeepsTheErrorChainMatchable. the paragraph justifying logging bodies and URLs at all. That argues for logging, not for redacting, and the two redaction tests cover the behaviour. "basic is the scheme the reference config ships" -- a fact about config/provider-adapter.yaml, checkable there. What stayed, because a reader could otherwise undo it: errors.New breaks the chain and %w undoes the redaction; the merged set must sort by length because one provider's token can be a substring of another's; the username and client id are deliberately not redacted; escaping uses the same function Encode does. 65 packages green, vet clean.
1,098 comment lines to 457. Comments only -- for each of the nine files, every
comment and blank line was stripped from both versions and the remainder
diffed: identical, line for line.
TWO REAL DEFECTS FOUND WHILE READING, both left by the earlier file split,
which reordered declarations and stranded two doc comments:
auth.go authenticate's doc had merged into missingCredential's, so
authenticate was undocumented and missingCredential's doc opened
with a sentence about a different function.
http.go call's doc ran straight into budget's, same shape. call had none.
Both now document what they are attached to.
The rule applied: keep the non-obvious why, especially anything a reader could
undo -- the int64 overflow in backoff that returns a NEGATIVE duration, the
405 a lowercase method earns, why redaction sorts across the merged set, why
the token cache is per profile, why null is not a query parameter. Cut what
restated the code, the incident narratives ("used to", "which is how this was
found"), hypotheticals, and third examples where one lands.
Nothing load-bearing went: 15 of the 16 phrases a reader could otherwise
"fix" are still stated, checked one by one. The sixteenth was registry-schema
detail (one Participant, two ProviderSchema rows); the consequence it existed
to explain -- a second config entry with the same plugin id colliding in the
handler's step map -- is still there, and the guard has its own error in
stdHandler.go.
65 packages green, vet clean, no races.
16 files, 204K of cached schema documents under pkg/plugin/implementation/AgricultureFacility/testdata/schema-cache/. They belong to PR #22, which is somebody else's work and not merged. They were sitting untracked in the checkout when I ran `git add -A` for the internal/ rename in 53b4bd7, and that swept them in. Not present on feat/7-knowledge-advisory or on development, so this branch introduced them. Nothing references them: the only matches for the name are inside the files themselves. 65 packages green after removing them. The mistake was `git add -A` on a path I did not fully own. Every commit since has staged named paths instead.
http.go was 408 lines: two Step methods and fourteen helpers that need nothing from Step. The helpers move to common/httputil, grouped by subject: httputil/retry.go 88 Budget, Backoff, Sleep, DoNotRetry, IsPermanent httputil/endpoint.go 107 BuildEndpoint, verifyPath, verifyBaseURL httputil/query.go 58 asQuery, asQueryValue httputil/request.go 49 RequestBody, HasBody, CanonicalMethod httputil/explain.go 22 Explain httputil/constant.go 40 the retry, timeout and explain constants http.go is now 125 lines holding call and attempt, which stay because they are Step methods -- a method must be in the same package as its receiver. THE COST, measured rather than estimated. Ten helpers become exported, because call and attempt still use them from common. Only four stay private -- verifyPath, verifyBaseURL, asQuery, asQueryValue -- reached solely through BuildEndpoint, which moved with them. CONSTANT.GO IS NOW SPLIT, which contradicts the one-constants-file decision. Seven constants had to move: httputil reads them and common imports httputil, so a shared file would be an import cycle. common/constant.go keeps the nine about auth schemes, redaction and the response cap; httputil/constant.go takes the seven about the call. Both files say why. Flagging it rather than burying it -- say the word and this commit is one revert. Thirteen tests moved to httputil/httputil_test.go as package httputil, not _test, so the four private helpers are still tested directly. common_test.go drops from 2,506 to 2,211 lines. The moved logic is unchanged: comparing code lines only, before and after normalising the rename, 251 against 252 -- the one difference being a single-line `import "strings"` the filter did not recognise. 66 packages green, vet clean, no races.
Each of the ten setting names was written twice in auth.go -- once as a case in
ParseProviderAuth's switch, once as a key in authFields -- and the two had to
stay in sync by hand.
The drift had an asymmetry worth naming. A setting in the switch but not in
authFields is REFUSED at startup, which is loud and fine. A setting in
authFields but not in the switch parsed, validated, and was then discarded:
the operator configured a credential, startup accepted it, and the provider
was called with nothing. Silent, and untested.
authFields is now a map from name to the field it fills, so the name and the
assignment are one entry:
"clientSecretEnv": func(p *AuthProfile, v string) { p.ClientSecretEnv = v },
Adding a scheme is one line and cannot half-land. Membership checks read the
map the same way, so it is still the vocabulary that makes the dash split
decidable.
Constants for the names were the other option. They would share the strings
but leave both lists, so the sync problem stays -- this removes the second list
instead.
Two tests, one per property the map now guarantees: every setting reaches a
field on the profile, found by reflection so a new field cannot be missed
either, and no setting name carries a dash. Negative control: making
clientSecretEnv set nothing fails the first with
`authFields["clientSecretEnv"] set nothing on the profile`.
66 packages green, vet clean.
The httputil move had split constant.go in two, which contradicted the one-constants-file decision. Only one direction could put them back together: common imports httputil, so a constant read by httputil cannot live in common without an import cycle. All sixteen are now in httputil/constant.go and common/constant.go is gone. Four that were package-private are now exported, since common reads them across the boundary: redactedMarker -> httputil.RedactedMarker codeUpstreamUnavailable -> httputil.CodeUpstreamUnavailable tokenRefreshSkew -> httputil.TokenRefreshSkew explainLimit -> httputil.ExplainLimit THE COST IS THE PACKAGE NAME. httputil now holds the auth schemes and the redaction marker, which are not HTTP plumbing, so its name undersells the contents. The file says so. One constants file that can be found beat two that were each in the right place -- a deliberate trade, not an oversight. All 16 verified present with identical values, compared name by name against both files before the merge. The three capability plugins' tests referenced common.AuthSchemeNone and now take it from httputil. 66 packages green, vet clean, no races.
The package stopped being about HTTP when all sixteen constants moved into it: it holds AuthSchemeOAuth2 and RedactedMarker alongside the retry budget, and "httputil" told a reader those were not there. util is plainer and does not promise a subject the contents do not have. 11 files, 93 references, 19 symbols. `util` collides with nothing in the repo. common/util/constant.go every constant, in one file common/util/retry.go Budget, Backoff, Sleep, DoNotRetry, IsPermanent common/util/endpoint.go BuildEndpoint, verifyPath, verifyBaseURL common/util/query.go asQuery, asQueryValue common/util/request.go RequestBody, HasBody, CanonicalMethod common/util/explain.go Explain The package now has a real doc comment, on retry.go, saying what it is and what it is not: a helper belongs beside the code that uses it unless a receiver forbids it. A package called util invites the opposite, so it is written down. CAUGHT BY READING THE DIFF, not by the compiler: the blanket rename rewrote two comments in core/module/handler/stdHandler.go that refer to Go's OWN net/http/httputil -- ReverseProxy and BufferPool. Reverted; that file is untouched and its four httputil references are the standard library's. 66 packages green, vet clean, no races.
The Security Scan gate has been red on every PR since the CVE entered Trivy's database: CVE-2026-84445 google.golang.org/grpc v1.83.1 HIGH fixed in 1.83.2 NOT INTRODUCED HERE. development carries the same version and its own scan passed, having run before the CVE was published. The bump is in this branch because that is where the red gate is; it applies equally to development. golang.org/x/net moves 0.57.0 -> 0.58.0 with it, indirect. go mod tidy leaves the diff at exactly those two lines and four go.sum entries. Verified rather than assumed: trivy fs over the tree now reports ZERO findings at CRITICAL,HIGH,MEDIUM,LOW, where it reported this one before. grpc here is the OTLP exporter's transport, so otelsetup was run separately -- green. 66 packages green with -count=1, vet clean.
A reviewer on #24 found this, and the diagnosis was exact. attempt wrapped every authenticate error in DoNotRetry, with the comment "a missing or unreadable credential is configuration, not weather". That was true before this branch, when authenticate could only fail by reading an unset environment variable. oauth2 made the same path do network I/O to a third-party issuer, so the blanket wrap then swallowed a transient failure: an unreachable token endpoint or a 5xx aborted the request after one attempt, while the IDENTICAL 5xx from the provider is retried twenty lines below. The branch's own test proved it and asserted it as correct: TestOAuth2DoesNotCacheAFailedExchange budgeted two attempts, pointed the issuer at a permanent 500, and checked for exactly one call per request. Classified at the source instead, so attempt no longer has to guess: retried endpoint unreachable, response unreadable, 5xx, 429 not retried tokenUrl that will not parse, 4xx other than 429, a 2xx that is not JSON, no access_token, no usable expires_in not retried an unset credential -- marked in missingCredential, which is shared with basic, header and query and unchanged for them The 5xx-and-429 rule is copied from the provider-response path in the same function rather than invented, so the two agree. TestOAuth2DoesNotCacheAFailedExchange now expects 6 calls for 3 requests rather than 3. Its subject is unchanged -- a cached failure would hold the count at 2 however many requests arrive. Two new tests cover both directions, which is what was missing: 500, 503 and 429 retry; 401 and 400 do not; an unset credential never dials the issuer. Negative control: reinstating the blanket wrap fails exactly the three transient cases and leaves the two permanent ones passing. 66 packages green, vet clean, no races.
feat: add the knowledge capability and give each provider its own credentials [#7]
…evelopment [beckn#91] PR #22's work, replayed onto development and adapted to it. Squashed from 22 commits: 12 of them modified internal/upstream/upstream.go, which development split into seven files, so each would have conflicted as a modify/delete. The net change to the shared package across all 22 was one exported function; the rest was churn -- fan-out added then removed, BindingPaths renamed three times. WHAT THE ADAPTATION NEEDED. development has since renamed internal/upstream to internal/common, folded capabilitybinding into it, and made auth per provider: internal/upstream -> internal/common capabilitybinding.From -> common.BindingFrom, exported for this plugin bindingPaths -> common.BindingPaths, exported for this plugin Config's flat auth fields -> Config.AuthByProvider, one profile per provider authScheme: none -> a pocra: block, nested under the participant id Two of those are exports the plugin needs and this branch adds, with the reason recorded at each: it answers "is this payload mine?" for a multi-type search, and it must answer it the same way serve does rather than reading the payload a second way. The config entry and the steps list gain AgricultureFacility alongside the three that were already there; build-plugins.sh gains it too, rather than replacing KnowledgeAdvisory as the original diff did -- that branch predated the knowledge plugin. Their tests needed the same adaptation. Every helper that builds a Config now declares pocra's auth, since a served provider without a block is refused at startup -- which is the new validation working, not a defect. 69 packages green, vet clean, no races. The other three capabilities were run separately to confirm the two new exports changed nothing for them. Original commits are preserved on feat/91-AgricultualFacility(Plugin) and on PR #22, which is untouched.
…y hand [beckn#91] facilityTypesFrom walked the document in Go -- dig(message, contract, commitments), then [0], then resources, then [0], then resourceAttributes -- with a type assertion and an error message at each level. That compiles the payload's shape into the binary: a spec change needs a rebuild, and eleven lines re-check what a walker already checks. The shape is the network's convention, not this adapter's, so it is now a PATH, defaulting to the Beckn v2 location and overridable per deployment -- exactly as common.Config's providerIdAt and capabilityCodeAt already are: facilityTypesAt: message.contract.commitments[].resources[]. resourceAttributes.supportedFacilityTypes[] The reading is common's walker, the same one that finds a binding key, so the traversal and its absent-field handling live in one place. A test proves the point: a payload carrying the types at search.facilities[].kinds[] is read correctly with nothing but a config path. WHAT THIS NEARLY BROKE, caught by their own tests. ValuesAt drops a non-string leaf, which is right for a binding key and wrong here: supportedFacilityTypes ["KrishiVigyanKendra", 42] would have searched for one type and reported success, a partial answer with nothing recording the loss -- the same class of bug the fan-out work existed to fix. common now exposes LeavesAt alongside ValuesAt: one walker, two views, type policy left to the caller. The plugin takes the leaves and refuses a non-string with the message it always used. An empty supportedFacilityTypes needed care too: it reads as zero leaves like an absent one, and the bare-string fallback would then hand back the empty list itself, reported as "[] is not a facility type" -- true and useless. A leaf that is itself a list is skipped, so an empty list keeps the "names no facility type" message. common's own tests pass untouched, so binding-key extraction is unchanged. 69 packages green, vet clean, no races. dig() remains for now: search.go still uses it to WRITE into a payload when splitting by type, which is a different primitive from reading and not something common offers.
…beckn#91] The plugin stamped a fresh UUID on every part it split out, so POCRA would not blend the answers. That put the reason in Go and the consequence in the mapping, which then had a comment explaining what the Go code had done for it. The mapping derives the id itself now, from the caller's messageId with its last hex digit replaced by a slot the facility type owns. The TYPE picks the slot rather than a call index, because every payload reaching the request half names exactly one type -- so no index has to be passed, and a retry reuses its own slot instead of taking a fresh one. WHY THE CALLER'S OWN ID CANNOT BE SENT, verified live against POCRA: its cache is keyed on message_id, holds for PT10M, and IGNORES LOCATION. The same id asked twice 350km apart returned the first location's facility alongside the second's, which no type filter can catch because the type matches. Reusing it would answer a farmer in Nagpur with a facility near Ahmednagar. Replacing the last character leaves a valid v4 UUID -- it sits in the node field, not the version or variant nibbles -- which POCRA's schema requires. Derived rather than random because JSONata here has no $uuid. That deletes, in order: the UUID stamping, the merged answer's messageId restore (parts never lose the caller's id now), messageIDOf, the uuid import, and dig -- the last hand-written traversal in this package's production code. splitByType writes through the SAME configured path the types were read from, so the read and the write cannot disagree about where they live. setAt and containersAt are local to this package, not added to common: common's step reads payloads and never rewrites one, and a write raises questions a read does not. They create nothing, so a path that does not resolve is a refusal rather than a payload invented to fit. Their own TestEachSearchCallCarriesItsOwnRequestId already asserted the exact property this change has to keep -- two calls, distinct ids, both valid UUIDs, neither the caller's. Negative control: giving two types the same slot fails it with "both calls used message_id ... so POCRA would blend their answers". 69 packages green, vet clean, no races.
… package [beckn#91] internal/concurrent was 116 lines of generic bounded fail-fast machinery plus 288 lines of tests to run one loop, with a single caller, at a configured concurrency of 1. golang.org/x/sync is already a dependency and errgroup does the same job here in a method that reads alongside the code it serves. The clamp moves into searchConcurrency and gains a reason: SetLimit reads a non-positive limit as UNBOUNDED, so a misread setting would fan out over every facility type at once against a provider that rate-limits into silent empty answers. concurrent.Bound treated zero the same way; the comment now says why it matters at the point where it is enforced. Order is kept by indexing the answers rather than appending, so each slot is written by exactly one goroutine and the merge does not depend on which call finished first. No behaviour change: the five search tests that pin sequential-by-default, the configured limit, fail-fast skipping calls not yet issued, one-type-one-call and the ceiling all pass unmodified, under -race.
…kn#91] Both lines were added by the facility plugin's branch and ignore nothing: no docs/ or dev_docs/ directory exists and neither has a tracked file. /docs/ is worse than unnecessary -- it is the conventional place for documentation, so the first person to add any there would find it silently uncommittable, with a rule in an unrelated feature branch to explain it. testdata/.gitignore stays. It ignores schema-cache/, which does exist, and is how schemacache_test.go keeps the fetched schema pack out of the repo instead of vendoring a copy.
…heir own [beckn#91] For an upstream that mints short-lived tokens from its own endpoint rather than an OAuth2 one. The live Agmarknet Vistaar API is the case: POST a JSON body of credentials, get {"token":"<uuid>"} back, send it as a QUERY PARAMETER. Neither existing scheme can serve that, and bending one would have been wrong in five separate ways: oauth2 posts form-encoded client_id/client_secret, reads access_token and expires_in, sets Authorization: Bearer tokenQuery posts JSON under CONFIGURED keys, reads a CONFIGURED key, sets a query parameter -- and the response carries NO expiry The field names are configured rather than fixed because access_name and password are one provider's spelling, not a standard. WHY tokenTtl IS REQUIRED. The response says nothing about how long the token lives, so there is nothing to read and guessing would be inventing a lifetime. An operator states what they believe instead. A ttl at or below the refresh skew is refused: every token would already be expired on arrival, making two round trips per request and hitting the token endpoint at the request rate. AND WHY A WRONG tokenTtl IS NOT AN OUTAGE. Because the lifetime is an estimate, a too-generous one leaves a dead token cached -- and without help every call would fail until it lapsed. A 401 or 403 from the provider now drops the held token, so the next call exchanges a fresh one. One request pays; the next recovers. This applies to oauth2 too, where an issuer can revoke a token before its expires_in runs out. REDACTION follows the placement, not the scheme it borrows from. A query string is the exposed spot -- proxies log it and Go's transport errors quote the whole URL -- so the token's URL-ESCAPED form is covered as well, or one containing + or = survives redaction. The identifier is left readable, as a client id is: it identifies, it does not authenticate. The HTTP half of the exchange is now shared by both schemes in postForToken. The interesting part there is the retry decision -- unreachable, unreadable, 5xx and 429 retry; a 4xx is configuration -- and two copies of that would drift. VERIFIED AGAINST THE LIVE PROVIDER, not just in unit tests: exchanged a real 36-character UUID from the actual token endpoint, placed it as ?token=<uuid>, and confirmed the log line reads token=REDACTED. That check read the endpoint and credentials from the environment and is not committed -- no IP and no credential is written down anywhere. Twenty test cases, and the self-heal one carries a negative control: with the forgetToken call removed it fails with "the rejected token was reused instead of re-exchanged", so it is testing the behaviour rather than passing by accident. Full suite green under -race.
🛡️ Trivy security scan (CRITICAL,HIGH,MEDIUM,LOW)Go dependencies
Container imageNo findings at CRITICAL,HIGH,MEDIUM,LOW. |
|
📊 Test Coverage: ✅ Passed — 84% of changed lines covered, min 80% |
Feat/91 agriculture facility
manjudr
approved these changes
Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Everything on
developmentsincerelease-0.0.1was cut — 128 commits across six merged PRs, 92 files, +19,931/−696.release-0.0.1has no commits of its own that are not already ondevelopment, so this is a clean fast-forward with nothing to reconcile.The bulk of it is
pkg/plugin(59 of the 92 files): four provider plugins that did not exist at the cut, the mapper plugin interface they need, and per-provider credentials so they can each authenticate to a different upstream.What's in it
Provider plugins (#13, #2)
Four new implementations under
pkg/plugin/implementation, plusjsonmapperand thedefinition/mapper.gointerface they are written against:Per-provider auth (#24)
upstream, with the bearer token exchanged and cached, and a retry on a transient exchange failure.internal/upstreamrenamed tointernal/common, withcapabilitybindingfolded into it.grpcbumped to 1.83.2 for CVE-2026-84445.Capability schema conformance (#17, #16)
schemav2validatorreads the list form of@typeand rejects what it cannot read.http/httpsare read for a payload-chosen schema.CI/CD (#14, #20)
Nine workflow files, a
trivy-cachecomposite action andtools/trivy-comment.jq: the Go CI, the release pipeline, plugin build-and-deploy, the GKE deploys, and the fix to the release image scan.Testing
developmentat this HEAD — run 34386967302, success, on the merge of feat: add the knowledge capability and give each provider its own credentials [#7] #24release-0.0.1is strictly behinddevelopment(0 divergent commits) — the merge is a fast-forwardNotes
Target repo. Opened against
OpenAgriNet/network-adapterexplicitly. This clone hasupstreampointed atbeckn/beckn-onixand noghdefault repository set, soghresolves the repo to the public upstream unless--repois passed — worth knowing before the next PR is raised from this directory.One CI failure in the range, on the merge of #2 (34237598324, 2026-09-08). The two later pushes both pass, so it was fixed forward rather than reverted — flagging it because it is inside the range this PR ships.