Fix issue with Schemas on Helm Hub (#503)

* Fix issue where chart download link was not absolute - Expected url, got chart file name - We now check for this, if url is not absoulte assume filename and append to repo url * Fix issue with Schemas on Helm Hub * Update following changes in #503 * Fix issue with missing schema on upgrade for local chart * Fix failing unit test Co-authored-by: Richard Cox <richard.cox@suse.com>
SUSE · Sep 22, 2020 · 4d36e00 · 4d36e00
1 parent ac2e8b7
commit 4d36e00
Show file tree

Hide file tree

Showing 15 changed files with 183 additions and 46 deletions.
diff --git a/...rc/custom/helm/monocular/chart-details/chart-details-info/chart-details-info.component.ts b/...rc/custom/helm/monocular/chart-details/chart-details-info/chart-details-info.component.ts
@@ -26,7 +26,7 @@ export class ChartDetailsInfoComponent implements OnInit {
   @Input() set currentVersion (version: ChartVersion) {
     this._currentVersion = version;
     if (version) {
-      this.getSchema(this._currentVersion);
+      this.getSchema(this._currentVersion, this.chart);
     }
   }
 
@@ -69,8 +69,8 @@ export class ChartDetailsInfoComponent implements OnInit {
     );
   }
 
-  private getSchema(currentVersion: ChartVersion) {
-    this.chartsService.getChartSchema(currentVersion).pipe(
+  private getSchema(currentVersion: ChartVersion, chart: Chart) {
+    this.chartsService.getChartSchema(currentVersion, chart).pipe(
       first(),
       catchError(() => of(null))
     ).subscribe(schema => {

diff --git a/...tend/packages/suse-extensions/src/custom/helm/monocular/shared/services/charts.service.ts b/...tend/packages/suse-extensions/src/custom/helm/monocular/shared/services/charts.service.ts
@@ -7,6 +7,7 @@ import { catchError, map, tap } from 'rxjs/operators';
 import { getMonocularEndpoint, stratosMonocularEndpointGuid } from '../../stratos-monocular.helper';
 import { Chart } from '../models/chart';
 import { ChartVersion } from '../models/chart-version';
+import { RepoAttributes } from '../models/repo';
 import { ConfigService } from './config.service';
 
 
@@ -22,7 +23,6 @@ export class ChartsService {
     config: ConfigService,
     private route: ActivatedRoute,
   ) {
-    this.hostname = `${config.backendHostname}/chartsvc`;
     this.cacheCharts = {};
     this.hostname = '/pp/v1/chartsvc';
   }
@@ -136,8 +136,53 @@ export class ChartsService {
    * @param version Chart version
    * @return An observable that will be the json schema
    */
-  getChartSchema(chartVersion: ChartVersion): Observable<any> {
-    return chartVersion.attributes.schema ? this.http.get(`${this.hostname}${chartVersion.attributes.schema}`) : of(null);
+  getChartSchema(chartVersion: ChartVersion, chart: Chart): Observable<any> {
+    const url = this.getChartSchemaURL(chartVersion, chart.attributes.name, chart.attributes.repo);
+    return url ? this.http.get(url) : of(null);
+  }
+
+  // Get the URL for obtaining a Chart's schema
+  getChartSchemaURL(chartVersion: ChartVersion, name: string, repo: RepoAttributes): string {
+    // Helm Hub does not give us the schema information so we have to use an additional backend API to fetch the chart and check
+    if (chartVersion.attributes.schema === undefined) {
+      let url = this.getChartURL(chartVersion, repo);
+      url = btoa(url);
+      return `/pp/v1/monocular/schema/${name}/${url}`;
+    }
+
+    // We have the schema URL, so we can fetch that directly
+    return chartVersion.attributes.schema ? `${this.hostname}${chartVersion.attributes.schema}` : null;
+  }
+
+  getChartURL(chartVersion: ChartVersion, repo?: RepoAttributes): string {
+    const firstUrl = this.getFirstChartUrl(chartVersion);
+    if (firstUrl.length > 0) {
+      // Check if url is absolute, if not assume it's a filename
+      if (!firstUrl.startsWith('http://') && !firstUrl.startsWith('https://')) {
+        const repoUrl = repo ? repo.url : '';
+        return repoUrl || this.getChartRepoUrl(chartVersion) + '/' + firstUrl;
+      }
+    }
+    return firstUrl;
+  }
+
+  private getFirstChartUrl(chart: ChartVersion): string {
+    if (chart && chart.attributes && chart.attributes.urls && chart.attributes.urls.length > 0) {
+      return chart.attributes.urls[0];
+    }
+    return '';
+  }
+
+  private getChartRepoUrl(chart: ChartVersion): string {
+    if (chart &&
+      chart.relationships &&
+      chart.relationships.chart &&
+      chart.relationships.chart.data &&
+      chart.relationships.chart.data.repo
+    ) {
+      return chart.relationships.chart.data.repo.url;
+    }
+    return '';
   }
 
   /**
@@ -168,6 +213,15 @@ export class ChartsService {
     );
   }
 
+  getVersionFromEndpoint(endpoint: string, repo: string, chartName: string, version: string): Observable<ChartVersion> {
+    const requestArgs  = { headers: { 'x-cap-cnsi-list': endpoint !== stratosMonocularEndpointGuid ? endpoint :'' } };
+    return this.http.get(
+      `${this.hostname}/v1/charts/${repo}/${chartName}/versions/${version}`, requestArgs).pipe(
+      map(this.extractData),
+      catchError(this.handleError)
+    );
+  }
+
   /**
    * Get the URL for retrieving the chart's icon
    *

diff --git a/src/frontend/packages/suse-extensions/src/custom/helm/store/helm.types.ts b/src/frontend/packages/suse-extensions/src/custom/helm/store/helm.types.ts
@@ -46,6 +46,7 @@ export enum HelmStatus {
 }
 
 export interface HelmChartReference {
+  endpoint?: string;
   name: string;
   repo: string;
   version: string;

diff --git a/...ns/src/custom/kubernetes/workloads/chart-values-editor/chart-values-editor.component.html b/...ns/src/custom/kubernetes/workloads/chart-values-editor/chart-values-editor.component.html
@@ -1,5 +1,5 @@
 <mat-card class="editor-card">
-  <div *ngIf="loading$ | async" class="editor-loading">
+  <div *ngIf="initing || (loading$ | async)" class="editor-loading">
     <div>
       <div class="editor-loading__msg">Loading ...</div>
       <mat-progress-bar class="editor-loading__progress-bar" [color]="'primary'" mode="indeterminate"></mat-progress-bar>      

diff --git a/...ions/src/custom/kubernetes/workloads/chart-values-editor/chart-values-editor.component.ts b/...ions/src/custom/kubernetes/workloads/chart-values-editor/chart-values-editor.component.ts
@@ -98,6 +98,8 @@ export class ChartValuesEditorComponent implements OnInit, OnDestroy, AfterViewI
   // Observable - are we still loading resources?
   public loading$: Observable<boolean>;
 
+  public initing = true;
+
   // Observable for tracking if the Monaco editor has loaded
   private monacoLoaded$ = new BehaviorSubject<boolean>(false);
 
@@ -191,6 +193,8 @@ export class ChartValuesEditorComponent implements OnInit, OnDestroy, AfterViewI
       map(([schema, values, loaded]) => !loaded),
       startWith(true)
     );
+
+    this.initing = false;
   }
 
   ngAfterViewInit(): void {

diff --git a/...xtensions/src/custom/kubernetes/workloads/create-release/create-release.component.spec.ts b/...xtensions/src/custom/kubernetes/workloads/create-release/create-release.component.spec.ts
@@ -47,6 +47,7 @@ describe('CreateReleaseComponent', () => {
 
   beforeEach(() => {
     fixture = TestBed.createComponent(CreateReleaseComponent);
+    httpMock.expectOne('/pp/v1/chartsvc/v1/charts/undefined/undefined/versions/undefined');
     component = fixture.componentInstance;
     fixture.detectChanges();
   });

diff --git a/...use-extensions/src/custom/kubernetes/workloads/create-release/create-release.component.ts b/...use-extensions/src/custom/kubernetes/workloads/create-release/create-release.component.ts
@@ -19,7 +19,6 @@ import { HelmChartReference, HelmInstallValues } from '../../../helm/store/helm.
 import { kubeEntityCatalog } from '../../kubernetes-entity-catalog';
 import { KUBERNETES_ENDPOINT_TYPE } from '../../kubernetes-entity-factory';
 import { KubernetesNamespace } from '../../store/kube.types';
-import { getFirstChartUrl } from '../workload.utils';
 import { ChartValuesConfig, ChartValuesEditorComponent } from './../chart-values-editor/chart-values-editor.component';
 
 @Component({
@@ -62,10 +61,13 @@ export class CreateReleaseComponent implements OnInit, OnDestroy {
     this.cancelUrl = this.chartsService.getChartSummaryRoute(chart.repo, chart.name, chart.version, this.route);
     this.chart = chart;
 
-    this.config = {
-      valuesUrl: `/pp/v1/chartsvc/v1/assets/${chart.repo}/${chart.name}/versions/${chart.version}/values.yaml`,
-      schemaUrl: `/pp/v1/chartsvc/v1/assets/${chart.repo}/${chart.name}/versions/${chart.version}/values.schema.json`,
-    };
+    // Fetch the Chart Version metadata so we can get the correct URL for the Chart's JSON Schema
+    this.chartsService.getVersion(this.chart.repo, this.chart.name, this.chart.version).pipe(first()).subscribe(ch => {
+      this.config = {
+        valuesUrl: `/pp/v1/monocular/values/${this.chart.endpoint}/${this.chart.repo}/${chart.name}/${this.chart.version}`,
+        schemaUrl: this.chartsService.getChartSchemaURL(ch, ch.relationships.chart.data.name, ch.relationships.chart.data.repo)
+      };
+    });
 
     this.setupDetailsStep();
   }
@@ -238,7 +240,7 @@ export class CreateReleaseComponent implements OnInit, OnDestroy {
           throw new Error('Could not get Chart URL');
         }
         // Add the chart url into the values
-        values.chartUrl = getFirstChartUrl(chartInfo);
+        values.chartUrl = this.chartsService.getChartURL(chartInfo);
         if (values.chartUrl.length === 0) {
           throw new Error('Could not get Chart URL');
         }

diff --git a/...se-extensions/src/custom/kubernetes/workloads/release/tabs/helm-release-helper.service.ts b/...se-extensions/src/custom/kubernetes/workloads/release/tabs/helm-release-helper.service.ts
@@ -257,21 +257,17 @@ export class HelmReleaseHelperService {
   // We don't know which Helm repository it came from, so use the name and sources to match
   private isProbablySameChart(a: ChartMetadata, b: ChartMetadata): boolean {
     // Basic properties must be the same
-    if ((a.name !== b.name) || (a.sources.length !== b.sources.length)) {
+    if (a.name !== b.name) {
       return false;
     }
 
-    // Sources must match
+    // Must have at least one source in common
     let count = 0;
     a.sources.forEach(source => {
       count += b.sources.findIndex((s) => s === source) === -1 ? 0 : 1;
     });
 
-    if (count !== a.sources.length) {
-      return false;
-    }
-
-    return true;
+    return count > 0;
   }
 
 }

diff --git a/...ensions/src/custom/kubernetes/workloads/upgrade-release/upgrade-release.component.spec.ts b/...ensions/src/custom/kubernetes/workloads/upgrade-release/upgrade-release.component.spec.ts
@@ -1,5 +1,8 @@
 import { async, ComponentFixture, TestBed } from '@angular/core/testing';
 
+import { MockChartService } from '../../../helm/monocular/shared/services/chart.service.mock';
+import { ChartsService } from '../../../helm/monocular/shared/services/charts.service';
+import { ConfigService } from '../../../helm/monocular/shared/services/config.service';
 import { HelmReleaseProviders, KubeBaseGuidMock } from '../../kubernetes.testing.module';
 import { KubernetesEndpointService } from '../../services/kubernetes-endpoint.service';
 import { WorkloadsBaseTestingModule } from '../workloads.testing.module';
@@ -20,6 +23,8 @@ describe('UpgradeReleaseComponent', () => {
         KubernetesEndpointService,
         KubeBaseGuidMock,
         ...HelmReleaseProviders,
+        { provide: ChartsService, useValue: new MockChartService() },
+        { provide: ConfigService, useValue: { appName: 'appName' } },
       ]
     })
     .compileComponents();

diff --git a/...e-extensions/src/custom/kubernetes/workloads/upgrade-release/upgrade-release.component.ts b/...e-extensions/src/custom/kubernetes/workloads/upgrade-release/upgrade-release.component.ts
@@ -1,7 +1,7 @@
 import { Component, ViewChild } from '@angular/core';
 import { ActivatedRoute } from '@angular/router';
 import { Store } from '@ngrx/store';
-import { Observable, of } from 'rxjs';
+import { combineLatest, Observable, of } from 'rxjs';
 import { filter, first, map, pairwise, tap } from 'rxjs/operators';
 
 import {
@@ -10,12 +10,13 @@ import {
   StepOnNextResult,
 } from '../../../../../../core/src/shared/components/stepper/step/step.component';
 import { ActionState } from '../../../../../../store/src/reducers/api-request-reducer/types';
+import { ChartsService } from '../../../helm/monocular/shared/services/charts.service';
+import { createMonocularProviders } from '../../../helm/monocular/stratos-monocular-providers.helpers';
 import { stratosMonocularEndpointGuid } from '../../../helm/monocular/stratos-monocular.helper';
 import { HelmUpgradeValues, MonocularVersion } from '../../../helm/store/helm.types';
 import { ChartValuesConfig, ChartValuesEditorComponent } from '../chart-values-editor/chart-values-editor.component';
 import { HelmReleaseHelperService } from '../release/tabs/helm-release-helper.service';
 import { HelmReleaseGuid } from '../workload.types';
-import { getFirstChartUrl } from '../workload.utils';
 import { workloadsEntityCatalog } from './../workloads-entity-catalog';
 import { ReleaseUpgradeVersionsListConfig } from './release-version-list-config';
 
@@ -33,7 +34,8 @@ import { ReleaseUpgradeVersionsListConfig } from './release-version-list-config'
       deps: [
         ActivatedRoute
       ]
-    }
+    },
+    ...createMonocularProviders()
   ]
 })
 export class UpgradeReleaseComponent {
@@ -54,7 +56,8 @@ export class UpgradeReleaseComponent {
 
   constructor(
     store: Store<any>,
-    public helper: HelmReleaseHelperService
+    public helper: HelmReleaseHelperService,
+    private chartsService: ChartsService,
   ) {
 
     this.cancelUrl = `/workloads/${this.helper.guid}`;
@@ -91,14 +94,19 @@ export class UpgradeReleaseComponent {
   onNext = (): Observable<StepOnNextResult> => {
     const chart = this.version.relationships.chart.data;
     const version = this.version.attributes.version;
+    const endpointID = this.monocularEndpointId || stratosMonocularEndpointGuid;
+
 
     // Fetch the release metadata so that we have the values used to install the current release
-    return this.helper.release$.pipe(
+    return combineLatest(
+      [this.helper.release$, this.chartsService.getVersionFromEndpoint(endpointID, chart.repo.name, chart.name, version)]
+    ).pipe(
       first(),
-      tap(release => {
+      tap(([release, chartVersionDetail]) => {
+        const schemaUrl = this.chartsService.getChartSchemaURL(chartVersionDetail, chart.name, chart.repo);
         this.config = {
-          schemaUrl: `/pp/v1/chartsvc/v1/assets/${chart.repo.name}/${chart.name}/versions/${version}/values.schema.json`,
-          valuesUrl: `/pp/v1/chartsvc/v1/assets/${chart.repo.name}/${chart.name}/versions/${version}/values.yaml`,
+          schemaUrl,
+          valuesUrl: `/pp/v1/monocular/values/${endpointID}/${chart.repo.name}/${chart.name}/${version}`,
           releaseValues: release.config
         };
       }),
@@ -129,7 +137,7 @@ export class UpgradeReleaseComponent {
         version: this.version.attributes.version,
       },
       monocularEndpoint: this.monocularEndpointId === stratosMonocularEndpointGuid ? null : this.monocularEndpointId,
-      chartUrl: getFirstChartUrl(this.version)
+      chartUrl: this.chartsService.getChartURL(this.version)
     };
 
     // Make the request

diff --git a/src/frontend/packages/suse-extensions/src/custom/kubernetes/workloads/workload.utils.ts b/src/frontend/packages/suse-extensions/src/custom/kubernetes/workloads/workload.utils.ts
diff --git a/src/frontend/packages/suse-extensions/src/custom/kubernetes/workloads/workloads.module.ts b/src/frontend/packages/suse-extensions/src/custom/kubernetes/workloads/workloads.module.ts
@@ -14,6 +14,7 @@ import { HelmReleaseTabBaseComponent } from './release/helm-release-tab-base/hel
 import {
   HelmReleaseAnalysisTabComponent,
 } from './release/tabs/helm-release-analysis-tab/helm-release-analysis-tab.component';
+import { HelmReleaseHistoryTabComponent } from './release/tabs/helm-release-history-tab/helm-release-history-tab.component';
 import { HelmReleaseNotesTabComponent } from './release/tabs/helm-release-notes-tab/helm-release-notes-tab.component';
 import { HelmReleasePodsTabComponent } from './release/tabs/helm-release-pods/helm-release-pods-tab.component';
 import {
@@ -22,12 +23,11 @@ import {
 import { HelmReleaseServicesTabComponent } from './release/tabs/helm-release-services/helm-release-services-tab.component';
 import { HelmReleaseSummaryTabComponent } from './release/tabs/helm-release-summary-tab/helm-release-summary-tab.component';
 import { HelmReleaseValuesTabComponent } from './release/tabs/helm-release-values-tab/helm-release-values-tab.component';
+import { WorkloadLiveReloadComponent } from './release/workload-live-reload/workload-live-reload.component';
 import { HelmReleasesTabComponent } from './releases-tab/releases-tab.component';
 import { WorkloadsStoreModule } from './store/workloads.store.module';
 import { UpgradeReleaseComponent } from './upgrade-release/upgrade-release.component';
 import { WorkloadsRouting } from './workloads.routing';
-import { HelmReleaseHistoryTabComponent } from './release/tabs/helm-release-history-tab/helm-release-history-tab.component';
-import { WorkloadLiveReloadComponent } from './release/workload-live-reload/workload-live-reload.component';
 
 // Default config for the Monaco edfior
 const monacoConfig: NgxMonacoEditorConfig = {
@@ -68,7 +68,7 @@ const monacoConfig: NgxMonacoEditorConfig = {
     HelmReleaseCardComponent
   ],
   providers: [
-    DatePipe
+    DatePipe,
   ]
 })
 export class WorkloadsModule { }
diff --git a/src/jetstream/plugins/monocular/chart.go b/src/jetstream/plugins/monocular/chart.go
@@ -51,7 +51,7 @@ type ChartVersion struct {
 	URLs       []string  `json:"urls"`
 	Readme     string    `json:"readme,omitempty"`
 	Values     string    `json:"values,omitempty"`
-	Schema     string    `json:"schema,omitempty"`
+	Schema     string    `json:"schema"`
 }
 
 //RepoCheck describes the state of a repository in terms its current checksum and last update time.