Skip to content

Commit

Permalink
add validation for IPAM IPPools annotation usage
Browse files Browse the repository at this point in the history
Signed-off-by: Icarus9913 <[email protected]>
  • Loading branch information
Icarus9913 committed Dec 14, 2023
1 parent cbd4be9 commit cddc5b5
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 7 deletions.
4 changes: 4 additions & 0 deletions cmd/spiderpool/cmd/command_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,5 +182,9 @@ func assembleResult(cniVersion, IfName string, ipamResponse *daemonset.PostIpamI
}
}

if len(result.IPs) == 0 {
return nil, fmt.Errorf("no Interface %s IP allocation found", IfName)
}

Check warning on line 187 in cmd/spiderpool/cmd/command_add.go

View check run for this annotation

Codecov / codecov/patch

cmd/spiderpool/cmd/command_add.go#L186-L187

Added lines #L186 - L187 were not covered by tests

return result, nil
}
6 changes: 3 additions & 3 deletions pkg/ipam/pool_selections.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (i *ipam) getPoolCandidates(ctx context.Context, addArgs *models.IpamAddArg

// Select IPPool candidates through the Pod annotation "ipam.spidernet.io/ippools".
if anno, ok := pod.Annotations[constant.AnnoPodIPPools]; ok {
return getPoolFromPodAnnoPools(ctx, anno)
return getPoolFromPodAnnoPools(ctx, anno, *addArgs.IfName)
}

// Select IPPool candidates through the Pod annotation "ipam.spidernet.io/ippool".
Expand Down Expand Up @@ -305,7 +305,7 @@ func (i *ipam) applyThirdControllerAutoPool(ctx context.Context, subnetName stri
return pool, nil
}

func getPoolFromPodAnnoPools(ctx context.Context, anno string) (ToBeAllocateds, error) {
func getPoolFromPodAnnoPools(ctx context.Context, anno, currentNIC string) (ToBeAllocateds, error) {
logger := logutils.FromContext(ctx)
logger.Sugar().Infof("Use IPPools from Pod annotation '%s'", constant.AnnoPodIPPools)

Expand All @@ -317,7 +317,7 @@ func getPoolFromPodAnnoPools(ctx context.Context, anno string) (ToBeAllocateds,
}

// validate and mutate the IPPools annotation value
err = validateAndMutateMultipleNICAnnotations(annoPodIPPools)
err = validateAndMutateMultipleNICAnnotations(annoPodIPPools, currentNIC)
if nil != err {
return nil, fmt.Errorf("%w: %v", errPrefix, err)
}
Expand Down
17 changes: 13 additions & 4 deletions pkg/ipam/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,16 @@ func IsMultipleNicWithNoName(anno map[string]string) bool {
return result
}

func validateAndMutateMultipleNICAnnotations(annoIPPoolsValue types.AnnoPodIPPoolsValue) error {
func validateAndMutateMultipleNICAnnotations(annoIPPoolsValue types.AnnoPodIPPoolsValue, currentNIC string) error {
if len(annoIPPoolsValue) == 0 {
return fmt.Errorf("value requires at least one item")
}

nicSet := map[string]struct{}{}
isEmpty := annoIPPoolsValue[0].NIC == ""
hasNIC := annoIPPoolsValue[0].NIC != ""
for index := range annoIPPoolsValue {
// require all item NIC should be same in specified or unspecified.
if (annoIPPoolsValue[index].NIC == "") != isEmpty {
if (annoIPPoolsValue[index].NIC != "") != hasNIC {
return fmt.Errorf("NIC should be either all specified or all unspecified")
}

Expand All @@ -209,10 +209,19 @@ func validateAndMutateMultipleNICAnnotations(annoIPPoolsValue types.AnnoPodIPPoo
}

// require no NIC name duplicated, this works for multiple NIC specified by the users
if _, ok := nicSet[annoIPPoolsValue[index].NIC]; ok {
if _, ok := nicSet[annoIPPoolsValue[index].NIC]; !ok {
nicSet[annoIPPoolsValue[index].NIC] = struct{}{}
} else {
return fmt.Errorf("duplicate interface %s", annoIPPoolsValue[index].NIC)
}
}

if hasNIC {
_, ok := nicSet[currentNIC]
if !ok {
return fmt.Errorf("interface %s must be specified", currentNIC)
}
}

return nil
}

0 comments on commit cddc5b5

Please sign in to comment.