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

policy: Serve EgressNetwork responses #13206

Merged
merged 11 commits into from
Oct 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
3 changes: 1 addition & 2 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1325,8 +1325,7 @@ dependencies = [
[[package]]
name = "linkerd2-proxy-api"
version = "0.14.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "26c72fb98d969e1e94e95d52a6fcdf4693764702c369e577934256e72fb5bc61"
source = "git+https://github.com/linkerd/linkerd2-proxy-api?branch=zd/add-error-type-to-tls-and-tcp-route#d7385b43d087da05ef4356e51d81be0a1a736a14"
dependencies = [
"http",
"ipnet",
Expand Down
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ members = [

[profile.release]
lto = "thin"

[patch.crates-io]
linkerd2-proxy-api = { git = 'https://github.com/linkerd/linkerd2-proxy-api', branch = 'zd/add-error-type-to-tls-and-tcp-route' }
91 changes: 86 additions & 5 deletions policy-controller/core/src/outbound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,16 @@ use ahash::AHashMap as HashMap;
use anyhow::Result;
use chrono::{offset::Utc, DateTime};
use futures::prelude::*;
use std::{net::IpAddr, num::NonZeroU16, pin::Pin, time};
use std::{
net::{IpAddr, SocketAddr},
num::NonZeroU16,
pin::Pin,
time,
};

pub trait Route {
fn creation_timestamp(&self) -> Option<DateTime<Utc>>;
}

/// Models outbound policy discovery.
#[async_trait::async_trait]
Expand All @@ -22,20 +31,40 @@ pub type OutboundPolicyStream = Pin<Box<dyn Stream<Item = OutboundPolicy> + Send

pub type HttpRoute = OutboundRoute<HttpRouteMatch, HttpRetryCondition>;
pub type GrpcRoute = OutboundRoute<GrpcRouteMatch, GrpcRetryCondition>;

pub type RouteSet<T> = HashMap<GroupKindNamespaceName, T>;

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
pub enum TrafficPolicy {
Allow,
Deny,
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
pub enum Kind {
EgressNetwork {
original_dst: SocketAddr,
traffic_policy: TrafficPolicy,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised to see TrafficPolicy here in the discover target. I think this means that the TrafficPolicy is looked up once at the beginning of the discovery lookup but then never updated. If the EgressNetwork resource is edited to change the TrafficPolicy, I don't think the discovery watches will see that change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually leads to a more difficult question about EgressNetwork mutability and ip address.

For services, a service's IP address will never change for the life of that services which means we can look up the service by IP once at the beginning of the discovery stream and never again.

For EgressNetworks, on the other hand, they encompass a range of IP addresses and that range can change if the EgressNetwork resource is edited. What happens when a discovery stream is started on an IP address that is in an EgressNetwork, but then the EgressNetwork is edited to no longer include that IP in it's network range? Or vice versa, what if an EgressNetwork is edited to include an IP address that was used for an already started discovery stream?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It really seems like we need to take a different approach for EgressNetworks compared to the approach we have for services in the outbound index. Perhaps we can take advantage of the fact that cluster networks are immutable. When a discovery lookup comes in, we can look at if it's in the cluster network and send it to the service index or if it's out of the cluster network, then we can send it to the egress index.

The egress index will likely need to have watches indexed by ip address so that we can update the appropriate watches whenever an EgressNetwork resource changes and may encompass watches that it did not before or stop encompassing watches that it used to.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points.. Will think about it a bit more.

},
Service,
}

#[derive(Clone, Debug)]
pub struct OutboundDiscoverTarget {
pub service_name: String,
pub service_namespace: String,
pub service_port: NonZeroU16,
pub name: String,
pub namespace: String,
pub port: NonZeroU16,
pub source_namespace: String,
pub kind: Kind,
}

#[derive(Clone, Debug, PartialEq)]
pub struct OutboundPolicy {
pub http_routes: RouteSet<HttpRoute>,
pub grpc_routes: RouteSet<GrpcRoute>,
pub authority: String,
pub tls_routes: RouteSet<TlsRoute>,
pub tcp_routes: RouteSet<TcpRoute>,
pub service_authority: String,
pub name: String,
pub namespace: String,
pub port: NonZeroU16,
Expand All @@ -56,6 +85,24 @@ pub struct OutboundRoute<M, R> {
pub creation_timestamp: Option<DateTime<Utc>>,
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct TlsRoute {
pub hostnames: Vec<HostMatch>,
pub rule: TcpRouteRule,
/// This is required for ordering returned routes
/// by their creation timestamp.
pub creation_timestamp: Option<DateTime<Utc>>,
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct TcpRoute {
pub rule: TcpRouteRule,

/// This is required for ordering returned routes
/// by their creation timestamp.
pub creation_timestamp: Option<DateTime<Utc>>,
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct OutboundRouteRule<M, R> {
pub matches: Vec<M>,
Expand All @@ -65,10 +112,16 @@ pub struct OutboundRouteRule<M, R> {
pub filters: Vec<Filter>,
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct TcpRouteRule {
pub backends: Vec<Backend>,
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub enum Backend {
Addr(WeightedAddr),
Service(WeightedService),
EgressNetwork(WeightedEgressNetwork),
Invalid { weight: u32, message: String },
}

Expand All @@ -90,6 +143,16 @@ pub struct WeightedService {
pub exists: bool,
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct WeightedEgressNetwork {
pub weight: u32,
pub name: String,
pub namespace: String,
pub port: Option<NonZeroU16>,
pub filters: Vec<Filter>,
pub exists: bool,
}

#[derive(Copy, Clone, Debug, PartialEq)]
pub enum FailureAccrual {
Consecutive { max_failures: u32, backoff: Backoff },
Expand Down Expand Up @@ -138,3 +201,21 @@ pub enum GrpcRetryCondition {
Internal,
Unavailable,
}

impl<M, R> Route for OutboundRoute<M, R> {
fn creation_timestamp(&self) -> Option<DateTime<Utc>> {
self.creation_timestamp
}
}

impl Route for TcpRoute {
fn creation_timestamp(&self) -> Option<DateTime<Utc>> {
self.creation_timestamp
}
}

impl Route for TlsRoute {
fn creation_timestamp(&self) -> Option<DateTime<Utc>> {
self.creation_timestamp
}
}
Loading
Loading