Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: uppercase ip address is not recommended #4372

Merged
merged 3 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 27 additions & 14 deletions pkg/controller/ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,6 @@ func (c *Controller) enqueueUpdateIP(oldObj, newObj interface{}) {
}
oldIP := oldObj.(*kubeovnv1.IP)
newIP := newObj.(*kubeovnv1.IP)
if !newIP.DeletionTimestamp.IsZero() {
klog.V(3).Infof("enqueue update ip %s", key)
c.updateIPQueue.Add(key)
return
}
if !reflect.DeepEqual(oldIP.Spec.AttachSubnets, newIP.Spec.AttachSubnets) {
klog.V(3).Infof("enqueue update status subnet %s", newIP.Spec.Subnet)
for _, as := range newIP.Spec.AttachSubnets {
klog.V(3).Infof("enqueue update status for attach subnet %s", as)
c.updateSubnetStatusQueue.Add(as)
}
}
// ip can not change these specs below
if oldIP.Spec.Subnet != "" && newIP.Spec.Subnet != oldIP.Spec.Subnet {
klog.Errorf("ip %s subnet can not change", newIP.Name)
Expand All @@ -85,8 +73,27 @@ func (c *Controller) enqueueUpdateIP(oldObj, newObj interface{}) {
if oldIP.Spec.V4IPAddress != "" && newIP.Spec.V4IPAddress != oldIP.Spec.V4IPAddress {
klog.Errorf("ip %s v4IPAddress can not change", newIP.Name)
}
if oldIP.Spec.V6IPAddress != "" && newIP.Spec.V6IPAddress != oldIP.Spec.V6IPAddress {
klog.Errorf("ip %s v6IPAddress can not change", newIP.Name)
if oldIP.Spec.V6IPAddress != "" {
// v6 ip address can not use upper case
if util.ContainsUppercase(newIP.Spec.V6IPAddress) {
err := fmt.Errorf("ip %s v6 ip address %s can not contain upper case", newIP.Name, newIP.Spec.V6IPAddress)
klog.Error(err)
}
if newIP.Spec.V6IPAddress != oldIP.Spec.V6IPAddress {
klog.Errorf("ip %s v6IPAddress can not change", newIP.Name)
}
}
if !newIP.DeletionTimestamp.IsZero() {
klog.V(3).Infof("enqueue update ip %s", key)
c.updateIPQueue.Add(key)
return
}
if !reflect.DeepEqual(oldIP.Spec.AttachSubnets, newIP.Spec.AttachSubnets) {
klog.V(3).Infof("enqueue update status subnet %s", newIP.Spec.Subnet)
for _, as := range newIP.Spec.AttachSubnets {
klog.V(3).Infof("enqueue update status for attach subnet %s", as)
c.updateSubnetStatusQueue.Add(as)
}
}
}

Expand Down Expand Up @@ -254,6 +261,12 @@ func (c *Controller) handleAddReservedIP(key string) error {
return nil
}

// v6 ip address can not use upper case
if util.ContainsUppercase(ip.Spec.V6IPAddress) {
err := fmt.Errorf("ip %s v6 ip address %s can not contain upper case", ip.Name, ip.Spec.V6IPAddress)
klog.Error(err)
return err
}
v4IP, v6IP, mac, err := c.ipAcquireAddress(ip, subnet)
if err != nil {
err = fmt.Errorf("failed to acquire ip address %w", err)
Expand Down
12 changes: 12 additions & 0 deletions pkg/controller/ovn_eip.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,12 @@ func (c *Controller) handleAddOvnEip(key string) error {
klog.Errorf("failed to get external subnet, %v", err)
return err
}
// v6 ip address can not use upper case
if util.ContainsUppercase(cachedEip.Spec.V6Ip) {
err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", cachedEip.Name, cachedEip.Spec.V6Ip)
klog.Error(err)
return err
}
portName := cachedEip.Name
if cachedEip.Spec.V4Ip != "" {
v4ip, v6ip, mac, err = c.acquireStaticIPAddress(subnet.Name, cachedEip.Name, portName, cachedEip.Spec.V4Ip)
Expand Down Expand Up @@ -298,6 +304,12 @@ func (c *Controller) handleUpdateOvnEip(key string) error {
klog.Error(err)
return err
}
// v6 ip address can not use upper case
if util.ContainsUppercase(cachedEip.Spec.V6Ip) {
err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", cachedEip.Name, cachedEip.Spec.V6Ip)
klog.Error(err)
return err
}
if cachedEip.Status.V6Ip != cachedEip.Spec.V6Ip {
err := fmt.Errorf("not support change v6 ip for ovn eip %s", cachedEip.Name)
klog.Error(err)
Expand Down
12 changes: 12 additions & 0 deletions pkg/controller/vip.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,12 @@ func (c *Controller) handleAddVirtualIP(key string) error {
portName := ovs.PodNameToPortName(vip.Name, vip.Spec.Namespace, subnet.Spec.Provider)
sourceV4Ip = vip.Spec.V4ip
sourceV6Ip = vip.Spec.V6ip
// v6 ip address can not use upper case
if util.ContainsUppercase(vip.Spec.V6ip) {
err := fmt.Errorf("vip %s v6 ip address %s can not contain upper case", vip.Name, vip.Spec.V6ip)
klog.Error(err)
return err
}
ipStr := util.GetStringIP(sourceV4Ip, sourceV6Ip)
if ipStr != "" {
v4ip, v6ip, mac, err = c.acquireStaticIPAddress(subnet.Name, vip.Name, portName, ipStr)
Expand Down Expand Up @@ -313,6 +319,12 @@ func (c *Controller) handleUpdateVirtualIP(key string) error {
}
return nil
}
// v6 ip address can not use upper case
if util.ContainsUppercase(vip.Spec.V6ip) {
err := fmt.Errorf("vip %s v6 ip address %s can not contain upper case", vip.Name, vip.Spec.V6ip)
klog.Error(err)
return err
}
// not support change
if vip.Status.Mac != "" && vip.Status.Mac != vip.Spec.MacAddress {
err = errors.New("not support change mac of vip")
Expand Down
12 changes: 12 additions & 0 deletions pkg/controller/vpc_nat_gw_eip.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,12 @@ func (c *Controller) handleAddIptablesEip(key string) error {
return err
}

// v6 ip address can not use upper case
if util.ContainsUppercase(cachedEip.Spec.V6ip) {
err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", cachedEip.Name, cachedEip.Spec.V6ip)
klog.Error(err)
return err
}
var v4ip, v6ip, mac string
portName := ovs.PodNameToPortName(cachedEip.Name, cachedEip.Namespace, subnet.Spec.Provider)
if cachedEip.Spec.V4ip != "" {
Expand Down Expand Up @@ -357,6 +363,12 @@ func (c *Controller) handleUpdateIptablesEip(key string) error {
return nil
}
klog.Infof("handle update eip %s", key)
// v6 ip address can not use upper case
if util.ContainsUppercase(cachedEip.Spec.V6ip) {
err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", cachedEip.Name, cachedEip.Spec.V6ip)
klog.Error(err)
return err
}
// eip change ip
if c.eipChangeIP(cachedEip) {
err := fmt.Errorf("not support eip change ip, old ip '%s', new ip '%s'", cachedEip.Status.IP, cachedEip.Spec.V4ip)
Expand Down
10 changes: 10 additions & 0 deletions pkg/util/net.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"strconv"
"strings"
"time"
"unicode"

"k8s.io/klog/v2"

Expand Down Expand Up @@ -656,3 +657,12 @@ func GetDefaultListenAddr() string {
}
return "0.0.0.0"
}

func ContainsUppercase(s string) bool {
for _, char := range s {
if unicode.IsUpper(char) {
return true
}
}
return false
}
35 changes: 35 additions & 0 deletions pkg/util/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ import (

func ValidateSubnet(subnet kubeovnv1.Subnet) error {
if subnet.Spec.Gateway != "" {
// v6 ip address can not use upper case
if ContainsUppercase(subnet.Spec.Gateway) {
err := fmt.Errorf("subnet gateway %s v6 ip address can not contain upper case", subnet.Spec.Gateway)
return err
}
if !CIDRContainIP(subnet.Spec.CIDRBlock, subnet.Spec.Gateway) {
return fmt.Errorf("gateway %s is not in cidr %s", subnet.Spec.Gateway, subnet.Spec.CIDRBlock)
}
Expand All @@ -31,6 +36,11 @@ func ValidateSubnet(subnet kubeovnv1.Subnet) error {
}
excludeIps := subnet.Spec.ExcludeIps
for _, ipr := range excludeIps {
// v6 ip address can not use upper case
if ContainsUppercase(ipr) {
err := fmt.Errorf("subnet exclude ip %s can not contain upper case", ipr)
return err
}
ips := strings.Split(ipr, "..")
if len(ips) > 2 {
return fmt.Errorf("%s in excludeIps is not a valid ip range", ipr)
Expand All @@ -55,13 +65,23 @@ func ValidateSubnet(subnet kubeovnv1.Subnet) error {
}

for _, cidr := range strings.Split(subnet.Spec.CIDRBlock, ",") {
// v6 ip address can not use upper case
if ContainsUppercase(subnet.Spec.CIDRBlock) {
err := fmt.Errorf("subnet cidr block %s v6 ip address can not contain upper case", subnet.Spec.Gateway)
return err
}
if _, _, err := net.ParseCIDR(cidr); err != nil {
return fmt.Errorf("subnet %s cidr %s is invalid", subnet.Name, cidr)
}
}

allow := subnet.Spec.AllowSubnets
for _, cidr := range allow {
// v6 ip address can not use upper case
if ContainsUppercase(subnet.Spec.CIDRBlock) {
err := fmt.Errorf("subnet %s allow subnet %s v6 ip address can not contain upper case", subnet.Name, cidr)
return err
}
if _, _, err := net.ParseCIDR(cidr); err != nil {
return fmt.Errorf("%s in allowSubnets is not a valid address", cidr)
}
Expand Down Expand Up @@ -90,6 +110,11 @@ func ValidateSubnet(subnet kubeovnv1.Subnet) error {
if subnet.Spec.NatOutgoing {
return errors.New("conflict configuration: natOutgoing and externalEgressGateway")
}
// v6 ip address can not use upper case
if ContainsUppercase(egw) {
err := fmt.Errorf("subnet %s external egress gateway %s v6 ip address can not contain upper case", subnet.Name, egw)
return err
}
ips := strings.Split(egw, ",")
if len(ips) > 2 {
return errors.New("invalid external egress gateway configuration")
Expand All @@ -107,6 +132,11 @@ func ValidateSubnet(subnet kubeovnv1.Subnet) error {

if len(subnet.Spec.Vips) != 0 {
for _, vip := range subnet.Spec.Vips {
// v6 ip address can not use upper case
if ContainsUppercase(vip) {
err := fmt.Errorf("subnet %s vips %s v6 ip address can not contain upper case", subnet.Name, vip)
return err
}
if !CIDRContainIP(subnet.Spec.CIDRBlock, vip) {
return fmt.Errorf("vip %s conflicts with subnet %s cidr %s", vip, subnet.Name, subnet.Spec.CIDRBlock)
}
Expand All @@ -124,6 +154,11 @@ func ValidateSubnet(subnet kubeovnv1.Subnet) error {
}

if subnet.Spec.U2OInterconnectionIP != "" {
// v6 ip address can not use upper case
if ContainsUppercase(subnet.Spec.U2OInterconnectionIP) {
err := fmt.Errorf("subnet %s U2O interconnection ip %s v6 ip address can not contain upper case", subnet.Name, subnet.Spec.U2OInterconnectionIP)
return err
}
if !CIDRContainIP(subnet.Spec.CIDRBlock, subnet.Spec.U2OInterconnectionIP) {
return fmt.Errorf("u2oInterconnectionIP %s is not in subnet %s cidr %s",
subnet.Spec.U2OInterconnectionIP,
Expand Down
7 changes: 7 additions & 0 deletions pkg/webhook/ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func (v *ValidatingHook) IPUpdateHook(ctx context.Context, req admission.Request
err := fmt.Errorf("ip %s v4IPAddress can not change", ipNew.Name)
return ctrlwebhook.Errored(http.StatusBadRequest, err)
}

if ipOld.Spec.V6IPAddress != "" && ipNew.Spec.V6IPAddress != ipOld.Spec.V6IPAddress {
err := fmt.Errorf("ip %s v6IPAddress can not change", ipNew.Name)
return ctrlwebhook.Errored(http.StatusBadRequest, err)
Expand Down Expand Up @@ -102,6 +103,12 @@ func (v *ValidatingHook) ValidateIP(ctx context.Context, ip *ovnv1.IP) error {
}

if ip.Spec.V6IPAddress != "" {
// v6 ip address can not use upper case
if util.ContainsUppercase(ip.Spec.V6IPAddress) {
err := fmt.Errorf("ip %s v6 ip address can not contain upper case", ip.Name)
return err
}

if net.ParseIP(ip.Spec.V6IPAddress) == nil {
err := fmt.Errorf("%s is not a valid ip", ip.Spec.V6IPAddress)
return err
Expand Down
6 changes: 6 additions & 0 deletions pkg/webhook/ovn_nat_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,12 @@ func (v *ValidatingHook) ValidateOvnEip(ctx context.Context, eip *ovnv1.OvnEip)
}

if eip.Spec.V6Ip != "" {
// v6 ip address can not use upper case
if util.ContainsUppercase(eip.Spec.V6Ip) {
err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", eip.Name, eip.Spec.V6Ip)
klog.Error(err)
return err
}
if net.ParseIP(eip.Spec.V6Ip) == nil {
err := fmt.Errorf("spec v6ip %s is not a valid", eip.Spec.V6Ip)
return err
Expand Down
6 changes: 6 additions & 0 deletions pkg/webhook/static_ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,12 @@ func (v *ValidatingHook) checkIPConflict(ipAddress, annoSubnet, name string, ipL
}

v4IP, v6IP := util.SplitStringIP(ipCR.Spec.IPAddress)
// v6 ip address can not use upper case
if util.ContainsUppercase(v6IP) {
err := fmt.Errorf("v6 ip address %s can not contain upper case", v6IP)
klog.Error(err)
return err
}
if ipAddr.String() == v4IP || ipAddr.String() == v6IP {
if name == ipCR.Spec.PodName {
klog.Infof("get same ip crd for %s", name)
Expand Down
5 changes: 5 additions & 0 deletions pkg/webhook/vip.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ func (v *ValidatingHook) ValidateVip(ctx context.Context, vip *ovnv1.Vip) error
}

if vip.Spec.V6ip != "" {
// v6 ip address can not use upper case
if util.ContainsUppercase(vip.Spec.V6ip) {
err := fmt.Errorf("vip %s v6 ip address %s can not contain upper case", vip.Name, vip.Spec.V6ip)
return err
}
if net.ParseIP(vip.Spec.V6ip) == nil {
err := fmt.Errorf("%s is not a valid ip", vip.Spec.V6ip)
return err
Expand Down
5 changes: 5 additions & 0 deletions pkg/webhook/vpc_nat_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,11 @@ func (v *ValidatingHook) ValidateIptablesEIP(ctx context.Context, eip *ovnv1.Ipt
}

if eip.Spec.V6ip != "" {
// v6 ip address can not use upper case
if util.ContainsUppercase(eip.Spec.V6ip) {
err := fmt.Errorf("eip %s v6 ip address %s can not contain upper case", eip.Name, eip.Spec.V6ip)
return err
}
if net.ParseIP(eip.Spec.V6ip) == nil {
err := fmt.Errorf("v6ip %s is not a valid", eip.Spec.V6ip)
return err
Expand Down
23 changes: 23 additions & 0 deletions test/e2e/vip/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ var _ = framework.Describe("[group:vip]", func() {
// test allowed address pair vip
var countingVipName, vip1Name, vip2Name, aapPodName1, aapPodName2, aapPodName3 string

// test ipv6 vip
var lowerCaseStaticIpv6VipName, upperCaseStaticIpv6VipName, lowerCaseV6IP, upperCaseV6IP string

// test allowed address pair connectivity in the security group scenario
var securityGroupName string

Expand All @@ -136,6 +139,13 @@ var _ = framework.Describe("[group:vip]", func() {
namespaceName = f.Namespace.Name
cidr = framework.RandomCIDR(f.ClusterIPFamily)

// should create lower case static ipv6 address vip in ovn-default
lowerCaseStaticIpv6VipName = "lower-case-static-ipv6-vip-" + framework.RandomSuffix()
lowerCaseV6IP = "fd00:10:16::a1"
// should not create upper case static ipv6 address vip in ovn-default
upperCaseStaticIpv6VipName = "Upper-Case-Static-Ipv6-Vip-" + framework.RandomSuffix()
upperCaseV6IP = "fd00:10:16::A1"

// should have the same mac, which mac is the same as its vpc overlay subnet gw mac
randomSuffix := framework.RandomSuffix()
switchLbVip1Name = "switch-lb-vip1-" + randomSuffix
Expand Down Expand Up @@ -226,6 +236,19 @@ var _ = framework.Describe("[group:vip]", func() {
framework.ExpectNotEqual(oldSubnet.Status.V6UsingIPRange, newSubnet.Status.V6UsingIPRange)
}
ginkgo.By("1. Test allowed address pair vip")
if f.IsIPv6() {
ginkgo.By("Should create lower case static ipv6 address vip " + lowerCaseStaticIpv6VipName)
lowerCaseStaticIpv6Vip := makeOvnVip(namespaceName, lowerCaseStaticIpv6VipName, util.DefaultSubnet, "", lowerCaseV6IP, "")
lowerCaseStaticIpv6Vip = vipClient.CreateSync(lowerCaseStaticIpv6Vip)
framework.ExpectEqual(lowerCaseStaticIpv6Vip.Status.V6ip, lowerCaseV6IP)
ginkgo.By("Should not create upper case static ipv6 address vip " + upperCaseStaticIpv6VipName)
upperCaseStaticIpv6Vip := makeOvnVip(namespaceName, upperCaseStaticIpv6VipName, util.DefaultSubnet, "", upperCaseV6IP, "")
_ = vipClient.Create(upperCaseStaticIpv6Vip)
time.Sleep(10 * time.Second)
upperCaseStaticIpv6Vip = vipClient.Get(upperCaseStaticIpv6VipName)
framework.ExpectEqual(upperCaseStaticIpv6Vip.Status.V6ip, "")
framework.ExpectFalse(upperCaseStaticIpv6Vip.Status.Ready)
}
// create vip1 and vip2, should have different ip and mac
ginkgo.By("Creating allowed address pair vip, should have different ip and mac")
ginkgo.By("Creating allowed address pair vip " + vip1Name)
Expand Down
Loading