Skip to content

Commit 36bf561

Browse files
committed
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>
1 parent 2aeaf2e commit 36bf561

File tree

7 files changed

+72
-64
lines changed

7 files changed

+72
-64
lines changed

components/authn-service/Makefile

+2-2
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ test:
5050

5151
test_with_db:
5252
@docker ps | grep authn-postgres || (echo "Docker postgres not up. Run 'make setup_docker_pg' and try this command again."; exit 1)
53-
@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)
53+
@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)
5454
@echo "Docker containers still up, run 'make kill_docker_pg' to bring them down or test again with make test_with_db."
5555

5656
setup_docker_pg:
57-
docker run --name authn-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=authn_test -p 5432:5432 -d postgres:9
57+
docker run --name authn-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=authn_test -e POSTGRES_PASSWORD=docker -p 5432:5432 -d postgres:9
5858

5959
kill_docker_pg:
6060
docker rm -f authn-postgres || true

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

+32-13
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"github.com/chef/automate/components/authn-service/constants"
2323
"github.com/chef/automate/components/authn-service/server"
2424
"github.com/chef/automate/components/authn-service/tokens/pg"
25-
"github.com/chef/automate/components/authn-service/tokens/pg/testconstants"
2625
"github.com/chef/automate/lib/grpc/grpctest"
2726
"github.com/chef/automate/lib/grpc/secureconn"
2827
"github.com/chef/automate/lib/tls/test/helpers"
@@ -36,6 +35,31 @@ func init() {
3635
logger, _ = cfg.Build()
3736
}
3837

38+
func initializePG() (*pg.Config, error) {
39+
ciMode := os.Getenv("CI") == "true"
40+
41+
// If in CI mode, use the default
42+
if ciMode {
43+
return &pg.Config{
44+
PGURL: constants.TestPgURL,
45+
MigrationsPath: "sql/",
46+
}, nil
47+
}
48+
49+
customPGURL, pgURLPassed := os.LookupEnv("PG_URL")
50+
51+
// If PG_URL wasn't passed (and we aren't in CI)
52+
// we shouldn't run the postgres tests, return nil.
53+
if !pgURLPassed {
54+
return nil, nil
55+
}
56+
57+
return &pg.Config{
58+
PGURL: customPGURL,
59+
MigrationsPath: "sql/",
60+
}, nil
61+
}
62+
3963
// In TestChefClientAuthn, we stand up a server that
4064
// - serves REST endpoints for tokens
4165
// - uses the tokens passed in to authenticate requests
@@ -61,14 +85,9 @@ func TestChefClientAuthn(t *testing.T) {
6185
t.Fatalf("parse test upstream URL %+v: %v", upstream, err)
6286
}
6387

64-
pgURLGiven := false
65-
pgCfg := pg.Config{
66-
PGURL: constants.TestPgURL,
67-
MigrationsPath: "../pg/sql/",
68-
}
69-
if v, found := os.LookupEnv("PG_URL"); found {
70-
pgCfg.PGURL = v
71-
pgURLGiven = true
88+
pgCfg, err := initializePG()
89+
if err != nil {
90+
t.Fatalf("couldn't initialize pg config for tests: %s", err.Error())
7291
}
7392

7493
serviceCerts := helpers.LoadDevCerts(t, "authn-service")
@@ -82,11 +101,11 @@ func TestChefClientAuthn(t *testing.T) {
82101
Headers: []string{"x-data-collector-token", "api-token"},
83102
Storage: tokenauthn.StorageConfig{
84103
Type: "postgres",
85-
Config: &pgCfg,
104+
Config: pgCfg,
86105
},
87106
},
88107
},
89-
Token: &pgCfg,
108+
Token: pgCfg,
90109
ServiceCerts: serviceCerts,
91110
}
92111

@@ -96,11 +115,11 @@ func TestChefClientAuthn(t *testing.T) {
96115
serv, err := server.NewServer(ctx, config, authorizationClient)
97116
if err != nil {
98117
// SKIP these tests if there's no PG_URL given -- and never skip during CI!
99-
if pgURLGiven || os.Getenv("CI") == "true" {
118+
if pgCfg == nil {
100119
t.Fatalf("opening connector: %s", err)
101120
} else {
102121
t.Logf("opening database: %s", err)
103-
t.Logf(testconstants.SkipPGMessageFmt, pgCfg.PGURL)
122+
t.Logf("failed to open test database with PG_URL: %q", pgCfg.PGURL)
104123
t.SkipNow()
105124
}
106125
t.Fatalf("opening connector: %s", err)

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func setup(t *testing.T) (tokens.Storage, *sql.DB) {
6969
backend, err := pgCfg.Open(nil, l, authzV2Client)
7070
require.NoError(t, err)
7171

72-
db := openDB(t)
72+
db := openDB(t, pgCfg.PGURL)
7373
reset(t, db)
7474

7575
return backend, db
@@ -105,9 +105,9 @@ func initializePG() (*pg.Config, error) {
105105
}, nil
106106
}
107107

108-
func openDB(t *testing.T) *sql.DB {
108+
func openDB(t *testing.T, pgURL string) *sql.DB {
109109
t.Helper()
110-
db, err := sql.Open("postgres", constants.TestPgURL)
110+
db, err := sql.Open("postgres", pgURL)
111111
require.NoError(t, err, "error opening db")
112112
err = db.Ping()
113113
require.NoError(t, err, "error pinging db")

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

-27
This file was deleted.

components/authn-service/tokens/tokens_test.go

+31-14
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"github.com/chef/automate/components/authn-service/constants"
2020
"github.com/chef/automate/components/authn-service/tokens/mock"
2121
"github.com/chef/automate/components/authn-service/tokens/pg"
22-
"github.com/chef/automate/components/authn-service/tokens/pg/testconstants"
2322
tokens "github.com/chef/automate/components/authn-service/tokens/types"
2423
tutil "github.com/chef/automate/components/authn-service/tokens/util"
2524
"github.com/chef/automate/lib/grpc/auth_context"
@@ -38,26 +37,44 @@ func init() {
3837
rand.Seed(time.Now().Unix())
3938
}
4039

40+
func initializePG() (*pg.Config, error) {
41+
ciMode := os.Getenv("CI") == "true"
42+
43+
// If in CI mode, use the default
44+
if ciMode {
45+
return &pg.Config{
46+
PGURL: constants.TestPgURL,
47+
MigrationsPath: "sql/",
48+
}, nil
49+
}
50+
51+
customPGURL, pgURLPassed := os.LookupEnv("PG_URL")
52+
53+
// If PG_URL wasn't passed (and we aren't in CI)
54+
// we shouldn't run the postgres tests, return nil.
55+
if !pgURLPassed {
56+
return nil, nil
57+
}
58+
59+
return &pg.Config{
60+
PGURL: customPGURL,
61+
MigrationsPath: "sql/",
62+
}, nil
63+
}
64+
4165
type adapterTestFunc func(context.Context, *testing.T, tokens.Storage)
4266

4367
// TestToken tests the mock and pg adapters via their implemented adapter
4468
// interface
4569
func TestToken(t *testing.T) {
46-
pgURLGiven := false
47-
48-
// Note: this matches CI
49-
pgCfg := pg.Config{
50-
PGURL: constants.TestPgURL,
51-
MigrationsPath: "pg/sql/",
52-
}
53-
if v, found := os.LookupEnv("PG_URL"); found {
54-
pgCfg.PGURL = v
55-
pgURLGiven = true
70+
pgCfg, err := initializePG()
71+
if err != nil {
72+
t.Fatalf("couldn't initialize pg config for tests: %s", err.Error())
5673
}
5774

5875
adapters := map[string]tokens.TokenConfig{
5976
"mock": &mock.Config{},
60-
"pg": &pgCfg,
77+
"pg": pgCfg,
6178
}
6279

6380
authzCerts := helpers.LoadDevCerts(t, "authz-service")
@@ -110,11 +127,11 @@ func TestToken(t *testing.T) {
110127
// - if this is running on CI, never skip
111128
// Why bother skipping? -- We don't want our test suite to require
112129
// a running postgres instance, as that we would be annoying.
113-
if pgURLGiven || os.Getenv("CI") == "true" {
130+
if pgCfg == nil {
114131
t.Fatalf("opening connector: %s", err)
115132
} else {
116133
t.Logf("opening database: %s", err)
117-
t.Logf(testconstants.SkipPGMessageFmt, pgCfg.PGURL)
134+
t.Logf("failed to open test database with PG_URL: %q", pgCfg.PGURL)
118135
t.SkipNow()
119136
}
120137
}

components/authz-service/Makefile

+2-3
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ test:
3737
test/%:
3838
go test -v -cover -count=1 -parallel=1 -p 1 $(PACKAGE_PATH)/$(subst test/,,$(@D))/... -run $(@F)
3939

40-
PG_URL ?= "postgresql://postgres@127.0.0.1:5432/authz_test?sslmode=disable"
40+
PG_URL ?= "postgresql://postgres@127.0.0.1:5432/authz_test?sslmode=disable&password=docker"
4141
DOCKER_CLEAN_MSG = "\nDocker container still up; run 'make kill_docker_pg' to clean it up when finished."
4242

4343
test_with_db: db_container
@@ -60,8 +60,7 @@ db_container:
6060
psql -d $(PG_URL) -c "CREATE EXTENSION IF NOT EXISTS \"uuid-ossp\""
6161

6262
setup_docker_pg:
63-
docker run --name authz-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=authz_test -p 5432:5432 -d postgres:9
64-
63+
docker run --name authz-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=authz_test -e POSTGRES_PASSWORD=docker -p 5432:5432 -d postgres:9
6564

6665
kill_docker_pg:
6766
docker rm -f authz-postgres || true

components/teams-service/Makefile

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ test:
4242

4343
test_with_db:
4444
@docker ps | grep teams-postgres || (echo "Docker postgres not up. Run make setup_docker_pg and try this command again."; exit 1)
45-
@PG_URL="postgresql://postgres@127.0.0.1:5432/teams_test?sslmode=disable&timezone=UTC" go test $(packages) -p 1 --parallel 1 -cover
45+
@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
4646
@echo "Docker containers still up, run 'make kill_docker_pg' to bring them down or test again with make test_with_db."
4747

4848
setup_docker_pg:
49-
docker run --name teams-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=teams_test -p 5432:5432 -d postgres:9
49+
docker run --name teams-postgres -e POSTGRES_USER=postgres -e POSTGRES_DB=teams_test -e POSTGRES_PASSWORD=docker -p 5432:5432 -d postgres:9
5050
sleep 5 # let docker come up
5151
# This creates the extension we need to use UUIDs in the migrations.
5252
# Done in habitat in prod. Not done in code because you must be a superuser.

0 commit comments

Comments
 (0)