Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: istio native sidecars #1032

Merged
merged 27 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
1577a49
wip: native sidecars
UnicornChance Nov 13, 2024
6b5b799
wip: cleanup upgrade sidecars
UnicornChance Nov 14, 2024
48cc74e
wip: revert injection changes
UnicornChance Nov 18, 2024
2563b11
fix: working example of istio native sidecars
UnicornChance Nov 18, 2024
cabc328
Merge branch 'main' into istio-native-sidecars
UnicornChance Nov 20, 2024
297cf9c
Merge branch 'main' into istio-native-sidecars
UnicornChance Dec 2, 2024
19280dc
Merge branch 'main' into istio-native-sidecars
UnicornChance Dec 2, 2024
b8d0d22
Merge branch 'main' into istio-native-sidecars
UnicornChance Jan 6, 2025
36d4540
fix: merge commit issues
UnicornChance Jan 6, 2025
634f0e5
Merge branch 'main' into istio-native-sidecars
UnicornChance Jan 6, 2025
b1a36af
fixes
UnicornChance Jan 7, 2025
96a4071
fix: pr comments
UnicornChance Jan 7, 2025
7af0c6c
Merge branch 'main' into istio-native-sidecars
UnicornChance Jan 7, 2025
ad90a0d
fix: more cleanup
UnicornChance Jan 7, 2025
c3f349f
fix: remove container checks
UnicornChance Jan 7, 2025
3eb6320
chore: add proxyv2 check
UnicornChance Jan 7, 2025
b94d896
fix: revert proxyv2 check
UnicornChance Jan 7, 2025
ecd86b8
Merge branch 'main' into istio-native-sidecars
UnicornChance Jan 7, 2025
4471b77
fix: istio native annotation cleanup
UnicornChance Jan 8, 2025
8e3326d
fix: add doc update and port check back in
UnicornChance Jan 8, 2025
2fccfcb
fix: remove annotation for cycling pods
UnicornChance Jan 9, 2025
930758f
Merge branch 'main' into istio-native-sidecars
UnicornChance Jan 9, 2025
291b825
fix: container sidecar edgecase and reference bug
UnicornChance Jan 10, 2025
e4647a2
Merge branch 'main' into istio-native-sidecars
UnicornChance Jan 10, 2025
cfd56c4
Merge branch 'main' into istio-native-sidecars
mjnagel Jan 10, 2025
5541fa3
Merge branch 'main' into istio-native-sidecars
UnicornChance Jan 13, 2025
152d97f
Merge branch 'main' into istio-native-sidecars
UnicornChance Jan 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions pepr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { PeprModule } from "pepr";

import cfg from "./package.json";

import { istio } from "./src/pepr/istio";
import { Component, setupLogger } from "./src/pepr/logger";
import { operator } from "./src/pepr/operator";
import { setupAuthserviceSecret } from "./src/pepr/operator/controllers/keycloak/authservice/config";
Expand All @@ -31,9 +30,6 @@ const log = setupLogger(Component.STARTUP);
// UDS Core Policies
policies,

// Istio service mesh
istio,

// Prometheus monitoring stack
prometheus,

Expand Down
1 change: 1 addition & 0 deletions src/istio/values/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ meshConfig:
pilot:
env:
PILOT_JWT_ENABLE_REMOTE_JWKS: hybrid
ENABLE_NATIVE_SIDECARS: true
3 changes: 0 additions & 3 deletions src/pepr/istio/README.md

This file was deleted.

108 changes: 0 additions & 108 deletions src/pepr/istio/index.ts

This file was deleted.

74 changes: 48 additions & 26 deletions src/pepr/operator/controllers/istio/injection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const log = setupLogger(Component.OPERATOR_ISTIO);

const injectionLabel = "istio-injection";
const injectionAnnotation = "uds.dev/original-istio-injection";
const nativeIstioAnnotation = "uds.dev/native-istio-sidecars";

/**
* Syncs the package namespace istio-injection label and adds a label for the package name
Expand All @@ -29,40 +30,60 @@ export async function enableInjection(pkg: UDSPackage) {
const originalInjectionLabel = labels[injectionLabel];
const annotations = sourceNS.metadata?.annotations || {};
const pkgKey = `uds.dev/pkg-${pkg.metadata.name}`;
const hasMigrationAnnotation = annotations[nativeIstioAnnotation] === "true";

// Mark the original namespace injection setting for if all packages are removed
if (!annotations[injectionAnnotation]) {
annotations[injectionAnnotation] = originalInjectionLabel || "non-existent";
}

// Ensure the namespace is configured
if (!annotations[pkgKey] || originalInjectionLabel !== "enabled") {
// Ensure Istio injection is enabled
let shouldRestartPods = false;

// Ensure native Istio migration annotation is present
if (!hasMigrationAnnotation) {
annotations[nativeIstioAnnotation] = "true";
log.info(`Adding native Istio migration annotation to namespace ${pkg.metadata.namespace}.`);
shouldRestartPods = true; // Pods need restarting for migration
}
UnicornChance marked this conversation as resolved.
Show resolved Hide resolved

// Ensure Istio injection is enabled
if (originalInjectionLabel !== "enabled") {
labels[injectionLabel] = "enabled";
log.info(`Enabling Istio injection for namespace ${pkg.metadata.namespace}.`);
shouldRestartPods = true; // Pods need restarting due to label change
}

// Add the package annotation
// Ensure package-specific annotation is updated
if (annotations[pkgKey] !== "true") {
annotations[pkgKey] = "true";

// Apply the updated Namespace
log.debug(`Updating namespace ${pkg.metadata.namespace} with istio injection label`);
await K8s(kind.Namespace).Apply(
{
metadata: {
name: pkg.metadata.namespace,
labels,
annotations,
},
},
{ force: true },
log.info(
`Updating package-specific annotation for ${pkg.metadata.name} in namespace ${pkg.metadata.namespace}.`,
);
}

// Kill the pods if we changed the value of the istio-injection label
if (originalInjectionLabel !== labels[injectionLabel]) {
log.debug(
`Attempting pod restart in ${pkg.metadata.namespace} based on istio injection label change`,
);
await killPods(pkg.metadata.namespace, true);
}
// Apply namespace updates if there are changes
const updatedNamespace = {
metadata: {
name: pkg.metadata.namespace,
labels,
annotations,
},
};

if (
JSON.stringify(sourceNS.metadata?.labels) !== JSON.stringify(labels) ||
JSON.stringify(sourceNS.metadata?.annotations) !== JSON.stringify(annotations)
) {
log.debug(`Applying updates to namespace ${pkg.metadata.namespace}.`);
await K8s(kind.Namespace).Apply(updatedNamespace, { force: true });
} else {
log.debug(`No namespace updates needed for ${pkg.metadata.namespace}.`);
}

// Restart pods if required
if (shouldRestartPods) {
log.debug(`Restarting pods in ${pkg.metadata.namespace} due to configuration changes.`);
await killPods(pkg.metadata.namespace, true);
}
}

UnicornChance marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -119,8 +140,8 @@ export async function cleanupNamespace(pkg: UDSPackage) {
/**
* Forces deletion of pods with the incorrect istio sidecar state
*
* @param ns
* @param enableInjection
* @param ns The namespace to target
* @param enableInjection Whether injection is being enabled
*/
async function killPods(ns: string, enableInjection: boolean) {
// Get all pods in the namespace
Expand All @@ -135,7 +156,8 @@ async function killPods(ns: string, enableInjection: boolean) {
continue;
}

const foundSidecar = pod.spec?.containers?.find(c => c.name === "istio-proxy");
// note that this is `initContainers` now because that is how native sidecar works
const foundSidecar = pod.spec?.initContainers?.find(c => c.name === "istio-proxy");

// If enabling injection, ignore pods that already have the istio sidecar
if (enableInjection && foundSidecar) {
Expand Down
16 changes: 7 additions & 9 deletions src/pepr/policies/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,19 +70,17 @@ export function isIstioInitContainer(
container?: V1Container,
) {
// Check for the sidecar.istio.io/status annotation
const hasAnnotation = request.HasAnnotation("sidecar.istio.io/status");
if (!hasAnnotation) {
if (!request.HasAnnotation("sidecar.istio.io/status")) {
return false;
}

// Check for what looks like an istio sidecar
const possibleSidecar = request.Raw.spec?.containers?.find(
c =>
c.name === "istio-proxy" &&
c.ports?.find(p => p.name === "http-envoy-prom") &&
c.args?.includes("proxy"),
// Check for an Istio proxy in initContainers
const hasInitContainerSidecar = request.Raw.spec?.initContainers?.some(
c => c.name === "istio-proxy" && c.args?.includes("proxy"),
noahpb marked this conversation as resolved.
Show resolved Hide resolved
);
if (!possibleSidecar) {

// Exit if no Istio proxy is found in initContainers
if (!hasInitContainerSidecar) {
return false;
}

Expand Down
6 changes: 3 additions & 3 deletions src/pepr/policies/security.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ describe("security policies", () => {
);
});

it("should restrict capabilities.add", async () => {
it("should restrict capabilities.add for non-istio-init containers", async () => {
const expected = (e: Error) =>
expect(e).toMatchObject({
ok: false,
Expand All @@ -425,7 +425,7 @@ describe("security policies", () => {
});

return Promise.all([
// Check for capabilities.add
// Check for capabilities.add with a regular container, which should fail
K8s(kind.Pod)
.Apply({
metadata: {
Expand All @@ -439,7 +439,7 @@ describe("security policies", () => {
image: "127.0.0.1/fake",
securityContext: {
capabilities: {
add: ["NET_ADMIN"],
add: ["NET_ADMIN"], // This should trigger a failure since `NET_ADMIN` is not authorized for non-istio-init
},
},
},
Expand Down
Loading