feat(config): support workspace configuration#80
Merged
feloy merged 6 commits intokortex-hub:mainfrom Mar 19, 2026
Merged
Conversation
Updates the kortex-cli-api/cli/go dependency to the latest commit abc0e6182754b595b5dc3087d0a6ef5ab51fde3f. Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Imports workspace configuration model from kortex-cli-api and adds comprehensive validation for environment variables and mount paths. Environment variable names must be valid Unix identifiers, and exactly one of value or secret must be defined. Mount paths must be relative and non-empty. Validation is integrated into the init command to catch configuration errors early. Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Documents the workspace.json configuration file format, including environment variables, mount paths, validation rules, and practical examples for configuring workspace runtime behavior. Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Adds detection for duplicate environment variable names in workspace configuration and a test to verify they are properly rejected with descriptive error messages. Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Changed runtime.CreateParams to accept WorkspaceConfiguration instead of ConfigPath. The init command now loads the configuration and passes it to manager.Add() via a new AddOptions struct, centralizing config handling at the command level and removing redundant loading. Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR adds a workspace config loader/validator ( Changes
Sequence Diagram(s)sequenceDiagram
participant InitCmd as Init Command
participant ConfigMgr as Config Manager
participant InstanceMgr as Instance Manager
participant Runtime as Runtime
InitCmd->>ConfigMgr: NewConfig(absConfigDir)
ConfigMgr->>ConfigMgr: ensure path absolute
InitCmd->>ConfigMgr: Load()
ConfigMgr->>ConfigMgr: read workspace.json
ConfigMgr->>ConfigMgr: unmarshal JSON
ConfigMgr->>ConfigMgr: validate env vars (name, unique, value xor secret)
ConfigMgr->>ConfigMgr: validate mounts (non-empty, relative)
ConfigMgr-->>InitCmd: WorkspaceConfiguration | error
rect rgba(100, 200, 100, 0.5)
InitCmd->>InstanceMgr: Add(ctx, AddOptions{Instance, RuntimeType, WorkspaceConfig})
end
InstanceMgr->>InstanceMgr: validate AddOptions
InstanceMgr->>Runtime: Create({SourcePath, WorkspaceConfig})
Runtime->>Runtime: augment Info if WorkspaceConfig present
Runtime-->>InstanceMgr: RuntimeInfo
InstanceMgr-->>InitCmd: Instance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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: 1
🧹 Nitpick comments (2)
pkg/config/config_test.go (1)
174-205: Add an unknown-key sibling to the malformed-JSON case.This file covers syntax errors well, but it still misses the common typo case where the JSON is valid and a field name is wrong. A subtest with an unexpected or misspelled key would lock in the fail-fast behavior this PR is aiming for.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/config/config_test.go` around lines 174 - 205, Add a subtest alongside the malformed-JSON case that writes syntactically valid JSON containing an unexpected/misspelled key (e.g., {"unkownKey": "value"} or a misspelled expected field) to WorkspaceConfigFile, then call NewConfig(configDir) and cfg.Load() and assert it returns a validation/unknown-key error (not nil and not ErrConfigNotFound); update the test to reference NewConfig, cfg.Load, WorkspaceConfigFile and ErrConfigNotFound so it fails fast when unknown keys are present.pkg/instances/manager_test.go (1)
267-298: Add oneWorkspaceConfigpropagation case inTestManager_Add.Every updated call here still passes only
InstanceandRuntimeType. SinceAddOptionsnow also carriesWorkspaceConfig, a regression that drops that payload before runtime creation would leave this suite green. A tiny spy/fake runtime that records the received config would be enough to cover it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/instances/manager_test.go` around lines 267 - 298, Update TestManager_Add to include a subtest that passes a non-nil WorkspaceConfig through AddOptions and asserts the runtime factory actually received it: create a small spy fake runtime/factory (e.g., newFakeRuntimeSpy or extend fakeInstanceFactory) that records the WorkspaceConfig it was invoked with, call manager.Add(ctx, AddOptions{Instance: inst, RuntimeType: "fake", WorkspaceConfig: myConfig}) and then assert the spy saw the exact myConfig; place the assertion alongside the existing success assertions so regressions that drop WorkspaceConfig during Manager.Add or during runtime creation are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/config/config.go`:
- Around line 111-133: The validation currently uses filepath.IsAbs(dep) /
filepath.IsAbs(conf) which is platform-dependent; update the checks inside the
loops over cfg.Mounts.Dependencies and cfg.Mounts.Configs to treat Unix-style
leading "/" (and leading backslash) as absolute as well (e.g. consider a path
absolute if filepath.IsAbs(...) OR strings.HasPrefix(path, "/") OR
strings.HasPrefix(path, "\\") or other appropriate detection), so
ErrInvalidConfig is returned for such paths consistently across platforms; make
the change where filepath.IsAbs is called in the dependency and config
validation blocks.
---
Nitpick comments:
In `@pkg/config/config_test.go`:
- Around line 174-205: Add a subtest alongside the malformed-JSON case that
writes syntactically valid JSON containing an unexpected/misspelled key (e.g.,
{"unkownKey": "value"} or a misspelled expected field) to WorkspaceConfigFile,
then call NewConfig(configDir) and cfg.Load() and assert it returns a
validation/unknown-key error (not nil and not ErrConfigNotFound); update the
test to reference NewConfig, cfg.Load, WorkspaceConfigFile and ErrConfigNotFound
so it fails fast when unknown keys are present.
In `@pkg/instances/manager_test.go`:
- Around line 267-298: Update TestManager_Add to include a subtest that passes a
non-nil WorkspaceConfig through AddOptions and asserts the runtime factory
actually received it: create a small spy fake runtime/factory (e.g.,
newFakeRuntimeSpy or extend fakeInstanceFactory) that records the
WorkspaceConfig it was invoked with, call manager.Add(ctx, AddOptions{Instance:
inst, RuntimeType: "fake", WorkspaceConfig: myConfig}) and then assert the spy
saw the exact myConfig; place the assertion alongside the existing success
assertions so regressions that drop WorkspaceConfig during Manager.Add or during
runtime creation are caught.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 294498e2-f3c9-4c5d-a45b-fe1f500bd198
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
AGENTS.mdREADME.mdgo.modpkg/cmd/init.gopkg/cmd/init_test.gopkg/cmd/workspace_list_test.gopkg/cmd/workspace_remove_test.gopkg/cmd/workspace_start_test.gopkg/cmd/workspace_stop_test.gopkg/config/config.gopkg/config/config_test.gopkg/instances/manager.gopkg/instances/manager_test.gopkg/runtime/fake/fake.gopkg/runtime/fake/fake_test.gopkg/runtime/runtime.go
Use platform-appropriate absolute paths in tests instead of hardcoded Unix-style paths. Use tempDir for absolute paths and filepath.ToSlash for JSON representation to ensure tests work on both Unix and Windows. Signed-off-by: Philippe Martin <phmartin@redhat.com>
benoitf
approved these changes
Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #79