diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 812997ea7..ce6743b05 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -23,6 +23,7 @@ import ( "os" "path" "path/filepath" + "sort" "strings" "sync" @@ -78,6 +79,11 @@ func (m *Manager) Build() error { return m.Update() } + // Ensure that the lock file is up-to-date with Chart.yaml dependencies. + if err := ensureLock(c.Metadata.Dependencies, lock.Dependencies); err != nil { + return err + } + // Check that all of the repos we're dependent on actually exist. if err := m.hasAllRepos(lock.Dependencies); err != nil { return err @@ -603,6 +609,82 @@ func (m *Manager) loadChartRepositories() (map[string]*repo.ChartRepository, err return indices, nil } +// ensureLock compares Chart.yaml and Chart.lock dependencies, and then ensures +// the lock file is still up-to-date. +func ensureLock(chartDeps []*chart.Dependency, lockDeps []*chart.Dependency) error { + var ( + vSorted = make([]*semver.Version, 0, len(lockDeps)) + vm = make(map[*semver.Version]*chart.Dependency, len(lockDeps)) + lm = make(map[*chart.Dependency]bool, len(lockDeps)) + cMissing []string + lMissing []string + ) + + // Filter & sort Chart.lock dependencies in desc order. + for _, ld := range lockDeps { + v, err := semver.NewVersion(ld.Version) + if err != nil || ld.Repository == "" { + continue + } + vm[v] = ld + vSorted = append(vSorted, v) + } + sort.Sort(sort.Reverse(semver.Collection(vSorted))) + + lSorted := make([]*chart.Dependency, len(vSorted)) + for i, v := range vSorted { + lSorted[i] = vm[v] + } + + // Filter dependencies in Chart.yaml which is not in Chart.lock. + for _, cd := range chartDeps { + if cd.Repository == "" { + // Don't care about local subcharts during building dependencies. + continue + } + + // Chart.yaml can have SemVer range. + constraint, err := semver.NewConstraint(cd.Version) + if err != nil { + return errors.Wrapf(err, "dependency %q in Chart.yaml has an invalid version/constraint format", cd.Name) + } + + found := false + for _, ld := range lSorted { + if cd.Name != ld.Name || cd.Repository != ld.Repository { + continue + } + ldVer, err := semver.NewVersion(ld.Version) + if err != nil { + return errors.Wrapf(err, "dependency %q in Chart.lock has an invalid version format", ld.Name) + } else if constraint.Check(ldVer) { + // Lock dependencies are sorted, so the matched version is the highest version among available candidates. + found = true + lm[ld] = true + break + } + } + if !found { + cMissing = append(cMissing, cd.Name) + } + } + if len(cMissing) > 0 { + return errors.Errorf("dependencies %s in Chart.yaml don't have a valid version resolution in Chart.lock. Try updating Chart.lock", strings.Join(cMissing, ", ")) + } + + // All Chart.yaml dependencies are resolved in Chart.lock. + // But we should check whether all Chart.lock dependencies are needed by Chart.yaml. + for _, ld := range lSorted { + if _, ok := lm[ld]; !ok { + lMissing = append(lMissing, ld.Name) + } + } + if len(lMissing) > 0 { + return errors.Errorf("dependencies %s in Chart.lock are not required by Chart.yaml. Try updating Chart.lock", strings.Join(lMissing, ", ")) + } + return nil +} + // writeLock writes a lockfile to disk func writeLock(chartpath string, lock *chart.Lock) error { data, err := yaml.Marshal(lock) diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index e7955b093..f1ea3e2a6 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -17,6 +17,7 @@ package downloader import ( "bytes" + "fmt" "path/filepath" "reflect" "testing" @@ -181,9 +182,172 @@ func TestGetRepoNames(t *testing.T) { } } +func TestEnsureLock(t *testing.T) { + repo := "http://example.com/charts" + tests := []struct { + name string + cDeps []*chart.Dependency + lDeps []*chart.Dependency + err bool + }{ + { + name: "no dep", + err: false, + }, + { + name: "extra lock dep", + cDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.1.0", Repository: repo}, + }, + lDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.1.0", Repository: repo}, + {Name: "extra-subchart", Version: "0.1.0", Repository: repo}, + }, + err: true, + }, + { + name: "extra lock dep without repo", + cDeps: []*chart.Dependency{ + {Name: "with-repo", Version: "0.1.0", Repository: repo}, + }, + lDeps: []*chart.Dependency{ + {Name: "with-repo", Version: "0.1.0", Repository: repo}, + {Name: "without-repo", Version: "0.1.0"}, + }, + err: false, + }, + { + name: "extra chart dep", + cDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.1.0", Repository: repo}, + {Name: "extra-subchart", Version: "0.1.0", Repository: repo}, + }, + lDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.1.0", Repository: repo}, + }, + err: true, + }, + { + name: "extra chart dep without repo", + cDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.1.0", Repository: repo}, + {Name: "extra-subchart", Version: "0.1.0"}, + }, + lDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.1.0", Repository: repo}, + }, + err: false, + }, + { + name: "lock version outdated", + cDeps: []*chart.Dependency{ + {Name: "subchart", Version: "^1.3.4", Repository: repo}, + }, + lDeps: []*chart.Dependency{ + {Name: "subchart", Version: "1.2.3", Repository: repo}, + }, + err: true, + }, + { + name: "resolution matched", + cDeps: []*chart.Dependency{ + {Name: "subchart", Version: ">=0.1.0", Repository: repo}, + }, + lDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.1.2", Repository: repo}, + }, + err: false, + }, + { + name: "chart deps of same chart that only one resolved", + cDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.1.0", Repository: repo, Alias: "subchart1"}, + {Name: "subchart", Version: "0.2.0", Repository: repo, Alias: "subchart2"}, + }, + lDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.1.0", Repository: repo}, + }, + err: true, + }, + { + name: "chart deps of same chart resolved to common version", + cDeps: []*chart.Dependency{ + {Name: "subchart", Version: ">=0.1.0", Repository: repo, Alias: "subchart1"}, + {Name: "subchart", Version: ">=0.2.0", Repository: repo, Alias: "subchart2"}, + }, + lDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.3.0", Repository: repo}, + }, + err: false, + }, + { + name: "chart deps of same chart with intersected version as smaller version of lock deps", + cDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.2.0", Repository: repo, Alias: "subchart1"}, + {Name: "subchart", Version: "~0.2.0", Repository: repo, Alias: "subchart2"}, + }, + lDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.2.0", Repository: repo}, + {Name: "subchart", Version: "0.2.1", Repository: repo}, + }, + err: false, + }, + { + name: "chart deps of same chart with intersected version as bigger version of lock deps", + cDeps: []*chart.Dependency{ + {Name: "subchart", Version: "~0.2.0", Repository: repo, Alias: "subchart2"}, + {Name: "subchart", Version: "~0.2.1", Repository: repo, Alias: "subchart2"}, + }, + lDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.2.0", Repository: repo}, + {Name: "subchart", Version: "0.2.1", Repository: repo}, + }, + err: true, + }, + { + name: "multiple lock deps of single chart dep resolutions", + cDeps: []*chart.Dependency{ + {Name: "subchart", Version: ">=0.1.0", Repository: repo}, + }, + lDeps: []*chart.Dependency{ + {Name: "subchart", Version: "0.1.0", Repository: repo}, + {Name: "subchart", Version: "0.2.0", Repository: repo}, + }, + err: true, + }, + { + name: "wrong version format", + cDeps: []*chart.Dependency{ + {Name: "subchart", Version: "wrong-version", Repository: repo}, + }, + lDeps: []*chart.Dependency{ + {Name: "subchart", Version: "wrong-version", Repository: repo}, + }, + err: true, + }, + } + + for _, tt := range tests { + err := ensureLock(tt.cDeps, tt.lDeps) + + if err != nil { + if tt.err { + continue + } + t.Fatal(fmt.Sprintf("Expected no error in test %q but:", tt.name), err) + } + + if tt.err { + t.Fatalf("Expected error in test %q", tt.name) + } + } +} + // This function is the skeleton test code of failing tests for #6416 and bugs due to #5874. +// // This function is used by below tests that ensures success of build operation // with optional fields, alias, condition, tags, and even with ranged version. +// // Parent chart includes local-subchart 0.1.0 subchart from a fake repository, by default. // If each of these main fields (name, version, repository) is not supplied by dep param, default value will be used. func checkBuildWithOptionalFields(t *testing.T, chartName string, dep chart.Dependency) {