Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ appCtx := api.NewAppContext("helmet-ex",
```go
app, _ := framework.NewAppFromTarball(appCtx, installer.InstallerTarball, cwd,
framework.WithIntegrations(appIntegrations...),
framework.WithMCPImage(mcpImage),
framework.WithImage(mcpImage),
)
```

Expand Down
2 changes: 1 addition & 1 deletion docs/cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ helmet-ex mcp-server [flags]

| Flag | Description |
|------|-------------|
| `--image` | Container image for installer (overrides default from `WithMCPImage()`) |
| `--image` | Container image for installer (overrides default from `WithImage()`) |

**Behavior:**
- Reads `instructions.md` from installer filesystem as server instructions
Expand Down
4 changes: 2 additions & 2 deletions docs/getting-started.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ app, err := framework.NewAppFromTarball(
installer.InstallerTarball,
cwd,
framework.WithIntegrations(appIntegrations...),
framework.WithMCPImage(mcpImage),
framework.WithImage(mcpImage),
)
if err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
Expand All @@ -118,7 +118,7 @@ if err := app.Run(); err != nil {
- `api.NewAppContext()` creates application metadata with functional options
- `framework.NewAppFromTarball()` constructs the app from the embedded tarball
- The `cwd` parameter enables the [overlay filesystem](installer-structure.md#overlay-filesystem) for development
- `framework.WithMCPImage()` sets the container image for [MCP Job-based deployments](mcp.md#container-image-for-job-based-deployment)
- `framework.WithImage()` sets the container image for [MCP Job-based deployments](mcp.md#container-image-for-job-based-deployment)

## Building

Expand Down
2 changes: 1 addition & 1 deletion docs/installer-structure.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func NewAppFromTarball(
| `appCtx` | `*api.AppContext` | Application metadata (name, version, namespace, descriptions) |
| `tarball` | `[]byte` | Embedded installer tarball bytes |
| `cwd` | `string` | Current working directory for local filesystem overlay |
| `opts` | `...Option` | Functional options (integrations, MCP image, etc.) |
| `opts` | `...Option` | Functional options (integrations, image, etc.) |

**Internal flow**:
1. Convert tarball bytes to `fs.FS` via `NewTarFS(tarball)`
Expand Down
10 changes: 5 additions & 5 deletions docs/mcp.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Configure your MCP client to run your installer binary with the `mcp-server` sub
helmet-ex mcp-server
```

The MCP server requires a [container image](#container-image-for-job-based-deployment) for Job-based deployments, configured via `WithMCPImage()` at framework initialization or overridden with `--image` at runtime.
The MCP server requires a [container image](#container-image-for-job-based-deployment) for Job-based deployments, configured via `WithImage()` at framework initialization or overridden with `--image` at runtime.

### Client Configuration

Expand Down Expand Up @@ -74,7 +74,7 @@ When `deploy` is invoked, the MCP server — authenticated via the user's kubeco
1. **`ServiceAccount`** named `{appName}` in the installer namespace (via [server-side apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/))
2. **`ClusterRoleBinding`** named `{appName}`, binding the `ServiceAccount` to the `cluster-admin` `ClusterRole` (via server-side apply)
3. **`Job`** named `{appName}-deploy-job` (via `Create` — only one Job is allowed; use `force: true` to replace an existing one) with:
- Container image: the consumer's installer application image (from `WithMCPImage()` or `--image`)
- Container image: the consumer's installer application image (from `WithImage()` or `--image`)
- Args: `["deploy"]` (with optional `--verbose`, `--dry-run`)
- Env: `KUBECONFIG=""` (forces [in-cluster authentication](https://kubernetes.io/docs/reference/access-authn-authz/authentication/#service-account-tokens))
- RestartPolicy: `Never`, BackoffLimit: `0`
Expand Down Expand Up @@ -105,11 +105,11 @@ make image IMAGE_REPOSITORY="quay.io/redhat-appstudio"
make image-push
```

`WithMCPImage()` is **required** at framework initialization — the framework returns an error if no image is configured. This value becomes the default for the optional `--image` flag, which overrides it at runtime.
`WithImage()` is no longer required at framework initialization. The `mcp-server` subcommand validates the image at runtime via `Complete()`. This value becomes the default for the optional `--image` flag, which overrides it at runtime.

```go
// At framework initialization (required) — from helmet-ex/main.go
framework.WithMCPImage("quay.io/redhat-appstudio/helmet-ex:latest")
// At framework initialization — from helmet-ex/main.go
framework.WithImage("quay.io/redhat-appstudio/helmet-ex:latest")
```

```sh
Expand Down
6 changes: 3 additions & 3 deletions example/helmet-ex/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func main() {
os.Exit(1)
}

// 3. Build MCP image reference
// 3. Build image reference
mcpImage := buildMCPImage()

// 4. Create application with framework options (GitHub URLs via CustomURLProvider; see framework/integrations.go)
Expand All @@ -37,7 +37,7 @@ func main() {
installer.InstallerTarball,
cwd,
framework.WithIntegrations(appIntegrations...),
framework.WithMCPImage(mcpImage),
framework.WithImage(mcpImage),
)
if err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
Expand Down Expand Up @@ -78,7 +78,7 @@ operators, storage, networking, integrations, and product layers.`),
)
}

// buildMCPImage constructs the container image reference for the MCP server.
// buildMCPImage constructs the container image reference for the installer.
// Uses the commit ID for versioning when available, falls back to 'latest'.
func buildMCPImage() string {
mcpImage := "quay.io/redhat-appstudio/helmet-ex"
Expand Down
12 changes: 3 additions & 9 deletions framework/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type App struct {
kube *k8s.Kube // kubernetes client

mcpToolsBuilder mcptools.MCPToolsBuilder // tools builder
mcpImage string // installer image
image string // installer image
installerTarball []byte // embedded installer tarball
}

Expand Down Expand Up @@ -94,18 +94,12 @@ func (a *App) setupRootCmd() error {
mcpBuilder = subcmd.StandardMCPToolsBuilder()
}

// Validate MCP image is configured.
if a.mcpImage == "" {
return fmt.Errorf(
"MCP server image not configured: use WithMCPImage() option")
}

// Other subcommands via api.Runner.
subs := []api.SubCommand{
subcmd.NewConfig(a.AppCtx, runCtx, a.flags),
subcmd.NewDeploy(a.AppCtx, runCtx, a.flags, a.integrationManager, a.installerTarball),
subcmd.NewInstaller(a.AppCtx, runCtx, a.flags, a.installerTarball),
subcmd.NewMCPServer(a.AppCtx, runCtx, a.flags, a.integrationManager, mcpBuilder, a.mcpImage),
subcmd.NewMCPServer(a.AppCtx, runCtx, a.flags, a.integrationManager, mcpBuilder, a.image),
subcmd.NewTemplate(a.AppCtx, runCtx, a.flags, a.installerTarball),
subcmd.NewTopology(a.AppCtx, runCtx),
}
Expand Down Expand Up @@ -154,7 +148,7 @@ func NewApp(
// - appCtx: Application metadata (name, version, etc.)
// - tarball: Embedded installer tarball bytes
// - cwd: Current working directory for local filesystem overlay
// - opts: Additional runtime options (integrations, MCP image, etc.)
// - opts: Additional runtime options (integrations, image, etc.)
//
// The function creates an overlay filesystem combining the embedded tarball
// contents with the local filesystem at cwd, then initializes the App.
Expand Down
64 changes: 64 additions & 0 deletions framework/app_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package framework

import (
"os"
"testing"

"github.com/redhat-appstudio/helmet/api"
"github.com/redhat-appstudio/helmet/internal/chartfs"
)

func testAppContext() *api.AppContext {
return api.NewAppContext(
"helmet-test",
api.WithNamespace("test-ns"),
)
}

func testChartFS(t *testing.T) *chartfs.ChartFS {
t.Helper()
return chartfs.New(os.DirFS("../test/charts"))
}

func TestNewApp_WithoutImage_Succeeds(t *testing.T) {
t.Parallel()
app, err := NewApp(testAppContext(), testChartFS(t))
if err != nil {
t.Fatalf("NewApp without WithImage should succeed, got: %v", err)
}
if app == nil {
t.Fatal("expected non-nil App")
}
}

func TestWithImage_SetsImage(t *testing.T) {
t.Parallel()
app, err := NewApp(testAppContext(), testChartFS(t),
WithImage("test:latest"),
)
if err != nil {
t.Fatalf("NewApp with WithImage should succeed, got: %v", err)
}
if app == nil {
t.Fatal("expected non-nil App")
}
if app.image != "test:latest" {
t.Fatalf("expected image %q, got %q", "test:latest", app.image)
}
}

func TestWithMCPImage_DeprecatedAlias(t *testing.T) {
t.Parallel()
app, err := NewApp(testAppContext(), testChartFS(t),
WithMCPImage("deprecated:latest"),
)
if err != nil {
t.Fatalf("NewApp with WithMCPImage should succeed, got: %v", err)
}
if app == nil {
t.Fatal("expected non-nil App")
}
if app.image != "deprecated:latest" {
t.Fatalf("expected image %q, got %q", "deprecated:latest", app.image)
}
}
11 changes: 8 additions & 3 deletions framework/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,18 @@ func WithIntegrations(modules ...api.IntegrationModule) Option {
}
}

// WithMCPImage sets the container image for the MCP server.
func WithMCPImage(image string) Option {
// WithImage sets the container image for the installer.
func WithImage(image string) Option {
return func(a *App) {
a.mcpImage = image
a.image = image
}
}

// Deprecated: use WithImage instead.
func WithMCPImage(image string) Option {
return WithImage(image)
}

// WithMCPToolsBuilder sets the MCP tools builder for the application.
func WithMCPToolsBuilder(builder mcptools.MCPToolsBuilder) Option {
return func(a *App) {
Expand Down
4 changes: 4 additions & 0 deletions internal/subcmd/mcpserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ func (m *MCPServer) Cmd() *cobra.Command {

// Complete implements api.SubCommand.
func (m *MCPServer) Complete(_ []string) error {
if m.image == "" {
return fmt.Errorf(
"container image not configured: use WithImage() option or --image flag")
}
return nil
}

Expand Down
65 changes: 65 additions & 0 deletions internal/subcmd/mcpserver_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package subcmd

import (
"testing"

"github.com/onsi/gomega"
"github.com/redhat-appstudio/helmet/internal/flags"
"github.com/redhat-appstudio/helmet/internal/mcptools"
)

func noopMCPToolsBuilder(_ mcptools.MCPToolsContext) ([]mcptools.Interface, error) {
return nil, nil
}

func TestMCPServer_Complete_EmptyImage_ReturnsError(t *testing.T) {
t.Parallel()
g := gomega.NewWithT(t)

appCtx := testAppContext()
runCtx := testRunContext(t)
manager := testManager(t, runCtx)

m := NewMCPServer(appCtx, runCtx, flags.NewFlags(), manager, noopMCPToolsBuilder, "")
err := m.Complete(nil)
g.Expect(err).To(gomega.HaveOccurred())
g.Expect(err.Error()).To(gomega.ContainSubstring("WithImage()"))
}

func TestMCPServer_Complete_WithImage_Succeeds(t *testing.T) {
t.Parallel()
g := gomega.NewWithT(t)

appCtx := testAppContext()
runCtx := testRunContext(t)
manager := testManager(t, runCtx)

m := NewMCPServer(appCtx, runCtx, flags.NewFlags(), manager, noopMCPToolsBuilder, "test:latest")
g.Expect(m.Complete(nil)).ToNot(gomega.HaveOccurred())
}

func TestMCPServer_Complete_ImageOverriddenByFlag(t *testing.T) {
t.Parallel()
g := gomega.NewWithT(t)

appCtx := testAppContext()
runCtx := testRunContext(t)
manager := testManager(t, runCtx)

m := NewMCPServer(appCtx, runCtx, flags.NewFlags(), manager, noopMCPToolsBuilder, "")
err := m.cmd.PersistentFlags().Set("image", "override:latest")
g.Expect(err).ToNot(gomega.HaveOccurred())
g.Expect(m.Complete(nil)).ToNot(gomega.HaveOccurred())
}

func TestMCPServer_Validate_AlwaysSucceeds(t *testing.T) {
t.Parallel()
g := gomega.NewWithT(t)

appCtx := testAppContext()
runCtx := testRunContext(t)
manager := testManager(t, runCtx)

m := NewMCPServer(appCtx, runCtx, flags.NewFlags(), manager, noopMCPToolsBuilder, "test:latest")
g.Expect(m.Validate()).ToNot(gomega.HaveOccurred())
}
Loading