Skip to content

Commit

Permalink
Decouple applications from topology internals (#3299)
Browse files Browse the repository at this point in the history
This refactor conserves existing semantics, while protecting
applications from changes in the topology file format.

Also, by precisely defining how information can be extracted from the
topology, a lot of duplicate code is removed.
  • Loading branch information
scrye authored Nov 1, 2019
1 parent ab43586 commit 60cd7a0
Show file tree
Hide file tree
Showing 66 changed files with 766 additions and 445 deletions.
4 changes: 3 additions & 1 deletion go/beacon_srv/internal/beaconing/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ go_library(
"//go/lib/ctrl/seg:go_default_library",
"//go/lib/infra:go_default_library",
"//go/lib/infra/messenger:go_default_library",
"//go/lib/infra/modules/itopo:go_default_library",
"//go/lib/infra/modules/segverifier:go_default_library",
"//go/lib/log:go_default_library",
"//go/lib/periodic:go_default_library",
"//go/lib/serrors:go_default_library",
"//go/lib/snet:go_default_library",
"//go/lib/snet/addrutil:go_default_library",
"//go/lib/spath:go_default_library",
"//go/lib/topology:go_default_library",
"//go/lib/util:go_default_library",
"//go/proto:go_default_library",
],
Expand Down Expand Up @@ -63,6 +63,8 @@ go_test(
"//go/lib/ctrl/seg:go_default_library",
"//go/lib/infra:go_default_library",
"//go/lib/infra/mock_infra:go_default_library",
"//go/lib/infra/modules/itopo:go_default_library",
"//go/lib/infra/modules/itopo/itopotest:go_default_library",
"//go/lib/infra/modules/trust:go_default_library",
"//go/lib/log:go_default_library",
"//go/lib/overlay:go_default_library",
Expand Down
19 changes: 10 additions & 9 deletions go/beacon_srv/internal/beaconing/extender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/scionproto/scion/go/lib/ctrl"
"github.com/scionproto/scion/go/lib/ctrl/seg"
"github.com/scionproto/scion/go/lib/infra"
"github.com/scionproto/scion/go/lib/infra/modules/itopo/itopotest"
"github.com/scionproto/scion/go/lib/infra/modules/trust"
"github.com/scionproto/scion/go/lib/scrypto"
"github.com/scionproto/scion/go/lib/spath"
Expand All @@ -41,7 +42,7 @@ import (
)

func TestExtenderExtend(t *testing.T) {
topoProvider := xtest.TopoProviderFromFile(t, topoNonCore)
topoProvider := itopotest.TopoProviderFromFile(t, topoNonCore)
mac, err := scrypto.InitMac(make(common.RawBytes, 16))
xtest.FailOnErr(t, err)
pub, priv, err := scrypto.GenKeyPair(scrypto.Ed25519)
Expand Down Expand Up @@ -112,13 +113,13 @@ func TestExtenderExtend(t *testing.T) {
defer mctrl.Finish()
g := graph.NewDefaultGraph(mctrl)
// Setup interfaces with active parent, child and one peer interface.
intfs := ifstate.NewInterfaces(topoProvider.Get().IFInfoMap, ifstate.Config{})
intfs := ifstate.NewInterfaces(topoProvider.Get().IFInfoMap(), ifstate.Config{})
intfs.Get(graph.If_111_B_120_X).Activate(graph.If_120_X_111_B)
intfs.Get(graph.If_111_A_112_X).Activate(graph.If_112_X_111_A)
intfs.Get(peer).Activate(graph.If_121_X_111_C)
ext, err := ExtenderConf{
MTU: 1337,
Signer: testSigner(t, priv, topoProvider.Get().ISD_AS),
Signer: testSigner(t, priv, topoProvider.Get().IA()),
Mac: mac,
Intfs: intfs,
GetMaxExpTime: maxExpTimeFactory(beacon.DefaultMaxExpTime),
Expand Down Expand Up @@ -155,7 +156,7 @@ func TestExtenderExtend(t *testing.T) {
SoMsg("TRCVer", entry.TrcVer, ShouldEqual, 84)
SoMsg("IfIDSize", entry.IfIDSize, ShouldEqual, DefaultIfidSize)
SoMsg("MTU", entry.MTU, ShouldEqual, 1337)
SoMsg("IA", entry.IA(), ShouldResemble, topoProvider.Get().ISD_AS)
SoMsg("IA", entry.IA(), ShouldResemble, topoProvider.Get().IA())
// Checks that inactive peers are ignored, even when provided.
SoMsg("HopEntries length", len(entry.HopEntries), ShouldEqual, 2)
})
Expand Down Expand Up @@ -184,12 +185,12 @@ func TestExtenderExtend(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
g := graph.NewDefaultGraph(mctrl)
intfs := ifstate.NewInterfaces(topoProvider.Get().IFInfoMap, ifstate.Config{})
intfs := ifstate.NewInterfaces(topoProvider.Get().IFInfoMap(), ifstate.Config{})
xtest.FailOnErr(t, err)
intfs.Get(graph.If_111_B_120_X).Activate(graph.If_120_X_111_B)
ext, err := ExtenderConf{
MTU: 1337,
Signer: testSigner(t, priv, topoProvider.Get().ISD_AS),
Signer: testSigner(t, priv, topoProvider.Get().IA()),
Mac: mac,
GetMaxExpTime: maxExpTimeFactory(1),
Intfs: intfs,
Expand All @@ -207,11 +208,11 @@ func TestExtenderExtend(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
g := graph.NewDefaultGraph(mctrl)
intfs := ifstate.NewInterfaces(topoProvider.Get().IFInfoMap, ifstate.Config{})
intfs := ifstate.NewInterfaces(topoProvider.Get().IFInfoMap(), ifstate.Config{})
xtest.FailOnErr(t, err)
ext, err := ExtenderConf{
MTU: 1337,
Signer: testSigner(t, priv, topoProvider.Get().ISD_AS),
Signer: testSigner(t, priv, topoProvider.Get().IA()),
Mac: mac,
Intfs: intfs,
GetMaxExpTime: maxExpTimeFactory(beacon.DefaultMaxExpTime),
Expand Down Expand Up @@ -252,7 +253,7 @@ func TestExtenderExtend(t *testing.T) {
Src: ctrl.SignSrcDef{
ChainVer: 42,
TRCVer: 84,
IA: topoProvider.Get().ISD_AS,
IA: topoProvider.Get().IA(),
},
Algo: scrypto.Ed25519,
ExpTime: time.Now(),
Expand Down
9 changes: 5 additions & 4 deletions go/beacon_srv/internal/beaconing/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ import (
"github.com/scionproto/scion/go/lib/ctrl/seg"
"github.com/scionproto/scion/go/lib/infra"
"github.com/scionproto/scion/go/lib/infra/mock_infra"
"github.com/scionproto/scion/go/lib/infra/modules/itopo"
"github.com/scionproto/scion/go/lib/infra/modules/itopo/itopotest"
"github.com/scionproto/scion/go/lib/log"
"github.com/scionproto/scion/go/lib/serrors"
"github.com/scionproto/scion/go/lib/snet"
"github.com/scionproto/scion/go/lib/spath"
"github.com/scionproto/scion/go/lib/topology"
"github.com/scionproto/scion/go/lib/xtest"
"github.com/scionproto/scion/go/lib/xtest/graph"
)
Expand All @@ -59,7 +60,7 @@ func TestNewHandler(t *testing.T) {
pseg := testBeacon(g, []common.IFIDType{graph.If_220_X_120_B, graph.If_120_A_110_X}).Segment
rw := mock_infra.NewMockResponseWriter(mctrl)
rw.EXPECT().SendAckReply(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
topoProvider := xtest.TopoProviderFromFile(t, topoCore)
topoProvider := itopotest.TopoProviderFromFile(t, topoCore)
t.Run("Correct beacon is inserted", func(t *testing.T) {
inserter := mock_beaconing.NewMockBeaconInserter(mctrl)
expectedBeacon := beacon.Beacon{Segment: pseg, InIfId: localIF}
Expand Down Expand Up @@ -222,8 +223,8 @@ func testPath(ingressIfid common.IFIDType) *spath.Path {
return path
}

func testInterfaces(topo *topology.Topo) *ifstate.Interfaces {
intfs := ifstate.NewInterfaces(topo.IFInfoMap, ifstate.Config{})
func testInterfaces(topo itopo.Topology) *ifstate.Interfaces {
intfs := ifstate.NewInterfaces(topo.IFInfoMap(), ifstate.Config{})
intfs.Get(graph.If_110_X_120_A).Activate(graph.If_120_A_110_X)
return intfs
}
15 changes: 8 additions & 7 deletions go/beacon_srv/internal/beaconing/originator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/scionproto/scion/go/lib/common"
"github.com/scionproto/scion/go/lib/ctrl"
"github.com/scionproto/scion/go/lib/ctrl/seg"
"github.com/scionproto/scion/go/lib/infra/modules/itopo/itopotest"
"github.com/scionproto/scion/go/lib/overlay"
"github.com/scionproto/scion/go/lib/scrypto"
"github.com/scionproto/scion/go/lib/snet"
Expand All @@ -47,20 +48,20 @@ const (
)

func TestOriginatorRun(t *testing.T) {
topoProvider := xtest.TopoProviderFromFile(t, topoCore)
topoProvider := itopotest.TopoProviderFromFile(t, topoCore)
mac, err := scrypto.InitMac(make(common.RawBytes, 16))
xtest.FailOnErr(t, err)
pub, priv, err := scrypto.GenKeyPair(scrypto.Ed25519)
xtest.FailOnErr(t, err)
signer := testSigner(t, priv, topoProvider.Get().ISD_AS)
signer := testSigner(t, priv, topoProvider.Get().IA())
Convey("Run originates ifid packets on all active core and child interfaces", t, func() {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
intfs := ifstate.NewInterfaces(topoProvider.Get().IFInfoMap, ifstate.Config{})
intfs := ifstate.NewInterfaces(topoProvider.Get().IFInfoMap(), ifstate.Config{})
conn := mock_snet.NewMockPacketConn(mctrl)
o, err := OriginatorConf{
Config: ExtenderConf{
MTU: uint16(topoProvider.Get().MTU),
MTU: topoProvider.Get().MTU(),
Signer: signer,
Intfs: intfs,
Mac: mac,
Expand Down Expand Up @@ -101,7 +102,7 @@ func TestOriginatorRun(t *testing.T) {
o.Run(nil)
for i, msg := range msgs {
Convey(fmt.Sprintf("Packet %d is correct", i), func() {
checkMsg(t, msg, pub, topoProvider.Get().IFInfoMap)
checkMsg(t, msg, pub, topoProvider.Get().IFInfoMap())
})
}
// The second run should not cause any beacons to originate.
Expand All @@ -110,11 +111,11 @@ func TestOriginatorRun(t *testing.T) {
Convey("Fast recovery", t, func() {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
intfs := ifstate.NewInterfaces(topoProvider.Get().IFInfoMap, ifstate.Config{})
intfs := ifstate.NewInterfaces(topoProvider.Get().IFInfoMap(), ifstate.Config{})
conn := mock_snet.NewMockPacketConn(mctrl)
o, err := OriginatorConf{
Config: ExtenderConf{
MTU: uint16(topoProvider.Get().MTU),
MTU: topoProvider.Get().MTU(),
Signer: signer,
Intfs: intfs,
Mac: mac,
Expand Down
23 changes: 12 additions & 11 deletions go/beacon_srv/internal/beaconing/propagator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/scionproto/scion/go/beacon_srv/internal/onehop"
"github.com/scionproto/scion/go/lib/addr"
"github.com/scionproto/scion/go/lib/common"
"github.com/scionproto/scion/go/lib/infra/modules/itopo/itopotest"
"github.com/scionproto/scion/go/lib/overlay"
"github.com/scionproto/scion/go/lib/scrypto"
"github.com/scionproto/scion/go/lib/snet"
Expand Down Expand Up @@ -141,24 +142,24 @@ func TestPropagatorRun(t *testing.T) {
Convey(test.name, t, func() {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
topoProvider := xtest.TopoProviderFromFile(t, topoFile[test.core])
topoProvider := itopotest.TopoProviderFromFile(t, topoFile[test.core])
provider := mock_beaconing.NewMockBeaconProvider(mctrl)
conn := mock_snet.NewMockPacketConn(mctrl)
cfg := PropagatorConf{
Config: ExtenderConf{
Signer: testSigner(t, priv, topoProvider.Get().ISD_AS),
Signer: testSigner(t, priv, topoProvider.Get().IA()),
Mac: macProp,
Intfs: ifstate.NewInterfaces(topoProvider.Get().IFInfoMap,
Intfs: ifstate.NewInterfaces(topoProvider.Get().IFInfoMap(),
ifstate.Config{}),
MTU: uint16(topoProvider.Get().MTU),
MTU: topoProvider.Get().MTU(),
GetMaxExpTime: maxExpTimeFactory(beacon.DefaultMaxExpTime),
},
Period: time.Hour,
BeaconProvider: provider,
Core: test.core,
BeaconSender: &onehop.BeaconSender{
Sender: onehop.Sender{
IA: topoProvider.Get().ISD_AS,
IA: topoProvider.Get().IA(),
Conn: conn,
Addr: &addr.AppAddr{
L3: addr.HostFromIPStr("127.0.0.1"),
Expand Down Expand Up @@ -203,7 +204,7 @@ func TestPropagatorRun(t *testing.T) {
p.Run(nil)
for i, msg := range msgs {
Convey(fmt.Sprintf("Packet %d is correct", i), func() {
checkMsg(t, msg, pub, topoProvider.Get().IFInfoMap)
checkMsg(t, msg, pub, topoProvider.Get().IFInfoMap())
})
}
// Check that no beacons are sent, since the period has not passed yet.
Expand All @@ -213,24 +214,24 @@ func TestPropagatorRun(t *testing.T) {
Convey("Fast recovery", t, func() {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
topoProvider := xtest.TopoProviderFromFile(t, topoCore)
topoProvider := itopotest.TopoProviderFromFile(t, topoCore)
provider := mock_beaconing.NewMockBeaconProvider(mctrl)
conn := mock_snet.NewMockPacketConn(mctrl)
cfg := PropagatorConf{
Config: ExtenderConf{
Signer: testSigner(t, priv, topoProvider.Get().ISD_AS),
Signer: testSigner(t, priv, topoProvider.Get().IA()),
Mac: macProp,
Intfs: ifstate.NewInterfaces(topoProvider.Get().IFInfoMap,
Intfs: ifstate.NewInterfaces(topoProvider.Get().IFInfoMap(),
ifstate.Config{}),
MTU: uint16(topoProvider.Get().MTU),
MTU: uint16(topoProvider.Get().MTU()),
GetMaxExpTime: maxExpTimeFactory(beacon.DefaultMaxExpTime),
},
Period: 2 * time.Second,
BeaconProvider: provider,
Core: true,
BeaconSender: &onehop.BeaconSender{
Sender: onehop.Sender{
IA: topoProvider.Get().ISD_AS,
IA: topoProvider.Get().IA(),
Conn: conn,
Addr: &addr.AppAddr{
L3: addr.HostFromIPStr("127.0.0.1"),
Expand Down
8 changes: 4 additions & 4 deletions go/beacon_srv/internal/beaconing/registrar.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ import (
"github.com/scionproto/scion/go/lib/ctrl/seg"
"github.com/scionproto/scion/go/lib/infra"
"github.com/scionproto/scion/go/lib/infra/messenger"
"github.com/scionproto/scion/go/lib/infra/modules/itopo"
"github.com/scionproto/scion/go/lib/log"
"github.com/scionproto/scion/go/lib/periodic"
"github.com/scionproto/scion/go/lib/snet"
"github.com/scionproto/scion/go/lib/snet/addrutil"
"github.com/scionproto/scion/go/lib/topology"
"github.com/scionproto/scion/go/proto"
)

Expand All @@ -48,7 +48,7 @@ var _ periodic.Task = (*Registrar)(nil)
type RegistrarConf struct {
Config ExtenderConf
SegProvider SegmentProvider
TopoProvider topology.Provider
TopoProvider itopo.ProviderI
Msgr infra.Messenger
Period time.Duration
SegType proto.PathSegType
Expand All @@ -61,7 +61,7 @@ type Registrar struct {
*segExtender
msgr infra.Messenger
segProvider SegmentProvider
topoProvider topology.Provider
topoProvider itopo.ProviderI
segType proto.PathSegType

// mutable fields
Expand Down Expand Up @@ -247,7 +247,7 @@ func (r *segmentRegistrar) onSuccess() {
func (r *segmentRegistrar) chooseServer(pseg *seg.PathSegment) (net.Addr, error) {
if r.segType != proto.PathSegType_down {
topo := r.topoProvider.Get()
return &snet.Addr{IA: topo.ISD_AS, Host: addr.NewSVCUDPAppAddr(addr.SvcPS)}, nil
return &snet.Addr{IA: topo.IA(), Host: addr.NewSVCUDPAppAddr(addr.SvcPS)}, nil
}
return addrutil.GetPath(addr.SvcPS, pseg, r.topoProvider)
}
Loading

0 comments on commit 60cd7a0

Please sign in to comment.