diff --git a/pkg/model/openstackmodel/BUILD.bazel b/pkg/model/openstackmodel/BUILD.bazel index 8b4f1ee25d..c497eaabd1 100644 --- a/pkg/model/openstackmodel/BUILD.bazel +++ b/pkg/model/openstackmodel/BUILD.bazel @@ -20,6 +20,7 @@ go_library( "//upup/pkg/fi:go_default_library", "//upup/pkg/fi/cloudup/openstack:go_default_library", "//upup/pkg/fi/cloudup/openstacktasks:go_default_library", + "//vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups:go_default_library", "//vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/rules:go_default_library", "//vendor/k8s.io/cloud-provider-openstack/pkg/util/openstack:go_default_library", "//vendor/k8s.io/klog/v2:go_default_library", diff --git a/pkg/model/openstackmodel/firewall.go b/pkg/model/openstackmodel/firewall.go index 1b28987994..be2d453ee5 100644 --- a/pkg/model/openstackmodel/firewall.go +++ b/pkg/model/openstackmodel/firewall.go @@ -17,7 +17,8 @@ limitations under the License. package openstackmodel import ( - "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/rules" + "fmt" + "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/upup/pkg/fi" "k8s.io/kops/upup/pkg/fi/cloudup/openstacktasks" @@ -25,6 +26,9 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/pkg/dns" "k8s.io/kops/pkg/wellknownports" + + sg "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups" + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/rules" ) const ( @@ -38,6 +42,7 @@ const ( type FirewallModelBuilder struct { *OpenstackModelContext Lifecycle *fi.Lifecycle + Rules map[string]*openstacktasks.SecurityGroupRule } var _ fi.ModelBuilder = &FirewallModelBuilder{} @@ -55,7 +60,7 @@ func (b *FirewallModelBuilder) usesOctavia() bool { // Example // Create an Ingress rule on source allowing traffic from dest with the options in the SecurityGroupRule // Create an Egress rule on source allowing traffic to dest with the options in the SecurityGroupRule -func addDirectionalGroupRule(c *fi.ModelBuilderContext, source, dest *openstacktasks.SecurityGroup, sgr *openstacktasks.SecurityGroupRule) { +func (b *FirewallModelBuilder) addDirectionalGroupRule(c *fi.ModelBuilderContext, source, dest *openstacktasks.SecurityGroup, sgr *openstacktasks.SecurityGroupRule) { t := &openstacktasks.SecurityGroupRule{ Direction: sgr.Direction, EtherType: sgr.EtherType, @@ -66,8 +71,12 @@ func addDirectionalGroupRule(c *fi.ModelBuilderContext, source, dest *openstackt RemoteGroup: dest, RemoteIPPrefix: sgr.RemoteIPPrefix, SecGroup: source, + Delete: fi.Bool(false), } - c.AddTask(t) + + klog.V(8).Infof("Adding rule %v", fi.StringValue(t.GetName())) + b.Rules[fi.StringValue(t.GetName())] = t + } // addSSHRules - sets the ssh rules based on the presence of a bastion @@ -100,11 +109,11 @@ func (b *FirewallModelBuilder) addSSHRules(c *fi.ModelBuilderContext, sgMap map[ PortRangeMax: i(22), RemoteIPPrefix: s(sshAccess), } - addDirectionalGroupRule(c, bastionSG, nil, sshRule) + b.addDirectionalGroupRule(c, bastionSG, nil, sshRule) } //Allow ingress ssh from the bastion on the masters and nodes - addDirectionalGroupRule(c, masterSG, bastionSG, sshIngress) - addDirectionalGroupRule(c, nodeSG, bastionSG, sshIngress) + b.addDirectionalGroupRule(c, masterSG, bastionSG, sshIngress) + b.addDirectionalGroupRule(c, nodeSG, bastionSG, sshIngress) } else { for _, sshAccess := range b.Cluster.Spec.SSHAccess { sshRule := &openstacktasks.SecurityGroupRule{ @@ -116,8 +125,8 @@ func (b *FirewallModelBuilder) addSSHRules(c *fi.ModelBuilderContext, sgMap map[ PortRangeMax: i(22), RemoteIPPrefix: s(sshAccess), } - addDirectionalGroupRule(c, masterSG, nil, sshRule) - addDirectionalGroupRule(c, nodeSG, nil, sshRule) + b.addDirectionalGroupRule(c, masterSG, nil, sshRule) + b.addDirectionalGroupRule(c, nodeSG, nil, sshRule) } } return nil @@ -148,8 +157,8 @@ func (b *FirewallModelBuilder) addETCDRules(c *fi.ModelBuilderContext, sgMap map PortRangeMin: i(2380), PortRangeMax: i(2381), } - addDirectionalGroupRule(c, masterSG, masterSG, etcdRule) - addDirectionalGroupRule(c, masterSG, masterSG, etcdPeerRule) + b.addDirectionalGroupRule(c, masterSG, masterSG, etcdRule) + b.addDirectionalGroupRule(c, masterSG, masterSG, etcdPeerRule) for _, portRange := range wellknownports.ETCDPortRanges() { etcdMgmrRule := &openstacktasks.SecurityGroupRule{ @@ -160,7 +169,7 @@ func (b *FirewallModelBuilder) addETCDRules(c *fi.ModelBuilderContext, sgMap map PortRangeMin: i(portRange.Min), PortRangeMax: i(portRange.Max), } - addDirectionalGroupRule(c, masterSG, masterSG, etcdMgmrRule) + b.addDirectionalGroupRule(c, masterSG, masterSG, etcdMgmrRule) } if b.Cluster.Spec.Networking.Calico != nil { @@ -175,7 +184,7 @@ func (b *FirewallModelBuilder) addETCDRules(c *fi.ModelBuilderContext, sgMap map } // Master access from other masters covered above // Allow nodes to reach ETCD endpoints - addDirectionalGroupRule(c, masterSG, nodeSG, etcdCNIRule) + b.addDirectionalGroupRule(c, masterSG, nodeSG, etcdCNIRule) } return nil } @@ -203,7 +212,7 @@ func (b *FirewallModelBuilder) addNodePortRules(c *fi.ModelBuilderContext, sgMap PortRangeMax: i(nodePortRange.Base + nodePortRange.Size - 1), RemoteIPPrefix: s(nodePortAccess), } - addDirectionalGroupRule(c, nodeSG, nil, nodePortRule) + b.addDirectionalGroupRule(c, nodeSG, nil, nodePortRule) } } return nil @@ -229,14 +238,14 @@ func (b *FirewallModelBuilder) addHTTPSRules(c *fi.ModelBuilderContext, sgMap ma } //Allow all local communication for kubernetes.svc and to the api.internal lb/gossip for kubelet's - addDirectionalGroupRule(c, masterSG, nodeSG, httpsIngress) - addDirectionalGroupRule(c, masterSG, masterSG, httpsIngress) + b.addDirectionalGroupRule(c, masterSG, nodeSG, httpsIngress) + b.addDirectionalGroupRule(c, masterSG, masterSG, httpsIngress) if b.UseLoadBalancerForAPI() { if !useVIPACL { //Allow API Access to the lb sg for _, apiAccess := range b.Cluster.Spec.KubernetesAPIAccess { - addDirectionalGroupRule(c, lbSG, nil, &openstacktasks.SecurityGroupRule{ + b.addDirectionalGroupRule(c, lbSG, nil, &openstacktasks.SecurityGroupRule{ Lifecycle: b.Lifecycle, Direction: s(string(rules.DirIngress)), Protocol: s(IPProtocolTCP), @@ -247,12 +256,12 @@ func (b *FirewallModelBuilder) addHTTPSRules(c *fi.ModelBuilderContext, sgMap ma }) } //Allow masters ingress from the sg - addDirectionalGroupRule(c, masterSG, lbSG, httpsIngress) + b.addDirectionalGroupRule(c, masterSG, lbSG, httpsIngress) } //FIXME: Octavia port traffic appears to be denied though its port is in lbSG if b.usesOctavia() { - addDirectionalGroupRule(c, masterSG, nil, &openstacktasks.SecurityGroupRule{ + b.addDirectionalGroupRule(c, masterSG, nil, &openstacktasks.SecurityGroupRule{ Lifecycle: b.Lifecycle, Direction: s(string(rules.DirIngress)), Protocol: s(IPProtocolTCP), @@ -267,7 +276,7 @@ func (b *FirewallModelBuilder) addHTTPSRules(c *fi.ModelBuilderContext, sgMap ma // Allow the masters to receive connections from KubernetesAPIAccess for _, apiAccess := range b.Cluster.Spec.KubernetesAPIAccess { - addDirectionalGroupRule(c, masterSG, nil, &openstacktasks.SecurityGroupRule{ + b.addDirectionalGroupRule(c, masterSG, nil, &openstacktasks.SecurityGroupRule{ Lifecycle: b.Lifecycle, Direction: s(string(rules.DirIngress)), Protocol: s(IPProtocolTCP), @@ -302,8 +311,8 @@ func (b *FirewallModelBuilder) addKubeletRules(c *fi.ModelBuilderContext, sgMap // allow node-node, node-master and master-master and master-node for _, sgName := range []*openstacktasks.SecurityGroup{masterSG, nodeSG} { - addDirectionalGroupRule(c, masterSG, sgName, kubeletRule) - addDirectionalGroupRule(c, nodeSG, sgName, kubeletRule) + b.addDirectionalGroupRule(c, masterSG, sgName, kubeletRule) + b.addDirectionalGroupRule(c, nodeSG, sgName, kubeletRule) } return nil } @@ -323,8 +332,8 @@ func (b *FirewallModelBuilder) addNodeExporterRules(c *fi.ModelBuilderContext, s PortRangeMax: i(9100), } // allow 9100 port from nodeSG - addDirectionalGroupRule(c, masterSG, nodeSG, nodeExporterIngress) - addDirectionalGroupRule(c, nodeSG, nodeSG, nodeExporterIngress) + b.addDirectionalGroupRule(c, masterSG, nodeSG, nodeExporterIngress) + b.addDirectionalGroupRule(c, nodeSG, nodeSG, nodeExporterIngress) return nil } @@ -344,9 +353,9 @@ func (b *FirewallModelBuilder) addDNSRules(c *fi.ModelBuilderContext, sgMap map[ PortRangeMin: i(53), PortRangeMax: i(53), } - addDirectionalGroupRule(c, masterSG, nodeSG, dnsRule) - addDirectionalGroupRule(c, nodeSG, masterSG, dnsRule) - addDirectionalGroupRule(c, masterSG, masterSG, dnsRule) + b.addDirectionalGroupRule(c, masterSG, nodeSG, dnsRule) + b.addDirectionalGroupRule(c, nodeSG, masterSG, dnsRule) + b.addDirectionalGroupRule(c, masterSG, masterSG, dnsRule) } return nil } @@ -414,10 +423,10 @@ func (b *FirewallModelBuilder) addCNIRules(c *fi.ModelBuilderContext, sgMap map[ PortRangeMin: i(udpPort), PortRangeMax: i(udpPort), } - addDirectionalGroupRule(c, masterSG, masterSG, udpRule) - addDirectionalGroupRule(c, nodeSG, masterSG, udpRule) - addDirectionalGroupRule(c, masterSG, nodeSG, udpRule) - addDirectionalGroupRule(c, nodeSG, nodeSG, udpRule) + b.addDirectionalGroupRule(c, masterSG, masterSG, udpRule) + b.addDirectionalGroupRule(c, nodeSG, masterSG, udpRule) + b.addDirectionalGroupRule(c, masterSG, nodeSG, udpRule) + b.addDirectionalGroupRule(c, nodeSG, nodeSG, udpRule) } for _, tcpPort := range tcpPorts { tcpRule := &openstacktasks.SecurityGroupRule{ @@ -428,10 +437,10 @@ func (b *FirewallModelBuilder) addCNIRules(c *fi.ModelBuilderContext, sgMap map[ PortRangeMin: i(tcpPort), PortRangeMax: i(tcpPort), } - addDirectionalGroupRule(c, masterSG, masterSG, tcpRule) - addDirectionalGroupRule(c, nodeSG, masterSG, tcpRule) - addDirectionalGroupRule(c, masterSG, nodeSG, tcpRule) - addDirectionalGroupRule(c, nodeSG, nodeSG, tcpRule) + b.addDirectionalGroupRule(c, masterSG, masterSG, tcpRule) + b.addDirectionalGroupRule(c, nodeSG, masterSG, tcpRule) + b.addDirectionalGroupRule(c, masterSG, nodeSG, tcpRule) + b.addDirectionalGroupRule(c, nodeSG, nodeSG, tcpRule) } for _, protocol := range protocols { @@ -441,8 +450,8 @@ func (b *FirewallModelBuilder) addCNIRules(c *fi.ModelBuilderContext, sgMap map[ Protocol: s(protocol), EtherType: s(string(rules.EtherType4)), } - addDirectionalGroupRule(c, masterSG, nil, protocolRule) - addDirectionalGroupRule(c, nodeSG, nil, protocolRule) + b.addDirectionalGroupRule(c, masterSG, nil, protocolRule) + b.addDirectionalGroupRule(c, nodeSG, nil, protocolRule) } return nil @@ -465,15 +474,90 @@ func (b *FirewallModelBuilder) addProtokubeRules(c *fi.ModelBuilderContext, sgMa PortRangeMin: i(portRange.Min), PortRangeMax: i(portRange.Max), } - addDirectionalGroupRule(c, masterSG, nodeSG, protokubeRule) - addDirectionalGroupRule(c, nodeSG, masterSG, protokubeRule) - addDirectionalGroupRule(c, masterSG, masterSG, protokubeRule) - addDirectionalGroupRule(c, nodeSG, nodeSG, protokubeRule) + b.addDirectionalGroupRule(c, masterSG, nodeSG, protokubeRule) + b.addDirectionalGroupRule(c, nodeSG, masterSG, protokubeRule) + b.addDirectionalGroupRule(c, masterSG, masterSG, protokubeRule) + b.addDirectionalGroupRule(c, nodeSG, nodeSG, protokubeRule) } } return nil } +func (b *FirewallModelBuilder) getExistingRules(sgMap map[string]*openstacktasks.SecurityGroup) error { + osCloud, err := b.createCloud() + if err != nil { + return err + } + sgIdMap := make(map[string]*openstacktasks.SecurityGroup) + for sgName, sgt := range sgMap { + + sgs, err := osCloud.ListSecurityGroups(sg.ListOpts{ + Name: sgName, + }) + if err != nil { + return err + } + if len(sgs) != 1 { + continue + } + sg := sgs[0] + sgt.Name = fi.String(sg.Name) + sgIdMap[sg.ID] = sgt + } + + for sgid := range sgIdMap { + + sgRules, err := osCloud.ListSecurityGroupRules(rules.ListOpts{ + SecGroupID: sgid, + }) + if err != nil { + return err + } + for _, rule := range sgRules { + + t := &openstacktasks.SecurityGroupRule{ + ID: fi.String(rule.ID), + Direction: fi.String(rule.Direction), + EtherType: fi.String(rule.EtherType), + PortRangeMax: fi.Int(rule.PortRangeMax), + PortRangeMin: fi.Int(rule.PortRangeMin), + Protocol: fi.String(rule.Protocol), + RemoteIPPrefix: fi.String(rule.RemoteIPPrefix), + RemoteGroup: sgIdMap[rule.RemoteGroupID], + Lifecycle: b.Lifecycle, + SecGroup: sgIdMap[rule.SecGroupID], + Delete: fi.Bool(true), + } + klog.V(8).Infof("Adding existing rule %v", t) + b.Rules[fi.StringValue(t.GetName())] = t + } + } + return nil + +} + +func (b *FirewallModelBuilder) addDefaultEgress(c *fi.ModelBuilderContext, sgMap map[string]*openstacktasks.SecurityGroup) { + for _, sg := range sgMap { + t := &openstacktasks.SecurityGroupRule{ + Lifecycle: b.Lifecycle, + Direction: s(string(rules.DirEgress)), + EtherType: s(string(rules.EtherType4)), + PortRangeMin: i(0), + PortRangeMax: i(0), + } + b.addDirectionalGroupRule(c, sg, nil, t) + + t = &openstacktasks.SecurityGroupRule{ + Lifecycle: b.Lifecycle, + Direction: s(string(rules.DirEgress)), + EtherType: s(string(rules.EtherType6)), + PortRangeMin: i(0), + PortRangeMax: i(0), + } + b.addDirectionalGroupRule(c, sg, nil, t) + } +} + // Build - schedule security groups and security group rule tasks for Openstack func (b *FirewallModelBuilder) Build(c *fi.ModelBuilderContext) error { roles := []kops.InstanceGroupRole{kops.InstanceGroupRoleMaster, kops.InstanceGroupRoleNode} @@ -517,6 +601,16 @@ func (b *FirewallModelBuilder) Build(c *fi.ModelBuilderContext) error { sgMap[groupName] = sg } + b.Rules = make(map[string]*openstacktasks.SecurityGroupRule) + + err := b.getExistingRules(sgMap) + // if this fails, it just means that existing rules won't be cleaned up + if err != nil { + klog.Warningf("Failed to list existing security groups: %v", err) + } + + b.addDefaultEgress(c, sgMap) + //Add API Server Rules b.addHTTPSRules(c, sgMap, useVIPACL) @@ -535,9 +629,13 @@ func (b *FirewallModelBuilder) Build(c *fi.ModelBuilderContext) error { //ETCD Leader Election b.addETCDRules(c, sgMap) // Add NodePort Rules: - err := b.addNodePortRules(c, sgMap) + err = b.addNodePortRules(c, sgMap) if err != nil { - return err + return fmt.Errorf("failed to add node port rules: %v", err) + } + + for _, r := range b.Rules { + c.AddTask(r) } return nil diff --git a/upup/pkg/fi/cloudup/openstacktasks/securitygrouprule.go b/upup/pkg/fi/cloudup/openstacktasks/securitygrouprule.go index d75346e7d9..81da3cedbb 100644 --- a/upup/pkg/fi/cloudup/openstacktasks/securitygrouprule.go +++ b/upup/pkg/fi/cloudup/openstacktasks/securitygrouprule.go @@ -38,6 +38,7 @@ func IntValue(v *int) int { type SecurityGroupRule struct { ID *string + Name *string Direction *string EtherType *string SecGroup *SecurityGroup @@ -47,6 +48,7 @@ type SecurityGroupRule struct { RemoteIPPrefix *string RemoteGroup *SecurityGroup Lifecycle *fi.Lifecycle + Delete *bool } // GetDependencies returns the dependencies of the Instance task @@ -104,12 +106,12 @@ func (r *SecurityGroupRule) Find(context *fi.Context) (*SecurityGroupRule, error PortRangeMin: Int(rule.PortRangeMin), Protocol: fi.String(rule.Protocol), RemoteIPPrefix: fi.String(rule.RemoteIPPrefix), - RemoteGroup: &SecurityGroup{ - ID: fi.String(rule.RemoteGroupID), - }, - SecGroup: &SecurityGroup{ID: fi.String(rule.SecGroupID)}, - Lifecycle: r.Lifecycle, + RemoteGroup: r.RemoteGroup, + SecGroup: r.SecGroup, + Lifecycle: r.Lifecycle, + Delete: fi.Bool(false), } + r.ID = actual.ID return actual, nil } @@ -118,7 +120,7 @@ func (r *SecurityGroupRule) Run(context *fi.Context) error { return fi.DefaultDeltaRunMethod(r, context) } -func (_ *SecurityGroupRule) CheckChanges(a, e, changes *SecurityGroupRule) error { +func (*SecurityGroupRule) CheckChanges(a, e, changes *SecurityGroupRule) error { if a == nil { if e.Direction == nil { return fi.RequiredField("Direction") @@ -146,7 +148,7 @@ func (_ *SecurityGroupRule) CheckChanges(a, e, changes *SecurityGroupRule) error return nil } -func (_ *SecurityGroupRule) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, changes *SecurityGroupRule) error { +func (*SecurityGroupRule) RenderOpenstack(t *openstack.OpenstackAPITarget, a, e, changes *SecurityGroupRule) error { if a == nil { klog.V(2).Infof("Creating SecurityGroupRule") @@ -198,5 +200,39 @@ func (o *SecurityGroupRule) GetName() *string { // String is the stringer function for the task, producing readable output using fi.TaskAsString func (o *SecurityGroupRule) String() string { - return fi.TaskAsString(o) + + var dst string + if o.RemoteGroup != nil { + dst = fi.StringValue(o.RemoteGroup.Name) + } else if o.RemoteIPPrefix != nil && fi.StringValue(o.RemoteIPPrefix) != "" { + dst = fi.StringValue(o.RemoteIPPrefix) + } else { + dst = "ANY" + } + var proto string + if o.Protocol == nil || fi.StringValue(o.Protocol) == "" { + proto = "AllProtos" + } else { + proto = fi.StringValue(o.Protocol) + } + + return fmt.Sprintf("%v-%v-%v-from-%v-to-%v-%v-%v", fi.StringValue(o.EtherType), fi.StringValue(o.Direction), + proto, fi.StringValue(o.SecGroup.Name), dst, fi.IntValue(o.PortRangeMin), fi.IntValue(o.PortRangeMax)) +} + +func (o *SecurityGroupRule) FindDeletions(c *fi.Context) ([]fi.Deletion, error) { + if !fi.BoolValue(o.Delete) { + return nil, nil + } + cloud := c.Cloud.(openstack.OpenstackCloud) + rule, err := sgr.Get(cloud.NetworkingClient(), fi.StringValue(o.ID)).Extract() + if err != nil { + return nil, err + } + return []fi.Deletion{ + &deleteSecurityGroupRule{ + rule: *rule, + securityGroup: o.SecGroup, + }, + }, nil }