Skip to content
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.

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

Create user in envtool for PostgreSQL #4133

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Taskfile.yml
Expand Up @@ -419,7 +419,7 @@ tasks:
--proxy-addr=127.0.0.1:47017
--mode=diff-normal
--handler=pg
--postgresql-url='postgres://127.0.0.1:5433/ferretdb?search_path='
--postgresql-url='postgres://username:password@127.0.0.1:5433/ferretdb?search_path='
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For run-secured to work, what other better thing can I do?

Copy link
Member

Choose a reason for hiding this comment

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

That target was supposed to be for the "old" authentication that does not need credentials in the PostgreSQL URI. Why are they needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's because postgres started on port 5433 is created from postgres_secured docker https://github.com/FerretDB/FerretDB/blob/main/docker-compose.yml#L28.
Unlike postgres docker which has POSTGRES_HOST_AUTH_METHOD=trust, postgres_secured doesn't have trust so username/password is required to connect to it.

--test-records-dir=tmp/records

run-proxy:
Expand Down
117 changes: 116 additions & 1 deletion cmd/envtool/envtool.go
Expand Up @@ -40,6 +40,9 @@
"github.com/FerretDB/FerretDB/build/version"
mysqlpool "github.com/FerretDB/FerretDB/internal/backends/mysql/metadata/pool"
"github.com/FerretDB/FerretDB/internal/backends/postgresql/metadata/pool"
"github.com/FerretDB/FerretDB/internal/clientconn"
"github.com/FerretDB/FerretDB/internal/clientconn/connmetrics"
"github.com/FerretDB/FerretDB/internal/handler/registry"
"github.com/FerretDB/FerretDB/internal/util/ctxutil"
"github.com/FerretDB/FerretDB/internal/util/debug"
"github.com/FerretDB/FerretDB/internal/util/lazyerrors"
Expand Down Expand Up @@ -132,7 +135,7 @@
return ctx.Err()
}

return nil
return setupUser(ctx, logger, uint16(port))
}

// setupPostgres configures `postgres` container.
Expand Down Expand Up @@ -219,6 +222,118 @@
return ctx.Err()
}

// setupUser creates a user in admin database with supported mechanisms.
// The user uses username/password credential which is the same as the PostgreSQL
// credentials.
//
// Without this, once the first user is created, the authentication fails
// as username/password does not exist in admin.system.users collection.
func setupUser(ctx context.Context, logger *zap.SugaredLogger, postgreSQLPort uint16) error {
Copy link
Contributor Author

@chilagrow chilagrow Feb 29, 2024

Choose a reason for hiding this comment

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

This does not replace setupUser() in integration test.
Because this function creates user in ferretdb database which postgreSQL schema, postgres://username@127.0.0.1:5432/ferretdb?pool_max_conns=50.

Each integration test create its own schema like so, postgres://username@127.0.0.1:5432/testauthentication?pool_max_conns=50.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification! I was worried this could jeopardize running tests directly with go test for a moment 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still there for SQLite. It only has handler level authentication, and once the first user is created without this code, auth would fail.

It will be handled in a separate PR.

if err := waitForPort(ctx, logger.Named("postgreSQL"), postgreSQLPort); err != nil {
return err
}

sp, err := state.NewProvider("")
if err != nil {
return err
}

postgreSQlURL := fmt.Sprintf("postgres://username:password@localhost:%d/ferretdb", postgreSQLPort)
listenerMetrics := connmetrics.NewListenerMetrics()
handlerOpts := &registry.NewHandlerOpts{
Logger: logger.Desugar(),
ConnMetrics: listenerMetrics.ConnMetrics,
StateProvider: sp,
PostgreSQLURL: postgreSQlURL,
TestOpts: registry.TestOpts{
CappedCleanupPercentage: 20,
EnableNewAuth: true,
},
}

h, closeBackend, err := registry.NewHandler("postgresql", handlerOpts)
if err != nil {
return err
}

defer closeBackend()

listenerOpts := clientconn.NewListenerOpts{
Mode: clientconn.NormalMode,
Metrics: listenerMetrics,
Handler: h,
Logger: logger.Desugar(),
TCP: "127.0.0.1:0",
}

l := clientconn.NewListener(&listenerOpts)

runErr := make(chan error)

go func() {
if err := l.Run(ctx); err != nil && !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) {

Check failure on line 274 in cmd/envtool/envtool.go

View workflow job for this annotation

GitHub Actions / golangci-lint

shadow: declaration of "err" shadows declaration at line 236 (govet)
runErr <- err

return
}
}()

defer close(runErr)

select {
case err := <-runErr:

Check failure on line 284 in cmd/envtool/envtool.go

View workflow job for this annotation

GitHub Actions / golangci-lint

shadow: declaration of "err" shadows declaration at line 236 (govet)
if err != nil {
return err
}
case <-time.After(time.Millisecond):
}

port := l.TCPAddr().(*net.TCPAddr).Port

var retry int64

eval := `'
if (db.getSiblingDB("admin").getUser("username") == null){
db.getSiblingDB("admin").createUser(
{user: "username", pwd: "password", roles: [], mechanisms: ["SCRAM-SHA-1","SCRAM-SHA-256","PLAIN"]}
)
}
'`
args := []string{
"compose",
"exec",
"-T",
"mongodb",
"mongosh",
"--host=host.docker.internal",
fmt.Sprintf("--port=%d", port),
"--eval",
eval,
}

var buf bytes.Buffer

for ctx.Err() == nil {
buf.Reset()

err = runCommand("docker", args, &buf, logger)
if err == nil {
break
}

logger.Infof("%s:\n%s", err, buf.String())

retry++
ctxutil.SleepWithJitter(ctx, time.Second, retry)
}

if err != nil {
return err
}

return ctx.Err()
}

// setupMongodbSecured configures `mongodb_secured` container.
func setupMongodbSecured(ctx context.Context, logger *zap.SugaredLogger) error {
if err := waitForPort(ctx, logger.Named("mongodb_secured"), 47018); err != nil {
Expand Down
25 changes: 25 additions & 0 deletions cmd/envtool/envtool_test.go
Expand Up @@ -16,8 +16,10 @@ package main

import (
"bytes"
"context"
"os"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -92,3 +94,26 @@ func TestPackageVersion(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, "1.0.0", output.String())
}

func TestSetupUser(t *testing.T) {
if testing.Short() {
t.Skip("skipping in -short mode")
}

t.Parallel()

ctx, cancel := context.WithTimeout(testutil.Ctx(t), 5*time.Second)
t.Cleanup(cancel)

l := testutil.Logger(t).Sugar()

err := setupUser(ctx, l, 5432)
require.NoError(t, err)

// if the user already exists, it should not fail
err = setupUser(ctx, l, 5432)
require.NoError(t, err)

err = setupUser(ctx, l, 5433)
require.NoError(t, err)
}