Confusion in the spec about metadata and merge logic

[avidal]

The JSON reference page shows a table of properties in the devcontainer.json with a marker for fields that may be pulled from image metadata.

The merge logic page shows a table of properties, with additional columns indicating if it comes from devcontainer.json or "Feature metadata".

The metadata in image labels section shows how to store metadata and where to store it.

I think that the confusion is around understanding the difference between metadata config that comes from a devcontainer.json vs from a feature. There's no affordances in the labels to delineate. My hunch is that if you're implementing merge logic you should (mostly) ignore the "feature metadata" column on the merge logic table, and if you're implementing labeling you should honor the markers in the JSON reference general properties table, whether the value came from a devcontainer-feature.json or a devcontainer.json.

For a more specific example, let's look at the containerEnv property:

• Is not part of devcontainer-feature.json
• Is part of devcontainer.json
• Is marked as being valid in metadata

If you're building and tagging a devcontainer, you may take your entire merged configuration, filter out properties not marked as valid for metadata, and then store it as a new entry in the devcontainer.metadata image label list. In our specific case, the devcontainer.metadata entry would contain containerEnv.

Later, if you are building another devcontainer that uses this built image as a base, you would:

• Read in devcontainer.json
• Read in all devcontainer-feature.json
• Merge these two together
• Read in all devcontainer.metadata entries
• Merge devcontainer.metadata entries, in order, based on a mix of the general properties markers and the merge logic

That last bullet is the tricky part: containerEnv is marked in the merge logic page as something that only comes from devcontainer.json and not from feature metadata. But it was stored as metadata according to the spec. So it would be valid for the "merged config" to include the containerEnv that was stored as metadata, which contradicts the merge logic table.

Am I misunderstanding something fundamental here?

[avidal]

After typing this up on the community Slack channel I think I rubber ducked myself into a little more clarity as to the intention and possible fixes, which I'll repost here.

---

I'll summarize it as: there's a mismatch between the merge logic table and the metadata property markers on the property table in the reference. I think it boils down to: certain properties are marked as being stored as metadata (such as containerEnv), but are explicitly not included in the merge logic. The confusion is that there's no structural difference in metadata entries for config that comes from a feature vs config that comes from a devcontainer.json.

It seems to me that the merge logic table needs a third column (or a different representation) that accounts for the three sources of data:

• devcontainer.json
• devcontainer-feature.json
• config stored within image labels

Instead of a third column, perhaps it's enough to rename "feature metadata" in the table header to "devcontainer-feature.json", and then add a paragraph above or below (perhaps a callout above with more description below) describing how to merge image label metadata (follow the same rules as the table above for merge logic, in cases where order matters or for conflict resolution, image metadata comes last)

[avidal]

erm, I somehow completely glossed over the "Image Metadata" section immediately preceding the merge logic table. I've read it before, but I think I was confused by the "Feature metadata" column in the merge logic table and thought of them as the same.

It may be cleaner to rename "Feature metadata" to "devcontainer-feature.json".

[joshspicer]

Hey @avidal! I agree that changing that header to devcontainer-feature.json would be more clear. The intention was for the Image Metadata section to describe the actual format on the label, and for Merge Logic to describe the merge semantics of each property. You've already found #206 where i'm trying to reconcile some inconsistencies - thank you for adding your review :)

I definitely think the precedence ordering could use some work in the docs. These 'last value wins' properties are a bit ambiguous in the spec.

The interesting spots in the reference implementation that I think are interesting to highlight are:

https://github.com/devcontainers/cli/blob/main/src/spec-node/imageMetadata.ts#L290, where a devcontainer.json config and the associated Feature devcontainer-feature.json files are converted into a a list of metadata objects (this is a bit confusing since later on we pass the config in and delete all the metadata properties - at this stage all the merged metadata is tracked here.

https://github.com/devcontainers/cli/blob/main/src/spec-node/imageMetadata.ts#L156, where the the merging semantics are enforced. That's the output of the merged configuration from the devcontainer read-configuration command, and what i'm working on making more consistent in the aforementioned spec proposal.

I think an example of all this working together would be helpful and may help me think of a better way to communicate this in the spec (besides your suggestions). Going to play around a bit with that

[joshspicer]

This section is what codifies the merge-logic in the reference implementation: https://github.com/devcontainers/cli/blob/main/src/spec-node/imageMetadata.ts#L16-L66

[joshspicer]

More conversation on this in the community slack - https://github.slack.com/archives/C0431R35H37/p1687969055272579