Skip to content

Commit

Permalink
Fix target connect url (metalbear-co#3070)
Browse files Browse the repository at this point in the history
* Fixed target url produced in OperatorApi

* Changelog

* fmt
  • Loading branch information
Razz4780 authored Feb 6, 2025
1 parent 4988a44 commit ac9d3dc
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 9 deletions.
1 change: 1 addition & 0 deletions changelog.d/+target-container.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed operator connect URL produced by the CLI when a target container is specified.
113 changes: 104 additions & 9 deletions mirrord/operator/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,8 +626,15 @@ impl OperatorApi<PreparedClientCert> {
}
}

let use_proxy = self
.operator
.spec
.supported_features()
.contains(&NewOperatorFeature::ProxyApi);

(
self.target_connect_url(
Self::target_connect_url(
use_proxy,
&target,
layer_config.feature.network.incoming.on_concurrent_steal,
),
Expand Down Expand Up @@ -664,24 +671,18 @@ impl OperatorApi<PreparedClientCert> {

/// Produces the URL for making a connection request to the operator.
fn target_connect_url(
&self,
use_proxy: bool,
target: &ResolvedTarget<true>,
concurrent_steal: ConcurrentSteal,
) -> String {
let use_proxy = self
.operator
.spec
.supported_features()
.contains(&NewOperatorFeature::ProxyApi);

let name = {
let mut urlfied_name = target.type_().to_string();
if let Some(target_name) = target.name() {
urlfied_name.push('.');
urlfied_name.push_str(target_name);
}
if let Some(target_container) = target.container() {
urlfied_name.push('.');
urlfied_name.push_str(".container.");
urlfied_name.push_str(target_container);
}
urlfied_name
Expand Down Expand Up @@ -826,3 +827,97 @@ impl OperatorApi<PreparedClientCert> {
))
}
}

#[cfg(test)]
mod test {
use k8s_openapi::api::apps::v1::Deployment;
use kube::api::ObjectMeta;
use mirrord_config::feature::network::incoming::ConcurrentSteal;
use mirrord_kube::resolved::{ResolvedResource, ResolvedTarget};
use rstest::rstest;

use super::OperatorApi;

/// Verifies that [`OperatorApi::target_connect_url`] produces expected URLs.
///
/// These URLs should not change for backward compatibility.
#[rstest]
#[case::deployment_no_container_no_proxy(
false,
ResolvedTarget::Deployment(ResolvedResource {
resource: Deployment {
metadata: ObjectMeta {
name: Some("py-serv-deployment".into()),
namespace: Some("default".into()),
..Default::default()
},
spec: None,
status: None,
},
container: None,
}),
ConcurrentSteal::Abort,
"/apis/operator.metalbear.co/v1/namespaces/default/targets/deployment.py-serv-deployment?on_concurrent_steal=abort&connect=true"
)]
#[case::deployment_no_container_proxy(
true,
ResolvedTarget::Deployment(ResolvedResource {
resource: Deployment {
metadata: ObjectMeta {
name: Some("py-serv-deployment".into()),
namespace: Some("default".into()),
..Default::default()
},
spec: None,
status: None,
},
container: None,
}),
ConcurrentSteal::Abort,
"/apis/operator.metalbear.co/v1/proxy/namespaces/default/targets/deployment.py-serv-deployment?on_concurrent_steal=abort&connect=true"
)]
#[case::deployment_container_no_proxy(
false,
ResolvedTarget::Deployment(ResolvedResource {
resource: Deployment {
metadata: ObjectMeta {
name: Some("py-serv-deployment".into()),
namespace: Some("default".into()),
..Default::default()
},
spec: None,
status: None,
},
container: Some("py-serv".into()),
}),
ConcurrentSteal::Abort,
"/apis/operator.metalbear.co/v1/namespaces/default/targets/deployment.py-serv-deployment.container.py-serv?on_concurrent_steal=abort&connect=true"
)]
#[case::deployment_container_proxy(
true,
ResolvedTarget::Deployment(ResolvedResource {
resource: Deployment {
metadata: ObjectMeta {
name: Some("py-serv-deployment".into()),
namespace: Some("default".into()),
..Default::default()
},
spec: None,
status: None,
},
container: Some("py-serv".into()),
}),
ConcurrentSteal::Abort,
"/apis/operator.metalbear.co/v1/proxy/namespaces/default/targets/deployment.py-serv-deployment.container.py-serv?on_concurrent_steal=abort&connect=true"
)]
#[test]
fn target_connect_url(
#[case] use_proxy: bool,
#[case] target: ResolvedTarget<true>,
#[case] concurrent_steal: ConcurrentSteal,
#[case] expected: &str,
) {
let produced = OperatorApi::target_connect_url(use_proxy, &target, concurrent_steal);
assert_eq!(produced, expected,)
}
}

0 comments on commit ac9d3dc

Please sign in to comment.