fix(context): an allocator with no pool refuses the work instead of panicking

context/db_test.go

@@ -273,3 +273,56 @@ func TestSMContextDBRoundTrip_PreservesFieldsAcrossSerializerMigration(t *testin
 		t.Errorf("expected CommunicationClient to be dropped across the DB round trip, got %+v", recovered.CommunicationClient)
 	}
 }
+
+// Releasing a session restored from MongoDB used to panic: the allocator it carries has no pool.
+// The address cannot be returned to the running configuration's pool, but the release itself has
+// to finish, and the session's own address is cleared either way.
+func TestReleaseUeIpAddr_OnRestoredContext(t *testing.T) {
+	allocator, err := NewIPAllocator("192.168.100.0/24")
+	if err != nil {
+		t.Fatalf("NewIPAllocator: %v", err)
+	}
+	ip, err := allocator.Allocate(testSupi)
+	if err != nil {
+		t.Fatalf("Allocate: %v", err)
+	}
+
+	original := &SMContext{
+		Ref:        "restored-release-ref",
+		Supi:       testSupi,
+		Dnn:        "restored-release-dnn",
+		PDUAddress: &UeIpAddr{Ip: ip},
+		DNNInfo:    &SnssaiSmfDnnInfo{UeIPAllocator: allocator, MTU: 1400},
+		AnType:     models.ACCESSTYPE__3_GPP_ACCESS,
+		// Real values for the same reason as the round-trip test above: AccessType, NfStatus and
+		// SscModes.DefaultSscMode reject "" in their own UnmarshalJSON.
+		AMFProfile: models.NFProfileDiscovery{
+			NfInstanceId: "amf-instance-1",
+			NfType:       models.NFTYPE_AMF,
+			NfStatus:     models.NFSTATUS_REGISTERED,
+		},
+		SelectedPCFProfile: models.NFProfileDiscovery{
+			NfInstanceId: "pcf-instance-1",
+			NfType:       models.NFTYPE_PCF,
+			NfStatus:     models.NFSTATUS_REGISTERED,
+		},
+		DnnConfiguration: models.DnnConfiguration{
+			SscModes: models.SscModes{DefaultSscMode: models.SSCMODE_SSC_MODE_1},
+		},
+	}
+
+	recovered := &SMContext{}
+	if err := recovered.UnmarshalJSON(mapToByte(ToBsonM(original))); err != nil {
+		t.Fatalf("round-trip unmarshal failed: %v", err)
+	}
+	if recovered.DNNInfo == nil || recovered.DNNInfo.UeIPAllocator == nil {
+		t.Fatalf("DNNInfo did not survive the round trip: %+v", recovered.DNNInfo)
+	}
+
+	if err := recovered.ReleaseUeIpAddr(); err != nil {
+		t.Errorf("ReleaseUeIpAddr: %v", err)
+	}
+	if got := recovered.PDUAddress.Ip; !got.Equal(net.IPv4zero) {
+		t.Errorf("PDUAddress.Ip = %v, want the 0.0.0.0 sentinel", got)
+	}
+}

context/ip_allocator.go

@@ -14,6 +14,12 @@ import (
 	"github.com/omec-project/smf/logger"
 )
 
+// IPAllocator hands out the UE addresses of one DNN's pool.
+//
+// A method can be reached on an allocator that has no pool: buildSnssaiSmfInfo stores a DNN whose
+// ipDomain carries no usable ueSubnet with UeIPAllocator nil, and an SMContext read back from
+// MongoDB carries an allocator that marshalled as {}, the type having only unexported fields.
+// Each method answers such a receiver instead of dereferencing it.
 type IPAllocator struct {
 	ipNetwork *net.IPNet
 	g         *_IDPool
@@ -89,6 +95,11 @@ func (a *IPAllocator) StaticIpForImsi(imsi string) net.IP {
 
 // Allocate will allocate the IP address and returns it
 func (a *IPAllocator) Allocate(imsi string) (net.IP, error) {
+	// No pool, no addresses; the caller answers this error with IpAllocError.
+	if a == nil || a.g == nil || a.ipNetwork == nil {
+		return nil, errors.New("ip allocation failed: this DNN has no usable UE subnet")
+	}
+
 	// check if static IP already reserved for this IMSI
 	if ip := a.StaticIpForImsi(imsi); ip != nil {
 		return ip, nil
@@ -110,6 +121,10 @@ func (a *IPAllocator) Allocate(imsi string) (net.IP, error) {
 }
 
 func (a *IPAllocator) ReserveStaticIps(ips *map[string]string) {
+	if a == nil || a.g == nil || a.ipNetwork == nil {
+		return
+	}
+
 	a.g.staticIps = ips
 	for _, ipStr := range *ips {
 		if ip := net.ParseIP(ipStr).To4(); ip != nil {
@@ -120,11 +135,24 @@ func (a *IPAllocator) ReserveStaticIps(ips *map[string]string) {
 }
 
 func (a *IPAllocator) BlockIp(ip net.IP) {
+	if a == nil || a.g == nil || a.ipNetwork == nil {
+		return
+	}
+
 	offset := IPAddrOffset(ip, a.ipNetwork.IP)
 	a.g.block(int64(offset))
 }
 
 func (a *IPAllocator) Release(imsi string, ip net.IP) {
+	// There is no pool to return the address to, so the release is skipped and logged rather
+	// than passed over. Reconciling the running pool with restored sessions is out of scope.
+	if a == nil || a.g == nil || a.ipNetwork == nil {
+		logger.CtxLog.Warnf("cannot release UE address %v for %s: this allocator has no pool, "+
+			"so the release is skipped", ip, imsi)
+
+		return
+	}
+
 	// Don't release static IPs
 	if a.StaticIpForImsi(imsi) != nil {
 		return

context/ip_allocator_test.go

@@ -169,3 +169,29 @@ func TestAReleasedAddressIsAllocatedAgainAfterRestarts(t *testing.T) {
 		t.Errorf("allocation after %v was released: %v", first, err)
 	}
 }
+
+// Every method of an allocator with no pool panicked before; see IPAllocator for the two ways a
+// caller comes to hold one.
+func TestIPAllocatorWithNoPool(t *testing.T) {
+	const imsi = "imsi-208930100007487"
+
+	for name, allocator := range map[string]*smf_context.IPAllocator{
+		"nil allocator":      nil,
+		"restored allocator": {},
+	} {
+		t.Run(name, func(t *testing.T) {
+			ip, err := allocator.Allocate(imsi)
+			if err == nil {
+				t.Errorf("Allocate returned %v and no error", ip)
+			}
+			if ip != nil {
+				t.Errorf("Allocate returned address %v alongside its error", ip)
+			}
+
+			allocator.Release(imsi, net.ParseIP("192.168.1.5"))
+			allocator.BlockIp(net.ParseIP("192.168.1.6"))
+			static := map[string]string{imsi: "192.168.1.7"}
+			allocator.ReserveStaticIps(&static)
+		})
+	}
+}