Skip to content

Commit af4909b

Browse files
aledbfclaude
andcommitted
fix(features): take an OCI Feature's id from its metadata
TS processFeatureIdentifier starts an OCI Feature as {id: <reference id>, ...} and buildDependencyGraph spreads the Feature's metadata over it - the manifest's dev.containers.metadata annotation, else the layer's devcontainer-feature.json - so a published Feature whose metadata names another id (a renamed one, such as ghcr.io/codspace/features/color:1, now new-color) takes that id. Its consecutiveId, the folder its content is staged in and the rename check of its install wrapper follow it. Go kept the reference's id, so read-configuration reported color_0 where TS reports new-color_0, and the build staged the Feature under that name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 3343608 commit af4909b

3 files changed

Lines changed: 47 additions & 1 deletion

File tree

‎docs/parity/parity-matrix.yaml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,6 +1273,18 @@ initial_cases:
12731273
current_status: match
12741274
notes: "An OCI Feature set as TS writes it: sourceInformation in tryGetOCIFeatureSet's order (type, manifest, manifestDigest, featureRef, userFeatureId, userFeatureIdWithoutVersion), the manifest as the registry served it (JSON.parse of its body), featureRef in getRef's order, then internalVersion before computedDigest (applyFeatureConfigToFeature). The Feature is the annotation's metadata spread over {id, included, value} and that over the layer's devcontainer-feature.json, so its options keep the published order. Go used to re-serialize the manifest and featureRef with sorted keys and write the Feature in the struct's order."
12751275

1276+
- id: read-configuration.features-configuration-oci-feature-pinned-success
1277+
lane: runtime
1278+
command: read-configuration
1279+
priority: p1
1280+
network_required: true
1281+
ts_cmd: "read-configuration --workspace-folder reference/src/test/container-features/configs/lockfile-frozen --include-features-configuration"
1282+
key_order: ["featuresConfiguration.featureSets.0.sourceInformation.featureRef", "featuresConfiguration.featureSets.0.features.0", "featuresConfiguration.featureSets.1.sourceInformation.featureRef"]
1283+
asserts: [exit_code, stdout_normalized]
1284+
class: output-shape
1285+
current_status: match
1286+
notes: "Both Features are pinned by the fixture's lockfile. featureRef is parsed from the reference as written (tryGetOCIFeatureSet), so it keeps tag and version 1 though the manifest is fetched by the lockfile's digest. color's published metadata names it new-color: the Feature's id, and so its consecutiveId and cachePath (new-color_0), are the metadata's (buildDependencyGraph spreads it over the Feature). Go used to report the pinned digest in featureRef, without the tag, and name the folder after the reference's id (color_0)."
1287+
12761288
- id: read-configuration.features-configuration-oci-feature-digest-success
12771289
lane: runtime
12781290
command: read-configuration

‎internal/cli/feature_install.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package cli
22

33
import (
44
"archive/tar"
5+
"cmp"
56
"compress/gzip"
67
"context"
78
"crypto/sha256"
@@ -419,8 +420,10 @@ func processInstallFeature(
419420
jsonc.Unmarshal(metaData, &m)
420421
}
421422

423+
// TS's Feature takes its id from the metadata it spreads over
424+
// {id: <ref id>, ...}: the layer's here, the annotation's below.
422425
feat := features.Feature{
423-
ID: ref.ID,
426+
ID: cmp.Or(m.ID, ref.ID),
424427
Version: m.Version,
425428
Name: m.Name,
426429
Description: m.Description,
@@ -498,6 +501,7 @@ func processInstallFeature(
498501
// TS reads the dependencies from the annotation and
499502
// spreads it over the Feature (buildDependencyGraph).
500503
set.Features[0].Metadata = am.Metadata
504+
set.Features[0].ID = cmp.Or(am.ID, ref.ID)
501505
}
502506
}
503507
}

‎internal/cli/feature_install_held_test.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,3 +961,33 @@ func TestProcessInstallFeatureOCISetJSON(t *testing.T) {
961961
t.Errorf("set JSON:\ngot %s\nwant %s", got, want)
962962
}
963963
}
964+
965+
// An OCI Feature's id is its metadata's, as TS spreads the metadata over
966+
// {id: <reference id>, ...}: the annotation's when the manifest has one,
967+
// else the layer's, else the reference's. consecutiveId follows it.
968+
func TestProcessInstallFeatureOCIIDIsTheMetadatas(t *testing.T) {
969+
const id = "ghcr.io/owner/coll/hello:1"
970+
renamed, anonymous := `{"id":"new-hello"}`, `{"version":"1.0.0"}`
971+
for name, tc := range map[string]struct {
972+
layer string
973+
annotation *string
974+
want string
975+
}{
976+
"annotation": {`{"id":"layer-hello"}`, &renamed, "new-hello"},
977+
"annotation without id": {`{"id":"layer-hello"}`, &anonymous, "hello"},
978+
"layer": {`{"id":"layer-hello"}`, nil, "layer-hello"},
979+
"neither": {`{"version":"1.0.0"}`, nil, "hello"},
980+
} {
981+
t.Run(name, func(t *testing.T) {
982+
reg := newOCIFeatureRegistry()
983+
reg.add(t, id, map[string]string{"devcontainer-feature.json": tc.layer}, tc.annotation)
984+
set, err := installFeature(t, reg, id, nil, nil)
985+
if err != nil {
986+
t.Fatal(err)
987+
}
988+
if got := set.Features[0].ID; got != tc.want {
989+
t.Errorf("id = %q, want %q", got, tc.want)
990+
}
991+
})
992+
}
993+
}

0 commit comments

Comments
 (0)