From 7f8aa4c40f26710855b76a25474b034f4626a899 Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 15:31:24 -0700 Subject: [PATCH] chore(pkger): handle edge cases for endpoints that manifest from user interaction references: #17434 --- cmd/influxd/launcher/pkger_test.go | 44 ++++++++++++++++++++ pkger/service.go | 67 ++++++++++++++++-------------- 2 files changed, 79 insertions(+), 32 deletions(-) diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go index b03c2af74b4..e888052d2a1 100644 --- 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 index 26ff29cc9d0..2a20fdd4950 100644 --- 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: