mirror of https://github.com/linkerd/linkerd2.git
policy: Support empty `requiredAuthenticationRefs` (#8185)
Currently, the policy admission controller requires that the `AuthorizationPolicy` resources include a non-empty `requiredAuthenticationRefs` field. This means that all authorization policies require at least a `NetworkAuthentication` to permit traffic. For example: ```yaml --- apiVersion: policy.linkerd.io/v1alpha1 kind: AuthorizationPolicy metadata: name: ingress spec: targetRef: group: policy.linkerd.io kind: Server name: ingress-http requiredAuthenticationRefs: - group: policy.linkerd.io kind: NetworkAuthentication name: all-nets --- apiVersion: policy.linkerd.io/v1alpha1 kind: NetworkAuthentication metadata: name: ingress-all-nets spec: networks: - cidr: 0.0.0.0/0 - cidr: ::/0 ``` This is needlessly verbose and can more simply be expressed as: ```yaml --- apiVersion: policy.linkerd.io/v1alpha1 kind: AuthorizationPolicy metadata: name: ingress spec: targetRef: group: policy.linkerd.io kind: Server name: ingress-http requiredAuthenticationRefs: [] ``` That is: there are explicitly no required authentications for this policy. This change updates the admission controller to permit such a policy. Note that the `requiredAuthenticationRefs` field is still required so that it's harder for simple misconfigurations to result in allowing traffic. This change also removes `Default` implementation for resources where do they not make sense because there are required fields. Signed-off-by: Oliver Gould <ver@buoyant.io>
This commit is contained in:
parent
7df00c5b4e
commit
1a0c1c3172
|
@ -1,13 +1,7 @@
|
|||
use super::{LocalTargetRef, NamespacedTargetRef};
|
||||
|
||||
#[derive(
|
||||
Clone,
|
||||
Debug,
|
||||
Default,
|
||||
kube::CustomResource,
|
||||
serde::Deserialize,
|
||||
serde::Serialize,
|
||||
schemars::JsonSchema,
|
||||
Clone, Debug, kube::CustomResource, serde::Deserialize, serde::Serialize, schemars::JsonSchema,
|
||||
)]
|
||||
#[kube(
|
||||
group = "policy.linkerd.io",
|
||||
|
|
|
@ -1,24 +1,18 @@
|
|||
#[derive(
|
||||
Clone, Debug, Default, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema,
|
||||
)]
|
||||
#[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema)]
|
||||
pub struct ClusterTargetRef {
|
||||
pub group: Option<String>,
|
||||
pub kind: String,
|
||||
pub name: String,
|
||||
}
|
||||
|
||||
#[derive(
|
||||
Clone, Debug, Default, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema,
|
||||
)]
|
||||
#[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema)]
|
||||
pub struct LocalTargetRef {
|
||||
pub group: Option<String>,
|
||||
pub kind: String,
|
||||
pub name: String,
|
||||
}
|
||||
|
||||
#[derive(
|
||||
Clone, Debug, Default, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema,
|
||||
)]
|
||||
#[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema)]
|
||||
pub struct NamespacedTargetRef {
|
||||
pub group: Option<String>,
|
||||
pub kind: String,
|
||||
|
@ -242,9 +236,9 @@ mod tests {
|
|||
#[test]
|
||||
fn cluster_targets_namespace() {
|
||||
let t = ClusterTargetRef {
|
||||
group: None,
|
||||
kind: "Namespace".to_string(),
|
||||
name: "appns".to_string(),
|
||||
..Default::default()
|
||||
};
|
||||
assert!(t.targets_kind::<Namespace>());
|
||||
assert!(t.targets(&Namespace {
|
||||
|
@ -260,10 +254,10 @@ mod tests {
|
|||
fn namespaced_targets_service_account() {
|
||||
for tgt in &[
|
||||
NamespacedTargetRef {
|
||||
group: None,
|
||||
kind: "ServiceAccount".to_string(),
|
||||
name: "default".to_string(),
|
||||
namespace: Some("appns".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
NamespacedTargetRef {
|
||||
group: Some("core".to_string()),
|
||||
|
@ -278,9 +272,10 @@ mod tests {
|
|||
namespace: Some("APPNS".to_string()),
|
||||
},
|
||||
NamespacedTargetRef {
|
||||
group: None,
|
||||
kind: "ServiceAccount".to_string(),
|
||||
name: "default".to_string(),
|
||||
..Default::default()
|
||||
namespace: None,
|
||||
},
|
||||
] {
|
||||
assert!(tgt.targets_kind::<ServiceAccount>());
|
||||
|
@ -317,9 +312,10 @@ mod tests {
|
|||
}
|
||||
|
||||
let tgt = NamespacedTargetRef {
|
||||
group: None,
|
||||
kind: "ServiceAccount".to_string(),
|
||||
name: "default".to_string(),
|
||||
..Default::default()
|
||||
namespace: None,
|
||||
};
|
||||
assert!(
|
||||
{
|
||||
|
|
|
@ -49,7 +49,7 @@ fn links_authorization_policy_with_mtls_name() {
|
|||
group: Some("policy.linkerd.io".to_string()),
|
||||
kind: "NetworkAuthentication".to_string(),
|
||||
name: "net-foo".to_string(),
|
||||
..Default::default()
|
||||
namespace: None,
|
||||
},
|
||||
NamespacedTargetRef {
|
||||
group: Some("policy.linkerd.io".to_string()),
|
||||
|
|
|
@ -229,10 +229,6 @@ impl Validate<AuthorizationPolicySpec> for Admission {
|
|||
bail!("unsupported authentication kind(s): {}", kinds.join(", "));
|
||||
}
|
||||
|
||||
if mtls_authns_count + net_authns_count == 0 {
|
||||
bail!("at least one authentication reference must be set");
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
|
|
@ -23,7 +23,7 @@ async fn accepts_valid() {
|
|||
group: Some("policy.linkerd.io".to_string()),
|
||||
kind: "MeshTLSAuthentication".to_string(),
|
||||
name: "mtls-clients".to_string(),
|
||||
..Default::default()
|
||||
namespace: None,
|
||||
},
|
||||
NamespacedTargetRef {
|
||||
group: Some("policy.linkerd.io".to_string()),
|
||||
|
@ -55,7 +55,7 @@ async fn accepts_valid_with_only_meshtls() {
|
|||
group: Some("policy.linkerd.io".to_string()),
|
||||
kind: "MeshTLSAuthentication".to_string(),
|
||||
name: "mtls-clients".to_string(),
|
||||
..Default::default()
|
||||
namespace: None,
|
||||
}],
|
||||
},
|
||||
})
|
||||
|
@ -88,21 +88,8 @@ async fn accepts_valid_with_only_network() {
|
|||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn rejects_empty() {
|
||||
admission::rejects(|ns| AuthorizationPolicy {
|
||||
metadata: api::ObjectMeta {
|
||||
namespace: Some(ns),
|
||||
name: Some("test".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
spec: AuthorizationPolicySpec::default(),
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn rejects_empty_required_authentications() {
|
||||
admission::rejects(|ns| AuthorizationPolicy {
|
||||
async fn accepts_empty_required_authentications() {
|
||||
admission::accepts(|ns| AuthorizationPolicy {
|
||||
metadata: api::ObjectMeta {
|
||||
namespace: Some(ns),
|
||||
name: Some("test".to_string()),
|
||||
|
@ -120,6 +107,46 @@ async fn rejects_empty_required_authentications() {
|
|||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn rejects_missing_required_authentications() {
|
||||
#[derive(
|
||||
Clone,
|
||||
Debug,
|
||||
kube::CustomResource,
|
||||
serde::Deserialize,
|
||||
serde::Serialize,
|
||||
schemars::JsonSchema,
|
||||
)]
|
||||
#[kube(
|
||||
group = "policy.linkerd.io",
|
||||
version = "v1alpha1",
|
||||
kind = "AuthorizationPolicy",
|
||||
namespaced
|
||||
)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct FakeAuthorizationPolicySpec {
|
||||
pub target_ref: LocalTargetRef,
|
||||
pub required_authentication_refs: Option<Vec<NamespacedTargetRef>>,
|
||||
}
|
||||
|
||||
admission::rejects(|ns| AuthorizationPolicy {
|
||||
metadata: api::ObjectMeta {
|
||||
namespace: Some(ns),
|
||||
name: Some("test".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
spec: FakeAuthorizationPolicySpec {
|
||||
target_ref: LocalTargetRef {
|
||||
group: Some("policy.linkerd.io".to_string()),
|
||||
kind: "Server".to_string(),
|
||||
name: "deny".to_string(),
|
||||
},
|
||||
required_authentication_refs: None,
|
||||
},
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn rejects_target_ref_deployment() {
|
||||
admission::rejects(|ns| AuthorizationPolicy {
|
||||
|
|
|
@ -14,9 +14,10 @@ async fn accepts_valid_ref() {
|
|||
},
|
||||
spec: MeshTLSAuthenticationSpec {
|
||||
identity_refs: Some(vec![NamespacedTargetRef {
|
||||
group: None,
|
||||
kind: "ServiceAccount".to_string(),
|
||||
name: "default".to_string(),
|
||||
..Default::default()
|
||||
namespace: None,
|
||||
}]),
|
||||
..Default::default()
|
||||
},
|
||||
|
@ -64,9 +65,10 @@ async fn rejects_both_refs_and_strings() {
|
|||
spec: MeshTLSAuthenticationSpec {
|
||||
identities: Some(vec!["example.id".to_string()]),
|
||||
identity_refs: Some(vec![NamespacedTargetRef {
|
||||
group: None,
|
||||
kind: "ServiceAccount".to_string(),
|
||||
name: "default".to_string(),
|
||||
..Default::default()
|
||||
namespace: None,
|
||||
}]),
|
||||
},
|
||||
})
|
||||
|
|
Loading…
Reference in New Issue