Skip to content

Commit

Permalink
AppRepositories list uses namespaced URI (#1502)
Browse files Browse the repository at this point in the history
  • Loading branch information
absoludity authored Feb 5, 2020
1 parent 0283bc4 commit 9cac48d
Show file tree
Hide file tree
Showing 14 changed files with 163 additions and 21 deletions.
3 changes: 2 additions & 1 deletion chart/kubeapps/templates/dashboard-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,6 @@ data:
"appVersion": "{{ .Chart.AppVersion }}",
"authProxyEnabled": {{ .Values.authProxy.enabled }},
"oauthLoginURI": "/oauth2/start",
"oauthLogoutURI": "/oauth2/sign_out"
"oauthLogoutURI": "/oauth2/sign_out",
"featureFlags": {{ .Values.featureFlags | toJson }}
}
5 changes: 4 additions & 1 deletion dashboard/public/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,8 @@
"appVersion": "DEVEL",
"authProxyEnabled": false,
"oauthLoginURI": "/oauth2/start",
"oauthLogoutURI": "/oauth2/sign_out"
"oauthLogoutURI": "/oauth2/sign_out",
"featureFlags": {
"reposPerNamespace": false
}
}
36 changes: 34 additions & 2 deletions dashboard/src/components/Header/Header.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ const defaultProps = {
fetchNamespaces: jest.fn(),
logout: jest.fn(),
namespace: {
current: "",
namespaces: [],
current: "default",
namespaces: ["default", "other"],
} as INamespaceState,
defaultNamespace: "kubeapps-user",
pathname: "",
Expand All @@ -34,6 +34,38 @@ it("renders the header links and titles", () => {
});
});

describe("settings", () => {
it("renders settings without reposPerNamespace", () => {
const wrapper = shallow(<Header {...defaultProps} />);
const settingsbar = wrapper.find(".header__nav__submenu").first();
const items = settingsbar.find("NavLink").map(p => p.props());
const expectedItems = [
{ children: "App Repositories", to: "/config/repos" },
{ children: "Service Brokers", to: "/config/brokers" },
];
items.forEach((item, index) => {
expect(item.children).toBe(expectedItems[index].children);
expect(item.to).toBe(expectedItems[index].to);
});
});

it("renders settings with reposPerNamespace", () => {
const wrapper = shallow(
<Header {...defaultProps} featureFlags={{ reposPerNamespace: true }} />,
);
const settingsbar = wrapper.find(".header__nav__submenu").first();
const items = settingsbar.find("NavLink").map(p => p.props());
const expectedItems = [
{ children: "App Repositories", to: "/config/ns/default/repos" },
{ children: "Service Brokers", to: "/config/brokers" },
];
items.forEach((item, index) => {
expect(item.children).toBe(expectedItems[index].children);
expect(item.to).toBe(expectedItems[index].to);
});
});
});

it("updates state when the path changes", () => {
const wrapper = shallow(<Header {...defaultProps} />);
wrapper.setState({ configOpen: true, mobileOpne: true });
Expand Down
10 changes: 9 additions & 1 deletion dashboard/src/components/Header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ interface IHeaderProps {
setNamespace: (ns: string) => void;
createNamespace: (ns: string) => Promise<boolean>;
getNamespace: (ns: string) => void;
featureFlags: { reposPerNamespace: boolean };
}

interface IHeaderState {
Expand All @@ -32,6 +33,10 @@ interface IHeaderState {
}

class Header extends React.Component<IHeaderProps, IHeaderState> {
public static defaultProps = {
featureFlags: { reposPerNamespace: false },
};

// Defines the route
private readonly links: IHeaderLinkProps[] = [
{
Expand Down Expand Up @@ -83,6 +88,9 @@ class Header extends React.Component<IHeaderProps, IHeaderState> {
this.state.configOpen ? "header__nav__submenu-open" : ""
}`;

const reposPath = this.props.featureFlags.reposPerNamespace
? `/config/ns/${namespace.current}/repos`
: "/config/repos";
return (
<section className="gradient-135-brand type-color-reverse type-color-reverse-anchor-reset">
<div className="container">
Expand Down Expand Up @@ -135,7 +143,7 @@ class Header extends React.Component<IHeaderProps, IHeaderState> {
</a>
<ul role="menu" aria-label="Products" className={submenu}>
<li role="none">
<NavLink to="/config/repos">App Repositories</NavLink>
<NavLink to={reposPath}>App Repositories</NavLink>
</li>
<li role="none">
<NavLink to="/config/brokers">Service Brokers</NavLink>
Expand Down
43 changes: 32 additions & 11 deletions dashboard/src/containers/HeaderContainer/HeaderContainer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,45 @@ const emptyLocation = {
search: "",
};

const makeStore = (authenticated: boolean, oidcAuthenticated: boolean) => {
const state: IAuthState = {
sessionExpired: false,
authenticated,
oidcAuthenticated,
authenticating: false,
defaultNamespace: "",
};
return mockStore({ auth: state, router: { location: emptyLocation } });
const defaultAuthState: IAuthState = {
sessionExpired: false,
authenticated: true,
oidcAuthenticated: true,
authenticating: false,
defaultNamespace: "",
};

describe("LoginFormContainer props", () => {
const defaultState = {
auth: defaultAuthState,
router: { location: emptyLocation },
config: {
featureFlags: { reposPerNamespace: true },
},
};

describe("HeaderContainer props", () => {
it("maps authentication redux states to props", () => {
const store = makeStore(true, true);
const store = mockStore(defaultState);
const wrapper = shallow(<Header store={store} />);
const form = wrapper.find("Header");
expect(form).toHaveProp({
authenticated: true,
});
});

it("maps featureFlags configuration to props", () => {
const store = mockStore({
...defaultState,
config: {
featureFlags: { reposPerNamespace: true },
},
});

const wrapper = shallow(<Header store={store} />);

const form = wrapper.find("Header");
expect(form).toHaveProp({
featureFlags: { reposPerNamespace: true },
});
});
});
2 changes: 2 additions & 0 deletions dashboard/src/containers/HeaderContainer/HeaderContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ function mapStateToProps({
router: {
location: { pathname },
},
config: { featureFlags },
}: IState) {
return {
authenticated,
namespace,
defaultNamespace,
pathname,
featureFlags,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const makeStore = (
namespace: "",
appVersion: "",
oauthLogoutURI: "",
featureFlags: { reposPerNamespace: false },
};
return mockStore({ auth, config });
};
Expand Down
51 changes: 50 additions & 1 deletion dashboard/src/containers/RoutesContainer/Routes.test.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { mount } from "enzyme";
import { mount, shallow } from "enzyme";
import { createMemoryHistory } from "history";
import * as React from "react";
import { Route, StaticRouter } from "react-router";
import { RouteComponentProps } from "react-router-dom";

import NotFound from "../../components/NotFound";
import RepoListContainer from "../../containers/RepoListContainer";
import Routes from "./Routes";

const emptyRouteComponentProps: RouteComponentProps<{}> = {
Expand Down Expand Up @@ -77,3 +78,51 @@ it("should render a redirect to the login page (when not authenticated)", () =>
.render().props.to,
).toEqual("/login");
});

describe("Routes depending on feature flags", () => {
const namespace = "default";
const perNamespacePath = "/config/ns/:namespace/repos";
const nonNamespacedPath = "/config/repos";

it("should use a non-namespaced route for app repos without feature flag", () => {
const wrapper = shallow(
<StaticRouter location="/config/repos" context={{}}>
<Routes
{...emptyRouteComponentProps}
namespace={namespace}
authenticated={true}
featureFlags={{ reposPerNamespace: false }}
/>
</StaticRouter>,
)
.dive()
.dive()
.dive();

const component = wrapper.find({ component: RepoListContainer });

expect(component.length).toBe(1);
expect(component.props().path).toEqual(nonNamespacedPath);
});

it("should use a namespaced route for app repos when feature flag set", () => {
const wrapper = shallow(
<StaticRouter location="/config/repos" context={{}}>
<Routes
{...emptyRouteComponentProps}
namespace={namespace}
authenticated={true}
featureFlags={{ reposPerNamespace: true }}
/>
</StaticRouter>,
)
.dive()
.dive()
.dive();

const component = wrapper.find({ component: RepoListContainer });

expect(component.length).toBe(1);
expect(component.props().path).toEqual(perNamespacePath);
});
});
19 changes: 18 additions & 1 deletion dashboard/src/containers/RoutesContainer/Routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ const privateRoutes = {
"/charts/:repo/:id": ChartViewContainer,
"/charts/:repo/:id/versions/:version": ChartViewContainer,
"/config/brokers": ServiceBrokerListContainer,
"/config/repos": RepoListContainer,
"/services/brokers/:brokerName/classes/:className": ServiceClassViewContainer,
"/services/brokers/:brokerName/instances/ns/:namespace/:instanceName": ServiceInstanceViewContainer,
"/services/classes": ServiceClassListContainer,
Expand All @@ -44,10 +43,22 @@ const routes = {
interface IRoutesProps extends IRouteComponentPropsAndRouteProps {
namespace: string;
authenticated: boolean;
featureFlags: {
reposPerNamespace: boolean;
};
}

class Routes extends React.Component<IRoutesProps> {
public static defaultProps = {
featureFlags: { reposPerNamespace: false },
};
public render() {
// The path used for AppRepository list depends on a feature flag.
// TODO(mnelson, #1256) Remove when feature becomes default.
const reposPath = this.props.featureFlags.reposPerNamespace
? "/config/ns/:namespace/repos"
: "/config/repos";

return (
<Switch>
<Route exact={true} path="/" render={this.rootNamespacedRedirect} />
Expand All @@ -57,6 +68,12 @@ class Routes extends React.Component<IRoutesProps> {
{Object.entries(privateRoutes).map(([route, component]) => (
<PrivateRouteContainer key={route} exact={true} path={route} component={component} />
))}
<PrivateRouteContainer
key={reposPath}
exact={true}
path={reposPath}
component={RepoListContainer}
/>
{/* If the route doesn't match any expected path redirect to a 404 page */}
<Route component={NotFound} />
</Switch>
Expand Down
3 changes: 2 additions & 1 deletion dashboard/src/containers/RoutesContainer/RoutesContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import { withRouter } from "react-router";
import { IStoreState } from "../../shared/types";
import Routes from "./Routes";

function mapStateToProps({ auth, namespace }: IStoreState) {
function mapStateToProps({ auth, namespace, config }: IStoreState) {
return {
namespace: namespace.current || auth.defaultNamespace,
authenticated: auth.authenticated,
featureFlags: config.featureFlags,
};
}

Expand Down
1 change: 1 addition & 0 deletions dashboard/src/reducers/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const initialState: IConfigState = {
authProxyEnabled: false,
oauthLoginURI: "",
oauthLogoutURI: "",
featureFlags: { reposPerNamespace: true },
};

const configReducer = (state: IConfigState = initialState, action: ConfigAction): IConfigState => {
Expand Down
2 changes: 2 additions & 0 deletions dashboard/src/shared/Auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ describe("Auth", () => {
oauthLogoutURI,
namespace: "ns",
appVersion: "2",
featureFlags: { reposPerNamespace: false },
});

expect(mockedAssign).toBeCalledWith(oauthLogoutURI);
Expand All @@ -184,6 +185,7 @@ describe("Auth", () => {
oauthLogoutURI: "",
namespace: "ns",
appVersion: "2",
featureFlags: { reposPerNamespace: false },
});

expect(mockedAssign).toBeCalledWith("/oauth2/sign_out");
Expand Down
3 changes: 3 additions & 0 deletions dashboard/src/shared/Config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ export interface IConfig {
oauthLoginURI: string;
oauthLogoutURI: string;
error?: Error;
featureFlags: {
reposPerNamespace: boolean;
};
}

export default class Config {
Expand Down
5 changes: 3 additions & 2 deletions script/deploy-dev.mk
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ deploy-dev: deploy-dex deploy-openldap update-apiserver-etc-hosts
--values ./docs/user/manifests/kubeapps-local-dev-auth-proxy-values.yaml \
--set useHelm3=true \
--set postgresql.enabled=true \
--set featureFlags.reposPerNamespace=true \
--set mongodb.enabled=false
kubectl apply -f ./docs/user/manifests/kubeapps-local-dev-users-rbac.yaml
@echo "\nEnsure you have the entry '127.0.0.1 dex.dex' in your /etc/hosts, then run\n"
Expand All @@ -54,8 +55,8 @@ reset-dev:
helm -n dex delete dex || true
helm -n ldap delete ldap || true
# In case helm installations fail, still delete non-namespaced resources.
kubectl delete clusterrole dex kubeapps:controller:apprepository-reader || true
kubectl delete clusterrolebinding dex kubeapps:controller:apprepository-reader || true
kubectl delete clusterrole dex kubeapps:controller:apprepository-reader-kubeapps || true
kubectl delete clusterrolebinding dex kubeapps:controller:apprepository-reader-kubeapps || true
kubectl delete namespace --wait dex ldap kubeapps || true
kubectl delete --wait -f ./docs/user/manifests/kubeapps-local-dev-users-rbac.yaml || true

Expand Down

0 comments on commit 9cac48d

Please sign in to comment.