From 6103a8883b6debe2ed6b8a48024564e09c064d5f Mon Sep 17 00:00:00 2001 From: Lleyton Gray Date: Tue, 19 Dec 2023 20:11:14 -0800 Subject: [PATCH] fix: dissallow multiple services from binding to same exitnode uwu Co-authored-by: Cappy Ishihara --- deploy/crd/exit-node.yaml | 135 +++++++++++++++++++----------------- src/cloud/aws.rs | 1 + src/cloud/digitalocean.rs | 1 + src/cloud/linode.rs | 1 + src/daemon.rs | 140 ++++++++++++++++++++++++++++++-------- src/ops.rs | 7 ++ 6 files changed, 193 insertions(+), 92 deletions(-) diff --git a/deploy/crd/exit-node.yaml b/deploy/crd/exit-node.yaml index 63fe544..0b57d10 100644 --- a/deploy/crd/exit-node.yaml +++ b/deploy/crd/exit-node.yaml @@ -12,65 +12,76 @@ spec: singular: exitnode scope: Namespaced versions: - - additionalPrinterColumns: [] - name: v1 - schema: - openAPIV3Schema: - description: Auto-generated derived type for ExitNodeSpec via `CustomResource` - properties: - spec: - description: ExitNode is a custom resource that represents a Chisel exit node. It will be used as the reverse proxy for all services in the cluster. - properties: - auth: - description: Optional authentication secret name to connect to the control plane - nullable: true - type: string - default_route: - default: false - description: Optional boolean value for whether to make the exit node the default route for the cluster If true, the exit node will be the default route for the cluster default value is false - type: boolean - external_host: - description: Optional real external hostname/IP of exit node If not provided, the host field will be used - nullable: true - type: string - fingerprint: - description: Optional but highly recommended fingerprint to perform host-key validation against the server's public key - nullable: true - type: string - host: - description: Hostname or IP address of the chisel server - type: string - port: - description: Control plane port of the chisel server - format: uint16 - minimum: 0.0 - type: integer - required: - - host - - port - type: object - status: - nullable: true - properties: - id: - nullable: true - type: string - ip: - type: string - name: - type: string - provider: - type: string - required: - - ip - - name - - provider - type: object - required: - - spec - title: ExitNode - type: object - served: true - storage: true - subresources: - status: {} + - additionalPrinterColumns: [] + name: v1 + schema: + openAPIV3Schema: + description: Auto-generated derived type for ExitNodeSpec via `CustomResource` + properties: + spec: + description: ExitNode is a custom resource that represents a Chisel exit node. It will be used as the reverse proxy for all services in the cluster. + properties: + auth: + description: Optional authentication secret name to connect to the control plane + nullable: true + type: string + default_route: + default: false + description: Optional boolean value for whether to make the exit node the default route for the cluster If true, the exit node will be the default route for the cluster default value is false + type: boolean + external_host: + description: Optional real external hostname/IP of exit node If not provided, the host field will be used + nullable: true + type: string + fingerprint: + description: Optional but highly recommended fingerprint to perform host-key validation against the server's public key + nullable: true + type: string + host: + description: Hostname or IP address of the chisel server + type: string + port: + description: Control plane port of the chisel server + format: uint16 + minimum: 0.0 + type: integer + required: + - host + - port + type: object + status: + nullable: true + properties: + id: + nullable: true + type: string + ip: + type: string + name: + type: string + provider: + type: string + service_binding: + nullable: true + properties: + name: + type: string + namespace: + type: string + required: + - name + - namespace + type: object + required: + - ip + - name + - provider + type: object + required: + - spec + title: ExitNode + type: object + served: true + storage: true + subresources: + status: {} diff --git a/src/cloud/aws.rs b/src/cloud/aws.rs index 98db8e7..bc6cd1b 100644 --- a/src/cloud/aws.rs +++ b/src/cloud/aws.rs @@ -180,6 +180,7 @@ impl Provisioner for AWSProvisioner { ip: public_ip, id: Some(instance.instance_id.unwrap()), provider: provisioner.clone(), + service_binding: None }; Ok(exit_node) diff --git a/src/cloud/digitalocean.rs b/src/cloud/digitalocean.rs index 1c6d025..1157c5a 100644 --- a/src/cloud/digitalocean.rs +++ b/src/cloud/digitalocean.rs @@ -120,6 +120,7 @@ impl Provisioner for DigitalOceanProvisioner { ip: droplet_ip.clone(), id: Some(droplet.id.to_string()), provider: provisioner.clone(), + service_binding: None }; Ok(exit_node) diff --git a/src/cloud/linode.rs b/src/cloud/linode.rs index 23e9700..532ffa0 100644 --- a/src/cloud/linode.rs +++ b/src/cloud/linode.rs @@ -100,6 +100,7 @@ impl Provisioner for LinodeProvisioner { name: instance.label, provider: provisioner.to_string(), id: Some(instance.id.to_string()), + service_binding: None }; Ok(status) diff --git a/src/daemon.rs b/src/daemon.rs index 7fbce15..4132b88 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -47,7 +47,8 @@ use std::time::Duration; use tracing::{debug, error, info, instrument, warn}; use crate::ops::{ - ExitNode, ExitNodeProvisioner, ExitNodeSpec, EXIT_NODE_NAME_LABEL, EXIT_NODE_PROVISIONER_LABEL, + ExitNode, ExitNodeProvisioner, ExitNodeSpec, ExitNodeStatus, ServiceBinding, + EXIT_NODE_NAME_LABEL, EXIT_NODE_PROVISIONER_LABEL, }; use crate::{deployment::create_owned_deployment, error::ReconcileError}; #[allow(dead_code)] @@ -152,21 +153,26 @@ async fn select_exit_node_local( // todo: We would want to add a status for every exit node and only bind one service to one exit node // (one to one mapping) let nodes: Api = Api::all(ctx.client.clone()); - let node_list = nodes.list(&ListParams::default().timeout(30)).await?; + let node_list: kube::core::ObjectList = + nodes.list(&ListParams::default().timeout(30)).await?; node_list .items .into_iter() .filter(|node| { - // Is the ExitNode not cloud provisioned OR is status set - !node + // Is the ExitNode not cloud provisioned + (!node .metadata .annotations .as_ref() - .map(|annotations| { - annotations.contains_key("chisel-operator.io/exit-node-provisioner") - }) + .map(|annotations| annotations.contains_key(EXIT_NODE_PROVISIONER_LABEL)) .unwrap_or(false) - // || node.status.is_some() + // OR is status set + || node.status.is_some()) + && !node + .status + .as_ref() + .map(|s| s.service_binding.is_some()) + .unwrap_or(false) }) .collect::>() .first() @@ -310,13 +316,27 @@ async fn reconcile_svcs(obj: Arc, ctx: Arc) -> Result = Api::namespaced(ctx.client.clone(), &obj.namespace().unwrap()); + let nodes: Api = Api::all(ctx.client.clone()); let mut svc = services.get_status(&obj.name_any()).await?; let obj = svc.clone(); + let node_list = nodes.list(&ListParams::default().timeout(30)).await?; + let existing_node = node_list.iter().find(|node| { + node.status + .as_ref() + .and_then(|status| status.service_binding.as_ref()) + .map(|binding| { + binding.name == obj.name_any() && binding.namespace == obj.namespace().unwrap() + }) + .unwrap_or(false) + }); + let node = { - if check_service_managed(&obj).await { + if let Some(node) = existing_node { + node.clone() + } else if check_service_managed(&obj).await { let provisioner = obj .metadata .annotations @@ -353,18 +373,18 @@ async fn reconcile_svcs(obj: Arc, ctx: Arc) -> Result, ctx: Arc) -> Result = + Api::namespaced(ctx.client.clone(), &node.namespace().unwrap()); + + let node_data = serde_json::json!({ + "status": { + "service_binding": ServiceBinding { + namespace: obj.namespace().unwrap(), + name: obj.name_any() + } + } + }); + + let _nodes = namespaced_nodes + .patch_status( + // We can unwrap safely since Service is guaranteed to have a name + node.name_any().as_str(), + &serverside.clone(), + &Patch::Merge(&node_data), + ) + .await?; + + info!(status = ?node, "Patched status for ExitNode {}", node.name_any()); + // Update the status for the LoadBalancer service // The ExitNode IP will always be set, so it is safe to unwrap the host @@ -393,7 +438,7 @@ async fn reconcile_svcs(obj: Arc, ctx: Arc) -> Result = @@ -440,7 +485,7 @@ fn error_policy_exit_node( error!(err = ?err); Action::requeue(Duration::from_secs(5)) } - +const UNMANAGED_PROVISIONER: &str = "unmanaged"; #[instrument(skip(ctx))] async fn reconcile_nodes(obj: Arc, ctx: Arc) -> Result { info!("exit node reconcile request: {}", obj.name_any()); @@ -449,9 +494,39 @@ async fn reconcile_nodes(obj: Arc, ctx: Arc) -> Result = Api::namespaced(ctx.client.clone(), &obj.namespace().unwrap()); + + let mut exitnode_patchtmpl = nodes.get(&obj.name_any()).await?; + + // now we set the status, but the provisioner is unmanaged + // so we just copy the IP from the exit node config to the status + + let exit_node_ip = obj.get_host(); + + exitnode_patchtmpl.status = Some(ExitNodeStatus { + provider: UNMANAGED_PROVISIONER.to_string(), + name: obj.name_any(), + ip: exit_node_ip, + id: None, + service_binding: None, + }); + + let serverside = PatchParams::apply(OPERATOR_MANAGER).validation_strict(); + + let _node = nodes + .patch_status( + // We can unwrap safely since Service is guaranteed to have a name + &obj.name_any(), + &serverside.clone(), + &Patch::Merge(exitnode_patchtmpl), + ) + .await?; + + return Ok(Action::await_change()); + } let provisioner = obj .metadata @@ -466,8 +541,6 @@ async fn reconcile_nodes(obj: Arc, ctx: Arc) -> Result = Api::namespaced(ctx.client.clone(), &obj.namespace().unwrap()); - let mut exitnode_patchtmpl = exit_nodes.get(&obj.name_any()).await?; - let provisioner_api = provisioner.clone().spec.get_inner(); // finalizer for exit node @@ -493,14 +566,17 @@ async fn reconcile_nodes(obj: Arc, ctx: Arc) -> Result color_eyre::Result<()> { info!("Starting reconcilers..."); + // TODO: figure out how to do this in a single controller because there is a potential race where the exit node reconciler runs at the same time as the service one + // This is an issue because both of these functions patch the status of the exit node + // or if we can figure out a way to atomically patch the status of the exit node, that could be fine too, since both ops are just updates anyways lmfao + reconcilers.push( Controller::new(services, Config::default()) .watches( diff --git a/src/ops.rs b/src/ops.rs index 3d1b78a..c227ea8 100644 --- a/src/ops.rs +++ b/src/ops.rs @@ -120,6 +120,13 @@ pub struct ExitNodeStatus { // pub password: String, pub ip: String, pub id: Option, + pub service_binding: Option, +} +#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema)] + +pub struct ServiceBinding { + pub namespace: String, + pub name: String, } #[derive(Serialize, Deserialize, Debug, CustomResource, Clone, JsonSchema)]