Simplify a lot of the mount tuning code (#3285)

http/sys_mount_test.go

@@ -602,7 +602,7 @@ func TestSysTuneMount(t *testing.T) {
 	resp = testHttpPost(t, token, addr+"/v1/sys/mounts/foo/tune", map[string]interface{}{
 		"default_lease_ttl": "72000h",
 	})
-	testResponseStatus(t, resp, 400)
+	testResponseStatus(t, resp, 204)
 
 	// Longer than system default
 	resp = testHttpPost(t, token, addr+"/v1/sys/mounts/foo/tune", map[string]interface{}{

vault/logical_system.go

@@ -230,6 +230,10 @@ func NewSystemBackend(core *Core) *SystemBackend {
 						Type:        framework.TypeString,
 						Description: strings.TrimSpace(sysHelp["tune_max_lease_ttl"][0]),
 					},
+					"description": &framework.FieldSchema{
+						Type:        framework.TypeString,
+						Description: strings.TrimSpace(sysHelp["auth_desc"][0]),
+					},
 				},
 				Callbacks: map[logical.Operation]framework.OperationFunc{
 					logical.ReadOperation:   b.handleAuthTuneRead,
@@ -255,6 +259,10 @@ func NewSystemBackend(core *Core) *SystemBackend {
 						Type:        framework.TypeString,
 						Description: strings.TrimSpace(sysHelp["tune_max_lease_ttl"][0]),
 					},
+					"description": &framework.FieldSchema{
+						Type:        framework.TypeString,
+						Description: strings.TrimSpace(sysHelp["auth_desc"][0]),
+					},
 				},
 
 				Callbacks: map[logical.Operation]framework.OperationFunc{
@@ -1544,40 +1552,52 @@ func (b *SystemBackend) handleTuneWriteCommon(
 		lock = &b.Core.mountsLock
 	}
 
+	lock.Lock()
+	defer lock.Unlock()
+
+	// Check again after grabbing the lock
+	mountEntry = b.Core.router.MatchingMountEntry(path)
+	if mountEntry == nil {
+		b.Backend.Logger().Error("sys: tune failed: no mount entry found", "path", path)
+		return handleError(fmt.Errorf("sys: tune of path '%s' failed: no mount entry found", path))
+	}
+	if mountEntry != nil && !mountEntry.Local && repState.HasState(consts.ReplicationPerformanceSecondary) {
+		return logical.ErrorResponse("cannot tune a non-local mount on a replication secondary"), nil
+	}
+
 	// Timing configuration parameters
 	{
-		var newDefault, newMax *time.Duration
+		var newDefault, newMax time.Duration
 		defTTL := data.Get("default_lease_ttl").(string)
 		switch defTTL {
 		case "":
+			newDefault = mountEntry.Config.DefaultLeaseTTL
 		case "system":
-			tmpDef := time.Duration(0)
-			newDefault = &tmpDef
+			newDefault = time.Duration(0)
 		default:
 			tmpDef, err := parseutil.ParseDurationSecond(defTTL)
 			if err != nil {
 				return handleError(err)
 			}
-			newDefault = &tmpDef
+			newDefault = tmpDef
 		}
 
 		maxTTL := data.Get("max_lease_ttl").(string)
 		switch maxTTL {
 		case "":
+			newMax = mountEntry.Config.MaxLeaseTTL
 		case "system":
-			tmpMax := time.Duration(0)
-			newMax = &tmpMax
+			newMax = time.Duration(0)
 		default:
 			tmpMax, err := parseutil.ParseDurationSecond(maxTTL)
 			if err != nil {
 				return handleError(err)
 			}
-			newMax = &tmpMax
+			newMax = tmpMax
 		}
 
-		if newDefault != nil || newMax != nil {
-			lock.Lock()
-			defer lock.Unlock()
+		if newDefault != mountEntry.Config.DefaultLeaseTTL ||
+			newMax != mountEntry.Config.MaxLeaseTTL {
 
 			if err := b.tuneMountTTLs(path, mountEntry, newDefault, newMax); err != nil {
 				b.Backend.Logger().Error("sys: tuning failed", "path", path, "error", err)
@@ -1586,6 +1606,28 @@ func (b *SystemBackend) handleTuneWriteCommon(
 		}
 	}
 
+	description := data.Get("description").(string)
+	if description != "" {
+		oldDesc := mountEntry.Description
+		mountEntry.Description = description
+
+		// Update the mount table
+		var err error
+		switch {
+		case strings.HasPrefix(path, "auth/"):
+			err = b.Core.persistAuth(b.Core.auth, mountEntry.Local)
+		default:
+			err = b.Core.persistMounts(b.Core.mounts, mountEntry.Local)
+		}
+		if err != nil {
+			mountEntry.Description = oldDesc
+			return handleError(err)
+		}
+		if b.Core.logger.IsInfo() {
+			b.Core.logger.Info("core: mount tuning of description successful", "path", path)
+		}
+	}
+
 	return nil, nil
 }
 

vault/logical_system_helpers.go

@@ -7,61 +7,31 @@ import (
 )
 
 // tuneMount is used to set config on a mount point
-func (b *SystemBackend) tuneMountTTLs(path string, me *MountEntry, newDefault, newMax *time.Duration) error {
-	meConfig := &me.Config
+func (b *SystemBackend) tuneMountTTLs(path string, me *MountEntry, newDefault, newMax time.Duration) error {
+	zero := time.Duration(0)
 
-	if newDefault == nil && newMax == nil {
-		return nil
-	}
-	if newDefault == nil && newMax != nil &&
-		*newMax == meConfig.MaxLeaseTTL {
-		return nil
-	}
-	if newMax == nil && newDefault != nil &&
-		*newDefault == meConfig.DefaultLeaseTTL {
-		return nil
-	}
-	if newMax != nil && newDefault != nil &&
-		*newDefault == meConfig.DefaultLeaseTTL &&
-		*newMax == meConfig.MaxLeaseTTL {
-		return nil
-	}
+	switch {
+	case newDefault == zero && newMax == zero:
+		// No checks needed
 
-	if newMax != nil && newDefault != nil && *newMax < *newDefault {
-		return fmt.Errorf("new backend max lease TTL of %d less than new backend default lease TTL of %d",
-			int(newMax.Seconds()), int(newDefault.Seconds()))
-	}
+	case newDefault == zero && newMax != zero:
+		// No default/max conflict, no checks needed
 
-	if newMax != nil && newDefault == nil {
-		if meConfig.DefaultLeaseTTL != 0 && *newMax < meConfig.DefaultLeaseTTL {
-			return fmt.Errorf("new backend max lease TTL of %d less than backend default lease TTL of %d",
-				int(newMax.Seconds()), int(meConfig.DefaultLeaseTTL.Seconds()))
+	case newDefault != zero && newMax == zero:
+		// No default/max conflict, no checks needed
+
+	case newDefault != zero && newMax != zero:
+		if newMax < newDefault {
+			return fmt.Errorf("backend max lease TTL of %d would be less than backend default lease TTL of %d",
+				int(newMax.Seconds()), int(newDefault.Seconds()))
 		}
 	}
 
-	if newDefault != nil {
-		if meConfig.MaxLeaseTTL == 0 {
-			if newMax == nil && *newDefault > b.Core.maxLeaseTTL {
-				return fmt.Errorf("new backend default lease TTL of %d greater than system max lease TTL of %d",
-					int(newDefault.Seconds()), int(b.Core.maxLeaseTTL.Seconds()))
-			}
-		} else {
-			if newMax == nil && *newDefault > meConfig.MaxLeaseTTL {
-				return fmt.Errorf("new backend default lease TTL of %d greater than backend max lease TTL of %d",
-					int(newDefault.Seconds()), int(meConfig.MaxLeaseTTL.Seconds()))
-			}
-		}
-	}
+	origMax := me.Config.MaxLeaseTTL
+	origDefault := me.Config.DefaultLeaseTTL
 
-	origMax := meConfig.MaxLeaseTTL
-	origDefault := meConfig.DefaultLeaseTTL
-
-	if newMax != nil {
-		meConfig.MaxLeaseTTL = *newMax
-	}
-	if newDefault != nil {
-		meConfig.DefaultLeaseTTL = *newDefault
-	}
+	me.Config.MaxLeaseTTL = newMax
+	me.Config.DefaultLeaseTTL = newDefault
 
 	// Update the mount table
 	var err error
@@ -72,13 +42,12 @@ func (b *SystemBackend) tuneMountTTLs(path string, me *MountEntry, newDefault, n
 		err = b.Core.persistMounts(b.Core.mounts, me.Local)
 	}
 	if err != nil {
-		meConfig.MaxLeaseTTL = origMax
-		meConfig.DefaultLeaseTTL = origDefault
+		me.Config.MaxLeaseTTL = origMax
+		me.Config.DefaultLeaseTTL = origDefault
 		return fmt.Errorf("failed to update mount table, rolling back TTL changes")
 	}
-
 	if b.Core.logger.IsInfo() {
-		b.Core.logger.Info("core: mount tuning successful", "path", path)
+		b.Core.logger.Info("core: mount tuning of leases successful", "path", path)
 	}
 
 	return nil