Skip to content

Commit

Permalink
feat(CSI-244): match subnets if existing in client rule (#315)
Browse files Browse the repository at this point in the history
### TL;DR

Improved NFS client group rule matching to include superset IP networks.

### What changed?

- Added `IsSupersetOf` method to `NfsClientGroupRule` to check if a rule covers a larger IP range.
- Updated `FindNfsClientGroupRulesByFilter` to include rules that are supersets of the query.
- Implemented `GetMaskBits` method for `Network` to calculate CIDR notation.
- Enhanced `ContainsIPAddress` method to handle cases where CIDR parsing fails.
- Added unit tests for the new `IsSupersetOf` functionality.

### How to test?

1. Run the new unit tests in `nfs_test.go`.
2. Test the `FindNfsClientGroupRulesByFilter` function with various IP ranges and ensure it returns both exact matches and superset rules.
3. Verify that the `ContainsIPAddress` method correctly identifies IP addresses within a given network range.

### Why make this change?

This change improves the flexibility and accuracy of NFS client group rule matching. By including superset IP networks, the system can now identify and apply rules that cover a broader range of IP addresses, enhancing the overall functionality of the NFS access control system.

When having an extremely large Kubernetes clusters, adding node IP addresses to client group could harm the rule matching performance or hit limits on max. number of rules. This allows using a subnet addresses (those should be configured by administrator)

---
  • Loading branch information
sergeyberezansky authored Sep 12, 2024
2 parents 9f5d1f4 + 2006fda commit 926d473
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 2 deletions.
12 changes: 12 additions & 0 deletions pkg/wekafs/apiclient/nfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,15 @@ func (r *NfsClientGroupRule) EQ(other ApiObject) bool {
return ObjectsAreEqual(r, other)
}

func (r *NfsClientGroupRule) IsSupersetOf(other *NfsClientGroupRule) bool {
if r.IsIPRule() && other.IsIPRule() {
n1 := r.GetNetwork()
n2 := other.GetNetwork()
return n1.ContainsIPAddress(n2.IP.String())
}
return false
}

func (r *NfsClientGroupRule) getImmutableFields() []string {
return []string{"Rule"}
}
Expand Down Expand Up @@ -566,6 +575,9 @@ func (a *ApiClient) FindNfsClientGroupRulesByFilter(ctx context.Context, query *
for _, r := range ret {
if r.EQ(query) {
*resultSet = append(*resultSet, r)
} else if r.IsSupersetOf(query) {
// if we have a rule that covers the IP address by bigger network segment, also add it
*resultSet = append(*resultSet, r)
}
}
return nil
Expand Down
30 changes: 30 additions & 0 deletions pkg/wekafs/apiclient/nfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,3 +312,33 @@ func TestInterfaceGroup(t *testing.T) {
// }
//}
}

func TestIsSupersetOf(t *testing.T) {
// Test case 1: IP rule superset
rule1 := &NfsClientGroupRule{
Type: NfsClientGroupRuleTypeIP,
Rule: "192.168.1.0/24",
}
rule2 := &NfsClientGroupRule{
Type: NfsClientGroupRuleTypeIP,
Rule: "192.168.1.1",
}
assert.True(t, rule1.IsSupersetOf(rule2))

// Test case 2: IP rule not superset
rule3 := &NfsClientGroupRule{
Type: NfsClientGroupRuleTypeIP,
Rule: "192.168.2.0/24",
}
assert.False(t, rule1.IsSupersetOf(rule3))

// Test case 3: Non-IP rule
rule4 := &NfsClientGroupRule{
Type: NfsClientGroupRuleTypeDNS,
Rule: "example.com",
}
assert.False(t, rule1.IsSupersetOf(rule4))

// Test case 4: Same rule
assert.True(t, rule1.IsSupersetOf(rule1))
}
26 changes: 24 additions & 2 deletions pkg/wekafs/apiclient/utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package apiclient

import (
"encoding/binary"
"fmt"
"github.com/rs/zerolog/log"
"hash/fnv"
Expand Down Expand Up @@ -73,6 +74,24 @@ func (n *Network) AsNfsRule() string {
return fmt.Sprintf("%s/%s", n.IP.String(), n.Subnet.String())
}

func (n *Network) GetMaskBits() int {
ip := n.Subnet.To4()
if ip == nil {
return 0
}
// Count the number of 1 bits
mask := binary.BigEndian.Uint32(ip)

// Count the number of set bits
cidrBits := 0
for mask != 0 {
cidrBits += int(mask & 1)
mask >>= 1
}

return cidrBits
}

func parseNetworkString(s string) (*Network, error) {
var ip, subnet net.IP
if strings.Contains(s, "/") {
Expand Down Expand Up @@ -107,8 +126,11 @@ func (n *Network) ContainsIPAddress(ipStr string) bool {
}

_, ipNet, err := net.ParseCIDR(fmt.Sprintf("%s/%s", n.IP.String(), n.Subnet.String()))
if err != nil {
return false
if err != nil || ipNet == nil {
_, ipNet, err = net.ParseCIDR(fmt.Sprintf("%s/%d", n.IP.String(), n.GetMaskBits()))
if err != nil || ipNet == nil {
return false
}
}
return ipNet.Contains(ip)
}
Expand Down

0 comments on commit 926d473

Please sign in to comment.