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

secrets/database: Fixes marshalling bug for json.Number types #11451

Merged
merged 7 commits into from
Apr 23, 2021

Conversation

austingebauer
Copy link
Contributor

@austingebauer austingebauer commented Apr 23, 2021

Overview

This PR fixes an issue that prevents numeric arguments from being provided to parameters such as max_*_connections in the database secrets engine. The Vault CLI provides the arguments as type string, so the error only occurs when using a different client, for example,curl. See an example of the error that's returned when providing an integer argument in the testing section below.

Testing

I tested that integer arguments can be provided to configure the database secrets engine with curl. I also wrote a unit test case which exercises the json.Number conversion in mapToStruct().

Example of error that's returned for integer arguments:

$ curl -X PUT -H "X-Vault-Request: true" -H "X-Vault-Token: $(vault print token)" -d '{"allowed_roles":"*","connection_url":"{{username}}/{{password}}@//localhost:1521/oraclepdb","max_open_connections":10,"max_idle_connections":13,"password":"vaultpwd","plugin_name":"vault-plugin-database-oracle","username":"vaultadmin"}' http://127.0.0.1:8200/v1/database/config/oracle

{"errors":["error creating database object: unable to marshal config: proto: invalid type: json.Number"]}

Example of successful request using changes in this PR:

$ curl -X PUT -H "X-Vault-Request: true" -H "X-Vault-Token: $(vault print token)" -d '{"allowed_roles":"*","connection_url":"{{username}}/{{password}}@//localhost:1521/oraclepdb","max_open_connections":10,"max_idle_connections":13,"password":"vaultpwd","plugin_name":"vault-plugin-database-oracle","username":"vaultadmin"}' http://127.0.0.1:8200/v1/database/config/oracle

$ vault read -format=json database/config/oracle
{
  "request_id": "bbc881b3-609a-1360-1a28-83ba8aa9fef5",
  "lease_id": "",
  "lease_duration": 0,
  "renewable": false,
  "data": {
    "allowed_roles": [
      "*"
    ],
    "connection_details": {
      "connection_url": "{{username}}/{{password}}@//localhost:1521/oraclepdb",
      "max_idle_connections": 13,
      "max_open_connections": 10,
      "username": "vaultadmin"
    },
    "password_policy": "",
    "plugin_name": "vault-plugin-database-oracle",
    "root_credentials_rotate_statements": []
  },
  "warnings": null
}

@vercel vercel bot temporarily deployed to Preview – vault April 23, 2021 18:01 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 23, 2021 18:01 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 23, 2021 18:16 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 23, 2021 18:16 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 23, 2021 18:18 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 23, 2021 18:18 Inactive
@calvn
Copy link
Contributor

calvn commented Apr 23, 2021

Can you add a changelog entry for this?

@austingebauer
Copy link
Contributor Author

@calvn - Will do!

@vercel vercel bot temporarily deployed to Preview – vault-storybook April 23, 2021 19:23 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 23, 2021 19:23 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 23, 2021 19:25 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 23, 2021 19:25 Inactive
Copy link
Contributor

@pcman312 pcman312 left a comment

Choose a reason for hiding this comment

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

Just a nit on the changelog entry (since this change affects external DB plugins only)

changelog/11451.txt Outdated Show resolved Hide resolved
Co-authored-by: Michael Golowka <[email protected]>
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 23, 2021 20:38 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 23, 2021 20:38 Inactive
@austingebauer austingebauer merged commit 0d2ae70 into master Apr 23, 2021
@austingebauer austingebauer deleted the db-json-number-fix branch April 23, 2021 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants