diff --git a/e2e/tests/up-features/testdata/docker-features-hooks-order/.devcontainer.json b/e2e/tests/up-features/testdata/docker-features-hooks-order/.devcontainer.json new file mode 100644 index 000000000..973474ec0 --- /dev/null +++ b/e2e/tests/up-features/testdata/docker-features-hooks-order/.devcontainer.json @@ -0,0 +1,8 @@ +{ + "name": "Feature Hook Order Test", + "image": "ghcr.io/devsy-org/test-images/base:ubuntu", + "features": { + "./.devcontainer/features/test-order": {} + }, + "onCreateCommand": "echo 'image' >> /tmp/hook-order.txt" +} diff --git a/e2e/tests/up-features/testdata/docker-features-hooks-order/.devcontainer/features/test-order/devcontainer-feature.json b/e2e/tests/up-features/testdata/docker-features-hooks-order/.devcontainer/features/test-order/devcontainer-feature.json new file mode 100644 index 000000000..fd5a4076e --- /dev/null +++ b/e2e/tests/up-features/testdata/docker-features-hooks-order/.devcontainer/features/test-order/devcontainer-feature.json @@ -0,0 +1,6 @@ +{ + "id": "test-order", + "version": "1.0.0", + "name": "Test Hook Order", + "onCreateCommand": "echo 'feature' >> /tmp/hook-order.txt" +} diff --git a/e2e/tests/up-features/testdata/docker-features-hooks-order/.devcontainer/features/test-order/install.sh b/e2e/tests/up-features/testdata/docker-features-hooks-order/.devcontainer/features/test-order/install.sh new file mode 100644 index 000000000..825798499 --- /dev/null +++ b/e2e/tests/up-features/testdata/docker-features-hooks-order/.devcontainer/features/test-order/install.sh @@ -0,0 +1,2 @@ +#!/bin/sh +echo "test-order feature installed" diff --git a/e2e/tests/up-features/up_features.go b/e2e/tests/up-features/up_features.go index 71efb6d75..fcfec1278 100644 --- a/e2e/tests/up-features/up_features.go +++ b/e2e/tests/up-features/up_features.go @@ -52,6 +52,31 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features", "suite framework.ExpectEqual(strings.TrimSpace(out), "feature-postStart") }, ginkgo.SpecTimeout(framework.TimeoutShort())) + ginkgo.It("lifecycle hooks order feature before image", func(ctx context.Context) { + f, err := setupDockerProvider(initialDir+"/bin", "docker") + framework.ExpectNoError(err) + + tempDir, err := framework.CopyToTempDir( + "tests/up-features/testdata/docker-features-hooks-order", + ) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir) + + wsName := filepath.Base(tempDir) + ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName) + + err = f.DevsyUp(ctx, tempDir) + framework.ExpectNoError(err) + + out, err := f.DevsySSH(ctx, wsName, "cat /tmp/hook-order.txt") + framework.ExpectNoError(err) + + lines := strings.Split(strings.TrimSpace(out), "\n") + gomega.Expect(lines).To(gomega.HaveLen(2)) + gomega.Expect(lines[0]).To(gomega.Equal("feature")) + gomega.Expect(lines[1]).To(gomega.Equal("image")) + }, ginkgo.SpecTimeout(framework.TimeoutShort())) + ginkgo.It("http headers download", func(ctx context.Context) { server := ghttp.NewServer() ginkgo.DeferCleanup(server.Close) diff --git a/pkg/devcontainer/config/merge.go b/pkg/devcontainer/config/merge.go index 9e04da23c..febbd207d 100644 --- a/pkg/devcontainer/config/merge.go +++ b/pkg/devcontainer/config/merge.go @@ -3,6 +3,7 @@ package config import ( "fmt" "maps" + "slices" "strconv" "strings" "unicode" @@ -349,10 +350,10 @@ func mergeLifestyleHooks( m func(entry *ImageMetadata) types.LifecycleHook, ) []types.LifecycleHook { var out []types.LifecycleHook - for _, entry := range entries { + for _, entry := range slices.Backward(entries) { val := m(entry) if len(val) > 0 { - out = append(out, m(entry)) + out = append(out, val) } } return out diff --git a/pkg/devcontainer/config/merge_test.go b/pkg/devcontainer/config/merge_test.go index 95038a77e..09c0d3759 100644 --- a/pkg/devcontainer/config/merge_test.go +++ b/pkg/devcontainer/config/merge_test.go @@ -3,6 +3,8 @@ package config import ( "slices" "testing" + + "github.com/devsy-org/devsy/pkg/types" ) const testPortRange = "3000-3002" @@ -342,3 +344,141 @@ func TestMergeForwardPorts_InvalidRangeSkipped(t *testing.T) { } } } + +func TestMergeLifestyleHooks_FeatureBeforeImage(t *testing.T) { + featureHook := types.LifecycleHook{"feature-cmd": {"echo feature"}} + imageHook := types.LifecycleHook{"image-cmd": {"echo image"}} + + // Simulate reversed entries as passed to mergeLifestyleHooks: + // [base_image_entry, feature_entry] + entries := []*ImageMetadata{ + {DevContainerActions: DevContainerActions{OnCreateCommand: imageHook}}, + {DevContainerActions: DevContainerActions{OnCreateCommand: featureHook}}, + } + + got := mergeLifestyleHooks(entries, func(e *ImageMetadata) types.LifecycleHook { + return e.OnCreateCommand + }) + + if len(got) != 2 { + t.Fatalf("expected 2 hooks, got %d", len(got)) + } + if _, ok := got[0]["feature-cmd"]; !ok { + t.Errorf("expected feature hook first, got %v", got[0]) + } + if _, ok := got[1]["image-cmd"]; !ok { + t.Errorf("expected image hook second, got %v", got[1]) + } +} + +func TestMergeLifestyleHooks_AllHookTypes(t *testing.T) { + featureHook := types.LifecycleHook{"feat": {"echo feat"}} + imageHook := types.LifecycleHook{"img": {"echo img"}} + + entries := []*ImageMetadata{ + {DevContainerActions: DevContainerActions{ + OnCreateCommand: imageHook, + UpdateContentCommand: imageHook, + PostCreateCommand: imageHook, + PostStartCommand: imageHook, + PostAttachCommand: imageHook, + }}, + {DevContainerActions: DevContainerActions{ + OnCreateCommand: featureHook, + UpdateContentCommand: featureHook, + PostCreateCommand: featureHook, + PostStartCommand: featureHook, + PostAttachCommand: featureHook, + }}, + } + + hookExtractors := []struct { + name string + fn func(e *ImageMetadata) types.LifecycleHook + }{ + {"onCreateCommand", func(e *ImageMetadata) types.LifecycleHook { + return e.OnCreateCommand + }}, + {"updateContentCommand", func(e *ImageMetadata) types.LifecycleHook { + return e.UpdateContentCommand + }}, + {"postCreateCommand", func(e *ImageMetadata) types.LifecycleHook { + return e.PostCreateCommand + }}, + {"postStartCommand", func(e *ImageMetadata) types.LifecycleHook { + return e.PostStartCommand + }}, + {"postAttachCommand", func(e *ImageMetadata) types.LifecycleHook { + return e.PostAttachCommand + }}, + } + + for _, tc := range hookExtractors { + t.Run(tc.name, func(t *testing.T) { + got := mergeLifestyleHooks(entries, tc.fn) + if len(got) != 2 { + t.Fatalf("expected 2 hooks, got %d", len(got)) + } + if _, ok := got[0]["feat"]; !ok { + t.Errorf("expected feature hook first, got %v", got[0]) + } + if _, ok := got[1]["img"]; !ok { + t.Errorf("expected image hook second, got %v", got[1]) + } + }) + } +} + +func TestMergeLifestyleHooks_SkipsEmpty(t *testing.T) { + featureHook := types.LifecycleHook{"feat": {"echo feat"}} + + entries := []*ImageMetadata{ + {}, + {DevContainerActions: DevContainerActions{OnCreateCommand: featureHook}}, + {}, + } + + got := mergeLifestyleHooks(entries, func(e *ImageMetadata) types.LifecycleHook { + return e.OnCreateCommand + }) + + if len(got) != 1 { + t.Fatalf("expected 1 hook, got %d", len(got)) + } + if _, ok := got[0]["feat"]; !ok { + t.Errorf("expected feature hook, got %v", got[0]) + } +} + +func TestMergeLifestyleHooks_MultipleFeatures(t *testing.T) { + imageHook := types.LifecycleHook{"img": {"echo img"}} + feature1Hook := types.LifecycleHook{"feat1": {"echo feat1"}} + feature2Hook := types.LifecycleHook{"feat2": {"echo feat2"}} + + // After ReverseSlice in MergeConfiguration, entries are: + // [base_image, feature2 (last applied), feature1 (first applied), user_config] + // mergeLifestyleHooks iterates in reverse producing: + // user_config hooks, feature1 hooks, feature2 hooks, base_image hooks + entries := []*ImageMetadata{ + {DevContainerActions: DevContainerActions{OnCreateCommand: imageHook}}, + {DevContainerActions: DevContainerActions{OnCreateCommand: feature2Hook}}, + {DevContainerActions: DevContainerActions{OnCreateCommand: feature1Hook}}, + } + + got := mergeLifestyleHooks(entries, func(e *ImageMetadata) types.LifecycleHook { + return e.OnCreateCommand + }) + + if len(got) != 3 { + t.Fatalf("expected 3 hooks, got %d", len(got)) + } + if _, ok := got[0]["feat1"]; !ok { + t.Errorf("expected feature1 hook first, got %v", got[0]) + } + if _, ok := got[1]["feat2"]; !ok { + t.Errorf("expected feature2 hook second, got %v", got[1]) + } + if _, ok := got[2]["img"]; !ok { + t.Errorf("expected image hook last, got %v", got[2]) + } +}