Repository: cerbos/cerbos
Pull request #992 (2022-06-10 10:02:22)

fix: Respect `WithRootDir` option in `(Index).ListSchemaIDs`
The `ListSchemaIDs` method currently assumes that schemas are located in `./_schemas`, but the index's filesystem is not rooted at the policies directory.

This PR refactors the method to delegate to the `SchemaLoader`, which is aware of the policies directory set by `WithRootDir`.


Patch:
diff --git a/internal/storage/index/index.go b/internal/storage/index/index.go
index 51de7141..331cff68 100644
--- a/internal/storage/index/index.go
+++ b/internal/storage/index/index.go
@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
-	"path/filepath"
 	"sort"
 	"sync"
 
@@ -22,7 +21,6 @@ import (
 	"github.com/cerbos/cerbos/internal/namer"
 	"github.com/cerbos/cerbos/internal/observability/metrics"
 	"github.com/cerbos/cerbos/internal/policy"
-	"github.com/cerbos/cerbos/internal/schema"
 	"github.com/cerbos/cerbos/internal/storage"
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
 )
@@ -390,31 +388,8 @@ func (idx *index) ListPolicyIDs(_ context.Context) ([]string, error) {
 	return entries, nil
 }
 
-func (idx *index) ListSchemaIDs(_ context.Context) ([]string, error) {
-	var schemaIds []string
-	err := fs.WalkDir(idx.fsys, schema.Directory, func(path string, d fs.DirEntry, err error) error {
-		if err != nil {
-			return err
-		}
-
-		if d.IsDir() {
-			return nil
-		}
-
-		id, err := filepath.Rel(schema.Directory, path)
-		if err != nil {
-			return fmt.Errorf("failed to generate id from path %s: %w", path, err)
-		}
-
-		schemaIds = append(schemaIds, id)
-
-		return nil
-	})
-	if err != nil {
-		return nil, fmt.Errorf("failed to walk schemas directory: %w", err)
-	}
-
-	return schemaIds, nil
+func (idx *index) ListSchemaIDs(ctx context.Context) ([]string, error) {
+	return idx.schemaLoader.ListIDs(ctx)
 }
 
 func (idx *index) LoadSchema(ctx context.Context, url string) (io.ReadCloser, error) {
diff --git a/internal/storage/index/schema.go b/internal/storage/index/schema.go
index b7f774ca..fe4bb616 100644
--- a/internal/storage/index/schema.go
+++ b/internal/storage/index/schema.go
@@ -5,6 +5,7 @@ package index
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"io/fs"
 	"path/filepath"
@@ -27,6 +28,28 @@ func NewSchemaLoader(fsys fs.FS, rootDir string) *SchemaLoader {
 	return &SchemaLoader{fsys: schemaFS}
 }
 
+func (sl *SchemaLoader) ListIDs(_ context.Context) ([]string, error) {
+	var schemaIds []string
+	err := fs.WalkDir(sl.fsys, ".", func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if d.IsDir() {
+			return nil
+		}
+
+		schemaIds = append(schemaIds, path)
+
+		return nil
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed to walk schemas directory: %w", err)
+	}
+
+	return schemaIds, nil
+}
+
 func (sl *SchemaLoader) Load(_ context.Context, id string) (io.ReadCloser, error) {
 	if sl.err != nil {
 		return nil, sl.err


Test patch:
diff --git a/internal/storage/index/index_test.go b/internal/storage/index/index_test.go
index 4d69f675..af998e1f 100644
--- a/internal/storage/index/index_test.go
+++ b/internal/storage/index/index_test.go
@@ -65,6 +65,24 @@ func TestIndexLoadPolicy(t *testing.T) {
 	})
 }
 
+func TestIndexListSchemaIDs(t *testing.T) {
+	ctx := context.Background()
+	fsys := os.DirFS(test.PathToDir(t, "."))
+
+	idx, err := index.Build(ctx, fsys, index.WithRootDir("store"))
+	require.NoError(t, err)
+
+	ids, err := idx.ListSchemaIDs(ctx)
+	require.NoError(t, err)
+
+	require.Equal(t, []string{
+		"leave_request.json",
+		"principal.json",
+		"purchase_order.json",
+		"salary_record.json",
+	}, ids)
+}
+
 // mkListOfFiles generates a list of policy files under the given filesystem and returns the list.
 func mkListOfFiles(t *testing.T, dir, base string) []string {
 	t.Helper()