From 083fb7700a064cd1f948b0e7191f47b98d11ab48 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Tue, 27 Aug 2024 16:56:26 -0400 Subject: [PATCH] Re-use cache for autoupdate service Add tests for resources command --- lib/auth/authclient/api.go | 3 + lib/auth/autoupdate/v1/service.go | 50 +++++++++----- lib/auth/grpcserver.go | 9 ++- lib/web/apiserver.go | 1 - tool/tctl/common/edit_command_test.go | 76 +++++++++++++++++++++ tool/tctl/common/resource_command_test.go | 81 +++++++++++++++++++++++ 6 files changed, 201 insertions(+), 19 deletions(-) diff --git a/lib/auth/authclient/api.go b/lib/auth/authclient/api.go index ca9ec09a81052..38c344f72a323 100644 --- a/lib/auth/authclient/api.go +++ b/lib/auth/authclient/api.go @@ -1183,6 +1183,9 @@ type Cache interface { // DatabaseObjectsGetter defines methods for fetching database objects. services.DatabaseObjectsGetter + // AutoupdateServiceGetter defines method for fetching the autoupdate config and version resources. + services.AutoupdateServiceGetter + // GetAccessGraphSettings returns the access graph settings. GetAccessGraphSettings(context.Context) (*clusterconfigpb.AccessGraphSettings, error) diff --git a/lib/auth/autoupdate/v1/service.go b/lib/auth/autoupdate/v1/service.go index 3ebca70445013..5d39baf340f4f 100644 --- a/lib/auth/autoupdate/v1/service.go +++ b/lib/auth/autoupdate/v1/service.go @@ -30,26 +30,46 @@ import ( "github.com/gravitational/teleport/lib/services" ) +// ServiceConfig holds configuration options for the autoupdate gRPC service. +type ServiceConfig struct { + // Authorizer is the authorizer used to check access to resources. + Authorizer authz.Authorizer + // Backend is the backend used to store autoupdate resources. + Backend services.AutoupdateService + // Cache is the cache used to store autoupdate resources. + Cache services.AutoupdateServiceGetter +} + // Service implements the gRPC API layer for the Autoupdate. type Service struct { // Opting out of forward compatibility, this service must implement all service methods. autoupdate.UnsafeAutoupdateServiceServer - storage services.AutoupdateService authorizer authz.Authorizer + backend services.AutoupdateService + cache services.AutoupdateServiceGetter } // NewService returns a new Autoupdate API service using the given storage layer and authorizer. -func NewService(storage services.AutoupdateService, authorizer authz.Authorizer) *Service { - return &Service{ - storage: storage, - authorizer: authorizer, +func NewService(cfg ServiceConfig) (*Service, error) { + switch { + case cfg.Backend == nil: + return nil, trace.BadParameter("backend is required") + case cfg.Authorizer == nil: + return nil, trace.BadParameter("authorizer is required") + case cfg.Cache == nil: + return nil, trace.BadParameter("cache is required") } + return &Service{ + authorizer: cfg.Authorizer, + backend: cfg.Backend, + cache: cfg.Cache, + }, nil } // GetAutoupdateConfig gets the current autoupdate config singleton. func (s *Service) GetAutoupdateConfig(ctx context.Context, req *autoupdate.GetAutoupdateConfigRequest) (*autoupdate.AutoupdateConfig, error) { - config, err := s.storage.GetAutoupdateConfig(ctx) + config, err := s.cache.GetAutoupdateConfig(ctx) if err != nil { return nil, trace.Wrap(err) } @@ -72,7 +92,7 @@ func (s *Service) CreateAutoupdateConfig(ctx context.Context, req *autoupdate.Cr return nil, trace.Wrap(err) } - config, err := s.storage.CreateAutoupdateConfig(ctx, req.Config) + config, err := s.backend.CreateAutoupdateConfig(ctx, req.Config) return config, trace.Wrap(err) } @@ -91,7 +111,7 @@ func (s *Service) UpdateAutoupdateConfig(ctx context.Context, req *autoupdate.Up return nil, trace.Wrap(err) } - config, err := s.storage.UpdateAutoupdateConfig(ctx, req.Config) + config, err := s.backend.UpdateAutoupdateConfig(ctx, req.Config) return config, trace.Wrap(err) } @@ -110,7 +130,7 @@ func (s *Service) UpsertAutoupdateConfig(ctx context.Context, req *autoupdate.Up return nil, trace.Wrap(err) } - config, err := s.storage.UpsertAutoupdateConfig(ctx, req.Config) + config, err := s.backend.UpsertAutoupdateConfig(ctx, req.Config) return config, trace.Wrap(err) } @@ -129,7 +149,7 @@ func (s *Service) DeleteAutoupdateConfig(ctx context.Context, req *autoupdate.De return nil, trace.Wrap(err) } - if err := s.storage.DeleteAutoupdateConfig(ctx); err != nil { + if err := s.backend.DeleteAutoupdateConfig(ctx); err != nil { return nil, trace.Wrap(err) } return &emptypb.Empty{}, nil @@ -137,7 +157,7 @@ func (s *Service) DeleteAutoupdateConfig(ctx context.Context, req *autoupdate.De // GetAutoupdateVersion gets the current autoupdate version singleton. func (s *Service) GetAutoupdateVersion(ctx context.Context, req *autoupdate.GetAutoupdateVersionRequest) (*autoupdate.AutoupdateVersion, error) { - version, err := s.storage.GetAutoupdateVersion(ctx) + version, err := s.cache.GetAutoupdateVersion(ctx) if err != nil { return nil, trace.Wrap(err) } @@ -160,7 +180,7 @@ func (s *Service) CreateAutoupdateVersion(ctx context.Context, req *autoupdate.C return nil, trace.Wrap(err) } - autoupdateVersion, err := s.storage.CreateAutoupdateVersion(ctx, req.Version) + autoupdateVersion, err := s.backend.CreateAutoupdateVersion(ctx, req.Version) return autoupdateVersion, trace.Wrap(err) } @@ -179,7 +199,7 @@ func (s *Service) UpdateAutoupdateVersion(ctx context.Context, req *autoupdate.U return nil, trace.Wrap(err) } - autoupdateVersion, err := s.storage.UpdateAutoupdateVersion(ctx, req.Version) + autoupdateVersion, err := s.backend.UpdateAutoupdateVersion(ctx, req.Version) return autoupdateVersion, trace.Wrap(err) } @@ -198,7 +218,7 @@ func (s *Service) UpsertAutoupdateVersion(ctx context.Context, req *autoupdate.U return nil, trace.Wrap(err) } - autoupdateVersion, err := s.storage.UpsertAutoupdateVersion(ctx, req.Version) + autoupdateVersion, err := s.backend.UpsertAutoupdateVersion(ctx, req.Version) return autoupdateVersion, trace.Wrap(err) } @@ -217,7 +237,7 @@ func (s *Service) DeleteAutoupdateVersion(ctx context.Context, req *autoupdate.D return nil, trace.Wrap(err) } - if err := s.storage.DeleteAutoupdateVersion(ctx); err != nil { + if err := s.backend.DeleteAutoupdateVersion(ctx); err != nil { return nil, trace.Wrap(err) } return &emptypb.Empty{}, nil diff --git a/lib/auth/grpcserver.go b/lib/auth/grpcserver.go index 3bddd4e91f017..aeb577d931404 100644 --- a/lib/auth/grpcserver.go +++ b/lib/auth/grpcserver.go @@ -79,7 +79,7 @@ import ( "github.com/gravitational/teleport/api/types/wrappers" "github.com/gravitational/teleport/lib/accessmonitoringrules/accessmonitoringrulesv1" "github.com/gravitational/teleport/lib/auth/authclient" - autoupd "github.com/gravitational/teleport/lib/auth/autoupdate/v1" + autoupdatev1 "github.com/gravitational/teleport/lib/auth/autoupdate/v1" "github.com/gravitational/teleport/lib/auth/clusterconfig/clusterconfigv1" "github.com/gravitational/teleport/lib/auth/crownjewel/crownjewelv1" "github.com/gravitational/teleport/lib/auth/dbobject/dbobjectv1" @@ -5435,11 +5435,14 @@ func NewGRPCServer(cfg GRPCServerConfig) (*GRPCServer, error) { } userprovisioningpb.RegisterStaticHostUsersServiceServer(server, staticHostUserServer) - autoupdateStorage, err := local.NewAutoupdateService(cfg.AuthServer.bk) + autoupdateServiceServer, err := autoupdatev1.NewService(autoupdatev1.ServiceConfig{ + Authorizer: cfg.Authorizer, + Backend: cfg.AuthServer.Services, + Cache: cfg.AuthServer.Cache, + }) if err != nil { return nil, trace.Wrap(err) } - autoupdateServiceServer := autoupd.NewService(autoupdateStorage, cfg.Authorizer) autoupdate.RegisterAutoupdateServiceServer(server, autoupdateServiceServer) // Only register the service if this is an open source build. Enterprise builds diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index ad87a73f4c5a2..6c7ec85b0e72e 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -1448,7 +1448,6 @@ func (h *Handler) find(w http.ResponseWriter, r *http.Request, p httprouter.Para if err != nil { return nil, trace.Wrap(err) } - return webclient.PingResponse{ Proxy: *proxyConfig, ServerVersion: teleport.Version, diff --git a/tool/tctl/common/edit_command_test.go b/tool/tctl/common/edit_command_test.go index b42517ac70382..e374266218d9a 100644 --- a/tool/tctl/common/edit_command_test.go +++ b/tool/tctl/common/edit_command_test.go @@ -30,7 +30,9 @@ import ( "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/constants" + autoupdatepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/types/autoupdate" "github.com/gravitational/teleport/entitlements" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/backend" @@ -73,6 +75,14 @@ func TestEditResources(t *testing.T) { kind: types.KindSessionRecordingConfig, edit: testEditSessionRecordingConfig, }, + { + kind: types.KindAutoupdateConfig, + edit: testEditAutoupdateConfig, + }, + { + kind: types.KindAutoupdateVersion, + edit: testEditAutoupdateVersion, + }, } for _, test := range tests { @@ -485,3 +495,69 @@ func testEditSAMLConnector(t *testing.T, clt *authclient.Client) { assert.Error(t, err, "stale connector was allowed to be updated") require.ErrorIs(t, err, backend.ErrIncorrectRevision, "expected an incorrect revision error, got %T", err) } + +func testEditAutoupdateConfig(t *testing.T, clt *authclient.Client) { + ctx := context.Background() + + expected, err := autoupdate.NewAutoupdateConfig(&autoupdatepb.AutoupdateConfigSpec{ToolsAutoupdate: true}) + require.NoError(t, err) + + initial, err := autoupdate.NewAutoupdateConfig(&autoupdatepb.AutoupdateConfigSpec{ToolsAutoupdate: false}) + require.NoError(t, err) + + _, err = clt.AutoupdateServiceClient().CreateAutoupdateConfig(ctx, initial) + require.NoError(t, err, "creating initial autoupdate config") + + editor := func(name string) error { + f, err := os.Create(name) + if err != nil { + return trace.Wrap(err, "opening file to edit") + } + expected.GetMetadata().Revision = initial.GetMetadata().GetRevision() + collection := &autoupdateConfigCollection{config: expected} + return trace.NewAggregate(writeYAML(collection, f), f.Close()) + } + + // Edit the autoupdate configuration resource. + _, err = runEditCommand(t, clt, []string{"edit", "autoupdate_config"}, withEditor(editor)) + require.NoError(t, err, "expected editing autoupdate config to succeed") + + actual, err := clt.GetAutoupdateConfig(ctx) + require.NoError(t, err, "failed to get autoupdate config after edit") + assert.NotEqual(t, initial.GetSpec().GetToolsAutoupdate(), actual.GetSpec().GetToolsAutoupdate(), + "tools_autoupdate should have been modified by edit") + assert.Equal(t, expected.GetSpec().GetToolsAutoupdate(), actual.GetSpec().GetToolsAutoupdate()) +} + +func testEditAutoupdateVersion(t *testing.T, clt *authclient.Client) { + ctx := context.Background() + + expected, err := autoupdate.NewAutoupdateVersion(&autoupdatepb.AutoupdateVersionSpec{ToolsVersion: "3.2.1"}) + require.NoError(t, err) + + initial, err := autoupdate.NewAutoupdateVersion(&autoupdatepb.AutoupdateVersionSpec{ToolsVersion: "1.2.3"}) + require.NoError(t, err) + + _, err = clt.AutoupdateServiceClient().CreateAutoupdateVersion(ctx, initial) + require.NoError(t, err, "creating initial autoupdate version") + + editor := func(name string) error { + f, err := os.Create(name) + if err != nil { + return trace.Wrap(err, "opening file to edit") + } + expected.GetMetadata().Revision = initial.GetMetadata().GetRevision() + collection := &autoupdateVersionCollection{version: expected} + return trace.NewAggregate(writeYAML(collection, f), f.Close()) + } + + // Edit the autoupdate version resource. + _, err = runEditCommand(t, clt, []string{"edit", "autoupdate_version"}, withEditor(editor)) + require.NoError(t, err, "expected editing autoupdate version to succeed") + + actual, err := clt.GetAutoupdateVersion(ctx) + require.NoError(t, err, "failed to get autoupdate version after edit") + assert.NotEqual(t, initial.GetSpec().GetToolsVersion(), actual.GetSpec().GetToolsVersion(), + "tools_autoupdate should have been modified by edit") + assert.Equal(t, expected.GetSpec().GetToolsVersion(), actual.GetSpec().GetToolsVersion()) +} diff --git a/tool/tctl/common/resource_command_test.go b/tool/tctl/common/resource_command_test.go index 43002008f28d1..d092e731d0acf 100644 --- a/tool/tctl/common/resource_command_test.go +++ b/tool/tctl/common/resource_command_test.go @@ -41,6 +41,7 @@ import ( "github.com/gravitational/teleport/api/constants" apidefaults "github.com/gravitational/teleport/api/defaults" + "github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1" headerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/header/v1" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/types/discoveryconfig" @@ -1405,6 +1406,14 @@ func TestCreateResources(t *testing.T) { kind: types.KindAppServer, create: testCreateAppServer, }, + { + kind: types.KindAutoupdateConfig, + create: testCreateAutoupdateConfig, + }, + { + kind: types.KindAutoupdateVersion, + create: testCreateAutoupdateVersion, + }, } for _, test := range tests { @@ -2205,6 +2214,78 @@ spec: require.NoError(t, err) } +func testCreateAutoupdateConfig(t *testing.T, clt *authclient.Client) { + const resourceYAML = `kind: autoupdate_config +metadata: + name: autoupdate-config + revision: 3a43b44a-201e-4d7f-aef1-ae2f6d9811ed +spec: + tools_autoupdate: true +version: v1 +` + _, err := runResourceCommand(t, clt, []string{"get", types.KindAutoupdateConfig, "--format=json"}) + require.ErrorContains(t, err, "doesn't exist") + + // Create the resource. + resourceYAMLPath := filepath.Join(t.TempDir(), "resource.yaml") + require.NoError(t, os.WriteFile(resourceYAMLPath, []byte(resourceYAML), 0644)) + _, err = runResourceCommand(t, clt, []string{"create", resourceYAMLPath}) + require.NoError(t, err) + + // Get the resource + buf, err := runResourceCommand(t, clt, []string{"get", types.KindAutoupdateConfig, "--format=json"}) + require.NoError(t, err) + resources := mustDecodeJSON[[]autoupdate.AutoupdateConfig](t, buf) + require.Len(t, resources, 1) + + // Compare with baseline + cmpOpts := []cmp.Option{ + protocmp.IgnoreFields(&headerv1.Metadata{}, "revision", "expires"), + protocmp.Transform(), + } + + var expected autoupdate.AutoupdateConfig + require.NoError(t, yaml.Unmarshal([]byte(resourceYAML), &expected)) + + require.Equal(t, "", cmp.Diff(expected, resources[0], cmpOpts...)) +} + +func testCreateAutoupdateVersion(t *testing.T, clt *authclient.Client) { + const resourceYAML = `kind: autoupdate_version +metadata: + name: autoupdate-version + revision: 3a43b44a-201e-4d7f-aef1-ae2f6d9811ed +spec: + tools_version: 1.2.3 +version: v1 +` + _, err := runResourceCommand(t, clt, []string{"get", types.KindAutoupdateVersion, "--format=json"}) + require.ErrorContains(t, err, "doesn't exist") + + // Create the resource. + resourceYAMLPath := filepath.Join(t.TempDir(), "resource.yaml") + require.NoError(t, os.WriteFile(resourceYAMLPath, []byte(resourceYAML), 0644)) + _, err = runResourceCommand(t, clt, []string{"create", resourceYAMLPath}) + require.NoError(t, err) + + // Get the resource + buf, err := runResourceCommand(t, clt, []string{"get", types.KindAutoupdateVersion, "--format=json"}) + require.NoError(t, err) + resources := mustDecodeJSON[[]autoupdate.AutoupdateVersion](t, buf) + require.Len(t, resources, 1) + + // Compare with baseline + cmpOpts := []cmp.Option{ + protocmp.IgnoreFields(&headerv1.Metadata{}, "revision", "expires"), + protocmp.Transform(), + } + + var expected autoupdate.AutoupdateVersion + require.NoError(t, yaml.Unmarshal([]byte(resourceYAML), &expected)) + + require.Equal(t, "", cmp.Diff(expected, resources[0], cmpOpts...)) +} + func TestPluginResourceWrapper(t *testing.T) { tests := []struct { name string