Add labels to Windows Desktop Service, add endpoint for searching them #16436

ryanclark · 2022-09-15T14:46:44Z

As part of the server access flow in Discover, we generate a JoinToken which has an id (the token used to authenticate with the auth server) and an internalResourceId.

The node-install.sh script creates a Teleport config, adding ssh_service.labels['teleport.internal/resource-id'] that is equal to the internalResourceId of the token.

ssh_service:
  enabled: yes
  labels:
    teleport.internal/resource-id: 308fd103-6ee2-4180-aa0d-61ed1e9a80ae

Currently, there isn't the same functionality for windows_desktop_service.

This means that we can't poll for the WindowsDesktopService that joins once the user has started Teleport (after configuring it with the output of the configure AD script).

In order to be able to ping WindowsDesktopServices by the teleport.internal/resource-id label, I've added windows_desktop_service.labels. The configure AD script should add this property in the config it outputs to the console.

This also adds an endpoint for the frontend to be able to query them - v1/webapi/sites/:site/desktopservices.

zmb3 · 2022-09-15T14:49:02Z

api/client/client.go

@@ -2767,6 +2767,8 @@ func (c *Client) ListResources(ctx context.Context, req proto.ListResourcesReque
 			resources[i] = respResource.GetKubeService()
 		case types.KindWindowsDesktop:
 			resources[i] = respResource.GetWindowsDesktop()
+		case types.KindWindowsDesktopService:


I would check with Lisa on this. I don't remember the details but there is some reason why ListResources isn't supported for desktops.

api/proto/teleport/legacy/client/proto/authservice.proto

lib/config/fileconf.go

zmb3 · 2022-09-15T14:54:26Z

lib/services/role.go

@@ -1958,6 +1958,8 @@ func (set RoleSet) checkAccess(r AccessCheckable, mfa AccessMFAParams, matchers
 	case types.KindWindowsDesktop:
 		getRoleLabels = types.Role.GetWindowsDesktopLabels
 		additionalDeniedMessage = "Confirm Windows user."
+	case types.KindWindowsDesktopService:
+		getRoleLabels = types.Role.GetWindowsDesktopLabels


Hmm is this right? This looks like it only applies to the actual resources, not the Teleport agents that report them.

lib/web/servers.go

lib/web/ui/server.go

docs/pages/includes/desktop-access/desktop-config.yaml

ptgott · 2022-09-19T16:58:40Z

docs/pages/includes/desktop-access/desktop-config.yaml

+
+  # Labels to attach to the Windows Desktop Service. This is used internally,
+  # any custom labels added won't affect the Windows hosts.


I'm not sure what the "This is used internally" sentence means. Since this field is only used internally, when would someone want to set a custom value for it?

They shouldn't, but I didn't know if we allowed for config properties that are only used internally to go undocumented on the config reference page.

lib/web/servers.go

This allows us to add labels to the `windows_desktop_service` section of the config and store them inside `WindowsDesktopServiceV3`.

During the Desktop Discover flow, we need to be able to search for any `WindowsDesktopServices` that have joined with the label generated in the config. This adds an endpoint that works the same as fetching `WindowsDesktop` resources, and shares the permissions of the `desktop` resource.

Co-authored-by: Paul Gottschling <[email protected]>

Co-authored-by: Yassine Bounekhla <[email protected]>

github-actions · 2022-09-23T22:08:46Z

@ryanclark See the table below for backport results.

Branch	Result
branch/v10	Failed

#16436)

* Init an uploader service for Windows Desktop Service This will result in multiple uploader services running if there are other Teleport services running in the same process, but that will be fixed by #14521. Updates #12549 * buddy: Fix incorrect use of loop variables (#16306) * Fix incorrect use of loop variables This commit fixes a few occurrences of loop variables being incorrectly used in the context of Go-routines or (most frequently) parallel tests. To fix the issues, we create a local copy of the range variables before the parallel tests (or Go-routine), as suggested in the documentation of the `testing` package: https://pkg.go.dev/testing#hdr-Subtests_and_Sub_benchmarks Issues were found using the `loopvarcapture` linter. Signed-off-by: Roman Tkachenko <[email protected]> * fix TestTraceProvider/spans_exported_with_gRPC+TLS * run TestSSH serially * operator: Conserve 'created_by' data in user spec Signed-off-by: Roman Tkachenko <[email protected]> Co-authored-by: Renato Costa <[email protected]> Co-authored-by: Tim Ross <[email protected]> Co-authored-by: Hugo Hervieux <[email protected]> * Correctly set the end time when completing an upload The upload completer would set the session end time based on the last known event in the session, but it left the timestamp in the event's metadata blank, which caused it to default to the time the upload was completed. Fixes #16555 * Speed up TestTerminalPing (#16481) This test takes between 10-20 seconds to run, because it waits for a websocket ping message and pings run on a 10s timer by default. It turns out, we allow specifying the keepalive interval in the terminal request, but due to a bug we would always overwrite that value with what's in the cluster networking config. Fix the bug and speed up the test by overriding the keepalive interval to something shorter for this test. Before: ➜ go test -run TestTerminalPing -race ./lib/web -count=1 ok github.com/gravitational/teleport/lib/web 13.122s After: ➜ go test -run TestTerminalPing -race ./lib/web -count=1 ok github.com/gravitational/teleport/lib/web 3.394s * Add labels to Windows Desktop Service, add endpoint for searching them (#16436) * export GetResourcesByResourceIDs * add WithCluserClientProvider * Improve unit tests in CI (#16698) - Increase timeout - Detect whether operator, CI, Rust, or Helm tests are necessary * Update `webassets` ref to latest `teleport-v10` Signed-off-by: Roman Tkachenko <[email protected]> Co-authored-by: Roman Tkachenko <[email protected]> Co-authored-by: Renato Costa <[email protected]> Co-authored-by: Tim Ross <[email protected]> Co-authored-by: Hugo Hervieux <[email protected]> Co-authored-by: Ryan Clark <[email protected]> Co-authored-by: Nic Klaassen <[email protected]> Co-authored-by: Ryan Clark <[email protected]>

github-actions bot added desktop-access documentation labels Sep 15, 2022

github-actions bot requested review from alexfornuto, probakowski, ptgott, r0mant, sagesyr, xacrimon, xinding33 and zmb3 September 15, 2022 14:47

zmb3 reviewed Sep 15, 2022

View reviewed changes

ryanclark force-pushed the ryan/add-list-desktop-services branch from ac10933 to 423f0e1 Compare September 16, 2022 23:47

zmb3 added the backport/branch/v10 label Sep 17, 2022

ptgott reviewed Sep 19, 2022

View reviewed changes

ryanclark force-pushed the ryan/add-list-desktop-services branch from 30b2c1f to d03de75 Compare September 22, 2022 17:18

zmb3 approved these changes Sep 22, 2022

View reviewed changes

ryanclark force-pushed the ryan/add-list-desktop-services branch 2 times, most recently from 377f7dc to 8beb245 Compare September 23, 2022 14:29

rudream reviewed Sep 23, 2022

View reviewed changes

lib/web/servers.go Outdated Show resolved Hide resolved

ryanclark force-pushed the ryan/add-list-desktop-services branch from 8beb245 to 090973d Compare September 23, 2022 19:27

rudream approved these changes Sep 23, 2022

View reviewed changes

github-actions bot removed request for r0mant, alexfornuto, probakowski, xacrimon, xinding33 and sagesyr September 23, 2022 20:13

ryanclark enabled auto-merge (squash) September 23, 2022 21:46

ryanclark added 2 commits September 23, 2022 22:46

Change NewWindowsDesktopServiceV3 to accept Metadata on creation

4de6493

This allows us to add labels to the `windows_desktop_service` section of the config and store them inside `WindowsDesktopServiceV3`.

ryanclark and others added 10 commits September 23, 2022 22:46

Add info about labels to the config reference

f93f41a

Add HostID to the desktop response

1129310

Add the WindowsDesktopService name to the API response

fa4e733

Code review improvements

53591ab

Fix failing test

42ec60b

Remove address from searching a windows desktop service

7702543

Update docs/pages/includes/desktop-access/desktop-config.yaml

8082921

Co-authored-by: Paul Gottschling <[email protected]>

Remove unused labels

da443e6

Move types.Server case to avoid it not being reached

fd6dcf7

Update lib/web/servers.go

493e03c

Co-authored-by: Yassine Bounekhla <[email protected]>

ryanclark force-pushed the ryan/add-list-desktop-services branch from 331a6fb to 493e03c Compare September 23, 2022 21:46

ryanclark merged commit 9f9461d into master Sep 23, 2022

ryanclark deleted the ryan/add-list-desktop-services branch September 23, 2022 22:08

ryanclark added a commit that referenced this pull request Sep 23, 2022

Add labels to Windows Desktop Service, add endpoint for searching them (

79527ef

#16436)

zmb3 mentioned this pull request Sep 23, 2022

[v10] Misc Backports #16674

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add labels to Windows Desktop Service, add endpoint for searching them #16436

Add labels to Windows Desktop Service, add endpoint for searching them #16436

ryanclark commented Sep 15, 2022

zmb3 Sep 15, 2022

zmb3 Sep 15, 2022

ptgott Sep 19, 2022

ryanclark Sep 22, 2022

github-actions bot commented Sep 23, 2022


		# Labels to attach to the Windows Desktop Service. This is used internally,
		# any custom labels added won't affect the Windows hosts.

Add labels to Windows Desktop Service, add endpoint for searching them #16436

Add labels to Windows Desktop Service, add endpoint for searching them #16436

Conversation

ryanclark commented Sep 15, 2022

zmb3 Sep 15, 2022

Choose a reason for hiding this comment

zmb3 Sep 15, 2022

Choose a reason for hiding this comment

ptgott Sep 19, 2022

Choose a reason for hiding this comment

ryanclark Sep 22, 2022

Choose a reason for hiding this comment

github-actions bot commented Sep 23, 2022