feat(opentofu-agent): add OpenTofu Agent support#1966
Conversation
There was a problem hiding this comment.
Pull request overview
Adds OpenTofu (tofu) agent support to Distr so vendors can provision infrastructure (via OpenTofu/Terraform-compatible workflows) alongside existing Docker/Kubernetes deployments, including a Hub-hosted HTTP state backend with S3 storage and PostgreSQL-based locking.
Changes:
- Introduces
opentofuas a newDeploymentType, extends ApplicationVersion/DeploymentRevision data model, and updates DB schema (enum + tofu columns +opentofu_statelock table + updated constraints). - Adds Hub-side HTTP state backend endpoints (GET/POST state + lock/unlock) protected by Basic Auth using deployment-target credentials.
- Adds a new OpenTofu agent binary (poll/reconcile/apply/destroy), an embedded connect manifest template, and an agent container Dockerfile.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/types/types.go | Adds DeploymentTypeOpenTofu constant. |
| internal/types/deployment_target.go | Allows OpenTofu deployment target type in validation switch. |
| internal/types/deployment_revision.go | Adds tofu vars/backend config/version fields on deployment revisions. |
| internal/types/deployment.go | Adds tofu fields to DeploymentWithLatestRevision (hidden from JSON). |
| internal/types/application_version.go | Adds tofu config URL/version fields and OpenTofu-specific validation. |
| internal/resources/embedded/agent/opentofu/v1/docker-compose.yaml.tmpl | New docker-compose template for OpenTofu agent. |
| internal/migrations/sql/84_add_opentofu_deployment_type.up.sql | Adds opentofu value to deployment_type enum. |
| internal/migrations/sql/84_add_opentofu_deployment_type.down.sql | No-op down migration for enum value. |
| internal/migrations/sql/85_add_opentofu_columns.up.sql | Adds tofu columns + opentofu_state table + relaxes namespace/scope constraints for OpenTofu. |
| internal/migrations/sql/85_add_opentofu_columns.down.sql | Drops new columns/table and restores original constraints. |
| internal/handlers/state.go | New HTTP state backend handlers backed by S3 + DB locking. |
| internal/handlers/applications.go | Allows creating OpenTofu application versions (requires config URL). |
| internal/handlers/agent.go | Adds /state routes w/ Basic Auth middleware; includes OpenTofu data in agent resources response. |
| internal/db/state.go | New DB helpers for opentofu_state metadata and lock/unlock. |
| internal/db/deployments.go | Includes tofu fields in deployment queries and revision creation return shape. |
| internal/db/applications.go | Reads/writes tofu config URL/version fields for application versions. |
| internal/agentmanifest/common.go | Selects OpenTofu agent template based on target type. |
| internal/agentconnect/connect.go | Generates connect command for OpenTofu targets (docker-style). |
| api/agent.go | Extends AgentDeployment with OpenTofu-specific fields. |
| cmd/agent/opentofu/main.go | New OpenTofu agent main loop with polling, apply, and destroy logic. |
| cmd/agent/opentofu/tofu_actions.go | Implements tofu init/plan/apply/destroy via terraform-exec and OpenTofu binary resolution. |
| cmd/agent/opentofu/oci_pull.go | Pulls OpenTofu configs as OCI artifacts into a workspace directory. |
| cmd/agent/opentofu/config.go | Scratch/workspace directory helpers for the agent. |
| cmd/agent/opentofu/agent_deployment.go | Persists local agent deployment state to disk. |
| Dockerfile.opentofu-agent | Builds and packages the OpenTofu agent container image (includes tofu binary). |
| DEP-terraform-agent.md | Design proposal / specification document for the feature. |
| go.mod | Adds dependencies for terraform-exec and tofudl. |
| go.sum | Updates module checksums for new dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0c9157e to
1f93973
Compare
d748ab5 to
6e656c7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6e656c7 to
9011b38
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9011b38 to
666aeef
Compare
| @@ -0,0 +1 @@ | |||
| ALTER TYPE deployment_type ADD VALUE IF NOT EXISTS 'opentofu'; | |||
There was a problem hiding this comment.
We should merge both migrations into one
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| log.Error("failed to get deployment target from basic auth", zap.Error(err)) | ||
| w.Header().Set("WWW-Authenticate", `Basic realm="distr"`) | ||
| w.WriteHeader(http.StatusUnauthorized) |
There was a problem hiding this comment.
basicAuthDeploymentTargetCtxMiddleware logs an error for any auth failure and always responds 401. For invalid credentials this is expected behavior and will likely generate noisy error logs (and may mask real server-side failures). Consider distinguishing apierrors.ErrUnauthorized from other errors like agentLoginHandler does and logging unauthorized attempts at Info/Debug, while keeping unexpected DB/system errors at Error.
| log.Error("failed to get deployment target from basic auth", zap.Error(err)) | |
| w.Header().Set("WWW-Authenticate", `Basic realm="distr"`) | |
| w.WriteHeader(http.StatusUnauthorized) | |
| if errors.Is(err, apierrors.ErrUnauthorized) { | |
| log.Info("agent unauthorized", zap.Error(err), zap.Stringer("deploymentTargetId", targetID)) | |
| w.Header().Set("WWW-Authenticate", `Basic realm="distr"`) | |
| w.WriteHeader(http.StatusUnauthorized) | |
| } else { | |
| log.Error("failed to get deployment target from basic auth", zap.Error(err)) | |
| w.WriteHeader(http.StatusInternalServerError) | |
| sentry.GetHubFromContext(ctx).CaptureException(err) | |
| } |
| binName := "tofu" | ||
| if version != "" { | ||
| binName = fmt.Sprintf("tofu-%s", version) | ||
| } | ||
|
|
||
| binDir := filepath.Join(ScratchDir(), "bin") | ||
| if err := os.MkdirAll(binDir, 0o755); err != nil { | ||
| return "", fmt.Errorf("could not create bin directory: %w", err) | ||
| } | ||
|
|
||
| binPath := filepath.Join(binDir, binName) | ||
| if err := os.WriteFile(binPath, binary, 0o755); err != nil { |
There was a problem hiding this comment.
downloadTofuVersion uses the untrusted version string directly in binName and then filepath.Join(binDir, binName). If version contains path separators (e.g. ../), this can write outside the intended scratch directory. Sanitize/validate version (e.g., allow only [0-9A-Za-z._-]), or use a safe filename derived from it, and reject/normalize anything else.
| } | ||
|
|
||
| varsPath := filepath.Join(workspaceDir, "terraform.tfvars") | ||
| return os.WriteFile(varsPath, []byte(b.String()), 0o644) |
There was a problem hiding this comment.
writeVarsFile writes terraform.tfvars with mode 0644. Since tofuVars may contain secrets (API keys, passwords, etc.), this makes them world-readable inside the container/volume. Use a more restrictive permission (e.g. 0600) and consider ensuring the workspace directory permissions are also restrictive.
| return os.WriteFile(varsPath, []byte(b.String()), 0o644) | |
| return os.WriteFile(varsPath, []byte(b.String()), 0o600) |
83c323e to
a3f84cc
Compare
|
@pmig — this PR has been significantly hardened since the last review round. Here's a summary of what changed: Migration fix:
Security hardening:
Critical bug fixes:
UX hardening:
Code quality:
All previous Copilot review comments have been addressed. CI workflows need approval to run (fork-based PR limitation). Would appreciate a re-review when you get a chance! 🙏 |
e791ac3 to
cdddb49
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 77 out of 79 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ALTER TABLE DeploymentRevision | ||
| DROP COLUMN IF EXISTS tofu_vars, | ||
| DROP COLUMN IF EXISTS tofu_backend_config, | ||
|
|
||
| ALTER TABLE ApplicationVersion | ||
| DROP COLUMN IF EXISTS tofu_config_url, | ||
| DROP COLUMN IF EXISTS tofu_config_version; |
There was a problem hiding this comment.
The down migration has invalid SQL: there’s a trailing comma after DROP COLUMN IF EXISTS tofu_backend_config and the statement isn’t terminated before the next ALTER TABLE. This will make the down migration fail to run. Remove the trailing comma and end the ALTER TABLE DeploymentRevision ... statement with a semicolon before starting the next ALTER TABLE.
| li, err := JsonBody[lockInfo](w, r) | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| if err := db.UnlockState(ctx, deploymentID, li["ID"]); err != nil { | ||
| if errors.Is(err, apierrors.ErrConflict) { | ||
| http.Error(w, "lock ID mismatch", http.StatusConflict) |
There was a problem hiding this comment.
handleStateUnlock calls db.UnlockState(ctx, deploymentID, li["ID"]) without verifying the lock payload contains a non-empty ID. If ID is missing/empty, unlock behavior becomes ambiguous and could lead to unintended conflicts. Add an explicit check for li["ID"] != "" and return 400 when missing.
| case DeploymentTypeOpenTofu: | ||
| if av.TofuConfigURL == nil || *av.TofuConfigURL == "" { | ||
| return errors.New("missing tofu config URL") | ||
| } | ||
| } |
There was a problem hiding this comment.
ApplicationVersion.Validate() for DeploymentTypeOpenTofu only checks that TofuConfigURL is set, but it doesn’t reject docker/kubernetes-specific fields (compose/values/template/chart fields). That allows storing “mixed” application versions that will be ambiguous to consumers. Extend this case to also error when any non-OpenTofu fields are populated.
| ### createdAt? | ||
|
|
||
| > `optional` **createdAt?**: `string` | ||
| > `optional` **createdAt**: `string` | ||
|
|
||
| --- | ||
|
|
||
| ### id? | ||
|
|
||
| > `optional` **id?**: `string` | ||
| > `optional` **id**: `string` |
There was a problem hiding this comment.
In these generated docs the heading still includes ? (e.g. ### createdAt?) but the property signature below drops it (optional createdAt). This inconsistency can confuse readers and may break intra-doc links/anchors. Regenerate/fix the docs so the headings and property names use a consistent representation for optional fields.
| ConnectScriptIsSudo bool `json:"connectScriptIsSudo"` | ||
| ArtifactVersionMutable bool `json:"artifactVersionMutable"` | ||
| PrePostScriptsEnabled bool `json:"prePostScriptsEnabled"` | ||
| OpenTofuEnabled bool `json:"openTofuEnabled"` |
There was a problem hiding this comment.
OpenTofuEnabled is added as a non-pointer bool on the create/update request. For update endpoints this is a backward-compatibility footgun: older clients that don’t send the new field will decode it as false and can unintentionally disable the feature. Consider making this field a *bool (apply only when non-nil) or separating create vs update request shapes.
| OpenTofuEnabled bool `json:"openTofuEnabled"` | |
| OpenTofuEnabled *bool `json:"openTofuEnabled"` |
| type zapLogWriter struct { | ||
| logger *zap.Logger | ||
| prefix string | ||
| } | ||
|
|
||
| func (w *zapLogWriter) Write(p []byte) (n int, err error) { | ||
| w.logger.Info(w.prefix, zap.String("output", string(p))) | ||
| return len(p), nil |
There was a problem hiding this comment.
zapLogWriter.Write logs all OpenTofu/Terraform stdout/stderr at INFO with the raw output. Terraform output can include secrets (provider errors, rendered values, outputs), so this risks leaking sensitive data into deployment target logs. Consider logging at DEBUG by default, redacting known secret patterns, and/or capturing only high-level progress while storing full logs in a protected location.
| li, err := JsonBody[lockInfo](w, r) | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| if _, err := db.GetOrCreateState(ctx, deploymentID, organizationID); err != nil { | ||
| log.Error("failed to upsert state metadata", zap.Error(err)) | ||
| sentry.GetHubFromContext(ctx).CaptureException(err) | ||
| http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| lockInfoJSON, err := json.Marshal(li) | ||
| if err != nil { | ||
| log.Error("failed to marshal lock info", zap.Error(err)) | ||
| sentry.GetHubFromContext(ctx).CaptureException(err) | ||
| http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| if err := db.LockState(ctx, deploymentID, li["ID"], string(lockInfoJSON)); err != nil { | ||
| if errors.Is(err, apierrors.ErrConflict) { |
There was a problem hiding this comment.
handleStateLock uses li["ID"] directly when calling db.LockState, but the request body may omit ID (or provide an empty string). That would allow locks/unlocks with an empty lock ID and can cause incorrect lock behavior. Validate that ID is present and non-empty (and ideally return 400 with a clear message) before proceeding.
| ### tofuVersion? | ||
|
|
||
| > `optional` **tofuVersion**: `string` | ||
|
|
There was a problem hiding this comment.
The generated SDK docs list a tofuVersion property on DeploymentRequest, but the actual TypeScript interface (sdk/js/src/types/deployment.ts) does not define it. This is misleading for SDK consumers. Either remove tofuVersion from the docs output or add the field to the type (and wire it through the API) if it’s intended.
| if request.OpenTofuEnabled != org.HasFeature(types.FeatureOpenTofu) { | ||
| org.SetFeature(types.FeatureOpenTofu, request.OpenTofuEnabled) | ||
| needsUpdate = true | ||
| } |
There was a problem hiding this comment.
handleUpdateOrganization will toggle the OpenTofu feature based on request.OpenTofuEnabled, but that field defaults to false when omitted by the client. That means older clients (or partial updates) can inadvertently disable OpenTofu. Use an optional/pointer field for updates (and only call SetFeature when provided), or otherwise ensure clients always send the field.
| connectScriptIsSudo: boolean; | ||
| artifactVersionMutable: boolean; | ||
| prePostScriptsEnabled: boolean; | ||
| openTofuEnabled?: boolean; |
There was a problem hiding this comment.
openTofuEnabled is optional in the UI request type, but the backend request struct expects a non-optional boolean. If callers omit this field, it will serialize as missing/undefined and the backend will treat it as false, potentially disabling the feature on update. Make this field required on the frontend (or update the backend to accept an optional/pointer boolean).
| openTofuEnabled?: boolean; | |
| openTofuEnabled: boolean; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 80 out of 82 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| log.Error("failed to unlock state", zap.Error(err)) | ||
| sentry.GetHubFromContext(ctx).CaptureException(err) | ||
| http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| w.WriteHeader(http.StatusOK) |
There was a problem hiding this comment.
li["ID"] is used without validating that the unlock request body contains a non-empty ID. If it’s missing, the handler currently falls through to a generic 409/500 path. Prefer returning 400 for malformed unlock requests.
| COALESCE(@tofuVars, '{}'::jsonb), | ||
| COALESCE(@tofuBackendConfig, '{}'::jsonb) |
There was a problem hiding this comment.
@tofuVars / @tofuBackendConfig are bound from Go as strings, but here they’re used as jsonb without an explicit cast. With pgx this can fail at runtime with a type mismatch (text vs jsonb). Cast the parameters (@tofuVars::jsonb, @tofuBackendConfig::jsonb) or bind a JSONB-typed value.
| HelmOptions *HelmOptions `db:"helm_options" json:"helmOptions,omitempty"` | ||
|
|
||
| TofuVars *string `db:"tofu_vars" json:"tofuVars,omitempty"` | ||
| TofuBackendConfig *string `db:"tofu_backend_config" json:"tofuBackendConfig,omitempty"` |
There was a problem hiding this comment.
TofuVars / TofuBackendConfig are *string but are exposed in JSON as tofuVars/tofuBackendConfig. Because the DB columns default to '{}'::jsonb, these pointers will likely be non-nil and serialize as JSON strings (e.g. "{}") even for non-OpenTofu deployments, which is both noisy and inconsistent with the request types (objects). Consider either hiding these from public JSON (json:"-") or changing the fields to structured types (e.g. map[string]any / map[string]string or json.RawMessage) so the API shape is consistent.
| w.WriteHeader(http.StatusConflict) | ||
| if state.LockInfo != nil { |
There was a problem hiding this comment.
li["ID"] is used without validating that the decoded lock body actually contains a non-empty ID. If it’s missing/empty, an empty lock ID can be persisted. Add an explicit check and return 400 when ID is absent.
| tfexec.BackendConfig("lock_method=POST"), | ||
| tfexec.BackendConfig("unlock_method=POST"), | ||
| tfexec.BackendConfig("username=agent"), | ||
| tfexec.BackendConfig(fmt.Sprintf("password=%s", client.RawToken())), | ||
| } |
There was a problem hiding this comment.
The Hub JWT is passed to OpenTofu via BackendConfig("password=..."), which ends up in the tofu init argv. That can leak credentials via process listings and some logs/crash reports. Prefer providing the password via a 0600 backend config file (-backend-config=/path) or another mechanism that keeps secrets out of argv.
|
@kosmoz thanks for the detailed review! All of your comments have been addressed. Summary: Architecture / pattern conformance
Scope reduction
UI / data fixes
R4 critical regressions caught during review
Agent shutdown blocking (
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 81 out of 83 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tofuVars: value.tofuVars ? (JSON.parse(value.tofuVars) as Record<string, unknown>) : undefined, | ||
| tofuBackendConfig: value.tofuBackendConfig | ||
| ? (JSON.parse(value.tofuBackendConfig) as Record<string, string>) | ||
| : undefined, |
There was a problem hiding this comment.
JSON.parse will throw on invalid JSON and can crash the save flow (and potentially the whole component) without a user-friendly validation error. Consider adding a form validator for JSON fields and handling parse failures by surfacing a validation error (and/or wrapping parsing in try/catch and returning a 400-friendly message).
| TofuVars *string `db:"tofu_vars" json:"tofuVars,omitempty"` | ||
| TofuBackendConfig *string `db:"tofu_backend_config" json:"tofuBackendConfig,omitempty"` |
There was a problem hiding this comment.
These fields are stored as JSONB but are exposed in API responses as *string, which will serialize as a JSON string containing JSON (double-encoded) rather than an object. If these are meant to be internal-only, mark them json:"-"; if they are meant to be part of the public API, consider using json.RawMessage or concrete map types and decode/encode consistently.
| if err := db.LockState(ctx, deploymentID, li["ID"], string(lockInfoJSON)); err != nil { | ||
| if errors.Is(err, apierrors.ErrConflict) { |
There was a problem hiding this comment.
handleStateLock uses li["ID"] without validating that the request body actually contained an ID. If it's missing/empty, the DB will store an empty lock ID and subsequent lock/unlock behavior becomes ambiguous. Validate that li["ID"] is present and non-empty and return a 400 if not.
178af99 to
bac9923
Compare
…nd and frontend UI
Adds a new OpenTofu agent to Distr, enabling vendors to distribute and manage
infrastructure-as-code deployments alongside existing Docker and Kubernetes agents.
Hub changes:
- DeploymentTypeOpenTofu enum value and OpenTofu-specific fields on
ApplicationVersion (tofuConfigUrl, tofuConfigVersion) and
DeploymentRevision (tofuVars, tofuBackendConfig, tofuVersion)
- HTTP state backend at /state/{deploymentID} with S3 storage and
PostgreSQL-based locking (GET/POST state, POST lock/unlock)
- Basic Auth middleware for state routes (OpenTofu HTTP backend sends
credentials as Basic Auth, not Bearer tokens)
- Rate limiting, lock TTL (1 hour), JSON validation, lock enforcement
- OpenTofu connect command template with registry env vars
Feature gate:
- FeatureOpenTofu organization feature controls access to OpenTofu
- Added to ProFeatures list (Pro/Trial/Enterprise orgs get it by default)
- Organization settings API: openTofuEnabled toggle
- Frontend: OpenTofu app type radio only visible when feature is enabled
- Migration adds 'opentofu' to both deployment_type and feature enums
Agent:
- Polling loop following Docker/Kubernetes agent patterns exactly
- OCI artifact pull for OpenTofu configurations from Distr registry
- terraform-exec SDK for tofu init/plan/apply/destroy
- Graceful shutdown: tofu operations use non-cancellable context so
SIGTERM does not kill apply mid-mutation
- Smart workspace caching: only re-pull on revision change
- Security: version string validation, HCL key validation, reserved
backend config key blocklist, 0600 file permissions
- Empty deployments safety guard against transient Hub errors
Frontend:
- Extended DeploymentType with 'opentofu' in SDK types
- Application version creation form (OCI Config Reference + Tag)
- Deployment wizard and form with OpenTofu-specific field handling
- Version table columns for Config URL and Config Version
- OpenTofu symbol icon
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove TofuVersion field entirely (kosmoz distr-sh#16): - Docker image already bakes a tofu binary; no need to support per-deployment version overrides which add a variable that could cause issues - Removed from agent (AgentDeployment, NewAgentDeployment, resolveTofuBinary, prepareTofuExecutor, downloadTofuVersion -> downloadDefaultTofu) - Removed from API (DeploymentRequest.TofuVersion, AgentDeployment.TofuVersion) - Removed from types (DeploymentRevision, DeploymentWithLatestRevision) - Removed from DB (INSERT, SELECT in CreateDeploymentRevision) - Removed from Hub handler (populateOpenTofuDeployment) - Removed from migration (tofu_version column) - Removed from SDK types (tofuVersion field) Fix versions table missing fields (kosmoz distr-sh#4): - applicationWithVersionsOutputExpr and applicationWithEntitledVersionsOutputExpr array_agg queries now include tofu_config_url and tofu_config_version, so the UI versions table shows the OCI ref and tag instead of empty URL and "latest"
- Rename state.go -> agent_state.go (kosmoz) - Move OpenTofuState struct from internal/db -> internal/types (kosmoz) - Use r.PathValue() instead of chi.URLParam() (kosmoz) - lockInfo struct -> map[string]string (forward compat with OpenTofu HTTP backend protocol; kosmoz) - Long line wrap in array_agg query
- Wizard now shows correct OpenTofu badge (faCubes icon, "OpenTofu" label) for opentofu apps instead of falling through to Kubernetes (kosmoz distr-sh#2) - Hide "Enable logs collection" toggle for OpenTofu deployments since log collection is not currently supported for tofu (kosmoz distr-sh#6)
- CreateDeploymentRevision: pre-seed args["tofuVars"] and args["tofuBackendConfig"] to nil. Without this, every Docker and Kubernetes deployment creation fails because pgx errors when a named argument referenced in SQL is missing from the args map. - routing: replace global chimiddleware.RequestSize(1MiB) with conditionalRequestSize that exempts /api/v1/state/. The 1 MiB cap was making the state handler's 20 MiB MaxBytesReader dead code, so any non-trivial Terraform state would be rejected at the middleware layer. - types/deployment.go: add JSON tags to TofuVars and TofuBackendConfig on DeploymentWithLatestRevision so the frontend can read existing values when editing deployments. Previously json:"-" stripped them.
Address kosmoz feedback: password hashing on every state request is too expensive, and the basic-auth-with-target-secret approach duplicates the agent auth flow. - AgentAuthentication now extracts JWT from either Bearer header or Basic Auth password. The Basic Auth path is required because OpenTofu's HTTP backend only sends Basic Auth credentials, but reusing the agent JWT means the same auth chain handles both /agent routes and the state backend with no per-request bcrypt. - State routes now use auth.AgentAuthentication.Middleware + agentAuthDeploymentTargetCtxMiddleware (the existing chain for /agent routes) instead of a custom Basic Auth middleware that reverified the target secret on every request. - Removed basicAuthDeploymentTargetCtxMiddleware (no longer used). - Agent prepareTofuExecutor: send client.RawToken() as the Basic Auth password instead of DISTR_TARGET_SECRET. Username is unused but retained because the protocol requires both fields. - Removed unused targetID and targetSecret state from initHubConfig.
Address kosmoz feedback: validation should run as middleware so handlers stay focused on their actual work, matching the pattern of deploymentMiddleware in handlers/deployments.go. stateDeploymentMiddleware now validates the deployment target type and ownership of the path's deploymentID before any handler runs. Each handler re-parses the UUID with uuid.MustParse since the middleware already verified it's valid.
Address kosmoz feedback: don't duplicate S3 client setup. Export ClientOpts from internal/registry/blob/s3 (was unexported clientOpts) and use it in newStateS3Client so credential, region, endpoint, and checksum options stay in sync between the registry blob handler and the OpenTofu state backend.
Address kosmoz distr-sh#5 — UI didn't provide a way to set tofuVars and tofuBackendConfig. Both are now JSON editors visible only for opentofu deployments and parsed before sending the deployment request.
- Add docs/opentofu-agent-example.md walking through the full end-to-end test against LocalStack (sample main.tf, oras push, UI flow, verification). Addresses kosmoz's request for a "hello world" OCI artifact example. - Replace OpenTofu icon with a monochrome version on a blue background to match Docker and Kubernetes icons in the application list. Addresses kosmoz distr-sh#18.
- Agent: call signal.NotifyContext's stop function on main exit so a second SIGTERM/SIGINT escalates to default behavior. Without this, repeated signals just keep cancelling the same context, so a hung agent could not be force-stopped (kosmoz #3131403354). - DB: pgx encodes map[string]any directly to JSONB, so drop the manual json.Marshal in CreateDeploymentRevision and pass the maps as-is via named args. The defaults seed @tofuVars/@tofuBackendConfig as nil for non-OpenTofu deployments so the @-named refs always resolve (kosmoz #3130750684).
Mirrors the existing `hub` ignore for the OpenTofu agent dev binary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds demos/opentofu-localstack-test/ with the bash script and sample main.tf used to validate the OpenTofu agent against LocalStack + the Hub state backend end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Re-runs `pnpm run docs` after rebase. Output is purely cosmetic — typedoc renders optional fields as `**name?**:` instead of `**name**:`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sion Migration 87 was added on origin/main (87_disk_metrics) while we were also using 87 for 87_add_opentofu_deployment_type. Highest existing migration on main is 95; bump ours to 96. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Formatting drift introduced during rebase. No behavior changes — just whitespace/wrapping that prettier wants. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
0e276e4 to
b1e23cb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 52 out of 55 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <app-editor | ||
| language="json" | ||
| class="block p-2.5 w-full font-mono text-sm text-gray-900 caret-gray-900 bg-gray-50 rounded-lg border border-gray-300 focus:ring-blue-500 focus:border-blue-500 dark:bg-gray-600 dark:border-gray-500 dark:placeholder-gray-400 dark:text-white dark:caret-white dark:focus:ring-blue-500 dark:focus:border-blue-500" | ||
| formControlName="tofuVars"> | ||
| </app-editor> | ||
| <div class="text-xs text-gray-500 dark:text-gray-400 mt-1"> | ||
| Variables passed to <code>tofu apply</code> as <code>terraform.tfvars.json</code>. Example: | ||
| <code>{{ '{' }}"region": "us-east-1", "instance_count": 3{{ '}' }}</code> | ||
| </div> | ||
| </div> | ||
| <div> | ||
| <label class="block mb-2 text-sm font-medium text-gray-900 dark:text-white" | ||
| >OpenTofu Backend Config (JSON)</label | ||
| > | ||
| <app-editor | ||
| language="json" | ||
| class="block p-2.5 w-full font-mono text-sm text-gray-900 caret-gray-900 bg-gray-50 rounded-lg border border-gray-300 focus:ring-blue-500 focus:border-blue-500 dark:bg-gray-600 dark:border-gray-500 dark:placeholder-gray-400 dark:text-white dark:caret-white dark:focus:ring-blue-500 dark:focus:border-blue-500" | ||
| formControlName="tofuBackendConfig"> | ||
| </app-editor> |
There was a problem hiding this comment.
These <app-editor> components have no content but are written with explicit opening/closing tags. The repo convention is to use self-closing tags for Angular components without content (and the same file already uses <app-editor ... /> elsewhere). Switch these to self-closing tags for consistency.
| for cmd in oras curl jq; do | ||
| if ! command -v "$cmd" &>/dev/null; then | ||
| error "$cmd is required but not found" | ||
| exit 1 | ||
| fi | ||
| done | ||
|
|
||
| # Check Hub is running | ||
| if ! curl -sf "${HUB_URL}/api/v1/health" &>/dev/null; then | ||
| # Try a simple GET | ||
| if ! curl -sf -o /dev/null -w "%{http_code}" "${HUB_URL}" &>/dev/null; then | ||
| error "Hub is not running at ${HUB_URL}. Start with: mise run serve" | ||
| exit 1 | ||
| fi | ||
| fi | ||
| info "Hub is reachable at ${HUB_URL}" | ||
|
|
||
| # Check LocalStack | ||
| if ! curl -sf "http://localhost:4566/_localstack/health" &>/dev/null; then | ||
| error "LocalStack is not running. Start with: docker compose up -d" | ||
| exit 1 | ||
| fi | ||
| info "LocalStack is running" | ||
|
|
||
| # ── 2. Register user ──────────────────────────────────────── | ||
| EMAIL="opentofu-test@distr.sh" | ||
| PASSWORD="TestPassword123!" | ||
| ORG_NAME="opentofu-test-org" | ||
|
|
||
| info "Registering user ${EMAIL}..." | ||
| REGISTER_RESP=$(curl -sf -X POST "${HUB_URL}/api/v1/auth/register" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d "{\"email\":\"${EMAIL}\",\"password\":\"${PASSWORD}\",\"organizationName\":\"${ORG_NAME}\"}" \ | ||
| 2>/dev/null) || true | ||
|
|
||
| # Auto-verify via Mailpit if available | ||
| if curl -sf "http://localhost:8025/api/v1/messages" &>/dev/null; then | ||
| info "Checking Mailpit for verification email..." | ||
| sleep 2 | ||
| VERIFY_JWT=$(curl -sf "http://localhost:8025/api/v1/messages" | python3 -c " | ||
| import sys, json, re | ||
| msgs = json.load(sys.stdin) | ||
| for m in msgs.get('messages', []): | ||
| msg = json.loads(sys.stdin.buffer.read()) if False else None | ||
| " 2>/dev/null) || true | ||
| # Extract JWT from latest email HTML | ||
| MSG_ID=$(curl -sf "http://localhost:8025/api/v1/messages" | python3 -c "import sys, json; msgs=json.load(sys.stdin)['messages']; print(msgs[0]['ID'] if msgs else '')" 2>/dev/null) |
There was a problem hiding this comment.
The script uses python3 (Mailpit parsing) but doesn’t include it in the prerequisites check (it only checks oras, curl, jq). Either add python3 to the required commands or remove the python-based Mailpit parsing block (the current VERIFY_JWT=... snippet is also dead/incomplete).
| li, err := JsonBody[lockInfo](w, r) | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| if err := db.UnlockState(ctx, deploymentID, li["ID"]); err != nil { | ||
| if errors.Is(err, apierrors.ErrConflict) { | ||
| http.Error(w, "lock ID mismatch", http.StatusConflict) | ||
| return |
There was a problem hiding this comment.
handleStateUnlock calls db.UnlockState(..., li["ID"]) without checking that the lock info includes a non-empty ID. If ID is missing, this will attempt an unlock with an empty lock ID and return a conflict, which is misleading. Validate li["ID"] and return 400 for malformed unlock requests.
| <fa-icon [icon]="faDocker" class="mr-1" /> | ||
| Docker | ||
| } @else if (app.type === 'opentofu') { | ||
| <fa-icon [icon]="faCubes" class="mr-1"></fa-icon> |
There was a problem hiding this comment.
This <fa-icon> has no content but uses an explicit closing tag. The repo convention (see CLAUDE.md) is to use self-closing tags for Angular components with no content; for consistency, use <fa-icon ... /> here as well.
| <fa-icon [icon]="faCubes" class="mr-1"></fa-icon> | |
| <fa-icon [icon]="faCubes" class="mr-1" /> |
|
|
||
| stateURL := fmt.Sprintf("%s/api/v1/state/%s", hubBase, deploymentID.String()) | ||
|
|
||
| // The Hub authenticates state requests via the agent JWT supplied as the | ||
| // Basic Auth password. Username is unused but required by the protocol. | ||
| initOpts := []tfexec.InitOption{ | ||
| tfexec.BackendConfig(fmt.Sprintf("address=%s", stateURL)), | ||
| tfexec.BackendConfig(fmt.Sprintf("lock_address=%s/lock", stateURL)), | ||
| tfexec.BackendConfig(fmt.Sprintf("unlock_address=%s/unlock", stateURL)), | ||
| tfexec.BackendConfig("lock_method=POST"), | ||
| tfexec.BackendConfig("unlock_method=POST"), | ||
| tfexec.BackendConfig("username=agent"), | ||
| tfexec.BackendConfig(fmt.Sprintf("password=%s", client.RawToken())), |
There was a problem hiding this comment.
The agent JWT is passed to tofu init via -backend-config password=.... This puts a bearer credential in the process argv, which can be readable by other local users/processes and may end up in diagnostics. Prefer providing the HTTP backend credentials via environment variables or a permissions-restricted config file to avoid exposing the token via command-line args.
| stateURL := fmt.Sprintf("%s/api/v1/state/%s", hubBase, deploymentID.String()) | |
| // The Hub authenticates state requests via the agent JWT supplied as the | |
| // Basic Auth password. Username is unused but required by the protocol. | |
| initOpts := []tfexec.InitOption{ | |
| tfexec.BackendConfig(fmt.Sprintf("address=%s", stateURL)), | |
| tfexec.BackendConfig(fmt.Sprintf("lock_address=%s/lock", stateURL)), | |
| tfexec.BackendConfig(fmt.Sprintf("unlock_address=%s/unlock", stateURL)), | |
| tfexec.BackendConfig("lock_method=POST"), | |
| tfexec.BackendConfig("unlock_method=POST"), | |
| tfexec.BackendConfig("username=agent"), | |
| tfexec.BackendConfig(fmt.Sprintf("password=%s", client.RawToken())), | |
| tf.SetEnv(append( | |
| os.Environ(), | |
| "TF_HTTP_USERNAME=agent", | |
| fmt.Sprintf("TF_HTTP_PASSWORD=%s", client.RawToken()), | |
| )) | |
| stateURL := fmt.Sprintf("%s/api/v1/state/%s", hubBase, deploymentID.String()) | |
| // The Hub authenticates state requests via the agent JWT supplied via | |
| // HTTP backend environment variables. Username is unused but required by | |
| // the protocol. | |
| initOpts := []tfexec.InitOption{ | |
| tfexec.BackendConfig(fmt.Sprintf("address=%s", stateURL)), | |
| tfexec.BackendConfig(fmt.Sprintf("lock_address=%s/lock", stateURL)), | |
| tfexec.BackendConfig(fmt.Sprintf("unlock_address=%s/unlock", stateURL)), | |
| tfexec.BackendConfig("lock_method=POST"), | |
| tfexec.BackendConfig("unlock_method=POST"), |
Summary
Adds OpenTofu Agent support to Distr, enabling infrastructure provisioning alongside container deployments. Vendors can now manage the full deployment lifecycle — applications and infrastructure — through a single Distr workflow.
Closes #1946
What's included
opentofudeployment type, tofu columns onapplicationversion/deploymentrevision,opentofu_statetable for lock managementcmd/agent/opentofu): Polling loop, reconciliation, OCI artifact pull,tofu init/plan/apply/destroyvia terraform-exec SDKArchitecture
The agent authenticates via target ID + secret (same as Docker/K8s agents), pulls OpenTofu configs packaged as OCI artifacts, and executes
tofu applywith state managed through the Hub's HTTP backend.Key design decisions
ws/notworkspaces/) to stay under macOS path length limits with.terraform/Test plan
Assisted by: Claude noreply@anthropic.com