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 1 commit
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
Next Next commit
Do not bind mount configs and secrets when source is a file but copy …
…it into the container

Signed-off-by: schaubl <schaubl@users.noreply.github.com>
  • Loading branch information
schaubl committed Feb 14, 2025
commit d9e531d8e433fbae4dfe6451b71d7b75492e47b4
103 changes: 28 additions & 75 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,52 +958,36 @@ 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

return nil
}

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

configsBaseDir := "/"
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 != "" {
@@ -1008,46 +998,26 @@ func buildContainerConfigMounts(p types.Project, s types.ServiceConfig) ([]mount
logrus.Warn("config `uid`, `gid` and `mode` are not supported, they will be ignored")
}

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 != "" {
@@ -1062,25 +1032,8 @@ func buildContainerSecretMounts(p types.Project, s types.ServiceConfig) ([]mount
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 {
33 changes: 27 additions & 6 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,7 +32,23 @@ 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 == "" {
content := file.Content

if file.Environment != "" {
env, ok := project.Environment[file.Environment]
if !ok {
return fmt.Errorf("environment variable %q required by file %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
}

@@ -41,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 file %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
}
@@ -64,13 +77,21 @@ 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 file %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
}
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) {