Initial IPv6 support for GCE

Supporting IPv6 values where they can be set by the user, and ensuring
that IPv4 and IPv6 firewall rules are split because on GCP they cannot
be in the same rule.

pkg/model/gcemodel/api_loadbalancer.go

@@ -84,16 +84,13 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 
 	// Allow traffic into the API (port 443) from KubernetesAPIAccess CIDRs
 	{
-		t := &gcetasks.FirewallRule{
-			Name:         s(b.NameForFirewallRule("https-api")),
+		b.AddFirewallRulesTasks(c, "https-api", &gcetasks.FirewallRule{
 			Lifecycle:    b.Lifecycle,
 			Network:      b.LinkToNetwork(),
 			SourceRanges: b.Cluster.Spec.KubernetesAPIAccess,
 			TargetTags:   []string{b.GCETagForRole(kops.InstanceGroupRoleMaster)},
 			Allowed:      []string{"tcp:443"},
-		}
-		c.AddTask(t)
+		})
 	}
 	return nil
-
 }

pkg/model/gcemodel/external_access.go

@@ -49,8 +49,7 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.ModelBuilderContext) error {
 		// But I think we can always add more permissions in this case later, but we can't easily take them away
 		klog.V(2).Infof("bastion is in use; won't configure SSH access to master / node instances")
 	} else {
-		c.AddTask(&gcetasks.FirewallRule{
-			Name:         s(b.SafeObjectName("ssh-external-to-master")),
+		b.AddFirewallRulesTasks(c, "ssh-external-to-master", &gcetasks.FirewallRule{
 			Lifecycle:    b.Lifecycle,
 			TargetTags:   []string{b.GCETagForRole(kops.InstanceGroupRoleMaster)},
 			Allowed:      []string{"tcp:22"},
@@ -58,8 +57,7 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.ModelBuilderContext) error {
 			Network:      b.LinkToNetwork(),
 		})
 
-		c.AddTask(&gcetasks.FirewallRule{
-			Name:         s(b.SafeObjectName("ssh-external-to-node")),
+		b.AddFirewallRulesTasks(c, "ssh-external-to-node", &gcetasks.FirewallRule{
 			Lifecycle:    b.Lifecycle,
 			TargetTags:   []string{b.GCETagForRole(kops.InstanceGroupRoleNode)},
 			Allowed:      []string{"tcp:22"},
@@ -74,9 +72,9 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.ModelBuilderContext) error {
 		if err != nil {
 			return err
 		}
+
 		nodePortRangeString := nodePortRange.String()
-		t := &gcetasks.FirewallRule{
-			Name:       s(b.SafeObjectName("nodeport-external-to-node")),
+		b.AddFirewallRulesTasks(c, "nodeport-external-to-node", &gcetasks.FirewallRule{
 			Lifecycle:  b.Lifecycle,
 			TargetTags: []string{b.GCETagForRole(kops.InstanceGroupRoleNode)},
 			Allowed: []string{
@@ -85,13 +83,7 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.ModelBuilderContext) error {
 			},
 			SourceRanges: b.Cluster.Spec.NodePortAccess,
 			Network:      b.LinkToNetwork(),
-		}
-		if len(t.SourceRanges) == 0 {
-			// Empty SourceRanges is interpreted as 0.0.0.0/0 if tags are empty, so we set a SourceTag
-			// This is already covered by the normal node-to-node rules, but avoids opening the NodePort range
-			t.SourceTags = []string{b.GCETagForRole(kops.InstanceGroupRoleNode)}
-		}
-		c.AddTask(t)
+		})
 	}
 
 	if !b.UseLoadBalancerForAPI() {
@@ -100,8 +92,7 @@ func (b *ExternalAccessModelBuilder) Build(c *fi.ModelBuilderContext) error {
 		// We need to open security groups directly to the master nodes (instead of via the ELB)
 
 		// HTTPS to the master is allowed (for API access)
-		c.AddTask(&gcetasks.FirewallRule{
-			Name:         s(b.SafeObjectName("kubernetes-master-https")),
+		b.AddFirewallRulesTasks(c, "kubernetes-master-https", &gcetasks.FirewallRule{
 			Lifecycle:    b.Lifecycle,
 			TargetTags:   []string{b.GCETagForRole(kops.InstanceGroupRoleMaster)},
 			Allowed:      []string{"tcp:443"},

pkg/model/gcemodel/firewall.go

@@ -17,6 +17,9 @@ limitations under the License.
 package gcemodel
 
 import (
+	"net"
+	"strings"
+
 	"k8s.io/klog/v2"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/upup/pkg/fi"
@@ -63,15 +66,13 @@ func (b *FirewallModelBuilder) Build(c *fi.ModelBuilderContext) error {
 		// The traffic is not recognized if it's on the overlay network?
 		klog.Warningf("Adding overlay network for X -> node rule - HACK")
 
-		t := &gcetasks.FirewallRule{
-			Name:         s(b.SafeObjectName("cidr-to-node")),
+		b.AddFirewallRulesTasks(c, "cidr-to-node", &gcetasks.FirewallRule{
 			Lifecycle:    b.Lifecycle,
 			Network:      b.LinkToNetwork(),
 			SourceRanges: []string{b.Cluster.Spec.NonMasqueradeCIDR},
 			TargetTags:   []string{b.GCETagForRole(kops.InstanceGroupRoleNode)},
 			Allowed:      []string{"tcp", "udp", "icmp", "esp", "ah", "sctp"},
-		}
-		c.AddTask(t)
+		})
 	}
 
 	// Allow full traffic from master -> master
@@ -116,15 +117,49 @@ func (b *FirewallModelBuilder) Build(c *fi.ModelBuilderContext) error {
 	if b.Cluster.Spec.NonMasqueradeCIDR != "" {
 		// The traffic is not recognized if it's on the overlay network?
 		klog.Warningf("Adding overlay network for X -> master rule - HACK")
-		t := &gcetasks.FirewallRule{
-			Name:         s(b.SafeObjectName("cidr-to-master")),
+
+		b.AddFirewallRulesTasks(c, "cidr-to-master", &gcetasks.FirewallRule{
 			Lifecycle:    b.Lifecycle,
 			Network:      b.LinkToNetwork(),
 			SourceRanges: []string{b.Cluster.Spec.NonMasqueradeCIDR},
 			TargetTags:   []string{b.GCETagForRole(kops.InstanceGroupRoleMaster)},
 			Allowed:      []string{"tcp:443", "tcp:4194"},
-		}
-		c.AddTask(t)
+		})
 	}
 	return nil
 }
+
+// AddFirewallRulesTasks creates and adds ipv4 and ipv6 gcetasks.FirewallRule Tasks.
+// GCE does not allow us to mix ipv4 and ipv6 in the same firewall rule, so we must create separate rules.
+// Furthermore, an empty SourceRange with empty SourceTags is interpreted as allow-everything,
+// but we intend for it to block everything; so we can Disabled to achieve the desired blocking.
+func (b *GCEModelContext) AddFirewallRulesTasks(c *fi.ModelBuilderContext, name string, rule *gcetasks.FirewallRule) {
+	var ipv4SourceRanges []string
+	var ipv6SourceRanges []string
+	for _, sourceRange := range rule.SourceRanges {
+		_, cidr, err := net.ParseCIDR(sourceRange)
+		if err != nil {
+			klog.Fatalf("failed to parse invalid sourceRange %q", sourceRange)
+		}
+
+		// Split into ipv4s and ipv6s, but treat IPv4-mapped IPv6 addresses as IPv6
+		if cidr.IP.To4() != nil && !strings.Contains(sourceRange, ":") {
+			ipv4SourceRanges = append(ipv4SourceRanges, sourceRange)
+		} else {
+			ipv6SourceRanges = append(ipv6SourceRanges, sourceRange)
+
+		}
+	}
+
+	ipv4 := *rule
+	ipv4.Name = s(b.NameForFirewallRule(name))
+	ipv4.SourceRanges = ipv4SourceRanges
+	ipv4.DisableIfEmptySourceRanges()
+	c.AddTask(&ipv4)
+
+	ipv6 := *rule
+	ipv6.Name = s(b.NameForFirewallRule(name + "-ipv6"))
+	ipv6.SourceRanges = ipv6SourceRanges
+	ipv6.DisableIfEmptySourceRanges()
+	c.AddTask(&ipv6)
+}

pkg/resources/gce/gce.go

@@ -49,7 +49,8 @@ const (
 )
 
 // Maximum number of `-` separated tokens in a name
-const maxPrefixTokens = 4
+// Example: nodeport-external-to-node-ipv6
+const maxPrefixTokens = 5
 
 func ListResourcesGCE(gceCloud gce.GCECloud, clusterName string, region string) (map[string]*resources.Resource, error) {
 	if region == "" {

upup/pkg/fi/cloudup/gcetasks/firewallrule.go

@@ -18,6 +18,7 @@ package gcetasks
 
 import (
 	"fmt"
+	"net"
 	"strings"
 
 	compute "google.golang.org/api/compute/v1"
@@ -38,6 +39,12 @@ type FirewallRule struct {
 	SourceRanges []string
 	TargetTags   []string
 	Allowed      []string
+
+	// Disabled: Denotes whether the firewall rule is disabled. When set to
+	// true, the firewall rule is not enforced and the network behaves as if
+	// it did not exist. If this is unspecified, the firewall rule will be
+	// enabled.
+	Disabled bool
 }
 
 var _ fi.CompareWithID = &FirewallRule{}
@@ -63,6 +70,7 @@ func (e *FirewallRule) Find(c *fi.Context) (*FirewallRule, error) {
 	actual.TargetTags = r.TargetTags
 	actual.SourceRanges = r.SourceRanges
 	actual.SourceTags = r.SourceTags
+	actual.Disabled = r.Disabled
 	for _, a := range r.Allowed {
 		actual.Allowed = append(actual.Allowed, serializeFirewallAllowed(a))
 	}
@@ -74,9 +82,60 @@ func (e *FirewallRule) Find(c *fi.Context) (*FirewallRule, error) {
 }
 
 func (e *FirewallRule) Run(c *fi.Context) error {
+	if err := e.sanityCheck(); err != nil {
+		return err
+	}
 	return fi.DefaultDeltaRunMethod(e, c)
 }
 
+// sanityCheck applies some validation that isn't technically required,
+// but avoids some problems with surprising behaviours.
+func (e *FirewallRule) sanityCheck() error {
+	if !e.Disabled {
+		// Treat it as an error if SourceRanges _and_ SourceTags empty with Disabled=false
+		// this is interpreted as SourceRanges="0.0.0.0/0", which is likely what was intended.
+		if len(e.SourceRanges) == 0 && len(e.SourceTags) == 0 {
+			return fmt.Errorf("either SourceRanges or SourceTags should be specified when Disabled is false")
+		}
+	} else {
+		// Treat it as an error if SourceRanges/SourceTags non-empty with Disabled
+		// this is allowed but is likely not what was intended.
+		if len(e.SourceRanges) != 0 || len(e.SourceTags) != 0 {
+			return fmt.Errorf("setting Disabled=true overrules SourceRanges or SourceTags")
+		}
+	}
+
+	// Treat it as an error if SourceRanges _and_ SourceTags both set;
+	// this is interpreted as OR, not AND, which is likely not what was intended.
+	if len(e.SourceRanges) != 0 && len(e.SourceTags) != 0 {
+		return fmt.Errorf("SourceRanges and SourceTags should not both be specified")
+	}
+
+	name := fi.StringValue(e.Name)
+
+	// Make sure we've split the ipv4 / ipv6 addresses.
+	// A single firewall rule can't mix ipv4 and ipv6 addresses, so we split them into two rules.
+	for _, sourceRange := range e.SourceRanges {
+		_, cidr, err := net.ParseCIDR(sourceRange)
+		if err != nil {
+			return fmt.Errorf("sourceRange %q is not valid: %w", sourceRange, err)
+		}
+		if cidr.IP.To4() != nil {
+			// IPv4
+			if strings.Contains(name, "-ipv6") {
+				return fmt.Errorf("ipv4 ranges should not be in a ipv6-named rule (found %s in %s)", sourceRange, name)
+			}
+		} else {
+			// IPv6
+			if !strings.Contains(name, "-ipv6") {
+				return fmt.Errorf("ipv6 ranges should be in a ipv6-named rule (found %s in %s)", sourceRange, name)
+			}
+		}
+	}
+
+	return nil
+}
+
 func (_ *FirewallRule) CheckChanges(a, e, changes *FirewallRule) error {
 	if e.Network == nil {
 		return fi.RequiredField("Network")
@@ -132,6 +191,7 @@ func (e *FirewallRule) mapToGCE(project string) (*compute.Firewall, error) {
 		SourceRanges: e.SourceRanges,
 		TargetTags:   e.TargetTags,
 		Allowed:      allowed,
+		Disabled:     e.Disabled,
 	}
 	return firewall, nil
 }
@@ -173,6 +233,8 @@ type terraformFirewall struct {
 
 	SourceRanges []string `json:"source_ranges,omitempty" cty:"source_ranges"`
 	TargetTags   []string `json:"target_tags,omitempty" cty:"target_tags"`
+
+	Disabled bool `json:"disabled,omitempty" cty:"disabled"`
 }
 
 func (_ *FirewallRule) RenderTerraform(t *terraform.TerraformTarget, a, e, changes *FirewallRule) error {
@@ -198,6 +260,7 @@ func (_ *FirewallRule) RenderTerraform(t *terraform.TerraformTarget, a, e, chang
 		TargetTags:   g.TargetTags,
 		SourceTags:   g.SourceTags,
 		Allowed:      allowed,
+		Disabled:     g.Disabled,
 	}
 
 	// TODO: This doesn't seem right, but it looks like a TF problem
@@ -205,3 +268,13 @@ func (_ *FirewallRule) RenderTerraform(t *terraform.TerraformTarget, a, e, chang
 
 	return t.RenderResource("google_compute_firewall", *e.Name, tf)
 }
+
+// DisableIfEmptySourceRanges sets Disabled if SourceRanges is empty.
+// This is helpful because empty SourceRanges and SourceTags are interpreted as allow everything,
+// but the intent is usually to block everything, which can be achieved with Disabled=true.
+func (e *FirewallRule) DisableIfEmptySourceRanges() *FirewallRule {
+	if len(e.SourceRanges) == 0 {
+		e.Disabled = true
+	}
+	return e
+}