Skip to content

Fix vault paths, enable itest coverage, and display coveralls coverage badge#24

Open
aakselrod wants to merge 3 commits intoNYDIG-OSS:mainfrom
aakselrod:fix-vault-paths
Open

Fix vault paths, enable itest coverage, and display coveralls coverage badge#24
aakselrod wants to merge 3 commits intoNYDIG-OSS:mainfrom
aakselrod:fix-vault-paths

Conversation

@aakselrod
Copy link
Member

@aakselrod aakselrod commented Jun 8, 2023

This PR updates the vault plugin paths to ensure that the node ID is in the path rather than in query parameters for the methods sign, ecdh, and accounts. This allows the node ID to be specified in granular ACLs to allow user assignment with various privileges to nodes.

For example, a user can have access to lnd-nodes/nodeApubkey but not lnd-nodes/nodeBpubkey. Alternatively, a user could have access to lnd-nodes/nodeApubkey/ecdh and lnd-nodes/nodeApubkey/accounts, but not lnd-nodes/nodeApubkey/sign, while having access to all of lnd-nodes/nodeBpubkey paths.

The PR also updates tests to work with the new path format, the README.md instructions, and adds an integration test to show that the node specified in the path isn't overridden by a query parameter specifying the node. This fixes #22.

The PR then updates lnd and bitcoind to the latest version, and vault to the version we use in our testnet sandbox.

Finally, the PR updates the integration tests to ensure lndsignerd and vault-plugin-lndsigner exit cleanly at the end of the tests, and enables coverage analysis on the integration tests. This fixes #21.

@aakselrod aakselrod requested a review from joostjager June 8, 2023 16:13
@aakselrod
Copy link
Member Author

@jwelsh-nydig Can you take a look at this and let me know if this is what you want? I can't request you as a reviewer but would like your eyes on it.

@aakselrod aakselrod force-pushed the fix-vault-paths branch 3 times, most recently from 88d6795 to bcd596e Compare June 26, 2023 23:30
@aakselrod aakselrod changed the title Fix vault paths Fix vault paths, enable itest coverage, and display coveralls coverage badge Jun 26, 2023
@joostjager joostjager removed their request for review February 24, 2025 19:24
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.

Update vault plugin to expect node ID in path rather than params for granular ACLs Add coverage for integration tests

1 participant