chore(pkger): handle edge cases for endpoints that manifest from user…

… interaction references: #17434
influxdata · May 6, 2020 · 9032510 · 9032510
1 parent b4b5257
commit 9032510
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 32 deletions.
diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go
@@ -1249,6 +1249,43 @@ func TestLauncher_Pkger(t *testing.T) {
 					})
 				})
 			})
+
+			t.Run("when a user has deleted a notification endpoint that was previously created by a stack", func(t *testing.T) {
+				testUserDeletedEndpoint := func(t *testing.T, actionFn func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary)) {
+					t.Helper()
+
+					obj := newEndpointHTTP("endpoint-1", "", "")
+					stackID, cleanup, initialSum := initializeStackPkg(t, newPkg(obj))
+					defer cleanup()
+
+					require.Len(t, initialSum.NotificationEndpoints, 1)
+					require.NotZero(t, initialSum.NotificationEndpoints[0].NotificationEndpoint.GetID())
+					resourceCheck.mustDeleteEndpoint(t, initialSum.NotificationEndpoints[0].NotificationEndpoint.GetID())
+
+					actionFn(t, stackID, obj, initialSum)
+				}
+
+				t.Run("should create new resource when attempting to update", func(t *testing.T) {
+					testUserDeletedEndpoint(t, func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary) {
+						pkg := newPkg(initialObj)
+						updateSum, _, err := svc.Apply(ctx, l.Org.ID, l.User.ID, pkg, pkger.ApplyWithStackID(stackID))
+						require.NoError(t, err)
+
+						require.Len(t, updateSum.NotificationEndpoints, 1)
+						initial, updated := initialSum.NotificationEndpoints[0].NotificationEndpoint, updateSum.NotificationEndpoints[0].NotificationEndpoint
+						assert.NotEqual(t, initial.GetID(), updated.GetID())
+						initial.SetID(0)
+						updated.SetID(0)
+						assert.Equal(t, initial, updated)
+					})
+				})
+
+				t.Run("should not error when attempting to remove", func(t *testing.T) {
+					testUserDeletedEndpoint(t, func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary) {
+						testValidRemoval(t, stackID)
+					})
+				})
+			})
 		})
 	})
 
@@ -2790,6 +2827,13 @@ func (r resourceChecker) mustGetEndpoint(t *testing.T, getOpt getResourceOptFn)
 	return e
 }
 
+func (r resourceChecker) mustDeleteEndpoint(t *testing.T, id influxdb.ID) {
+	t.Helper()
+
+	_, _, err := r.tl.NotificationEndpointService(t).DeleteNotificationEndpoint(ctx, id)
+	require.NoError(t, err)
+}
+
 func (r resourceChecker) getLabel(t *testing.T, getOpt getResourceOptFn) (influxdb.Label, error) {
 	t.Helper()
 

diff --git a/pkger/service.go b/pkger/service.go
@@ -1340,17 +1340,15 @@ func (s *Service) applyBuckets(ctx context.Context, buckets []*stateBucket) appl
 
 func (s *Service) rollbackBuckets(ctx context.Context, buckets []*stateBucket) error {
 	rollbackFn := func(b *stateBucket) error {
+		if !IsNew(b.stateStatus) && b.existing == nil {
+			return nil
+		}
+
 		var err error
 		switch {
 		case IsRemoval(b.stateStatus):
-			if b.existing == nil {
-				return nil
-			}
 			err = ierrors.Wrap(s.bucketSVC.CreateBucket(ctx, b.existing), "rolling back removed bucket")
 		case IsExisting(b.stateStatus):
-			if b.existing == nil {
-				return nil
-			}
 			_, err = s.bucketSVC.UpdateBucket(ctx, b.ID(), influxdb.BucketUpdate{
 				Description:     &b.existing.Description,
 				RetentionPeriod: &b.existing.RetentionPeriod,
@@ -1796,29 +1794,31 @@ func (s *Service) applyNotificationEndpoints(ctx context.Context, userID influxd
 		}
 
 		mutex.Do(func() {
-			endpoints[i].id = influxEndpoint.GetID()
-			for _, secret := range influxEndpoint.SecretFields() {
-				switch {
-				case strings.HasSuffix(secret.Key, "-routing-key"):
-					if endpoints[i].parserEndpoint.routingKey == nil {
-						endpoints[i].parserEndpoint.routingKey = new(references)
-					}
-					endpoints[i].parserEndpoint.routingKey.Secret = secret.Key
-				case strings.HasSuffix(secret.Key, "-token"):
-					if endpoints[i].parserEndpoint.token == nil {
-						endpoints[i].parserEndpoint.token = new(references)
-					}
-					endpoints[i].parserEndpoint.token.Secret = secret.Key
-				case strings.HasSuffix(secret.Key, "-username"):
-					if endpoints[i].parserEndpoint.username == nil {
-						endpoints[i].parserEndpoint.username = new(references)
+			if influxEndpoint != nil {
+				endpoints[i].id = influxEndpoint.GetID()
+				for _, secret := range influxEndpoint.SecretFields() {
+					switch {
+					case strings.HasSuffix(secret.Key, "-routing-key"):
+						if endpoints[i].parserEndpoint.routingKey == nil {
+							endpoints[i].parserEndpoint.routingKey = new(references)
+						}
+						endpoints[i].parserEndpoint.routingKey.Secret = secret.Key
+					case strings.HasSuffix(secret.Key, "-token"):
+						if endpoints[i].parserEndpoint.token == nil {
+							endpoints[i].parserEndpoint.token = new(references)
+						}
+						endpoints[i].parserEndpoint.token.Secret = secret.Key
+					case strings.HasSuffix(secret.Key, "-username"):
+						if endpoints[i].parserEndpoint.username == nil {
+							endpoints[i].parserEndpoint.username = new(references)
+						}
+						endpoints[i].parserEndpoint.username.Secret = secret.Key
+					case strings.HasSuffix(secret.Key, "-password"):
+						if endpoints[i].parserEndpoint.password == nil {
+							endpoints[i].parserEndpoint.password = new(references)
+						}
+						endpoints[i].parserEndpoint.password.Secret = secret.Key
 					}
-					endpoints[i].parserEndpoint.username.Secret = secret.Key
-				case strings.HasSuffix(secret.Key, "-password"):
-					if endpoints[i].parserEndpoint.password == nil {
-						endpoints[i].parserEndpoint.password = new(references)
-					}
-					endpoints[i].parserEndpoint.password.Secret = secret.Key
 				}
 			}
 			rollbackEndpoints = append(rollbackEndpoints, endpoints[i])
@@ -1845,14 +1845,14 @@ func (s *Service) applyNotificationEndpoints(ctx context.Context, userID influxd
 }
 
 func (s *Service) applyNotificationEndpoint(ctx context.Context, e *stateEndpoint, userID influxdb.ID) (influxdb.NotificationEndpoint, error) {
-	switch e.stateStatus {
-	case StateStatusRemove:
+	switch {
+	case IsRemoval(e.stateStatus):
 		_, _, err := s.endpointSVC.DeleteNotificationEndpoint(ctx, e.ID())
-		if err != nil {
+		if err != nil && influxdb.ErrorCode(err) != influxdb.ENotFound {
 			return nil, err
 		}
 		return e.existing, nil
-	case StateStatusExists:
+	case IsExisting(e.stateStatus) && e.existing != nil:
 		// stub out userID since we're always using hte http client which will fill it in for us with the token
 		// feels a bit broken that is required.
 		// TODO: look into this userID requirement
@@ -1875,6 +1875,9 @@ func (s *Service) applyNotificationEndpoint(ctx context.Context, e *stateEndpoin
 
 func (s *Service) rollbackNotificationEndpoints(ctx context.Context, userID influxdb.ID, endpoints []*stateEndpoint) error {
 	rollbackFn := func(e *stateEndpoint) error {
+		if !IsNew(e.stateStatus) && e.existing == nil {
+			return nil
+		}
 		var err error
 		switch e.stateStatus {
 		case StateStatusRemove: