From c8af1768283a779300bca2f736317cb54e3ac0d9 Mon Sep 17 00:00:00 2001 From: Zac Bergquist Date: Mon, 6 Jun 2022 21:38:33 -0600 Subject: [PATCH] Remove more deprecated code (#13132) * Remove more deprecated code In the case where we were using the PostalCode, StreetAddress, and Province fields rather than ASN.1 extensions we could have removed more code, but I opted for a phase approach where we stop populating the fields in v10 (but still accept them), and then stop reading them entirely in v11. * Fix lints and remove more unused code * Restore StreetAddress / PostalCode We don't yet have dedicated extensions for these --- lib/auth/helpers.go | 13 ++------ lib/cache/collections.go | 7 ----- lib/client/api.go | 44 ++------------------------- lib/client/client_test.go | 4 +-- lib/client/session.go | 11 +------ lib/services/local/desktops.go | 5 ---- lib/session/session.go | 7 ----- lib/srv/db/access_test.go | 55 ---------------------------------- lib/srv/db/proxyserver_test.go | 1 + lib/tlsca/ca.go | 27 ++++++++--------- 10 files changed, 20 insertions(+), 154 deletions(-) diff --git a/lib/auth/helpers.go b/lib/auth/helpers.go index 42db54063ae..a9592711f2a 100644 --- a/lib/auth/helpers.go +++ b/lib/auth/helpers.go @@ -91,17 +91,8 @@ func (cfg *TestAuthServerConfig) CheckAndSetDefaults() error { // CreateUploaderDir creates directory for file uploader service func CreateUploaderDir(dir string) error { - // DELETE IN(5.1.0) - // this folder is no longer used past 5.0 upgrade - err := os.MkdirAll(filepath.Join(dir, teleport.LogsDir, teleport.ComponentUpload, - events.SessionLogsDir, apidefaults.Namespace), teleport.SharedDirMode) - if err != nil { - return trace.ConvertSystemError(err) - } - - err = os.MkdirAll(filepath.Join(dir, teleport.LogsDir, teleport.ComponentUpload, - events.StreamingLogsDir, apidefaults.Namespace), teleport.SharedDirMode) - if err != nil { + if err := os.MkdirAll(filepath.Join(dir, teleport.LogsDir, teleport.ComponentUpload, + events.StreamingLogsDir, apidefaults.Namespace), teleport.SharedDirMode); err != nil { return trace.ConvertSystemError(err) } diff --git a/lib/cache/collections.go b/lib/cache/collections.go index edfbb63fbe4..c641705441f 100644 --- a/lib/cache/collections.go +++ b/lib/cache/collections.go @@ -18,7 +18,6 @@ package cache import ( "context" - "strings" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" @@ -833,12 +832,6 @@ func (c *certAuthority) fetch(ctx context.Context) (apply func(ctx context.Conte func (c *certAuthority) fetchCertAuthorities(ctx context.Context, caType types.CertAuthType) (apply func(ctx context.Context) error, err error) { authorities, err := c.Trust.GetCertAuthorities(ctx, caType, c.watch.LoadSecrets) if err != nil { - // DELETE IN: 5.1 - // - // All clusters will support JWT signers in 5.1. - if strings.Contains(err.Error(), "authority type is not supported") { - return func(ctx context.Context) error { return nil }, nil - } return nil, trace.Wrap(err) } diff --git a/lib/client/api.go b/lib/client/api.go index 79accbf23c8..e4c88422203 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -2355,7 +2355,7 @@ func (tc *TeleportClient) runCommandOnNodes( // runCommand executes a given bash command on an established NodeClient. func (tc *TeleportClient) runCommand(ctx context.Context, nodeClient *NodeClient, command []string) error { - nodeSession, err := newSession(nodeClient, nil, tc.Config.Env, tc.Stdin, tc.Stdout, tc.Stderr, tc.useLegacyID(nodeClient), tc.EnableEscapeSequences) + nodeSession, err := newSession(nodeClient, nil, tc.Config.Env, tc.Stdin, tc.Stdout, tc.Stderr, tc.EnableEscapeSequences) if err != nil { return trace.Wrap(err) } @@ -2397,7 +2397,7 @@ func (tc *TeleportClient) runShell(ctx context.Context, nodeClient *NodeClient, env[key] = value } - nodeSession, err := newSession(nodeClient, sessToJoin, env, tc.Stdin, tc.Stdout, tc.Stderr, tc.useLegacyID(nodeClient), tc.EnableEscapeSequences) + nodeSession, err := newSession(nodeClient, sessToJoin, env, tc.Stdin, tc.Stdout, tc.Stderr, tc.EnableEscapeSequences) if err != nil { return trace.Wrap(err) } @@ -3524,46 +3524,6 @@ func (tc *TeleportClient) AskPassword(ctx context.Context) (pwd string, err erro ctx, tc.Stderr, prompt.Stdin(), fmt.Sprintf("Enter password for Teleport user %v", tc.Config.Username)) } -// DELETE IN: 4.1.0 -// -// useLegacyID returns true if an old style (UUIDv1) session ID should be -// generated because the client is talking with a older server. -func (tc *TeleportClient) useLegacyID(nodeClient *NodeClient) bool { - _, err := tc.getServerVersion(nodeClient) - return trace.IsNotFound(err) -} - -type serverResponse struct { - version string - err error -} - -// getServerVersion makes a SSH global request to the server to request the -// version. -func (tc *TeleportClient) getServerVersion(nodeClient *NodeClient) (string, error) { - responseCh := make(chan serverResponse) - - go func() { - ok, payload, err := nodeClient.Client.SendRequest(teleport.VersionRequest, true, nil) - if err != nil { - responseCh <- serverResponse{err: trace.NotFound(err.Error())} - } else if !ok { - responseCh <- serverResponse{err: trace.NotFound("server does not support version request")} - } - responseCh <- serverResponse{version: string(payload)} - }() - - select { - case resp := <-responseCh: - if resp.err != nil { - return "", trace.Wrap(resp.err) - } - return resp.version, nil - case <-time.After(500 * time.Millisecond): - return "", trace.NotFound("timed out waiting for server response") - } -} - // loadTLS returns the user's TLS configuration for an external identity if the SkipLocalAuth flag was set // or teleport core TLS certificate for the local agent. func (tc *TeleportClient) loadTLSConfig() (*tls.Config, error) { diff --git a/lib/client/client_test.go b/lib/client/client_test.go index 5add34578ce..1936c2f7707 100644 --- a/lib/client/client_test.go +++ b/lib/client/client_test.go @@ -63,7 +63,7 @@ func (s *ClientTestSuite) TestNewSession(c *check.C) { } // defaults: - ses, err := newSession(nc, nil, nil, nil, nil, nil, false, true) + ses, err := newSession(nc, nil, nil, nil, nil, nil, true) c.Assert(err, check.IsNil) c.Assert(ses, check.NotNil) c.Assert(ses.NodeClient(), check.Equals, nc) @@ -77,7 +77,7 @@ func (s *ClientTestSuite) TestNewSession(c *check.C) { env := map[string]string{ sshutils.SessionEnvVar: "session-id", } - ses, err = newSession(nc, nil, env, nil, nil, nil, false, true) + ses, err = newSession(nc, nil, env, nil, nil, nil, true) c.Assert(err, check.IsNil) c.Assert(ses, check.NotNil) c.Assert(ses.env, check.DeepEquals, env) diff --git a/lib/client/session.go b/lib/client/session.go index 10affc8e958..c9e2d603401 100644 --- a/lib/client/session.go +++ b/lib/client/session.go @@ -102,7 +102,6 @@ func newSession(client *NodeClient, stdin io.Reader, stdout io.Writer, stderr io.Writer, - legacyID bool, enableEscapeSequences bool, ) (*NodeSession, error) { // Initialize the terminal. Note that at this point, we don't know if this @@ -149,15 +148,7 @@ func newSession(client *NodeClient, } else { sid, ok := ns.env[sshutils.SessionEnvVar] if !ok { - // DELETE IN: 4.1.0. - // - // Always send UUIDv4 after 4.1. - if legacyID { - sid = string(session.NewLegacyID()) - } else { - sid = string(session.NewID()) - } - + sid = string(session.NewID()) } ns.id = session.ID(sid) } diff --git a/lib/services/local/desktops.go b/lib/services/local/desktops.go index 1eae39b1cc5..b01cdf1d5fd 100644 --- a/lib/services/local/desktops.go +++ b/lib/services/local/desktops.go @@ -137,11 +137,6 @@ func (s *WindowsDesktopService) DeleteWindowsDesktop(ctx context.Context, hostID } key := backend.Key(windowsDesktopsPrefix, hostID, name) - // legacy behavior, we didn't have host IDs - // DELETE IN 10.0 (zmb3, lxea) - if hostID == "" { - key = backend.Key(windowsDesktopsPrefix, name) - } err := s.Delete(ctx, key) if err != nil { diff --git a/lib/session/session.go b/lib/session/session.go index 5fdac07c9a7..41273fc9e7a 100644 --- a/lib/session/session.go +++ b/lib/session/session.go @@ -71,13 +71,6 @@ func NewID() ID { return ID(uuid.New().String()) } -// DELETE IN: 4.1.0. -// -// NewLegacyID creates a new session ID in the UUIDv1 legacy format. -func NewLegacyID() ID { - return ID(uuid.New().String()) -} - // Session is an interactive collaboration session that represents one // or many SSH session started by teleport user type Session struct { diff --git a/lib/srv/db/access_test.go b/lib/srv/db/access_test.go index 4d6d8de69c3..f3f4ab2b658 100644 --- a/lib/srv/db/access_test.go +++ b/lib/srv/db/access_test.go @@ -926,61 +926,6 @@ func TestPostgresInjectionUser(t *testing.T) { require.NoError(t, err) } -// TestCompatibilityWithOldAgents verifies that older database agents where -// each database was represented as a DatabaseServer are supported. -// -// DELETE IN 9.0. -func TestCompatibilityWithOldAgents(t *testing.T) { - ctx := context.Background() - testCtx := setupTestContext(ctx, t) - go testCtx.startProxy() - - postgresServer, err := postgres.NewTestServer(common.TestServerConfig{ - Name: "postgres", - AuthClient: testCtx.authClient, - }) - require.NoError(t, err) - go postgresServer.Serve() - t.Cleanup(func() { postgresServer.Close() }) - - database, err := types.NewDatabaseV3(types.Metadata{ - Name: "postgres", - }, types.DatabaseSpecV3{ - Protocol: defaults.ProtocolPostgres, - URI: net.JoinHostPort("localhost", postgresServer.Port()), - }) - require.NoError(t, err) - databaseServer := testCtx.setupDatabaseServer(ctx, t, agentParams{ - Databases: []types.Database{database}, - GetServerInfoFn: func(database types.Database) func() (types.Resource, error) { - return func() (types.Resource, error) { - return types.NewDatabaseServerV3(types.Metadata{ - Name: database.GetName(), - }, types.DatabaseServerSpecV3{ - Protocol: database.GetProtocol(), - URI: database.GetURI(), - HostID: testCtx.hostID, - Hostname: constants.APIDomain, - }) - } - }, - }) - go func() { - for conn := range testCtx.fakeRemoteSite.ProxyConn() { - go databaseServer.HandleConnection(conn) - } - }() - - testCtx.createUserAndRole(ctx, t, "alice", "admin", []string{"postgres"}, []string{"postgres"}) - - // Make sure we can connect successfully. - psql, err := testCtx.postgresClient(ctx, "alice", "postgres", "postgres", "postgres") - require.NoError(t, err) - - err = psql.Close(ctx) - require.NoError(t, err) -} - func TestRedisGetSet(t *testing.T) { ctx := context.Background() testCtx := setupTestContext(ctx, t, withSelfHostedRedis("redis")) diff --git a/lib/srv/db/proxyserver_test.go b/lib/srv/db/proxyserver_test.go index c889ac8761f..86baab4f681 100644 --- a/lib/srv/db/proxyserver_test.go +++ b/lib/srv/db/proxyserver_test.go @@ -107,6 +107,7 @@ func TestProxyConnectionLimiting(t *testing.T) { // When a connection is released a new can be established t.Run("reconnect one", func(t *testing.T) { // Get one open connection. + require.GreaterOrEqual(t, len(connsClosers), 1) oneConn := connsClosers[len(connsClosers)-1] connsClosers = connsClosers[:len(connsClosers)-1] diff --git a/lib/tlsca/ca.go b/lib/tlsca/ca.go index 553a9cf49d6..d100a28969b 100644 --- a/lib/tlsca/ca.go +++ b/lib/tlsca/ca.go @@ -401,20 +401,16 @@ func (id *Identity) Subject() (pkix.Name, error) { } subject := pkix.Name{ - CommonName: id.Username, - } - subject.Organization = append([]string{}, id.Groups...) - subject.OrganizationalUnit = append([]string{}, id.Usage...) - subject.Locality = append([]string{}, id.Principals...) + CommonName: id.Username, + Organization: append([]string{}, id.Groups...), + OrganizationalUnit: append([]string{}, id.Usage...), + Locality: append([]string{}, id.Principals...), - // DELETE IN (5.0.0) - // Groups are marshaled to both ASN1 extension - // and old Province section, for backwards-compatibility, - // however begin migration to ASN1 extensions in the future - // for this and other properties - subject.Province = append([]string{}, id.KubernetesGroups...) - subject.StreetAddress = []string{id.RouteToCluster} - subject.PostalCode = []string{string(rawTraits)} + // TODO: create ASN.1 extensions for traits and RouteToCluster + // and move away from using StreetAddress and PostalCode + StreetAddress: []string{id.RouteToCluster}, + PostalCode: []string{string(rawTraits)}, + } for i := range id.KubernetesUsers { kubeUser := id.KubernetesUsers[i] @@ -765,9 +761,10 @@ func FromSubject(subject pkix.Name, expires time.Time) (*Identity, error) { } } - // DELETE IN(5.0.0): This logic is using Province field + // DELETE IN 11.0.0: This logic is using Province field // from subject in case if Kubernetes groups were not populated - // from ASN1 extension, after 5.0 Province field will be ignored + // from ASN1 extension, after 5.0 Province field will be ignored, + // and after 10.0.0 Province field is never populated if len(id.KubernetesGroups) == 0 { id.KubernetesGroups = subject.Province }