feat: add experimental coderd_ai_provider resource#368
Conversation
7c12298 to
844cbf5
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dde8c9a39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 891e3b5e17
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
b8ee453 to
3ca1539
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ca1539a17
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
28abf98 to
4e28632
Compare
|
@codex review re:
Verified this against coder/coder at current main — the premise is correct, but loosening only the provider here wouldn't actually unblock proxy deployments, so I'm holding off. The provider is faithfully mirroring the server's create gate. CreateAIProviderRequest.Validate() in codersdk/aiproviders.go rejects type=bedrock when req.Settings.Bedrock == nil || !req.Settings.Bedrock.IsConfigured() — and IsConfigured() only checks region + credentials, ignoring base_url, exactly like this check. The create handler (coderd/ai_providers.go) calls that same Validate(), so a base-URL-only Bedrock provider (AWS SDK default credential chain, no region) currently returns an HTTP 400 from Coder regardless of what Terraform does. The base-URL-aware IsBedrockConfigured(baseURL, settings) does exist, but it's only used by legacy env-var seeding (cli/server.go) and migration detection (coderd/ai_providers_migrate.go) — never in the create/update API validation path. So if I switch this check to be base-URL-aware in isolation, the apply still fails — just with a server-side 400 instead of a clean client-side diagnostic, which is worse UX. This is an inconsistency in Coder itself between the Bedrock docs and CreateAIProviderRequest.Validate(). The right fix is upstream: have CreateAIProviderRequest.Validate() use IsBedrockConfigured(baseURL, ...) (or correct the docs). Once the API accepts base-URL-only Bedrock, I'll mirror it here |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e28632a3c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
78ae1a4 to
1cf58fc
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
1cf58fc to
ee995a9
Compare
82b36d9 to
2219c80
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee995a996b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ee995a9 to
d2f68e3
Compare
2219c80 to
293eac1
Compare
|
@codex review re:
Verified this against coder/coder at current main — the premise is correct, but loosening only the provider here wouldn't actually unblock proxy deployments, so I'm holding off. The provider is faithfully mirroring the server's create gate. CreateAIProviderRequest.Validate() in codersdk/aiproviders.go rejects type=bedrock when req.Settings.Bedrock == nil || !req.Settings.Bedrock.IsConfigured() — and IsConfigured() only checks region + credentials, ignoring base_url, exactly like this check. The create handler (coderd/ai_providers.go) calls that same Validate(), so a base-URL-only Bedrock provider (AWS SDK default credential chain, no region) currently returns an HTTP 400 from Coder regardless of what Terraform does. The base-URL-aware IsBedrockConfigured(baseURL, settings) does exist, but it's only used by legacy env-var seeding (cli/server.go) and migration detection (coderd/ai_providers_migrate.go) — never in the create/update API validation path. So if I switch this check to be base-URL-aware in isolation, the apply still fails — just with a server-side 400 instead of a clean client-side diagnostic, which is worse UX. This is an inconsistency in Coder itself between the Bedrock docs and CreateAIProviderRequest.Validate(). The right fix is upstream: have CreateAIProviderRequest.Validate() use IsBedrockConfigured(baseURL, ...) (or correct the docs). Once the API accepts base-URL-only Bedrock, I'll mirror it here |
|
/coder-agents-review |
|
Chat: Review posted | View chat Review historydeep-review v0.9.0 | Round 3 | Last posted: Round 3, 21 findings (5 P2, 8 P3, 4 Nit, 4 Note), APPROVE. Review Finding inventoryFinding InventoryFindings
Law analysisEffective LOC: 1222. Head SHA: d2f68e3. Verdict: Don't split. Enforcement: Advisory. Round logRound 1Panel. 4 P2, 6 P3, 2 Nit, 4 Note. 16 posted. Reviewed against 293eac1..d2f68e3. Panel: Bisky, Hisoka, Mafu-san, Mafuuu, Pariston, Gon, Leorio, Ging-Go, Kurapika, Chopper, Komugi, Meruem, Kite (wildcard), Knov (wildcard). Netero clean, Law advisory don't-split. Round 2Churn guard: PROCEED (12 addressed, 4 acknowledged). Netero clean, all R1 fixes verified. Panel: Bisky, Mafuuu, Hisoka, Chopper, Meruem, Knov (wildcard). 1 P2, 2 P3, 2 Nit new. Reviewed against 293eac1..1ee8a77. Round 3Churn guard: PROCEED (5 addressed). Netero clean, all R2 fixes verified. Panel: Mafuuu, Hisoka, Meruem. 0 new findings. All 21 findings resolved (17 fixed, 4 acknowledged). Reviewed against 293eac1..d51dad0. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
d51dad0 to
b1d81a3
Compare
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
All 21 findings across 3 rounds resolved: 17 fixed, 4 acknowledged. No new findings from the R3 panel (Mafuuu, Hisoka, Meruem) or Netero.
The R2 fixes are solid. bedrockRegionPlanModifier correctly derives region from base_url during planning, eliminating both the stale-state and plan-noise variants. created_at now has UseStateForUnknown. Credential rotation test uses distinct non-empty values. API key rotation test uses t.Run subtests. Single-key invariant is documented.
CI note: Terraform Provider Acceptance Tests (1.6.*) is failing. This may be pre-existing or related to the base branch (#370). Worth checking before merge.
The resource is well-engineered: clean write-only secret handling, thoughtful validation deferral for unknowns, proper credential lifecycle semantics, and thorough test coverage (45.8% test density, 1:1 production-to-test ratio).
🤖 This review was automatically generated with Coder Agents.
matifali
left a comment
There was a problem hiding this comment.
I have concerns about the choice of the name for this resource.
|
Yeah honestly I agree, I think putting experimental in the title is a mistake. It'll be extremely painful for customers to migrate when we remove it. |
b1d81a3 to
6221b88
Compare
a3b1745 to
936dab2
Compare
c4c0db1 to
a97aba5
Compare
> **Stack** — base PR; #368 (experimental AI provider resource) builds on this. ## Problem The Terraform acceptance matrix was failing intermittently across all lanes with `coder failed to become ready in time`. `integration.StartCoder` boots `ghcr.io/coder/coder` without `CODER_PG_CONNECTION_URL`, so Coder falls back to its embedded PostgreSQL. The image doesn't bundle the Postgres binary, so each startup downloads the embedded-postgres jar from Maven Central. GitHub runners' shared egress IPs get rate-limited by Cloudflare, and a single non-200 crashes Coder before it binds — reddening the lane. ## Fix Give Coder a real PostgreSQL instead of the embedded one. Setting `CODER_PG_CONNECTION_URL` bypasses the Maven download entirely. Per test, `integration.StartCoder` now starts a Postgres sidecar on a user-defined Docker network (aliased `postgres`), wires Coder onto it, and points `CODER_PG_CONNECTION_URL` at it. No readiness wait needed — Coder retries its DB connection for ~30s, covering sidecar boot. The image is `us-docker.pkg.dev/coder-v2-images-public/public/postgres:17`, the public mirror coder/coder uses in its own tests, which avoids Docker Hub's anonymous pull rate limit. This is environment-level, so it also covers the version-pinned back-compat tests. CI wall time is essentially unchanged (~206s/lane): the Postgres pull and boot costs about what the Maven download it replaces did. ## Also: raise the Terraform support floor to 1.5 Terraform 1.0–1.4 are EOL. This drops them from the CI matrix (now `1.5.*`–`1.14.*`, so we also test the latest releases), sets the README floor to `>= 1.5`, and removes the `template_resource_test.go` skip guard for a Terraform 1.0 panic.
Adds a Terraform resource for declarative Coder AI provider configuration, supporting all SDK provider types with AWS Bedrock and API-key providers. Secrets use Terraform 1.11+ write-only arguments and are never stored in state.
936dab2 to
9fba23a
Compare
a97aba5 to
839989b
Compare
839989b to
0d0e3b8
Compare
|
|
||
| ### Optional | ||
|
|
||
| > **NOTE**: [Write-only arguments](https://developer.hashicorp.com/terraform/language/resources/ephemeral#write-only-arguments) are supported in Terraform 1.11 and later. |
There was a problem hiding this comment.
| > **NOTE**: [Write-only arguments](https://developer.hashicorp.com/terraform/language/resources/ephemeral#write-only-arguments) are supported in Terraform 1.11 and later. | |
| -> [Write-only arguments](https://developer.hashicorp.com/terraform/language/resources/ephemeral#write-only-arguments) are supported in Terraform 1.11 and later. |
This could also turn into a Terraform registry-style note.
Preview from: https://registry.terraform.io/tools/doc-preview
There was a problem hiding this comment.
There's already one at the top, I'm going to get rid of it
There was a problem hiding this comment.
Nevermind, it's auto-generated and you can't get rid of or edit it.
There was a problem hiding this comment.
I think the block at the top saying the whole resource requires 1.11 or later is worth keeping, so keeping both unfortunately.
There was a problem hiding this comment.
Yes we can drop this block and keep the top only.
There was a problem hiding this comment.
> **NOTE**: [Write-only arguments](https://developer.hashicorp.com/terraform/language/resources/ephemeral#write-only-arguments) are supported in Terraform 1.11 and later.
You can't get rid of this, it's added by the docs generator.
0d0e3b8 to
de42560
Compare
de42560 to
2e2abe6
Compare
| name = "openai" | ||
| display_name = "OpenAI" | ||
| enabled = true | ||
| base_url = "https://api.openai.com" |
There was a problem hiding this comment.
Note to future self to investigate: does coderd handle omitting the /v1 for openAI base urls? otherwise we should probably catch this during validation/plan

Summary
Adds
coderd_ai_providerfor managing Coder AI Gateway providers from Terraform — OpenAI-style API-key providers and AWS Bedrock providers — with full CRUD + import, generated docs/examples, and acceptance/unit tests. The resource is marked experimental via a warning callout in its docs.Approach
*_wo), so secrets reach Coder but never land in state. Each secret is paired with a*_versionfield; bumping the version rotates the secret. A null version means "unmanaged/preserve" rather than "clear", so removing it never silently wipes server-side secrets.ValidateConfighandles type-dependent combinations (e.g.api_key_wois rejected forbedrock/copilotor anysettings.bedrockconfig, mirroring the server) and Bedrock's region-or-credentials requirement. Validation defers while values are unknown during the validate/plan walk.regionis omitted it derives from a canonical Bedrockbase_url(bedrock-runtime.<region>.amazonaws.com), so it re-derives correctly whenbase_urlchanges regions.Schema
Computed:
id,display_name,enabled,api_key_masked,created_at,updated_at, andsettings.bedrock.region(derived frombase_url).Notes
Requires Terraform 1.11+ when configured (write-only arguments); acceptance cases skip earlier versions.
An integration test combining this with the model resource is planned.
Relates to CODAGT-607