Skip to content

[BUG] AdminNetworkPolicyIngressRule.Ports has two nil values? #247

@fasaxc

Description

@fasaxc

While reviewing our WIP implementation of ANP, I noticed that the Ports field is defined as

	// Ports allows for matching traffic based on port and protocols.
	// This field is a list of ports which should be matched on
	// the pods selected for this policy i.e the subject of the policy.
	// So it matches on the destination port for the ingress traffic.
	// If Ports is not set then the rule does not filter traffic via port.
	//
	// Support: Core
	//
	// +optional
	// +kubebuilder:validation:MaxItems=100
	Ports *[]AdminNetworkPolicyPort `json:"ports,omitempty"`

which means that it has two/three "empty" values:

  • (*[]AdminNetworkPolicyPort)(nil)
  • &([]AdminNetworkPolicyPort(nil))
  • &([]AdminNetworkPolicyPort{})

It's not clear what they're each supposed to mean, "If Ports is not set then the rule does not filter traffic via port". Clearly a nil pointer is "not set" but what about a pointer to an empty/nil slice? Would those mean "match no ports" (which in turn is a bit ambiguous since not all protocols have ports)?

I suggest dropping the extra pointer indirection and interpreting any zero-length slice (nil or otherwise) as "do not filter on ports". If we can't do that, we should at least document the meanings (where I think we should probably make them all mean "do not filter on ports").

Metadata

Metadata

Assignees

Labels

kind/bugCategorizes issue or PR as related to a bug.

Type

No type

Projects

Status

Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions