Respond to Round 2 of Dan Comments

Fix some nits
Remove a level of indirection: `PodPeer`
Change Ports protocol default
Update some comment documentation

Signed-off-by: Andrew Stoycos <[email protected]>

Author: astoycos

apis/v1alpha1/adminnetworkpolicy_types.go

@@ -35,6 +35,11 @@ type AdminNetworkPolicy struct {
 	Status AdminNetworkPolicyStatus `json:"status,omitempty"`
 }
 
+// AdminNetworkPolicyStatus defines the observed state of AdminNetworkPolicy.
+type AdminNetworkPolicyStatus struct {
+	Conditions []metav1.Condition `json:"conditions"`
+}
+
 // AdminNetworkPolicySpec defines the desired state of AdminNetworkPolicy.
 type AdminNetworkPolicySpec struct {
 	// Priority is an int32 value bound to 0 - 1000, the lowest priority,
@@ -48,15 +53,15 @@ type AdminNetworkPolicySpec struct {
 	Subject AdminNetworkPolicySubject `json:"subject"`
 
 	// List of Ingress rules to be applied to the selected pods BEFORE any
-	// NetworkPolicy or BaslineAdminNetworkPolicy rules have been applied.
+	// NetworkPolicy or BaselineAdminNetworkPolicy rules have been applied.
 	// A total of 100 rules will be allowed in each ANP instance.
 	// ANPs with no ingress rules do not affect ingress traffic.
 	// +optional
 	// +kubebuilder:validation:MaxItems=100
 	Ingress []AdminNetworkPolicyIngressRule `json:"ingress,omitempty"`
 
 	// List of Egress rules to be applied to the selected pods BEFORE any
-	// NetworkPolicy or BaslineAdminNetworkPolicy rules have been applied.
+	// NetworkPolicy or BaselineAdminNetworkPolicy rules have been applied.
 	// A total of 100 rules will be allowed in each ANP instance.
 	// ANPs with no egress rules do not affect egress traffic.
 	// +optional
@@ -69,12 +74,15 @@ type AdminNetworkPolicySpec struct {
 // Subject field. The traffic must match both ports and from.
 type AdminNetworkPolicyIngressRule struct {
 	// Name is an identifier for this rule, that may be no more than 100 characters
-	// in length.
+	// in length. This field should be used by the implementation to help
+	// improve observability, readability and error-reporting for any applied
+	// AdminNetworkPolicies.
 	// +optional
 	// +kubebuilder:validation:MaxLength=100
 	Name string `json:"name,omitempty"`
 
-	// Action specifies whether this rule must pass, allow or deny traffic.
+	// Action specifies the affect this rule will have on matching traffic,
+	// currently the following actions are supported:
 	// Allow: allows the selected traffic
 	// Deny: denies the selected traffic
 	// Pass: instructs the selected traffic to skip any remaining ANP rules, and
@@ -84,15 +92,16 @@ type AdminNetworkPolicyIngressRule struct {
 	// This field is mandatory.
 	Action AdminNetworkPolicyRuleAction `json:"action"`
 
-	// Ports allows for matching  traffic based on port and protocols.
-	// If Ports is empty or missing then traffic is not filtered via port.
+	// Ports allows for matching traffic based on port and protocols.
+	// If Ports is not set then traffic is not filtered via port.
 	// +optional
 	// +kubebuilder:validation:MaxItems=100
 	Ports []AdminNetworkPolicyPort `json:"ports,omitempty"`
 
 	// List of sources whose traffic this AdminNetworkPolicyRule applies to.
-	// Items in this list are combined using a logical OR
-	// operation. This field must be defined and contain at least one item.
+	// If any adminNetworkPolicyPeer matches the source of incoming
+	// traffic then the specified action is applied.
+	// This field must be defined and contain at least one item.
 	// +kubebuilder:validation:MinItems=1
 	// +kubebuilder:validation:MaxItems=100
 	From []AdminNetworkPolicyPeer `json:"from"`
@@ -103,12 +112,15 @@ type AdminNetworkPolicyIngressRule struct {
 // Subject field. The traffic must match both ports and to.
 type AdminNetworkPolicyEgressRule struct {
 	// Name is an identifier for this rule, that may be no more than 100 characters
-	// in length.
+	// in length. This field should be used by the implementation to help
+	// improve observability, readability and error-reporting for any applied
+	// AdminNetworkPolicies.
 	// +optional
 	// +kubebuilder:validation:MaxLength=100
 	Name string `json:"name,omitempty"`
 
-	// Action specifies whether this rule must pass, allow or deny traffic.
+	// Action specifies the affect this rule will have on matching traffic,
+	// currently the following actions are supported:
 	// Allow: allows the selected traffic (even if it would otherwise have been denied by NetworkPolicy)
 	// Deny: denies the selected traffic (even if it would otherwise have been denied by NetworkPolicy)
 	// Pass: instructs the selected traffic to skip any remaining ANP rules, and
@@ -119,14 +131,15 @@ type AdminNetworkPolicyEgressRule struct {
 	Action AdminNetworkPolicyRuleAction `json:"action"`
 
 	// Ports allows for matching traffic based on port and protocols.
-	// If Ports is empty or missing then traffic is not filtered via port.
+	// If Ports is not set then traffic is not filtered via port.
 	// +optional
 	// +kubebuilder:validation:MaxItems=100
 	Ports []AdminNetworkPolicyPort `json:"ports,omitempty"`
 
-	// List of destinations to which traffic will be allowed/denied/passed from the entities
-	// selected by this AdminNetworkPolicyRule. Items in this list are combined using a logical OR
-	// operation. This field must be defined and contain at least one item.
+	// List of destinations whose traffic this adminNetworkPolicyRule applies to.
+	// If any adminNetworkPolicyPeer matches the destination of outgoing
+	// traffic then the specified action is applied.
+	// This field must be defined and contain at least one item.
 	// +kubebuilder:validation:MinItems=1
 	// +kubebuilder:validation:MaxItems=100
 	To []AdminNetworkPolicyPeer `json:"to"`
@@ -137,12 +150,13 @@ type AdminNetworkPolicyEgressRule struct {
 type AdminNetworkPolicyRuleAction string
 
 const (
-	// RuleActionPass enables admins to provide exceptions to AdminNetworkPolicies and delegate this rule to
-	// K8s NetworkPolicies.
+	// AdminNetworkPolicyRuleActionPass enables admins to provide exceptions to
+	// AdminNetworkPolicies by passing rule execution directly to any matching
+	// K8s networkPolicies.
 	AdminNetworkPolicyRuleActionPass AdminNetworkPolicyRuleAction = "Pass"
-	// RuleActionDeny enables admins to deny specific traffic.
+	// AdminNetworkPolicyRuleActionDeny enables admins to deny specific traffic.
 	AdminNetworkPolicyRuleActionDeny AdminNetworkPolicyRuleAction = "Deny"
-	// RuleActionAllow enables admins to specifically allow certain traffic.
+	// AdminNetworkPolicyRuleActionAllow enables admins to specifically allow certain traffic.
 	AdminNetworkPolicyRuleActionAllow AdminNetworkPolicyRuleAction = "Allow"
 )
 

apis/v1alpha1/baselineadminnetworkpolicy_types.go

@@ -23,7 +23,7 @@ import (
 //+kubebuilder:subresource:status
 
 // BaselineAdminNetworkPolicy is a cluster level resource that is part of the
-// adminNetworkPolicy api.
+// adminNetworkPolicy API.
 type BaselineAdminNetworkPolicy struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata"`
@@ -33,7 +33,13 @@ type BaselineAdminNetworkPolicy struct {
 
 	// Status is the status to be reported by the implementation.
 	// +optional
-	Status AdminNetworkPolicyStatus `json:"status,omitempty"`
+	Status BaselineAdminNetworkPolicyStatus `json:"status,omitempty"`
+}
+
+// BaselineAdminNetworkPolicyStatus defines the observed state of
+// BaselineAdminNetworkPolicy.
+type BaselineAdminNetworkPolicyStatus struct {
+	Conditions []metav1.Condition `json:"conditions"`
 }
 
 // BaselineAdminNetworkPolicySpec defines the desired state of
@@ -42,47 +48,51 @@ type BaselineAdminNetworkPolicySpec struct {
 	// Subject defines the pods to which this BaselineAdminNetworkPolicy applies.
 	Subject AdminNetworkPolicySubject `json:"subject"`
 
-	// List of Ingress rules to be applied to the selected pods AFTER all
-	// AdminNetworkPolicy and NetworkPolicy rules have been applied.
+	// List of Ingress rules to be applied to the selected pods if they are not
+	// matched by any AdminNetworkPolicy or NetworkPolicy rules.
 	// A total of 100 Ingress rules will be allowed in each BANP instance.
 	// BANPs with no ingress rules do not affect ingress traffic.
 	// +optional
 	// +kubebuilder:validation:MaxItems=100
-	Ingress []AdminNetworkPolicyIngressRule `json:"ingress,omitempty"`
+	Ingress []BaselineAdminNetworkPolicyIngressRule `json:"ingress,omitempty"`
 
-	// List of Egress rules to be applied to the selected pods AFTER all
-	// AdminNetworkPolicy and NetworkPolicy rules have been applied.
-	// A total of 100 Egress rules will be allowed in each BANP instance. ANPs
+	// List of Egress rules to be applied to the selected pods if they are not
+	// matched by any AdminNetworkPolicy or NetworkPolicy rules.
+	// A total of 100 Egress rules will be allowed in each BANP instance. BANPs
 	// with no egress rules do not affect egress traffic.
 	// +optional
 	// +kubebuilder:validation:MaxItems=100
-	Egress []AdminNetworkPolicyEgressRule `json:"egress,omitempty"`
+	Egress []BaselineAdminNetworkPolicyEgressRule `json:"egress,omitempty"`
 }
 
 // BaselineAdminNetworkPolicyIngressRule describes an action to take on a particular
 // set of traffic destined for pods selected by a BaselineAdminNetworkPolicy's
 // Subject field. The traffic must match both ports and from.
 type BaselineAdminNetworkPolicyIngressRule struct {
 	// Name is an identifier for this rule, that may be no more than 100 characters
-	// in length.
+	// in length. This field should be used by the implementation to help
+	// improve observability, readability and error-reporting for any applied
+	// BaselineAdminNetworkPolicies.
 	// +optional
 	// +kubebuilder:validation:MaxLength=100
 	Name string `json:"name,omitempty"`
 
-	// Action specifies whether this rule must allow or deny traffic.
+	// Action specifies the affect this rule will have on matching traffic,
+	// currently the following actions are supported:
 	// Allow: allows the selected traffic
 	// Deny: denies the selected traffic
 	// This field is mandatory.
 	Action BaselineAdminNetworkPolicyRuleAction `json:"action"`
 
 	// Ports allows for matching traffic based on port and protocols.
-	// If Ports is empty or missing then traffic is not filtered via port.
+	// If Ports is not set then traffic is not filtered via port.
 	// +optional
 	Ports []AdminNetworkPolicyPort `json:"ports,omitempty"`
 
 	// List of sources whose traffic this AdminNetworkPolicyRule applies to.
-	// Items in this list are combined using a logical OR
-	// operation. This field must be defined and contain at least one item.
+	// If any adminNetworkPolicyPeer matches the source of incoming
+	// traffic then the specified action is applied.
+	// This field must be defined and contain at least one item.
 	// +kubebuilder:validation:MinItems=1
 	From []AdminNetworkPolicyPeer `json:"from"`
 }
@@ -92,25 +102,29 @@ type BaselineAdminNetworkPolicyIngressRule struct {
 // Subject field. The traffic must match both ports and to.
 type BaselineAdminNetworkPolicyEgressRule struct {
 	// Name is an identifier for this rule, that may be no more than 100 characters
-	// in length.
+	// in length. This field should be used by the implementation to help
+	// improve observability, readability and error-reporting for any applied
+	// BaselineAdminNetworkPolicies.
 	// +optional
 	// +kubebuilder:validation:MaxLength=100
 	Name string `json:"name,omitempty"`
 
-	// Action specifies whether this rule must pass, allow or deny traffic.
+	// Action specifies the affect this rule will have on matching traffic,
+	// currently the following actions are supported:
 	// Allow: allows the selected traffic
 	// Deny: denies the selected traffic
 	// This field is mandatory.
 	Action BaselineAdminNetworkPolicyRuleAction `json:"action"`
 
-	// Ports allows for matching  traffic based on port and protocols.
-	// If Ports is empty or missing then traffic is not filtered via port.
+	// Ports allows for matching traffic based on port and protocols.
+	// If Ports is not set then traffic is not filtered via port.
 	// +optional
 	Ports []AdminNetworkPolicyPort `json:"ports,omitempty"`
 
-	// List of destinations to which traffic will be allowed/denied/passed from the entities
-	// selected by this AdminNetworkPolicyRule. Items in this list are combined using a logical OR
-	// operation. This field must be defined and contain at least one item.
+	// List of destinations whose traffic this adminNetworkPolicyRule applies to.
+	// If any adminNetworkPolicyPeer matches the destination of outgoing
+	// traffic then the specified action is applied.
+	// This field must be defined and contain at least one item.
 	// +kubebuilder:validation:MinItems=1
 	To []AdminNetworkPolicyPeer `json:"to"`
 }
@@ -122,10 +136,10 @@ type BaselineAdminNetworkPolicyRuleAction string
 
 const (
 
-	// RuleActionDeny enables admins to deny specific traffic.
-	BaselineAdminNetworkPolicyRuleActionDeny AdminNetworkPolicyRuleAction = "Deny"
-	// RuleActionAllow enables admins to specifically allow certain traffic.
-	BaselineAdminNetworkPolicyRuleActionAllow AdminNetworkPolicyRuleAction = "Allow"
+	// BaselineAdminNetworkPolicyRuleActionDeny enables admins to deny specific traffic.
+	BaselineAdminNetworkPolicyRuleActionDeny BaselineAdminNetworkPolicyRuleAction = "Deny"
+	// BaselineAdminNetworkPolicyRuleActionAllow enables admins to specifically allow certain traffic.
+	BaselineAdminNetworkPolicyRuleActionAllow BaselineAdminNetworkPolicyRuleAction = "Allow"
 )
 
 //+kubebuilder:object:root=true

apis/v1alpha1/shared_types.go

@@ -20,11 +20,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// AdminNetworkPolicyStatus defines the observed state of AdminNetworkPolicy.
-type AdminNetworkPolicyStatus struct {
-	Conditions []metav1.Condition `json:"conditions"`
-}
-
 // AdminNetworkPolicySubject defines what resources the policy applies to.
 // Exactly one field must be set.
 // +kubebuilder:validation:MaxProperties=1
@@ -41,11 +36,11 @@ type AdminNetworkPolicySubject struct {
 // NamespacedPodSubject allows the user to select a given set of pod(s) in
 // selected namespace(s).
 type NamespacedPodSubject struct {
-	// This field follows standard label selector semantics; if empty,
+	// NamespaceSelector follows standard label selector semantics; if empty,
 	// it selects all Namespaces.
 	NamespaceSelector metav1.LabelSelector `json:"namespaceSelector"`
 
-	// Used to explicitly select pods within a namespace; if empty,
+	// PodSelector is used to explicitly select pods within a namespace; if empty,
 	// it selects all Pods.
 	PodSelector metav1.LabelSelector `json:"podSelector"`
 }
@@ -63,17 +58,17 @@ type AdminNetworkPolicyPort struct {
 	// +optional
 	Port *Port `json:"port,omitempty"`
 
-	// PortRange selects a portrange on a pod(s) based on provided start and end
+	// PortRange selects a port range on a pod(s) based on provided start and end
 	// values.
 	// +optional
 	PortRange *PortRange `json:"portRange,omitempty"`
 }
 
 type Port struct {
-	// The protocol (TCP, UDP, or SCTP) which traffic must match. If not specified, traffic
-	// is not filtered by protocol.
-	// +optional
-	Protocol *v1.Protocol `json:"protocol,omitempty"`
+	// The protocol (TCP, UDP, or SCTP) which traffic must match. If not specified,
+	// this field defaults to TCP.
+	// +kubebuilder:default=TCP
+	Protocol *v1.Protocol `json:"protocol"`
 
 	// Number defines a network port value.
 	// +kubebuilder:validation:Minimum=1
@@ -84,20 +79,20 @@ type Port struct {
 // PortRange defines an inclusive range of ports from the the assigned Start value
 // to End value.
 type PortRange struct {
-	// The protocol (TCP, UDP, or SCTP) which traffic must match. If not specified, traffic
-	// is not filtered by protocol.
-	// +optional
+	// The protocol (TCP, UDP, or SCTP) which traffic must match. If not specified,
+	// this field defaults to TCP.
+	// +kubebuilder:default=TCP
 	Protocol *v1.Protocol `json:"protocol,omitempty"`
 
-	// Start defines a network port that is the start of a portrange, the Start
+	// Start defines a network port that is the start of a port range, the Start
 	// value must be less than End.
 	// +kubebuilder:validation:Minimum=1
-	// +kubebuilder:validation:Maximum=65534
+	// +kubebuilder:validation:Maximum=65535
 	Start int32 `json:"start"`
 
-	// End defines a network port that is the end of a portrange, the End value
+	// End defines a network port that is the end of a port range, the End value
 	// must be greater than Start.
-	// +kubebuilder:validation:Minimum=2
+	// +kubebuilder:validation:Minimum=1
 	// +kubebuilder:validation:Maximum=65535
 	End int32 `json:"end"`
 }
@@ -138,11 +133,11 @@ type NamespacedPeer struct {
 	// +optional
 	Related *NamespaceRelation `json:"related,omitempty"`
 
-	// Selector is a labelSelector used to select Namespaces, This field
+	// NamespaceSelector is a labelSelector used to select Namespaces, This field
 	// follows standard label selector semantics; if present but empty, it selects
 	// all Namespaces.
 	// +optional
-	Selector *metav1.LabelSelector `json:"selector,omitempty"`
+	NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"`
 
 	// SameLabels is used to select a set of Namespaces that share the same values
 	// for a set of labels.
@@ -161,24 +156,14 @@ type NamespacedPeer struct {
 	NotSameLabels []string `json:"notSameLabels,omitempty"`
 }
 
-// PodPeer defines a flexible way to select pods in a cluster. Exactly one of the
-// selectors must be set.  If a consumer observes none of its fields are set,
-// they must assume an unknown option has been specified and fail closed.
-// +kubebuilder:validation:MaxProperties=1
-// +kubebuilder:validation:MinProperties=1
-type PodPeer struct {
-	// PodSelector is a labelSelector used to select Pods, This field is NOT optional,
-	// follows standard label selector semantics and if present but empty, it selects
-	// all Pods.
-	PodSelector *metav1.LabelSelector `json:"podSelector"`
-}
-
 // NamespacedPodPeer defines a flexible way to select Namespaces and pods in a
 // cluster. The `Namespaces` and `PodSelector` fields are required.
 type NamespacedPodPeer struct {
 	// Namespaces is used to select a set of Namespaces.
 	Namespaces NamespacedPeer `json:"namespaces"`
 
-	// Pods is used to select a set of Pods in the set of Namespaces.
-	Pods PodPeer `json:"pods"`
+	// PodSelector is a labelSelector used to select Pods, This field is NOT optional,
+	// follows standard label selector semantics and if present but empty, it selects
+	// all Pods.
+	PodSelector *metav1.LabelSelector `json:"podSelector"`
 }