Skip to content

Commit daab93e

Browse files
committed
Steve: address AI review comments
Signed-off-by: Mark Yen <mark.yen@suse.com>
1 parent 24e9a1b commit daab93e

File tree

4 files changed

+40
-24
lines changed

4 files changed

+40
-24
lines changed

pkg/rancher-desktop/backend/steve.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,15 @@ export class Steve {
184184

185185
req.on('response', (res) => resolve(res.statusCode === 200));
186186
req.on('error', () => resolve(false));
187+
// Timeout if we don't get a response in a reasonable time.
188+
setTimeout(1_000).then(() => {
189+
try {
190+
req.abort();
191+
} catch {
192+
// ignore
193+
}
194+
resolve(false);
195+
});
187196
req.end();
188197
});
189198
}

pkg/rancher-desktop/main/dashboardServer/index.ts

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export class DashboardServer {
2727
private host = '127.0.0.1';
2828
private port = 6120;
2929
private stevePort = 0;
30-
private proxies: Record<string, ReturnType<typeof createProxyMiddleware>> = Object.create(null);
30+
private proxies: Record<ProxyKeys, ReturnType<typeof createProxyMiddleware>> = Object.create(null);
3131

3232
/**
3333
* Checks for an existing instance of Dashboard server.
@@ -95,15 +95,13 @@ export class DashboardServer {
9595

9696
// Register wrapper functions so that when createProxies() replaces
9797
// this.proxies (on each Steve restart), express and the upgrade
98-
// handler automatically use the new instances. The optional
99-
// chaining is safe: proxies are always created before the UI is
100-
// notified that Kubernetes is ready, and the dashboard button is
101-
// disabled until then.
102-
//
103-
// ?? next() fires only when the proxy is absent: createProxyMiddleware
104-
// returns an async function, so the call always yields a Promise (truthy).
98+
// handler automatically use the new instances. The call is safe: proxies
99+
// are always created before the UI is notified that Kubernetes is ready,
100+
// and the dashboard button is disabled until then.
105101
ProxyKeys.forEach((key) => {
106-
this.dashboardServer.use(key, (req, res, next) => this.proxies[key]?.(req, res, next) ?? next());
102+
this.dashboardServer.use(key, (req, res, next) => {
103+
return this.proxies[key] ? this.proxies[key](req, res, next) : next();
104+
});
107105
});
108106

109107
this.dashboardApp = this.dashboardServer
@@ -133,20 +131,15 @@ export class DashboardServer {
133131
return;
134132
}
135133

136-
// TODO: drive this from ProxyKeys instead of maintaining a parallel list.
137-
const key = req.url?.startsWith('/v1')
138-
? '/v1'
139-
: req.url?.startsWith('/v3')
140-
? '/v3'
141-
: req.url?.startsWith('/k8s/')
142-
? '/k8s'
143-
: req.url?.startsWith('/api/')
144-
? '/api'
145-
: undefined;
146-
147-
if (key) {
148-
return this.proxies[key]?.upgrade(req, socket, head);
134+
const upgradeKeys = new Set<ProxyKeys>(['/v1', '/v3', '/k8s', '/api']);
135+
const key = Array.from(upgradeKeys).find((key) => {
136+
return req.url === key || req.url?.startsWith(key + '/');
137+
});
138+
139+
if (key && this.proxies[key]) {
140+
return this.proxies[key].upgrade(req, socket, head);
149141
}
142+
150143
console.log(`Unknown Web socket upgrade request for ${ req.url }`);
151144
socket.destroy();
152145
});

pkg/rancher-desktop/utils/__tests__/networks.spec.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,11 @@ describe('getAvailablePorts', () => {
4040

4141
expect(ports[0]).not.toBe(ports[1]);
4242
});
43+
44+
it('can accept dynamic counts', async() => {
45+
const count = Math.floor(1.0); // Force number type, not a literal.
46+
const ports = await getAvailablePorts(count);
47+
48+
expect(ports).toHaveLength(count);
49+
});
4350
});

pkg/rancher-desktop/utils/networks.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,21 @@ export enum networkStatus {
77
OFFLINE = 'offline',
88
}
99

10+
/**
11+
* TupleOf<T, N> is a tuple of N elements of type T.
12+
*/
13+
type TupleOf<T, N extends number, R extends T[] = []> =
14+
number extends N ? T[] : // If N is not a literal, return a normal array.
15+
R['length'] extends N ? R : TupleOf<T, N, [...R, T]>;
16+
1017
/**
1118
* Ask the OS to assign free ports on localhost. All ports are bound
1219
* simultaneously before any are released, guaranteeing distinct values.
1320
* The ports are released before returning, so there is a small TOCTOU
1421
* race before the caller binds them. In practice the risk is negligible
1522
* because the caller (Steve) binds within seconds.
1623
*/
17-
export async function getAvailablePorts(count: number): Promise<number[]> {
24+
export async function getAvailablePorts<N extends number>(count: N): Promise<TupleOf<number, N>> {
1825
const servers: net.Server[] = [];
1926

2027
try {
@@ -28,7 +35,7 @@ export async function getAvailablePorts(count: number): Promise<number[]> {
2835
});
2936
}
3037

31-
return servers.map(s => (s.address() as net.AddressInfo).port);
38+
return servers.map(s => (s.address() as net.AddressInfo).port) as TupleOf<number, N>;
3239
} finally {
3340
await Promise.all(servers.map(s =>
3441
new Promise<void>(resolve => s.close(() => resolve()))));

0 commit comments

Comments
 (0)