From 32cd26fc8ed9859885432f68c9755b7a9647d37e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Wed, 9 Sep 2026 09:14:59 +0200 Subject: [PATCH] refactor(sdk/go): unify functional-option handling with shared applier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a single generic applier, options.Apply[T any, O ~func(*T)], in a new leaf package openshell/v1/internal/options, and route all option-application sites through it. Nil option entries are now ignored uniformly instead of panicking; non-nil options still apply in order, and each site keeps its existing pre-seeding and defaulting. - Add internal/options package with Apply and unit tests - Route the 15 application sites (root, types, oidc, fake, gateway, edge) through options.Apply; exported types.Apply*Options keep their signatures and delegate to it - Add one nil-handling test per affected package - Document the nil-option rule in README.md and the docs book No public API signature changes. Closes #3213 Signed-off-by: Roland Huß --- sdk/go/README.md | 4 +- sdk/go/docs/src/architecture.md | 21 ++++ sdk/go/openshell/v1/auth_refresh.go | 5 +- sdk/go/openshell/v1/edge/options_nil_test.go | 23 +++++ sdk/go/openshell/v1/edge/tunnel.go | 5 +- sdk/go/openshell/v1/fake/fake.go | 5 +- sdk/go/openshell/v1/fake/options_nil_test.go | 30 ++++++ sdk/go/openshell/v1/gateway/gateway.go | 5 +- .../openshell/v1/gateway/options_nil_test.go | 23 +++++ .../openshell/v1/internal/options/options.go | 19 ++++ .../v1/internal/options/options_test.go | 98 +++++++++++++++++++ sdk/go/openshell/v1/oidc/credentials.go | 5 +- sdk/go/openshell/v1/oidc/device.go | 5 +- sdk/go/openshell/v1/oidc/oidc.go | 5 +- sdk/go/openshell/v1/oidc/options_nil_test.go | 23 +++++ sdk/go/openshell/v1/options_nil_test.go | 54 ++++++++++ sdk/go/openshell/v1/ssh_client.go | 5 +- sdk/go/openshell/v1/tcp_client.go | 9 +- sdk/go/openshell/v1/types/log.go | 10 +- sdk/go/openshell/v1/types/options_nil_test.go | 29 ++++++ sdk/go/openshell/v1/types/policy.go | 22 ++--- 21 files changed, 357 insertions(+), 48 deletions(-) create mode 100644 sdk/go/openshell/v1/edge/options_nil_test.go create mode 100644 sdk/go/openshell/v1/fake/options_nil_test.go create mode 100644 sdk/go/openshell/v1/gateway/options_nil_test.go create mode 100644 sdk/go/openshell/v1/internal/options/options.go create mode 100644 sdk/go/openshell/v1/internal/options/options_test.go create mode 100644 sdk/go/openshell/v1/oidc/options_nil_test.go create mode 100644 sdk/go/openshell/v1/options_nil_test.go create mode 100644 sdk/go/openshell/v1/types/options_nil_test.go diff --git a/sdk/go/README.md b/sdk/go/README.md index e011af01d8..d4ca13a44e 100644 --- a/sdk/go/README.md +++ b/sdk/go/README.md @@ -28,7 +28,9 @@ patterns will look familiar: - **Watch primitives**: channel-based watchers with `ResultChan()` and `Stop()`, identical to `watch.Interface` in client-go - **Functional options**: variadic option patterns for list filtering, - pagination, and watch configuration + pagination, and watch configuration. Nil options are silently ignored + at every entry point, so conditional option lists are safe to pass + without filtering out nil entries. - **Composable auth with token refresh**: wraps `oauth2.TokenSource` for automatic token caching and coalesced refresh, following the k8s client-go `cachingTokenSource` pattern diff --git a/sdk/go/docs/src/architecture.md b/sdk/go/docs/src/architecture.md index b08abf09b0..c9c6948d80 100644 --- a/sdk/go/docs/src/architecture.md +++ b/sdk/go/docs/src/architecture.md @@ -80,6 +80,27 @@ profiles, err := client.Providers().Profiles().List(ctx, "default") status, err := client.Providers().Refresh().GetStatus(ctx, "default", "openai", "api-key") ``` +## Functional Options + +Many SDK entry points accept variadic option parameters that configure the call: + +```go +auth, err := v1.RefreshableToken(tokenSource, + v1.WithLeeway(30*time.Second), + v1.WithLogger(logger), +) +``` + +Options are applied in the order they are passed, so later options override earlier ones when they set the same field. **Nil options are silently ignored at every entry point.** You can safely build option lists conditionally without filtering out nil entries: + +```go +opts := []v1.RefreshOption{ + v1.WithLeeway(30 * time.Second), + maybeLogger(), // may return nil +} +auth, err := v1.RefreshableToken(tokenSource, opts...) +``` + ## gRPC Layer Underneath, the SDK communicates with the OpenShell gateway over gRPC. The single `OpenShell` service in `proto/openshell.proto` defines all RPCs. The SDK maps each interface method to one or more RPCs: diff --git a/sdk/go/openshell/v1/auth_refresh.go b/sdk/go/openshell/v1/auth_refresh.go index ef7a6c8743..86c475bf12 100644 --- a/sdk/go/openshell/v1/auth_refresh.go +++ b/sdk/go/openshell/v1/auth_refresh.go @@ -12,6 +12,7 @@ import ( "golang.org/x/oauth2" "golang.org/x/sync/singleflight" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" ) @@ -160,9 +161,7 @@ func RefreshableToken(src oauth2.TokenSource, opts ...RefreshOption) (AuthProvid } cfg := defaultRefreshConfig() - for _, o := range opts { - o(&cfg) - } + options.Apply(&cfg, opts) return &refreshableAuth{ source: src, diff --git a/sdk/go/openshell/v1/edge/options_nil_test.go b/sdk/go/openshell/v1/edge/options_nil_test.go new file mode 100644 index 0000000000..720e357131 --- /dev/null +++ b/sdk/go/openshell/v1/edge/options_nil_test.go @@ -0,0 +1,23 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package edge + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" +) + +func TestTunnelOption_NilIgnored(t *testing.T) { + cfg := tunnelConfig{closeTimeout: defaultCloseTimeout} + options.Apply(&cfg, []TunnelOption{ + WithTunnelLogger(nil), + nil, + WithCloseTimeout(10 * time.Second), + }) + assert.Equal(t, 10*time.Second, cfg.closeTimeout) +} diff --git a/sdk/go/openshell/v1/edge/tunnel.go b/sdk/go/openshell/v1/edge/tunnel.go index 72cfedceb0..6576c9275a 100644 --- a/sdk/go/openshell/v1/edge/tunnel.go +++ b/sdk/go/openshell/v1/edge/tunnel.go @@ -17,6 +17,7 @@ import ( "github.com/coder/websocket" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" ) @@ -107,9 +108,7 @@ func NewTunnelProxy(gatewayURL, edgeToken string, opts ...TunnelOption) (*Tunnel cfg := tunnelConfig{ closeTimeout: defaultCloseTimeout, } - for _, o := range opts { - o(&cfg) - } + options.Apply(&cfg, opts) listener, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { diff --git a/sdk/go/openshell/v1/fake/fake.go b/sdk/go/openshell/v1/fake/fake.go index 16a35fed8a..31b9ce8301 100644 --- a/sdk/go/openshell/v1/fake/fake.go +++ b/sdk/go/openshell/v1/fake/fake.go @@ -8,6 +8,7 @@ import ( "sync" v1 "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" ) @@ -97,9 +98,7 @@ func NewClient(opts ...ClientOption) *Client { fc.workspaces = newFakeWorkspaceClient(fc.workspaceStore, fc.memberStore, fc.isClosed) fc.inference = newFakeInferenceClient(fc.isClosed) - for _, opt := range opts { - opt(fc) - } + options.Apply(fc, opts) return fc } diff --git a/sdk/go/openshell/v1/fake/options_nil_test.go b/sdk/go/openshell/v1/fake/options_nil_test.go new file mode 100644 index 0000000000..b58fcdd8e9 --- /dev/null +++ b/sdk/go/openshell/v1/fake/options_nil_test.go @@ -0,0 +1,30 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package fake + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" +) + +func TestClientOption_NilIgnored(t *testing.T) { + hr := &types.HealthResult{Healthy: false, Version: "1.2.3"} + cu := &types.CurrentUser{Subject: "user-42", DisplayName: "Test User"} + + fc := NewClient( + WithHealthResult(hr), + nil, + WithCurrentUser(cu), + ) + require.NotNil(t, fc) + + healthClient := fc.health.(*fakeHealthClient) + assert.Equal(t, false, healthClient.result.Healthy) + assert.Equal(t, "1.2.3", healthClient.result.Version) + assert.Equal(t, "Test User", healthClient.currentUser.DisplayName) +} diff --git a/sdk/go/openshell/v1/gateway/gateway.go b/sdk/go/openshell/v1/gateway/gateway.go index 8d1bd9d87b..da7c9814b4 100644 --- a/sdk/go/openshell/v1/gateway/gateway.go +++ b/sdk/go/openshell/v1/gateway/gateway.go @@ -7,6 +7,7 @@ import ( "fmt" v1 "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" ) @@ -27,9 +28,7 @@ func NewClient(name string, opts ...ClientOption) (*v1.Client, error) { // Apply caller options. cc := &clientConfig{} - for _, o := range opts { - o(cc) - } + options.Apply(cc, opts) // Resolve auth provider: caller override takes precedence. auth := cc.auth diff --git a/sdk/go/openshell/v1/gateway/options_nil_test.go b/sdk/go/openshell/v1/gateway/options_nil_test.go new file mode 100644 index 0000000000..8aa8061d15 --- /dev/null +++ b/sdk/go/openshell/v1/gateway/options_nil_test.go @@ -0,0 +1,23 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package gateway + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" +) + +func TestClientOption_NilIgnored(t *testing.T) { + var cfg clientConfig + options.Apply(&cfg, []ClientOption{ + WithTimeout(30 * time.Second), + nil, + WithLogger(nil), + }) + assert.Equal(t, 30*time.Second, cfg.timeout) +} diff --git a/sdk/go/openshell/v1/internal/options/options.go b/sdk/go/openshell/v1/internal/options/options.go new file mode 100644 index 0000000000..f8092cecef --- /dev/null +++ b/sdk/go/openshell/v1/internal/options/options.go @@ -0,0 +1,19 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package options provides a shared mechanism for applying functional options +// across all SDK entry points. It enforces a single nil-option rule: nil +// entries in an option list are silently ignored. +package options + +// Apply folds opts into target in order, skipping nil entries. +// O is constrained to any named function type whose underlying type is +// func(*T), so callers write options.Apply(&cfg, opts) and both type +// parameters are inferred. +func Apply[T any, O ~func(*T)](target *T, opts []O) { + for _, opt := range opts { + if opt != nil { + opt(target) + } + } +} diff --git a/sdk/go/openshell/v1/internal/options/options_test.go b/sdk/go/openshell/v1/internal/options/options_test.go new file mode 100644 index 0000000000..693326d308 --- /dev/null +++ b/sdk/go/openshell/v1/internal/options/options_test.go @@ -0,0 +1,98 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package options + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// testConfig is a plain config struct for testing Apply. +type testConfig struct { + a int + b string +} + +// testOption is a named option type, mirroring the SDK's pattern. +type testOption func(*testConfig) + +func TestApply_EmptySlice(t *testing.T) { + var cfg testConfig + Apply(&cfg, []testOption{}) + assert.Equal(t, testConfig{}, cfg) +} + +func TestApply_NilSlice(t *testing.T) { + var cfg testConfig + Apply[testConfig, testOption](&cfg, nil) + assert.Equal(t, testConfig{}, cfg) +} + +func TestApply_AllNil(t *testing.T) { + var cfg testConfig + Apply(&cfg, []testOption{nil, nil, nil}) + assert.Equal(t, testConfig{}, cfg) +} + +func TestApply_NilFirst(t *testing.T) { + var cfg testConfig + Apply(&cfg, []testOption{ + nil, + func(c *testConfig) { c.a = 1 }, + }) + assert.Equal(t, 1, cfg.a) +} + +func TestApply_NilMiddle(t *testing.T) { + var cfg testConfig + Apply(&cfg, []testOption{ + func(c *testConfig) { c.a = 1 }, + nil, + func(c *testConfig) { c.b = "two" }, + }) + assert.Equal(t, 1, cfg.a) + assert.Equal(t, "two", cfg.b) +} + +func TestApply_NilLast(t *testing.T) { + var cfg testConfig + Apply(&cfg, []testOption{ + func(c *testConfig) { c.a = 1 }, + nil, + }) + assert.Equal(t, 1, cfg.a) +} + +func TestApply_OrderPreserved(t *testing.T) { + var cfg testConfig + Apply(&cfg, []testOption{ + func(c *testConfig) { c.a = 1 }, + func(c *testConfig) { c.a = 2 }, + func(c *testConfig) { c.a = 3 }, + }) + // The last option wins because options apply in order. + assert.Equal(t, 3, cfg.a) +} + +// liveObject simulates the fake.Client pattern where options mutate a live +// object rather than a plain config struct. +type liveObject struct { + name string + count int +} + +type liveOption func(*liveObject) + +func TestApply_LiveObjectTarget(t *testing.T) { + obj := &liveObject{name: "initial", count: 0} + Apply(obj, []liveOption{ + nil, + func(o *liveObject) { o.name = "updated" }, + nil, + func(o *liveObject) { o.count = 42 }, + }) + assert.Equal(t, "updated", obj.name) + assert.Equal(t, 42, obj.count) +} diff --git a/sdk/go/openshell/v1/oidc/credentials.go b/sdk/go/openshell/v1/oidc/credentials.go index e0971442b2..98bfd6219b 100644 --- a/sdk/go/openshell/v1/oidc/credentials.go +++ b/sdk/go/openshell/v1/oidc/credentials.go @@ -16,6 +16,7 @@ import ( "golang.org/x/oauth2" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/gateway" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" ) // ClientCredentials performs a non-interactive OAuth2 client credentials @@ -37,9 +38,7 @@ func ClientCredentials(ctx context.Context, opts ...LoginOption) (*oauth2.Token, func resolveClientCredentialsConfig(opts ...LoginOption) (*loginConfig, error) { cfg := &loginConfig{} - for _, opt := range opts { - opt(cfg) - } + options.Apply(cfg, opts) cfg.applyDefaults() // Client credentials should not send interactive scopes by default. diff --git a/sdk/go/openshell/v1/oidc/device.go b/sdk/go/openshell/v1/oidc/device.go index ed73654181..049d053d71 100644 --- a/sdk/go/openshell/v1/oidc/device.go +++ b/sdk/go/openshell/v1/oidc/device.go @@ -16,6 +16,7 @@ import ( "golang.org/x/oauth2" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/gateway" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" ) // deviceAuthResponse holds the parsed response from the device @@ -46,9 +47,7 @@ type deviceAuthResponse struct { // - any other error: return [ErrDeviceCode] func DeviceLogin(ctx context.Context, opts ...LoginOption) (*oauth2.Token, error) { cfg := &loginConfig{} - for _, opt := range opts { - opt(cfg) - } + options.Apply(cfg, opts) cfg.applyDefaults() if _, hasDeadline := ctx.Deadline(); !hasDeadline && cfg.timeout > 0 { var cancel context.CancelFunc diff --git a/sdk/go/openshell/v1/oidc/oidc.go b/sdk/go/openshell/v1/oidc/oidc.go index 916e31b78c..b80ae1ca8a 100644 --- a/sdk/go/openshell/v1/oidc/oidc.go +++ b/sdk/go/openshell/v1/oidc/oidc.go @@ -13,6 +13,7 @@ import ( "golang.org/x/oauth2" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/gateway" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" ) // Login performs an interactive OIDC authorization code login. @@ -33,9 +34,7 @@ import ( // fallback flow is used instead. func Login(ctx context.Context, gatewayName string, opts ...LoginOption) (*oauth2.Token, error) { cfg := &loginConfig{} - for _, opt := range opts { - opt(cfg) - } + options.Apply(cfg, opts) cfg.applyDefaults() // Apply configured timeout if the caller's context has no deadline. diff --git a/sdk/go/openshell/v1/oidc/options_nil_test.go b/sdk/go/openshell/v1/oidc/options_nil_test.go new file mode 100644 index 0000000000..49ec64cf6a --- /dev/null +++ b/sdk/go/openshell/v1/oidc/options_nil_test.go @@ -0,0 +1,23 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package oidc + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" +) + +func TestLoginOption_NilIgnored(t *testing.T) { + var cfg loginConfig + options.Apply(&cfg, []LoginOption{ + WithIssuer("https://auth.example.com"), + nil, + WithClientID("my-app"), + }) + assert.Equal(t, "https://auth.example.com", cfg.issuer) + assert.Equal(t, "my-app", cfg.clientID) +} diff --git a/sdk/go/openshell/v1/options_nil_test.go b/sdk/go/openshell/v1/options_nil_test.go new file mode 100644 index 0000000000..3952bedbf4 --- /dev/null +++ b/sdk/go/openshell/v1/options_nil_test.go @@ -0,0 +1,54 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package v1 + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" +) + +func TestRefreshOption_NilIgnored(t *testing.T) { + cfg := defaultRefreshConfig() + options.Apply(&cfg, []RefreshOption{ + WithLeeway(5 * time.Second), + nil, + WithLeeway(20 * time.Second), + }) + assert.Equal(t, 20*time.Second, cfg.leeway) +} + +func TestForwardOption_NilIgnored(t *testing.T) { + var cfg forwardConfig + options.Apply(&cfg, []ForwardOption{ + WithForwardServiceID("svc-a"), + nil, + WithForwardServiceID("svc-b"), + }) + assert.Equal(t, "svc-b", cfg.serviceID) +} + +func TestListenOption_NilIgnored(t *testing.T) { + cfg := listenConfig{bindAddress: "127.0.0.1"} + options.Apply(&cfg, []ListenOption{ + WithBindAddress("0.0.0.0"), + nil, + WithListenServiceID("svc-listen"), + }) + assert.Equal(t, "0.0.0.0", cfg.bindAddress) + assert.Equal(t, "svc-listen", cfg.serviceID) +} + +func TestTunnelOption_NilIgnored(t *testing.T) { + var cfg tunnelConfig + options.Apply(&cfg, []TunnelOption{ + WithTunnelServiceID("svc-x"), + nil, + WithTunnelServiceID("svc-y"), + }) + assert.Equal(t, "svc-y", cfg.serviceID) +} diff --git a/sdk/go/openshell/v1/ssh_client.go b/sdk/go/openshell/v1/ssh_client.go index f2120e2cea..6e7ee1a462 100644 --- a/sdk/go/openshell/v1/ssh_client.go +++ b/sdk/go/openshell/v1/ssh_client.go @@ -11,6 +11,7 @@ import ( "time" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/converter" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" pb "github.com/NVIDIA/OpenShell/sdk/go/proto/openshellv1" "google.golang.org/grpc" ) @@ -64,9 +65,7 @@ func (s *sshClient) Tunnel(ctx context.Context, workspace, sandboxName string, p } var cfg tunnelConfig - for _, o := range opts { - o(&cfg) - } + options.Apply(&cfg, opts) sandbox, err := s.sandboxes.Get(ctx, workspace, sandboxName) if err != nil { diff --git a/sdk/go/openshell/v1/tcp_client.go b/sdk/go/openshell/v1/tcp_client.go index 0945aff1cf..e0795a9682 100644 --- a/sdk/go/openshell/v1/tcp_client.go +++ b/sdk/go/openshell/v1/tcp_client.go @@ -12,6 +12,7 @@ import ( "sync" "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/converter" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" pb "github.com/NVIDIA/OpenShell/sdk/go/proto/openshellv1" "google.golang.org/grpc" ) @@ -43,9 +44,7 @@ func (t *tcpClient) Forward(ctx context.Context, workspace, sandboxName string, } var cfg forwardConfig - for _, o := range opts { - o(&cfg) - } + options.Apply(&cfg, opts) streamCtx, cancel := context.WithCancel(ctx) stream, err := t.client.ForwardTcp(streamCtx) @@ -103,9 +102,7 @@ func (t *tcpClient) Listen(ctx context.Context, workspace, sandboxName string, r } cfg := listenConfig{bindAddress: "127.0.0.1"} - for _, o := range opts { - o(&cfg) - } + options.Apply(&cfg, opts) if cfg.useSSHTunnel && t.ssh == nil { return nil, &StatusError{ diff --git a/sdk/go/openshell/v1/types/log.go b/sdk/go/openshell/v1/types/log.go index 85a62c29db..291a46004d 100644 --- a/sdk/go/openshell/v1/types/log.go +++ b/sdk/go/openshell/v1/types/log.go @@ -3,7 +3,11 @@ package types -import "time" +import ( + "time" + + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" +) // LogLine represents a single log entry from a sandbox. type LogLine struct { @@ -71,9 +75,7 @@ func WithLogMinLevel(level string) LogOption { // ApplyLogOptions applies options and returns the config. func ApplyLogOptions(opts []LogOption) logConfig { //nolint:revive // unexported return is intentional; consumed only by v1 package var cfg logConfig - for _, opt := range opts { - opt(&cfg) - } + options.Apply(&cfg, opts) return cfg } diff --git a/sdk/go/openshell/v1/types/options_nil_test.go b/sdk/go/openshell/v1/types/options_nil_test.go new file mode 100644 index 0000000000..d629837f54 --- /dev/null +++ b/sdk/go/openshell/v1/types/options_nil_test.go @@ -0,0 +1,29 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package types + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestLogOption_NilIgnored(t *testing.T) { + cfg := ApplyLogOptions([]LogOption{ + WithLogLines(100), + nil, + WithLogMinLevel("error"), + }) + assert.Equal(t, uint32(100), cfg.Lines()) + assert.Equal(t, "error", cfg.MinLevel()) +} + +func TestGetDraftOption_NilIgnored(t *testing.T) { + cfg := ApplyGetDraftOptions([]GetDraftOption{ + WithStatusFilter("pending"), + nil, + WithStatusFilter("approved"), + }) + assert.Equal(t, "approved", cfg.StatusFilter()) +} diff --git a/sdk/go/openshell/v1/types/policy.go b/sdk/go/openshell/v1/types/policy.go index 9b6082b9ec..4cd39eb081 100644 --- a/sdk/go/openshell/v1/types/policy.go +++ b/sdk/go/openshell/v1/types/policy.go @@ -3,7 +3,11 @@ package types -import "time" +import ( + "time" + + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/options" +) // PolicyLoadStatus represents the load state of a policy revision. type PolicyLoadStatus int @@ -266,9 +270,7 @@ func WithStatusFilter(status string) GetDraftOption { // ApplyGetDraftOptions applies options and returns the config. func ApplyGetDraftOptions(opts []GetDraftOption) getDraftConfig { //nolint:revive // unexported return is intentional; consumed only by v1 package var cfg getDraftConfig - for _, opt := range opts { - opt(&cfg) - } + options.Apply(&cfg, opts) return cfg } @@ -303,9 +305,7 @@ func WithIncludeSecurityFlagged() ApproveAllOption { // ApplyApproveAllOptions applies options and returns the config. func ApplyApproveAllOptions(opts []ApproveAllOption) approveAllConfig { //nolint:revive // unexported return is intentional; consumed only by v1 package var cfg approveAllConfig - for _, opt := range opts { - opt(&cfg) - } + options.Apply(&cfg, opts) return cfg } @@ -347,9 +347,7 @@ func WithStatusGlobal(global bool) GetStatusOption { // ApplyGetStatusOptions applies options and returns the config. func ApplyGetStatusOptions(opts []GetStatusOption) getStatusConfig { //nolint:revive // unexported return is intentional; consumed only by v1 package var cfg getStatusConfig - for _, opt := range opts { - opt(&cfg) - } + options.Apply(&cfg, opts) return cfg } @@ -399,9 +397,7 @@ func WithListGlobal(global bool) ListPolicyOption { // ApplyListPolicyOptions applies options and returns the config. func ApplyListPolicyOptions(opts []ListPolicyOption) listPolicyConfig { //nolint:revive // unexported return is intentional; consumed only by v1 package var cfg listPolicyConfig - for _, opt := range opts { - opt(&cfg) - } + options.Apply(&cfg, opts) return cfg }