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

Return the repo name in a carvel a pkg summary #4716

Merged
merged 9 commits into from
May 19, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ func chartCacheKeyFor(namespace, chartID, chartVersion string) (string, error) {
}

var err error
if chartID, err = pkgutils.GetUnescapedChartID(chartID); err != nil {
if chartID, err = pkgutils.GetUnescapedPackageID(chartID); err != nil {
return "", fmt.Errorf("invalid chart ID in chartCacheKeyFor: [%s]: %v", chartID, err)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (s *Server) getChartInCluster(ctx context.Context, key types.NamespacedName
func (s *Server) availableChartDetail(ctx context.Context, packageRef *corev1.AvailablePackageReference, chartVersion string) (*corev1.AvailablePackageDetail, error) {
log.Infof("+availableChartDetail(%s, %s)", packageRef, chartVersion)

repoN, chartName, err := pkgutils.SplitChartIdentifier(packageRef.Identifier)
repoN, chartName, err := pkgutils.SplitPackageIdentifier(packageRef.Identifier)
if err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ func (s *Server) getReleaseViaHelmApi(ctx context.Context, key types.NamespacedN
}

func (s *Server) newRelease(ctx context.Context, packageRef *corev1.AvailablePackageReference, targetName types.NamespacedName, versionRef *corev1.VersionReference, reconcile *corev1.ReconciliationOptions, valuesString string) (*corev1.InstalledPackageReference, error) {
repoName, chartName, err := pkgutils.SplitChartIdentifier(packageRef.Identifier)
repoName, chartName, err := pkgutils.SplitPackageIdentifier(packageRef.Identifier)
if err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func (s *Server) GetAvailablePackageVersions(ctx context.Context, request *corev
cluster)
}

repoName, chartName, err := pkgutils.SplitChartIdentifier(packageRef.Identifier)
repoName, chartName, err := pkgutils.SplitPackageIdentifier(packageRef.Identifier)
if err != nil {
return nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ func (s *Server) GetAvailablePackageDetail(ctx context.Context, request *corev1.
return nil, err
}

unescapedChartID, err := pkgutils.GetUnescapedChartID(request.AvailablePackageRef.Identifier)
unescapedChartID, err := pkgutils.GetUnescapedPackageID(request.AvailablePackageRef.Identifier)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -371,7 +371,7 @@ func (s *Server) GetAvailablePackageVersions(ctx context.Context, request *corev
return nil, err
}

unescapedChartID, err := pkgutils.GetUnescapedChartID(request.GetAvailablePackageRef().GetIdentifier())
unescapedChartID, err := pkgutils.GetUnescapedPackageID(request.GetAvailablePackageRef().GetIdentifier())
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -697,7 +697,7 @@ func (s *Server) CreateInstalledPackage(ctx context.Context, request *corev1.Cre
}
chartID := request.GetAvailablePackageRef().GetIdentifier()
repoNamespace := request.GetAvailablePackageRef().GetContext().GetNamespace()
repoName, chartName, err := pkgutils.SplitChartIdentifier(chartID)
repoName, chartName, err := pkgutils.SplitPackageIdentifier(chartID)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -769,7 +769,7 @@ func (s *Server) UpdateInstalledPackage(ctx context.Context, request *corev1.Upd
return nil, status.Errorf(codes.Internal, "Unable to create kubernetes clientset: %v", err)
}
chartID := availablePkgRef.GetIdentifier()
repoName, chartName, err := pkgutils.SplitChartIdentifier(chartID)
repoName, chartName, err := pkgutils.SplitPackageIdentifier(chartID)
if err != nil {
return nil, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (s *Server) GetAvailablePackageSummaries(ctx context.Context, request *core
if currentPkg.Spec.RefName > pkgMetadata.Name {
return nil, status.Errorf(codes.Internal, fmt.Sprintf("unexpected order for kapp-controller packages, expected %q, found %q", pkgMetadata.Name, currentPkg.Spec.RefName))
}
log.Errorf("Package %q did not have a corresponding metadata", currentPkg.Spec.RefName)
log.Errorf("Package %q did not have a corresponding metadata (want %q)", currentPkg.Spec.RefName, pkgMetadata.Name)
currentPkg = <-getPkgsChannel
}
// Collect the packages for a particular refName to be able to send the
Expand Down Expand Up @@ -165,8 +165,13 @@ func (s *Server) GetAvailablePackageVersions(ctx context.Context, request *corev
cluster = s.globalPackagingCluster
}

_, pkgName, err := pkgutils.SplitPackageIdentifier(identifier)
if err != nil {
return nil, err
}

// Use the field selector to return only Package CRs that match on the spec.refName.
fieldSelector := fmt.Sprintf("spec.refName=%s", identifier)
fieldSelector := fmt.Sprintf("spec.refName=%s", pkgName)
pkgs, err := s.getPkgsWithFieldSelector(ctx, cluster, namespace, fieldSelector)
if err != nil {
return nil, statuserror.FromK8sError("get", "Package", "", err)
Expand All @@ -179,8 +184,8 @@ func (s *Server) GetAvailablePackageVersions(ctx context.Context, request *corev

// TODO(minelson): support configurable version summary for kapp-controller pkgs
// as already done for Helm (see #3588 for more info).
versions := make([]*corev1.PackageAppVersion, len(pkgVersionsMap[identifier]))
for i, v := range pkgVersionsMap[identifier] {
versions := make([]*corev1.PackageAppVersion, len(pkgVersionsMap[pkgName]))
for i, v := range pkgVersionsMap[pkgName] {
// Currently, PkgVersion and AppVersion are the same
// https://kubernetes.slack.com/archives/CH8KCCKA5/p1636386358322000?thread_ts=1636371493.320900&cid=CH8KCCKA5
versions[i] = &corev1.PackageAppVersion{
Expand Down Expand Up @@ -215,17 +220,22 @@ func (s *Server) GetAvailablePackageDetail(ctx context.Context, request *corev1.
cluster = s.globalPackagingCluster
}

_, pkgName, err := pkgutils.SplitPackageIdentifier(identifier)
if err != nil {
return nil, err
}

// fetch the package metadata
pkgMetadata, err := s.getPkgMetadata(ctx, cluster, namespace, identifier)
pkgMetadata, err := s.getPkgMetadata(ctx, cluster, namespace, pkgName)
if err != nil {
return nil, statuserror.FromK8sError("get", "PackageMetadata", identifier, err)
return nil, statuserror.FromK8sError("get", "PackageMetadata", pkgName, err)
}

// Use the field selector to return only Package CRs that match on the spec.refName.
fieldSelector := fmt.Sprintf("spec.refName=%s", identifier)
fieldSelector := fmt.Sprintf("spec.refName=%s", pkgName)
pkgs, err := s.getPkgsWithFieldSelector(ctx, cluster, namespace, fieldSelector)
if err != nil {
return nil, statuserror.FromK8sError("get", "Package", identifier, err)
return nil, statuserror.FromK8sError("get", "Package", pkgName, err)
}
pkgVersionsMap, err := getPkgVersionsMap(pkgs)
if err != nil {
Expand All @@ -235,22 +245,22 @@ func (s *Server) GetAvailablePackageDetail(ctx context.Context, request *corev1.
var foundPkgSemver = &pkgSemver{}
if requestedPkgVersion != "" {
// Ensure the version is available.
for _, v := range pkgVersionsMap[identifier] {
for _, v := range pkgVersionsMap[pkgName] {
if v.version.String() == requestedPkgVersion {
foundPkgSemver = &v
break
}
}
if foundPkgSemver.version == nil {
return nil, status.Errorf(codes.NotFound, fmt.Sprintf("unable to find %q package with version %q", identifier, requestedPkgVersion))
return nil, status.Errorf(codes.NotFound, fmt.Sprintf("unable to find %q package with version %q", pkgName, requestedPkgVersion))
}
} else {
// If the pkgVersion wasn't specified, grab the packages to find the latest.
if len(pkgVersionsMap[identifier]) > 0 {
foundPkgSemver = &pkgVersionsMap[identifier][0]
if len(pkgVersionsMap[pkgName]) > 0 {
foundPkgSemver = &pkgVersionsMap[pkgName][0]
requestedPkgVersion = foundPkgSemver.version.String()
} else {
return nil, status.Errorf(codes.NotFound, fmt.Sprintf("unable to find any versions for the package %q", identifier))
return nil, status.Errorf(codes.NotFound, fmt.Sprintf("unable to find any versions for the package %q", pkgName))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,17 @@ func (s *Server) buildAvailablePackageSummary(pkgMetadata *datapackagingv1alpha1
iconStringBuilder.WriteString(pkgMetadata.Spec.IconSVGBase64)
}

// build package identifier based on the metadata
identifier := buildPackageIdentifier(pkgMetadata)

availablePackageSummary := &corev1.AvailablePackageSummary{
AvailablePackageRef: &corev1.AvailablePackageReference{
Context: &corev1.Context{
Cluster: cluster,
Namespace: pkgMetadata.Namespace,
},
Plugin: &pluginDetail,
Identifier: pkgMetadata.Name,
Identifier: identifier,
},
Name: pkgMetadata.Name,
// Currently, PkgVersion and AppVersion are the same
Expand Down Expand Up @@ -85,6 +88,9 @@ func (s *Server) buildAvailablePackageDetail(pkgMetadata *datapackagingv1alpha1.
})
}

// build package identifier based on the metadata
identifier := buildPackageIdentifier(pkgMetadata)

// build readme
readme := buildReadme(pkgMetadata, foundPkgSemver)

Expand All @@ -102,7 +108,7 @@ func (s *Server) buildAvailablePackageDetail(pkgMetadata *datapackagingv1alpha1.
Namespace: pkgMetadata.Namespace,
},
Plugin: &pluginDetail,
Identifier: pkgMetadata.Name,
Identifier: identifier,
},
Name: pkgMetadata.Name,
IconUrl: iconStringBuilder.String(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ import (
"k8s.io/client-go/rest"
)

const REPO_REF_ANNOTATION = "packaging.carvel.dev/package-repository-ref"
const DEFAULT_REPO_NAME = "unknown"

type pkgSemver struct {
pkg *datapackagingv1alpha1.Package
version *semver.Version
Expand Down Expand Up @@ -127,6 +130,20 @@ func buildReadme(pkgMetadata *datapackagingv1alpha1.PackageMetadata, foundPkgSem
return readmeSB.String()
}

// buildPackageIdentifier generates the package identifier (repoName/pkgName) for a given package
func buildPackageIdentifier(pkgMetadata *datapackagingv1alpha1.PackageMetadata) string {
// default repo name if using kapp controller < v0.36.1
repoName := DEFAULT_REPO_NAME

// See https://github.com/vmware-tanzu/carvel-kapp-controller/pull/532
if repoRefAnnotation := pkgMetadata.Annotations[REPO_REF_ANNOTATION]; repoRefAnnotation != "" {
// this annotation returns "namespace/reponame", for instance "default/tce-repo",
// but we just want the "reponame" part
repoName = strings.Split(repoRefAnnotation, "/")[1]
}
return fmt.Sprintf("%s/%s", repoName, pkgMetadata.Name)
}

// buildPostInstallationNotes generates the installation notes based on the application status
func buildPostInstallationNotes(app *kappctrlv1alpha1.App) string {
var postInstallNotesSB strings.Builder
Expand Down
28 changes: 14 additions & 14 deletions cmd/kubeapps-apis/plugins/pkg/pkgutils/pkgutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,29 +182,29 @@ func AvailablePackageSummaryFromChart(chart *models.Chart, plugin *plugins.Plugi
// https://github.com/vmware-tanzu/kubeapps/pull/4094#discussion_r790349962.
// Will come back to this

// GetUnescapedChartID takes a chart id with URI-encoded characters and decode them. Ex: 'foo%2Fbar' becomes 'foo/bar'
// also checks that the chart ID is in the expected format, namely "repoName/chartName"
func GetUnescapedChartID(chartID string) (string, error) {
unescapedChartID, err := url.QueryUnescape(chartID)
// GetUnescapedPackageID takes a package id with URI-encoded characters and decode them. Ex: 'foo%2Fbar' becomes 'foo/bar'
// also checks that the package ID is in the expected format, namely "repoName/packageName"
func GetUnescapedPackageID(packageID string) (string, error) {
unescapedPackageID, err := url.QueryUnescape(packageID)
if err != nil {
return "", status.Errorf(codes.InvalidArgument, "Unable to decode chart ID chart: %v", chartID)
return "", status.Errorf(codes.InvalidArgument, "Unable to decode package ID: %v", packageID)
}
// TODO(agamez): support ID with multiple slashes, eg: aaa/bbb/ccc
chartIDParts := strings.Split(unescapedChartID, "/")
if len(chartIDParts) != 2 {
return "", status.Errorf(codes.InvalidArgument, "Incorrect package ref dentifier, currently just 'foo/bar' patterns are supported: %s", chartID)
packageIDParts := strings.Split(unescapedPackageID, "/")
if len(packageIDParts) != 2 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW (not for this PR), but this is the code that is very much related to #4284

return "", status.Errorf(codes.InvalidArgument, "Incorrect package ref dentifier, currently just 'foo/bar' patterns are supported: %s", packageID)
}
return unescapedChartID, nil
return unescapedPackageID, nil
}

func SplitChartIdentifier(chartID string) (repoName, chartName string, err error) {
// getUnescapedChartID also ensures that there are two parts (ie. repo/chart-name only)
unescapedChartID, err := GetUnescapedChartID(chartID)
func SplitPackageIdentifier(packageID string) (repoName, packageName string, err error) {
// getUnescapedChartID also ensures that there are two parts (ie. repo/package-name only)
unescapedPackageID, err := GetUnescapedPackageID(packageID)
if err != nil {
return "", "", err
}
chartIDParts := strings.Split(unescapedChartID, "/")
return chartIDParts[0], chartIDParts[1], nil
packageIDParts := strings.Split(unescapedPackageID, "/")
return packageIDParts[0], packageIDParts[1], nil
}

// DefaultValuesFromSchema returns a yaml string with default values generated from an OpenAPI v3 Schema
Expand Down
4 changes: 2 additions & 2 deletions cmd/kubeapps-apis/plugins/pkg/pkgutils/pkgutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ func TestGetUnescapedChartID(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
actualOut, err := GetUnescapedChartID(tc.in)
actualOut, err := GetUnescapedPackageID(tc.in)
if got, want := status.Code(err), tc.statusCode; got != want {
t.Fatalf("got: %+v, want: %+v, err: %+v", got, want, err)
}
Expand Down Expand Up @@ -671,7 +671,7 @@ func TestSplitChartIdentifier(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
repoName, chartName, err := SplitChartIdentifier(tc.in)
repoName, chartName, err := SplitPackageIdentifier(tc.in)
if got, want := status.Code(err), tc.statusCode; got != want {
t.Fatalf("got: %+v, want: %+v, err: %+v", got, want, err)
}
Expand Down
13 changes: 5 additions & 8 deletions dashboard/src/components/Catalog/PackageCatalogItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

import * as url from "shared/url";
import { getPluginIcon, getPluginName, PluginNames, trimDescription } from "shared/utils";
import { getPluginIcon, getPluginName, trimDescription } from "shared/utils";
import placeholder from "../../placeholder.png";
import InfoCard from "../InfoCard/InfoCard";
import { IPackageCatalogItem } from "./CatalogItem";
Expand All @@ -22,13 +22,10 @@ export default function PackageCatalogItem(props: IPackageCatalogItem) {
const pkgPluginName = getPluginName(availablePackageSummary.availablePackageRef?.plugin);

// Get the pkg repository for the plugins that have one.
switch (availablePackageSummary.availablePackageRef?.plugin?.name) {
case (PluginNames.PACKAGES_HELM, PluginNames.PACKAGES_FLUX):
pkgRepository = availablePackageSummary.availablePackageRef?.identifier.split("/")[0];
break;
case PluginNames.PACKAGES_KAPP:
// TODO: get repo from kapp-controller
break;
// Assuming an identifier will always be like: "repo/pkgName"
const splitIdentifier = availablePackageSummary.availablePackageRef?.identifier.split("/");
if (splitIdentifier && splitIdentifier?.length > 0) {
pkgRepository = splitIdentifier[0];
}

return (
Expand Down