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

remove restriction for extends to refer to another resource #591

Merged
merged 1 commit into from
Feb 29, 2024
Merged
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
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