From c53a6c712b0ad262f3fda0ab7184adf2dbc4630f Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 4 May 2026 07:18:28 -0500 Subject: [PATCH] feat(feature): resolve legacy IDs during dependency resolution When a feature's dependsOn references an old/legacy ID, the resolver now checks the legacyIds field of all known features before attempting to fetch from a registry. This maps deprecated feature names to their current counterparts without requiring registry-level redirects. --- cmd/runusercommands.go | 2 +- .../.devcontainer.json | 7 ++ .../devcontainer-feature.json | 9 ++ .../features/consumer-feature/install.sh | 17 +++ .../current-feature/devcontainer-feature.json | 7 ++ .../features/current-feature/install.sh | 11 ++ e2e/tests/up-features/up_features.go | 27 +++++ pkg/devcontainer/feature/extend.go | 27 +++++ pkg/devcontainer/feature/extend_test.go | 103 ++++++++++++++++++ 9 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/.devcontainer.json create mode 100644 e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/consumer-feature/devcontainer-feature.json create mode 100644 e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/consumer-feature/install.sh create mode 100644 e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/current-feature/devcontainer-feature.json create mode 100644 e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/current-feature/install.sh diff --git a/cmd/runusercommands.go b/cmd/runusercommands.go index de1e77f08..179a6a824 100644 --- a/cmd/runusercommands.go +++ b/cmd/runusercommands.go @@ -90,7 +90,7 @@ func (cmd *RunUserCommandsCmd) Run(ctx context.Context) error { user := devcconfig.GetRemoteUser(result) log.Infof("lifecycle commands completed for container %s", params.containerID) - _ = devcconfig.WriteResultJSON(os.Stderr, params.containerID, user, params.workdir) + _ = devcconfig.WriteResultJSON(os.Stderr, params.containerID, user, params.workdir, nil) return nil } diff --git a/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/.devcontainer.json b/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/.devcontainer.json new file mode 100644 index 000000000..03d6c5589 --- /dev/null +++ b/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/.devcontainer.json @@ -0,0 +1,7 @@ +{ + "image": "ghcr.io/devsy-org/test-images/base:alpine", + "features": { + "./features/current-feature": {}, + "./features/consumer-feature": {} + } +} diff --git a/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/consumer-feature/devcontainer-feature.json b/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/consumer-feature/devcontainer-feature.json new file mode 100644 index 000000000..fec371d51 --- /dev/null +++ b/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/consumer-feature/devcontainer-feature.json @@ -0,0 +1,9 @@ +{ + "id": "consumer-feature", + "version": "1.0.0", + "name": "Consumer Feature", + "description": "A feature that depends on another feature using its legacy ID", + "dependsOn": { + "old-feature-name": {} + } +} diff --git a/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/consumer-feature/install.sh b/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/consumer-feature/install.sh new file mode 100644 index 000000000..56697a073 --- /dev/null +++ b/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/consumer-feature/install.sh @@ -0,0 +1,17 @@ +#!/bin/bash +set -e + +echo "Installing consumer-feature" + +cat >/usr/local/bin/test-legacy-resolution <<'EOF' +#!/bin/bash +if command -v legacy-resolved >/dev/null 2>&1; then + echo "SUCCESS: legacy ID resolution worked" + legacy-resolved +else + echo "FAILURE: legacy-resolved command not found" + exit 1 +fi +EOF + +chmod +x /usr/local/bin/test-legacy-resolution diff --git a/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/current-feature/devcontainer-feature.json b/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/current-feature/devcontainer-feature.json new file mode 100644 index 000000000..23cadc14c --- /dev/null +++ b/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/current-feature/devcontainer-feature.json @@ -0,0 +1,7 @@ +{ + "id": "current-feature", + "version": "1.0.0", + "name": "Current Feature", + "description": "A feature with legacy IDs for testing resolution", + "legacyIds": ["old-feature-name", "ancient-feature-name"] +} diff --git a/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/current-feature/install.sh b/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/current-feature/install.sh new file mode 100644 index 000000000..075ad9a85 --- /dev/null +++ b/e2e/tests/up-features/testdata/docker-features-legacy-id-resolution/features/current-feature/install.sh @@ -0,0 +1,11 @@ +#!/bin/bash +set -e + +echo "Installing current-feature" + +cat >/usr/local/bin/legacy-resolved <<'EOF' +#!/bin/bash +echo "legacy-id-resolved-successfully" +EOF + +chmod +x /usr/local/bin/legacy-resolved diff --git a/e2e/tests/up-features/up_features.go b/e2e/tests/up-features/up_features.go index fcfec1278..06e1ba541 100644 --- a/e2e/tests/up-features/up_features.go +++ b/e2e/tests/up-features/up_features.go @@ -576,6 +576,33 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features", "suite framework.ExpectEqual(strings.TrimSpace(out), "ubuntu") }, ginkgo.SpecTimeout(framework.TimeoutShort())) + ginkgo.It( + "should resolve legacy feature IDs in dependsOn", + ginkgo.Label("features", "depends-on", "legacy-id"), + func(ctx context.Context) { + f, err := setupDockerProvider(initialDir+"/bin", "docker") + framework.ExpectNoError(err) + + tempDir, err := framework.CopyToTempDir( + "tests/up-features/testdata/docker-features-legacy-id-resolution", + ) + 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, "test-legacy-resolution") + framework.ExpectNoError(err) + gomega.Expect(out).To(gomega.ContainSubstring("SUCCESS: legacy ID resolution worked")) + gomega.Expect(out).To(gomega.ContainSubstring("legacy-id-resolved-successfully")) + }, + ginkgo.SpecTimeout(framework.TimeoutShort()), + ) + ginkgo.It( "should reject overrideFeatureInstallOrder that violates dependsOn", ginkgo.Label("features", "override"), diff --git a/pkg/devcontainer/feature/extend.go b/pkg/devcontainer/feature/extend.go index 605467ca8..8e07166b2 100644 --- a/pkg/devcontainer/feature/extend.go +++ b/pkg/devcontainer/feature/extend.go @@ -365,6 +365,7 @@ type featureDependencyResolver struct { resolved map[string]*config.FeatureSet visiting map[string]bool processor *featureProcessor + legacyMap map[string]string } func (r *featureDependencyResolver) resolveFeatureDependency( @@ -386,6 +387,13 @@ func (r *featureDependencyResolver) resolveFeatureDependency( normalizedDepID := normalizeFeatureID(depID) depFeatureSet, exists := r.features[normalizedDepID] if !exists { + if currentID, legacyMatch := r.legacyMap[normalizedDepID]; legacyMatch { + log.Debugf("resolved legacy ID %s to current feature %s", depID, currentID) + depFeatureSet = r.features[currentID] + normalizedDepID = currentID + } + } + if depFeatureSet == nil { log.Debugf("installing dependency feature %s", depID) var err error depFeatureSet, err = r.processor.processFeature(depID, depOptions) @@ -393,6 +401,7 @@ func (r *featureDependencyResolver) resolveFeatureDependency( return fmt.Errorf("failed to resolve dependency %s: %w", depID, err) } r.features[normalizedDepID] = depFeatureSet + r.rebuildLegacyMap() } err := r.resolveFeatureDependency(normalizedDepID, depFeatureSet) @@ -405,6 +414,10 @@ func (r *featureDependencyResolver) resolveFeatureDependency( return nil } +func (r *featureDependencyResolver) rebuildLegacyMap() { + r.legacyMap = buildLegacyIDMap(r.features) +} + func resolveDependencies( processor *featureProcessor, features map[string]*config.FeatureSet, @@ -414,6 +427,7 @@ func resolveDependencies( resolved: make(map[string]*config.FeatureSet), visiting: make(map[string]bool), processor: processor, + legacyMap: buildLegacyIDMap(features), } for featureID, featureSet := range features { @@ -426,6 +440,19 @@ func resolveDependencies( return resolver.resolved, nil } +func buildLegacyIDMap(features map[string]*config.FeatureSet) map[string]string { + legacyMap := make(map[string]string) + for configID, featureSet := range features { + if featureSet.Config == nil { + continue + } + for _, legacyID := range featureSet.Config.LegacyIds { + legacyMap[normalizeFeatureID(legacyID)] = configID + } + } + return legacyMap +} + func normalizeFeatureID(featureID string) string { ref, err := name.ParseReference(featureID) if err != nil { diff --git a/pkg/devcontainer/feature/extend_test.go b/pkg/devcontainer/feature/extend_test.go index 814e548d7..997dc4a10 100644 --- a/pkg/devcontainer/feature/extend_test.go +++ b/pkg/devcontainer/feature/extend_test.go @@ -487,3 +487,106 @@ func (suite *ExtendTestSuite) TestFindContainerUsersUsesMetadataAndImageUserFall suite.Equal("nonroot", containerUser) suite.Equal("vscode", remoteUser) } + +func (suite *ExtendTestSuite) TestBuildLegacyIDMap() { + features := map[string]*config.FeatureSet{ + "ghcr.io/org/features/current-name": { + ConfigID: "ghcr.io/org/features/current-name", + Config: &config.FeatureConfig{ + LegacyIds: []string{ + "ghcr.io/org/features/old-name", + "ghcr.io/org/features/ancient-name", + }, + }, + }, + "ghcr.io/org/features/other": { + ConfigID: "ghcr.io/org/features/other", + Config: &config.FeatureConfig{ + LegacyIds: []string{}, + }, + }, + "feature-no-config": { + ConfigID: "feature-no-config", + Config: nil, + }, + } + + legacyMap := buildLegacyIDMap(features) + + suite.Equal("ghcr.io/org/features/current-name", legacyMap["ghcr.io/org/features/old-name"]) + suite.Equal("ghcr.io/org/features/current-name", legacyMap["ghcr.io/org/features/ancient-name"]) + _, hasOther := legacyMap["ghcr.io/org/features/other"] + suite.False(hasOther) +} + +func (suite *ExtendTestSuite) TestBuildLegacyIDMap_NormalizesVersionTags() { + features := map[string]*config.FeatureSet{ + "ghcr.io/org/features/node": { + ConfigID: "ghcr.io/org/features/node", + Config: &config.FeatureConfig{ + LegacyIds: []string{"ghcr.io/org/features/nodejs:1"}, + }, + }, + } + + legacyMap := buildLegacyIDMap(features) + + suite.Equal("ghcr.io/org/features/node", legacyMap["ghcr.io/org/features/nodejs"]) +} + +func (suite *ExtendTestSuite) TestResolveDependencies_LegacyIDResolution() { + features := map[string]*config.FeatureSet{ + "current-feature": { + ConfigID: "current-feature", + Config: &config.FeatureConfig{ + LegacyIds: []string{"old-feature-name"}, + DependsOn: config.DependsOnField{}, + }, + }, + "consumer-feature": { + ConfigID: "consumer-feature", + Config: &config.FeatureConfig{ + DependsOn: config.DependsOnField{ + "old-feature-name": map[string]any{}, + }, + }, + }, + } + + resolved, err := resolveDependencies(&featureProcessor{}, features) + suite.Require().NoError(err) + suite.Len(resolved, 2) + suite.NotNil(resolved["current-feature"]) + suite.NotNil(resolved["consumer-feature"]) +} + +func (suite *ExtendTestSuite) TestResolveDependencies_LegacyIDNotUsedWhenPrimaryExists() { + features := map[string]*config.FeatureSet{ + "feature-a": { + ConfigID: "feature-a", + Config: &config.FeatureConfig{ + LegacyIds: []string{"feature-b"}, + DependsOn: config.DependsOnField{}, + }, + }, + "feature-b": { + ConfigID: "feature-b", + Config: &config.FeatureConfig{ + DependsOn: config.DependsOnField{}, + }, + }, + "consumer": { + ConfigID: "consumer", + Config: &config.FeatureConfig{ + DependsOn: config.DependsOnField{ + "feature-b": map[string]any{}, + }, + }, + }, + } + + resolved, err := resolveDependencies(&featureProcessor{}, features) + suite.Require().NoError(err) + suite.Len(resolved, 3) + suite.NotNil(resolved["feature-b"]) +}