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

Enable configs.file's on remote docker hosts #11871

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
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
121 changes: 0 additions & 121 deletions pkg/compose/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -918,130 +918,9 @@ func fillBindMounts(p types.Project, s types.ServiceConfig, m map[string]mount.M
m[bindMount.Target] = bindMount
}

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

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

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

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
}

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

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

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

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
}
mounts[target] = bindMount
}
values := make([]mount.Mount, 0, len(mounts))
for _, v := range mounts {
values = append(values, v)
}
return values, nil
}

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

secretsDir := "/run/secrets/"
for _, secret := range s.Secrets {
target := secret.Target
if secret.Target == "" {
target = secretsDir + secret.Source
} else if !isAbsTarget(secret.Target) {
target = secretsDir + secret.Target
}

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

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

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

if definedSecret.Environment != "" {
continue
}

mnt, err := buildMount(p, types.ServiceVolumeConfig{
Type: types.VolumeTypeBind,
Source: definedSecret.File,
Target: target,
ReadOnly: true,
})
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
}

func isAbsTarget(p string) bool {
return isUnixAbs(p) || isWindowsAbs(p)
}
Expand Down
202 changes: 155 additions & 47 deletions pkg/compose/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ import (
"bytes"
"context"
"fmt"
"io"
"io/fs"
"os"
"path/filepath"
"strconv"
"time"

Expand All @@ -30,27 +34,36 @@ 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
}

if config.Target == "" {
config.Target = "/run/secrets/" + config.Source
} else if !isAbsTarget(config.Target) {
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)
file := project.Secrets[config.Source]
var tarArchive bytes.Buffer
var err error
switch {
Copy link
Author

Choose a reason for hiding this comment

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

Note: not handling file.Content as this seems to be rejected some other place, causing the printout "validating docker-compose.yml: secrets.content_secret Additional property content is not allowed"

Copy link

Choose a reason for hiding this comment

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

I think it comes from the compose-spec.
There is no content property in secrets, only in configs.
This correspond to the doc where secrets can only use file or environment.
I assume the reason is to prevent writing secrets value directly in the compose file.

case file.File != "":
tarArchive, err = createTarArchiveOf(file.File, types.FileReferenceConfig(config))
case 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)
}
tarArchive, err = createTarredFileOf(env, types.FileReferenceConfig(config))
}
b, err := createTar(env, types.FileReferenceConfig(config))

if err != nil {
return err
}

err = s.apiClient().CopyToContainer(ctx, id, "/", &b, moby.CopyToContainerOptions{
// secret was handled elsewhere (e.g it was external)
if tarArchive.Len() == 0 {
continue
}

err = s.apiClient().CopyToContainer(ctx, id, "/", &tarArchive, moby.CopyToContainerOptions{
CopyUIDGID: config.UID != "" || config.GID != "",
})
if err != nil {
Expand All @@ -62,29 +75,36 @@ func (s *composeService) injectSecrets(ctx context.Context, project *types.Proje

func (s *composeService) injectConfigs(ctx context.Context, project *types.Project, service types.ServiceConfig, id string) error {
for _, config := range service.Configs {
if config.Target == "" {
config.Target = "/" + config.Source
}

file := project.Configs[config.Source]
content := file.Content
if file.Environment != "" {
var tarArchive bytes.Buffer
var err error
switch {
case file.File != "":
tarArchive, err = createTarArchiveOf(file.File, types.FileReferenceConfig(config))
case file.Content != "":
tarArchive, err = createTarredFileOf(file.Content, types.FileReferenceConfig(config))
case 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
}
if content == "" {
continue
}

if config.Target == "" {
config.Target = "/" + config.Source
tarArchive, err = createTarredFileOf(env, types.FileReferenceConfig(config))
}

b, err := createTar(content, types.FileReferenceConfig(config))
if err != nil {
return err
}

err = s.apiClient().CopyToContainer(ctx, id, "/", &b, moby.CopyToContainerOptions{
// config was handled elsewhere (e.g it was external)
if tarArchive.Len() == 0 {
continue
}

err = s.apiClient().CopyToContainer(ctx, id, "/", &tarArchive, moby.CopyToContainerOptions{
CopyUIDGID: config.UID != "" || config.GID != "",
})
if err != nil {
Expand All @@ -94,47 +114,135 @@ func (s *composeService) injectConfigs(ctx context.Context, project *types.Proje
return nil
}

func createTar(env string, config types.FileReferenceConfig) (bytes.Buffer, error) {
value := []byte(env)
func createTarredFileOf(value string, config types.FileReferenceConfig) (bytes.Buffer, error) {
mode, uid, gid, err := makeTarFileEntryParams(config)
if err != nil {
return bytes.Buffer{}, fmt.Errorf("failed parsing target file parameters")
}

b := bytes.Buffer{}
tarWriter := tar.NewWriter(&b)
mode := uint32(0o444)
valueAsBytes := []byte(value)
header := &tar.Header{
Name: config.Target,
Size: int64(len(valueAsBytes)),
Mode: mode,
ModTime: time.Now(),
Uid: uid,
Gid: gid,
}
err = tarWriter.WriteHeader(header)
if err != nil {
return bytes.Buffer{}, err
}
_, err = tarWriter.Write(valueAsBytes)
if err != nil {
return bytes.Buffer{}, err
}
err = tarWriter.Close()
return b, err
}

func createTarArchiveOf(path string, config types.FileReferenceConfig) (bytes.Buffer, error) {
// need to treat files and directories differently
fi, err := os.Stat(path)
if err != nil {
return bytes.Buffer{}, err
}

// if path is not directory, try to treat it as a file by reading its value
if !fi.IsDir() {
buf, err := os.ReadFile(path)
if err == nil {
return createTarredFileOf(string(buf), config)
}
}

mode, uid, gid, err := makeTarFileEntryParams(config)
if err != nil {
return bytes.Buffer{}, fmt.Errorf("failed parsing target file parameters")
}

subdir := os.DirFS(path)
b := bytes.Buffer{}
tarWriter := tar.NewWriter(&b)

// build the tar by walking instead of using archive/tar.Writer.AddFS to be able to adjust mode, gid and uid
err = fs.WalkDir(subdir, ".", func(filePath string, d fs.DirEntry, err error) error {
header := &tar.Header{
Name: filepath.Join(config.Target, filePath),
Mode: mode,
Copy link
Author

Choose a reason for hiding this comment

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

❓ Should directories have the same mode as files? Or should they perhaps have the exec bit set for the owner/group/other to access them?

ModTime: time.Now(),
Uid: uid,
Gid: gid,
}

if d.IsDir() {
// tar requires that directory headers ends with a slash
header.Name = header.Name + "/"
err = tarWriter.WriteHeader(header)
if err != nil {
return fmt.Errorf("failed writing tar header of directory %v while walking diretory structure, error was: %w", filePath, err)
}
} else {
f, err := subdir.Open(filePath)
if err != nil {
return err
}
defer f.Close()

valueAsBytes, err := io.ReadAll(f)
if err != nil {
return fmt.Errorf("failed reading file %v for to send to container, error was: %w", filePath, err)
}

header.Size = int64(len(valueAsBytes))
err = tarWriter.WriteHeader(header)
if err != nil {
return fmt.Errorf("failed writing tar header for file %v while walking diretory structure, error was: %w", filePath, err)
}

_, err = tarWriter.Write(valueAsBytes)
if err != nil {
return fmt.Errorf("failed writing file content of %v into tar archive while walking directory structure, error was: %w", filePath, err)
}
}

return nil
})

if err != nil {
return bytes.Buffer{}, fmt.Errorf("failed building tar archive while walking config directory structure, error was: %w", err)
}

err = tarWriter.Close()
if err != nil {
return bytes.Buffer{}, fmt.Errorf("failed closing tar archive after writing, error was: %w", err)
}

return b, err
}

func makeTarFileEntryParams(config types.FileReferenceConfig) (mode int64, uid, gid int, err error) {
mode = 0o444
Copy link
Author

Choose a reason for hiding this comment

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

❓ should the default mode be different for secrets compared to configs?

Copy link

@schaubl schaubl Jul 17, 2024

Choose a reason for hiding this comment

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

I think it would make sense to remove at least public read (0o440) for secrets

if config.Mode != nil {
mode = *config.Mode
mode = int64(*config.Mode)
}

var uid, gid int
if config.UID != "" {
v, err := strconv.Atoi(config.UID)
if err != nil {
return b, err
return 0, 0, 0, err
}
uid = v
}
if config.GID != "" {
v, err := strconv.Atoi(config.GID)
if err != nil {
return b, err
return 0, 0, 0, err
}
gid = v
}

header := &tar.Header{
Name: config.Target,
Size: int64(len(value)),
Mode: int64(mode),
ModTime: time.Now(),
Uid: uid,
Gid: gid,
}
err := tarWriter.WriteHeader(header)
if err != nil {
return bytes.Buffer{}, err
}
_, err = tarWriter.Write(value)
if err != nil {
return bytes.Buffer{}, err
}
err = tarWriter.Close()
return b, err
return mode, uid, gid, nil
}