Skip to content

Refactor lib/teleterm gateway #27685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
greedy52 wants to merge 1 commit into from
Closed

Refactor lib/teleterm gateway #27685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions integration/proxy/teleterm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/gravitational/teleport/lib/teleterm/api/uri"
"github.com/gravitational/teleport/lib/teleterm/clusters"
"github.com/gravitational/teleport/lib/teleterm/daemon"
"github.com/gravitational/teleport/lib/tlsca"
)

// testTeletermGatewaysCertRenewal is run from within TestALPNSNIProxyDatabaseAccess to amortize the
Expand Down Expand Up @@ -120,17 +121,23 @@ func testGatewayCertRenewal(t *testing.T, pack *dbhelpers.DatabasePack, albAddr
})

// Here the test setup ends and actual test code starts.

gateway, err := daemonService.CreateGateway(context.Background(), daemon.CreateGatewayParams{
TargetURI: databaseURI.String(),
TargetUser: "root",
})
require.NoError(t, err, trace.DebugReport(err))

routeToDatabase := tlsca.RouteToDatabase{
ServiceName: databaseURI.GetDbName(),
Protocol: "mysql",
Username: "root",
}

// Open a new connection.
client, err := mysql.MakeTestClientWithoutTLS(
net.JoinHostPort(gateway.LocalAddress(), gateway.LocalPort()),
gateway.RouteToDatabase())
routeToDatabase)

require.NoError(t, err)

// Execute a query.
Expand Down Expand Up @@ -160,7 +167,7 @@ func testGatewayCertRenewal(t *testing.T, pack *dbhelpers.DatabasePack, albAddr
// will let the connection through.
client, err = mysql.MakeTestClientWithoutTLS(
net.JoinHostPort(gateway.LocalAddress(), gateway.LocalPort()),
gateway.RouteToDatabase())
routeToDatabase)
require.NoError(t, err)

// Execute a query.
Expand Down
21 changes: 1 addition & 20 deletions lib/srv/alpnproxy/local_proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,26 +369,7 @@ func (l *LocalProxy) CheckDBCerts(dbRoute tlsca.RouteToDatabase) error {
return trace.Wrap(err)
}

return trace.Wrap(CheckCertSubject(cert, dbRoute))
}

// CheckCertSubject checks if the route to the database from the cert matches the provided route in
// terms of username and database (if present).
func CheckCertSubject(cert *x509.Certificate, dbRoute tlsca.RouteToDatabase) error {
identity, err := tlsca.FromSubject(cert.Subject, cert.NotAfter)
if err != nil {
return trace.Wrap(err)
}
if dbRoute.Username != "" && dbRoute.Username != identity.RouteToDatabase.Username {
return trace.Errorf("certificate subject is for user %s, but need %s",
identity.RouteToDatabase.Username, dbRoute.Username)
}
if dbRoute.Database != "" && dbRoute.Database != identity.RouteToDatabase.Database {
return trace.Errorf("certificate subject is for database name %s, but need %s",
identity.RouteToDatabase.Database, dbRoute.Database)
}

return nil
return trace.Wrap(dbRoute.CheckCertSubject(cert))
}

// SetCerts sets the local proxy's configured TLS certificates.
Expand Down
35 changes: 33 additions & 2 deletions lib/teleterm/clusters/cluster_databases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ package clusters

import (
"context"
"crypto/tls"

"github.com/gravitational/trace"

apiclient "github.com/gravitational/teleport/api/client"
"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/api/utils/keys"
api "github.com/gravitational/teleport/gen/proto/go/teleport/lib/teleterm/v1"
"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/client"
Expand All @@ -33,6 +35,7 @@ import (
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/teleterm/api/uri"
"github.com/gravitational/teleport/lib/tlsca"
"github.com/gravitational/teleport/lib/utils"
)

// Database describes database
Expand Down Expand Up @@ -150,8 +153,8 @@ func (c *Cluster) GetDatabases(ctx context.Context, r *api.GetDatabasesRequest)
return response, nil
}

// ReissueDBCerts issues new certificates for specific DB access and saves them to disk.
func (c *Cluster) ReissueDBCerts(ctx context.Context, routeToDatabase tlsca.RouteToDatabase) error {
// reissueDBCerts issues new certificates for specific DB access and saves them to disk.
func (c *Cluster) reissueDBCerts(ctx context.Context, routeToDatabase tlsca.RouteToDatabase) error {
// When generating certificate for MongoDB access, database username must
// be encoded into it. This is required to be able to tell which database
// user to authenticate the connection as.
Expand Down Expand Up @@ -197,6 +200,34 @@ func (c *Cluster) ReissueDBCerts(ctx context.Context, routeToDatabase tlsca.Rout

return nil
}
func (c *Cluster) loadDBCert(routeToDatabase tlsca.RouteToDatabase) (tls.Certificate, error) {
tlsCert, err := keys.LoadX509KeyPair(
c.status.DatabaseCertPathForCluster(c.clusterClient.SiteName, routeToDatabase.ServiceName),
c.status.KeyPath(),
)
if err != nil {
return tls.Certificate{}, trace.Wrap(err)
}

cert, err := utils.TLSCertLeaf(tlsCert)
if err != nil {
return tls.Certificate{}, trace.Wrap(err)
}

if err := routeToDatabase.CheckCertSubject(cert); err != nil {
return tls.Certificate{}, trace.Wrap(err, "database certificate check failed, try restarting the database connection")
}

return tlsCert, nil
}

func (c *Cluster) reissueAndLoadDBCert(ctx context.Context, routeToDatabase tlsca.RouteToDatabase) (tls.Certificate, error) {
if err := c.reissueDBCerts(ctx, routeToDatabase); err != nil {
return tls.Certificate{}, trace.Wrap(err)
}
tlsCert, err := c.loadDBCert(routeToDatabase)
Comment on lines +225 to +228
Copy link
Copy Markdown
Contributor

@smallinsky smallinsky Jun 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the LoadDBCert is always called after reissueDBCerts since the reissueDBCerts uses the same information to issue a new cert at first glance the routeToDatabase.CheckCertSubject validation inside loadDBCert function is not needed.

nit: Also instead of loadingCert from disc directly we can use the introduce a helper function for our local agent c.clusterClient.LocalAgent() that will be responsible for loading the cert.

Copy link
Copy Markdown
Member

@ravicious ravicious Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking the cert subject after calling reissueDBCerts was originally added because reissueDBCerts saves the cert to disk without returning it from the function. As such, we need to load the cert then and there's a small chance that between saving and loading, the cert on disk might be modified by e.g. tsh db login being ran from a separate shell session. See #18740.

After we switch from using ReissueUserCerts to IssueUserCertsWithMFA (which returns the cert), checking the cert subject will become unnecessary.

return tlsCert, trace.Wrap(err)
}

// GetAllowedDatabaseUsers returns allowed users for the given database based on the role set.
func (c *Cluster) GetAllowedDatabaseUsers(ctx context.Context, dbURI string) ([]string, error) {
Expand Down
46 changes: 39 additions & 7 deletions lib/teleterm/clusters/cluster_gateways.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,25 @@ package clusters

import (
"context"
"crypto/tls"

"github.com/gravitational/trace"

"github.com/gravitational/teleport/lib/client/db/dbcmd"
"github.com/gravitational/teleport/lib/teleterm/gateway"
"github.com/gravitational/teleport/lib/tlsca"
)

// ReissueCertFunc is a callback function for Cluster to actually do the issue
// of user certificates with TeleportClient.
type ReissueCertFunc func(context.Context) error

// GatewayCertReissuer defines an interface of a helper that manages the
// process of reissuing certificates.
type GatewayCertReissuer interface {
Copy link
Copy Markdown
Contributor

@smallinsky smallinsky Jun 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems that is used in scope of this package so we can make it enexported:

Suggested change
type GatewayCertReissuer interface {
type gatewayCertReissuer interface {

ReissueCert(ctx context.Context, gateway *gateway.Gateway, doReissueCert ReissueCertFunc) error
}

type CreateGatewayParams struct {
// TargetURI is the cluster resource URI
TargetURI string
Expand All @@ -36,12 +48,15 @@ type CreateGatewayParams struct {
// LocalPort is the gateway local port
LocalPort string
CLICommandProvider gateway.CLICommandProvider
TCPPortAllocator gateway.TCPPortAllocator
OnExpiredCert gateway.OnExpiredCertFunc
CertReissuer GatewayCertReissuer
Copy link
Copy Markdown
Contributor

@smallinsky smallinsky Jun 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: go docs.

}

// CreateGateway creates a gateway
func (c *Cluster) CreateGateway(ctx context.Context, params CreateGatewayParams) (*gateway.Gateway, error) {
if params.CLICommandProvider == nil {
params.CLICommandProvider = NewDbcmdCLICommandProvider(c, dbcmd.SystemExecer{})
Copy link
Copy Markdown
Member

@ravicious ravicious Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does moving DbcmdCLICommandProvider initialization from the daemon to the cluster help with adding kube gateways?

My original comment pointed that the creation of the CLI command provider should be moved up to outer layers, not further down.

Instead of creating a new struct for every gateway, we'd have only two CLI command provider structs, say DbcmdCLICommandProvider and KubeCLICommandProvider, both initialized before calling daemon.New and passed to that function as a field on Config. Then when CreateGateway on the daemon is called, we inspect the target URI of the gateway and pass a reference to an appropriate provider.

This goes back to how dependencies are typically injected in Go and how we should approach mocking them out in tests. I'm no expert on this, so by all means please push back on this if you disagree!

I'm not sure if any of this matters to be honest. Because of how clusters.Cluster is written, it is usually mocked out wholesale anyway. So with how things are right now, there won't be a situation where I want to use clusters.Cluster or daemon.Service but I want the CLICommandProvider to be mocked out – CLICommandProvider always passes through clusters.Cluster. As I mentiode, clusters.Cluster has to be mocked out itself due to how it's designed.

This is clearly demonstrated by daemon_test.go where you were able to shift passing a mocked out TCP port allocator to the mock gateway creator.

Copy link
Copy Markdown
Member

@ravicious ravicious Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think about it, DbcmdCLICommandProvider and KubeCLICommandProvider could be initialized within daemon.New. If DbcmdCLICommandProvider needs clusters.Storage for initialization, then daemon.New would simply pass the Config.Storage.

Copy link
Copy Markdown
Member

@ravicious ravicious Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked for some opinions on Slack regarding passing the deps up. https://gravitational.slack.com/archives/C08HF8E9F/p1686740351896489

Copy link
Copy Markdown
Contributor Author

@greedy52 greedy52 Jun 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, daemon.Service has to own CLICommandProvider only if the db command provider has to depend on storage so a fresh cluster is required. That may not be the case. Let me see what I can do on the DbcmdCLICommandProvider side to decouple the need for TeleportClient and ProfileStatus. If that's possible, cluster can be the one creating it and there is really no need for daemon.Service to know anything about the command provider.

Copy link
Copy Markdown
Contributor Author

@greedy52 greedy52 Jun 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I think the same problem applies to the way I am doing the reissue cert callback. Do we need the storage to get a fresh Cluster for reissuing the database cert?

Seems so right? so the current refactor on issue cert won't work. Let me rethink through this.

Copy link
Copy Markdown
Contributor Author

@greedy52 greedy52 Jun 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if we can just have a Cluster.Reload() function after relogin, or recreate the gateways after relogin.

Copy link
Copy Markdown
Member

@ravicious ravicious Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if a fresh cluster is needed for the db cert, but if we're to have Cluster.Reload, I wonder if it wouldn't be better to just pass the storage there. 🤔

AFAIK, TeleportClient was originally made for tsh which doesn't really have the concept of long-running processes, maybe other than proxies. Actually, I'd check how this those TeleportClient instances are managed in the current local proxy middlewares (or if TeleportClient is used there in the first place).

Copy link
Copy Markdown
Contributor Author

@greedy52 greedy52 Jun 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if a fresh cluster is needed for the db cert, but if we're to have Cluster.Reload, I wonder if it wouldn't be better to just pass the storage there. 🤔

I think it's safer to get a fresh cluster for reissuing db cert the same way a fresh cluster is used for DBCliCommandProvider. If so, I would say the current implemented way is better so refactoring is probably not needed at all =D.

Thinking through this, I found a problem with this change I made earlier:

teleport/lib/teleterm/gateway/config.go

Lines 77 to 82 in 0bd2517

// TLSRoutingConnUpgradeRequired indicates that ALPN connection upgrades
// are required for making TLS routing requests.
TLSRoutingConnUpgradeRequired bool
// RootClusterCACertPoolFunc is callback function to fetch Root cluster CAs
// when ALPN connection upgrade is required.
RootClusterCACertPoolFunc alpnproxy.GetClusterCACertPoolFunc

So the gateway is caching TLSRoutingConnUpgradeRequired and RootClusterCACertPoolFunc. It's rare they change but when they do change after login, gateway has no way to reload these values, at least not supported in the current local proxy implementation. Might be cleaner to recreate the gateway by the new cluster after relogin (which will break active connections though).

}

db, err := c.GetDatabase(ctx, params.TargetURI)
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -53,7 +68,8 @@ func (c *Cluster) CreateGateway(ctx context.Context, params CreateGatewayParams)
Username: params.TargetUser,
}

if err := c.ReissueDBCerts(ctx, routeToDatabase); err != nil {
tlsCert, err := c.reissueAndLoadDBCert(ctx, routeToDatabase)
if err != nil {
return nil, trace.Wrap(err)
}

Expand All @@ -63,15 +79,14 @@ func (c *Cluster) CreateGateway(ctx context.Context, params CreateGatewayParams)
TargetUser: params.TargetUser,
TargetName: db.GetName(),
TargetSubresourceName: params.TargetSubresourceName,
Cert: tlsCert,
Protocol: db.GetProtocol(),
KeyPath: c.status.KeyPath(),
CertPath: c.status.DatabaseCertPathForCluster(c.clusterClient.SiteName, db.GetName()),
Insecure: c.clusterClient.InsecureSkipVerify,
WebProxyAddr: c.clusterClient.WebProxyAddr,
Log: c.Log,
CLICommandProvider: params.CLICommandProvider,
TCPPortAllocator: params.TCPPortAllocator,
OnExpiredCert: params.OnExpiredCert,
TCPPortAllocator: gateway.NetTCPPortAllocator{},
ReissueCert: c.makeGatewayReissueDBCertFunc(params.CertReissuer, routeToDatabase),
Clock: c.clock,
TLSRoutingConnUpgradeRequired: c.clusterClient.TLSRoutingConnUpgradeRequired,
RootClusterCACertPoolFunc: c.clusterClient.RootClusterCACertPool,
Expand All @@ -82,3 +97,20 @@ func (c *Cluster) CreateGateway(ctx context.Context, params CreateGatewayParams)

return gw, nil
}

// makeGatewayReissueDBCertFunc creates a gateway.ReissueCertFunc that reissues
// the database certificate using provided GatewayCertReissuer, then loads the
// certificate.
func (c *Cluster) makeGatewayReissueDBCertFunc(certReissuer GatewayCertReissuer, routeToDatabase tlsca.RouteToDatabase) gateway.ReissueCertFunc {
return func(ctx context.Context, gateway *gateway.Gateway) (tls.Certificate, error) {
err := certReissuer.ReissueCert(ctx, gateway, func(ctx context.Context) error {
return trace.Wrap(c.reissueDBCerts(ctx, routeToDatabase))
})
if err != nil {
return tls.Certificate{}, trace.Wrap(err)
}

tlsCert, err := c.loadDBCert(routeToDatabase)
return tlsCert, trace.Wrap(err)
}
}
19 changes: 5 additions & 14 deletions lib/teleterm/clusters/dbcmd_cli_command_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,42 +27,33 @@ import (
// DbcmdCLICommandProvider provides CLI commands for database gateways. It needs Storage to read
// fresh profile state from the disk.
type DbcmdCLICommandProvider struct {
storage StorageByResourceURI
cluster *Cluster
execer dbcmd.Execer
}

type StorageByResourceURI interface {
GetByResourceURI(string) (*Cluster, error)
}

func NewDbcmdCLICommandProvider(storage StorageByResourceURI, execer dbcmd.Execer) DbcmdCLICommandProvider {
func NewDbcmdCLICommandProvider(cluster *Cluster, execer dbcmd.Execer) DbcmdCLICommandProvider {
return DbcmdCLICommandProvider{
storage: storage,
cluster: cluster,
execer: execer,
}
}

func (d DbcmdCLICommandProvider) GetCommand(gateway *gateway.Gateway) (*exec.Cmd, error) {
cluster, err := d.storage.GetByResourceURI(gateway.TargetURI())
Copy link
Copy Markdown
Member

@ravicious ravicious Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I originally made it so that DbcmdCLICommandProvider holds storage instead of cluster because I wanted to guarantee that the cluster.clusterClient passed to dbcmd.NewCmdBuilder will be a fresh client constructed from the state of the profile on disk, which is what storage.GetByResourceURI returns.

Do you foresee any problems with essentially "caching" it in a long-lived object such as the gateway? I'm not totally sure how TeleportClient handles scenarios where the cert expires after TeleportClient is instantiated. It might be that there are no problems with that, I've just never checked if that's the case.

Same goes for cluster.status which holds things such as Roles and Username. If I start a gateway, the user cert expires, Connect asks me to relogin and I log in again, will DbcmdCLICommandProvider have access to fresh profile status?

Copy link
Copy Markdown
Contributor Author

@greedy52 greedy52 Jun 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When connecting to the local proxy (tunnel mode), DbcmdCLICommandProvider only needs tc.SiteName (for things like psql profile name, which is probably not required either) and maybe tc.InsecureSkipVerify. TeleportClient and cluster.status are used more when NOT connecting to the local proxy (it has to know how to load the cert and where to send to, which both not required for local proxy), or connecting to local proxy with TLS cert.

One exception to above is Oracle where profile.OracleWalletDir is called by DbcmdCLICommandProvider. But I don't think Oracle works anyway in Connect since custom logic is needed to create that OracleWalletDir in the first place.

So maybe I can refactor DbcmdCLICommandProvider to decouple it from TeleportClient and ProfileStatus. What do guys think?

Copy link
Copy Markdown
Member

@ravicious ravicious Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dbcmd depending on TeleportClient instead of on a more specific data structure feels like an oversight which just naturally happened over time. I think it might be another side effect of tsh not having to care about long-lived TeleportClient instances – why make dbcmd depend on something else than TeleportClient when you can just read stuff from tc and you can be sure that it's fresh on each tsh call.

This might be another argument for returning a fresh TeleportClient in dbcmd command builder, so that the semantics of using TeleportClient in Connect resemble those of using TeleportClient in tsh. I don't have a horse in this race though so feel free to pick what works best.

if err != nil {
return nil, trace.Wrap(err)
}

routeToDb := tlsca.RouteToDatabase{
ServiceName: gateway.TargetName(),
Protocol: gateway.Protocol(),
Username: gateway.TargetUser(),
Database: gateway.TargetSubresourceName(),
}

cmd, err := dbcmd.NewCmdBuilder(cluster.clusterClient, &cluster.status, routeToDb,
cmd, err := dbcmd.NewCmdBuilder(d.cluster.clusterClient, &d.cluster.status, routeToDb,
// TODO(ravicious): Pass the root cluster name here. cluster.Name returns leaf name for leaf
// clusters.
//
// At this point it doesn't matter though because this argument is used only for
// generating correct CA paths. We use dbcmd.WithNoTLS here which means that the CA paths aren't
// included in the returned CLI command.
cluster.Name,
d.cluster.Name,
dbcmd.WithLogger(gateway.Log()),
dbcmd.WithLocalProxy(gateway.LocalAddress(), gateway.LocalPortInt(), ""),
dbcmd.WithNoTLS(),
Expand Down
66 changes: 3 additions & 63 deletions lib/teleterm/clusters/dbcmd_cli_command_provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@ package clusters
import (
"os/exec"
"path/filepath"
"strings"
"testing"

"github.com/gravitational/trace"
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/lib/client"
Expand All @@ -47,20 +45,6 @@ func (f fakeExec) Command(name string, arg ...string) *exec.Cmd {
return cmd
}

type fakeStorage struct {
clusters []*Cluster
}

func (f fakeStorage) GetByResourceURI(resourceURI string) (*Cluster, error) {
for _, cluster := range f.clusters {
if strings.HasPrefix(resourceURI, cluster.URI.String()) {
return cluster, nil
}
}

return nil, trace.NotFound("not found")
}

func TestDbcmdCLICommandProviderGetCommand(t *testing.T) {
testCases := []struct {
name string
Expand All @@ -87,12 +71,9 @@ func TestDbcmdCLICommandProviderGetCommand(t *testing.T) {
},
},
}
fakeStorage := fakeStorage{
clusters: []*Cluster{&cluster},
}
dbcmdCLICommandProvider := NewDbcmdCLICommandProvider(fakeStorage, fakeExec{})
dbcmdCLICommandProvider := NewDbcmdCLICommandProvider(&cluster, fakeExec{})

keyPairPaths := gatewaytest.MustGenAndSaveCert(t, tlsca.Identity{
tlsCert := gatewaytest.MustGenDBCert(t, tlsca.Identity{
Username: "alice",
Groups: []string{"test-group"},
RouteToDatabase: tlsca.RouteToDatabase{
Expand All @@ -112,8 +93,7 @@ func TestDbcmdCLICommandProviderGetCommand(t *testing.T) {
LocalAddress: "localhost",
WebProxyAddr: "localhost:1337",
Insecure: true,
CertPath: keyPairPaths.CertPath,
KeyPath: keyPairPaths.KeyPath,
Cert: tlsCert,
CLICommandProvider: dbcmdCLICommandProvider,
TCPPortAllocator: gateway.NetTCPPortAllocator{},
},
Expand All @@ -130,43 +110,3 @@ func TestDbcmdCLICommandProviderGetCommand(t *testing.T) {
})
}
}

func TestDbcmdCLICommandProviderGetCommand_ReturnsErrorIfClusterIsNotFound(t *testing.T) {
fakeStorage := fakeStorage{
clusters: []*Cluster{},
}
dbcmdCLICommandProvider := NewDbcmdCLICommandProvider(fakeStorage, fakeExec{})

keyPairPaths := gatewaytest.MustGenAndSaveCert(t, tlsca.Identity{
Username: "alice",
Groups: []string{"test-group"},
RouteToDatabase: tlsca.RouteToDatabase{
ServiceName: "foo",
Protocol: defaults.ProtocolPostgres,
Username: "alice",
},
})

gateway, err := gateway.New(
gateway.Config{
TargetURI: uri.NewClusterURI("quux").AppendDB("foo").String(),
TargetName: "foo",
TargetUser: "alice",
TargetSubresourceName: "",
Protocol: defaults.ProtocolPostgres,
LocalAddress: "localhost",
WebProxyAddr: "localhost:1337",
Insecure: true,
CertPath: keyPairPaths.CertPath,
KeyPath: keyPairPaths.KeyPath,
CLICommandProvider: dbcmdCLICommandProvider,
TCPPortAllocator: gateway.NetTCPPortAllocator{},
},
)
require.NoError(t, err)
t.Cleanup(func() { gateway.Close() })

_, err = dbcmdCLICommandProvider.GetCommand(gateway)
require.Error(t, err)
require.True(t, trace.IsNotFound(err), "err is not trace.NotFound")
}
Loading