refactor: read the plugin catalog from marketplace-api - #408
refactor: read the plugin catalog from marketplace-api#408ChielTimmermans wants to merge 1 commit into
Conversation
PR Environment
Commit:
Test credentials (all passwords:
|
876db95 to
014d39a
Compare
There was a problem hiding this comment.
Looks mostly GTM, but see comments below.
Not tied to a line in the diff
console-frontend/public/assets/config/config.json:3
Is console-frontend/public/assets/config/config.json:3 missing marketplaceApiUrl? Then baseUrl is undefined instead of '' when the chart ConfigMap is not mounted. The transport factory throws instead of falling back to a relative URL.
cluster-worker/README.md:256
cluster-worker/README.md:256 still documents PLUGIN_ORGANIZATION_API_URL / ORGANIZATION_API_URL. After the rename, setting those leaves Config.Enabled() false and shoot provisioning silently off.
| dcimApi: https://dcim-api.fundament.projects.digilab.network | ||
| dcim: https://dcim.fundament.projects.digilab.network | ||
| dcimAuthn: https://dcim-authn.fundament.projects.digilab.network | ||
| marketplace: https://marketplace-api.fundament.projects.digilab.network |
There was a problem hiding this comment.
externalUrls.marketplace points at marketplace-api.fundament.projects.digilab.network, but apparently this file has no marketplaceCatalogApi block, so the service defaults to disabled and gets no route. The console's plugins page will fail to load on sandbox.
There was a problem hiding this comment.
Right — added a marketplaceCatalogApi block (enabled: true, replicas: 2, matching the other APIs in that file). The HTTPRoute already comes from pr-overlay.yaml; helm template -f values-sandbox.yaml now renders the Deployment.
| dcimApi: https://dcim-api.fundament.localhost:8443 | ||
| dcim: https://dcim.fundament.localhost:8443 | ||
| dcimAuthn: https://dcim-authn.fundament.localhost:8443 | ||
| marketplace: https://marketplace-catalog-api.fundament.localhost:8443 |
There was a problem hiding this comment.
According to Claude, the management-cluster plugin-controller lost its in-cluster default and now falls back to https://marketplace-catalog-api.fundament.localhost:8443. That host resolves to the host machine's 127.0.0.1:8443 with no CoreDNS rewrite, and the mkcert CA is not trusted inside the pod, so the controller cannot reach the catalog from local dev.
Needs an explicit pluginController.catalogApiUrl: http://marketplace-catalog-api:8080.
There was a problem hiding this comment.
Good catch, and it was a regression: the chart used to default to the in-cluster http://organization-api:8080 and I replaced that with externalUrls.marketplace, which is the external HTTPS host.
Fixed at the source rather than per environment — the chart now defaults to http://marketplace-catalog-api:8080, mirroring the old semantics, so the management-cluster controller stays in-cluster everywhere and only out-of-cluster deployments override. plugins/sandbox/values.yaml keeps its host.k3d.internal:38080 relay, and cluster-worker still passes externalUrls.marketplace for shoots. Verified by rendering all three.
| id: backendPlugin.id, | ||
| name: backendPlugin.name, | ||
| organizationName: backendPlugin.organizationName, | ||
| organizationName: publisherNames.get(backendPlugin.organizationId) ?? '', |
There was a problem hiding this comment.
Comment from Claude about this:
publisherNames.get(...) ?? '' silently yields an empty organizationName, which then feeds install identity and installed-state matching. Same pattern at plugin-details.component.ts:157.
The sibling category mapping three lines down falls back to the id instead, which at least keeps the value locatable. Worth matching that, or surfacing the miss rather than swallowing it.
There was a problem hiding this comment.
Agreed. Both now fall back to the publisher id, matching the category mapping, with a comment on why: the invariant is that every listed plugin's publisher has a live listing (same RLS gates both), so a miss is an inconsistency — and the id keeps it traceable instead of installing under a nameless --<plugin>.
| } | ||
| if p.OrganizationAPIURL == "" { | ||
| if p.CatalogAPIURL == "" { | ||
| return fmt.Errorf("organization-api URL is empty") |
There was a problem hiding this comment.
Error text still names the old field.
| return fmt.Errorf("organization-api URL is empty") | |
| return fmt.Errorf("catalog-api URL is empty") |
014d39a to
ee4116c
Compare
|
Addressed all six — pushed as The two not tied to a diff line:
One I did not touch, for a second opinion: Verification after the changes: |
Moves the storefront readers of organization.v1.PluginService onto catalog.v1, which is what #395 deprecated them for. organization-api keeps serving the RPCs; nothing is deleted here, so the two can be deployed in either order. plugin-controller fetches definitions from marketplace-catalog-api by (organization, plugin) name — the spelling a PluginInstallation carries. ORGANIZATION_API_URL becomes MARKETPLACE_CATALOG_API_URL, defaulting to the in-cluster marketplace-catalog-api Service as the old value did. That rename reaches two deployments: the chart's controller, and the shoot-side one cluster-worker provisions, whose PLUGIN_ORGANIZATION_API_URL becomes PLUGIN_MARKETPLACE_CATALOG_API_URL — a shoot controller would otherwise come up without the URL it now requires. Setting the old pluginController.organizationApiUrl key now fails the render rather than silently falling back to the default. The local sandbox relay (`just plugins sandbox-catalog`) bridges the catalog's NodePort instead of organization-api's. The console's plugins page and plugin detail page read catalog.v1 over their own unauthenticated transport: no cookie, no Fun-Organization header. catalog.v1 returns ids where organization.v1 returned nested names, so publisher and category names are resolved once through ListPublishers and ListCategories rather than per plugin, and the detail page maps the response into a view model so its template is unchanged apart from tags, which are plain labels now. The install modal's version picker reads ListPluginVersions, which returns only published versions — organization.v1's ListPluginDefinitions returned drafts too, so a draft is no longer offered as something to install. Still on organization.v1, deliberately: ListPresets, which has no catalog equivalent, and the three cluster-side views (cluster-plugins, cluster-details, shared-plugins-form) that list plugins to name what is installed — the catalog only returns PUBLIC listings, so moving them would hide an organization's own private plugins from its clusters. Adds externalUrls.marketplace per environment, the console's marketplaceApiUrl config key, and that origin to CONNECT_SRC — without which the browser's CSP blocks every catalog call. Regenerates db/fundament.sql from the model: #395 relaxed the three catalog policies in fundament.dbm and in migration 036 but left the schema of record carrying the old published-only predicates, which the next generated migration would have reverted.
ee4116c to
07fdb82
Compare
Follow-up to #395 (merged). Moves every reader of
organization.v1.PluginServiceontocatalog.v1. organization-api keeps serving the deprecated RPCs, so the two can deploy in either order.plugin-controller
defclientfetches definitions from marketplace-catalog-api by(organization, plugin)name — the spelling aPluginInstallationcarries.ORGANIZATION_API_URL→MARKETPLACE_CATALOG_API_URL.That rename reaches two deployments: the chart's controller and the shoot-side one cluster-worker provisions (
PLUGIN_ORGANIZATION_API_URL→PLUGIN_MARKETPLACE_CATALOG_API_URL). Without the second, a shoot controller would come up missing a URL it now requires — that path landed in #371 while this was in progress. The local sandbox relay is nowjust plugins sandbox-catalog, bridging the catalog's NodePort.console
Browsing, detail and the install modal's version picker read
catalog.v1over a separate unauthenticated transport (no cookie, noFun-Organizationheader).catalog.v1returns ids whereorganization.v1returned nested names, so publisher and category names are resolved once viaListPublishers/ListCategoriesrather than per plugin; the detail page maps into a view model to keep its template intact.One behaviour change:
ListPluginVersionsreturns only published versions, whereListPluginDefinitionsreturned drafts too despite its name. The picker no longer offers a draft as something to install.ListPresetshas no catalog equivalent and stays onorganization.v1.wiring
externalUrls.marketplaceper environment, the console'smarketplaceApiUrlconfig key, and that origin added toCONNECT_SRC— without which the browser's CSP blocks every catalog call.verification
go build ./...,go test ./cluster-worker/... ./plugin-controller/... ./marketplace-api/...,golangci-lint --new-from-rev origin/master— all cleanbunx tsc -p tsconfig.app.json,bun run build,bun run test— tests at baseline (app.spec.tsalready fails on master)bun run lintreports one error insrc/connect/observable-client.ts, untouched here and introduced by 853ab48 on masterhelm templaterenders for local and sandbox values