Move segutil from PS to revcache

go/lib/revcache/BUILD.bazel

@@ -2,30 +2,36 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
-    srcs = ["revcache.go"],
+    srcs = [
+        "revcache.go",
+        "util.go",
+    ],
     importpath = "github.com/scionproto/scion/go/lib/revcache",
     visibility = ["//visibility:public"],
     deps = [
         "//go/lib/addr:go_default_library",
         "//go/lib/common:go_default_library",
         "//go/lib/ctrl/path_mgmt:go_default_library",
+        "//go/lib/ctrl/seg:go_default_library",
         "//go/lib/infra/modules/cleaner:go_default_library",
         "//go/lib/infra/modules/db:go_default_library",
     ],
 )
 
 go_test(
     name = "go_default_test",
-    srcs = ["revcache_test.go"],
+    srcs = ["util_test.go"],
     embed = [":go_default_library"],
     deps = [
         "//go/lib/addr:go_default_library",
         "//go/lib/common:go_default_library",
         "//go/lib/ctrl/path_mgmt:go_default_library",
+        "//go/lib/ctrl/seg:go_default_library",
         "//go/lib/infra:go_default_library",
         "//go/lib/revcache/mock_revcache:go_default_library",
         "//go/lib/util:go_default_library",
         "//go/lib/xtest:go_default_library",
+        "//go/lib/xtest/graph:go_default_library",
         "//go/proto:go_default_library",
         "@com_github_golang_mock//gomock:go_default_library",
         "@com_github_smartystreets_goconvey//convey:go_default_library",

go/lib/revcache/revcache.go

@@ -22,7 +22,6 @@ import (
 	"github.com/scionproto/scion/go/lib/addr"
 	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/lib/ctrl/path_mgmt"
-	"github.com/scionproto/scion/go/lib/infra/modules/cleaner"
 	"github.com/scionproto/scion/go/lib/infra/modules/db"
 )
 
@@ -144,33 +143,3 @@ func (r Revocations) FilterNew(ctx context.Context, revCache RevCache) error {
 	}
 	return nil
 }
-
-// NewCleaner creates a cleaner task that deletes expired revocations.
-func NewCleaner(rc RevCache) *cleaner.Cleaner {
-	return cleaner.New(func(ctx context.Context) (int, error) {
-		cnt, err := rc.DeleteExpired(ctx)
-		return int(cnt), err
-	}, "revocations")
-}
-
-// FilterNew filters the given revocations against the revCache, only the ones which are not in the
-// cache are returned. This is a convenience wrapper around the Revocations type and its filter new
-// method.
-func FilterNew(ctx context.Context, revCache RevCache,
-	revocations []*path_mgmt.SignedRevInfo) ([]*path_mgmt.SignedRevInfo, error) {
-
-	rMap, err := RevocationToMap(revocations)
-	if err != nil {
-		return nil, err
-	}
-	if err = rMap.FilterNew(ctx, revCache); err != nil {
-		return nil, err
-	}
-	return rMap.ToSlice(), nil
-}
-
-// newerInfo returns whether the received info is newer than the existing.
-func newerInfo(existing, received *path_mgmt.RevInfo) bool {
-	return !received.SameIntf(existing) ||
-		received.Timestamp().After(existing.Timestamp())
-}

go/path_srv/internal/segutil/segutil.go → go/lib/revcache/util.go

@@ -12,31 +12,61 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package segutil
+package revcache
 
 import (
 	"context"
 
 	"github.com/scionproto/scion/go/lib/ctrl/path_mgmt"
 	"github.com/scionproto/scion/go/lib/ctrl/seg"
-	"github.com/scionproto/scion/go/lib/revcache"
+	"github.com/scionproto/scion/go/lib/infra/modules/cleaner"
 )
 
+// NewCleaner creates a cleaner task that deletes expired revocations.
+func NewCleaner(rc RevCache) *cleaner.Cleaner {
+	return cleaner.New(func(ctx context.Context) (int, error) {
+		cnt, err := rc.DeleteExpired(ctx)
+		return int(cnt), err
+	}, "revocations")
+}
+
+// FilterNew filters the given revocations against the revCache, only the ones
+// which are not in the cache are returned. This is a convenience wrapper
+// around the Revocations type and its filter new method.
+func FilterNew(ctx context.Context, revCache RevCache,
+	revocations []*path_mgmt.SignedRevInfo) ([]*path_mgmt.SignedRevInfo, error) {
+
+	rMap, err := RevocationToMap(revocations)
+	if err != nil {
+		return nil, err
+	}
+	if err = rMap.FilterNew(ctx, revCache); err != nil {
+		return nil, err
+	}
+	return rMap.ToSlice(), nil
+}
+
+// newerInfo returns whether the received info is newer than the existing.
+func newerInfo(existing, received *path_mgmt.RevInfo) bool {
+	return !received.SameIntf(existing) ||
+		received.Timestamp().After(existing.Timestamp())
+}
+
 // NoRevokedHopIntf returns true if there is no on-segment revocation.
-func NoRevokedHopIntf(ctx context.Context, revCache revcache.RevCache,
+func NoRevokedHopIntf(ctx context.Context, revCache RevCache,
 	s *seg.PathSegment) (bool, error) {
 
-	revKeys := make(revcache.KeySet)
+	revKeys := make(KeySet)
 	addRevKeys([]*seg.PathSegment{s}, revKeys, true)
 	revs, err := revCache.Get(ctx, revKeys)
 	return len(revs) == 0, err
 }
 
 // RelevantRevInfos finds all revocations for the given segments.
-func RelevantRevInfos(ctx context.Context, revCache revcache.RevCache,
+func RelevantRevInfos(ctx context.Context, revCache RevCache,
 	allSegs ...[]*seg.PathSegment) ([]*path_mgmt.SignedRevInfo, error) {
 
-	revKeys := make(revcache.KeySet)
+	revKeys := make(KeySet)
 	for _, segs := range allSegs {
 		addRevKeys(segs, revKeys, false)
 	}
@@ -53,7 +83,7 @@ func RelevantRevInfos(ctx context.Context, revCache revcache.RevCache,
 
 // addRevKeys adds all revocations keys for the given segments to the keys set.
 // If hopOnly is set, only the first hop entry is considered.
-func addRevKeys(segs []*seg.PathSegment, keys revcache.KeySet, hopOnly bool) {
+func addRevKeys(segs []*seg.PathSegment, keys KeySet, hopOnly bool) {
 	for _, s := range segs {
 		for _, asEntry := range s.ASEntries {
 			for _, entry := range asEntry.HopEntries {
@@ -64,10 +94,10 @@ func addRevKeys(segs []*seg.PathSegment, keys revcache.KeySet, hopOnly bool) {
 					panic(err)
 				}
 				if hf.ConsIngress != 0 {
-					keys[*revcache.NewKey(asEntry.IA(), hf.ConsIngress)] = struct{}{}
+					keys[*NewKey(asEntry.IA(), hf.ConsIngress)] = struct{}{}
 				}
 				if hf.ConsEgress != 0 {
-					keys[*revcache.NewKey(asEntry.IA(), hf.ConsEgress)] = struct{}{}
+					keys[*NewKey(asEntry.IA(), hf.ConsEgress)] = struct{}{}
 				}
 				if hopOnly {
 					break

go/lib/revcache/revcache_test.go → go/lib/revcache/util_test.go

@@ -1,4 +1,4 @@
-// Copyright 2019 Anapaya Systems
+// Copyright 2018 Anapaya Systems
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -25,23 +25,27 @@ import (
 	"github.com/scionproto/scion/go/lib/addr"
 	"github.com/scionproto/scion/go/lib/common"
 	"github.com/scionproto/scion/go/lib/ctrl/path_mgmt"
+	"github.com/scionproto/scion/go/lib/ctrl/seg"
 	"github.com/scionproto/scion/go/lib/infra"
 	"github.com/scionproto/scion/go/lib/revcache"
 	"github.com/scionproto/scion/go/lib/revcache/mock_revcache"
 	"github.com/scionproto/scion/go/lib/util"
 	"github.com/scionproto/scion/go/lib/xtest"
+	"github.com/scionproto/scion/go/lib/xtest/graph"
 	"github.com/scionproto/scion/go/proto"
 )
 
 var (
 	ia110  = xtest.MustParseIA("1-ff00:0:110")
+	ia211  = xtest.MustParseIA("2-ff00:0:211")
 	ifid10 = common.IFIDType(10)
 	ifid11 = common.IFIDType(11)
 
-	now = time.Now()
+	timeout = time.Second
 )
 
 func TestFilterNew(t *testing.T) {
+	now := time.Now()
 	sr10, err := path_mgmt.NewSignedRevInfo(defaultRevInfo(ia110, ifid10, now), infra.NullSigner)
 	xtest.FailOnErr(t, err)
 	sr11, err := path_mgmt.NewSignedRevInfo(defaultRevInfo(ia110, ifid11, now), infra.NullSigner)
@@ -97,6 +101,70 @@ func TestFilterNew(t *testing.T) {
 
 }
 
+func TestNoRevokedHopIntf(t *testing.T) {
+	now := time.Now()
+	Convey("NoRevokedHopIntf", t, func() {
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		seg210_222_1 := createSeg(ctrl)
+		ctx, cancelF := context.WithTimeout(context.Background(), timeout)
+		defer cancelF()
+		revCache := mock_revcache.NewMockRevCache(ctrl)
+		Convey("Given an empty revcache", func() {
+			revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any())
+			noR, err := revcache.NoRevokedHopIntf(ctx, revCache, seg210_222_1)
+			SoMsg("No err expected", err, ShouldBeNil)
+			SoMsg("No revocation expected", noR, ShouldBeTrue)
+		})
+		Convey("Given a revcache with an on segment revocation", func() {
+			sRev, err := path_mgmt.NewSignedRevInfo(
+				defaultRevInfo(ia211, graph.If_210_X_211_A, now), infra.NullSigner)
+			xtest.FailOnErr(t, err)
+			revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any()).Return(
+				revcache.Revocations{
+					revcache.Key{IA: xtest.MustParseIA("2-ff00:0:211"),
+						IfId: graph.If_210_X_211_A}: sRev,
+				}, nil,
+			)
+			noR, err := revcache.NoRevokedHopIntf(ctx, revCache, seg210_222_1)
+			SoMsg("No err expected", err, ShouldBeNil)
+			SoMsg("Revocation expected", noR, ShouldBeFalse)
+		})
+		Convey("Given an error in the revache it is propagated", func() {
+			revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any()).Return(
+				nil, common.NewBasicError("TestError", nil),
+			)
+			_, err := revcache.NoRevokedHopIntf(ctx, revCache, seg210_222_1)
+			SoMsg("Err expected", err, ShouldNotBeNil)
+		})
+	})
+}
+
+func TestRelevantRevInfos(t *testing.T) {
+	Convey("TestRelevantRevInfos", t, func() {
+		ctx, cancelF := context.WithTimeout(context.Background(), timeout)
+		defer cancelF()
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+		segs := []*seg.PathSegment{createSeg(ctrl)}
+		revCache := mock_revcache.NewMockRevCache(ctrl)
+		Convey("Given an empty revcache", func() {
+			revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any())
+			revs, err := revcache.RelevantRevInfos(ctx, revCache, segs)
+			SoMsg("No err expected", err, ShouldBeNil)
+			SoMsg("No revocation expected", revs, ShouldBeEmpty)
+		})
+		// TODO(lukedirtwalker): Add test with revocations
+		Convey("Given an error in the revache it is propagated", func() {
+			revCache.EXPECT().Get(gomock.Eq(ctx), gomock.Any()).Return(
+				nil, common.NewBasicError("TestError", nil),
+			)
+			_, err := revcache.RelevantRevInfos(ctx, revCache, segs)
+			SoMsg("Err expected", err, ShouldNotBeNil)
+		})
+	})
+}
+
 func copy(revs revcache.Revocations) revcache.Revocations {
 	res := make(revcache.Revocations, len(revs))
 	for k, v := range revs {
@@ -114,3 +182,8 @@ func defaultRevInfo(ia addr.IA, ifId common.IFIDType, ts time.Time) *path_mgmt.R
 		RawTTL:       uint32((time.Duration(10) * time.Second).Seconds()),
 	}
 }
+
+func createSeg(ctrl *gomock.Controller) *seg.PathSegment {
+	g := graph.NewDefaultGraph(ctrl)
+	return g.Beacon([]common.IFIDType{graph.If_210_X_211_A, graph.If_211_A_222_X})
+}

go/path_srv/internal/handlers/BUILD.bazel

@@ -34,7 +34,6 @@ go_library(
         "//go/lib/snet:go_default_library",
         "//go/lib/snet/addrutil:go_default_library",
         "//go/lib/topology:go_default_library",
-        "//go/path_srv/internal/segutil:go_default_library",
         "//go/proto:go_default_library",
         "@com_github_opentracing_opentracing_go//:go_default_library",
     ],

go/path_srv/internal/handlers/common.go

@@ -35,7 +35,6 @@ import (
 	"github.com/scionproto/scion/go/lib/pathdb/query"
 	"github.com/scionproto/scion/go/lib/revcache"
 	"github.com/scionproto/scion/go/lib/topology"
-	"github.com/scionproto/scion/go/path_srv/internal/segutil"
 )
 
 const (
@@ -91,7 +90,7 @@ func (h *baseHandler) fetchSegsFromDB(ctx context.Context,
 	segs := query.Results(res).Segs()
 	// XXX(lukedirtwalker): Consider cases where segment with revoked interfaces should be returned.
 	_, err = segs.FilterSegsErr(func(s *seg.PathSegment) (bool, error) {
-		noRevoked, err := segutil.NoRevokedHopIntf(ctx, h.revCache, s)
+		noRevoked, err := revcache.NoRevokedHopIntf(ctx, h.revCache, s)
 		if err != nil {
 			return false, err
 		}

go/path_srv/internal/handlers/segreq.go

@@ -33,7 +33,6 @@ import (
 	"github.com/scionproto/scion/go/lib/pathdb/query"
 	"github.com/scionproto/scion/go/lib/revcache"
 	"github.com/scionproto/scion/go/lib/snet"
-	"github.com/scionproto/scion/go/path_srv/internal/segutil"
 	"github.com/scionproto/scion/go/proto"
 )
 
@@ -203,7 +202,7 @@ func (h *segReqHandler) sendReply(ctx context.Context, rw infra.ResponseWriter,
 	upSegs, coreSegs, downSegs []*seg.PathSegment, segReq *path_mgmt.SegReq) {
 
 	logger := log.FromCtx(ctx)
-	revs, err := segutil.RelevantRevInfos(ctx, h.revCache, upSegs, coreSegs, downSegs)
+	revs, err := revcache.RelevantRevInfos(ctx, h.revCache, upSegs, coreSegs, downSegs)
 	if err != nil {
 		logger.Error("[segReqHandler] Failed to find relevant revocations for reply", "err", err)
 		// the client might still be able to use the segments so continue here.

go/path_srv/internal/segreq/BUILD.bazel

@@ -28,7 +28,6 @@ go_library(
         "//go/lib/snet/addrutil:go_default_library",
         "//go/lib/topology:go_default_library",
         "//go/path_srv/internal/handlers:go_default_library",
-        "//go/path_srv/internal/segutil:go_default_library",
         "//go/proto:go_default_library",
     ],
 )

go/path_srv/internal/segreq/handler.go

@@ -26,7 +26,6 @@ import (
 	"github.com/scionproto/scion/go/lib/pathdb"
 	"github.com/scionproto/scion/go/lib/revcache"
 	"github.com/scionproto/scion/go/path_srv/internal/handlers"
-	"github.com/scionproto/scion/go/path_srv/internal/segutil"
 )
 
 type handler struct {
@@ -75,7 +74,7 @@ func (h *handler) Handle(request *infra.Request) *infra.HandlerResult {
 	if err != nil {
 		return infra.MetricsErrInternal
 	}
-	revs, err := segutil.RelevantRevInfos(ctx, h.revCache, segs.Up, segs.Core, segs.Down)
+	revs, err := revcache.RelevantRevInfos(ctx, h.revCache, segs.Up, segs.Core, segs.Down)
 	if err != nil {
 		logger.Error("[segReqHandler] Failed to find relevant revocations for reply", "err", err)
 		// the client might still be able to use the segments so continue here.

go/path_srv/internal/segreq/provider.go

@@ -30,7 +30,6 @@ import (
 	"github.com/scionproto/scion/go/lib/revcache"
 	"github.com/scionproto/scion/go/lib/snet/addrutil"
 	"github.com/scionproto/scion/go/lib/topology"
-	"github.com/scionproto/scion/go/path_srv/internal/segutil"
 	"github.com/scionproto/scion/go/proto"
 )
 
@@ -50,7 +49,7 @@ func (s *SegSelector) SelectSeg(ctx context.Context,
 	}
 	segs := query.Results(res).Segs()
 	_, err = segs.FilterSegsErr(func(ps *seg.PathSegment) (bool, error) {
-		return segutil.NoRevokedHopIntf(ctx, s.RevCache, ps)
+		return revcache.NoRevokedHopIntf(ctx, s.RevCache, ps)
 	})
 	if err != nil {
 		return nil, common.NewBasicError("Failed to filter segments", err)

go/path_srv/internal/segsyncer/BUILD.bazel

@@ -20,7 +20,6 @@ go_library(
         "//go/lib/snet/addrutil:go_default_library",
         "//go/lib/topology:go_default_library",
         "//go/path_srv/internal/handlers:go_default_library",
-        "//go/path_srv/internal/segutil:go_default_library",
         "//go/proto:go_default_library",
     ],
 )