From ac2ea039b5c16440cd6f7074b9b6042e9835c7c6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Magiera?= <magik6k@gmail.com>
Date: Tue, 12 Jul 2022 13:55:18 +0200
Subject: [PATCH] path index: Raise alerts on bad filter config

---
 cmd/lotus-miner/init.go               |  2 +-
 itests/path_type_filters_test.go      | 34 ++++++++++++++
 journal/alerting/alerts.go            |  7 +++
 storage/paths/index.go                | 65 ++++++++++++++++++++++++---
 storage/paths/index_test.go           |  6 +--
 storage/paths/local_test.go           |  2 +-
 storage/paths/remote_test.go          |  2 +-
 storage/sealer/manager_test.go        |  2 +-
 storage/sealer/piece_provider_test.go |  2 +-
 storage/sealer/sched_test.go          |  4 +-
 10 files changed, 109 insertions(+), 17 deletions(-)

diff --git a/cmd/lotus-miner/init.go b/cmd/lotus-miner/init.go
index 2daced42e32..fc7dc03f6b7 100644
--- a/cmd/lotus-miner/init.go
+++ b/cmd/lotus-miner/init.go
@@ -458,7 +458,7 @@ func storageMinerInit(ctx context.Context, cctx *cli.Context, api v1api.FullNode
 			wsts := statestore.New(namespace.Wrap(mds, modules.WorkerCallsPrefix))
 			smsts := statestore.New(namespace.Wrap(mds, modules.ManagerWorkPrefix))
 
-			si := paths.NewIndex()
+			si := paths.NewIndex(nil)
 
 			lstor, err := paths.NewLocal(ctx, lr, si, nil)
 			if err != nil {
diff --git a/itests/path_type_filters_test.go b/itests/path_type_filters_test.go
index 8ab18f7e0bb..03dd5ea16f9 100644
--- a/itests/path_type_filters_test.go
+++ b/itests/path_type_filters_test.go
@@ -2,6 +2,7 @@ package itests
 
 import (
 	"context"
+	"strings"
 	"testing"
 	"time"
 
@@ -43,6 +44,39 @@ func TestPathTypeFilters(t *testing.T) {
 		})
 	}
 
+	runTest(t, "invalid-type-alert", func(t *testing.T, ctx context.Context, miner *kit.TestMiner, run func()) {
+		slU := miner.AddStorage(ctx, t, func(meta *paths.LocalStorageMeta) {
+			meta.CanSeal = true
+			meta.AllowTypes = []string{"unsealed", "seeled"}
+		})
+
+		storlist, err := miner.StorageList(ctx)
+		require.NoError(t, err)
+
+		require.Len(t, storlist, 2) // 1 path we've added + preseal
+
+		si, err := miner.StorageInfo(ctx, slU)
+		require.NoError(t, err)
+
+		// check that bad entries are filtered
+		require.Len(t, si.DenyTypes, 0)
+		require.Len(t, si.AllowTypes, 1)
+		require.Equal(t, "unsealed", si.AllowTypes[0])
+
+		as, err := miner.LogAlerts(ctx)
+		require.NoError(t, err)
+
+		var found bool
+		for _, a := range as {
+			if a.Active && a.Type.System == "sector-index" && strings.HasPrefix(a.Type.Subsystem, "pathconf-") {
+				require.False(t, found)
+				require.Contains(t, string(a.LastActive.Message), "unknown sector file type 'seeled'")
+				found = true
+			}
+		}
+		require.True(t, found)
+	})
+
 	runTest(t, "seal-to-stor-unseal-allowdeny", func(t *testing.T, ctx context.Context, miner *kit.TestMiner, run func()) {
 		// allow all types in the sealing path
 		sealScratch := miner.AddStorage(ctx, t, func(meta *paths.LocalStorageMeta) {
diff --git a/journal/alerting/alerts.go b/journal/alerting/alerts.go
index b99638f7773..290d06e2aa7 100644
--- a/journal/alerting/alerts.go
+++ b/journal/alerting/alerts.go
@@ -160,3 +160,10 @@ func (a *Alerting) GetAlerts() []Alert {
 
 	return out
 }
+
+func (a *Alerting) IsRaised(at AlertType) bool {
+	a.lk.Lock()
+	defer a.lk.Unlock()
+
+	return a.alerts[at].Active
+}
diff --git a/storage/paths/index.go b/storage/paths/index.go
index d5c54d7b131..63fff738301 100644
--- a/storage/paths/index.go
+++ b/storage/paths/index.go
@@ -17,6 +17,7 @@ import (
 	"github.com/filecoin-project/go-state-types/abi"
 	"github.com/filecoin-project/go-state-types/big"
 
+	"github.com/filecoin-project/lotus/journal/alerting"
 	"github.com/filecoin-project/lotus/metrics"
 	"github.com/filecoin-project/lotus/storage/sealer/fsutil"
 	"github.com/filecoin-project/lotus/storage/sealer/storiface"
@@ -63,15 +64,23 @@ type Index struct {
 	*indexLocks
 	lk sync.RWMutex
 
+	// optional
+	alerting   *alerting.Alerting
+	pathAlerts map[storiface.ID]alerting.AlertType
+
 	sectors map[storiface.Decl][]*declMeta
 	stores  map[storiface.ID]*storageEntry
 }
 
-func NewIndex() *Index {
+func NewIndex(al *alerting.Alerting) *Index {
 	return &Index{
 		indexLocks: &indexLocks{
 			locks: map[abi.SectorID]*sectorLock{},
 		},
+
+		alerting:   al,
+		pathAlerts: map[storiface.ID]alerting.AlertType{},
+
 		sectors: map[storiface.Decl][]*declMeta{},
 		stores:  map[storiface.ID]*storageEntry{},
 	}
@@ -107,20 +116,62 @@ func (i *Index) StorageList(ctx context.Context) (map[storiface.ID][]storiface.D
 }
 
 func (i *Index) StorageAttach(ctx context.Context, si storiface.StorageInfo, st fsutil.FsStat) error {
-	for i, typ := range si.AllowTypes {
+	var allow, deny = make([]string, 0, len(si.AllowTypes)), make([]string, 0, len(si.DenyTypes))
+
+	if _, hasAlert := i.pathAlerts[si.ID]; i.alerting != nil && !hasAlert {
+		i.pathAlerts[si.ID] = i.alerting.AddAlertType("sector-index", "pathconf-"+string(si.ID))
+	}
+
+	var hasConfigIsses bool
+
+	for id, typ := range si.AllowTypes {
 		_, err := storiface.TypeFromString(typ)
 		if err != nil {
 			// No need no hard-fail here, just warn the user
 			// (note that even with all-invalid entries we'll deny all types, so nothing unexpected should enter the path)
-			log.Errorw("bad path type in AllowTypes", "path", si.ID, "idx", i, "type", typ, "error", err)
+			hasConfigIsses = true
+
+			if i.alerting != nil {
+				i.alerting.Raise(i.pathAlerts[si.ID], map[string]interface{}{
+					"message":   "bad path type in AllowTypes",
+					"path":      string(si.ID),
+					"idx":       id,
+					"path_type": typ,
+					"error":     err.Error(),
+				})
+			}
+
+			continue
 		}
+		allow = append(allow, typ)
 	}
-	for i, typ := range si.DenyTypes {
+	for id, typ := range si.DenyTypes {
 		_, err := storiface.TypeFromString(typ)
 		if err != nil {
 			// No need no hard-fail here, just warn the user
-			log.Errorw("bad path type in DenyTypes", "path", si.ID, "idx", i, "type", typ, "error", err)
+			hasConfigIsses = true
+
+			if i.alerting != nil {
+				i.alerting.Raise(i.pathAlerts[si.ID], map[string]interface{}{
+					"message":   "bad path type in DenyTypes",
+					"path":      string(si.ID),
+					"idx":       id,
+					"path_type": typ,
+					"error":     err.Error(),
+				})
+			}
+
+			continue
 		}
+		deny = append(deny, typ)
+	}
+	si.AllowTypes = allow
+	si.DenyTypes = deny
+
+	if i.alerting != nil && !hasConfigIsses && i.alerting.IsRaised(i.pathAlerts[si.ID]) {
+		i.alerting.Resolve(i.pathAlerts[si.ID], map[string]string{
+			"message": "path config is now correct",
+		})
 	}
 
 	i.lk.Lock()
@@ -152,8 +203,8 @@ func (i *Index) StorageAttach(ctx context.Context, si storiface.StorageInfo, st
 		i.stores[si.ID].info.CanStore = si.CanStore
 		i.stores[si.ID].info.Groups = si.Groups
 		i.stores[si.ID].info.AllowTo = si.AllowTo
-		i.stores[si.ID].info.AllowTypes = si.AllowTypes
-		i.stores[si.ID].info.DenyTypes = si.DenyTypes
+		i.stores[si.ID].info.AllowTypes = allow
+		i.stores[si.ID].info.DenyTypes = deny
 
 		return nil
 	}
diff --git a/storage/paths/index_test.go b/storage/paths/index_test.go
index 3eb2dbec47f..9a241da23e0 100644
--- a/storage/paths/index_test.go
+++ b/storage/paths/index_test.go
@@ -42,7 +42,7 @@ const s32g = 32 << 30
 func TestFindSimple(t *testing.T) {
 	ctx := context.Background()
 
-	i := NewIndex()
+	i := NewIndex(nil)
 	stor1 := newTestStorage()
 	stor2 := newTestStorage()
 
@@ -79,7 +79,7 @@ func TestFindSimple(t *testing.T) {
 func TestFindNoAllow(t *testing.T) {
 	ctx := context.Background()
 
-	i := NewIndex()
+	i := NewIndex(nil)
 	stor1 := newTestStorage()
 	stor1.AllowTo = []storiface.Group{"grp1"}
 	stor2 := newTestStorage()
@@ -111,7 +111,7 @@ func TestFindNoAllow(t *testing.T) {
 func TestFindAllow(t *testing.T) {
 	ctx := context.Background()
 
-	i := NewIndex()
+	i := NewIndex(nil)
 
 	stor1 := newTestStorage()
 	stor1.AllowTo = []storiface.Group{"grp1"}
diff --git a/storage/paths/local_test.go b/storage/paths/local_test.go
index c4891811a33..83e8e27fdb2 100644
--- a/storage/paths/local_test.go
+++ b/storage/paths/local_test.go
@@ -81,7 +81,7 @@ func TestLocalStorage(t *testing.T) {
 		root: root,
 	}
 
-	index := NewIndex()
+	index := NewIndex(nil)
 
 	st, err := NewLocal(ctx, tstor, index, nil)
 	require.NoError(t, err)
diff --git a/storage/paths/remote_test.go b/storage/paths/remote_test.go
index d59b2c27065..ec58fb84dfb 100644
--- a/storage/paths/remote_test.go
+++ b/storage/paths/remote_test.go
@@ -60,7 +60,7 @@ func createTestStorage(t *testing.T, p string, seal bool, att ...*paths.Local) s
 func TestMoveShared(t *testing.T) {
 	logging.SetAllLoggers(logging.LevelDebug)
 
-	index := paths.NewIndex()
+	index := paths.NewIndex(nil)
 
 	ctx := context.Background()
 
diff --git a/storage/sealer/manager_test.go b/storage/sealer/manager_test.go
index 75f12261c62..d287462e0b6 100644
--- a/storage/sealer/manager_test.go
+++ b/storage/sealer/manager_test.go
@@ -100,7 +100,7 @@ var _ paths.LocalStorage = &testStorage{}
 func newTestMgr(ctx context.Context, t *testing.T, ds datastore.Datastore) (*Manager, *paths.Local, *paths.Remote, *paths.Index, func()) {
 	st := newTestStorage(t)
 
-	si := paths.NewIndex()
+	si := paths.NewIndex(nil)
 
 	lstor, err := paths.NewLocal(ctx, st, si, nil)
 	require.NoError(t, err)
diff --git a/storage/sealer/piece_provider_test.go b/storage/sealer/piece_provider_test.go
index 8da155604c5..c4c71bc5308 100644
--- a/storage/sealer/piece_provider_test.go
+++ b/storage/sealer/piece_provider_test.go
@@ -206,7 +206,7 @@ func newPieceProviderTestHarness(t *testing.T, mgrConfig Config, sectorProofType
 	require.NoError(t, err)
 
 	// create index, storage, local store & remote store.
-	index := paths.NewIndex()
+	index := paths.NewIndex(nil)
 	storage := newTestStorage(t)
 	localStore, err := paths.NewLocal(ctx, storage, index, []string{"http://" + nl.Addr().String() + "/remote"})
 	require.NoError(t, err)
diff --git a/storage/sealer/sched_test.go b/storage/sealer/sched_test.go
index 258383e90c6..bca8ace6ed9 100644
--- a/storage/sealer/sched_test.go
+++ b/storage/sealer/sched_test.go
@@ -226,7 +226,7 @@ func TestSchedStartStop(t *testing.T) {
 	require.NoError(t, err)
 	go sched.runSched()
 
-	addTestWorker(t, sched, paths.NewIndex(), "fred", nil, decentWorkerResources, false)
+	addTestWorker(t, sched, paths.NewIndex(nil), "fred", nil, decentWorkerResources, false)
 
 	require.NoError(t, sched.Close(context.TODO()))
 }
@@ -350,7 +350,7 @@ func TestSched(t *testing.T) {
 
 	testFunc := func(workers []workerSpec, tasks []task) func(t *testing.T) {
 		return func(t *testing.T) {
-			index := paths.NewIndex()
+			index := paths.NewIndex(nil)
 
 			sched, err := newScheduler("")
 			require.NoError(t, err)