Support loadBalancerSourceRanges

[onitake]

Fixes #3 .

⚠️ This should be merged after the following PRs, if possible:

• Some fixes for Go 1.13 #8
• Fix incorrect path for injected git version tags #10
• Don't copy the whole build context from build to runtime container #11

[onitake]

Also pinging @josvo so he can take a look.

So far, I've only fixed the dependencies for Go 1.13 (separate PR #8 , please merge) and added some code to interpret the loadBalancerSourceRanges flag and disable automatic rule generation.

The actual firewall code will be a bit more involved I think, because we need to add full CRUD support to avoid stray rules. AFAIK the CloudStack API doesn't offer some sort of atomic firewall rule set update. Each rule has to be deleted and recreated one-by-one. Correct @GabrielBrascher ?

[onitake]

Proposed solution:

• Implement a new function updateFirewallRules that takes one argument - the LB NAT IP object's UUID and the new rule set from loadBalancerSourceRanges
  • Replace loadBalancerSourceRanges with ["0.0.0.0/0"] if the list is empty
  • Fetch the NAT IP's current rule set via listFirewallRules
  • Compare the current rule set against loadBalancerSourceRanges
  • If they are identical, return
  • If they are not, add all rules via createFirewallRule then
  • remove all previous rules from the current rule set via deleteFirewallRule
• Each time EnsureLoadBalancer is called, call updateFirewallRules
• Call p.SetOpenfirewall(false) unconditionally

This ensures that the firewall rules can be updated without service interruption.

Caveat: What happens if an identical rule is added twice? Will it be ignore by CS? If yes, additional care needs to be taken not to remove it in step three. This could be done by looking up the returned id in the list of previous IP addresses.

[onitake]

@josvo Ready to be tested.

[onitake]

Test results:

• Upon creating new load balancer rules or changing ports/protocols, the firewall rules are created
• Old entries with different IP are not removed
• Only changing the loadBalancerSourceRanges has no effect

We'll keep working on it.

[onitake]

Figured out how to create and update the firewall rules - create, update and delete is now working properly.
Ready to merge.

[onitake]

I reverted all changes that are covered by the other PRs for easier review.

[joschi36]

Hi @onitake
I tested the CCM and saw some issues, here my tests:

Tests:

☑️ Creating Service LoadBalancer without loadBalancerSourceRanges

Configured:

No loadBalancerSourceRanges

Expected

0.0.0.0/0

Got

0.0.0.0/0

☑️ Creating Service LoadBalancer with one loadBalancerSourceRanges entry

Configured:

loadBalancerSourceRanges:
- 1.2.3.4/32

Expected

1.2.3.4/32

Got

1.2.3.4/32

❎ Extending Service LoadBalancer with one loadBalancerSourceRanges entry

Before:

loadBalancerSourceRanges:
- 1.2.3.4/32

Configured:

loadBalancerSourceRanges:
- 1.2.3.4/32
- 5.6.7.8/32

Expected

1.2.3.4/32,5.6.7.8/32

Got

1.2.3.4/32

Log

I0814 11:10:41.639262       1 reflector.go:243] k8s.io/client-go/informers/factory.go:133: forcing resync
I0814 11:10:48.523517       1 service_controller.go:329] Ensuring load balancer for service http-echo/test
I0814 11:10:48.524439       1 event.go:258] Event(v1.ObjectReference{Kind:"Service", Namespace:"http-echo", Name:"test", UID:"2364b2e3-78a2-4ab6-92cf-3f42942ac88a", APIVersion:"v1", ResourceVersion:"79821811", FieldPath:""}): type: 'Normal' reason: 'LoadBalancerSourceRanges' [1.2.3.4/32] -> [1.2.3.4/32 5.6.7.8/32]
I0814 11:10:48.524551       1 event.go:258] Event(v1.ObjectReference{Kind:"Service", Namespace:"http-echo", Name:"test", UID:"2364b2e3-78a2-4ab6-92cf-3f42942ac88a", APIVersion:"v1", ResourceVersion:"79821811", FieldPath:""}): type: 'Normal' reason: 'EnsuringLoadBalancer' Ensuring load balancer
I0814 11:10:48.523610       1 cloudstack_loadbalancer.go:77] EnsureLoadBalancer(kubernetes, http-echo, test, , [{ TCP 80 {0 80 } 31324}], REDACT METADATA (useless info)
I0814 11:10:48.994986       1 cloudstack_loadbalancer.go:330] Load balancer a2364b2e378a24ab692cf3f42942ac88 contains 1 rule(s)
I0814 11:10:49.165197       1 cloudstack_loadbalancer.go:122] Load balancer a2364b2e378a24ab692cf3f42942ac88 is associated with IP 10.101.47.58
I0814 11:10:49.165241       1 cloudstack_loadbalancer.go:152] Load balancer rule a2364b2e378a24ab692cf3f42942ac88-tcp-80 is up-to-date
I0814 11:10:49.165250       1 cloudstack_loadbalancer.go:170] Creating firewall rules for load balancer rule: a2364b2e378a24ab692cf3f42942ac88-tcp-80 (tcp:10.101.47.58:80)
I0814 11:10:49.165260       1 cloudstack_loadbalancer.go:646] Listing firewall rules for &{map[ipaddressid:f309a2f1-7d27-4e1c-87ce-644372347c92 listall:true projectid:175d37b9-0478-4f9e-ab23-9ebba6c1eb64]}
I0814 11:10:49.216858       1 cloudstack_loadbalancer.go:651] All firewall rules for 10.101.47.58: [0xc000b237a0]
I0814 11:10:49.216889       1 cloudstack_loadbalancer.go:662] Matching rules for 10.101.47.58: map[0xc000b237a0:true]
E0814 11:10:49.300285       1 cloudstack_loadbalancer.go:172] Error updating firewall rules for load balancer rule a2364b2e378a24ab692cf3f42942ac88-tcp-80: error creating new firewall rule for public IP f309a2f1-7d27-4e1c-87ce-644372347c92, proto tcp, port 80, allowed [1.2.3.4/32 5.6.7.8/32]: CloudStack API error 537 (CSExceptionErrorCode: 9999): The range specified, 80-80, conflicts with rule 50627 which has 80-80
I0814 11:10:49.300378       1 service_controller.go:719] Finished syncing service "http-echo/test" (776.883593ms)
I0814 11:10:49.300476       1 event.go:258] Event(v1.ObjectReference{Kind:"Service", Namespace:"http-echo", Name:"test", UID:"2364b2e3-78a2-4ab6-92cf-3f42942ac88a", APIVersion:"v1", ResourceVersion:"79821811", FieldPath:""}): type: 'Normal' reason: 'EnsuredLoadBalancer' Ensured load balancer

☑️ Removing loadBalancerSourceRanges with one entry from Service LoadBalancer

Before:

loadBalancerSourceRanges:
- 1.2.3.4/32

Expected

0.0.0.0/0

Got

0.0.0.0/0

☑️ Creating Service LoadBalancer with two loadBalancerSourceRanges

Configured:

loadBalancerSourceRanges:
- 1.2.3.4/32
- 5.6.7.8/32

Expected

1.2.3.4/32,5.6.7.8/32

Got

1.2.3.4/32,5.6.7.8/32

☑️ Removing loadBalancerSourceRanges with two entries from Service LoadBalancer

Before:

loadBalancerSourceRanges:
- 1.2.3.4/32
- 5.6.7.8/32

Expected

0.0.0.0/0

Got

0.0.0.0/0

[onitake]

Thanks for testing!
We should put all this into automated unit tests.

The case that doesn't work is kind of expected... the internal logic is "create before delete" to avoid having no firewall rules during the transition. But of course that doesn't work when entries are added to the rule list.
I'll see what can be done about it.

As a workaround, you can replace the rules with "safe" ones (such as 127.0.0.1/32), then change them to the correct list.

[onitake]

Ok, so.... as a first measure, I can make EnsureLoadBalancer fail when it can't create the firewall rules.
This may leave it in an invalid state, however.

[onitake]

I flipped the logic around.
Firewall rules are now deleted before the new rules are added.
This should resolve the issue.

[yadvr]

@onitake can you advise when this is ready for merging, thnx

[onitake]

@joschi36 Is this working as expected now, or do I need to take another look?

[joschi36]

Works like a charm.

[yadvr]

Is this ready for merging @onitake @joschi36 ? I can merge after #8

[onitake]

@rhtyd Please go ahead and merge #8 and #9.