Respond to Round 3 of Dan's Comments

- Some smaller changes in grammer and wording.
- Internal ordering
- TODOs, figure out if Pointer to a slice is valid,
and Combine Same/NotSame Labels into a single field

Signed-off-by: astoycos <[email protected]>

Author: astoycos

apis/v1alpha1/adminnetworkpolicy_types.go

@@ -22,7 +22,8 @@ import (
 //+kubebuilder:object:root=true
 //+kubebuilder:subresource:status
 
-// AdminNetworkPolicy is the Schema for the adminnetworkpolicies API.
+// AdminNetworkPolicy is  a cluster level resource that is part of the
+// AdminNetworkPolicy API.
 type AdminNetworkPolicy struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata"`
@@ -42,36 +43,35 @@ type AdminNetworkPolicyStatus struct {
 
 // AdminNetworkPolicySpec defines the desired state of AdminNetworkPolicy.
 type AdminNetworkPolicySpec struct {
-	// Priority is an int32 value bound to 0 - 1000, the lowest priority,
-	// "0" corresponds to the highest importance, while higher priorities have
-	// lower importance.
+	// Priority is a value from 0 to 1000. Rules with lower priority values have
+	// higher precedence, and are checked before rules with higher priority values.
+	// All AdminNetworkPolicy rules have higher precedence than NetworkPolicy or
+	// BaselineAdminNetworkPolicy rules
 	// +kubebuilder:validation:Minimum=0
 	// +kubebuilder:validation:Maximum=1000
 	Priority int32 `json:"priority"`
 
 	// Subject defines the pods to which this AdminNetworkPolicy applies.
 	Subject AdminNetworkPolicySubject `json:"subject"`
 
-	// List of Ingress rules to be applied to the selected pods BEFORE any
-	// 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.
+	// Ingress is the list of Ingress rules to be applied to the selected pods.
+	// 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 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.
+	// Egress is the list of Egress rules to be applied to the selected pods.
+	// A total of 100 rules will be allowed in each ANP instance. ANPs with no
+	// egress rules do not affect egress traffic.
 	// +optional
 	// +kubebuilder:validation:MaxItems=100
 	Egress []AdminNetworkPolicyEgressRule `json:"egress,omitempty"`
 }
 
 // AdminNetworkPolicyIngressRule describes an action to take on a particular
 // set of traffic destined for pods selected by an AdminNetworkPolicy's
-// Subject field. The traffic must match both ports and from.
+// Subject field.
 type AdminNetworkPolicyIngressRule struct {
 	// Name is an identifier for this rule, that may be no more than 100 characters
 	// in length. This field should be used by the implementation to help
@@ -81,35 +81,34 @@ type AdminNetworkPolicyIngressRule struct {
 	// +kubebuilder:validation:MaxLength=100
 	Name string `json:"name,omitempty"`
 
-	// Action specifies the affect this rule will have on matching traffic,
-	// currently the following actions are supported:
-	// Allow: allows the selected traffic
+	// Action specifies the effect 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
 	// Pass: instructs the selected traffic to skip any remaining ANP rules, and
 	// then pass execution to any NetworkPolicies that select the pod.
 	// If the pod is not selected by any NetworkPolicies then execution
 	// is passed to any BaselineAdminNetworkPolicies that select the pod.
-	// This field is mandatory.
 	Action AdminNetworkPolicyRuleAction `json:"action"`
 
-	// 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.
-	// If any adminNetworkPolicyPeer matches the source of incoming
+	// From is the list of sources whose traffic this rule applies to.
+	// 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"`
+
+	// Ports allows for matching traffic based on port and protocols.
+	// If Ports is not set then the rule does not filter traffic via port.
+	// +optional
+	// +kubebuilder:validation:MaxItems=100
+	Ports *[]AdminNetworkPolicyPort `json:"ports,omitempty"`
 }
 
 // AdminNetworkPolicyEgressRule describes an action to take on a particular
 // set of traffic originating from pods selected by a AdminNetworkPolicy's
-// Subject field. The traffic must match both ports and to.
+// Subject field.
 type AdminNetworkPolicyEgressRule struct {
 	// Name is an identifier for this rule, that may be no more than 100 characters
 	// in length. This field should be used by the implementation to help
@@ -119,45 +118,51 @@ type AdminNetworkPolicyEgressRule struct {
 	// +kubebuilder:validation:MaxLength=100
 	Name string `json:"name,omitempty"`
 
-	// Action specifies the affect this rule will have on matching traffic,
-	// currently the following actions are supported:
+	// Action specifies the effect 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)
+	// Deny: denies the selected traffic
 	// Pass: instructs the selected traffic to skip any remaining ANP rules, and
 	// then pass execution to any NetworkPolicies that select the pod.
 	// If the pod is not selected by any NetworkPolicies then execution
 	// is passed to any BaselineAdminNetworkPolicies that select the pod.
-	// This field is mandatory.
 	Action AdminNetworkPolicyRuleAction `json:"action"`
 
-	// 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 destinations whose traffic this adminNetworkPolicyRule applies to.
-	// If any adminNetworkPolicyPeer matches the destination of outgoing
+	// To is the List of destinations whose traffic this rule 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"`
+
+	// Ports allows for matching traffic based on port and protocols.
+	// If Ports is not set then the rule does not filter traffic via port.
+	// +optional
+	// +kubebuilder:validation:MaxItems=100
+	Ports *[]AdminNetworkPolicyPort `json:"ports,omitempty"`
 }
 
 // AdminNetworkPolicyRuleAction string describes the AdminNetworkPolicy action type.
 // +enum
 type AdminNetworkPolicyRuleAction string
 
 const (
-	// AdminNetworkPolicyRuleActionPass enables admins to provide exceptions to
-	// AdminNetworkPolicies by passing rule execution directly to any matching
-	// K8s networkPolicies.
-	AdminNetworkPolicyRuleActionPass AdminNetworkPolicyRuleAction = "Pass"
-	// AdminNetworkPolicyRuleActionDeny enables admins to deny specific traffic.
-	AdminNetworkPolicyRuleActionDeny AdminNetworkPolicyRuleAction = "Deny"
-	// AdminNetworkPolicyRuleActionAllow enables admins to specifically allow certain traffic.
+	// AdminNetworkPolicyRuleActionAllow indicates that matching traffic will be
+	// allowed regardless of NetworkPolicy and BaselineAdminNetworkPolicy
+	// rules. Users cannot block traffic which has been matched by an "Allow"
+	// rule in an AdminNetworkPolicy.
 	AdminNetworkPolicyRuleActionAllow AdminNetworkPolicyRuleAction = "Allow"
+	// AdminNetworkPolicyRuleActionDeny indicates that matching traffic will be
+	// denied before being checked against NetworkPolicy or
+	// BaselineAdminNetworkPolicy rules. Pods will never receive traffic which
+	// has been matched by a "Deny" rule in an AdminNetworkPolicy.
+	AdminNetworkPolicyRuleActionDeny AdminNetworkPolicyRuleAction = "Deny"
+	// AdminNetworkPolicyRuleActionPass indicates that matching traffic will
+	// bypass further AdminNetworkPolicy processing (ignoring rules with lower
+	// precedence) and be allowed or denied based on NetworkPolicy and
+	// BaselineAdminNetworkPolicy rules.
+	AdminNetworkPolicyRuleActionPass AdminNetworkPolicyRuleAction = "Pass"
 )
 
 //+kubebuilder:object:root=true

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"`
@@ -48,16 +48,16 @@ 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 if they are not
-	// matched by any AdminNetworkPolicy or NetworkPolicy rules.
+	// Ingress is the 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 []BaselineAdminNetworkPolicyIngressRule `json:"ingress,omitempty"`
 
-	// List of Egress rules to be applied to the selected pods if they are not
-	// matched by any AdminNetworkPolicy or NetworkPolicy rules.
+	// Egress is the 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
@@ -67,7 +67,7 @@ type BaselineAdminNetworkPolicySpec struct {
 
 // 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.
+// Subject field.
 type BaselineAdminNetworkPolicyIngressRule struct {
 	// Name is an identifier for this rule, that may be no more than 100 characters
 	// in length. This field should be used by the implementation to help
@@ -77,29 +77,29 @@ type BaselineAdminNetworkPolicyIngressRule struct {
 	// +kubebuilder:validation:MaxLength=100
 	Name string `json:"name,omitempty"`
 
-	// Action specifies the affect this rule will have on matching traffic,
-	// currently the following actions are supported:
+	// Action specifies the effect 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 not set then traffic is not filtered via port.
-	// +optional
-	Ports []AdminNetworkPolicyPort `json:"ports,omitempty"`
-
-	// List of sources whose traffic this AdminNetworkPolicyRule applies to.
-	// If any adminNetworkPolicyPeer matches the source of incoming
+	// From is the list of sources whose traffic this rule applies to.
+	// 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"`
+
+	// Ports allows for matching traffic based on port and protocols.
+	// If Ports is not set then the rule does not filter traffic via port.
+	// +optional
+	// +kubebuilder:validation:MaxItems=100
+	Ports *[]AdminNetworkPolicyPort `json:"ports,omitempty"`
 }
 
 // AdminNetworkPolicyEgressRule describes an action to take on a particular
 // set of traffic originating from pods selected by a BaselineAdminNetworkPolicy's
-// Subject field. The traffic must match both ports and to.
+// Subject field.
 type BaselineAdminNetworkPolicyEgressRule struct {
 	// Name is an identifier for this rule, that may be no more than 100 characters
 	// in length. This field should be used by the implementation to help
@@ -109,24 +109,24 @@ type BaselineAdminNetworkPolicyEgressRule struct {
 	// +kubebuilder:validation:MaxLength=100
 	Name string `json:"name,omitempty"`
 
-	// Action specifies the affect this rule will have on matching traffic,
-	// currently the following actions are supported:
+	// Action specifies the effect 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 not set then traffic is not filtered via port.
-	// +optional
-	Ports []AdminNetworkPolicyPort `json:"ports,omitempty"`
-
-	// List of destinations whose traffic this adminNetworkPolicyRule applies to.
-	// If any adminNetworkPolicyPeer matches the destination of outgoing
+	// To is the list of destinations whose traffic this rule 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"`
+
+	// Ports allows for matching traffic based on port and protocols.
+	// If Ports is not set then the rule does not filter traffic via port.
+	// +optional
+	// +kubebuilder:validation:MaxItems=100
+	Ports *[]AdminNetworkPolicyPort `json:"ports,omitempty"`
 }
 
 // BaselineAdminNetworkPolicyRuleAction string describes the BaselineAdminNetworkPolicy
@@ -135,10 +135,9 @@ type BaselineAdminNetworkPolicyEgressRule struct {
 type BaselineAdminNetworkPolicyRuleAction string
 
 const (
-
-	// BaselineAdminNetworkPolicyRuleActionDeny enables admins to deny specific traffic.
+	// BaselineAdminNetworkPolicyRuleActionDeny enables admins to deny traffic.
 	BaselineAdminNetworkPolicyRuleActionDeny BaselineAdminNetworkPolicyRuleAction = "Deny"
-	// BaselineAdminNetworkPolicyRuleActionAllow enables admins to specifically allow certain traffic.
+	// BaselineAdminNetworkPolicyRuleActionAllow enables admins to allow certain traffic.
 	BaselineAdminNetworkPolicyRuleActionAllow BaselineAdminNetworkPolicyRuleAction = "Allow"
 )
 

apis/v1alpha1/shared_types.go

@@ -50,13 +50,13 @@ type NamespacedPodSubject struct {
 // +kubebuilder:validation:MaxProperties=1
 // +kubebuilder:validation:MinProperties=1
 type AdminNetworkPolicyPort struct {
-	// NamedPort selects a port on a pod(s) based on name.
+	// Port selects a port on a pod(s) based on number.
 	// +optional
-	NamedPort *string `json:"namedPort,omitempty"`
+	PortNumber *Port `json:"portNumber,omitempty"`
 
-	// Port selects a port on a pod(s) based on number.
+	// NamedPort selects a port on a pod(s) based on name.
 	// +optional
-	Port *Port `json:"port,omitempty"`
+	NamedPort *string `json:"namedPort,omitempty"`
 
 	// PortRange selects a port range on a pod(s) based on provided start and end
 	// values.
@@ -65,24 +65,24 @@ type AdminNetworkPolicyPort struct {
 }
 
 type Port struct {
-	// The protocol (TCP, UDP, or SCTP) which traffic must match. If not specified,
-	// this field defaults to TCP.
+	// Protocol is the network 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"`
+	Protocol v1.Protocol `json:"protocol"`
 
 	// Number defines a network port value.
 	// +kubebuilder:validation:Minimum=1
 	// +kubebuilder:validation:Maximum=65535
-	Number int32 `json:"number"`
+	Port int32 `json:"port"`
 }
 
 // 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,
-	// this field defaults to TCP.
+	// Protocol is the network 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"`
+	Protocol v1.Protocol `json:"protocol,omitempty"`
 
 	// Start defines a network port that is the start of a port range, the Start
 	// value must be less than End.
@@ -107,7 +107,7 @@ type AdminNetworkPolicyPeer struct {
 	// Namespaces defines a way to select a set of Namespaces.
 	// +optional
 	Namespaces *NamespacedPeer `json:"namespaces,omitempty"`
-	// NamespacedPods defines a way to select a set of pods in
+	// Pods defines a way to select a set of pods in
 	// in a set of namespaces.
 	// +optional
 	Pods *NamespacedPodPeer `json:"pods,omitempty"`
@@ -116,8 +116,8 @@ type AdminNetworkPolicyPeer struct {
 type NamespaceRelation string
 
 const (
-	NamespaceSame    NamespaceRelation = "Self"
-	NamespaceNotSame NamespaceRelation = "NotSelf"
+	NamespaceSelf    NamespaceRelation = "Self"
+	NamespaceNotSelf NamespaceRelation = "NotSelf"
 )
 
 // NamespacedPeer defines a flexible way to select Namespaces in a cluster.
@@ -127,9 +127,10 @@ const (
 // +kubebuilder:validation:MaxProperties=1
 // +kubebuilder:validation:MinProperties=1
 type NamespacedPeer struct {
-	// Related provides a mechanism for selecting namespaces. It can be set to
-	// "Self" to select all pods in the subject's namespace, and set to "NotSelf"
-	// to select all pods not in the subject's namespace.
+	// Related provides a mechanism for selecting namespaces relative to the
+	// subject pod. A value of "Self" matches the subject pod's namespace,
+	// while a value of "NotSelf" matches namespaces other than the subject
+	// pod's namespace.
 	// +optional
 	Related *NamespaceRelation `json:"related,omitempty"`
 
@@ -165,5 +166,5 @@ type NamespacedPodPeer 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"`
+	PodSelector metav1.LabelSelector `json:"podSelector"`
 }