Use fault bus and alerts instead of error for config verify (#5122)

Switch to using alerts and the fault bus instead of errors. Hopefully
this will make it easier to ensure this verify code doesn't fail
maintenance overall and that callers can consume the info in a
standardized manner

---

#### Does this PR need a docs update or release note?

- [ ] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x] ⛔ No

#### Type of change

- [x] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

merge after:
* #5117
* #5118

#### Test Plan

- [ ] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E

src/internal/kopia/conn.go

@@ -25,11 +25,14 @@ import (
 	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/kopia/retention"
 	"github.com/alcionai/corso/src/pkg/control/repository"
+	"github.com/alcionai/corso/src/pkg/fault"
 	"github.com/alcionai/corso/src/pkg/logger"
 	"github.com/alcionai/corso/src/pkg/storage"
 )
 
 const (
+	corsoWrapperAlertNamespace = "corso-kopia-wrapper"
+
 	defaultKopiaConfigDir   = "/tmp/"
 	kopiaConfigFileTemplate = "repository-%s.config"
 	defaultCompressor       = "zstd-better-compression"
@@ -738,59 +741,97 @@ func (w *conn) updatePersistentConfig(
 		OrNil()
 }
 
-func (w *conn) verifyDefaultPolicyConfigOptions(ctx context.Context) error {
+func (w *conn) verifyDefaultPolicyConfigOptions(
-	var errs *clues.Err
+	ctx context.Context,
+	errs *fault.Bus,
+) {
+	const alertName = "kopia-global-policy"
 
 	globalPol, err := w.getGlobalPolicyOrEmpty(ctx)
 	if err != nil {
-		return clues.Stack(err)
+		errs.AddAlert(ctx, fault.NewAlert(
+			err.Error(),
+			corsoWrapperAlertNamespace,
+			"fetch-policy",
+			alertName,
+			nil))
+
+		return
 	}
 
 	ctx = clues.Add(ctx, "current_global_policy", globalPol.String())
 
 	if globalPol.CompressionPolicy.CompressorName != defaultCompressor {
-		errs = clues.Stack(errs, clues.NewWC(
+		errs.AddAlert(ctx, fault.NewAlert(
-			ctx,
+			"unexpected compressor",
-			"current compressor doesn't match default").
+			corsoWrapperAlertNamespace,
-			With("expected_compression_policy", defaultCompressor))
+			"compressor",
+			alertName,
+			nil))
 	}
 
 	// Need to use deep equals because the values are pointers to optional types.
 	// That makes regular equality checks fail even if the data contained in each
 	// policy is the same.
 	if !reflect.DeepEqual(globalPol.RetentionPolicy, defaultRetention) {
-		// Unfortunately the policy has pointers to things and doesn't serialize
+		errs.AddAlert(ctx, fault.NewAlert(
-		// well. This makes it difficult to add the expected retention policy.
+			"unexpected retention policy",
-		errs = clues.Stack(errs, clues.NewWC(
+			corsoWrapperAlertNamespace,
-			ctx,
+			"retention-policy",
-			"current snapshot retention policy doesn't match default"))
+			alertName,
+			nil))
 	}
 
 	if globalPol.SchedulingPolicy.Interval() != defaultSchedulingInterval {
-		errs = clues.Stack(errs, clues.NewWC(
+		errs.AddAlert(ctx, fault.NewAlert(
-			ctx,
+			"unexpected scheduling interval",
-			"current scheduling policy doesn't match default").
+			corsoWrapperAlertNamespace,
-			With(
+			"scheduling-interval",
-				"expected_scheduling_policy", defaultSchedulingInterval))
+			alertName,
+			nil))
 	}
-
-	return errs.OrNil()
 }
 
-func (w *conn) verifyRetentionConfig(ctx context.Context) error {
+func (w *conn) verifyRetentionConfig(
+	ctx context.Context,
+	errs *fault.Bus,
+) {
+	const alertName = "kopia-object-locking"
+
 	directRepo, ok := w.Repository.(repo.DirectRepository)
 	if !ok {
-		return clues.NewWC(ctx, "getting repo handle")
+		errs.AddAlert(ctx, fault.NewAlert(
+			"",
+			corsoWrapperAlertNamespace,
+			"fetch-direct-repo",
+			alertName,
+			nil))
+
+		return
 	}
 
 	blobConfig, maintenanceParams, err := getRetentionConfigs(ctx, directRepo)
 	if err != nil {
-		return clues.Stack(err)
+		errs.AddAlert(ctx, fault.NewAlert(
+			err.Error(),
+			corsoWrapperAlertNamespace,
+			"fetch-config",
+			alertName,
+			nil))
+
+		return
 	}
 
-	return clues.Stack(retention.OptsFromConfigs(
+	err = retention.OptsFromConfigs(*blobConfig, *maintenanceParams).
-		*blobConfig,
+		Verify(ctx)
-		*maintenanceParams).Verify(ctx)).OrNil()
+	if err != nil {
+		errs.AddAlert(ctx, fault.NewAlert(
+			err.Error(),
+			corsoWrapperAlertNamespace,
+			"config-values",
+			alertName,
+			nil))
+	}
 }
 
 // verifyDefaultConfigOptions checks the following configurations:
@@ -802,9 +843,10 @@ func (w *conn) verifyRetentionConfig(ctx context.Context) error {
 // object locking:
 //   - maintenance and blob config blob parameters are consistent (i.e. all
 //     enabled or all disabled)
-func (w *conn) verifyDefaultConfigOptions(ctx context.Context) error {
+func (w *conn) verifyDefaultConfigOptions(
-	errs := clues.Stack(w.verifyDefaultPolicyConfigOptions(ctx))
+	ctx context.Context,
-	errs = clues.Stack(errs, w.verifyRetentionConfig(ctx))
+	errs *fault.Bus,
-
+) {
-	return errs.OrNil()
+	w.verifyDefaultPolicyConfigOptions(ctx, errs)
+	w.verifyRetentionConfig(ctx, errs)
 }

src/internal/kopia/conn_test.go

@@ -20,6 +20,7 @@ import (
 	strTD "github.com/alcionai/corso/src/internal/common/str/testdata"
 	"github.com/alcionai/corso/src/internal/tester"
 	"github.com/alcionai/corso/src/pkg/control/repository"
+	"github.com/alcionai/corso/src/pkg/fault"
 	"github.com/alcionai/corso/src/pkg/storage"
 	storeTD "github.com/alcionai/corso/src/pkg/storage/testdata"
 )
@@ -788,14 +789,13 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 	nonzeroOpt := policy.OptionalInt(42)
 
 	table := []struct {
-		name      string
+		name         string
-		setupRepo func(context.Context, *testing.T, *conn)
+		setupRepo    func(context.Context, *testing.T, *conn)
-		expectErr assert.ErrorAssertionFunc
+		expectAlerts int
 	}{
 		{
 			name:      "ValidConfigs NoRetention",
 			setupRepo: func(context.Context, *testing.T, *conn) {},
-			expectErr: assert.NoError,
 		},
 		{
 			name: "ValidConfigs Retention",
@@ -809,7 +809,6 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 					})
 				require.NoError(t, err, clues.ToCore(err))
 			},
-			expectErr: assert.NoError,
 		},
 		{
 			name: "ValidRetentionButNotExtending",
@@ -823,7 +822,7 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 					})
 				require.NoError(t, err, clues.ToCore(err))
 			},
-			expectErr: assert.Error,
+			expectAlerts: 1,
 		},
 		{
 			name: "ExtendingRetentionButNotConfigured",
@@ -835,7 +834,7 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 					})
 				require.NoError(t, err, clues.ToCore(err))
 			},
-			expectErr: assert.Error,
+			expectAlerts: 1,
 		},
 		{
 			name: "NonZeroScheduleInterval",
@@ -848,7 +847,7 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 				err = con.writeGlobalPolicy(ctx, "test", pol)
 				require.NoError(t, err, clues.ToCore(err))
 			},
-			expectErr: assert.Error,
+			expectAlerts: 1,
 		},
 		{
 			name: "NonDefaultCompression",
@@ -862,7 +861,7 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 				err = con.writeGlobalPolicy(ctx, "test", pol)
 				require.NoError(t, err, clues.ToCore(err))
 			},
-			expectErr: assert.Error,
+			expectAlerts: 1,
 		},
 		{
 			name: "NonZeroSnapshotRetentionLatest",
@@ -883,7 +882,7 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 				err = con.writeGlobalPolicy(ctx, "test", pol)
 				require.NoError(t, err, clues.ToCore(err))
 			},
-			expectErr: assert.Error,
+			expectAlerts: 1,
 		},
 		{
 			name: "NonZeroSnapshotRetentionHourly",
@@ -904,7 +903,7 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 				err = con.writeGlobalPolicy(ctx, "test", pol)
 				require.NoError(t, err, clues.ToCore(err))
 			},
-			expectErr: assert.Error,
+			expectAlerts: 1,
 		},
 		{
 			name: "NonZeroSnapshotRetentionWeekly",
@@ -925,7 +924,7 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 				err = con.writeGlobalPolicy(ctx, "test", pol)
 				require.NoError(t, err, clues.ToCore(err))
 			},
-			expectErr: assert.Error,
+			expectAlerts: 1,
 		},
 		{
 			name: "NonZeroSnapshotRetentionDaily",
@@ -946,7 +945,7 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 				err = con.writeGlobalPolicy(ctx, "test", pol)
 				require.NoError(t, err, clues.ToCore(err))
 			},
-			expectErr: assert.Error,
+			expectAlerts: 1,
 		},
 		{
 			name: "NonZeroSnapshotRetentionMonthly",
@@ -967,7 +966,7 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 				err = con.writeGlobalPolicy(ctx, "test", pol)
 				require.NoError(t, err, clues.ToCore(err))
 			},
-			expectErr: assert.Error,
+			expectAlerts: 1,
 		},
 		{
 			name: "NonZeroSnapshotRetentionAnnual",
@@ -988,7 +987,32 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 				err = con.writeGlobalPolicy(ctx, "test", pol)
 				require.NoError(t, err, clues.ToCore(err))
 			},
-			expectErr: assert.Error,
+			expectAlerts: 1,
+		},
+		{
+			name: "MultipleAlerts",
+			setupRepo: func(ctx context.Context, t *testing.T, con *conn) {
+				err := con.setRetentionParameters(
+					ctx,
+					repository.Retention{
+						Mode:     ptr.To(repository.GovernanceRetention),
+						Duration: ptr.To(48 * time.Hour),
+						Extend:   ptr.To(false),
+					})
+				require.NoError(t, err, clues.ToCore(err))
+
+				pol, err := con.getGlobalPolicyOrEmpty(ctx)
+				require.NoError(t, err, clues.ToCore(err))
+
+				updateSchedulingOnPolicy(time.Hour, pol)
+
+				_, err = updateCompressionOnPolicy("pgzip-best-speed", pol)
+				require.NoError(t, err, clues.ToCore(err))
+
+				err = con.writeGlobalPolicy(ctx, "test", pol)
+				require.NoError(t, err, clues.ToCore(err))
+			},
+			expectAlerts: 3,
 		},
 	}
 
@@ -1010,8 +1034,13 @@ func (suite *ConnRetentionIntegrationSuite) TestVerifyDefaultConfigOptions() {
 
 			test.setupRepo(ctx, t, con)
 
-			err = con.verifyDefaultConfigOptions(ctx)
+			errs := fault.New(true)
-			test.expectErr(t, err, clues.ToCore(err))
+			con.verifyDefaultConfigOptions(ctx, errs)
+
+			// There shouldn't be any reported failures because this is just to check
+			// if things are alright.
+			assert.NoError(t, errs.Failure(), clues.ToCore(errs.Failure()))
+			assert.Len(t, errs.Alerts(), test.expectAlerts)
 		})
 	}
 }