fix(features)!: map and refuse legacy Feature ids as the reference does

docs/migration.md

@@ -81,6 +81,21 @@ runs the official CLI is unaffected:
   `devcontainer features package . --output-folder output`.
 - A configuration that references a legacy GitHub Releases Feature fails, in
   `read-configuration` too, instead of silently leaving the Feature out.
+- Only the exact legacy Feature names (for example `node`, `golang`, `gradle`)
+  map to `ghcr.io/devcontainers/features/<name>:1`, with a
+  `(!) WARNING: Using the deprecated '<name>' Feature.` line. A legacy name with
+  a tag, such as `node:18` or `node:1`, and any other name without a `/` fail
+  with `Legacy feature '<id>' not supported.` Replace them with the published
+  id, for example `ghcr.io/devcontainers/features/node:1` with the option
+  `"version": "18"`.
+- An id such as `owner/repo/feature` is an OCI reference whose registry is
+  `owner`; it no longer means `ghcr.io/owner/repo/feature`. Write the registry
+  explicitly.
+- `--skip-feature-auto-mapping` no longer turns the legacy mapping off: the
+  official CLI accepts the flag and ignores it.
+- `outdated` leaves legacy ids out, and reports a Feature whose versions cannot
+  be listed with empty values (`{}` in JSON, `-` in text) instead of leaving it
+  out.
 - `up` with `--id-label` and no workspace folder or configuration fails with
   `No dev container config and no workspace found.` instead of attaching to the
   matching container.

docs/parity/parity-matrix.yaml

@@ -348,6 +348,19 @@ initial_cases:
     current_status: match
     notes: "outdated writes JSON.stringify(outdated, undefined, process.stdout.isTTY ? '  ' : undefined): with stdout not a terminal, as here, the result is one unindented line ({\"features\":{}}, a local Feature having no versions). Go used to indent it always."
 
+  - id: outdated.legacy-ids-success
+    lane: contract
+    command: outdated
+    priority: p1
+    docker_required: false
+    setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer/local; printf '%s' '{\"id\":\"local\",\"version\":\"1.0.0\"}' > $WORKSPACE/.devcontainer/local/devcontainer-feature.json; printf '%s' '{\"image\":\"alpine:3\",\"features\":{\"node\":{},\"node:18\":{},\"terraform\":\"latest\",\"my-custom-feature\":{},\"owner/feat\":{},\"localhost:1/ns/feat:1\":{},\"./local\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE"
+    cleanup_cmd: "rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}"
+    ts_cmd: "outdated --workspace-folder ${WORKSPACE} --output-format json"
+    asserts: [exit_code, stdout_text]
+    class: outdated
+    current_status: match
+    notes: "loadVersionInfo calls getRef on each id as written, without the legacy mapping: node, node:18, terraform, my-custom-feature and owner/feat do not parse and are left out, as is the local Feature. localhost:1/ns/feat:1 parses; its tags cannot be listed (connection refused), and it is still reported, with no versions: {\"features\":{\"localhost:1/ns/feat:1\":{}}}. Go used to map the legacy ids to ghcr.io and report them, and to leave out a Feature without versions."
+
   - id: outdated.text-without-features-success
     lane: contract
     command: outdated
@@ -856,6 +869,60 @@ initial_cases:
     current_status: match
     notes: "A bare Feature id ('myfeature') is a legacy id TS rejects ('Legacy feature myfeature not supported...'), exit 1 with no result. Go used to drop the Feature error silently and print a configuration without the Feature."
 
+  - id: read-configuration.legacy-feature-id-with-tag-error
+    lane: contract
+    command: read-configuration
+    priority: p1
+    docker_required: false
+    setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer; printf '%s' '{\"image\":\"alpine:3\",\"features\":{\"node:18\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE"
+    cleanup_cmd: "rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}"
+    ts_cmd: "read-configuration --workspace-folder ${WORKSPACE} --include-merged-configuration"
+    asserts: [exit_code, stderr_normalized]
+    class: features
+    current_status: match
+    notes: "getBackwardCompatibleFeatureId maps only the exact legacy names: 'node:18' stays as written and getFeatureIdType refuses it ('Legacy feature 'node:18' not supported...'), exit 1. Go used to map it to ghcr.io/devcontainers/features/node:18 and fail fetching it."
+
+  - id: read-configuration.deprecated-feature-into-options-error
+    lane: contract
+    command: read-configuration
+    priority: p1
+    docker_required: false
+    setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer; printf '%s' '{\"image\":\"alpine:3\",\"features\":{\"gradle\":{},\"node:18\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE"
+    cleanup_cmd: "rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}"
+    ts_cmd: "read-configuration --workspace-folder ${WORKSPACE} --include-merged-configuration"
+    asserts: [exit_code, stderr_normalized]
+    class: features
+    current_status: match
+    notes: "updateDeprecatedFeaturesIntoOptions moves gradle into a ghcr.io/devcontainers/features/java:1 it appends after the other Features and logs '(!) WARNING: Using the deprecated 'gradle' Feature...' before any Feature is resolved, so node:18 is refused before java is requested: no network. Go used to resolve gradle in place, requesting java first."
+
+  - id: read-configuration.feature-shorthand-registry-error
+    lane: contract
+    command: read-configuration
+    priority: p1
+    docker_required: false
+    network_required: true
+    setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer; printf '%s' '{\"image\":\"alpine:3\",\"features\":{\"devcontainers/features/node:1\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE"
+    cleanup_cmd: "rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}"
+    ts_cmd: "read-configuration --workspace-folder ${WORKSPACE} --include-merged-configuration"
+    asserts: [exit_code]
+    class: features
+    current_status: match
+    notes: "An id with a slash and no dot in its first segment is an OCI reference whose registry is that segment (getRef): 'devcontainers/features/node:1' is requested from the host devcontainers, which does not resolve, and the GitHub Releases fallback refuses the id, exit 1. Go used to prefix ghcr.io/ and install ghcr.io/devcontainers/features/node:1. Only the exit code is compared: TS ends with its GitHub Releases fallback, which this CLI does not implement (DIVERGENCES.md). network_required for the DNS lookup."
+
+  - id: read-configuration.skip-feature-auto-mapping-success
+    lane: contract
+    command: read-configuration
+    priority: p1
+    docker_required: false
+    network_required: true
+    setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer; printf '%s' '{\"image\":\"alpine:3\",\"features\":{\"node\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE"
+    cleanup_cmd: "rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}"
+    ts_cmd: "read-configuration --workspace-folder ${WORKSPACE} --include-features-configuration --skip-feature-auto-mapping"
+    asserts: [exit_code]
+    class: features
+    current_status: match
+    notes: "TS declares --skip-feature-auto-mapping but generateFeaturesConfig never passes it to processFeatureIdentifier, so 'node' is still mapped to ghcr.io/devcontainers/features/node:1 with its deprecation warning, exit 0. Go used to honor the flag and fail to parse 'node' as a reference."
+
   - id: read-configuration.local-feature-outside-devcontainer-error
     lane: contract
     command: read-configuration

internal/cli/build.go

@@ -8,7 +8,6 @@ import (
 	"os"
 	"path/filepath"
 	"slices"
-	"strings"
 
 	"github.com/devcontainers/cli/internal/config"
 	"github.com/devcontainers/cli/internal/docker"
@@ -37,7 +36,6 @@ type buildOpts struct {
 	labels                     []string
 	output                     string
 	additionalFeatures         string
-	skipFeatureAutoMapping     bool
 	skipPersistCustoms         bool
 	experimentalLockfile       bool
 	experimentalFrozenLockfile bool
@@ -85,7 +83,7 @@ func newBuildCmd() *cobra.Command {
 	f.StringVar(&opts.dockerComposePath, "docker-compose-path", "", "Docker Compose CLI path.")
 	f.StringVar(&opts.additionalFeatures, "additional-features", "", "Additional features JSON.")
 	f.String("user-data-folder", "", "User data folder.")
-	f.BoolVar(&opts.skipFeatureAutoMapping, "skip-feature-auto-mapping", false, "")
+	f.Bool("skip-feature-auto-mapping", false, "")
 	f.BoolVar(&opts.skipPersistCustoms, "skip-persisting-customizations-from-features", false, "")
 	f.BoolVar(&opts.experimentalLockfile, "experimental-lockfile", false, "")
 	f.BoolVar(&opts.experimentalFrozenLockfile, "experimental-frozen-lockfile", false, "")
@@ -580,15 +578,10 @@ func imageBuildFor(cfg *config.DevContainer, opts *buildOpts) imageBuild {
 }
 
 func checkLegacyFeatures(feats map[string]any) error {
-	for id := range feats {
-		if features.ClassifyID(id) != features.SourceLegacyShorthand {
-			continue
-		}
-		name := id
-		if i := strings.LastIndexByte(id, ':'); i != -1 {
-			name = id[:i]
-		}
-		if !features.IsKnownLegacyFeature(name) {
+	userFeatures, _ := features.UpdateDeprecatedFeaturesIntoOptions(sortedUserFeatures(feats))
+	for _, f := range userFeatures {
+		id, _ := features.BackwardCompatibleID(f.UserFeatureID)
+		if features.ClassifyID(id) == features.SourceLegacyShorthand {
 			return &coreerrors.ContainerError{Description: fmt.Sprintf(msgLegacyFeature, id)}
 		}
 	}
@@ -751,7 +744,6 @@ func (r *buildRunner) buildCompose(ctx context.Context, cfg *config.DevContainer
 			workspaceFolder:            opts.workspaceFolder,
 			cacheFrom:                  opts.cacheFrom,
 			buildNoCache:               opts.noCache,
-			skipFeatureAutoMapping:     opts.skipFeatureAutoMapping,
 			experimentalFrozenLockfile: opts.experimentalFrozenLockfile,
 			noLockfile:                 opts.noLockfile,
 			lockfileExcludeIDs:         opts.lockfileExcludeIDs,
@@ -840,7 +832,6 @@ func (r *buildRunner) featureBuildOptions(cfg *config.DevContainer, hostSub *con
 		Push:                        opts.push,
 		Output:                      opts.output,
 		ContainerEnv:                cfg.ContainerEnv,
-		SkipFeatureAutoMapping:      opts.skipFeatureAutoMapping,
 		FrozenLockfile:              opts.experimentalFrozenLockfile,
 		NoLockfile:                  opts.noLockfile,
 		ConfigPath:                  cfg.ConfigFilePath,

internal/cli/build_held_test.go

@@ -108,12 +108,12 @@ func TestImageBuildFor(t *testing.T) {
 }
 
 func TestCheckLegacyFeatures(t *testing.T) {
-	for _, id := range []string{"node", "node:1", "golang", "ghcr.io/devcontainers/features/node:1", "./local-feature", "owner/repo/feature"} {
+	for _, id := range []string{"node", "golang", "gradle", "ghcr.io/devcontainers/features/node:1", "./local-feature", "owner/repo/feature"} {
 		if err := checkLegacyFeatures(map[string]any{id: true}); err != nil {
 			t.Errorf("%s: unexpected error %v", id, err)
 		}
 	}
-	for _, id := range []string{"not-a-feature", "not-a-feature:1"} {
+	for _, id := range []string{"not-a-feature", "not-a-feature:1", "node:1", "node:18", "gradle:1"} {
 		err := checkLegacyFeatures(map[string]any{id: true})
 		if err == nil || !strings.Contains(err.Error(), "Legacy feature '"+id+"' not supported") {
 			t.Errorf("%s: error = %v, want the legacy feature error naming it", id, err)

internal/cli/compose_up.go

@@ -274,7 +274,7 @@ func (r *upRunner) composeFeatureSets(ctx context.Context, cfg *config.DevContai
 			return nil, "", err
 		}
 	}
-	res, err := fetchFeatureSetsWithOrder(ctx, r.log, cfg.Features, localFeatureRoots{filepath.Dir(cfg.ConfigFilePath), opts.workspaceFolder}, opts.skipFeatureAutoMapping, lockfile, cfg.OverrideFeatureInstallOrder)
+	res, err := fetchFeatureSetsWithOrder(ctx, r.log, cfg.Features, localFeatureRoots{filepath.Dir(cfg.ConfigFilePath), opts.workspaceFolder}, lockfile, cfg.OverrideFeatureInstallOrder)
 	if err != nil {
 		return nil, "", err
 	}

internal/cli/feature_deps.go

@@ -32,7 +32,10 @@ func renderDependencyMermaid(client oci.Registry, logger log.Logger, roots []str
 func newMetadataProcessFeature(client oci.Registry, logger log.Logger, local localFeatureRoots, lockfile *features.Lockfile) features.ProcessFeature {
 	return func(node *features.FNode) (*features.Set, error) {
 		id := node.UserFeatureID
-		srcType := features.ClassifyID(id)
+		resolvedID, srcType, err := resolveFeatureID(logger, id)
+		if err != nil {
+			return nil, err
+		}
 
 		switch srcType {
 		case features.SourceLocalPath:
@@ -68,13 +71,12 @@ func newMetadataProcessFeature(client oci.Registry, logger log.Logger, local loc
 			}, nil
 
 		default:
-			resolvedID, _ := features.ResolveID(id, false)
 			ref, err := oci.ParseRef(resolvedID)
 			if err != nil {
 				return nil, fmt.Errorf("ERR: Feature '%s' could not be processed.  %w", id, err)
 			}
 			if lockfile != nil {
-				if integrity := lockfile.Features[id].Integrity; integrity != "" {
+				if integrity := lockfile.Features[resolvedID].Integrity; integrity != "" {
 					if pinned, perr := oci.ParseRef(ref.Resource + "@" + integrity); perr == nil {
 						ref = pinned
 					}

internal/cli/feature_deps_held_test.go

@@ -2,9 +2,11 @@ package cli
 
 import (
 	"bytes"
+	"fmt"
 	"net/http"
 	"path/filepath"
 	"reflect"
+	"slices"
 	"strings"
 	"testing"
 
@@ -170,6 +172,34 @@ func TestMetadataProcessFeatureOCI(t *testing.T) {
 	}
 }
 
+func TestMetadataProcessFeatureLegacyIDs(t *testing.T) {
+	reg := newOCIFeatureRegistry()
+	reg.add(t, "ghcr.io/devcontainers/features/node:1", map[string]string{"devcontainer-feature.json": `{"id":"node"}`}, nil)
+	var logged bytes.Buffer
+	logger := log.New(log.Options{Writer: &logged, Level: log.LevelInfo})
+	set, err := newMetadataProcessFeature(reg, logger, localFeatureRoots{}, nil)(&features.FNode{UserFeatureID: "node"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := set.SourceInfo.(*features.OCISource); got.Resource != "ghcr.io/devcontainers/features/node" || got.UserID != "node" {
+		t.Errorf("source = %+v, want node:1 for the id as written", got)
+	}
+	if !strings.Contains(logged.String(), "(!) WARNING: Using the deprecated 'node' Feature.") {
+		t.Errorf("log = %q, want the deprecated Feature warning", logged.String())
+	}
+	for _, id := range []string{"node:18", "gradle", "my-custom-feature"} {
+		reg.fetched = nil
+		_, err := resolveMetadata(t, reg, localFeatureRoots{}, nil, id, nil)
+		if err == nil || err.Error() != fmt.Sprintf(msgLegacyFeature, id) || len(reg.fetched) != 0 {
+			t.Errorf("%s: err = %v, fetched %v; want the legacy Feature error and no request", id, err, reg.fetched)
+		}
+	}
+	reg.fetched = nil
+	if _, err := resolveMetadata(t, reg, localFeatureRoots{}, nil, "owner/repo/feat", nil); err == nil || !slices.Equal(reg.fetched, []string{"owner/repo/feat:latest"}) {
+		t.Errorf("owner/repo/feat: err = %v, fetched %v; want registry owner", err, reg.fetched)
+	}
+}
+
 func TestRenderDependencyMermaid(t *testing.T) {
 	reg := newOCIFeatureRegistry()
 	root := `{"id":"root","dependsOn":{"ghcr.io/owner/coll/dep:1":{}}}`