Skip to content

Commit

Permalink
Catch forbidden errors when checking if OLM is installed (#1625)
Browse files Browse the repository at this point in the history
  • Loading branch information
Andres Martinez Gotor authored Apr 6, 2020
1 parent 823821d commit 6e22eac
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 38 deletions.
9 changes: 6 additions & 3 deletions dashboard/src/actions/operators.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,17 @@ describe("checkOLMInstalled", () => {
expect(store.getActions()).toEqual(expectedActions);
});

it("dispatches OLM_NOT_INSTALLED when failed", async () => {
Operators.isOLMInstalled = jest.fn(() => false);
it("dispatches error", async () => {
Operators.isOLMInstalled = jest.fn(() => {
throw new Error("nope");
});
const expectedActions = [
{
type: getType(operatorActions.checkingOLM),
},
{
type: getType(operatorActions.OLMNotInstalled),
type: getType(operatorActions.errorOperators),
payload: new Error("nope"),
},
];
await store.dispatch(operatorActions.checkOLMInstalled());
Expand Down
15 changes: 10 additions & 5 deletions dashboard/src/actions/operators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { IClusterServiceVersion, IPackageManifest, IResource, IStoreState } from

export const checkingOLM = createAction("CHECKING_OLM");
export const OLMInstalled = createAction("OLM_INSTALLED");
export const OLMNotInstalled = createAction("OLM_NOT_INSTALLED");

export const requestOperators = createAction("REQUEST_OPERATORS");
export const receiveOperators = createAction("RECEIVE_OPERATORS", resolve => {
Expand Down Expand Up @@ -77,7 +76,6 @@ export const receiveCustomResource = createAction("RECEIVE_CUSTOM_RESOURCE", res
const actions = [
checkingOLM,
OLMInstalled,
OLMNotInstalled,
requestOperators,
receiveOperators,
errorOperators,
Expand Down Expand Up @@ -114,9 +112,16 @@ export function checkOLMInstalled(): ThunkAction<
> {
return async dispatch => {
dispatch(checkingOLM());
const installed = await Operators.isOLMInstalled();
installed ? dispatch(OLMInstalled()) : dispatch(OLMNotInstalled());
return installed;
try {
const installed = await Operators.isOLMInstalled();
if (installed) {
dispatch(OLMInstalled());
}
return installed;
} catch (e) {
dispatch(errorOperators(e));
return false;
}
};
}

Expand Down
11 changes: 10 additions & 1 deletion dashboard/src/components/OperatorList/OperatorList.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { shallow } from "enzyme";
import * as React from "react";
import { IPackageManifest } from "shared/types";
import itBehavesLike from "../../shared/specs";
import { ForbiddenError, IPackageManifest } from "../../shared/types";
import { CardGrid } from "../Card";
import { ErrorSelector } from "../ErrorAlert";
import InfoCard from "../InfoCard";
Expand Down Expand Up @@ -78,6 +78,15 @@ it("call the OLM check and render the NotFound message if not found", () => {
expect(wrapper.find(OLMNotFound)).toExist();
});

it("call the OLM check and render the a Forbidden message if it could do the check", () => {
const checkOLMInstalled = jest.fn();
const wrapper = shallow(<OperatorList {...defaultProps} checkOLMInstalled={checkOLMInstalled} />);
wrapper.setProps({ error: new ForbiddenError("nope") });
expect(checkOLMInstalled).toHaveBeenCalled();
expect(wrapper.find(ErrorSelector)).toExist();
expect(wrapper.find(OLMNotFound)).not.toExist();
});

it("re-request operators if the namespace changes", () => {
const getOperators = jest.fn();
const getCSVs = jest.fn();
Expand Down
23 changes: 17 additions & 6 deletions dashboard/src/components/OperatorList/OperatorList.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { RouterAction } from "connected-react-router";
import * as React from "react";

import { IClusterServiceVersion, IPackageManifest } from "shared/types";
import { ForbiddenError, IClusterServiceVersion, IPackageManifest } from "../../shared/types";
import { api, app } from "../../shared/url";
import { escapeRegExp } from "../../shared/utils";
import { CardGrid } from "../Card";
Expand Down Expand Up @@ -55,7 +55,7 @@ class OperatorList extends React.Component<IOperatorListProps, IOperatorListStat
}

public render() {
const { isFetching, isOLMInstalled, pushSearchFilter } = this.props;
const { isFetching, pushSearchFilter } = this.props;
return (
<div>
<PageHeader>
Expand All @@ -78,17 +78,28 @@ class OperatorList extends React.Component<IOperatorListProps, IOperatorListStat
</a>
</div>
</MessageAlert>
<LoadingWrapper loaded={!isFetching}>
{isOLMInstalled ? this.renderOperators() : <OLMNotFound />}
</LoadingWrapper>
<LoadingWrapper loaded={!isFetching}>{this.renderOperators()}</LoadingWrapper>
</main>
</div>
);
}

private renderOperators() {
const { operators, error, csvs } = this.props;
const { operators, error, csvs, isOLMInstalled } = this.props;
const { filter } = this.state;
if (error && error.constructor === ForbiddenError) {
return (
<ErrorSelector
error={error}
action="list"
resource="Operators"
namespace={this.props.namespace}
/>
);
}
if (!isOLMInstalled) {
return <OLMNotFound />;
}
if (error) {
return (
<ErrorSelector
Expand Down
13 changes: 0 additions & 13 deletions dashboard/src/reducers/operators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ describe("catalogReducer", () => {
const actionTypes = {
checkingOLM: getType(actions.operators.checkingOLM),
OLMInstalled: getType(actions.operators.OLMInstalled),
OLMNotInstalled: getType(actions.operators.OLMNotInstalled),
requestOperators: getType(actions.operators.requestOperators),
receiveOperators: getType(actions.operators.receiveOperators),
errorOperators: getType(actions.operators.errorOperators),
Expand Down Expand Up @@ -68,18 +67,6 @@ describe("catalogReducer", () => {
).toEqual({ ...initialState, isOLMInstalled: true });
});

it("unsets isFetching and mark OLM as not installed", () => {
const state = operatorReducer(undefined, {
type: actionTypes.checkingOLM as any,
});
expect(state).toEqual({ ...initialState, isFetching: true });
expect(
operatorReducer(undefined, {
type: actionTypes.OLMNotInstalled as any,
}),
).toEqual({ ...initialState, isOLMInstalled: false });
});

it("sets receive operators", () => {
const state = operatorReducer(undefined, {
type: actionTypes.requestOperators as any,
Expand Down
4 changes: 1 addition & 3 deletions dashboard/src/reducers/operators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ const catalogReducer = (
case getType(operators.checkingOLM):
return { ...state, isFetching: true };
case getType(operators.OLMInstalled):
return { ...state, isOLMInstalled: true, isFetching: false };
case getType(operators.OLMNotInstalled):
return { ...state, isOLMInstalled: false, isFetching: false };
return { ...state, isOLMInstalled: true };
case getType(operators.requestOperators):
return { ...state, isFetching: true };
case getType(operators.receiveOperators):
Expand Down
2 changes: 1 addition & 1 deletion dashboard/src/shared/Operators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ it("check if the OLM has been installed", async () => {

it("OLM is not installed if the request fails", async () => {
axiosWithAuth.get = jest.fn(() => {
throw new Error("nope");
return { status: 404 };
});
expect(await Operators.isOLMInstalled()).toBe(false);
});
Expand Down
8 changes: 2 additions & 6 deletions dashboard/src/shared/Operators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,8 @@ import { IClusterServiceVersion, IK8sList, IPackageManifest, IResource } from ".

export class Operators {
public static async isOLMInstalled() {
try {
const { status } = await axiosWithAuth.get(urls.api.operators.crd);
return status === 200;
} catch (err) {
return false;
}
const { status } = await axiosWithAuth.get(urls.api.operators.crd);
return status === 200;
}

public static async getOperators(namespace: string) {
Expand Down

0 comments on commit 6e22eac

Please sign in to comment.