feat(pulumi-esc): add pulumi esc provider and lease backend - #589
feat(pulumi-esc): add pulumi esc provider and lease backend#589NorthIsUp wants to merge 19 commits into
Conversation
📝 WalkthroughWalkthroughAdds Pulumi ESC support across fnox-core with a shared REST client, provider and lease backend implementations, CLI/schema/docs wiring, profile-aware provider resolution, and updated multi-backend lease error handling. ChangesPulumi ESC Integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant PulumiEscProvider
participant EscClient
participant PulumiCloudAPI
Caller->>PulumiEscProvider: get_secret or get_secrets_batch
PulumiEscProvider->>EscClient: open_and_read environment reference
EscClient->>PulumiCloudAPI: POST /open
PulumiCloudAPI-->>EscClient: session id
EscClient->>PulumiCloudAPI: GET /open/{session_id}
PulumiCloudAPI-->>EscClient: resolved environment JSON
EscClient-->>PulumiEscProvider: environment properties
PulumiEscProvider-->>Caller: requested secret values
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds a Pulumi ESC secrets provider and lease backend, sharing a common
Confidence Score: 5/5Safe to merge. The new provider and lease backend follow established patterns throughout the codebase, the auth token is properly threaded to all prerequisite checks, and the PR has been validated in production for over a month. All three new modules are internally consistent, follow project conventions (centralized env::var, tracing logging, FnoxError domain errors), and are backed by focused unit tests. The previously identified gap—config-level token not being threaded to check_prerequisites and required_env_vars—has been properly resolved. The single-backend error improvement in lease.rs is also correct and safe. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "fix(pulumi-esc): address review — honor ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/fnox-core/src/pulumi_esc_api.rs (1)
58-95: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueBlocking file I/O inside an async call path.
resolve_auth()usesstd::fs::read_to_stringsynchronously, and it's invoked fromEscClient::new, which is called from async provider/lease-backend methods. This blocks the async executor thread for the duration of the read. Given this is a small local credentials file and fnox is a CLI tool (not a long-lived server), the practical impact is minimal, but considertokio::fs::read_to_stringorspawn_blockingfor correctness under the async runtime's cooperative scheduling model.Also applies to: 170-178
🤖 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 `@crates/fnox-core/src/pulumi_esc_api.rs` around lines 58 - 95, resolve_auth() currently performs blocking file I/O with std::fs::read_to_string inside an async call path, which can stall the executor when EscClient::new is reached from async provider and lease-backend methods. Update resolve_auth() to use async-friendly file loading, such as tokio::fs::read_to_string or wrapping the credential read in spawn_blocking, and keep the existing error handling and EscAuth construction intact.crates/fnox-core/src/lease_backends/pulumi_esc.rs (1)
111-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting credential extraction/filtering for direct unit testing.
The filter-vs-surface-all branching and
coerce_scalarusage here are only exercised indirectly (viaresolve_refstests elsewhere); a pure helper taking theenv_vars_objmap and optional filter, returning the resultingIndexMap, would let this logic be unit tested without mockingEscClient.🤖 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 `@crates/fnox-core/src/lease_backends/pulumi_esc.rs` around lines 111 - 158, The credential extraction logic inside the Pulumi ESC resolution flow is hard to unit test because it is embedded in the `resolve_refs` path and coupled to `EscClient`. Extract the filter-vs-surface-all loop and `pulumi_esc_api::coerce_scalar` handling into a pure helper that takes the `env_vars_obj` map plus the optional `self.env_vars` filter and returns the `IndexMap` of credentials. Then update `resolve_refs` to call that helper and add direct unit tests for the helper covering filtered, unfiltered, missing, and non-scalar values.
🤖 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 `@crates/fnox-core/src/lease_backends/pulumi_esc.rs`:
- Around line 66-95: The `resolve_refs` function is leaking secret material by
embedding the full `raw` interpolation input in the `ProviderInvalidResponse`
for unterminated markers. Update the error construction in `resolve_refs` to
omit `raw` from `details` and use only a generic message that identifies the
malformed `{sigil}{...` sequence; keep the existing `path`-based unresolved
reference message unchanged. This should prevent sensitive ESC
`environmentVariables` values from being echoed through `run_all` error output.
In `@docs/public/schema.json`:
- Around line 1137-1162: The pulumi-esc ProviderConfig schema variant is missing
the daemon_cache property, so configs using ProviderConfig::PulumiEsc can fail
validation due to additionalProperties being false. Update the pulumi-esc object
schema in schema.json to include daemon_cache alongside auth_command,
environment, organization, project, token, and type, matching the
ProviderConfig::PulumiEsc field so validation accepts it.
---
Nitpick comments:
In `@crates/fnox-core/src/lease_backends/pulumi_esc.rs`:
- Around line 111-158: The credential extraction logic inside the Pulumi ESC
resolution flow is hard to unit test because it is embedded in the
`resolve_refs` path and coupled to `EscClient`. Extract the
filter-vs-surface-all loop and `pulumi_esc_api::coerce_scalar` handling into a
pure helper that takes the `env_vars_obj` map plus the optional `self.env_vars`
filter and returns the `IndexMap` of credentials. Then update `resolve_refs` to
call that helper and add direct unit tests for the helper covering filtered,
unfiltered, missing, and non-scalar values.
In `@crates/fnox-core/src/pulumi_esc_api.rs`:
- Around line 58-95: resolve_auth() currently performs blocking file I/O with
std::fs::read_to_string inside an async call path, which can stall the executor
when EscClient::new is reached from async provider and lease-backend methods.
Update resolve_auth() to use async-friendly file loading, such as
tokio::fs::read_to_string or wrapping the credential read in spawn_blocking, and
keep the existing error handling and EscAuth construction intact.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 6fd0b112-ebd9-4222-b664-c1fa2a5f020a
📒 Files selected for processing (18)
crates/fnox-core/providers/pulumi-esc.tomlcrates/fnox-core/src/lease_backends/mod.rscrates/fnox-core/src/lease_backends/pulumi_esc.rscrates/fnox-core/src/lib.rscrates/fnox-core/src/providers/mod.rscrates/fnox-core/src/providers/pulumi_esc.rscrates/fnox-core/src/pulumi_esc_api.rsdocs/.vitepress/config.mjsdocs/cli/commands.jsondocs/cli/provider/add.mddocs/leases/pulumi-esc.mddocs/providers/overview.mddocs/providers/pulumi-esc.mddocs/public/schema.jsonfnox.usage.kdlsrc/commands/lease.rssrc/commands/provider/add.rssrc/commands/provider/mod.rs
|
This PR currently has merge conflicts. If this continues for 7 days, it will be closed automatically. This is warning day 1 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
Provider fetches individual values from ESC environments via `esc env get <ref> <path> --value string --show-secrets`, with a batch path that opens the environment once and extracts dot-delimited paths from the resolved JSON. Lease backend runs `esc env open --format json` and surfaces entries from `environmentVariables` so ESC-minted dynamic credentials (AWS OIDC, GCP OIDC, etc.) can be consumed through fnox's lease system. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace tokio::process::Command shell-out with direct calls to the
Pulumi ESC REST API (POST /api/esc/environments/{ref}/open, then GET
.../open/{id}). Removes the hard dependency on the `esc` CLI for lease
creation.
Credential resolution now mirrors the esc CLI: config token →
FNOX_PULUMI_ACCESS_TOKEN → PULUMI_ACCESS_TOKEN → $PULUMI_HOME or
~/.pulumi credentials.json. The `current` backend URL from the file is
honored so self-hosted Pulumi Cloud works without extra config.
check_prerequisites runs the real resolver instead of the broken
"must have PULUMI_ACCESS_TOKEN" check.
Add an `interpolate` option (char, default none) on the pulumi-esc
lease backend. When set (e.g. `interpolate = "%"`), single-pass
`<sigil>{path}` substitution runs on each surfaced env var, resolving
against the response's properties tree. Missing references are a hard
error. No recursion — `%{foo.%{bar}}` parses as `%{foo.%{bar}` and
errors on lookup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extract the shared ESC REST client into `src/pulumi_esc_api.rs`
(credential discovery, env_ref builder, `EscClient` with `open`/`read`,
and a `properties`-tree `lookup` walker). Both the lease backend and
the secret provider now drive this module instead of shelling out to
the `esc` CLI.
Provider: fetches individual values via POST /open + GET /open/{id},
extracting dot-delimited paths locally (both single-get and batch).
Drops `tokio::process::Command`, the `classify_cli_error` heuristics,
and the separate `walk`/`extract_path`/`value_to_string` helpers —
the shared `lookup` walker covers all paths the CLI JSON format did.
Lease backend: same refactor, plus it now re-uses the shared auth
resolver for `check_prerequisites`. `resolve_refs` and `extract_value`
stay local since they're lease-specific.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- docs: both pulumi-esc.md files said the `esc` CLI was a prerequisite
and described the old CLI shell-out (`esc env open --format json`,
`esc env get ... --show-secrets`). Rewritten to reflect the native
HTTP path. Kept `esc login` as one of the auth options since it
populates `~/.pulumi/credentials.json`, which fnox still reads.
- `EscClient::open` now sends `Content-Type: application/json` and an
explicit `{}` body on the POST so middleware/proxies that enforce
a media type don't reject with 415.
- `pulumi_esc_api::lookup` now early-returns None for empty paths so
the intent is explicit rather than relying on `cur.get("")`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses PR jdx#429 review feedback: - Revert dev version bump in Cargo.toml / fnox.usage.kdl (jdx flagged as unintentional). The fork-only dev tag is no longer needed. - Switch std::env::var → crate::env::var for PULUMI_HOME and PULUMI_BACKEND_URL to match the rest of the codebase (gemini-code-assist). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pulumi Cloud org/project/env names are typically [a-zA-Z0-9-], but the env ref is interpolated directly into the request path. Encoding each segment is cheap insurance against a corrupted URL if a name ever contains `/`, `?`, `#`, or whitespace. Addresses Greptile P2 on PR jdx#429. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The provider metadata still pointed at the esc CLI for auth-prompting and setup — contradicting the rest of the PR where fnox talks to the Pulumi Cloud REST API directly. Drop auth_command (no CLI to invoke) and rewrite setup_instructions to describe the token / credentials.json paths. Addresses Greptile P1 on PR jdx#429. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Move `## Authentication` to `### Authentication` under `## Configuration` - Add missing `## Usage` section - Rename `## Leases (dynamic credentials)` to `## Leases` - Add `### GitHub Actions` subsection under `## CI/CD Example` https://claude.ai/code/session_016UhCuqGUPhdmZ5sYXe3Y15
…ction Module lives in crates/fnox-core/src/pulumi_esc_api.rs after the fnox-core crate refactor. https://claude.ai/code/session_016UhCuqGUPhdmZ5sYXe3Y15
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eview
The percent-encoding fix for env-ref path segments was applied to the
stale `src/pulumi_esc_api.rs` before the crate extraction and silently
lost when that file was deleted. `build_env_ref` shipped unencoded,
interpolating org/project/env straight into the request path. Restore
per-segment `urlencoding::encode` (the canonical idiom, as in
passwordstate) and add a test.
Quality cleanups from a code-quality review:
- Collapse the duplicate `{value, trace}` scalar coercion: the lease
backend's `extract_value` and the api's inline match are now one
shared `pulumi_esc_api::coerce_scalar`; `lookup` reuses it.
- Replace the dishonest `(String, String)` auth tuple with a named
`EscAuth { base, token }` struct.
- Add `EscClient::{api_err, invalid_err}` helpers to remove the repeated
provider/url error plumbing in `open`/`read`/`http_error`.
- Use `PROVIDER_NAME` instead of bare "Pulumi ESC" literals in the
lease backend.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address gemini-code-assist and greptile review on jdx#507: use the project's `env::var` wrapper (re-exported, test-mockable) instead of `std::env::var` for PULUMI_HOME and PULUMI_BACKEND_URL, matching the convention used by the token lookups and every other provider. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…URL fallback Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rebase onto upstream added daemon_cache to ProviderConfig variants; the pulumi-esc arm in `provider add` predated it and broke the binary build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`lease create --all` flattened every backend error through Display into a plain String and re-wrapped it in FnoxError::Config, discarding the rich miette help/url (e.g. "Run 'esc login'..."). When exactly one backend fails — the common case — propagate the original error so its actionable hint and docs URL survive. Multi-failure aggregation is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ation error - thread config-level `token` through check_prerequisites/required_env_vars (matches the vault pattern) so fnox status doesn't false-negative when the token comes from fnox.toml instead of PULUMI_ACCESS_TOKEN - omit the raw credential value from the unterminated-sigil error message Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both new doc pages routed lease credentials with
`AWS_ACCESS_KEY_ID = { lease = "aws" }` under `[secrets]`. `SecretConfig`
is `deny_unknown_fields` and has no `lease` field, so copying either
example makes every fnox command in that directory fail to load the
config. Lease routing already comes from `env_vars` via
`produces_env_var()`; no `[secrets]` entry participates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014PNgnZWmx3Qpr1Y7C6vcjv
d56bca7 to
0668bca
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/providers/overview.md (1)
42-42: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winHyphenate “open source” when used adjectivally.
Use “Open-source preference” for consistent grammar.
🤖 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 `@docs/providers/overview.md` at line 42, Update the Bitwarden row in the providers overview table so the adjectival phrase in the description reads “Open-source preference,” preserving the rest of the row unchanged.Source: Linters/SAST tools
src/commands/provider/add.rs (1)
80-98: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
--profilefor the target section, not--configwith-g.
--globalselects the globalconfig.toml, while--profile/--write-profileshould select the profile section. Gating onself.global || write_profile == "default"makesfnox provider add --global --profile work foo pulumi-escwriteconfig.tomlbut into[providers]instead of[profiles.work.providers]; this is also mirrored inprovider remove.🤖 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/commands/provider/add.rs` around lines 80 - 98, Update provider add and the mirrored provider remove logic to select the target section from --profile/--write-profile rather than treating --global as sufficient. When a profile is specified, including with --global, read or modify that profile’s providers; only use the top-level providers section when the effective write profile is default.
🧹 Nitpick comments (1)
crates/fnox-core/src/pulumi_esc_api.rs (1)
39-73: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBlocking file I/O (
std::fs::read_to_string) runs on the async call path.
resolve_auth()performs a blocking file read;resolve_auth_for→EscClient::neware plain sync fns invoked inline from the asyncfetch_envinproviders/pulumi_esc.rs(self.client()?.open_and_read(...).await), so this blocking read happens on the async runtime's worker thread on every call that doesn't pass an explicittoken. This is more relevant given fnox's daemon mode (daemon serve) runs a persistent async runtime handling many requests. Considertokio::task::spawn_blockingortokio::fs::read_to_stringfor the credentials-file path.🤖 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 `@crates/fnox-core/src/pulumi_esc_api.rs` around lines 39 - 73, Move the credentials-file read in resolve_auth out of the async runtime’s blocking path by making the authentication resolution asynchronous and using tokio::fs::read_to_string or spawn_blocking for credentials.json. Update resolve_auth_for and EscClient::new, plus the fetch_env call chain, to await the resolver while preserving environment-token handling and existing error messages.
🤖 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 `@crates/fnox-core/src/pulumi_esc_api.rs`:
- Around line 155-158: Extend OpenResponse and the open_and_read flow to
deserialize the optional diagnostics array, including each diagnostic’s summary
and severity fields. Before calling read() after a successful open(), inspect
diagnostics and return an error for any diagnostic whose severity is "error",
preserving the diagnostic summary as the root-cause message; continue to read
normally when no error-severity diagnostics are present.
- Around line 58-66: Update resolve_auth to treat empty FNOX_PULUMI_ACCESS_TOKEN
and PULUMI_ACCESS_TOKEN values as absent before constructing EscAuth, so
resolution continues to credentials.json when both are empty. Preserve the
existing precedence of the FNOX-specific variable over PULUMI_ACCESS_TOKEN and
the current non-empty token behavior.
---
Outside diff comments:
In `@docs/providers/overview.md`:
- Line 42: Update the Bitwarden row in the providers overview table so the
adjectival phrase in the description reads “Open-source preference,” preserving
the rest of the row unchanged.
In `@src/commands/provider/add.rs`:
- Around line 80-98: Update provider add and the mirrored provider remove logic
to select the target section from --profile/--write-profile rather than treating
--global as sufficient. When a profile is specified, including with --global,
read or modify that profile’s providers; only use the top-level providers
section when the effective write profile is default.
---
Nitpick comments:
In `@crates/fnox-core/src/pulumi_esc_api.rs`:
- Around line 39-73: Move the credentials-file read in resolve_auth out of the
async runtime’s blocking path by making the authentication resolution
asynchronous and using tokio::fs::read_to_string or spawn_blocking for
credentials.json. Update resolve_auth_for and EscClient::new, plus the fetch_env
call chain, to await the resolver while preserving environment-token handling
and existing error messages.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 42adc879-7167-4d00-9a9a-648d2f7baa81
📒 Files selected for processing (18)
crates/fnox-core/providers/pulumi-esc.tomlcrates/fnox-core/src/lease_backends/mod.rscrates/fnox-core/src/lease_backends/pulumi_esc.rscrates/fnox-core/src/lib.rscrates/fnox-core/src/providers/mod.rscrates/fnox-core/src/providers/pulumi_esc.rscrates/fnox-core/src/pulumi_esc_api.rsdocs/.vitepress/config.mjsdocs/cli/commands.jsondocs/cli/provider/add.mddocs/leases/pulumi-esc.mddocs/providers/overview.mddocs/providers/pulumi-esc.mddocs/public/schema.jsonfnox.usage.kdlsrc/commands/lease.rssrc/commands/provider/add.rssrc/commands/provider/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/fnox-core/src/lib.rs
- docs/cli/provider/add.md
- docs/.vitepress/config.mjs
| pub fn resolve_auth() -> std::result::Result<EscAuth, String> { | ||
| if let Ok(token) = | ||
| env::var("FNOX_PULUMI_ACCESS_TOKEN").or_else(|_| env::var("PULUMI_ACCESS_TOKEN")) | ||
| { | ||
| return Ok(EscAuth { | ||
| base: backend_base(), | ||
| token, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Empty PULUMI_ACCESS_TOKEN/FNOX_PULUMI_ACCESS_TOKEN is treated as a valid token instead of falling back.
env::var(...).or_else(...) succeeds on an empty string (Ok("")), so an accidentally-exported PULUMI_ACCESS_TOKEN= short-circuits the fallback to credentials.json and produces an opaque auth failure with token as the bearer header. Other providers in this codebase (e.g. foks::resolve_with_env, proton_pass::env_value) explicitly filter out empty env values for this reason.
🐛 Suggested fix
- if let Ok(token) =
- env::var("FNOX_PULUMI_ACCESS_TOKEN").or_else(|_| env::var("PULUMI_ACCESS_TOKEN"))
- {
+ if let Ok(token) =
+ env::var("FNOX_PULUMI_ACCESS_TOKEN").or_else(|_| env::var("PULUMI_ACCESS_TOKEN"))
+ && !token.is_empty()
+ {
return Ok(EscAuth {
base: backend_base(),
token,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn resolve_auth() -> std::result::Result<EscAuth, String> { | |
| if let Ok(token) = | |
| env::var("FNOX_PULUMI_ACCESS_TOKEN").or_else(|_| env::var("PULUMI_ACCESS_TOKEN")) | |
| { | |
| return Ok(EscAuth { | |
| base: backend_base(), | |
| token, | |
| }); | |
| } | |
| pub fn resolve_auth() -> std::result::Result<EscAuth, String> { | |
| if let Ok(token) = | |
| env::var("FNOX_PULUMI_ACCESS_TOKEN").or_else(|_| env::var("PULUMI_ACCESS_TOKEN")) | |
| && !token.is_empty() | |
| { | |
| return Ok(EscAuth { | |
| base: backend_base(), | |
| token, | |
| }); | |
| } |
🤖 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 `@crates/fnox-core/src/pulumi_esc_api.rs` around lines 58 - 66, Update
resolve_auth to treat empty FNOX_PULUMI_ACCESS_TOKEN and PULUMI_ACCESS_TOKEN
values as absent before constructing EscAuth, so resolution continues to
credentials.json when both are empty. Preserve the existing precedence of the
FNOX-specific variable over PULUMI_ACCESS_TOKEN and the current non-empty token
behavior.
| #[derive(Deserialize)] | ||
| struct OpenResponse { | ||
| id: String, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg 'pulumi_esc_api.rs|pulumi_esc.rs' || true
echo "== file outline =="
ast-grep outline crates/fnox-core/src/pulumi_esc_api.rs || true
echo "== relevant pulumi_esc_api.rs sections =="
wc -l crates/fnox-core/src/pulumi_esc_api.rs
sed -n '130,260p' crates/fnox-core/src/pulumi_esc_api.rs | cat -n
echo "== usages of diagnostics/open/open_and_read/read =="
rg -n "diagnostics|OpenResponse|fn open|open_and_read|read\\(" crates/fnox-core/src/pulumi_esc_api.rs crates || true
echo "== pulumi_esc provider section =="
rg -n "open\\(|open_and_read\\(|fetch_env|PulumiCredentials|PulumiAccount|EscClient" crates 2>/dev/null || true
sed -n '1,220p' crates/fnox-core/src/providers/pulumi_esc.rs 2>/dev/null | cat -n || trueRepository: jdx/fnox
Length of output: 19690
🌐 Web query:
Pulumi ESC open API OpenEnvironmentResponse diagnostics id schema severity summary
💡 Result:
The Pulumi ESC OpenEnvironmentResponse API schema represents the structure returned when opening an environment session [1][2]. The core components of this response schema are described below: id (string, required): The unique identifier for the opened environment session [1][3]. This session ID is used to perform subsequent operations, such as reading the environment's resolved values [2][4]. diagnostics (array of EnvironmentDiagnostic, optional): An optional field containing a list of diagnostics encountered during the environment opening process [1][3][4]. Each EnvironmentDiagnostic object in this array typically contains the following properties: summary (string): A summary of the diagnostic message [1][4]. path (string, optional): The path within the environment definition where the diagnostic occurred [1][4]. severity (string, optional): The severity level of the diagnostic, which is typically either "error" or "warning" [1]. range (EscRange, optional): The source range within the environment definition file corresponding to the diagnostic [1][4]. The OpenEnvironmentResponse is defined within the Pulumi Cloud REST API and the Pulumi ESC SDK [1][4]. It ensures that users can both obtain the necessary session identifier and identify any potential issues (via diagnostics) that occurred during the resolution of dynamic values, provider integrations, and other environment expressions [2].
Citations:
- 1: https://www.pulumi.com/docs/reference/cloud-rest-api/environments/
- 2: https://raw.githubusercontent.com/api-evangelist/pulumi/refs/heads/main/openapi/pulumi-environments-api-openapi.yml
- 3: https://www.pulumi.com/docs/reference/pkg/nodejs/pulumi/esc-sdk/interfaces/OpenEnvironment.html
- 4: https://github.com/pulumi/esc-sdk/blob/c56628e50930f3d942a085407be8cc25695006ef/sdk/typescript/esc/raw/api.ts
Handle diagnostics from Pulumi ESC’s open response.
OpenResponse only captures id, and open_and_read() immediately calls read() when open() returns successfully. The ESC open response can include an optional diagnostics array with summary, severity, and other fields; severity: "error" diagnostics represent environment-definition issues that should cause secret resolution to fail with the actual root cause rather than a downstream “path not found” secret error. Extend OpenResponse to deserialize diagnostics and return errors for any diagnostic with severity == "error".
🤖 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 `@crates/fnox-core/src/pulumi_esc_api.rs` around lines 155 - 158, Extend
OpenResponse and the open_and_read flow to deserialize the optional diagnostics
array, including each diagnostic’s summary and severity fields. Before calling
read() after a successful open(), inspect diagnostics and return an error for
any diagnostic whose severity is "error", preserving the diagnostic summary as
the root-cause message; continue to read normally when no error-severity
diagnostics are present.
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 1 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 2 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
2 participants
Adds a Pulumi ESC provider and lease backend, as proposed in #568.
Our team has been running this branch in production for over a month — Pulumi is our primary IaC tool and ESC is where our secrets already live.
What's included
pulumi-escprovider (crates/fnox-core/src/providers/pulumi_esc.rs): reads secrets from ESC environments over the native HTTP API (noescCLI dependency), withPULUMI_ACCESS_TOKEN/~/.pulumi/credentials.jsonauth andPULUMI_BACKEND_URLsupport for self-hosted backends.pulumi-esclease backend (crates/fnox-core/src/lease_backends/pulumi_esc.rs): opens short-lived ESC environment sessions and mapsenvironmentVariablesinto the lease. Includes an opt-ininterpolateoption for late-binding%{path}references resolved against the environment'spropertiestree at lease-creation time.docs/providers/pulumi-esc.md,docs/leases/pulumi-esc.md, overview table entry, regenerated schema/usage/CLI docs.fnox provider add pulumi-escwiring.Notes for review
crate::envper project convention.interpolatefeature feels out of scope for a first landing, it's cleanly severable (opt-in config field, off by default) — happy to split it into a follow-up PR.Discussion: #568
🤖 Generated with Claude Code
Summary by CodeRabbit