Skip to content

Commit

Permalink
GetAvailablePackageVersions returns non-semver versions unchanged (#6249
Browse files Browse the repository at this point in the history
)

### Description of the change

When Kubeapps was encountering Helm charts with non-semver versions,
rather than just returning the versions as they are, we were implicitly
converting them to the closest semver. This resulted in Kubeapps then
not being able to upgrade to a different version once an app with a
non-semver version was installed.

See #6099 for more detail.

### Benefits

If people choose to use non-semver versions, we don't block them from
upgrading.

### Possible drawbacks

We were previously ignoring errors when creating semver versions whereas
with this PR we now simply return all the versions as they were
provided. This may change some untested behavior, though unlikely.

### Applicable issues

- fixes #6099 

### Additional information

Also fixes an accidental change to our dev environment Makefile (I'd
accidentally committed a change in #6209 that had our dev environment
installing the upstream bitnami chart rather than the dev kubeapps one).

Signed-off-by: Michael Nelson <minelson@vmware.com>
  • Loading branch information
absoludity authored May 10, 2023
1 parent 92392e2 commit c144f0d
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 8 deletions.
30 changes: 23 additions & 7 deletions cmd/kubeapps-apis/plugins/pkg/pkgutils/pkgutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

/*
Utility functions that apply to "packages", e.g. helm charts or carvel packages
Utility functions that apply to "packages", e.g. helm charts or carvel packages
*/
package pkgutils

Expand All @@ -26,6 +26,7 @@ import (
structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
log "k8s.io/klog/v2"
)

// Contains miscellaneous package-utilities used by multiple plug-ins
Expand Down Expand Up @@ -59,12 +60,15 @@ type packageSemVersion struct {
appVersion string
}

func sortByPackageVersion(versions []models.ChartVersion) []*packageSemVersion {
func sortByPackageVersion(versions []models.ChartVersion) ([]*packageSemVersion, error) {
var sortedVersions []*packageSemVersion
for _, v := range versions {
version, err := semver.NewVersion(v.Version)
if err != nil {
continue
return nil, err
}
if version.String() != v.Version {
return nil, fmt.Errorf("Chart version %q is not semver", v.Version)
}

sortedVersions = append(sortedVersions, &packageSemVersion{
Expand All @@ -75,17 +79,29 @@ func sortByPackageVersion(versions []models.ChartVersion) []*packageSemVersion {
sort.Slice(sortedVersions, func(i, j int) bool {
return sortedVersions[i].Version.GreaterThan(sortedVersions[j].Version)
})
return sortedVersions
return sortedVersions, nil
}

// PackageAppVersionsSummary converts the model chart versions into the required version summary.
func PackageAppVersionsSummary(versions []models.ChartVersion, versionInSummary VersionsInSummary) []*corev1.PackageAppVersion {

// Sort versions
sortedVersions := sortByPackageVersion(versions)

var pav []*corev1.PackageAppVersion

// Sort versions
sortedVersions, err := sortByPackageVersion(versions)
if err != nil {
// If there was an error parsing a version as semver, we log the error
// and simply return the versions, as Helm does.
log.Errorf("error parsing versions as semver: %w", err)
for _, version := range versions {
pav = append(pav, &corev1.PackageAppVersion{
PkgVersion: version.Version,
AppVersion: version.AppVersion,
})
}
return pav
}

// Use a version map to be able to count how many major, minor and patch versions
// we have included.
versionMap := map[uint64]map[uint64][]uint64{}
Expand Down
12 changes: 12 additions & 0 deletions cmd/kubeapps-apis/plugins/pkg/pkgutils/pkgutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,18 @@ func TestPackageAppVersionsSummary(t *testing.T) {
},
input_versions_in_summary: GetDefaultVersionsInSummary(),
},
{
name: "it just returns versions that are not semver without any filtering or ordering",
chart_versions: []models.ChartVersion{
{Version: "v12-main", AppVersion: DefaultAppVersion},
{Version: "v11-main", AppVersion: DefaultAppVersion},
},
version_summary: []*corev1.PackageAppVersion{
{PkgVersion: "v12-main", AppVersion: DefaultAppVersion},
{PkgVersion: "v11-main", AppVersion: DefaultAppVersion},
},
input_versions_in_summary: GetDefaultVersionsInSummary(),
},
}

opts := cmpopts.IgnoreUnexported(corev1.PackageAppVersion{})
Expand Down
2 changes: 1 addition & 1 deletion script/makefiles/deploy-dev.mk
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ deploy-dependencies: deploy-dex deploy-openldap devel/localhost-cert.pem
--from-literal=password=dev-only-fake-password

deploy-dev-kubeapps:
helm --kubeconfig=${CLUSTER_CONFIG} upgrade --install kubeapps bitnami/kubeapps --namespace kubeapps --create-namespace \
helm --kubeconfig=${CLUSTER_CONFIG} upgrade --install kubeapps ./chart/kubeapps --namespace kubeapps --create-namespace \
--values ./site/content/docs/latest/reference/manifests/kubeapps-local-dev-values.yaml \
--values ./site/content/docs/latest/reference/manifests/kubeapps-local-dev-auth-proxy-values.yaml \
--values ./site/content/docs/latest/reference/manifests/kubeapps-local-dev-additional-kind-cluster.yaml \
Expand Down

0 comments on commit c144f0d

Please sign in to comment.