Migrate to gears#1314
Merged
cyberantonz merged 4 commits intoJun 15, 2026
Merged
Conversation
Flip every modkit/*-cf legacy dep to the renamed cf-gears-* crates on
crates.io. Bumps toolkit family from 0.6.2 to 0.6.7-0.7.4, and system
gears + SDKs to current published versions (gears-rust's local 1-patch-
ahead bumps aren't on crates.io yet, so those are pinned to released).
Workspace aliases also realign with gears-rust convention: modkit* →
toolkit*, module-orchestrator → gear_orchestrator, api-gateway-module →
api_gateway, grpc-hub → grpc_hub, types-registry → types_registry.
Upstream API drift forced two refactors:
* Module trait + #[toolkit::module] macro renamed to Gear +
#[toolkit::gear]; ModuleCtx → GearCtx; BaseModkitPluginV1 → PluginV1.
* toolkit::api::Problem moved — direct import from
toolkit_canonical_errors::Problem.
Drop services/api-gateway/src/core_types.rs: cf-gears-types-registry
now auto-seeds from link-time toolkit-gts inventory at startup, so the
explicit CoreTypes registration gear is redundant.
Drop dead not_found_problem() helper + its two tests from proxy.rs:
they hand-rolled an urn:insight:error:not_found Problem (forbidden by
the canonical-errors rule); function was #[allow(dead_code)]. The live
upstream-failure path moves from Problem::new(BAD_GATEWAY, ...) to
CanonicalError::service_unavailable().with_detail(...).create() per the
canonical-errors closed-category set (no 502 slot).
Also clean up 14 unused deps (10 per-crate + 4 workspace-level) that
fell out of the migration: toolkit-macros / toolkit-http / tower-http /
authz-resolver-sdk had no remaining consumers.
cargo check + clippy -D warnings clean; 289 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
The toolkit bootstrap renamed the root config key from `modules:` to
`gears:` (and `module-orchestrator:` to `gear-orchestrator:`), so the
api-gateway refused to start in docker-compose with
"unknown field: found `modules`" out of figment.
Also drop the `tenants:` list from `single-tenant-tr-plugin.config`:
gears-rust split the legacy plugin into a zero-config
`single-tenant-tr-plugin` (derives the tenant from the SecurityContext
at request time) and a separate `static-tr-plugin` (carries multiple
tenants). Insight only needs the zero-config behavior — when
api-gateway.auth_disabled=true, cf-gears-toolkit-security injects a
SecurityContext with DEFAULT_TENANT_ID = 00000000-df51-5b42-9538-…,
which happens to be the exact UUID insight had hardcoded, so the new
plugin yields the same tenant identity with no config.
Scope:
* config/no-auth.yaml, config/insight.yaml — modules: → gears:,
module-orchestrator: → gear-orchestrator:, drop tenants: list.
* helm/templates/configmap.yaml + secret.yaml — same renames; also
APP__modules__*** → APP__gears__*** env-var prefix.
* auth_info.rs, oidc-authn-plugin/src/module.rs — runtime warning
text "Set modules.<gear>.config…" → "Set gears.<gear>.config…".
* README.md (×2) + DESIGN.md — doc copy alignment ("Module" → "Gear",
APP__modules__ → APP__gears__).
Verified: api-gateway boots cleanly in docker-compose; /health,
/api/v1/auth/config, and the /api/analytics/* proxy all return 200.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
📝 WalkthroughWalkthroughThis pull request migrates the backend from modkit-based crates and module semantics to toolkit/gears equivalents: workspace dependency consolidation, Module→Gear macro/trait rewiring, configuration namespaces moved from ChangesUnified ModKit to Toolkit Migration
🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/backend/plugins/oidc-authn-plugin/src/module.rs (1)
78-86:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe eager JWKS refresh breaks the documented no-auth contract.
This startup fetch still runs unconditionally, but
src/backend/services/api-gateway/config/no-auth.yamldocuments that the placeholderissuer_urlis never contacted whenauth_disabled=true. In the current wiring, no-auth deployments still attempt this network call duringinit(), so the gear keeps a startup-time dependency on JWKS reachability even in the bypassed auth path. Either gate the eager refresh behind an auth-enabled signal, or change the no-auth contract/config so the placeholder endpoint is actually safe to contact. As per coding guidelines, actively cross-reference actual configuration contracts across files rather than checking for renamed values in isolation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/plugins/oidc-authn-plugin/src/module.rs` around lines 78 - 86, The unconditional eager JWKS refresh (toolkit_auth::traits::KeyProvider::refresh_keys called on key_provider during init()) violates the no-auth contract and must be skipped when auth is disabled; update init() to detect the runtime auth-enabled flag (e.g. the configuration boolean that represents auth_disabled / is_auth_enabled) and only call refresh_keys when auth is enabled, otherwise omit the network call (or make it a no-op) so no-auth deployments never contact the placeholder issuer_url.src/backend/plugins/oidc-authn-plugin/README.md (1)
92-106:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFinish the terminology migration in this README section.
This block still says the plugin is discovered by the gateway module and still references the
modkit-authcrate, while the code in this PR has already moved to#[toolkit::gear]andtoolkit_auth. Leaving the old names here will mislead operators during rollout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/plugins/oidc-authn-plugin/README.md` around lines 92 - 106, Update the README terminology: replace references to "gateway module" and "authn-resolver" discovery with the current toolkit gear discovery (mention the #[toolkit::gear] attribute and that the plugin is discovered by the API Gateway via the toolkit types-registry), and replace the old crate name usages (modkit-auth, JwksKeyProvider, validate_claims) with the new crate/tool names (toolkit_auth and the corresponding types/functions from it) while keeping the same flow and symbols like AuthNResolverPluginClient and SecurityContext so operators see the correct runtime discovery and dependency names.src/backend/services/analytics-api/src/api/handlers.rs (1)
6-10:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThese handlers still bypass the canonical rejection mapper.
catalog.rsalready switched toCanonicalJson, but the body-bearing endpoints here still useaxum::Json. That means malformed JSON or missing/wrongContent-Typeon these routes will keep returning Axum's default rejection shape instead of the RFC 9457 envelope this migration is introducing.Suggested fix pattern
-use axum::Json; +use axum::Json; ... use super::AppState; +use super::canonical_json::CanonicalJson; use super::error::{MetricError, PersonError, ThresholdError};- Json(req): Json<CreateMetricRequest>, + CanonicalJson(req): CanonicalJson<CreateMetricRequest>,Apply the same extractor swap to the other request-body handlers in this file:
update_metricquery_metricquery_metrics_batchcreate_thresholdupdate_thresholdAlso applies to: 93-97, 138-143, 197-211, 969-974, 1031-1036
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/services/analytics-api/src/api/handlers.rs` around lines 6 - 10, Handlers that accept request bodies are still using axum::Json and therefore bypass the canonical rejection mapper; replace axum::Json with the canonical extractor so malformed JSON and wrong Content-Type return the RFC 9457 envelope. For each listed handler (update_metric, query_metric, query_metrics_batch, create_threshold, update_threshold — and the other occurrences around lines referenced: 93-97, 138-143, 197-211, 969-974, 1031-1036) change the parameter type from axum::Json<T> to toolkit_canonical_errors::CanonicalJson<T> (import it), and adjust the handler body to use the inner T (e.g., let req = body.into_inner()) if needed; also remove or update any axum::Json imports so the canonical extractor is used consistently.src/backend/services/api-gateway/README.md (1)
44-54:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFinish migrating the auth-info env-var examples.
This section now documents the
APP__gears__...convention, but the same README still later tells operators to setOIDC_CLIENT_IDandOIDC_REDIRECT_URIfor/auth/config. That no longer matches the Secret template in this PR, which populatesAPP__gears__auth-info__config__client_idandAPP__gears__auth-info__config__redirect_uri. Please update the later example so the README has one consistent configuration story.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/services/api-gateway/README.md` around lines 44 - 54, Update the README example for the /auth/config operator instructions to use the migrated APP__gears__... env var names so it matches the Secret template: replace references to OIDC_CLIENT_ID and OIDC_REDIRECT_URI with APP__gears__auth-info__config__client_id and APP__gears__auth-info__config__redirect_uri (and similarly any other OIDC_* examples) and ensure the /auth/config example and Secret template consistently reference the same APP__gears__auth-info__config__* keys.src/backend/services/api-gateway/src/proxy.rs (1)
13-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale YAML example in module documentation.
The example configuration in the doc comment still uses
modules:but should now begears:to match the migration.📝 Suggested fix
//! # Configuration //! //! ```yaml -//! modules: +//! gears: //! proxy:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/services/api-gateway/src/proxy.rs` around lines 13 - 14, The module-level doc comment in proxy.rs contains a stale YAML example using the old key "modules:"; update that example to use the new key "gears:" (e.g., replace the leading "modules:" line in the //! doc comment for the proxy module with "gears:") so the documentation matches the migrated configuration schema.src/backend/services/api-gateway/src/auth_info.rs (1)
44-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale environment variable path in documentation comment.
The comment references the old
APP__modules__auth-info__config__scopespath, but the migration changes this toAPP__gears__auth-info__config__scopes. This inconsistency could confuse developers trying to override the config.📝 Suggested fix
/// Scopes to request, as a space-separated string (matches OAuth2's wire /// format). Stored as `String` so the standard - /// `APP__modules__auth-info__config__scopes` env-var override works + /// `APP__gears__auth-info__config__scopes` env-var override works /// without a custom Vec deserializer; split on whitespace when building🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/services/api-gateway/src/auth_info.rs` around lines 44 - 46, Update the stale documentation comment that mentions the old env-var path `APP__modules__auth-info__config__scopes` to the new migrated path `APP__gears__auth-info__config__scopes` in the auth info code (the doc comment that describes storing scopes as a `String` and splitting on whitespace); locate the comment string in auth_info.rs and replace the old env-var token with the new one so the documentation matches the migration.
🧹 Nitpick comments (2)
src/backend/Cargo.toml (1)
83-96: Pin the remainingcf-gears-*crates to exact versions for reproducible lockfile refreshes.
Cargo.lockcurrently resolves these crates to the exact versions listed inCargo.toml, but leaving caret ranges would still allow them to advance on a future lockfile update.Suggested change
-authn-resolver-sdk = { package = "cf-gears-authn-resolver-sdk", version = "0.3.17" } -types-registry-sdk = { package = "cf-gears-types-registry-sdk", version = "0.2.3" } +authn-resolver-sdk = { package = "cf-gears-authn-resolver-sdk", version = "=0.3.17" } +types-registry-sdk = { package = "cf-gears-types-registry-sdk", version = "=0.2.3" } -api_gateway = { package = "cf-gears-api-gateway", version = "0.2.8" } -authn-resolver = { package = "cf-gears-authn-resolver", version = "0.2.17" } -authz-resolver = { package = "cf-gears-authz-resolver", version = "0.1.24" } -tenant-resolver = { package = "cf-gears-tenant-resolver", version = "0.1.21" } -static-authz-plugin = { package = "cf-gears-static-authz-plugin", version = "0.1.20" } -single-tenant-tr-plugin = { package = "cf-gears-single-tenant-tr-plugin", version = "0.1.22" } -grpc_hub = { package = "cf-gears-grpc-hub", version = "0.2.7" } -gear_orchestrator = { package = "cf-gears-gear-orchestrator", version = "0.1.26" } -types_registry = { package = "cf-gears-types-registry", version = "0.1.22" } +api_gateway = { package = "cf-gears-api-gateway", version = "=0.2.8" } +authn-resolver = { package = "cf-gears-authn-resolver", version = "=0.2.17" } +authz-resolver = { package = "cf-gears-authz-resolver", version = "=0.1.24" } +tenant-resolver = { package = "cf-gears-tenant-resolver", version = "=0.1.21" } +static-authz-plugin = { package = "cf-gears-static-authz-plugin", version = "=0.1.20" } +single-tenant-tr-plugin = { package = "cf-gears-single-tenant-tr-plugin", version = "=0.1.22" } +grpc_hub = { package = "cf-gears-grpc-hub", version = "=0.2.7" } +gear_orchestrator = { package = "cf-gears-gear-orchestrator", version = "=0.1.26" } +types_registry = { package = "cf-gears-types-registry", version = "=0.1.22" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/Cargo.toml` around lines 83 - 96, Update the listed cf-gears crate dependencies so they use exact-version pinning instead of a semver range: for each dependency symbol (authn-resolver-sdk, types-registry-sdk, api_gateway, authn-resolver, authz-resolver, tenant-resolver, static-authz-plugin, single-tenant-tr-plugin, grpc_hub, gear_orchestrator, types_registry) change the version field to an exact pin (e.g., version = "=0.3.17") so future Cargo.lock refreshes cannot advance those crates.src/backend/services/api-gateway/src/auth_info.rs (1)
91-93: 💤 Low valueError message still references "module" terminology.
The error message says "auth-info module already initialized" but this is now a gear. Consider updating for consistency with the new framework terminology.
📝 Suggested fix
self.config .set(Arc::new(config)) - .map_err(|_| anyhow::anyhow!("auth-info module already initialized"))?; + .map_err(|_| anyhow::anyhow!("auth-info gear already initialized"))?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/services/api-gateway/src/auth_info.rs` around lines 91 - 93, Update the error message text to use the new framework terminology ("gear" instead of "module") where the config is set; locate the call on self.config.set(Arc::new(config)).map_err(|_| anyhow::anyhow!("auth-info module already initialized"))? and change the string to something like "auth-info gear already initialized" (or similar consistent phrasing) so the error reflects current terminology.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/plugins/oidc-authn-plugin/src/domain/service.rs`:
- Around line 6-10: The import block containing KeyProvider, JwksKeyProvider,
ValidationConfig, validate_claims, SecurityContext, SecretString and Arc is not
formatted per rustfmt; run rustfmt/cargo fmt to reorder and format the use
statements (or manually reorder them to match rustfmt rules: std imports first,
extern crates grouped, and items alphabetized within groups) so the imports for
toolkit_auth, toolkit_security, secrecy and std::sync::Arc conform to rustfmt
and resolve the CI failure.
In `@src/backend/services/analytics-api/src/api/admin/error_map.rs`:
- Around line 40-43: Imports in the top of error_map.rs are misformatted causing
cargo fmt --check to fail; run rustfmt (cargo fmt --all) or reorder/format the
use statements to match rustfmt's style (e.g., proper grouping and whitespace)
for the items listed: axum::http::{HeaderValue, header},
axum::response::{IntoResponse, Response},
toolkit_canonical_errors::{CanonicalError, Problem}, and sea_orm::DbErr so the
file passes formatting checks.
In `@src/backend/services/api-gateway/helm/templates/secret.yaml`:
- Around line 10-12: The Helm template is requiring .Values.oidc.issuer while
the chart README uses oidc.issuerUrl; update the template references used for
APP__gears__oidc-authn-plugin__config__issuer_url and
APP__gears__auth-info__config__issuer_url to use .Values.oidc.issuerUrl (and
update any required(...) messages accordingly) so the Secret reads the same
values key the docs advertise; verify
APP__gears__oidc-authn-plugin__config__audience still uses .Values.oidc.audience
and adjust docs or template consistently if other keys differ.
---
Outside diff comments:
In `@src/backend/plugins/oidc-authn-plugin/README.md`:
- Around line 92-106: Update the README terminology: replace references to
"gateway module" and "authn-resolver" discovery with the current toolkit gear
discovery (mention the #[toolkit::gear] attribute and that the plugin is
discovered by the API Gateway via the toolkit types-registry), and replace the
old crate name usages (modkit-auth, JwksKeyProvider, validate_claims) with the
new crate/tool names (toolkit_auth and the corresponding types/functions from
it) while keeping the same flow and symbols like AuthNResolverPluginClient and
SecurityContext so operators see the correct runtime discovery and dependency
names.
In `@src/backend/plugins/oidc-authn-plugin/src/module.rs`:
- Around line 78-86: The unconditional eager JWKS refresh
(toolkit_auth::traits::KeyProvider::refresh_keys called on key_provider during
init()) violates the no-auth contract and must be skipped when auth is disabled;
update init() to detect the runtime auth-enabled flag (e.g. the configuration
boolean that represents auth_disabled / is_auth_enabled) and only call
refresh_keys when auth is enabled, otherwise omit the network call (or make it a
no-op) so no-auth deployments never contact the placeholder issuer_url.
In `@src/backend/services/analytics-api/src/api/handlers.rs`:
- Around line 6-10: Handlers that accept request bodies are still using
axum::Json and therefore bypass the canonical rejection mapper; replace
axum::Json with the canonical extractor so malformed JSON and wrong Content-Type
return the RFC 9457 envelope. For each listed handler (update_metric,
query_metric, query_metrics_batch, create_threshold, update_threshold — and the
other occurrences around lines referenced: 93-97, 138-143, 197-211, 969-974,
1031-1036) change the parameter type from axum::Json<T> to
toolkit_canonical_errors::CanonicalJson<T> (import it), and adjust the handler
body to use the inner T (e.g., let req = body.into_inner()) if needed; also
remove or update any axum::Json imports so the canonical extractor is used
consistently.
In `@src/backend/services/api-gateway/README.md`:
- Around line 44-54: Update the README example for the /auth/config operator
instructions to use the migrated APP__gears__... env var names so it matches the
Secret template: replace references to OIDC_CLIENT_ID and OIDC_REDIRECT_URI with
APP__gears__auth-info__config__client_id and
APP__gears__auth-info__config__redirect_uri (and similarly any other OIDC_*
examples) and ensure the /auth/config example and Secret template consistently
reference the same APP__gears__auth-info__config__* keys.
In `@src/backend/services/api-gateway/src/auth_info.rs`:
- Around line 44-46: Update the stale documentation comment that mentions the
old env-var path `APP__modules__auth-info__config__scopes` to the new migrated
path `APP__gears__auth-info__config__scopes` in the auth info code (the doc
comment that describes storing scopes as a `String` and splitting on
whitespace); locate the comment string in auth_info.rs and replace the old
env-var token with the new one so the documentation matches the migration.
In `@src/backend/services/api-gateway/src/proxy.rs`:
- Around line 13-14: The module-level doc comment in proxy.rs contains a stale
YAML example using the old key "modules:"; update that example to use the new
key "gears:" (e.g., replace the leading "modules:" line in the //! doc comment
for the proxy module with "gears:") so the documentation matches the migrated
configuration schema.
---
Nitpick comments:
In `@src/backend/Cargo.toml`:
- Around line 83-96: Update the listed cf-gears crate dependencies so they use
exact-version pinning instead of a semver range: for each dependency symbol
(authn-resolver-sdk, types-registry-sdk, api_gateway, authn-resolver,
authz-resolver, tenant-resolver, static-authz-plugin, single-tenant-tr-plugin,
grpc_hub, gear_orchestrator, types_registry) change the version field to an
exact pin (e.g., version = "=0.3.17") so future Cargo.lock refreshes cannot
advance those crates.
In `@src/backend/services/api-gateway/src/auth_info.rs`:
- Around line 91-93: Update the error message text to use the new framework
terminology ("gear" instead of "module") where the config is set; locate the
call on self.config.set(Arc::new(config)).map_err(|_| anyhow::anyhow!("auth-info
module already initialized"))? and change the string to something like
"auth-info gear already initialized" (or similar consistent phrasing) so the
error reflects current terminology.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99c5b408-2aaa-4375-9c9a-bd9743a1dafa
⛔ Files ignored due to path filters (1)
src/backend/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
src/backend/Cargo.tomlsrc/backend/libs/insight-clickhouse/Cargo.tomlsrc/backend/plugins/oidc-authn-plugin/Cargo.tomlsrc/backend/plugins/oidc-authn-plugin/README.mdsrc/backend/plugins/oidc-authn-plugin/src/domain/service.rssrc/backend/plugins/oidc-authn-plugin/src/module.rssrc/backend/services/analytics-api/Cargo.tomlsrc/backend/services/analytics-api/src/api/admin/error_map.rssrc/backend/services/analytics-api/src/api/canonical_json.rssrc/backend/services/analytics-api/src/api/catalog.rssrc/backend/services/analytics-api/src/api/error.rssrc/backend/services/analytics-api/src/api/handlers.rssrc/backend/services/analytics-api/src/auth.rssrc/backend/services/analytics-api/src/domain/query.rssrc/backend/services/api-gateway/Cargo.tomlsrc/backend/services/api-gateway/README.mdsrc/backend/services/api-gateway/config/insight.yamlsrc/backend/services/api-gateway/config/no-auth.yamlsrc/backend/services/api-gateway/helm/templates/configmap.yamlsrc/backend/services/api-gateway/helm/templates/secret.yamlsrc/backend/services/api-gateway/specs/DESIGN.mdsrc/backend/services/api-gateway/src/auth_info.rssrc/backend/services/api-gateway/src/core_types.rssrc/backend/services/api-gateway/src/main.rssrc/backend/services/api-gateway/src/proxy.rs
💤 Files with no reviewable changes (2)
- src/backend/services/api-gateway/src/core_types.rs
- src/backend/libs/insight-clickhouse/Cargo.toml
Comment on lines
6
to
10
| use toolkit_auth::traits::KeyProvider; | ||
| use toolkit_auth::{JwksKeyProvider, ValidationConfig, validate_claims}; | ||
| use toolkit_security::SecurityContext; | ||
| use secrecy::SecretString; | ||
| use std::sync::Arc; |
There was a problem hiding this comment.
Run cargo fmt on this import block.
CI is already failing on this file because rustfmt wants the use items reordered here. Please format before merge.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/plugins/oidc-authn-plugin/src/domain/service.rs` around lines 6 -
10, The import block containing KeyProvider, JwksKeyProvider, ValidationConfig,
validate_claims, SecurityContext, SecretString and Arc is not formatted per
rustfmt; run rustfmt/cargo fmt to reorder and format the use statements (or
manually reorder them to match rustfmt rules: std imports first, extern crates
grouped, and items alphabetized within groups) so the imports for toolkit_auth,
toolkit_security, secrecy and std::sync::Arc conform to rustfmt and resolve the
CI failure.
Source: Pipeline failures
Comment on lines
+10
to
+12
| APP__gears__oidc-authn-plugin__config__issuer_url: {{ required "oidc.issuer is required when auth is enabled and oidc.existingSecret is not set" .Values.oidc.issuer | quote }} | ||
| APP__gears__oidc-authn-plugin__config__audience: {{ .Values.oidc.audience | quote }} | ||
| APP__gears__auth-info__config__issuer_url: {{ required "oidc.issuer is required when auth is enabled and oidc.existingSecret is not set" .Values.oidc.issuer | quote }} |
There was a problem hiding this comment.
Align the Helm values key with the documented key.
Line 10 and Line 12 require .Values.oidc.issuer, but the README examples for this chart use oidc.issuerUrl. As written, an operator can follow the README and still fail this template’s required(...) check because the Secret is reading a different values key. Standardize the key name across this template and the chart docs before merge.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/services/api-gateway/helm/templates/secret.yaml` around lines 10
- 12, The Helm template is requiring .Values.oidc.issuer while the chart README
uses oidc.issuerUrl; update the template references used for
APP__gears__oidc-authn-plugin__config__issuer_url and
APP__gears__auth-info__config__issuer_url to use .Values.oidc.issuerUrl (and
update any required(...) messages accordingly) so the Secret reads the same
values key the docs advertise; verify
APP__gears__oidc-authn-plugin__config__audience still uses .Values.oidc.audience
and adjust docs or template consistently if other keys differ.
The bootstrap rename touched the helm chart and the runtime configs but
missed three operator-facing surfaces that also follow the env-var
schema: the secrets-store.yaml.sample (External Secrets / Vault
template), the secrets/oidc.yaml.example (kustomize Secret example),
and a stale `APP__modules__auth-info__config__scopes` reference in the
`scopes` field doc-comment in auth_info.rs.
Operators copying these into real Secrets/ESO entries with the old
prefix would have produced a Secret whose env-var keys the new
`cf-gears-toolkit` bootstrap silently ignores (figment doesn't know
the prefix), leaving OIDC unconfigured at runtime.
Note: live SealedSecret YAMLs in insight-gitops
(`environments/{dev,local,virtuozzo}/sealed-secrets/insight/insight-oidc-sealedsecret.yaml`)
still carry `APP__modules__*` keys. They need re-sealing from the
Passbolt plaintexts with the new `APP__gears__*` prefix as a separate
ops step before the cf→gears rollout reaches each cluster.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Anton Zelenov <antonz@constructor.tech>
…1843 comment `cargo fmt` reordered the `use` blocks touched by the cf→gears rename into canonical alphabetical groups (std → external → internal). No behavior change. Also remove an orphaned doc comment in proxy.rs that referenced the not-yet-disabled-here authenticated 404 fallback (whose helper + tests I deleted earlier in the migration); the explanatory text was left dangling above the `tracing::warn!` and ended mid-sentence. The warn message above is enough on its own. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/services/api-gateway/secrets/oidc.yaml.example (1)
3-3:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the env-key format comment to match the migrated
gearsschema.Line [3] still documents
APP__<module>__config__<key>, but this file now usesAPP__gears__<gear-name>__config__<key>. Keeping the old placeholder can cause misconfigured secrets.Suggested patch
-# Keys are API Gateway env-var overrides (APP__<module>__config__<key>). +# Keys are API Gateway env-var overrides (APP__gears__<gear-name>__config__<key>).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/services/api-gateway/secrets/oidc.yaml.example` at line 3, Update the explanatory comment that currently reads "APP__<module>__config__<key>" to the new schema used by the migrated gears system by replacing it with "APP__gears__<gear-name>__config__<key>" so the env-var override format documented in the secrets template matches the actual keys used; specifically, change the placeholder text "APP__<module>__config__<key>" in the top comment to "APP__gears__<gear-name>__config__<key>".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/backend/services/api-gateway/secrets/oidc.yaml.example`:
- Line 3: Update the explanatory comment that currently reads
"APP__<module>__config__<key>" to the new schema used by the migrated gears
system by replacing it with "APP__gears__<gear-name>__config__<key>" so the
env-var override format documented in the secrets template matches the actual
keys used; specifically, change the placeholder text
"APP__<module>__config__<key>" in the top comment to
"APP__gears__<gear-name>__config__<key>".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba65ba3b-48a2-421b-8184-a0240c601f87
📒 Files selected for processing (10)
docs/deploy/secrets-store.yaml.samplesrc/backend/plugins/oidc-authn-plugin/src/domain/service.rssrc/backend/services/analytics-api/src/api/admin/error_map.rssrc/backend/services/analytics-api/src/api/canonical_json.rssrc/backend/services/analytics-api/src/api/handlers.rssrc/backend/services/analytics-api/src/domain/query.rssrc/backend/services/api-gateway/secrets/oidc.yaml.examplesrc/backend/services/api-gateway/src/auth_info.rssrc/backend/services/api-gateway/src/main.rssrc/backend/services/api-gateway/src/proxy.rs
✅ Files skipped from review due to trivial changes (2)
- docs/deploy/secrets-store.yaml.sample
- src/backend/services/analytics-api/src/api/canonical_json.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- src/backend/services/analytics-api/src/domain/query.rs
- src/backend/services/analytics-api/src/api/handlers.rs
- src/backend/plugins/oidc-authn-plugin/src/domain/service.rs
- src/backend/services/api-gateway/src/main.rs
- src/backend/services/analytics-api/src/api/admin/error_map.rs
- src/backend/services/api-gateway/src/proxy.rs
mitasovr
approved these changes
Jun 15, 2026
Contributor
|
@cyberantonz — flagging an overlap so we don't double-edit the same files. The gears migration here touches files that QA PRs already in review also change:
Suggested order: land #1314 first, then rebase #1327/#1333 so the tests assert the new gears config/keys instead of the old shape. Happy to do that rebase once this merges, or to review the auth-config/query changes against the existing tests beforehand if that's easier. Just want to avoid conflicting edits on the same files. |
This was referenced Jun 15, 2026
This was referenced Jun 15, 2026
SharedQA
added a commit
to SharedQA/insight
that referenced
this pull request
Jun 15, 2026
…s (22 tests) Rebased onto main after the cf-modkit -> cf-gears-toolkit migration (constructorfabric#1314), which restructured these files. Adapts the suite to the gears API: - auth_info: re-extract AuthInfoResponse::from_config as a pure fn (called by register_rest) so scope-splitting / response_type / config parsing stay unit-testable; constructorfabric#1314 had inlined it into the gear method. 9 tests. - proxy: keep is_hop_by_hop, RouteConfig/ProxyConfig defaults, and forward_request plumbing (prefix strip, query, end-to-end headers). The unreachable-upstream path is now a canonical 503 (was 502); assert that. Drop the not_found_problem / RFC9457 fallback tests — gears removed that helper (404 fallback 'not yet wired up against cf-gears-api-gateway'). 8 tests. - main: clap CLI surface — subcommands, flags, verbosity, unknown-flag. 5 tests. - Cargo.toml: swap unused tower dev-dep for serde_json. cargo test + clippy -D warnings + fmt all clean against current main. Signed-off-by: Kenan Salim <kenan.salim@rolos.com>
This was referenced Jul 1, 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
3 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 by CodeRabbit
Documentation
Chores
Behavior