Skip to content

Commit

Permalink
fix: address network policy generation inter-namespace bug (#564)
Browse files Browse the repository at this point in the history
## Description

A bug exists when combining `remoteNamespace` and `remoteSelector` in a
single network.allow policy. The generated network policy results in two
separate peers.

## Related Issue

Fixes #528

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [] Test, docs, adr added or updated as needed
- [ ] [Contributor Guide
Steps](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)(https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md#submitting-a-pull-request)
followed

---------

Co-authored-by: Rob Ferguson <rjferguson21@gmail.com>
  • Loading branch information
MxNxPx and rjferguson21 committed Aug 1, 2024
1 parent 235702e commit 9b14c2c
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 52 deletions.
113 changes: 113 additions & 0 deletions src/pepr/operator/controllers/network/generate.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import { describe, expect, it } from "@jest/globals";
import { kind } from "pepr";
import { Direction } from "../../crd";
import { generate } from "./generate";

describe("network policy generate", () => {
it("should generate correct network policy", async () => {
const policy = generate("test", {
description: "test",
direction: Direction.Ingress,
selector: { app: "test" },
remoteNamespace: "foo",
remoteSelector: { app: "foo" },
});

expect(policy.metadata?.name).toEqual("Ingress-test");
expect(policy.spec).toEqual({
ingress: [
{
from: [
{
namespaceSelector: {
matchLabels: {
"kubernetes.io/metadata.name": "foo",
},
},
podSelector: {
matchLabels: {
app: "foo",
},
},
},
],
ports: [],
},
],
podSelector: { matchLabels: { app: "test" } },
policyTypes: ["Ingress"],
} as kind.NetworkPolicy["spec"]);
});
});

describe("network policy generate", () => {
it("should generate correct network policy for just remoteNamespace", async () => {
const policy = generate("test", {
description: "test",
direction: Direction.Ingress,
selector: { app: "test" },
remoteNamespace: "foo",
});

expect(policy.metadata?.name).toEqual("Ingress-test");
expect(policy.spec).toEqual({
ingress: [
{
from: [
{
namespaceSelector: {
matchLabels: {
"kubernetes.io/metadata.name": "foo",
},
},
},
],
ports: [],
},
],
podSelector: { matchLabels: { app: "test" } },
policyTypes: ["Ingress"],
} as kind.NetworkPolicy["spec"]);
});
});

describe("network policy generate", () => {
it("should generate correct network policy for empty string and wildcard remoteNamespace", async () => {
const policy = generate("test", {
description: "test",
direction: Direction.Egress,
selector: { app: "test" },
remoteNamespace: "",
});

expect(policy.metadata?.name).toEqual("Egress-test");
expect(policy.spec).toEqual({
egress: [
{
ports: [],
to: [{ namespaceSelector: {} }],
},
],
podSelector: { matchLabels: { app: "test" } },
policyTypes: ["Egress"],
} as kind.NetworkPolicy["spec"]);
});

const policyWildcard = generate("test", {
description: "test",
direction: Direction.Egress,
selector: { app: "test" },
remoteNamespace: "*",
});

expect(policyWildcard.spec).toEqual({
egress: [
{
ports: [],
to: [{ namespaceSelector: {} }],
},
],
podSelector: { matchLabels: { app: "test" } },
policyTypes: ["Egress"],
} as kind.NetworkPolicy["spec"]);
});
105 changes: 53 additions & 52 deletions src/pepr/operator/controllers/network/generate.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { V1LabelSelector, V1NetworkPolicyPeer, V1NetworkPolicyPort } from "@kubernetes/client-node";
import { V1NetworkPolicyPeer, V1NetworkPolicyPort } from "@kubernetes/client-node";
import { kind } from "pepr";

import { Allow, RemoteGenerated } from "../../crd";
Expand All @@ -7,6 +7,56 @@ import { cloudMetadata } from "./generators/cloudMetadata";
import { intraNamespace } from "./generators/intraNamespace";
import { kubeAPI } from "./generators/kubeAPI";

function isWildcardNamespace(namespace: string) {
return namespace === "" || namespace === "*";
}

function getPeers(policy: Allow): V1NetworkPolicyPeer[] {
let peers: V1NetworkPolicyPeer[] = [];

if (policy.remoteGenerated) {
switch (policy.remoteGenerated) {
case RemoteGenerated.KubeAPI:
peers = kubeAPI();
break;

case RemoteGenerated.CloudMetadata:
peers = cloudMetadata;
break;

case RemoteGenerated.IntraNamespace:
peers = [intraNamespace];
break;

case RemoteGenerated.Anywhere:
peers = [anywhere];
break;
}
} else if (policy.remoteNamespace !== undefined || policy.remoteSelector !== undefined) {
const peer: V1NetworkPolicyPeer = {};

if (policy.remoteNamespace !== undefined) {
if (isWildcardNamespace(policy.remoteNamespace)) {
peer.namespaceSelector = {};
} else {
peer.namespaceSelector = {
matchLabels: { "kubernetes.io/metadata.name": policy.remoteNamespace },
};
}
}

if (policy.remoteSelector !== undefined) {
peer.podSelector = {
matchLabels: policy.remoteSelector,
};
}

peers.push(peer);
}

return peers;
}

export function generate(namespace: string, policy: Allow): kind.NetworkPolicy {
// Generate a unique name for the NetworkPolicy
const name = generateName(policy);
Expand Down Expand Up @@ -35,57 +85,8 @@ export function generate(namespace: string, policy: Allow): kind.NetworkPolicy {
};
}

// Create the remote (peer) to match against
let peers: V1NetworkPolicyPeer[] = [];

// Add the remoteNamespace if they exist
if (policy.remoteNamespace !== undefined) {
const namespaceSelector: V1LabelSelector = {};

// Add the remoteNamespace to the namespaceSelector if it exists and is not "*", otherwise match all namespaces
if (policy.remoteNamespace !== "" && policy.remoteNamespace !== "*") {
namespaceSelector.matchLabels = {
"kubernetes.io/metadata.name": policy.remoteNamespace,
};
}

// Add the remoteNamespace to the peers
peers.push({ namespaceSelector });
}

// Add the remoteSelector if they exist
if (policy.remoteSelector) {
peers.push({
podSelector: {
matchLabels: policy.remoteSelector,
},
});
}

// Check if remoteGenerated is set
if (policy.remoteGenerated) {
// Add the remoteGenerated label
generated.metadata!.labels!["uds/generated"] = policy.remoteGenerated;

// Check if remoteGenerated is set
switch (policy.remoteGenerated) {
case RemoteGenerated.KubeAPI:
peers = kubeAPI();
break;

case RemoteGenerated.CloudMetadata:
peers = cloudMetadata;
break;

case RemoteGenerated.IntraNamespace:
peers.push(intraNamespace);
break;

case RemoteGenerated.Anywhere:
peers = [anywhere];
break;
}
}
// Create the network policy peers
const peers: V1NetworkPolicyPeer[] = getPeers(policy);

// Define the ports to allow from the ports property
const ports: V1NetworkPolicyPort[] = (policy.ports ?? []).map(port => ({ port }));
Expand Down

0 comments on commit 9b14c2c

Please sign in to comment.