fix(deploy): address final review — sensitiveKeys snapshot, dirty scrubbing, transition race, refetch invalidations
- Issue 1: add List<String> sensitiveKeys as 4th field to DeploymentConfigSnapshot; populate from agentConfig.getSensitiveKeys() in DeploymentExecutor; handleRestore hydrates from snap.sensitiveKeys directly; Deployment type in apps.ts gains sensitiveKeys field - Issue 2: after createApp succeeds, refetchQueries(['apps', envSlug]) before navigate so the new app is in cache before the router renders the deployed view (eliminates transient Save- disabled flash) - Issue 3: useDeploymentPageState useEffect now uses prevServerStateRef to detect local edits; background refetches only overwrite form when no local changes are present - Issue 5: handleRedeploy invalidates dirty-state + versions queries after createDeployment resolves; handleSave invalidates dirty-state after staged save - Issue 10: DirtyStateCalculator strips volatile agentConfig keys (version, updatedAt, updatedBy, environment, application) before JSON comparison via scrubAgentConfig(); adds versionBumpDoesNotMarkDirty test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -261,10 +261,12 @@ public class DeploymentExecutor {
|
|||||||
ApplicationConfig agentConfig = applicationConfigRepository
|
ApplicationConfig agentConfig = applicationConfigRepository
|
||||||
.findByApplicationAndEnvironment(app.slug(), env.slug())
|
.findByApplicationAndEnvironment(app.slug(), env.slug())
|
||||||
.orElse(null);
|
.orElse(null);
|
||||||
|
List<String> snapshotSensitiveKeys = agentConfig != null ? agentConfig.getSensitiveKeys() : null;
|
||||||
DeploymentConfigSnapshot snapshot = new DeploymentConfigSnapshot(
|
DeploymentConfigSnapshot snapshot = new DeploymentConfigSnapshot(
|
||||||
deployment.appVersionId(),
|
deployment.appVersionId(),
|
||||||
agentConfig,
|
agentConfig,
|
||||||
app.containerConfig()
|
app.containerConfig(),
|
||||||
|
snapshotSensitiveKeys
|
||||||
);
|
);
|
||||||
pgDeployRepo.saveDeployedConfigSnapshot(deployment.id(), snapshot);
|
pgDeployRepo.saveDeployedConfigSnapshot(deployment.id(), snapshot);
|
||||||
|
|
||||||
|
|||||||
@@ -61,7 +61,8 @@ class PostgresDeploymentRepositoryIT extends AbstractPostgresIT {
|
|||||||
DeploymentConfigSnapshot snapshot = new DeploymentConfigSnapshot(
|
DeploymentConfigSnapshot snapshot = new DeploymentConfigSnapshot(
|
||||||
jarVersionId,
|
jarVersionId,
|
||||||
agentConfig,
|
agentConfig,
|
||||||
Map.of("memoryLimitMb", 1024, "replicas", 2)
|
Map.of("memoryLimitMb", 1024, "replicas", 2),
|
||||||
|
null
|
||||||
);
|
);
|
||||||
|
|
||||||
UUID deploymentId = repository.create(appId, appVersionId, envId, "test-container");
|
UUID deploymentId = repository.create(appId, appVersionId, envId, "test-container");
|
||||||
@@ -92,7 +93,8 @@ class PostgresDeploymentRepositoryIT extends AbstractPostgresIT {
|
|||||||
DeploymentConfigSnapshot snapshot = new DeploymentConfigSnapshot(
|
DeploymentConfigSnapshot snapshot = new DeploymentConfigSnapshot(
|
||||||
jarVersionId,
|
jarVersionId,
|
||||||
new ApplicationConfig(),
|
new ApplicationConfig(),
|
||||||
Map.of()
|
Map.of(),
|
||||||
|
null
|
||||||
);
|
);
|
||||||
|
|
||||||
UUID deploymentId = repository.create(appId, appVersionId, envId, "test-container-clear");
|
UUID deploymentId = repository.create(appId, appVersionId, envId, "test-container-clear");
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package com.cameleer.server.core.runtime;
|
|||||||
|
|
||||||
import com.cameleer.common.model.ApplicationConfig;
|
import com.cameleer.common.model.ApplicationConfig;
|
||||||
|
|
||||||
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
@@ -15,6 +16,7 @@ import java.util.UUID;
|
|||||||
public record DeploymentConfigSnapshot(
|
public record DeploymentConfigSnapshot(
|
||||||
UUID jarVersionId,
|
UUID jarVersionId,
|
||||||
ApplicationConfig agentConfig,
|
ApplicationConfig agentConfig,
|
||||||
Map<String, Object> containerConfig
|
Map<String, Object> containerConfig,
|
||||||
|
List<String> sensitiveKeys
|
||||||
) {
|
) {
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import java.util.ArrayList;
|
|||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.Objects;
|
import java.util.Objects;
|
||||||
|
import java.util.Set;
|
||||||
import java.util.TreeSet;
|
import java.util.TreeSet;
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
@@ -22,12 +23,23 @@ import java.util.UUID;
|
|||||||
*/
|
*/
|
||||||
public class DirtyStateCalculator {
|
public class DirtyStateCalculator {
|
||||||
|
|
||||||
|
private static final Set<String> AGENT_CONFIG_IGNORED_KEYS = Set.of(
|
||||||
|
"version", "updatedAt", "updatedBy", "environment", "application"
|
||||||
|
);
|
||||||
|
|
||||||
private final ObjectMapper mapper;
|
private final ObjectMapper mapper;
|
||||||
|
|
||||||
public DirtyStateCalculator(ObjectMapper mapper) {
|
public DirtyStateCalculator(ObjectMapper mapper) {
|
||||||
this.mapper = mapper;
|
this.mapper = mapper;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private JsonNode scrubAgentConfig(JsonNode node) {
|
||||||
|
if (!(node instanceof ObjectNode obj)) return node;
|
||||||
|
ObjectNode copy = obj.deepCopy();
|
||||||
|
for (String k : AGENT_CONFIG_IGNORED_KEYS) copy.remove(k);
|
||||||
|
return copy;
|
||||||
|
}
|
||||||
|
|
||||||
public DirtyStateResult compute(UUID desiredJarVersionId,
|
public DirtyStateResult compute(UUID desiredJarVersionId,
|
||||||
ApplicationConfig desiredAgentConfig,
|
ApplicationConfig desiredAgentConfig,
|
||||||
Map<String, Object> desiredContainerConfig,
|
Map<String, Object> desiredContainerConfig,
|
||||||
@@ -44,8 +56,10 @@ public class DirtyStateCalculator {
|
|||||||
String.valueOf(desiredJarVersionId), String.valueOf(snapshot.jarVersionId())));
|
String.valueOf(desiredJarVersionId), String.valueOf(snapshot.jarVersionId())));
|
||||||
}
|
}
|
||||||
|
|
||||||
compareJson("agentConfig", mapper.valueToTree(desiredAgentConfig),
|
compareJson("agentConfig",
|
||||||
mapper.valueToTree(snapshot.agentConfig()), diffs);
|
scrubAgentConfig(mapper.valueToTree(desiredAgentConfig)),
|
||||||
|
scrubAgentConfig(mapper.valueToTree(snapshot.agentConfig())),
|
||||||
|
diffs);
|
||||||
compareJson("containerConfig", mapper.valueToTree(desiredContainerConfig),
|
compareJson("containerConfig", mapper.valueToTree(desiredContainerConfig),
|
||||||
mapper.valueToTree(snapshot.containerConfig()), diffs);
|
mapper.valueToTree(snapshot.containerConfig()), diffs);
|
||||||
|
|
||||||
|
|||||||
@@ -39,7 +39,7 @@ class DirtyStateCalculatorTest {
|
|||||||
Map<String, Object> container = Map.of("memoryLimitMb", 512);
|
Map<String, Object> container = Map.of("memoryLimitMb", 512);
|
||||||
|
|
||||||
UUID jarId = UUID.randomUUID();
|
UUID jarId = UUID.randomUUID();
|
||||||
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, cfg, container);
|
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, cfg, container, null);
|
||||||
DirtyStateResult result = calc.compute(jarId, cfg, container, snap);
|
DirtyStateResult result = calc.compute(jarId, cfg, container, snap);
|
||||||
|
|
||||||
assertThat(result.dirty()).isFalse();
|
assertThat(result.dirty()).isFalse();
|
||||||
@@ -53,7 +53,7 @@ class DirtyStateCalculatorTest {
|
|||||||
Map<String, Object> container = Map.of();
|
Map<String, Object> container = Map.of();
|
||||||
UUID v1 = UUID.randomUUID();
|
UUID v1 = UUID.randomUUID();
|
||||||
UUID v2 = UUID.randomUUID();
|
UUID v2 = UUID.randomUUID();
|
||||||
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(v1, cfg, container);
|
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(v1, cfg, container, null);
|
||||||
|
|
||||||
DirtyStateResult result = calc.compute(v2, cfg, container, snap);
|
DirtyStateResult result = calc.compute(v2, cfg, container, snap);
|
||||||
|
|
||||||
@@ -71,7 +71,7 @@ class DirtyStateCalculatorTest {
|
|||||||
ApplicationConfig desiredCfg = new ApplicationConfig();
|
ApplicationConfig desiredCfg = new ApplicationConfig();
|
||||||
desiredCfg.setSamplingRate(1.0);
|
desiredCfg.setSamplingRate(1.0);
|
||||||
UUID jarId = UUID.randomUUID();
|
UUID jarId = UUID.randomUUID();
|
||||||
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, deployedCfg, Map.of());
|
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, deployedCfg, Map.of(), null);
|
||||||
|
|
||||||
DirtyStateResult result = calc.compute(jarId, desiredCfg, Map.of(), snap);
|
DirtyStateResult result = calc.compute(jarId, desiredCfg, Map.of(), snap);
|
||||||
|
|
||||||
@@ -85,7 +85,7 @@ class DirtyStateCalculatorTest {
|
|||||||
DirtyStateCalculator calc = CALC;
|
DirtyStateCalculator calc = CALC;
|
||||||
ApplicationConfig cfg = new ApplicationConfig();
|
ApplicationConfig cfg = new ApplicationConfig();
|
||||||
UUID jarId = UUID.randomUUID();
|
UUID jarId = UUID.randomUUID();
|
||||||
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, cfg, Map.of("memoryLimitMb", 512));
|
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, cfg, Map.of("memoryLimitMb", 512), null);
|
||||||
|
|
||||||
DirtyStateResult result = calc.compute(jarId, cfg, Map.of("memoryLimitMb", 1024), snap);
|
DirtyStateResult result = calc.compute(jarId, cfg, Map.of("memoryLimitMb", 1024), snap);
|
||||||
|
|
||||||
@@ -100,7 +100,7 @@ class DirtyStateCalculatorTest {
|
|||||||
ApplicationConfig desired = new ApplicationConfig();
|
ApplicationConfig desired = new ApplicationConfig();
|
||||||
desired.setSamplingRate(1.0);
|
desired.setSamplingRate(1.0);
|
||||||
UUID jarId = UUID.randomUUID();
|
UUID jarId = UUID.randomUUID();
|
||||||
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, null, Map.of());
|
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, null, Map.of(), null);
|
||||||
|
|
||||||
DirtyStateResult result = calc.compute(jarId, desired, Map.of(), snap);
|
DirtyStateResult result = calc.compute(jarId, desired, Map.of(), snap);
|
||||||
|
|
||||||
@@ -118,7 +118,7 @@ class DirtyStateCalculatorTest {
|
|||||||
ApplicationConfig desired = new ApplicationConfig();
|
ApplicationConfig desired = new ApplicationConfig();
|
||||||
desired.setTracedProcessors(Map.of("proc-1", "TRACE"));
|
desired.setTracedProcessors(Map.of("proc-1", "TRACE"));
|
||||||
UUID jarId = UUID.randomUUID();
|
UUID jarId = UUID.randomUUID();
|
||||||
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, deployed, Map.of());
|
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, deployed, Map.of(), null);
|
||||||
|
|
||||||
DirtyStateResult result = calc.compute(jarId, desired, Map.of(), snap);
|
DirtyStateResult result = calc.compute(jarId, desired, Map.of(), snap);
|
||||||
|
|
||||||
@@ -136,7 +136,7 @@ class DirtyStateCalculatorTest {
|
|||||||
ApplicationConfig desired = new ApplicationConfig();
|
ApplicationConfig desired = new ApplicationConfig();
|
||||||
desired.setApplicationLogLevel("DEBUG");
|
desired.setApplicationLogLevel("DEBUG");
|
||||||
UUID jarId = UUID.randomUUID();
|
UUID jarId = UUID.randomUUID();
|
||||||
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, deployed, Map.of());
|
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, deployed, Map.of(), null);
|
||||||
|
|
||||||
DirtyStateResult result = calc.compute(jarId, desired, Map.of(), snap);
|
DirtyStateResult result = calc.compute(jarId, desired, Map.of(), snap);
|
||||||
|
|
||||||
@@ -146,4 +146,19 @@ class DirtyStateCalculatorTest {
|
|||||||
assertThat(diff.staged()).isEqualTo("DEBUG");
|
assertThat(diff.staged()).isEqualTo("DEBUG");
|
||||||
assertThat(diff.deployed()).isEqualTo("INFO");
|
assertThat(diff.deployed()).isEqualTo("INFO");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void versionBumpDoesNotMarkDirty() {
|
||||||
|
ApplicationConfig deployedCfg = new ApplicationConfig();
|
||||||
|
deployedCfg.setSamplingRate(0.5);
|
||||||
|
deployedCfg.setVersion(1);
|
||||||
|
ApplicationConfig desiredCfg = new ApplicationConfig();
|
||||||
|
desiredCfg.setSamplingRate(0.5);
|
||||||
|
desiredCfg.setVersion(2); // bumped by save
|
||||||
|
UUID jarId = UUID.randomUUID();
|
||||||
|
DeploymentConfigSnapshot snap = new DeploymentConfigSnapshot(jarId, deployedCfg, Map.of(), null);
|
||||||
|
|
||||||
|
DirtyStateResult result = CALC.compute(jarId, desiredCfg, Map.of(), snap);
|
||||||
|
assertThat(result.dirty()).isFalse();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -45,6 +45,7 @@ export interface Deployment {
|
|||||||
jarVersionId: string;
|
jarVersionId: string;
|
||||||
agentConfig: Record<string, unknown> | null;
|
agentConfig: Record<string, unknown> | null;
|
||||||
containerConfig: Record<string, unknown>;
|
containerConfig: Record<string, unknown>;
|
||||||
|
sensitiveKeys: string[] | null;
|
||||||
} | null;
|
} | null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
// ui/src/pages/AppsTab/AppDeploymentPage/hooks/useDeploymentPageState.ts
|
// ui/src/pages/AppsTab/AppDeploymentPage/hooks/useDeploymentPageState.ts
|
||||||
import { useState, useEffect, useMemo } from 'react';
|
import { useState, useEffect, useMemo, useRef } from 'react';
|
||||||
import type { ApplicationConfig } from '../../../../api/queries/commands';
|
import type { ApplicationConfig } from '../../../../api/queries/commands';
|
||||||
import type { App } from '../../../../api/queries/admin/apps';
|
import type { App } from '../../../../api/queries/admin/apps';
|
||||||
|
|
||||||
@@ -115,9 +115,18 @@ export function useDeploymentPageState(
|
|||||||
}, [app, agentConfig, envDefaults]);
|
}, [app, agentConfig, envDefaults]);
|
||||||
|
|
||||||
const [form, setForm] = useState<DeploymentPageFormState>(serverState);
|
const [form, setForm] = useState<DeploymentPageFormState>(serverState);
|
||||||
|
const prevServerStateRef = useRef<DeploymentPageFormState>(serverState);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
setForm(serverState);
|
// Only overwrite form if the current form value still matches the previous
|
||||||
|
// server state (i.e., the user has no local edits). Otherwise preserve
|
||||||
|
// user edits through background refetches.
|
||||||
|
setForm((current) => {
|
||||||
|
const hadLocalEdits =
|
||||||
|
JSON.stringify(current) !== JSON.stringify(prevServerStateRef.current);
|
||||||
|
prevServerStateRef.current = serverState;
|
||||||
|
return hadLocalEdits ? current : serverState;
|
||||||
|
});
|
||||||
}, [serverState]);
|
}, [serverState]);
|
||||||
|
|
||||||
return { form, setForm, reset: () => setForm(serverState), serverState };
|
return { form, setForm, reset: () => setForm(serverState), serverState };
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { useState, useEffect, useRef } from 'react';
|
import { useState, useEffect, useRef } from 'react';
|
||||||
import { useParams, useLocation, useNavigate } from 'react-router';
|
import { useParams, useLocation, useNavigate } from 'react-router';
|
||||||
|
import { useQueryClient } from '@tanstack/react-query';
|
||||||
import { AlertDialog, Button, Tabs, useToast } from '@cameleer/design-system';
|
import { AlertDialog, Button, Tabs, useToast } from '@cameleer/design-system';
|
||||||
import { useEnvironmentStore } from '../../../api/environment-store';
|
import { useEnvironmentStore } from '../../../api/environment-store';
|
||||||
import { useEnvironments } from '../../../api/queries/admin/environments';
|
import { useEnvironments } from '../../../api/queries/admin/environments';
|
||||||
@@ -45,6 +46,7 @@ export default function AppDeploymentPage() {
|
|||||||
const location = useLocation();
|
const location = useLocation();
|
||||||
const navigate = useNavigate();
|
const navigate = useNavigate();
|
||||||
const { toast } = useToast();
|
const { toast } = useToast();
|
||||||
|
const queryClient = useQueryClient();
|
||||||
const selectedEnv = useEnvironmentStore((s) => s.environment);
|
const selectedEnv = useEnvironmentStore((s) => s.environment);
|
||||||
const { data: environments = [], isLoading: envLoading } = useEnvironments();
|
const { data: environments = [], isLoading: envLoading } = useEnvironments();
|
||||||
const { data: apps = [], isLoading: appsLoading } = useApps(selectedEnv);
|
const { data: apps = [], isLoading: appsLoading } = useApps(selectedEnv);
|
||||||
@@ -218,8 +220,14 @@ export default function AppDeploymentPage() {
|
|||||||
|
|
||||||
setStagedJar(null);
|
setStagedJar(null);
|
||||||
|
|
||||||
|
// Invalidate dirty-state so the button reflects the new saved state
|
||||||
|
await queryClient.invalidateQueries({ queryKey: ['apps', envSlug, targetApp.slug, 'dirty-state'] });
|
||||||
|
|
||||||
if (!app) {
|
if (!app) {
|
||||||
// Transition to the existing-app view
|
// Transition to the existing-app view — refetch apps first so the new app
|
||||||
|
// is in the cache before the router renders the deployed view (prevents
|
||||||
|
// the transient Save-disabled flash while useApps is loading).
|
||||||
|
await queryClient.refetchQueries({ queryKey: ['apps', envSlug] });
|
||||||
navigate(`/apps/${targetApp.slug}`);
|
navigate(`/apps/${targetApp.slug}`);
|
||||||
}
|
}
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
@@ -242,7 +250,6 @@ export default function AppDeploymentPage() {
|
|||||||
if (stagedJar) {
|
if (stagedJar) {
|
||||||
const newVersion = await uploadJar.mutateAsync({ envSlug, appSlug: app.slug, file: stagedJar });
|
const newVersion = await uploadJar.mutateAsync({ envSlug, appSlug: app.slug, file: stagedJar });
|
||||||
versionId = newVersion.id;
|
versionId = newVersion.id;
|
||||||
setStagedJar(null);
|
|
||||||
} else {
|
} else {
|
||||||
if (!currentVersion) {
|
if (!currentVersion) {
|
||||||
toast({
|
toast({
|
||||||
@@ -257,6 +264,10 @@ export default function AppDeploymentPage() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
await createDeployment.mutateAsync({ envSlug, appSlug: app.slug, appVersionId: versionId });
|
await createDeployment.mutateAsync({ envSlug, appSlug: app.slug, appVersionId: versionId });
|
||||||
|
setStagedJar(null);
|
||||||
|
// Invalidate dirty-state and versions so button recomputes after deploy
|
||||||
|
queryClient.invalidateQueries({ queryKey: ['apps', envSlug, app.slug, 'dirty-state'] });
|
||||||
|
queryClient.invalidateQueries({ queryKey: ['apps', envSlug, app.slug, 'versions'] });
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
toast({
|
toast({
|
||||||
title: 'Redeploy failed',
|
title: 'Redeploy failed',
|
||||||
@@ -344,7 +355,9 @@ export default function AppDeploymentPage() {
|
|||||||
: prev.variables.envVars,
|
: prev.variables.envVars,
|
||||||
},
|
},
|
||||||
sensitiveKeys: {
|
sensitiveKeys: {
|
||||||
sensitiveKeys: Array.isArray(a.sensitiveKeys)
|
sensitiveKeys: Array.isArray(snap.sensitiveKeys)
|
||||||
|
? snap.sensitiveKeys
|
||||||
|
: Array.isArray(a.sensitiveKeys)
|
||||||
? (a.sensitiveKeys as string[])
|
? (a.sensitiveKeys as string[])
|
||||||
: prev.sensitiveKeys.sensitiveKeys,
|
: prev.sensitiveKeys.sensitiveKeys,
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in New Issue
Block a user