Repository navigation
fix(features): enforce lockfile pins on baked features and empty configs #153
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,22 +65,45 @@ func (l *featureLock) pin(f config.ResolvedFeature) (feature.LockedFeature, bool | |
| return l.existing.Entry(f.Ref) | ||
| } | ||
|
|
||
| // acceptsBaked reports whether the base image's installation of f, | ||
| // recorded by the label entry baked, may stand in for fetching f. With | ||
| // the lockfile disabled the label match alone decides. Otherwise the | ||
| // baked artifact must be the one the lockfile pins, or, with no pin, | ||
| // one the label records, so that it can be locked. Upgrade mode accepts | ||
| // none: it resolves every reference afresh, and locking the baked | ||
| // artifact could replace a newer pin with an older one. A baked feature | ||
| // that is not accepted is fetched and installed like any other; the | ||
| // label records no artifact when another tool built the image. | ||
| func (l *featureLock) acceptsBaked(f config.ResolvedFeature, baked config.FeatureMetadata) bool { | ||
| switch l.mode { | ||
| case LockfileDisabled: | ||
| return true | ||
| case LockfileUpgrade: | ||
| return false | ||
| } | ||
| if baked.ResolvedRef == "" { | ||
| return false | ||
| } | ||
| pin, pinned := l.pin(f) | ||
| return !pinned || feature.CheckIntegrity(f, feature.Fetched{ResolvedRef: baked.ResolvedRef}, pin) == nil | ||
|
Comment on lines
+87
to
+88
There was a problem hiding this comment. 🟡 MEDIUM · Unpinned baked features skip the lockfile's dependency metadata The serialized lockfile is consumed by the reference-compatible feature lockfile path: at devcontainers/cli v0.87.0 Fix — When an unpinned baked feature is accepted, retain its declared dependency list from its feature manifest (for example by resolving the manifest or preserving the relevant dependency data in the image label) before generating the lock entry; alternatively fetch the feature artifact to build a complete lock entry. WitnessAt lockfile.go:87-88
Member
Author
There was a problem hiding this comment. Valid for this commit taken alone. It's resolved by #151, which is stacked on this PR and merges right after it: there, There was a problem hiding this comment. I can't settle this from the review record alone — Settling this requires reading the stacked branch #151 — layerFeatures, the label schema, and the new test — which is not in this record; the finding stands on the sequence this diff shows. To have me read the code and take another look, comment |
||
| } | ||
|
|
||
| // commit records what features resolved to: written in default and | ||
| // upgrade modes when it differs from the existing lockfile, compared | ||
| // in frozen mode. | ||
| // | ||
| // A feature the base image carries keeps its pin, which acceptsBaked | ||
| // checked it against; the label does not record dependsOn, so an entry | ||
| // rebuilt from it could differ. Without a pin it is locked from the | ||
| // artifact the label records. | ||
| func (l *featureLock) commit(features []config.ResolvedFeature) error { | ||
| if l.mode == LockfileDisabled { | ||
| return nil | ||
| } | ||
| next := &feature.Lockfile{Features: map[string]feature.LockedFeature{}} | ||
| for _, f := range features { | ||
| if pin, pinned := l.pin(f); f.AlreadyInstalled && pinned { | ||
| next.Features[f.Ref] = pin | ||
| continue | ||
| } | ||
| if entry, ok := feature.Lock(f); ok { | ||
|
|
@@ -95,3 +118,21 @@ func (l *featureLock) commit(features []config.ResolvedFeature) error { | |
| } | ||
| return feature.WriteLockfile(l.path, next) | ||
| } | ||
|
|
||
| // commitNoFeatures reconciles the lockfile of a configuration that | ||
| // declares no features. Nothing is pinned, but entries left by removed | ||
| // features are stale: default and upgrade modes empty the lockfile and | ||
| // frozen mode reports them. Without a lockfile there is nothing to do | ||
| // in any mode, as with the reference CLI. | ||
| func commitNoFeatures(mode LockfileMode, configPath string) error { | ||
| if mode == LockfileDisabled { | ||
| return nil | ||
| } | ||
| l := &featureLock{mode: mode, path: feature.LockfilePath(configPath)} | ||
| existing, err := feature.ReadLockfile(l.path) | ||
| if err != nil || existing == nil { | ||
| return err | ||
| } | ||
| l.existing = existing | ||
| return l.commit(nil) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.