Skip to content

Commit

Permalink
Refactor default namespace resolution (#2161)
Browse files Browse the repository at this point in the history
  • Loading branch information
Andres Martinez Gotor committed Nov 16, 2020
1 parent 59553bd commit c295043
Show file tree
Hide file tree
Showing 23 changed files with 261 additions and 367 deletions.
45 changes: 3 additions & 42 deletions dashboard/src/actions/auth.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { IAuthState } from "reducers/auth";
import configureMockStore from "redux-mock-store";
import thunk from "redux-thunk";
import { IResource } from "shared/types";
import { getType } from "typesafe-actions";
import actions from ".";
import { Auth } from "../shared/Auth";
Expand All @@ -20,7 +19,6 @@ beforeEach(() => {
authenticated: false,
authenticating: false,
oidcAuthenticated: false,
defaultNamespace: "_all",
};

Auth.validateToken = jest.fn();
Expand Down Expand Up @@ -72,11 +70,7 @@ describe("authenticate", () => {
type: getType(actions.auth.authenticating),
},
{
type: getType(actions.namespace.receiveNamespaces),
payload: { cluster: "default", namespaces: [] },
},
{
payload: { authenticated: true, oidc: false, defaultNamespace: "_all" },
payload: { authenticated: true, oidc: false },
type: getType(actions.auth.setAuthenticated),
},
];
Expand All @@ -94,11 +88,7 @@ describe("authenticate", () => {
type: getType(actions.auth.authenticating),
},
{
type: getType(actions.namespace.receiveNamespaces),
payload: { cluster: "default", namespaces: [] },
},
{
payload: { authenticated: true, oidc: true, defaultNamespace: "_all" },
payload: { authenticated: true, oidc: true },
type: getType(actions.auth.setAuthenticated),
},
{
Expand All @@ -112,31 +102,6 @@ describe("authenticate", () => {
expect(Auth.validateToken).not.toHaveBeenCalled();
});
});

it("uses a namespace from the list", () => {
Auth.validateToken = jest.fn();
Namespace.list = async () => {
return { namespaces: [{ metadata: { name: "foo" } } as IResource] };
};
const expectedActions = [
{
type: getType(actions.auth.authenticating),
},
{
type: getType(actions.namespace.receiveNamespaces),
payload: { cluster: "default", namespaces: ["foo"] },
},
{
payload: { authenticated: true, oidc: false, defaultNamespace: "foo" },
type: getType(actions.auth.setAuthenticated),
},
];

return store.dispatch(actions.auth.authenticate(defaultCluster, token, false)).then(() => {
expect(store.getActions()).toEqual(expectedActions);
expect(Auth.validateToken).toHaveBeenCalledWith(defaultCluster, token);
});
});
});

describe("OIDC authentication", () => {
Expand All @@ -150,11 +115,7 @@ describe("OIDC authentication", () => {
type: getType(actions.auth.authenticating),
},
{
type: getType(actions.namespace.receiveNamespaces),
payload: { cluster: "default", namespaces: [] },
},
{
payload: { authenticated: true, oidc: true, defaultNamespace: "_all" },
payload: { authenticated: true, oidc: true },
type: getType(actions.auth.setAuthenticated),
},
{
Expand Down
24 changes: 5 additions & 19 deletions dashboard/src/actions/auth.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import { ThunkAction } from "redux-thunk";
import { definedNamespaces } from "shared/Namespace";
import { ActionType, createAction } from "typesafe-actions";

import { Auth } from "../shared/Auth";
import { IStoreState } from "../shared/types";
import { clearClusters, fetchNamespaces, NamespaceAction } from "./namespace";
import { clearClusters, NamespaceAction } from "./namespace";

export const setAuthenticated = createAction("SET_AUTHENTICATED", resolve => {
return (authenticated: boolean, oidc: boolean, defaultNamespace: string) =>
resolve({ authenticated, oidc, defaultNamespace });
return (authenticated: boolean, oidc: boolean) => resolve({ authenticated, oidc });
});

export const authenticating = createAction("AUTHENTICATING");
Expand Down Expand Up @@ -37,19 +35,7 @@ export function authenticate(
await Auth.validateToken(cluster, token);
}
Auth.setAuthToken(token, oidc);
// TODO(andresmgot): This is a workaround while #2018 gets properly implemented.
// If the current sa is not associated to a namespace, list all namespaces and pick
// one.
let ns = Auth.defaultNamespaceFromToken(token);
if (!ns) {
const availableNamespaces = await dispatch(fetchNamespaces(cluster));
if (availableNamespaces.length) {
ns = availableNamespaces[0];
} else {
ns = definedNamespaces.all;
}
}
dispatch(setAuthenticated(true, oidc, ns));
dispatch(setAuthenticated(true, oidc));
if (oidc) {
dispatch(setSessionExpired(false));
}
Expand All @@ -73,7 +59,7 @@ export function logout(): ThunkAction<
Auth.unsetAuthCookie(config);
} else {
Auth.unsetAuthToken();
dispatch(setAuthenticated(false, false, ""));
dispatch(setAuthenticated(false, false));
dispatch(clearClusters());
}
};
Expand All @@ -100,7 +86,7 @@ export function checkCookieAuthentication(
if (isAuthed) {
await dispatch(authenticate(cluster, "", true));
} else {
dispatch(setAuthenticated(false, false, ""));
dispatch(setAuthenticated(false, false));
}
return isAuthed;
};
Expand Down
2 changes: 1 addition & 1 deletion dashboard/src/components/Header/ContextSelector.scss
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
font-weight: 500;
}

.dropdown-menu {
.header-actions .dropdown-menu {
background-color: #25333d;
width: 15rem;
border: 0px;
Expand Down
5 changes: 1 addition & 4 deletions dashboard/src/components/Header/ContextSelector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,11 @@ afterEach(() => {
spyOnUseDispatch.mockRestore();
});

it("fetches namespaces", () => {
const fetchNamespaces = jest.fn();
it("gets a namespace", () => {
const getNamespace = jest.fn();
actions.namespace.fetchNamespaces = fetchNamespaces;
actions.namespace.getNamespace = getNamespace;
mountWrapper(defaultStore, <ContextSelector />);

expect(fetchNamespaces).toHaveBeenCalled();
expect(getNamespace).toHaveBeenCalledWith(
initialState.clusters.currentCluster,
initialState.clusters.clusters[initialState.clusters.currentCluster].currentNamespace,
Expand Down
10 changes: 3 additions & 7 deletions dashboard/src/components/Header/ContextSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,9 @@ import "./ContextSelector.css";

function ContextSelector() {
const dispatch: ThunkDispatch<IStoreState, null, Action> = useDispatch();
const {
clusters,
auth: { defaultNamespace },
} = useSelector((state: IStoreState) => state);
const { clusters } = useSelector((state: IStoreState) => state);
const currentCluster = clusters.clusters[clusters.currentCluster];
const namespaceSelected = currentCluster.currentNamespace || defaultNamespace;
const namespaceSelected = currentCluster.currentNamespace;
const error = currentCluster.error;
const [open, setOpen] = useState(false);
const [cluster, setStateCluster] = useState(clusters.currentCluster);
Expand All @@ -38,8 +35,7 @@ function ContextSelector() {
useOutsideClick(setOpen, [ref], open);

useEffect(() => {
dispatch(actions.namespace.fetchNamespaces(clusters.currentCluster));
if (namespaceSelected !== definedNamespaces.all) {
if (namespaceSelected && namespaceSelected !== definedNamespaces.all) {
dispatch(actions.namespace.getNamespace(clusters.currentCluster, namespaceSelected));
}
}, [dispatch, namespaceSelected, clusters.currentCluster]);
Expand Down
70 changes: 53 additions & 17 deletions dashboard/src/components/Header/Header.test.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,28 @@
import { shallow } from "enzyme";
import actions from "actions";
import * as React from "react";
import { IClustersState } from "../../reducers/cluster";
import * as ReactRedux from "react-redux";
import { NavLink } from "react-router-dom";
import { getStore, mountWrapper } from "shared/specs/mountWrapper";
import { app } from "../../shared/url";
import Header from "./Header";

const defaultProps = {
authenticated: true,
fetchNamespaces: jest.fn(),
logout: jest.fn(),
let spyOnUseDispatch: jest.SpyInstance;
const kubeaActions = { ...actions.namespace };
beforeEach(() => {
actions.namespace = {
...actions.namespace,
fetchNamespaces: jest.fn(),
};
const mockDispatch = jest.fn(res => res);
spyOnUseDispatch = jest.spyOn(ReactRedux, "useDispatch").mockReturnValue(mockDispatch);
});

afterEach(() => {
actions.namespace = { ...kubeaActions };
spyOnUseDispatch.mockRestore();
});

const defaultState = {
clusters: {
currentCluster: "default",
clusters: {
Expand All @@ -16,19 +31,14 @@ const defaultProps = {
namespaces: ["default", "other"],
},
},
} as IClustersState,
defaultNamespace: "kubeapps-user",
pathname: "",
push: jest.fn(),
setNamespace: jest.fn(),
createNamespace: jest.fn(),
getNamespace: jest.fn(),
appVersion: "v2.0.0",
},
auth: { authenticated: true },
config: { appVersion: "v2.0.0" },
};

it("renders the header links and titles", () => {
const wrapper = shallow(<Header {...defaultProps} />);
const items = wrapper.find(".nav-link");
const wrapper = mountWrapper(getStore(defaultState), <Header />);
const items = wrapper.find(".header-nav").find(NavLink);
const expectedItems = [
{ children: "Applications", to: app.apps.list("default", "default") },
{ children: "Catalog", to: app.catalog("default", "default") },
Expand All @@ -41,7 +51,33 @@ it("renders the header links and titles", () => {
});

it("should skip the links if it's not authenticated", () => {
const wrapper = shallow(<Header {...defaultProps} authenticated={false} />);
const wrapper = mountWrapper(
getStore({
...defaultState,
auth: { authenticated: false },
}),
<Header />,
);
const items = wrapper.find(".nav-link");
expect(items).not.toExist();
});

it("should skip the links if the namespace info is not available", () => {
const wrapper = mountWrapper(
getStore({
...defaultState,
clusters: {
currentCluster: "default",
clusters: {
default: {
currentNamespace: "",
namespaces: [],
},
},
},
}),
<Header />,
);
const items = wrapper.find(".nav-link");
expect(items).not.toExist();
});
41 changes: 22 additions & 19 deletions dashboard/src/components/Header/Header.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,33 @@
import * as React from "react";
import actions from "actions";
import React, { useEffect } from "react";
import { useDispatch, useSelector } from "react-redux";
import { NavLink } from "react-router-dom";
import { Action } from "redux";
import { ThunkDispatch } from "redux-thunk";
import { IStoreState } from "shared/types";

import logo from "../../logo.svg";
import { IClustersState } from "../../reducers/cluster";
import { app } from "../../shared/url";
import ContextSelector from "./ContextSelector";
import "./Header.css";
import Menu from "./Menu";

interface IHeaderProps {
authenticated: boolean;
logout: () => void;
clusters: IClustersState;
defaultNamespace: string;
appVersion: string;
push: (path: string) => void;
}

function Header(props: IHeaderProps) {
const { appVersion, clusters, authenticated: showNav, defaultNamespace, logout } = props;
function Header() {
const dispatch: ThunkDispatch<IStoreState, null, Action> = useDispatch();
const {
auth: { authenticated },
clusters,
config: { appVersion },
} = useSelector((state: IStoreState) => state);
const cluster = clusters.clusters[clusters.currentCluster];
const showNav = authenticated && clusters.currentCluster && cluster.currentNamespace;
const logout = () => dispatch(actions.auth.logout());

useEffect(() => {
if (authenticated && clusters.currentCluster) {
dispatch(actions.namespace.fetchNamespaces(clusters.currentCluster));
}
}, [dispatch, authenticated, clusters.currentCluster]);

const routesToRender = [
{
Expand Down Expand Up @@ -62,12 +70,7 @@ function Header(props: IHeaderProps) {
{showNav && (
<section className="header-actions">
<ContextSelector />
<Menu
clusters={clusters}
defaultNamespace={defaultNamespace}
appVersion={appVersion}
logout={logout}
/>
<Menu clusters={clusters} appVersion={appVersion} logout={logout} />
</section>
)}
</header>
Expand Down
5 changes: 2 additions & 3 deletions dashboard/src/components/Header/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,14 @@ import "./Menu.css";

export interface IContextSelectorProps {
clusters: IClustersState;
defaultNamespace: string;
appVersion: string;
logout: () => void;
}

function Menu({ clusters, defaultNamespace, appVersion, logout }: IContextSelectorProps) {
function Menu({ clusters, appVersion, logout }: IContextSelectorProps) {
const [open, setOpen] = useState(false);
const currentCluster = clusters.clusters[clusters.currentCluster];
const namespaceSelected = currentCluster.currentNamespace || defaultNamespace;
const namespaceSelected = currentCluster.currentNamespace;
// Control when users click outside
const ref = useRef(null);
useOutsideClick(setOpen, [ref], open);
Expand Down

0 comments on commit c295043

Please sign in to comment.