Skip to content

Commit

Permalink
Config: New incluster and incluster_dns constructors (#1001)
Browse files Browse the repository at this point in the history 
* client: Expose a Config constructor to support legacy configurations

The `Config::from_cluster_env` constructor is misleadingly named: it
doesn't use the environment, it uses the default cluster configurations.

This change deprecates the `Config::from_cluster_env` constructor in
favor of `Config::load_in_cluster`. An additional constructor,
`Config::load_in_cluster_from_legacy_env`, uses the
`KUBERNETES_SERVICE_HOST` and `KUBERNETES_SERVICE_PORT` environment
variables to match client-go's behavior.

This changes does NOT alter the default inferred configuration in any
way. It simply allows users to opt-in to using the old behavior.

Related to kubernetes/kubernetes#112263
Closes #1000

Signed-off-by: Oliver Gould <ver@buoyant.io>

* constify "https" scheme to make accidental "http" harder

Signed-off-by: Oliver Gould <ver@buoyant.io>

* Restore `Config::from_cluster_env` naming

Add `Config::from_cluster_dns` to support the current behavior.

Signed-off-by: Oliver Gould <ver@buoyant.io>

* Disable the in-cluster rustls test

Signed-off-by: Oliver Gould <ver@buoyant.io>

* fix typo

Signed-off-by: Oliver Gould <ver@buoyant.io>

* client: Make discovery conditional on the TLS impl

When `rustls-tls` is enabled, the `kubernetes.default.svc` DNS name is
used. Otherwise, the `KUBERNETES_SERVICE_{HOST,PORT}` environment
variables are used.

Signed-off-by: Oliver Gould <ver@buoyant.io>

* Review feedback

* Make `Config::incluster_env` and `Config::incluster_dns` public
  regardless of what features are enabled.
* Restrict visibility for `pub` helpers that are not actually publicly
  exported.

Signed-off-by: Oliver Gould <ver@buoyant.io>

* Add URI-formatting tests

Signed-off-by: Oliver Gould <ver@buoyant.io>

* fmt

Signed-off-by: Oliver Gould <ver@buoyant.io>

Signed-off-by: Oliver Gould <ver@buoyant.io>
  • Loading branch information
@olix0r
olix0r committed Sep 9, 2022
1 parent b757859 commit 79e4e09
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 17 deletions.
2 changes: 1 addition & 1 deletion .devcontainer/devcontainer.json
View file
Expand Up @@ -8,7 +8,7 @@

// These extensions are loaded for all users by default.
"extensions": [
"matklad.rust-analyzer",
"rust-lang.rust-analyzer",
"NathanRidley.autotrim",
"samverschueren.final-newline",
"tamasfe.even-better-toml",
Expand Down
109 changes: 108 additions & 1 deletion kube-client/src/config/incluster_config.rs
View file
@@ -1,5 +1,9 @@
use std::env;
use thiserror::Error;

const SERVICE_HOSTENV: &str = "KUBERNETES_SERVICE_HOST";
const SERVICE_PORTENV: &str = "KUBERNETES_SERVICE_PORT";

// Mounted credential files
const SERVICE_TOKENFILE: &str = "/var/run/secrets/kubernetes.io/serviceaccount/token";
const SERVICE_CERTFILE: &str = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt";
Expand All @@ -12,10 +16,18 @@ pub enum Error {
#[error("failed to read the default namespace: {0}")]
ReadDefaultNamespace(#[source] std::io::Error),

/// Failed to read the in-cluster environment variables
#[error("failed to read an incluster environment variable: {0}")]
ReadEnvironmentVariable(#[source] env::VarError),

/// Failed to read a certificate bundle
#[error("failed to read a certificate bundle: {0}")]
ReadCertificateBundle(#[source] std::io::Error),

/// Failed to parse cluster port value
#[error("failed to parse cluster port: {0}")]
ParseClusterPort(#[source] std::num::ParseIntError),

/// Failed to parse cluster url
#[error("failed to parse cluster url: {0}")]
ParseClusterUrl(#[source] http::uri::InvalidUri),
Expand All @@ -25,10 +37,56 @@ pub enum Error {
ParseCertificates(#[source] pem::PemError),
}

pub fn kube_dns() -> http::Uri {
/// Returns the URI of the Kubernetes API server using the in-cluster DNS name
/// `kubernetes.default.svc`.
pub(super) fn kube_dns() -> http::Uri {
http::Uri::from_static("https://kubernetes.default.svc/")
}

/// Returns the URI of the Kubernetes API server by reading the
/// `KUBERNETES_SERVICE_HOST` and `KUBERNETES_SERVICE_PORT` environment
/// variables.
pub(super) fn try_kube_from_env() -> Result<http::Uri, Error> {
// client-go requires that both environment variables are set.
let host = env::var(SERVICE_HOSTENV).map_err(Error::ReadEnvironmentVariable)?;
let port = env::var(SERVICE_PORTENV)
.map_err(Error::ReadEnvironmentVariable)?
.parse::<u16>()
.map_err(Error::ParseClusterPort)?;

try_uri(&host, port)
}

fn try_uri(host: &str, port: u16) -> Result<http::Uri, Error> {
// Format a host and, if not using 443, a port.
//
// Ensure that IPv6 addresses are properly bracketed.
const HTTPS: &str = "https";
let uri = match host.parse::<std::net::IpAddr>() {
Ok(ip) => {
if port == 443 {
if ip.is_ipv6() {
format!("{HTTPS}://[{ip}]")
} else {
format!("{HTTPS}://{ip}")
}
} else {
let addr = std::net::SocketAddr::new(ip, port);
format!("{HTTPS}://{addr}")
}
}
Err(_) => {
if port == 443 {
format!("{HTTPS}://{host}")
} else {
format!("{HTTPS}://{host}:{port}")
}
}
};

uri.parse().map_err(Error::ParseClusterUrl)
}

pub fn token_file() -> String {
SERVICE_TOKENFILE.to_owned()
}
Expand All @@ -43,3 +101,52 @@ pub fn load_cert() -> Result<Vec<Vec<u8>>, Error> {
pub fn load_default_ns() -> Result<String, Error> {
std::fs::read_to_string(SERVICE_DEFAULT_NS).map_err(Error::ReadDefaultNamespace)
}

#[test]
fn test_kube_name() {
assert_eq!(
try_uri("fake.io", 8080).unwrap().to_string(),
"https://fake.io:8080/"
);
}

#[test]
fn test_kube_name_default_port() {
assert_eq!(try_uri("kubernetes.default.svc", 443).unwrap(), kube_dns())
}

#[test]
fn test_kube_ipv4() {
assert_eq!(
try_uri("10.11.12.13", 6443).unwrap().to_string(),
"https://10.11.12.13:6443/"
);
}

#[test]
fn test_kube_ipv4_default_port() {
assert_eq!(
try_uri("10.11.12.13", 443).unwrap().to_string(),
"https://10.11.12.13/"
);
}

#[test]
fn test_kube_ipv6() {
assert_eq!(
try_uri("2001:0db8:85a3:0000:0000:8a2e:0370:7334", 6443)
.unwrap()
.to_string(),
"https://[2001:db8:85a3::8a2e:370:7334]:6443/"
);
}

#[test]
fn test_kube_ipv6_default_port() {
assert_eq!(
try_uri("2001:0db8:85a3:0000:0000:8a2e:0370:7334", 443)
.unwrap()
.to_string(),
"https://[2001:db8:85a3::8a2e:370:7334]/"
);
}
69 changes: 54 additions & 15 deletions kube-client/src/config/mod.rs
View file
Expand Up @@ -179,14 +179,15 @@ impl Config {
}
}

/// Infer the configuration from the environment
/// Infer a Kubernetes client configuration.
///
/// Done by attempting to load the local kubec-config first, and
/// then if that fails, trying the in-cluster environment variables .
/// First, a user's kubeconfig is loaded from `KUBECONFIG` or
/// `~/.kube/config`. If that fails, an in-cluster config is loaded via
/// [`Config::incluster`]. If inference from both sources fails, then an
/// error is returned.
///
/// Fails if inference from both sources fails
///
/// Applies debug overrides, see [`Config::apply_debug_overrides`] for more details
/// [`Config::apply_debug_overrides`] is used to augment the loaded
/// configuration based on the environment.
pub async fn infer() -> Result<Self, InferConfigError> {
let mut config = match Self::from_kubeconfig(&KubeConfigOptions::default()).await {
Err(kubeconfig_err) => {
Expand All @@ -195,8 +196,8 @@ impl Config {
"no local config found, falling back to local in-cluster config"
);

Self::from_cluster_env().map_err(|in_cluster_err| InferConfigError {
in_cluster: in_cluster_err,
Self::incluster().map_err(|in_cluster| InferConfigError {
in_cluster,
kubeconfig: kubeconfig_err,
})?
}
Expand All @@ -206,13 +207,52 @@ impl Config {
Ok(config)
}

/// Create configuration from the cluster's environment variables
/// Load an in-cluster Kubernetes client configuration using
/// [`Config::incluster_env`].
#[cfg(not(feature = "rustls-tls"))]
pub fn incluster() -> Result<Self, InClusterError> {
Self::incluster_env()
}

/// Load an in-cluster Kubernetes client configuration using
/// [`Config::incluster_dns`].
///
/// The `rustls-tls` feature is currently incompatible with
/// [`Config::incluster_env`]. See
/// <https://github.com/kube-rs/kube-rs/issues/1003>.
#[cfg(feature = "rustls-tls")]
pub fn incluster() -> Result<Self, InClusterError> {
Self::incluster_dns()
}

/// Load an in-cluster config using the `KUBERNETES_SERVICE_HOST` and
/// `KUBERNETES_SERVICE_PORT` environment variables.
///
/// A service account's token must be available in
/// `/var/run/secrets/kubernetes.io/serviceaccount/`.
///
/// This method matches the behavior of the official Kubernetes client
/// libraries, but it is not compatible with the `rustls-tls` feature . When
/// this feature is enabled, [`Config::incluster_dns`] should be used
/// instead. See <https://github.com/kube-rs/kube-rs/issues/1003>.
pub fn incluster_env() -> Result<Self, InClusterError> {
let uri = incluster_config::try_kube_from_env()?;
Self::incluster_with_uri(uri)
}

/// Load an in-cluster config using the API server at
/// `https://kubernetes.default.svc`.
///
/// A service account's token must be available in
/// `/var/run/secrets/kubernetes.io/serviceaccount/`.
///
/// This follows the standard [API Access from a Pod](https://kubernetes.io/docs/tasks/access-application-cluster/access-cluster/#accessing-the-api-from-a-pod)
/// and relies on you having the service account's token mounted,
/// as well as having given the service account rbac access to do what you need.
pub fn from_cluster_env() -> Result<Self, InClusterError> {
let cluster_url = incluster_config::kube_dns();
/// This behavior does not match that of the official Kubernetes clients,
/// but this approach is compatible with the `rustls-tls` feature.
pub fn incluster_dns() -> Result<Self, InClusterError> {
Self::incluster_with_uri(incluster_config::kube_dns())
}

fn incluster_with_uri(cluster_url: http::uri::Uri) -> Result<Self, InClusterError> {
let default_namespace = incluster_config::load_default_ns()?;
let root_cert = incluster_config::load_cert()?;

Expand Down Expand Up @@ -378,7 +418,6 @@ pub use file_config::{
NamedContext, NamedExtension, Preferences,
};


#[cfg(test)]
mod tests {
#[cfg(not(feature = "client"))] // want to ensure this works without client features
Expand Down

0 comments on commit 79e4e09

Please sign in to comment.