Skip to content

Commit

Permalink
Add MountType to logical.Request
Browse files Browse the repository at this point in the history
  • Loading branch information
@joelthompson
joelthompson committed Apr 20, 2017
1 parent c98898c commit 91a1ad7
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 2 deletions.
5 changes: 5 additions & 0 deletions logical/request.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ type Request struct {
// request path with the MountPoint trimmed off.
MountPoint string `json:"mount_point" structs:"mount_point" mapstructure:"mount_point"`

// MountType is provided so that a logical backend can make decisions
// based on the specific mount type (e.g., if a mount type has different
// aliases, generating different defaults depending on the alias)
MountType string `json:"mount_type" structs:"mount_type" mapstructure:"mount_type"`

// WrapInfo contains requested response wrapping parameters
WrapInfo *RequestWrapInfo `json:"wrap_info" structs:"wrap_info" mapstructure:"wrap_info"`

Expand Down
7 changes: 5 additions & 2 deletions vault/core_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1956,7 +1956,7 @@ path "secret/*" {
}
}

func TestCore_HandleRequest_MountPoint(t *testing.T) {
func TestCore_HandleRequest_MountPointType(t *testing.T) {
noop := &NoopBackend{
Response: &logical.Response{},
}
Expand Down Expand Up @@ -1986,13 +1986,16 @@ func TestCore_HandleRequest_MountPoint(t *testing.T) {
t.Fatalf("err: %v", err)
}

// Verify Path and MountPoint
// Verify Path, MountPoint, and MountType
if noop.Requests[0].Path != "test" {
t.Fatalf("bad: %#v", noop.Requests)
}
if noop.Requests[0].MountPoint != "foo/" {
t.Fatalf("bad: %#v", noop.Requests)
}
if noop.Requests[0].MountType != "noop" {
t.Fatalf("bad: %#v", noop.Requests)
}
}

func TestCore_Standby_Rotate(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions vault/router.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ func (r *Router) routeCommon(req *logical.Request, existenceCheck bool) (*logica
originalPath := req.Path
req.Path = strings.TrimPrefix(req.Path, mount)
req.MountPoint = mount
req.MountType = re.mountEntry.Type
if req.Path == "/" {
req.Path = ""
}
Expand Down Expand Up @@ -304,6 +305,7 @@ func (r *Router) routeCommon(req *logical.Request, existenceCheck bool) (*logica
defer func() {
req.Path = originalPath
req.MountPoint = ""
req.MountType = ""

This comment has been minimized.

Sign in to view

Copy link
@jefferai

jefferai Apr 20, 2017

Member

This looks good, but, for reasons, can you actually replace the values of MountPoint and MountType here with the values from re.mountEntry.Type and mount? So the values carry back to the core logic but with known good values.

This comment has been minimized.

Sign in to view

Copy link
@joelthompson

joelthompson Apr 20, 2017

Author Contributor

So you want me to duplicate the code that's on lines 259-260 here?

This comment has been minimized.

Sign in to view

Copy link
@jefferai

jefferai Apr 20, 2017

Member

Yes. The request is a pointer, and we want to ensure that the backend has not modified it, so that when the request goes back to the core we can trust the values in it. That's the point of the deferred function, mostly.

This comment has been minimized.

Sign in to view

Copy link
@joelthompson

joelthompson Apr 21, 2017

Author Contributor

OK -- looking at the comment and the code for MountPoint (which was apparently written by Armon back in the day), I thought the intention was to do the exact opposite -- undo the changes that were done to the pointer before returning.

Anyway, done!
req.Connection = originalConn
req.ID = originalReqID
req.Storage = nil
Expand Down

1 comment on commit 91a1ad7

@jefferai
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right about that comment, but, the router is the right place to attach that info and for future reasons it will be useful to have in the core after the call!

Please sign in to comment.