Properly apply client_idle_timeout to database access sessions

[Tener]

In database service, clientConn returned from MonitorConn was never used, causing unjustified idle timeouts.

Fixes #32073.

[r0mant]

Can we add test coverage?

[greedy52]

not a result from this change but when reviewing this function, should _, diagErr := s.cfg.AuthClient.AppendDiagnosticTrace(ctx, use ctx? I am worried ctx can be canceled when uploading diagnostic.

[Tener]

Can we add test coverage?

Sure, will do.

[Tener]

@greedy52

not a result from this change but when reviewing this function, should _, diagErr := s.cfg.AuthClient.AppendDiagnosticTrace(ctx, use ctx? I am worried ctx can be canceled when uploading diagnostic.

Could be a valid worry, although I'm not 100% certain. I think cancelCtx or s.closeContext might be safer here?

[greedy52]

Could be a valid worry, although I'm not 100% certain. I think cancelCtx or s.closeContext might be safer here?

I believe ctx can be cancelled for example when the user is locked. cancelCtx should be good.

[Tener]

Can we add test coverage?

@r0mant Added, PTAL.

[Tener]

Hmm...

WARNING: DATA RACE
Read at 0x00c005f72880 by goroutine 7359:
  github.com/jackc/pgconn.(*PgConn).IsClosed()
      /go/pkg/mod/github.com/jackc/pgconn@v1.14.1/pgconn.go:695 +0x49d
  github.com/gravitational/teleport/lib/srv/db/postgres.(*Engine).receiveFromServer.func1()
      /__w/teleport/teleport/lib/srv/db/postgres/engine.go:436 +0x4aa

Previous write at 0x00c005f72880 by goroutine 7325:
  github.com/jackc/pgconn.(*PgConn).Close()
      /go/pkg/mod/github.com/jackc/pgconn@v1.14.1/pgconn.go:627 +0x92
  github.com/gravitational/teleport/lib/srv/db/postgres.(*Engine).HandleConnection.func2()
      /__w/teleport/teleport/lib/srv/db/postgres/engine.go:167 +0x64
  runtime.deferreturn()
      /opt/go/src/runtime/panic.go:477 +0x30
  github.com/gravitational/teleport/lib/srv/db/common.(*reportingEngine).HandleConnection()
      /__w/teleport/teleport/lib/srv/db/common/reporting.go:87 +0x21a
  github.com/gravitational/teleport/lib/srv/db.(*Server).handleConnection()
      /__w/teleport/teleport/lib/srv/db/server.go:1021 +0x764
  github.com/gravitational/teleport/lib/srv/db.(*Server).HandleConnection()
      /__w/teleport/teleport/lib/srv/db/server.go:945 +0x95a
  github.com/gravitational/teleport/lib/srv/db.(*testContext).startHandlingConnections.func1()
      /__w/teleport/teleport/lib/srv/db/access_test.go:1488 +0x4f

Goroutine 7359 (running) created at:
  github.com/gravitational/teleport/lib/srv/db/postgres.(*Engine).receiveFromServer()
      /__w/teleport/teleport/lib/srv/db/postgres/engine.go:[421](https://github.com/gravitational/teleport/actions/runs/6313772578/job/17142603054?pr=32485#step:10:425) +0x4cf
  github.com/gravitational/teleport/lib/srv/db/postgres.(*Engine).HandleConnection.func6()
      /__w/teleport/teleport/lib/srv/db/postgres/engine.go:180 +0x5d

Goroutine 7325 (running) created at:
  github.com/gravitational/teleport/lib/srv/db.(*testContext).startHandlingConnections()
      /__w/teleport/teleport/lib/srv/db/access_test.go:1488 +0x84
  github.com/gravitational/teleport/lib/srv/db.TestDatabaseServerAutoDisconnect.func4()
      /__w/teleport/teleport/lib/srv/db/server_test.go:201 +0x33

[Tener]

The race above is preexisting and genuine, if benign. I'll raise a separate PR with the fix. This one will be blocked before that one lands.

[Tener]

After changes, the test above no longer triggers the data race. Therefore, I think it is OK to merge it as is. The data race fix is pending in another PR.

[public-teleport-github-review-bot]

@Tener See the table below for backport results.

Branch	Result
branch/v12	Failed
branch/v13	Failed
branch/v14	Create PR