Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is allowed to have other files in the releases created by chart-releaser? #233

Closed
jvanz opened this issue Nov 18, 2022 · 3 comments · Fixed by #235
Closed

Is allowed to have other files in the releases created by chart-releaser? #233

jvanz opened this issue Nov 18, 2022 · 3 comments · Fixed by #235

Comments

@jvanz
Copy link
Contributor

jvanz commented Nov 18, 2022

This repository uses the chart-releaser to do the job of releasing all the 3 helm charts. After the release is created some additional files are added to the release. This is causing issues. In the next time that new changes are pushed to the repository, chart-releaser fails because it trys to open the additional files as Helm charts. This is makes the CI job to failed.

Is it fine to add more assets in the releases created by chart-releaser? If so, how can I tell the tool to ignore the files that are not charts?

If this is not supported, what do you think to add this feature? (I'm willing to work on that)

@mattfarina
Copy link

mattfarina commented Nov 21, 2022

I think this is a bug in the way the index files are updated. Take a look at ....

for _, asset := range release.Assets {
downloadURL, _ := url.Parse(asset.URL)
name := filepath.Base(downloadURL.Path)
baseName := strings.TrimSuffix(name, filepath.Ext(name))
tagParts := r.splitPackageNameAndVersion(baseName)
packageName, packageVersion := tagParts[0], tagParts[1]
fmt.Printf("Found %s-%s.tgz\n", packageName, packageVersion)
if _, err := indexFile.Get(packageName, packageVersion); err != nil {
if err := r.addToIndexFile(indexFile, downloadURL.String()); err != nil {
return false, err
}
update = true
break
}
}

In this code it assumes all files on the release are charts and does not check if they have a valid name or extension. I would suggest checking for that would be a valid fix.

@scottrigby
Copy link
Member

I think you’re right Matt. Adding addtl files during CI to .cr-release-packages dir seems like a feature not a bug. We should't assume all assets on the release are charts 👌

@jvanz
Copy link
Contributor Author

jvanz commented Nov 22, 2022

Cool, I'll work on this issue and open a PR with a fix for your review. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants