diff --git a/README.md b/README.md index c3f9ba93..353d6029 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,14 @@ On VPC tiers the controller also calls `updateNetworkACLItem`, to take over Netw earlier releases created (see [Per-Service Network ACL rules](#per-service-network-acl-rules)). Without it the controller creates its own rules and leaves the old ones for you to delete. +From CloudStack 4.23 on, the controller also calls `listVPCs` on VPC tiers, to find out whether the +VPC has the Firewall service (see [VPC Networks](#vpc-networks)). If the account may not call it, +the controller manages Network ACL rules only, as earlier releases did, and logs a warning once per +VPC. As of CloudStack 4.23, the role that CloudStack's Kubernetes service creates for its accounts +in projects (`Project Kubernetes Service Role`) includes neither `listVPCs` nor +`listNetworkACLLists`, which the controller needs to manage Network ACL rules. Add them to that +role, before the rule that denies everything else. + To create the secret, use the following command: ```bash kubectl -n kube-system create secret generic cloudstack-secret --from-file=cloud-config @@ -320,18 +328,18 @@ annotation by hand — doing so can leak a public IP or release one that you all ### Restricting Source Traffic Traffic is filtered at two independent layers, which are configured separately. The second layer is -either a firewall rule or a Network ACL rule, depending on what the network offers: +a firewall rule, a Network ACL rule, or both, depending on the network: | Layer | Configured by | Default | | --- | --- | --- | | CloudStack load balancer rule | `service.beta.kubernetes.io/cloudstack-load-balancer-source-cidrs` annotation | `0.0.0.0/0` | -| Firewall rule — isolated networks, and VPC networks that offer the Firewall service | `spec.loadBalancerSourceRanges` | `0.0.0.0/0` | -| Network ACL rule — VPC networks without the Firewall service | Not configurable | `0.0.0.0/0` | +| Firewall rule — isolated networks, and VPC tiers where the tier or its VPC offers the Firewall service | `spec.loadBalancerSourceRanges` | `0.0.0.0/0` | +| Network ACL rule — VPC tiers whose own offering does not include the Firewall service | Not configurable | `0.0.0.0/0` | -Traffic has to be allowed by both layers, so where firewall rules are used, either setting alone is +Traffic has to be allowed by every layer, so where firewall rules are used, either setting alone is enough to block unwanted sources. Setting both keeps the two rules consistent in CloudStack. -Where Network ACL rules are used, `spec.loadBalancerSourceRanges` has no effect and the annotation -is the only way to restrict sources. +Where only Network ACL rules are used, `spec.loadBalancerSourceRanges` has no effect and the +annotation is the only way to restrict sources. The two layers turn traffic away differently. The firewall discards the packets on the virtual router, so a blocked client simply times out. The load balancer rule lets the connection be @@ -405,10 +413,13 @@ controller creates **Network ACL** rules instead of firewall rules for the manag associates public IPs with the VPC rather than with the network. CloudStack 4.23 adds support for firewall rules on public IPs in VPC networks. This requires a -network offering that includes the Firewall service and is not enabled by default. The controller -chooses the mechanism based on the services the network offers, so on such networks it manages -firewall rules instead of Network ACL rules, and `spec.loadBalancerSourceRanges` is applied to -them. +VPC offering that includes the Firewall service and is not enabled by default. A tier lists the +service only if its own network offering includes it, and the VPC's virtual router drops traffic to +a public IP with a load balancer rule unless a firewall rule allows it. On a tier the controller +therefore reads the VPC with `listVPCs`, and if the VPC has the Firewall service, it manages a +firewall rule on the public IP in addition to the tier's Network ACL rule, with +`spec.loadBalancerSourceRanges` applied to the firewall rule. A network whose own offering includes +the Firewall service gets firewall rules instead of Network ACL rules. When the controller manages Network ACL rules: * Each Service gets its own ACL rule for each port, even when another Service on the tier uses the @@ -561,8 +572,52 @@ The controller keeps the Service's load balancer rule until its ACL rule is dele Kubernetes only retries the cleanup while a load balancer rule is left. Fix the cause shown in the log, for example the account's permissions, and the cleanup finishes on its own. +### A deleted Service stays `Terminating`, log shows `Error deleting firewall rule, keeping load balancer rule` + +On a tier of a VPC with the Firewall service the Service also has a firewall rule, and the +controller keeps the load balancer rule until that is deleted too, for the same reason. The same +applies when the log shows `Error reading the VPC`: without the VPC the controller cannot tell +whether there is a firewall rule. Fix the cause shown in the log and the cleanup finishes on its +own. + +### A port in a VPC with the Firewall service stays closed, log shows `Cannot read VPC` + +The controller could not read the VPC to find out whether it has the Firewall service, so it added +the Network ACL rule but no firewall rule, and the VPC's virtual router drops the traffic. Allow the +controller's account to call `listVPCs` (see [Kubernetes](#kubernetes)). The warning is logged once +per VPC until the controller restarts. The controller adds the firewall rule the next time it syncs +the Service, which happens when the Service changes or the controller restarts. + ## Upgrading +### Firewall rules on tiers of a VPC with the Firewall service + +This affects CloudStack 4.23 and later, on VPC tiers whose VPC offering includes the Firewall +service but whose own offering does not. Earlier releases added only a Network ACL rule there, so +the VPC's firewall kept the Service's ports closed. A tier whose own offering includes the Firewall +service still gets a firewall rule only, which is now also deleted with the Service. Other networks +behave as before. + +Before upgrading: + +* As on isolated networks, the controller treats the firewall rules on a Service's IP for its ports + and protocols as the Service's: it replaces a rule with another CIDR list and deletes the rules + with the Service. If you added such rules by hand, for example to work around the missing rule, + and they allow fewer sources than `0.0.0.0/0`, put those CIDRs into the Service's + `spec.loadBalancerSourceRanges`. Otherwise the first sync replaces them with a rule for + `0.0.0.0/0`. +* Allow the controller's account to call `listVPCs`; see [Kubernetes](#kubernetes). Without it the + controller keeps managing only the ACL rule. +* If the cloud config sets `version`, keep it at the CloudStack version in use. The controller reads + the VPC only when that version is 4.23 or later. + +After upgrading, the first sync of each Service adds a firewall rule on its public IP for each port, +allowing the CIDRs in `spec.loadBalancerSourceRanges`, or `0.0.0.0/0` if it is empty. + +An earlier release only deletes the ACL rule. After a downgrade, the firewall rule of a Service or +port that is removed later stays on its IP if the IP is kept, for example the cluster's API IP used +as `loadBalancerIP`. Delete such rules by hand. + ### Per-Service Network ACL rules This affects VPC tiers whose ACL rules the controller manages, that is tiers with a custom ACL list. diff --git a/cloudstack.go b/cloudstack.go index c0dfe338..71b3700a 100644 --- a/cloudstack.go +++ b/cloudstack.go @@ -27,6 +27,7 @@ import ( "io" "os" "strings" + "sync" "time" "github.com/apache/cloudstack-go/v2/cloudstack" @@ -69,6 +70,10 @@ type CSCloud struct { region string version semver.Version clientBuilder cloudprovider.ControllerClientBuilder + + // vpcReadRefused holds the VPCs the account may not read, so that is logged once per VPC. + // Its zero value is ready to use, also in a CSCloud built without newCSCloud. + vpcReadRefused sync.Map } func init() { diff --git a/cloudstack_loadbalancer.go b/cloudstack_loadbalancer.go index f94bc01a..0d1ff08c 100644 --- a/cloudstack_loadbalancer.go +++ b/cloudstack_loadbalancer.go @@ -30,6 +30,7 @@ import ( "sort" "strconv" "strings" + "sync" "unicode" "github.com/apache/cloudstack-go/v2/cloudstack" @@ -77,6 +78,10 @@ const ( // the rule and creating it again. var cidrListUpdateVersion = semver.Version{Major: 4, Minor: 22, Patch: 0} +// vpcFirewallVersion is the first CloudStack release in which a VPC offering can include the +// Firewall service. Below it no VPC has a firewall, so the VPC is not read. +var vpcFirewallVersion = semver.Version{Major: 4, Minor: 23, Patch: 0} + type loadBalancer struct { *cloudstack.CloudStackClient @@ -90,6 +95,9 @@ type loadBalancer struct { rules map[string]*cloudstack.LoadBalancerRule duplicateRules []*cloudstack.LoadBalancerRule networks map[string]*cloudstack.Network + vpcFirewalls map[string]bool + vpcReadRefused *sync.Map + version semver.Version ipAssociatedByController bool } @@ -190,6 +198,13 @@ func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, s if err := stickiness.normalise(supportedStickinessMethods(network)); err != nil { return nil, err } + // Likewise read the VPC of a tier now, so a failed lookup fails before an IP is associated. + // The answer is remembered for applyLoadBalancerRules. + if usesNetworkACL(network) { + if _, err := lb.vpcHasFirewall(network); err != nil { + return nil, err + } + } rulesApplied := false if !lb.hasLoadBalancerIP() { @@ -333,6 +348,64 @@ func isNetworkACLSupported(services []cloudstack.NetworkServiceInternal) bool { return false } +// vpcHasFirewall reports whether the VPC a network belongs to has the Firewall service. Since +// CloudStack 4.23 a VPC offering can include it. A tier lists the service only if its own network +// offering includes it, and the VPC virtual router drops traffic to a public IP with a load +// balancer rule unless a firewall rule admits it, on top of the tier's Network ACL. A network +// outside a VPC, or on an earlier release, has no VPC firewall. +// +// The answer is remembered per VPC. An account that may not call listVPCs is treated as if the +// VPC had no Firewall service, as earlier releases did. The account CloudStack's Kubernetes +// service creates for clusters in projects is one such account, and most VPCs have no Firewall +// service, so the warning that names the missing API is logged once per VPC. +func (lb *loadBalancer) vpcHasFirewall(network *cloudstack.Network) (bool, error) { + if network.Vpcid == "" || lb.version.LT(vpcFirewallVersion) { + return false, nil + } + if hasFirewall, ok := lb.vpcFirewalls[network.Vpcid]; ok { + return hasFirewall, nil + } + + hasFirewall := false + vpc, _, err := lb.VPC.GetVPCByID(network.Vpcid, cloudstack.WithProject(lb.projectID)) + switch { + case err != nil && isNotAllowed(err): + if lb.firstVPCReadRefusal(network.Vpcid) { + klog.Warningf("Cannot read VPC %v to check for the Firewall service, so no firewall rules are managed there; allow the account to call listVPCs: %v", network.Vpcid, err) + } else { + klog.V(4).Infof("Cannot read VPC %v to check for the Firewall service for load balancer %v: %v", network.Vpcid, lb.name, err) + } + case err != nil: + return false, fmt.Errorf("error fetching VPC %v of network %v: %v", network.Vpcid, network.Id, err) + default: + hasFirewall = slices.ContainsFunc(vpc.Service, func(svc cloudstack.VPCServiceInternal) bool { + return svc.Name == "Firewall" + }) + } + + if lb.vpcFirewalls == nil { + lb.vpcFirewalls = make(map[string]bool) + } + lb.vpcFirewalls[network.Vpcid] = hasFirewall + + return hasFirewall, nil +} + +// firstVPCReadRefusal reports whether the account was refused reading the VPC for the first time. +func (lb *loadBalancer) firstVPCReadRefusal(vpcID string) bool { + if lb.vpcReadRefused == nil { + return true + } + _, seen := lb.vpcReadRefused.LoadOrStore(vpcID, struct{}{}) + return !seen +} + +// usesNetworkACL reports whether the controller opens ports on the network with Network ACL rules +// rather than firewall rules. +func usesNetworkACL(network *cloudstack.Network) bool { + return !isFirewallSupported(network.Service) && isNetworkACLSupported(network.Service) +} + // EnsureLoadBalancerDeleted deletes the specified load balancer if it exists, returning // nil if the load balancer specified either didn't exist or was successfully deleted. func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName string, service *corev1.Service) error { @@ -351,7 +424,7 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st klog.Errorf("Error removing duplicate load balancer rules for %v/%v: %v", service.Namespace, service.Name, sweepErr) } - var aclErr error + var openingErr error for _, lbRule := range lb.rules { klog.V(4).Infof("Deleting firewall rules / Network ACLs for load balancer: %v", lbRule.Name) protocol := ProtocolFromLoadBalancer(lbRule.Protocol) @@ -383,14 +456,36 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st klog.Errorf("Error deleting firewall rule: %v", err) } } else { + // In a VPC with the Firewall service the port also has a firewall rule on the + // IP. Like the ACL rule below, it is reached only through this rule, which is + // kept for the retry if the VPC cannot be read or the rule cannot be deleted. + vpcFirewall, err := lb.vpcHasFirewall(network) + if err != nil { + klog.Errorf("Error reading the VPC of network %v, keeping load balancer rule %v for the retry: %v", networkId, lbRule.Name, err) + if openingErr == nil { + openingErr = err + } + continue + } + if vpcFirewall { + klog.V(4).Infof("Deleting firewall rules for %v - %v", int(port), protocol) + if _, err := lb.deleteFirewallRule(lbRule.Publicipid, int(port), protocol); err != nil { + klog.Errorf("Error deleting firewall rule, keeping load balancer rule %v for the retry: %v", lbRule.Name, err) + if openingErr == nil { + openingErr = err + } + continue + } + } + klog.V(4).Infof("Deleting network ACLs for %v - %v", int(port), protocol) _, err = lb.deleteNetworkACLRule(int(port), protocol, networkId) if err != nil { // Kubernetes stops retrying once no load balancer rule is left, and only // this rule leads back to the tier and port of its ACL rule, so it is kept. klog.Errorf("Error deleting Network ACL rule, keeping load balancer rule %v for the retry: %v", lbRule.Name, err) - if aclErr == nil { - aclErr = err + if openingErr == nil { + openingErr = err } continue } @@ -405,8 +500,8 @@ func (cs *CSCloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName st } // Releasing the IP would make CloudStack revoke the rules kept for the retry. - if aclErr != nil { - return errors.Join(aclErr, sweepErr) + if openingErr != nil { + return errors.Join(openingErr, sweepErr) } if lb.ipAddr != "" { @@ -473,6 +568,8 @@ func (cs *CSCloud) getLoadBalancer(service *corev1.Service) (*loadBalancer, erro name: cs.GetLoadBalancerName(context.TODO(), "", service), projectID: cs.projectID, rules: make(map[string]*cloudstack.LoadBalancerRule), + vpcReadRefused: &cs.vpcReadRefused, + version: cs.version, } p := cs.client.LoadBalancer.NewListLoadBalancerRulesParams() @@ -986,7 +1083,8 @@ func (lb *loadBalancer) ruleNetwork(lbRule *cloudstack.LoadBalancerRule) (*cloud // pruneRuleOpening deletes the firewall or network ACL rule admitting traffic to an obsolete // load balancer rule, unless a desired service port still claims that same opening. Which of // the two a rule has follows the network that rule belongs to, not the one being reconciled -// towards, so a rule left behind in a network of the other kind does not keep its opening. +// towards, so a rule left behind in a network of the other kind does not keep its opening. A +// tier of a VPC with the Firewall service has both. // // A rule with an uninterpretable protocol keeps its opening, which cannot be identified without // one. A rule whose network cannot be established still has its firewall rule deleted, that @@ -1009,6 +1107,15 @@ func (lb *loadBalancer) pruneRuleOpening(o obsoleteRule, claimed map[portProtoco case isFirewallSupported(network.Service): return lb.pruneFirewallRule(o, claimed) case isNetworkACLSupported(network.Service): + vpcFirewall, err := lb.vpcHasFirewall(network) + if err != nil { + return err + } + if vpcFirewall { + if err := lb.pruneFirewallRule(o, claimed); err != nil { + return err + } + } return lb.pruneNetworkACLRule(o, claimed, network.Id) } @@ -1109,6 +1216,16 @@ func (lb *loadBalancer) repairEmptyRule(lbRule *cloudstack.LoadBalancerRule) err // port and reconciles the firewall or network ACL rules it needs. It returns the rule of each // desired port, in the same order. func (lb *loadBalancer) applyLoadBalancerRules(desired []desiredLBRule, service *corev1.Service, network *cloudstack.Network, version semver.Version) ([]*cloudstack.LoadBalancerRule, error) { + // A tier of a VPC with the Firewall service needs a firewall rule as well as its ACL rule. + // This is checked before any rule is created. + vpcFirewall := false + if usesNetworkACL(network) { + var err error + if vpcFirewall, err = lb.vpcHasFirewall(network); err != nil { + return nil, err + } + } + rules := make([]*cloudstack.LoadBalancerRule, 0, len(desired)) for _, d := range desired { lbRule, err := lb.ensureLoadBalancerRule(d, service, version) @@ -1127,6 +1244,12 @@ func (lb *loadBalancer) applyLoadBalancerRules(desired []desiredLBRule, service if _, err := lb.updateNetworkACL(int(d.port.Port), d.protocol, network.Id); err != nil { return nil, err } + if vpcFirewall { + klog.V(4).Infof("Creating firewall rules for load balancer rule: %v (%v:%v:%v)", d.name, d.protocol, lbRule.Publicip, d.port.Port) + if _, err := lb.updateFirewallRule(lbRule.Publicipid, int(d.port.Port), d.protocol, service.Spec.LoadBalancerSourceRanges); err != nil { + return nil, err + } + } } } @@ -2183,19 +2306,22 @@ func (lb *loadBalancer) deleteFirewallRule(publicIpId string, publicPort int, pr } } - // delete all rules + // delete all rules; the first failure is returned, so a later success does not hide it deleted := false + var firstErr error for _, rule := range filtered { p := lb.Firewall.NewDeleteFirewallRuleParams(rule.Id) - _, err = lb.Firewall.DeleteFirewallRule(p) - if err != nil { + if _, err := lb.Firewall.DeleteFirewallRule(p); err != nil { klog.Errorf("Error deleting old firewall rule %v: %v", rule.Id, err) + if firstErr == nil { + firstErr = err + } } else { deleted = true } } - return deleted, err + return deleted, firstErr } // deleteNetworkACLRule deletes this load balancer's Network ACL rules for a port on a tier and diff --git a/cloudstack_loadbalancer_test.go b/cloudstack_loadbalancer_test.go index 91ac15c4..6a6f9361 100644 --- a/cloudstack_loadbalancer_test.go +++ b/cloudstack_loadbalancer_test.go @@ -27,6 +27,7 @@ import ( "slices" "sort" "strings" + "sync" "testing" "github.com/apache/cloudstack-go/v2/cloudstack" @@ -2942,6 +2943,39 @@ func TestListFirewallRulesDeduplicates(t *testing.T) { } func TestDeleteFirewallRule(t *testing.T) { + t.Run("a failed delete is reported even when a later one succeeds", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockFirewall := cloudstack.NewMockFirewallServiceIface(ctrl) + listResp := &cloudstack.ListFirewallRulesResponse{ + Count: 2, + FirewallRules: []*cloudstack.FirewallRule{ + {Id: "fw-1", Protocol: "tcp", Startport: 80, Endport: 80, Cidrlist: defaultAllowedCIDR}, + {Id: "fw-2", Protocol: "tcp", Startport: 80, Endport: 80, Cidrlist: "10.0.0.0/8"}, + }, + } + deleteFirst, deleteSecond := &cloudstack.DeleteFirewallRuleParams{}, &cloudstack.DeleteFirewallRuleParams{} + gomock.InOrder( + mockFirewall.EXPECT().NewListFirewallRulesParams().Return(&cloudstack.ListFirewallRulesParams{}), + mockFirewall.EXPECT().ListFirewallRules(gomock.Any()).Return(listResp, nil), + mockFirewall.EXPECT().NewDeleteFirewallRuleParams("fw-1").Return(deleteFirst), + mockFirewall.EXPECT().DeleteFirewallRule(deleteFirst).Return(nil, fmt.Errorf("router unreachable")), + mockFirewall.EXPECT().NewDeleteFirewallRuleParams("fw-2").Return(deleteSecond), + mockFirewall.EXPECT().DeleteFirewallRule(deleteSecond).Return(&cloudstack.DeleteFirewallRuleResponse{}, nil), + ) + + lb := &loadBalancer{CloudStackClient: &cloudstack.CloudStackClient{Firewall: mockFirewall}} + + deleted, err := lb.deleteFirewallRule("ip-123", 80, LoadBalancerProtocolTCP) + if err == nil || !strings.Contains(err.Error(), "router unreachable") { + t.Fatalf("error = %v, want the failed delete", err) + } + if !deleted { + t.Errorf("deleted = false, want true for the rule that was deleted") + } + }) + t.Run("delete matching rule", func(t *testing.T) { ctrl := gomock.NewController(t) t.Cleanup(ctrl.Finish) @@ -5063,6 +5097,7 @@ type ensureLBTestEnv struct { firewall *cloudstack.MockFirewallServiceIface address *cloudstack.MockAddressServiceIface acl *cloudstack.MockNetworkACLServiceIface + vpc *cloudstack.MockVPCServiceIface service *corev1.Service nodes []*corev1.Node @@ -5080,6 +5115,7 @@ func newEnsureLBTestEnv(ctrl *gomock.Controller, annotations map[string]string, firewall: cloudstack.NewMockFirewallServiceIface(ctrl), address: cloudstack.NewMockAddressServiceIface(ctrl), acl: cloudstack.NewMockNetworkACLServiceIface(ctrl), + vpc: cloudstack.NewMockVPCServiceIface(ctrl), } e.cs = &CSCloud{ @@ -5090,6 +5126,7 @@ func newEnsureLBTestEnv(ctrl *gomock.Controller, annotations map[string]string, Firewall: e.firewall, Address: e.address, NetworkACL: e.acl, + VPC: e.vpc, }, version: semver.Version{Major: 4, Minor: 22, Patch: 0}, } @@ -6295,6 +6332,476 @@ func TestEnsureLoadBalancerDeletedFirewallUsesRuleNetwork(t *testing.T) { } } +func TestVPCHasFirewall(t *testing.T) { + tier := &cloudstack.Network{Id: "net-1", Vpcid: "vpc-1", Service: []cloudstack.NetworkServiceInternal{{Name: "NetworkACL"}}} + vpcWith := func(services ...string) *cloudstack.VPC { + vpc := &cloudstack.VPC{Id: "vpc-1"} + for _, name := range services { + vpc.Service = append(vpc.Service, cloudstack.VPCServiceInternal{Name: name}) + } + return vpc + } + notAllowed := fmt.Errorf("CloudStack API error 432 (CSExceptionErrorCode: 9999): The API [listVPCs] does not exist or is not available for the account") + + tests := []struct { + name string + vpc *cloudstack.VPC + err error + want bool + wantErr bool + }{ + {name: "a VPC with the Firewall service", vpc: vpcWith("Dhcp", "Firewall", "NetworkACL"), want: true}, + {name: "a VPC without the Firewall service", vpc: vpcWith("Dhcp", "NetworkACL"), want: false}, + {name: "an account that may not list VPCs is treated as having no VPC firewall", err: notAllowed, want: false}, + {name: "a failed lookup is reported", err: fmt.Errorf("API unavailable"), wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + count := 1 + if tt.err != nil { + count = -1 + } + mockVPC := cloudstack.NewMockVPCServiceIface(ctrl) + // Once only: an answer is remembered for the VPC, a failure is not. + times := 1 + if tt.wantErr { + times = 2 + } + mockVPC.EXPECT().GetVPCByID("vpc-1", gomock.Any()).Return(tt.vpc, count, tt.err).Times(times) + lb := &loadBalancer{CloudStackClient: &cloudstack.CloudStackClient{VPC: mockVPC}, name: "atestuid", version: vpcFirewallVersion} + + for i := 0; i < 2; i++ { + got, err := lb.vpcHasFirewall(tier) + if tt.wantErr { + if err == nil || !strings.Contains(err.Error(), "API unavailable") { + t.Fatalf("error = %v, want the lookup failure", err) + } + continue + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Errorf("vpcHasFirewall() = %v, want %v", got, tt.want) + } + } + }) + } + + t.Run("a network outside a VPC is not looked up", func(t *testing.T) { + lb := &loadBalancer{CloudStackClient: &cloudstack.CloudStackClient{}, version: vpcFirewallVersion} + got, err := lb.vpcHasFirewall(&cloudstack.Network{Id: "net-iso", Service: []cloudstack.NetworkServiceInternal{{Name: "Firewall"}}}) + if err != nil || got { + t.Errorf("vpcHasFirewall() = %v, %v, want false, nil", got, err) + } + }) + + t.Run("a refused lookup is reported once per VPC", func(t *testing.T) { + refused := &sync.Map{} + lb := &loadBalancer{vpcReadRefused: refused} + if !lb.firstVPCReadRefusal("vpc-1") || lb.firstVPCReadRefusal("vpc-1") { + t.Errorf("vpc-1 should be reported the first time only") + } + if !(&loadBalancer{vpcReadRefused: refused}).firstVPCReadRefusal("vpc-2") { + t.Errorf("vpc-2 should be reported, also by another sync") + } + unshared := &loadBalancer{} + first := unshared.firstVPCReadRefusal("vpc-1") + second := unshared.firstVPCReadRefusal("vpc-1") + if !first || !second { + t.Errorf("without shared state every refusal should be reported") + } + }) + + t.Run("syncs of a CSCloud built without newCSCloud share the refusals", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + mockLB := cloudstack.NewMockLoadBalancerServiceIface(ctrl) + mockLB.EXPECT().NewListLoadBalancerRulesParams().Return(&cloudstack.ListLoadBalancerRulesParams{}).Times(2) + mockLB.EXPECT().ListLoadBalancerRules(gomock.Any()).Return(&cloudstack.ListLoadBalancerRulesResponse{}, nil).Times(2) + + cs := &CSCloud{client: &cloudstack.CloudStackClient{LoadBalancer: mockLB}} + service := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test-service", Namespace: "default"}} + first, err := cs.getLoadBalancer(service) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + second, err := cs.getLoadBalancer(service) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !first.firstVPCReadRefusal("vpc-1") || second.firstVPCReadRefusal("vpc-1") { + t.Errorf("vpc-1 should be reported by the first sync only") + } + }) + + t.Run("before CloudStack 4.23 the VPC is not looked up", func(t *testing.T) { + lb := &loadBalancer{CloudStackClient: &cloudstack.CloudStackClient{}, version: semver.Version{Major: 4, Minor: 22, Patch: 1}} + got, err := lb.vpcHasFirewall(tier) + if err != nil || got { + t.Errorf("vpcHasFirewall() = %v, %v, want false, nil", got, err) + } + }) +} + +// In a VPC with the Firewall service the tiers list NetworkACL but not Firewall, and the VPC +// virtual router only admits traffic to a public IP that a firewall rule allows. Such a tier needs +// a firewall rule next to its Network ACL rule. +func TestEnsureLoadBalancerVPCFirewall(t *testing.T) { + vpcTier := &cloudstack.Network{ + Id: "net-1", + Vpcid: "vpc-1", + Aclid: "acl-1", + Service: []cloudstack.NetworkServiceInternal{{Name: "NetworkACL"}}, + } + tierACLList := &cloudstack.NetworkACLList{Id: "acl-1", Name: "tier-acl", Vpcid: "vpc-1"} + firewallVPC := &cloudstack.VPC{Id: "vpc-1", Service: []cloudstack.VPCServiceInternal{{Name: "NetworkACL"}, {Name: "Firewall"}}} + existingRule := func() *cloudstack.LoadBalancerRule { + return &cloudstack.LoadBalancerRule{ + Id: "rule-1", + Name: "atestuid-tcp-80", + Publicip: "10.0.0.1", + Publicipid: "ip-1", + Publicport: "80", + Privateport: "30000", + Cidrlist: defaultAllowedCIDR, + Algorithm: "roundrobin", + Protocol: "tcp", + Networkid: "net-1", + } + } + + firewallRules := &cloudstack.ListFirewallRulesResponse{ + Count: 1, + FirewallRules: []*cloudstack.FirewallRule{{Id: "fw-80", Protocol: "tcp", Startport: 80, Endport: 80, Cidrlist: defaultAllowedCIDR}}, + } + tierRules := []*cloudstack.NetworkACL{ownACLRule("acl-own-80")} + + // portChange registers a sync that moves the service from port 80 to 8080: the 8080 load + // balancer rule, its ACL rule and its firewall rule, which CloudStack answers with createErr. + // Removing the old port is left to each subtest. + portChange := func(t *testing.T, createErr error) (*ensureLBTestEnv, *cloudstack.CreateFirewallRuleParams, *cloudstack.CreateNetworkACLParams) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + env := newEnsureLBTestEnv(ctrl, nil, []corev1.ServicePort{{Port: 8080, NodePort: 30000, Protocol: corev1.ProtocolTCP}}) + env.cs.version = vpcFirewallVersion + env.service.Spec.LoadBalancerSourceRanges = []string{"192.0.2.0/24"} + env.expectHosts() + env.expectRules(existingRule()) + env.network.EXPECT().GetNetworkByID("net-1", gomock.Any()).Return(vpcTier, 1, nil).Times(2) + env.vpc.EXPECT().GetVPCByID("vpc-1", gomock.Any()).Return(firewallVPC, 1, nil) + + env.acl.EXPECT().GetNetworkACLListByID("acl-1", gomock.Any()).Return(tierACLList, 1, nil) + env.acl.EXPECT().NewListNetworkACLsParams().Return(&cloudstack.ListNetworkACLsParams{}) + env.acl.EXPECT().ListNetworkACLs(gomock.Any()).Return(&cloudstack.ListNetworkACLsResponse{Count: len(tierRules), NetworkACLs: tierRules}, nil) + createACL := &cloudstack.CreateNetworkACLParams{} + env.acl.EXPECT().NewCreateNetworkACLParams("tcp").Return(createACL) + env.acl.EXPECT().CreateNetworkACL(createACL).Return(&cloudstack.CreateNetworkACLResponse{Id: "acl-own-8080"}, nil) + + // The firewall rule follows the load balancer rule: on CloudStack releases that keep a + // public IP of a tier off the router once it has a firewall rule, the other order would + // leave a new IP unconfigured. + createFirewall := &cloudstack.CreateFirewallRuleParams{} + var created *cloudstack.CreateFirewallRuleResponse + if createErr == nil { + created = &cloudstack.CreateFirewallRuleResponse{Id: "fw-8080"} + } + gomock.InOrder( + env.lb.EXPECT().NewCreateLoadBalancerRuleParams("roundrobin", "atestuid-tcp-8080", 30000, 8080).Return(&cloudstack.CreateLoadBalancerRuleParams{}), + env.lb.EXPECT().CreateLoadBalancerRule(gomock.Any()).Return(&cloudstack.CreateLoadBalancerRuleResponse{ + Id: "rule-2", Name: "atestuid-tcp-8080", Publicip: "10.0.0.1", Publicipid: "ip-1", Publicport: "8080", Protocol: "tcp", Networkid: "net-1", + }, nil), + env.firewall.EXPECT().NewListFirewallRulesParams().Return(&cloudstack.ListFirewallRulesParams{}), + env.firewall.EXPECT().ListFirewallRules(gomock.Any()).Return(firewallRules, nil), + env.firewall.EXPECT().NewCreateFirewallRuleParams("ip-1", "tcp").Return(createFirewall), + env.firewall.EXPECT().CreateFirewallRule(createFirewall).Return(created, createErr), + ) + env.lb.EXPECT().NewAssignToLoadBalancerRuleParams("rule-2").Return(&cloudstack.AssignToLoadBalancerRuleParams{}) + env.lb.EXPECT().AssignToLoadBalancerRule(gomock.Any()).Return(&cloudstack.AssignToLoadBalancerRuleResponse{}, nil) + + return env, createFirewall, createACL + } + + t.Run("a port change opens the new port in the firewall and the ACL, and closes the old one", func(t *testing.T) { + env, createFirewall, createACL := portChange(t, nil) + + env.firewall.EXPECT().NewListFirewallRulesParams().Return(&cloudstack.ListFirewallRulesParams{}) + env.firewall.EXPECT().ListFirewallRules(gomock.Any()).Return(firewallRules, nil) + deleteFirewall := &cloudstack.DeleteFirewallRuleParams{} + env.firewall.EXPECT().NewDeleteFirewallRuleParams("fw-80").Return(deleteFirewall) + env.firewall.EXPECT().DeleteFirewallRule(deleteFirewall).Return(&cloudstack.DeleteFirewallRuleResponse{}, nil) + env.acl.EXPECT().NewListNetworkACLsParams().Return(&cloudstack.ListNetworkACLsParams{}) + env.acl.EXPECT().ListNetworkACLs(gomock.Any()).Return(&cloudstack.ListNetworkACLsResponse{Count: len(tierRules), NetworkACLs: tierRules}, nil) + deleteACL := &cloudstack.DeleteNetworkACLParams{} + env.acl.EXPECT().NewDeleteNetworkACLParams("acl-own-80").Return(deleteACL) + env.acl.EXPECT().DeleteNetworkACL(deleteACL).Return(&cloudstack.DeleteNetworkACLResponse{}, nil) + env.lb.EXPECT().NewDeleteLoadBalancerRuleParams("rule-1").Return(&cloudstack.DeleteLoadBalancerRuleParams{}) + env.lb.EXPECT().DeleteLoadBalancerRule(gomock.Any()).Return(&cloudstack.DeleteLoadBalancerRuleResponse{}, nil) + + if _, err := env.cs.EnsureLoadBalancer(context.TODO(), "test-cluster", env.service, env.nodes); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cidrs, _ := createFirewall.GetCidrlist(); !reflect.DeepEqual(cidrs, []string{"192.0.2.0/24"}) { + t.Errorf("firewall rule created for %v, want spec.loadBalancerSourceRanges", cidrs) + } + start, _ := createFirewall.GetStartport() + end, _ := createFirewall.GetEndport() + if start != 8080 || end != 8080 { + t.Errorf("firewall rule created for ports %v-%v, want 8080", start, end) + } + if aclPort, _ := createACL.GetStartport(); aclPort != 8080 { + t.Errorf("ACL rule created for port %v, want 8080", aclPort) + } + }) + + t.Run("an old port whose firewall rule cannot be deleted keeps its load balancer and ACL rules", func(t *testing.T) { + env, _, _ := portChange(t, nil) + + env.firewall.EXPECT().NewListFirewallRulesParams().Return(&cloudstack.ListFirewallRulesParams{}) + env.firewall.EXPECT().ListFirewallRules(gomock.Any()).Return(firewallRules, nil) + deleteFirewall := &cloudstack.DeleteFirewallRuleParams{} + env.firewall.EXPECT().NewDeleteFirewallRuleParams("fw-80").Return(deleteFirewall) + env.firewall.EXPECT().DeleteFirewallRule(deleteFirewall).Return(nil, fmt.Errorf("router unreachable")) + + _, err := env.cs.EnsureLoadBalancer(context.TODO(), "test-cluster", env.service, env.nodes) + if err == nil || !strings.Contains(err.Error(), "router unreachable") { + t.Fatalf("error = %v, want the firewall rule failure reported for a retry", err) + } + }) + + t.Run("a firewall rule that cannot be created fails the sync", func(t *testing.T) { + env, _, _ := portChange(t, fmt.Errorf("router unreachable")) + + _, err := env.cs.EnsureLoadBalancer(context.TODO(), "test-cluster", env.service, env.nodes) + if err == nil || !strings.Contains(err.Error(), "router unreachable") { + t.Fatalf("error = %v, want the firewall rule failure", err) + } + }) + + t.Run("an account that may not list VPCs keeps only the ACL rule", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + env := newEnsureLBTestEnv(ctrl, nil, []corev1.ServicePort{{Port: 80, NodePort: 30000, Protocol: corev1.ProtocolTCP}}) + env.cs.version = vpcFirewallVersion + env.expectHosts() + env.expectRules(existingRule()) + env.network.EXPECT().GetNetworkByID("net-1", gomock.Any()).Return(vpcTier, 1, nil).Times(2) + env.vpc.EXPECT().GetVPCByID("vpc-1", gomock.Any()).Return(nil, -1, + fmt.Errorf("CloudStack API error 432 (CSExceptionErrorCode: 9999): The API [listVPCs] does not exist or is not available for the account")) + env.acl.EXPECT().GetNetworkACLListByID("acl-1", gomock.Any()).Return(tierACLList, 1, nil) + env.acl.EXPECT().NewListNetworkACLsParams().Return(&cloudstack.ListNetworkACLsParams{}) + env.acl.EXPECT().ListNetworkACLs(gomock.Any()).Return(&cloudstack.ListNetworkACLsResponse{Count: 1, NetworkACLs: []*cloudstack.NetworkACL{ownACLRule("acl-own-80")}}, nil) + + if _, err := env.cs.EnsureLoadBalancer(context.TODO(), "test-cluster", env.service, env.nodes); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("a failed VPC lookup for a new service associates no IP", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + env := newEnsureLBTestEnv(ctrl, nil, []corev1.ServicePort{{Port: 80, NodePort: 30000, Protocol: corev1.ProtocolTCP}}) + env.cs.version = vpcFirewallVersion + env.expectHosts() + env.expectRules() + env.network.EXPECT().GetNetworkByID("net-1", gomock.Any()).Return(vpcTier, 1, nil) + env.vpc.EXPECT().GetVPCByID("vpc-1", gomock.Any()).Return(nil, -1, fmt.Errorf("API unavailable")) + + _, err := env.cs.EnsureLoadBalancer(context.TODO(), "test-cluster", env.service, env.nodes) + if err == nil || !strings.Contains(err.Error(), "API unavailable") { + t.Fatalf("error = %v, want the lookup failure", err) + } + }) + + t.Run("a failed VPC lookup creates nothing", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + env := newEnsureLBTestEnv(ctrl, nil, []corev1.ServicePort{{Port: 8080, NodePort: 30000, Protocol: corev1.ProtocolTCP}}) + env.cs.version = vpcFirewallVersion + env.expectHosts() + env.expectRules(existingRule()) + env.network.EXPECT().GetNetworkByID("net-1", gomock.Any()).Return(vpcTier, 1, nil) + env.vpc.EXPECT().GetVPCByID("vpc-1", gomock.Any()).Return(nil, -1, fmt.Errorf("API unavailable")) + + _, err := env.cs.EnsureLoadBalancer(context.TODO(), "test-cluster", env.service, env.nodes) + if err == nil || !strings.Contains(err.Error(), "API unavailable") { + t.Fatalf("error = %v, want the lookup failure", err) + } + }) +} + +func TestEnsureLoadBalancerDeletedVPCFirewall(t *testing.T) { + service := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "test-service", Namespace: "default", UID: "test-uid"}} + firewallVPC := &cloudstack.VPC{Id: "vpc-1", Service: []cloudstack.VPCServiceInternal{{Name: "NetworkACL"}, {Name: "Firewall"}}} + + type mocks struct { + lb *cloudstack.MockLoadBalancerServiceIface + firewall *cloudstack.MockFirewallServiceIface + acl *cloudstack.MockNetworkACLServiceIface + address *cloudstack.MockAddressServiceIface + vpc *cloudstack.MockVPCServiceIface + } + newEnv := func(t *testing.T) (*CSCloud, mocks) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + m := mocks{ + lb: cloudstack.NewMockLoadBalancerServiceIface(ctrl), + firewall: cloudstack.NewMockFirewallServiceIface(ctrl), + acl: cloudstack.NewMockNetworkACLServiceIface(ctrl), + address: cloudstack.NewMockAddressServiceIface(ctrl), + vpc: cloudstack.NewMockVPCServiceIface(ctrl), + } + mockNetwork := cloudstack.NewMockNetworkServiceIface(ctrl) + + m.lb.EXPECT().NewListLoadBalancerRulesParams().Return(&cloudstack.ListLoadBalancerRulesParams{}) + m.lb.EXPECT().ListLoadBalancerRules(gomock.Any()).Return(&cloudstack.ListLoadBalancerRulesResponse{ + Count: 1, + LoadBalancerRules: []*cloudstack.LoadBalancerRule{{ + Id: "rule-1", Name: "atestuid-tcp-80", Publicip: "10.0.0.1", Publicipid: "ip-1", Publicport: "80", Protocol: "tcp", Networkid: "net-tier", + }}, + }, nil) + mockNetwork.EXPECT().GetNetworkByID("net-tier", gomock.Any()).Return(&cloudstack.Network{Id: "net-tier", Vpcid: "vpc-1"}, 1, nil) + + cs := &CSCloud{ + client: &cloudstack.CloudStackClient{ + LoadBalancer: m.lb, Network: mockNetwork, Firewall: m.firewall, NetworkACL: m.acl, Address: m.address, VPC: m.vpc, + }, + version: vpcFirewallVersion, + } + return cs, m + } + firewallRules := &cloudstack.ListFirewallRulesResponse{ + Count: 1, + FirewallRules: []*cloudstack.FirewallRule{{Id: "fw-80", Protocol: "tcp", Startport: 80, Endport: 80, Cidrlist: defaultAllowedCIDR}}, + } + + t.Run("deletes the firewall rule on the rule's IP and its own ACL rule, then the rule and the IP", func(t *testing.T) { + cs, m := newEnv(t) + m.vpc.EXPECT().GetVPCByID("vpc-1", gomock.Any()).Return(firewallVPC, 1, nil) + listFirewall := &cloudstack.ListFirewallRulesParams{} + deleteFirewall := &cloudstack.DeleteFirewallRuleParams{} + deleteACL := &cloudstack.DeleteNetworkACLParams{} + deleteRule := &cloudstack.DeleteLoadBalancerRuleParams{} + release := &cloudstack.DisassociateIpAddressParams{} + gomock.InOrder( + m.firewall.EXPECT().NewListFirewallRulesParams().Return(listFirewall), + m.firewall.EXPECT().ListFirewallRules(listFirewall).Return(firewallRules, nil), + m.firewall.EXPECT().NewDeleteFirewallRuleParams("fw-80").Return(deleteFirewall), + m.firewall.EXPECT().DeleteFirewallRule(deleteFirewall).Return(&cloudstack.DeleteFirewallRuleResponse{}, nil), + m.acl.EXPECT().NewListNetworkACLsParams().Return(&cloudstack.ListNetworkACLsParams{}), + m.acl.EXPECT().ListNetworkACLs(gomock.Any()).Return(&cloudstack.ListNetworkACLsResponse{Count: 1, NetworkACLs: []*cloudstack.NetworkACL{ownACLRule("acl-own")}}, nil), + m.acl.EXPECT().NewDeleteNetworkACLParams("acl-own").Return(deleteACL), + m.acl.EXPECT().DeleteNetworkACL(deleteACL).Return(&cloudstack.DeleteNetworkACLResponse{}, nil), + m.lb.EXPECT().NewDeleteLoadBalancerRuleParams("rule-1").Return(deleteRule), + m.lb.EXPECT().DeleteLoadBalancerRule(deleteRule).Return(&cloudstack.DeleteLoadBalancerRuleResponse{}, nil), + m.address.EXPECT().NewDisassociateIpAddressParams("ip-1").Return(release), + m.address.EXPECT().DisassociateIpAddress(release).Return(&cloudstack.DisassociateIpAddressResponse{}, nil), + ) + + if err := cs.EnsureLoadBalancerDeleted(context.TODO(), "test-cluster", service); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ipID, _ := listFirewall.GetIpaddressid(); ipID != "ip-1" { + t.Errorf("firewall rules listed on IP %q, want ip-1", ipID) + } + }) + + t.Run("keeps the rule and the IP when the firewall rule cannot be deleted", func(t *testing.T) { + cs, m := newEnv(t) + m.vpc.EXPECT().GetVPCByID("vpc-1", gomock.Any()).Return(firewallVPC, 1, nil) + m.firewall.EXPECT().NewListFirewallRulesParams().Return(&cloudstack.ListFirewallRulesParams{}) + m.firewall.EXPECT().ListFirewallRules(gomock.Any()).Return(firewallRules, nil) + deleteFirewall := &cloudstack.DeleteFirewallRuleParams{} + m.firewall.EXPECT().NewDeleteFirewallRuleParams("fw-80").Return(deleteFirewall) + m.firewall.EXPECT().DeleteFirewallRule(deleteFirewall).Return(nil, fmt.Errorf("router unreachable")) + + err := cs.EnsureLoadBalancerDeleted(context.TODO(), "test-cluster", service) + if err == nil || !strings.Contains(err.Error(), "router unreachable") { + t.Fatalf("error = %v, want the firewall rule failure reported for a retry", err) + } + }) + + t.Run("an account that may not list VPCs still deletes the ACL rule, the rule and the IP", func(t *testing.T) { + cs, m := newEnv(t) + m.vpc.EXPECT().GetVPCByID("vpc-1", gomock.Any()).Return(nil, -1, + fmt.Errorf("CloudStack API error 432 (CSExceptionErrorCode: 9999): The API [listVPCs] does not exist or is not available for the account")) + deleteACL := &cloudstack.DeleteNetworkACLParams{} + deleteRule := &cloudstack.DeleteLoadBalancerRuleParams{} + release := &cloudstack.DisassociateIpAddressParams{} + gomock.InOrder( + m.acl.EXPECT().NewListNetworkACLsParams().Return(&cloudstack.ListNetworkACLsParams{}), + m.acl.EXPECT().ListNetworkACLs(gomock.Any()).Return(&cloudstack.ListNetworkACLsResponse{Count: 1, NetworkACLs: []*cloudstack.NetworkACL{ownACLRule("acl-own")}}, nil), + m.acl.EXPECT().NewDeleteNetworkACLParams("acl-own").Return(deleteACL), + m.acl.EXPECT().DeleteNetworkACL(deleteACL).Return(&cloudstack.DeleteNetworkACLResponse{}, nil), + m.lb.EXPECT().NewDeleteLoadBalancerRuleParams("rule-1").Return(deleteRule), + m.lb.EXPECT().DeleteLoadBalancerRule(deleteRule).Return(&cloudstack.DeleteLoadBalancerRuleResponse{}, nil), + m.address.EXPECT().NewDisassociateIpAddressParams("ip-1").Return(release), + m.address.EXPECT().DisassociateIpAddress(release).Return(&cloudstack.DisassociateIpAddressResponse{}, nil), + ) + + if err := cs.EnsureLoadBalancerDeleted(context.TODO(), "test-cluster", service); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("keeps the rule and the IP when the VPC cannot be read", func(t *testing.T) { + cs, m := newEnv(t) + m.vpc.EXPECT().GetVPCByID("vpc-1", gomock.Any()).Return(nil, -1, fmt.Errorf("API unavailable")) + + err := cs.EnsureLoadBalancerDeleted(context.TODO(), "test-cluster", service) + if err == nil || !strings.Contains(err.Error(), "API unavailable") { + t.Fatalf("error = %v, want the lookup failure reported for a retry", err) + } + }) +} + +// An obsolete rule in another VPC whose VPC cannot be read keeps its load balancer, firewall and +// ACL rules, so the next sync can retry. +func TestPruneRulesKeepsTheRuleWhenItsVPCCannotBeRead(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + mockNetwork := cloudstack.NewMockNetworkServiceIface(ctrl) + mockVPC := cloudstack.NewMockVPCServiceIface(ctrl) + mockNetwork.EXPECT().GetNetworkByID("net-old", gomock.Any()).Return(&cloudstack.Network{ + Id: "net-old", Vpcid: "vpc-old", Service: []cloudstack.NetworkServiceInternal{{Name: "NetworkACL"}}, + }, 1, nil) + mockVPC.EXPECT().GetVPCByID("vpc-old", gomock.Any()).Return(nil, -1, fmt.Errorf("API unavailable")) + + lb := &loadBalancer{ + CloudStackClient: &cloudstack.CloudStackClient{ + LoadBalancer: cloudstack.NewMockLoadBalancerServiceIface(ctrl), + Network: mockNetwork, + NetworkACL: cloudstack.NewMockNetworkACLServiceIface(ctrl), + Firewall: cloudstack.NewMockFirewallServiceIface(ctrl), + VPC: mockVPC, + }, + name: "atestuid", + networkID: "net-new", + ipAddrID: "ip-current", + version: vpcFirewallVersion, + } + obsolete := []obsoleteRule{{ + rule: &cloudstack.LoadBalancerRule{Id: "rule-old", Name: "atestuid-tcp-80", Publicipid: "ip-2", Publicport: "80", Protocol: "tcp", Networkid: "net-old"}, + protocol: LoadBalancerProtocolTCP, + tuple: portProtocol{"tcp", 80}, + }} + + err := lb.pruneRules(obsolete, nil, &cloudstack.Network{Id: "net-new", Service: []cloudstack.NetworkServiceInternal{{Name: "Firewall"}}}) + if err == nil || !strings.Contains(err.Error(), "API unavailable") { + t.Fatalf("error = %v, want the lookup failure", err) + } +} + // A Network ACL rule is scoped to a network, so an obsolete rule left over in another network // is not protected by a desired port that happens to share its protocol and port. func TestPruneRulesScopesNetworkACLsToTheirNetwork(t *testing.T) {