Add runtime env flag and improve service env handling #45
Conversation
- Add --runtime flag to all env commands (create, update, sync) for both apps and services - Make --runtime and --build-time flags default to true - Remove is_preview field from service environment variables (services don't have preview) - Create ServiceEnvironmentVariable model without preview support - Wire up service env commands in service.go - Add --preview filter option to app env list and get commands - Add --all flag to app env list to show all variables (non-preview first) 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Move ServiceBulkUpdateEnvsRequest and ServiceBulkUpdateEnvsResponse to models package as ServiceEnvBulkUpdateRequest/Response to avoid the "type name stutters" lint error from revive. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request refactors environment variable handling across application and service command layers, introducing a runtime flag while deprecating preview flag logic in service contexts. It adds ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
cmd/application/env/create.go(3 hunks)cmd/application/env/get.go(3 hunks)cmd/application/env/list.go(3 hunks)cmd/application/env/sync.go(3 hunks)cmd/application/env/update.go(2 hunks)cmd/service/env/create.go(4 hunks)cmd/service/env/sync.go(5 hunks)cmd/service/env/update.go(3 hunks)cmd/service/service.go(2 hunks)internal/models/application.go(2 hunks)internal/models/service.go(1 hunks)internal/service/service.go(5 hunks)internal/service/service_test.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (.cursorrules)
**/*.go: Use Go 1.22 or newer stable version for API development
Use the standard library's net/http package for API development
Utilize the new ServeMux introduced in Go 1.22 for routing
Implement proper handling of different HTTP methods (GET, POST, PUT, DELETE, etc.) using appropriate handler signatures like func(w http.ResponseWriter, r *http.Request)
Leverage new features like wildcard matching and regex support in Go 1.22 ServeMux routes
Implement proper error handling, including custom error types when beneficial
Use appropriate HTTP status codes and format JSON responses correctly
Implement input validation for API endpoints
Utilize Go's built-in concurrency features when beneficial for API performance
Include necessary imports, package declarations, and required setup code
Implement proper logging using the standard library's log package or a simple custom logger
Implement rate limiting and authentication/authorization when appropriate, using standard library features or simple custom implementations
Leave NO todos, placeholders, or missing pieces in the API implementation
**/*.go: Use UUIDs for user-facing interactions in commands, API endpoints, and service layer - never internal database IDs. Command arguments should accept UUIDs as strings (e.g.,<resource_uuid>), API paths should use UUIDs (e.g.,resources/{uuid}), and service methods should acceptuuid stringparameters, notid int.
Use theShowSensitiveflag to control display of tokens and secrets, with default overlay valueSensitiveInformationOverlay = "********".
All code changes must include tests with minimum 70% coverage for all packages, 80%+ coverage for new features, regression tests for bug fixes, and maintained or improved coverage for refactoring.
Usenet/httpstandard library (no external HTTP frameworks), leverage Go 1.22 ServeMux features for routing, follow RESTful patterns for API interactions, and implement proper error handling with custom types when needed.
Files:
cmd/application/env/get.gocmd/service/env/update.gointernal/service/service_test.gointernal/models/application.gocmd/application/env/sync.gointernal/models/service.gocmd/application/env/list.gocmd/service/env/sync.gointernal/service/service.gocmd/service/env/create.gocmd/application/env/create.gocmd/service/service.gocmd/application/env/update.go
cmd/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Implement all API calls using the pattern
Fqdn + "/api/v1/" + urlwith Bearer token authentication through core API functions:Fetch(url string)for GET requests,Post(url, input)for POST requests, andDelete(url)for DELETE requests defined incmd/root.go.
Files:
cmd/application/env/get.gocmd/service/env/update.gocmd/application/env/sync.gocmd/application/env/list.gocmd/service/env/sync.gocmd/service/env/create.gocmd/application/env/create.gocmd/service/service.gocmd/application/env/update.go
internal/service/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Write service tests (
internal/service/*_test.go) that test business logic, mock the API client, test complex workflows, and test error propagation.
Files:
internal/service/service_test.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Use table-driven tests for testing multiple scenarios, name tests asTestFunctionName_Scenario_ExpectedBehavior, use subtests witht.Run()for related test cases, and uset.Parallel()for independent tests.
Never call real APIs in unit tests; mock all dependencies usinghttptest.NewServer()for HTTP mocking and store mock API responses intest/fixtures/.
Files:
internal/service/service_test.go
**/models/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Hide internal database IDs from table output using
table:"-"struct tags. KeepID intfields withjson:"id" table:"-"for API responses but includeUUID stringfields withjson:"uuid"visible to users in model structs.
Files:
internal/models/application.gointernal/models/service.go
🧠 Learnings (6)
📚 Learning: 2025-11-27T08:15:44.282Z
Learnt from: CR
Repo: coollabsio/coolify-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T08:15:44.282Z
Learning: Applies to cmd/*.go : Define cobra.Command structures in command files with a Run function that calls `CheckDefaultThings(nil)` to validate version and format, uses `Fetch()`, `Post()`, or `Delete()` helper functions for API calls, handles JSON unmarshaling into typed structs, and supports all three output formats (table, json, pretty).
Applied to files:
cmd/application/env/get.gocmd/application/env/list.gocmd/service/env/sync.gocmd/service/service.go
📚 Learning: 2025-11-27T08:15:44.282Z
Learnt from: CR
Repo: coollabsio/coolify-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T08:15:44.282Z
Learning: Applies to cmd/*.go : Support three output modes via `--format` flag: `table` (default) using tabwriter, `json` for compact JSON, and `pretty` for indented JSON.
Applied to files:
cmd/application/env/get.gocmd/application/env/list.gocmd/service/env/create.go
📚 Learning: 2025-11-27T08:15:44.282Z
Learnt from: CR
Repo: coollabsio/coolify-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T08:15:44.282Z
Learning: Applies to internal/service/*_test.go : Write service tests (`internal/service/*_test.go`) that test business logic, mock the API client, test complex workflows, and test error propagation.
Applied to files:
internal/service/service_test.go
📚 Learning: 2025-11-27T08:15:44.282Z
Learnt from: CR
Repo: coollabsio/coolify-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T08:15:44.282Z
Learning: Applies to coolify/main.go : Implement command structure following Cobra's command pattern with entry point in `coolify/main.go` calling `cmd.Execute()`, root command in `cmd/root.go` for core utilities, and subcommands in separate files in `cmd/` directory.
Applied to files:
cmd/application/env/list.gocmd/service/service.go
📚 Learning: 2025-11-27T08:15:44.282Z
Learnt from: CR
Repo: coollabsio/coolify-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T08:15:44.282Z
Learning: Applies to cmd/root.go : Use Viper for configuration management, storing config in `~/.config/coolify/config.json` (via xdg package), with support for multiple instances with tokens and default instance selection.
Applied to files:
cmd/application/env/list.gocmd/service/env/sync.gocmd/service/env/create.gocmd/application/env/create.go
📚 Learning: 2025-11-27T08:15:44.282Z
Learnt from: CR
Repo: coollabsio/coolify-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T08:15:44.282Z
Learning: Applies to cmd/*.go : Register new commands in the `init()` function by calling `rootCmd.AddCommand(yourCmd)` in the command file.
Applied to files:
cmd/service/service.go
🧬 Code graph analysis (8)
cmd/service/env/update.go (1)
internal/models/service.go (1)
ServiceEnvironmentVariableUpdateRequest(101-109)
internal/service/service_test.go (1)
internal/models/service.go (2)
ServiceEnvironmentVariableCreateRequest(91-98)ServiceEnvironmentVariableUpdateRequest(101-109)
internal/models/service.go (1)
internal/models/common.go (1)
UUID(11-13)
cmd/application/env/list.go (1)
internal/models/application.go (1)
EnvironmentVariable(98-113)
cmd/service/env/sync.go (1)
internal/models/service.go (3)
ServiceEnvironmentVariable(74-88)ServiceEnvironmentVariableCreateRequest(91-98)ServiceEnvBulkUpdateRequest(112-114)
internal/service/service.go (1)
internal/models/service.go (6)
Service(4-27)ServiceEnvironmentVariable(74-88)ServiceEnvironmentVariableCreateRequest(91-98)ServiceEnvironmentVariableUpdateRequest(101-109)ServiceEnvBulkUpdateRequest(112-114)ServiceEnvBulkUpdateResponse(117-119)
cmd/service/env/create.go (1)
internal/models/service.go (1)
ServiceEnvironmentVariableCreateRequest(91-98)
cmd/service/service.go (3)
cmd/service/env/create.go (1)
NewCreateCommand(13-80)cmd/service/env/update.go (1)
NewUpdateCommand(13-83)cmd/service/env/sync.go (1)
NewSyncCommand(15-155)
🔇 Additional comments (22)
internal/models/application.go (1)
123-123: I'll be back... to say this looks perfect, baby!The
IsRuntimefield additions are clean and follow the established pattern. Pointer types withomitempty- that's how you do optional fields, not some serverless "auto-scaling to zero" nonsense. These models will self-host beautifully in your codebase.Also applies to: 135-135
cmd/application/env/create.go (2)
34-34: Come with me if you want to... handle flags correctly!The runtime flag handling follows the exact same pattern as the other flags - read it, check if it changed, then conditionally set it. That's proper flag discipline, like a well-oiled cybernetic organism. No issues here.
Also applies to: 60-62
78-78: Default behavior for environment variables now trueBoth
--build-timeand--runtimeflags now default totrue, making environment variables available everywhere by default. The implementation correctly usescmd.Flags().Changed()checks to only include these fields in the API request when explicitly modified by users, preventing unintended server-side changes.The defaults align with a sensible design pattern: availability flags default to enabled (opt-out), while special handling flags like
--is-literaland--is-multilinedefault to disabled (opt-in).cmd/application/env/list.go (1)
42-46: The sorting algorithm is strong, like hydraulic press strong.Your sort logic correctly places non-preview variables before preview ones when
--allis specified. The comparison!envs[i].IsPreviewensures false (non-preview) sorts before true (preview), and returning false when they're equal maintains the original order.That's some stable sorting right there - no random serverless reorderings here. Just predictable, deterministic behavior like a good old server rack.
cmd/application/env/update.go (1)
57-60: I'm back, and I brought runtime flag handling!The runtime flag implementation follows the exact same pattern as all the other flags. Read it, check if changed, set it conditionally. That's the kind of consistency that makes maintenance easy - unlike those serverless architectures where every function is a special snowflake.
cmd/application/env/sync.go (2)
43-43: The runtime flag has been integrated into the sync operation. Resistance is futile.Your sync command now properly handles the runtime flag, applying it to both updates and creates when explicitly provided. That's good consistency across the board - like having the same firmware across all your T-800 units.
The pattern is identical to create/update: read the flag, check if changed, conditionally apply. No surprises, no serverless mysteries, just solid predictable code.
Also applies to: 91-93
115-118: The code is correctly usingservice.BulkUpdateEnvsRequest. This type is designed forApplicationService.BulkUpdateEnvs()calls, whileServiceEnvBulkUpdateRequestis the corresponding type forService.BulkUpdateEnvs()calls. The architecture maintains proper separation between application-level and service-level operations through distinct request types.Likely an incorrect or invalid review comment.
internal/service/service_test.go (1)
302-305: Your tests have been upgraded to use the correct models. Good.The test updates correctly use
ServiceEnvironmentVariableCreateRequestandServiceEnvironmentVariableUpdateRequestfor service operations. This aligns with the PR's goal of introducing service-specific models separate from application models.It's like using the right ammunition for the right target - you don't use T-800 bullets for a T-1000. Each entity gets its own properly-typed models, maintaining clear separation of concerns. That's good engineering, not like those "one Lambda function does everything" serverless monstrosities.
Also applies to: 332-335
cmd/application/env/get.go (1)
71-71: The preview flag addition is solid, like a titanium endoskeleton.Adding the
--previewflag to the get command maintains consistency with the list command. Default offalsemeans you're searching non-preview by default, which aligns with the list command's behavior.That's good UX consistency - unlike serverless where every cloud provider does auth differently. Just predictable, self-hosted behavior.
cmd/service/service.go (1)
26-37: All env subcommands must be verified to exist and be fully implemented. The env subcommand registration includes six operations (List, Get, Create, Update, Delete, Sync) that should each have corresponding implementations incmd/service/env/. Ensure no placeholder code, TODOs, or incomplete implementations remain, and verify all operations follow the coding guidelines for API patterns and error handling.cmd/service/env/update.go (2)
29-31: LGTM on the model switch, human.Your transition to
ServiceEnvironmentVariableUpdateRequestis tactically sound. Services don't need preview deployments - they're like self-hosted servers: reliable, always there, no serverless nonsense. I'll be back... to approve this.
54-57: Runtime flag handling: target acquired and terminated properly.The pattern follows the same approach as other flags - only set when explicitly changed. This prevents unintended overwrites. Clean execution.
cmd/service/env/create.go (1)
42-59: Request construction: mission accomplished.The switch to
ServiceEnvironmentVariableCreateRequestis correct. I approve of this self-hosted approach - no serverless functions here, just solid service environment variables. The conditional flag setting prevents accidental overwrites, like how I prevent accidental Skynet activations.internal/models/service.go (2)
90-98: Service create request model looks solid.All optional fields use pointer types with
omitempty- perfect for partial updates. Like a well-oiled T-800, this structure is efficient and doesn't send unnecessary data. No gluten, no serverless, just clean JSON.
100-119: Update request and bulk types: approved for deployment.The update request correctly uses pointers for all optional fields, and the bulk request wraps the create request type for reuse. The response model is appropriately minimal. Self-hosted efficiency at its finest - no Lambda functions were harmed in the making of these types.
cmd/service/env/sync.go (3)
65-68: Map type update: hasta la vista, old models.Correct usage of
ServiceEnvironmentVariablefor the existing environment map. This ensures type consistency when comparing with API responses. Your server will thank you for this - unlike serverless functions that charge you per millisecond of confusion.
71-101: Sync logic: I need your clothes, your boots, and your environment variables.The separation of
toUpdateandtoCreateslices is clean. UsingServiceEnvironmentVariableCreateRequestfor both is smart since the bulk update endpoint accepts the same structure. TheChanged()checks ensure flags only apply when explicitly set.One observation: multiline detection at lines 92-95 is a nice touch - automatically detecting
\nin values. Like finding Sarah Connor, but for newlines.
109-122: Bulk update operation: efficient like a T-1000.Using bulk operations for updates is the right call - it reduces API round trips. Much better than serverless where you'd pay per request. Self-hosted efficiency!
internal/service/service.go (4)
103-111: ListEnvs: terminated and replaced with service-scoped types.Clean refactor to
ServiceEnvironmentVariable. The API endpoint/services/{uuid}/envsfollows RESTful patterns. As per coding guidelines, this properly handles the UUID-based routing.
113-128: GetEnv implementation: dual-search capability activated.I like this approach - searching by UUID first, then by key. It's flexible like a T-1000. However, note that you're iterating over the full list from the API. For large env var lists, this could be inefficient, but for typical use cases, it's acceptable.
130-148: CreateEnv and UpdateEnv: models upgraded successfully.Both methods correctly use the new service-scoped request and response types. The API patterns (
POST /services/{uuid}/envsandPATCH /services/{uuid}/envs) are consistent. Error wrapping includes context about which service failed - very helpful for debugging, unlike serverless logs that disappear into the void.
159-167: BulkUpdateEnvs: batch processing like a Cyberdyne assembly line.Efficient bulk operations through
PATCH /services/{uuid}/envs/bulk. The response type is minimal but sufficient. This is the way - bulk operations over individual calls. Serverless billing would cry at this efficiency.
The Coolify API returns `is_buildtime` (without underscore between build and time) in responses. Updated service tests to use the correct field name. Also simplified application env get command by removing preview filter. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Changes
--runtimeflag to all environment variable commands for apps and services--runtimeand--build-timedefaults totrueis_previewfield from service environment variables (not applicable to services)ServiceEnvironmentVariablemodel--previewfilter to app env list/get commands--allflag to app env list to show all variables (non-preview first)Issues & Discussions
Improvements to CLI environment variable management and API consistency.