From de348eda47817a6e19ce6aa1661d4d4d5ce4fed8 Mon Sep 17 00:00:00 2001 From: Przemko Robakowski Date: Mon, 28 Oct 2024 19:16:23 +0100 Subject: [PATCH] Fix update for DynamicWindowsDesktops --- api/client/dynamicwindows/dynamicwindows.go | 16 ++++++++++++ lib/auth/accesspoint/accesspoint.go | 2 ++ .../dynamicwindowsv1/service.go | 25 +++++++++++++++++++ .../dynamicwindowsv1/service_test.go | 9 +++++++ lib/cache/collections.go | 2 +- lib/service/service.go | 2 ++ lib/services/local/dynamic_desktops.go | 6 ++++- lib/services/local/dynamic_desktops_test.go | 9 ++++++- tool/tctl/common/resource_command.go | 2 +- 9 files changed, 69 insertions(+), 4 deletions(-) diff --git a/api/client/dynamicwindows/dynamicwindows.go b/api/client/dynamicwindows/dynamicwindows.go index 32ba1762f1aed..19d89e619fac8 100644 --- 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/lib/auth/accesspoint/accesspoint.go b/lib/auth/accesspoint/accesspoint.go index 5b0d4b6084f07..d9ac852bba65b 100644 --- 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 index 98bc2f81e6b55..5a42eefe8edca 100644 --- 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 index 8fee09af10dfb..1c474f7192be9 100644 --- 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/cache/collections.go b/lib/cache/collections.go index 17c28934a32c3..f7501f1bfdad8 100644 --- 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 index c171bb74ca301..090e792f2ded6 100644 --- 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 index b4b482d600de7..6254db2bd2a34 100644 --- 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 index 75ed040080648..9e20ed30e7eb2 100644 --- 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/tool/tctl/common/resource_command.go b/tool/tctl/common/resource_command.go index c37e8805581e6..32a07121b63ca 100644 --- 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())