Backport to v0.13.x: properly handle init queue for hierarchical paths (

#116)

* properly handle init queue for heirarchical paths

* changelog

CHANGELOG.md

@@ -1,5 +1,10 @@
 ## Unreleased
 
+## v0.13.1
+
+BUG FIXES:
+* fix a panic on init when static roles have names defined as hierarchical paths (https://github.com/hashicorp/vault-plugin-secrets-openldap/pull/115)
+
 ## v0.13.0
 
 FEATURES:

rotation.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/hashicorp/go-secure-stdlib/base62"
@@ -32,6 +33,7 @@ const (
 // rotations have been processed. It lists the roles from storage and searches
 // for any that have an associated static account, then adds them to the
 // priority queue for rotations.
+
 func (b *backend) populateQueue(ctx context.Context, s logical.Storage) {
 	log := b.Logger()
 	log.Info("populating role rotation queue")
@@ -42,9 +44,9 @@ func (b *backend) populateQueue(ctx context.Context, s logical.Storage) {
 		log.Warn("unable to load rotation WALs", "error", err)
 	}
 
-	roles, err := s.List(ctx, staticRolePath)
+	roles, err := b.getAllStaticRoles(ctx, s)
 	if err != nil {
-		log.Warn("unable to list role for enqueueing", "error", err)
+		log.Warn("unable to list roles for enqueueing", "error", err)
 		return
 	}
 
@@ -61,6 +63,10 @@ func (b *backend) populateQueue(ctx context.Context, s logical.Storage) {
 			log.Warn("unable to read static role", "error", err, "role", roleName)
 			continue
 		}
+		if role == nil {
+			log.Error("role not found in storage", "roleName", roleName)
+			continue
+		}
 
 		item := queue.Item{
 			Key:      roleName,
@@ -579,3 +585,50 @@ func (b *backend) popFromRotationQueueByKey(name string) (*queue.Item, error) {
 	}
 	return nil, queue.ErrEmpty
 }
+
+// getAllStaticRoles returns a slice of all static roles from storage
+func (b *backend) getAllStaticRoles(ctx context.Context, s logical.Storage) ([]string, error) {
+	var allRoles []string
+
+	roles, err := s.List(ctx, staticRolePath)
+	if err != nil {
+		return allRoles, fmt.Errorf("unable to list roles: %w", err)
+	}
+
+	// Roles can be defined with hierarchical paths, e.g. "foo/bar/baz". But
+	// the storage.List() call to the top-level static role path will only
+	// return the top-level keys in the hierarchy. So we perform a
+	// non-recursive breadth-first search through all the keys returned from
+	// storage.List(). If a given node points to a static role, we append to
+	// the return slice.
+	for i := 0; i < len(roles); i++ {
+		path := roles[i]
+
+		role, err := b.staticRole(ctx, s, path)
+		if err != nil {
+			return allRoles, fmt.Errorf("unable to read static role %q: %w", path, err)
+		}
+
+		if role == nil && strings.HasSuffix(path, "/") {
+			// this is a directory
+			subPaths, err := s.List(ctx, staticRolePath+path)
+			if err != nil {
+				return allRoles, fmt.Errorf("unable to list roles: %w", err)
+			}
+
+			// append to the roles slice to continue search in the sub-directory
+			for _, subPath := range subPaths {
+				// prevent infinite loop but this should never happen
+				if subPath == "" {
+					continue
+				}
+				subPath = fmt.Sprintf("%s%s", path, subPath)
+				roles = append(roles, subPath)
+			}
+		} else {
+			// a role exists at this path, append to the return slice
+			allRoles = append(allRoles, path)
+		}
+	}
+	return allRoles, nil
+}

rotation_test.go

@@ -16,6 +16,85 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// TestInitQueueHierarchicalPaths tests that the static role rotation queue
+// gets initialized with all the roles from storage.
+func TestInitQueueHierarchicalPaths(t *testing.T) {
+	for _, tc := range []struct {
+		name  string
+		roles []string
+	}{
+		{
+			"empty",
+			[]string{},
+		},
+		{
+			"single-role-non-hierarchical-path",
+			[]string{"a"},
+		},
+		{
+			"single-hierarchical-path",
+			[]string{"a/b/c/d"},
+		},
+		{
+			"multi-role-non-hierarchical-path",
+			[]string{"a", "b"},
+		},
+		{
+			"multi-role-with-hierarchical-path",
+			[]string{"a", "a/b"},
+		},
+		{
+			"multi-role-multi-hierarchical-path",
+			[]string{"a", "a/b", "a/b/c/d/e", "f"},
+		},
+		{
+			"multi-role-all-hierarchical-path",
+			[]string{"a/b", "a/b/c", "d/e/f", "d/e/f/h/i/j", "d/e/f/h/x"},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			config := &logical.BackendConfig{
+				Logger: logging.NewVaultLogger(log.Debug),
+
+				System: &logical.StaticSystemView{
+					DefaultLeaseTTLVal: defaultLeaseTTLVal,
+					MaxLeaseTTLVal:     maxLeaseTTLVal,
+				},
+				StorageView: &logical.InmemStorage{},
+			}
+
+			b := Backend(&fakeLdapClient{throwErrs: false})
+			b.Setup(context.Background(), config)
+
+			b.credRotationQueue = queue.New()
+			initCtx := context.Background()
+			ictx, cancel := context.WithCancel(initCtx)
+			b.cancelQueue = cancel
+
+			defer b.Cleanup(ctx)
+			configureOpenLDAPMount(t, b, config.StorageView)
+
+			for _, r := range tc.roles {
+				createRole(t, b, config.StorageView, r)
+			}
+
+			// Reset the rotation queue to simulate startup memory state
+			b.credRotationQueue = queue.New()
+
+			// Now finish the startup process by populating the queue
+			b.initQueue(ictx, &logical.InitializationRequest{
+				Storage: config.StorageView,
+			})
+
+			queueLen := b.credRotationQueue.Len()
+			if queueLen != len(tc.roles) {
+				t.Fatalf("unexpected rotated queue length: got=%d, want=%d", queueLen, len(tc.roles))
+			}
+		})
+	}
+}
+
 func TestAutoRotate(t *testing.T) {
 	t.Run("auto rotate role", func(t *testing.T) {
 		b, storage := getBackend(false)