Skip to content
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.

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

Add EnableDefaultDeny field to Network Policy #30572

Merged
merged 4 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/actions/ginkgo/main-focus.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ include:

###
# K8sAgentPolicyTest Clusterwide policies Test clusterwide connectivity with policies
# K8sAgentPolicyTest Clusterwide policies Tests connectivity with default-allow policies
# K8sAgentPolicyTest External services To Services first endpoint creation
# K8sAgentPolicyTest External services To Services first endpoint creation match service by labels
# K8sAgentPolicyTest External services To Services first policy
Expand Down Expand Up @@ -82,6 +83,8 @@ include:
# K8sAgentPolicyTest Basic Test Traffic redirections to proxy Tests DNS proxy visibility without policy
# K8sAgentPolicyTest Basic Test Traffic redirections to proxy Tests HTTP proxy visibility without policy
# K8sAgentPolicyTest Basic Test Traffic redirections to proxy Tests proxy visibility interactions with policy lifecycle operations
# K8sAgentPolicyTest Basic Test Traffic redirections to proxy Tests proxy visibility with L7 default-allow rules
# K8sAgentPolicyTest Basic Test Traffic redirections to proxy Tests proxy visibility with L7 rules
# K8sPolicyTestExtended Validate toEntities KubeAPIServer Allows connection to KubeAPIServer
# K8sPolicyTestExtended Validate toEntities KubeAPIServer Denies connection to KubeAPIServer
# K8sPolicyTestExtended Validate toEntities KubeAPIServer Still allows connection to KubeAPIServer with a duplicate policy
Expand Down
21 changes: 21 additions & 0 deletions daemon/cmd/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@ func (ds *DaemonSuite) TestUpdateConsumerMap(c *C) {
},
},
}
for i := range rules {
rules[i].Sanitize()
}

ds.d.envoyXdsServer.RemoveAllNetworkPolicies()

Expand Down Expand Up @@ -466,6 +469,9 @@ func (ds *DaemonSuite) TestL4_L7_Shadowing(c *C) {
},
},
}
for i := range rules {
rules[i].Sanitize()
}

ds.d.envoyXdsServer.RemoveAllNetworkPolicies()

Expand Down Expand Up @@ -549,6 +555,9 @@ func (ds *DaemonSuite) TestL4_L7_ShadowingShortCircuit(c *C) {
},
},
}
for i := range rules {
rules[i].Sanitize()
}

ds.d.envoyXdsServer.RemoveAllNetworkPolicies()

Expand Down Expand Up @@ -636,6 +645,9 @@ func (ds *DaemonSuite) TestL3_dependent_L7(c *C) {
},
},
}
for i := range rules {
rules[i].Sanitize()
}

ds.d.envoyXdsServer.RemoveAllNetworkPolicies()

Expand Down Expand Up @@ -711,6 +723,9 @@ func (ds *DaemonSuite) TestReplacePolicy(c *C) {
EndpointSelector: api.NewESFromLabels(lblBar),
},
}
for i := range rules {
rules[i].Sanitize()
}

_, err := ds.d.PolicyAdd(rules, policyAddOptions)
c.Assert(err, IsNil)
Expand Down Expand Up @@ -792,6 +807,9 @@ func (ds *DaemonSuite) TestRemovePolicy(c *C) {
},
},
}
for i := range rules {
rules[i].Sanitize()
}

ds.d.envoyXdsServer.RemoveAllNetworkPolicies()

Expand Down Expand Up @@ -877,6 +895,9 @@ func (ds *DaemonSuite) TestIncrementalPolicy(c *C) {
},
},
}
for i := range rules {
rules[i].Sanitize()
}

ds.d.envoyXdsServer.RemoveAllNetworkPolicies()

Expand Down
joestringer marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -1545,6 +1545,28 @@ spec:
type: array
type: object
type: array
enableDefaultDeny:
description: "EnableDefaultDeny determines whether this policy configures
the subject endpoint(s) to have a default deny mode. If enabled,
this causes all traffic not explicitly allowed by a network policy
to be dropped. \n If not specified, the default is true for each
christarazi marked this conversation as resolved.
Show resolved Hide resolved
traffic direction that has rules, and false otherwise. For example,
if a policy only has Ingress or IngressDeny rules, then the default
for ingress is true and egress is false. \n If multiple policies
apply to an endpoint, that endpoint's default deny will be enabled
if any policy requests it. \n This is useful for creating broad-based
network policies that will not cause endpoints to enter default-deny
mode."
properties:
egress:
description: Whether or not the endpoint should have a default-deny
rule applied to egress traffic.
type: boolean
ingress:
description: Whether or not the endpoint should have a default-deny
rule applied to ingress traffic.
type: boolean
type: object
endpointSelector:
description: EndpointSelector selects all endpoints which should be
subject to this rule. EndpointSelector and NodeSelector cannot be
Expand Down Expand Up @@ -4427,6 +4449,28 @@ spec:
type: array
type: object
type: array
enableDefaultDeny:
description: "EnableDefaultDeny determines whether this policy configures
the subject endpoint(s) to have a default deny mode. If enabled,
this causes all traffic not explicitly allowed by a network policy
to be dropped. \n If not specified, the default is true for each
traffic direction that has rules, and false otherwise. For example,
if a policy only has Ingress or IngressDeny rules, then the default
for ingress is true and egress is false. \n If multiple policies
apply to an endpoint, that endpoint's default deny will be enabled
if any policy requests it. \n This is useful for creating broad-based
network policies that will not cause endpoints to enter default-deny
mode."
properties:
egress:
description: Whether or not the endpoint should have a default-deny
rule applied to egress traffic.
type: boolean
ingress:
description: Whether or not the endpoint should have a default-deny
rule applied to ingress traffic.
type: boolean
type: object
endpointSelector:
description: EndpointSelector selects all endpoints which should
be subject to this rule. EndpointSelector and NodeSelector cannot
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1549,6 +1549,28 @@ spec:
type: array
type: object
type: array
enableDefaultDeny:
description: "EnableDefaultDeny determines whether this policy configures
the subject endpoint(s) to have a default deny mode. If enabled,
this causes all traffic not explicitly allowed by a network policy
to be dropped. \n If not specified, the default is true for each
traffic direction that has rules, and false otherwise. For example,
if a policy only has Ingress or IngressDeny rules, then the default
for ingress is true and egress is false. \n If multiple policies
apply to an endpoint, that endpoint's default deny will be enabled
if any policy requests it. \n This is useful for creating broad-based
network policies that will not cause endpoints to enter default-deny
mode."
properties:
egress:
description: Whether or not the endpoint should have a default-deny
rule applied to egress traffic.
type: boolean
ingress:
description: Whether or not the endpoint should have a default-deny
rule applied to ingress traffic.
type: boolean
type: object
endpointSelector:
description: EndpointSelector selects all endpoints which should be
subject to this rule. EndpointSelector and NodeSelector cannot be
Expand Down Expand Up @@ -4431,6 +4453,28 @@ spec:
type: array
type: object
type: array
enableDefaultDeny:
description: "EnableDefaultDeny determines whether this policy configures
the subject endpoint(s) to have a default deny mode. If enabled,
this causes all traffic not explicitly allowed by a network policy
to be dropped. \n If not specified, the default is true for each
traffic direction that has rules, and false otherwise. For example,
if a policy only has Ingress or IngressDeny rules, then the default
for ingress is true and egress is false. \n If multiple policies
apply to an endpoint, that endpoint's default deny will be enabled
if any policy requests it. \n This is useful for creating broad-based
network policies that will not cause endpoints to enter default-deny
mode."
properties:
egress:
description: Whether or not the endpoint should have a default-deny
rule applied to egress traffic.
type: boolean
ingress:
description: Whether or not the endpoint should have a default-deny
rule applied to ingress traffic.
type: boolean
type: object
endpointSelector:
description: EndpointSelector selects all endpoints which should
be subject to this rule. EndpointSelector and NodeSelector cannot
Expand Down
1 change: 1 addition & 0 deletions pkg/k8s/apis/cilium.io/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ func ParseToCiliumRule(namespace, name string, uid types.UID, r *api.Rule) *api.
retRule.Labels = ParseToCiliumLabels(namespace, name, uid, r.Labels)

retRule.Description = r.Description
retRule.EnableDefaultDeny = r.EnableDefaultDeny

return retRule
}
Expand Down
1 change: 1 addition & 0 deletions pkg/k8s/apis/cilium.io/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ func Test_ParseToCiliumRule(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.args.rule.Sanitize()
got := ParseToCiliumRule(tt.args.namespace, tt.name, tt.args.uid, tt.args.rule)

// Sanitize to set AggregatedSelectors field.
Expand Down
61 changes: 49 additions & 12 deletions pkg/policy/api/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,22 @@ type Authentication struct {
Mode AuthenticationMode `json:"mode"`
}

// DefaultDenyConfig expresses a policy's desired default mode for the subject
// endpoints.
type DefaultDenyConfig struct {
// Whether or not the endpoint should have a default-deny rule applied
// to ingress traffic.
//
// +kubebuilder:validation:Optional
Ingress *bool `json:"ingress,omitempty"`

// Whether or not the endpoint should have a default-deny rule applied
// to egress traffic.
//
// +kubebuilder:validation:Optional
Egress *bool `json:"egress,omitempty"`
}

// Rule is a policy rule which must be applied to all endpoints which match the
// labels contained in the endpointSelector
//
Expand Down Expand Up @@ -93,6 +109,25 @@ type Rule struct {
// +kubebuilder:validation:Optional
Labels labels.LabelArray `json:"labels,omitempty"`

// EnableDefaultDeny determines whether this policy configures the
// subject endpoint(s) to have a default deny mode. If enabled,
// this causes all traffic not explicitly allowed by a network policy
// to be dropped.
//
// If not specified, the default is true for each traffic direction
// that has rules, and false otherwise. For example, if a policy
// only has Ingress or IngressDeny rules, then the default for
// ingress is true and egress is false.
//
// If multiple policies apply to an endpoint, that endpoint's default deny
// will be enabled if any policy requests it.
//
// This is useful for creating broad-based network policies that will not
// cause endpoints to enter default-deny mode.
//
// +kubebuilder:validation:Optional
EnableDefaultDeny DefaultDenyConfig `json:"enableDefaultDeny,omitempty"`

// Description is a free form string, it can be used by the creator of
// the rule to store human readable explanation of the purpose of this
// rule. Rules cannot be identified by comment.
Expand All @@ -105,22 +140,24 @@ type Rule struct {
// enforce omitempty on the EndpointSelector nested structures.
func (r *Rule) MarshalJSON() ([]byte, error) {
type common struct {
Ingress []IngressRule `json:"ingress,omitempty"`
IngressDeny []IngressDenyRule `json:"ingressDeny,omitempty"`
Egress []EgressRule `json:"egress,omitempty"`
EgressDeny []EgressDenyRule `json:"egressDeny,omitempty"`
Labels labels.LabelArray `json:"labels,omitempty"`
Description string `json:"description,omitempty"`
Ingress []IngressRule `json:"ingress,omitempty"`
IngressDeny []IngressDenyRule `json:"ingressDeny,omitempty"`
Egress []EgressRule `json:"egress,omitempty"`
EgressDeny []EgressDenyRule `json:"egressDeny,omitempty"`
Labels labels.LabelArray `json:"labels,omitempty"`
EnableDefaultDeny DefaultDenyConfig `json:"enableDefaultDeny,omitempty"`
Description string `json:"description,omitempty"`
}

var a interface{}
ruleCommon := common{
Ingress: r.Ingress,
IngressDeny: r.IngressDeny,
Egress: r.Egress,
EgressDeny: r.EgressDeny,
Labels: r.Labels,
Description: r.Description,
Ingress: r.Ingress,
IngressDeny: r.IngressDeny,
Egress: r.Egress,
EgressDeny: r.EgressDeny,
Labels: r.Labels,
EnableDefaultDeny: r.EnableDefaultDeny,
Description: r.Description,
}

// Only one of endpointSelector or nodeSelector is permitted.
Expand Down
15 changes: 14 additions & 1 deletion pkg/policy/api/rule_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,20 @@ const (
// Sanitize validates and sanitizes a policy rule. Minor edits such as
// capitalization of the protocol name are automatically fixed up. More
// fundamental violations will cause an error to be returned.
func (r Rule) Sanitize() error {
func (r *Rule) Sanitize() error {
// Fill in the default traffic posture of this Rule.
// Default posture is per-direction (ingress or egress),
// if there is a peer selector for that direction, the
// default is deny, else allow.
if r.EnableDefaultDeny.Egress == nil {
x := len(r.Egress) > 0 || len(r.EgressDeny) > 0
r.EnableDefaultDeny.Egress = &x
}
if r.EnableDefaultDeny.Ingress == nil {
x := len(r.Ingress) > 0 || len(r.IngressDeny) > 0
r.EnableDefaultDeny.Ingress = &x
}

if r.EndpointSelector.LabelSelector == nil && r.NodeSelector.LabelSelector == nil {
return fmt.Errorf("rule must have one of EndpointSelector or NodeSelector")
}
Expand Down