Skip to content

Commit

Permalink
remove restriction for extends to refer to another resource
Browse files Browse the repository at this point in the history
Signed-off-by: Nicolas De Loof <[email protected]>
  • Loading branch information
ndeloof committed Feb 29, 2024
1 parent a0507e9 commit e2eab1f
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 52 deletions.
30 changes: 0 additions & 30 deletions loader/extends.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"fmt"
"path/filepath"
"strings"

"github.com/compose-spec/compose-go/v2/consts"
"github.com/compose-spec/compose-go/v2/override"
Expand Down Expand Up @@ -106,11 +105,6 @@ func applyServiceExtends(ctx context.Context, name string, services map[string]a
}
source := deepClone(base).(map[string]any)

err = validateExtendSource(source, ref)
if err != nil {
return nil, err
}

for _, processor := range post {
processor.Apply(map[string]any{
"services": map[string]any{
Expand All @@ -127,30 +121,6 @@ func applyServiceExtends(ctx context.Context, name string, services map[string]a
return merged, nil
}

// validateExtendSource check the source for `extends` doesn't refer to another container/service
func validateExtendSource(source map[string]any, ref string) error {
forbidden := []string{"links", "volumes_from", "depends_on"}
for _, key := range forbidden {
if _, ok := source[key]; ok {
return fmt.Errorf("service %q can't be used with `extends` as it declare `%s`", ref, key)
}
}

sharedNamespace := []string{"network_mode", "ipc", "pid", "net", "cgroup", "userns_mode", "uts"}
for _, key := range sharedNamespace {
if v, ok := source[key]; ok {
val := v.(string)
if strings.HasPrefix(val, types.ContainerPrefix) {
return fmt.Errorf("service %q can't be used with `extends` as it shares `%s` with another container", ref, key)
}
if strings.HasPrefix(val, types.ServicePrefix) {
return fmt.Errorf("service %q can't be used with `extends` as it shares `%s` with another service", ref, key)
}
}
}
return nil
}

func getExtendsBaseFromFile(ctx context.Context, name string, path string, opts *Options, ct *cycleTracker) (map[string]any, error) {
for _, loader := range opts.ResourceLoaders {
if !loader.Accept(path) {
Expand Down
45 changes: 25 additions & 20 deletions loader/extends_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ services:
assert.Equal(t, extendsCount, 3)
}

func TestRejectExtendsWithServiceRef(t *testing.T) {
func TestExtendsWithServiceRef(t *testing.T) {
tests := []struct {
name string
yaml string
Expand All @@ -317,54 +317,48 @@ func TestRejectExtendsWithServiceRef(t *testing.T) {
yaml: `
name: test-extends_with_volumes_from
services:
foo:
volumes_from:
- zot
bar:
bar:
extends:
service: foo
file: ./testdata/extends/depends_on.yaml
service: with_volumes_from
`,
wantErr: "service \"foo\" can't be used with `extends` as it declare `volumes_from`",
wantErr: `service "bar" depends on undefined service "zot"`,
},
{
name: "depends_on",
yaml: `
name: test-extends_with_depends_on
services:
foo:
depends_on:
- zot
bar:
extends:
service: foo
file: ./testdata/extends/depends_on.yaml
service: with_depends_on
`,
wantErr: "service \"foo\" can't be used with `extends` as it declare `depends_on`",
wantErr: `service "bar" depends on undefined service "zot"`,
},
{
name: "shared ipc",
yaml: `
name: test-extends_with_shared_ipc
services:
foo:
ipc: "service:zot"
bar:
extends:
service: foo
file: ./testdata/extends/depends_on.yaml
service: with_ipc
`,
wantErr: "service \"foo\" can't be used with `extends` as it shares `ipc` with another service",
wantErr: `service "bar" depends on undefined service "zot"`,
},
{
name: "shared network_mode",
yaml: `
name: test-extends_with_shared_network_mode
services:
foo:
network_mode: "container:123abc"
bar:
extends:
service: foo
file: ./testdata/extends/depends_on.yaml
service: with_network_mode
`,
wantErr: "service \"foo\" can't be used with `extends` as it shares `network_mode` with another container",
wantErr: `service "bar" depends on undefined service "zot"`,
},
}

Expand All @@ -376,6 +370,17 @@ services:
}},
})
assert.ErrorContains(t, err, tt.wantErr)

// Do the same but with a local `zot` service matching the imported reference
_, err = LoadWithContext(context.Background(), types.ConfigDetails{
ConfigFiles: []types.ConfigFile{{
Content: []byte(tt.yaml + `
zot:
image: zot
`),
}},
})
assert.NilError(t, err)
})
}
}
Expand Down
21 changes: 21 additions & 0 deletions loader/testdata/extends/depends_on.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
services:
with_depends_on:
image: foo
depends_on:
- zot

with_volumes_from:
image: foo
volumes_from:
- zot

with_ipc:
image: foo
ipc: "service:zot"

with_network_mode:
image: foo
network_mode: "service:zot"

zot:
image: hidden
2 changes: 1 addition & 1 deletion loader/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func checkConsistency(project *types.Project) error {

for dependedService := range s.DependsOn {
if _, err := project.GetService(dependedService); err != nil {
return fmt.Errorf("service %q depends on undefined service %s: %w", s.Name, dependedService, errdefs.ErrInvalid)
return fmt.Errorf("service %q depends on undefined service %q: %w", s.Name, dependedService, errdefs.ErrInvalid)
}
}
// Check there isn't a cycle in depends_on declarations
Expand Down
2 changes: 1 addition & 1 deletion loader/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ func TestValidateDependsOn(t *testing.T) {
},
}
err := checkConsistency(&project)
assert.Error(t, err, `service "myservice" depends on undefined service missingservice: invalid compose project`)
assert.Error(t, err, `service "myservice" depends on undefined service "missingservice": invalid compose project`)
}

func TestValidateContainerName(t *testing.T) {
Expand Down

0 comments on commit e2eab1f

Please sign in to comment.