Skip to content

Explicitly explain "fail closed". #252

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.

Sign up for GitHub

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

Merged
k8s-ci-robot merged 3 commits into from
Mar 25, 2025
Merged

Explicitly explain "fail closed". #252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
95e8380
Explicitly explain "fail closed".
fasaxc Oct 14, 2024
2f20006
Hint that mismatched CRD is a misconfig.
fasaxc Nov 8, 2024
c93ddbd
make generate
fasaxc Mar 24, 2025
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
13 changes: 10 additions & 3 deletions apis/v1alpha1/adminnetworkpolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,16 @@ type AdminNetworkPolicyEgressRule struct {
type AdminNetworkPolicyRuleAction string

// AdminNetworkPolicyEgressPeer defines a peer to allow traffic to.
// Exactly one of the selector pointers must be set for a given peer. If a
// consumer observes none of its fields are set, they must assume an unknown
// option has been specified and fail closed.
//
// Exactly one of the fields must be set for a given peer and this is enforced
// by the validation rules on the CRD. If an implementation sees no fields are
// set then it can infer that the deployed CRD is of an incompatible version
// with an unknown field. In that case it should fail closed.
//
// For "Allow" rules, "fail closed" means: "treat the rule as matching no
// traffic". For "Deny" and "Pass" rules, "fail closed" means: "treat the rule
// as a 'Deny all' rule".
Copy link
Contributor

@danwinship danwinship Dec 4, 2024

Choose a reason for hiding this comment

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

I'm happy with the current wording here. We might want to explain the logic behind this in a little more detail somewhere, but the API documentation isn't really the right place for that.

/approve
anyone else want to lgtm?

Copy link
Contributor

@bowei bowei Mar 14, 2025

Choose a reason for hiding this comment

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

Shouldn't the comment about failed closed be on the field, not the type?

The doc for the type should say that if zero properties are set, the Peer description is "invalid".

The field referencing the type should say: invalid Peers, allow => match none, pass, deny => match all.

//
// +kubebuilder:validation:MaxProperties=1
// +kubebuilder:validation:MinProperties=1
type AdminNetworkPolicyEgressPeer struct {
Expand Down
13 changes: 10 additions & 3 deletions apis/v1alpha1/baselineadminnetworkpolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,16 @@ type BaselineAdminNetworkPolicyEgressRule struct {
type BaselineAdminNetworkPolicyRuleAction string

// BaselineAdminNetworkPolicyEgressPeer defines a peer to allow traffic to.
// Exactly one of the selector pointers must be set for a given peer. If a
// consumer observes none of its fields are set, they must assume an unknown
// option has been specified and fail closed.
//
// Exactly one of the fields must be set for a given peer and this is enforced
// by the validation rules on the CRD. If an implementation sees no fields are
// set then it can infer that the deployed CRD is of an incompatible version
// with an unknown field. In that case it should fail closed.
//
// For "Allow" rules, "fail closed" means: "treat the rule as matching no
// traffic". For "Deny" and "Pass" rules, "fail closed" means: "treat the rule
// as a 'Deny all' rule".
//
// +kubebuilder:validation:MaxProperties=1
// +kubebuilder:validation:MinProperties=1
type BaselineAdminNetworkPolicyEgressPeer struct {
Expand Down
15 changes: 11 additions & 4 deletions apis/v1alpha1/shared_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,17 @@ type PortRange struct {
End int32 `json:"end"`
}

// AdminNetworkPolicyIngressPeer defines an in-cluster peer to allow traffic from.
// Exactly one of the selector pointers must be set for a given peer. If a
// consumer observes none of its fields are set, they must assume an unknown
// option has been specified and fail closed.
// AdminNetworkPolicyIngressPeer defines a peer to allow traffic to.
//
// Exactly one of the fields must be set for a given peer and this is enforced
// by the validation rules on the CRD. If an implementation sees no fields are
// set then it can infer that the deployed CRD is of an incompatible version
// with an unknown field. In that case it should fail closed.
//
// For "Allow" rules, "fail closed" means: "treat the rule as matching no
// traffic". For "Deny" and "Pass" rules, "fail closed" means: "treat the rule
// as a 'Deny all' rule".
//
// +kubebuilder:validation:MaxProperties=1
// +kubebuilder:validation:MinProperties=1
type AdminNetworkPolicyIngressPeer struct {
Expand Down
26 changes: 19 additions & 7 deletions config/crd/experimental/policy.networking.k8s.io_adminnetworkpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,15 @@ spec:
items:
description: |-
AdminNetworkPolicyEgressPeer defines a peer to allow traffic to.
Exactly one of the selector pointers must be set for a given peer. If a
consumer observes none of its fields are set, they must assume an unknown
option has been specified and fail closed.

Exactly one of the fields must be set for a given peer and this is enforced
by the validation rules on the CRD. If an implementation sees no fields are
set then it can infer that the deployed CRD is of an incompatible version
with an unknown field. In that case it should fail closed.

For "Allow" rules, "fail closed" means: "treat the rule as matching no
traffic". For "Deny" and "Pass" rules, "fail closed" means: "treat the rule
as a 'Deny all' rule".
maxProperties: 1
minProperties: 1
properties:
Expand Down Expand Up @@ -551,10 +557,16 @@ spec:
Support: Core
items:
description: |-
AdminNetworkPolicyIngressPeer defines an in-cluster peer to allow traffic from.
Exactly one of the selector pointers must be set for a given peer. If a
consumer observes none of its fields are set, they must assume an unknown
option has been specified and fail closed.
AdminNetworkPolicyIngressPeer defines a peer to allow traffic to.

Exactly one of the fields must be set for a given peer and this is enforced
by the validation rules on the CRD. If an implementation sees no fields are
set then it can infer that the deployed CRD is of an incompatible version
with an unknown field. In that case it should fail closed.

For "Allow" rules, "fail closed" means: "treat the rule as matching no
traffic". For "Deny" and "Pass" rules, "fail closed" means: "treat the rule
as a 'Deny all' rule".
maxProperties: 1
minProperties: 1
properties:
Expand Down
26 changes: 19 additions & 7 deletions config/crd/experimental/policy.networking.k8s.io_baselineadminnetworkpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,15 @@ spec:
items:
description: |-
BaselineAdminNetworkPolicyEgressPeer defines a peer to allow traffic to.
Exactly one of the selector pointers must be set for a given peer. If a
consumer observes none of its fields are set, they must assume an unknown
option has been specified and fail closed.

Exactly one of the fields must be set for a given peer and this is enforced
by the validation rules on the CRD. If an implementation sees no fields are
set then it can infer that the deployed CRD is of an incompatible version
with an unknown field. In that case it should fail closed.

For "Allow" rules, "fail closed" means: "treat the rule as matching no
traffic". For "Deny" and "Pass" rules, "fail closed" means: "treat the rule
as a 'Deny all' rule".
maxProperties: 1
minProperties: 1
properties:
Expand Down Expand Up @@ -498,10 +504,16 @@ spec:
Support: Core
items:
description: |-
AdminNetworkPolicyIngressPeer defines an in-cluster peer to allow traffic from.
Exactly one of the selector pointers must be set for a given peer. If a
consumer observes none of its fields are set, they must assume an unknown
option has been specified and fail closed.
AdminNetworkPolicyIngressPeer defines a peer to allow traffic to.

Exactly one of the fields must be set for a given peer and this is enforced
by the validation rules on the CRD. If an implementation sees no fields are
set then it can infer that the deployed CRD is of an incompatible version
with an unknown field. In that case it should fail closed.

For "Allow" rules, "fail closed" means: "treat the rule as matching no
traffic". For "Deny" and "Pass" rules, "fail closed" means: "treat the rule
as a 'Deny all' rule".
maxProperties: 1
minProperties: 1
properties:
Expand Down
26 changes: 19 additions & 7 deletions config/crd/standard/policy.networking.k8s.io_adminnetworkpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,15 @@ spec:
items:
description: |-
AdminNetworkPolicyEgressPeer defines a peer to allow traffic to.
Exactly one of the selector pointers must be set for a given peer. If a
consumer observes none of its fields are set, they must assume an unknown
option has been specified and fail closed.

Exactly one of the fields must be set for a given peer and this is enforced
by the validation rules on the CRD. If an implementation sees no fields are
set then it can infer that the deployed CRD is of an incompatible version
with an unknown field. In that case it should fail closed.

For "Allow" rules, "fail closed" means: "treat the rule as matching no
traffic". For "Deny" and "Pass" rules, "fail closed" means: "treat the rule
as a 'Deny all' rule".
maxProperties: 1
minProperties: 1
properties:
Expand Down Expand Up @@ -412,10 +418,16 @@ spec:
Support: Core
items:
description: |-
AdminNetworkPolicyIngressPeer defines an in-cluster peer to allow traffic from.
Exactly one of the selector pointers must be set for a given peer. If a
consumer observes none of its fields are set, they must assume an unknown
option has been specified and fail closed.
AdminNetworkPolicyIngressPeer defines a peer to allow traffic to.

Exactly one of the fields must be set for a given peer and this is enforced
by the validation rules on the CRD. If an implementation sees no fields are
set then it can infer that the deployed CRD is of an incompatible version
with an unknown field. In that case it should fail closed.

For "Allow" rules, "fail closed" means: "treat the rule as matching no
traffic". For "Deny" and "Pass" rules, "fail closed" means: "treat the rule
as a 'Deny all' rule".
maxProperties: 1
minProperties: 1
properties:
Expand Down
26 changes: 19 additions & 7 deletions config/crd/standard/policy.networking.k8s.io_baselineadminnetworkpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,15 @@ spec:
items:
description: |-
BaselineAdminNetworkPolicyEgressPeer defines a peer to allow traffic to.
Exactly one of the selector pointers must be set for a given peer. If a
consumer observes none of its fields are set, they must assume an unknown
option has been specified and fail closed.

Exactly one of the fields must be set for a given peer and this is enforced
by the validation rules on the CRD. If an implementation sees no fields are
set then it can infer that the deployed CRD is of an incompatible version
with an unknown field. In that case it should fail closed.

For "Allow" rules, "fail closed" means: "treat the rule as matching no
traffic". For "Deny" and "Pass" rules, "fail closed" means: "treat the rule
as a 'Deny all' rule".
maxProperties: 1
minProperties: 1
properties:
Expand Down Expand Up @@ -399,10 +405,16 @@ spec:
Support: Core
items:
description: |-
AdminNetworkPolicyIngressPeer defines an in-cluster peer to allow traffic from.
Exactly one of the selector pointers must be set for a given peer. If a
consumer observes none of its fields are set, they must assume an unknown
option has been specified and fail closed.
AdminNetworkPolicyIngressPeer defines a peer to allow traffic to.

Exactly one of the fields must be set for a given peer and this is enforced
by the validation rules on the CRD. If an implementation sees no fields are
set then it can infer that the deployed CRD is of an incompatible version
with an unknown field. In that case it should fail closed.

For "Allow" rules, "fail closed" means: "treat the rule as matching no
traffic". For "Deny" and "Pass" rules, "fail closed" means: "treat the rule
as a 'Deny all' rule".
maxProperties: 1
minProperties: 1
properties:
Expand Down