Skip to content

Commit

Permalink
Fix issue with pre-allocated addresses
Browse files Browse the repository at this point in the history
In case of pre-allocated addresses, the claims were not given the
proper prefix, gateway and dns server overrides. In addition to
fixing this, some validations on the pool updates are added to
verify that no addresses would be out of bonds.
  • Loading branch information
fmuyassarov committed Apr 15, 2021
1 parent b276c4f commit 9eb06d5
Show file tree
Hide file tree
Showing 6 changed files with 574 additions and 295 deletions.
65 changes: 65 additions & 0 deletions api/v1alpha1/ippool_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,78 @@ func (c *IPPool) ValidateUpdate(old runtime.Object) error {
),
)
}
allocationOutOfBonds, inUseOutOfBonds := c.checkPoolBonds(oldM3ipp)
if len(allocationOutOfBonds) != 0 {
for _, address := range allocationOutOfBonds {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("spec", "preAllocations"),
address,
"is out of bonds of the pools given",
),
)
}
}
if len(inUseOutOfBonds) != 0 {
for _, address := range inUseOutOfBonds {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("spec", "pools"),
address,
"is in use but out of bonds of the pools given",
),
)
}
}

if len(allErrs) == 0 {
return nil
}
return apierrors.NewInvalid(GroupVersion.WithKind("Metal3Data").GroupKind(), c.Name, allErrs)
}

func (c *IPPool) checkPoolBonds(old *IPPool) ([]IPAddressStr, []IPAddressStr) {
allocationOutOfBonds := []IPAddressStr{}
inUseOutOfBonds := []IPAddressStr{}
for _, address := range c.Spec.PreAllocations {
inBonds := c.isAddressInBonds(address)

if !inBonds {
allocationOutOfBonds = append(allocationOutOfBonds, address)
}
}
for _, address := range old.Status.Allocations {
inBonds := c.isAddressInBonds(address)

if !inBonds {
inUseOutOfBonds = append(inUseOutOfBonds, address)
}
}
return allocationOutOfBonds, inUseOutOfBonds
}

func (c *IPPool) isAddressInBonds(address IPAddressStr) bool {
inBonds := false
for _, pool := range c.Spec.Pools {
if inBonds {
break
}
index := 0
for !inBonds {
allocatedAddress, err := GetIPAddress(pool, index)
if err != nil {
break
}
index++
if allocatedAddress == address {
inBonds = true
break
}
}
}
return inBonds
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (c *IPPool) ValidateDelete() error {
return nil
Expand Down
102 changes: 85 additions & 17 deletions api/v1alpha1/ippool_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,33 +71,100 @@ func TestIPPoolValidation(t *testing.T) {

func TestIPPoolUpdateValidation(t *testing.T) {

startAddr := IPAddressStr("192.168.0.1")
endAddr := IPAddressStr("192.168.0.10")

tests := []struct {
name string
expectErr bool
newPool *IPPoolSpec
oldPool *IPPoolSpec
name string
expectErr bool
newPoolSpec *IPPoolSpec
oldPoolSpec *IPPoolSpec
oldPoolStatus IPPoolStatus
}{
{
name: "should succeed when values and templates correct",
expectErr: false,
newPool: &IPPoolSpec{},
oldPool: &IPPoolSpec{},
name: "should succeed when values and templates correct",
expectErr: false,
newPoolSpec: &IPPoolSpec{},
oldPoolSpec: &IPPoolSpec{},
},
{
name: "should fail when oldPool is nil",
expectErr: true,
newPool: &IPPoolSpec{},
oldPool: nil,
name: "should fail when oldPoolSpec is nil",
expectErr: true,
newPoolSpec: &IPPoolSpec{},
oldPoolSpec: nil,
},
{
name: "should fail when namePrefix value changes",
expectErr: true,
newPool: &IPPoolSpec{
newPoolSpec: &IPPoolSpec{
NamePrefix: "abcde",
},
oldPool: &IPPoolSpec{
oldPoolSpec: &IPPoolSpec{
NamePrefix: "abcd",
},
},
{
name: "should succeed when preAllocations are correct",
expectErr: false,
newPoolSpec: &IPPoolSpec{
NamePrefix: "abcd",
Pools: []Pool{
{Start: &startAddr, End: &endAddr},
},
PreAllocations: map[string]IPAddressStr{
"alloc": IPAddressStr("192.168.0.2"),
},
},
oldPoolSpec: &IPPoolSpec{
NamePrefix: "abcd",
},
oldPoolStatus: IPPoolStatus{
Allocations: map[string]IPAddressStr{
"inuse": IPAddressStr("192.168.0.3"),
},
},
},
{
name: "should fail when preAllocations are incorrect",
expectErr: true,
newPoolSpec: &IPPoolSpec{
NamePrefix: "abcd",
Pools: []Pool{
{Start: &startAddr, End: &endAddr},
},
PreAllocations: map[string]IPAddressStr{
"alloc": IPAddressStr("192.168.0.20"),
},
},
oldPoolSpec: &IPPoolSpec{
NamePrefix: "abcd",
},
oldPoolStatus: IPPoolStatus{
Allocations: map[string]IPAddressStr{
"inuse": IPAddressStr("192.168.0.3"),
},
},
},
{
name: "should fail when ip in use",
expectErr: true,
newPoolSpec: &IPPoolSpec{
NamePrefix: "abcd",
Pools: []Pool{
{Start: &startAddr, End: &endAddr},
},
PreAllocations: map[string]IPAddressStr{
"alloc": IPAddressStr("192.168.0.2"),
},
},
oldPoolSpec: &IPPoolSpec{
NamePrefix: "abcd",
},
oldPoolStatus: IPPoolStatus{
Allocations: map[string]IPAddressStr{
"inuse": IPAddressStr("192.168.0.30"),
},
},
},
}

Expand All @@ -109,15 +176,16 @@ func TestIPPoolUpdateValidation(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
},
Spec: *tt.newPool,
Spec: *tt.newPoolSpec,
}

if tt.oldPool != nil {
if tt.oldPoolSpec != nil {
oldPool = &IPPool{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
},
Spec: *tt.oldPool,
Spec: *tt.oldPoolSpec,
Status: tt.oldPoolStatus,
}
} else {
oldPool = nil
Expand Down
126 changes: 126 additions & 0 deletions api/v1alpha1/utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/*
Copyright 2019 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1

import (
"fmt"
"math/big"
"net"

"github.com/pkg/errors"
)

// GetIPAddress renders the IP address, taking the index, offset and step into
// account, it is IP version agnostic
func GetIPAddress(entry Pool, index int) (IPAddressStr, error) {

if entry.Start == nil && entry.Subnet == nil {
return "", errors.New("Either Start or Subnet is required for ipAddress")
}
var ip net.IP
var err error
var ipNet *net.IPNet
offset := index

// If start is given, use it to add the offset
if entry.Start != nil {
var endIP net.IP
if entry.End != nil {
endIP = net.ParseIP(string(*entry.End))
}
ip, err = addOffsetToIP(net.ParseIP(string(*entry.Start)), endIP, offset)
if err != nil {
return "", err
}

// Verify that the IP is in the subnet
if entry.Subnet != nil {
_, ipNet, err = net.ParseCIDR(string(*entry.Subnet))
if err != nil {
return "", err
}
if !ipNet.Contains(ip) {
return "", errors.New("IP address out of bonds")
}
}

// If it is not given, use the CIDR ip address and increment the offset by 1
} else {
ip, ipNet, err = net.ParseCIDR(string(*entry.Subnet))
if err != nil {
return "", err
}
offset++
ip, err = addOffsetToIP(ip, nil, offset)
if err != nil {
return "", err
}

// Verify that the ip is in the subnet
if !ipNet.Contains(ip) {
return "", errors.New("IP address out of bonds")
}
}
return IPAddressStr(ip.String()), nil
}

// addOffsetToIP computes the value of the IP address with the offset. It is
// IP version agnostic
// Note that if the resulting IP address is in the format ::ffff:xxxx:xxxx then
// ip.String will fail to select the correct type of ip
func addOffsetToIP(ip, endIP net.IP, offset int) (net.IP, error) {
ip4 := false
//ip := net.ParseIP(ipString)
if ip.To4() != nil {
ip4 = true
}

// Create big integers
IPInt := big.NewInt(0)
OffsetInt := big.NewInt(int64(offset))

// Transform the ip into an int. (big endian function)
IPInt = IPInt.SetBytes(ip)

// add the two integers
IPInt = IPInt.Add(IPInt, OffsetInt)

// return the bytes list
IPBytes := IPInt.Bytes()

IPBytesLen := len(IPBytes)

// Verify that the IPv4 or IPv6 fulfills theirs constraints
if (ip4 && IPBytesLen > 6 && IPBytes[4] != 255 && IPBytes[5] != 255) ||
(!ip4 && IPBytesLen > 16) {
return nil, errors.New(fmt.Sprintf("IP address overflow for : %s", ip.String()))
}

//transform the end ip into an Int to compare
if endIP != nil {
endIPInt := big.NewInt(0)
endIPInt = endIPInt.SetBytes(endIP)
// Computed IP is higher than the end IP
if IPInt.Cmp(endIPInt) > 0 {
return nil, errors.New(fmt.Sprintf("IP address out of bonds for : %s", ip.String()))
}
}

// COpy the output back into an ip
copy(ip[16-IPBytesLen:], IPBytes)
return ip, nil
}
Loading

0 comments on commit 9eb06d5

Please sign in to comment.