From cef760234f43c38b2fa5b76272d573d699d03570 Mon Sep 17 00:00:00 2001 From: Daniel Campbell Date: Thu, 23 Apr 2020 12:20:36 -0700 Subject: [PATCH 01/24] chore(ui): move org nav items --- ui/src/pageLayout/components/UserWidget.tsx | 37 +++++++++++ .../constants/navigationHierarchy.ts | 64 ------------------- 2 files changed, 37 insertions(+), 64 deletions(-) diff --git a/ui/src/pageLayout/components/UserWidget.tsx b/ui/src/pageLayout/components/UserWidget.tsx index 37ff4528dc..d263b3c8cf 100644 --- a/ui/src/pageLayout/components/UserWidget.tsx +++ b/ui/src/pageLayout/components/UserWidget.tsx @@ -16,6 +16,7 @@ import { CLOUD_URL, CLOUD_USAGE_PATH, CLOUD_BILLING_PATH, + CLOUD_USERS_PATH, } from 'src/shared/constants' // Types @@ -24,6 +25,7 @@ import {MeState} from 'src/shared/reducers/me' // Selectors import {getOrg} from 'src/organizations/selectors' +import {getNavItemActivation} from '../utils' interface StateProps { org: Organization @@ -51,6 +53,8 @@ const UserWidget: FC = ({ handleShowOverlay('switch-organizations', {}, handleDismissOverlay) } + const orgPrefix = `/orgs/${org.id}` + return ( @@ -71,8 +75,41 @@ const UserWidget: FC = ({ /> )} /> + ( + + )} + /> + ( + + )} + /> + ( + + )} + /> + ( + + )} + /> { }, activeKeywords: ['data-explorer'], }, - { - id: 'org', - testID: 'nav-item-org', - icon: IconFont.UsersTrio, - label: 'Organization', - shortLabel: 'Org', - link: { - type: 'link', - location: `${orgPrefix}/members`, - }, - cloudExclude: true, - activeKeywords: ['members', 'about'], - menu: [ - { - id: 'members', - testID: 'nav-subitem-members', - label: 'Members', - link: { - type: 'link', - location: `${orgPrefix}/members`, - }, - }, - { - id: 'about', - testID: 'nav-subitem-about', - label: 'About', - link: { - type: 'link', - location: `${orgPrefix}/about`, - }, - }, - ], - }, - { - id: 'org-quartz', - testID: 'nav-item-quartz-org', - icon: IconFont.UsersTrio, - label: 'Organization', - shortLabel: 'Org', - cloudOnly: true, - link: quartzMembersHeaderLink, - activeKeywords: ['members', 'about'], - menu: [ - { - id: 'users', - testID: 'nav-subitem-users', - label: 'Members', - featureFlag: 'multiUser', - link: { - type: 'href', - location: `${CLOUD_URL}/organizations/${orgID}${CLOUD_USERS_PATH}`, - }, - }, - { - id: 'about', - testID: 'nav-subitem-about', - label: 'About', - link: { - type: 'link', - location: `${orgPrefix}/about`, - }, - }, - ], - }, { id: 'dashboards', testID: 'nav-item-dashboards', From 0b5a6b68d618287aebca3b2a9c033ef04a773618 Mon Sep 17 00:00:00 2001 From: Daniel Campbell Date: Thu, 23 Apr 2020 12:30:15 -0700 Subject: [PATCH 02/24] fix(ui): about url --- ui/src/pageLayout/components/UserWidget.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/pageLayout/components/UserWidget.tsx b/ui/src/pageLayout/components/UserWidget.tsx index d263b3c8cf..2d7ba97733 100644 --- a/ui/src/pageLayout/components/UserWidget.tsx +++ b/ui/src/pageLayout/components/UserWidget.tsx @@ -89,7 +89,7 @@ const UserWidget: FC = ({ id="about" label="About" linkElement={className => ( - + )} /> From a86143a000729def52b4af316c6e136f9faefd8b Mon Sep 17 00:00:00 2001 From: Daniel Campbell Date: Thu, 23 Apr 2020 13:27:45 -0700 Subject: [PATCH 03/24] chore(ui): rename members tab to users in cloud --- ui/src/organizations/components/OrgNavigation.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/organizations/components/OrgNavigation.tsx b/ui/src/organizations/components/OrgNavigation.tsx index bc78bd44e3..dc5cd975d9 100644 --- a/ui/src/organizations/components/OrgNavigation.tsx +++ b/ui/src/organizations/components/OrgNavigation.tsx @@ -44,7 +44,7 @@ class OrgNavigation extends PureComponent { link: `/orgs/${orgID}/members`, }, { - text: 'Members', + text: 'Users', id: 'members-quartz', cloudOnly: true, link: `${CLOUD_URL}/organizations/${orgID}${CLOUD_USERS_PATH}`, From 08e0c74e52df565bbf0c84b14518b0600c796736 Mon Sep 17 00:00:00 2001 From: Daniel Campbell Date: Thu, 23 Apr 2020 13:37:36 -0700 Subject: [PATCH 04/24] chore(ui): move multiUser feature flag --- ui/src/pageLayout/components/UserWidget.tsx | 23 +++++++++++-------- .../constants/navigationHierarchy.ts | 12 ---------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/ui/src/pageLayout/components/UserWidget.tsx b/ui/src/pageLayout/components/UserWidget.tsx index 2d7ba97733..3b9b3c3e75 100644 --- a/ui/src/pageLayout/components/UserWidget.tsx +++ b/ui/src/pageLayout/components/UserWidget.tsx @@ -26,6 +26,7 @@ import {MeState} from 'src/shared/reducers/me' // Selectors import {getOrg} from 'src/organizations/selectors' import {getNavItemActivation} from '../utils' +import {FeatureFlag} from 'src/shared/utils/featureFlag' interface StateProps { org: Organization @@ -75,16 +76,18 @@ const UserWidget: FC = ({ /> )} /> - ( - - )} - /> + + ( + + )} + /> + { const orgPrefix = `/orgs/${orgID}` - const isMultiUserEnabled = isFlagEnabled('multiUser') - - const quartzMembersHeaderLink: NavItemLink = isMultiUserEnabled - ? { - type: 'href', - location: `${CLOUD_URL}/organizations/${orgID}${CLOUD_USERS_PATH}`, - } - : { - type: 'link', - location: `${orgPrefix}/about`, - } - return [ { id: 'load-data', From fd8dc3544abd65ab02dd0f35265e34cbf7b8bb2b Mon Sep 17 00:00:00 2001 From: Daniel Campbell Date: Thu, 23 Apr 2020 13:38:31 -0700 Subject: [PATCH 05/24] fix(ui): remove unused imports --- ui/src/pageLayout/constants/navigationHierarchy.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/ui/src/pageLayout/constants/navigationHierarchy.ts b/ui/src/pageLayout/constants/navigationHierarchy.ts index 25db832b7c..b2e9cf3b98 100644 --- a/ui/src/pageLayout/constants/navigationHierarchy.ts +++ b/ui/src/pageLayout/constants/navigationHierarchy.ts @@ -1,6 +1,4 @@ import {IconFont} from '@influxdata/clockface' -import {CLOUD_URL, CLOUD_USERS_PATH} from 'src/shared/constants' -import {isFlagEnabled} from 'src/shared/utils/featureFlag' export interface NavItemLink { type: 'link' | 'href' From 856b069886b769b4c72a92ff185c9349f6c25081 Mon Sep 17 00:00:00 2001 From: Daniel Campbell Date: Thu, 23 Apr 2020 13:45:46 -0700 Subject: [PATCH 06/24] chore(influxdb): upadte changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfae6b2fd0..e336d16983 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### UI Improvements +1. [17849](https://github.com/influxdata/influxdb/pull/17849): Move Organization navigation items to user menu. 1. [17714](https://github.com/influxdata/influxdb/pull/17714): Cloud environments no longer render markdown images, for security reasons. 1. [17321](https://github.com/influxdata/influxdb/pull/17321): Improve UI for sorting resources 1. [17740](https://github.com/influxdata/influxdb/pull/17740): Add single-color color schemes for visualizations From 862a727dd2f293589ac8db1e68f465e596db3e68 Mon Sep 17 00:00:00 2001 From: Daniel Campbell Date: Fri, 24 Apr 2020 09:47:27 -0700 Subject: [PATCH 07/24] chore(ui): update changelog order --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e336d16983..64752f689c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,10 +10,10 @@ ### UI Improvements -1. [17849](https://github.com/influxdata/influxdb/pull/17849): Move Organization navigation items to user menu. 1. [17714](https://github.com/influxdata/influxdb/pull/17714): Cloud environments no longer render markdown images, for security reasons. 1. [17321](https://github.com/influxdata/influxdb/pull/17321): Improve UI for sorting resources 1. [17740](https://github.com/influxdata/influxdb/pull/17740): Add single-color color schemes for visualizations +1. [17849](https://github.com/influxdata/influxdb/pull/17849): Move Organization navigation items to user menu. ## v2.0.0-beta.8 [2020-04-10] From 5028b8a9698102e8cbf1e61836dcd020a3cc2b57 Mon Sep 17 00:00:00 2001 From: Daniel Campbell Date: Fri, 24 Apr 2020 10:01:53 -0700 Subject: [PATCH 08/24] fix(tests): update test-ids --- ui/cypress/e2e/orgs.test.ts | 3 ++- ui/src/pageLayout/components/UserWidget.tsx | 11 ++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/ui/cypress/e2e/orgs.test.ts b/ui/cypress/e2e/orgs.test.ts index c0e10cc851..5f749de07e 100644 --- a/ui/cypress/e2e/orgs.test.ts +++ b/ui/cypress/e2e/orgs.test.ts @@ -27,7 +27,8 @@ describe('Orgs', () => { }) it('should be able to rename the org', () => { const extraText = '_my_renamed_org_in_e2e' - cy.getByTestID('nav-item-org').click() + cy.getByTestID('user-nav').click() + cy.getByTestID('user-nav-item-about').click() cy.get('span:contains("About")').click() cy.get('span:contains("Rename")').click() cy.get('button.cf-button.cf-button-danger').click() diff --git a/ui/src/pageLayout/components/UserWidget.tsx b/ui/src/pageLayout/components/UserWidget.tsx index 3b9b3c3e75..52873cfd08 100644 --- a/ui/src/pageLayout/components/UserWidget.tsx +++ b/ui/src/pageLayout/components/UserWidget.tsx @@ -57,11 +57,12 @@ const UserWidget: FC = ({ const orgPrefix = `/orgs/${org.id}` return ( - + ( )} @@ -69,6 +70,7 @@ const UserWidget: FC = ({ ( = ({ ( = ({ ( )} @@ -100,6 +104,7 @@ const UserWidget: FC = ({ ( @@ -108,6 +113,7 @@ const UserWidget: FC = ({ ( @@ -116,11 +122,13 @@ const UserWidget: FC = ({ ( )} @@ -129,6 +137,7 @@ const UserWidget: FC = ({ } /> From 9ab44476170a651a46672f8338f01e72e94c4e29 Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 11:06:39 -0700 Subject: [PATCH 09/24] chore(pkger): ensure sorting is done by PkgName for all parser resource summaries --- pkger/parser.go | 14 ++++---- pkger/parser_models.go | 2 +- pkger/parser_test.go | 76 +++++++++++++++++++++--------------------- 3 files changed, 46 insertions(+), 46 deletions(-) diff --git a/pkger/parser.go b/pkger/parser.go index 42e63f8d46..9eb999c179 100644 --- a/pkger/parser.go +++ b/pkger/parser.go @@ -530,7 +530,7 @@ func (p *Pkg) buckets() []*bucket { buckets = append(buckets, b) } - sort.Slice(buckets, func(i, j int) bool { return buckets[i].Name() < buckets[j].Name() }) + sort.Slice(buckets, func(i, j int) bool { return buckets[i].PkgName() < buckets[j].PkgName() }) return buckets } @@ -541,7 +541,7 @@ func (p *Pkg) checks() []*check { checks = append(checks, c) } - sort.Slice(checks, func(i, j int) bool { return checks[i].Name() < checks[j].Name() }) + sort.Slice(checks, func(i, j int) bool { return checks[i].PkgName() < checks[j].PkgName() }) return checks } @@ -574,7 +574,7 @@ func (p *Pkg) notificationEndpoints() []*notificationEndpoint { sort.Slice(endpoints, func(i, j int) bool { ei, ej := endpoints[i], endpoints[j] if ei.kind == ej.kind { - return ei.Name() < ej.Name() + return ei.PkgName() < ej.PkgName() } return ei.kind < ej.kind }) @@ -586,7 +586,7 @@ func (p *Pkg) notificationRules() []*notificationRule { for _, r := range p.mNotificationRules { rules = append(rules, r) } - sort.Slice(rules, func(i, j int) bool { return rules[i].Name() < rules[j].Name() }) + sort.Slice(rules, func(i, j int) bool { return rules[i].PkgName() < rules[j].PkgName() }) return rules } @@ -618,7 +618,7 @@ func (p *Pkg) tasks() []*task { tasks = append(tasks, t) } - sort.Slice(tasks, func(i, j int) bool { return tasks[i].Name() < tasks[j].Name() }) + sort.Slice(tasks, func(i, j int) bool { return tasks[i].PkgName() < tasks[j].PkgName() }) return tasks } @@ -630,7 +630,7 @@ func (p *Pkg) telegrafs() []*telegraf { teles = append(teles, t) } - sort.Slice(teles, func(i, j int) bool { return teles[i].Name() < teles[j].Name() }) + sort.Slice(teles, func(i, j int) bool { return teles[i].PkgName() < teles[j].PkgName() }) return teles } @@ -641,7 +641,7 @@ func (p *Pkg) variables() []*variable { vars = append(vars, v) } - sort.Slice(vars, func(i, j int) bool { return vars[i].Name() < vars[j].Name() }) + sort.Slice(vars, func(i, j int) bool { return vars[i].PkgName() < vars[j].PkgName() }) return vars } diff --git a/pkger/parser_models.go b/pkger/parser_models.go index 01dd54bd93..5276a3875d 100644 --- a/pkger/parser_models.go +++ b/pkger/parser_models.go @@ -1199,7 +1199,7 @@ func (s sortedLabels) Len() int { } func (s sortedLabels) Less(i, j int) bool { - return s[i].Name() < s[j].Name() + return s[i].PkgName() < s[j].PkgName() } func (s sortedLabels) Swap(i, j int) { diff --git a/pkger/parser_test.go b/pkger/parser_test.go index a92a6c2cd5..79618afa0b 100644 --- a/pkger/parser_test.go +++ b/pkger/parser_test.go @@ -27,19 +27,19 @@ func TestParse(t *testing.T) { actual := buckets[0] expectedBucket := SummaryBucket{ - PkgName: "rucket-22", - Name: "display name", - Description: "bucket 2 description", + PkgName: "rucket-11", + Name: "rucket-11", + Description: "bucket 1 description", + RetentionPeriod: time.Hour, LabelAssociations: []SummaryLabel{}, } assert.Equal(t, expectedBucket, actual) actual = buckets[1] expectedBucket = SummaryBucket{ - PkgName: "rucket-11", - Name: "rucket-11", - Description: "bucket 1 description", - RetentionPeriod: time.Hour, + PkgName: "rucket-22", + Name: "display name", + Description: "bucket 2 description", LabelAssociations: []SummaryLabel{}, } assert.Equal(t, expectedBucket, actual) @@ -160,19 +160,7 @@ spec: labels := pkg.Summary().Labels require.Len(t, labels, 3) - expectedLabel0 := SummaryLabel{ - PkgName: "label-3", - Name: "display name", - Properties: struct { - Color string `json:"color"` - Description string `json:"description"` - }{ - Description: "label 3 description", - }, - } - assert.Equal(t, expectedLabel0, labels[0]) - - expectedLabel1 := SummaryLabel{ + expectedLabel := SummaryLabel{ PkgName: "label-1", Name: "label-1", Properties: struct { @@ -183,9 +171,9 @@ spec: Description: "label 1 description", }, } - assert.Equal(t, expectedLabel1, labels[1]) + assert.Equal(t, expectedLabel, labels[0]) - expectedLabel2 := SummaryLabel{ + expectedLabel = SummaryLabel{ PkgName: "label-2", Name: "label-2", Properties: struct { @@ -196,7 +184,19 @@ spec: Description: "label 2 description", }, } - assert.Equal(t, expectedLabel2, labels[2]) + assert.Equal(t, expectedLabel, labels[1]) + + expectedLabel = SummaryLabel{ + PkgName: "label-3", + Name: "display name", + Properties: struct { + Color string `json:"color"` + Description string `json:"description"` + }{ + Description: "label 3 description", + }, + } + assert.Equal(t, expectedLabel, labels[2]) }) }) @@ -3669,6 +3669,21 @@ spec: assert.Equal(t, vals, v.Arguments.Values) } + // validates we support all known variable types + varEquals(t, + "var-const-3", + "constant", + influxdb.VariableConstantValues([]string{"first val"}), + sum.Variables[0], + ) + + varEquals(t, + "var-map-4", + "map", + influxdb.VariableMapValues{"k1": "v1"}, + sum.Variables[1], + ) + varEquals(t, "query var", "query", @@ -3676,21 +3691,6 @@ spec: Query: `buckets() |> filter(fn: (r) => r.name !~ /^_/) |> rename(columns: {name: "_value"}) |> keep(columns: ["_value"])`, Language: "flux", }, - sum.Variables[0], - ) - - // validates we support all known variable types - varEquals(t, - "var-const-3", - "constant", - influxdb.VariableConstantValues([]string{"first val"}), - sum.Variables[1], - ) - - varEquals(t, - "var-map-4", - "map", - influxdb.VariableMapValues{"k1": "v1"}, sum.Variables[2], ) From f575ea8d41ba01b2715357101b2833a23f2d3fdd Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 11:09:16 -0700 Subject: [PATCH 10/24] chore(pkger): handle edge cases that can manifest from users in the platform and stateful pkg applications references: #17434 --- cmd/influxd/launcher/pkger_test.go | 75 +++++++++++++++++++++++++++++- pkger/service.go | 37 ++++++++------- pkger/service_test.go | 12 +++++ 3 files changed, 106 insertions(+), 18 deletions(-) diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go index 3601555d5f..1ef2f790f8 100644 --- a/cmd/influxd/launcher/pkger_test.go +++ b/cmd/influxd/launcher/pkger_test.go @@ -7,6 +7,7 @@ import ( "io/ioutil" nethttp "net/http" "net/http/httptest" + "sort" "testing" "time" @@ -591,7 +592,7 @@ func TestLauncher_Pkger(t *testing.T) { }) }) - t.Run("apply a pkg with a stack", func(t *testing.T) { + t.Run("apply a pkg with a stack and all resources", func(t *testing.T) { testStackApplyFn := func(t *testing.T) (pkger.Summary, pkger.Stack, func()) { t.Helper() @@ -1038,6 +1039,55 @@ func TestLauncher_Pkger(t *testing.T) { } }) }) + + t.Run("apply should handle cases where users have changed platform data", func(t *testing.T) { + t.Run("when a user has deleted a variable that was previously created by a stack", func(t *testing.T) { + testUserDeletedVariable := func(t *testing.T, actionFn func(t *testing.T, stackID influxdb.ID, initialVarObj pkger.Object, initialSum pkger.Summary)) { + t.Helper() + + stack, cleanup := newStackFn(t, pkger.Stack{}) + defer cleanup() + + varObj := newVariableObject("var-1", "", "") + pkg := newPkg(varObj) + initialSum, _, err := svc.Apply(ctx, l.Org.ID, l.User.ID, pkg, pkger.ApplyWithStackID(stack.ID)) + require.NoError(t, err) + + require.Len(t, initialSum.Variables, 1) + require.NotZero(t, initialSum.Variables[0].ID) + resourceCheck.mustDeleteVariable(t, influxdb.ID(initialSum.Variables[0].ID)) + + actionFn(t, stack.ID, varObj, initialSum) + } + + t.Run("should create new resource when attempting to update", func(t *testing.T) { + testUserDeletedVariable(t, func(t *testing.T, stackID influxdb.ID, initialVarObj pkger.Object, initialSum pkger.Summary) { + pkg := newPkg(initialVarObj) + updateSum, _, err := svc.Apply(ctx, l.Org.ID, l.User.ID, pkg, pkger.ApplyWithStackID(stackID)) + require.NoError(t, err) + + require.Len(t, updateSum.Variables, 1) + initVar, updateVar := initialSum.Variables[0], updateSum.Variables[0] + assert.NotEqual(t, initVar.ID, updateVar.ID) + initVar.ID, updateVar.ID = 0, 0 + assert.Equal(t, initVar, updateVar) + }) + }) + + t.Run("should not error when attempting to remove", func(t *testing.T) { + testUserDeletedVariable(t, func(t *testing.T, stackID influxdb.ID, initialVarObj pkger.Object, initialSum pkger.Summary) { + _, _, err := svc.Apply( + ctx, + l.Org.ID, + l.User.ID, + newPkg( /* empty stack to remove prev variable */ ), + pkger.ApplyWithStackID(stackID), + ) + require.NoError(t, err) + }) + }) + }) + }) }) t.Run("errors incurred during application of package rolls back to state before package", func(t *testing.T) { @@ -1386,6 +1436,7 @@ spec: labels := sum.Labels require.Len(t, labels, 2) + sortLabels(labels) assert.Equal(t, "label-1", labels[0].Name) assert.Equal(t, "the 2nd label", labels[1].Name) @@ -1397,6 +1448,7 @@ spec: checks := sum.Checks require.Len(t, checks, 2) + sortChecks(checks) assert.Equal(t, "check 0 name", checks[0].Check.GetName()) hasLabelAssociations(t, checks[0].LabelAssociations, 1, "label-1") assert.Equal(t, "check-1", checks[1].Check.GetName()) @@ -1691,6 +1743,7 @@ spec: labels := newSum.Labels require.Len(t, labels, 2) + sortLabels(labels) assert.Zero(t, labels[0].ID) assert.Equal(t, "label-1", labels[0].Name) assert.Zero(t, labels[1].ID) @@ -1704,6 +1757,7 @@ spec: checks := newSum.Checks require.Len(t, checks, 2) + sortChecks(checks) assert.Equal(t, "check 0 name", checks[0].Check.GetName()) hasLabelAssociations(t, checks[0].LabelAssociations, 1, "label-1") assert.Equal(t, "check-1", checks[1].Check.GetName()) @@ -2774,3 +2828,22 @@ func (r resourceChecker) mustGetVariable(t *testing.T, getOpt getResourceOptFn) require.NoError(t, err) return l } + +func (r resourceChecker) mustDeleteVariable(t *testing.T, id influxdb.ID) { + t.Helper() + + err := r.tl.VariableService(t).DeleteVariable(ctx, id) + require.NoError(t, err) +} + +func sortChecks(checks []pkger.SummaryCheck) { + sort.Slice(checks, func(i, j int) bool { + return checks[i].Check.GetName() < checks[j].Check.GetName() + }) +} + +func sortLabels(labels []pkger.SummaryLabel) { + sort.Slice(labels, func(i, j int) bool { + return labels[i].Name < labels[j].Name + }) +} diff --git a/pkger/service.go b/pkger/service.go index fbd6bd685b..731def6dce 100644 --- a/pkger/service.go +++ b/pkger/service.go @@ -957,11 +957,9 @@ func (s *Service) dryRunVariables(ctx context.Context, orgID influxdb.ID, vars m } for _, v := range vars { - var existing *influxdb.Variable + existing := mNames[v.parserVar.Name()] if v.ID() != 0 { existing = mIDs[v.ID()] - } else { - existing = mNames[v.parserVar.Name()] } if IsNew(v.stateStatus) && existing != nil { v.stateStatus = StateStatusExists @@ -1095,8 +1093,9 @@ func (s *Service) dryRunResourceLabelMapping(ctx context.Context, state *stateCo ResourceID: ident.id, ResourceType: ident.resourceType, }) - if err != nil { - return nil, err + if err != nil && influxdb.ErrorCode(err) != influxdb.ENotFound { + msgFmt := fmt.Sprintf("failed to find labels mappings for %s resource[%q]", ident.resourceType, ident.id) + return nil, ierrors.Wrap(err, msgFmt) } pkgLabels := labelSlcToMap(pkgResourceLabels) @@ -1235,7 +1234,7 @@ func (s *Service) Apply(ctx context.Context, orgID, userID influxdb.ID, pkg *Pkg func (s *Service) applyState(ctx context.Context, coordinator *rollbackCoordinator, orgID, userID influxdb.ID, state *stateCoordinator, missingSecrets map[string]string) (e error) { endpointApp, ruleApp, err := s.applyNotificationGenerator(ctx, userID, state.rules(), state.endpoints()) if err != nil { - return err + return ierrors.Wrap(err, "failed to setup notification generator") } // each grouping here runs for its entirety, then returns an error that @@ -2395,9 +2394,9 @@ func (s *Service) rollbackVariables(ctx context.Context, variables []*stateVaria err = ierrors.Wrap(s.varSVC.CreateVariable(ctx, v.existing), "rolling back removed variable") case StateStatusExists: _, err = s.varSVC.UpdateVariable(ctx, v.ID(), &influxdb.VariableUpdate{ - Name: v.parserVar.Name(), - Description: v.parserVar.Description, - Arguments: v.parserVar.influxVarArgs(), + Name: v.existing.Name, + Description: v.existing.Description, + Arguments: v.existing.Arguments, }) err = ierrors.Wrap(err, "rolling back updated variable") default: @@ -2421,23 +2420,28 @@ func (s *Service) rollbackVariables(ctx context.Context, variables []*stateVaria } func (s *Service) applyVariable(ctx context.Context, v *stateVariable) (influxdb.Variable, error) { - switch v.stateStatus { - case StateStatusRemove: - if err := s.varSVC.DeleteVariable(ctx, v.id); err != nil { - return influxdb.Variable{}, err + switch { + case IsRemoval(v.stateStatus): + if err := s.varSVC.DeleteVariable(ctx, v.id); err != nil && influxdb.ErrorCode(err) != influxdb.ENotFound { + return influxdb.Variable{}, ierrors.Wrap(err, "removing existing variable") + } + if v.existing == nil { + return influxdb.Variable{}, nil } return *v.existing, nil - case StateStatusExists: + case IsExisting(v.stateStatus) && v.existing != nil: updatedVar, err := s.varSVC.UpdateVariable(ctx, v.ID(), &influxdb.VariableUpdate{ Name: v.parserVar.Name(), Description: v.parserVar.Description, Arguments: v.parserVar.influxVarArgs(), }) if err != nil { - return influxdb.Variable{}, err + return influxdb.Variable{}, ierrors.Wrap(err, "updating existing variable") } return *updatedVar, nil default: + // when an existing variable (referenced in stack) has been deleted by a user + // then the resource is created anew to get it back to the expected state. influxVar := influxdb.Variable{ OrganizationID: v.orgID, Name: v.parserVar.Name(), @@ -2446,9 +2450,8 @@ func (s *Service) applyVariable(ctx context.Context, v *stateVariable) (influxdb } err := s.varSVC.CreateVariable(ctx, &influxVar) if err != nil { - return influxdb.Variable{}, err + return influxdb.Variable{}, ierrors.Wrap(err, "creating new variable") } - return influxVar, nil } } diff --git a/pkger/service_test.go b/pkger/service_test.go index c2c208e4fa..5bf6eabfaa 100644 --- a/pkger/service_test.go +++ b/pkger/service_test.go @@ -7,6 +7,7 @@ import ( "fmt" "math/rand" "regexp" + "sort" "strconv" "testing" "time" @@ -1482,6 +1483,12 @@ func TestService(t *testing.T) { } } + sortLabelsByName := func(labels []SummaryLabel) { + sort.Slice(labels, func(i, j int) bool { + return labels[i].Name < labels[j].Name + }) + } + t.Run("with existing resources", func(t *testing.T) { encodeAndDecode := func(t *testing.T, pkg *Pkg) *Pkg { t.Helper() @@ -2853,9 +2860,13 @@ func TestService(t *testing.T) { sum := newPkg.Summary() bkts := sum.Buckets + sort.Slice(bkts, func(i, j int) bool { + return bkts[i].Name < bkts[j].Name + }) require.Len(t, bkts, 2) for i, actual := range bkts { + sortLabelsByName(actual.LabelAssociations) assert.Equal(t, strconv.Itoa((i+1)*10), actual.Name) require.Len(t, actual.LabelAssociations, 2) assert.Equal(t, "label_1", actual.LabelAssociations[0].Name) @@ -2863,6 +2874,7 @@ func TestService(t *testing.T) { } labels := sum.Labels + sortLabelsByName(labels) require.Len(t, labels, 2) assert.Equal(t, "label_1", labels[0].Name) assert.Equal(t, "label_2", labels[1].Name) From ce2baee4d66e1492266647931e308ffe2ed2a0b5 Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 13:19:38 -0700 Subject: [PATCH 11/24] chore(pkger): make export test assertions deterministic --- pkger/service_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkger/service_test.go b/pkger/service_test.go index 5bf6eabfaa..6b7f45815e 100644 --- a/pkger/service_test.go +++ b/pkger/service_test.go @@ -2666,6 +2666,9 @@ func TestService(t *testing.T) { sum := newPkg.Summary() teles := sum.TelegrafConfigs + sort.Slice(teles, func(i, j int) bool { + return teles[i].TelegrafConfig.Name < teles[j].TelegrafConfig.Name + }) require.Len(t, teles, len(resourcesToClone)) for i := range resourcesToClone { From 24dd13bcc67a48db99e28cb548250c83ff4f8e63 Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 13:48:50 -0700 Subject: [PATCH 12/24] chore(pkger): handle edge cases for buckets that manifest from user interaction references: #17434 --- cmd/influxd/launcher/pkger_test.go | 86 +++++++++++++++++++++++++----- pkger/service.go | 38 ++++++++----- 2 files changed, 98 insertions(+), 26 deletions(-) diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go index 1ef2f790f8..1494c0c5b7 100644 --- a/cmd/influxd/launcher/pkger_test.go +++ b/cmd/influxd/launcher/pkger_test.go @@ -1041,23 +1041,47 @@ func TestLauncher_Pkger(t *testing.T) { }) t.Run("apply should handle cases where users have changed platform data", func(t *testing.T) { + initializeStackPkg := func(t *testing.T, pkg *pkger.Pkg) (influxdb.ID, func(), pkger.Summary) { + t.Helper() + + stack, cleanup := newStackFn(t, pkger.Stack{}) + defer func() { + if t.Failed() { + cleanup() + } + }() + + initialSum, _, err := svc.Apply(ctx, l.Org.ID, l.User.ID, pkg, pkger.ApplyWithStackID(stack.ID)) + require.NoError(t, err) + + return stack.ID, cleanup, initialSum + } + + testValidRemoval := func(t *testing.T, stackID influxdb.ID) { + t.Helper() + _, _, err := svc.Apply( + ctx, + l.Org.ID, + l.User.ID, + newPkg( /* empty stack to remove prev resource */ ), + pkger.ApplyWithStackID(stackID), + ) + require.NoError(t, err) + } + t.Run("when a user has deleted a variable that was previously created by a stack", func(t *testing.T) { testUserDeletedVariable := func(t *testing.T, actionFn func(t *testing.T, stackID influxdb.ID, initialVarObj pkger.Object, initialSum pkger.Summary)) { t.Helper() - stack, cleanup := newStackFn(t, pkger.Stack{}) + obj := newVariableObject("var-1", "", "") + stackID, cleanup, initialSum := initializeStackPkg(t, newPkg(obj)) defer cleanup() - varObj := newVariableObject("var-1", "", "") - pkg := newPkg(varObj) - initialSum, _, err := svc.Apply(ctx, l.Org.ID, l.User.ID, pkg, pkger.ApplyWithStackID(stack.ID)) - require.NoError(t, err) - require.Len(t, initialSum.Variables, 1) require.NotZero(t, initialSum.Variables[0].ID) resourceCheck.mustDeleteVariable(t, influxdb.ID(initialSum.Variables[0].ID)) - actionFn(t, stack.ID, varObj, initialSum) + actionFn(t, stackID, obj, initialSum) } t.Run("should create new resource when attempting to update", func(t *testing.T) { @@ -1076,14 +1100,43 @@ func TestLauncher_Pkger(t *testing.T) { t.Run("should not error when attempting to remove", func(t *testing.T) { testUserDeletedVariable(t, func(t *testing.T, stackID influxdb.ID, initialVarObj pkger.Object, initialSum pkger.Summary) { - _, _, err := svc.Apply( - ctx, - l.Org.ID, - l.User.ID, - newPkg( /* empty stack to remove prev variable */ ), - pkger.ApplyWithStackID(stackID), - ) + testValidRemoval(t, stackID) + }) + }) + }) + + t.Run("when a user has deleted a bucket that was previously created by a stack", func(t *testing.T) { + testUserDeletedBucket := func(t *testing.T, actionFn func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary)) { + t.Helper() + + obj := newBucketObject("bucket-1", "", "") + stackID, cleanup, initialSum := initializeStackPkg(t, newPkg(obj)) + defer cleanup() + + require.Len(t, initialSum.Buckets, 1) + require.NotZero(t, initialSum.Buckets[0].ID) + resourceCheck.mustDeleteBucket(t, influxdb.ID(initialSum.Buckets[0].ID)) + + actionFn(t, stackID, obj, initialSum) + } + + t.Run("should create new resource when attempting to update", func(t *testing.T) { + testUserDeletedBucket(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.Buckets, 1) + intial, updated := initialSum.Buckets[0], updateSum.Buckets[0] + assert.NotEqual(t, intial.ID, updated.ID) + intial.ID, updated.ID = 0, 0 + assert.Equal(t, intial, updated) + }) + }) + + t.Run("should not error when attempting to remove", func(t *testing.T) { + testUserDeletedBucket(t, func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary) { + testValidRemoval(t, stackID) }) }) }) @@ -2492,6 +2545,11 @@ func (r resourceChecker) mustGetBucket(t *testing.T, getOpt getResourceOptFn) in return bkt } +func (r resourceChecker) mustDeleteBucket(t *testing.T, id influxdb.ID) { + t.Helper() + require.NoError(t, r.tl.BucketService(t).DeleteBucket(ctx, id)) +} + func (r resourceChecker) getCheck(t *testing.T, getOpt getResourceOptFn) (influxdb.Check, error) { t.Helper() diff --git a/pkger/service.go b/pkger/service.go index 731def6dce..f5f86cece0 100644 --- a/pkger/service.go +++ b/pkger/service.go @@ -1340,14 +1340,19 @@ 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 { var err error - switch b.stateStatus { - case StateStatusRemove: + 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 StateStatusExists: - rp := b.parserBkt.RetentionRules.RP() + case IsExisting(b.stateStatus): + if b.existing == nil { + return nil + } _, err = s.bucketSVC.UpdateBucket(ctx, b.ID(), influxdb.BucketUpdate{ - Description: &b.parserBkt.Description, - RetentionPeriod: &rp, + Description: &b.existing.Description, + RetentionPeriod: &b.existing.RetentionPeriod, }) err = ierrors.Wrap(err, "rolling back existing bucket to previous state") default: @@ -1372,13 +1377,16 @@ func (s *Service) rollbackBuckets(ctx context.Context, buckets []*stateBucket) e } func (s *Service) applyBucket(ctx context.Context, b *stateBucket) (influxdb.Bucket, error) { - switch b.stateStatus { - case StateStatusRemove: + switch { + case IsRemoval(b.stateStatus): if err := s.bucketSVC.DeleteBucket(ctx, b.ID()); err != nil { + if influxdb.ErrorCode(err) == influxdb.ENotFound { + return influxdb.Bucket{}, nil + } return influxdb.Bucket{}, fmt.Errorf("failed to delete bucket[%q]: %w", b.ID(), err) } return *b.existing, nil - case StateStatusExists: + case IsExisting(b.stateStatus) && b.existing != nil: rp := b.parserBkt.RetentionRules.RP() newName := b.parserBkt.Name() influxBucket, err := s.bucketSVC.UpdateBucket(ctx, b.ID(), influxdb.BucketUpdate{ @@ -2389,10 +2397,16 @@ func (s *Service) applyVariables(ctx context.Context, vars []*stateVariable) app func (s *Service) rollbackVariables(ctx context.Context, variables []*stateVariable) error { rollbackFn := func(v *stateVariable) error { var err error - switch v.stateStatus { - case StateStatusRemove: + switch { + case IsRemoval(v.stateStatus): + if v.existing == nil { + return nil + } err = ierrors.Wrap(s.varSVC.CreateVariable(ctx, v.existing), "rolling back removed variable") - case StateStatusExists: + case IsExisting(v.stateStatus): + if v.existing == nil { + return nil + } _, err = s.varSVC.UpdateVariable(ctx, v.ID(), &influxdb.VariableUpdate{ Name: v.existing.Name, Description: v.existing.Description, From 485cc3ea54b955b6f0c6e4fc44c49d80aa00e534 Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 14:37:53 -0700 Subject: [PATCH 13/24] chore(pkger): handle edge cases for checks that manifest from user interaction references: #17434 --- cmd/influxd/launcher/pkger_test.go | 43 ++++++++++++++++++++++++++++++ pkger/service.go | 23 ++++++++++------ 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go index 1494c0c5b7..676bcb969c 100644 --- a/cmd/influxd/launcher/pkger_test.go +++ b/cmd/influxd/launcher/pkger_test.go @@ -1140,6 +1140,43 @@ func TestLauncher_Pkger(t *testing.T) { }) }) }) + + t.Run("when a user has deleted a check that was previously created by a stack", func(t *testing.T) { + testUserDeletedCheck := func(t *testing.T, actionFn func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary)) { + t.Helper() + + obj := newCheckDeadmanObject(t, "check-1", "", time.Hour) + stackID, cleanup, initialSum := initializeStackPkg(t, newPkg(obj)) + defer cleanup() + + require.Len(t, initialSum.Checks, 1) + require.NotZero(t, initialSum.Checks[0].Check.GetID()) + resourceCheck.mustDeleteCheck(t, initialSum.Checks[0].Check.GetID()) + + actionFn(t, stackID, obj, initialSum) + } + + t.Run("should create new resource when attempting to update", func(t *testing.T) { + testUserDeletedCheck(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.Checks, 1) + intial, updated := initialSum.Checks[0].Check, updateSum.Checks[0].Check + assert.NotEqual(t, intial.GetID(), updated.GetID()) + intial.SetID(0) + updated.SetID(0) + assert.Equal(t, intial, updated) + }) + }) + + t.Run("should not error when attempting to remove", func(t *testing.T) { + testUserDeletedCheck(t, func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary) { + testValidRemoval(t, stackID) + }) + }) + }) }) }) @@ -2582,6 +2619,12 @@ func (r resourceChecker) mustGetCheck(t *testing.T, getOpt getResourceOptFn) inf return c } +func (r resourceChecker) mustDeleteCheck(t *testing.T, id influxdb.ID) { + t.Helper() + + require.NoError(t, r.tl.CheckService().DeleteCheck(ctx, id)) +} + func (r resourceChecker) getDashboard(t *testing.T, getOpt getResourceOptFn) (influxdb.Dashboard, error) { t.Helper() diff --git a/pkger/service.go b/pkger/service.go index f5f86cece0..947d5d9f72 100644 --- a/pkger/service.go +++ b/pkger/service.go @@ -12,6 +12,7 @@ import ( "github.com/influxdata/influxdb/v2" ierrors "github.com/influxdata/influxdb/v2/kit/errors" + icheck "github.com/influxdata/influxdb/v2/notification/check" "github.com/influxdata/influxdb/v2/notification/rule" "github.com/influxdata/influxdb/v2/snowflake" "github.com/influxdata/influxdb/v2/task/options" @@ -1458,8 +1459,8 @@ func (s *Service) applyChecks(ctx context.Context, checks []*stateCheck) applier func (s *Service) rollbackChecks(ctx context.Context, checks []*stateCheck) error { rollbackFn := func(c *stateCheck) error { var err error - switch c.stateStatus { - case StateStatusRemove: + switch { + case IsRemoval(c.stateStatus): err = s.checkSVC.CreateCheck( ctx, influxdb.CheckCreate{ @@ -1469,13 +1470,16 @@ func (s *Service) rollbackChecks(ctx context.Context, checks []*stateCheck) erro c.existing.GetOwnerID(), ) c.id = c.existing.GetID() - case StateStatusNew: - err = s.checkSVC.DeleteCheck(ctx, c.ID()) - default: + case IsExisting(c.stateStatus): + if c.existing == nil { + return nil + } _, err = s.checkSVC.UpdateCheck(ctx, c.ID(), influxdb.CheckCreate{ Check: c.summarize().Check, Status: influxdb.Status(c.parserCheck.status), }) + default: + err = s.checkSVC.DeleteCheck(ctx, c.ID()) } return err } @@ -1495,13 +1499,16 @@ func (s *Service) rollbackChecks(ctx context.Context, checks []*stateCheck) erro } func (s *Service) applyCheck(ctx context.Context, c *stateCheck, userID influxdb.ID) (influxdb.Check, error) { - switch c.stateStatus { - case StateStatusRemove: + switch { + case IsRemoval(c.stateStatus): if err := s.checkSVC.DeleteCheck(ctx, c.ID()); err != nil { + if influxdb.ErrorCode(err) == influxdb.ENotFound { + return &icheck.Threshold{Base: icheck.Base{ID: c.ID()}}, nil + } return nil, fmt.Errorf("failed to delete check[%q]: %w", c.ID(), err) } return c.existing, nil - case StateStatusExists: + case IsExisting(c.stateStatus) && c.existing != nil: influxCheck, err := s.checkSVC.UpdateCheck(ctx, c.ID(), influxdb.CheckCreate{ Check: c.summarize().Check, Status: c.parserCheck.Status(), From d4aa3be8dbc88ebb01a86ba1dea8764ab3550c23 Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 14:53:34 -0700 Subject: [PATCH 14/24] chore(pkger): handle edge cases for dashboards that manifest from user interaction references: #17434 --- cmd/influxd/launcher/pkger_test.go | 42 ++++++++++++++++++++++++++++++ pkger/service.go | 19 +++++++++----- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go index 676bcb969c..17fae137a1 100644 --- a/cmd/influxd/launcher/pkger_test.go +++ b/cmd/influxd/launcher/pkger_test.go @@ -1177,6 +1177,42 @@ func TestLauncher_Pkger(t *testing.T) { }) }) }) + + t.Run("when a user has deleted a dashboard that was previously created by a stack", func(t *testing.T) { + testUserDeletedDashboard := func(t *testing.T, actionFn func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary)) { + t.Helper() + + obj := newDashObject("dash-1", "", "") + stackID, cleanup, initialSum := initializeStackPkg(t, newPkg(obj)) + defer cleanup() + + require.Len(t, initialSum.Dashboards, 1) + require.NotZero(t, initialSum.Dashboards[0].ID) + resourceCheck.mustDeleteDashboard(t, influxdb.ID(initialSum.Dashboards[0].ID)) + + actionFn(t, stackID, obj, initialSum) + } + + t.Run("should create new resource when attempting to update", func(t *testing.T) { + testUserDeletedDashboard(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.Dashboards, 1) + initial, updated := initialSum.Dashboards[0], updateSum.Dashboards[0] + assert.NotEqual(t, initial.ID, updated.ID) + initial.ID, updated.ID = 0, 0 + assert.Equal(t, initial, updated) + }) + }) + + t.Run("should not error when attempting to remove", func(t *testing.T) { + testUserDeletedDashboard(t, func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary) { + testValidRemoval(t, stackID) + }) + }) + }) }) }) @@ -2670,6 +2706,12 @@ func (r resourceChecker) mustGetDashboard(t *testing.T, getOpt getResourceOptFn) return dash } +func (r resourceChecker) mustDeleteDashboard(t *testing.T, id influxdb.ID) { + t.Helper() + + require.NoError(t, r.tl.DashboardService(t).DeleteDashboard(ctx, id)) +} + func (r resourceChecker) getEndpoint(t *testing.T, getOpt getResourceOptFn) (influxdb.NotificationEndpoint, error) { t.Helper() diff --git a/pkger/service.go b/pkger/service.go index 947d5d9f72..ef4b570829 100644 --- a/pkger/service.go +++ b/pkger/service.go @@ -1573,13 +1573,16 @@ func (s *Service) applyDashboards(ctx context.Context, dashboards []*stateDashbo } func (s *Service) applyDashboard(ctx context.Context, d *stateDashboard) (influxdb.Dashboard, error) { - switch d.stateStatus { - case StateStatusRemove: + switch { + case IsRemoval(d.stateStatus): if err := s.dashSVC.DeleteDashboard(ctx, d.ID()); err != nil { + if influxdb.ErrorCode(err) == influxdb.ENotFound { + return influxdb.Dashboard{}, nil + } return influxdb.Dashboard{}, fmt.Errorf("failed to delete dashboard[%q]: %w", d.ID(), err) } return *d.existing, nil - case StateStatusExists: + case IsExisting(d.stateStatus) && d.existing != nil: name := d.parserDash.Name() cells := convertChartsToCells(d.parserDash.Charts) dash, err := s.dashSVC.UpdateDashboard(ctx, d.ID(), influxdb.DashboardUpdate{ @@ -1609,11 +1612,15 @@ func (s *Service) applyDashboard(ctx context.Context, d *stateDashboard) (influx func (s *Service) rollbackDashboards(ctx context.Context, dashs []*stateDashboard) error { rollbackFn := func(d *stateDashboard) error { + if !IsNew(d.stateStatus) && d.existing == nil { + return nil + } + var err error - switch d.stateStatus { - case StateStatusRemove: + switch { + case IsRemoval(d.stateStatus): err = ierrors.Wrap(s.dashSVC.CreateDashboard(ctx, d.existing), "rolling back removed dashboard") - case StateStatusExists: + case IsExisting(d.stateStatus): _, err := s.dashSVC.UpdateDashboard(ctx, d.ID(), influxdb.DashboardUpdate{ Name: &d.existing.Name, Description: &d.existing.Description, From b4b52570b427fbec33132e45419c5f5105fe17cd Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 15:10:08 -0700 Subject: [PATCH 15/24] chore(pkger): handle edge cases for labels that manifest from user interaction references: #17434 --- cmd/influxd/launcher/pkger_test.go | 41 ++++++++++++++++++++++++++++++ pkger/service.go | 19 +++++++++----- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go index 17fae137a1..b03c2af74b 100644 --- a/cmd/influxd/launcher/pkger_test.go +++ b/cmd/influxd/launcher/pkger_test.go @@ -1213,6 +1213,42 @@ func TestLauncher_Pkger(t *testing.T) { }) }) }) + + t.Run("when a user has deleted a label that was previously created by a stack", func(t *testing.T) { + testUserDeletedLabel := func(t *testing.T, actionFn func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary)) { + t.Helper() + + obj := newLabelObject("label-1", "", "", "") + stackID, cleanup, initialSum := initializeStackPkg(t, newPkg(obj)) + defer cleanup() + + require.Len(t, initialSum.Labels, 1) + require.NotZero(t, initialSum.Labels[0].ID) + resourceCheck.mustDeleteLabel(t, influxdb.ID(initialSum.Labels[0].ID)) + + actionFn(t, stackID, obj, initialSum) + } + + t.Run("should create new resource when attempting to update", func(t *testing.T) { + testUserDeletedLabel(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.Labels, 1) + initial, updated := initialSum.Labels[0], updateSum.Labels[0] + assert.NotEqual(t, initial.ID, updated.ID) + initial.ID, updated.ID = 0, 0 + assert.Equal(t, initial, updated) + }) + }) + + t.Run("should not error when attempting to remove", func(t *testing.T) { + testUserDeletedLabel(t, func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary) { + testValidRemoval(t, stackID) + }) + }) + }) }) }) @@ -2800,6 +2836,11 @@ func (r resourceChecker) mustGetLabel(t *testing.T, getOpt getResourceOptFn) inf return l } +func (r resourceChecker) mustDeleteLabel(t *testing.T, id influxdb.ID) { + t.Helper() + require.NoError(t, r.tl.LabelService(t).DeleteLabel(ctx, id)) +} + func (r resourceChecker) getRule(t *testing.T, getOpt getResourceOptFn) (influxdb.NotificationRule, error) { t.Helper() diff --git a/pkger/service.go b/pkger/service.go index ef4b570829..26ff29cc9d 100644 --- a/pkger/service.go +++ b/pkger/service.go @@ -1714,11 +1714,15 @@ func (s *Service) applyLabels(ctx context.Context, labels []*stateLabel) applier func (s *Service) rollbackLabels(ctx context.Context, labels []*stateLabel) error { rollbackFn := func(l *stateLabel) error { + if !IsNew(l.stateStatus) && l.existing == nil { + return nil + } + var err error - switch l.stateStatus { - case StateStatusRemove: + switch { + case IsRemoval(l.stateStatus): err = s.labelSVC.CreateLabel(ctx, l.existing) - case StateStatusExists: + case IsExisting(l.stateStatus): _, err = s.labelSVC.UpdateLabel(ctx, l.ID(), influxdb.LabelUpdate{ Name: l.parserLabel.Name(), Properties: l.existing.Properties, @@ -1748,10 +1752,10 @@ func (s *Service) applyLabel(ctx context.Context, l *stateLabel) (influxdb.Label influxLabel *influxdb.Label err error ) - switch l.stateStatus { - case StateStatusRemove: + switch { + case IsRemoval(l.stateStatus): influxLabel, err = l.existing, s.labelSVC.DeleteLabel(ctx, l.ID()) - case StateStatusExists: + case IsExisting(l.stateStatus) && l.existing != nil: influxLabel, err = s.labelSVC.UpdateLabel(ctx, l.ID(), influxdb.LabelUpdate{ Name: l.parserLabel.Name(), Properties: l.properties(), @@ -1762,6 +1766,9 @@ func (s *Service) applyLabel(ctx context.Context, l *stateLabel) (influxdb.Label influxLabel = &creatLabel err = ierrors.Wrap(s.labelSVC.CreateLabel(ctx, &creatLabel), "creating") } + if influxdb.ErrorCode(err) == influxdb.ENotFound { + return influxdb.Label{}, nil + } if err != nil || influxLabel == nil { return influxdb.Label{}, err } From 7f8aa4c40f26710855b76a25474b034f4626a899 Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 15:31:24 -0700 Subject: [PATCH 16/24] 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 b03c2af74b..e888052d2a 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 26ff29cc9d..2a20fdd495 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) + 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.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 } } 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: From 4e0672e78e85607cda2005987edf56308605426c Mon Sep 17 00:00:00 2001 From: Timmy Luong Date: Tue, 5 May 2020 18:21:27 -0700 Subject: [PATCH 17/24] feat(ui): hide the "Upgrade Now" button for Cloud users when appropriate --- ui/src/cloud/actions/orgsettings.ts | 48 ++++ ui/src/cloud/apis/orgsettings.ts | 16 ++ ui/src/cloud/components/OrgSettings.tsx | 34 +++ ui/src/cloud/constants/index.ts | 10 + ui/src/cloud/reducers/orgsettings.ts | 26 ++ ui/src/pageLayout/containers/TreeNav.tsx | 243 ++++++++++-------- .../shared/components/CloudUpgradeButton.tsx | 49 +++- ui/src/store/configureStore.ts | 11 +- ui/src/types/cloud.ts | 9 + ui/src/types/stores.ts | 7 +- 10 files changed, 330 insertions(+), 123 deletions(-) create mode 100644 ui/src/cloud/actions/orgsettings.ts create mode 100644 ui/src/cloud/apis/orgsettings.ts create mode 100644 ui/src/cloud/components/OrgSettings.tsx create mode 100644 ui/src/cloud/reducers/orgsettings.ts diff --git a/ui/src/cloud/actions/orgsettings.ts b/ui/src/cloud/actions/orgsettings.ts new file mode 100644 index 0000000000..44956cae20 --- /dev/null +++ b/ui/src/cloud/actions/orgsettings.ts @@ -0,0 +1,48 @@ +// API +import {getOrgSettings as getOrgSettingsAJAX} from 'src/cloud/apis/orgsettings' + +// Constants +import {FREE_ORG_HIDE_UPGRADE_SETTING} from 'src/cloud/constants' + +// Types +import {GetState, OrgSetting} from 'src/types' + +// Selectors +import {getOrg} from 'src/organizations/selectors' + +export enum ActionTypes { + SetOrgSettings = 'SET_ORG_SETTINGS', +} + +export type Actions = SetOrgSettings + +export interface SetOrgSettings { + type: ActionTypes.SetOrgSettings + payload: {orgSettings: OrgSetting[]} +} + +export const setOrgSettings = (settings: OrgSetting[]): SetOrgSettings => { + return { + type: ActionTypes.SetOrgSettings, + payload: {orgSettings: settings}, + } +} + +export const setFreeOrgSettings = (): SetOrgSettings => { + return { + type: ActionTypes.SetOrgSettings, + payload: {orgSettings: [FREE_ORG_HIDE_UPGRADE_SETTING]}, + } +} + +export const getOrgSettings = () => async (dispatch, getState: GetState) => { + try { + const org = getOrg(getState()) + + const result = await getOrgSettingsAJAX(org.id) + dispatch(setOrgSettings(result.settings)) + } catch (error) { + dispatch(setFreeOrgSettings()) + console.error(error) + } +} diff --git a/ui/src/cloud/apis/orgsettings.ts b/ui/src/cloud/apis/orgsettings.ts new file mode 100644 index 0000000000..a5ef85d9a1 --- /dev/null +++ b/ui/src/cloud/apis/orgsettings.ts @@ -0,0 +1,16 @@ +import AJAX from 'src/utils/ajax' +import {OrgSettings} from 'src/types' + +export const getOrgSettings = async (orgID: string): Promise => { + try { + const {data} = await AJAX({ + method: 'GET', + url: `/api/v2private/orgs/${orgID}/settings`, + }) + + return data + } catch (error) { + console.error(error) + throw error + } +} diff --git a/ui/src/cloud/components/OrgSettings.tsx b/ui/src/cloud/components/OrgSettings.tsx new file mode 100644 index 0000000000..87ac96b84d --- /dev/null +++ b/ui/src/cloud/components/OrgSettings.tsx @@ -0,0 +1,34 @@ +// Libraries +import {PureComponent} from 'react' +import {connect} from 'react-redux' + +// Constants +import {CLOUD} from 'src/shared/constants' + +// Actions +import {getOrgSettings as getOrgSettingsAction} from 'src/cloud/actions/orgsettings' + +interface DispatchProps { + getOrgSettings: typeof getOrgSettingsAction +} + +class OrgSettings extends PureComponent { + public componentDidMount() { + if (CLOUD) { + this.props.getOrgSettings() + } + } + + public render() { + return this.props.children + } +} + +const mdtp: DispatchProps = { + getOrgSettings: getOrgSettingsAction, +} + +export default connect<{}, DispatchProps, {}>( + null, + mdtp +)(OrgSettings) diff --git a/ui/src/cloud/constants/index.ts b/ui/src/cloud/constants/index.ts index 9118e6e493..d4c5a6053b 100644 --- a/ui/src/cloud/constants/index.ts +++ b/ui/src/cloud/constants/index.ts @@ -25,3 +25,13 @@ export const DemoDataTemplates = { DemoDataDashboards[WebsiteMonitoringBucket] ), } + +export const HIDE_UPGRADE_CTA_KEY = 'hide_upgrade_cta' +export const FREE_ORG_HIDE_UPGRADE_SETTING = { + key: HIDE_UPGRADE_CTA_KEY, + value: 'false', +} +export const PAID_ORG_HIDE_UPGRADE_SETTING = { + key: HIDE_UPGRADE_CTA_KEY, + value: 'true', +} diff --git a/ui/src/cloud/reducers/orgsettings.ts b/ui/src/cloud/reducers/orgsettings.ts new file mode 100644 index 0000000000..a0f1327f56 --- /dev/null +++ b/ui/src/cloud/reducers/orgsettings.ts @@ -0,0 +1,26 @@ +import {produce} from 'immer' + +import {Actions, ActionTypes} from 'src/cloud/actions/orgsettings' +import {OrgSetting} from 'src/types' + +import {PAID_ORG_HIDE_UPGRADE_SETTING} from 'src/cloud/constants' + +export interface OrgSettingsState { + settings: OrgSetting[] +} + +export const defaultState: OrgSettingsState = { + settings: [PAID_ORG_HIDE_UPGRADE_SETTING], +} + +export const orgSettingsReducer = ( + state = defaultState, + action: Actions +): OrgSettingsState => + produce(state, draftState => { + if (action.type === ActionTypes.SetOrgSettings) { + const {orgSettings} = action.payload + draftState.settings = orgSettings + } + return + }) diff --git a/ui/src/pageLayout/containers/TreeNav.tsx b/ui/src/pageLayout/containers/TreeNav.tsx index 4f66edf7aa..ee0682a798 100644 --- a/ui/src/pageLayout/containers/TreeNav.tsx +++ b/ui/src/pageLayout/containers/TreeNav.tsx @@ -11,16 +11,21 @@ import NavHeader from 'src/pageLayout/components/NavHeader' import CloudUpgradeNavBanner from 'src/shared/components/CloudUpgradeNavBanner' import CloudExclude from 'src/shared/components/cloud/CloudExclude' import CloudOnly from 'src/shared/components/cloud/CloudOnly' +import OrgSettings from 'src/cloud/components/OrgSettings' import {FeatureFlag} from 'src/shared/utils/featureFlag' // Constants import {generateNavItems} from 'src/pageLayout/constants/navigationHierarchy' +import { + HIDE_UPGRADE_CTA_KEY, + PAID_ORG_HIDE_UPGRADE_SETTING, +} from 'src/cloud/constants' // Utils import {getNavItemActivation} from 'src/pageLayout/utils' // Types -import {AppState, NavBarState} from 'src/types' +import {AppState, NavBarState, OrgSetting} from 'src/types' // Actions import {setNavBarState} from 'src/shared/actions/app' @@ -31,6 +36,7 @@ import {ErrorHandling} from 'src/shared/decorators/errors' interface StateProps { isHidden: boolean navBarState: NavBarState + showUpgradeButton: boolean } interface DispatchProps { @@ -47,6 +53,7 @@ class TreeSidebar extends PureComponent { params: {orgID}, navBarState, handleSetNavBarState, + showUpgradeButton, } = this.props if (isHidden) { @@ -67,132 +74,134 @@ class TreeSidebar extends PureComponent { const navItems = generateNavItems(orgID) return ( - } - userElement={} - onToggleClick={handleToggleNavExpansion} - bannerElement={} - > - {navItems.map(item => { - const linkElement = (className: string): JSX.Element => { - if (item.link.type === 'href') { - return - } + + } + userElement={} + onToggleClick={handleToggleNavExpansion} + bannerElement={showUpgradeButton ? : null} + > + {navItems.map(item => { + const linkElement = (className: string): JSX.Element => { + if (item.link.type === 'href') { + return + } + + return + } + let navItemElement = ( + } + label={item.label} + shortLabel={item.shortLabel} + active={getNavItemActivation( + item.activeKeywords, + location.pathname + )} + linkElement={linkElement} + > + {Boolean(item.menu) && ( + + {item.menu.map(menuItem => { + const linkElement = (className: string): JSX.Element => { + if (menuItem.link.type === 'href') { + return ( + + ) + } - return - } - let navItemElement = ( - } - label={item.label} - shortLabel={item.shortLabel} - active={getNavItemActivation( - item.activeKeywords, - location.pathname - )} - linkElement={linkElement} - > - {Boolean(item.menu) && ( - - {item.menu.map(menuItem => { - const linkElement = (className: string): JSX.Element => { - if (menuItem.link.type === 'href') { return ( - ) } - return ( - ) - } - let navSubItemElement = ( - - ) + if (menuItem.cloudExclude) { + navSubItemElement = ( + + {navSubItemElement} + + ) + } - if (menuItem.cloudExclude) { - navSubItemElement = ( - - {navSubItemElement} - - ) - } + if (menuItem.cloudOnly) { + navSubItemElement = ( + + {navSubItemElement} + + ) + } - if (menuItem.cloudOnly) { - navSubItemElement = ( - - {navSubItemElement} - - ) - } + if (menuItem.featureFlag) { + navSubItemElement = ( + + {navSubItemElement} + + ) + } - if (menuItem.featureFlag) { - navSubItemElement = ( - - {navSubItemElement} - - ) - } - - return navSubItemElement - })} - - )} - - ) - - if (item.cloudExclude) { - navItemElement = ( - {navItemElement} + return navSubItemElement + })} + + )} + ) - } - if (item.cloudOnly) { - navItemElement = ( - {navItemElement} - ) - } + if (item.cloudExclude) { + navItemElement = ( + {navItemElement} + ) + } - if (item.featureFlag) { - navItemElement = ( - - {navItemElement} - - ) - } + if (item.cloudOnly) { + navItemElement = ( + {navItemElement} + ) + } - return navItemElement - })} - + if (item.featureFlag) { + navItemElement = ( + + {navItemElement} + + ) + } + + return navItemElement + })} + + ) } } @@ -204,8 +213,18 @@ const mdtp: DispatchProps = { const mstp = (state: AppState): StateProps => { const isHidden = get(state, 'app.ephemeral.inPresentationMode', false) const navBarState = get(state, 'app.persisted.navBarState', 'collapsed') - - return {isHidden, navBarState} + const {settings} = get(state, 'cloud.orgSettings') + let showUpgradeButton = false + const hideUpgradeCTA = settings.find( + (setting: OrgSetting) => setting.key === HIDE_UPGRADE_CTA_KEY + ) + if ( + !hideUpgradeCTA || + hideUpgradeCTA.value !== PAID_ORG_HIDE_UPGRADE_SETTING.value + ) { + showUpgradeButton = true + } + return {isHidden, navBarState, showUpgradeButton} } export default connect( diff --git a/ui/src/shared/components/CloudUpgradeButton.tsx b/ui/src/shared/components/CloudUpgradeButton.tsx index 4e06709d3f..84ce91f79f 100644 --- a/ui/src/shared/components/CloudUpgradeButton.tsx +++ b/ui/src/shared/components/CloudUpgradeButton.tsx @@ -1,25 +1,56 @@ // Libraries import React, {FC} from 'react' import {Link} from 'react-router' +import {connect} from 'react-redux' // Components import CloudOnly from 'src/shared/components/cloud/CloudOnly' // Constants import {CLOUD_URL, CLOUD_CHECKOUT_PATH} from 'src/shared/constants' +import { + HIDE_UPGRADE_CTA_KEY, + PAID_ORG_HIDE_UPGRADE_SETTING, +} from 'src/cloud/constants' -const CloudUpgradeButton: FC = () => { +// Types +import {AppState, OrgSetting} from 'src/types' + +interface StateProps { + show: boolean +} + +const CloudUpgradeButton: FC = ({show}) => { return ( - - Upgrade Now - + {show ? ( + + Upgrade Now + + ) : null} ) } -export default CloudUpgradeButton +const mstp = ({ + cloud: { + orgSettings: {settings}, + }, +}: AppState) => { + const hideUpgradeCTA = settings.find( + (setting: OrgSetting) => setting.key === HIDE_UPGRADE_CTA_KEY + ) + if ( + !hideUpgradeCTA || + hideUpgradeCTA.value !== PAID_ORG_HIDE_UPGRADE_SETTING.value + ) { + return {show: true} + } + return {show: false} +} + +export default connect(mstp)(CloudUpgradeButton) diff --git a/ui/src/store/configureStore.ts b/ui/src/store/configureStore.ts index 803be750ef..01e4dd55d7 100644 --- a/ui/src/store/configureStore.ts +++ b/ui/src/store/configureStore.ts @@ -37,6 +37,10 @@ import {membersReducer} from 'src/members/reducers' import {autoRefreshReducer} from 'src/shared/reducers/autoRefresh' import {limitsReducer, LimitsState} from 'src/cloud/reducers/limits' import {demoDataReducer, DemoDataState} from 'src/cloud/reducers/demodata' +import { + orgSettingsReducer, + OrgSettingsState, +} from 'src/cloud/reducers/orgsettings' import checksReducer from 'src/checks/reducers' import rulesReducer from 'src/notifications/rules/reducers' import endpointsReducer from 'src/notifications/endpoints/reducers' @@ -58,9 +62,14 @@ export const rootReducer = combineReducers({ ...sharedReducers, autoRefresh: autoRefreshReducer, alertBuilder: alertBuilderReducer, - cloud: combineReducers<{limits: LimitsState; demoData: DemoDataState}>({ + cloud: combineReducers<{ + limits: LimitsState + demoData: DemoDataState + orgSettings: OrgSettingsState + }>({ limits: limitsReducer, demoData: demoDataReducer, + orgSettings: orgSettingsReducer, }), currentPage: currentPageReducer, currentDashboard: currentDashboardReducer, diff --git a/ui/src/types/cloud.ts b/ui/src/types/cloud.ts index e6a31ad6f2..5a7068dc7d 100644 --- a/ui/src/types/cloud.ts +++ b/ui/src/types/cloud.ts @@ -39,3 +39,12 @@ export interface LimitsStatus { status: string } } +export interface OrgSetting { + key: string + value: string +} + +export interface OrgSettings { + orgID: string + settings: OrgSetting[] +} diff --git a/ui/src/types/stores.ts b/ui/src/types/stores.ts index b3046fb184..2e225ed033 100644 --- a/ui/src/types/stores.ts +++ b/ui/src/types/stores.ts @@ -25,6 +25,7 @@ import {LimitsState} from 'src/cloud/reducers/limits' import {AlertBuilderState} from 'src/alerting/reducers/alertBuilder' import {CurrentPage} from 'src/shared/reducers/currentPage' import {DemoDataState} from 'src/cloud/reducers/demodata' +import {OrgSettingsState} from 'src/cloud/reducers/orgsettings' import {ResourceState} from 'src/types' @@ -32,7 +33,11 @@ export interface AppState { alertBuilder: AlertBuilderState app: AppPresentationState autoRefresh: AutoRefreshState - cloud: {limits: LimitsState; demoData: DemoDataState} + cloud: { + limits: LimitsState + demoData: DemoDataState + orgSettings: OrgSettingsState + } currentPage: CurrentPage currentDashboard: CurrentDashboardState dataLoading: DataLoadingState From 0a3cd47047e16c3774a0856c87abadb2ec610c5f Mon Sep 17 00:00:00 2001 From: Timmy Luong Date: Tue, 5 May 2020 19:54:57 -0700 Subject: [PATCH 18/24] refactor(ui): use fetch instead of axios --- ui/src/cloud/actions/orgsettings.ts | 2 +- ui/src/cloud/apis/orgsettings.ts | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/ui/src/cloud/actions/orgsettings.ts b/ui/src/cloud/actions/orgsettings.ts index 44956cae20..a42c2290e2 100644 --- a/ui/src/cloud/actions/orgsettings.ts +++ b/ui/src/cloud/actions/orgsettings.ts @@ -21,7 +21,7 @@ export interface SetOrgSettings { payload: {orgSettings: OrgSetting[]} } -export const setOrgSettings = (settings: OrgSetting[]): SetOrgSettings => { +export const setOrgSettings = (settings: OrgSetting[] = []): SetOrgSettings => { return { type: ActionTypes.SetOrgSettings, payload: {orgSettings: settings}, diff --git a/ui/src/cloud/apis/orgsettings.ts b/ui/src/cloud/apis/orgsettings.ts index a5ef85d9a1..e748498c4e 100644 --- a/ui/src/cloud/apis/orgsettings.ts +++ b/ui/src/cloud/apis/orgsettings.ts @@ -1,13 +1,12 @@ -import AJAX from 'src/utils/ajax' import {OrgSettings} from 'src/types' +import {getAPIBasepath} from 'src/utils/basepath' export const getOrgSettings = async (orgID: string): Promise => { try { - const {data} = await AJAX({ - method: 'GET', - url: `/api/v2private/orgs/${orgID}/settings`, - }) - + const response = await fetch( + `${getAPIBasepath()}/api/v2private/orgs/${orgID}/settings` + ) + const data = await response.json() return data } catch (error) { console.error(error) From d984d35c0614f68991f9830ef230c1ebaf6877a5 Mon Sep 17 00:00:00 2001 From: Timmy Luong Date: Wed, 6 May 2020 14:22:40 -0700 Subject: [PATCH 19/24] refactor(ui): update variable names, code patterns, and CloudUpgradeNavBanner --- ui/src/cloud/actions/orgsettings.ts | 40 ++++---- ui/src/cloud/apis/orgsettings.ts | 18 +--- ui/src/cloud/reducers/orgsettings.ts | 11 +-- ui/src/pageLayout/containers/TreeNav.tsx | 24 +---- .../shared/components/CloudUpgradeButton.tsx | 16 ++-- .../components/CloudUpgradeNavBanner.tsx | 95 ++++++++++++------- ui/src/types/cloud.ts | 6 ++ 7 files changed, 110 insertions(+), 100 deletions(-) diff --git a/ui/src/cloud/actions/orgsettings.ts b/ui/src/cloud/actions/orgsettings.ts index a42c2290e2..58a1a0bac6 100644 --- a/ui/src/cloud/actions/orgsettings.ts +++ b/ui/src/cloud/actions/orgsettings.ts @@ -1,5 +1,5 @@ // API -import {getOrgSettings as getOrgSettingsAJAX} from 'src/cloud/apis/orgsettings' +import {fetchOrgSettings} from 'src/cloud/apis/orgsettings' // Constants import {FREE_ORG_HIDE_UPGRADE_SETTING} from 'src/cloud/constants' @@ -10,36 +10,34 @@ import {GetState, OrgSetting} from 'src/types' // Selectors import {getOrg} from 'src/organizations/selectors' -export enum ActionTypes { - SetOrgSettings = 'SET_ORG_SETTINGS', -} +export const SET_ORG_SETTINGS = 'SET_ORG_SETTINGS' -export type Actions = SetOrgSettings +export type Action = ReturnType -export interface SetOrgSettings { - type: ActionTypes.SetOrgSettings - payload: {orgSettings: OrgSetting[]} -} - -export const setOrgSettings = (settings: OrgSetting[] = []): SetOrgSettings => { - return { - type: ActionTypes.SetOrgSettings, +export const setOrgSettings = (settings: OrgSetting[] = []) => + ({ + type: SET_ORG_SETTINGS, payload: {orgSettings: settings}, - } -} + } as const) -export const setFreeOrgSettings = (): SetOrgSettings => { - return { - type: ActionTypes.SetOrgSettings, +export const setFreeOrgSettings = () => + ({ + type: SET_ORG_SETTINGS, payload: {orgSettings: [FREE_ORG_HIDE_UPGRADE_SETTING]}, - } -} + } as const) export const getOrgSettings = () => async (dispatch, getState: GetState) => { try { const org = getOrg(getState()) - const result = await getOrgSettingsAJAX(org.id) + const response = await fetchOrgSettings(org.id) + + if (response.status !== 200) { + throw new Error( + `Unable to get organization settings: ${response.statusText}` + ) + } + const result = await response.json() dispatch(setOrgSettings(result.settings)) } catch (error) { dispatch(setFreeOrgSettings()) diff --git a/ui/src/cloud/apis/orgsettings.ts b/ui/src/cloud/apis/orgsettings.ts index e748498c4e..1c70cb2a01 100644 --- a/ui/src/cloud/apis/orgsettings.ts +++ b/ui/src/cloud/apis/orgsettings.ts @@ -1,15 +1,7 @@ -import {OrgSettings} from 'src/types' import {getAPIBasepath} from 'src/utils/basepath' +import {OrgSettingsResponse} from 'src/types' -export const getOrgSettings = async (orgID: string): Promise => { - try { - const response = await fetch( - `${getAPIBasepath()}/api/v2private/orgs/${orgID}/settings` - ) - const data = await response.json() - return data - } catch (error) { - console.error(error) - throw error - } -} +export const fetchOrgSettings = async ( + orgID: string +): Promise => + await fetch(`${getAPIBasepath()}/api/v2private/orgs/${orgID}/settings`) diff --git a/ui/src/cloud/reducers/orgsettings.ts b/ui/src/cloud/reducers/orgsettings.ts index a0f1327f56..6f41aaa502 100644 --- a/ui/src/cloud/reducers/orgsettings.ts +++ b/ui/src/cloud/reducers/orgsettings.ts @@ -1,6 +1,6 @@ import {produce} from 'immer' -import {Actions, ActionTypes} from 'src/cloud/actions/orgsettings' +import {Action, SET_ORG_SETTINGS} from 'src/cloud/actions/orgsettings' import {OrgSetting} from 'src/types' import {PAID_ORG_HIDE_UPGRADE_SETTING} from 'src/cloud/constants' @@ -14,13 +14,12 @@ export const defaultState: OrgSettingsState = { } export const orgSettingsReducer = ( - state = defaultState, - action: Actions + state: OrgSettingsState = defaultState, + action: Action ): OrgSettingsState => produce(state, draftState => { - if (action.type === ActionTypes.SetOrgSettings) { - const {orgSettings} = action.payload - draftState.settings = orgSettings + if (action.type === SET_ORG_SETTINGS) { + draftState.settings = action.payload.orgSettings } return }) diff --git a/ui/src/pageLayout/containers/TreeNav.tsx b/ui/src/pageLayout/containers/TreeNav.tsx index ee0682a798..523040f8b8 100644 --- a/ui/src/pageLayout/containers/TreeNav.tsx +++ b/ui/src/pageLayout/containers/TreeNav.tsx @@ -16,16 +16,12 @@ import {FeatureFlag} from 'src/shared/utils/featureFlag' // Constants import {generateNavItems} from 'src/pageLayout/constants/navigationHierarchy' -import { - HIDE_UPGRADE_CTA_KEY, - PAID_ORG_HIDE_UPGRADE_SETTING, -} from 'src/cloud/constants' // Utils import {getNavItemActivation} from 'src/pageLayout/utils' // Types -import {AppState, NavBarState, OrgSetting} from 'src/types' +import {AppState, NavBarState} from 'src/types' // Actions import {setNavBarState} from 'src/shared/actions/app' @@ -36,7 +32,6 @@ import {ErrorHandling} from 'src/shared/decorators/errors' interface StateProps { isHidden: boolean navBarState: NavBarState - showUpgradeButton: boolean } interface DispatchProps { @@ -53,7 +48,6 @@ class TreeSidebar extends PureComponent { params: {orgID}, navBarState, handleSetNavBarState, - showUpgradeButton, } = this.props if (isHidden) { @@ -80,7 +74,7 @@ class TreeSidebar extends PureComponent { headerElement={} userElement={} onToggleClick={handleToggleNavExpansion} - bannerElement={showUpgradeButton ? : null} + bannerElement={} > {navItems.map(item => { const linkElement = (className: string): JSX.Element => { @@ -213,18 +207,8 @@ const mdtp: DispatchProps = { const mstp = (state: AppState): StateProps => { const isHidden = get(state, 'app.ephemeral.inPresentationMode', false) const navBarState = get(state, 'app.persisted.navBarState', 'collapsed') - const {settings} = get(state, 'cloud.orgSettings') - let showUpgradeButton = false - const hideUpgradeCTA = settings.find( - (setting: OrgSetting) => setting.key === HIDE_UPGRADE_CTA_KEY - ) - if ( - !hideUpgradeCTA || - hideUpgradeCTA.value !== PAID_ORG_HIDE_UPGRADE_SETTING.value - ) { - showUpgradeButton = true - } - return {isHidden, navBarState, showUpgradeButton} + + return {isHidden, navBarState} } export default connect( diff --git a/ui/src/shared/components/CloudUpgradeButton.tsx b/ui/src/shared/components/CloudUpgradeButton.tsx index 84ce91f79f..854e73e892 100644 --- a/ui/src/shared/components/CloudUpgradeButton.tsx +++ b/ui/src/shared/components/CloudUpgradeButton.tsx @@ -17,13 +17,13 @@ import { import {AppState, OrgSetting} from 'src/types' interface StateProps { - show: boolean + inView: boolean } -const CloudUpgradeButton: FC = ({show}) => { +const CloudUpgradeButton: FC = ({inView}) => { return ( - {show ? ( + {inView ? ( { - const hideUpgradeCTA = settings.find( + const hideUpgradeButtonSetting = settings.find( (setting: OrgSetting) => setting.key === HIDE_UPGRADE_CTA_KEY ) if ( - !hideUpgradeCTA || - hideUpgradeCTA.value !== PAID_ORG_HIDE_UPGRADE_SETTING.value + !hideUpgradeButtonSetting || + hideUpgradeButtonSetting.value !== PAID_ORG_HIDE_UPGRADE_SETTING.value ) { - return {show: true} + return {inView: true} } - return {show: false} + return {inView: false} } export default connect(mstp)(CloudUpgradeButton) diff --git a/ui/src/shared/components/CloudUpgradeNavBanner.tsx b/ui/src/shared/components/CloudUpgradeNavBanner.tsx index 102989682c..507e51f66d 100644 --- a/ui/src/shared/components/CloudUpgradeNavBanner.tsx +++ b/ui/src/shared/components/CloudUpgradeNavBanner.tsx @@ -1,6 +1,7 @@ // Libraries import React, {FC} from 'react' import {Link} from 'react-router' +import {connect} from 'react-redux' // Components import { @@ -17,44 +18,74 @@ import CloudOnly from 'src/shared/components/cloud/CloudOnly' // Constants import {CLOUD_URL, CLOUD_CHECKOUT_PATH} from 'src/shared/constants' +import { + HIDE_UPGRADE_CTA_KEY, + PAID_ORG_HIDE_UPGRADE_SETTING, +} from 'src/cloud/constants' -const CloudUpgradeNavBanner: FC = () => { +// Types +import {AppState, OrgSetting} from 'src/types' + +interface StateProps { + inView: boolean +} + +const CloudUpgradeNavBanner: FC = ({inView}) => { return ( <> - - - + - - Need more wiggle room? - - - - - Upgrade Now - - - - - - Upgrade Now - - + + Need more wiggle room? + + + + + Upgrade Now + + + + + + Upgrade Now + + + ) : null} ) } -export default CloudUpgradeNavBanner +const mstp = ({ + cloud: { + orgSettings: {settings}, + }, +}: AppState) => { + const hideUpgradeButtonSetting = settings.find( + (setting: OrgSetting) => setting.key === HIDE_UPGRADE_CTA_KEY + ) + if ( + !hideUpgradeButtonSetting || + hideUpgradeButtonSetting.value !== PAID_ORG_HIDE_UPGRADE_SETTING.value + ) { + return {inView: true} + } + return {inView: false} +} + +export default connect(mstp)(CloudUpgradeNavBanner) diff --git a/ui/src/types/cloud.ts b/ui/src/types/cloud.ts index 5a7068dc7d..9206a97912 100644 --- a/ui/src/types/cloud.ts +++ b/ui/src/types/cloud.ts @@ -48,3 +48,9 @@ export interface OrgSettings { orgID: string settings: OrgSetting[] } + +export interface OrgSettingsResponse { + status: number + statusText: string + json: () => Promise +} From 7be0d8289bc7d953575bf1c3d67d8f36e09721cd Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 15:54:14 -0700 Subject: [PATCH 20/24] chore(pkger): handle edge cases for rules that manifest from user interaction references: #17434 --- cmd/influxd/launcher/pkger_test.go | 45 ++++++++++++++++++++++++++++++ pkger/service.go | 17 ++++++++--- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go index e888052d2a..f7132ec186 100644 --- a/cmd/influxd/launcher/pkger_test.go +++ b/cmd/influxd/launcher/pkger_test.go @@ -1286,6 +1286,45 @@ func TestLauncher_Pkger(t *testing.T) { }) }) }) + + t.Run("when a user has deleted a notification rule that was previously created by a stack", func(t *testing.T) { + testUserDeletedRule := func(t *testing.T, actionFn func(t *testing.T, stackID influxdb.ID, initialObjects []pkger.Object, initialSum pkger.Summary)) { + t.Helper() + + endpointObj := newEndpointHTTP("endpoint-1", "", "") + ruleObj := newRuleObject(t, "rule-0", "", endpointObj.Name(), "") + stackID, cleanup, initialSum := initializeStackPkg(t, newPkg(endpointObj, ruleObj)) + defer cleanup() + + require.Len(t, initialSum.NotificationEndpoints, 1) + require.NotZero(t, initialSum.NotificationEndpoints[0].NotificationEndpoint.GetID()) + require.Len(t, initialSum.NotificationRules, 1) + require.NotZero(t, initialSum.NotificationRules[0].ID) + resourceCheck.mustDeleteRule(t, influxdb.ID(initialSum.NotificationRules[0].ID)) + + actionFn(t, stackID, []pkger.Object{ruleObj, endpointObj}, initialSum) + } + + t.Run("should create new resource when attempting to update", func(t *testing.T) { + testUserDeletedRule(t, func(t *testing.T, stackID influxdb.ID, initialObjects []pkger.Object, initialSum pkger.Summary) { + pkg := newPkg(initialObjects...) + updateSum, _, err := svc.Apply(ctx, l.Org.ID, l.User.ID, pkg, pkger.ApplyWithStackID(stackID)) + require.NoError(t, err) + + require.Len(t, updateSum.NotificationRules, 1) + initial, updated := initialSum.NotificationRules[0], updateSum.NotificationRules[0] + assert.NotEqual(t, initial.ID, updated.ID) + initial.ID, updated.ID = 0, 0 + assert.Equal(t, initial, updated) + }) + }) + + t.Run("should not error when attempting to remove", func(t *testing.T) { + testUserDeletedRule(t, func(t *testing.T, stackID influxdb.ID, _ []pkger.Object, _ pkger.Summary) { + testValidRemoval(t, stackID) + }) + }) + }) }) }) @@ -2927,6 +2966,12 @@ func (r resourceChecker) mustGetRule(t *testing.T, getOpt getResourceOptFn) infl return rule } +func (r resourceChecker) mustDeleteRule(t *testing.T, id influxdb.ID) { + t.Helper() + + require.NoError(t, r.tl.NotificationRuleService().DeleteNotificationRule(ctx, id)) +} + func (r resourceChecker) getTask(t *testing.T, getOpt getResourceOptFn) (http.Task, error) { t.Helper() diff --git a/pkger/service.go b/pkger/service.go index 2a20fdd495..03b7101ebd 100644 --- a/pkger/service.go +++ b/pkger/service.go @@ -1973,7 +1973,9 @@ func (s *Service) applyNotificationRules(ctx context.Context, userID influxdb.ID } mutex.Do(func() { - rules[i].id = influxRule.GetID() + if influxRule != nil { + rules[i].id = influxRule.GetID() + } rollbackEndpoints = append(rollbackEndpoints, rules[i]) }) @@ -1996,13 +1998,16 @@ func (s *Service) applyNotificationRules(ctx context.Context, userID influxdb.ID } func (s *Service) applyNotificationRule(ctx context.Context, r *stateRule, userID influxdb.ID) (influxdb.NotificationRule, error) { - switch r.stateStatus { - case StateStatusRemove: + switch { + case IsRemoval(r.stateStatus): if err := s.ruleSVC.DeleteNotificationRule(ctx, r.ID()); err != nil { + if influxdb.ErrorCode(err) == influxdb.ENotFound { + return nil, nil + } return nil, ierrors.Wrap(err, "failed to remove notification rule") } return r.existing, nil - case StateStatusExists: + case IsExisting(r.stateStatus) && r.existing != nil: ruleCreate := influxdb.NotificationRuleCreate{ NotificationRule: r.toInfluxRule(), Status: r.parserRule.Status(), @@ -2027,6 +2032,10 @@ func (s *Service) applyNotificationRule(ctx context.Context, r *stateRule, userI func (s *Service) rollbackNotificationRules(ctx context.Context, userID influxdb.ID, rules []*stateRule) error { rollbackFn := func(r *stateRule) error { + if !IsNew(r.stateStatus) && r.existing == nil { + return nil + } + existingRuleFn := func(endpointID influxdb.ID) influxdb.NotificationRule { switch rr := r.existing.(type) { case *rule.HTTP: From 3eb5af969a8055c370db6ca3b94ec891f72bde02 Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 16:09:38 -0700 Subject: [PATCH 21/24] chore(pkger): handle edge cases for tasks that manifest from user interaction references: #17434 --- cmd/influxd/launcher/pkger_test.go | 42 ++++++++++++++++++++++++++++++ pkger/service.go | 13 ++++++--- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go index f7132ec186..9ecee1be1f 100644 --- a/cmd/influxd/launcher/pkger_test.go +++ b/cmd/influxd/launcher/pkger_test.go @@ -1325,6 +1325,42 @@ func TestLauncher_Pkger(t *testing.T) { }) }) }) + + t.Run("when a user has deleted a task that was previously created by a stack", func(t *testing.T) { + testUserDeletedTask := func(t *testing.T, actionFn func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary)) { + t.Helper() + + obj := newTaskObject("task-1", "", "") + stackID, cleanup, initialSum := initializeStackPkg(t, newPkg(obj)) + defer cleanup() + + require.Len(t, initialSum.Tasks, 1) + require.NotZero(t, initialSum.Tasks[0].ID) + resourceCheck.mustDeleteTask(t, influxdb.ID(initialSum.Tasks[0].ID)) + + actionFn(t, stackID, obj, initialSum) + } + + t.Run("should create new resource when attempting to update", func(t *testing.T) { + testUserDeletedTask(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.Tasks, 1) + initial, updated := initialSum.Tasks[0], updateSum.Tasks[0] + assert.NotEqual(t, initial.ID, updated.ID) + initial.ID, updated.ID = 0, 0 + assert.Equal(t, initial, updated) + }) + }) + + t.Run("should not error when attempting to remove", func(t *testing.T) { + testUserDeletedTask(t, func(t *testing.T, stackID influxdb.ID, _ pkger.Object, _ pkger.Summary) { + testValidRemoval(t, stackID) + }) + }) + }) }) }) @@ -3016,6 +3052,12 @@ func (r resourceChecker) mustGetTask(t *testing.T, getOpt getResourceOptFn) http return task } +func (r resourceChecker) mustDeleteTask(t *testing.T, id influxdb.ID) { + t.Helper() + + require.NoError(t, r.tl.TaskService(t).DeleteTask(ctx, id)) +} + func (r resourceChecker) getTelegrafConfig(t *testing.T, getOpt getResourceOptFn) (influxdb.TelegrafConfig, error) { t.Helper() diff --git a/pkger/service.go b/pkger/service.go index 03b7101ebd..a6d6a4c319 100644 --- a/pkger/service.go +++ b/pkger/service.go @@ -2183,13 +2183,16 @@ func (s *Service) applyTasks(ctx context.Context, tasks []*stateTask) applier { } func (s *Service) applyTask(ctx context.Context, userID influxdb.ID, t *stateTask) (influxdb.Task, error) { - switch t.stateStatus { - case StateStatusRemove: + switch { + case IsRemoval(t.stateStatus): if err := s.taskSVC.DeleteTask(ctx, t.ID()); err != nil { + if influxdb.ErrorCode(err) == influxdb.ENotFound { + return influxdb.Task{}, nil + } return influxdb.Task{}, ierrors.Wrap(err, "failed to delete task") } return *t.existing, nil - case StateStatusExists: + case IsExisting(t.stateStatus) && t.existing != nil: newFlux := t.parserTask.flux() newStatus := string(t.parserTask.Status()) opt := options.Options{ @@ -2231,6 +2234,10 @@ func (s *Service) applyTask(ctx context.Context, userID influxdb.ID, t *stateTas func (s *Service) rollbackTasks(ctx context.Context, tasks []*stateTask) error { rollbackFn := func(t *stateTask) error { + if !IsNew(t.stateStatus) && t.existing == nil { + return nil + } + var err error switch t.stateStatus { case StateStatusRemove: From 37d44d882d782b81a0d6c3c60b40d3bc1d70fbfd Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 May 2020 16:21:07 -0700 Subject: [PATCH 22/24] chore(pkger): handle edge cases for telegraf configs that manifest from user interaction closes: #17434 --- CHANGELOG.md | 3 ++- cmd/influxd/launcher/pkger_test.go | 42 ++++++++++++++++++++++++++++++ pkger/service.go | 13 ++++++--- 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e3428160e..00bb0891ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,8 @@ ### Features 1. [17934](https://github.com/influxdata/influxdb/pull/17934): Add ability to delete a stack and all the resources associated with it -1. [17941](https://github.com/influxdata/influxdb/pull/17941): Encorce dns name compliance on all pkger resources' metadata.name field +1. [17941](https://github.com/influxdata/influxdb/pull/17941): Enforce DNS name compliance on all pkger resources' metadata.name field +1. [17989](https://github.com/influxdata/influxdb/pull/17989): Add stateful pkg management with stacks ### Bug Fixes diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go index 9ecee1be1f..6a2f34a030 100644 --- a/cmd/influxd/launcher/pkger_test.go +++ b/cmd/influxd/launcher/pkger_test.go @@ -1361,6 +1361,42 @@ func TestLauncher_Pkger(t *testing.T) { }) }) }) + + t.Run("when a user has deleted a telegraf config that was previously created by a stack", func(t *testing.T) { + testUserDeletedTelegraf := func(t *testing.T, actionFn func(t *testing.T, stackID influxdb.ID, initialObj pkger.Object, initialSum pkger.Summary)) { + t.Helper() + + obj := newTelegrafObject("tele-1", "", "") + stackID, cleanup, initialSum := initializeStackPkg(t, newPkg(obj)) + defer cleanup() + + require.Len(t, initialSum.TelegrafConfigs, 1) + require.NotZero(t, initialSum.TelegrafConfigs[0].TelegrafConfig.ID) + resourceCheck.mustDeleteTelegrafConfig(t, initialSum.TelegrafConfigs[0].TelegrafConfig.ID) + + actionFn(t, stackID, obj, initialSum) + } + + t.Run("should create new resource when attempting to update", func(t *testing.T) { + testUserDeletedTelegraf(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.TelegrafConfigs, 1) + initial, updated := initialSum.TelegrafConfigs[0].TelegrafConfig, updateSum.TelegrafConfigs[0].TelegrafConfig + assert.NotEqual(t, initial.ID, updated.ID) + initial.ID, updated.ID = 0, 0 + assert.Equal(t, initial, updated) + }) + }) + + t.Run("should not error when attempting to remove", func(t *testing.T) { + testUserDeletedTelegraf(t, func(t *testing.T, stackID influxdb.ID, _ pkger.Object, _ pkger.Summary) { + testValidRemoval(t, stackID) + }) + }) + }) }) }) @@ -3098,6 +3134,12 @@ func (r resourceChecker) mustGetTelegrafConfig(t *testing.T, getOpt getResourceO return tele } +func (r resourceChecker) mustDeleteTelegrafConfig(t *testing.T, id influxdb.ID) { + t.Helper() + + require.NoError(t, r.tl.TelegrafService(t).DeleteTelegrafConfig(ctx, id)) +} + func (r resourceChecker) getVariable(t *testing.T, getOpt getResourceOptFn) (influxdb.Variable, error) { t.Helper() diff --git a/pkger/service.go b/pkger/service.go index a6d6a4c319..6594810f18 100644 --- a/pkger/service.go +++ b/pkger/service.go @@ -2340,13 +2340,16 @@ func (s *Service) applyTelegrafs(ctx context.Context, userID influxdb.ID, teles } func (s *Service) applyTelegrafConfig(ctx context.Context, userID influxdb.ID, t *stateTelegraf) (influxdb.TelegrafConfig, error) { - switch t.stateStatus { - case StateStatusRemove: + switch { + case IsRemoval(t.stateStatus): if err := s.teleSVC.DeleteTelegrafConfig(ctx, t.ID()); err != nil { + if influxdb.ErrorCode(err) == influxdb.ENotFound { + return influxdb.TelegrafConfig{}, nil + } return influxdb.TelegrafConfig{}, ierrors.Wrap(err, "failed to delete config") } return *t.existing, nil - case StateStatusExists: + case IsExisting(t.stateStatus) && t.existing != nil: cfg := t.summarize().TelegrafConfig updatedConfig, err := s.teleSVC.UpdateTelegrafConfig(ctx, t.ID(), &cfg, userID) if err != nil { @@ -2365,6 +2368,10 @@ func (s *Service) applyTelegrafConfig(ctx context.Context, userID influxdb.ID, t func (s *Service) rollbackTelegrafConfigs(ctx context.Context, userID influxdb.ID, cfgs []*stateTelegraf) error { rollbackFn := func(t *stateTelegraf) error { + if !IsNew(t.stateStatus) && t.existing == nil { + return nil + } + var err error switch t.stateStatus { case StateStatusRemove: From 33f77d7fea4207fe391a3516dadebbdbc79fbd3b Mon Sep 17 00:00:00 2001 From: Deniz Kusefoglu Date: Wed, 6 May 2020 17:26:49 -0700 Subject: [PATCH 23/24] feat(demodata): get error message from error for notification (#17983) --- ui/src/cloud/actions/demodata.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ui/src/cloud/actions/demodata.ts b/ui/src/cloud/actions/demodata.ts index 9dcbef503a..8eb6d825e1 100644 --- a/ui/src/cloud/actions/demodata.ts +++ b/ui/src/cloud/actions/demodata.ts @@ -34,6 +34,7 @@ import { Dashboard, } from 'src/types' import {reportError} from 'src/shared/utils/errors' +import {getErrorMessage} from 'src/utils/api' export type Actions = | ReturnType @@ -124,7 +125,7 @@ export const getDemoDataBucketMembership = ({ dispatch(notify(demoDataSucceeded(bucketName, url))) } catch (error) { - dispatch(notify(demoDataAddBucketFailed(error))) + dispatch(notify(demoDataAddBucketFailed(getErrorMessage(error)))) reportError(error, { name: 'getDemoDataBucketMembership function', From 9a1e31a12f907a975ac9ef61b5bd21f074dccb35 Mon Sep 17 00:00:00 2001 From: jlapacik Date: Thu, 7 May 2020 10:46:29 -0700 Subject: [PATCH 24/24] chore: update flux to latest revision --- go.mod | 2 +- go.sum | 4 ++-- query/promql/internal/promqltests/go.mod | 2 +- query/promql/internal/promqltests/go.sum | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index b3a9bfc1db..41dc7a65c2 100644 --- a/go.mod +++ b/go.mod @@ -42,7 +42,7 @@ require ( github.com/hashicorp/raft v1.0.0 // indirect github.com/hashicorp/vault/api v1.0.2 github.com/influxdata/cron v0.0.0-20191203200038-ded12750aac6 - github.com/influxdata/flux v0.67.1-0.20200506164116-7432bbda91d7 + github.com/influxdata/flux v0.67.1-0.20200507153142-7a0c6ca988e1 github.com/influxdata/httprouter v1.3.1-0.20191122104820-ee83e2772f69 github.com/influxdata/influxql v0.0.0-20180925231337-1cbfca8e56b6 github.com/influxdata/pkg-config v0.2.0 diff --git a/go.sum b/go.sum index bfd6bfdf8f..4fe9139a5d 100644 --- a/go.sum +++ b/go.sum @@ -246,8 +246,8 @@ github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NH github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= github.com/influxdata/cron v0.0.0-20191203200038-ded12750aac6 h1:OtjKkeWDjUbyMi82C7XXy7Tvm2LXMwiBBXyFIGNPaGA= github.com/influxdata/cron v0.0.0-20191203200038-ded12750aac6/go.mod h1:XabtPPW2qsCg0tl+kjaPU+cFS+CjQXEXbT1VJvHT4og= -github.com/influxdata/flux v0.67.1-0.20200506164116-7432bbda91d7 h1:9bRTK6KToiAp4UP2cLm06NznCOdMvtnIUSwxgpaDw2s= -github.com/influxdata/flux v0.67.1-0.20200506164116-7432bbda91d7/go.mod h1:AdzL5HnjdFlcBiNz0wE69rSTGRX9CQHqtJUF8ptiDeY= +github.com/influxdata/flux v0.67.1-0.20200507153142-7a0c6ca988e1 h1:tT0L4qNrdcSnfAB4Srb/J4W9m5vZ14VUf22STppqZrc= +github.com/influxdata/flux v0.67.1-0.20200507153142-7a0c6ca988e1/go.mod h1:AdzL5HnjdFlcBiNz0wE69rSTGRX9CQHqtJUF8ptiDeY= github.com/influxdata/goreleaser v0.97.0-influx h1:jT5OrcW7WfS0e2QxfwmTBjhLvpIC9CDLRhNgZJyhj8s= github.com/influxdata/goreleaser v0.97.0-influx/go.mod h1:MnjA0e0Uq6ISqjG1WxxMAl+3VS1QYjILSWVnMYDxasE= github.com/influxdata/httprouter v1.3.1-0.20191122104820-ee83e2772f69 h1:WQsmW0fXO4ZE/lFGIE84G6rIV5SJN3P3sjIXAP1a8eU= diff --git a/query/promql/internal/promqltests/go.mod b/query/promql/internal/promqltests/go.mod index b705da6fe4..b24200f576 100644 --- a/query/promql/internal/promqltests/go.mod +++ b/query/promql/internal/promqltests/go.mod @@ -12,7 +12,7 @@ require ( github.com/go-kit/kit v0.10.0 // indirect github.com/google/go-cmp v0.4.0 github.com/hashicorp/go-rootcerts v1.0.2 // indirect - github.com/influxdata/flux v0.67.1-0.20200506164116-7432bbda91d7 + github.com/influxdata/flux v0.67.1-0.20200507153142-7a0c6ca988e1 github.com/influxdata/influxdb/v2 v2.0.0-00010101000000-000000000000 github.com/influxdata/influxql v1.0.1 // indirect github.com/influxdata/promql/v2 v2.12.0 diff --git a/query/promql/internal/promqltests/go.sum b/query/promql/internal/promqltests/go.sum index b9e733550b..af072f9ca1 100644 --- a/query/promql/internal/promqltests/go.sum +++ b/query/promql/internal/promqltests/go.sum @@ -321,8 +321,8 @@ github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NH github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= github.com/influxdata/cron v0.0.0-20191203200038-ded12750aac6 h1:OtjKkeWDjUbyMi82C7XXy7Tvm2LXMwiBBXyFIGNPaGA= github.com/influxdata/cron v0.0.0-20191203200038-ded12750aac6/go.mod h1:XabtPPW2qsCg0tl+kjaPU+cFS+CjQXEXbT1VJvHT4og= -github.com/influxdata/flux v0.67.1-0.20200506164116-7432bbda91d7 h1:9bRTK6KToiAp4UP2cLm06NznCOdMvtnIUSwxgpaDw2s= -github.com/influxdata/flux v0.67.1-0.20200506164116-7432bbda91d7/go.mod h1:AdzL5HnjdFlcBiNz0wE69rSTGRX9CQHqtJUF8ptiDeY= +github.com/influxdata/flux v0.67.1-0.20200507153142-7a0c6ca988e1 h1:tT0L4qNrdcSnfAB4Srb/J4W9m5vZ14VUf22STppqZrc= +github.com/influxdata/flux v0.67.1-0.20200507153142-7a0c6ca988e1/go.mod h1:AdzL5HnjdFlcBiNz0wE69rSTGRX9CQHqtJUF8ptiDeY= github.com/influxdata/httprouter v1.3.1-0.20191122104820-ee83e2772f69 h1:WQsmW0fXO4ZE/lFGIE84G6rIV5SJN3P3sjIXAP1a8eU= github.com/influxdata/httprouter v1.3.1-0.20191122104820-ee83e2772f69/go.mod h1:pwymjR6SrP3gD3pRj9RJwdl1j5s3doEEV8gS4X9qSzA= github.com/influxdata/influxdb1-client v0.0.0-20191209144304-8bf82d3c094d/go.mod h1:qj24IKcXYK6Iy9ceXlo3Tc+vtHo9lIhSX5JddghvEPo=