Skip to content

Commit 03eaaa3

Browse files
Merge pull request #262 from mrc-ide/check-orderly-runner-enabled
If runner feature disabled, don't show 'runner' navlink or pages
2 parents 1ca780a + 730fd77 commit 03eaaa3

38 files changed

Lines changed: 549 additions & 100 deletions

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Before submitting a PR or set of changes to a PR, if any changes have been made to the front end, ensure linting and formatting are clean, e.g. with `npm run lint:fix --prefix=app` or `npm run format:write --prefix=app`. Don't make these pass by using comments to ignore linter warnings: actually fix the highlighted issues. No exceptions.

api/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ This API is built with [Spring boot framework](https://spring.io)
1010
## Run dependencies
1111

1212
Before you can start packit you need to run `./scripts/run-dependencies` from the project root
13-
to start database and `outpack_server` instances.
13+
to start database, `orderly.runner` server and worker, and `outpack_server` instances.
1414

1515
## Starting App
1616

api/app/src/main/kotlin/packit/controllers/RunnerController.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ import packit.service.RunnerService
1818
@RequestMapping("/runner")
1919
@PreAuthorize("hasAuthority('packet.run')")
2020
class RunnerController(private val runnerService: RunnerService) {
21+
@PreAuthorize("permitAll()")
22+
@GetMapping("/enabled")
23+
fun getEnabled(): ResponseEntity<Boolean> {
24+
return ResponseEntity.ok(runnerService.getEnabled())
25+
}
26+
2127
@GetMapping("/version")
2228
fun getVersion(): ResponseEntity<OrderlyRunnerVersion> {
2329
return ResponseEntity.ok(runnerService.getVersion())

api/app/src/main/kotlin/packit/security/WebSecurityConfig.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ class WebSecurityConfig(
123123
.requestMatchers("/auth/**", "/oauth2/**").permitAll()
124124
.requestMatchers("/deviceAuth", "/deviceAuth/token").permitAll()
125125
.requestMatchers("/branding/config").permitAll()
126+
.requestMatchers("/runner/enabled").permitAll()
126127
.dispatcherTypeMatchers(DispatcherType.FORWARD, DispatcherType.ERROR).permitAll()
127128
.anyRequest().authenticated()
128129
}

api/app/src/main/kotlin/packit/service/RunnerService.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import packit.model.dto.*
1414
import packit.repository.RunInfoRepository
1515

1616
interface RunnerService {
17+
fun getEnabled(): Boolean = true
1718
fun getVersion(): OrderlyRunnerVersion
1819
fun gitFetch()
1920
fun getBranches(): GitBranches
@@ -167,6 +168,7 @@ class DisabledRunnerService : RunnerService {
167168
throw PackitException("runnerDisabled", HttpStatus.FORBIDDEN)
168169
}
169170

171+
override fun getEnabled(): Boolean = false
170172
override fun getVersion(): OrderlyRunnerVersion = error()
171173
override fun gitFetch() = error()
172174
override fun getBranches(): GitBranches = error()

api/app/src/test/kotlin/packit/integration/controllers/RunnerControllerTest.kt

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,32 @@ class RunnerControllerTest : IntegrationTest() {
9696
userRepository.delete(testUser)
9797
}
9898

99+
@Test
100+
fun `reports enabled status when not logged in`() {
101+
val res: ResponseEntity<JsonNode> = restTemplate.exchange(
102+
"/runner/enabled",
103+
HttpMethod.GET,
104+
)
105+
106+
assertSuccess(res)
107+
108+
assertEquals(true, res.body!!.asBoolean())
109+
}
110+
111+
@Test
112+
@WithAuthenticatedUser(authorities = [])
113+
fun `reports enabled status when no packet run permision`() {
114+
val res: ResponseEntity<JsonNode> = restTemplate.exchange(
115+
"/runner/enabled",
116+
HttpMethod.GET,
117+
getTokenizedHttpEntity()
118+
)
119+
120+
assertSuccess(res)
121+
122+
assertEquals(true, res.body!!.asBoolean())
123+
}
124+
99125
@Test
100126
@WithAuthenticatedUser(authorities = ["packet.run"])
101127
fun `can get orderly runner version`() {
@@ -334,6 +360,32 @@ class UnknownRepoRunnerControllerTest : IntegrationTest() {
334360

335361
@TestPropertySource(properties = ["orderly.runner.enabled=false"])
336362
class DisabledRunnerControllerTest : IntegrationTest() {
363+
@Test
364+
fun `reports disabled status when not logged in`() {
365+
val res: ResponseEntity<JsonNode> = restTemplate.exchange(
366+
"/runner/enabled",
367+
HttpMethod.GET,
368+
)
369+
370+
assertSuccess(res)
371+
372+
assertEquals(false, res.body!!.asBoolean())
373+
}
374+
375+
@Test
376+
@WithAuthenticatedUser(authorities = [])
377+
fun `reports disabled status when no packet run permission`() {
378+
val res: ResponseEntity<JsonNode> = restTemplate.exchange(
379+
"/runner/enabled",
380+
HttpMethod.GET,
381+
getTokenizedHttpEntity()
382+
)
383+
384+
assertSuccess(res)
385+
386+
assertEquals(false, res.body!!.asBoolean())
387+
}
388+
337389
@Test
338390
@WithAuthenticatedUser(authorities = ["packet.run"])
339391
fun `cannot get orderly runner version`() {

api/app/src/test/kotlin/packit/unit/service/RunnerServiceTest.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ class RunnerServiceTest {
7676
userService
7777
)
7878

79+
@Test
80+
fun `can report enabled status`() {
81+
val result = sut.getEnabled()
82+
assertEquals(result, true)
83+
}
84+
7985
@Test
8086
fun `can get version`() {
8187
val result = sut.getVersion()

app/src/app/components/contents/runner/PacketRunnerLayout.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { SidebarItem } from "@lib/types/SidebarItem";
44
import { useUser } from "../../providers/UserProvider";
55
import { Sidebar } from "../common/Sidebar";
66
import { Unauthorized } from "../common/Unauthorized";
7+
import { useRunnerConfig } from "../../providers/RunnerConfigProvider";
78

89
const sidebarItems: SidebarItem[] = [
910
{
@@ -18,7 +19,9 @@ const sidebarItems: SidebarItem[] = [
1819

1920
export const PacketRunnerLayout = () => {
2021
const { authorities } = useUser();
21-
if (!hasPacketRunPermission(authorities)) {
22+
const isRunnerEnabled = useRunnerConfig();
23+
24+
if (!hasPacketRunPermission(authorities) || isRunnerEnabled === false) {
2225
return <Unauthorized />;
2326
}
2427
return (

app/src/app/components/header/NavMenu.tsx

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,35 +8,30 @@ import { DropdownMenu, DropdownMenuTrigger, DropdownMenuContent, DropdownMenuIte
88
import { Menu } from "lucide-react";
99
import { NavLink } from "react-router-dom";
1010
import { Button, buttonVariants } from "../Base/Button";
11-
11+
import { useRunnerConfig } from "../providers/RunnerConfigProvider";
1212
interface NavMenuProps extends React.HTMLAttributes<HTMLElement> {
1313
authorities: string[];
1414
}
1515
export const NavMenu = ({ className, authorities, ...props }: NavMenuProps) => {
16-
const NavItems: { [key: string]: string } = {
17-
runner: "Runner"
18-
// accessibility: "Accessibility",
19-
};
16+
const isRunnerEnabled = useRunnerConfig();
17+
const isLoading = isRunnerEnabled === null;
18+
19+
const NavItems: { [key: string]: string } = {};
2020

21-
const displayableItems = Object.keys(NavItems).filter((to) => {
22-
if (to === "runner" && !hasPacketRunPermission(authorities)) {
23-
return false;
24-
} else {
25-
return true;
26-
}
27-
});
21+
if (hasPacketRunPermission(authorities) && !isLoading && isRunnerEnabled) {
22+
NavItems.runner = "Runner";
23+
}
2824

2925
// Special case: route "Admin" to appropriate tab depending on user perms
3026
if (hasUserManagePermission(authorities) || hasGlobalPacketManagePermission(authorities)) {
3127
const key = hasUserManagePermission(authorities) ? "manage-roles" : "resync-packets";
3228
NavItems[key] = "Admin";
33-
displayableItems.push(key);
3429
}
3530

3631
return (
3732
<div className="flex-1">
3833
<nav className={cn("flex items-center space-x-2 pr-4 lg:pr-6 justify-end", className)} {...props}>
39-
{displayableItems.map((to) => (
34+
{Object.entries(NavItems).map(([to, label]) => (
4035
<NavLink
4136
to={to}
4237
key={to}
@@ -51,7 +46,7 @@ export const NavMenu = ({ className, authorities, ...props }: NavMenuProps) => {
5146
)
5247
}
5348
>
54-
{NavItems[to]}
49+
{label}
5550
</NavLink>
5651
))}
5752
</nav>
@@ -63,9 +58,9 @@ export const NavMenu = ({ className, authorities, ...props }: NavMenuProps) => {
6358
</Button>
6459
</DropdownMenuTrigger>
6560
<DropdownMenuContent className="w-48">
66-
{displayableItems.map((to) => (
61+
{Object.entries(NavItems).map(([to, label]) => (
6762
<DropdownMenuItem key={to} asChild>
68-
<NavLink to={to}>{NavItems[to]}</NavLink>
63+
<NavLink to={to}>{label}</NavLink>
6964
</DropdownMenuItem>
7065
))}
7166
</DropdownMenuContent>
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import useSWR from "swr";
2+
import appConfig from "@config/appConfig";
3+
import { fetcher } from "@lib/fetch";
4+
5+
export const useGetRunnerEnabled = (runnerEnabled: boolean | null) => {
6+
const { data, error, isLoading } = useSWR<boolean>(
7+
runnerEnabled === null ? `${appConfig.apiUrl()}/runner/enabled` : null,
8+
(url: string) => fetcher({ url, noAuth: true }),
9+
{ revalidateOnFocus: false }
10+
);
11+
12+
return {
13+
isRunnerEnabled: data,
14+
isLoading,
15+
error
16+
};
17+
};

0 commit comments

Comments
 (0)