fix(onboarding): deprecate misleading retentionPeriodHrs key (#20798)

CHANGELOG.md

@@ -16,6 +16,7 @@
 1. [20754](https://github.com/influxdata/influxdb/pull/20754): Update references to docs site to use current URLs.
 1. [20773](https://github.com/influxdata/influxdb/pull/20773): Fix data race in TSM engine when inspecting tombstone stats.
 1. [20797](https://github.com/influxdata/influxdb/pull/20797): Fix data race in TSM cache. Thanks @StoneYunZhao!
+1. [20798](https://github.com/influxdata/influxdb/pull/20798): Deprecate misleading `retentionPeriodHrs` key in onboarding API.
 
 ## v2.0.4 [2021-02-08]
 

authorizer/task_test.go

@@ -26,11 +26,11 @@ func TestOnboardingValidation(t *testing.T) {
 	ts := authorizer.NewTaskService(zaptest.NewLogger(t), mockTaskService(3, 2, 1))
 
 	r, err := onboard.OnboardInitialUser(context.Background(), &influxdb.OnboardingRequest{
-		User:            "Setec Astronomy",
-		Password:        "too many secrets",
-		Org:             "thing",
-		Bucket:          "holder",
-		RetentionPeriod: 1,
+		User:                   "Setec Astronomy",
+		Password:               "too many secrets",
+		Org:                    "thing",
+		Bucket:                 "holder",
+		RetentionPeriodSeconds: 1,
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -126,11 +126,11 @@ func TestValidations(t *testing.T) {
 	svc, onboard := setup(t)
 
 	r, err := onboard.OnboardInitialUser(context.Background(), &influxdb.OnboardingRequest{
-		User:            "Setec Astronomy",
-		Password:        "too many secrets",
-		Org:             "thing",
-		Bucket:          "holder",
-		RetentionPeriod: 1,
+		User:                   "Setec Astronomy",
+		Password:               "too many secrets",
+		Org:                    "thing",
+		Bucket:                 "holder",
+		RetentionPeriodSeconds: 1,
 	})
 	if err != nil {
 		t.Fatal(err)

cmd/influx/setup.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math"
 	"path/filepath"
 	"strconv"
 	"time"
@@ -226,20 +227,24 @@ func onboardingRequest(ui *input.UI) (*influxdb.OnboardingRequest, error) {
 	}
 
 	req := &influxdb.OnboardingRequest{
-		User:            setupFlags.username,
-		Password:        setupFlags.password,
-		Org:             setupFlags.org,
-		Bucket:          setupFlags.bucket,
-		RetentionPeriod: influxdb.InfiniteRetention,
-		Token:           setupFlags.token,
+		User:                   setupFlags.username,
+		Password:               setupFlags.password,
+		Org:                    setupFlags.org,
+		Bucket:                 setupFlags.bucket,
+		RetentionPeriodSeconds: influxdb.InfiniteRetention,
+		Token:                  setupFlags.token,
 	}
 
 	if setupFlags.retention != "" {
 		dur, err := internal2.RawDurationToTimeDuration(setupFlags.retention)
 		if err != nil {
 			return nil, err
 		}
-		req.RetentionPeriod = dur
+		secs, nanos := math.Modf(dur.Seconds())
+		if nanos > 0 {
+			return nil, fmt.Errorf("retention policy %q is too precise, must be divisible by 1s", setupFlags.retention)
+		}
+		req.RetentionPeriodSeconds = int64(secs)
 	}
 
 	if setupFlags.force {
@@ -268,16 +273,16 @@ func onboardingRequest(ui *input.UI) (*influxdb.OnboardingRequest, error) {
 				"Please type your retention period in hours.\nOr press ENTER for infinite", infiniteStr)
 			rp, err := strconv.Atoi(rpStr)
 			if rp >= 0 && err == nil {
-				req.RetentionPeriod = time.Duration(rp) * time.Hour
+				req.RetentionPeriodSeconds = int64((time.Duration(rp) * time.Hour).Seconds())
 				break
 			}
 		}
 	}
 
 	if confirmed := internal2.GetConfirm(ui, func() string {
 		rp := "infinite"
-		if req.RetentionPeriod > 0 {
-			rp = fmt.Sprintf("%d hrs", req.RetentionPeriod/time.Hour)
+		if req.RetentionPeriodSeconds > 0 {
+			rp = (time.Duration(req.RetentionPeriodSeconds) * time.Second).String()
 		}
 		return fmt.Sprintf(`
 You have entered:

cmd/influxd/upgrade/security_test.go

@@ -183,11 +183,11 @@ func TestUpgradeSecurity(t *testing.T) {
 
 			// onboard admin
 			oReq := &influxdb.OnboardingRequest{
-				User:            "admin",
-				Password:        "12345678",
-				Org:             "testers",
-				Bucket:          "def",
-				RetentionPeriod: influxdb.InfiniteRetention,
+				User:                   "admin",
+				Password:               "12345678",
+				Org:                    "testers",
+				Bucket:                 "def",
+				RetentionPeriodSeconds: influxdb.InfiniteRetention,
 			}
 			oResp, err := setupAdmin(ctx, v2, oReq)
 			require.NoError(t, err)

cmd/influxd/upgrade/setup.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math"
 	"path/filepath"
 	"strconv"
 	"time"
@@ -30,20 +31,24 @@ func onboardingRequest(ui *input.UI, options *options) (*influxdb.OnboardingRequ
 	}
 
 	req := &influxdb.OnboardingRequest{
-		User:            options.target.userName,
-		Password:        options.target.password,
-		Org:             options.target.orgName,
-		Bucket:          options.target.bucket,
-		RetentionPeriod: influxdb.InfiniteRetention,
-		Token:           options.target.token,
+		User:                   options.target.userName,
+		Password:               options.target.password,
+		Org:                    options.target.orgName,
+		Bucket:                 options.target.bucket,
+		RetentionPeriodSeconds: influxdb.InfiniteRetention,
+		Token:                  options.target.token,
 	}
 
 	if options.target.retention != "" {
 		dur, err := internal.RawDurationToTimeDuration(options.target.retention)
 		if err != nil {
 			return nil, err
 		}
-		req.RetentionPeriod = dur
+		secs, nanos := math.Modf(dur.Seconds())
+		if nanos > 0 {
+			return nil, fmt.Errorf("retention policy %q is too precise, must be divisible by 1s", options.target.retention)
+		}
+		req.RetentionPeriodSeconds = int64(secs)
 	}
 
 	if options.force {
@@ -72,16 +77,16 @@ func onboardingRequest(ui *input.UI, options *options) (*influxdb.OnboardingRequ
 				"Please type your retention period in hours.\nOr press ENTER for infinite", infiniteStr)
 			rp, err := strconv.Atoi(rpStr)
 			if rp >= 0 && err == nil {
-				req.RetentionPeriod = time.Duration(rp) * time.Hour
+				req.RetentionPeriodSeconds = int64((time.Duration(rp) * time.Hour).Seconds())
 				break
 			}
 		}
 	}
 
 	if confirmed := internal.GetConfirm(ui, func() string {
 		rp := "infinite"
-		if req.RetentionPeriod > 0 {
-			rp = fmt.Sprintf("%d hrs", req.RetentionPeriod/time.Hour)
+		if req.RetentionPeriodSeconds > 0 {
+			rp = (time.Duration(req.RetentionPeriodSeconds) * time.Second).String()
 		}
 		return fmt.Sprintf(`
 You have entered:

http/swagger.yml

@@ -11143,8 +11143,14 @@ components:
           type: string
         bucket:
           type: string
+        retentionPeriodSeconds:
+          type: integer
         retentionPeriodHrs:
           type: integer
+          deprecated: true
+          description: >
+            Retention period *in nanoseconds* for the new bucket. This key's name has been misleading since OSS 2.0 GA,
+            please transition to use `retentionPeriodSeconds`
       required:
         - username
         - org

onboarding.go

@@ -28,12 +28,13 @@ type OnboardingResults struct {
 // OnboardingRequest is the request
 // to setup defaults.
 type OnboardingRequest struct {
-	User            string        `json:"username"`
-	Password        string        `json:"password"`
-	Org             string        `json:"org"`
-	Bucket          string        `json:"bucket"`
-	RetentionPeriod time.Duration `json:"retentionPeriodHrs,omitempty"`
-	Token           string        `json:"token,omitempty"`
+	User                      string        `json:"username"`
+	Password                  string        `json:"password"`
+	Org                       string        `json:"org"`
+	Bucket                    string        `json:"bucket"`
+	RetentionPeriodSeconds    int64         `json:"retentionPeriodSeconds,omitempty"`
+	RetentionPeriodDeprecated time.Duration `json:"retentionPeriodHrs,omitempty"`
+	Token                     string        `json:"token,omitempty"`
 }
 
 func (r *OnboardingRequest) Valid() error {
@@ -59,3 +60,10 @@ func (r *OnboardingRequest) Valid() error {
 	}
 	return nil
 }
+
+func (r *OnboardingRequest) RetentionPeriod() time.Duration {
+	if r.RetentionPeriodSeconds > 0 {
+		return time.Duration(r.RetentionPeriodSeconds) * time.Second
+	}
+	return r.RetentionPeriodDeprecated
+}

tenant/service_onboarding.go

@@ -141,7 +141,7 @@ func (s *OnboardService) onboardUser(ctx context.Context, req *influxdb.Onboardi
 		OrgID:           org.ID,
 		Name:            req.Bucket,
 		Type:            influxdb.BucketTypeUser,
-		RetentionPeriod: req.RetentionPeriod,
+		RetentionPeriod: req.RetentionPeriod(),
 	}
 
 	if err := s.service.CreateBucket(ctx, ub); err != nil {

tenant/service_onboarding_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/require"
 
-	influxdb "github.com/influxdata/influxdb/v2"
+	"github.com/influxdata/influxdb/v2"
 	"github.com/influxdata/influxdb/v2/authorization"
 	icontext "github.com/influxdata/influxdb/v2/context"
 	"github.com/influxdata/influxdb/v2/kv"
@@ -186,12 +186,44 @@ func TestOnboardService_RetentionPolicy(t *testing.T) {
 		UserID: 123,
 	})
 
+	var retention int64 = 72 * 3600 // 72h
+	onboard, err := svc.OnboardInitialUser(ctx, &influxdb.OnboardingRequest{
+		User:                   "name",
+		Org:                    "name",
+		Bucket:                 "name",
+		RetentionPeriodSeconds: retention,
+	})
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assert.Equal(t, onboard.Bucket.RetentionPeriod, time.Duration(retention) * time.Second, "Retention policy should pass through")
+}
+
+func TestOnboardService_RetentionPolicyDeprecated(t *testing.T) {
+	s, _, _ := NewTestInmemStore(t)
+	storage := tenant.NewStore(s)
+	ten := tenant.NewService(storage)
+
+	authStore, err := authorization.NewStore(s)
+	require.NoError(t, err)
+
+	authSvc := authorization.NewService(authStore, ten)
+
+	// we will need an auth service as well
+	svc := tenant.NewOnboardService(ten, authSvc)
+
+	ctx := icontext.SetAuthorizer(context.Background(), &influxdb.Authorization{
+		UserID: 123,
+	})
+
 	retention := 72 * time.Hour
 	onboard, err := svc.OnboardInitialUser(ctx, &influxdb.OnboardingRequest{
-		User:            "name",
-		Org:             "name",
-		Bucket:          "name",
-		RetentionPeriod: retention,
+		User:                      "name",
+		Org:                       "name",
+		Bucket:                    "name",
+		RetentionPeriodDeprecated: retention,
 	})
 
 	if err != nil {

testing/onboarding.go

@@ -154,11 +154,11 @@ func OnboardInitialUser(
 			},
 			args: args{
 				request: &platform.OnboardingRequest{
-					User:            "admin",
-					Org:             "org1",
-					Bucket:          "bucket1",
-					Password:        "password1",
-					RetentionPeriod: time.Hour * 24 * 7, // 1 week
+					User:                   "admin",
+					Org:                    "org1",
+					Bucket:                 "bucket1",
+					Password:               "password1",
+					RetentionPeriodSeconds: 3600 * 24 * 7, // 1 week
 				},
 			},
 			wants: wants{

tests/pipeline_helpers.go

@@ -55,12 +55,12 @@ func NewPipeline(tb testing.TB, opts ...launcher.OptSetter) *Pipeline {
 
 	// setup default operator
 	res := p.Launcher.OnBoardOrFail(tb, &influxdb.OnboardingRequest{
-		User:            DefaultUsername,
-		Password:        DefaultPassword,
-		Org:             DefaultOrgName,
-		Bucket:          DefaultBucketName,
-		RetentionPeriod: 0, // infinite retention period
-		Token:           OperToken,
+		User:                   DefaultUsername,
+		Password:               DefaultPassword,
+		Org:                    DefaultOrgName,
+		Bucket:                 DefaultBucketName,
+		RetentionPeriodSeconds: influxdb.InfiniteRetention,
+		Token:                  OperToken,
 	})
 
 	p.DefaultOrgID = res.Org.ID