From cc3b623d1e50f202d8a48ab0d1818f55ad38653b Mon Sep 17 00:00:00 2001 From: Lukas Vogel Date: Wed, 25 Nov 2020 10:11:00 +0100 Subject: [PATCH] hp: add topology support Adds support to specify hidden segment services in the topology file. It also implements the discovery support and adds minimal prodspec support. --- go/lib/topology/interface.go | 14 ++- go/lib/topology/json/json.go | 12 +-- go/lib/topology/mock_topology/mock.go | 5 +- go/lib/topology/servicetype.go | 10 +++ go/lib/topology/servicetype_test.go | 2 + go/lib/topology/topology.go | 50 +++++++---- go/pkg/discovery/BUILD.bazel | 1 + go/pkg/discovery/testdata/topology.json | 2 +- go/pkg/discovery/toposervice.go | 30 ++++++- go/pkg/discovery/toposervice_test.go | 115 ++++++++++++++++++++++-- go/pkg/sciond/internal/servers/grpc.go | 6 +- 11 files changed, 207 insertions(+), 40 deletions(-) diff --git a/go/lib/topology/interface.go b/go/lib/topology/interface.go index e4bd0f228a..ddbfd18f9c 100644 --- a/go/lib/topology/interface.go +++ b/go/lib/topology/interface.go @@ -87,7 +87,7 @@ type Topology interface { UnderlayNextHop(ifID common.IFIDType) (*net.UDPAddr, bool) // MakeHostInfos returns the underlay addresses of all services for the specified service type. - MakeHostInfos(st ServiceType) []net.UDPAddr + MakeHostInfos(st ServiceType) ([]net.UDPAddr, error) // Gateways returns an array of all gateways. Gateways() ([]GatewayInfo, error) @@ -190,20 +190,18 @@ func (t *topologyS) UnderlayNextHop2(ifid common.IFIDType) (*net.UDPAddr, bool) return copyUDPAddr(ifInfo.InternalAddr), true } -func (t *topologyS) MakeHostInfos(st ServiceType) []net.UDPAddr { +func (t *topologyS) MakeHostInfos(st ServiceType) ([]net.UDPAddr, error) { var hostInfos []net.UDPAddr - addresses, err := t.Topology.GetAllTopoAddrs(st) + addresses, err := t.Topology.getAllTopoAddrs(st) if err != nil { - // FIXME(lukedirtwalker): inform client about this: - // see https://github.com/scionproto/scion/issues/1673 - return hostInfos + return nil, err } for _, a := range addresses { if tmp := a.SCIONAddress; tmp != nil { hostInfos = append(hostInfos, *tmp) } } - return hostInfos + return hostInfos, nil } func (t *topologyS) Core() bool { @@ -339,7 +337,7 @@ func (t *topologyS) UnderlayMulticast(svc addr.HostSVC) ([]*net.UDPAddr, error) if err != nil { return nil, err } - topoAddrs, err := t.Topology.GetAllTopoAddrs(st) + topoAddrs, err := t.Topology.getAllTopoAddrs(st) if err != nil { return nil, serrors.Wrap(addr.ErrUnsupportedSVCAddress, err, "svc", svc) } diff --git a/go/lib/topology/json/json.go b/go/lib/topology/json/json.go index 71f313c816..c253cf29bf 100644 --- a/go/lib/topology/json/json.go +++ b/go/lib/topology/json/json.go @@ -68,11 +68,13 @@ type Topology struct { MTU int `json:"mtu"` // Attributes are the primary AS attributes as described in // https://github.com/scionproto/scion/blob/master/doc/ControlPlanePKI.md#primary-ases - Attributes []Attribute `json:"attributes"` - BorderRouters map[string]*BRInfo `json:"border_routers,omitempty"` - ControlService map[string]*ServerInfo `json:"control_service,omitempty"` - DiscoveryService map[string]*ServerInfo `json:"discovery_service,omitempty"` - SIG map[string]*GatewayInfo `json:"sigs,omitempty"` + Attributes []Attribute `json:"attributes"` + BorderRouters map[string]*BRInfo `json:"border_routers,omitempty"` + ControlService map[string]*ServerInfo `json:"control_service,omitempty"` + DiscoveryService map[string]*ServerInfo `json:"discovery_service,omitempty"` + HiddenSegmentLookup map[string]*ServerInfo `json:"hidden_segment_lookup_service,omitempty"` + HiddenSegmentReg map[string]*ServerInfo `json:"hidden_segment_registration_service,omitempty"` + SIG map[string]*GatewayInfo `json:"sigs,omitempty"` } // ServerInfo contains the information for a SCION application running in the local AS. diff --git a/go/lib/topology/mock_topology/mock.go b/go/lib/topology/mock_topology/mock.go index eeccdc4b7d..79503f0669 100644 --- a/go/lib/topology/mock_topology/mock.go +++ b/go/lib/topology/mock_topology/mock.go @@ -195,11 +195,12 @@ func (mr *MockTopologyMockRecorder) MTU() *gomock.Call { } // MakeHostInfos mocks base method -func (m *MockTopology) MakeHostInfos(arg0 topology.ServiceType) []net.UDPAddr { +func (m *MockTopology) MakeHostInfos(arg0 topology.ServiceType) ([]net.UDPAddr, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "MakeHostInfos", arg0) ret0, _ := ret[0].([]net.UDPAddr) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // MakeHostInfos indicates an expected call of MakeHostInfos diff --git a/go/lib/topology/servicetype.go b/go/lib/topology/servicetype.go index b2f7043ade..c4c7066ef1 100644 --- a/go/lib/topology/servicetype.go +++ b/go/lib/topology/servicetype.go @@ -26,6 +26,8 @@ const ( Control Discovery Gateway + HiddenSegmentLookup + HiddenSegmentRegistration ) func (t ServiceType) String() string { @@ -38,6 +40,10 @@ func (t ServiceType) String() string { return "discovery" case Gateway: return "gateway" + case HiddenSegmentLookup: + return "hiddensegmentlookup" + case HiddenSegmentRegistration: + return "hiddensegmentregistration" default: return "unknown" } @@ -54,6 +60,10 @@ func ServiceTypeFromString(s string) ServiceType { return Discovery case "gateway": return Gateway + case "hiddensegmentlookup": + return HiddenSegmentLookup + case "hiddensegmentregistration": + return HiddenSegmentRegistration default: return Unknown } diff --git a/go/lib/topology/servicetype_test.go b/go/lib/topology/servicetype_test.go index 82f7d73d0c..5521a8aeb5 100644 --- a/go/lib/topology/servicetype_test.go +++ b/go/lib/topology/servicetype_test.go @@ -29,6 +29,8 @@ func TestServiceTypeStringAndParse(t *testing.T) { topology.Control, topology.Discovery, topology.Gateway, + topology.HiddenSegmentLookup, + topology.HiddenSegmentRegistration, } for _, st := range serviceTypes { t.Run(st.String(), func(t *testing.T) { diff --git a/go/lib/topology/topology.go b/go/lib/topology/topology.go index ccddc1823b..90e418094c 100644 --- a/go/lib/topology/topology.go +++ b/go/lib/topology/topology.go @@ -34,6 +34,9 @@ import ( // EndhostPort is the underlay port that the dispatcher binds to on non-routers. const EndhostPort = underlay.EndhostPort +// ErrAddressNotFound indicates the address was not found. +var ErrAddressNotFound = serrors.New("address not found") + type ( // RWTopology is the topology type for applications and libraries that need write // access to AS topology information (e.g., discovery, topology reloaders). @@ -64,9 +67,11 @@ type ( BRNames []string IFInfoMap IfInfoMap - CS IDAddrMap - DS IDAddrMap - SIG map[string]GatewayInfo + CS IDAddrMap + DS IDAddrMap + HiddenSegmentLookup IDAddrMap + HiddenSegmentRegistration IDAddrMap + SIG map[string]GatewayInfo } // GatewayInfo describes a scion gateway. @@ -137,11 +142,13 @@ type ( // NewRWTopology creates new empty Topo object, including all possible service maps etc. func NewRWTopology() *RWTopology { return &RWTopology{ - BR: make(map[string]BRInfo), - CS: make(IDAddrMap), - DS: make(IDAddrMap), - SIG: make(map[string]GatewayInfo), - IFInfoMap: make(IfInfoMap), + BR: make(map[string]BRInfo), + CS: make(IDAddrMap), + DS: make(IDAddrMap), + HiddenSegmentLookup: make(IDAddrMap), + HiddenSegmentRegistration: make(IDAddrMap), + SIG: make(map[string]GatewayInfo), + IFInfoMap: make(IfInfoMap), } } @@ -306,7 +313,14 @@ func (t *RWTopology) populateServices(raw *jsontopo.Topology) error { if err != nil { return serrors.WrapStr("unable to extract DS address", err) } - + t.HiddenSegmentLookup, err = svcMapFromRaw(raw.HiddenSegmentLookup) + if err != nil { + return serrors.WrapStr("unable to extract hidden segment lookup address", err) + } + t.HiddenSegmentRegistration, err = svcMapFromRaw(raw.HiddenSegmentReg) + if err != nil { + return serrors.WrapStr("unable to extract hidden segment registration address", err) + } return nil } @@ -330,15 +344,15 @@ func (t *RWTopology) GetTopoAddr(id string, svc ServiceType) (*TopoAddr, error) return topoAddr, nil } -// GetAllTopoAddrs returns the address information of all processes of the requested type. -func (t *RWTopology) GetAllTopoAddrs(svc ServiceType) ([]TopoAddr, error) { +// getAllTopoAddrs returns the address information of all processes of the requested type. +func (t *RWTopology) getAllTopoAddrs(svc ServiceType) ([]TopoAddr, error) { svcInfo, err := t.getSvcInfo(svc) if err != nil { return nil, err } topoAddrs := svcInfo.getAllTopoAddrs() if topoAddrs == nil { - return nil, serrors.New("Address not found") + return nil, ErrAddressNotFound } return topoAddrs, nil } @@ -351,6 +365,10 @@ func (t *RWTopology) getSvcInfo(svc ServiceType) (*svcInfo, error) { return &svcInfo{idTopoAddrMap: t.DS}, nil case Control: return &svcInfo{idTopoAddrMap: t.CS}, nil + case HiddenSegmentLookup: + return &svcInfo{idTopoAddrMap: t.HiddenSegmentLookup}, nil + case HiddenSegmentRegistration: + return &svcInfo{idTopoAddrMap: t.HiddenSegmentRegistration}, nil case Gateway: m := make(IDAddrMap) for k, v := range t.SIG { @@ -377,9 +395,11 @@ func (t *RWTopology) Copy() *RWTopology { BRNames: append(t.BRNames[:0:0], t.BRNames...), IFInfoMap: t.IFInfoMap.copy(), - CS: t.CS.copy(), - DS: t.DS.copy(), - SIG: copySIGMap(t.SIG), + CS: t.CS.copy(), + DS: t.DS.copy(), + SIG: copySIGMap(t.SIG), + HiddenSegmentLookup: t.HiddenSegmentLookup.copy(), + HiddenSegmentRegistration: t.HiddenSegmentRegistration.copy(), } } diff --git a/go/pkg/discovery/BUILD.bazel b/go/pkg/discovery/BUILD.bazel index a81d20ea63..89e055aa05 100644 --- a/go/pkg/discovery/BUILD.bazel +++ b/go/pkg/discovery/BUILD.bazel @@ -28,5 +28,6 @@ go_test( "//go/lib/topology:go_default_library", "//go/pkg/proto/discovery:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", + "@com_github_stretchr_testify//require:go_default_library", ], ) diff --git a/go/pkg/discovery/testdata/topology.json b/go/pkg/discovery/testdata/topology.json index 9a69f29070..7e4f5bd260 100644 --- a/go/pkg/discovery/testdata/topology.json +++ b/go/pkg/discovery/testdata/topology.json @@ -1,4 +1,4 @@ - { +{ "isd_as": "1-ff00:0:311", "sigs": { "sig1-ff00:0:311-1": { diff --git a/go/pkg/discovery/toposervice.go b/go/pkg/discovery/toposervice.go index 1116de8db2..9a6849c8e9 100644 --- a/go/pkg/discovery/toposervice.go +++ b/go/pkg/discovery/toposervice.go @@ -16,6 +16,7 @@ package discovery import ( "context" + "errors" "net" "github.com/opentracing/opentracing-go" @@ -86,9 +87,32 @@ func (t Topology) HiddenSegmentServices(ctx context.Context, labels := requestLabels{ReqType: "hidden_segment_services"} logger := log.FromCtx(ctx) - logger.Debug("Hidden segment services currently not supported") - t.updateTelemetry(span, labels.WithResult("err_unimplemented"), nil) - return nil, status.Error(codes.Unimplemented, "not supported") + topo := t.Provider.Get() + lookups, err := topo.MakeHostInfos(topology.HiddenSegmentLookup) + if err != nil && !errors.Is(err, topology.ErrAddressNotFound) { + return nil, err + } + registration, err := topo.MakeHostInfos(topology.HiddenSegmentRegistration) + if err != nil && !errors.Is(err, topology.ErrAddressNotFound) { + return nil, err + } + + response := &dpb.HiddenSegmentServicesResponse{} + for _, l := range lookups { + response.Lookup = append(response.Lookup, &dpb.HiddenSegmentLookupServer{ + Address: l.String(), + }) + } + for _, r := range registration { + response.Registration = append(response.Registration, &dpb.HiddenSegmentRegistrationServer{ + Address: r.String(), + }) + } + + logger.Debug("Replied with hidden segment services", + "lookups", len(lookups), "registration", len(registration)) + t.updateTelemetry(span, labels.WithResult(prom.Success), nil) + return response, nil } // RequestsLabels exposes the labels required by the Requests metric. diff --git a/go/pkg/discovery/toposervice_test.go b/go/pkg/discovery/toposervice_test.go index 4b3d6db9c2..53506c5d00 100644 --- a/go/pkg/discovery/toposervice_test.go +++ b/go/pkg/discovery/toposervice_test.go @@ -16,10 +16,12 @@ package discovery_test import ( "context" + "sort" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/scionproto/scion/go/lib/infra/modules/itopo/itopotest" "github.com/scionproto/scion/go/lib/topology" @@ -29,9 +31,9 @@ import ( func TestGateways(t *testing.T) { testCases := map[string]struct { - provider topology.Provider - want *dpb.GatewaysResponse - asserError assert.ErrorAssertionFunc + provider topology.Provider + want *dpb.GatewaysResponse + assertError assert.ErrorAssertionFunc }{ "valid": { provider: itopotest.TopoProviderFromFile(t, "testdata/topology.json"), @@ -50,7 +52,7 @@ func TestGateways(t *testing.T) { }, }, }, - asserError: assert.NoError, + assertError: assert.NoError, }, } @@ -67,7 +69,110 @@ func TestGateways(t *testing.T) { defer cancel() got, err := d.Gateways(ctx, nil) - tc.asserError(t, err) + tc.assertError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestHiddenSegmentServices(t *testing.T) { + testCases := map[string]struct { + topo []byte + want *dpb.HiddenSegmentServicesResponse + assertError assert.ErrorAssertionFunc + }{ + "no service": { + topo: []byte(` + { + "isd_as": "1-ff00:0:311" + } + `), + want: &dpb.HiddenSegmentServicesResponse{}, + assertError: assert.NoError, + }, + "only lookup service": { + topo: []byte(` + { + "isd_as": "1-ff00:0:311", + "hidden_segment_lookup_service": { + "hsls-1": {"addr": "10.1.0.1:30254"}, + "hsls-2": {"addr": "10.1.0.2:30254"} + } + } + `), + want: &dpb.HiddenSegmentServicesResponse{ + Lookup: []*dpb.HiddenSegmentLookupServer{ + {Address: "10.1.0.1:30254"}, + {Address: "10.1.0.2:30254"}, + }, + }, + assertError: assert.NoError, + }, + "only registration service": { + topo: []byte(` + { + "isd_as": "1-ff00:0:311", + "hidden_segment_registration_service": { + "hsls-3": {"addr": "10.1.0.3:30254"}, + "hsls-4": {"addr": "10.1.0.4:30254"} + } + } + `), + want: &dpb.HiddenSegmentServicesResponse{ + Registration: []*dpb.HiddenSegmentRegistrationServer{ + {Address: "10.1.0.3:30254"}, + {Address: "10.1.0.4:30254"}, + }, + }, + assertError: assert.NoError, + }, + "both services": { + topo: []byte(` + { + "isd_as": "1-ff00:0:311", + "hidden_segment_lookup_service": { + "hsls-1": {"addr": "10.1.0.1:30254"}, + "hsls-2": {"addr": "10.1.0.2:30254"} + }, + "hidden_segment_registration_service": { + "hsls-3": {"addr": "10.1.0.3:30254"}, + "hsls-4": {"addr": "10.1.0.4:30254"} + } + } + `), + want: &dpb.HiddenSegmentServicesResponse{ + Lookup: []*dpb.HiddenSegmentLookupServer{ + {Address: "10.1.0.1:30254"}, + {Address: "10.1.0.2:30254"}, + }, + Registration: []*dpb.HiddenSegmentRegistrationServer{ + {Address: "10.1.0.3:30254"}, + {Address: "10.1.0.4:30254"}, + }, + }, + assertError: assert.NoError, + }, + } + for name, tc := range testCases { + name, tc := name, tc + t.Run(name, func(t *testing.T) { + t.Parallel() + + rwTopo, err := topology.RWTopologyFromJSONBytes(tc.topo) + require.NoError(t, err) + d := discovery.Topology{Provider: &itopotest.TestTopoProvider{RWTopology: rwTopo}} + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + got, err := d.HiddenSegmentServices(ctx, nil) + tc.assertError(t, err) + sort.Slice(got.Lookup, func(i, j int) bool { + return got.Lookup[i].Address < got.Lookup[j].Address + }) + sort.Slice(got.Registration, func(i, j int) bool { + return got.Registration[i].Address < got.Registration[j].Address + }) assert.Equal(t, tc.want, got) }) } diff --git a/go/pkg/sciond/internal/servers/grpc.go b/go/pkg/sciond/internal/servers/grpc.go index df2b6315dc..b86be0b0fe 100644 --- a/go/pkg/sciond/internal/servers/grpc.go +++ b/go/pkg/sciond/internal/servers/grpc.go @@ -16,6 +16,7 @@ package servers import ( "context" + "errors" "time" durationpb "github.com/golang/protobuf/ptypes/duration" @@ -267,7 +268,10 @@ func (s DaemonServer) services(ctx context.Context, serviceTypes := []topology.ServiceType{topology.Control, topology.Gateway} for _, t := range serviceTypes { list := &sdpb.ListService{} - svcHosts := topo.MakeHostInfos(t) + svcHosts, err := topo.MakeHostInfos(t) + if err != nil && !errors.Is(topology.ErrAddressNotFound, err) { + return nil, err + } for _, h := range svcHosts { // TODO(lukedirtwalker): build actual URI after it's defined (anapapaya/scion#3587) list.Services = append(list.Services, &sdpb.Service{Uri: h.String()})