Skip to content

Commit 2351d86

Browse files
committed
address review comments
1 parent 1d4e97a commit 2351d86

File tree

5 files changed

+63
-48
lines changed

5 files changed

+63
-48
lines changed

client/src/features/legacy/ProjectGitLabWarnBanner.tsx

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,7 @@ import { useGetProjectsByProjectIdSessionLaunchersQuery } from "~/features/sessi
2727
import { useGetUserQueryState } from "~/features/usersV2/api/users.api";
2828
import { Links } from "~/utils/constants/Docs";
2929

30-
import {
31-
DEFAULT_INTERNAL_GITLAB_HOSTS,
32-
doesProjectReferenceRenkulabGitLab,
33-
} from "./legacy.utils";
30+
import { doesProjectReferenceRenkulabGitLab } from "./legacy.utils";
3431

3532
function ProjectEditorWarnBanner() {
3633
return (
@@ -82,13 +79,7 @@ export default function ProjectGitLabWarnBanner({
8279
} = useGetProjectsByProjectIdSessionLaunchersQuery({ projectId: project.id });
8380
if (currentUser == null) return null;
8481
if (isLoadingLaunchers || launchersError || launchers == null) return null;
85-
if (
86-
!doesProjectReferenceRenkulabGitLab(
87-
project.repositories,
88-
launchers,
89-
DEFAULT_INTERNAL_GITLAB_HOSTS
90-
)
91-
)
82+
if (!doesProjectReferenceRenkulabGitLab(project.repositories, launchers))
9283
return null;
9384
return (
9485
<PermissionsGuard

client/src/features/legacy/RepositoryGitLabWarnBadge.tsx

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,7 @@ import useProjectPermissions from "~/features/ProjectPageV2/utils/useProjectPerm
2121
import type { Project } from "~/features/projectsV2/api/projectV2.api";
2222

2323
import InternalGitLabReferenceWarnBadge from "./InternalGitLabWarnBadge";
24-
import {
25-
DEFAULT_INTERNAL_GITLAB_HOSTS,
26-
doesProjectReferenceRenkulabGitLab,
27-
} from "./legacy.utils";
24+
import { doesProjectReferenceRenkulabGitLab } from "./legacy.utils";
2825

2926
interface RepositoryGitLabWarnBadgeProps {
3027
project: Project;
@@ -50,13 +47,7 @@ function RepositoryGitLabWarnBadgeForProject({
5047
export default function RepositoryGitLabWarnBadge({
5148
project,
5249
}: RepositoryGitLabWarnBadgeProps) {
53-
if (
54-
!doesProjectReferenceRenkulabGitLab(
55-
project.repositories,
56-
[],
57-
DEFAULT_INTERNAL_GITLAB_HOSTS
58-
)
59-
)
50+
if (!doesProjectReferenceRenkulabGitLab(project.repositories, []))
6051
return null;
6152

6253
return <RepositoryGitLabWarnBadgeForProject project={project} />;

client/src/features/legacy/SessionEnvironmentGitLabWarnBadge.tsx

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,7 @@ import useProjectPermissions from "~/features/ProjectPageV2/utils/useProjectPerm
2121
import { SessionLauncher } from "~/features/sessionsV2/api/sessionLaunchersV2.api";
2222

2323
import InternalGitLabReferenceWarnBadge from "./InternalGitLabWarnBadge";
24-
import {
25-
DEFAULT_INTERNAL_GITLAB_HOSTS,
26-
doesProjectReferenceRenkulabGitLab,
27-
} from "./legacy.utils";
24+
import { doesProjectReferenceRenkulabGitLab } from "./legacy.utils";
2825

2926
type SessionEnvironmentGitLabWarningBadgeForLauncherProps = {
3027
launcher: SessionLauncher;
@@ -54,14 +51,7 @@ export default function SessionEnvironmentGitLabWarningBadge({
5451
launcher,
5552
}: SessionEnvironmentGitLabWarningBadgeProps) {
5653
if (!launcher) return null;
57-
if (
58-
!doesProjectReferenceRenkulabGitLab(
59-
undefined,
60-
[launcher],
61-
DEFAULT_INTERNAL_GITLAB_HOSTS
62-
)
63-
)
64-
return null;
54+
if (!doesProjectReferenceRenkulabGitLab(undefined, [launcher])) return null;
6555
return (
6656
<SessionEnvironmentGitLabWarningBadgeForLauncher launcher={launcher} />
6757
);
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { describe, expect, it } from "vitest";
2+
3+
import { doesUrlHostMatchHost } from "./legacy.utils";
4+
5+
describe("doesUrlHostMatchHost", () => {
6+
it("should return true for matching hosts", () => {
7+
expect(
8+
doesUrlHostMatchHost("https://gitlab.renkulab.io", "gitlab.renkulab.io")
9+
).toBe(true);
10+
expect(
11+
doesUrlHostMatchHost(
12+
"registry.renkulab.io/ns/project:abc7ba5e",
13+
"registry.renkulab.io"
14+
)
15+
).toBe(true);
16+
});
17+
18+
it("should return false for non-matching hosts", () => {
19+
expect(
20+
doesUrlHostMatchHost("https://gitlab.renkulab.io", "gitlab.example.com")
21+
).toBe(false);
22+
expect(
23+
doesUrlHostMatchHost(
24+
"registry.renkulab.io/ns/project:abc7ba5e",
25+
"registry.example.io"
26+
)
27+
).toBe(false);
28+
});
29+
30+
it("should return false for invalid URLs", () => {
31+
expect(doesUrlHostMatchHost("invalid-url", "gitlab.renkulab.io")).toBe(
32+
false
33+
);
34+
});
35+
});

client/src/features/legacy/legacy.utils.ts

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,38 +19,46 @@
1919
import type { Project } from "~/features/projectsV2/api/projectV2.api";
2020
import type { SessionLaunchersList } from "~/features/sessionsV2/api/sessionLaunchersV2.api";
2121

22-
interface InternalGitLabHosts {
23-
repository: string;
24-
images: string;
25-
}
26-
27-
export const DEFAULT_INTERNAL_GITLAB_HOSTS: InternalGitLabHosts = {
22+
const DEFAULT_INTERNAL_GITLAB_HOSTS = {
2823
repository: "gitlab.renkulab.io",
2924
images: "registry.renkulab.io",
3025
};
3126

27+
// exported for testing
28+
export function doesUrlHostMatchHost(url: string, host: string) {
29+
const urlString = /^https?:\/\//.test(url) ? url : "http://" + url;
30+
try {
31+
const parsedUrl = new URL(urlString);
32+
return parsedUrl.host === host;
33+
} catch (e) {
34+
return false;
35+
}
36+
}
37+
3238
export function doesProjectReferenceRenkulabGitLab(
3339
allRepositories: Project["repositories"],
34-
allLaunchers: SessionLaunchersList,
35-
hosts: InternalGitLabHosts
40+
allLaunchers: SessionLaunchersList
3641
) {
3742
const { repositories, launchers } = projectReferencesToRenkulabGitLab(
3843
allRepositories,
39-
allLaunchers,
40-
hosts
44+
allLaunchers
4145
);
4246
return repositories.length > 0 || launchers.length > 0;
4347
}
4448

4549
function projectReferencesToRenkulabGitLab(
4650
allRepositories: Project["repositories"],
47-
allLaunchers: SessionLaunchersList,
48-
hosts: InternalGitLabHosts
51+
allLaunchers: SessionLaunchersList
4952
) {
53+
const hosts = DEFAULT_INTERNAL_GITLAB_HOSTS;
5054
const repositories =
51-
allRepositories?.filter((repo) => repo.includes(hosts.repository)) ?? [];
55+
allRepositories?.filter((repo) =>
56+
doesUrlHostMatchHost(repo, hosts.repository)
57+
) ?? [];
5258
const launchers = allLaunchers.filter((launcher) =>
53-
launcher.environment.container_image?.includes(hosts.images)
59+
launcher.environment.container_image == null
60+
? false
61+
: doesUrlHostMatchHost(launcher.environment.container_image, hosts.images)
5462
);
5563
return { repositories, launchers };
5664
}

0 commit comments

Comments
 (0)