Skip to content

Commit

Permalink
Walk config path symlinks when verifying Containerd configuration
Browse files Browse the repository at this point in the history
  • Loading branch information
phillebaba committed Aug 28, 2024
1 parent 81cfa52 commit 2c8005c
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- [#535](https://github.com/spegel-org/spegel/pull/535) Fix Docker build casing checks.
- [#571](https://github.com/spegel-org/spegel/pull/571) Walk config path symlinks when verifying Containerd configuration.

### Security

Expand Down
31 changes: 28 additions & 3 deletions pkg/oci/containerd.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,16 +126,41 @@ func verifyStatusResponse(resp *runtimeapi.StatusResponse, configPath string) er
if cfg.Registry.ConfigPath == "" {
return errors.New("Containerd registry config path needs to be set for mirror configuration to take effect")
}
linkPaths, err := walkSymbolicLinks(configPath)
if err != nil {
return err

Check warning on line 131 in pkg/oci/containerd.go

View check run for this annotation

Codecov / codecov/patch

pkg/oci/containerd.go#L131

Added line #L131 was not covered by tests
}
paths := filepath.SplitList(cfg.Registry.ConfigPath)
for _, path := range paths {
if path != configPath {
continue
for _, linkedPath := range linkPaths {
if linkedPath == path {
return nil
}
}
return nil
}
return fmt.Errorf("Containerd registry config path is %s but needs to contain path %s for mirror configuration to take effect", cfg.Registry.ConfigPath, configPath)
}

func walkSymbolicLinks(path string) ([]string, error) {
fi, err := os.Lstat(path)
if err != nil {
return nil, err

Check warning on line 147 in pkg/oci/containerd.go

View check run for this annotation

Codecov / codecov/patch

pkg/oci/containerd.go#L147

Added line #L147 was not covered by tests
}
if fi.Mode()&os.ModeSymlink != os.ModeSymlink {
return []string{path}, nil
}
linkPath, err := os.Readlink(path)
if err != nil {
return nil, err

Check warning on line 154 in pkg/oci/containerd.go

View check run for this annotation

Codecov / codecov/patch

pkg/oci/containerd.go#L154

Added line #L154 was not covered by tests
}
paths, err := walkSymbolicLinks(linkPath)
if err != nil {
return nil, err

Check warning on line 158 in pkg/oci/containerd.go

View check run for this annotation

Codecov / codecov/patch

pkg/oci/containerd.go#L158

Added line #L158 was not covered by tests
}
paths = append(paths, path)
return paths, nil
}

func (c *Containerd) Subscribe(ctx context.Context) (<-chan ImageEvent, <-chan error, error) {
imgCh := make(chan ImageEvent)
errCh := make(chan error)
Expand Down
88 changes: 72 additions & 16 deletions pkg/oci/containerd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import (
"fmt"
iofs "io/fs"
"net/url"
"os"
"path/filepath"
"strings"
"testing"

eventtypes "github.com/containerd/containerd/api/events"
Expand All @@ -31,40 +34,54 @@ func TestNewContainerd(t *testing.T) {
func TestVerifyStatusResponse(t *testing.T) {
t.Parallel()

tmpDir := t.TempDir()

err := os.MkdirAll(filepath.Join(tmpDir, "etc", "target", "certs.d"), 0o777)
require.NoError(t, err)
err = os.MkdirAll(filepath.Join(tmpDir, "etc", "symlink"), 0o777)
require.NoError(t, err)
err = os.Symlink(filepath.Join(tmpDir, "etc", "target", "certs.d"), filepath.Join(tmpDir, "etc", "symlink", "certs.d"))
require.NoError(t, err)

tests := []struct {

Check failure on line 46 in pkg/oci/containerd_test.go

View workflow job for this annotation

GitHub Actions / lint

fieldalignment: struct with 64 pointer bytes could be 56 (govet)
name string
configPath string
configPaths []string
requiredConfigPath string
expectedErrMsg string
discardUnpackedLayers bool
}{
{
name: "empty config path",
configPath: "",
name: "single config path",
configPaths: []string{"/etc/containerd/certs.d"},
requiredConfigPath: "/etc/containerd/certs.d",
expectedErrMsg: "Containerd registry config path needs to be set for mirror configuration to take effect",
},
{
name: "single config path",
configPath: "/etc/containerd/certs.d",
name: "multiple config paths",
configPaths: []string{"/etc/containerd/certs.d", "/etc/docker/certs.d"},
requiredConfigPath: "/etc/containerd/certs.d",
},
{
name: "missing single config path",
configPath: "/etc/containerd/certs.d",
requiredConfigPath: "/var/lib/containerd/certs.d",
expectedErrMsg: "Containerd registry config path is /etc/containerd/certs.d but needs to contain path /var/lib/containerd/certs.d for mirror configuration to take effect",
name: "symlinked config path",
configPaths: []string{"/etc/target/certs.d"},
requiredConfigPath: "/etc/symlink/certs.d",
},
{
name: "multiple config paths",
configPath: "/etc/containerd/certs.d:/etc/docker/certs.d",
name: "empty config path",
configPaths: nil,
requiredConfigPath: "/etc/containerd/certs.d",
expectedErrMsg: "Containerd registry config path needs to be set for mirror configuration to take effect",
},
{
name: "missing single config path",
configPaths: []string{"/etc/containerd/certs.d"},
requiredConfigPath: "/var/lib/containerd/certs.d",
expectedErrMsg: fmt.Sprintf("Containerd registry config path is %[1]s/etc/containerd/certs.d but needs to contain path %[1]s/var/lib/containerd/certs.d for mirror configuration to take effect", tmpDir),
},
{
name: "missing multiple config paths",
configPath: "/etc/containerd/certs.d:/etc/docker/certs.d",
configPaths: []string{"/etc/containerd/certs.d", "/etc/docker/certs.d"},
requiredConfigPath: "/var/lib/containerd/certs.d",
expectedErrMsg: "Containerd registry config path is /etc/containerd/certs.d:/etc/docker/certs.d but needs to contain path /var/lib/containerd/certs.d for mirror configuration to take effect",
expectedErrMsg: fmt.Sprintf("Containerd registry config path is %[1]s/etc/containerd/certs.d:%[1]s/etc/docker/certs.d but needs to contain path %[1]s/var/lib/containerd/certs.d for mirror configuration to take effect", tmpDir),
},
{
name: "discard unpacked layers enabled",
Expand All @@ -76,12 +93,21 @@ func TestVerifyStatusResponse(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

tmpConfigPaths := []string{}
for _, configPath := range tt.configPaths {
tmpConfigPaths = append(tmpConfigPaths, filepath.Join(tmpDir, configPath))
}
tmpConfigPath := strings.Join(tmpConfigPaths, string(os.PathListSeparator))
tmpRequiredPath := filepath.Join(tmpDir, tt.requiredConfigPath)
err := os.MkdirAll(tmpRequiredPath, 0o777)
require.NoError(t, err)

resp := &runtimeapi.StatusResponse{
Info: map[string]string{
"config": fmt.Sprintf(`{"registry": {"configPath": %q}, "containerd": {"runtimes":{"discardUnpackedLayers": %v}}}`, tt.configPath, tt.discardUnpackedLayers),
"config": fmt.Sprintf(`{"registry": {"configPath": %q}, "containerd": {"runtimes":{"discardUnpackedLayers": %v}}}`, tmpConfigPath, tt.discardUnpackedLayers),
},
}
err := verifyStatusResponse(resp, tt.requiredConfigPath)
err = verifyStatusResponse(resp, tmpRequiredPath)
if tt.expectedErrMsg != "" {
require.EqualError(t, err, tt.expectedErrMsg)
return
Expand All @@ -91,6 +117,36 @@ func TestVerifyStatusResponse(t *testing.T) {
}
}

func TestWalkSymbolicLinks(t *testing.T) {
t.Parallel()

tmpDir := t.TempDir()
targetPath := filepath.Join(tmpDir, "data.txt")
err := os.WriteFile(targetPath, []byte("hello world"), 0o777)
require.NoError(t, err)
firstOrderPath := filepath.Join(tmpDir, "first.txt")
err = os.Symlink(targetPath, firstOrderPath)
require.NoError(t, err)
secondOrderPath := filepath.Join(tmpDir, "second.txt")
err = os.Symlink(firstOrderPath, secondOrderPath)
require.NoError(t, err)

// Second order symlink
paths, err := walkSymbolicLinks(secondOrderPath)
require.NoError(t, err)
require.Equal(t, paths, []string{targetPath, firstOrderPath, secondOrderPath})

Check failure on line 137 in pkg/oci/containerd_test.go

View workflow job for this annotation

GitHub Actions / lint

expected-actual: need to reverse actual and expected values (testifylint)

// First order symlink
paths, err = walkSymbolicLinks(firstOrderPath)
require.NoError(t, err)
require.Equal(t, paths, []string{targetPath, firstOrderPath})

Check failure on line 142 in pkg/oci/containerd_test.go

View workflow job for this annotation

GitHub Actions / lint

expected-actual: need to reverse actual and expected values (testifylint)

// No symnlink
paths, err = walkSymbolicLinks(targetPath)
require.NoError(t, err)
require.Equal(t, paths, []string{targetPath})

Check failure on line 147 in pkg/oci/containerd_test.go

View workflow job for this annotation

GitHub Actions / lint

expected-actual: need to reverse actual and expected values (testifylint)
}

func TestCreateFilter(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit 2c8005c

Please sign in to comment.