Skip to content

Commit

Permalink
fix(*): Helm v3 handling of APIVersion v1 charts dependencies (helm#7009
Browse files Browse the repository at this point in the history
)

* Include requirements.* as Files in APIVersionV1

Fixes helm#6974.

This ensures that when reading a Chart marked with APIVersion v1, we
maintain the behaviour of Helm v2 and include the requirements.yaml and
requirements.lock in the Files collection, and hence produce charts that
work correctly with Helm v2.

Signed-off-by: Paul "Hampy" Hampson <p_hampson@wargaming.net>

* Write out requirements.lock for APIVersion1 Charts

This keeps the on-disk format consistent after `helm dependency update`
of an APIVersion1 Chart.

Signed-off-by: Paul "Hampy" Hampson <p_hampson@wargaming.net>

* Exclude 'dependencies' from APVersion1 Chart.yaml

This fixes `helm lint` against an APIVersion1 chart packaged with Helm
v3.

Signed-off-by: Paul "Hampy" Hampson <p_hampson@wargaming.net>

* Generate APIVersion v2 charts for dependency tests

As the generated chart contains no requirements.yaml in its files list,
but has dependencies in its metadata, it is not a valid APIVersion v1
chart.

Signed-off-by: Paul "Hampy" Hampson <p_hampson@wargaming.net>

* Generate APIVersion v2 charts for manager tests

Specifically for the charts that have dependencies, the generated chart
contains no requirements.yaml in its files but has dependencies in its
metadata. Hence it is not a valid APIVersion v1 chart.

Signed-off-by: Paul "Hampy" Hampson <p_hampson@wargaming.net>
  • Loading branch information
TBBle authored and bamachrn committed Feb 28, 2020
1 parent 68af2e2 commit 837e100
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 6 deletions.
2 changes: 1 addition & 1 deletion cmd/helm/dependency_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func TestDependencyUpdateCmd_DontDeleteOldChartsOnError(t *testing.T) {
func createTestingMetadata(name, baseURL string) *chart.Chart {
return &chart.Chart{
Metadata: &chart.Metadata{
APIVersion: chart.APIVersionV1,
APIVersion: chart.APIVersionV2,
Name: name,
Version: "1.2.3",
Dependencies: []*chart.Dependency{
Expand Down
6 changes: 6 additions & 0 deletions pkg/chart/loader/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,18 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) {
if err := yaml.Unmarshal(f.Data, c.Metadata); err != nil {
return c, errors.Wrap(err, "cannot load requirements.yaml")
}
if c.Metadata.APIVersion == chart.APIVersionV1 {
c.Files = append(c.Files, &chart.File{Name: f.Name, Data: f.Data})
}
// Deprecated: requirements.lock is deprecated use Chart.lock.
case f.Name == "requirements.lock":
c.Lock = new(chart.Lock)
if err := yaml.Unmarshal(f.Data, &c.Lock); err != nil {
return c, errors.Wrap(err, "cannot load requirements.lock")
}
if c.Metadata.APIVersion == chart.APIVersionV1 {
c.Files = append(c.Files, &chart.File{Name: f.Name, Data: f.Data})
}

case strings.HasPrefix(f.Name, "templates/"):
c.Templates = append(c.Templates, &chart.File{Name: f.Name, Data: f.Data})
Expand Down
9 changes: 9 additions & 0 deletions pkg/chartutil/chartfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,16 @@ func LoadChartfile(filename string) (*chart.Metadata, error) {
//
// 'filename' should be the complete path and filename ('foo/Chart.yaml')
func SaveChartfile(filename string, cf *chart.Metadata) error {
// Pull out the dependencies of a v1 Chart, since there's no way
// to tell the serialiser to skip a field for just this use case
savedDependencies := cf.Dependencies
if cf.APIVersion == chart.APIVersionV1 {
cf.Dependencies = nil
}
out, err := yaml.Marshal(cf)
if cf.APIVersion == chart.APIVersionV1 {
cf.Dependencies = savedDependencies
}
if err != nil {
return err
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/chartutil/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,17 @@ func Save(c *chart.Chart, outDir string) (string, error) {
func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
base := filepath.Join(prefix, c.Name())

// Pull out the dependencies of a v1 Chart, since there's no way
// to tell the serialiser to skip a field for just this use case
savedDependencies := c.Metadata.Dependencies
if c.Metadata.APIVersion == chart.APIVersionV1 {
c.Metadata.Dependencies = nil
}
// Save Chart.yaml
cdata, err := yaml.Marshal(c.Metadata)
if c.Metadata.APIVersion == chart.APIVersionV1 {
c.Metadata.Dependencies = savedDependencies
}
if err != nil {
return err
}
Expand Down
10 changes: 7 additions & 3 deletions pkg/downloader/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (m *Manager) Update() error {
}

// Finally, we need to write the lockfile.
return writeLock(m.ChartPath, lock)
return writeLock(m.ChartPath, lock, c.Metadata.APIVersion == chart.APIVersionV1)
}

func (m *Manager) loadChartDir() (*chart.Chart, error) {
Expand Down Expand Up @@ -634,12 +634,16 @@ func (m *Manager) loadChartRepositories() (map[string]*repo.ChartRepository, err
}

// writeLock writes a lockfile to disk
func writeLock(chartpath string, lock *chart.Lock) error {
func writeLock(chartpath string, lock *chart.Lock, legacyLockfile bool) error {
data, err := yaml.Marshal(lock)
if err != nil {
return err
}
dest := filepath.Join(chartpath, "Chart.lock")
lockfileName := "Chart.lock"
if legacyLockfile {
lockfileName = "requirements.lock"
}
dest := filepath.Join(chartpath, lockfileName)
return ioutil.WriteFile(dest, data, 0644)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/downloader/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func TestUpdateBeforeBuild(t *testing.T) {
Metadata: &chart.Metadata{
Name: "with-dependency",
Version: "0.1.0",
APIVersion: "v1",
APIVersion: "v2",
Dependencies: []*chart.Dependency{{
Name: d.Metadata.Name,
Version: ">=0.1.0",
Expand Down Expand Up @@ -285,7 +285,7 @@ func checkBuildWithOptionalFields(t *testing.T, chartName string, dep chart.Depe
Metadata: &chart.Metadata{
Name: chartName,
Version: "0.1.0",
APIVersion: "v1",
APIVersion: "v2",
Dependencies: []*chart.Dependency{&dep},
},
}
Expand Down

0 comments on commit 837e100

Please sign in to comment.