[v17] Fix update and list for DynamicWindowsDesktops (#48159)

* proto * Fix update for DynamicWindowsDesktops * fix test server * fix test * fix test * fix test * Update api/proto/teleport/dynamicwindows/v1/dynamicwindows_service.proto Co-authored-by: Zac Bergquist <[email protected]> * update proto * fix tests --------- Co-authored-by: Zac Bergquist <[email protected]>
gravitational · Oct 30, 2024 · 4bb8cd4 · 4bb8cd4
1 parent 346dd40
commit 4bb8cd4
Show file tree

Hide file tree

Showing 15 changed files with 240 additions and 48 deletions.
diff --git a/api/client/dynamicwindows/dynamicwindows.go b/api/client/dynamicwindows/dynamicwindows.go
@@ -85,9 +85,25 @@ func (c *Client) UpdateDynamicWindowsDesktop(ctx context.Context, desktop types.
 	}
 }
 
+func (c *Client) UpsertDynamicWindowsDesktop(ctx context.Context, desktop types.DynamicWindowsDesktop) (types.DynamicWindowsDesktop, error) {
+	switch desktop := desktop.(type) {
+	case *types.DynamicWindowsDesktopV1:
+		desktop, err := c.grpcClient.UpsertDynamicWindowsDesktop(ctx, &dynamicwindows.UpsertDynamicWindowsDesktopRequest{
+			Desktop: desktop,
+		})
+		return desktop, trace.Wrap(err)
+	default:
+		return nil, trace.BadParameter("unknown desktop type: %T", desktop)
+	}
+}
+
 func (c *Client) DeleteDynamicWindowsDesktop(ctx context.Context, name string) error {
 	_, err := c.grpcClient.DeleteDynamicWindowsDesktop(ctx, &dynamicwindows.DeleteDynamicWindowsDesktopRequest{
 		Name: name,
 	})
 	return trace.Wrap(err)
 }
+
+func (c *Client) DeleteAllDynamicWindowsDesktops(ctx context.Context) error {
+	return trace.NotImplemented("DeleteAllDynamicWindowsDesktops is not supported in the gRPC client")
+}
diff --git a/api/gen/proto/go/teleport/dynamicwindows/v1/dynamicwindows_service.pb.go b/api/gen/proto/go/teleport/dynamicwindows/v1/dynamicwindows_service.pb.go
diff --git a/api/gen/proto/go/teleport/dynamicwindows/v1/dynamicwindows_service_grpc.pb.go b/api/gen/proto/go/teleport/dynamicwindows/v1/dynamicwindows_service_grpc.pb.go
diff --git a/api/proto/teleport/dynamicwindows/v1/dynamicwindows_service.proto b/api/proto/teleport/dynamicwindows/v1/dynamicwindows_service.proto
@@ -35,6 +35,8 @@ service DynamicWindowsService {
   rpc CreateDynamicWindowsDesktop(CreateDynamicWindowsDesktopRequest) returns (types.DynamicWindowsDesktopV1);
   // UpdateDynamicWindowsDesktop updates an existing dynamic Windows desktop.
   rpc UpdateDynamicWindowsDesktop(UpdateDynamicWindowsDesktopRequest) returns (types.DynamicWindowsDesktopV1);
+  // UpsertDynamicWindowsDesktop updates an existing dynamic Windows desktop or creates new if it doesn't exist.
+  rpc UpsertDynamicWindowsDesktop(UpsertDynamicWindowsDesktopRequest) returns (types.DynamicWindowsDesktopV1);
   // DeleteDynamicWindowsDesktop removes the specified dynamic Windows desktop.
   rpc DeleteDynamicWindowsDesktop(DeleteDynamicWindowsDesktopRequest) returns (google.protobuf.Empty);
 }
@@ -63,18 +65,24 @@ message GetDynamicWindowsDesktopRequest {
   string name = 1;
 }
 
-// CreateDynamicWindowsDesktopRequest is a request for a specific dynamic Windows desktop.
+// CreateDynamicWindowsDesktopRequest is used for creating new dynamic Windows desktops.
 message CreateDynamicWindowsDesktopRequest {
   // desktop to be created
   types.DynamicWindowsDesktopV1 desktop = 1;
 }
 
-// UpdateDynamicWindowsDesktopRequest is a request for a specific dynamic Windows desktop.
+// UpdateDynamicWindowsDesktopRequest is used for updating existing dynamic Windows desktops.
 message UpdateDynamicWindowsDesktopRequest {
   // desktop to be updated
   types.DynamicWindowsDesktopV1 desktop = 1;
 }
 
+// UpsertDynamicWindowsDesktopRequest is used for upserting dynamic Windows desktops.
+message UpsertDynamicWindowsDesktopRequest {
+  // desktop to be upserted
+  types.DynamicWindowsDesktopV1 desktop = 1;
+}
+
 // DeleteDynamicWindowsDesktopRequest is a request to delete a Windows desktop host.
 message DeleteDynamicWindowsDesktopRequest {
   // name is the name of the Windows desktop host.

diff --git a/lib/auth/accesspoint/accesspoint.go b/lib/auth/accesspoint/accesspoint.go
@@ -103,6 +103,7 @@ type Config struct {
 	Users                   services.UsersService
 	WebSession              types.WebSessionInterface
 	WebToken                types.WebTokenInterface
+	DynamicWindowsDesktops  services.DynamicWindowsDesktops
 	WindowsDesktops         services.WindowsDesktops
 	AutoUpdateService       services.AutoUpdateServiceGetter
 	ProvisioningStates      services.ProvisioningStates
@@ -201,6 +202,7 @@ func NewCache(cfg Config) (*cache.Cache, error) {
 		WebSession:              cfg.WebSession,
 		WebToken:                cfg.WebToken,
 		WindowsDesktops:         cfg.WindowsDesktops,
+		DynamicWindowsDesktops:  cfg.DynamicWindowsDesktops,
 		ProvisioningStates:      cfg.ProvisioningStates,
 		IdentityCenter:          cfg.IdentityCenter,
 	}

diff --git a/lib/auth/dynamicwindows/dynamicwindowsv1/service.go b/lib/auth/dynamicwindows/dynamicwindowsv1/service.go
@@ -196,6 +196,31 @@ func (s *Service) UpdateDynamicWindowsDesktop(ctx context.Context, req *dynamicw
 	return updatedDesktop, nil
 }
 
+// UpsertDynamicWindowsDesktop updates an existing dynamic Windows desktop or creates one if it doesn't exist.
+func (s *Service) UpsertDynamicWindowsDesktop(ctx context.Context, req *dynamicwindowspb.UpsertDynamicWindowsDesktopRequest) (*types.DynamicWindowsDesktopV1, error) {
+	auth, err := s.authorizer.Authorize(ctx)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	if err := auth.AuthorizeAdminAction(); err != nil {
+		return nil, trace.Wrap(err)
+	}
+	if err := auth.CheckAccessToKind(types.KindDynamicWindowsDesktop, types.VerbCreate, types.VerbUpdate); err != nil {
+		return nil, trace.Wrap(err)
+	}
+	d, err := s.backend.UpsertDynamicWindowsDesktop(ctx, req.Desktop)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	updatedDesktop, ok := d.(*types.DynamicWindowsDesktopV1)
+	if !ok {
+		return nil, trace.BadParameter("unexpected type %T", d)
+	}
+
+	return updatedDesktop, nil
+}
+
 // DeleteDynamicWindowsDesktop removes the specified dynamic Windows desktop.
 func (s *Service) DeleteDynamicWindowsDesktop(ctx context.Context, req *dynamicwindowspb.DeleteDynamicWindowsDesktopRequest) (*emptypb.Empty, error) {
 	auth, err := s.authorizer.Authorize(ctx)

diff --git a/lib/auth/dynamicwindows/dynamicwindowsv1/service_test.go b/lib/auth/dynamicwindows/dynamicwindowsv1/service_test.go
@@ -55,6 +55,11 @@ func TestServiceAccess(t *testing.T) {
 			allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
 			allowedVerbs:  []string{types.VerbUpdate},
 		},
+		{
+			name:          "UpsertDynamicWindowsDesktop",
+			allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
+			allowedVerbs:  []string{types.VerbCreate, types.VerbUpdate},
+		},
 		{
 			name:          "DeleteDynamicWindowsDesktop",
 			allowedStates: []authz.AdminActionAuthState{authz.AdminActionAuthNotRequired, authz.AdminActionAuthMFAVerified},
@@ -160,6 +165,10 @@ func callMethod(service *Service, method string) error {
 					arg.Desktop, _ = types.NewDynamicWindowsDesktopV1("test", nil, types.DynamicWindowsDesktopSpecV1{
 						Addr: "test",
 					})
+				case *dynamicwindowsv1.UpsertDynamicWindowsDesktopRequest:
+					arg.Desktop, _ = types.NewDynamicWindowsDesktopV1("test", nil, types.DynamicWindowsDesktopSpecV1{
+						Addr: "test",
+					})
 				}
 				return nil
 			}, nil)

diff --git a/lib/auth/helpers.go b/lib/auth/helpers.go
@@ -366,6 +366,7 @@ func NewTestAuthServer(cfg TestAuthServerConfig) (*TestAuthServer, error) {
 			WebSession:              svces.Identity.WebSessions(),
 			WebToken:                svces.WebTokens(),
 			WindowsDesktops:         svces.WindowsDesktops,
+			DynamicWindowsDesktops:  svces.DynamicWindowsDesktops,
 		})
 		if err != nil {
 			return nil, trace.Wrap(err)

diff --git a/lib/cache/cache_test.go b/lib/cache/cache_test.go
@@ -121,6 +121,7 @@ type testPack struct {
 	webSessionS             types.WebSessionInterface
 	webTokenS               types.WebTokenInterface
 	windowsDesktops         services.WindowsDesktops
+	dynamicWindowsDesktops  services.DynamicWindowsDesktops
 	samlIDPServiceProviders services.SAMLIdPServiceProviders
 	userGroups              services.UserGroups
 	okta                    services.Okta
@@ -269,6 +270,11 @@ func newPackWithoutCache(dir string, opts ...packOption) (*testPack, error) {
 		return nil, trace.Wrap(err)
 	}
 
+	dynamicWindowsDesktopService, err := local.NewDynamicWindowsDesktopService(p.backend)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
 	p.trustS = local.NewCAService(p.backend)
 	p.clusterConfigS = clusterConfig
 	p.provisionerS = local.NewProvisioningService(p.backend)
@@ -288,6 +294,7 @@ func newPackWithoutCache(dir string, opts ...packOption) (*testPack, error) {
 	p.databases = local.NewDatabasesService(p.backend)
 	p.databaseServices = local.NewDatabaseServicesService(p.backend)
 	p.windowsDesktops = local.NewWindowsDesktopService(p.backend)
+	p.dynamicWindowsDesktops = dynamicWindowsDesktopService
 	p.samlIDPServiceProviders, err = local.NewSAMLIdPServiceProviderService(p.backend)
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -428,6 +435,7 @@ func newPack(dir string, setupConfig func(c Config) Config, opts ...packOption)
 		DatabaseServices:        p.databaseServices,
 		Databases:               p.databases,
 		WindowsDesktops:         p.windowsDesktops,
+		DynamicWindowsDesktops:  p.dynamicWindowsDesktops,
 		SAMLIdPServiceProviders: p.samlIDPServiceProviders,
 		UserGroups:              p.userGroups,
 		Okta:                    p.okta,
@@ -657,6 +665,7 @@ func TestNodeCAFiltering(t *testing.T) {
 		WebSession:              p.cache.webSessionCache,
 		WebToken:                p.cache.webTokenCache,
 		WindowsDesktops:         p.cache.windowsDesktopsCache,
+		DynamicWindowsDesktops:  p.cache.dynamicWindowsDesktopsCache,
 		SAMLIdPServiceProviders: p.samlIDPServiceProviders,
 		UserGroups:              p.userGroups,
 		StaticHostUsers:         p.staticHostUsers,
@@ -838,6 +847,7 @@ func TestCompletenessInit(t *testing.T) {
 			DatabaseServices:        p.databaseServices,
 			Databases:               p.databases,
 			WindowsDesktops:         p.windowsDesktops,
+			DynamicWindowsDesktops:  p.dynamicWindowsDesktops,
 			SAMLIdPServiceProviders: p.samlIDPServiceProviders,
 			UserGroups:              p.userGroups,
 			Okta:                    p.okta,
@@ -921,6 +931,7 @@ func TestCompletenessReset(t *testing.T) {
 		DatabaseServices:        p.databaseServices,
 		Databases:               p.databases,
 		WindowsDesktops:         p.windowsDesktops,
+		DynamicWindowsDesktops:  p.dynamicWindowsDesktops,
 		SAMLIdPServiceProviders: p.samlIDPServiceProviders,
 		UserGroups:              p.userGroups,
 		Okta:                    p.okta,
@@ -1130,6 +1141,7 @@ func TestListResources_NodesTTLVariant(t *testing.T) {
 		DatabaseServices:        p.databaseServices,
 		Databases:               p.databases,
 		WindowsDesktops:         p.windowsDesktops,
+		DynamicWindowsDesktops:  p.dynamicWindowsDesktops,
 		SAMLIdPServiceProviders: p.samlIDPServiceProviders,
 		UserGroups:              p.userGroups,
 		Okta:                    p.okta,
@@ -1224,6 +1236,7 @@ func initStrategy(t *testing.T) {
 		DatabaseServices:        p.databaseServices,
 		Databases:               p.databases,
 		WindowsDesktops:         p.windowsDesktops,
+		DynamicWindowsDesktops:  p.dynamicWindowsDesktops,
 		SAMLIdPServiceProviders: p.samlIDPServiceProviders,
 		UserGroups:              p.userGroups,
 		Okta:                    p.okta,

diff --git a/lib/cache/collections.go b/lib/cache/collections.go
@@ -2328,7 +2328,7 @@ func (dynamicWindowsDesktopsExecutor) getAll(ctx context.Context, cache *Cache,
 	var desktops []types.DynamicWindowsDesktop
 	next := ""
 	for {
-		d, token, err := cache.dynamicWindowsDesktopsCache.ListDynamicWindowsDesktops(ctx, defaults.MaxIterationLimit, next)
+		d, token, err := cache.Config.DynamicWindowsDesktops.ListDynamicWindowsDesktops(ctx, defaults.MaxIterationLimit, next)
 		if err != nil {
 			return nil, err
 		}

diff --git a/lib/service/service.go b/lib/service/service.go
@@ -2529,6 +2529,7 @@ func (process *TeleportProcess) newAccessCacheForServices(cfg accesspoint.Config
 	cfg.WebSession = services.Identity.WebSessions()
 	cfg.WebToken = services.Identity.WebTokens()
 	cfg.WindowsDesktops = services.WindowsDesktops
+	cfg.DynamicWindowsDesktops = services.DynamicWindowsDesktops
 	cfg.AutoUpdateService = services.AutoUpdateService
 	cfg.ProvisioningStates = services.ProvisioningStates
 	cfg.IdentityCenter = services.IdentityCenter
@@ -2576,6 +2577,7 @@ func (process *TeleportProcess) newAccessCacheForClient(cfg accesspoint.Config,
 	cfg.WebSession = client.WebSessions()
 	cfg.WebToken = client.WebTokens()
 	cfg.WindowsDesktops = client
+	cfg.DynamicWindowsDesktops = client.DynamicDesktopClient()
 	cfg.AutoUpdateService = client
 
 	return accesspoint.NewCache(cfg)

diff --git a/lib/services/local/dynamic_desktops.go b/lib/services/local/dynamic_desktops.go
@@ -73,7 +73,11 @@ func (s *DynamicWindowsDesktopService) CreateDynamicWindowsDesktop(ctx context.C
 
 // UpdateDynamicWindowsDesktop updates a dynamic Windows desktop resource.
 func (s *DynamicWindowsDesktopService) UpdateDynamicWindowsDesktop(ctx context.Context, desktop types.DynamicWindowsDesktop) (types.DynamicWindowsDesktop, error) {
-	d, err := s.service.UpdateResource(ctx, desktop)
+	// ConditionalUpdateResource can return invalid revision instead of not found, so we'll check if resource exists first
+	if _, err := s.service.GetResource(ctx, desktop.GetName()); trace.IsNotFound(err) {
+		return nil, err
+	}
+	d, err := s.service.ConditionalUpdateResource(ctx, desktop)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}

diff --git a/lib/services/local/dynamic_desktops_test.go b/lib/services/local/dynamic_desktops_test.go
@@ -182,8 +182,15 @@ func TestDynamicWindowsService_UpdateDynamicDesktop(t *testing.T) {
 		require.Error(t, err)
 		require.True(t, trace.IsNotFound(err))
 	})
+	t.Run("revision doesn't match", func(t *testing.T) {
+		want := newDynamicDesktop(t, "example1")
+		_, err := service.CreateDynamicWindowsDesktop(ctx, want.Copy())
+		require.NoError(t, err)
+		_, err = service.UpdateDynamicWindowsDesktop(ctx, want)
+		require.Error(t, err)
+	})
 	t.Run("ok", func(t *testing.T) {
-		want := newDynamicDesktop(t, "example")
+		want := newDynamicDesktop(t, "example2")
 		created, err := service.CreateDynamicWindowsDesktop(ctx, want.Copy())
 		require.NoError(t, err)
 		updated, err := service.UpdateDynamicWindowsDesktop(ctx, created.Copy())

diff --git a/lib/srv/desktop/discovery_test.go b/lib/srv/desktop/discovery_test.go
@@ -279,7 +279,7 @@ func TestDynamicWindowsDiscovery(t *testing.T) {
 			}
 
 			desktop.Spec.Addr = "addr2"
-			_, err = dynamicWindowsClient.UpdateDynamicWindowsDesktop(ctx, desktop)
+			_, err = dynamicWindowsClient.UpsertDynamicWindowsDesktop(ctx, desktop)
 			require.NoError(t, err)
 
 			time.Sleep(10 * time.Millisecond)

diff --git a/tool/tctl/common/resource_command.go b/tool/tctl/common/resource_command.go
@@ -904,7 +904,7 @@ func (rc *ResourceCommand) createDynamicWindowsDesktop(ctx context.Context, clie
 			if !rc.force {
 				return trace.AlreadyExists("application %q already exists", wd.GetName())
 			}
-			if _, err := dynamicDesktopClient.UpdateDynamicWindowsDesktop(ctx, wd); err != nil {
+			if _, err := dynamicDesktopClient.UpsertDynamicWindowsDesktop(ctx, wd); err != nil {
 				return trace.Wrap(err)
 			}
 			fmt.Printf("dynamic windows desktop %q has been updated\n", wd.GetName())