Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes trailing slash issue for namespaces #391

Conversation

petems
Copy link
Contributor

@petems petems commented Apr 10, 2019

  • Currently the answer from the API returns a trailing slash
  • Looking in other resources, we validate against having a trailing slash
  • use filepath.Clean to trim trailing slash from API response
  • Added test to delete resource so test could be run multiple times

Fixes #390

@petems
Copy link
Contributor Author

petems commented Apr 10, 2019

Enterprise binary acceptance test:

$ vault-ent server -dev
==> Vault server configuration:

             Api Address: http://127.0.0.1:8200
                     Cgo: disabled
         Cluster Address: https://127.0.0.1:8201
              Listener 1: tcp (addr: "127.0.0.1:8200", cluster address: "127.0.0.1:8201", max_request_duration: "1m30s", max_request_size: "33554432", tls: "disabled")
               Log Level: info
                   Mlock: supported: false, enabled: false
                 Storage: inmem
                 Version: Vault v1.1.0+prem
             Version Sha: 385f51f541491d531c9bd986697d391523d70c3c
TF_ACC_ENTERPRISE=true make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v  -timeout 120m
=== RUN   TestNamespace_basic
--- PASS: TestNamespace_basic (0.06s)
2019-04-10T15:33:47.306+0100 [INFO]  core: successful mount: namespace=test-namespace-884024748716240442/ path=sys/ type=ns_system
2019-04-10T15:33:47.306+0100 [INFO]  core: successful mount: namespace=test-namespace-884024748716240442/ path=identity/ type=ns_identity
2019-04-10T15:33:47.306+0100 [INFO]  core: successful mount: namespace=test-namespace-884024748716240442/ path=cubbyhole/ type=ns_cubbyhole
2019-04-10T15:33:47.306+0100 [INFO]  core: enabled credential backend: path=token/ type=ns_token
2019-04-10T15:33:47.344+0100 [INFO]  core: disabled credential backend: path=auth/token/
2019-04-10T15:33:47.344+0100 [INFO]  core: successfully unmounted: path=cubbyhole/ namespace=test-namespace-884024748716240442/
2019-04-10T15:33:47.346+0100 [INFO]  core: successfully unmounted: path=identity/ namespace=test-namespace-884024748716240442/
2019-04-10T15:33:47.347+0100 [INFO]  core: successfully unmounted: path=sys/ namespace=test-namespace-884024748716240442/

* Currently the answer from the API returns a trailing slash
* Looking in other resources, we validate against having a trailing slash
* use `filepath.Clean` to trim trailing slash from API response
* Added test to delete resource so test could be run multiple times
@petems petems force-pushed the fix_trailing_slash_issue_namespaces branch from 8d105f5 to b269941 Compare April 10, 2019 15:13
@petems
Copy link
Contributor Author

petems commented Apr 10, 2019

This is actually how it should be done, the API has it even documented that way 😄

	// Allowing writing to a path ending in / makes it extremely difficult to
	// understand user intent for the filesystem-like backends (kv,
	// cubbyhole) -- did they want a key named foo/ or did they want to write
	// to a directory foo/ with no (or forgotten) key, or...? It also affects
	// lookup, because paths ending in / are considered prefixes by some
	// backends. Basically, it's all just terrible, so don't allow it.
	if strings.HasSuffix(req.Path, "/") &&
		(req.Operation == logical.UpdateOperation ||
			req.Operation == logical.CreateOperation) {
		return logical.ErrorResponse("cannot write to a path ending in '/'"), nil
	}

https://github.com/hashicorp/vault/blob/8d15b09ed13423d206425ed070e3a72ebdba70b7/vault/request_handling.go#L430

@tyrannosaurus-becks tyrannosaurus-becks self-assigned this Apr 29, 2019
Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

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

Thanks @petems !

@tyrannosaurus-becks tyrannosaurus-becks merged commit 3c5d9f1 into hashicorp:master Apr 29, 2019
@petems petems deleted the fix_trailing_slash_issue_namespaces branch June 10, 2019 15:48
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
…_namespaces

Fixes trailing slash issue for namespaces
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vault_namespace resource breaks without trailing slash upon reapply
2 participants