Skip to content

Commit

Permalink
Charts: Check for output dir conflicts (#710)
Browse files Browse the repository at this point in the history
Closes #508

Whenever two charts will end up in the same dir, tanka will exit before doing anything

Ex, these two versions would end up in the flagger dir:
```yaml
directory: charts
repositories:
- name: flagger
  url: https://flagger.app
requires:
- chart: flagger/flagger
  version: 1.16.1
- chart: flagger/flagger
  version: 1.17.0
version: 1
```

```console
$ tk tool charts vendor
Error: Output directory conflicts found:
 - output directory "flagger" is used twice, by charts "flagger/[email protected]" and "flagger/[email protected]"
```

It checks in both `add` and `vendor` commands to handle the case where someone writes their YAML manually
  • Loading branch information
julienduchesne authored Jun 2, 2022
1 parent 6ec7c1c commit 5ac0dfb
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 4 deletions.
18 changes: 14 additions & 4 deletions pkg/helm/charts.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ func (c Charts) Vendor(prune bool) error {
return err
}

// Check that there are no output conflicts before vendoring
if err := c.Manifest.Requires.CheckForOutputConflicts(); err != nil {
return err
}

expectedDirs := make(map[string]bool)

repositoriesUpdated := false
Expand Down Expand Up @@ -176,25 +181,30 @@ func (c *Charts) Add(reqs []string) error {
log.Printf("Adding %v Charts ...", len(reqs))

// parse new charts, append in memory
added := 0
requirements := c.Manifest.Requires
for _, s := range reqs {
r, err := parseReq(s)
if err != nil {
skip(s, err)
continue
}

if c.Manifest.Requires.Has(*r) {
if requirements.Has(*r) {
skip(s, fmt.Errorf("already exists"))
continue
}

c.Manifest.Requires = append(c.Manifest.Requires, *r)
added++
requirements = append(requirements, *r)
log.Println(" OK:", s)
}

if err := requirements.CheckForOutputConflicts(); err != nil {
return err
}

// write out
added := len(requirements) - len(c.Manifest.Requires)
c.Manifest.Requires = requirements
if err := write(c.Manifest, c.ManifestFile()); err != nil {
return err
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/helm/charts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ func TestAdd(t *testing.T) {
err = c.Add([]string{"stable/[email protected]"})
assert.EqualError(t, err, "1 Chart(s) were skipped. Please check above logs for details")

// Adding a chart with a different version to the same path, causes a conflict
err = c.Add([]string{"stable/[email protected]"})
assert.EqualError(t, err, `Output directory conflicts found:
- output directory "prometheus" is used twice, by charts "stable/[email protected]" and "stable/[email protected]"`)

// Add a chart with a specific extract directory
err = c.Add([]string{"stable/[email protected]:prometheus-11.12.0"})
assert.NoError(t, err)
Expand Down
23 changes: 23 additions & 0 deletions pkg/helm/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package helm

import (
"fmt"
"strings"

"github.com/Masterminds/semver"
)
Expand Down Expand Up @@ -87,3 +88,25 @@ func (r Requirements) Has(req Requirement) bool {

return false
}

func (r Requirements) CheckForOutputConflicts() error {
outputDirs := make(map[string]Requirement)
errs := make([]string, 0)

for _, req := range r {
dir := req.Directory
if dir == "" {
dir = parseReqName(req.Chart)
}
if previous, ok := outputDirs[dir]; ok {
errs = append(errs, fmt.Sprintf(`output directory %q is used twice, by charts "%s@%s" and "%s@%s"`, dir, previous.Chart, previous.Version.String(), req.Chart, req.Version.String()))
}
outputDirs[dir] = req
}

if len(errs) > 0 {
return fmt.Errorf("Output directory conflicts found:\n - " + strings.Join(errs, "\n - "))
}

return nil
}

0 comments on commit 5ac0dfb

Please sign in to comment.