Skip to content

Commit

Permalink
Switch Dashboard to use new gRPC client throughout. (#6044)
Browse files Browse the repository at this point in the history
<!--
Before you open the request please review the following guidelines and
tips to help it be more easily integrated:

 - Describe the scope of your change - i.e. what the change does.
 - Describe any known limitations with your change.
- Please run any tests or examples that can exercise your modified code.

 Thank you for contributing!
 -->

### Description of the change

This PR switches the dashboard over to use the new connect generated
grpc-web client and associated generated classes.

It was pretty tedious, with some complexity due to the different
generated classes and client.

## Non-tedious actual changes
- Updating the way the proto one-of (single choice for different values)
work throughout to match the new library (which actually ensures there
is only a single value, actually using a `OneOf` concept, whereas the
old improbable library just allowed all the options at once). This was
quite confusing to change in the actual code functionality as the code
is written as if all options may be specified, but I didn't want to
change that in this PR, just switch.
- Updating the `shared/ResourceRef` class to be a child of the generated
`ResourceRef` class so that we can continue to use it in place of the
generated one as we're currently doing (compiler won't accept it
otherwise now). Created #6062 to track removing our shared one if we
can.
- Updating the method used in the frontend to stream resources (and
changes) for an installed package, since improbable used subscriptions
while the connect one uses async iterators (need to verify this works,
since I don't think we test it in CI, but it should just fall back to
polling if it doesn't).
- Update the `KubeappsGRPCClient` to use the new client (and the
different way of setting headers/metadata).

## Tedious parts

- Any object that is generated from the proto messages now needs to be
instantiated with, for example, `new AvailablePackageSummary({...})` to
pass the compilation, rather than the previous `{...} as
AvailablePackageSummary` as this type assertion fails due to the missing
methods.
- Updating all the generated class imports with `_pb` to import the new
versions (without deleting the old in this PR, as that blows it out by
16k lines).
- Updating all the enums to the more sensible names used by the new lib
- Update a lot of tests that used jest to spy on the old implementation.
- Update all API method names since the connect one uses snake-case with
an initial lower case :/

### Benefits

<!-- What benefits will be realized by the code change? -->

Let's see what CI says, but possibly removing 16k lines of the old
generated code (I'll do that in a follow-up PR). The main benefit is
that we'll be using a supported grpc-web typescript client and can
remove the old improbable generation in the kubeapps apis service.

### Possible drawbacks

<!-- Describe any known limitations with your change -->

It's a lot of changes, there maybe unexpected side-effects.

### Applicable issues

<!-- Enter any applicable Issues here (You can reference an issue using
#) -->

- ref #6013 

### Additional information

<!-- If there's anything else that's important and relevant to your pull
request, mention that information here.-->

---------

Signed-off-by: Michael Nelson <minelson@vmware.com>
  • Loading branch information
absoludity authored Mar 14, 2023
1 parent f19a2c5 commit eb82234
Show file tree
Hide file tree
Showing 105 changed files with 1,457 additions and 1,475 deletions.
2 changes: 1 addition & 1 deletion dashboard/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@
"!src/**/*.d.ts"
],
"transformIgnorePatterns": [
"node_modules/(?!@cds|@clr|@lit|axios|bail|ccount|cds|character-entities|comma-separated-tokens|decode-named-character-reference|escape-string-regexp|hast-util-whitespace|is-plain-obj|lit|lodash-es|markdown-table|mdast-util-definitions|mdast-util-find-and-replace|mdast-util-from-markdown|mdast-util-gfm-autolink-literal|mdast-util-gfm|mdast-util-to-hast|mdast-util-to-markdown|mdast-util-to-string|micromark-core-commonmark|monaco-editor|react-monaco-editor|micromark|parse-entities|property-information|ramda|react-markdown|react-syntax-highlighter|remark-breaks|remark-gfm|remark-parse|remark-rehype|space-separated-tokens|swagger-client|swagger-ui-react|trim-lines|trough|unified|unist-builder|unist-util-generated|unist-util-is|unist-util-position|unist-util-stringify-position|unist-util-visit-parents|unist-util-visit|util-find-and-replace|vfile-message|vfile|.*css)"
"node_modules/(?!@cds|@clr|@lit|@bufbuild|axios|bail|ccount|cds|character-entities|comma-separated-tokens|decode-named-character-reference|escape-string-regexp|hast-util-whitespace|is-plain-obj|lit|lodash-es|markdown-table|mdast-util-definitions|mdast-util-find-and-replace|mdast-util-from-markdown|mdast-util-gfm-autolink-literal|mdast-util-gfm|mdast-util-to-hast|mdast-util-to-markdown|mdast-util-to-string|micromark-core-commonmark|monaco-editor|react-monaco-editor|micromark|parse-entities|property-information|ramda|react-markdown|react-syntax-highlighter|remark-breaks|remark-gfm|remark-parse|remark-rehype|space-separated-tokens|swagger-client|swagger-ui-react|trim-lines|trough|unified|unist-builder|unist-util-generated|unist-util-is|unist-util-position|unist-util-stringify-position|unist-util-visit-parents|unist-util-visit|util-find-and-replace|vfile-message|vfile|.*css)"
]
},
"browserslist": {
Expand Down
51 changes: 26 additions & 25 deletions dashboard/src/actions/availablepackages.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import {
GetAvailablePackageDetailResponse,
GetAvailablePackageSummariesResponse,
GetAvailablePackageVersionsResponse,
} from "gen/kubeappsapis/core/packages/v1alpha1/packages";
import { Plugin } from "gen/kubeappsapis/core/plugins/v1alpha1/plugins";
PackageAppVersion,
} from "gen/kubeappsapis/core/packages/v1alpha1/packages_pb";
import { Plugin } from "gen/kubeappsapis/core/plugins/v1alpha1/plugins_pb";
import configureMockStore from "redux-mock-store";
import thunk from "redux-thunk";
import PackagesService from "shared/PackagesService";
Expand All @@ -29,21 +30,21 @@ const defaultPaginationToken = "defaultPaginationToken";
const defaultSize = 0;
const plugin = { name: "my.plugin", version: "0.0.1" } as Plugin;

const defaultAvailablePackageSummary: AvailablePackageSummary = {
const defaultAvailablePackageSummary = new AvailablePackageSummary({
name: "foo",
categories: [""],
displayName: "foo",
iconUrl: "",
latestVersion: { appVersion: "v1.0.0", pkgVersion: "" },
latestVersion: new PackageAppVersion({ appVersion: "v1.0.0", pkgVersion: "" }),
shortDescription: "",
availablePackageRef: {
availablePackageRef: new AvailablePackageReference({
identifier: "foo/foo",
context: { cluster: "", namespace: "package-namespace" } as Context,
plugin: plugin,
},
};
}),
});

const defaultAvailablePackageDetail: AvailablePackageDetail = {
const defaultAvailablePackageDetail = new AvailablePackageDetail({
name: "foo",
categories: [""],
displayName: "foo",
Expand All @@ -67,7 +68,7 @@ const defaultAvailablePackageDetail: AvailablePackageDetail = {
pkgVersion: "1.2.3",
appVersion: "4.5.6",
},
};
});

beforeEach(() => {
store = mockStore({
Expand Down Expand Up @@ -97,11 +98,11 @@ const nextPageToken = "nextPageToken";
const fetchAvailablePackageSummariesTestCases: IfetchAvailablePackageSummariesTestCase[] = [
{
name: "fetches packages with query",
response: {
response: new GetAvailablePackageSummariesResponse({
availablePackageSummaries: [defaultAvailablePackageSummary],
nextPageToken,
categories: ["foo"],
},
}),
requestedRepos: "",
requestedPageToken: currentPageToken,
requestedQuery: "foo",
Expand All @@ -126,11 +127,11 @@ const fetchAvailablePackageSummariesTestCases: IfetchAvailablePackageSummariesTe
},
{
name: "fetches packages from a repo (first page)",
response: {
response: new GetAvailablePackageSummariesResponse({
availablePackageSummaries: [defaultAvailablePackageSummary],
nextPageToken,
categories: ["foo"],
},
}),
requestedRepos: repos,
requestedPageToken: "",
expectedActions: [
Expand All @@ -151,11 +152,11 @@ const fetchAvailablePackageSummariesTestCases: IfetchAvailablePackageSummariesTe
},
{
name: "fetches packages from a repo (middle page)",
response: {
response: new GetAvailablePackageSummariesResponse({
availablePackageSummaries: [defaultAvailablePackageSummary],
nextPageToken,
categories: ["foo"],
},
}),
requestedRepos: repos,
requestedPageToken: currentPageToken,
expectedActions: [
Expand All @@ -179,11 +180,11 @@ const fetchAvailablePackageSummariesTestCases: IfetchAvailablePackageSummariesTe
},
{
name: "fetches packages from a repo (last page)",
response: {
response: new GetAvailablePackageSummariesResponse({
availablePackageSummaries: [defaultAvailablePackageSummary],
nextPageToken: "",
categories: ["foo"],
},
}),
requestedRepos: repos,
requestedPageToken: currentPageToken,
expectedActions: [
Expand All @@ -207,11 +208,11 @@ const fetchAvailablePackageSummariesTestCases: IfetchAvailablePackageSummariesTe
},
{
name: "fetches packages from a repo (already processed page)",
response: {
response: new GetAvailablePackageSummariesResponse({
availablePackageSummaries: [defaultAvailablePackageSummary],
nextPageToken,
categories: ["foo"],
},
}),
requestedRepos: repos,
requestedPageToken: currentPageToken,
expectedActions: [
Expand All @@ -235,11 +236,11 @@ const fetchAvailablePackageSummariesTestCases: IfetchAvailablePackageSummariesTe
},
{
name: "fetches packages from a repo (off-limits page)",
response: {
response: new GetAvailablePackageSummariesResponse({
availablePackageSummaries: [defaultAvailablePackageSummary],
nextPageToken: "3",
categories: ["foo"],
},
}),
requestedRepos: repos,
requestedPageToken: "next-page-token",
expectedActions: [
Expand Down Expand Up @@ -380,9 +381,9 @@ describe("fetchAvailablePackageSummaries", () => {

describe("fetchAvailablePackageVersions", () => {
const packageAppVersions = [{ pkgVersion: "1.2.3", appVersion: "4.5.6" }];
const availableVersionsResponse: GetAvailablePackageVersionsResponse = {
const availableVersionsResponse = new GetAvailablePackageVersionsResponse({
packageAppVersions,
};
});
let mockGetAvailablePackageVersions: jest.Mock;
beforeEach(() => {
mockGetAvailablePackageVersions = jest
Expand Down Expand Up @@ -422,9 +423,9 @@ describe("fetchAvailablePackageVersions", () => {
describe("fetchAndSelectAvailablePackageDetail", () => {
let mockGetAvailablePackageDetail: jest.Mock;
beforeEach(() => {
const response: GetAvailablePackageDetailResponse = {
const response = new GetAvailablePackageDetailResponse({
availablePackageDetail: defaultAvailablePackageDetail,
};
});
mockGetAvailablePackageDetail = jest.fn().mockImplementation(() => Promise.resolve(response));
jest
.spyOn(PackagesService, "getAvailablePackageDetail")
Expand Down
2 changes: 1 addition & 1 deletion dashboard/src/actions/availablepackages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
AvailablePackageDetail,
AvailablePackageReference,
GetAvailablePackageVersionsResponse,
} from "gen/kubeappsapis/core/packages/v1alpha1/packages";
} from "gen/kubeappsapis/core/packages/v1alpha1/packages_pb";
import { ThunkAction } from "redux-thunk";
import { ActionType, deprecated } from "typesafe-actions";
import PackagesService from "../shared/PackagesService";
Expand Down
25 changes: 13 additions & 12 deletions dashboard/src/actions/installedpackages.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import {
AvailablePackageDetail,
Context,
GetInstalledPackageSummariesResponse,
InstalledPackageDetail,
InstalledPackageReference,
Expand All @@ -11,8 +12,8 @@ import {
InstalledPackageSummary,
ReconciliationOptions,
VersionReference,
} from "gen/kubeappsapis/core/packages/v1alpha1/packages";
import { Plugin } from "gen/kubeappsapis/core/plugins/v1alpha1/plugins";
} from "gen/kubeappsapis/core/packages/v1alpha1/packages_pb";
import { Plugin } from "gen/kubeappsapis/core/plugins/v1alpha1/plugins_pb";
import { InstalledPackage } from "shared/InstalledPackage";
import { getStore, initialState } from "shared/specs/mountWrapper";
import { IStoreState, PluginNames, UnprocessableEntityError, UpgradeError } from "shared/types";
Expand Down Expand Up @@ -40,16 +41,16 @@ beforeEach(() => {
});

describe("fetches installed packages", () => {
const validInstalledPackageSummary: InstalledPackageSummary = {
installedPackageRef: {
context: { cluster: "second-cluster", namespace: "my-ns" },
const validInstalledPackageSummary = new InstalledPackageSummary({
installedPackageRef: new InstalledPackageReference({
context: new Context({ cluster: "second-cluster", namespace: "my-ns" }),
identifier: "some-name",
},
}),
iconUrl: "",
name: "foo",
pkgDisplayName: "foo",
shortDescription: "some description",
};
});
let requestInstalledPackageListMock: jest.Mock;
const installedPackageSummaries: InstalledPackageSummary[] = [validInstalledPackageSummary];
beforeEach(() => {
Expand Down Expand Up @@ -333,16 +334,16 @@ describe("rollbackInstalledPackage", () => {
);

it("success and re-request apps info", async () => {
const installedPackageDetail = {
const installedPackageDetail = new InstalledPackageDetail({
availablePackageRef: {
context: { cluster: "default", namespace: "my-ns" },
identifier: "test",
plugin: { name: PluginNames.PACKAGES_HELM, version: "0.0.1" } as Plugin,
},
currentVersion: { appVersion: "4.5.6", pkgVersion: "1.2.3" },
} as InstalledPackageDetail;
});

const availablePackageDetail = { name: "test" } as AvailablePackageDetail;
const availablePackageDetail = new AvailablePackageDetail({ name: "test" });

InstalledPackage.RollbackInstalledPackage = jest.fn().mockImplementationOnce(() => true);
InstalledPackage.GetInstalledPackageDetail = jest.fn().mockReturnValue({
Expand All @@ -352,7 +353,7 @@ describe("rollbackInstalledPackage", () => {
expect(res).toBe(true);

const selectCMD = actions.installedpackages.selectInstalledPackage(
installedPackageDetail as any,
installedPackageDetail,
availablePackageDetail,
);
const res2 = await store.dispatch(selectCMD);
Expand Down Expand Up @@ -410,7 +411,7 @@ describe("getInstalledPkgStatus", () => {
plugin: { name: "bad-plugin", version: "0.0.1" } as Plugin,
} as InstalledPackageReference;
const status = {
reason: InstalledPackageStatus_StatusReason.STATUS_REASON_INSTALLED,
reason: InstalledPackageStatus_StatusReason.INSTALLED,
} as InstalledPackageStatus;
const installedPackageDetail = { status } as InstalledPackageDetail;
InstalledPackage.GetInstalledPackageDetail = jest.fn().mockReturnValue({
Expand Down
2 changes: 1 addition & 1 deletion dashboard/src/actions/installedpackages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
InstalledPackageSummary,
ReconciliationOptions,
VersionReference,
} from "gen/kubeappsapis/core/packages/v1alpha1/packages";
} from "gen/kubeappsapis/core/packages/v1alpha1/packages_pb";
import { ThunkAction } from "redux-thunk";
import PackagesService from "shared/PackagesService";
import {
Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/actions/kube.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import {
InstalledPackageReference,
ResourceRef as APIResourceRef,
} from "gen/kubeappsapis/core/packages/v1alpha1/packages";
import { GetResourcesResponse } from "gen/kubeappsapis/plugins/resources/v1alpha1/resources";
} from "gen/kubeappsapis/core/packages/v1alpha1/packages_pb";
import { GetResourcesResponse } from "gen/kubeappsapis/plugins/resources/v1alpha1/resources_pb";
import configureMockStore from "redux-mock-store";
import thunk from "redux-thunk";
import { Kube } from "shared/Kube";
Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/actions/kube.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import { ActionType, deprecated } from "typesafe-actions";
import {
ResourceRef as APIResourceRef,
InstalledPackageReference,
} from "gen/kubeappsapis/core/packages/v1alpha1/packages";
import { GetResourcesResponse } from "gen/kubeappsapis/plugins/resources/v1alpha1/resources";
} from "gen/kubeappsapis/core/packages/v1alpha1/packages_pb";
import { GetResourcesResponse } from "gen/kubeappsapis/plugins/resources/v1alpha1/resources_pb";
import actions from "actions";
import { debounce } from "lodash";

Expand Down
Loading

0 comments on commit eb82234

Please sign in to comment.