Skip to content

Commit d023d85

Browse files
authored
Add permission profile object to API (#1027)
1 parent 19e8434 commit d023d85

File tree

7 files changed

+181
-48
lines changed

7 files changed

+181
-48
lines changed

docs/server/docs.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/swagger.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/swagger.yaml

Lines changed: 6 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/api/v1/workloads.go

Lines changed: 27 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,8 @@ func (s *WorkloadRoutes) createWorkload(w http.ResponseWriter, r *http.Request)
230230

231231
// Mimic behavior of the CLI by defaulting to the "network" permission profile.
232232
// TODO: Consider moving this into the run config creation logic.
233-
if req.PermissionProfile == "" {
234-
req.PermissionProfile = permissions.ProfileNetwork
233+
if req.PermissionProfile != nil {
234+
req.PermissionProfile = permissions.BuiltinNetworkProfile()
235235
}
236236

237237
// Fetch or build the requested image
@@ -253,43 +253,28 @@ func (s *WorkloadRoutes) createWorkload(w http.ResponseWriter, r *http.Request)
253253

254254
// NOTE: None of the k8s-related config logic is included here.
255255
runSecrets := secrets.SecretParametersToCLI(req.Secrets)
256-
runConfig, err := runner.NewRunConfigFromFlags(
257-
ctx,
258-
s.containerRuntime,
259-
req.CmdArguments,
260-
req.Name,
261-
imageURL,
262-
imageMetadata,
263-
req.Host,
264-
s.debugMode,
265-
req.Volumes,
266-
runSecrets,
267-
req.AuthzConfig,
268-
"", // req.AuditConfig not set - auditing not exposed through API yet.
269-
false, // req.EnableAudit not set - auditing not exposed through API yet.
270-
req.PermissionProfile,
271-
transport.LocalhostIPv4, // Seems like a reasonable default for now.
272-
req.Transport,
273-
0, // Let the workloadManager figure out which port to use.
274-
req.TargetPort,
275-
req.EnvVars,
276-
req.OIDC.Issuer,
277-
req.OIDC.Audience,
278-
req.OIDC.JwksURL,
279-
req.OIDC.ClientID,
280-
req.OIDC.AllowOpaqueTokens,
281-
"", // otelEndpoint - not exposed through API yet
282-
"", // otelServiceName - not exposed through API yet
283-
0.0, // otelSamplingRate - default value
284-
nil, // otelHeaders - not exposed through API yet
285-
false, // otelInsecure - not exposed through API yet
286-
false, // otelEnablePrometheusMetricsPath - not exposed through API yet
287-
nil, // otelEnvironmentVariables - not exposed through API yet
288-
false, // isolateNetwork - not exposed through API yet
289-
"", // k8s patch - not relevant here.
290-
&runner.DetachedEnvVarValidator{},
291-
types.ProxyMode(req.ProxyMode),
292-
)
256+
runConfig, err := runner.NewRunConfigBuilder().
257+
WithRuntime(s.containerRuntime).
258+
WithCmdArgs(req.CmdArguments).
259+
WithName(req.Name).
260+
WithImage(imageURL).
261+
WithHost(req.Host).
262+
WithTargetHost(transport.LocalhostIPv4).
263+
WithDebug(s.debugMode).
264+
WithVolumes(req.Volumes).
265+
WithSecrets(runSecrets).
266+
WithAuthzConfigPath(req.AuthzConfig).
267+
WithAuditConfigPath("").
268+
WithPermissionProfile(req.PermissionProfile).
269+
WithNetworkIsolation(req.NetworkIsolation).
270+
WithK8sPodPatch("").
271+
WithProxyMode(types.ProxyMode(req.ProxyMode)).
272+
WithTransportAndPorts(req.Transport, 0, req.TargetPort).
273+
WithAuditEnabled(false, "").
274+
WithOIDCConfig(req.OIDC.Issuer, req.OIDC.Audience, req.OIDC.JwksURL, req.OIDC.ClientID, req.OIDC.AllowOpaqueTokens).
275+
WithTelemetryConfig("", false, "", 0.0, nil, false, nil). // Not exposed through API yet.
276+
Build(ctx, imageMetadata, req.EnvVars, &runner.DetachedEnvVarValidator{})
277+
293278
if err != nil {
294279
logger.Errorf("Failed to create run config: %v", err)
295280
http.Error(w, "Failed to create run config", http.StatusBadRequest)
@@ -504,9 +489,11 @@ type createRequest struct {
504489
// OIDC configuration options
505490
OIDC oidcOptions `json:"oidc"`
506491
// Permission profile to apply
507-
PermissionProfile string `json:"permission_profile"`
492+
PermissionProfile *permissions.Profile `json:"permission_profile"`
508493
// Proxy mode to use
509494
ProxyMode string `json:"proxy_mode"`
495+
// Whether network isolation is turned on. This applies the rules in the permission profile.
496+
NetworkIsolation bool `json:"network_isolation"`
510497
}
511498

512499
// oidcOptions represents OIDC configuration options

pkg/runner/config.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,10 @@ func (c *RunConfig) WithPorts(port, targetPort int) (*RunConfig, error) {
245245

246246
// ParsePermissionProfile loads and sets the permission profile
247247
func (c *RunConfig) ParsePermissionProfile() (*RunConfig, error) {
248+
if c.PermissionProfile != nil {
249+
return c, nil
250+
}
251+
248252
if c.PermissionProfileNameOrPath == "" {
249253
return c, fmt.Errorf("permission profile name or path is required")
250254
}

pkg/runner/config_builder.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/stacklok/toolhive/pkg/authz"
1111
rt "github.com/stacklok/toolhive/pkg/container/runtime"
1212
"github.com/stacklok/toolhive/pkg/logger"
13+
"github.com/stacklok/toolhive/pkg/permissions"
1314
"github.com/stacklok/toolhive/pkg/registry"
1415
"github.com/stacklok/toolhive/pkg/telemetry"
1516
"github.com/stacklok/toolhive/pkg/transport"
@@ -108,9 +109,21 @@ func (b *RunConfigBuilder) WithAuditConfigPath(path string) *RunConfigBuilder {
108109
return b
109110
}
110111

111-
// WithPermissionProfileNameOrPath sets the permission profile name or path
112+
// WithPermissionProfileNameOrPath sets the permission profile name or path.
113+
// If called multiple times or mixed with WithPermissionProfile,
114+
// the last call takes precedence.
112115
func (b *RunConfigBuilder) WithPermissionProfileNameOrPath(profile string) *RunConfigBuilder {
113116
b.config.PermissionProfileNameOrPath = profile
117+
b.config.PermissionProfile = nil // Clear any existing profile
118+
return b
119+
}
120+
121+
// WithPermissionProfile sets the permission profile directly.
122+
// If called multiple times or mixed with WithPermissionProfile,
123+
// the last call takes precedence.
124+
func (b *RunConfigBuilder) WithPermissionProfile(profile *permissions.Profile) *RunConfigBuilder {
125+
b.config.PermissionProfile = profile
126+
b.config.PermissionProfileNameOrPath = "" // Clear any existing name or path
114127
return b
115128
}
116129

@@ -290,7 +303,7 @@ func (b *RunConfigBuilder) validateConfig(imageMetadata *registry.ImageMetadata)
290303
}
291304

292305
// If we are missing the permission profile, attempt to load one from the image metadata.
293-
if c.PermissionProfileNameOrPath == "" && imageMetadata != nil {
306+
if c.PermissionProfileNameOrPath == "" && c.PermissionProfile == nil && imageMetadata != nil {
294307
permProfilePath, err := CreatePermissionProfileFile(c.Name, imageMetadata.Permissions)
295308
if err != nil {
296309
// Just log the error and continue with the default permission profile

pkg/runner/config_builder_test.go

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
package runner
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/stacklok/toolhive/pkg/logger"
11+
"github.com/stacklok/toolhive/pkg/permissions"
12+
"github.com/stacklok/toolhive/pkg/registry"
13+
)
14+
15+
func TestRunConfigBuilder_Build_WithPermissionProfile(t *testing.T) {
16+
t.Parallel()
17+
18+
// Needed to prevent a nil pointer dereference in the logger.
19+
logger.Initialize()
20+
21+
// Create a mock environment variable validator
22+
mockValidator := &mockEnvVarValidator{}
23+
24+
imageMetadata := &registry.ImageMetadata{
25+
Name: "test-image",
26+
Permissions: &permissions.Profile{
27+
Network: &permissions.NetworkPermissions{
28+
Outbound: &permissions.OutboundNetworkPermissions{
29+
InsecureAllowAll: true,
30+
AllowTransport: []string{"http", "https"},
31+
},
32+
},
33+
Read: []permissions.MountDeclaration{permissions.MountDeclaration("/test/read")},
34+
Write: []permissions.MountDeclaration{permissions.MountDeclaration("/test/write")},
35+
},
36+
}
37+
38+
testCases := []struct {
39+
name string
40+
setupBuilder func(*RunConfigBuilder) *RunConfigBuilder
41+
expectedPermissionProfile *permissions.Profile
42+
imageMetadata *registry.ImageMetadata
43+
}{
44+
{
45+
name: "Build with direct permission profile",
46+
setupBuilder: func(b *RunConfigBuilder) *RunConfigBuilder {
47+
return b.WithPermissionProfile(permissions.BuiltinNetworkProfile())
48+
},
49+
imageMetadata: imageMetadata,
50+
expectedPermissionProfile: permissions.BuiltinNetworkProfile(),
51+
},
52+
{
53+
name: "Build with permission profile name",
54+
setupBuilder: func(b *RunConfigBuilder) *RunConfigBuilder {
55+
return b.WithPermissionProfileNameOrPath(permissions.ProfileNetwork)
56+
},
57+
imageMetadata: imageMetadata,
58+
expectedPermissionProfile: permissions.BuiltinNetworkProfile(),
59+
},
60+
{
61+
name: "Build after profile name overrides direct profile",
62+
setupBuilder: func(b *RunConfigBuilder) *RunConfigBuilder {
63+
return b.WithPermissionProfile(permissions.BuiltinNoneProfile()).
64+
WithPermissionProfileNameOrPath(permissions.ProfileNetwork)
65+
},
66+
imageMetadata: imageMetadata,
67+
expectedPermissionProfile: permissions.BuiltinNetworkProfile(),
68+
},
69+
{
70+
name: "Build after direct profile overrides profile name",
71+
setupBuilder: func(b *RunConfigBuilder) *RunConfigBuilder {
72+
return b.WithPermissionProfileNameOrPath(permissions.ProfileNetwork).
73+
WithPermissionProfile(permissions.BuiltinNoneProfile())
74+
},
75+
imageMetadata: imageMetadata,
76+
expectedPermissionProfile: permissions.BuiltinNoneProfile(),
77+
},
78+
{
79+
name: "Build with permissions from image metadata",
80+
setupBuilder: func(b *RunConfigBuilder) *RunConfigBuilder {
81+
return b.WithName("test-container")
82+
},
83+
imageMetadata: imageMetadata,
84+
expectedPermissionProfile: &permissions.Profile{
85+
Network: &permissions.NetworkPermissions{
86+
Outbound: &permissions.OutboundNetworkPermissions{
87+
InsecureAllowAll: true,
88+
AllowTransport: []string{"http", "https"},
89+
},
90+
},
91+
Read: []permissions.MountDeclaration{permissions.MountDeclaration("/test/read")},
92+
Write: []permissions.MountDeclaration{permissions.MountDeclaration("/test/read")},
93+
},
94+
},
95+
}
96+
97+
for _, tc := range testCases {
98+
t.Run(tc.name, func(t *testing.T) {
99+
t.Parallel()
100+
101+
// Create a new builder and apply the setup
102+
builder := tc.setupBuilder(NewRunConfigBuilder())
103+
require.NotNil(t, builder, "Builder should not be nil")
104+
105+
// Build the configuration with required parameters
106+
ctx := context.Background()
107+
config, err := builder.Build(ctx, tc.imageMetadata, nil, mockValidator)
108+
require.NoError(t, err, "Build should not return an error")
109+
require.NotNil(t, config, "Built config should not be nil")
110+
111+
assert.NotNil(t, config.PermissionProfile, "Built config's PermissionProfile should not be nil")
112+
113+
// Compare specific fields of the permission profile
114+
assert.Equal(t, tc.expectedPermissionProfile.Network.Outbound.InsecureAllowAll,
115+
config.PermissionProfile.Network.Outbound.InsecureAllowAll,
116+
"Network outbound setting allow all should match in built config")
117+
assert.Equal(t, tc.expectedPermissionProfile.Network.Outbound.AllowTransport,
118+
config.PermissionProfile.Network.Outbound.AllowTransport,
119+
"Network outbound setting transport should match in built config")
120+
121+
assert.Equal(t, len(tc.expectedPermissionProfile.Read), len(config.PermissionProfile.Read),
122+
"Number of read permissions should match in built config")
123+
assert.Equal(t, len(tc.expectedPermissionProfile.Write), len(config.PermissionProfile.Write),
124+
"Number of write permissions should match in built config")
125+
})
126+
}
127+
}

0 commit comments

Comments
 (0)