diff --git a/pkg/devcontainer/config/feature.go b/pkg/devcontainer/config/feature.go index b06c948e8..12d60abb4 100644 --- a/pkg/devcontainer/config/feature.go +++ b/pkg/devcontainer/config/feature.go @@ -1,12 +1,52 @@ package config import ( + "bytes" "encoding/json" "fmt" + "sort" "github.com/devsy-org/devsy/pkg/types" ) +func objectKeyOrder(data []byte, field string) ([]string, error) { + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return nil, err + } + value, ok := raw[field] + if !ok { + return nil, nil + } + + var obj map[string]json.RawMessage + if err := json.Unmarshal(value, &obj); err != nil { + return nil, nil //nolint:nilerr // non-object values carry no key order + } + return decodeObjectKeys(value) +} + +func decodeObjectKeys(value json.RawMessage) ([]string, error) { + dec := json.NewDecoder(bytes.NewReader(value)) + if _, err := dec.Token(); err != nil { + return nil, err + } + + var keys []string + for dec.More() { + keyTok, err := dec.Token() + if err != nil { + return nil, err + } + keys = append(keys, keyTok.(string)) + var skip json.RawMessage + if err := dec.Decode(&skip); err != nil { + return nil, err + } + } + return keys, nil +} + type FeatureSet struct { ConfigID string Version string @@ -78,6 +118,45 @@ type FeatureConfig struct { // Origin is the path where the feature was loaded from Origin string `json:"-"` + + dependsOnOrder []string `json:"-"` +} + +func (c *FeatureConfig) UnmarshalJSON(data []byte) error { + type alias FeatureConfig + if err := json.Unmarshal(data, (*alias)(c)); err != nil { + return err + } + order, err := objectKeyOrder(data, "dependsOn") + if err != nil { + return err + } + c.dependsOnOrder = order + return nil +} + +func (c *FeatureConfig) DependsOnKeys() []string { + if c.capturedOrderMatchesDeps() { + return c.dependsOnOrder + } + keys := make([]string, 0, len(c.DependsOn)) + for k := range c.DependsOn { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + +func (c *FeatureConfig) capturedOrderMatchesDeps() bool { + if len(c.dependsOnOrder) != len(c.DependsOn) { + return false + } + for _, k := range c.dependsOnOrder { + if _, ok := c.DependsOn[k]; !ok { + return false + } + } + return true } type DependsOnField map[string]any diff --git a/pkg/devcontainer/config/feature_test.go b/pkg/devcontainer/config/feature_test.go new file mode 100644 index 000000000..48565250e --- /dev/null +++ b/pkg/devcontainer/config/feature_test.go @@ -0,0 +1,71 @@ +package config + +import ( + "encoding/json" + "reflect" + "testing" +) + +const ( + depZeta = "ghcr.io/x/zeta:1" + depAlpha = "ghcr.io/x/alpha:1" + depMid = "ghcr.io/x/mid:1" +) + +func TestFeatureConfig_DependsOnKeysPreservesDeclarationOrder(t *testing.T) { + data := []byte(`{ + "id": "example", + "dependsOn": { + "ghcr.io/x/zeta:1": {}, + "ghcr.io/x/alpha:1": {}, + "ghcr.io/x/mid:1": {} + } + }`) + + var cfg FeatureConfig + if err := json.Unmarshal(data, &cfg); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + want := []string{depZeta, depAlpha, depMid} + if got := cfg.DependsOnKeys(); !reflect.DeepEqual(got, want) { + t.Errorf("DependsOnKeys() = %v, want %v", got, want) + } +} + +func TestFeatureConfig_DependsOnKeysFallsBackToSorted(t *testing.T) { + cfg := FeatureConfig{DependsOn: DependsOnField{ + depZeta: map[string]any{}, + depAlpha: map[string]any{}, + }} + + want := []string{depAlpha, depZeta} + if got := cfg.DependsOnKeys(); !reflect.DeepEqual(got, want) { + t.Errorf("DependsOnKeys() = %v, want %v", got, want) + } +} + +func TestFeatureConfig_DependsOnKeysFallsBackWhenKeyMutated(t *testing.T) { + data := []byte( + `{"id": "example", "dependsOn": {"ghcr.io/x/zeta:1": {}, "ghcr.io/x/alpha:1": {}}}`, + ) + + var cfg FeatureConfig + if err := json.Unmarshal(data, &cfg); err != nil { + t.Fatalf("unmarshal: %v", err) + } + delete(cfg.DependsOn, depAlpha) + cfg.DependsOn[depMid] = map[string]any{} + + want := []string{depMid, depZeta} + if got := cfg.DependsOnKeys(); !reflect.DeepEqual(got, want) { + t.Errorf("DependsOnKeys() = %v, want %v", got, want) + } +} + +func TestFeatureConfig_DependsOnKeysEmpty(t *testing.T) { + var cfg FeatureConfig + if got := cfg.DependsOnKeys(); len(got) != 0 { + t.Errorf("DependsOnKeys() = %v, want empty", got) + } +} diff --git a/pkg/devcontainer/feature/extend.go b/pkg/devcontainer/feature/extend.go index db306e351..3b6aa08d2 100644 --- a/pkg/devcontainer/feature/extend.go +++ b/pkg/devcontainer/feature/extend.go @@ -529,7 +529,7 @@ func (p *featureProcessor) recordLockEntry( Integrity: res.integrity, } if len(cfg.DependsOn) > 0 { - entry.DependsOn = map[string]any(cfg.DependsOn) + entry.DependsOn = cfg.DependsOnKeys() } p.lock.record(featureID, entry) } diff --git a/pkg/devcontainer/feature/lockfile.go b/pkg/devcontainer/feature/lockfile.go index f4cc59a0b..61483daf6 100644 --- a/pkg/devcontainer/feature/lockfile.go +++ b/pkg/devcontainer/feature/lockfile.go @@ -16,10 +16,10 @@ import ( // LockedFeature is a single pinned entry in a devcontainer-lock.json file. It // mirrors the structure produced by the reference devcontainer CLI. type LockedFeature struct { - Version string `json:"version,omitempty"` - Resolved string `json:"resolved,omitempty"` - Integrity string `json:"integrity,omitempty"` - DependsOn map[string]any `json:"dependsOn,omitempty"` + Version string `json:"version,omitempty"` + Resolved string `json:"resolved,omitempty"` + Integrity string `json:"integrity,omitempty"` + DependsOn []string `json:"dependsOn,omitempty"` } // Lockfile mirrors the devcontainer-lock.json structure: a map of feature diff --git a/pkg/devcontainer/feature/lockfile_test.go b/pkg/devcontainer/feature/lockfile_test.go index dbfab6d80..2b93056c2 100644 --- a/pkg/devcontainer/feature/lockfile_test.go +++ b/pkg/devcontainer/feature/lockfile_test.go @@ -125,6 +125,31 @@ func TestWriteLockfile_CreatesSortedStable(t *testing.T) { } } +func TestWriteLockfile_DependsOnIsArray(t *testing.T) { + path := filepath.Join(t.TempDir(), "devcontainer-lock.json") + lf := &Lockfile{Features: map[string]LockedFeature{ + lockTestFeatureA: { + Version: lockTestVersion, + Resolved: lockTestResolvedA, + Integrity: lockTestShaA, + DependsOn: []string{"ghcr.io/b/feature:1"}, + }, + }} + + if err := WriteLockfile(path, lf, false); err != nil { + t.Fatalf("WriteLockfile: %v", err) + } + + data, err := os.ReadFile(filepath.Clean(path)) + if err != nil { + t.Fatal(err) + } + content := string(data) + if !strings.Contains(content, "\"dependsOn\": [") { + t.Errorf("expected dependsOn as JSON array, got:\n%s", content) + } +} + func TestWriteLockfile_SkipsUnchanged(t *testing.T) { path := filepath.Join(t.TempDir(), "devcontainer-lock.json") lf := &Lockfile{Features: map[string]LockedFeature{