Skip to content

Commit

Permalink
Fix unit tests that rely on pseudo-random numbers (antrea-io#4148)
Browse files Browse the repository at this point in the history 
Test_ofPacketOutBuilder_Done relies on specific pseudo-random numbers to
be generated and will fail if another test also calls the pseudo-random
number generator. This patch creates a dedicated random number generator
for ofPacketOutBuilder and specifies a hardcoded seed for unit tests to
make test outputs predictable.

Signed-off-by: Quan Tian <qtian@vmware.com>
  • Loading branch information
@tnqn
tnqn committed Aug 25, 2022
1 parent 25ff93d commit 52db699
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 42 deletions.
22 changes: 15 additions & 7 deletions pkg/ovs/openflow/ofctrl_packetout.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"encoding/binary"
"math/rand"
"net"
"time"

"antrea.io/libOpenflow/openflow15"
"antrea.io/libOpenflow/protocol"
Expand All @@ -26,6 +27,9 @@ import (
"k8s.io/klog/v2"
)

// #nosec G404: random number generator not used for security purposes
var pktRand = rand.New(rand.NewSource(time.Now().UnixNano()))

type ofPacketOutBuilder struct {
pktOut *ofctrl.PacketOut
icmpID *uint16
Expand Down Expand Up @@ -341,11 +345,13 @@ func (b *ofPacketOutBuilder) Done() *ofctrl.PacketOut {
b.pktOut.IPHeader.Length = 20 + b.pktOut.ICMPHeader.Len()
} else if b.pktOut.TCPHeader != nil {
b.pktOut.TCPHeader.HdrLen = 5
// #nosec G404: random number generator not used for security purposes
b.pktOut.TCPHeader.SeqNum = rand.Uint32()
if b.pktOut.TCPHeader.SeqNum == 0 {
// #nosec G404: random number generator not used for security purposes
b.pktOut.TCPHeader.SeqNum = pktRand.Uint32()
}
if b.pktOut.TCPHeader.AckNum == 0 {
// #nosec G404: random number generator not used for security purposes
b.pktOut.TCPHeader.AckNum = rand.Uint32()
b.pktOut.TCPHeader.AckNum = pktRand.Uint32()
}
b.pktOut.TCPHeader.Checksum = b.tcpHeaderChecksum()
b.pktOut.IPHeader.Length = 20 + b.pktOut.TCPHeader.Len()
Expand All @@ -368,7 +374,7 @@ func (b *ofPacketOutBuilder) Done() *ofctrl.PacketOut {
}
if b.pktOut.IPHeader.Id == 0 {
// #nosec G404: random number generator not used for security purposes
b.pktOut.IPHeader.Id = uint16(rand.Uint32())
b.pktOut.IPHeader.Id = uint16(pktRand.Uint32())
}
// Set IP version in the IP Header.
b.pktOut.IPHeader.Version = 0x4
Expand All @@ -382,11 +388,13 @@ func (b *ofPacketOutBuilder) Done() *ofctrl.PacketOut {
b.pktOut.IPv6Header.Length = b.pktOut.ICMPHeader.Len()
} else if b.pktOut.TCPHeader != nil {
b.pktOut.TCPHeader.HdrLen = 5
// #nosec G404: random number generator not used for security purposes
b.pktOut.TCPHeader.SeqNum = rand.Uint32()
if b.pktOut.TCPHeader.SeqNum == 0 {
// #nosec G404: random number generator not used for security purposes
b.pktOut.TCPHeader.SeqNum = pktRand.Uint32()
}
if b.pktOut.TCPHeader.AckNum == 0 {
// #nosec G404: random number generator not used for security purposes
b.pktOut.TCPHeader.AckNum = rand.Uint32()
b.pktOut.TCPHeader.AckNum = pktRand.Uint32()
}
b.pktOut.TCPHeader.Checksum = b.tcpHeaderChecksum()
b.pktOut.IPv6Header.Length = b.pktOut.TCPHeader.Len()
Expand Down
52 changes: 17 additions & 35 deletions pkg/ovs/openflow/ofctrl_packetout_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@
package openflow

import (
"math/rand"
"net"
"reflect"
"testing"

"antrea.io/libOpenflow/protocol"
"antrea.io/ofnet/ofctrl"
"github.com/stretchr/testify/assert"
)

func Test_ofPacketOutBuilder_SetSrcIP(t *testing.T) {
Expand Down Expand Up @@ -644,6 +646,7 @@ func Test_ofPacketOutBuilder_Done(t *testing.T) {
Version: 0x4,
Length: 28,
Checksum: 46753,
Id: 1090,
},
ICMPHeader: &protocol.ICMP{
Type: 3,
Expand All @@ -669,13 +672,16 @@ func Test_ofPacketOutBuilder_Done(t *testing.T) {
IPHeader: &protocol.IPv4{
Version: 0x4,
Length: 40,
Checksum: 45409,
Checksum: 8009,
Id: 39822,
},
TCPHeader: &protocol.TCP{
PortSrc: 10000,
PortDst: 10001,
HdrLen: 5,
Checksum: 63286,
Checksum: 40408,
SeqNum: 2596996162,
AckNum: 4039455774,
},
},
},
Expand All @@ -694,7 +700,8 @@ func Test_ofPacketOutBuilder_Done(t *testing.T) {
IPHeader: &protocol.IPv4{
Version: 0x4,
Length: 28,
Checksum: 45025,
Checksum: 46753,
Id: 1090,
},
UDPHeader: &protocol.UDP{
PortSrc: 10000,
Expand Down Expand Up @@ -751,7 +758,9 @@ func Test_ofPacketOutBuilder_Done(t *testing.T) {
PortSrc: 10000,
PortDst: 10001,
HdrLen: 5,
Checksum: 26538,
Checksum: 40408,
SeqNum: 2596996162,
AckNum: 4039455774,
},
},
},
Expand Down Expand Up @@ -782,43 +791,16 @@ func Test_ofPacketOutBuilder_Done(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Specify a hardcoded seed for testing to make output predictable.
// #nosec G404: random number generator not used for security purposes
pktRand = rand.New(rand.NewSource(1))
b := &ofPacketOutBuilder{
pktOut: tt.fields.pktOut,
icmpID: tt.fields.icmpID,
icmpSeq: tt.fields.icmpSeq,
}
got := b.Done()
if got == nil {
if tt.want != nil {
t.Errorf("Done() = %v, want %v", got, tt.want)
}
return
}
if got.IPHeader != nil {
got.IPHeader.Id = 0
}
if got.TCPHeader != nil {
got.TCPHeader.SeqNum = 0
got.TCPHeader.AckNum = 0
}
if !reflect.DeepEqual(got.ICMPHeader, tt.want.ICMPHeader) {
t.Errorf("Done() = %v, want %v", got.ICMPHeader, tt.want.ICMPHeader)
}
if !reflect.DeepEqual(got.TCPHeader, tt.want.TCPHeader) {
t.Errorf("Done() = %+v, want %+v", got.TCPHeader, tt.want.TCPHeader)
}
if !reflect.DeepEqual(got.UDPHeader, tt.want.UDPHeader) {
t.Errorf("Done() = %v, want %v", got.UDPHeader, tt.want.UDPHeader)
}
if !reflect.DeepEqual(got.IPHeader, tt.want.IPHeader) {
t.Errorf("Done() = %v, want %v", got.IPHeader, tt.want.IPHeader)
}
if !reflect.DeepEqual(got.IPv6Header, tt.want.IPv6Header) {
t.Errorf("Done() = %v, want %v", got.IPv6Header, tt.want.IPv6Header)
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Done() = %+v, want %+v", got, tt.want)
}
assert.Equal(t, tt.want, got)
})
}
}

0 comments on commit 52db699

Please sign in to comment.