Skip to content

🤖 feat: add the CoderAIProvider reconciler skeleton - #245

Merged
ThomasK33 merged 2 commits into
mainfrom
ai-provider-reconciler
Oct 13, 2026
Merged

ThomasK33 merged 2 commits into
mainfrom
ai-provider-reconciler

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Summary

This PR adds CoderAIProviderReconciler, the first reconciler for the dormant CoderAIProvider kind. It is not registered, so the operator does not run it, and users see no change. It checks the spec, the key Secret and the control plane, and it reports a Coder provider name that something else already uses. It writes nothing to Coder. Creating providers comes in a later layer.

Background

#154 adds AI providers, chat models and AI policies as controller CRDs. This is the third CoderAIProvider layer, after the API types (#235) and the fake Coder routes (#237). Every check here runs before any Coder write that later layers add.

Reconcile order

  1. A deleted object releases its finalizer only when nothing of it can exist in Coder: no status.providerID and no create marker. Otherwise it keeps the finalizer with reason DeletePending and deletes nothing.
  2. The cleanup finalizer coder.com/ai-provider-cleanup is added with a JSON merge patch and an optimistic lock, never a full update.
  3. spec.baseURL is parsed exactly as written, with no trimming, the way Coder validates base_url. The check also requires a host name and refuses user info, as admission does. A bad percent escape, including one in the fragment, gives CredentialMalformed. Admission already rejects fragment escapes too: isURL uses url.ParseRequestURI, which rejects #%zz (envtest-verified). My reply on 🤖 feat(api): add the dormant CoderAIProvider API types #235 said otherwise, and it was wrong. The controller check makes the rule hold without admission.
  4. The key is read through internal/credentialsecret, with spec.baseURL as the destination. Every contract failure gives its own reason (CredentialRefused, CredentialNotOptedIn, CredentialDestinationNotAllowed, CredentialMalformed, CredentialSecretNotFound, CredentialKeyMissing).
  5. Coder access comes from controlPlaneCoderClient (🤖 refactor: share CoderControlPlane access for controllers #231). A missing or unready control plane gives ControlPlaneNotReady or OperatorAccessNotReady, and the controller retries after 30 s, because no watch sees an operator token Secret appear.
  6. The provider name is looked up read-only with GET /api/v2/ai/providers/{name}:
    • 404: Pending, and this version creates nothing.
    • A provider exists: NameConflict. Coder providers carry no owner, so the controller never changes it (no adoption).
    • Coder error: CoderUnavailable with the status code and Coder's short message only, never Detail or validations.
    • An answer for another name: CoderAnswerMismatch.

Steps 3 to 5 send nothing to Coder. Every condition message is also scrubbed of the key value. Conditions follow kstatus (Ready, Reconciling, Stalled). Credential, URL and name failures are Stalled. Control plane and Coder waits are Reconciling. Status is written only when it changes.

SetupWithManager indexes providers by spec.controlPlaneRef.name and spec.apiKeySecretRef.name. It watches control planes, and maps each event to the providers that use it and to the providers whose key Secret it names as its operator token Secret (the contract refuses those). It watches Secrets with credentialsecret.WatchPredicate (old-or-new label, every delete). No RBAC marker, no registration in controllerapp, and no CRD move: config/rbac/role.yaml and dist/install.yaml do not change.

Validation

internal/controller/coderaiprovider_controller_test.go (envtest plus the fake Coder from #237):

  • Credential checks (plan tests S1, S2, S10, S12): 18 cases. Each failure stalls with its reason and zero Coder requests. Positive controls (a consented key, an uppercase host, an explicit default port) reach the name lookup with exactly one request. Refusals cover the labeled operator token Secret, a non-Opaque Secret and a Secret owned by a coder.com object. Destination refusals cover another port, scheme, subdomain and a trailing dot. Malformed values are empty, oversized or whitespace-padded. Every case scans status, annotations and captured logs for the key.
  • A missing control plane gives ControlPlaneNotReady. A ready control plane without its operator token Secret gives OperatorAccessNotReady. Both retry after 30 s with zero Coder requests, and the provider recovers once the Secret exists.
  • Name lookup: an existing provider gives NameConflict with one GET and no write. A 500 whose Detail holds the key gives CoderUnavailable, and the key appears nowhere. A rewritten answer gives CoderAnswerMismatch. A second reconcile in each case sends one GET, no write, and leaves the stored status unchanged.
  • Finalizer: added by merge patch with enabled: false kept, released on delete when nothing reached Coder, and kept with DeletePending when status.providerID is set.
  • Watches through a real manager: removing the label, restoring it, creating the control plane, creating and deleting another control plane that names the key Secret as its operator token Secret, and deleting the Secret each re-queue the provider at once.
  • API: a UUID-shaped name, with or without hyphens, is rejected, because Coder looks such a name up as a provider ID. A 32-character name that is not hex is accepted.

coderaiprovider_internal_test.go checks the URL rule, including #%zz, #a%2, a trailing space, user info and http://:8080.

Mutation check: I broke the reconciler in 14 ways (no retry on a control plane wait, control plane events mapped only to its users, the UUID name rule off, credential errors ignored, Coder lookup before the credential check, full error text in status, an existing provider adopted, Secret watch mapping nothing, no control plane watch, finalizer released with a Coder ID, trimmed URL, user info allowed, answer not checked). Each change made a test fail. One more mutation, an unconditional status write, is not observable, because the API server does not change the resourceVersion for a no-op update.

Local gates: make verify-vendor, make test, make test-integration, make build, make lint, GOFLAGS=-mod=vendor go tool govulncheck ./..., and go test -race ./internal/controller/. Running the generators again leaves no diff.

Known gap

The design plan's S1 also refuses a control plane's license Secret and database Secret. internal/credentialsecret on main refuses only operator-generated Secrets. Today a license or database Secret is usable only if its writer labels it and lists the provider origin, which is explicit consent. Adding that refusal changes the shared contract for CoderWorkspaceProxy and CoderProvisioner too, so it belongs in its own PR, not here.

Review round 1

The automatic Codex review on c825229b left three P2 findings. All three are fixed in 649f6508:

  1. "Requeue after an unavailable operator token": control plane waits now retry after 30 s, with a test for a missing operator token Secret.
  2. "Requeue credentials when any control plane changes": control plane events now also map through operator token Secret names, with a refusal and recovery test.
  3. "Handle UUID-shaped provider names as names": a CEL rule rejects UUID-shaped names, with tests.

The branch is rebased onto main at f9198ca1c. That brings in #238, #239 and #241. #239 already fixed the dormancy guard's grep exit-code handling.

Size

298 production lines added and 1 removed (reconciler and API type). 406 test lines added and 0 removed. Generated CRD: 6 added and 1 removed. This is above the 500-line target. The S1, S2, S10 and S12 tests belong with the code they test, so I kept them together.

Refs #154


Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high • Cost: $13.59

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T14:55:14.539954Z 649f650 Manual request
🔒 Security Review ✅ Completed 2026-10-13T14:56:38.594859Z 649f650 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c825229b59

ℹ️ 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".

Comment thread internal/controller/coderaiprovider_controller.go Outdated
Comment thread internal/controller/coderaiprovider_controller.go Outdated
Comment thread internal/controller/coderaiprovider_controller.go
Add a CoderAIProviderReconciler that is not registered yet. It checks the
base URL the way Coder does (including bad percent escapes), reads the
key through the credential Secret contract with the base URL as the
destination, gets Coder access through controlPlaneCoderClient, and looks
up the provider name read-only: a provider that the object did not create
gives NameConflict. It adds the finalizer by merge patch, writes kstatus
conditions, and watches the key Secret and the control plane. It writes
nothing to Coder.

Refs #154

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---
_Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_

Change-Id: I83ba9f01530f9358cb8df386607345b8640e37d1
…anges

Retry control plane waits every 30 s, because no watch sees an operator
token Secret appear. Map a control plane event also to providers whose key
Secret it names as its operator token Secret. Reject UUID-shaped names,
because Coder looks them up as provider IDs.

Refs #154

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---
_Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_

Change-Id: I652464b72098823d40ee3fa7bfaf958eca0fdb3c
@ThomasK33
ThomasK33 force-pushed the ai-provider-reconciler branch from c825229 to 649f650 Compare October 13, 2026 14:50
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 649f6508d9

ℹ️ 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".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 649f6508d9

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ThomasK33
ThomasK33 added this pull request to the merge queue Oct 13, 2026
Merged via the queue into main with commit 4442f86 Oct 13, 2026
10 checks passed
@ThomasK33
ThomasK33 deleted the ai-provider-reconciler branch October 13, 2026 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant