From 5b8e14f37b0599fe46c305498a855be5d6ec0711 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 12 Nov 2026 21:11:29 -0300 Subject: [PATCH] fix(features)!: map and refuse legacy Feature ids as the reference does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Legacy and shorthand Feature ids now follow the reference CLI's getBackwardCompatibleFeatureId, getFeatureIdType and updateDeprecatedFeaturesIntoOptions in up, build, read-configuration, upgrade, features resolve-dependencies, features info and outdated: - Only exact legacy names (node, golang, …) map to ghcr.io/devcontainers/features/:1, and log TS's "(!) WARNING: Using the deprecated '' Feature" line. - A legacy name with a tag (node:18, node:1) and any other id without a '/' fail with "Legacy feature '' not supported". - gradle, maven and jupyterlab move into the java/python Feature they are now an option of, before any Feature is resolved. - owner/repo/feat is an OCI reference whose registry is "owner", no longer ghcr.io/owner/repo/feat. - --skip-feature-auto-mapping is accepted and ignored, as in TS. - outdated reports what getRef parses as written: legacy ids are left out, and a Feature without published versions is reported empty. - The lockfile's integrity is looked up by the mapped id, so an entry under a legacy id doesn't pin it, as in TS. A ts-oracle golden pins the three TS functions, and five parity cases compare the user-visible results with the reference. BREAKING CHANGE: node:18-style ids, other names without a '/', and owner/repo/feat ids that relied on an implicit ghcr.io now fail or resolve as in the official CLI; docs/migration.md says how to write them. Co-Authored-By: Claude Opus 5.5 --- docs/migration.md | 15 + docs/parity/parity-matrix.yaml | 67 +++ internal/cli/build.go | 19 +- internal/cli/build_held_test.go | 4 +- internal/cli/compose_up.go | 2 +- internal/cli/feature_deps.go | 8 +- internal/cli/feature_deps_held_test.go | 30 ++ internal/cli/feature_install.go | 70 ++- internal/cli/feature_install_held_test.go | 81 ++- internal/cli/feature_install_test.go | 19 +- internal/cli/outdated.go | 20 +- internal/cli/outdated_held_test.go | 12 +- internal/cli/pure_helpers_test.go | 11 - internal/cli/read_configuration.go | 5 +- internal/cli/up.go | 4 +- internal/features/resolve.go | 100 ++-- internal/features/resolve_held_test.go | 21 +- internal/features/resolve_test.go | 132 ++--- .../features/testdata/ts-oracle/legacy.json | 501 ++++++++++++++++++ internal/features/ts_oracle_test.go | 54 ++ tools/tsoracle/oracle.ts | 17 +- 21 files changed, 948 insertions(+), 244 deletions(-) create mode 100644 internal/features/testdata/ts-oracle/legacy.json diff --git a/docs/migration.md b/docs/migration.md index 0812e15..c1ac1aa 100644 --- a/docs/migration.md +++ b/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/:1`, with a + `(!) WARNING: Using the deprecated '' 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 '' 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. diff --git a/docs/parity/parity-matrix.yaml b/docs/parity/parity-matrix.yaml index 06d7d83..6f848ce 100644 --- a/docs/parity/parity-matrix.yaml +++ b/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 diff --git a/internal/cli/build.go b/internal/cli/build.go index 9a04663..4499b26 100644 --- a/internal/cli/build.go +++ b/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, diff --git a/internal/cli/build_held_test.go b/internal/cli/build_held_test.go index 5fc40df..225dcae 100644 --- a/internal/cli/build_held_test.go +++ b/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) diff --git a/internal/cli/compose_up.go b/internal/cli/compose_up.go index 51d49b3..bf999da 100644 --- a/internal/cli/compose_up.go +++ b/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 } diff --git a/internal/cli/feature_deps.go b/internal/cli/feature_deps.go index b8d4a99..269b596 100644 --- a/internal/cli/feature_deps.go +++ b/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 } diff --git a/internal/cli/feature_deps_held_test.go b/internal/cli/feature_deps_held_test.go index 11a3ebe..74b584f 100644 --- a/internal/cli/feature_deps_held_test.go +++ b/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":{}}}` diff --git a/internal/cli/feature_install.go b/internal/cli/feature_install.go index 9b1135f..bea61bc 100644 --- a/internal/cli/feature_install.go +++ b/internal/cli/feature_install.go @@ -11,7 +11,6 @@ import ( "errors" "fmt" "io" - "maps" "net/http" "net/url" "os" @@ -48,7 +47,6 @@ type FeatureBuildOptions struct { Secrets []string CABundle string ContainerEnv map[string]string - SkipFeatureAutoMapping bool SkipPersistCustoms bool FeaturesBasePath string WorkspaceFolder string @@ -66,8 +64,16 @@ type fetchFeatureResult struct { TmpDir string } -func fetchFeatureSets(ctx context.Context, logger log.Logger, featuresCfg map[string]any, local localFeatureRoots, skipAutoMapping bool) (*fetchFeatureResult, error) { - return fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, local, skipAutoMapping, nil, nil) +func fetchFeatureSets(ctx context.Context, logger log.Logger, featuresCfg map[string]any, local localFeatureRoots) (*fetchFeatureResult, error) { + return fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, local, nil, nil) +} + +func sortedUserFeatures(featuresCfg map[string]any) []features.DevContainerFeature { + userFeatures := features.UserFeaturesToArray(featuresCfg) + sort.Slice(userFeatures, func(i, j int) bool { + return userFeatures[i].UserFeatureID < userFeatures[j].UserFeatureID + }) + return userFeatures } type localFeatureRoots struct { @@ -104,15 +110,16 @@ func createFeaturesFolder() (string, error) { } } -func fetchFeatureSetsWithOrder(ctx context.Context, logger log.Logger, featuresCfg map[string]any, local localFeatureRoots, skipAutoMapping bool, lockfile *features.Lockfile, overrideOrder []string) (*fetchFeatureResult, error) { +func fetchFeatureSetsWithOrder(ctx context.Context, logger log.Logger, featuresCfg map[string]any, local localFeatureRoots, lockfile *features.Lockfile, overrideOrder []string) (*fetchFeatureResult, error) { if len(featuresCfg) == 0 { return nil, nil } - userFeatures := features.UserFeaturesToArray(featuresCfg) - sort.Slice(userFeatures, func(i, j int) bool { - return userFeatures[i].UserFeatureID < userFeatures[j].UserFeatureID - }) + userFeatures := sortedUserFeatures(featuresCfg) + userFeatures, warnings := features.UpdateDeprecatedFeaturesIntoOptions(userFeatures) + for _, w := range warnings { + logger.Write(w, log.LevelWarning) + } logger.Write(fmt.Sprintf("Installing %d feature(s)...", len(userFeatures)), log.LevelInfo) @@ -131,7 +138,7 @@ func fetchFeatureSetsWithOrder(ctx context.Context, logger log.Logger, featuresC } processFeature := func(node *features.FNode) (*features.Set, error) { - return processInstallFeature(logger, ociClient, node, local, skipAutoMapping, lockfile, nextStageDir) + return processInstallFeature(logger, ociClient, node, local, lockfile, nextStageDir) } graph, err := features.BuildDependencyGraph(logger, processFeature, userFeatures, overrideOrder, lockfile) @@ -201,29 +208,14 @@ func processInstallFeature( ociClient oci.Registry, node *features.FNode, local localFeatureRoots, - skipAutoMapping bool, lockfile *features.Lockfile, nextStageDir func() string, ) (*features.Set, error) { id := node.UserFeatureID opts := node.Options - srcType := features.ClassifyID(id) - - if srcType == features.SourceLegacyShorthand { - name := id - // mutate-exempt: idx > 1 differs only for a colon at index 1, which leaves a one-character name, and no legacy Feature has one: both bounds reject the id with the same error - if idx := strings.LastIndex(id, ":"); idx > 0 { - name = id[:idx] - } - if !features.IsKnownLegacyFeature(name) { - return nil, &coreerrors.ContainerError{Description: fmt.Sprintf(msgLegacyFeature, id)} - } - if m, ok := features.DeprecatedFeatureIntoOptions[name]; ok { - logger.Write(fmt.Sprintf("(!) WARNING: Falling back to the deprecated '%s' Feature. It is now part of the '%s' Feature.", name, m.MapTo), log.LevelWarning) - id = fmt.Sprintf("ghcr.io/devcontainers/features/%s:1", m.MapTo) - opts = addFeatureOption(opts, m.Option) - srcType = features.ClassifyID(id) - } + resolvedID, srcType, err := resolveFeatureID(logger, id) + if err != nil { + return nil, err } switch srcType { @@ -307,7 +299,6 @@ func processInstallFeature( }, nil default: - resolvedID, _ := features.ResolveID(id, skipAutoMapping) ref, err := oci.ParseRef(resolvedID) if err != nil { return nil, fmt.Errorf("ERR: Feature '%s' could not be processed. %w", id, err) @@ -315,7 +306,7 @@ func processInstallFeature( userRef := ref 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 } @@ -326,6 +317,8 @@ func processInstallFeature( manifest, err := ociClient.FetchManifest(ref, "") if err != nil { + // TS logs this before its GitHub Releases fallback, which this CLI lacks. + logger.Write(fmt.Sprintf("Could not resolve Feature manifest for '%s'. If necessary, provide registry credentials with 'docker login '.", resolvedID), log.LevelWarning) return nil, fmt.Errorf("ERR: Feature '%s' could not be processed. You may not have permission to access this Feature, or may not be logged in. If the issue persists, report this to the Feature author.", id) } if len(manifest.Manifest.Layers) == 0 { @@ -606,7 +599,7 @@ func extendImageWithFeatures( return nil, err } } - result, err := fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, localFeatureRoots{fbOpts.FeaturesBasePath, fbOpts.WorkspaceFolder}, fbOpts.SkipFeatureAutoMapping, lockfile, fbOpts.OverrideFeatureInstallOrder) + result, err := fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, localFeatureRoots{fbOpts.FeaturesBasePath, fbOpts.WorkspaceFolder}, lockfile, fbOpts.OverrideFeatureInstallOrder) if err != nil { return nil, err } @@ -996,13 +989,16 @@ func isPlainHTTPURL(u string) bool { return parsed.Scheme == "http" || parsed.Hostname() == "localhost" } -func addFeatureOption(opts any, key string) map[string]any { - m := map[string]any{} - if existing, ok := opts.(map[string]any); ok { - maps.Copy(m, existing) +func resolveFeatureID(logger log.Logger, id string) (string, features.SourceType, error) { + resolvedID, warning := features.BackwardCompatibleID(id) + if warning != "" { + logger.Write(warning, log.LevelWarning) + } + srcType := features.ClassifyID(resolvedID) + if srcType == features.SourceLegacyShorthand { + return "", srcType, &coreerrors.ContainerError{Description: fmt.Sprintf(msgLegacyFeature, resolvedID)} } - m[key] = true - return m + return resolvedID, srcType, nil } func featureSetIDs(sets []*features.Set) string { diff --git a/internal/cli/feature_install_held_test.go b/internal/cli/feature_install_held_test.go index 5b151b7..17aaf4c 100644 --- a/internal/cli/feature_install_held_test.go +++ b/internal/cli/feature_install_held_test.go @@ -113,7 +113,7 @@ func stageDirs(t *testing.T) func() string { func installFeature(t *testing.T, reg oci.Registry, id string, opts any, lockfile *features.Lockfile) (*features.Set, error) { t.Helper() - return processInstallFeature(log.Null, reg, &features.FNode{UserFeatureID: id, Options: opts}, localFeatureRoots{}, false, lockfile, stageDirs(t)) + return processInstallFeature(log.Null, reg, &features.FNode{UserFeatureID: id, Options: opts}, localFeatureRoots{}, lockfile, stageDirs(t)) } func TestProcessInstallFeatureOCI(t *testing.T) { @@ -217,15 +217,70 @@ func TestProcessInstallFeatureOCIPinsToTheLockfile(t *testing.T) { } } -func TestProcessInstallFeatureLegacyShorthandWithVersion(t *testing.T) { - reg := newOCIFeatureRegistry() - reg.add(t, "ghcr.io/devcontainers/features/node:1", map[string]string{"devcontainer-feature.json": `{"id":"node"}`}, nil) - set, err := installFeature(t, reg, "node:1", nil, nil) - if err != nil { - t.Fatal(err) - } - if got := set.SourceInfo.(*features.OCISource).Resource; got != "ghcr.io/devcontainers/features/node" { - t.Errorf("resource = %q", got) +func TestProcessInstallFeatureLegacyIDs(t *testing.T) { + const digest = "sha256:0000000000000000000000000000000000000000000000000000000000000001" + legacyError := func(id string) string { return fmt.Sprintf(msgLegacyFeature, id) } + for name, tc := range map[string]struct { + id string + lockfile *features.Lockfile + wantFetched []string + wantErr string + wantLog string + }{ + "migrated name maps to major 1": { + id: "node", wantFetched: []string{"ghcr.io/devcontainers/features/node:1"}, + wantLog: "(!) WARNING: Using the deprecated 'node' Feature. See https://github.com/devcontainers/features/tree/main/src/node#example-usage for the updated Feature.", + }, + "renamed name maps to its new name": { + id: "golang", wantFetched: []string{"ghcr.io/devcontainers/features/go:1"}, + wantLog: "(!) WARNING: Using the deprecated 'golang' Feature. See https://github.com/devcontainers/features/tree/main/src/go#example-usage for the updated Feature.", + }, + "lockfile entry of the id as written": { + id: "node", lockfile: &features.Lockfile{Features: map[string]features.LockfileEntry{"node": {Integrity: digest}}}, + wantFetched: []string{"ghcr.io/devcontainers/features/node:1"}, + }, + "lockfile entry of the mapped id": { + id: "node", lockfile: &features.Lockfile{Features: map[string]features.LockfileEntry{"ghcr.io/devcontainers/features/node:1": {Integrity: digest}}}, + wantFetched: []string{"ghcr.io/devcontainers/features/node@" + digest}, + wantErr: "could not be processed", + }, + "tagged legacy name": {id: "node:18", wantErr: legacyError("node:18")}, + "legacy name with tag 1": {id: "node:1", wantErr: legacyError("node:1")}, + "unknown legacy name": {id: "my-custom-feature", wantErr: legacyError("my-custom-feature")}, + "Feature now an option": { + id: "gradle", wantErr: legacyError("gradle"), + wantLog: "(!) WARNING: Falling back to the deprecated 'gradle' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature.", + }, + "first segment is the registry": { + id: "owner/repo/feat", wantFetched: []string{"owner/repo/feat:latest"}, wantErr: "could not be processed", + wantLog: "Could not resolve Feature manifest for 'owner/repo/feat'. If necessary, provide registry credentials with 'docker login '.", + }, + } { + t.Run(name, func(t *testing.T) { + reg := newOCIFeatureRegistry() + for _, f := range []string{"node", "go"} { + reg.add(t, "ghcr.io/devcontainers/features/"+f+":1", map[string]string{"devcontainer-feature.json": `{"id":"` + f + `"}`}, nil) + } + reg.add(t, "ghcr.io/owner/repo/feat:latest", map[string]string{"devcontainer-feature.json": `{"id":"feat"}`}, nil) + var logged strings.Builder + logger := log.New(log.Options{Writer: &logged, Level: log.LevelInfo}) + set, err := processInstallFeature(logger, reg, &features.FNode{UserFeatureID: tc.id}, localFeatureRoots{}, tc.lockfile, stageDirs(t)) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("err = %v, want %q", err, tc.wantErr) + } + } else if err != nil { + t.Fatal(err) + } else if got := set.SourceInfo.UserFeatureID(); got != tc.id { + t.Errorf("userFeatureId = %q, want the id as written", got) + } + if !slices.Equal(reg.fetched, tc.wantFetched) { + t.Errorf("fetched %v, want %v", reg.fetched, tc.wantFetched) + } + if tc.wantLog != "" && !strings.Contains(logged.String(), tc.wantLog) { + t.Errorf("log = %q, want %q", logged.String(), tc.wantLog) + } + }) } } @@ -272,7 +327,7 @@ func TestProcessInstallFeatureOCIErrors(t *testing.T) { if stage == nil { stage = stageDirs(t) } - _, err := processInstallFeature(log.Null, reg, &features.FNode{UserFeatureID: featureID}, localFeatureRoots{}, false, nil, stage) + _, err := processInstallFeature(log.Null, reg, &features.FNode{UserFeatureID: featureID}, localFeatureRoots{}, nil, stage) if err == nil || !strings.Contains(err.Error(), tc.wantErr) { t.Fatalf("err = %v, want %q", err, tc.wantErr) } @@ -353,7 +408,7 @@ func TestProcessInstallFeatureTarballErrors(t *testing.T) { }) t.Run("staging folder not writable", func(t *testing.T) { uri := serveBytes(t, blob, http.StatusOK) + "/f.tgz" - _, err := processInstallFeature(log.Null, nil, &features.FNode{UserFeatureID: uri}, localFeatureRoots{}, false, nil, func() string { return filepath.Join(blocker, "stage") }) + _, err := processInstallFeature(log.Null, nil, &features.FNode{UserFeatureID: uri}, localFeatureRoots{}, nil, func() string { return filepath.Join(blocker, "stage") }) if err == nil || !strings.Contains(err.Error(), "write feature tarball") { t.Fatalf("err = %v", err) } @@ -374,7 +429,7 @@ func TestTarballFeatureIDUsesTheURLPath(t *testing.T) { func TestFetchFeatureSetsReportsTheFirstSortedFailure(t *testing.T) { for range 5 { - _, err := fetchFeatureSets(t.Context(), log.Null, map[string]any{"./b-missing": true, "./a-missing": true, "./c-missing": true}, devcontainerRoots(t), false) + _, err := fetchFeatureSets(t.Context(), log.Null, map[string]any{"./b-missing": true, "./a-missing": true, "./c-missing": true}, devcontainerRoots(t)) if err == nil || !strings.Contains(err.Error(), "'./a-missing'") { t.Fatalf("err = %v, want the failure of ./a-missing", err) } diff --git a/internal/cli/feature_install_test.go b/internal/cli/feature_install_test.go index 5161691..dca727f 100644 --- a/internal/cli/feature_install_test.go +++ b/internal/cli/feature_install_test.go @@ -148,7 +148,7 @@ func TestFetchFeatureSets(t *testing.T) { t.Run(tt.name, func(t *testing.T) { roots := devcontainerRoots(t) tt.setup(t, roots.configDir) - result, err := fetchFeatureSets(t.Context(), log.Null, tt.entries, roots, false) + result, err := fetchFeatureSets(t.Context(), log.Null, tt.entries, roots) if result != nil && result.TmpDir != "" { defer os.RemoveAll(result.TmpDir) } @@ -232,7 +232,7 @@ func TestFetchFeatureSets_StagingLayout(t *testing.T) { if err := os.WriteFile(filepath.Join(dirB, "install.sh"), []byte("#!/bin/sh\n"), 0o755); err != nil { t.Fatal(err) } - result, err := fetchFeatureSets(t.Context(), log.Null, map[string]any{"./dir-a": true}, roots, false) + result, err := fetchFeatureSets(t.Context(), log.Null, map[string]any{"./dir-a": true}, roots) if err != nil { t.Fatal(err) } @@ -396,7 +396,7 @@ func TestFetchFeatureSetsRefusesLocalFeaturesOutsideDevcontainer(t *testing.T) { t.Run(tc.id, func(t *testing.T) { var logs bytes.Buffer logger := log.New(log.Options{Level: log.LevelInfo, Format: "text", Writer: &logs}) - result, err := fetchFeatureSets(t.Context(), logger, map[string]any{tc.id: true}, roots, false) + result, err := fetchFeatureSets(t.Context(), logger, map[string]any{tc.id: true}, roots) if result != nil { t.Fatalf("result = %+v, want none", result) } @@ -417,6 +417,19 @@ func TestFetchFeatureSetsRefusesLocalFeaturesOutsideDevcontainer(t *testing.T) { } } +func TestFetchFeatureSetsMovesDeprecatedFeaturesIntoOptions(t *testing.T) { + var logs bytes.Buffer + logger := log.New(log.Options{Level: log.LevelInfo, Format: "text", Writer: &logs}) + result, err := fetchFeatureSets(t.Context(), logger, map[string]any{"gradle": map[string]any{}, "node:18": map[string]any{}}, devcontainerRoots(t)) + if result != nil || err == nil || err.Error() != fmt.Sprintf(msgLegacyFeature, "node:18") { + t.Fatalf("result = %+v, err = %v; want the legacy Feature error for node:18", result, err) + } + warning := "(!) WARNING: Using the deprecated 'gradle' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature.\n" + if !strings.Contains(logs.String(), warning) || strings.Contains(logs.String(), "Fetching feature") { + t.Errorf("logs = %q, want the gradle warning and no Feature fetched", logs.String()) + } +} + func TestReadConfigurationRefusesALocalFeatureOutsideDevcontainer(t *testing.T) { startFakeEngine(t, nil) ws := writeConfigFixture(t, map[string]string{ diff --git a/internal/cli/outdated.go b/internal/cli/outdated.go index e79f59d..6f739d1 100644 --- a/internal/cli/outdated.go +++ b/internal/cli/outdated.go @@ -85,10 +85,10 @@ type outdatedOpts struct { } type outdatedEntry struct { - Current string `json:"current"` - Wanted string `json:"wanted"` + Current string `json:"current,omitempty"` + Wanted string `json:"wanted,omitempty"` WantedMajor string `json:"wantedMajor,omitempty"` - Latest string `json:"latest"` + Latest string `json:"latest,omitempty"` LatestMajor string `json:"latestMajor,omitempty"` } @@ -200,18 +200,16 @@ func runOutdated(ctx context.Context, out Output, opts outdatedOpts) error { result := make(map[string]outdatedEntry) for id := range cfg.Features { - resolvedID, _ := features.ResolveID(id, false) - - ref, err := oci.ParseRef(resolvedID) + ref, err := oci.ParseRef(id) if err != nil { continue } versions := resolvePublishedVersions(ociClient, ref) - if len(versions) == 0 { - continue - } - latest := versions[len(versions)-1].Original() + var latest string + if len(versions) > 0 { + latest = versions[len(versions)-1].Original() + } wanted := highestSatisfyingTag(versions, ref.Tag) current := wanted @@ -449,7 +447,7 @@ func newUpgradeCmd() *cobra.Command { lockfilePath := cfg.ConfigFilePath - res, err := fetchFeatureSetsWithOrder(cmd.Context(), logger, cfg.Features, localFeatureRoots{filepath.Dir(lockfilePath), ws}, false, nil, nil) + res, err := fetchFeatureSetsWithOrder(cmd.Context(), logger, cfg.Features, localFeatureRoots{filepath.Dir(lockfilePath), ws}, nil, nil) if err != nil { return err } diff --git a/internal/cli/outdated_held_test.go b/internal/cli/outdated_held_test.go index 23a8d1b..059fca4 100644 --- a/internal/cli/outdated_held_test.go +++ b/internal/cli/outdated_held_test.go @@ -38,7 +38,7 @@ func TestOutdatedReportsVersions(t *testing.T) { reg.tags["ns/blank"] = []string{"1.0.0"} reg.tags["ns/none"] = []string{} h := reg.host - features := `{"` + h + `/ns/multi:1":{}, "` + h + `/ns/single:3":{}, "` + h + `/ns/blank:1":{}, "` + h + `/ns/none:1":{}, "./local":{}}` + features := `{"` + h + `/ns/multi:1":{}, "` + h + `/ns/single:3":{}, "` + h + `/ns/blank:1":{}, "` + h + `/ns/none:1":{}, "` + h + `/ns/single:9":{}, "./local":{}, "node":{}, "node:18":{}, "terraform":"latest", "owner/feat":{}}` lock := `{"features":{"` + h + `/ns/multi:1":{"version":"1.0.0"},"` + h + `/ns/blank:1":{"version":""}}}` ws := writeOutdatedWorkspace(t, features, lock) @@ -59,8 +59,13 @@ func TestOutdatedReportsVersions(t *testing.T) { h + "/ns/multi:1": {Current: "1.0.0", Wanted: "1.2.0", WantedMajor: "1", Latest: "2.0.0", LatestMajor: "2"}, h + "/ns/single:3": {Current: "3.1.0", Wanted: "3.1.0", WantedMajor: "3", Latest: "3.1.0", LatestMajor: "3"}, h + "/ns/blank:1": {Current: "1.0.0", Wanted: "1.0.0", WantedMajor: "1", Latest: "1.0.0", LatestMajor: "1"}, + h + "/ns/none:1": {}, + h + "/ns/single:9": {Latest: "3.1.0", LatestMajor: "3"}, } - if keys, err := jsonKeyOrder(got, "features"); err != nil || !slices.Equal(keys, []string{h + "/ns/multi:1", h + "/ns/single:3", h + "/ns/blank:1"}) { + if !strings.Contains(got, `"`+h+`/ns/none:1":{}`) || !strings.Contains(got, `"`+h+`/ns/single:9":{"latest":"3.1.0","latestMajor":"3"}`) { + t.Errorf("json output = %s, want the versions TS leaves undefined left out", got) + } + if keys, err := jsonKeyOrder(got, "features"); err != nil || !slices.Equal(keys, []string{h + "/ns/multi:1", h + "/ns/single:3", h + "/ns/blank:1", h + "/ns/none:1", h + "/ns/single:9"}) { t.Errorf("features order = %v, %v; want the configuration's", keys, err) } if len(doc.Features) != len(want) { @@ -77,7 +82,8 @@ func TestOutdatedReportsVersions(t *testing.T) { t.Fatal(err) } lines := strings.Split(strings.TrimSuffix(text, "\n"), "\n") - if len(lines) != 4 || !strings.HasPrefix(lines[0], "Feature") || !strings.HasPrefix(lines[1], h+"/ns/multi ") { + if len(lines) != 6 || !strings.HasPrefix(lines[0], "Feature") || !strings.HasPrefix(lines[1], h+"/ns/multi ") || + strings.Join(strings.Fields(lines[4]), " ") != h+"/ns/none - - -" { t.Errorf("text table:\n%s", text) } } diff --git a/internal/cli/pure_helpers_test.go b/internal/cli/pure_helpers_test.go index 3544641..88a62ef 100644 --- a/internal/cli/pure_helpers_test.go +++ b/internal/cli/pure_helpers_test.go @@ -316,17 +316,6 @@ func TestWriteSuccessJSON(t *testing.T) { } } -func TestAddFeatureOption(t *testing.T) { - got := addFeatureOption(map[string]any{"version": "1"}, "gradle") - if got["version"] != "1" || got["gradle"] != true { - t.Errorf("addFeatureOption merge wrong: %v", got) - } - got = addFeatureOption(true, "maven") - if got["maven"] != true || len(got) != 1 { - t.Errorf("addFeatureOption(non-map) = %v", got) - } -} - func TestOCIAnnotationID(t *testing.T) { mk := func(ann map[string]string) *oci.ManifestContainer { return &oci.ManifestContainer{Manifest: &oci.Manifest{Annotations: ann}} diff --git a/internal/cli/read_configuration.go b/internal/cli/read_configuration.go index b1477af..7afcba4 100644 --- a/internal/cli/read_configuration.go +++ b/internal/cli/read_configuration.go @@ -46,7 +46,6 @@ type readConfigOpts struct { idLabels []string dockerPath string additionalFeatures string - skipFeatureAutoMapping bool terminalColumns givenInt terminalRows givenInt cacheKey bool @@ -82,7 +81,7 @@ func newReadConfigurationCmd() *cobra.Command { f.Var(&opts.terminalColumns, "terminal-columns", "Terminal columns.") f.Var(&opts.terminalRows, "terminal-rows", "Terminal rows.") f.StringVar(&opts.additionalFeatures, "additional-features", "", "Additional features JSON.") - f.BoolVar(&opts.skipFeatureAutoMapping, "skip-feature-auto-mapping", false, "") + f.Bool("skip-feature-auto-mapping", false, "") cmd.Flags().MarkHidden("skip-feature-auto-mapping") f.BoolVar(&opts.cacheKey, "cache-key", false, "Also emit a deterministic content-addressed cache key for the configuration.") @@ -184,7 +183,7 @@ func runReadConfiguration(ctx context.Context, out Output, opts *readConfigOpts) if needsFeaturesConfiguration(opts, containerID, result) { lgr := log.New(log.Options{Level: log.ParseLevel(opts.logLevel), Format: opts.logFormat, Writer: logDst}) var featErr error - featResult, featErr = fetchFeatureSets(ctx, lgr, result.Config.Features, localFeatureRoots{filepath.Dir(result.Config.ConfigFilePath), workspaceFolder}, opts.skipFeatureAutoMapping) + featResult, featErr = fetchFeatureSets(ctx, lgr, result.Config.Features, localFeatureRoots{filepath.Dir(result.Config.ConfigFilePath), workspaceFolder}) if featErr != nil { return readConfigurationFailure(lgr, featErr) } diff --git a/internal/cli/up.go b/internal/cli/up.go index bf437d8..43b687e 100644 --- a/internal/cli/up.go +++ b/internal/cli/up.go @@ -74,7 +74,6 @@ type upOpts struct { platform string defaultUserEnvProbe string containerSessionDataFolder string - skipFeatureAutoMapping bool composeProjectName string } @@ -136,7 +135,7 @@ func newUpCmd() *cobra.Command { f.StringArrayVar(&opts.cacheFrom, "cache-from", nil, "Cache from images.") f.StringVar(&opts.cacheTo, "cache-to", "", "Cache to destination.") f.StringVar(&opts.additionalFeatures, "additional-features", "", "Additional features JSON.") - f.BoolVar(&opts.skipFeatureAutoMapping, "skip-feature-auto-mapping", false, "") + f.Bool("skip-feature-auto-mapping", false, "") f.StringVar(&opts.dotfilesRepo, "dotfiles-repository", "", "Dotfiles repo.") f.StringVar(&opts.dotfilesCommand, "dotfiles-install-command", "", "Dotfiles install command.") f.StringVar(&opts.dotfilesTarget, "dotfiles-target-path", "~/dotfiles", "Dotfiles target.") @@ -1183,7 +1182,6 @@ func (r *upRunner) featureBuildOptions(cfg *config.DevContainer, loadResult *con OverrideFeatureInstallOrder: cfg.OverrideFeatureInstallOrder, FeaturesBasePath: filepath.Dir(cfg.ConfigFilePath), WorkspaceFolder: opts.workspaceFolder, - SkipFeatureAutoMapping: opts.skipFeatureAutoMapping, FrozenLockfile: opts.experimentalFrozenLockfile, NoLockfile: opts.noLockfile, ConfigPath: cfg.ConfigFilePath, diff --git a/internal/features/resolve.go b/internal/features/resolve.go index 2454701..ea9b231 100644 --- a/internal/features/resolve.go +++ b/internal/features/resolve.go @@ -2,7 +2,9 @@ package features import ( "fmt" + "maps" "path/filepath" + "slices" "strings" ) @@ -13,25 +15,20 @@ const ( SourceDirectTarball SourceLocalPath SourceLegacyShorthand - SourceGitHubShorthand ) func ClassifyID(id string) SourceType { + if !strings.ContainsAny(id, `/\`) { + return SourceLegacyShorthand + } + // TS takes only https:// as a tarball URL. if strings.HasPrefix(id, "http://") || strings.HasPrefix(id, "https://") { return SourceDirectTarball } if strings.HasPrefix(id, "./") || strings.HasPrefix(id, "../") || strings.HasPrefix(id, "/") { return SourceLocalPath } - firstSlash := strings.Index(id, "/") - if firstSlash > 0 { - firstSegment := id[:firstSlash] - if strings.Contains(firstSegment, ".") || strings.HasPrefix(firstSegment, "localhost") { - return SourceOCI - } - return SourceGitHubShorthand - } - return SourceLegacyShorthand + return SourceOCI } type InvalidLocalFeatureError struct{ Msg string } @@ -72,8 +69,6 @@ func LocalFeaturePath(id, configDir, workspaceRoot string) (string, error) { return featureFolderPath, nil } -const versionBackwardComp = "1" - const newFeaturePath = "ghcr.io/devcontainers/features" var migratedFeatures = map[string]struct{}{ @@ -89,52 +84,61 @@ var renamedFeatures = map[string]string{ "common": "common-utils", } -var DeprecatedFeatureIntoOptions = map[string]struct { - MapTo string - Option string +var deprecatedFeaturesIntoOptions = map[string]struct { + mapTo string + option string }{ "gradle": {"java", "installGradle"}, "maven": {"java", "installMaven"}, "jupyterlab": {"python", "installJupyterlab"}, } -func IsKnownLegacyFeature(name string) bool { - if _, migrated := migratedFeatures[name]; migrated || renamedFeatures[name] != "" { - return true - } - _, ok := DeprecatedFeatureIntoOptions[name] - return ok -} - -func ResolveID(id string, skipAutoMapping bool) (string, bool) { - srcType := ClassifyID(id) - - if srcType == SourceGitHubShorthand { - return "ghcr.io/" + id, true +func BackwardCompatibleID(id string) (mapped, warning string) { + if _, ok := migratedFeatures[id]; ok { + return newFeaturePath + "/" + id + ":1", deprecatedFeatureWarning(id, id) } - - if srcType != SourceLegacyShorthand { - return id, false + if to, ok := renamedFeatures[id]; ok { + return newFeaturePath + "/" + to + ":1", deprecatedFeatureWarning(id, to) } - - if skipAutoMapping { - return id, false + if m, ok := deprecatedFeaturesIntoOptions[id]; ok { + return id, fmt.Sprintf("(!) WARNING: Falling back to the deprecated '%s' Feature. It is now part of the '%s' Feature. See https://github.com/devcontainers/features/tree/main/src/%s#options for the updated Feature.", id, m.mapTo, m.mapTo) } + return id, "" +} - // Split the name from an explicit version tag. Auto-mapped legacy features - // are pinned to versionBackwardComp (:1) unless the user gave an explicit - // version — matching the TS getBackwardCompatibleFeatureId (which pins :1), - // while still honoring an explicit tag if provided. - name := id - tag := ":" + versionBackwardComp - if idx := strings.LastIndex(id, ":"); idx > 0 { - name = id[:idx] - tag = id[idx:] - } +func deprecatedFeatureWarning(id, src string) string { + return fmt.Sprintf("(!) WARNING: Using the deprecated '%s' Feature. See https://github.com/devcontainers/features/tree/main/src/%s#example-usage for the updated Feature.", id, src) +} - target := name - if renamed, ok := renamedFeatures[name]; ok { - target = renamed +func UpdateDeprecatedFeaturesIntoOptions(userFeatures []DevContainerFeature) (updated []DevContainerFeature, warnings []string) { + updated = slices.Clone(userFeatures) + for _, f := range userFeatures { + m, ok := deprecatedFeaturesIntoOptions[f.UserFeatureID] + if !ok { + continue + } + warnings = append(warnings, fmt.Sprintf("(!) WARNING: Using the deprecated '%s' Feature. It is now part of the '%s' Feature. See https://github.com/devcontainers/features/tree/main/src/%s#options for the updated Feature.", f.UserFeatureID, m.mapTo, m.mapTo)) + qualified := newFeaturePath + "/" + m.mapTo + i := slices.IndexFunc(updated, func(u DevContainerFeature) bool { + return u.UserFeatureID == m.mapTo || u.UserFeatureID == qualified || strings.HasPrefix(u.UserFeatureID, qualified+":") + }) + if i == -1 { + updated = append(updated, DevContainerFeature{UserFeatureID: qualified + ":1", Options: map[string]any{m.option: true}}) + continue + } + options := map[string]any{} + switch o := updated[i].Options.(type) { + case map[string]any: + maps.Copy(options, o) + case string: + options["version"] = o + } + options[m.option] = true + updated[i].Options = options } - return fmt.Sprintf("%s/%s%s", newFeaturePath, target, tag), true + updated = slices.DeleteFunc(updated, func(u DevContainerFeature) bool { + _, deprecated := deprecatedFeaturesIntoOptions[u.UserFeatureID] + return deprecated + }) + return updated, warnings } diff --git a/internal/features/resolve_held_test.go b/internal/features/resolve_held_test.go index 98c8d2c..1b612e4 100644 --- a/internal/features/resolve_held_test.go +++ b/internal/features/resolve_held_test.go @@ -2,14 +2,21 @@ package features import "testing" -func TestResolveIDReadsOneCharacterNames(t *testing.T) { - for _, tt := range []struct{ id, want string }{ - {"a/b", "ghcr.io/a/b"}, - {"a/b:2", "ghcr.io/a/b:2"}, - {"a:2", "ghcr.io/devcontainers/features/a:2"}, +func TestClassifyIDReadsOneCharacterNames(t *testing.T) { + for _, tt := range []struct { + id string + want SourceType + }{ + {"a/b", SourceOCI}, + {"a/b:2", SourceOCI}, + {"a:2", SourceLegacyShorthand}, + {"a", SourceLegacyShorthand}, } { - if got, mapped := ResolveID(tt.id, false); got != tt.want || !mapped { - t.Errorf("ResolveID(%q) = %q, %v; want %q, true", tt.id, got, mapped, tt.want) + if got := ClassifyID(tt.id); got != tt.want { + t.Errorf("ClassifyID(%q) = %d, want %d", tt.id, got, tt.want) + } + if got, warning := BackwardCompatibleID(tt.id); got != tt.id || warning != "" { + t.Errorf("BackwardCompatibleID(%q) = %q, %q; want it unchanged", tt.id, got, warning) } } } diff --git a/internal/features/resolve_test.go b/internal/features/resolve_test.go index 452b1c8..951c5ff 100644 --- a/internal/features/resolve_test.go +++ b/internal/features/resolve_test.go @@ -3,6 +3,8 @@ package features import ( "errors" "path/filepath" + "reflect" + "strings" "testing" ) @@ -15,6 +17,9 @@ func TestClassifyFeatureID(t *testing.T) { {"ghcr.io/devcontainers/features/node:latest", SourceOCI}, {"myregistry.azurecr.io/myns/myfeature:1.0", SourceOCI}, {"localhost:5000/ns/feat:1", SourceOCI}, + {"owner/repo/feat", SourceOCI}, + {"owner/feat", SourceOCI}, + {`a\b`, SourceOCI}, {"https://example.com/feature.tgz", SourceDirectTarball}, {"http://example.com/feature.tgz", SourceDirectTarball}, {"./local-feature", SourceLocalPath}, @@ -22,6 +27,7 @@ func TestClassifyFeatureID(t *testing.T) { {"/abs/feature", SourceLocalPath}, {"go", SourceLegacyShorthand}, {"node", SourceLegacyShorthand}, + {"node:18", SourceLegacyShorthand}, {"my-feature", SourceLegacyShorthand}, } for _, tt := range tests { @@ -32,97 +38,55 @@ func TestClassifyFeatureID(t *testing.T) { } } -func TestResolveFeatureID(t *testing.T) { +func TestBackwardCompatibleID(t *testing.T) { + deprecated := func(id, src string) string { + return "(!) WARNING: Using the deprecated '" + id + "' Feature. See https://github.com/devcontainers/features/tree/main/src/" + src + "#example-usage for the updated Feature." + } tests := []struct { - name string - id string - skipAutoMapping bool - wantID string - wantMapped bool + id, wantID, wantWarning string }{ - { - name: "OCI", - id: "ghcr.io/devcontainers/features/go:1", - wantID: "ghcr.io/devcontainers/features/go:1", - wantMapped: false, - }, - { - name: "LegacyMigratedPinsV1", - id: "git", - wantID: "ghcr.io/devcontainers/features/git:1", - wantMapped: true, - }, - { - name: "LegacyRenamed_golang", - id: "golang", - wantID: "ghcr.io/devcontainers/features/go:1", - wantMapped: true, - }, - { - name: "LegacyRenamed_common", - id: "common", - wantID: "ghcr.io/devcontainers/features/common-utils:1", - wantMapped: true, - }, - { - name: "LegacyWithVersion", - id: "node:18", - wantID: "ghcr.io/devcontainers/features/node:18", - wantMapped: true, - }, - { - name: "SkipAutoMapping", - id: "go", - skipAutoMapping: true, - wantID: "go", - wantMapped: false, - }, - { - // unknown shorthand should be auto-mapped to ghcr.io/devcontainers/features/. - name: "LegacyUnknown", - id: "my-custom-feature", - wantID: "ghcr.io/devcontainers/features/my-custom-feature:1", - wantMapped: true, - }, - { - name: "LocalPath", - id: "./local-feature", - wantID: "./local-feature", - wantMapped: false, - }, - { - name: "Tarball", - id: "https://example.com/feature.tgz", - wantID: "https://example.com/feature.tgz", - wantMapped: false, - }, + {"git", "ghcr.io/devcontainers/features/git:1", deprecated("git", "git")}, + {"node", "ghcr.io/devcontainers/features/node:1", deprecated("node", "node")}, + {"golang", "ghcr.io/devcontainers/features/go:1", deprecated("golang", "go")}, + {"common", "ghcr.io/devcontainers/features/common-utils:1", deprecated("common", "common-utils")}, + {"gradle", "gradle", "(!) WARNING: Falling back to the deprecated 'gradle' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature."}, + {"node:18", "node:18", ""}, + {"node:1", "node:1", ""}, + {"golang:1", "golang:1", ""}, + {"my-custom-feature", "my-custom-feature", ""}, + {"owner/repo/feat", "owner/repo/feat", ""}, + {"ghcr.io/devcontainers/features/go:1", "ghcr.io/devcontainers/features/go:1", ""}, + {"./local-feature", "./local-feature", ""}, } for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - id, mapped := ResolveID(tt.id, tt.skipAutoMapping) - if id != tt.wantID { - t.Errorf("id = %q, want %q", id, tt.wantID) - } - if mapped != tt.wantMapped { - t.Errorf("mapped = %v, want %v", mapped, tt.wantMapped) - } - }) + id, warning := BackwardCompatibleID(tt.id) + if id != tt.wantID || warning != tt.wantWarning { + t.Errorf("BackwardCompatibleID(%q) = %q, %q; want %q, %q", tt.id, id, warning, tt.wantID, tt.wantWarning) + } } } -func TestDeprecatedFeatureIntoOptions(t *testing.T) { - for name, want := range map[string]struct{ mapTo, opt string }{ - "gradle": {"java", "installGradle"}, - "maven": {"java", "installMaven"}, - "jupyterlab": {"python", "installJupyterlab"}, - } { - m, ok := DeprecatedFeatureIntoOptions[name] - if !ok || m.MapTo != want.mapTo || m.Option != want.opt { - t.Errorf("%s → %+v, want %+v", name, m, want) - } - if !IsKnownLegacyFeature(name) { - t.Errorf("%s should be a known legacy feature", name) - } +func TestUpdateDeprecatedFeaturesIntoOptions(t *testing.T) { + in := []DevContainerFeature{ + {UserFeatureID: "ghcr.io/devcontainers/features/java:1", Options: "17"}, + {UserFeatureID: "gradle", Options: map[string]any{"ignored": true}}, + {UserFeatureID: "jupyterlab", Options: true}, + {UserFeatureID: "node:18", Options: map[string]any{}}, + } + got, warnings := UpdateDeprecatedFeaturesIntoOptions(in) + want := []DevContainerFeature{ + {UserFeatureID: "ghcr.io/devcontainers/features/java:1", Options: map[string]any{"version": "17", "installGradle": true}}, + {UserFeatureID: "node:18", Options: map[string]any{}}, + {UserFeatureID: "ghcr.io/devcontainers/features/python:1", Options: map[string]any{"installJupyterlab": true}}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("features = %v, want %v", got, want) + } + if len(warnings) != 2 || !strings.HasPrefix(warnings[0], "(!) WARNING: Using the deprecated 'gradle' Feature. It is now part of the 'java' Feature.") { + t.Errorf("warnings = %q", warnings) + } + if in[0].Options != "17" || len(in) != 4 { + t.Errorf("the input was modified: %v", in) } } diff --git a/internal/features/testdata/ts-oracle/legacy.json b/internal/features/testdata/ts-oracle/legacy.json new file mode 100644 index 0000000..4aea475 --- /dev/null +++ b/internal/features/testdata/ts-oracle/legacy.json @@ -0,0 +1,501 @@ +{ + "comment": "containerFeaturesConfiguration.ts getBackwardCompatibleFeatureId, updateDeprecatedFeaturesIntoOptions and the legacy, tarball and local-path classification of getFeatureIdType, ported in resolve.go (BackwardCompatibleID, UpdateDeprecatedFeaturesIntoOptions, ClassifyID).", + "cases": [ + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "node" + ], + "result": { + "id": "ghcr.io/devcontainers/features/node:1", + "log": [ + "(!) WARNING: Using the deprecated 'node' Feature. See https://github.com/devcontainers/features/tree/main/src/node#example-usage for the updated Feature." + ] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "terraform" + ], + "result": { + "id": "ghcr.io/devcontainers/features/terraform:1", + "log": [ + "(!) WARNING: Using the deprecated 'terraform' Feature. See https://github.com/devcontainers/features/tree/main/src/terraform#example-usage for the updated Feature." + ] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "golang" + ], + "result": { + "id": "ghcr.io/devcontainers/features/go:1", + "log": [ + "(!) WARNING: Using the deprecated 'golang' Feature. See https://github.com/devcontainers/features/tree/main/src/go#example-usage for the updated Feature." + ] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "common" + ], + "result": { + "id": "ghcr.io/devcontainers/features/common-utils:1", + "log": [ + "(!) WARNING: Using the deprecated 'common' Feature. See https://github.com/devcontainers/features/tree/main/src/common-utils#example-usage for the updated Feature." + ] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "gradle" + ], + "result": { + "id": "gradle", + "log": [ + "(!) WARNING: Falling back to the deprecated 'gradle' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature." + ] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "maven" + ], + "result": { + "id": "maven", + "log": [ + "(!) WARNING: Falling back to the deprecated 'maven' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature." + ] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "jupyterlab" + ], + "result": { + "id": "jupyterlab", + "log": [ + "(!) WARNING: Falling back to the deprecated 'jupyterlab' Feature. It is now part of the 'python' Feature. See https://github.com/devcontainers/features/tree/main/src/python#options for the updated Feature." + ] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "node:18" + ], + "result": { + "id": "node:18", + "log": [] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "node:1" + ], + "result": { + "id": "node:1", + "log": [] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "golang:1" + ], + "result": { + "id": "golang:1", + "log": [] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "go" + ], + "result": { + "id": "go", + "log": [] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "my-custom-feature" + ], + "result": { + "id": "my-custom-feature", + "log": [] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "owner/repo/feat" + ], + "result": { + "id": "owner/repo/feat", + "log": [] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "devcontainers/features/node:1" + ], + "result": { + "id": "devcontainers/features/node:1", + "log": [] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "ghcr.io/devcontainers/features/node:1" + ], + "result": { + "id": "ghcr.io/devcontainers/features/node:1", + "log": [] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "./node" + ], + "result": { + "id": "./node", + "log": [] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "https://example.com/devcontainer-feature-node.tgz" + ], + "result": { + "id": "https://example.com/devcontainer-feature-node.tgz", + "log": [] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "Node" + ], + "result": { + "id": "Node", + "log": [] + } + }, + { + "fn": "getBackwardCompatibleFeatureId", + "args": [ + "" + ], + "result": { + "id": "", + "log": [] + } + }, + { + "fn": "updateDeprecatedFeaturesIntoOptions", + "args": [ + [ + { + "userFeatureId": "gradle", + "options": {} + } + ] + ], + "result": { + "userFeatures": [ + { + "userFeatureId": "ghcr.io/devcontainers/features/java:1", + "options": { + "installGradle": true + } + } + ], + "log": [ + "(!) WARNING: Using the deprecated 'gradle' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature." + ] + } + }, + { + "fn": "updateDeprecatedFeaturesIntoOptions", + "args": [ + [ + { + "userFeatureId": "gradle", + "options": { + "version": "8" + } + }, + { + "userFeatureId": "maven", + "options": true + } + ] + ], + "result": { + "userFeatures": [ + { + "userFeatureId": "ghcr.io/devcontainers/features/java:1", + "options": { + "installGradle": true, + "installMaven": true + } + } + ], + "log": [ + "(!) WARNING: Using the deprecated 'gradle' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature.", + "(!) WARNING: Using the deprecated 'maven' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature." + ] + } + }, + { + "fn": "updateDeprecatedFeaturesIntoOptions", + "args": [ + [ + { + "userFeatureId": "java", + "options": "17" + }, + { + "userFeatureId": "gradle", + "options": {} + } + ] + ], + "result": { + "userFeatures": [ + { + "userFeatureId": "java", + "options": { + "version": "17", + "installGradle": true + } + } + ], + "log": [ + "(!) WARNING: Using the deprecated 'gradle' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature." + ] + } + }, + { + "fn": "updateDeprecatedFeaturesIntoOptions", + "args": [ + [ + { + "userFeatureId": "ghcr.io/devcontainers/features/java:1", + "options": { + "version": "21" + } + }, + { + "userFeatureId": "maven", + "options": {} + } + ] + ], + "result": { + "userFeatures": [ + { + "userFeatureId": "ghcr.io/devcontainers/features/java:1", + "options": { + "version": "21", + "installMaven": true + } + } + ], + "log": [ + "(!) WARNING: Using the deprecated 'maven' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature." + ] + } + }, + { + "fn": "updateDeprecatedFeaturesIntoOptions", + "args": [ + [ + { + "userFeatureId": "ghcr.io/devcontainers/features/java", + "options": true + }, + { + "userFeatureId": "gradle", + "options": {} + } + ] + ], + "result": { + "userFeatures": [ + { + "userFeatureId": "ghcr.io/devcontainers/features/java", + "options": { + "installGradle": true + } + } + ], + "log": [ + "(!) WARNING: Using the deprecated 'gradle' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature." + ] + } + }, + { + "fn": "updateDeprecatedFeaturesIntoOptions", + "args": [ + [ + { + "userFeatureId": "ghcr.io/devcontainers/features/javascript:1", + "options": {} + }, + { + "userFeatureId": "gradle", + "options": {} + } + ] + ], + "result": { + "userFeatures": [ + { + "userFeatureId": "ghcr.io/devcontainers/features/javascript:1", + "options": {} + }, + { + "userFeatureId": "ghcr.io/devcontainers/features/java:1", + "options": { + "installGradle": true + } + } + ], + "log": [ + "(!) WARNING: Using the deprecated 'gradle' Feature. It is now part of the 'java' Feature. See https://github.com/devcontainers/features/tree/main/src/java#options for the updated Feature." + ] + } + }, + { + "fn": "updateDeprecatedFeaturesIntoOptions", + "args": [ + [ + { + "userFeatureId": "jupyterlab", + "options": {} + }, + { + "userFeatureId": "node:18", + "options": {} + } + ] + ], + "result": { + "userFeatures": [ + { + "userFeatureId": "node:18", + "options": {} + }, + { + "userFeatureId": "ghcr.io/devcontainers/features/python:1", + "options": { + "installJupyterlab": true + } + } + ], + "log": [ + "(!) WARNING: Using the deprecated 'jupyterlab' Feature. It is now part of the 'python' Feature. See https://github.com/devcontainers/features/tree/main/src/python#options for the updated Feature." + ] + } + }, + { + "fn": "updateDeprecatedFeaturesIntoOptions", + "args": [ + [ + { + "userFeatureId": "gradle:1", + "options": {} + }, + { + "userFeatureId": "node", + "options": {} + } + ] + ], + "result": { + "userFeatures": [ + { + "userFeatureId": "gradle:1", + "options": {} + }, + { + "userFeatureId": "node", + "options": {} + } + ], + "log": [] + } + }, + { + "fn": "getFeatureIdType", + "args": [ + "node" + ], + "error": "Legacy feature 'node' not supported. Please check https://containers.dev/features for replacements.\nIf you were hoping to use local Features, remember to prepend your Feature name with \"./\". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information." + }, + { + "fn": "getFeatureIdType", + "args": [ + "node:18" + ], + "error": "Legacy feature 'node:18' not supported. Please check https://containers.dev/features for replacements.\nIf you were hoping to use local Features, remember to prepend your Feature name with \"./\". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information." + }, + { + "fn": "getFeatureIdType", + "args": [ + "gradle" + ], + "error": "Legacy feature 'gradle' not supported. Please check https://containers.dev/features for replacements.\nIf you were hoping to use local Features, remember to prepend your Feature name with \"./\". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information." + }, + { + "fn": "getFeatureIdType", + "args": [ + "my-custom-feature" + ], + "error": "Legacy feature 'my-custom-feature' not supported. Please check https://containers.dev/features for replacements.\nIf you were hoping to use local Features, remember to prepend your Feature name with \"./\". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information." + }, + { + "fn": "getFeatureIdType", + "args": [ + "https://example.com/devcontainer-feature-node.tgz" + ], + "result": "direct-tarball" + }, + { + "fn": "getFeatureIdType", + "args": [ + "./node" + ], + "result": "file-path" + }, + { + "fn": "getFeatureIdType", + "args": [ + "../node" + ], + "result": "file-path" + }, + { + "fn": "getFeatureIdType", + "args": [ + "/node" + ], + "result": "file-path" + } + ] +} diff --git a/internal/features/ts_oracle_test.go b/internal/features/ts_oracle_test.go index 23315ec..1a30e1e 100644 --- a/internal/features/ts_oracle_test.go +++ b/internal/features/ts_oracle_test.go @@ -67,3 +67,57 @@ func TestTSOracleLockfile(t *testing.T) { }, }) } + +func TestTSOracleLegacyFeatureIDs(t *testing.T) { + oracletest.Run(t, "testdata/ts-oracle/legacy.json", map[string]oracletest.Func{ + "getBackwardCompatibleFeatureId": func(args []json.RawMessage) (any, error) { + var id string + if err := oracletest.Args(args, &id); err != nil { + return nil, err + } + mapped, warning := BackwardCompatibleID(id) + log := []string{} + if warning != "" { + log = append(log, warning) + } + return map[string]any{"id": mapped, "log": log}, nil + }, + "updateDeprecatedFeaturesIntoOptions": func(args []json.RawMessage) (any, error) { + var in []struct { + UserFeatureID string `json:"userFeatureId"` + Options any `json:"options"` + } + if err := oracletest.Args(args, &in); err != nil { + return nil, err + } + userFeatures := make([]DevContainerFeature, len(in)) + for i, f := range in { + userFeatures[i] = DevContainerFeature{UserFeatureID: f.UserFeatureID, Options: f.Options} + } + updated, warnings := UpdateDeprecatedFeaturesIntoOptions(userFeatures) + out := make([]map[string]any, len(updated)) + for i, f := range updated { + out[i] = map[string]any{"userFeatureId": f.UserFeatureID, "options": f.Options} + } + if warnings == nil { + warnings = []string{} + } + return map[string]any{"userFeatures": out, "log": warnings}, nil + }, + "getFeatureIdType": func(args []json.RawMessage) (any, error) { + var id string + if err := oracletest.Args(args, &id); err != nil { + return nil, err + } + switch ClassifyID(id) { + case SourceLegacyShorthand: + return nil, oracletest.ErrThrows + case SourceDirectTarball: + return "direct-tarball", nil + case SourceLocalPath: + return "file-path", nil + } + return "oci", nil + }, + }) +} diff --git a/tools/tsoracle/oracle.ts b/tools/tsoracle/oracle.ts index 26c986f..e4a0194 100644 --- a/tools/tsoracle/oracle.ts +++ b/tools/tsoracle/oracle.ts @@ -11,7 +11,7 @@ import { parseVersion, isEarlierVersion, getEntPasswdShellCommand } from '../../ import { substitute, beforeContainerSubstitute, containerSubstitute } from '../../reference/src/spec-common/variableSubstitution'; import { extractDockerfile, findBaseImage, findUserStatement, ensureDockerfileHasFinalStageName, supportsBuildContexts } from '../../reference/src/spec-node/dockerfileUtils'; import { mergeConfiguration, getDevcontainerMetadataLabel } from '../../reference/src/spec-node/imageMetadata'; -import { generateContainerEnvs, getFeatureValueObject, parseMount, escapeQuotesForShell } from '../../reference/src/spec-configuration/containerFeaturesConfiguration'; +import { generateContainerEnvs, getFeatureValueObject, parseMount, escapeQuotesForShell, getBackwardCompatibleFeatureId, getFeatureIdType, updateDeprecatedFeaturesIntoOptions } from '../../reference/src/spec-configuration/containerFeaturesConfiguration'; import { getSafeId } from '../../reference/src/spec-node/containerFeatures'; import { getFeatureIdWithoutVersion } from '../../reference/src/spec-configuration/containerFeaturesOCI'; import { generateLockfile } from '../../reference/src/spec-configuration/lockfile'; @@ -28,6 +28,9 @@ import { readLockfile } from '../../reference/src/spec-configuration/lockfile'; const nullOutput: any = { write() { }, raw() { }, start() { return 0; }, stop() { }, event() { } }; +// An output that keeps the text of every line written to it. +const capturingOutput = (lines: string[]): any => ({ ...nullOutput, write(text: string) { lines.push(text); } }); + // A SubstitutionContext as JSON: configFile is a path, made the URI TS carries. function substitutionContext(c: any) { return { ...c, configFile: c.configFile === undefined ? undefined : URI.file(c.configFile) }; @@ -64,6 +67,18 @@ const fns: Record any> = { // The value object's key order is the env file's line order: entries keep it. getFeatureValueObject: (feature: any) => Object.entries(getFeatureValueObject(feature)), escapeQuotesForShell: (s: string) => escapeQuotesForShell(s), + // The id a Feature is resolved by and the lines logged on the way. + getBackwardCompatibleFeatureId: (id: string) => { + const log: string[] = []; + return { id: getBackwardCompatibleFeatureId(capturingOutput(log), id), log }; + }, + updateDeprecatedFeaturesIntoOptions: (userFeatures: any[]) => { + const log: string[] = []; + return { userFeatures: updateDeprecatedFeaturesIntoOptions(userFeatures, capturingOutput(log)), log }; + }, + // Only ids it classifies without a registry request: legacy ids (it + // throws), tarball URLs and local paths. + getFeatureIdType: async (id: string) => (await getFeatureIdType({ output: nullOutput, env: {} } as any, id, undefined)).type, // spec-node/containerFeatures.ts getSafeId: (s: string) => getSafeId(s), // spec-configuration/containerFeaturesOCI.ts