Skip to content

Commit

Permalink
Merge pull request openshift#7466 from rohitkrai03/helm-repo-name-fix
Browse files Browse the repository at this point in the history
Bug 1906718: Use chart repository spec name when available
  • Loading branch information
openshift-merge-robot authored Dec 11, 2020
2 parents 86d2ed1 + a613672 commit 17cb2b6
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const helmCatalogPlugin: Plugin<HelmCatalogConsumedExtensions> = [
filters: [
{
label: 'Chart Repositories',
attribute: 'chartRepositoryName',
attribute: 'chartRepositoryTitle',
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,14 @@ import { safeLoad } from 'js-yaml';
import { coFetch } from '@console/internal/co-fetch';
import { APIError } from '@console/shared';
import { CatalogExtensionHook, CatalogItem } from '@console/plugin-sdk';
import {
useK8sWatchResource,
WatchK8sResource,
} from '@console/internal/components/utils/k8s-watch-hook';
import { K8sResourceKind, referenceForModel } from '@console/internal/module/k8s';
import { HelmChartEntries } from '../../types/helm-types';
import { normalizeHelmCharts } from '../utils/catalog-utils';
import { HelmChartRepositoryModel } from '../../models';

const useHelmCharts: CatalogExtensionHook<CatalogItem[]> = ({
namespace,
Expand All @@ -14,6 +20,15 @@ const useHelmCharts: CatalogExtensionHook<CatalogItem[]> = ({
const [helmCharts, setHelmCharts] = React.useState<HelmChartEntries>();
const [loadedError, setLoadedError] = React.useState<APIError>();

const resourceSelector: WatchK8sResource = {
isList: true,
kind: referenceForModel(HelmChartRepositoryModel),
};

const [chartRepositories, chartRepositoriesLoaded] = useK8sWatchResource<K8sResourceKind[]>(
resourceSelector,
);

React.useEffect(() => {
coFetch('/api/helm/charts/index.yaml')
.then(async (res) => {
Expand All @@ -25,11 +40,11 @@ const useHelmCharts: CatalogExtensionHook<CatalogItem[]> = ({
}, []);

const normalizedHelmCharts: CatalogItem[] = React.useMemo(
() => normalizeHelmCharts(helmCharts, namespace, t),
[helmCharts, namespace, t],
() => normalizeHelmCharts(helmCharts, chartRepositories, namespace, t),
[chartRepositories, helmCharts, namespace, t],
);

const loaded = !!helmCharts;
const loaded = !!helmCharts && chartRepositoriesLoaded;

return [normalizedHelmCharts, loaded, loadedError];
};
Expand Down
16 changes: 10 additions & 6 deletions frontend/packages/helm-plugin/src/catalog/utils/catalog-utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,31 @@ import { CatalogItem } from '@console/plugin-sdk';
import { toTitleCase } from '@console/shared';
import { ExternalLink } from '@console/internal/components/utils';
import { getImageForIconClass } from '@console/internal/components/catalog/catalog-item-icon';
import { K8sResourceKind } from '@console/internal/module/k8s';
import { HelmChartEntries, HelmChartMetaData } from '../../types/helm-types';
import { getChartRepositoryTitle } from '../../utils/helm-utils';
import HelmReadmeLoader from '../components/HelmReadmeLoader';

export const normalizeHelmCharts = (
chartEntries: HelmChartEntries,
chartRepositories: K8sResourceKind[],
activeNamespace: string = '',
t: TFunction,
): CatalogItem[] => {
return _.reduce(
chartEntries,
(normalizedCharts, charts, key) => {
const chartRepositoryName = key.split('--').pop();
const chartRepoName = key.split('--').pop();
const chartRepositoryTitle = getChartRepositoryTitle(chartRepositories, chartRepoName);

charts.forEach((chart: HelmChartMetaData) => {
const { name, digest, created, version, appVersion, description, keywords } = chart;

const displayName = `${toTitleCase(name)} v${version}`;
const provider = toTitleCase(chartRepositoryName);
const imgUrl = chart.icon || getImageForIconClass('icon-helm');
const chartURL = chart.urls[0];
const encodedChartURL = encodeURIComponent(chartURL);
const href = `/catalog/helm-install?chartName=${name}&chartRepoName=${chartRepositoryName}&chartURL=${encodedChartURL}&preselected-ns=${activeNamespace}`;
const href = `/catalog/helm-install?chartName=${name}&chartRepoName=${chartRepoName}&chartURL=${encodedChartURL}&preselected-ns=${activeNamespace}`;

const maintainers = chart.maintainers?.length > 0 && (
<>
Expand Down Expand Up @@ -77,12 +81,12 @@ export const normalizeHelmCharts = (
type: 'HelmChart',
name: displayName,
description,
provider,
provider: chartRepositoryTitle,
tags: keywords,
creationTimestamp: created,
attributes: {
name,
chartRepositoryName,
chartRepositoryTitle,
version,
},
icon: {
Expand All @@ -103,7 +107,7 @@ export const normalizeHelmCharts = (
const existingChartIndex = normalizedCharts.findIndex((currentChart) => {
return (
currentChart.attributes?.name === name &&
currentChart.attributes?.chartRepositoryName === chartRepositoryName
currentChart.attributes?.chartRepositoryTitle === chartRepositoryTitle
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export const mockIBMHelmChartData: HelmChartMetaData[] = [
'https://raw.githubusercontent.com/IBM/charts/master/repo/community/hazelcast-enterprise-1.0.3.tgz',
],
version: '1.0.3',
repoName: 'ibm-helm-repo',
repoName: 'IBM Helm Repo',
},
{
apiVersion: 'v1',
Expand All @@ -146,7 +146,7 @@ export const mockIBMHelmChartData: HelmChartMetaData[] = [
'https://raw.githubusercontent.com/IBM/charts/master/repo/community/hazelcast-enterprise-1.0.2.tgz',
],
version: '1.0.2',
repoName: 'ibm-helm-repo',
repoName: 'IBM Helm Repo',
},
{
appVersion: '3.10.5',
Expand All @@ -157,7 +157,7 @@ export const mockIBMHelmChartData: HelmChartMetaData[] = [
'https://raw.githubusercontent.com/IBM/charts/master/repo/community/hazelcast-enterprise-1.0.1.tgz',
],
version: '1.0.1',
repoName: 'ibm-helm-repo',
repoName: 'IBM Helm Repo',
},
];

Expand All @@ -170,7 +170,7 @@ export const mockRedhatHelmChartData: HelmChartMetaData[] = [
'https://raw.githubusercontent.com/redhat-helm-charts/master/repo/stable/hazelcast-enterprise-1.0.2.tgz',
],
version: '1.0.2',
repoName: 'redhat-helm-repo',
repoName: 'Red Hat Helm Repo',
},
{
appVersion: '3.10.5',
Expand All @@ -181,7 +181,36 @@ export const mockRedhatHelmChartData: HelmChartMetaData[] = [
'https://raw.githubusercontent.com/redhat-helm-charts/master/repo/stable/hazelcast-enterprise-1.0.1.tgz',
],
version: '1.0.1',
repoName: 'redhat-helm-repo',
repoName: 'Red Hat Helm Repo',
},
];

export const mockHelmChartRepositories: K8sResourceKind[] = [
{
apiVersion: 'helm.openshift.io/v1beta1',
kind: 'HelmChartRepository',
metadata: {
name: 'ibm-helm-repo',
},
spec: {
connectionConfig: {
url: 'https://raw.githubusercontent.com/IBM/charts/master/repo/community',
},
name: 'IBM Helm Repo',
},
},
{
apiVersion: 'helm.openshift.io/v1beta1',
kind: 'HelmChartRepository',
metadata: {
name: 'redhat-helm-repo',
},
spec: {
connectionConfig: {
url: 'https://redhat-developer.github.io/redhat-helm-charts',
},
name: 'Red Hat Helm Repo',
},
},
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ import { coFetchJSON, coFetch } from '@console/internal/co-fetch';
import { DropdownField } from '@console/shared';
import { confirmModal } from '@console/internal/components/modals/confirm-modal';
import { EditorType } from '@console/shared/src/components/synced-editor/editor-toggle';
import {
useK8sWatchResource,
WatchK8sResource,
} from '@console/internal/components/utils/k8s-watch-hook';
import { K8sResourceKind, referenceForModel } from '@console/internal/module/k8s';
import {
HelmChartMetaData,
HelmChart,
Expand All @@ -22,7 +27,9 @@ import {
getChartReadme,
concatVersions,
getChartEntriesByName,
getChartRepositoryTitle,
} from '../../../utils/helm-utils';
import { HelmChartRepositoryModel } from '../../../models';

export type HelmChartVersionDropdownProps = {
chartVersion: string;
Expand All @@ -49,6 +56,12 @@ const HelmChartVersionDropdown: React.FunctionComponent<HelmChartVersionDropdown
const [initialYamlData, setInitialYamlData] = React.useState<string>('');
const [initialFormData, setInitialFormData] = React.useState<object>();

const resourceSelector: WatchK8sResource = {
isList: true,
kind: referenceForModel(HelmChartRepositoryModel),
};
const [chartRepositories] = useK8sWatchResource<K8sResourceKind[]>(resourceSelector);

const warnOnChartVersionChange = (
onAccept: ModalCallback,
currentVersion: string,
Expand Down Expand Up @@ -106,15 +119,20 @@ const HelmChartVersionDropdown: React.FunctionComponent<HelmChartVersionDropdown
if (ignore) return;
}
if (ignore) return;
const chartEntries = getChartEntriesByName(json?.entries, chartName, chartRepoName);
const chartEntries = getChartEntriesByName(
json?.entries,
chartName,
chartRepoName,
chartRepositories,
);
setHelmChartEntries(chartEntries);
setHelmChartVersions(getChartVersions(chartEntries, t));
};
fetchChartVersions();
return () => {
ignore = true;
};
}, [chartName, chartRepoName, t]);
}, [chartName, chartRepoName, chartRepositories, t]);

const onChartVersionChange = (value: string) => {
const [version, repoName] = value.split('--');
Expand Down Expand Up @@ -167,7 +185,12 @@ const HelmChartVersionDropdown: React.FunctionComponent<HelmChartVersionDropdown
_.isEmpty(helmChartVersions) && !chartVersion
? t('helm-plugin~No versions available')
: helmChartVersions[`${chartVersion}`] ||
concatVersions(chartVersion, appVersion, t, chartRepoName);
concatVersions(
chartVersion,
appVersion,
t,
getChartRepositoryTitle(chartRepositories, chartRepoName),
);

return (
<GridItem span={6}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
flattenedMockReleaseResources,
mockChartEntries,
mockRedhatHelmChartData,
mockHelmChartRepositories,
} from '../../components/__tests__/helm-release-mock-data';

const t = (key: TFunction) => key;
Expand Down Expand Up @@ -56,7 +57,7 @@ describe('Helm Releases Utils', () => {

it('should return the helm chart url from ibm repo', () => {
const chartVersion = '1.0.2';
const chartRepoName = 'ibm-helm-repo';
const chartRepoName = 'IBM Helm Repo';
const chartURL = getChartURL(mockHelmChartData, chartVersion, chartRepoName);
expect(chartURL).toBe(
'https://raw.githubusercontent.com/IBM/charts/master/repo/community/hazelcast-enterprise-1.0.2.tgz',
Expand All @@ -65,7 +66,7 @@ describe('Helm Releases Utils', () => {

it('should return the helm chart url from redhat repo', () => {
const chartVersion = '1.0.1';
const chartRepoName = 'redhat-helm-repo';
const chartRepoName = 'Red Hat Helm Repo';
const chartURL = getChartURL(mockHelmChartData, chartVersion, chartRepoName);
expect(chartURL).toBe(
'https://raw.githubusercontent.com/redhat-helm-charts/master/repo/stable/hazelcast-enterprise-1.0.1.tgz',
Expand All @@ -75,13 +76,13 @@ describe('Helm Releases Utils', () => {
it('should return the chart versions, concatenated with the App Version, available for the helm chart', () => {
const chartVersions = getChartVersions(mockHelmChartData, t);
expect(chartVersions).toEqual({
'1.0.1--ibm-helm-repo':
'1.0.1--IBM Helm Repo':
'1.0.1helm-plugin~ / App Version {{appVersion}}helm-plugin~ (Provided by {{chartRepoName}})',
'1.0.1--redhat-helm-repo':
'1.0.1--Red Hat Helm Repo':
'1.0.1helm-plugin~ / App Version {{appVersion}}helm-plugin~ (Provided by {{chartRepoName}})',
'1.0.2--ibm-helm-repo': '1.0.2helm-plugin~ (Provided by {{chartRepoName}})',
'1.0.2--redhat-helm-repo': '1.0.2helm-plugin~ (Provided by {{chartRepoName}})',
'1.0.3--ibm-helm-repo':
'1.0.2--IBM Helm Repo': '1.0.2helm-plugin~ (Provided by {{chartRepoName}})',
'1.0.2--Red Hat Helm Repo': '1.0.2helm-plugin~ (Provided by {{chartRepoName}})',
'1.0.3--IBM Helm Repo':
'1.0.3helm-plugin~ / App Version {{appVersion}}helm-plugin~ (Provided by {{chartRepoName}})',
});
});
Expand All @@ -91,20 +92,38 @@ describe('Helm Releases Utils', () => {
mockChartEntries,
'hazelcast-enterprise',
'redhat-helm-repo',
mockHelmChartRepositories,
);
expect(chartEntries).toEqual(mockRedhatHelmChartData);
});

it('should return chart entries by name from all repos if repo name not provided', () => {
const chartEntries = getChartEntriesByName(mockChartEntries, 'hazelcast-enterprise');
const chartEntries = getChartEntriesByName(
mockChartEntries,
'hazelcast-enterprise',
'',
mockHelmChartRepositories,
);
expect(chartEntries).toEqual(mockHelmChartData);
});

it('should return empty array if wrong chart name or repo name provided', () => {
expect(
getChartEntriesByName(mockChartEntries, 'hazelcast-enterprise', 'stable-helm-repo'),
getChartEntriesByName(
mockChartEntries,
'hazelcast-enterprise',
'stable-helm-repo',
mockHelmChartRepositories,
),
).toEqual([]);
expect(
getChartEntriesByName(
mockChartEntries,
'hazelcast-enterprise-prod',
'',
mockHelmChartRepositories,
),
).toEqual([]);
expect(getChartEntriesByName(mockChartEntries, 'hazelcast-enterprise-prod')).toEqual([]);
});

it('should omit resources with no data and flatten them', () => {
Expand Down
15 changes: 13 additions & 2 deletions frontend/packages/helm-plugin/src/utils/helm-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,26 +92,37 @@ export const getChartURL = (
return chartData?.urls[0];
};

export const getChartRepositoryTitle = (
chartRepositories: K8sResourceKind[],
chartRepoName: string,
) => {
const chartRepository = chartRepositories?.find((repo) => repo.metadata.name === chartRepoName);
return chartRepository?.spec?.name || toTitleCase(chartRepoName);
};

export const getChartEntriesByName = (
chartEntries: HelmChartEntries,
chartName: string,
chartRepoName?: string,
chartRepositories?: K8sResourceKind[],
): HelmChartMetaData[] => {
if (chartName && chartRepoName) {
const chartRepositoryTitle = getChartRepositoryTitle(chartRepositories, chartRepoName);
return (
chartEntries?.[`${chartName}--${chartRepoName}`]?.map((e) => ({
...e,
repoName: chartRepoName,
repoName: chartRepositoryTitle,
})) ?? []
);
}
const entries = _.reduce(
chartEntries,
(acc, charts, key) => {
const repoName = key.split('--').pop();
const chartRepositoryTitle = getChartRepositoryTitle(chartRepositories, repoName);
charts.forEach((chart: HelmChartMetaData) => {
if (chart.name === chartName) {
acc.push({ ...chart, repoName });
acc.push({ ...chart, repoName: chartRepositoryTitle });
}
});
return acc;
Expand Down

0 comments on commit 17cb2b6

Please sign in to comment.