Add support to configure multiple helm chart repo using HelmChartRepository CRD #5933

akashshinde · 2020-07-09T16:09:21Z

This PR adds support to configure multiple helm chart repos in openshift.

It uses HelmChartRepository CRD to add multiple repository support. ref PR: openshift/api#598

How to test

Register HelmChartRepository CRD

oc apply -f https://raw.githubusercontent.com/openshift/api/master/helm/v1beta1/0000_10-helm-chart-repository.crd.yaml

Add two helm chart repository

Add Redhat chart repo.

apiVersion: helm.openshift.io/v1beta1
kind: HelmChartRepository
metadata:
  name: redhat-chart-repo
spec:
  name: redhat-chart-repo
  connectionConfig:
    url: https://redhat-developer.github.io/redhat-helm-charts

Add Azure sample helm chart repo.

apiVersion: helm.openshift.io/v1beta1
kind: HelmChartRepository
metadata:
  name: azure-sample-repo
spec:
  name: azure-sample-repo
  connectionConfig:
    url: https://raw.githubusercontent.com/Azure-Samples/helm-charts/master/docs

Run console in dev mode

Build

./build.sh

Setup environment to use openshift cluster context

source ./contrib/oc-environment.sh

Start console backend

./bin/bridge

Make rest call to helm chart endpoint

 curl localhost:9000/api/helm/charts/index.yaml

Demo Link: https://youtu.be/r68ZWHzZMVA

sbose78 · 2020-07-13T03:49:20Z

The client should only call /api/helm/charts/index.yaml ( the way it does today ..) and the response should be the plain old index.yaml which the UI can currently parse.

Behind the scenes, it is a..

A "long" index.yaml which is a concatenated list of index.yamls from multiple HelmChartRepositories, OR
if there are no HelmChartRepositories, return the index.yaml of the default Red Hat Helm Chart Repo.

As a test, the UI should render fine with zero UI changes.

sbose78 · 2020-07-13T03:51:33Z

@rohitkrai03 @parvathyvr
For a helm chart in the developer catalog, does the UI need to know which chart repo the chart is being served from ?

sbose78 · 2020-07-13T04:32:30Z

@akashshinde
Can we pass additional metadata per index.yaml entry to specify which chart repo a chart is from?

rohitkrai03 · 2020-07-13T18:25:23Z

@rohitkrai03 @parvathyvr
For a helm chart in the developer catalog, does the UI need to know which chart repo the chart is being served from ?

@sbose78 Early designs for the epic used the chart repository name and URL in the side panel of chart in the catalog. We might also need that data to visually distinguish between same charts coming from different repositories similar to how its done in Helm Hub. @parvathyvr can add more details from UX side.

sbose78 · 2020-07-13T19:00:57Z

Gotcha, @rohitkrai03

@akashshinde What's your suggestion on how the UI could get that data from the returned long index.yaml ?

akashshinde · 2020-07-14T03:48:03Z

@rohitkrai03 Just wondering how UI get to know the chart repo information currently, I see backed only returns https://github.com/helm/helm/blob/9c0fca9aeb9f93c0743c3e234df7e73832b46576/pkg/repo/index.go#L79 struct and this does struct does not really hold the information about the chart repo origin.

sbose78 · 2020-07-14T04:07:17Z

The UI probably doesn't care now since there's only one possible repo.

akashshinde · 2020-07-14T04:51:16Z

There are following ways to embed chart repo metadata in the IndexFile which gets received from chart repo url.

1. Embed chart metadata in chart annotations.

index.yaml response would be

apiVersion: v1
entries:
  aks-helloworld:
  - apiVersion: v1
    created: "2020-03-30T16:27:13.4350608-05:00"
    description: A Helm chart for Kubernetes
    digest: a9bc374461e31cb1429021a898ef83d0a650783033f3bc121ed536ad99301bbc
    name: aks-helloworld
    urls:
    - https://azure-samples.github.io/helm-charts/aks-helloworld-0.1.1.tgz
    version: 0.1.1
    annotations:
      chart_repo: azure
      url: AZURE_REPO_URL
  - apiVersion: v1
    created: "2020-03-30T16:27:13.433497-05:00"
    description: A Helm chart for Kubernetes
    digest: f4eb20e7116b24c4825861e6ff6f4303bbe83bcee4a7b2b6ea9fe6929852f34d
    name: k8s-helloworld
    urls:
    - https://k8s-samples.github.io/helm-charts/k8s-helloworld-0.1.0.tgz
    version: 0.1.0
    annotations:
      chart_repo: kubernetes
      url: K8S_REPO_URL
  ibm-cpq-prod:
  - apiVersion: v2
    appVersion: 10.0.0.6
    created: "2020-06-23T21:48:51.90919-05:00"
    description: |-
      IBM Sterling Configure, Price, Quote (CPQ) solution enables you to quickly configure, price, quote and order the right products and services.\n
      To sell competitively in todayâ€™s multichannel environment, organizations need a way to easily manage product and service configuration and pricing rules that allow prospects, customers, sales staff, call center representatives and Business Partners to quickly find, configure and order the right products and services.\n
      IBMÂ® Sterling Configure, Price, Quote solution automates every step of the configure, price and quote process to help organizations easily create Web storefronts, offer dynamic catalog and pricing information,  and direct customers and partners to find, configure and order the right products and services.\n
      This enables you to transform how you sell complex products and services by removing the internal complexity of multi-tiered selling within your organization and with your partners.\n

      Documentation\n
      Additional information about installation can be found at https://ibm.biz/BdqqUS.

      License\n
      Sterling Configure Price Quote Software is licensed under https://ibm.biz/Bdqqgd which must be accepted during the install of the Product.
    digest: 2d93c570ff47b27858901dacafc22fe64d868e9a8ecb4742867396f2912eded8
    icon: https://raw.githubusercontent.com/IBM/charts/master/logo/ibm-oms-logo.png
    keywords:
    - cpq
    - ifs
    - sterling
    - yantra
    - order
    - fulfillment
    - om
    - amd64
    - framework
    - Commercial
    - RHOCP
    - All items
    - Languages
    - Databases
    - Middleware
    - CI/CD
    - Other
    kubeVersion: '>=1.11.0'
    maintainers:
    - name: IBM
    name: ibm-cpq-prod
    urls:
    - https://redhat-developer.github.com/redhat-helm-charts/charts/ibm-cpq-prod-2.0.0.tgz
    version: 2.0.0
    annotations: 
      chart_repo: IBM
      url: IBM_REPO_URL

Pros

No need to change UI, since response would be aggregated single index.yaml

Cons

Backend needs to iterate over entire chart listing to update Annotations field (https://github.com/helm/helm/blob/9c0fca9aeb9f93c0743c3e234df7e73832b46576/pkg/chart/metadata.go#L58)
Backend would take more time to respond than usual.

2. Have a extra field in the root of each yaml file response, and embed chart metadata in it.

index.yaml response would be

apiVersion: v1
annotations: 
  chart_repo: IBM
  url: IBM_REPO_URL
entries:
  aks-helloworld:
  - apiVersion: v1
    created: "2020-03-30T16:27:13.4350608-05:00"
    description: A Helm chart for Kubernetes
    digest: a9bc374461e31cb1429021a898ef83d0a650783033f3bc121ed536ad99301bbc
    name: aks-helloworld
    urls:
    - https://azure-samples.github.io/helm-charts/aks-helloworld-0.1.1.tgz
    version: 0.1.1
    annotations:
      chart_repo: azure
      url: AZURE_REPO_URL
---
apiVersion: v1
annotations:
  chart_repo: kubernetes
  url: K8S_REPO_URL
entries:
  - apiVersion: v1
    created: "2020-03-30T16:27:13.433497-05:00"
    description: A Helm chart for Kubernetes
    digest: f4eb20e7116b24c4825861e6ff6f4303bbe83bcee4a7b2b6ea9fe6929852f34d
    name: k8s-helloworld
    urls:
    - https://k8s-samples.github.io/helm-charts/k8s-helloworld-0.1.0.tgz
    version: 0.1.0
---
apiVersion: v1
annotations: 
  chart_repo: IBM
  url: IBM_REPO_URL
entries:    
  ibm-cpq-prod:
  - apiVersion: v2
    appVersion: 10.0.0.6
    created: "2020-06-23T21:48:51.90919-05:00"
    description: |-
      IBM Sterling Configure, Price, Quote (CPQ) solution enables you to quickly configure, price, quote and order the right products and services.\n
      To sell competitively in todayâ€™s multichannel environment, organizations need a way to easily manage product and service configuration and pricing rules that allow prospects, customers, sales staff, call center representatives and Business Partners to quickly find, configure and order the right products and services.\n
      IBMÂ® Sterling Configure, Price, Quote solution automates every step of the configure, price and quote process to help organizations easily create Web storefronts, offer dynamic catalog and pricing information,  and direct customers and partners to find, configure and order the right products and services.\n
      This enables you to transform how you sell complex products and services by removing the internal complexity of multi-tiered selling within your organization and with your partners.\n

      Documentation\n
      Additional information about installation can be found at https://ibm.biz/BdqqUS.

      License\n
      Sterling Configure Price Quote Software is licensed under https://ibm.biz/Bdqqgd which must be accepted during the install of the Product.
    digest: 2d93c570ff47b27858901dacafc22fe64d868e9a8ecb4742867396f2912eded8
    icon: https://raw.githubusercontent.com/IBM/charts/master/logo/ibm-oms-logo.png
    keywords:
    - cpq
    - ifs
    - sterling
    - yantra
    - order
    - fulfillment
    - om
    - amd64
    - framework
    - Commercial
    - RHOCP
    - All items
    - Languages
    - Databases
    - Middleware
    - CI/CD
    - Other
    kubeVersion: '>=1.11.0'
    maintainers:
    - name: IBM
    name: ibm-cpq-prod
    urls:
    - https://redhat-developer.github.com/redhat-helm-charts/charts/ibm-cpq-prod-2.0.0.tgz
    version: 2.0.0

Pros

No need to iterate over entire chart listing, would need to iterate over list of chart repos.

Cons
1, Need to introduce new field annotations to index.yaml in console get repo api call

Need to update index.yaml response received from chart repo url
Need to update new field annotations to the response before we send back to console UI.
Chart repo response would be array of index.yaml instead of single index.yaml, therefore UI needs to be tweaked to support that.

cc: @sbose78 @rohitkrai03

sbose78 · 2020-07-14T04:59:42Z

I'll go for option 1

akashshinde · 2020-07-20T05:16:22Z

There are couple of use cases we need to tackle.

When should we fallback to proxy response ?
a. If HelmChartRepository CRD does not exist ?

b. If HelmChartRepository has zero resources exist in the cluster ? (This point will be void If we are going to create redhat
chart repo CR.)
This is something I've not tested yet, but just had a thought about it.
a. Since HelmChartRepository is a cluster-scoped CRD, we would need enough permissions to access it, this works
currently since we're using admin privileges, but eventually might fail if logged user doesn't have enough permission.

b. Do we have special service account/token which would give access to cluster scoped CRD ?

cc: @sbose78

sbose78 · 2020-07-20T16:06:54Z

@akashshinde
If the total number of CRs = 0 or the CRD doesn't exist, we show the default one.

sbose78 · 2020-07-20T16:08:12Z

Non-kube-admin users should be able to have view privileges on HelmChartRepository CRs, we'll have to ensure that works, outside the scope of this PR itself.

akashshinde · 2020-07-20T16:18:52Z

@akashshinde
If the total number of CRs = 0 or the CRD doesn't exist, we show the default one.

Great, that's the way implementation is done .

sbose78 · 2020-07-20T16:35:27Z

Could you please attach GIFs/screenshots/demo videos, @akashshinde ?

akashshinde · 2020-07-20T19:16:50Z

Could you please attach GIFs/screenshots/demo videos, @akashshinde ?

uploaded a demo video: https://youtu.be/r68ZWHzZMVA

pkg/proxy/proxy.go

sbose78 · 2020-07-20T20:14:48Z

uploaded a demo video: https://youtu.be/r68ZWHzZMVA

I see a cache issue there. As discussed, could we address it please?

akashshinde · 2020-07-21T05:12:41Z

I see a cache issue there. As discussed, could we address it please?
I found why we are getting this issue.

Client request for /api/helm/charts/index.yaml the first time - If no HelmChartRepository is configured proxy comes into play. Reverse proxy sets response of redhat helm chart repo and also copies all the headers from redhat chart repo url.
Which copies header Cache-Control=600, asks browser to cache it for 600secs.
Now when we add HelmChartReposiory CRs, then browser makes another call to chart index api to fetch latest yaml. Here browser has already cached response for 600secs so report back the same previous yaml response and ingores the one received from server.

Solution:
Finding a way to ignore Cache-Control header being set in the first attempt.

cc: @sbose78

akashshinde · 2020-07-21T06:32:18Z

Raised PR #6044 to handle backlisting unwanted headers.

pkg/helm/actions/get_repos.go

sbose78 · 2020-07-27T15:52:34Z

/assign @spadgett
/assign @pedjak

spadgett · 2020-07-31T15:24:55Z

pkg/helm/actions/get_repos.go

Just a heads up that this will conflict with #1862, which switches logging to klog. We might hold #1862 until after this merges.

cc @zherman0 @jhadvig

pkg/helm/handlers/handlers.go

sbose78 · 2020-07-31T18:27:03Z

/lgtm

sbose78 · 2020-07-31T18:27:08Z

/retest

spadgett · 2020-07-31T18:30:44Z

/approve

openshift-ci-robot · 2020-07-31T18:31:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akashshinde, sbose78, spadgett

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

akashshinde · 2020-07-31T19:17:42Z

/test e2e-gcp-console

pedjak · 2020-07-31T20:46:37Z

/retest

sbose78 · 2020-07-31T23:13:23Z

/retest

akashshinde · 2020-08-01T02:31:12Z

/test e2e-gcp-console

Although console endpoint `/api/helm/charts/index.yaml` handles the situation when there is no HelmChartRepository CR present in the cluster, we should align us to other default cluster settings and provide the default HelmChartRepository CR in the payload Prior introducing openshift/console#5933 all authenticated users could browse the charts from the chart repo. This PR restores that functionality by introducing additional `helm-chartrepos-viewer` ClusterRole, binding it to all authenticated users.

Although console endpoint `/api/helm/charts/index.yaml` handles the situation when there is no HelmChartRepository CR present in the cluster, we should align us to other default cluster settings and provide the default HelmChartRepository CR in the payload. Default configuration can be removed/edited by cluster admin. Prior introducing openshift/console#5933 all authenticated users could browse the charts from the chart repo. This PR restores that functionality by introducing additional `helm-chartrepos-viewer` ClusterRole, binding it to all authenticated users.

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. component/backend Related to backend labels Jul 9, 2020

openshift-ci-robot requested review from alecmerdler and jhadvig July 9, 2020 16:09

akashshinde changed the title ~~[WIP] Add support to configure multiple helm chart repo using HelmChartRepository CRD~~ Add support to configure multiple helm chart repo using HelmChartRepository CRD Jul 20, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 20, 2020

akashshinde commented Jul 20, 2020

View reviewed changes

pkg/proxy/proxy.go Outdated Show resolved Hide resolved

Avni-Sharma reviewed Jul 21, 2020

View reviewed changes

pkg/helm/actions/get_repos.go Outdated Show resolved Hide resolved

sbose78 mentioned this pull request Jul 23, 2020

Design proposal for implementing helm support in OpenShift Console openshift/enhancements#175

Merged

akashshinde mentioned this pull request Jul 27, 2020

Added ability to configure Helm chart repository accessible within cluster openshift/api#598

Merged

openshift-ci-robot assigned pedjak Jul 27, 2020

akashshinde force-pushed the support_multi_repo branch from 2d5787f to deaea19 Compare July 31, 2020 15:08

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 31, 2020

akashshinde force-pushed the support_multi_repo branch from deaea19 to b89daa9 Compare July 31, 2020 15:11

spadgett reviewed Jul 31, 2020

View reviewed changes

akashshinde force-pushed the support_multi_repo branch from b89daa9 to 24991e5 Compare July 31, 2020 15:54

pedjak suggested changes Jul 31, 2020

View reviewed changes

pkg/helm/handlers/handlers.go Outdated Show resolved Hide resolved

spadgett reviewed Jul 31, 2020

View reviewed changes

pkg/helm/handlers/handlers.go Outdated Show resolved Hide resolved

akashshinde force-pushed the support_multi_repo branch 2 times, most recently from 982ae27 to 464aa8c Compare July 31, 2020 17:37

akashshinde added 2 commits July 31, 2020 23:11

Update deps

45cfd11

Add support to configure multiple helm chart repo

54701da

akashshinde force-pushed the support_multi_repo branch from 464aa8c to 54701da Compare July 31, 2020 17:42

akashshinde mentioned this pull request Jul 31, 2020

Use HeaderBlacklist to filter unwated headers in reverse proxy #6044

Closed

1 task

openshift-ci-robot assigned sbose78 Jul 31, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 31, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 31, 2020

openshift-merge-robot merged commit f98adf8 into openshift:master Aug 1, 2020

spadgett added this to the v4.6 milestone Aug 1, 2020

akashshinde mentioned this pull request Aug 13, 2020

Bug 1878016: Add documentation to describe configuring multiple helm chart repos #6324

Merged

pedjak mentioned this pull request Aug 17, 2020

Bug 1872375: Add RedHat Helm chart repository as default repo in OCP payload openshift/console-operator#458

Merged

Add support to configure multiple helm chart repo using HelmChartRepository CRD #5933

Add support to configure multiple helm chart repo using HelmChartRepository CRD #5933

Uh oh!

Conversation

akashshinde commented Jul 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to test

Add two helm chart repository

Run console in dev mode

Make rest call to helm chart endpoint

Uh oh!

sbose78 commented Jul 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbose78 commented Jul 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbose78 commented Jul 13, 2020

Uh oh!

rohitkrai03 commented Jul 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbose78 commented Jul 13, 2020

Uh oh!

akashshinde commented Jul 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbose78 commented Jul 14, 2020

Uh oh!

akashshinde commented Jul 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Embed chart metadata in chart annotations.

2. Have a extra field in the root of each yaml file response, and embed chart metadata in it.

Uh oh!

sbose78 commented Jul 14, 2020

Uh oh!

akashshinde commented Jul 20, 2020

Uh oh!

sbose78 commented Jul 20, 2020

Uh oh!

sbose78 commented Jul 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akashshinde commented Jul 20, 2020

Uh oh!

sbose78 commented Jul 20, 2020

Uh oh!

akashshinde commented Jul 20, 2020

Uh oh!

Uh oh!

sbose78 commented Jul 20, 2020

Uh oh!

akashshinde commented Jul 21, 2020

Uh oh!

akashshinde commented Jul 21, 2020

Uh oh!

Uh oh!

sbose78 commented Jul 27, 2020

Uh oh!

spadgett Jul 31, 2020

Choose a reason for hiding this comment

Uh oh!

akashshinde Jul 31, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sbose78 commented Jul 31, 2020

Uh oh!

sbose78 commented Jul 31, 2020

Uh oh!

spadgett commented Jul 31, 2020

Uh oh!

openshift-ci-robot commented Jul 31, 2020

Uh oh!

akashshinde commented Jul 31, 2020

Uh oh!

pedjak commented Jul 31, 2020

Uh oh!

sbose78 commented Jul 31, 2020

akashshinde commented Jul 9, 2020 •

edited

Loading

sbose78 commented Jul 13, 2020 •

edited

Loading

sbose78 commented Jul 13, 2020 •

edited

Loading

rohitkrai03 commented Jul 13, 2020 •

edited

Loading

akashshinde commented Jul 14, 2020 •

edited

Loading

akashshinde commented Jul 14, 2020 •

edited

Loading

sbose78 commented Jul 20, 2020 •

edited

Loading