Skip to content

Commit 287ab4d

Browse files
slaskawimjnagel
andauthored
fix: ensure ambient mode is the default in all operator code (#2326) (backport-0.60) (#2327)
Backport of commit: bda5384 Co-authored-by: Micah Nagel <micah.nagel@defenseunicorns.com>
1 parent 944ff5b commit 287ab4d

10 files changed

Lines changed: 79 additions & 27 deletions

File tree

src/pepr/operator/controllers/istio/ambient-waypoint.spec.ts

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { ambientEgressNamespace, sharedEgressPkgId } from "./istio-resources";
2525
const createMockPackage = (
2626
name: string,
2727
selector: Record<string, string> = {},
28-
mode: "ambient" | "sidecar" = "ambient",
28+
mode: "ambient" | "sidecar" | "unset" = "ambient",
2929
sso: Sso[] = [
3030
{
3131
clientId: "test-client",
@@ -40,11 +40,14 @@ const createMockPackage = (
4040
uid: "test-uid",
4141
},
4242
spec: {
43-
network: {
44-
serviceMesh: {
45-
mode: mode === "ambient" ? Mode.Ambient : Mode.Sidecar,
46-
},
47-
},
43+
network:
44+
mode === "unset"
45+
? {}
46+
: {
47+
serviceMesh: {
48+
mode: mode === "ambient" ? Mode.Ambient : Mode.Sidecar,
49+
},
50+
},
4851
sso,
4952
},
5053
});
@@ -319,6 +322,37 @@ describe("reconcileService and reconcilePod", () => {
319322
}
320323
},
321324
);
325+
326+
it.each(testCases)(
327+
"$name - defaults to ambient mode when serviceMesh.mode is undefined",
328+
async ({ createResource, expectedLabels, name }) => {
329+
const resource = createResource();
330+
331+
const pkg = createMockPackage("test-pkg", { "app.kubernetes.io/name": "test-app" }, "unset", [
332+
{
333+
clientId: "test-client",
334+
name: "test-sso",
335+
enableAuthserviceSelector: { "app.kubernetes.io/name": "test-app" },
336+
},
337+
]);
338+
339+
(
340+
PackageStore.getPackageByNamespace as MockedFunction<
341+
typeof PackageStore.getPackageByNamespace
342+
>
343+
).mockImplementation(namespace => {
344+
return namespace === testNamespace ? pkg : undefined;
345+
});
346+
347+
if (name === "service") {
348+
await reconcileService(resource as a.Service);
349+
} else {
350+
await reconcilePod(resource as a.Pod);
351+
}
352+
353+
expect(resource.metadata?.labels).toMatchObject(expectedLabels);
354+
},
355+
);
322356
});
323357

324358
describe("setupAmbientWaypoint", () => {

src/pepr/operator/controllers/istio/ambient-waypoint.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,8 @@ export async function reconcileService(svc: a.Service): Promise<void> {
213213
}
214214

215215
const pkg = PackageStore.getPackageByNamespace(namespace);
216-
if (
217-
!pkg ||
218-
pkg.metadata?.deletionTimestamp ||
219-
pkg.spec?.network?.serviceMesh?.mode !== Mode.Ambient
220-
) {
216+
const istioMode = pkg?.spec?.network?.serviceMesh?.mode || Mode.Ambient;
217+
if (!pkg || pkg.metadata?.deletionTimestamp || istioMode !== Mode.Ambient) {
221218
return;
222219
}
223220

@@ -270,11 +267,8 @@ export async function reconcilePod(pod: a.Pod): Promise<void> {
270267
}
271268

272269
const pkg = PackageStore.getPackageByNamespace(namespace);
273-
if (
274-
!pkg ||
275-
pkg.metadata?.deletionTimestamp ||
276-
pkg.spec?.network?.serviceMesh?.mode !== Mode.Ambient
277-
) {
270+
const istioMode = pkg?.spec?.network?.serviceMesh?.mode || Mode.Ambient;
271+
if (!pkg || pkg.metadata?.deletionTimestamp || istioMode !== Mode.Ambient) {
278272
return;
279273
}
280274

src/pepr/operator/controllers/istio/waypoint-utils.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ describe("shouldUseAmbientWaypoint", () => {
9393
expected: false,
9494
},
9595
{
96-
name: "should return false when no serviceMesh config exists",
96+
name: "should return true when no serviceMesh config exists (ambient default)",
9797
pkg: {
9898
metadata: { name: "test", namespace: "test" },
9999
spec: {
@@ -106,7 +106,7 @@ describe("shouldUseAmbientWaypoint", () => {
106106
],
107107
},
108108
} as UDSPackage,
109-
expected: false,
109+
expected: true,
110110
},
111111
];
112112

src/pepr/operator/controllers/istio/waypoint-utils.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ const WAYPOINT_SUFFIX = "-waypoint"; // Suffix for waypoint resource names
1414
* Determines if a package should use ambient waypoint networking
1515
*/
1616
export const shouldUseAmbientWaypoint = (pkg: UDSPackage): boolean => {
17-
return pkg.spec?.network?.serviceMesh?.mode === Mode.Ambient && hasAuthserviceSSO(pkg);
17+
const istioMode = pkg.spec?.network?.serviceMesh?.mode || Mode.Ambient;
18+
return istioMode === Mode.Ambient && hasAuthserviceSSO(pkg);
1819
};
1920

2021
/**

src/pepr/operator/controllers/network/authorizationPolicies.spec.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,9 @@ describe("authorization policy generation", () => {
273273
},
274274
],
275275
network: {
276+
serviceMesh: {
277+
mode: Mode.Sidecar,
278+
},
276279
expose: [
277280
{
278281
service: "httpbin",
@@ -1142,6 +1145,19 @@ describe("findMatchingSsoClient", () => {
11421145
expect(findMatchingSsoClient(pkg, { app: "a" })).toBeUndefined();
11431146
});
11441147

1148+
test("defaults to ambient when serviceMesh.mode is undefined", () => {
1149+
const pkg: UDSPackage = {
1150+
metadata: { name: "app", namespace: "ns" },
1151+
spec: {
1152+
// No serviceMesh block -> defaults to Ambient
1153+
sso: [{ clientId: "c1", name: "", enableAuthserviceSelector: { app: "a" } }],
1154+
network: {},
1155+
},
1156+
};
1157+
1158+
expect(findMatchingSsoClient(pkg, { app: "a" })).toMatchObject({ clientId: "c1" });
1159+
});
1160+
11451161
test("prefilters to enabled clients only (missing/null/undefined selector are ignored)", () => {
11461162
const pkg: UDSPackage = {
11471163
metadata: { name: "app", namespace: "ns" },

src/pepr/operator/controllers/network/authorizationPolicies.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,8 @@ export function findMatchingSsoClient(
424424
pkg: UDSPackage,
425425
selector: Record<string, string> | undefined,
426426
) {
427-
if (!selector || pkg.spec?.network?.serviceMesh?.mode !== Mode.Ambient) {
427+
const istioMode = pkg.spec?.network?.serviceMesh?.mode || Mode.Ambient;
428+
if (!selector || istioMode !== Mode.Ambient) {
428429
return undefined;
429430
}
430431

src/pepr/operator/controllers/packages/package-store.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,16 +224,16 @@ describe("Package Store", () => {
224224
const ambientPkg2 = makeMockReq({
225225
metadata: { namespace: "ns2", name: "ambient2" },
226226
spec: {
227-
network: {
228-
serviceMesh: { mode: Mode.Ambient },
229-
},
227+
network: {},
230228
},
231229
}).Raw;
232230

233231
const nonAmbientPkg = makeMockReq({
234232
metadata: { namespace: "ns3", name: "non-ambient" },
235233
spec: {
236-
network: {},
234+
network: {
235+
serviceMesh: { mode: Mode.Sidecar },
236+
},
237237
},
238238
}).Raw;
239239

src/pepr/operator/controllers/packages/package-store.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,8 @@ function getAmbientPackages(): UDSPackage[] {
182182
const result: UDSPackage[] = [];
183183
for (const namespaceMap of packageNamespaceMap.values()) {
184184
for (const pkg of namespaceMap.values()) {
185-
if (pkg.spec?.network?.serviceMesh?.mode === Mode.Ambient) {
185+
const istioMode = pkg.spec?.network?.serviceMesh?.mode || Mode.Ambient;
186+
if (istioMode === Mode.Ambient) {
186187
result.push(pkg);
187188
}
188189
}

src/test/chart/templates/package.yaml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ metadata:
88
namespace: podinfo
99
spec:
1010
network:
11-
serviceMesh:
12-
mode: ambient
11+
# Relying on default mesh mode
12+
# serviceMesh:
13+
# mode: ambient
1314
expose:
1415
- service: podinfo
1516
selector:

test/playwright/podinfo.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ test.describe("Podinfo Authservice protection and metrics behavior", () => {
4545

4646
// Basic sanity check that podinfo content is rendered
4747
await expect(page).toHaveURL(podinfoUrl, { timeout: 10000 });
48+
49+
// Verify non-error response (indicates successful JWT validation)
50+
const response = await page.reload();
51+
expect(response?.status()).toBeLessThan(400);
4852
} finally {
4953
await context.close();
5054
}

0 commit comments

Comments
 (0)