Merge pull request #15280 from justinsb/gce_firewall_rule_family

GCE FirewallRule: Use an explicit field for ipv4 vs ipv6
This commit is contained in:
Kubernetes Prow Robot 2023-03-31 07:19:50 -07:00 committed by GitHub
commit 853165f6e6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 19 additions and 4 deletions

View File

@ -144,8 +144,7 @@ func (b *FirewallModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
if err != nil { if err != nil {
return err return err
} }
c.AddTask(&gcetasks.FirewallRule{ b.AddFirewallRulesTasks(c, "pod-cidrs-to-node", &gcetasks.FirewallRule{
Name: s(b.NameForFirewallRule("pod-cidrs-to-node")),
Lifecycle: b.Lifecycle, Lifecycle: b.Lifecycle,
Network: network, Network: network,
SourceRanges: []string{b.Cluster.Spec.Networking.PodCIDR}, SourceRanges: []string{b.Cluster.Spec.Networking.PodCIDR},
@ -180,6 +179,7 @@ func (b *GCEModelContext) AddFirewallRulesTasks(c *fi.CloudupModelBuilderContext
ipv4 := *rule ipv4 := *rule
ipv4.Name = s(b.NameForFirewallRule(name)) ipv4.Name = s(b.NameForFirewallRule(name))
ipv4.Family = gcetasks.AddressFamilyIPv4
ipv4.SourceRanges = ipv4SourceRanges ipv4.SourceRanges = ipv4SourceRanges
if len(ipv4.SourceRanges) == 0 { if len(ipv4.SourceRanges) == 0 {
// This is helpful because empty SourceRanges and SourceTags are interpreted as allow everything, // This is helpful because empty SourceRanges and SourceTags are interpreted as allow everything,
@ -191,6 +191,7 @@ func (b *GCEModelContext) AddFirewallRulesTasks(c *fi.CloudupModelBuilderContext
ipv6 := *rule ipv6 := *rule
ipv6.Name = s(b.NameForFirewallRule(name + "-ipv6")) ipv6.Name = s(b.NameForFirewallRule(name + "-ipv6"))
ipv6.Family = gcetasks.AddressFamilyIPv6
ipv6.SourceRanges = ipv6SourceRanges ipv6.SourceRanges = ipv6SourceRanges
if len(ipv6.SourceRanges) == 0 { if len(ipv6.SourceRanges) == 0 {
// We specify explicitly so the rule is in IPv6 mode // We specify explicitly so the rule is in IPv6 mode

View File

@ -28,10 +28,16 @@ import (
"k8s.io/kops/upup/pkg/fi/cloudup/terraformWriter" "k8s.io/kops/upup/pkg/fi/cloudup/terraformWriter"
) )
type AddressFamily string
const AddressFamilyIPv4 AddressFamily = "ipv4"
const AddressFamilyIPv6 AddressFamily = "ipv6"
// FirewallRule represents a GCE firewall rules // FirewallRule represents a GCE firewall rules
// +kops:fitask // +kops:fitask
type FirewallRule struct { type FirewallRule struct {
Name *string Name *string
Family AddressFamily
Lifecycle fi.Lifecycle Lifecycle fi.Lifecycle
Network *Network Network *Network
@ -78,6 +84,7 @@ func (e *FirewallRule) Find(c *fi.CloudupContext) (*FirewallRule, error) {
// Ignore "system" fields // Ignore "system" fields
actual.Lifecycle = e.Lifecycle actual.Lifecycle = e.Lifecycle
actual.Family = e.Family
return actual, nil return actual, nil
} }
@ -112,14 +119,21 @@ func (e *FirewallRule) Normalize(c *fi.CloudupContext) error {
if err != nil { if err != nil {
return fmt.Errorf("sourceRange %q is not valid: %w", sourceRange, err) return fmt.Errorf("sourceRange %q is not valid: %w", sourceRange, err)
} }
if e.Family == "" {
// This is our own requirement, just for consistency checking.
// Previous we used the name, but that was confused when the cluster name was ipv6.example.com
return fmt.Errorf("must set Family when using SourceRanges")
}
if cidr.IP.To4() != nil { if cidr.IP.To4() != nil {
// IPv4 // IPv4
if strings.Contains(name, "-ipv6") { if e.Family != AddressFamilyIPv4 {
return fmt.Errorf("ipv4 ranges should not be in a ipv6-named rule (found %s in %s)", sourceRange, name) return fmt.Errorf("ipv4 ranges should not be in a ipv6-named rule (found %s in %s)", sourceRange, name)
} }
} else { } else {
// IPv6 // IPv6
if !strings.Contains(name, "-ipv6") { if e.Family != AddressFamilyIPv6 {
return fmt.Errorf("ipv6 ranges should be in a ipv6-named rule (found %s in %s)", sourceRange, name) return fmt.Errorf("ipv6 ranges should be in a ipv6-named rule (found %s in %s)", sourceRange, name)
} }
} }