[proposal] Update the devcontainer.json schema to be able to represent a 'mergedConfiguration'#206
[proposal] Update the devcontainer.json schema to be able to represent a 'mergedConfiguration'#206joshspicer wants to merge 12 commits intomainfrom
devcontainer.json schema to be able to represent a 'mergedConfiguration'#206Conversation
devcontainer.json schema
devcontainer.json schemadevcontainer.json schema
chrmarti
left a comment
There was a problem hiding this comment.
Additionally:
customizationsis a map of objects in the devcontainer.json and a map of object arrays in the merged config.entrypointsis not in the devcontainer.json, but is an array of strings in the merged config.
How would we represent the origins of the lifecycle commands, so we can log them?
devcontainer.json schemadevcontainer.json schema
devcontainer.json schemadevcontainer.json schema to be able to represent a 'mergedConfiguration'
Co-authored-by: Brigit Murtaugh <[email protected]>
|
📢 Open questions <NONE> |
| } | ||
| ``` | ||
|
|
||
| An optional parameter `$origin` is added to the `LifecycleCommandParallel` that supporting tools can use to indicate the source of the command. For example, this is useful for outputting in the creation log which Feature provided a certain lifecycle hook. The `$` notation is used to indicate this property is additional tooling metadata that should not be present in a user `devcontainer.json`. |
There was a problem hiding this comment.
Would $origin be the qualified feature id with the version OR devcontainer.json?
There was a problem hiding this comment.
Now that we have Feature dependencies and may have more than one Feature contributing configuration, we should use the "canonical ID" (for OCI Features, expanded with the SHA hash).
There was a problem hiding this comment.
Although i'm going back and forth here. At the end of the day, the intention I have for $origin is to make it easier to understand from the logs what is happening. It may be more intuitive to print verbatim what is in the devcontainer.json.
| In the below example, the `entrypoint` property is contributed from two different sources. The merged `$entrypoints` property would be an array of two strings, one for each entrypoint. | ||
|
|
||
| ```json | ||
| "$entrypoints": [ |
There was a problem hiding this comment.
Are we ok with users including this in their devcontainer.json? I guess we might see this if we don't add a regular entrypoint property.
We don't have the $origin here which would make sense for consistency.
There was a problem hiding this comment.
I think this is ok. I don't think we'll necessarily do anything with this value if it's in a devcontainer.json as input, right? The source of truth is still in the image label - this section of the spec is as a standardized output for read-configuration. I imagine all the $ are ignored during a build and used by humans/loggers.
|
Is there still an appetite for this? In our SDK/CLI I'm tempted to implement something similar to help wrangle types (ours is written in Go, so I unfortunately can't leverage such rich types to keep everything in one place). Something that would help additionally would be some test cases for how to merge configuration that implementing tools can run against, but that's a matter for another time I think. |
|
Thanks for the nudge @avidal, this is definitely something I still want us to move forward with. I'll give this spec's comments another pass right now |
| } | ||
| ``` | ||
|
|
||
| An optional parameter `$origin` is added to the `LifecycleCommandParallel` that supporting tools can use to indicate the source of the command. For example, this is useful for outputting in the creation log which Feature provided a certain lifecycle hook. The `$` notation is used to indicate this property is additional tooling metadata that should not be present in a user `devcontainer.json`. |
There was a problem hiding this comment.
What is the $origin of a bit of config that comes from the devcontainer.metadata image label (assuming it doesn't carry one already)?
There was a problem hiding this comment.
If a section of metadata in devcontainer.metadata doesn't already have an $origin, we wouldn't anything. It had to come from somewhere (either a base dev container that defined the image, a Feature, or the user's current dev container). If omitted, we don't have any insight into where it came from and we'd need to log something generically in those cases.
| ```typescript | ||
| { | ||
| // ...other devcontainer.json properties... | ||
| onCreateCommand?: LifecycleCommand | LifecycleCommandParallel | LifecycleCommandParallel[] |
There was a problem hiding this comment.
I think that this means that a devcontainer-feature.json could specify something like:
{"onCreateCommand": [{
"foo": "script/foo",
"$origin": "foo"
}, {
"foo": ["date > /tmp/last-ran"]
}]Should it be considered an error if a devcontainer-feature.json specifies a list of objects? Should tools ignore an $origin property when reading from a json file directly (versus from image metadata)?
There was a problem hiding this comment.
Oh, this change specifically affects the schema for devcontainer.json, I see. I hadn't realized that devcontainer-feature.json has a distinct json schema.
There was a problem hiding this comment.
Yes, I don't think we'd want to update the devcontainer-feature.json schema, which is very close to a devcontainer.json but distinct. The goal of the $ properties as members of the devcontainer.json is so that a command such as devcontainer read-configuration can output a json object that is also a devcontainer.json. That's not the case today without these changes.
There was a problem hiding this comment.
The goal of the
$properties as members of thedevcontainer.jsonis so that a command such asdevcontainer read-configurationcan output a json object that is also a devcontainer.json. That's not the case today without these changes.
I've been thinking more about this, and I'm not sure that's a good thing to have in the spec itself...it seems like an implementation detail for the specific tool in question. The way some of the other comments read indicates that it'd be "valid" in a devcontainer.json but not actually used (re: a comment above about $entrypoints)...which kind of implies that the output of read-configuration is not actually a devcontainer.json.
That is, it seems like a "proper" implementation would use this schema to encode/decode metadata stored in the image label, and use it internally as the result of merging all of the various metadata sources, but it's not actually valid devcontainer.json config. Effectively splitting out merged metadata into a separate schema and leaving the existing devcontainer.json and devcontainer-feature.json alone.
The way I've been thinking about it lately is that a *.json contains two sets of data: config and metadata. I parse either devcontainer.json or devcontainer-feature.json into Config or FeatureConfig respectively, plus a Metadata entry. I parse the image label into a MergedMetadata entry. Internally I store the config and the merged metadata to drive the build process. I use the merged metadata to render into an image label. I print the config and merged metadata during debug output as two separate entities.
edit: The main reason I'm bringing this up is because it seems to me that this change may require significant explanation in the docs to reduce confusion for implementors vs declaring it as a distinct schema and indicating that it represents the results of the "merge logic table" and should be used as the entry for the devcontainer.metadata image label.
|
Another couple of questions, since this is still in review: this proposal adds an Do we think it's just not important to be able to report on the origin of those properties? Whereas lifecycle commands can use the origin for output logging? |
The goal of this proposal is to:
devcontainer.jsonschema