Revisit Security Groups (#20)

Typical trying to be too clever without any end user testing scenarios.
The top level API isn't too far off, however breaking the changes up at
the API into the CRDs is just wrong.  We now do that much lower down the
stack, and ultimately should do it in the region controller to avoid too
much cohesion between all the moving parts.

charts/compute/crds/compute.unikorn-cloud.org_computeclusters.yaml

@@ -122,60 +122,41 @@ spec:
                           x-kubernetes-int-or-string: true
                         firewall:
                           description: Firewall is the workload pool firewall configuration.
-                          properties:
-                            ingress:
-                              description: Ingress is a list of firewall rules applied
-                                to a workload pool.
-                              items:
-                                properties:
-                                  cidr:
-                                    description: CIDR is the CIDR block to allow traffic
-                                      from.
-                                    pattern: ^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])\/(?:3[0-2]|[1-2]?[0-9])$
-                                    type: string
-                                  id:
-                                    description: ID is the firewall rule identifier.
-                                    type: string
-                                  port:
-                                    description: Port is the port or range of ports.
-                                    properties:
-                                      number:
-                                        description: Number is the port number.
-                                        type: integer
-                                      range:
-                                        description: Range is the port range.
-                                        properties:
-                                          end:
-                                            description: End is the end of the range.
-                                            maximum: 65535
-                                            type: integer
-                                          start:
-                                            description: Start is the start of the
-                                              range.
-                                            minimum: 1
-                                            type: integer
-                                        required:
-                                        - end
-                                        - start
-                                        type: object
-                                    type: object
-                                    x-kubernetes-validations:
-                                    - message: at least one of number or range must
-                                        be defined
-                                      rule: (has(self.number) || has(self.range))
-                                  protocol:
-                                    description: Protocol The protocol to allow.
-                                    enum:
-                                    - tcp
-                                    - udp
-                                    type: string
-                                required:
-                                - cidr
-                                - port
-                                - protocol
-                                type: object
-                              type: array
-                          type: object
+                          items:
+                            properties:
+                              cidr:
+                                description: Prefixes is the CIDR block to allow traffic
+                                  from.
+                                items:
+                                  pattern: ^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])\/(?:3[0-2]|[1-2]?[0-9])$
+                                  type: string
+                                type: array
+                              direction:
+                                description: Direction of traffic flow.
+                                enum:
+                                - ingress
+                                - egress
+                                type: string
+                              port:
+                                description: Port is the port or start of a range
+                                  of ports.
+                                type: integer
+                              portMax:
+                                description: PortMax is the end of a range of ports.
+                                type: integer
+                              protocol:
+                                description: Protocol The protocol to allow.
+                                enum:
+                                - tcp
+                                - udp
+                                type: string
+                            required:
+                            - cidr
+                            - direction
+                            - port
+                            - protocol
+                            type: object
+                          type: array
                         flavorId:
                           description: Flavor is the regions service flavor to deploy
                             with.

pkg/apis/unikorn/v1alpha1/helpers.go

@@ -18,7 +18,6 @@ package v1alpha1
 
 import (
 	"errors"
-	"fmt"
 
 	unikornv1core "github.com/unikorn-cloud/core/pkg/apis/unikorn/v1alpha1"
 	"github.com/unikorn-cloud/core/pkg/constants"
@@ -93,11 +92,3 @@ func (c *ComputeCluster) GetWorkloadPoolStatus(name string) *WorkloadPoolStatus
 
 	return &c.Status.WorkloadPools[len(c.Status.WorkloadPools)-1]
 }
-
-func (p *FirewallRulePort) String() string {
-	if p.Number != nil {
-		return fmt.Sprintf("%d", *p.Number)
-	}
-
-	return fmt.Sprintf("%d-%d", p.Range.Start, p.Range.End)
-}

pkg/apis/unikorn/v1alpha1/types.go

@@ -31,7 +31,7 @@ type ComputeWorkloadPoolSpec struct {
 	// PublicIPAllocation is the workload pool public IP allocation configuration.
 	PublicIPAllocation *PublicIPAllocationSpec `json:"publicIpAllocation,omitempty"`
 	// Firewall is the workload pool firewall configuration.
-	Firewall *FirewallSpec `json:"firewall,omitempty"`
+	Firewall []FirewallRule `json:"firewall,omitempty"`
 	// UserData contains configuration information or scripts to use upon launch.
 	UserData []byte `json:"userData,omitempty"`
 }
@@ -41,21 +41,13 @@ type PublicIPAllocationSpec struct {
 	Enabled bool `json:"enabled,omitempty"`
 }
 
-type FirewallSpec struct {
-	// Ingress is a list of firewall rules applied to a workload pool.
-	Ingress []FirewallRule `json:"ingress,omitempty"`
-}
+// +kubebuilder:validation:Enum=ingress;egress
+type FirewallRuleDirection string
 
-type FirewallRule struct {
-	// ID is the firewall rule identifier.
-	ID string `json:"id,omitempty"`
-	// Protocol The protocol to allow.
-	Protocol FirewallRuleProtocol `json:"protocol"`
-	// CIDR is the CIDR block to allow traffic from.
-	CIDR unikornv1core.IPv4Prefix `json:"cidr"`
-	// Port is the port or range of ports.
-	Port FirewallRulePort `json:"port"`
-}
+const (
+	Ingress FirewallRuleDirection = "ingress"
+	Egress  FirewallRuleDirection = "egress"
+)
 
 // +kubebuilder:validation:Enum=tcp;udp
 type FirewallRuleProtocol string
@@ -65,21 +57,17 @@ const (
 	UDP FirewallRuleProtocol = "udp"
 )
 
-// +kubebuilder:validation:XValidation:message="at least one of number or range must be defined",rule=(has(self.number) || has(self.range))
-type FirewallRulePort struct {
-	// Number is the port number.
-	Number *int `json:"number,omitempty"`
-	// Range is the port range.
-	Range *FirewallRulePortRange `json:"range,omitempty"`
-}
-
-type FirewallRulePortRange struct {
-	// Start is the start of the range.
-	// +kubebuilder:validation:Minimum=1
-	Start int `json:"start"`
-	// End is the end of the range.
-	// +kubebuilder:validation:Maximum=65535
-	End int `json:"end"`
+type FirewallRule struct {
+	// Direction of traffic flow.
+	Direction FirewallRuleDirection `json:"direction"`
+	// Protocol The protocol to allow.
+	Protocol FirewallRuleProtocol `json:"protocol"`
+	// Prefixes is the CIDR block to allow traffic from.
+	Prefixes []unikornv1core.IPv4Prefix `json:"cidr"`
+	// Port is the port or start of a range of ports.
+	Port int `json:"port"`
+	// PortMax is the end of a range of ports.
+	PortMax *int `json:"portMax,omitempty"`
 }
 
 // ComputeClusterList is a typed list of compute clusters.