Skip to content

Commit

Permalink
update to only remove nodes if they are being deleted
Browse files Browse the repository at this point in the history
Signed-off-by: catsby <[email protected]>
  • Loading branch information
catsby committed Dec 19, 2024
1 parent 5fd238c commit 1d4f0af
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 21 deletions.
19 changes: 19 additions & 0 deletions docs/reference/configuration/uds-networking-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,25 @@ This configuration directs the operator to use the specified CIDR range (`172.0.

When configuring a static CIDR range, it is important to make the range as restrictive as possible to limit the potential for unexpected networking access. An overly broad range could inadvertently allow egress traffic to destinations beyond the intended scope. Additionally, careful alignment with the actual IP addresses used by the Kubernetes API server is essential. A mismatch between the specified CIDR range and the cluster's configuration can result in network policy enforcement issues or disrupted connectivity.

## KubeNodes CIDRs

The UDS operator is responsible for dynamically updating network policies that use the `remoteGenerated: KubeNodes` custom selector, in response to changes to nodes in the Kubernetes cluster. As nodes are added, updated, or removed from a cluster, the operator will ensure that policies remain accurate and include all the nodes in the cluster.

UDS operator provides an option to configure a set of static CIDR ranges in place of offering a dynamically updated list by setting an override to `operator.KUBENODE_CIDRS` in your bundle as a value or variable. The value should be a single string of comma (`,`) separated values for the individual IP addresses, using `/32` notation. For example:

```yaml
packages:
- name: uds-core
repository: ghcr.io/defenseunicorns/packages/uds/core
ref: x.x.x
overrides:
uds-operator-config:
uds-operator-config:
values:
- path: operator.KUBENODE_CIDRS
value: "172.28.0.2/32,172.28.0.3/32,172.28.0./32"
```

## Additional Network Allowances

Applications deployed in UDS Core utilize [Network Policies](https://kubernetes.io/docs/concepts/services-networking/network-policies/) with a "Deny by Default" configuration to ensure network traffic is restricted to only what is necessary. Some applications in UDS Core allow for overrides to accommodate environment-specific requirements.
Expand Down
4 changes: 2 additions & 2 deletions src/pepr/operator/controllers/network/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { anywhere, anywhereInCluster } from "./generators/anywhere";
import { cloudMetadata } from "./generators/cloudMetadata";
import { intraNamespace } from "./generators/intraNamespace";
import { kubeAPI } from "./generators/kubeAPI";
import { nodeCIDRs } from "./generators/kubeNodes";
import { kubeNodes } from "./generators/kubeNodes";
import { remoteCidr } from "./generators/remoteCidr";

function isWildcardNamespace(namespace: string) {
Expand All @@ -28,7 +28,7 @@ function getPeers(policy: Allow): V1NetworkPolicyPeer[] {
break;

case RemoteGenerated.KubeNodes:
peers = nodeCIDRs();
peers = kubeNodes();
break;

case RemoteGenerated.CloudMetadata:
Expand Down
31 changes: 18 additions & 13 deletions src/pepr/operator/controllers/network/generators/kubeNodes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { beforeEach, beforeAll, describe, expect, it, jest } from "@jest/globals

import {
initAllNodesTarget,
nodeCIDRs,
kubeNodes,
updateKubeNodesFromCreateUpdate,
updateKubeNodesFromDelete,
} from "./kubeNodes";
Expand Down Expand Up @@ -97,7 +97,7 @@ describe("kubeNodes module", () => {
it("should initialize nodeSet with internal IPs from nodes", async () => {
mockK8sGetNodes.mockResolvedValue(mockNodeList);
await initAllNodesTarget();
const cidrs = nodeCIDRs();
const cidrs = kubeNodes();
// Should have two IPs from mockNodeList
expect(cidrs).toHaveLength(2);
expect(cidrs).toEqual(
Expand All @@ -113,7 +113,7 @@ describe("kubeNodes module", () => {
it("should return anywhere if no nodes known", async () => {
mockK8sGetNodes.mockResolvedValue({ items: [] });
await initAllNodesTarget();
const cidrs = nodeCIDRs();
const cidrs = kubeNodes();
// expect it to match "anywhere"
expect(cidrs).toEqual([anywhere]);
});
Expand All @@ -125,13 +125,13 @@ describe("kubeNodes module", () => {
mockGetNetworkPolicies.mockResolvedValue(mockNetworkPolicyList);
await initAllNodesTarget(); // start empty
await updateKubeNodesFromCreateUpdate(mockNodeList.items[0]);
let cidrs = nodeCIDRs();
let cidrs = kubeNodes();
expect(cidrs).toHaveLength(1);
expect(cidrs[0].ipBlock?.cidr).toBe("10.0.0.1/32");
expect(mockApply).toHaveBeenCalled();

await updateKubeNodesFromCreateUpdate(mockNodeList.items[1]);
cidrs = nodeCIDRs();
cidrs = kubeNodes();
expect(cidrs).toHaveLength(2);
expect(cidrs[1].ipBlock?.cidr).toBe("10.0.0.2/32");
expect(mockApply).toHaveBeenCalled();
Expand All @@ -148,15 +148,15 @@ describe("kubeNodes module", () => {
mockK8sGetNodes.mockResolvedValueOnce({ items: [] });
await initAllNodesTarget(); // start empty
await updateKubeNodesFromCreateUpdate(notReadyNode);
const cidrs = nodeCIDRs();
const cidrs = kubeNodes();
expect(cidrs).toEqual([anywhere]);
expect(mockApply).toHaveBeenCalled(); // Still called to update polices even if empty
});

it("should remove a node that's no longer ready", async () => {
it("should not remove a node that's no longer ready", async () => {
mockK8sGetNodes.mockResolvedValue(mockNodeList);
await initAllNodesTarget();
let cidrs = nodeCIDRs();
let cidrs = kubeNodes();
// Should have two IPs from mockNodeList
expect(cidrs).toHaveLength(2);
expect(cidrs).toEqual(
Expand All @@ -174,9 +174,14 @@ describe("kubeNodes module", () => {
},
};
await updateKubeNodesFromCreateUpdate(notReadyNode);
cidrs = nodeCIDRs();
expect(cidrs).toHaveLength(1);
expect(cidrs).toEqual(expect.arrayContaining([{ ipBlock: { cidr: "10.0.0.2/32" } }]));
cidrs = kubeNodes();
expect(cidrs).toHaveLength(2);
expect(cidrs).toEqual(
expect.arrayContaining([
{ ipBlock: { cidr: "10.0.0.1/32" } },
{ ipBlock: { cidr: "10.0.0.2/32" } },
]),
);
expect(mockApply).toHaveBeenCalled(); // Still called to update polices even if empty
});
});
Expand All @@ -185,11 +190,11 @@ describe("kubeNodes module", () => {
it("should remove the node IP from nodeSet", async () => {
mockK8sGetNodes.mockResolvedValueOnce(mockNodeList);
await initAllNodesTarget();
const cidrsBeforeDelete = nodeCIDRs();
const cidrsBeforeDelete = kubeNodes();
expect(cidrsBeforeDelete).toHaveLength(2);

await updateKubeNodesFromDelete(mockNodeList.items[0]);
const cidrsAfterDelete = nodeCIDRs();
const cidrsAfterDelete = kubeNodes();
expect(cidrsAfterDelete).toHaveLength(1);
expect(cidrsAfterDelete[0].ipBlock?.cidr).toBe("10.0.0.2/32");
expect(mockApply).toHaveBeenCalled();
Expand Down
10 changes: 5 additions & 5 deletions src/pepr/operator/controllers/network/generators/kubeNodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export async function initAllNodesTarget() {
* Returns the egress CIDRs of all known nodes as network policy peers.
* If none are known, defaults to 0.0.0.0/0 and logs a warning.
*/
export function nodeCIDRs(): V1NetworkPolicyPeer[] {
export function kubeNodes(): V1NetworkPolicyPeer[] {
const policies = buildNodePolicies([...nodeSet]);
if (policies.length > 0) return policies;

Expand All @@ -66,12 +66,12 @@ export async function updateKubeNodesFromCreateUpdate(node: kind.Node) {
);

const ip = getNodeInternalIP(node);
// in order to be added to the node set, the node must be ready. Additional
// status conditions will not cause the node to be removed however, e.g. disck
// pressure, memory pressure, etc. Nodes are only removed from the node set
// when they are deleted, and done so in the updateKubeNodesFromDelete function.
if (isReady) {
if (ip) nodeSet.add(ip);
} else {
// it's possible that the node is not ready, but it has an IP, so we remove
// it for now
if (ip) nodeSet.delete(ip);
}

await updateKubeNodesNetworkPolicies();
Expand Down
2 changes: 1 addition & 1 deletion src/pepr/operator/controllers/network/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export async function networkPolicies(pkg: UDSPackage, namespace: string) {
}
if (
UDSConfig.kubeNodeCidrs &&
policy.metadata.labels["uds/generated"] === RemoteGenerated.KubeAPI
policy.metadata.labels["uds/generated"] === RemoteGenerated.KubeNodes
) {
message +=
", ensure that the KUBENODE_CIDRS override configured for the operator is correct.";
Expand Down

0 comments on commit 1d4f0af

Please sign in to comment.