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

Update config/secret handling: Copy files into containers instead of bind mounting #12448

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
120 changes: 36 additions & 84 deletions pkg/compose/create.go
Original file line number Diff line number Diff line change
@@ -294,6 +294,12 @@ func (s *composeService) getCreateConfigs(ctx context.Context,
return createConfigs{}, err
}

// CONFIGS and SECRETS
err = checkContainerConfigsSecrets(*p, service)
if err != nil {
return createConfigs{}, err
}

// NETWORKING
links, err := s.getLinks(ctx, p.Name, service, number)
if err != nil {
@@ -952,135 +958,81 @@ func fillBindMounts(p types.Project, s types.ServiceConfig, m map[string]mount.M
m[bindMount.Target] = bindMount
}

secrets, err := buildContainerSecretMounts(p, s)
return m, nil
}

func checkContainerConfigsSecrets(p types.Project, s types.ServiceConfig) error {
err := checkContainerConfigs(p, s)
if err != nil {
return nil, err
}
for _, s := range secrets {
if _, found := m[s.Target]; found {
continue
}
m[s.Target] = s
return err
}

configs, err := buildContainerConfigMounts(p, s)
err = checkContainerSecrets(p, s)
if err != nil {
return nil, err
}
for _, c := range configs {
if _, found := m[c.Target]; found {
continue
}
m[c.Target] = c
return err
}
return m, nil
}

func buildContainerConfigMounts(p types.Project, s types.ServiceConfig) ([]mount.Mount, error) {
mounts := map[string]mount.Mount{}
return nil
}

configsBaseDir := "/"
func checkContainerConfigs(p types.Project, s types.ServiceConfig) error {
for _, config := range s.Configs {
target := config.Target
if config.Target == "" {
target = configsBaseDir + config.Source
} else if !isAbsTarget(config.Target) {
target = configsBaseDir + config.Target
}

definedConfig := p.Configs[config.Source]
if definedConfig.External {
return nil, fmt.Errorf("unsupported external config %s", definedConfig.Name)
return fmt.Errorf("unsupported external config %s", definedConfig.Name)
}

if definedConfig.Driver != "" {
return nil, errors.New("Docker Compose does not support configs.*.driver")
return errors.New("Docker Compose does not support configs.*.driver")
}
if definedConfig.TemplateDriver != "" {
return nil, errors.New("Docker Compose does not support configs.*.template_driver")
return errors.New("Docker Compose does not support configs.*.template_driver")
}

if definedConfig.Environment != "" || definedConfig.Content != "" {
continue
if s.ReadOnly {
return fmt.Errorf("cannot create config %q in read-only service %q", definedConfig.Name, s.Name)
}

if config.UID != "" || config.GID != "" || config.Mode != nil {
logrus.Warn("config `uid`, `gid` and `mode` are not supported, they will be ignored")
if definedConfig.Environment != "" || definedConfig.Content != "" {
continue
}

bindMount, err := buildMount(p, types.ServiceVolumeConfig{
Type: types.VolumeTypeBind,
Source: definedConfig.File,
Target: target,
ReadOnly: true,
})
if err != nil {
return nil, err
if _, err := os.Stat(definedConfig.File); os.IsNotExist(err) {
logrus.Warnf("config file %s does not exist", definedConfig.Name)
}
mounts[target] = bindMount
}
values := make([]mount.Mount, 0, len(mounts))
for _, v := range mounts {
values = append(values, v)
}
return values, nil
return nil
}

func buildContainerSecretMounts(p types.Project, s types.ServiceConfig) ([]mount.Mount, error) {
mounts := map[string]mount.Mount{}

secretsDir := "/run/secrets/"
func checkContainerSecrets(p types.Project, s types.ServiceConfig) error {
for _, secret := range s.Secrets {
target := secret.Target
if secret.Target == "" {
target = secretsDir + secret.Source
} else if !isAbsTarget(secret.Target) {
target = secretsDir + secret.Target
}

definedSecret := p.Secrets[secret.Source]
if definedSecret.External {
return nil, fmt.Errorf("unsupported external secret %s", definedSecret.Name)
return fmt.Errorf("unsupported external secret %s", definedSecret.Name)
}

if definedSecret.Driver != "" {
return nil, errors.New("Docker Compose does not support secrets.*.driver")
return errors.New("Docker Compose does not support secrets.*.driver")
}
if definedSecret.TemplateDriver != "" {
return nil, errors.New("Docker Compose does not support secrets.*.template_driver")
return errors.New("Docker Compose does not support secrets.*.template_driver")
}

if definedSecret.Environment != "" {
continue
if s.ReadOnly {
return fmt.Errorf("cannot create secret %q in read-only service %q", definedSecret.Name, s.Name)
}

if secret.UID != "" || secret.GID != "" || secret.Mode != nil {
logrus.Warn("secrets `uid`, `gid` and `mode` are not supported, they will be ignored")
if definedSecret.Environment != "" {
continue
}

if _, err := os.Stat(definedSecret.File); os.IsNotExist(err) {
logrus.Warnf("secret file %s does not exist", definedSecret.Name)
}

mnt, err := buildMount(p, types.ServiceVolumeConfig{
Type: types.VolumeTypeBind,
Source: definedSecret.File,
Target: target,
ReadOnly: true,
Bind: &types.ServiceVolumeBind{
CreateHostPath: false,
},
})
if err != nil {
return nil, err
}
mounts[target] = mnt
}
values := make([]mount.Mount, 0, len(mounts))
for _, v := range mounts {
values = append(values, v)
}
return values, nil
return nil

Choose a reason for hiding this comment

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

just curious - why not keeping the convention of value, error instead of value or error

Copy link
Author

Choose a reason for hiding this comment

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

Hello,
It's not value or error but just error if any, nil otherwise.
This function now only do checks, it doesn't return any value (its name&definition has been changed accordingly).
See point "9. Return nil for Success" of Best Practices For Error Handling in Go.

}

func isAbsTarget(p string) bool {
39 changes: 26 additions & 13 deletions pkg/compose/secrets.go
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ import (
"bytes"
"context"
"fmt"
"os"
"strconv"
"time"

@@ -31,12 +32,24 @@ import (
func (s *composeService) injectSecrets(ctx context.Context, project *types.Project, service types.ServiceConfig, id string) error {
for _, config := range service.Secrets {
file := project.Secrets[config.Source]
if file.Environment == "" {
continue
content := file.Content

if file.Environment != "" {
env, ok := project.Environment[file.Environment]
if !ok {
return fmt.Errorf("environment variable %q required by secret %q is not set", file.Environment, file.Name)
}
content = env
} else if file.File != "" {
data, err := os.ReadFile(file.File)
if err != nil {
return err
}
content = string(data)
}

if service.ReadOnly {
return fmt.Errorf("cannot create secret %q in read-only service %s: `file` is the sole supported option", file.Name, service.Name)
if content == "" {
continue
}

if config.Target == "" {
@@ -45,11 +58,7 @@ func (s *composeService) injectSecrets(ctx context.Context, project *types.Proje
config.Target = "/run/secrets/" + config.Target
}

env, ok := project.Environment[file.Environment]
if !ok {
return fmt.Errorf("environment variable %q required by secret %q is not set", file.Environment, file.Name)
}
b, err := createTar(env, types.FileReferenceConfig(config))
b, err := createTar(content, types.FileReferenceConfig(config))
if err != nil {
return err
}
@@ -68,21 +77,25 @@ func (s *composeService) injectConfigs(ctx context.Context, project *types.Proje
for _, config := range service.Configs {
file := project.Configs[config.Source]
content := file.Content

if file.Environment != "" {
env, ok := project.Environment[file.Environment]
if !ok {
return fmt.Errorf("environment variable %q required by config %q is not set", file.Environment, file.Name)
}
content = env
} else if file.File != "" {
data, err := os.ReadFile(file.File)
if err != nil {
return err
}
content = string(data)
}

if content == "" {
continue
}

if service.ReadOnly {
return fmt.Errorf("cannot create config %q in read-only service %s: `file` is the sole supported option", file.Name, service.Name)
}

if config.Target == "" {
config.Target = "/" + config.Source
}
2 changes: 1 addition & 1 deletion pkg/e2e/volumes_test.go
Original file line number Diff line number Diff line change
@@ -53,7 +53,7 @@ func TestLocalComposeVolume(t *testing.T) {
res := c.RunDockerCmd(t, "inspect", "compose-e2e-volume-nginx2-1", "--format", "{{ json .Mounts }}")
output := res.Stdout()
assert.Assert(t, strings.Contains(output, `"Destination":"/usr/src/app/node_modules","Driver":"local","Mode":"z","RW":true,"Propagation":""`), output)
assert.Assert(t, strings.Contains(output, `"Destination":"/myconfig","Mode":"","RW":false,"Propagation":"rprivate"`), output)
assert.Assert(t, !strings.Contains(output, `"Destination":"/myconfig","Mode":"","RW":false,"Propagation":"rprivate"`), output)
})

t.Run("check config content", func(t *testing.T) {