Cleaned up docker locally for breaking change in postgres image

Recently, the docker image's default security config was changed because it defaulted to super insecure settings:

docker-library/postgres#580

We didn't care about that since we were only using it locally for testing. Updating the Makefiles for auth related services to work with newer versions of the docker postgres image. Also cleaned up some of authn's use of PG_URL so there was less inconsistency and hard-codedness.

Signed-off-by: Tyler Cloke <tylercloke@gmail.com>

Author: tylercloke

components/authn-service/Makefile

@@ -50,11 +50,11 @@ test:
 
 test_with_db:
 	@docker ps | grep authn-postgres || (echo "Docker postgres not up. Run 'make setup_docker_pg' and try this command again."; exit 1)
-	@PG_URL="postgresql://postgres@127.0.0.1:5432/authn_test?sslmode=disable&timezone=UTC" go test -cover -count=1 -parallel=1 -p 1 $(packages)
+	@PGTZ=UTC PG_URL="postgresql://postgres@127.0.0.1:5432/authn_test?sslmode=disable&password=docker" go test -cover -count=1 -parallel=1 -p 1 $(packages)
 	@echo "Docker containers still up, run 'make kill_docker_pg' to bring them down or test again with make test_with_db."
 
 setup_docker_pg:
-	docker run --name authn-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=authn_test -p 5432:5432 -d postgres:9
+	docker run --name authn-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=authn_test -e POSTGRES_PASSWORD=docker -p 5432:5432 -d postgres:9
 
 kill_docker_pg:
 	docker rm -f authn-postgres || true

components/authn-service/tokens/integration/token_integration_test.go

@@ -22,7 +22,6 @@ import (
 	"github.com/chef/automate/components/authn-service/constants"
 	"github.com/chef/automate/components/authn-service/server"
 	"github.com/chef/automate/components/authn-service/tokens/pg"
-	"github.com/chef/automate/components/authn-service/tokens/pg/testconstants"
 	"github.com/chef/automate/lib/grpc/grpctest"
 	"github.com/chef/automate/lib/grpc/secureconn"
 	"github.com/chef/automate/lib/tls/test/helpers"
@@ -36,6 +35,31 @@ func init() {
 	logger, _ = cfg.Build()
 }
 
+func initializePG() (*pg.Config, error) {
+	ciMode := os.Getenv("CI") == "true"
+
+	// If in CI mode, use the default
+	if ciMode {
+		return &pg.Config{
+			PGURL:          constants.TestPgURL,
+			MigrationsPath: "sql/",
+		}, nil
+	}
+
+	customPGURL, pgURLPassed := os.LookupEnv("PG_URL")
+
+	// If PG_URL wasn't passed (and we aren't in CI)
+	// we shouldn't run the postgres tests, return nil.
+	if !pgURLPassed {
+		return nil, nil
+	}
+
+	return &pg.Config{
+		PGURL:          customPGURL,
+		MigrationsPath: "sql/",
+	}, nil
+}
+
 // In TestChefClientAuthn, we stand up a server that
 // - serves REST endpoints for tokens
 // - uses the tokens passed in to authenticate requests
@@ -61,14 +85,9 @@ func TestChefClientAuthn(t *testing.T) {
 		t.Fatalf("parse test upstream URL %+v: %v", upstream, err)
 	}
 
-	pgURLGiven := false
-	pgCfg := pg.Config{
-		PGURL:          constants.TestPgURL,
-		MigrationsPath: "../pg/sql/",
-	}
-	if v, found := os.LookupEnv("PG_URL"); found {
-		pgCfg.PGURL = v
-		pgURLGiven = true
+	pgCfg, err := initializePG()
+	if err != nil {
+		t.Fatalf("couldn't initialize pg config for tests: %s", err.Error())
 	}
 
 	serviceCerts := helpers.LoadDevCerts(t, "authn-service")
@@ -82,11 +101,11 @@ func TestChefClientAuthn(t *testing.T) {
 				Headers: []string{"x-data-collector-token", "api-token"},
 				Storage: tokenauthn.StorageConfig{
 					Type:   "postgres",
-					Config: &pgCfg,
+					Config: pgCfg,
 				},
 			},
 		},
-		Token:        &pgCfg,
+		Token:        pgCfg,
 		ServiceCerts: serviceCerts,
 	}
 
@@ -96,11 +115,11 @@ func TestChefClientAuthn(t *testing.T) {
 	serv, err := server.NewServer(ctx, config, authorizationClient)
 	if err != nil {
 		// SKIP these tests if there's no PG_URL given -- and never skip during CI!
-		if pgURLGiven || os.Getenv("CI") == "true" {
+		if pgCfg == nil {
 			t.Fatalf("opening connector: %s", err)
 		} else {
 			t.Logf("opening database: %s", err)
-			t.Logf(testconstants.SkipPGMessageFmt, pgCfg.PGURL)
+			t.Logf("failed to open test database with PG_URL: %q", pgCfg.PGURL)
 			t.SkipNow()
 		}
 		t.Fatalf("opening connector: %s", err)

components/authn-service/tokens/pg/pg_test.go

@@ -69,7 +69,7 @@ func setup(t *testing.T) (tokens.Storage, *sql.DB) {
 	backend, err := pgCfg.Open(nil, l, authzV2Client)
 	require.NoError(t, err)
 
-	db := openDB(t)
+	db := openDB(t, pgCfg.PGURL)
 	reset(t, db)
 
 	return backend, db
@@ -105,9 +105,9 @@ func initializePG() (*pg.Config, error) {
 	}, nil
 }
 
-func openDB(t *testing.T) *sql.DB {
+func openDB(t *testing.T, pgURL string) *sql.DB {
 	t.Helper()
-	db, err := sql.Open("postgres", constants.TestPgURL)
+	db, err := sql.Open("postgres", pgURL)
 	require.NoError(t, err, "error opening db")
 	err = db.Ping()
 	require.NoError(t, err, "error pinging db")

components/authn-service/tokens/pg/testconstants/testconstants.go

@@ -19,7 +19,6 @@ import (
 	"github.com/chef/automate/components/authn-service/constants"
 	"github.com/chef/automate/components/authn-service/tokens/mock"
 	"github.com/chef/automate/components/authn-service/tokens/pg"
-	"github.com/chef/automate/components/authn-service/tokens/pg/testconstants"
 	tokens "github.com/chef/automate/components/authn-service/tokens/types"
 	tutil "github.com/chef/automate/components/authn-service/tokens/util"
 	"github.com/chef/automate/lib/grpc/auth_context"
@@ -38,26 +37,44 @@ func init() {
 	rand.Seed(time.Now().Unix())
 }
 
+func initializePG() (*pg.Config, error) {
+	ciMode := os.Getenv("CI") == "true"
+
+	// If in CI mode, use the default
+	if ciMode {
+		return &pg.Config{
+			PGURL:          constants.TestPgURL,
+			MigrationsPath: "sql/",
+		}, nil
+	}
+
+	customPGURL, pgURLPassed := os.LookupEnv("PG_URL")
+
+	// If PG_URL wasn't passed (and we aren't in CI)
+	// we shouldn't run the postgres tests, return nil.
+	if !pgURLPassed {
+		return nil, nil
+	}
+
+	return &pg.Config{
+		PGURL:          customPGURL,
+		MigrationsPath: "sql/",
+	}, nil
+}
+
 type adapterTestFunc func(context.Context, *testing.T, tokens.Storage)
 
 // TestToken tests the mock and pg adapters via their implemented adapter
 // interface
 func TestToken(t *testing.T) {
-	pgURLGiven := false
-
-	// Note: this matches CI
-	pgCfg := pg.Config{
-		PGURL:          constants.TestPgURL,
-		MigrationsPath: "pg/sql/",
-	}
-	if v, found := os.LookupEnv("PG_URL"); found {
-		pgCfg.PGURL = v
-		pgURLGiven = true
+	pgCfg, err := initializePG()
+	if err != nil {
+		t.Fatalf("couldn't initialize pg config for tests: %s", err.Error())
 	}
 
 	adapters := map[string]tokens.TokenConfig{
 		"mock": &mock.Config{},
-		"pg":   &pgCfg,
+		"pg":   pgCfg,
 	}
 
 	authzCerts := helpers.LoadDevCerts(t, "authz-service")
@@ -110,11 +127,11 @@ func TestToken(t *testing.T) {
 					//   - if this is running on CI, never skip
 					// Why bother skipping? -- We don't want our test suite to require
 					// a running postgres instance, as that we would be annoying.
-					if pgURLGiven || os.Getenv("CI") == "true" {
+					if pgCfg == nil {
 						t.Fatalf("opening connector: %s", err)
 					} else {
 						t.Logf("opening database: %s", err)
-						t.Logf(testconstants.SkipPGMessageFmt, pgCfg.PGURL)
+						t.Logf("failed to open test database with PG_URL: %q", pgCfg.PGURL)
 						t.SkipNow()
 					}
 				}

components/authn-service/tokens/tokens_test.go

@@ -37,7 +37,7 @@ test:
 test/%:
 	go test -v -cover -count=1 -parallel=1 -p 1 $(PACKAGE_PATH)/$(subst test/,,$(@D))/... -run $(@F)
 
-PG_URL ?= "postgresql://postgres@127.0.0.1:5432/authz_test?sslmode=disable"
+PG_URL ?= "postgresql://postgres@127.0.0.1:5432/authz_test?sslmode=disable&password=docker"
 DOCKER_CLEAN_MSG = "\nDocker container still up; run 'make kill_docker_pg' to clean it up when finished."
 
 test_with_db: db_container
@@ -60,8 +60,7 @@ db_container:
 	psql -d $(PG_URL) -c "CREATE EXTENSION IF NOT EXISTS \"uuid-ossp\""
 
 setup_docker_pg:
-	docker run --name authz-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=authz_test -p 5432:5432 -d postgres:9
-
+	docker run --name authz-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=authz_test -e POSTGRES_PASSWORD=docker -p 5432:5432 -d postgres:9
 
 kill_docker_pg:
 	docker rm -f authz-postgres || true

components/authz-service/Makefile

@@ -52,7 +52,7 @@ func TestPostgres(t *testing.T) {
 	// between database content and hardcoded storage default policies actually
 	// compares the migrated policies with the hardcoded ones (and NOT the
 	// hardcoded policies with the hardcoded policies).
-	db := openDB(t)
+	db := openDB(t, migrationConfig.PGURL.String())
 	_, err = db.ExecContext(ctx, resetDatabaseStatement)
 	require.NoError(t, err)
 
@@ -85,7 +85,7 @@ func TestPostgres(t *testing.T) {
 	// Reset db before each test. This will restore default policies.
 	r := backend.(storage.Resetter)
 
-	testFuncs := map[string]func(*testing.T, storage.Storage, context.Context){
+	testFuncs := map[string]func(*testing.T, storage.Storage, context.Context, string){
 		"ListPolicies":             testListPolicies,
 		"StorePolicy":              testStorePolicy,
 		"DeletePolicy":             testDeletePolicy,
@@ -103,7 +103,7 @@ func TestPostgres(t *testing.T) {
 					dumpResp(t, resp)
 				}
 			})
-			test(t, backend, ctx)
+			test(t, backend, ctx, migrationConfig.PGURL.String())
 		})
 	}
 }
@@ -112,7 +112,7 @@ func dumpResp(t *testing.T, resp interface{}) {
 	t.Errorf("resp: %s", spew.Sdump(resp))
 }
 
-func testListPolicies(t *testing.T, s storage.Storage, ctx context.Context) {
+func testListPolicies(t *testing.T, s storage.Storage, ctx context.Context, _ string) {
 	t.Run("database with default policies", func(t *testing.T) {
 		resp, err := s.ListPolicies(ctx)
 		require.NoError(t, err)
@@ -122,7 +122,7 @@ func testListPolicies(t *testing.T, s storage.Storage, ctx context.Context) {
 	})
 }
 
-func testStorePolicy(t *testing.T, s storage.Storage, ctx context.Context) {
+func testStorePolicy(t *testing.T, s storage.Storage, ctx context.Context, _ string) {
 	t.Run("database with one row in addition to default policies", func(t *testing.T) {
 		resp, err := s.StorePolicy(
 			ctx,
@@ -143,8 +143,8 @@ func testStorePolicy(t *testing.T, s storage.Storage, ctx context.Context) {
 	})
 }
 
-func testDeletePolicy(t *testing.T, s storage.Storage, ctx context.Context) {
-	db := openDB(t)
+func testDeletePolicy(t *testing.T, s storage.Storage, ctx context.Context, pgurl string) {
+	db := openDB(t, pgurl)
 
 	t.Run("database with one row", func(t *testing.T) {
 		u := uuid.Must(uuid.NewV4())
@@ -187,7 +187,7 @@ func testDeletePolicy(t *testing.T, s storage.Storage, ctx context.Context) {
 	})
 }
 
-func testPurgeSubjectFromPolicies(t *testing.T, s storage.Storage, ctx context.Context) {
+func testPurgeSubjectFromPolicies(t *testing.T, s storage.Storage, ctx context.Context, pgurl string) {
 	t.Run("database with only default policies", func(t *testing.T) {
 		noopCases := map[string]string{
 			"subject not in policy":           "user:local:purge",
@@ -217,7 +217,7 @@ func testPurgeSubjectFromPolicies(t *testing.T, s storage.Storage, ctx context.C
 
 		for desc, tc := range cases {
 			t.Run(desc, func(t *testing.T) {
-				db := openDB(t)
+				db := openDB(t, pgurl)
 				u := uuid.Must(uuid.NewV4())
 				p := []byte(fmt.Sprintf(`{"subjects": %s, "action": "*", "resource": "auth:*", "effect": "allow"}`, tc))
 				_, err := db.ExecContext(ctx, `INSERT INTO policies (id, policy_data, version) VALUES ($1, $2, $3)`,
@@ -292,9 +292,9 @@ func migrationConfigIfPGTestsToBeRun(l logger.Logger, migrationPath string) (*mi
 	}, nil
 }
 
-func openDB(t *testing.T) *sql.DB {
+func openDB(t *testing.T, pgurl string) *sql.DB {
 	t.Helper()
-	db, err := sql.Open("postgres", "postgres://postgres:postgres@127.0.0.1:5432/authz_test?sslmode=disable")
+	db, err := sql.Open("postgres", pgurl)
 	require.NoError(t, err, "error opening db")
 	err = db.Ping()
 	require.NoError(t, err, "error pinging db")

components/authz-service/storage/v1/postgres/postgres_test.go

@@ -189,7 +189,7 @@ func SetupTestDBWithLimit(t *testing.T, projectLimit int) (storage.Storage, *Tes
 	// between database content and hardcoded storage default policies actually
 	// compares the migrated policies with the hardcoded ones (and NOT the
 	// hardcoded policies with the hardcoded policies).
-	db := openDB(t)
+	db := openDB(t, migrationConfig.PGURL.String())
 	_, err = db.ExecContext(ctx, resetDatabaseStatement)
 	require.NoError(t, err, "error resetting database")
 	_, err = db.Exec(`CREATE EXTENSION IF NOT EXISTS "uuid-ossp"`)
@@ -199,7 +199,7 @@ func SetupTestDBWithLimit(t *testing.T, projectLimit int) (storage.Storage, *Tes
 	require.NoError(t, err)
 	return postgres.GetInstance(), &TestDB{
 			DB:      db,
-			ConnURI: "postgres://postgres:postgres@127.0.0.1:5432/authz_test?sslmode=disable",
+			ConnURI: migrationConfig.PGURL.String(),
 		},
 		opaInstance, prng.Seed(t), migrationConfig
 }
@@ -259,9 +259,9 @@ func migrationConfigIfPGTestsToBeRun(l logger.Logger, migrationFolder string) (*
 	}, nil
 }
 
-func openDB(t *testing.T) *sql.DB {
+func openDB(t *testing.T, pgurl string) *sql.DB {
 	t.Helper()
-	db, err := sql.Open("postgres", "postgres://postgres:postgres@127.0.0.1:5432/authz_test?sslmode=disable")
+	db, err := sql.Open("postgres", pgurl)
 	require.NoError(t, err, "error opening db")
 	err = db.Ping()
 	require.NoError(t, err, "error pinging db")

components/authz-service/testhelpers/testhelpers.go

@@ -42,11 +42,11 @@ test:
 
 test_with_db:
 	@docker ps | grep teams-postgres || (echo "Docker postgres not up. Run make setup_docker_pg and try this command again."; exit 1)
-	@PG_URL="postgresql://postgres@127.0.0.1:5432/teams_test?sslmode=disable&timezone=UTC" go test $(packages) -p 1 --parallel 1 -cover
+	@PGTZ=UTC PG_URL="postgresql://postgres@127.0.0.1:5432/teams_test?sslmode=disable&password=docker" go test $(packages) -p 1 --parallel 1 -cover
 	@echo "Docker containers still up, run 'make kill_docker_pg' to bring them down or test again with make test_with_db."
 
 setup_docker_pg:
-	docker run --name teams-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=teams_test -p 5432:5432 -d postgres:9
+	docker run --name teams-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=teams_test -e POSTGRES_PASSWORD=docker -p 5432:5432 -d postgres:9
 	sleep 5 # let docker come up
 	# This creates the extension we need to use UUIDs in the migrations.
 	# Done in habitat in prod. Not done in code because you must be a superuser.