From c897baad201e3328588adfb47658a9035dd94e9d Mon Sep 17 00:00:00 2001 From: Steven Kang Date: Tue, 24 Jun 2025 15:46:10 +1200 Subject: [PATCH] fix: fetching values from both install and upgrade views - develop [R8S-368] (#820) --- .../ChartActions/UpgradeButton.tsx | 20 ++++-- .../ChartActions/UpgradeHelmModal.tsx | 46 +++++++------ .../helm/queries/useHelmRepoVersions.ts | 7 +- app/react/kubernetes/helm/types.ts | 3 +- pkg/libhelm/release/release.go | 2 + pkg/libhelm/sdk/search_repo.go | 14 +++- pkg/libhelm/sdk/show.go | 64 ++++++++++++++----- 7 files changed, 111 insertions(+), 45 deletions(-) diff --git a/app/react/kubernetes/helm/HelmApplicationView/ChartActions/UpgradeButton.tsx b/app/react/kubernetes/helm/HelmApplicationView/ChartActions/UpgradeButton.tsx index 2c3f05ab6..648e3628d 100644 --- a/app/react/kubernetes/helm/HelmApplicationView/ChartActions/UpgradeButton.tsx +++ b/app/react/kubernetes/helm/HelmApplicationView/ChartActions/UpgradeButton.tsx @@ -44,7 +44,6 @@ export function UpgradeButton({ useCache ); const versions = helmRepoVersionsQuery.data; - const repo = versions?.[0]?.Repo; // Combined loading state const isLoading = @@ -63,17 +62,28 @@ export function UpgradeButton({ latestVersionQuery?.data && semverCompare(latestVersionAvailable, latestVersionQuery?.data) === 1 ); - const currentVersion = release?.chart.metadata?.version; + + const currentRepo = versions?.find( + (v) => + v.Chart === release?.chart.metadata?.name && + v.AppVersion === release?.chart.metadata?.appVersion && + v.Version === release?.chart.metadata?.version + )?.Repo; const editableHelmRelease: UpdateHelmReleasePayload = { name: releaseName, namespace: namespace || '', values: release?.values?.userSuppliedValues, chart: release?.chart.metadata?.name || '', - version: currentVersion, - repo, + appVersion: release?.chart.metadata?.appVersion, + version: release?.chart.metadata?.version, + repo: currentRepo ?? '', }; + const filteredVersions = currentRepo + ? versions?.filter((v) => v.Repo === currentRepo) || [] + : versions || []; + return (
; - values: UpdateHelmReleasePayload; + payload: UpdateHelmReleasePayload; versions: ChartVersion[]; chartName: string; - repo: string; } export function UpgradeHelmModal({ - values, + payload, versions, onSubmit, chartName, - repo, }: Props) { const versionOptions: Option[] = versions.map((version) => { - const isCurrentVersion = version.Version === values.version; - const label = `${version.Repo}@${version.Version}${ + const repo = payload.repo === version.Repo ? version.Repo : ''; + const isCurrentVersion = + version.AppVersion === payload.appVersion && + version.Version === payload.version; + + const label = `${repo}@${version.Version}${ isCurrentVersion ? ' (current)' : '' }`; + return { + repo, label, value: version, }; }); + const defaultVersion = - versionOptions.find((v) => v.value.Version === values.version)?.value || - versionOptions[0]?.value; + versionOptions.find( + (v) => + v.value.AppVersion === payload.appVersion && + v.value.Version === payload.version && + v.value.Repo === payload.repo + )?.value || versionOptions[0]?.value; const [version, setVersion] = useState(defaultVersion); - const [userValues, setUserValues] = useState(values.values || ''); + const [userValues, setUserValues] = useState(payload.values || ''); const [atomic, setAtomic] = useState(true); const chartValuesRefQuery = useHelmChartValues({ chart: chartName, - repo, + repo: version.Repo, version: version.Version, }); @@ -75,7 +84,7 @@ export function UpgradeHelmModal({ > onSubmit({ - name: values.name, + name: payload.name, values: userValues, - namespace: values.namespace, - chart: values.chart, + namespace: payload.namespace, + chart: payload.chart, repo: version.Repo, version: version.Version, atomic, @@ -165,13 +174,12 @@ export function UpgradeHelmModal({ } export async function openUpgradeHelmModal( - values: UpdateHelmReleasePayload, + payload: UpdateHelmReleasePayload, versions: ChartVersion[] ) { return openModal(withReactQuery(withCurrentUser(UpgradeHelmModal)), { - values, + payload, versions, - chartName: values.chart, - repo: values.repo ?? '', + chartName: payload.chart, }); } diff --git a/app/react/kubernetes/helm/queries/useHelmRepoVersions.ts b/app/react/kubernetes/helm/queries/useHelmRepoVersions.ts index 006b74599..5543de3d7 100644 --- a/app/react/kubernetes/helm/queries/useHelmRepoVersions.ts +++ b/app/react/kubernetes/helm/queries/useHelmRepoVersions.ts @@ -10,12 +10,15 @@ interface HelmSearch { } interface Entries { - [key: string]: { version: string }[]; + [key: string]: { version: string; appVersion: string }[]; } export interface ChartVersion { + Chart?: string; Repo: string; + Label?: string; Version: string; + AppVersion?: string; } /** @@ -77,8 +80,10 @@ async function getSearchHelmRepo( const versions = data.entries[chart]; return ( versions?.map((v) => ({ + Chart: chart, Repo: repo, Version: v.version, + AppVersion: v.appVersion, })) ?? [] ); } catch (err) { diff --git a/app/react/kubernetes/helm/types.ts b/app/react/kubernetes/helm/types.ts index 3094f84b2..b1e958471 100644 --- a/app/react/kubernetes/helm/types.ts +++ b/app/react/kubernetes/helm/types.ts @@ -120,9 +120,10 @@ export interface InstallChartPayload { export interface UpdateHelmReleasePayload { namespace: string; values?: string; - repo?: string; + repo: string; name: string; chart: string; + appVersion?: string; version?: string; atomic?: boolean; } diff --git a/pkg/libhelm/release/release.go b/pkg/libhelm/release/release.go index 56888291c..acc3328b1 100644 --- a/pkg/libhelm/release/release.go +++ b/pkg/libhelm/release/release.go @@ -36,6 +36,8 @@ type Release struct { Manifest string `json:"manifest,omitempty"` // Hooks are all of the hooks declared for this release. Hooks []*Hook `json:"hooks,omitempty"` + // AppVersion is the app version of the release. + AppVersion string `json:"appVersion,omitempty"` // Version is an int which represents the revision of the release. Version int `json:"version,omitempty"` // Namespace is the kubernetes namespace of the release. diff --git a/pkg/libhelm/sdk/search_repo.go b/pkg/libhelm/sdk/search_repo.go index 63015330c..a97b7d606 100644 --- a/pkg/libhelm/sdk/search_repo.go +++ b/pkg/libhelm/sdk/search_repo.go @@ -90,8 +90,17 @@ func (hspm *HelmSDKPackageManager) SearchRepo(searchRepoOpts options.SearchRepoO return nil, errors.Wrap(err, "failed to ensure Helm directories exist") } + repoName, err := getRepoNameFromURL(repoURL.String()) + if err != nil { + log.Error(). + Str("context", "HelmClient"). + Err(err). + Msg("Failed to get hostname from URL") + return nil, err + } + // Download the index file and update repository configuration - indexPath, err := downloadRepoIndex(repoURL.String(), repoSettings, searchRepoOpts.Repo) + indexPath, err := downloadRepoIndex(repoURL.String(), repoSettings, repoName) if err != nil { log.Error(). Str("context", "HelmClient"). @@ -163,7 +172,8 @@ func downloadRepoIndex(repoURLString string, repoSettings *cli.EnvSettings, repo // Create chart repository object rep, err := repo.NewChartRepository( &repo.Entry{ - URL: repoURLString, + Name: repoName, + URL: repoURLString, }, getter.All(repoSettings), ) diff --git a/pkg/libhelm/sdk/show.go b/pkg/libhelm/sdk/show.go index 4dc9c402d..9ad2d6007 100644 --- a/pkg/libhelm/sdk/show.go +++ b/pkg/libhelm/sdk/show.go @@ -2,7 +2,8 @@ package sdk import ( "fmt" - "os" + "net/url" + "strings" "github.com/pkg/errors" "github.com/portainer/portainer/pkg/libhelm/options" @@ -32,24 +33,32 @@ func (hspm *HelmSDKPackageManager) Show(showOpts options.ShowOptions) ([]byte, e Str("output_format", string(showOpts.OutputFormat)). Msg("Showing chart information") - // Initialize action configuration (no namespace or cluster access needed) - actionConfig := new(action.Configuration) - err := hspm.initActionConfig(actionConfig, "", nil) + repoURL, err := parseRepoURL(showOpts.Repo) if err != nil { - // error is already logged in initActionConfig - return nil, fmt.Errorf("failed to initialize helm configuration: %w", err) + log.Error(). + Str("context", "HelmClient"). + Str("repo", showOpts.Repo). + Err(err). + Msg("Invalid repository URL") + return nil, err } - // Create temporary directory for chart download - tempDir, err := os.MkdirTemp("", "helm-show-*") + repoName, err := getRepoNameFromURL(repoURL.String()) if err != nil { log.Error(). Str("context", "HelmClient"). Err(err). - Msg("Failed to create temp directory") - return nil, fmt.Errorf("failed to create temp directory: %w", err) + Msg("Failed to get hostname from URL") + return nil, err + } + + // Initialize action configuration (no namespace or cluster access needed) + actionConfig := new(action.Configuration) + err = hspm.initActionConfig(actionConfig, "", nil) + if err != nil { + // error is already logged in initActionConfig + return nil, fmt.Errorf("failed to initialize helm configuration: %w", err) } - defer os.RemoveAll(tempDir) // Create showClient action showClient, err := initShowClient(actionConfig, showOpts) @@ -68,11 +77,12 @@ func (hspm *HelmSDKPackageManager) Show(showOpts options.ShowOptions) ([]byte, e Str("repo", showOpts.Repo). Msg("Locating chart") - chartPath, err := showClient.ChartPathOptions.LocateChart(showOpts.Chart, hspm.settings) + fullChartPath := fmt.Sprintf("%s/%s", repoName, showOpts.Chart) + chartPath, err := showClient.ChartPathOptions.LocateChart(fullChartPath, hspm.settings) if err != nil { log.Error(). Str("context", "HelmClient"). - Str("chart", showOpts.Chart). + Str("chart", fullChartPath). Str("repo", showOpts.Repo). Err(err). Msg("Failed to locate chart") @@ -104,13 +114,10 @@ func (hspm *HelmSDKPackageManager) Show(showOpts options.ShowOptions) ([]byte, e // and return the show client. func initShowClient(actionConfig *action.Configuration, showOpts options.ShowOptions) (*action.Show, error) { showClient := action.NewShowWithConfig(action.ShowAll, actionConfig) - showClient.ChartPathOptions.RepoURL = showOpts.Repo - showClient.ChartPathOptions.Version = showOpts.Version // If version is "", it will use the latest version + showClient.ChartPathOptions.Version = showOpts.Version // Set output type based on ShowOptions switch showOpts.OutputFormat { - case options.ShowAll: - showClient.OutputFormat = action.ShowAll case options.ShowChart: showClient.OutputFormat = action.ShowChart case options.ShowValues: @@ -127,3 +134,26 @@ func initShowClient(actionConfig *action.Configuration, showOpts options.ShowOpt return showClient, nil } + +// getRepoNameFromURL extracts a unique repository identifier from a URL string. +// It combines hostname and path to ensure uniqueness across different repositories on the same host. +// Examples: +// - https://portainer.github.io/test-public-repo/ -> portainer.github.io-test-public-repo +// - https://portainer.github.io/another-repo/ -> portainer.github.io-another-repo +// - https://charts.helm.sh/stable -> charts.helm.sh-stable +func getRepoNameFromURL(urlStr string) (string, error) { + parsedURL, err := url.Parse(urlStr) + if err != nil { + return "", fmt.Errorf("failed to parse URL: %w", err) + } + + hostname := parsedURL.Hostname() + path := parsedURL.Path + path = strings.Trim(path, "/") + path = strings.ReplaceAll(path, "/", "-") + + if path == "" { + return hostname, nil + } + return fmt.Sprintf("%s-%s", hostname, path), nil +}