diff --git a/lib/auth/apiserver.go b/lib/auth/apiserver.go index 4f8050beddb..ddd689645fb 100644 --- a/lib/auth/apiserver.go +++ b/lib/auth/apiserver.go @@ -132,12 +132,6 @@ func NewAPIServer(config *APIConfig) (http.Handler, error) { srv.DELETE("/:version/tunnelconnections/:cluster", srv.WithAuth(srv.deleteTunnelConnections)) srv.DELETE("/:version/tunnelconnections", srv.WithAuth(srv.deleteAllTunnelConnections)) - // Reverse tunnels - // TODO(noah): DELETE IN 18.0.0 - all these methods are now gRPC. - srv.POST("/:version/reversetunnels", srv.WithAuth(srv.upsertReverseTunnel)) - srv.GET("/:version/reversetunnels", srv.WithAuth(srv.getReverseTunnels)) - srv.DELETE("/:version/reversetunnels/:domain", srv.WithAuth(srv.deleteReverseTunnel)) - // trusted clusters srv.POST("/:version/trustedclusters/validate", srv.WithAuth(srv.validateTrustedCluster)) @@ -154,6 +148,11 @@ func NewAPIServer(config *APIConfig) (http.Handler, error) { // SSO validation handlers srv.POST("/:version/github/requests/validate", srv.WithAuth(srv.validateGithubAuthCallback)) + // Migrated/deleted endpoints with 501 Not Implemented handlers. + srv.POST("/:version/reversetunnels", httpMigratedHandler) + srv.GET("/:version/reversetunnels", httpMigratedHandler) + srv.DELETE("/:version/reversetunnels/:domain", httpMigratedHandler) + if config.PluginRegistry != nil { if err := config.PluginRegistry.RegisterAuthWebHandlers(&srv); err != nil { return nil, trace.Wrap(err) @@ -167,6 +166,16 @@ func NewAPIServer(config *APIConfig) (http.Handler, error) { ), nil } +// httpMigratedHandler returns a 501 Not Implemented error. This is used when +// removing a HTTP handler that has been migrated to gRPC, where, a 404 NotFound +// could be misinterpreted as the resource not existing, rather than the RPC +// no longer existing. +var httpMigratedHandler = httplib.MakeHandler(func( + w http.ResponseWriter, r *http.Request, p httprouter.Params, +) (any, error) { + return nil, trace.NotImplemented("HTTP RPC migrated to gRPC, check client version") +}) + // HandlerWithAuthFunc is http handler with passed auth context type HandlerWithAuthFunc func(auth *ServerWithRoles, w http.ResponseWriter, r *http.Request, p httprouter.Params, version string) (interface{}, error) @@ -329,63 +338,6 @@ func marshalServers(servers []types.Server, version string) (interface{}, error) return items, nil } -type upsertReverseTunnelRawReq struct { - ReverseTunnel json.RawMessage `json:"reverse_tunnel"` - TTL time.Duration `json:"ttl"` -} - -// upsertReverseTunnel is called by admin to create a reverse tunnel to remote proxy -// TODO(noah): DELETE IN 18.0.0 - all these methods are now gRPC. -func (s *APIServer) upsertReverseTunnel(auth *ServerWithRoles, w http.ResponseWriter, r *http.Request, p httprouter.Params, version string) (interface{}, error) { - var req upsertReverseTunnelRawReq - if err := httplib.ReadJSON(r, &req); err != nil { - return nil, trace.Wrap(err) - } - tun, err := services.UnmarshalReverseTunnel(req.ReverseTunnel) - if err != nil { - return nil, trace.Wrap(err) - } - if err := services.ValidateReverseTunnel(tun); err != nil { - return nil, trace.Wrap(err) - } - if req.TTL != 0 { - tun.SetExpiry(s.Now().UTC().Add(req.TTL)) - } - if err := auth.UpsertReverseTunnel(r.Context(), tun); err != nil { - return nil, trace.Wrap(err) - } - return message("ok"), nil -} - -// getReverseTunnels returns a list of reverse tunnels -// TODO(noah): DELETE IN 18.0.0 - all these methods are now gRPC. -func (s *APIServer) getReverseTunnels(auth *ServerWithRoles, w http.ResponseWriter, r *http.Request, p httprouter.Params, version string) (interface{}, error) { - reverseTunnels, err := auth.GetReverseTunnels(r.Context()) - if err != nil { - return nil, trace.Wrap(err) - } - items := make([]json.RawMessage, len(reverseTunnels)) - for i, tunnel := range reverseTunnels { - data, err := services.MarshalReverseTunnel(tunnel, services.WithVersion(version), services.PreserveRevision()) - if err != nil { - return nil, trace.Wrap(err) - } - items[i] = data - } - return items, nil -} - -// deleteReverseTunnel deletes reverse tunnel -// TODO(noah): DELETE IN 18.0.0 - all these methods are now gRPC. -func (s *APIServer) deleteReverseTunnel(auth *ServerWithRoles, w http.ResponseWriter, r *http.Request, p httprouter.Params, version string) (interface{}, error) { - domainName := p.ByName("domain") - err := auth.DeleteReverseTunnel(r.Context(), domainName) - if err != nil { - return nil, trace.Wrap(err) - } - return message(fmt.Sprintf("reverse tunnel %v deleted", domainName)), nil -} - func (s *APIServer) validateTrustedCluster(auth *ServerWithRoles, w http.ResponseWriter, r *http.Request, p httprouter.Params, version string) (interface{}, error) { var validateRequestRaw authclient.ValidateTrustedClusterRequestRaw if err := httplib.ReadJSON(r, &validateRequestRaw); err != nil { diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 7012ee9f8e1..5aa578e3807 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -2247,30 +2247,6 @@ func (a *ServerWithRoles) DeleteProxy(ctx context.Context, name string) error { return a.authServer.DeleteProxy(ctx, name) } -// TODO(noah): DELETE IN 18.0.0 - all these methods are now gRPC. -func (a *ServerWithRoles) UpsertReverseTunnel(ctx context.Context, r types.ReverseTunnel) error { - if err := a.action(types.KindReverseTunnel, types.VerbCreate, types.VerbUpdate); err != nil { - return trace.Wrap(err) - } - return a.authServer.UpsertReverseTunnel(ctx, r) -} - -// TODO(noah): DELETE IN 18.0.0 - all these methods are now gRPC. -func (a *ServerWithRoles) GetReverseTunnels(ctx context.Context, opts ...services.MarshalOption) ([]types.ReverseTunnel, error) { - if err := a.action(types.KindReverseTunnel, types.VerbList, types.VerbRead); err != nil { - return nil, trace.Wrap(err) - } - return a.authServer.GetReverseTunnels(ctx) -} - -// TODO(noah): DELETE IN 18.0.0 - all these methods are now gRPC. -func (a *ServerWithRoles) DeleteReverseTunnel(ctx context.Context, domainName string) error { - if err := a.action(types.KindReverseTunnel, types.VerbDelete); err != nil { - return trace.Wrap(err) - } - return a.authServer.DeleteReverseTunnel(ctx, domainName) -} - func (a *ServerWithRoles) DeleteToken(ctx context.Context, token string) error { if err := a.action(types.KindToken, types.VerbDelete); err != nil { return trace.Wrap(err) diff --git a/lib/auth/authclient/api.go b/lib/auth/authclient/api.go index 1d3eda602f9..66e9631b6f7 100644 --- a/lib/auth/authclient/api.go +++ b/lib/auth/authclient/api.go @@ -215,8 +215,8 @@ type ReadProxyAccessPoint interface { // GetAuthServers returns a list of auth servers registered in the cluster GetAuthServers() ([]types.Server, error) - // GetReverseTunnels returns a list of reverse tunnels - GetReverseTunnels(ctx context.Context) ([]types.ReverseTunnel, error) + // ListReverseTunnels returns a list of reverse tunnels with pagination. + ListReverseTunnels(ctx context.Context, pageSize int, nextToken string) ([]types.ReverseTunnel, string, error) // GetAllTunnelConnections returns all tunnel connections GetAllTunnelConnections(opts ...services.MarshalOption) ([]types.TunnelConnection, error) @@ -382,9 +382,6 @@ type ReadRemoteProxyAccessPoint interface { // GetAuthServers returns a list of auth servers registered in the cluster GetAuthServers() ([]types.Server, error) - // GetReverseTunnels returns a list of reverse tunnels - GetReverseTunnels(ctx context.Context) ([]types.ReverseTunnel, error) - // GetAllTunnelConnections returns all tunnel connections GetAllTunnelConnections(opts ...services.MarshalOption) ([]types.TunnelConnection, error) @@ -932,9 +929,6 @@ type Cache interface { // NewWatcher returns a new event watcher. NewWatcher(ctx context.Context, watch types.Watch) (types.Watcher, error) - // GetReverseTunnels returns a list of reverse tunnels - GetReverseTunnels(ctx context.Context) ([]types.ReverseTunnel, error) - // ListReverseTunnels returns a paginated list of reverse tunnels. ListReverseTunnels(ctx context.Context, pageSize int, pageToken string) ([]types.ReverseTunnel, string, error) diff --git a/lib/auth/authclient/httpfallback.go b/lib/auth/authclient/httpfallback.go index 58d665e12ba..7287e1ec923 100644 --- a/lib/auth/authclient/httpfallback.go +++ b/lib/auth/authclient/httpfallback.go @@ -18,111 +18,5 @@ package authclient -import ( - "context" - "encoding/json" - "net/url" - "strings" - - "github.com/gravitational/trace" - - "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/lib/services" -) - // httpfallback.go holds endpoints that have been converted to gRPC // but still need http fallback logic in the old client. - -// GetReverseTunnels returns the list of created reverse tunnels -// TODO(noah): DELETE IN 18.0.0 -func (c *Client) GetReverseTunnels(ctx context.Context) ([]types.ReverseTunnel, error) { - var rcs []types.ReverseTunnel - pageToken := "" - for { - page, nextToken, err := c.APIClient.ListReverseTunnels(ctx, 0, pageToken) - if err != nil { - if trace.IsNotImplemented(err) { - return c.getReverseTunnelsLegacy(ctx) - } - return nil, trace.Wrap(err) - } - rcs = append(rcs, page...) - if nextToken == "" { - return rcs, nil - } - pageToken = nextToken - } -} - -func (c *Client) getReverseTunnelsLegacy(ctx context.Context) ([]types.ReverseTunnel, error) { - out, err := c.Get(ctx, c.Endpoint("reversetunnels"), url.Values{}) - if err != nil { - return nil, trace.Wrap(err) - } - var items []json.RawMessage - if err := json.Unmarshal(out.Bytes(), &items); err != nil { - return nil, trace.Wrap(err) - } - tunnels := make([]types.ReverseTunnel, len(items)) - for i, raw := range items { - tunnel, err := services.UnmarshalReverseTunnel(raw) - if err != nil { - return nil, trace.Wrap(err) - } - tunnels[i] = tunnel - } - return tunnels, nil -} - -// UpsertReverseTunnel upserts a reverse tunnel -// TODO: DELETE IN 18.0.0 -func (c *Client) UpsertReverseTunnel(ctx context.Context, tunnel types.ReverseTunnel) error { - _, err := c.APIClient.UpsertReverseTunnel(ctx, tunnel) - if err == nil { - return nil - } - if !trace.IsNotImplemented(err) { - return trace.Wrap(err) - } - return c.upsertReverseTunnelLegacy(context.Background(), tunnel) -} - -type upsertReverseTunnelRawReq struct { - ReverseTunnel json.RawMessage `json:"reverse_tunnel"` -} - -func (c *Client) upsertReverseTunnelLegacy(ctx context.Context, tunnel types.ReverseTunnel) error { - data, err := services.MarshalReverseTunnel(tunnel) - if err != nil { - return trace.Wrap(err) - } - args := &upsertReverseTunnelRawReq{ - ReverseTunnel: data, - } - _, err = c.PostJSON(ctx, c.Endpoint("reversetunnels"), args) - return trace.Wrap(err) -} - -// DeleteReverseTunnel deletes reverse tunnel by name -// TODO(noah): DELETE IN 18.0.0 -func (c *Client) DeleteReverseTunnel(ctx context.Context, name string) error { - err := c.APIClient.DeleteReverseTunnel(ctx, name) - if err == nil { - return nil - } - if !trace.IsNotImplemented(err) { - return trace.Wrap(err) - } - return c.deleteReverseTunnelLegacy(ctx, name) -} - -func (c *Client) deleteReverseTunnelLegacy(ctx context.Context, domainName string) error { - // this is to avoid confusing error in case if domain empty for example - // HTTP route will fail producing generic not found error - // instead we catch the error here - if strings.TrimSpace(domainName) == "" { - return trace.BadParameter("empty domain name") - } - _, err := c.Delete(ctx, c.Endpoint("reversetunnels", domainName)) - return trace.Wrap(err) -} diff --git a/lib/auth/init.go b/lib/auth/init.go index 87d3400b4c7..1506dba5f15 100644 --- a/lib/auth/init.go +++ b/lib/auth/init.go @@ -462,7 +462,7 @@ func initCluster(ctx context.Context, cfg InitConfig, asrv *Server) error { } } for _, tunnel := range cfg.ReverseTunnels { - if err := asrv.UpsertReverseTunnel(ctx, tunnel); err != nil { + if _, err := asrv.UpsertReverseTunnel(ctx, tunnel); err != nil { return trace.Wrap(err) } asrv.logger.InfoContext(ctx, "Created reverse tunnel", "tunnel", tunnel.GetName()) diff --git a/lib/auth/presence/presencev1/service.go b/lib/auth/presence/presencev1/service.go index 3eb28af2e74..f16b2191325 100644 --- a/lib/auth/presence/presencev1/service.go +++ b/lib/auth/presence/presencev1/service.go @@ -44,7 +44,7 @@ type Backend interface { UpdateRemoteCluster(ctx context.Context, rc types.RemoteCluster) (types.RemoteCluster, error) PatchRemoteCluster(ctx context.Context, name string, updateFn func(rc types.RemoteCluster) (types.RemoteCluster, error)) (types.RemoteCluster, error) - UpsertReverseTunnelV2(ctx context.Context, tunnel types.ReverseTunnel) (types.ReverseTunnel, error) + UpsertReverseTunnel(ctx context.Context, tunnel types.ReverseTunnel) (types.ReverseTunnel, error) DeleteReverseTunnel(ctx context.Context, tunnelName string) error } @@ -383,7 +383,7 @@ func (s *Service) UpsertReverseTunnel( return nil, trace.Wrap(err) } - res, err := s.backend.UpsertReverseTunnelV2(ctx, req.ReverseTunnel) + res, err := s.backend.UpsertReverseTunnel(ctx, req.ReverseTunnel) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/presence/presencev1/service_test.go b/lib/auth/presence/presencev1/service_test.go index 47259dbead0..13b61684efb 100644 --- a/lib/auth/presence/presencev1/service_test.go +++ b/lib/auth/presence/presencev1/service_test.go @@ -712,7 +712,7 @@ func TestListReverseTunnels(t *testing.T) { for i := 0; i < 10; i++ { rc, err := types.NewReverseTunnel(fmt.Sprintf("rt-%d", i), []string{"example.com:443"}) require.NoError(t, err) - err = srv.Auth().Services.UpsertReverseTunnel(ctx, rc) + _, err = srv.Auth().Services.UpsertReverseTunnel(ctx, rc) require.NoError(t, err) created = append(created, rc.(*types.ReverseTunnelV2)) } @@ -822,7 +822,7 @@ func TestDeleteReverseTunnel(t *testing.T) { rt, err := types.NewReverseTunnel("example.com", []string{"example.com:443"}) require.NoError(t, err) - rt, err = srv.Auth().UpsertReverseTunnelV2(ctx, rt) + rt, err = srv.Auth().UpsertReverseTunnel(ctx, rt) require.NoError(t, err) tests := []struct { diff --git a/lib/auth/tls_test.go b/lib/auth/tls_test.go index c0db8566be1..b95f51b30ba 100644 --- a/lib/auth/tls_test.go +++ b/lib/auth/tls_test.go @@ -1478,20 +1478,6 @@ func TestAppServerCRUD(t *testing.T) { suite.AppServerCRUD(t) } -func TestReverseTunnelsCRUD(t *testing.T) { - t.Parallel() - - testSrv := newTestTLSServer(t) - - clt, err := testSrv.NewClient(TestAdmin()) - require.NoError(t, err) - - suite := &suite.ServicesTestSuite{ - PresenceS: clt, - } - suite.ReverseTunnelsCRUD(t) -} - func TestUsersCRUD(t *testing.T) { t.Parallel() diff --git a/lib/auth/trustedcluster.go b/lib/auth/trustedcluster.go index eb23df58771..b41d11a79f1 100644 --- a/lib/auth/trustedcluster.go +++ b/lib/auth/trustedcluster.go @@ -755,5 +755,6 @@ func (a *Server) createReverseTunnel(ctx context.Context, t types.TrustedCluster if err != nil { return trace.Wrap(err) } - return trace.Wrap(a.UpsertReverseTunnel(ctx, reverseTunnel)) + _, err = a.UpsertReverseTunnel(ctx, reverseTunnel) + return trace.Wrap(err) } diff --git a/lib/cache/resource_reverse_tunnel.go b/lib/cache/resource_reverse_tunnel.go index 34e963ca970..796d26b1270 100644 --- a/lib/cache/resource_reverse_tunnel.go +++ b/lib/cache/resource_reverse_tunnel.go @@ -25,20 +25,6 @@ import ( "github.com/gravitational/teleport/api/types" ) -// GetReverseTunnels is a part of auth.Cache implementation -// Deprecated: use ListReverseTunnels -func (c *Cache) GetReverseTunnels(ctx context.Context) ([]types.ReverseTunnel, error) { - ctx, span := c.Tracer.Start(ctx, "cache/GetReverseTunnels") - defer span.End() - - rg, err := readCollectionCache(c, c.collections.reverseTunnels) - if err != nil { - return nil, trace.Wrap(err) - } - defer rg.Release() - return rg.reader.GetReverseTunnels(ctx) -} - // ListReverseTunnels is a part of auth.Cache implementation func (c *Cache) ListReverseTunnels(ctx context.Context, pageSize int, pageToken string) ([]types.ReverseTunnel, string, error) { ctx, span := c.Tracer.Start(ctx, "cache/ListReverseTunnels") @@ -53,7 +39,6 @@ func (c *Cache) ListReverseTunnels(ctx context.Context, pageSize int, pageToken } type reverseTunnelGetter interface { - GetReverseTunnels(ctx context.Context) ([]types.ReverseTunnel, error) ListReverseTunnels(ctx context.Context, pageSize int, pageToken string) ([]types.ReverseTunnel, string, error) } @@ -62,11 +47,28 @@ var _ executor[types.ReverseTunnel, reverseTunnelGetter] = reverseTunnelExecutor type reverseTunnelExecutor struct{} func (reverseTunnelExecutor) getAll(ctx context.Context, cache *Cache, loadSecrets bool) ([]types.ReverseTunnel, error) { - return cache.Presence.GetReverseTunnels(ctx) + var out []types.ReverseTunnel + var nextToken string + for { + var page []types.ReverseTunnel + var err error + + const defaultPageSize = 0 + page, nextToken, err = cache.Presence.ListReverseTunnels(ctx, defaultPageSize, nextToken) + if err != nil { + return nil, trace.Wrap(err) + } + out = append(out, page...) + if nextToken == "" { + break + } + } + return out, nil } func (reverseTunnelExecutor) upsert(ctx context.Context, cache *Cache, resource types.ReverseTunnel) error { - return cache.presenceCache.UpsertReverseTunnel(ctx, resource) + _, err := cache.presenceCache.UpsertReverseTunnel(ctx, resource) + return err } func (reverseTunnelExecutor) deleteAll(ctx context.Context, cache *Cache) error { diff --git a/lib/cache/resource_reverse_tunnel_test.go b/lib/cache/resource_reverse_tunnel_test.go index a228cb3be48..ba75c2ad797 100644 --- a/lib/cache/resource_reverse_tunnel_test.go +++ b/lib/cache/resource_reverse_tunnel_test.go @@ -20,6 +20,7 @@ import ( "context" "testing" + "github.com/gravitational/trace" "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/types" @@ -37,14 +38,23 @@ func TestReverseTunnels(t *testing.T) { newResource: func(name string) (types.ReverseTunnel, error) { return types.NewReverseTunnel(name, []string{"example.com:2023"}) }, - create: p.presenceS.UpsertReverseTunnel, + create: func(ctx context.Context, tunnel types.ReverseTunnel) error { + _, err := p.presenceS.UpsertReverseTunnel(ctx, tunnel) + return err + }, list: func(ctx context.Context) ([]types.ReverseTunnel, error) { - return p.presenceS.GetReverseTunnels(ctx) + items, _, err := p.presenceS.ListReverseTunnels(ctx, 0, "") + return items, trace.Wrap(err) }, - cacheList: func(ctx context.Context) ([]types.ReverseTunnel, error) { - return p.cache.GetReverseTunnels(ctx) + update: func(ctx context.Context, tunnel types.ReverseTunnel) error { + _, err := p.presenceS.UpsertReverseTunnel(ctx, tunnel) + return err }, - update: p.presenceS.UpsertReverseTunnel, deleteAll: p.presenceS.DeleteAllReverseTunnels, + + cacheList: func(ctx context.Context) ([]types.ReverseTunnel, error) { + items, _, err := p.cache.ListReverseTunnels(ctx, 0, "") + return items, trace.Wrap(err) + }, }) } diff --git a/lib/reversetunnel/rc_manager.go b/lib/reversetunnel/rc_manager.go index f1e539ac3bf..2dbe72d73e1 100644 --- a/lib/reversetunnel/rc_manager.go +++ b/lib/reversetunnel/rc_manager.go @@ -173,12 +173,32 @@ func (w *RemoteClusterTunnelManager) Run(ctx context.Context) { } } +func (w *RemoteClusterTunnelManager) listAllReverseTunnels(ctx context.Context) ([]apitypes.ReverseTunnel, error) { + var out []apitypes.ReverseTunnel + var nextToken string + for { + var page []apitypes.ReverseTunnel + var err error + + const defaultPageSize = 0 + page, nextToken, err = w.cfg.AccessPoint.ListReverseTunnels(ctx, defaultPageSize, nextToken) + if err != nil { + return nil, trace.Wrap(err) + } + out = append(out, page...) + if nextToken == "" { + break + } + } + return out, nil +} + // Sync does a one-time sync of trusted clusters with running agent pools. // Non-test code should use Run() instead. func (w *RemoteClusterTunnelManager) Sync(ctx context.Context) error { // Fetch desired reverse tunnels and convert them to a set of // remoteClusterKeys. - wantTunnels, err := w.cfg.AccessPoint.GetReverseTunnels(ctx) + wantTunnels, err := w.listAllReverseTunnels(ctx) if err != nil { return trace.Wrap(err) } diff --git a/lib/reversetunnel/rc_manager_test.go b/lib/reversetunnel/rc_manager_test.go index 9fd1399ae79..0df6e92f9fc 100644 --- a/lib/reversetunnel/rc_manager_test.go +++ b/lib/reversetunnel/rc_manager_test.go @@ -188,8 +188,10 @@ type mockAuthClient struct { reverseTunnelsErr error } -func (c mockAuthClient) GetReverseTunnels(context.Context) ([]types.ReverseTunnel, error) { - return c.reverseTunnels, c.reverseTunnelsErr +func (c mockAuthClient) ListReverseTunnels( + ctx context.Context, pageSize int, token string, +) ([]types.ReverseTunnel, string, error) { + return c.reverseTunnels, "", c.reverseTunnelsErr } func mustNewReverseTunnel(t *testing.T, clusterName string, dialAddrs []string) types.ReverseTunnel { diff --git a/lib/services/local/presence.go b/lib/services/local/presence.go index e7934ccac42..93f398cc2be 100644 --- a/lib/services/local/presence.go +++ b/lib/services/local/presence.go @@ -373,16 +373,7 @@ func (s *PresenceService) DeleteAllReverseTunnels(ctx context.Context) error { } // UpsertReverseTunnel upserts reverse tunnel entry -func (s *PresenceService) UpsertReverseTunnel(ctx context.Context, tunnel types.ReverseTunnel) error { - _, err := s.UpsertReverseTunnelV2(ctx, tunnel) - return trace.Wrap(err) -} - -// UpsertReverseTunnelV2 upserts reverse tunnel entry and returns the upserted -// value. -// TODO(noah): In v18, we can rename this to UpsertReverseTunnel and remove the -// version which does not return the upserted value. -func (s *PresenceService) UpsertReverseTunnelV2(ctx context.Context, tunnel types.ReverseTunnel) (types.ReverseTunnel, error) { +func (s *PresenceService) UpsertReverseTunnel(ctx context.Context, tunnel types.ReverseTunnel) (types.ReverseTunnel, error) { if err := services.ValidateReverseTunnel(tunnel); err != nil { return nil, trace.Wrap(err) } @@ -418,35 +409,6 @@ func (s *PresenceService) GetReverseTunnel(ctx context.Context, name string) (ty ) } -// GetReverseTunnels returns a list of registered servers -// Deprecated: use ListReverseTunnels -// TODO(noah): REMOVE IN 18.0.0 - replace with calls to ListReverseTunnels -func (s *PresenceService) GetReverseTunnels(ctx context.Context) ([]types.ReverseTunnel, error) { - startKey := backend.ExactKey(reverseTunnelsPrefix) - result, err := s.GetRange(ctx, startKey, backend.RangeEnd(startKey), backend.NoLimit) - if err != nil { - return nil, trace.Wrap(err) - } - tunnels := make([]types.ReverseTunnel, len(result.Items)) - if len(result.Items) == 0 { - return tunnels, nil - } - for i, item := range result.Items { - tunnel, err := services.UnmarshalReverseTunnel( - item.Value, - services.WithExpires(item.Expires), - services.WithRevision(item.Revision), - ) - if err != nil { - return nil, trace.Wrap(err) - } - tunnels[i] = tunnel - } - // sorting helps with tests and makes it all deterministic - sort.Sort(services.SortedReverseTunnels(tunnels)) - return tunnels, nil -} - // DeleteReverseTunnel deletes reverse tunnel by its cluster name func (s *PresenceService) DeleteReverseTunnel(ctx context.Context, clusterName string) error { err := s.Delete(ctx, backend.NewKey(reverseTunnelsPrefix, clusterName)) diff --git a/lib/services/local/presence_test.go b/lib/services/local/presence_test.go index 7ed3d8ded71..d3ae308347f 100644 --- a/lib/services/local/presence_test.go +++ b/lib/services/local/presence_test.go @@ -1323,7 +1323,7 @@ func TestPresenceService_ListReverseTunnels(t *testing.T) { for i := 0; i < 10; i++ { rc, err := types.NewReverseTunnel(fmt.Sprintf("rt-%d", i), []string{"example.com:443"}) require.NoError(t, err) - err = presenceService.UpsertReverseTunnel(ctx, rc) + _, err = presenceService.UpsertReverseTunnel(ctx, rc) require.NoError(t, err) } @@ -1373,7 +1373,7 @@ func TestPresenceService_UpsertReverseTunnel(t *testing.T) { require.NoError(t, err) // Upsert a reverse tunnel - got, err := presenceService.UpsertReverseTunnelV2(ctx, rt) + got, err := presenceService.UpsertReverseTunnel(ctx, rt) require.NoError(t, err) // Check that the returned resource is the same as the one we upserted require.Empty(t, cmp.Diff(rt, got, cmpopts.IgnoreFields(types.Metadata{}, "Revision"))) diff --git a/lib/services/local/services_test.go b/lib/services/local/services_test.go index 2badd8a6c7f..842a92ea609 100644 --- a/lib/services/local/services_test.go +++ b/lib/services/local/services_test.go @@ -93,7 +93,6 @@ func TestCRUD(t *testing.T) { t.Run("TestUserCACRUD", tt.suite.CertAuthCRUD) t.Run("TestServerCRUD", tt.suite.ServerCRUD) t.Run("TestAppServerCRUD", tt.suite.AppServerCRUD) - t.Run("TestReverseTunnelsCRUD", tt.suite.ReverseTunnelsCRUD) t.Run("TestUsersCRUD", tt.suite.UsersCRUD) t.Run("TestUsersExpiry", tt.suite.UsersExpiry) t.Run("TestLoginAttempts", tt.suite.LoginAttempts) diff --git a/lib/services/presence.go b/lib/services/presence.go index ea760f31aa8..c3942618eb9 100644 --- a/lib/services/presence.go +++ b/lib/services/presence.go @@ -108,15 +108,11 @@ type Presence interface { DeleteAllProxies() error // UpsertReverseTunnel upserts reverse tunnel entry temporarily or permanently - UpsertReverseTunnel(ctx context.Context, tunnel types.ReverseTunnel) error + UpsertReverseTunnel(ctx context.Context, tunnel types.ReverseTunnel) (types.ReverseTunnel, error) // GetReverseTunnel returns reverse tunnel by name GetReverseTunnel(ctx context.Context, name string) (types.ReverseTunnel, error) - // GetReverseTunnels returns a list of registered servers - // Deprecated: use ListReverseTunnels - GetReverseTunnels(ctx context.Context) ([]types.ReverseTunnel, error) - // DeleteReverseTunnel deletes reverse tunnel by its domain name DeleteReverseTunnel(ctx context.Context, domainName string) error @@ -196,6 +192,5 @@ type PresenceInternal interface { UpsertHostUserInteractionTime(ctx context.Context, name string, loginTime time.Time) error GetHostUserInteractionTime(ctx context.Context, name string) (time.Time, error) - UpsertReverseTunnelV2(ctx context.Context, tunnel types.ReverseTunnel) (types.ReverseTunnel, error) UpdateNode(ctx context.Context, server types.Server) (types.Server, error) } diff --git a/lib/services/suite/suite.go b/lib/services/suite/suite.go index 7fc649b05eb..14475716dc6 100644 --- a/lib/services/suite/suite.go +++ b/lib/services/suite/suite.go @@ -612,38 +612,6 @@ func newReverseTunnel(clusterName string, dialAddrs []string) *types.ReverseTunn } } -func (s *ServicesTestSuite) ReverseTunnelsCRUD(t *testing.T) { - ctx := context.Background() - - out, err := s.PresenceS.GetReverseTunnels(ctx) - require.NoError(t, err) - require.Empty(t, out) - - tunnel := newReverseTunnel("example.com", []string{"example.com:2023"}) - require.NoError(t, s.PresenceS.UpsertReverseTunnel(ctx, tunnel)) - - out, err = s.PresenceS.GetReverseTunnels(ctx) - require.NoError(t, err) - require.Len(t, out, 1) - require.Empty(t, cmp.Diff(out, []types.ReverseTunnel{tunnel}, cmpopts.IgnoreFields(types.Metadata{}, "Revision"))) - - err = s.PresenceS.DeleteReverseTunnel(ctx, tunnel.Spec.ClusterName) - require.NoError(t, err) - - out, err = s.PresenceS.GetReverseTunnels(ctx) - require.NoError(t, err) - require.Empty(t, out) - - err = s.PresenceS.UpsertReverseTunnel(ctx, newReverseTunnel("", []string{"127.0.0.1:1234"})) - require.True(t, trace.IsBadParameter(err)) - - err = s.PresenceS.UpsertReverseTunnel(ctx, newReverseTunnel("example.com", []string{""})) - require.True(t, trace.IsBadParameter(err)) - - err = s.PresenceS.UpsertReverseTunnel(ctx, newReverseTunnel("example.com", []string{})) - require.True(t, trace.IsBadParameter(err)) -} - func (s *ServicesTestSuite) PasswordCRUD(t *testing.T) { ctx := context.Background() @@ -1846,9 +1814,12 @@ func (s *ServicesTestSuite) Events(t *testing.T) { }, crud: func(context.Context) types.Resource { tunnel := newReverseTunnel("example.com", []string{"example.com:2023"}) - require.NoError(t, s.PresenceS.UpsertReverseTunnel(ctx, tunnel)) + _, err := s.PresenceS.UpsertReverseTunnel(ctx, tunnel) + require.NoError(t, err) - out, err := s.PresenceS.GetReverseTunnels(context.Background()) + out, _, err := s.PresenceS.ListReverseTunnels( + ctx, 0, "", + ) require.NoError(t, err) err = s.PresenceS.DeleteReverseTunnel(ctx, tunnel.Spec.ClusterName) diff --git a/lib/services/tunnel_test.go b/lib/services/tunnel_test.go new file mode 100644 index 00000000000..82d4273408f --- /dev/null +++ b/lib/services/tunnel_test.go @@ -0,0 +1,84 @@ +/* + * Teleport + * Copyright (C) 2025 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package services + +import ( + "testing" + + "github.com/gravitational/trace" + "github.com/stretchr/testify/require" + + apidefaults "github.com/gravitational/teleport/api/defaults" + "github.com/gravitational/teleport/api/types" +) + +func newReverseTunnel(clusterName string, dialAddrs []string) *types.ReverseTunnelV2 { + return &types.ReverseTunnelV2{ + Kind: types.KindReverseTunnel, + Version: types.V2, + Metadata: types.Metadata{ + Name: clusterName, + Namespace: apidefaults.Namespace, + }, + Spec: types.ReverseTunnelSpecV2{ + ClusterName: clusterName, + DialAddrs: dialAddrs, + }, + } +} + +func TestValidateReverseTunnel(t *testing.T) { + tests := []struct { + name string + tunnel types.ReverseTunnel + requireErr require.ErrorAssertionFunc + }{ + { + name: "valid tunnel", + tunnel: newReverseTunnel("example.com", []string{"example.com:3022"}), + requireErr: require.NoError, + }, + { + name: "empty cluster name", + tunnel: newReverseTunnel("", []string{"example.com:3022"}), + requireErr: func(t require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsBadParameter(err)) + }, + }, + { + name: "empty dial address", + tunnel: newReverseTunnel("example.com", []string{""}), + requireErr: func(t require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsBadParameter(err)) + }, + }, + { + name: "no dial address", + tunnel: newReverseTunnel("example.com", []string{}), + requireErr: func(t require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsBadParameter(err)) + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.requireErr(t, ValidateReverseTunnel(tt.tunnel)) + }) + } +} diff --git a/tool/tctl/common/resource_command.go b/tool/tctl/common/resource_command.go index f255f0a371d..9d9f9645c01 100644 --- a/tool/tctl/common/resource_command.go +++ b/tool/tctl/common/resource_command.go @@ -2275,9 +2275,21 @@ func (rc *ResourceCommand) getCollection(ctx context.Context, client *authclient if rc.ref.Name != "" { return nil, trace.BadParameter("reverse tunnel cannot be searched by name") } - tunnels, err := client.GetReverseTunnels(ctx) - if err != nil { - return nil, trace.Wrap(err) + var tunnels []types.ReverseTunnel + var nextToken string + for { + var page []types.ReverseTunnel + var err error + + const defaultPageSize = 0 + page, nextToken, err = client.ListReverseTunnels(ctx, defaultPageSize, nextToken) + if err != nil { + return nil, trace.Wrap(err) + } + tunnels = append(tunnels, page...) + if nextToken == "" { + break + } } return &reverseTunnelCollection{tunnels: tunnels}, nil case types.KindCertAuthority: