Skip to content

Commit 188d9b5

Browse files
committed
fix: address review follow-ups
- guard kubectl exec terminal sizing against nil queues and add adapter tests - make runtime error handler overrides resettable and add handler/formatter coverage - make devspacehelper e2e pod checks explicit after the wait loop - document CustomNavLink leaf-route matching and clean up webpack/postcss/sass config - update assets Signed-off-by: Ryan Swanson <ryan.swanson@loft.sh>
1 parent b9a900c commit 188d9b5

14 files changed

Lines changed: 355 additions & 190 deletions

File tree

assets/assets.go

Lines changed: 10 additions & 12 deletions
Large diffs are not rendered by default.

pkg/devspace/kubectl/exec.go

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,13 @@ import (
44
"bytes"
55
"context"
66
"io"
7-
7+
88
"github.com/loft-sh/devspace/pkg/util/terminal"
9-
"k8s.io/kubectl/pkg/util/term"
10-
119
corev1 "k8s.io/api/core/v1"
1210
"k8s.io/client-go/tools/remotecommand"
1311
kubectlExec "k8s.io/client-go/util/exec"
1412
"k8s.io/kubectl/pkg/scheme"
13+
"k8s.io/kubectl/pkg/util/term"
1514
)
1615

1716
// termSizeQueueAdapter adapts k8s.io/kubectl/pkg/util/term.TerminalSizeQueue to
@@ -20,7 +19,13 @@ type termSizeQueueAdapter struct {
2019
q term.TerminalSizeQueue
2120
}
2221

22+
var _ remotecommand.TerminalSizeQueue = (*termSizeQueueAdapter)(nil)
23+
2324
func (a *termSizeQueueAdapter) Next() *remotecommand.TerminalSize {
25+
if a == nil || a.q == nil {
26+
return nil
27+
}
28+
2429
size := a.q.Next()
2530
if size == nil {
2631
return nil
@@ -34,7 +39,7 @@ type SubResource string
3439
const (
3540
// SubResourceExec creates a new process in the container and attaches to that
3641
SubResourceExec SubResource = "exec"
37-
42+
3843
// SubResourceAttach attaches to the top process of the container
3944
SubResourceAttach SubResource = "attach"
4045
)
@@ -47,34 +52,36 @@ func (client *client) execStreamWithTransport(ctx context.Context, options *Exec
4752
streamOptions remotecommand.StreamOptions
4853
tty = options.TTY
4954
)
50-
55+
5156
if options.SubResource == "" {
5257
options.SubResource = SubResourceExec
5358
}
54-
59+
5560
wrapper, upgradeRoundTripper, err := GetUpgraderWrapper(client)
5661
if err != nil {
5762
return err
5863
}
59-
64+
6065
execRequest := client.KubeClient().CoreV1().RESTClient().Post().
6166
Resource("pods").
6267
Name(options.Pod.Name).
6368
Namespace(options.Pod.Namespace).
6469
SubResource(string(options.SubResource))
65-
70+
6671
if tty {
6772
tty, t = terminal.SetupTTY(options.Stdin, options.Stdout)
6873
if options.ForceTTY || tty {
6974
tty = true
7075
if t.Raw && options.TerminalSizeQueue == nil {
7176
// this call spawns a goroutine to monitor/update the terminal size
72-
sizeQueue = &termSizeQueueAdapter{q: t.MonitorSize(t.GetSize())}
77+
if q := t.MonitorSize(t.GetSize()); q != nil {
78+
sizeQueue = &termSizeQueueAdapter{q: q}
79+
}
7380
} else if options.TerminalSizeQueue != nil {
7481
sizeQueue = options.TerminalSizeQueue
7582
t.Raw = true
7683
}
77-
84+
7885
// unset options.Stderr if it was previously set because both stdout and stderr
7986
// go over t.Out when tty is true
8087
options.Stderr = nil
@@ -93,7 +100,7 @@ func (client *client) execStreamWithTransport(ctx context.Context, options *Exec
93100
Stderr: options.Stderr,
94101
}
95102
}
96-
103+
97104
switch options.SubResource {
98105
case SubResourceExec:
99106
execRequest.VersionedParams(&corev1.PodExecOptions{
@@ -113,21 +120,23 @@ func (client *client) execStreamWithTransport(ctx context.Context, options *Exec
113120
TTY: tty,
114121
}, scheme.ParameterCodec)
115122
}
116-
123+
117124
exec, err := remotecommand.NewSPDYExecutorForTransports(wrapper, upgradeRoundTripper, "POST", execRequest.URL())
118125
if err != nil {
119126
return err
120127
}
121-
128+
122129
errChan := make(chan error)
123130
go func() {
124131
errChan <- t.Safe(func() error {
125132
return exec.StreamWithContext(ctx, streamOptions)
126133
})
127134
}()
128-
135+
129136
select {
130137
case <-ctx.Done():
138+
// Nothing actionable can be done with a close error during shutdown, and the stream goroutine
139+
// will still surface any real streaming error via errChan.
131140
_ = upgradeRoundTripper.Close()
132141
<-errChan
133142
return nil
@@ -139,18 +148,18 @@ func (client *client) execStreamWithTransport(ctx context.Context, options *Exec
139148
// ExecStreamOptions are the options for ExecStream
140149
type ExecStreamOptions struct {
141150
Pod *corev1.Pod
142-
151+
143152
Container string
144153
Command []string
145-
154+
146155
ForceTTY bool
147156
TTY bool
148157
TerminalSizeQueue remotecommand.TerminalSizeQueue
149-
158+
150159
Stdin io.Reader
151160
Stdout io.Writer
152161
Stderr io.Writer
153-
162+
154163
SubResource SubResource
155164
}
156165

@@ -163,7 +172,7 @@ func (client *client) ExecStream(ctx context.Context, options *ExecStreamOptions
163172
func (client *client) ExecBuffered(ctx context.Context, pod *corev1.Pod, container string, command []string, stdin io.Reader) ([]byte, []byte, error) {
164173
stdoutBuffer := &bytes.Buffer{}
165174
stderrBuffer := &bytes.Buffer{}
166-
175+
167176
kubeExecError := client.ExecStream(ctx, &ExecStreamOptions{
168177
Pod: pod,
169178
Container: container,
@@ -177,7 +186,7 @@ func (client *client) ExecBuffered(ctx context.Context, pod *corev1.Pod, contain
177186
return nil, nil, kubeExecError
178187
}
179188
}
180-
189+
181190
return stdoutBuffer.Bytes(), stderrBuffer.Bytes(), kubeExecError
182191
}
183192

@@ -197,6 +206,6 @@ func (client *client) ExecBufferedCombined(ctx context.Context, pod *corev1.Pod,
197206
return nil, kubeExecError
198207
}
199208
}
200-
209+
201210
return stdoutBuffer.Bytes(), kubeExecError
202211
}

pkg/devspace/kubectl/exec_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package kubectl
2+
3+
import (
4+
"testing"
5+
6+
"k8s.io/kubectl/pkg/util/term"
7+
)
8+
9+
type fakeTerminalSizeQueue struct {
10+
sizes []*term.TerminalSize
11+
}
12+
13+
func (f *fakeTerminalSizeQueue) Next() *term.TerminalSize {
14+
if len(f.sizes) == 0 {
15+
return nil
16+
}
17+
18+
size := f.sizes[0]
19+
f.sizes = f.sizes[1:]
20+
return size
21+
}
22+
23+
func TestTermSizeQueueAdapterNext(t *testing.T) {
24+
t.Run("nil adapter", func(t *testing.T) {
25+
var adapter *termSizeQueueAdapter
26+
if size := adapter.Next(); size != nil {
27+
t.Fatalf("expected nil size, got %#v", size)
28+
}
29+
})
30+
31+
t.Run("nil queue", func(t *testing.T) {
32+
adapter := &termSizeQueueAdapter{}
33+
if size := adapter.Next(); size != nil {
34+
t.Fatalf("expected nil size, got %#v", size)
35+
}
36+
})
37+
38+
t.Run("convert terminal size", func(t *testing.T) {
39+
adapter := &termSizeQueueAdapter{
40+
q: &fakeTerminalSizeQueue{
41+
sizes: []*term.TerminalSize{
42+
{Width: 80, Height: 24},
43+
},
44+
},
45+
}
46+
47+
size := adapter.Next()
48+
if size == nil {
49+
t.Fatal("expected terminal size, got nil")
50+
}
51+
if size.Width != 80 || size.Height != 24 {
52+
t.Fatalf("expected 80x24 terminal size, got %#v", size)
53+
}
54+
})
55+
}

0 commit comments

Comments
 (0)