Add NamespaceStore to the namespace branch #896

tabilet · 2025-01-15T23:34:23Z

This is the first step toward the full namespace feature in Openbao, described in RFC - Add Namespace Support to OpenBao #787

Changes:

vault/namespace_oss.go, which describes the NamespaceStore struct, is renamed to vault/namespace_openbao.go
Changes in http/util.go to pass namespace string in request context.
API-based testing cases using backend file are placed in command/ns_openbao/file
More changes are needed to support plugins, mounts and auths in namespace.

To run the tests:

$ ./bin/bao server -config=./command/ns_openbao/file.hcl

go to another terminal and run

$ cd command/ns_openbao/file/

$ go test ./...

klaus-sap · 2025-01-20T13:23:09Z

vault/namespaces_openbao.go

+}
+
+func (c *Core) ListNamespaces(includePath bool) []*namespace.Namespace {
+	return []*namespace.Namespace{namespace.RootNamespace}


Function should return all namespaces, not only root namespace

klaus-sap · 2025-01-20T13:23:45Z

vault/namespaces_openbao.go

+	return namespaceByID(ctx, nsID, c)
+}
+
+func (c *Core) ListNamespaces(includePath bool) []*namespace.Namespace {


Function should remain in namespaces_oss.go with copyright HashiCorp

klaus-sap · 2025-01-20T13:24:20Z

vault/namespaces_openbao.go

+	"github.com/openbao/openbao/sdk/v2/logical"
+)
+
+func (c *Core) NamespaceByID(ctx context.Context, nsID string) (*namespace.Namespace, error) {


Function should remain in namespaces_oss.go with copyright HashiCorp

klaus-sap · 2025-01-20T13:24:44Z

vault/namespaces_openbao.go

@@ -0,0 +1,475 @@
+// Copyright (c) HashiCorp, Inc.


New files should have copyright OpenBao, right?

Correct:

Suggested change

// Copyright (c) HashiCorp, Inc.

// Copyright (c) 2025 OpenBao a Series of LF Projects, LLC

klaus-sap · 2025-01-20T13:41:36Z

vault/namespaces_openbao.go

+)
+
+func (c *Core) NamespaceByID(ctx context.Context, nsID string) (*namespace.Namespace, error) {
+	return namespaceByID(ctx, nsID, c)


Implementation of namespaceByID in namespaces.go should be fixed (returns root namespace or error)

cipherboy

First round of comments!

cipherboy · 2025-01-16T00:02:39Z

vault/namespaces_openbao.go

+	"identity",
+}
+
+func (b *SystemBackend) namespacePaths() []*framework.Path {


I think these would best go in logical_system_namespaces.go to separate them from the store implementation and for consistency with the existing code layout.

Shouldn't we rename the file namespaces_openbao.go to namespaces_store.go? The suffix openbao does not indicate the purpose of the file

@klaus-sap I agree, but just NamespaceStore (the struct) should be in that new file named namespace_store.go; the API handlers should follow the naming convention of that subsystem (and thus be logical_system_namespaces). My 2c.

@cipherboy I agree. The API handlers should be in logical_system, but the struct and functions like NewNamespaceStore, setupNamespaceStore, teardownNamespaceStore, SetNamespace, GetNamespace, ListNamespaces, DeleteNamespace should be in namespace_store.go (in the same way as in policy_store.go)

cipherboy · 2025-01-16T00:03:04Z

vault/namespaces_openbao.go

+}
+
+// handleNamespaceSet handles the "/sys/namespaces/<path>" endpoint to set a namespace
+func (b *SystemBackend) handleNamespacesSet() framework.OperationFunc {


Handlers as well.

cipherboy · 2025-01-22T00:33:19Z

http/util.go

@@ -21,7 +22,88 @@ import (
 )

 var (
+	restrictedAPIs = []string{
+		"sys/activation-flags/secrets-sync/activate",


This path does not exist in OpenBao:

Suggested change

"sys/activation-flags/secrets-sync/activate",

cipherboy · 2025-01-22T00:33:40Z

http/util.go

+		"sys/audit",
+		"sys/audit-hash",
+		"sys/config/cors",
+		"sys/config/group-policy-application",


As above:

Suggested change

"sys/config/group-policy-application",

What is the suggested change? :-)

@klaus-sap Ah, ty, looks like that one didn't apply. I've fixed it (remove this line).

cipherboy · 2025-01-22T00:34:05Z

http/util.go

+		"sys/config/state",
+		"sys/config/ui",
+		"sys/decode-token",
+		"sys/experiments",


As above:

Suggested change

"sys/experiments",

cipherboy · 2025-01-22T00:38:17Z

http/util.go

+		"/sys/replication/dr/primary/",
+		"/sys/replication/dr/secondary/",
+		"/sys/replication/performance/primary/",
+		"/sys/replication/performance/secondary/",


These will likely never be in OpenBao:

Suggested change

"/sys/replication/dr/primary/",

"/sys/replication/dr/secondary/",

"/sys/replication/performance/primary/",

"/sys/replication/performance/secondary/",

cipherboy · 2025-01-22T00:39:34Z

vault/namespaces_openbao.go

@@ -0,0 +1,475 @@
+// Copyright (c) HashiCorp, Inc.


Correct:

Suggested change

// Copyright (c) HashiCorp, Inc.

// Copyright (c) 2025 OpenBao a Series of LF Projects, LLC

cipherboy · 2025-01-22T00:40:40Z

vault/namespaces_openbao.go

+
+// NewNamespaceStore creates a new NamespaceStore that is backed
+// using a given view. It used used to durable store and manage named namespace.
+func NewNamespaceStore(ctx context.Context, core *Core, baseView BarrierView, system logical.SystemView, logger hclog.Logger) (*NamespaceStore, error) {


Suggested change

func NewNamespaceStore(ctx context.Context, core *Core, baseView BarrierView, system logical.SystemView, logger hclog.Logger) (*NamespaceStore, error) {

func NewNamespaceStore(ctx context.Context, core *Core, baseView BarrierView, logger hclog.Logger) (*NamespaceStore, error) {

SystemView is unused and I don't think it has a purpose in this code.

This has ramifications elsewhere.

cipherboy · 2025-01-22T00:43:24Z

vault/namespaces_openbao.go

+	return nil
+}
+
+func (ps *NamespaceStore) invalidate(ctx context.Context, path string) error {


Suggested change

func (ps *NamespaceStore) invalidate(ctx context.Context, path string) error {

func (ns *NamespaceStore) invalidate(ctx context.Context, path string) error {

How about ns rather than ps for NamespaceStore?

cipherboy · 2025-01-22T00:43:43Z

vault/request_handling.go

-	if err != nil {
-		return nil, fmt.Errorf("could not parse namespace from http context: %w", err)
-	}
+	//ns, err := namespace.FromContext(ctx)


Not quite sure I understand this? Don't we want to be grabbing the namespace?

klaus-sap · 2025-01-22T13:14:08Z

vault/namespaces_openbao.go

+}
+
+// PatchNamespace is used to update the given namespace
+func (ps *NamespaceStore) PatchNamespace(ctx context.Context, path string, meta map[string]string) error {


PatchNamespace should be supported to update custom_metadata, see https://github.com/openbao/openbao/blob/main/website/content/api-docs/system/namespaces.mdx#patch-namespace

genelet added 3 commits January 7, 2025 12:23

First namespace feature tested in filebackend

aee4152

use inherited storage view for CRUD in NamespaceStore

9099ebf

go mod tidy

3108c40

klaus-sap suggested changes Jan 20, 2025

View reviewed changes

cipherboy reviewed Jan 22, 2025

View reviewed changes

klaus-sap reviewed Jan 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add NamespaceStore to the namespace branch #896

Add NamespaceStore to the namespace branch #896

tabilet commented Jan 15, 2025

klaus-sap Jan 20, 2025

klaus-sap Jan 20, 2025

klaus-sap Jan 20, 2025

klaus-sap Jan 20, 2025

cipherboy Jan 22, 2025

klaus-sap Jan 20, 2025

cipherboy left a comment

cipherboy Jan 16, 2025

klaus-sap Jan 22, 2025 •

edited

Loading

cipherboy Jan 22, 2025

klaus-sap Jan 22, 2025 •

edited

Loading

cipherboy Jan 16, 2025

cipherboy Jan 22, 2025

cipherboy Jan 22, 2025 •

edited

Loading

klaus-sap Jan 22, 2025 •

edited

Loading

cipherboy Jan 22, 2025

cipherboy Jan 22, 2025

cipherboy Jan 22, 2025

cipherboy Jan 22, 2025

cipherboy Jan 22, 2025

cipherboy Jan 22, 2025

cipherboy Jan 22, 2025

cipherboy Jan 22, 2025

klaus-sap Jan 22, 2025

	// Copyright (c) HashiCorp, Inc.
	// Copyright (c) 2025 OpenBao a Series of LF Projects, LLC

	func NewNamespaceStore(ctx context.Context, core Core, baseView BarrierView, system logical.SystemView, logger hclog.Logger) (NamespaceStore, error) {
	func NewNamespaceStore(ctx context.Context, core Core, baseView BarrierView, logger hclog.Logger) (NamespaceStore, error) {

	func (ps *NamespaceStore) invalidate(ctx context.Context, path string) error {
	func (ns *NamespaceStore) invalidate(ctx context.Context, path string) error {

Add NamespaceStore to the namespace branch #896

Are you sure you want to change the base?

Add NamespaceStore to the namespace branch #896

Conversation

tabilet commented Jan 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cipherboy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klaus-sap Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klaus-sap Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cipherboy Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

klaus-sap Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klaus-sap Jan 22, 2025 •

edited

Loading

klaus-sap Jan 22, 2025 •

edited

Loading

cipherboy Jan 22, 2025 •

edited

Loading

klaus-sap Jan 22, 2025 •

edited

Loading