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.

Sign up for GitHub

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

Jump to bottom

refactor: read RYUK_CONTAINER_PRIVILEGED once #475

Merged
merged 1 commit into from
Jul 5, 2022
Merged

refactor: read RYUK_CONTAINER_PRIVILEGED once #475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,8 @@ func WithDefaultBridgeNetwork(bridgeNetworkName string) DockerProviderOption {
}

func NewDockerClient() (cli *client.Client, host string, tcConfig TestContainersConfig, err error) {
tcConfig = readTCPropsFile()
tcConfig = configureTC()

host = tcConfig.Host

opts := []client.Opt{client.FromEnv}
Expand Down Expand Up @@ -732,8 +733,9 @@ func NewDockerProvider(provOpts ...DockerProviderOption) (*DockerProvider, error
return p, nil
}

// readTCPropsFile reads from testcontainers properties file, if it exists
func readTCPropsFile() TestContainersConfig {
// configureTC reads from testcontainers properties file, if it exists
zregvart marked this conversation as resolved.
Show resolved Hide resolved
// it is possible that certain values get overridden when set as environment variables
func configureTC() TestContainersConfig {
home, err := os.UserHomeDir()
if err != nil {
return TestContainersConfig{}
Expand All @@ -752,6 +754,11 @@ func readTCPropsFile() TestContainersConfig {
return TestContainersConfig{}
Copy link
Member

Choose a reason for hiding this comment

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

I'm going to merge this one, although I wonder if the struct we are returning here should consider the env variables too 🤔

}

ryukPrivilegedEnv := os.Getenv("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED")
if ryukPrivilegedEnv != "" {
cfg.RyukPrivileged = ryukPrivilegedEnv == "true"
}

return cfg
}

Expand Down
171 changes: 134 additions & 37 deletions docker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1242,7 +1242,7 @@ func TestReadTCPropsFile(t *testing.T) {
tmpDir := fs.NewDir(t, os.TempDir())
env.Patch(t, "HOME", tmpDir.Path())

config := readTCPropsFile()
config := configureTC()

assert.Empty(t, config, "TC props file should not exist")
})
Expand All @@ -1251,97 +1251,194 @@ func TestReadTCPropsFile(t *testing.T) {
tmpDir := fs.NewDir(t, os.TempDir())
env.Patch(t, "HOME", tmpDir.Path())

config := readTCPropsFile()
config := configureTC()

assert.Empty(t, config, "TC props file should not exist")
})

t.Run("HOME contains TC properties file", func(t *testing.T) {
tests := []struct {
content string
expectedHost string
expectedTLSVerify int
expectedCertPath string
content string
env map[string]string
expected TestContainersConfig
}{
{
"docker.host = tcp://127.0.0.1:33293",
"tcp://127.0.0.1:33293",
0,
"",
map[string]string{},
TestContainersConfig{
Host: "tcp://127.0.0.1:33293",
TLSVerify: 0,
CertPath: "",
},
},
{
"docker.host = tcp://127.0.0.1:33293",
"tcp://127.0.0.1:33293",
0,
"",
map[string]string{},
TestContainersConfig{
Host: "tcp://127.0.0.1:33293",
TLSVerify: 0,
CertPath: "",
},
},
{
`docker.host = tcp://127.0.0.1:33293
docker.host = tcp://127.0.0.1:4711
`,
"tcp://127.0.0.1:4711",
0,
"",
map[string]string{},
TestContainersConfig{
Host: "tcp://127.0.0.1:4711",
TLSVerify: 0,
CertPath: "",
},
},
{
`docker.host = tcp://127.0.0.1:33293
docker.host = tcp://127.0.0.1:4711
docker.host = tcp://127.0.0.1:1234
docker.tls.verify = 1
`,
"tcp://127.0.0.1:1234",
1,
"",
map[string]string{},
TestContainersConfig{
Host: "tcp://127.0.0.1:1234",
TLSVerify: 1,
CertPath: "",
},
},
{
"",
"",
0,
"",
map[string]string{},
TestContainersConfig{
Host: "",
TLSVerify: 0,
CertPath: "",
},
},
{
`foo = bar
docker.host = tcp://127.0.0.1:1234
`,
"tcp://127.0.0.1:1234",
0,
"",
map[string]string{},
TestContainersConfig{
Host: "tcp://127.0.0.1:1234",
TLSVerify: 0,
CertPath: "",
},
},
{
"docker.host=tcp://127.0.0.1:33293",
"tcp://127.0.0.1:33293",
0,
"",
map[string]string{},
TestContainersConfig{
Host: "tcp://127.0.0.1:33293",
TLSVerify: 0,
CertPath: "",
},
},
{
`#docker.host=tcp://127.0.0.1:33293`,
"",
0,
"",
map[string]string{},
TestContainersConfig{
Host: "",
TLSVerify: 0,
CertPath: "",
},
},
{
`#docker.host = tcp://127.0.0.1:33293
docker.host = tcp://127.0.0.1:4711
docker.host = tcp://127.0.0.1:1234
docker.cert.path=/tmp/certs`,
"tcp://127.0.0.1:1234",
0,
"/tmp/certs",
map[string]string{},
TestContainersConfig{
Host: "tcp://127.0.0.1:1234",
TLSVerify: 0,
CertPath: "/tmp/certs",
},
},
{
`ryuk.container.privileged=true`,
map[string]string{},
TestContainersConfig{
Host: "",
TLSVerify: 0,
CertPath: "",
RyukPrivileged: true,
},
},
{
``,
map[string]string{
"TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true",
},
TestContainersConfig{
Host: "",
TLSVerify: 0,
CertPath: "",
RyukPrivileged: true,
},
},
{
`ryuk.container.privileged=true`,
map[string]string{
"TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true",
},
TestContainersConfig{
Host: "",
TLSVerify: 0,
CertPath: "",
RyukPrivileged: true,
},
},
{
`ryuk.container.privileged=false`,
map[string]string{
"TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "true",
},
TestContainersConfig{
Host: "",
TLSVerify: 0,
CertPath: "",
RyukPrivileged: true,
},
},
{
`ryuk.container.privileged=true`,
map[string]string{
"TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "false",
},
TestContainersConfig{
Host: "",
TLSVerify: 0,
CertPath: "",
RyukPrivileged: false,
},
},
{
`ryuk.container.privileged=false`,
map[string]string{
"TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED": "false",
},
TestContainersConfig{
Host: "",
TLSVerify: 0,
CertPath: "",
RyukPrivileged: false,
},
},
}
for _, tt := range tests {
tmpDir := fs.NewDir(t, os.TempDir())
env.Patch(t, "HOME", tmpDir.Path())
for k, v := range tt.env {
env.Patch(t, k, v)
}
if err := ioutil.WriteFile(tmpDir.Join(".testcontainers.properties"), []byte(tt.content), 0o600); err != nil {
t.Errorf("Failed to create the file: %v", err)
return
}

config := readTCPropsFile()
config := configureTC()

assert.Equal(t, tt.expectedHost, config.Host, "Hosts do not match")
assert.Equal(t, tt.expectedTLSVerify, config.TLSVerify, "TLS verifies do not match")
assert.Equal(t, tt.expectedCertPath, config.CertPath, "Cert paths do not match")
assert.Equal(t, tt.expected, config, "Configuration doesn't not match")
}
})
}
Expand Down
4 changes: 1 addition & 3 deletions reaper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,7 @@ func NewReaper(ctx context.Context, sessionID string, provider ReaperProvider, r
}

tcConfig := provider.Config()
if tcConfig.RyukPrivileged || os.Getenv("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED") == "true" {
req.Privileged = true
}
req.Privileged = tcConfig.RyukPrivileged

// Attach reaper container to a requested network if it is specified
if p, ok := provider.(*DockerProvider); ok {
Expand Down