Skip to content

Commit 3de8e24

Browse files
skkra0skkra0
authored
Merge pull request #32 from PurdueRCAC/fix-diff
EnvVarGrid fixes
2 parents 9c00187 + 281f9bd commit 3de8e24

8 files changed

Lines changed: 132 additions & 162 deletions

File tree

‎builders/dockerfile/Dockerfile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ RUN rm -f /etc/apt/apt.conf.d/docker-clean; echo 'Binary::apt::APT::Keep-Downloa
55
RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
66
--mount=type=cache,target=/var/lib/apt,sharing=locked \
77
apt-get update && \
8-
apt-get install -y --no-install-recommends software-properties-common=0.99.49.3 wget=1.21.4-1ubuntu4.1 libssl3=3.0.13-0ubuntu3.6 ca-certificates=20240203 && \
8+
apt-get install -y --no-install-recommends software-properties-common=0.99.49.3 wget=1.21.4-1ubuntu4.1 libssl3=3.0.13-0ubuntu3.7 ca-certificates=20240203 && \
99
# ^ This package includes `add-apt-repository`
1010
add-apt-repository ppa:git-core/ppa && \
1111
apt-get update && \
1212
# ^ Get the latest version of Git from their PPA (we need 2.49.0 or later because we use the `git clone --revision` flag)
13-
apt-get install -y --no-install-recommends git=1:2.52.0-0ppa1~ubuntu24.04.3 && \
13+
apt-get install -y --no-install-recommends git=1:2.53.0-0ppa1~ubuntu24.04.1 && \
1414
apt-get remove -y software-properties-common && \
1515
apt-get autoremove -y
1616

‎builders/railpack/Dockerfile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ RUN rm -f /etc/apt/apt.conf.d/docker-clean; echo 'Binary::apt::APT::Keep-Downloa
77
RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
88
--mount=type=cache,target=/var/lib/apt,sharing=locked \
99
apt-get update && \
10-
apt-get install -y --no-install-recommends software-properties-common=0.99.49.3 wget=1.21.4-1ubuntu4.1 libssl3=3.0.13-0ubuntu3.6 ca-certificates=20240203 jq=1.7.1-3ubuntu0.24.04.1 && \
10+
apt-get install -y --no-install-recommends software-properties-common=0.99.49.3 wget=1.21.4-1ubuntu4.1 libssl3=3.0.13-0ubuntu3.7 ca-certificates=20240203 jq=1.7.1-3ubuntu0.24.04.1 && \
1111
# ^ This package includes `add-apt-repository`
1212
add-apt-repository ppa:git-core/ppa && \
1313
apt-get update && \
1414
# ^ Get the latest version of Git from their PPA (we need 2.49.0 or later because we use the `git clone --revision` flag)
15-
apt-get install -y --no-install-recommends git=1:2.52.0-0ppa1~ubuntu24.04.3 && \
15+
apt-get install -y --no-install-recommends git=1:2.53.0-0ppa1~ubuntu24.04.1 && \
1616
apt-get remove -y software-properties-common && \
1717
apt-get autoremove -y
1818

‎frontend/src/components/config/workload/CommonWorkloadConfigFields.tsx‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,16 @@ export const CommonWorkloadConfigFields = ({
125125
}
126126
}, [state.git.repositoryId, state.image.imageTag]);
127127

128-
const fixedSensitiveNames =
128+
const fixedSensitiveVars =
129129
originalConfig?.appType === "workload"
130-
? new Set(
131-
originalConfig.env
132-
.filter((env) => env.isSensitive)
133-
.map((env) => env.name),
134-
)
135-
: new Set<string>();
130+
? originalConfig.env
131+
.map((env, idx) => ({ env, idx }))
132+
.filter(({ env }) => env.isSensitive)
133+
.reduce(
134+
(obj, cur) => Object.assign(obj, { [cur.env.name]: cur.idx }),
135+
{},
136+
)
137+
: {};
136138

137139
return (
138140
<>
@@ -323,7 +325,7 @@ export const CommonWorkloadConfigFields = ({
323325
setValue={(updater) => {
324326
setState((prev) => ({ ...prev, env: updater(prev.env) }));
325327
}}
326-
fixedSensitiveNames={fixedSensitiveNames}
328+
fixedSensitiveVars={fixedSensitiveVars}
327329
disabled={disabled ?? false}
328330
/>
329331
</AccordionContent>

‎frontend/src/components/config/workload/EnvVarGrid.tsx‎

Lines changed: 71 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -3,46 +3,22 @@ import { Button } from "@/components/ui/button";
33
import { Checkbox } from "@/components/ui/checkbox";
44
import { Input } from "@/components/ui/input";
55
import { Trash2 } from "lucide-react";
6-
import { Fragment, useEffect, useState } from "react";
6+
import { Fragment } from "react";
77

88
type EnvVars = { name: string; value: string | null; isSensitive: boolean }[];
99

10-
// TODO: show error message on duplicate env names
1110
export const EnvVarGrid = ({
1211
value: envVars,
1312
setValue: setEnvironmentVariables,
14-
fixedSensitiveNames,
13+
fixedSensitiveVars,
1514
disabled = false,
1615
}: {
1716
value: EnvVars;
1817
setValue: (updater: (envVars: EnvVars) => EnvVars) => void;
19-
fixedSensitiveNames: Set<string>;
18+
fixedSensitiveVars: Record<string, number>;
2019
disabled: boolean;
2120
}) => {
22-
const [error, setError] = useState("");
23-
useEffect(() => {
24-
for (const i of envVars.keys()) {
25-
if (
26-
envVars[i].name === "" &&
27-
envVars[i].value === "" &&
28-
!envVars[i].isSensitive &&
29-
i < envVars.length - 1
30-
) {
31-
setEnvironmentVariables((prev) => prev.toSpliced(i, 1));
32-
return;
33-
}
34-
}
35-
if (
36-
envVars[envVars.length - 1]?.name !== "" ||
37-
envVars[envVars.length - 1]?.value !== "" ||
38-
envVars[envVars.length - 1]?.isSensitive
39-
) {
40-
setEnvironmentVariables((prev) => [
41-
...prev,
42-
{ name: "", value: "", isSensitive: false },
43-
]);
44-
}
45-
}, [envVars, setEnvironmentVariables]);
21+
const error = getEnvError(envVars);
4622

4723
return (
4824
<div className="grid grid-cols-[1fr_min-content_1fr_min-content_min-content] items-center gap-2">
@@ -57,7 +33,7 @@ export const EnvVarGrid = ({
5733
</span>
5834
<span></span>
5935
{envVars.map(({ name, value, isSensitive }, index) => {
60-
const isFixedSensitive = fixedSensitiveNames.has(name);
36+
const isFixedSensitive = fixedSensitiveVars[name] === index;
6137
return (
6238
<Fragment key={index}>
6339
<Input
@@ -76,21 +52,14 @@ export const EnvVarGrid = ({
7652
value={name}
7753
onChange={(e) => {
7854
const value = e.currentTarget.value;
79-
setEnvironmentVariables((prev) => {
80-
const newList = prev.toSpliced(index, 1, {
81-
...prev[index],
82-
name: value,
83-
});
84-
const duplicates = getDuplicates(newList);
85-
if (duplicates.length != 0) {
86-
setError(
87-
`Duplicate environment variable(s): ${duplicates.join(", ")}`,
88-
);
89-
} else {
90-
setError("");
91-
}
92-
return newList;
93-
});
55+
setEnvironmentVariables((prev) =>
56+
getCorrectEnvBlanks(
57+
prev.toSpliced(index, 1, {
58+
...prev[index],
59+
name: value,
60+
}),
61+
),
62+
);
9463
}}
9564
/>
9665
<span className="w-fit align-middle text-xl">=</span>
@@ -103,13 +72,14 @@ export const EnvVarGrid = ({
10372
value={value ?? ""}
10473
onChange={(e) => {
10574
const value = e.currentTarget.value;
106-
setEnvironmentVariables((prev) => {
107-
const newList = prev.toSpliced(index, 1, {
108-
...prev[index],
109-
value: value,
110-
});
111-
return newList;
112-
});
75+
setEnvironmentVariables((prev) =>
76+
getCorrectEnvBlanks(
77+
prev.toSpliced(index, 1, {
78+
...prev[index],
79+
value: value,
80+
}),
81+
),
82+
);
11383
}}
11484
autoComplete="off"
11585
autoCorrect="off"
@@ -123,10 +93,12 @@ export const EnvVarGrid = ({
12393
checked={isSensitive}
12494
onCheckedChange={(checked) => {
12595
setEnvironmentVariables((prev) =>
126-
prev.toSpliced(index, 1, {
127-
...prev[index],
128-
isSensitive: checked === true,
129-
}),
96+
getCorrectEnvBlanks(
97+
prev.toSpliced(index, 1, {
98+
...prev[index],
99+
isSensitive: checked === true,
100+
}),
101+
),
130102
);
131103
}}
132104
/>
@@ -136,7 +108,9 @@ export const EnvVarGrid = ({
136108
variant="secondary"
137109
type="button"
138110
onClick={() =>
139-
setEnvironmentVariables((prev) => prev.toSpliced(index, 1))
111+
setEnvironmentVariables((prev) =>
112+
index != prev.length - 1 ? prev.toSpliced(index, 1) : prev,
113+
)
140114
}
141115
>
142116
<Trash2 />
@@ -149,17 +123,48 @@ export const EnvVarGrid = ({
149123
);
150124
};
151125

152-
const getDuplicates = (values: EnvVars): string[] => {
153-
const names = new Set();
154-
const result = [];
155-
for (const env of values) {
156-
if (env.name === "") {
157-
continue;
126+
export const getCorrectEnvBlanks = (envVars: EnvVars): EnvVars => {
127+
const indicesToDelete = new Set<number>();
128+
envVars.forEach((env, idx) => {
129+
if (
130+
env.name === "" &&
131+
env.value === "" &&
132+
!env.isSensitive &&
133+
idx < envVars.length - 1
134+
) {
135+
indicesToDelete.add(idx);
158136
}
159-
if (names.has(env.name)) {
160-
result.push(env.name);
137+
});
138+
const cleanedVars = envVars.filter((_, idx) => !indicesToDelete.has(idx));
139+
140+
const last = cleanedVars[cleanedVars.length - 1];
141+
if (last.name !== "" || last.value !== "" || last.isSensitive) {
142+
cleanedVars.push({ name: "", value: "", isSensitive: false });
143+
}
144+
return cleanedVars;
145+
};
146+
147+
const getDuplicates = (values: string[]) => {
148+
const unique = new Set<string>();
149+
const duplicates = new Set<string>();
150+
151+
values.forEach((value) => {
152+
if (unique.has(value)) {
153+
duplicates.add(value);
154+
} else {
155+
unique.add(value);
161156
}
162-
names.add(env.name);
157+
});
158+
159+
return duplicates;
160+
};
161+
162+
export const getEnvError = (env: EnvVars) => {
163+
const duplicates = getDuplicates(env.map((ev) => ev.name).filter(Boolean));
164+
165+
if (duplicates.size !== 0) {
166+
return `Duplicate environment variable(s): ${[...duplicates.values()].join(", ")}`;
163167
}
164-
return result;
168+
169+
return "";
165170
};

‎frontend/src/components/diff/workload/CommonWorkloadConfigDiff.tsx‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,14 @@ export const CommonWorkloadConfigDiff = ({
4040
const appConfig = useAppConfig();
4141
const baseWorkloadState = base.appType === "workload" ? base.workload : null;
4242

43-
const fixedSensitiveNames = new Set(
43+
const fixedSensitiveVars =
4444
baseWorkloadState?.env
45-
.filter((env) => env.isSensitive)
46-
.map((env) => env.name) ?? [],
47-
);
48-
45+
.map((env, idx) => ({ env, idx }))
46+
.filter(({ env }) => env.isSensitive)
47+
.reduce(
48+
(obj, cur) => Object.assign(obj, { [cur.env.name]: cur.idx }),
49+
{},
50+
) ?? {};
4951
return (
5052
<>
5153
<h3 className="mt-4 border-b pb-1 font-bold">Deployment Options</h3>
@@ -189,7 +191,7 @@ export const CommonWorkloadConfigDiff = ({
189191
env: updater(prev.env),
190192
}))
191193
}
192-
fixedSensitiveNames={fixedSensitiveNames}
194+
fixedSensitiveVars={fixedSensitiveVars}
193195
/>
194196
</AccordionContent>
195197
</AccordionItem>

0 commit comments

Comments
 (0)