Skip to content
Open
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
6 changes: 6 additions & 0 deletions docs/server/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions docs/server/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions docs/server/swagger.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

35 changes: 33 additions & 2 deletions pkg/api/v1/workload_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import (
"context"
"errors"
"fmt"
"strings"
"time"

groupval "github.com/stacklok/toolhive-core/validation/group"
httpval "github.com/stacklok/toolhive-core/validation/http"
"github.com/stacklok/toolhive/pkg/auth/remote"
"github.com/stacklok/toolhive/pkg/config"
"github.com/stacklok/toolhive/pkg/container/runtime"
"github.com/stacklok/toolhive/pkg/container/templates"
"github.com/stacklok/toolhive/pkg/groups"
"github.com/stacklok/toolhive/pkg/logger"
"github.com/stacklok/toolhive/pkg/networking"
Expand Down Expand Up @@ -162,6 +164,7 @@ func (s *WorkloadService) BuildFullRunConfig(
var imageURL string
var imageMetadata *regtypes.ImageMetadata
var serverMetadata regtypes.ServerMetadata
runtimeConfigOverride := runtimeConfigFromRequest(req)

if req.URL != "" {
// Configure remote authentication if OAuth config is provided
Expand All @@ -180,8 +183,8 @@ func (s *WorkloadService) BuildFullRunConfig(
req.Image,
"", // We do not let the user specify a CA cert path here.
retriever.VerifyImageWarn,
"", // TODO Add support for registry groups lookups for API
nil, // No runtime override from API (yet)
"", // TODO Add support for registry groups lookups for API
runtimeConfigOverride,
)
if err != nil {
// Check if the error is due to context timeout
Expand Down Expand Up @@ -272,6 +275,11 @@ func (s *WorkloadService) BuildFullRunConfig(
runner.WithTelemetryConfigFromFlags("", false, false, false, "", 0.0, nil, false, nil, false),
}

// Runtime overrides only apply to protocol-scheme image builds.
if runtimeConfigOverride != nil && req.URL == "" {
options = append(options, runner.WithRuntimeConfig(runtimeConfigOverride))
}

Comment on lines +278 to +282
Copy link
Collaborator

Choose a reason for hiding this comment

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

The comment says "Runtime overrides only apply to protocol-scheme image builds" but if a user passes runtime_config with a regular Docker image (not go://, npx://, uvx://), no warning or error is returned -- it's silently ignored. This will confuse users who expect it to have an effect.

Consider returning a validation error (or at least a warning in the response) when runtime_config is provided alongside a non-protocol-scheme image.

// Add header forward configuration if specified
if req.HeaderForward != nil {
if len(req.HeaderForward.AddPlaintextHeaders) > 0 {
Expand Down Expand Up @@ -361,6 +369,29 @@ func createRequestToRemoteAuthConfig(
return remoteAuthConfig
}

func runtimeConfigFromRequest(req *createRequest) *templates.RuntimeConfig {
if req == nil || req.RuntimeConfig == nil {
return nil
}

runtimeConfig := &templates.RuntimeConfig{}
if builderImage := strings.TrimSpace(req.RuntimeConfig.BuilderImage); builderImage != "" {
runtimeConfig.BuilderImage = builderImage
}
if len(req.RuntimeConfig.AdditionalPackages) > 0 {
for _, pkg := range req.RuntimeConfig.AdditionalPackages {
if trimmedPkg := strings.TrimSpace(pkg); trimmedPkg != "" {
runtimeConfig.AdditionalPackages = append(runtimeConfig.AdditionalPackages, trimmedPkg)
}
}
}
if runtimeConfig.BuilderImage == "" && len(runtimeConfig.AdditionalPackages) == 0 {
return nil
}

return runtimeConfig
}

// GetWorkloadNamesFromRequest gets workload names from either the names field or group
func (s *WorkloadService) GetWorkloadNamesFromRequest(ctx context.Context, req bulkOperationRequest) ([]string, error) {
if len(req.Names) > 0 {
Expand Down
85 changes: 85 additions & 0 deletions pkg/api/v1/workload_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"go.uber.org/mock/gomock"

"github.com/stacklok/toolhive/pkg/config"
"github.com/stacklok/toolhive/pkg/container/templates"
groupsmocks "github.com/stacklok/toolhive/pkg/groups/mocks"
workloadsmocks "github.com/stacklok/toolhive/pkg/workloads/mocks"
)
Expand Down Expand Up @@ -150,3 +151,87 @@ func TestNewWorkloadService(t *testing.T) {
service := NewWorkloadService(nil, nil, nil, false)
require.NotNil(t, service)
}

func TestRuntimeConfigFromRequest(t *testing.T) {
t.Parallel()

t.Run("nil request", func(t *testing.T) {
t.Parallel()
assert.Nil(t, runtimeConfigFromRequest(nil))
})

t.Run("nil runtime config", func(t *testing.T) {
t.Parallel()
req := &createRequest{}
assert.Nil(t, runtimeConfigFromRequest(req))
})

t.Run("empty runtime config returns nil", func(t *testing.T) {
t.Parallel()

req := &createRequest{
updateRequest: updateRequest{
RuntimeConfig: &templates.RuntimeConfig{
BuilderImage: " ",
AdditionalPackages: []string{"", " "},
},
},
}

assert.Nil(t, runtimeConfigFromRequest(req))
})

t.Run("trims builder image", func(t *testing.T) {
t.Parallel()

req := &createRequest{
updateRequest: updateRequest{
RuntimeConfig: &templates.RuntimeConfig{
BuilderImage: " golang:1.24-alpine ",
},
},
}

result := runtimeConfigFromRequest(req)
require.NotNil(t, result)
assert.Equal(t, "golang:1.24-alpine", result.BuilderImage)
})

t.Run("trims and filters additional packages", func(t *testing.T) {
t.Parallel()

req := &createRequest{
updateRequest: updateRequest{
RuntimeConfig: &templates.RuntimeConfig{
AdditionalPackages: []string{" git ", "", " ", "curl"},
},
},
}

result := runtimeConfigFromRequest(req)
require.NotNil(t, result)
assert.Equal(t, []string{"git", "curl"}, result.AdditionalPackages)
})

t.Run("copies runtime config", func(t *testing.T) {
t.Parallel()

req := &createRequest{
updateRequest: updateRequest{
RuntimeConfig: &templates.RuntimeConfig{
BuilderImage: "golang:1.24-alpine",
AdditionalPackages: []string{"git"},
},
},
}

result := runtimeConfigFromRequest(req)
require.NotNil(t, result)
assert.Equal(t, "golang:1.24-alpine", result.BuilderImage)
assert.Equal(t, []string{"git"}, result.AdditionalPackages)

// Verify a copy is made for slice fields.
req.RuntimeConfig.AdditionalPackages[0] = "curl"
assert.Equal(t, []string{"git"}, result.AdditionalPackages)
})
}
4 changes: 4 additions & 0 deletions pkg/api/v1/workload_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

httpval "github.com/stacklok/toolhive-core/validation/http"
"github.com/stacklok/toolhive/pkg/container/runtime"
"github.com/stacklok/toolhive/pkg/container/templates"
"github.com/stacklok/toolhive/pkg/core"
"github.com/stacklok/toolhive/pkg/permissions"
"github.com/stacklok/toolhive/pkg/registry/registry"
Expand Down Expand Up @@ -39,6 +40,8 @@ type workloadStatusResponse struct {
type updateRequest struct {
// Docker image to use
Image string `json:"image"`
// RuntimeConfig allows overriding runtime build configuration for protocol schemes.
RuntimeConfig *templates.RuntimeConfig `json:"runtime_config,omitempty"`
// Host to bind to
Host string `json:"host"`
// Command arguments to pass to the container
Expand Down Expand Up @@ -295,6 +298,7 @@ func runConfigToCreateRequest(runConfig *runner.RunConfig) *createRequest {
return &createRequest{
updateRequest: updateRequest{
Image: runConfig.Image,
RuntimeConfig: runConfig.RuntimeConfig,
Host: runConfig.Host,
CmdArguments: runConfig.CmdArgs,
TargetPort: runConfig.TargetPort,
Expand Down
47 changes: 40 additions & 7 deletions pkg/api/v1/workloads_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,13 @@ func TestCreateWorkload(t *testing.T) {
logger.Initialize()

tests := []struct {
name string
requestBody string
setupMock func(*testing.T, *workloadsmocks.MockManager, *runtimemocks.MockRuntime, *groupsmocks.MockManager)
expectedStatus int
expectedBody string
name string
requestBody string
setupMock func(*testing.T, *workloadsmocks.MockManager, *runtimemocks.MockRuntime, *groupsmocks.MockManager)
expectedServerOrImage string
expectedRuntimeConfig *templates.RuntimeConfig
expectedStatus int
expectedBody string
}{
{
name: "invalid JSON",
Expand Down Expand Up @@ -137,6 +139,28 @@ func TestCreateWorkload(t *testing.T) {
expectedStatus: http.StatusBadRequest,
expectedBody: "Invalid proxy_mode",
},
{
name: "with runtime config override",
requestBody: `{"name": "test-workload", "image": "go://github.com/example/server", "runtime_config": {"builder_image": "golang:1.24-alpine", "additional_packages": ["ca-certificates"]}}`,
setupMock: func(_ *testing.T, wm *workloadsmocks.MockManager, _ *runtimemocks.MockRuntime, gm *groupsmocks.MockManager) {
wm.EXPECT().DoesWorkloadExist(gomock.Any(), "test-workload").Return(false, nil)
gm.EXPECT().Exists(gomock.Any(), "default").Return(true, nil)
wm.EXPECT().RunWorkloadDetached(gomock.Any(), gomock.Any()).
DoAndReturn(func(_ context.Context, runConfig *runner.RunConfig) error {
assert.NotNil(t, runConfig.RuntimeConfig)
assert.Equal(t, "golang:1.24-alpine", runConfig.RuntimeConfig.BuilderImage)
assert.Equal(t, []string{"ca-certificates"}, runConfig.RuntimeConfig.AdditionalPackages)
return nil
})
},
expectedRuntimeConfig: &templates.RuntimeConfig{
BuilderImage: "golang:1.24-alpine",
AdditionalPackages: []string{"ca-certificates"},
},
expectedServerOrImage: "go://github.com/example/server",
expectedStatus: http.StatusCreated,
expectedBody: "test-workload",
},
{
name: "with tool filters",
Comment on lines +142 to 165
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good test for the happy path! A few edge cases are missing that would strengthen coverage:

  • Empty runtime_config ({}): Verify it doesn't override defaults (i.e., runtimeConfigFromRequest returns nil).
  • runtime_config with a non-protocol-scheme image: e.g., "image": "nginx:latest" + runtime_config set. Verify the behavior is well-defined (currently silently ignored).
  • Update path round-trip: Update a workload without specifying runtime_config and verify whether a previously stored config is preserved or cleared. This matters for UX since users may not expect an update to drop their runtime overrides.

requestBody: `{"name": "test-workload", "image": "test-image", "tools": ["filter1", "filter2"]}`,
Expand Down Expand Up @@ -212,12 +236,17 @@ func TestCreateWorkload(t *testing.T) {
mockGroupManager := groupsmocks.NewMockManager(ctrl)

tt.setupMock(t, mockWorkloadManager, mockRuntime, mockGroupManager)
expectedServerOrImage := tt.expectedServerOrImage
if expectedServerOrImage == "" {
expectedServerOrImage = "test-image"
}

mockRetriever := makeMockRetriever(t,
"test-image",
expectedServerOrImage,
"test-image",
&regtypes.ImageMetadata{Image: "test-image"},
nil,
tt.expectedRuntimeConfig,
)

routes := &WorkloadRoutes{
Expand Down Expand Up @@ -411,6 +440,7 @@ func TestUpdateWorkload(t *testing.T) {
"test-image",
&regtypes.ImageMetadata{Image: "test-image"},
nil,
nil,
)

routes := &WorkloadRoutes{
Expand Down Expand Up @@ -556,6 +586,7 @@ func TestUpdateWorkload_PortReuse(t *testing.T) {
"test-image",
&regtypes.ImageMetadata{Image: "test-image"},
nil,
nil,
)

routes := &WorkloadRoutes{
Expand Down Expand Up @@ -595,12 +626,14 @@ func makeMockRetriever(
returnedImage string,
returnedServerMetadata regtypes.ServerMetadata,
returnedError error,
expectedRuntimeConfig *templates.RuntimeConfig,
) retriever.Retriever {
t.Helper()

return func(_ context.Context, serverOrImage string, _ string, verificationType string, _ string, _ *templates.RuntimeConfig) (string, regtypes.ServerMetadata, error) {
return func(_ context.Context, serverOrImage string, _ string, verificationType string, _ string, runtimeConfig *templates.RuntimeConfig) (string, regtypes.ServerMetadata, error) {
assert.Equal(t, expectedServerOrImage, serverOrImage)
assert.Equal(t, retriever.VerifyImageWarn, verificationType)
assert.Equal(t, expectedRuntimeConfig, runtimeConfig)
return returnedImage, returnedServerMetadata, returnedError
}
}
20 changes: 20 additions & 0 deletions pkg/api/v1/workloads_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/stacklok/toolhive/pkg/auth"
"github.com/stacklok/toolhive/pkg/auth/remote"
"github.com/stacklok/toolhive/pkg/container/templates"
"github.com/stacklok/toolhive/pkg/permissions"
"github.com/stacklok/toolhive/pkg/runner"
"github.com/stacklok/toolhive/pkg/secrets"
Expand Down Expand Up @@ -400,6 +401,25 @@ func TestRunConfigToCreateRequest(t *testing.T) {
assert.Empty(t, result.ToolsOverride["read"].Description)
})

t.Run("with runtime config", func(t *testing.T) {
t.Parallel()

runConfig := &runner.RunConfig{
Name: "test-workload",
RuntimeConfig: &templates.RuntimeConfig{
BuilderImage: "node:20-alpine",
AdditionalPackages: []string{"git"},
},
}

result := runConfigToCreateRequest(runConfig)

require.NotNil(t, result)
require.NotNil(t, result.RuntimeConfig)
assert.Equal(t, "node:20-alpine", result.RuntimeConfig.BuilderImage)
assert.Equal(t, []string{"git"}, result.RuntimeConfig.AdditionalPackages)
})

t.Run("nil runConfig", func(t *testing.T) {
t.Parallel()

Expand Down
Loading
Loading