diff --git a/docs/migration.md b/docs/migration.md index eef2e28..6a954b1 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -208,6 +208,20 @@ runs the official CLI is unaffected: - Only a registry whose host is `localhost` is reached over plain HTTP. Earlier releases also used plain HTTP for `127.0.0.1`, which is now addressed over HTTPS like any other host. Refer to a local plain-HTTP registry as `localhost:`. +- Only an `https://` Feature id is a tarball URL. An `http://` id is now handled + as the official CLI handles it: as a registry reference it cannot parse, so + `build`, `up`, `read-configuration` and `upgrade` exit 1 with `ERR: Feature + '' could not be processed. ...` without downloading anything. Earlier + releases downloaded the tarball over plain HTTP. Serve the tarball over HTTPS + (with `NODE_EXTRA_CA_CERTS` or `SSL_CERT_FILE` naming a private CA if needed), + or publish the Feature to an OCI registry. +- Features are resolved in the order the configuration declares them, as the + official CLI resolves them: integer-like ids such as `"9"` first, ascending, + then the others in declaration order, followed by the `--additional-features` + ids the configuration does not name. When several Features fail, the one + reported is the first in that order; earlier releases reported the first by + sorted id. `build` no longer reports a legacy Feature id before trying the + Features declared ahead of it, or before pulling the image. ## Rollback diff --git a/docs/parity/parity-matrix.yaml b/docs/parity/parity-matrix.yaml index 3c114b1..692cb4c 100644 --- a/docs/parity/parity-matrix.yaml +++ b/docs/parity/parity-matrix.yaml @@ -975,6 +975,71 @@ initial_cases: current_status: match notes: "With the configuration at the workspace root (.devcontainer.json), './feat' resolves outside the .devcontainer folder, which processFeatureIdentifier refuses; build reports the plain Error buildDependencyGraph throws as its message: {\"outcome\":\"error\",\"message\":\"ERR: Feature './feat' could not be processed. You may not have permission ...\",\"description\":\"An error occurred building the container.\"}, exit 1. Go used to build the Feature." + - id: features.resolve-dependencies-declaration-order-success + lane: contract + command: features + priority: p1 + docker_required: false + setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer/zeta $WORKSPACE/.devcontainer/alpha $WORKSPACE/.devcontainer/mid $WORKSPACE/.devcontainer/beta $WORKSPACE/.devcontainer/omega; printf '%s' '{\"id\":\"zeta\",\"version\":\"1.0.0\"}' > $WORKSPACE/.devcontainer/zeta/devcontainer-feature.json; printf '%s' '{\"id\":\"alpha\",\"version\":\"1.0.0\"}' > $WORKSPACE/.devcontainer/alpha/devcontainer-feature.json; printf '%s' '{\"id\":\"mid\",\"version\":\"1.0.0\"}' > $WORKSPACE/.devcontainer/mid/devcontainer-feature.json; printf '%s' '{\"id\":\"beta\",\"version\":\"1.0.0\"}' > $WORKSPACE/.devcontainer/beta/devcontainer-feature.json; printf '%s' '{\"id\":\"omega\",\"version\":\"1.0.0\"}' > $WORKSPACE/.devcontainer/omega/devcontainer-feature.json; printf '%s' '{\"image\":\"alpine:3\",\"features\":{\"./zeta\":{},\"./alpha\":{},\"./mid\":{},\"./beta\":{},\"./omega\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE" + cleanup_cmd: "rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}" + ts_cmd: "features resolve-dependencies --workspace-folder ${WORKSPACE}" + asserts: [exit_code, stdout_text] + class: features + current_status: match + notes: "userFeaturesToArray (containerFeaturesConfiguration.ts) lists the configuration's Features in Object.keys order, and the mermaid graph lists the dependency graph's worklist in that order: ./zeta, ./alpha, ./mid, ./beta, ./omega. Go used to take them in Go map order." + + - id: build.features-first-failure-in-declaration-order-error + lane: runtime + command: build + priority: p1 + docker_required: true + setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer $WORKSPACE/zeta; printf '%s' '{\"image\":\"alpine:3\",\"features\":{\"../zeta\":{},\"zz\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE" + cleanup_cmd: "rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}" + ts_cmd: "build --workspace-folder ${WORKSPACE}" + asserts: [exit_code, stdout_normalized] + class: features + current_status: match + notes: "Features are processed in Object.keys order and the first that fails is reported: '../zeta' is refused (outside .devcontainer) before the legacy id 'zz' is classified, so build reports ERR: Feature '../zeta' could not be processed. Go used to check every id for a legacy one before resolving any Feature and reported 'Legacy feature 'zz' not supported.'." + + - id: build.features-integer-like-ids-first-error + lane: runtime + command: build + priority: p1 + docker_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\":{\"../zeta\":{},\"10\":{},\"9\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE" + cleanup_cmd: "rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}" + ts_cmd: "build --workspace-folder ${WORKSPACE}" + asserts: [exit_code, stdout_normalized] + class: features + current_status: match + notes: "Object.keys lists integer-like keys first, ascending, so '9' is processed first and build reports 'Legacy feature '9' not supported.'. Go used to sort the ids and reported '10'." + + - id: build.additional-features-after-config-error + lane: runtime + command: build + priority: p1 + docker_required: true + setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer $WORKSPACE/zeta $WORKSPACE/alpha; printf '%s' '{\"image\":\"alpine:3\",\"features\":{\"../zeta\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE" + cleanup_cmd: "rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}" + ts_cmd: "build --workspace-folder ${WORKSPACE} --additional-features '{\"../alpha\":{}}'" + asserts: [exit_code, stdout_normalized] + class: features + current_status: match + notes: "userFeaturesToArray appends the --additional-features ids after the configuration's, so '../zeta' is processed and refused first. Go used to sort the merged ids and reported '../alpha'." + + - id: build.features-http-id-not-a-tarball-error + lane: runtime + command: build + priority: p1 + docker_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\":{\"http://127.0.0.1:9/devcontainer-feature-a.tgz\":{}}}' > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE" + cleanup_cmd: "rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}" + ts_cmd: "build --workspace-folder ${WORKSPACE}" + asserts: [exit_code, stdout_normalized] + class: features + current_status: match + notes: "getFeatureIdType takes only https:// as a tarball URL; an http:// id goes to the OCI path, where getRef rejects it, then to the GitHub Releases fallback, which refuses an id that is not owner/repo/feature. buildDependencyGraph throws and build reports ERR: Feature 'http://...' could not be processed. You may not have permission ... without any request. Go used to download it as a tarball and reported the connection error." + - id: up.update-uid-remote-user-from-image-metadata-success lane: runtime command: up diff --git a/internal/cli/build.go b/internal/cli/build.go index 13b395f..914141b 100644 --- a/internal/cli/build.go +++ b/internal/cli/build.go @@ -12,7 +12,6 @@ import ( "github.com/devcontainers/cli/internal/config" "github.com/devcontainers/cli/internal/docker" coreerrors "github.com/devcontainers/cli/internal/errors" - "github.com/devcontainers/cli/internal/features" "github.com/devcontainers/cli/internal/imagemeta" "github.com/devcontainers/cli/internal/log" "github.com/devcontainers/cli/internal/stringify" @@ -329,10 +328,6 @@ func (r *buildRunner) buildImage(ctx context.Context, cfg *config.DevContainer, return nil, fmt.Errorf("no image specified in devcontainer.json") } - if err := checkLegacyFeatures(cfg.Features); err != nil { - return nil, err - } - logger.Write(fmt.Sprintf("Using image: %s", cfg.Image), log.LevelInfo) folderImage := folderImageName(resolvePath(opts.workspaceFolder)) @@ -456,17 +451,6 @@ func imageBuildFor(cfg *config.DevContainer, opts *buildOpts) imageBuild { return imageAsIs } -func checkLegacyFeatures(feats map[string]any) error { - userFeatures, _ := features.UpdateDeprecatedFeaturesIntoOptions(sortedUserFeatures(feats, nil)) - for _, f := range userFeatures { - id, _ := features.BackwardCompatibleID(f.UserFeatureID) - if features.ClassifyID(id) == features.SourceLegacyShorthand { - return &coreerrors.ContainerError{Description: fmt.Sprintf(msgLegacyFeature, id)} - } - } - return nil -} - func imageNamesOr(names []string, fallback string) []string { if len(names) == 0 { return []string{fallback} @@ -693,6 +677,7 @@ func (r *buildRunner) featureBuildOptions(cfg *config.DevContainer, hostSub *con Output: opts.output, ContainerEnv: cfg.ContainerEnv, ContainerEnvOrder: cfg.ContainerEnvOrder, + FeatureOrder: cfg.FeatureOrder, FeatureOptionOrder: cfg.FeatureOptionOrder, FrozenLockfile: opts.experimentalFrozenLockfile, NoLockfile: opts.noLockfile, diff --git a/internal/cli/build_held_test.go b/internal/cli/build_held_test.go index 9337ae7..3b347d7 100644 --- a/internal/cli/build_held_test.go +++ b/internal/cli/build_held_test.go @@ -106,14 +106,14 @@ func TestImageBuildFor(t *testing.T) { } } -func TestCheckLegacyFeatures(t *testing.T) { - 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 { +func TestResolveFeatureIDRefusesLegacyIDs(t *testing.T) { + for _, id := range []string{"node", "golang", "ghcr.io/devcontainers/features/node:1", "./local-feature", "owner/repo/feature"} { + if _, _, err := resolveFeatureID(log.Null, id); err != nil { t.Errorf("%s: unexpected error %v", id, err) } } for _, id := range []string{"not-a-feature", "not-a-feature:1", "node:1", "node:18", "gradle:1"} { - err := checkLegacyFeatures(map[string]any{id: true}) + _, _, err := resolveFeatureID(log.Null, id) 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 82663ab..abe6d1d 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, cfg.FeatureOptionOrder, localFeatureRoots{filepath.Dir(cfg.ConfigFilePath), opts.workspaceFolder}, lockfile, cfg.OverrideFeatureInstallOrder) + res, err := fetchFeatureSetsWithOrder(ctx, r.log, cfg.Features, cfg.FeatureOrder, cfg.FeatureOptionOrder, localFeatureRoots{filepath.Dir(cfg.ConfigFilePath), opts.workspaceFolder}, lockfile, cfg.OverrideFeatureInstallOrder) if err != nil { return nil, "", err } diff --git a/internal/cli/compose_up_held_test.go b/internal/cli/compose_up_held_test.go index 404a488..e041b8b 100644 --- a/internal/cli/compose_up_held_test.go +++ b/internal/cli/compose_up_held_test.go @@ -9,7 +9,6 @@ import ( "errors" "io" "net/http" - "net/http/httptest" "os" "path/filepath" "regexp" @@ -685,9 +684,7 @@ func TestComposeFeatureSets(t *testing.T) { }) t.Run("lockfile pins a tarball", func(t *testing.T) { tgz := featureTarball(t, "tarred") - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(tgz) })) - defer srv.Close() - uri := srv.URL + "/devcontainer-feature-tarred.tgz" + uri := serveTrustedTLS(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Write(tgz) })) + "/devcontainer-feature-tarred.tgz" for _, noLockfile := range []bool{false, true} { h := newComposeHarness(t) h.r.opts.noLockfile = noLockfile diff --git a/internal/cli/feature_deps.go b/internal/cli/feature_deps.go index f3f340f..b867996 100644 --- a/internal/cli/feature_deps.go +++ b/internal/cli/feature_deps.go @@ -73,7 +73,7 @@ func newMetadataProcessFeature(client oci.Registry, logger log.Logger, local loc default: ref, err := oci.ParseRef(resolvedID) if err != nil { - return nil, fmt.Errorf("ERR: Feature '%s' could not be processed. %w", id, err) + return nil, unparseableFeatureRef(logger, id, resolvedID, err) } if lockfile != nil { if integrity := lockfile.Features[resolvedID].Integrity; integrity != "" { diff --git a/internal/cli/feature_install.go b/internal/cli/feature_install.go index 4f75788..c10bc1a 100644 --- a/internal/cli/feature_install.go +++ b/internal/cli/feature_install.go @@ -18,7 +18,6 @@ import ( "path/filepath" "regexp" "slices" - "sort" "strconv" "strings" "time" @@ -48,6 +47,7 @@ type FeatureBuildOptions struct { CABundle string ContainerEnv map[string]string ContainerEnvOrder []string + FeatureOrder []string FeatureOptionOrder map[string][]string SkipPersistCustoms bool FeaturesBasePath string @@ -67,16 +67,8 @@ type fetchFeatureResult struct { TmpDir string } -func fetchFeatureSets(ctx context.Context, logger log.Logger, featuresCfg map[string]any, optionOrder map[string][]string, local localFeatureRoots) (*fetchFeatureResult, error) { - return fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, optionOrder, local, nil, nil) -} - -func sortedUserFeatures(featuresCfg map[string]any, optionOrder map[string][]string) []features.DevContainerFeature { - userFeatures := features.UserFeaturesToArray(featuresCfg, optionOrder) - sort.Slice(userFeatures, func(i, j int) bool { - return userFeatures[i].UserFeatureID < userFeatures[j].UserFeatureID - }) - return userFeatures +func fetchFeatureSets(ctx context.Context, logger log.Logger, featuresCfg map[string]any, order []string, optionOrder map[string][]string, local localFeatureRoots) (*fetchFeatureResult, error) { + return fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, order, optionOrder, local, nil, nil) } type localFeatureRoots struct { @@ -113,13 +105,12 @@ func createFeaturesFolder() (string, error) { } } -func fetchFeatureSetsWithOrder(ctx context.Context, logger log.Logger, featuresCfg map[string]any, optionOrder map[string][]string, local localFeatureRoots, lockfile *features.Lockfile, overrideOrder []string) (*fetchFeatureResult, error) { +func fetchFeatureSetsWithOrder(ctx context.Context, logger log.Logger, featuresCfg map[string]any, order []string, optionOrder map[string][]string, local localFeatureRoots, lockfile *features.Lockfile, overrideOrder []string) (*fetchFeatureResult, error) { if len(featuresCfg) == 0 { return nil, nil } - userFeatures := sortedUserFeatures(featuresCfg, optionOrder) - userFeatures, warnings := features.UpdateDeprecatedFeaturesIntoOptions(userFeatures) + userFeatures, warnings := features.UpdateDeprecatedFeaturesIntoOptions(features.UserFeaturesToArray(featuresCfg, order, optionOrder)) for _, w := range warnings { logger.Write(w, log.LevelWarning) } @@ -306,7 +297,7 @@ func processInstallFeature( default: ref, err := oci.ParseRef(resolvedID) if err != nil { - return nil, fmt.Errorf("ERR: Feature '%s' could not be processed. %w", id, err) + return nil, unparseableFeatureRef(logger, id, resolvedID, err) } userRef := ref @@ -588,7 +579,7 @@ func fetchBuildFeatures(ctx context.Context, logger log.Logger, featuresCfg map[ return nil, err } } - result, err := fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, fbOpts.FeatureOptionOrder, localFeatureRoots{fbOpts.FeaturesBasePath, fbOpts.WorkspaceFolder}, lockfile, fbOpts.OverrideFeatureInstallOrder) + result, err := fetchFeatureSetsWithOrder(ctx, logger, featuresCfg, fbOpts.FeatureOrder, fbOpts.FeatureOptionOrder, localFeatureRoots{fbOpts.FeaturesBasePath, fbOpts.WorkspaceFolder}, lockfile, fbOpts.OverrideFeatureInstallOrder) if err != nil { return nil, err } @@ -995,6 +986,27 @@ func isPlainHTTPURL(u string) bool { return parsed.Scheme == "http" || parsed.Hostname() == "localhost" } +var githubReleaseFeatureName = regexp.MustCompile(`^[a-zA-Z0-9_-]*$`) + +// unparseableFeatureRef returns nil, after logging why, for a reference that +// TS's GitHub Releases fallback refuses, which leaves the Feature +// unprocessed; this CLI lacks that fallback, so it fails on any other. +func unparseableFeatureRef(logger log.Logger, id, resolvedID string, parseErr error) error { + at := strings.Split(resolvedID, "@") + var refusal string + switch parts := strings.Split(at[0], "/"); { + case len(at) > 2: + refusal = "Parse error. Use the '@' symbol only to designate a version tag." + case len(parts) != 3 || slices.Contains(parts, "") || !githubReleaseFeatureName.MatchString(parts[2]): + refusal = fmt.Sprintf("Could not resolve Feature '%s'. Ensure the Feature is published and accessible from your current environment.", resolvedID) + default: + return fmt.Errorf("ERR: Feature '%s' could not be processed. %w", id, parseErr) + } + logger.Write(fmt.Sprintf("Could not resolve Feature manifest for '%s'. If necessary, provide registry credentials with 'docker login '.", resolvedID), log.LevelWarning) + logger.Write(refusal, log.LevelError) + return nil +} + func resolveFeatureID(logger log.Logger, id string) (string, features.SourceType, error) { resolvedID, warning := features.BackwardCompatibleID(id) if warning != "" { diff --git a/internal/cli/feature_install_held_test.go b/internal/cli/feature_install_held_test.go index 5cfcb8d..5fad5f7 100644 --- a/internal/cli/feature_install_held_test.go +++ b/internal/cli/feature_install_held_test.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "encoding/pem" "errors" "fmt" "io" @@ -337,11 +338,23 @@ func TestProcessInstallFeatureOCIErrors(t *testing.T) { func serveBytes(t *testing.T, body []byte, status int) string { t.Helper() - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + return serveTrustedTLS(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(status) w.Write(body) })) +} + +func serveTrustedTLS(t *testing.T, h http.Handler) string { + t.Helper() + srv := httptest.NewTLSServer(h) t.Cleanup(srv.Close) + caFile := filepath.Join(t.TempDir(), "ca.pem") + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: srv.Certificate().Raw}) + if err := os.WriteFile(caFile, certPEM, 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("NODE_EXTRA_CA_CERTS", "") + t.Setenv("SSL_CERT_FILE", caFile) return srv.URL } @@ -429,7 +442,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}, nil, devcontainerRoots(t)) + _, err := fetchFeatureSets(t.Context(), log.Null, map[string]any{"./b-missing": true, "./a-missing": true, "./c-missing": true}, nil, nil, 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 eb48c14..7fd4206 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, nil, roots) + result, err := fetchFeatureSets(t.Context(), log.Null, tt.entries, nil, nil, 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}, nil, roots) + result, err := fetchFeatureSets(t.Context(), log.Null, map[string]any{"./dir-a": true}, nil, nil, 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}, nil, roots) + result, err := fetchFeatureSets(t.Context(), logger, map[string]any{tc.id: true}, nil, nil, roots) if result != nil { t.Fatalf("result = %+v, want none", result) } @@ -420,7 +420,7 @@ 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{}}, nil, devcontainerRoots(t)) + result, err := fetchFeatureSets(t.Context(), logger, map[string]any{"gradle": map[string]any{}, "node:18": map[string]any{}}, nil, nil, 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) } @@ -480,7 +480,7 @@ func TestFetchedFeaturesKeepTheDeclaredOrder(t *testing.T) { write("dep", `{"id": "dep", "version": "1.0.0"}`) result, err := fetchFeatureSets(t.Context(), log.Null, - map[string]any{"./sdk": map[string]any{"zz": "1", "version": "2", "aa": "3"}}, + map[string]any{"./sdk": map[string]any{"zz": "1", "version": "2", "aa": "3"}}, nil, map[string][]string{"./sdk": {"zz", "version", "aa"}}, roots) if err != nil { t.Fatal(err) diff --git a/internal/cli/feature_order_test.go b/internal/cli/feature_order_test.go new file mode 100644 index 0000000..c6c764c --- /dev/null +++ b/internal/cli/feature_order_test.go @@ -0,0 +1,142 @@ +package cli + +import ( + "bytes" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "sync/atomic" + "testing" + + "github.com/devcontainers/cli/internal/features" + "github.com/devcontainers/cli/internal/log" +) + +func TestReadConfigurationReportsTheFirstFeatureInDeclarationOrder(t *testing.T) { + legacy9 := "Legacy feature '9' not supported." + refusedZeta := "ERR: Feature '../zeta' could not be processed." + for name, tc := range map[string]struct { + features, override, additional string + want string + }{ + "declaration order": {features: `{"../zeta":{},"../alpha":{}}`, want: refusedZeta}, + "integer-like ids first": {features: `{"../zeta":{},"10":{},"9":{}}`, want: legacy9}, + "override adds a Feature after": {features: `{"../zeta":{}}`, override: `{"features":{"../alpha":{}}}`, want: refusedZeta}, + "override integer-like id first": {features: `{"../zeta":{}}`, override: `{"features":{"9":{}}}`, want: legacy9}, + "additional Features after config": {features: `{"../zeta":{}}`, additional: `{"9":{},"../alpha":{}}`, want: refusedZeta}, + "additional integer-like id first": {features: `{"../zeta":{}}`, additional: `{"../alpha":{},"10":{},"9":{}}`, want: refusedZeta}, + } { + t.Run(name, func(t *testing.T) { + startFakeEngine(t, nil) + files := map[string]string{".devcontainer/devcontainer.json": `{"image":"alpine:3","features":` + tc.features + `}`} + if tc.override != "" { + files["override.json"] = tc.override + } + ws := writeConfigFixture(t, files) + args := []string{"--workspace-folder", ws, "--include-features-configuration"} + if tc.override != "" { + args = append(args, "--override-config", filepath.Join(ws, "override.json")) + } + if tc.additional != "" { + args = append(args, "--additional-features", tc.additional) + } + stdout, err := readConfigurationStdout(args...) + if err == nil || !strings.HasPrefix(err.Error(), tc.want) || stdout != "" { + t.Fatalf("stdout = %q, err = %v; want %q", stdout, err, tc.want) + } + }) + } +} + +func TestReadConfigurationReportsAnAdditionalIntegerLikeIDAfterTheConfigs(t *testing.T) { + startFakeEngine(t, nil) + ws := writeConfigFixture(t, map[string]string{".devcontainer/devcontainer.json": `{"image":"alpine:3","features":{"zz":{}}}`}) + _, err := readConfigurationStdout("--workspace-folder", ws, "--include-features-configuration", "--additional-features", `{"9":{}}`) + if err == nil || !strings.HasPrefix(err.Error(), "Legacy feature 'zz' not supported.") { + t.Fatalf("err = %v; want the config's legacy Feature zz first", err) + } +} + +func TestFeaturesResolveDependenciesListsFeaturesInDeclarationOrder(t *testing.T) { + ids := []string{"zeta", "alpha", "mid", "beta", "omega"} + local := map[string]string{} + var decl []string + for _, id := range ids { + local[id] = `{"id":"` + id + `","version":"1.0.0"}` + decl = append(decl, `"./`+id+`":{}`) + } + ws, _ := resolveDepsWorkspace(t, "{"+strings.Join(decl, ",")+"}", local) + out, err := runResolveDeps(t, "--workspace-folder", ws) + if err != nil { + t.Fatal(err) + } + prev := -1 + for _, id := range ids { + i := strings.Index(out, "[./"+id+"<") + if i <= prev { + t.Fatalf("mermaid nodes not in declaration order %v:\n%s", ids, out) + } + prev = i + } +} + +func TestPlainHTTPFeatureIDIsNotATarball(t *testing.T) { + var requests atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + w.Write(featureTgzBytes(t, map[string]string{"devcontainer-feature.json": `{"id":"x"}`})) + })) + t.Cleanup(srv.Close) + id := srv.URL + "/devcontainer-feature-x.tgz" + wantLogs := []string{ + "Could not resolve Feature manifest for '" + id + "'. If necessary, provide registry credentials with 'docker login '.\n", + "Could not resolve Feature '" + id + "'. Ensure the Feature is published and accessible from your current environment.\n", + } + + for name, process := range map[string]func(log.Logger) (*features.Set, error){ + "install": func(l log.Logger) (*features.Set, error) { + return processInstallFeature(l, nil, &features.FNode{UserFeatureID: id}, localFeatureRoots{}, nil, stageDirs(t)) + }, + "metadata": func(l log.Logger) (*features.Set, error) { + return newMetadataProcessFeature(nil, l, localFeatureRoots{}, nil)(&features.FNode{UserFeatureID: id}) + }, + } { + t.Run(name, func(t *testing.T) { + var logs bytes.Buffer + set, err := process(log.New(log.Options{Level: log.LevelInfo, Format: "text", Writer: &logs})) + if set != nil || err != nil { + t.Fatalf("set, err = %+v, %v; want neither", set, err) + } + for _, line := range wantLogs { + if !strings.Contains(logs.String(), line) { + t.Errorf("logs = %q, want %q", logs.String(), line) + } + } + }) + } + if n := requests.Load(); n != 0 { + t.Errorf("%d requests to the http:// URL, want none", n) + } +} + +func TestUnparseableFeatureReferences(t *testing.T) { + for id, want := range map[string]string{ + "owner/repo/feat@1@2": "Parse error. Use the '@' symbol only to designate a version tag.", + "owner/repo/fe.at@bad": "Could not resolve Feature 'owner/repo/fe.at@bad'. Ensure the Feature is published and accessible from your current environment.", + "owner/repo/sub/feat@v1": "Could not resolve Feature 'owner/repo/sub/feat@v1'. Ensure the Feature is published and accessible from your current environment.", + "owner//feat@v1": "Could not resolve Feature 'owner//feat@v1'. Ensure the Feature is published and accessible from your current environment.", + } { + var logs bytes.Buffer + set, err := newMetadataProcessFeature(nil, log.New(log.Options{Level: log.LevelInfo, Format: "text", Writer: &logs}), localFeatureRoots{}, nil)(&features.FNode{UserFeatureID: id}) + if set != nil || err != nil { + t.Errorf("%s: set, err = %+v, %v; want neither", id, set, err) + } + if !strings.HasSuffix(logs.String(), want+"\n") { + t.Errorf("%s: logs = %q, want them to end with %q", id, logs.String(), want) + } + } + if _, err := newMetadataProcessFeature(nil, log.Null, localFeatureRoots{}, nil)(&features.FNode{UserFeatureID: "owner/repo/feat@v1"}); err == nil || !strings.HasPrefix(err.Error(), "ERR: Feature 'owner/repo/feat@v1' could not be processed. ") { + t.Errorf("err = %v; want the reference's parse failure", err) + } +} diff --git a/internal/cli/feature_tarball_test.go b/internal/cli/feature_tarball_test.go index df978cd..6a89f65 100644 --- a/internal/cli/feature_tarball_test.go +++ b/internal/cli/feature_tarball_test.go @@ -214,11 +214,9 @@ func TestDownloadFeatureTarball_2xxAccepted(t *testing.T) { } func TestDirectTarballDigestCheckedAgainstTheLockfile(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + uri := serveTrustedTLS(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("not-a-real-tarball")) - })) - defer srv.Close() - uri := srv.URL + "/feature.tgz" + })) + "/feature.tgz" sum := sha256.Sum256([]byte("not-a-real-tarball")) digest := "sha256:" + hex.EncodeToString(sum[:]) diff --git a/internal/cli/features_resolve_deps.go b/internal/cli/features_resolve_deps.go index 059c26a..0f33917 100644 --- a/internal/cli/features_resolve_deps.go +++ b/internal/cli/features_resolve_deps.go @@ -38,7 +38,7 @@ func realFeaturesResolveDepsCmd() *cobra.Command { } cfg := loadResult.Config - userFeatures := features.UserFeaturesToArray(cfg.Features, cfg.FeatureOptionOrder) + userFeatures := features.UserFeaturesToArray(cfg.Features, cfg.FeatureOrder, cfg.FeatureOptionOrder) if len(userFeatures) == 0 { logger.Write(fmt.Sprintf("Could not parse features object in configuration '%s'", cfg.ConfigFilePath), log.LevelError) return &coreerrors.ExitCodeError{Code: 1} diff --git a/internal/cli/helpers.go b/internal/cli/helpers.go index 305650d..ff2dced 100644 --- a/internal/cli/helpers.go +++ b/internal/cli/helpers.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/devcontainers/cli/internal/config" + "github.com/devcontainers/cli/internal/features" "github.com/devcontainers/cli/internal/jsonc" "github.com/devcontainers/cli/internal/product" ) @@ -24,10 +25,12 @@ func mergeAdditionalFeatures(cfg *config.DevContainer, additionalFeaturesJSON st cfg.Features = make(map[string]any) } + cfg.FeatureOrder = features.PropertyOrder(cfg.Features, cfg.FeatureOrder) additionalOnly := make(map[string]bool) - for k, v := range additional { + for _, k := range features.PropertyOrder(additional, jsonc.MemberNames([]byte(additionalFeaturesJSON))) { if _, exists := cfg.Features[k]; !exists { - cfg.Features[k] = v + cfg.Features[k] = additional[k] + cfg.FeatureOrder = append(cfg.FeatureOrder, k) additionalOnly[k] = true if names := jsonc.MemberNames([]byte(additionalFeaturesJSON), k); len(names) > 0 { if cfg.FeatureOptionOrder == nil { diff --git a/internal/cli/helpers_held_test.go b/internal/cli/helpers_held_test.go index 1598e8b..bf10a6d 100644 --- a/internal/cli/helpers_held_test.go +++ b/internal/cli/helpers_held_test.go @@ -77,3 +77,16 @@ func TestMergeAdditionalFeaturesRecordsASingleOptionWithoutPriorOrder(t *testing t.Errorf("FeatureOptionOrder[./g] = %v, want [only]", got) } } + +func TestMergeAdditionalFeaturesAppendsTheirOrderAfterTheConfigs(t *testing.T) { + cfg := &config.DevContainer{ + Features: map[string]any{"./b": true, "./a": true, "./u": true}, + FeatureOrder: []string{"./b", "./a"}, + } + if _, err := mergeAdditionalFeatures(cfg, `{"./d": true, "7": true, "./a": true, "./c": true}`); err != nil { + t.Fatal(err) + } + if want := []string{"./b", "./a", "./u", "7", "./d", "./c"}; !slices.Equal(cfg.FeatureOrder, want) { + t.Errorf("FeatureOrder = %v, want %v", cfg.FeatureOrder, want) + } +} diff --git a/internal/cli/outdated.go b/internal/cli/outdated.go index ad2c180..0d87327 100644 --- a/internal/cli/outdated.go +++ b/internal/cli/outdated.go @@ -447,7 +447,7 @@ func newUpgradeCmd() *cobra.Command { lockfilePath := cfg.ConfigFilePath - res, err := fetchFeatureSetsWithOrder(cmd.Context(), logger, cfg.Features, cfg.FeatureOptionOrder, localFeatureRoots{filepath.Dir(lockfilePath), ws}, nil, nil) + res, err := fetchFeatureSetsWithOrder(cmd.Context(), logger, cfg.Features, cfg.FeatureOrder, cfg.FeatureOptionOrder, localFeatureRoots{filepath.Dir(lockfilePath), ws}, nil, nil) if err != nil { return err } diff --git a/internal/cli/pure_helpers_test.go b/internal/cli/pure_helpers_test.go index 88a62ef..1e74518 100644 --- a/internal/cli/pure_helpers_test.go +++ b/internal/cli/pure_helpers_test.go @@ -12,6 +12,7 @@ import ( "github.com/devcontainers/cli/internal/config" "github.com/devcontainers/cli/internal/features" + "github.com/devcontainers/cli/internal/log" "github.com/devcontainers/cli/internal/oci" ) @@ -258,7 +259,7 @@ func TestBuildArgsAndOptionsFromConfig(t *testing.T) { } func TestCommandErrorOutputKeepsAContainerError(t *testing.T) { - legacy := checkLegacyFeatures(map[string]any{"myfeature": true}) + _, _, legacy := resolveFeatureID(log.Null, "myfeature") out := commandErrorOutput(fmt.Errorf("install features: %w", legacy), "An error occurred setting up the container") want := fmt.Sprintf(msgLegacyFeature, "myfeature") if out.Description != want || out.Message != want { diff --git a/internal/cli/read_configuration.go b/internal/cli/read_configuration.go index 70e3b2f..d621967 100644 --- a/internal/cli/read_configuration.go +++ b/internal/cli/read_configuration.go @@ -183,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, result.Config.FeatureOptionOrder, localFeatureRoots{filepath.Dir(result.Config.ConfigFilePath), workspaceFolder}) + featResult, featErr = fetchFeatureSets(ctx, lgr, result.Config.Features, result.Config.FeatureOrder, result.Config.FeatureOptionOrder, 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 be390a6..65e48ac 100644 --- a/internal/cli/up.go +++ b/internal/cli/up.go @@ -1170,6 +1170,7 @@ func (r *upRunner) featureBuildOptions(cfg *config.DevContainer, loadResult *con HostSub: &loadResult.HostSub, ContainerEnv: cfg.ContainerEnv, ContainerEnvOrder: cfg.ContainerEnvOrder, + FeatureOrder: cfg.FeatureOrder, FeatureOptionOrder: cfg.FeatureOptionOrder, AdditionalMetadata: []imagemeta.Entry{configMetadataEntry(cfg, false)}, LabelMetadata: []imagemeta.Entry{r.configLabel}, diff --git a/internal/config/key_order_test.go b/internal/config/key_order_test.go index 4ab7aa3..14bb3d9 100644 --- a/internal/config/key_order_test.go +++ b/internal/config/key_order_test.go @@ -48,6 +48,9 @@ func TestLoadKeepsContainerEnvAndFeatureOptionOrder(t *testing.T) { if want := []string{"ZED", "ALPHA", "MID", "ZED"}; !slices.Equal(cfg.ContainerEnvOrder, want) { t.Errorf("ContainerEnvOrder = %v, want %v", cfg.ContainerEnvOrder, want) } + if want := []string{"./f", "./g", "./h"}; !slices.Equal(cfg.FeatureOrder, want) { + t.Errorf("FeatureOrder = %v, want %v", cfg.FeatureOrder, want) + } want := map[string][]string{"./f": {"zz", "aa", "mm"}, "./h": {"b", "a"}} if len(cfg.FeatureOptionOrder) != len(want) { t.Errorf("FeatureOptionOrder = %v, want %v", cfg.FeatureOptionOrder, want) @@ -80,3 +83,22 @@ func TestLoadKeepsBuildArgsOrder(t *testing.T) { t.Errorf("BuildArgsOrder = %v, want %v", res.Config.BuildArgsOrder, want) } } + +func TestLoadOrdersFeatureIDsAsJavaScriptEnumeratesThem(t *testing.T) { + dir := t.TempDir() + base := filepath.Join(dir, "devcontainer.json") + override := filepath.Join(dir, "override.json") + if err := os.WriteFile(base, []byte(`{"image": "i", "features": {"./zeta": {}, "10": {}, "./alpha": {}, "2": {}}}`), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(override, []byte(`{"features": {"./beta": {}, "./zeta": {}, "1": {}}}`), 0o644); err != nil { + t.Fatal(err) + } + res, err := LoadDevContainerConfig(dir, base, override) + if err != nil { + t.Fatal(err) + } + if want := []string{"1", "2", "10", "./zeta", "./alpha", "./beta"}; !slices.Equal(res.Config.FeatureOrder, want) { + t.Errorf("FeatureOrder = %v, want %v", res.Config.FeatureOrder, want) + } +} diff --git a/internal/config/loader.go b/internal/config/loader.go index ba201aa..864840b 100644 --- a/internal/config/loader.go +++ b/internal/config/loader.go @@ -13,6 +13,7 @@ import ( "runtime" "strings" + "github.com/devcontainers/cli/internal/features" "github.com/devcontainers/cli/internal/jsonc" "github.com/devcontainers/cli/internal/pfs" ) @@ -211,7 +212,7 @@ func LoadDevContainerConfigWithMounts(workspaceFolder, configPath, overrideConfi } config.ConfigFilePath = configFilePath - config.ContainerEnvOrder, config.FeatureOptionOrder, config.BuildArgsOrder = declaredOrder(docs) + config.ContainerEnvOrder, config.FeatureOrder, config.FeatureOptionOrder, config.BuildArgsOrder = declaredOrder(docs) return &LoadResult{ Config: &config, @@ -223,11 +224,13 @@ func LoadDevContainerConfigWithMounts(workspaceFolder, configPath, overrideConfi }, nil } -func declaredOrder(docs [][]byte) (containerEnv []string, featureOptions map[string][]string, buildArgs []string) { +func declaredOrder(docs [][]byte) (containerEnv, featureIDs []string, featureOptions map[string][]string, buildArgs []string) { for _, data := range docs { containerEnv = append(containerEnv, jsonc.MemberNames(data, "containerEnv")...) buildArgs = append(buildArgs, jsonc.MemberNames(data, "build", "args")...) - for _, id := range jsonc.MemberNames(data, "features") { + ids := jsonc.MemberNames(data, "features") + featureIDs = append(featureIDs, ids...) + for _, id := range ids { if names := jsonc.MemberNames(data, "features", id); len(names) > 0 { if featureOptions == nil { featureOptions = map[string][]string{} @@ -236,7 +239,7 @@ func declaredOrder(docs [][]byte) (containerEnv []string, featureOptions map[str } } } - return containerEnv, featureOptions, buildArgs + return containerEnv, features.KeyOrder(featureIDs), featureOptions, buildArgs } func deepMergeConfig(base, override map[string]any) map[string]any { diff --git a/internal/config/types.go b/internal/config/types.go index 3eea9fa..15de5a9 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -58,6 +58,7 @@ type DevContainer struct { DevPort any `json:"devPort,omitempty"` ContainerEnvOrder []string `json:"-"` + FeatureOrder []string `json:"-"` FeatureOptionOrder map[string][]string `json:"-"` BuildArgsOrder []string `json:"-"` } diff --git a/internal/features/resolve.go b/internal/features/resolve.go index bd72815..1797742 100644 --- a/internal/features/resolve.go +++ b/internal/features/resolve.go @@ -21,8 +21,7 @@ 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://") { + if strings.HasPrefix(id, "https://") { return SourceDirectTarball } if strings.HasPrefix(id, "./") || strings.HasPrefix(id, "../") || strings.HasPrefix(id, "/") { diff --git a/internal/features/resolve_test.go b/internal/features/resolve_test.go index 30b4b73..fbfb71d 100644 --- a/internal/features/resolve_test.go +++ b/internal/features/resolve_test.go @@ -22,7 +22,7 @@ func TestClassifyFeatureID(t *testing.T) { {"owner/feat", SourceOCI}, {`a\b`, SourceOCI}, {"https://example.com/feature.tgz", SourceDirectTarball}, - {"http://example.com/feature.tgz", SourceDirectTarball}, + {"http://example.com/feature.tgz", SourceOCI}, {"./local-feature", SourceLocalPath}, {"../sibling-feature", SourceLocalPath}, {"/abs/feature", SourceLocalPath}, @@ -109,14 +109,32 @@ func TestUserFeaturesToArray(t *testing.T) { "ghcr.io/devcontainers/features/go:1": map[string]any{"version": "1.21"}, "ghcr.io/devcontainers/features/node:1": true, } - arr := UserFeaturesToArray(features, nil) + arr := UserFeaturesToArray(features, nil, nil) if len(arr) != 2 { t.Errorf("len = %d, want 2", len(arr)) } } +func TestUserFeaturesToArrayFollowsTheOrderThenSortsTheRest(t *testing.T) { + features := map[string]any{"./z": true, "./a": 1, "./m": "x", "./b": false} + arr := UserFeaturesToArray(features, []string{"./z", "./gone", "./m", "./z"}, map[string][]string{"./m": {"o"}}) + var ids []string + for _, f := range arr { + ids = append(ids, f.UserFeatureID) + if f.Options != features[f.UserFeatureID] { + t.Errorf("%s options = %v, want %v", f.UserFeatureID, f.Options, features[f.UserFeatureID]) + } + } + if want := []string{"./z", "./m", "./a", "./b"}; !slices.Equal(ids, want) { + t.Errorf("ids = %v, want %v", ids, want) + } + if !slices.Equal(arr[1].OptionOrder, []string{"o"}) || arr[0].OptionOrder != nil { + t.Errorf("option order = %v, %v", arr[0].OptionOrder, arr[1].OptionOrder) + } +} + func TestUserFeaturesToArray_Nil(t *testing.T) { - arr := UserFeaturesToArray(nil, nil) + arr := UserFeaturesToArray(nil, nil, nil) if arr != nil { t.Errorf("expected nil, got %v", arr) } diff --git a/internal/features/testdata/ts-oracle/legacy.json b/internal/features/testdata/ts-oracle/legacy.json index 4aea475..14bdb1a 100644 --- a/internal/features/testdata/ts-oracle/legacy.json +++ b/internal/features/testdata/ts-oracle/legacy.json @@ -476,6 +476,13 @@ ], "result": "direct-tarball" }, + { + "fn": "getFeatureIdType", + "args": [ + "http://example.com/devcontainer-feature-node.tgz" + ], + "result": "github-repo" + }, { "fn": "getFeatureIdType", "args": [ diff --git a/internal/features/ts_oracle_test.go b/internal/features/ts_oracle_test.go index 1a30e1e..0b60e1f 100644 --- a/internal/features/ts_oracle_test.go +++ b/internal/features/ts_oracle_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/devcontainers/cli/internal/oci" "github.com/devcontainers/cli/internal/tsoracle/oracletest" ) @@ -117,6 +118,9 @@ func TestTSOracleLegacyFeatureIDs(t *testing.T) { case SourceLocalPath: return "file-path", nil } + if _, err := oci.ParseRef(id); err != nil { + return "github-repo", nil + } return "oci", nil }, }) diff --git a/internal/features/types.go b/internal/features/types.go index f1df351..7441f32 100644 --- a/internal/features/types.go +++ b/internal/features/types.go @@ -322,14 +322,22 @@ type DevContainerFeature struct { OptionOrder []string } -// optionOrder maps a Feature id to the member names of its options object -// in devcontainer.json; an id it lacks has its options in sorted order. -func UserFeaturesToArray(features map[string]any, optionOrder map[string][]string) []DevContainerFeature { +// UserFeaturesToArray lists features in order, then the ids order does not +// name, sorted. optionOrder maps a Feature id to the member names of its +// options object in devcontainer.json; an id it lacks has its options in +// sorted order. +func UserFeaturesToArray(features map[string]any, order []string, optionOrder map[string][]string) []DevContainerFeature { if features == nil { return nil } result := make([]DevContainerFeature, 0, len(features)) - for id, opts := range features { + seen := make(map[string]bool, len(features)) + for _, id := range slices.Concat(order, slices.Sorted(maps.Keys(features))) { + opts, ok := features[id] + if !ok || seen[id] { + continue + } + seen[id] = true result = append(result, DevContainerFeature{ UserFeatureID: id, Options: opts, diff --git a/tools/tsoracle/oracle.ts b/tools/tsoracle/oracle.ts index 3358472..97e0359 100644 --- a/tools/tsoracle/oracle.ts +++ b/tools/tsoracle/oracle.ts @@ -79,7 +79,7 @@ const fns: Record any> = { return { userFeatures: updateDeprecatedFeaturesIntoOptions(userFeatures, capturingOutput(log)), log }; }, // Only ids it classifies without a registry request: legacy ids (it - // throws), tarball URLs and local paths. + // throws), tarball URLs, local paths and references getRef rejects. getFeatureIdType: async (id: string) => (await getFeatureIdType({ output: nullOutput, env: {} } as any, id, undefined)).type, // spec-node/containerFeatures.ts getSafeId: (s: string) => getSafeId(s),