From 0139fe4dc6bb98607682cd1378acbb3f050f83f2 Mon Sep 17 00:00:00 2001 From: Hang Park Date: Sun, 20 Oct 2019 19:03:23 +0900 Subject: [PATCH] Revert "fix(pkg/downloader): Implement ensureLock() instead of digest comparison" This reverts commit 237acc1f98f71d127e677b1b5ff1efeda37c302f. Signed-off-by: Hang Park --- pkg/downloader/manager.go | 82 ----------------- pkg/downloader/manager_test.go | 164 --------------------------------- 2 files changed, 246 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index ce6743b05..812997ea7 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -23,7 +23,6 @@ import ( "os" "path" "path/filepath" - "sort" "strings" "sync" @@ -79,11 +78,6 @@ 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 @@ -609,82 +603,6 @@ 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 f1ea3e2a6..e7955b093 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -17,7 +17,6 @@ package downloader import ( "bytes" - "fmt" "path/filepath" "reflect" "testing" @@ -182,172 +181,9 @@ 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) {