Skip to content

feat(runtime/podman): add configurable runtime settings (#84)#94

Merged
feloy merged 11 commits intokortex-hub:mainfrom
feloy:podman-config
Mar 26, 2026
Merged

feat(runtime/podman): add configurable runtime settings (#84)#94
feloy merged 11 commits intokortex-hub:mainfrom
feloy:podman-config

Conversation

@feloy
Copy link
Copy Markdown
Contributor

@feloy feloy commented Mar 23, 2026

Introduces a configuration system for the Podman runtime, allowing customization of the base image, packages, sudo permissions, and agent setup through JSON files.

Configuration files are auto-generated in the runtime storage directory (image.json for base image, claude.json for agent). The Containerfile generation is now dynamic, using these configs instead of hardcoded values.

Adds cleanup of temporary build artifacts (Containerfile and sudoers) after the image is built. Uses defer to ensure cleanup happens even if the build fails, preventing accumulation of unused files in the storage directory.

Fixes #84

Implementation Plan: Podman Runtime Configuration (Issue #84)

Context

Issue #84 requires making the Podman runtime configurable instead of using hardcoded values. Currently, pkg/runtime/podman/create.go hardcodes the base image (Fedora latest), packages, sudoers configuration, and Claude installation. This plan introduces a configuration system that:

  • Stores configuration in ~/.kortex-cli/runtimes/podman/config/ (or similar when KORTEX_CLI_STORAGE is defined)
  • Separates base image config (image.json) from agent-specific config (claude.json, goose.json, etc)
  • Auto-generates default configurations when missing
  • Follows existing codebase patterns (interface-based design)

Configurable:

  • Fedora version (e.g., "latest", "40", "41")
  • Packages to install via DNF
  • Sudo-authorized binaries
  • Custom RUN commands
  • Agent installation details

Hardcoded (not configurable):

  • Base image registry: registry.fedoraproject.org/fedora
  • User/group
  • Sudoers file structure (only binary list is configurable)

Configuration Schema

image.json (Base Image Configuration)

Base image registry is hardcoded as registry.fedoraproject.org/fedora, only the version tag is configurable.

{
  "version": "latest",
  "packages": ["which", "procps-ng", "wget2", "@development-tools", "jq", "gh", "golang", "golangci-lint", "python3", "python3-pip"],
  "sudo": ["/usr/bin/dnf", "/bin/nice", "/bin/kill", "/usr/bin/kill", "/usr/bin/killall"],
  "run_commands": []
}

claude.json (Agent-Specific Configuration)

{
  "packages": [],
  "run_commands": [
    "curl -fsSL --proto-redir -all,https --tlsv1.3 https://claude.ai/install.sh | bash",
    "mkdir /home/claude/.config"
  ],
  "terminal_command": ["claude"]
}

Implementation Steps

1. Create Config Package (pkg/runtime/podman/config/)

config.go:

  • Config interface with methods:
    • LoadImage() (*ImageConfig, error)
    • LoadAgent(agentName string) (*AgentConfig, error)
    • GenerateDefaults() error
  • Unexported config struct implementation
  • Factory: NewConfig(configDir string) (Config, error)
  • Error types: ErrInvalidPath, ErrConfigNotFound, ErrInvalidConfig
  • Compile-time check: var _ Config = (*config)(nil)

types.go:

  • ImageConfig struct with fields: Version (string), Packages ([]string), Sudo ([]string), RunCommands ([]string)
  • AgentConfig struct with fields: Packages ([]string), RunCommands ([]string), TerminalCommand ([]string)
  • All with JSON tags

defaults.go:

  • defaultImageConfig() *ImageConfig - Current hardcoded values
  • defaultClaudeConfig() *AgentConfig - Current Claude installation
  • Constants for default values

validation.go:

  • validateImageConfig(cfg *ImageConfig) error
  • validateAgentConfig(cfg *AgentConfig) error
  • Validation rules:
    • Version cannot be empty
    • Packages can be empty (valid use case)
    • Sudo binaries must be absolute paths
    • Terminal command must have at least one element

2. Integrate with Podman Runtime

pkg/runtime/podman/podman.go:

  • Add config config.Config field to podmanRuntime struct
  • Modify Initialize() method:
    1. Create config directory: filepath.Join(storageDir, "config")
    2. Create config instance: config.NewConfig(configDir)
    3. Generate defaults: config.GenerateDefaults()
    4. Store in struct field

pkg/runtime/podman/create.go:

  • Modify createContainerfile() to accept *ImageConfig and *AgentConfig parameters
  • Replace hardcoded values with config values:
    • Base image: registry.fedoraproject.org/fedora:{imageConfig.Version}
    • Packages: merge imageConfig.Packages + agentConfig.Packages
    • User/group: hardcoded as claude:claude (not configurable)
    • Sudo binaries: from imageConfig.Sudo
    • RUN commands: from imageConfig.RunCommands + agentConfig.RunCommands
  • Modify Create() to load configs before calling createContainerfile()
    • Agent parameter: For now, hardcode the agent name as "claude" when calling config.LoadAgent(). In a future update, the agent name will be passed from the init command to the runtime's Create() method.
  • Add cleanup of instance directory:
    • Use defer os.RemoveAll(instanceDir) immediately after createInstanceDirectory()
    • Ensures cleanup happens even if subsequent steps fail
    • The Containerfile and sudoers files are only needed during image build

pkg/runtime/podman/containerfile.go (new):

  • Extract Containerfile generation logic
  • generateContainerfile(imageConfig, agentConfig) (string, error)
  • generateSudoers(sudoBinaries []string) string - Generate sudoers file from list of allowed binaries
  • Build content dynamically from configs
  • Hardcode user/group as claude:claude
  • Generate sudoers aliases and rules from sudo binaries list
  • Include COPY Containerfile /home/claude/Containerfile instruction to copy the Containerfile into the container for reference

3. Testing

Unit Tests:

  • pkg/runtime/podman/config/config_test.go - Config loading, validation, defaults
  • pkg/runtime/podman/containerfile_test.go - Containerfile generation

Integration Tests:

  • Update pkg/runtime/podman/create_test.go for config-driven creation
  • Test with custom and default configs
  • Cross-platform compatibility with t.TempDir() and filepath.Join()

Critical Files

  • /workspace/sources/pkg/runtime/podman/config/config.go (NEW)
  • /workspace/sources/pkg/runtime/podman/config/types.go (NEW)
  • /workspace/sources/pkg/runtime/podman/config/defaults.go (NEW)
  • /workspace/sources/pkg/runtime/podman/config/validation.go (NEW)
  • /workspace/sources/pkg/runtime/podman/containerfile.go (NEW)
  • /workspace/sources/pkg/runtime/podman/podman.go (MODIFY)
  • /workspace/sources/pkg/runtime/podman/create.go (MODIFY)

Reference Patterns

  • Interface-based design: /workspace/sources/pkg/config/config.go
  • Nested JSON structure: /workspace/sources/pkg/instances/instance.go
  • StorageAware integration: /workspace/sources/pkg/runtime/registry.go

Verification

  1. Run make test - All tests pass
  2. Create new workspace: ./kortex-cli init /path/to/source --runtime podman
  3. Verify config directory created: ls ~/.kortex-cli/runtimes/podman/config/
  4. Verify default configs exist: cat ~/.kortex-cli/runtimes/podman/config/image.json
  5. Modify config and create new workspace to verify custom config is used
  6. Start the workspace and verify Containerfile is available: podman exec <container> cat /home/claude/Containerfile
  7. Verify instance directory is cleaned up after workspace registration (should not exist after successful build)

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 23, 2026

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 23, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds disk-backed Podman runtime configuration: JSON image.json and <agent>.json under the CLI storage directory, a config loader/validator with defaults, Containerfile and sudoers generators that merge image+agent settings, and integrates config usage into Podman runtime initialization and container creation. Tests and docs updated.

Changes

Cohort / File(s) Summary
Docs & Plans
AGENTS.md, README.md, docs/plans/84.md
New documentation and an implementation plan describing JSON config schemas (image.json, <agent>.json), default generation behavior, Containerfile generation semantics, and when config changes take effect (at workspace registration/init).
Config Types & Defaults
pkg/runtime/podman/config/types.go, pkg/runtime/podman/config/defaults.go
Add ImageConfig and AgentConfig types, filename/version constants, and default factory functions that produce default image.json and claude.json.
Config Management
pkg/runtime/podman/config/config.go, pkg/runtime/podman/config/validation.go
Introduce Config interface and disk-backed implementation with NewConfig, LoadImage, LoadAgent, GenerateDefaults; add sentinel errors and validation rules (non-empty image version, absolute sudo paths, non-empty terminal command).
Config Tests
pkg/runtime/podman/config/config_test.go, pkg/runtime/podman/config/validation_test.go
Comprehensive unit tests for config creation, defaults generation, loading, validation failures, agent-name sanitization, and sudo-path rules.
Containerfile & Sudoers Generation
pkg/runtime/podman/containerfile.go, pkg/runtime/podman/containerfile_test.go
Extract Containerfile and sudoers generation using merged image+agent configs; add BaseImageRegistry, ContainerUser, ContainerGroup; tests cover package merging, run-command ordering, and sudoers content.
Runtime Integration
pkg/runtime/podman/podman.go, pkg/runtime/podman/podman_test.go
Podman runtime now holds a config.Config; Initialize creates config dir, constructs config, and calls GenerateDefaults; tests verify default files and non-overwrite behavior.
Create Flow Refactor & Tests
pkg/runtime/podman/create.go, pkg/runtime/podman/create_test.go
Create now loads image/agent configs and uses generateSudoers/generateContainerfile; instance-directory cleanup deferred; tests added for generated sudoers/containerfile and instance-dir cleanup on success/failure.

Sequence Diagram

sequenceDiagram
    participant Client as CLI Client
    participant Runtime as Podman Runtime
    participant Config as Config Manager
    participant Gen as Containerfile Generator
    participant Exec as Executor

    Client->>Runtime: Initialize(storageDir)
    Runtime->>Config: NewConfig(configDir)
    Runtime->>Config: GenerateDefaults()
    Config-->>Runtime: defaults created/loaded

    Client->>Runtime: Create(ctx, params)
    Runtime->>Config: LoadImage()
    Config-->>Runtime: ImageConfig
    Runtime->>Config: LoadAgent("claude")
    Config-->>Runtime: AgentConfig

    Runtime->>Gen: generateSudoers(ImageConfig.Sudo)
    Gen-->>Runtime: sudoers content
    Runtime->>Gen: generateContainerfile(ImageConfig, AgentConfig)
    Gen-->>Runtime: Containerfile

    Runtime->>Exec: podman build / podman run ...
    Exec-->>Runtime: build/run result
    Runtime-->>Client: Create result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • PR #83: Prior Podman runtime implementation; this PR replaces hardcoded image/Containerfile logic with config-driven generation and integrates the new config package.
  • PR #6: Earlier AGENTS.md changes; documentation additions here extend or overlap that file.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a configurable Podman runtime system with settable parameters via JSON files.
Linked Issues check ✅ Passed The PR fully implements all requirements from issue #84: image.json for base image/packages/sudoers/run commands, agent JSON files with packages/run commands/terminal command, and auto-generation of default config directory.
Out of Scope Changes check ✅ Passed All changes directly support the configuration system objective; documentation updates (AGENTS.md, README.md) and plan file (docs/plans/84.md) provide context without introducing unrelated code modifications.
Description check ✅ Passed The PR description comprehensively relates to the changeset, documenting the configuration system, JSON schema, implementation steps, and cleanup of temporary artifacts.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pkg/runtime/podman/create_test.go (1)

518-525: ⚠️ Potential issue | 🟡 Minor

Hardcoded Unix-style path may cause issues on Windows CI.

Line 520 uses "/path/to/source" which could cause inconsistencies on Windows when filepath.Abs() or similar path operations are applied. Consider using t.TempDir() for consistency, as done in other tests in this file.

🔧 Suggested fix
 	t.Run("with config mounts", func(t *testing.T) {
 		t.Parallel()

 		p := &podmanRuntime{}

 		configs := []string{".claude", ".gitconfig"}
 		mounts := workspace.Mounts{
 			Configs: &configs,
 		}

+		sourcePath := t.TempDir()
 		params := runtime.CreateParams{
 			Name:       "test-workspace",
-			SourcePath: "/path/to/source",
+			SourcePath: sourcePath,
 			WorkspaceConfig: &workspace.WorkspaceConfiguration{
 				Mounts: &mounts,
 			},
 		}

As per coding guidelines: "Always use 't.TempDir()' for temporary directories in tests - never hardcode paths".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/runtime/podman/create_test.go` around lines 518 - 525, The test uses a
hardcoded Unix path in runtime.CreateParams.SourcePath (" /path/to/source")
which can break on Windows CI; update the test to call t.TempDir() and assign
that value to SourcePath when constructing runtime.CreateParams (the same block
that sets Name, WorkspaceConfig and Mounts), ensuring WorkspaceConfiguration and
mounts usage remains unchanged and all references (e.g., params,
runtime.CreateParams, SourcePath, WorkspaceConfiguration) use the temp directory
instead of the hardcoded string.
🧹 Nitpick comments (4)
pkg/runtime/podman/config/validation_test.go (1)

183-196: Consider adding a test for TerminalCommand with empty first element.

The validation logic at line 58 of validation.go handles the case where TerminalCommand[0] == "", but this specific edge case isn't explicitly tested. The current "rejects empty terminal command" test uses an empty slice.

💡 Optional: Add explicit test for empty first element
	t.Run("rejects terminal command with empty first element", func(t *testing.T) {
		t.Parallel()

		cfg := &AgentConfig{
			Packages:        []string{},
			RunCommands:     []string{},
			TerminalCommand: []string{"", "arg"},
		}

		err := validateAgentConfig(cfg)
		if err == nil {
			t.Error("Expected error for terminal command with empty first element")
		}
	})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/runtime/podman/config/validation_test.go` around lines 183 - 196, Add a
new unit test that covers the edge case where AgentConfig.TerminalCommand is a
non-empty slice but its first element is an empty string; specifically, in
pkg/runtime/podman/config/validation_test.go add a t.Run that constructs cfg :=
&AgentConfig{Packages: []string{}, RunCommands: []string{}, TerminalCommand:
[]string{"", "arg"}} and asserts validateAgentConfig(cfg) returns an error. This
ensures the validateAgentConfig function’s check for TerminalCommand[0] == "" is
exercised.
docs/plans/84.md (1)

43-46: Documentation example differs from implementation defaults.

The example run_commands here differs slightly from the actual defaults in defaults.go:

  • Missing -p flag in mkdir command (defaults.go uses mkdir -p)
  • Missing single quotes around '-all,https' in the curl proto-redir argument

Consider aligning the documentation with the actual default values.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plans/84.md` around lines 43 - 46, The documented run_commands array
doesn't match the implementation defaults: update the example run_commands to
match defaults.go by adding the -p flag to the mkdir invocation and by quoting
the proto-redir argument in the curl command (i.e., change "mkdir
/home/claude/.config" to include -p and change --proto-redir -all,https to
--proto-redir '-all,https') so the docs align with the defaults used by the code
(referencing run_commands and defaults.go).
pkg/runtime/podman/containerfile.go (1)

56-62: Silent failure on nil configs may mask bugs.

Returning an empty string when imageConfig or agentConfig is nil silently produces an invalid Containerfile. Consider returning an error or panicking since these are programming errors that should never occur in correct usage.

♻️ Proposed fix to fail explicitly on nil configs
 // generateContainerfile generates the Containerfile content from image and agent configurations.
-func generateContainerfile(imageConfig *config.ImageConfig, agentConfig *config.AgentConfig) string {
+func generateContainerfile(imageConfig *config.ImageConfig, agentConfig *config.AgentConfig) (string, error) {
 	if imageConfig == nil {
-		return ""
+		return "", fmt.Errorf("imageConfig cannot be nil")
 	}
 	if agentConfig == nil {
-		return ""
+		return "", fmt.Errorf("agentConfig cannot be nil")
 	}
-		
+
 	var lines []string

This would require updating callers to handle the error.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/runtime/podman/containerfile.go` around lines 56 - 62, The function
generateContainerfile currently returns an empty string on nil inputs which
silently produces invalid output; change its signature to return (string, error)
and validate inputs at the top (if imageConfig == nil or agentConfig == nil)
returning a descriptive error (e.g., fmt.Errorf("generateContainerfile: nil
imageConfig") or similar) instead of ""; update all callers of
generateContainerfile to handle the error and propagate or fail loudly; keep the
rest of the function logic unchanged and ensure imports include fmt or errors
for error construction.
pkg/runtime/podman/containerfile_test.go (1)

139-160: Consider adding a test for nil config handling.

The generateContainerfile function returns empty string when configs are nil. Consider adding a test case to document this behavior (or the error behavior if the suggested refactor is adopted).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/runtime/podman/containerfile_test.go` around lines 139 - 160, Add a test
case in containerfile_test.go that calls generateContainerfile with nil image
and/or agent configs to document expected behavior: call
generateContainerfile(nil, nil) (and optionally generateContainerfile(nil,
&config.AgentConfig{}) and vice versa) and assert the returned string matches
the current contract (e.g., empty string) or the expected error behavior;
reference generateContainerfile to locate where to add the test and use the same
testing style (t.Run, t.Parallel, and t.Errorf) to keep tests consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/plans/84.md`:
- Line 8: The sentence "Separates base image config (`image.json`) from
agent-specific config (`claude.json`, `goose.json`, etc`)" is missing a period
after the abbreviation "etc"; update that sentence to read "etc." (add a period
after etc) so the line ends with "etc." to follow American English punctuation
conventions.

In `@pkg/runtime/podman/config/config_test.go`:
- Line 1: The file containing the line "Copyright 2026 Red Hat, Inc." has
formatting issues; run the formatter (make fmt or gofmt -w) to reformat the
file, stage the updated file, and commit the changes so CI passes.

In `@pkg/runtime/podman/containerfile.go`:
- Line 63: Remove the stray extra whitespace/indentation before the declaration
of "var lines" so the declaration aligns with the surrounding code, then run
"make fmt" to apply project formatting; specifically locate the "var lines"
declaration in containerfile.go and fix its leading spaces (or reformat the
file) so CI formatting passes.

---

Outside diff comments:
In `@pkg/runtime/podman/create_test.go`:
- Around line 518-525: The test uses a hardcoded Unix path in
runtime.CreateParams.SourcePath (" /path/to/source") which can break on Windows
CI; update the test to call t.TempDir() and assign that value to SourcePath when
constructing runtime.CreateParams (the same block that sets Name,
WorkspaceConfig and Mounts), ensuring WorkspaceConfiguration and mounts usage
remains unchanged and all references (e.g., params, runtime.CreateParams,
SourcePath, WorkspaceConfiguration) use the temp directory instead of the
hardcoded string.

---

Nitpick comments:
In `@docs/plans/84.md`:
- Around line 43-46: The documented run_commands array doesn't match the
implementation defaults: update the example run_commands to match defaults.go by
adding the -p flag to the mkdir invocation and by quoting the proto-redir
argument in the curl command (i.e., change "mkdir /home/claude/.config" to
include -p and change --proto-redir -all,https to --proto-redir '-all,https') so
the docs align with the defaults used by the code (referencing run_commands and
defaults.go).

In `@pkg/runtime/podman/config/validation_test.go`:
- Around line 183-196: Add a new unit test that covers the edge case where
AgentConfig.TerminalCommand is a non-empty slice but its first element is an
empty string; specifically, in pkg/runtime/podman/config/validation_test.go add
a t.Run that constructs cfg := &AgentConfig{Packages: []string{}, RunCommands:
[]string{}, TerminalCommand: []string{"", "arg"}} and asserts
validateAgentConfig(cfg) returns an error. This ensures the validateAgentConfig
function’s check for TerminalCommand[0] == "" is exercised.

In `@pkg/runtime/podman/containerfile_test.go`:
- Around line 139-160: Add a test case in containerfile_test.go that calls
generateContainerfile with nil image and/or agent configs to document expected
behavior: call generateContainerfile(nil, nil) (and optionally
generateContainerfile(nil, &config.AgentConfig{}) and vice versa) and assert the
returned string matches the current contract (e.g., empty string) or the
expected error behavior; reference generateContainerfile to locate where to add
the test and use the same testing style (t.Run, t.Parallel, and t.Errorf) to
keep tests consistent.

In `@pkg/runtime/podman/containerfile.go`:
- Around line 56-62: The function generateContainerfile currently returns an
empty string on nil inputs which silently produces invalid output; change its
signature to return (string, error) and validate inputs at the top (if
imageConfig == nil or agentConfig == nil) returning a descriptive error (e.g.,
fmt.Errorf("generateContainerfile: nil imageConfig") or similar) instead of "";
update all callers of generateContainerfile to handle the error and propagate or
fail loudly; keep the rest of the function logic unchanged and ensure imports
include fmt or errors for error construction.
🪄 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: c3937ee0-5d52-4532-ac1d-8b02d67d0eff

📥 Commits

Reviewing files that changed from the base of the PR and between ef191d7 and 6dfd333.

📒 Files selected for processing (15)
  • AGENTS.md
  • README.md
  • docs/plans/84.md
  • pkg/runtime/podman/config/config.go
  • pkg/runtime/podman/config/config_test.go
  • pkg/runtime/podman/config/defaults.go
  • pkg/runtime/podman/config/types.go
  • pkg/runtime/podman/config/validation.go
  • pkg/runtime/podman/config/validation_test.go
  • pkg/runtime/podman/containerfile.go
  • pkg/runtime/podman/containerfile_test.go
  • pkg/runtime/podman/create.go
  • pkg/runtime/podman/create_test.go
  • pkg/runtime/podman/podman.go
  • pkg/runtime/podman/podman_test.go

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
pkg/runtime/podman/containerfile.go (2)

103-108: Consider adding a consistent trailing newline.

generateSudoers always appends a trailing newline, but generateContainerfile only gets one when RUN commands exist (via the empty string on line 105). When no RUN commands are present, the output lacks a trailing newline. For consistency and proper file handling, consider always adding one.

♻️ Suggested change
-	// Add final newline if there are RUN commands
-	if len(imageConfig.RunCommands) > 0 || len(agentConfig.RunCommands) > 0 {
-		lines = append(lines, "")
-	}
-
-	return strings.Join(lines, "\n")
+	// Add final newline
+	lines = append(lines, "")
+
+	return strings.Join(lines, "\n")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/runtime/podman/containerfile.go` around lines 103 - 108,
generateContainerfile currently only appends a trailing empty string when
imageConfig.RunCommands or agentConfig.RunCommands exist, which causes the
returned string from strings.Join(lines, "\n") to lack a final newline
otherwise; update the function (generateContainerfile) to always ensure a final
newline by appending an empty string to lines unconditionally (or by ensuring
the returned string ends with "\n") so the output consistently has a trailing
newline regardless of imageConfig.RunCommands or agentConfig.RunCommands.

86-86: Consider using ContainerUser constant for consistency.

The sudoers destination path hardcodes "claude" while other lines (87, 91) use the ContainerUser constant. If the constant is ever changed, this line would be overlooked.

♻️ Suggested change
-	lines = append(lines, "COPY sudoers /etc/sudoers.d/claude")
+	lines = append(lines, fmt.Sprintf("COPY sudoers /etc/sudoers.d/%s", ContainerUser))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/runtime/podman/containerfile.go` at line 86, Replace the hardcoded
sudoers destination name in the appended line "COPY sudoers
/etc/sudoers.d/claude" with the ContainerUser constant so it stays consistent if
ContainerUser changes; update the code that appends to lines (the append call
that pushes "COPY sudoers /etc/sudoers.d/claude") to build the path using
ContainerUser (e.g., format or string concatenation) instead of the literal
"claude" so other entries that rely on ContainerUser remain aligned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/runtime/podman/containerfile.go`:
- Around line 103-108: generateContainerfile currently only appends a trailing
empty string when imageConfig.RunCommands or agentConfig.RunCommands exist,
which causes the returned string from strings.Join(lines, "\n") to lack a final
newline otherwise; update the function (generateContainerfile) to always ensure
a final newline by appending an empty string to lines unconditionally (or by
ensuring the returned string ends with "\n") so the output consistently has a
trailing newline regardless of imageConfig.RunCommands or
agentConfig.RunCommands.
- Line 86: Replace the hardcoded sudoers destination name in the appended line
"COPY sudoers /etc/sudoers.d/claude" with the ContainerUser constant so it stays
consistent if ContainerUser changes; update the code that appends to lines (the
append call that pushes "COPY sudoers /etc/sudoers.d/claude") to build the path
using ContainerUser (e.g., format or string concatenation) instead of the
literal "claude" so other entries that rely on ContainerUser remain aligned.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 88bb7b9d-a0b9-44d8-81a7-54f506e1edd6

📥 Commits

Reviewing files that changed from the base of the PR and between 6dfd333 and 7b78021.

📒 Files selected for processing (2)
  • pkg/runtime/podman/config/config_test.go
  • pkg/runtime/podman/containerfile.go
✅ Files skipped from review due to trivial changes (1)
  • pkg/runtime/podman/config/config_test.go

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/plans/84.md`:
- Line 147: Update the doc step that instructs checking the generated
Containerfile: note that pkg/runtime/podman/create.go now defers
os.RemoveAll(instanceDir) inside Create(), so the Containerfile is cleaned up
after Create() returns; change Step 6 to either instruct inspecting the
Containerfile during the build (e.g., run the build step before Create()
completes or attach a debugger to Create()), or provide an explicit debug option
that skips the deferred cleanup (describe invoking Create() with a temporary
skip-cleanup flag or using a test helper to prevent os.RemoveAll(instanceDir))
and point readers to instanceDir while the build is in progress.

In `@pkg/runtime/podman/config/config.go`:
- Around line 79-83: The JSON unmarshal error paths currently return the raw
decoder error (json.Unmarshal into ImageConfig) which breaks callers checking
errors.Is(err, ErrInvalidConfig); update both LoadImage and LoadAgent to wrap
the unmarshal failure with ErrInvalidConfig (e.g., return nil, fmt.Errorf("%w:
%v", ErrInvalidConfig, err) or errors.Join/Wrap) so that callers can detect
invalid config via errors.Is(err, ErrInvalidConfig) while preserving the
original decoder message.
- Around line 137-159: GenerateDefaults() currently treats any non-IsNotExist
result from os.Stat as success; update the checks around imageConfigPath and
claudeConfigPath so that after calling os.Stat you: (1) if err != nil and
!os.IsNotExist(err) return a wrapped error (include the path variable like
imageConfigPath or claudeConfigPath in the message), and (2) if err == nil check
the returned FileInfo.IsDir() and return an error if it is a directory (again
referencing imageConfigPath/claudeConfigPath). Apply these checks for both the
image JSON block (imageConfigPath / defaultImageConfig) and the Claude JSON
block (claudeConfigPath / defaultClaudeConfig) so GenerateDefaults() fails fast
on permission or type errors instead of silently continuing.

In `@pkg/runtime/podman/create.go`:
- Around line 233-235: Create() currently hardcodes the agent name by calling
p.config.LoadAgent("claude"), which prevents per-agent configs from being used;
change Create() to use the selected agent name instead of the literal "claude"
(e.g., thread the selected agent into Create() from the caller or read
p.config.SelectedAgent / p.selectedAgent) and call
p.config.LoadAgent(selectedAgent) so LoadAgent receives the actual agent
identifier; ensure the selected agent value is validated and propagated where
Create() is invoked so other agents' <agent>.json files are reachable.
- Around line 93-96: The sudoers file is written with mode 0644 which is
owner-writable and will be rejected by sudo when copied into /etc/sudoers.d;
update the write to create the include with restrictive permissions (0440) by
changing the os.WriteFile call that writes sudoersPath (produced by
generateSudoers) to use file mode 0440, and if the build pipeline normalizes
modes on COPY, also add a RUN chmod 0440 /etc/sudoers.d/claude immediately after
the COPY that places sudoers into the image (see the containerfile/COPY step
that uses that sudoers file).
🪄 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: b239ae51-2d99-489e-8039-afbc6e50e435

📥 Commits

Reviewing files that changed from the base of the PR and between 7b78021 and ab2f6a3.

📒 Files selected for processing (15)
  • AGENTS.md
  • README.md
  • docs/plans/84.md
  • pkg/runtime/podman/config/config.go
  • pkg/runtime/podman/config/config_test.go
  • pkg/runtime/podman/config/defaults.go
  • pkg/runtime/podman/config/types.go
  • pkg/runtime/podman/config/validation.go
  • pkg/runtime/podman/config/validation_test.go
  • pkg/runtime/podman/containerfile.go
  • pkg/runtime/podman/containerfile_test.go
  • pkg/runtime/podman/create.go
  • pkg/runtime/podman/create_test.go
  • pkg/runtime/podman/podman.go
  • pkg/runtime/podman/podman_test.go
✅ Files skipped from review due to trivial changes (6)
  • AGENTS.md
  • pkg/runtime/podman/config/validation.go
  • pkg/runtime/podman/config/types.go
  • pkg/runtime/podman/create_test.go
  • pkg/runtime/podman/config/config_test.go
  • pkg/runtime/podman/containerfile_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
  • pkg/runtime/podman/podman.go
  • pkg/runtime/podman/config/validation_test.go
  • pkg/runtime/podman/podman_test.go
  • pkg/runtime/podman/config/defaults.go
  • README.md

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
docs/plans/84.md (1)

8-8: ⚠️ Potential issue | 🟡 Minor

Minor punctuation fix: add period after “etc.”

Line 8 should use “etc.” (with period) for standard American English punctuation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plans/84.md` at line 8, Update the sentence "Separates base image config
(`image.json`) from agent-specific config (`claude.json`, `goose.json`, etc)" to
use the standard abbreviation with a period: change "etc" to "etc." so it reads
"..., etc." in the docs content referencing `image.json`, `claude.json`, and
`goose.json`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@docs/plans/84.md`:
- Line 8: Update the sentence "Separates base image config (`image.json`) from
agent-specific config (`claude.json`, `goose.json`, etc)" to use the standard
abbreviation with a period: change "etc" to "etc." so it reads "..., etc." in
the docs content referencing `image.json`, `claude.json`, and `goose.json`.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f5e36cef-7b68-4ad4-bbb9-2bc238a6d15c

📥 Commits

Reviewing files that changed from the base of the PR and between ab2f6a3 and 383a92c.

📒 Files selected for processing (4)
  • README.md
  • docs/plans/84.md
  • pkg/runtime/podman/containerfile.go
  • pkg/runtime/podman/containerfile_test.go
✅ Files skipped from review due to trivial changes (1)
  • pkg/runtime/podman/containerfile_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • README.md
  • pkg/runtime/podman/containerfile.go

@feloy feloy requested review from benoitf and jeffmaury March 23, 2026 17:01
docs/plans/84.md Outdated
@@ -0,0 +1,153 @@
# Implementation Plan: Podman Runtime Configuration (Issue #84)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it expected to have the plan committed ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed the plan from sources and placed it in the PR body

@benoitf
Copy link
Copy Markdown
Contributor

benoitf commented Mar 25, 2026

is it internal configuration or user-facing configuration ?

@feloy
Copy link
Copy Markdown
Contributor Author

feloy commented Mar 25, 2026

is it internal configuration or user-facing configuration ?

This is user-facing configuration

@benoitf
Copy link
Copy Markdown
Contributor

benoitf commented Mar 25, 2026

so there is no abstraction, we configure each runtime ?

@feloy
Copy link
Copy Markdown
Contributor Author

feloy commented Mar 25, 2026

so there is no abstraction, we configure each runtime ?

A next step will be to add a "feature" abstraction, which will change how the 'packages' are configured.

For the image version for example, it is for this specific runtime.

Run commands and agents configuration could be abstracted.

My intent would be to have a first way to configure the podman runtime to be able to demonstrate that it works, and introduce an abstraction later when we are sure to know what we want, with at least another runtime implementation

feloy and others added 11 commits March 26, 2026 08:52
Introduces a configuration system for the Podman runtime, allowing
customization of the base image, packages, sudo permissions, and
agent setup through JSON files.

Configuration files are auto-generated in the runtime storage
directory (image.json for base image, claude.json for agent).
The Containerfile generation is now dynamic, using these configs
instead of hardcoded values.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>

Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com>
Adds cleanup of temporary build artifacts (Containerfile and sudoers)
after the image is built. Uses defer to ensure cleanup happens even
if the build fails, preventing accumulation of unused files in the
storage directory.

Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>

Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com>
The generated Containerfile is now copied to /home/claude/Containerfile
inside the container, allowing users to inspect the exact build
configuration used for their workspace.

Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
…dConfig

LoadImage and LoadAgent now wrap json.Unmarshal errors with
ErrInvalidConfig, allowing callers to detect invalid configuration
using errors.Is() while preserving the original error message.

Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
GenerateDefaults now returns errors immediately when encountering
permission issues or when configuration paths are directories
instead of files, rather than silently continuing. This provides
clearer feedback when the configuration directory is misconfigured.

Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Added RUN chmod 0440 command after copying the sudoers file into
/etc/sudoers.d/claude to ensure sudo accepts the configuration.
Sudo requires sudoers files to have restrictive permissions (0440)
and rejects files that are owner-writable.

Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
@feloy feloy merged commit 5e2f562 into kortex-hub:main Mar 26, 2026
6 checks passed
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.

podman runtime configurable

3 participants