Skip to content

Commit

Permalink
chore(policy): fix HttpLocalRateLimit type casing (#13324)
Browse files Browse the repository at this point in the history
Our other types (e.g. HttpRoute) follow the Rust convention of using "Http"
instead of "HTTP".

This commit updates the policy controller to reflect this convention.

No user-facing changes.
  • Loading branch information
olix0r authored Nov 14, 2024
1 parent 59be08e commit 6e38368
Show file tree
Hide file tree
Showing 12 changed files with 44 additions and 43 deletions.
2 changes: 1 addition & 1 deletion policy-controller/k8s/api/src/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub use self::{
network::{Cidr, Network},
network_authentication::{NetworkAuthentication, NetworkAuthenticationSpec},
ratelimit_policy::{
HTTPLocalRateLimitPolicy, HTTPLocalRateLimitPolicyStatus, Limit, Override,
HttpLocalRateLimitPolicy, HttpLocalRateLimitPolicyStatus, Limit, Override,
RateLimitPolicySpec,
},
server::{Server, ServerSpec},
Expand Down
5 changes: 3 additions & 2 deletions policy-controller/k8s/api/src/policy/ratelimit_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use k8s_openapi::apimachinery::pkg::apis::meta::v1::Condition;
group = "policy.linkerd.io",
version = "v1alpha1",
kind = "HTTPLocalRateLimitPolicy",
status = "HTTPLocalRateLimitPolicyStatus",
root = "HttpLocalRateLimitPolicy",
status = "HttpLocalRateLimitPolicyStatus",
namespaced
)]
#[serde(rename_all = "camelCase")]
Expand All @@ -21,7 +22,7 @@ pub struct RateLimitPolicySpec {

#[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema)]
#[serde(rename_all = "camelCase")]
pub struct HTTPLocalRateLimitPolicyStatus {
pub struct HttpLocalRateLimitPolicyStatus {
pub conditions: Vec<Condition>,
pub target_ref: LocalTargetRef,
}
Expand Down
6 changes: 3 additions & 3 deletions policy-controller/k8s/index/src/inbound/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -938,8 +938,8 @@ impl kubert::index::IndexNamespacedResource<k8s::policy::NetworkAuthentication>
}
}

impl kubert::index::IndexNamespacedResource<k8s::policy::HTTPLocalRateLimitPolicy> for Index {
fn apply(&mut self, policy: k8s::policy::HTTPLocalRateLimitPolicy) {
impl kubert::index::IndexNamespacedResource<k8s::policy::HttpLocalRateLimitPolicy> for Index {
fn apply(&mut self, policy: k8s::policy::HttpLocalRateLimitPolicy) {
let ns = policy.namespace().unwrap();
let name = policy.name_unchecked();
let _span = info_span!("apply", %ns, saz = %name).entered();
Expand All @@ -963,7 +963,7 @@ impl kubert::index::IndexNamespacedResource<k8s::policy::HTTPLocalRateLimitPolic

fn reset(
&mut self,
policies: Vec<k8s::policy::HTTPLocalRateLimitPolicy>,
policies: Vec<k8s::policy::HttpLocalRateLimitPolicy>,
deleted: HashMap<String, HashSet<String>>,
) {
let _span = info_span!("reset");
Expand Down
4 changes: 2 additions & 2 deletions policy-controller/k8s/index/src/inbound/ratelimit_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ impl Spec {
}
}

impl TryFrom<k8s::policy::HTTPLocalRateLimitPolicy> for Spec {
impl TryFrom<k8s::policy::HttpLocalRateLimitPolicy> for Spec {
type Error = anyhow::Error;

fn try_from(rl: k8s::policy::HTTPLocalRateLimitPolicy) -> Result<Self> {
fn try_from(rl: k8s::policy::HttpLocalRateLimitPolicy) -> Result<Self> {
let creation_timestamp = rl.metadata.creation_timestamp.map(|Time(t)| t);
let conditions = rl.status.map_or(vec![], |status| {
status
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ fn mk_ratelimit(
total: Option<k8s::policy::Limit>,
overrides: Vec<k8s::policy::Override>,
server_name: impl ToString,
) -> k8s::policy::HTTPLocalRateLimitPolicy {
k8s::policy::HTTPLocalRateLimitPolicy {
) -> k8s::policy::HttpLocalRateLimitPolicy {
k8s::policy::HttpLocalRateLimitPolicy {
metadata: k8s::ObjectMeta {
namespace: Some(ns.to_string()),
name: Some(name.to_string()),
Expand All @@ -105,7 +105,7 @@ fn mk_ratelimit(
identity: None,
overrides: Some(overrides),
},
status: Some(k8s::policy::HTTPLocalRateLimitPolicyStatus {
status: Some(k8s::policy::HttpLocalRateLimitPolicyStatus {
conditions: vec![k8s::Condition {
last_transition_time: k8s::Time(chrono::DateTime::<chrono::Utc>::MIN_UTC),
message: "".to_string(),
Expand Down
28 changes: 14 additions & 14 deletions policy-controller/k8s/status/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub struct Index {
route_refs: HashMap<NamespaceGroupKindName, RouteRef>,

/// Maps rate limit ids to a list of details about these rate limits.
ratelimits: HashMap<ResourceId, HTTPLocalRateLimitPolicyRef>,
ratelimits: HashMap<ResourceId, HttpLocalRateLimitPolicyRef>,

/// Maps egress network ids to a list of details about these networks.
egress_networks: HashMap<ResourceId, EgressNetworkRef>,
Expand All @@ -115,7 +115,7 @@ struct RouteRef {
}

#[derive(Clone, PartialEq)]
struct HTTPLocalRateLimitPolicyRef {
struct HttpLocalRateLimitPolicyRef {
creation_timestamp: Option<DateTime<Utc>>,
target_ref: ratelimit::TargetReference,
status_conditions: Vec<k8s_core_api::Condition>,
Expand Down Expand Up @@ -279,8 +279,8 @@ impl Controller {
self.patch_status::<k8s_gateway_api::TcpRoute>(&id.gkn.name, &id.namespace, patch).await;
} else if id.gkn.group == k8s_gateway_api::TlsRoute::group(&()) && id.gkn.kind == k8s_gateway_api::TlsRoute::kind(&()) {
self.patch_status::<k8s_gateway_api::TlsRoute>(&id.gkn.name, &id.namespace, patch).await;
} else if id.gkn.group == linkerd_k8s_api::HTTPLocalRateLimitPolicy::group(&()) && id.gkn.kind == linkerd_k8s_api::HTTPLocalRateLimitPolicy::kind(&()) {
self.patch_status::<linkerd_k8s_api::HTTPLocalRateLimitPolicy>(&id.gkn.name, &id.namespace, patch).await;
} else if id.gkn.group == linkerd_k8s_api::HttpLocalRateLimitPolicy::group(&()) && id.gkn.kind == linkerd_k8s_api::HttpLocalRateLimitPolicy::kind(&()) {
self.patch_status::<linkerd_k8s_api::HttpLocalRateLimitPolicy>(&id.gkn.name, &id.namespace, patch).await;
} else if id.gkn.group == linkerd_k8s_api::EgressNetwork::group(&()) && id.gkn.kind == linkerd_k8s_api::EgressNetwork::kind(&()) {
self.patch_status::<linkerd_k8s_api::EgressNetwork>(&id.gkn.name, &id.namespace, patch).await;
}
Expand Down Expand Up @@ -430,7 +430,7 @@ impl Index {
fn update_ratelimit(
&mut self,
id: ResourceId,
ratelimit: &HTTPLocalRateLimitPolicyRef,
ratelimit: &HttpLocalRateLimitPolicyRef,
) -> bool {
match self.ratelimits.entry(id) {
Entry::Vacant(entry) => {
Expand Down Expand Up @@ -672,7 +672,7 @@ impl Index {
&self,
id: &NamespaceGroupKindName,
target_ref: &ratelimit::TargetReference,
) -> Option<linkerd_k8s_api::HTTPLocalRateLimitPolicyStatus> {
) -> Option<linkerd_k8s_api::HttpLocalRateLimitPolicyStatus> {
match target_ref {
ratelimit::TargetReference::Server(server) => {
let condition = if self.servers.contains(server) {
Expand Down Expand Up @@ -708,7 +708,7 @@ impl Index {
no_matching_target()
};

Some(linkerd_k8s_api::HTTPLocalRateLimitPolicyStatus {
Some(linkerd_k8s_api::HttpLocalRateLimitPolicyStatus {
conditions: vec![condition],
target_ref: linkerd_k8s_api::LocalTargetRef {
group: Some(POLICY_API_GROUP.to_string()),
Expand All @@ -724,7 +724,7 @@ impl Index {
fn make_ratelimit_patch(
&self,
id: &NamespaceGroupKindName,
ratelimit: &HTTPLocalRateLimitPolicyRef,
ratelimit: &HttpLocalRateLimitPolicyRef,
) -> Option<k8s_core_api::Patch<serde_json::Value>> {
let status = self.target_ref_status(id, &ratelimit.target_ref);

Expand Down Expand Up @@ -828,8 +828,8 @@ impl Index {
let id = NamespaceGroupKindName {
namespace: id.namespace.clone(),
gkn: GroupKindName {
group: linkerd_k8s_api::HTTPLocalRateLimitPolicy::group(&()),
kind: linkerd_k8s_api::HTTPLocalRateLimitPolicy::kind(&()),
group: linkerd_k8s_api::HttpLocalRateLimitPolicy::group(&()),
kind: linkerd_k8s_api::HttpLocalRateLimitPolicy::kind(&()),
name: id.name.clone().into(),
},
};
Expand Down Expand Up @@ -1220,8 +1220,8 @@ impl kubert::index::IndexNamespacedResource<k8s_core_api::Service> for Index {
// to handle resets specially.
}

impl kubert::index::IndexNamespacedResource<linkerd_k8s_api::HTTPLocalRateLimitPolicy> for Index {
fn apply(&mut self, resource: linkerd_k8s_api::HTTPLocalRateLimitPolicy) {
impl kubert::index::IndexNamespacedResource<linkerd_k8s_api::HttpLocalRateLimitPolicy> for Index {
fn apply(&mut self, resource: linkerd_k8s_api::HttpLocalRateLimitPolicy) {
let namespace = resource
.namespace()
.expect("HTTPLocalRateLimitPolicy must have a namespace");
Expand All @@ -1237,7 +1237,7 @@ impl kubert::index::IndexNamespacedResource<linkerd_k8s_api::HTTPLocalRateLimitP
let creation_timestamp = resource.metadata.creation_timestamp.map(|Time(t)| t);
let target_ref = ratelimit::TargetReference::make_target_ref(&namespace, &resource.spec);

let rl = HTTPLocalRateLimitPolicyRef {
let rl = HttpLocalRateLimitPolicyRef {
creation_timestamp,
target_ref,
status_conditions,
Expand Down Expand Up @@ -1349,7 +1349,7 @@ impl Index {
self.reconcile()
}

fn index_ratelimit(&mut self, id: ResourceId, ratelimit: HTTPLocalRateLimitPolicyRef) {
fn index_ratelimit(&mut self, id: ResourceId, ratelimit: HttpLocalRateLimitPolicyRef) {
// Insert into the index; if the route is already in the index, and it hasn't
// changed, skip creating a patch.
if !self.update_ratelimit(id.clone(), &ratelimit) {
Expand Down
2 changes: 1 addition & 1 deletion policy-controller/k8s/status/src/resource_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl NamespaceGroupKindName {
match (self.gkn.group.as_ref(), self.gkn.kind.as_ref()) {
(POLICY_API_GROUP, "HTTPRoute") => Ok(linkerd_k8s_api::HttpRoute::api_version(&())),
(POLICY_API_GROUP, "HTTPLocalRateLimitPolicy") => {
Ok(linkerd_k8s_api::HTTPLocalRateLimitPolicy::api_version(&()))
Ok(linkerd_k8s_api::HttpLocalRateLimitPolicy::api_version(&()))
}
(POLICY_API_GROUP, "EgressNetwork") => {
Ok(linkerd_k8s_api::EgressNetwork::api_version(&()))
Expand Down
16 changes: 8 additions & 8 deletions policy-controller/k8s/status/src/tests/ratelimit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn ratelimit_accepted() {
let (ratelimit_id, ratelimit) = make_ratelimit("rl-1".to_string(), "server-1".to_string());
index.write().apply(ratelimit);

let expected_status = linkerd_k8s_api::HTTPLocalRateLimitPolicyStatus {
let expected_status = linkerd_k8s_api::HttpLocalRateLimitPolicyStatus {
conditions: vec![accepted()],
target_ref: linkerd_k8s_api::LocalTargetRef {
group: Some("policy.linkerd.io".to_string()),
Expand Down Expand Up @@ -73,7 +73,7 @@ fn ratelimit_not_accepted_no_matching_target() {
let (ratelimit_id, ratelimit) = make_ratelimit("rl-1".to_string(), "server-2".to_string());
index.write().apply(ratelimit);

let expected_status = linkerd_k8s_api::HTTPLocalRateLimitPolicyStatus {
let expected_status = linkerd_k8s_api::HttpLocalRateLimitPolicyStatus {
conditions: vec![no_matching_target()],
target_ref: linkerd_k8s_api::LocalTargetRef {
group: Some("policy.linkerd.io".to_string()),
Expand Down Expand Up @@ -109,7 +109,7 @@ fn ratelimit_not_accepted_already_exists() {
let (rl_1_id, rl_1) = make_ratelimit("rl-1".to_string(), "server-1".to_string());
index.write().apply(rl_1);

let expected_status = linkerd_k8s_api::HTTPLocalRateLimitPolicyStatus {
let expected_status = linkerd_k8s_api::HttpLocalRateLimitPolicyStatus {
conditions: vec![accepted()],
target_ref: linkerd_k8s_api::LocalTargetRef {
group: Some("policy.linkerd.io".to_string()),
Expand All @@ -129,7 +129,7 @@ fn ratelimit_not_accepted_already_exists() {
let (rl_2_id, rl_2) = make_ratelimit("rl-2".to_string(), "server-1".to_string());
index.write().apply(rl_2);

let expected_status = linkerd_k8s_api::HTTPLocalRateLimitPolicyStatus {
let expected_status = linkerd_k8s_api::HttpLocalRateLimitPolicyStatus {
conditions: vec![ratelimit_already_exists()],
target_ref: linkerd_k8s_api::LocalTargetRef {
group: Some("policy.linkerd.io".to_string()),
Expand Down Expand Up @@ -182,18 +182,18 @@ fn make_ratelimit(
server: String,
) -> (
NamespaceGroupKindName,
linkerd_k8s_api::HTTPLocalRateLimitPolicy,
linkerd_k8s_api::HttpLocalRateLimitPolicy,
) {
let ratelimit_id = NamespaceGroupKindName {
namespace: "ns".to_string(),
gkn: GroupKindName {
group: linkerd_k8s_api::HTTPLocalRateLimitPolicy::group(&()),
kind: linkerd_k8s_api::HTTPLocalRateLimitPolicy::kind(&()),
group: linkerd_k8s_api::HttpLocalRateLimitPolicy::group(&()),
kind: linkerd_k8s_api::HttpLocalRateLimitPolicy::kind(&()),
name: name.clone().into(),
},
};

let ratelimit = linkerd_k8s_api::HTTPLocalRateLimitPolicy {
let ratelimit = linkerd_k8s_api::HttpLocalRateLimitPolicy {
metadata: k8s_core_api::ObjectMeta {
name: Some(name),
namespace: Some("ns".to_string()),
Expand Down
4 changes: 2 additions & 2 deletions policy-controller/src/admission.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::validation;
use crate::k8s::policy::{
httproute, server::Selector, AuthorizationPolicy, AuthorizationPolicySpec, EgressNetwork,
EgressNetworkSpec, HTTPLocalRateLimitPolicy, HttpRoute, HttpRouteSpec, LocalTargetRef,
EgressNetworkSpec, HttpLocalRateLimitPolicy, HttpRoute, HttpRouteSpec, LocalTargetRef,
MeshTLSAuthentication, MeshTLSAuthenticationSpec, NamespacedTargetRef, Network,
NetworkAuthentication, NetworkAuthenticationSpec, RateLimitPolicySpec, Server,
ServerAuthorization, ServerAuthorizationSpec, ServerSpec,
Expand Down Expand Up @@ -149,7 +149,7 @@ impl Admission {
return self.admit_spec::<k8s_gateway_api::TcpRouteSpec>(req).await;
}

if is_kind::<HTTPLocalRateLimitPolicy>(&req) {
if is_kind::<HttpLocalRateLimitPolicy>(&req) {
return self.admit_spec::<RateLimitPolicySpec>(req).await;
}

Expand Down
2 changes: 1 addition & 1 deletion policy-controller/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ async fn main() -> Result<()> {
);

let ratelimit_policies =
runtime.watch_all::<k8s::policy::HTTPLocalRateLimitPolicy>(watcher::Config::default());
runtime.watch_all::<k8s::policy::HttpLocalRateLimitPolicy>(watcher::Config::default());
let ratelimit_policies_indexes = IndexList::new(inbound_index.clone())
.push(status_index.clone())
.shared();
Expand Down
8 changes: 4 additions & 4 deletions policy-test/tests/admit_http_local_ratelimit_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use k8s_openapi::chrono;
use linkerd_policy_controller_k8s_api::{
self as api,
policy::{
HTTPLocalRateLimitPolicy, HTTPLocalRateLimitPolicyStatus, Limit, LocalTargetRef,
HttpLocalRateLimitPolicy, HttpLocalRateLimitPolicyStatus, Limit, LocalTargetRef,
NamespacedTargetRef, Override, RateLimitPolicySpec,
},
};
Expand Down Expand Up @@ -74,8 +74,8 @@ fn mk_ratelimiter(
total_rps: u32,
identity_rps: u32,
overrides: Vec<Override>,
) -> HTTPLocalRateLimitPolicy {
HTTPLocalRateLimitPolicy {
) -> HttpLocalRateLimitPolicy {
HttpLocalRateLimitPolicy {
metadata: api::ObjectMeta {
namespace: Some(namespace),
name: Some("test".to_string()),
Expand All @@ -91,7 +91,7 @@ fn mk_ratelimiter(
}),
overrides: Some(overrides),
},
status: Some(HTTPLocalRateLimitPolicyStatus {
status: Some(HttpLocalRateLimitPolicyStatus {
conditions: vec![api::Condition {
last_transition_time: api::Time(chrono::DateTime::<chrono::Utc>::MIN_UTC),
message: "".to_string(),
Expand Down
4 changes: 2 additions & 2 deletions policy-test/tests/inbound_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ async fn http_local_rate_limit_policy() {
// Create a rate-limit policy associated to the server
create(
&client,
k8s::policy::ratelimit_policy::HTTPLocalRateLimitPolicy {
k8s::policy::ratelimit_policy::HttpLocalRateLimitPolicy {
metadata: k8s::ObjectMeta {
namespace: Some(ns.to_string()),
name: Some("rl-0".to_string()),
Expand All @@ -356,7 +356,7 @@ async fn http_local_rate_limit_policy() {
}],
}]),
},
status: Some(k8s::policy::HTTPLocalRateLimitPolicyStatus {
status: Some(k8s::policy::HttpLocalRateLimitPolicyStatus {
conditions: vec![k8s::Condition {
last_transition_time: k8s::Time(chrono::DateTime::<chrono::Utc>::MIN_UTC),
message: "".to_string(),
Expand Down

0 comments on commit 6e38368

Please sign in to comment.