Skip to content

Commit a7f6efd

Browse files
rhussrhuss
committed
fix(sdk/go): address review feedback on PR #2271
- Make scheme parsing drive transport selection: http:// uses plaintext gRPC, https:// or no scheme uses TLS. Add regression tests. - Add Resources and DriverConfig fields to SandboxTemplate and update both converter directions (SandboxFromProto/SandboxSpecToProto). - Regenerate proto bindings from current canonical proto sources to eliminate drift (SigV4/MCP fields, params matchers, reserved fields). - Run gofmt/goimports on all handwritten Go files. Signed-off-by: Roland Huß <rhuss@redhat.com>
1 parent 4790b3c commit a7f6efd

20 files changed

Lines changed: 756 additions & 433 deletions

‎sdk/go/openshell/v1/client.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ type ClientInterface interface {
3838

3939
// FileInterface is defined in file.go
4040

41-
4241
// Client implements ClientInterface. It holds a gRPC connection and provides
4342
// sub-client accessors following the Kubernetes client-go pattern.
4443
type Client struct {

‎sdk/go/openshell/v1/internal/converter/copy.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
package converter
55

6+
import "google.golang.org/protobuf/types/known/structpb"
7+
68
// CopyStringMap returns a shallow copy of a string-to-string map.
79
// Returns nil for nil input.
810
func CopyStringMap(m map[string]string) map[string]string {
@@ -48,6 +50,21 @@ func CopyByteSlice(b []byte) []byte {
4850
return c
4951
}
5052

53+
func structToMap(s *structpb.Struct) map[string]any {
54+
if s == nil {
55+
return nil
56+
}
57+
return s.AsMap()
58+
}
59+
60+
func mapToStruct(m map[string]any) *structpb.Struct {
61+
if m == nil {
62+
return nil
63+
}
64+
s, _ := structpb.NewStruct(m)
65+
return s
66+
}
67+
5168
func boolCount(flags ...bool) int {
5269
n := 0
5370
for _, f := range flags {

‎sdk/go/openshell/v1/internal/converter/errors.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,15 @@ import (
1111
)
1212

1313
var grpcToSDK = map[codes.Code]types.ErrorCode{
14-
codes.NotFound: types.ErrorNotFound,
15-
codes.AlreadyExists: types.ErrorAlreadyExists,
16-
codes.Unavailable: types.ErrorUnavailable,
17-
codes.PermissionDenied: types.ErrorPermissionDenied,
18-
codes.Unauthenticated: types.ErrorPermissionDenied,
19-
codes.InvalidArgument: types.ErrorInvalidArgument,
20-
codes.DeadlineExceeded: types.ErrorDeadlineExceeded,
21-
codes.Canceled: types.ErrorCancelled,
22-
codes.Internal: types.ErrorInternal,
14+
codes.NotFound: types.ErrorNotFound,
15+
codes.AlreadyExists: types.ErrorAlreadyExists,
16+
codes.Unavailable: types.ErrorUnavailable,
17+
codes.PermissionDenied: types.ErrorPermissionDenied,
18+
codes.Unauthenticated: types.ErrorPermissionDenied,
19+
codes.InvalidArgument: types.ErrorInvalidArgument,
20+
codes.DeadlineExceeded: types.ErrorDeadlineExceeded,
21+
codes.Canceled: types.ErrorCancelled,
22+
codes.Internal: types.ErrorInternal,
2323
codes.Unimplemented: types.ErrorUnimplemented,
2424
codes.Aborted: types.ErrorConflict,
2525
codes.FailedPrecondition: types.ErrorConflict,

‎sdk/go/openshell/v1/internal/converter/network_policy.go‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -64,19 +64,19 @@ func NetworkPolicyRuleToProto(r *types.NetworkPolicyRule) *sbv1.NetworkPolicyRul
6464

6565
func policyNetworkEndpointFromProto(ep *sbv1.NetworkEndpoint) types.PolicyNetworkEndpoint {
6666
result := types.PolicyNetworkEndpoint{
67-
Host: ep.GetHost(),
68-
Port: ep.GetPort(),
69-
Protocol: ep.GetProtocol(),
70-
TLS: ep.GetTls(),
71-
Enforcement: ep.GetEnforcement(),
72-
Access: ep.GetAccess(),
73-
AllowEncodedSlash: ep.GetAllowEncodedSlash(),
74-
PersistedQueries: ep.GetPersistedQueries(),
75-
GraphqlMaxBodyBytes: ep.GetGraphqlMaxBodyBytes(),
76-
Path: ep.GetPath(),
77-
WebsocketCredentialRewrite: ep.GetWebsocketCredentialRewrite(),
67+
Host: ep.GetHost(),
68+
Port: ep.GetPort(),
69+
Protocol: ep.GetProtocol(),
70+
TLS: ep.GetTls(),
71+
Enforcement: ep.GetEnforcement(),
72+
Access: ep.GetAccess(),
73+
AllowEncodedSlash: ep.GetAllowEncodedSlash(),
74+
PersistedQueries: ep.GetPersistedQueries(),
75+
GraphqlMaxBodyBytes: ep.GetGraphqlMaxBodyBytes(),
76+
Path: ep.GetPath(),
77+
WebsocketCredentialRewrite: ep.GetWebsocketCredentialRewrite(),
7878
RequestBodyCredentialRewrite: ep.GetRequestBodyCredentialRewrite(),
79-
AdvisorProposed: ep.GetAdvisorProposed(),
79+
AdvisorProposed: ep.GetAdvisorProposed(),
8080
}
8181
if ports := ep.GetPorts(); len(ports) > 0 {
8282
result.Ports = make([]uint32, len(ports))
@@ -114,19 +114,19 @@ func policyNetworkEndpointFromProto(ep *sbv1.NetworkEndpoint) types.PolicyNetwor
114114

115115
func policyNetworkEndpointToProto(ep *types.PolicyNetworkEndpoint) *sbv1.NetworkEndpoint {
116116
result := &sbv1.NetworkEndpoint{
117-
Host: ep.Host,
118-
Port: ep.Port,
119-
Protocol: ep.Protocol,
120-
Tls: ep.TLS,
121-
Enforcement: ep.Enforcement,
122-
Access: ep.Access,
123-
AllowEncodedSlash: ep.AllowEncodedSlash,
124-
PersistedQueries: ep.PersistedQueries,
125-
GraphqlMaxBodyBytes: ep.GraphqlMaxBodyBytes,
126-
Path: ep.Path,
127-
WebsocketCredentialRewrite: ep.WebsocketCredentialRewrite,
117+
Host: ep.Host,
118+
Port: ep.Port,
119+
Protocol: ep.Protocol,
120+
Tls: ep.TLS,
121+
Enforcement: ep.Enforcement,
122+
Access: ep.Access,
123+
AllowEncodedSlash: ep.AllowEncodedSlash,
124+
PersistedQueries: ep.PersistedQueries,
125+
GraphqlMaxBodyBytes: ep.GraphqlMaxBodyBytes,
126+
Path: ep.Path,
127+
WebsocketCredentialRewrite: ep.WebsocketCredentialRewrite,
128128
RequestBodyCredentialRewrite: ep.RequestBodyCredentialRewrite,
129-
AdvisorProposed: ep.AdvisorProposed,
129+
AdvisorProposed: ep.AdvisorProposed,
130130
}
131131
if len(ep.Ports) > 0 {
132132
result.Ports = make([]uint32, len(ep.Ports))

‎sdk/go/openshell/v1/internal/converter/sandbox.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ func sandboxSpecFromProto(spec *pb.SandboxSpec) types.SandboxSpec {
5454
Labels: CopyStringMap(tmpl.GetLabels()),
5555
Annotations: CopyStringMap(tmpl.GetAnnotations()),
5656
Environment: CopyStringMap(tmpl.GetEnvironment()),
57+
Resources: structToMap(tmpl.GetResources()),
5758
UserNamespaces: CopyBoolPtr(tmpl.UserNamespaces),
59+
DriverConfig: structToMap(tmpl.GetDriverConfig()),
5860
}
5961
}
6062

@@ -165,7 +167,9 @@ func SandboxSpecToProto(spec *types.SandboxSpec) *pb.SandboxSpec {
165167
Labels: CopyStringMap(spec.Template.Labels),
166168
Annotations: CopyStringMap(spec.Template.Annotations),
167169
Environment: CopyStringMap(spec.Template.Environment),
170+
Resources: mapToStruct(spec.Template.Resources),
168171
UserNamespaces: CopyBoolPtr(spec.Template.UserNamespaces),
172+
DriverConfig: mapToStruct(spec.Template.DriverConfig),
169173
}
170174
}
171175

‎sdk/go/openshell/v1/internal/grpc/conn.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,22 @@ type TLSParams struct {
2626

2727
// NewConnection creates a gRPC client connection.
2828
// The address may include an http:// or https:// scheme (as written by the
29-
// upstream gateway), which is stripped since gRPC expects host:port.
29+
// upstream gateway). The scheme drives transport selection: http:// uses
30+
// plaintext, https:// or no scheme uses TLS.
3031
func NewConnection(address string, tlsCfg *TLSParams, auth credentials.PerRPCCredentials) (*grpc.ClientConn, error) {
31-
address = strings.TrimPrefix(address, "https://")
32-
address = strings.TrimPrefix(address, "http://")
32+
usePlaintext := false
33+
if strings.HasPrefix(address, "http://") {
34+
usePlaintext = true
35+
address = strings.TrimPrefix(address, "http://")
36+
} else {
37+
address = strings.TrimPrefix(address, "https://")
38+
}
3339
opts := []grpc.DialOption{}
3440

3541
if tlsCfg != nil && tlsCfg.Insecure {
3642
opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
43+
} else if usePlaintext {
44+
opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
3745
} else if tlsCfg != nil {
3846
creds, err := buildTLSCredentials(tlsCfg)
3947
if err != nil {

‎sdk/go/openshell/v1/internal/grpc/conn_test.go‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package grpc
5+
6+
import (
7+
"net"
8+
"testing"
9+
10+
"google.golang.org/grpc"
11+
"google.golang.org/grpc/credentials/insecure"
12+
)
13+
14+
func TestNewConnectionHTTPSchemeUsesPlaintext(t *testing.T) {
15+
lis, err := net.Listen("tcp", "127.0.0.1:0")
16+
if err != nil {
17+
t.Fatalf("listen: %v", err)
18+
}
19+
defer lis.Close()
20+
21+
srv := grpc.NewServer()
22+
go func() { _ = srv.Serve(lis) }()
23+
defer srv.Stop()
24+
25+
conn, err := NewConnection("http://"+lis.Addr().String(), nil, nil)
26+
if err != nil {
27+
t.Fatalf("NewConnection with http:// scheme failed: %v", err)
28+
}
29+
defer conn.Close()
30+
}
31+
32+
func TestNewConnectionHTTPSSchemeUsesTLS(t *testing.T) {
33+
// https:// with nil TLS config should default to system TLS.
34+
// We cannot dial a real TLS server here, but we can verify the
35+
// connection is created (it will fail on handshake, not on dial).
36+
conn, err := NewConnection("https://127.0.0.1:1", nil, nil)
37+
if err != nil {
38+
t.Fatalf("NewConnection with https:// scheme should not fail on create: %v", err)
39+
}
40+
defer conn.Close()
41+
}
42+
43+
func TestNewConnectionNoSchemeUsesTLS(t *testing.T) {
44+
conn, err := NewConnection("127.0.0.1:1", nil, nil)
45+
if err != nil {
46+
t.Fatalf("NewConnection without scheme should not fail on create: %v", err)
47+
}
48+
defer conn.Close()
49+
}
50+
51+
func TestNewConnectionInsecureTLSConfig(t *testing.T) {
52+
lis, err := net.Listen("tcp", "127.0.0.1:0")
53+
if err != nil {
54+
t.Fatalf("listen: %v", err)
55+
}
56+
defer lis.Close()
57+
58+
srv := grpc.NewServer(grpc.Creds(insecure.NewCredentials()))
59+
go func() { _ = srv.Serve(lis) }()
60+
defer srv.Stop()
61+
62+
conn, err := NewConnection(lis.Addr().String(), &TLSParams{Insecure: true}, nil)
63+
if err != nil {
64+
t.Fatalf("NewConnection with Insecure TLS config failed: %v", err)
65+
}
66+
defer conn.Close()
67+
}

‎sdk/go/openshell/v1/sandbox_client_test.go‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,20 @@ const bufSize = 1024 * 1024
2626

2727
type mockSandboxServer struct {
2828
pb.UnimplementedOpenShellServer
29-
mu sync.Mutex
30-
sandboxes map[string]*pb.Sandbox
31-
providers map[string][]*dm.Provider
32-
createErr error
33-
getErr error
34-
listErr error
35-
deleteErr error
36-
attachErr error
37-
detachErr error
38-
listProvErr error
39-
watchEvents []*pb.SandboxStreamEvent
40-
watchErr error
41-
watchKeepOpen chan struct{} // if non-nil, WatchSandbox blocks after sending events until closed
42-
watchRequest *pb.WatchSandboxRequest // recorded request
29+
mu sync.Mutex
30+
sandboxes map[string]*pb.Sandbox
31+
providers map[string][]*dm.Provider
32+
createErr error
33+
getErr error
34+
listErr error
35+
deleteErr error
36+
attachErr error
37+
detachErr error
38+
listProvErr error
39+
watchEvents []*pb.SandboxStreamEvent
40+
watchErr error
41+
watchKeepOpen chan struct{} // if non-nil, WatchSandbox blocks after sending events until closed
42+
watchRequest *pb.WatchSandboxRequest // recorded request
4343

4444
// GetLogs fields
4545
getLogsResp *pb.GetSandboxLogsResponse

‎sdk/go/openshell/v1/stub_clients.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func (s *stubProviders) Ensure(_ context.Context, _ *Provider) (*Provider, error
6868
return nil, stubError("Providers.Ensure")
6969
}
7070
func (s *stubProviders) Profiles() ProfileInterface { return &stubProfiles{} }
71-
func (s *stubProviders) Refresh() RefreshInterface { return &stubRefresh{} }
71+
func (s *stubProviders) Refresh() RefreshInterface { return &stubRefresh{} }
7272

7373
// stubProfiles implements ProfileInterface as a placeholder.
7474
type stubProfiles struct{}

‎sdk/go/openshell/v1/types/errors.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ type ErrorCode int
1313

1414
// ErrorCode values for classifying gRPC errors.
1515
const (
16-
ErrorNotFound ErrorCode = iota + 1
16+
ErrorNotFound ErrorCode = iota + 1
1717
ErrorAlreadyExists
1818
ErrorUnavailable
1919
ErrorPermissionDenied

0 commit comments

Comments
 (0)