Delete SG rules that kops don't explicitly add to managed SGs

This commit is contained in:
Ole Markus With 2020-08-26 10:33:58 +02:00
parent 6cc7153bbe
commit 14a6f92f53
3 changed files with 185 additions and 50 deletions

View File

@ -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",

View File

@ -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

View File

@ -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)},
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
}