From 6ea9ff5551b1dce96441e7c5129a3ab397de7e70 Mon Sep 17 00:00:00 2001 From: Mike Seese Date: Thu, 4 Apr 2024 20:43:55 -0700 Subject: [PATCH 1/7] API: add post/delete commands for port forwarding Signed-off-by: Mike Seese --- background.ts | 20 +++- bats/tests/k8s/wasm.bats | 18 ++++ .../assets/specs/command-api.yaml | 41 ++++++++ .../main/commandServer/httpCommandServer.ts | 96 +++++++++++++++++++ 4 files changed, 173 insertions(+), 2 deletions(-) diff --git a/background.ts b/background.ts index c848c8d51eb..cc2e5db520c 100644 --- a/background.ts +++ b/background.ts @@ -842,12 +842,20 @@ ipcMainProxy.handle('service-forward', async(_, service, state) => { if (state) { const hostPort = service.listenPort ?? 0; - await k8smanager.kubeBackend.forwardPort(namespace, service.name, service.port, hostPort); + await doForwardPort(namespace, service.name, service.port, hostPort); } else { - await k8smanager.kubeBackend.cancelForward(namespace, service.name, service.port); + await doCancelForward(namespace, service.name, service.port); } }); +async function doForwardPort(namespace: string, service: string, k8sPort: string | number, hostPort: number) { + return await k8smanager.kubeBackend.forwardPort(namespace, service, k8sPort, hostPort); +} + +async function doCancelForward(namespace: string, service: string, k8sPort: string | number) { + return await k8smanager.kubeBackend.cancelForward(namespace, service, k8sPort); +} + ipcMainProxy.on('k8s-integrations', async() => { mainEvents.emit('integration-update', await integrationManager.listIntegrations() ?? {}); }); @@ -1354,6 +1362,14 @@ class BackgroundCommandWorker implements CommandWorkerInterface { doFactoryReset(keepSystemImages); } + async forwardPort(namespace: string, service: string, k8sPort: string | number, hostPort: number) { + return await doForwardPort(namespace, service, k8sPort, hostPort); + } + + async cancelForward(namespace: string, service: string, k8sPort: string | number) { + return await doCancelForward(namespace, service, k8sPort); + } + /** * Execute the preference update for services that don't require a backend restart. */ diff --git a/bats/tests/k8s/wasm.bats b/bats/tests/k8s/wasm.bats index ab5d8896ac7..57334176de5 100644 --- a/bats/tests/k8s/wasm.bats +++ b/bats/tests/k8s/wasm.bats @@ -141,3 +141,21 @@ EOF assert_success assert_output "Hello world from Spin!" } + +@test 'fail to connect to the service on localhost without port forwarding' { + run try curl --silent --fail "http://localhost:8080/hello" + assert_failure +} + +@test 'connect to the service on localhost with port forwarding' { + rdctl api -X POST -b '{ "namespace": "default", "service": "hello-spin", "k8sPort": 80, "hostPort": 8080 }' port_forwarding + run try curl --silent --fail "http://localhost:8080/hello" + assert_success + assert_output "Hello world from Spin!" +} + +@test 'fail to connect to the service on localhost after removing port forwarding' { + rdctl api -X DELETE "port_forwarding?namespace=default&service=hello-spin&k8sPort=80" + run try curl --silent --fail "http://localhost:8080/hello" + assert_failure +} diff --git a/pkg/rancher-desktop/assets/specs/command-api.yaml b/pkg/rancher-desktop/assets/specs/command-api.yaml index 3e66dfc3c65..4a138fd302f 100644 --- a/pkg/rancher-desktop/assets/specs/command-api.yaml +++ b/pkg/rancher-desktop/assets/specs/command-api.yaml @@ -208,6 +208,47 @@ paths: '400': description: An error occurred + /v1/port_forwarding: + post: + operationId: createPortForward + summary: Create a new port forwarding + requestBody: + description: JSON block consisting of the port forwarding details + content: + application/json: + schema: + type: object + properties: + namespace: + type: string + service: + type: string + k8sPort: + type: string + hostPort: + type: integer + required: true + responses: + '200': + description: The port forwarding was created. + '400': + description: The port forwarding could not be created. + delete: + operationId: deletePortForward + summary: Delete a port forwarding + parameters: + - in: query + name: namespace + - in: query + name: service + - in: query + name: k8sPort + responses: + '200': + description: The port forwarding was deleted. + '400': + description: The port forwarding could not be deleted. + /v1/propose_settings: put: operationId: proposeSettings diff --git a/pkg/rancher-desktop/main/commandServer/httpCommandServer.ts b/pkg/rancher-desktop/main/commandServer/httpCommandServer.ts index fbb9391acc2..f933d3e1796 100644 --- a/pkg/rancher-desktop/main/commandServer/httpCommandServer.ts +++ b/pkg/rancher-desktop/main/commandServer/httpCommandServer.ts @@ -104,6 +104,10 @@ export class HttpCommandServer { }, delete: { '/v1/snapshots': [0, this.deleteSnapshot] }, } as const, + { + post: { '/v1/port_forwarding': [1, this.createPortForwarding] }, + delete: { '/v1/port_forwarding': [1, this.deletePortForwarding] }, + } as const, ); constructor(commandWorker: CommandWorkerInterface) { @@ -548,6 +552,95 @@ export class HttpCommandServer { } } + protected async createPortForwarding(request: express.Request, response: express.Response, _: commandContext): Promise { + let values: Record = {}; + const [data, payloadError] = await serverHelper.getRequestBody(request, MAX_REQUEST_BODY_LENGTH); + let error = ''; + let namespace = ''; + let service = ''; + let k8sPort: string | number = 0; + let hostPort = 0; + + if (!payloadError) { + try { + console.debug(`Request data: ${ data }`); + values = JSON.parse(data); + if ('namespace' in values && 'service' in values && 'k8sPort' in values && 'hostPort' in values) { + namespace = values.namespace; + + service = values.service; + + if (Number.isNaN(values.k8sPort)) { + k8sPort = values.k8sPort; + } else { + k8sPort = parseInt(values.k8sPort, 10); + } + + hostPort = values.hostPort; + } else { + error = 'missing required parameters'; + } + } catch (err) { + // TODO: Revisit this log stmt if sensitive values (e.g. PII, IPs, creds) can be provided via this command + console.log(`updateSettings: error processing JSON request block\n${ data }\n`, err); + error = 'error processing JSON request block'; + } + } else { + error = payloadError; + } + if (!error) { + try { + const result = await this.commandWorker.forwardPort(namespace, service, k8sPort, hostPort); + + if (typeof result === 'number') { + console.debug('createPortForwarding: succeeded 200'); + response.status(200).type('txt').send(`${ result }`); + } else { + console.debug(`createPortForwarding: write back status 400, error forwarding port`); + response.status(400).type('txt').send('Could not forward port'); + } + } catch (err) { + console.error(`createPortForwarding: error forwarding port:`, err); + response.status(400).type('txt').send('Could not forward port'); + } + } else { + console.debug(`createPortForwarding: write back status 400, error: ${ error }`); + response.status(400).type('txt').send(error); + } + } + + protected async deletePortForwarding(request: express.Request, response: express.Response, context: commandContext): Promise { + const namespace = request.query.namespace ?? ''; + const service = request.query.service ?? ''; + const k8sPort = request.query.k8sPort ?? ''; + + if (!namespace) { + response.status(400).type('txt').send('Port forwarding namespace is required in query parameters'); + } else if (!service) { + response.status(400).type('txt').send('Port forwarding service is required in query parameters'); + } else if (!k8sPort) { + response.status(400).type('txt').send('Port forwarding k8sPort is required in query parameters'); + } else if (typeof namespace !== 'string') { + response.status(400).type('txt').send(`Invalid port forwarding namespace ${ JSON.stringify(namespace) }: not a string.`); + } else if (typeof service !== 'string') { + response.status(400).type('txt').send(`Invalid port forwarding service ${ JSON.stringify(service) }: not a string.`); + } else if (typeof k8sPort !== 'string') { + response.status(400).type('txt').send(`Invalid port forwarding k8sPort ${ JSON.stringify(k8sPort) }: not a string.`); + } else { + const k8sPortResolved = Number.isNaN(k8sPort) ? k8sPort : parseInt(k8sPort, 10); + + try { + await this.commandWorker.cancelForward(namespace, service, k8sPortResolved); + + console.debug('deletePortForwarding: succeeded 200'); + response.status(200).type('txt').send('Port forwarding successfully deleted'); + } catch (error: any) { + console.error(`deletePortForwarding: error deleting port forwarding:`, error); + response.status(400).type('txt').send('Could not delete port forwarding'); + } + } + } + wrapShutdown(request: express.Request, response: express.Response, context: commandContext): Promise { console.debug('shutdown: succeeded 202'); response.status(202).type('txt').send('Shutting down.'); @@ -819,6 +912,9 @@ export interface CommandWorkerInterface { deleteSnapshot: (context: commandContext, name: string) => Promise; restoreSnapshot: (context: commandContext, name: string) => Promise; cancelSnapshot: () => Promise; + + forwardPort: (namespace: string, service: string, k8sPort: string | number, hostPort: number) => Promise; + cancelForward: (namespace: string, service: string, k8sPort: string | number) => Promise; } // Extend CommandWorkerInterface to have extra types, as these types are used by From 5b417bb0b6d94bcddd7d5c280e760935a606480b Mon Sep 17 00:00:00 2001 From: Mike Seese Date: Mon, 8 Apr 2024 14:21:59 -0700 Subject: [PATCH 2/7] initial port-forwarding API PR feedback Signed-off-by: Mike Seese --- bats/tests/k8s/port-forwarding.bats | 134 ++++++++++++++++++ bats/tests/k8s/wasm.bats | 18 --- .../assets/specs/command-api.yaml | 12 +- 3 files changed, 143 insertions(+), 21 deletions(-) create mode 100644 bats/tests/k8s/port-forwarding.bats diff --git a/bats/tests/k8s/port-forwarding.bats b/bats/tests/k8s/port-forwarding.bats new file mode 100644 index 00000000000..1b2dde7c8c2 --- /dev/null +++ b/bats/tests/k8s/port-forwarding.bats @@ -0,0 +1,134 @@ +load '../helpers/load' + +local_setup() { + if using_docker; then + skip "this test only works on containerd right now" + fi +} + +assert_traefik_crd_established() { + local jsonpath="{.status.conditions[?(@.type=='Established')].status}" + run kubectl get crd traefikservices.traefik.containo.us --output jsonpath="$jsonpath" + assert_success || return + assert_output 'True' +} + +@test 'start k8s' { + factory_reset + start_kubernetes + wait_for_kubelet + + # The manifests in /var/lib/rancher/k3s/server/manifests are processed + # in alphabetical order. So when traefik.yaml has been loaded we know that + # rd-runtime.yaml has already been processed. + try assert_traefik_crd_established +} + +@test 'deploy sample app' { + kubectl apply --filename - < Date: Mon, 8 Apr 2024 15:48:48 -0700 Subject: [PATCH 3/7] get port forwarding test mostly working, but skip on Windows Signed-off-by: Mike Seese --- bats/tests/k8s/port-forwarding.bats | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/bats/tests/k8s/port-forwarding.bats b/bats/tests/k8s/port-forwarding.bats index 1b2dde7c8c2..8b82a00e682 100644 --- a/bats/tests/k8s/port-forwarding.bats +++ b/bats/tests/k8s/port-forwarding.bats @@ -1,8 +1,8 @@ load '../helpers/load' local_setup() { - if using_docker; then - skip "this test only works on containerd right now" + if is_windows; then + skip "this test doesn't work on Windows" fi } @@ -26,7 +26,7 @@ assert_traefik_crd_established() { @test 'deploy sample app' { kubectl apply --filename - < Date: Mon, 8 Apr 2024 16:40:23 -0700 Subject: [PATCH 4/7] add the error code in response for forward port errors if available Signed-off-by: Mike Seese --- pkg/rancher-desktop/main/commandServer/httpCommandServer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/rancher-desktop/main/commandServer/httpCommandServer.ts b/pkg/rancher-desktop/main/commandServer/httpCommandServer.ts index f933d3e1796..53aaf9ef529 100644 --- a/pkg/rancher-desktop/main/commandServer/httpCommandServer.ts +++ b/pkg/rancher-desktop/main/commandServer/httpCommandServer.ts @@ -599,9 +599,9 @@ export class HttpCommandServer { console.debug(`createPortForwarding: write back status 400, error forwarding port`); response.status(400).type('txt').send('Could not forward port'); } - } catch (err) { + } catch (err: any) { console.error(`createPortForwarding: error forwarding port:`, err); - response.status(400).type('txt').send('Could not forward port'); + response.status(400).type('txt').send(`Could not forward port; error code: ${ typeof err.code === 'string' ? err.code : 'unknown, check the logs' }`); } } else { console.debug(`createPortForwarding: write back status 400, error: ${ error }`); From dd64d819265bcf3abdc6729705f2bc543165fb88 Mon Sep 17 00:00:00 2001 From: Mike Seese Date: Tue, 9 Apr 2024 11:32:37 -0700 Subject: [PATCH 5/7] remove obsolete assert_traefik_crd_established from port forwarding test Signed-off-by: Mike Seese --- bats/tests/k8s/port-forwarding.bats | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/bats/tests/k8s/port-forwarding.bats b/bats/tests/k8s/port-forwarding.bats index 8b82a00e682..bf87df62d8e 100644 --- a/bats/tests/k8s/port-forwarding.bats +++ b/bats/tests/k8s/port-forwarding.bats @@ -6,22 +6,10 @@ local_setup() { fi } -assert_traefik_crd_established() { - local jsonpath="{.status.conditions[?(@.type=='Established')].status}" - run kubectl get crd traefikservices.traefik.containo.us --output jsonpath="$jsonpath" - assert_success || return - assert_output 'True' -} - @test 'start k8s' { factory_reset start_kubernetes wait_for_kubelet - - # The manifests in /var/lib/rancher/k3s/server/manifests are processed - # in alphabetical order. So when traefik.yaml has been loaded we know that - # rd-runtime.yaml has already been processed. - try assert_traefik_crd_established } @test 'deploy sample app' { From 5ae3966f05fbc3d572d21e8ab96d651062d9883f Mon Sep 17 00:00:00 2001 From: Mike Seese Date: Tue, 9 Apr 2024 17:09:36 -0700 Subject: [PATCH 6/7] remove port forwarding test that leveraged traefik auto forwarding Signed-off-by: Mike Seese --- bats/tests/k8s/port-forwarding.bats | 7 ------- 1 file changed, 7 deletions(-) diff --git a/bats/tests/k8s/port-forwarding.bats b/bats/tests/k8s/port-forwarding.bats index bf87df62d8e..6e71c57b915 100644 --- a/bats/tests/k8s/port-forwarding.bats +++ b/bats/tests/k8s/port-forwarding.bats @@ -85,13 +85,6 @@ spec: EOF } -@test 'connect to the service' { - # This can take 100s with old versions of traefik, and 15s with newer ones. - run try curl --silent --fail "http://localhost" - assert_success - assert_output "Hello World!" -} - @test 'fail to connect to the service on localhost without port forwarding' { run try --max 5 curl --silent --fail "http://localhost:8080" assert_failure From 63577d93c21b05d378e6c40a366ad54831339ce6 Mon Sep 17 00:00:00 2001 From: Mike Seese Date: Tue, 9 Apr 2024 17:58:14 -0700 Subject: [PATCH 7/7] don't skip windows for port forwarding tests Signed-off-by: Mike Seese --- bats/tests/k8s/port-forwarding.bats | 6 ------ 1 file changed, 6 deletions(-) diff --git a/bats/tests/k8s/port-forwarding.bats b/bats/tests/k8s/port-forwarding.bats index 6e71c57b915..e23e77e4728 100644 --- a/bats/tests/k8s/port-forwarding.bats +++ b/bats/tests/k8s/port-forwarding.bats @@ -1,11 +1,5 @@ load '../helpers/load' -local_setup() { - if is_windows; then - skip "this test doesn't work on Windows" - fi -} - @test 'start k8s' { factory_reset start_kubernetes