search

cloudfoundry / credhub-cli

CredHub CLI provides a command line interface to interact with CredHub servers
Apache License 2.0
39 stars 44 forks source link

Add CLI support for client certificates #103

Closed 46bit closed 4 years ago

46bit commented 4 years ago

Hi CredHub. This is a small enhancement that will be very valuable to some CredHub app secrets work that I'm doing.

What

This PR allows specifying a client certificate to use to authenticate with CredHub. I added two new environment variables for this, CREDHUB_CLIENT_CERT_PATH and CREDHUB_CLIENT_KEY_PATH. The credhub library inside this codebase already supported client certificates so this PR is mostly refactoring and new tests.

Why

We want to use the CF Instance Identity CredHub roles (named mtls-app:APP_GUID) to control access to secrets. There is already a credhub-buildpack for flexibly pulling secrets from CredHub. That buildpack relies on this CLI, and so without this PR it can't make use of those Instance Identity roles.

Testing

I've added tests for things that have changed or are new.

I've checked that with this PR I can get and set credentials in CredHub using the CF instance identity client certificates.

Some of the "when a new API is provided" tests in commands were already failing for me before this work.

cf-gitbot commented 4 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/174312139

The labels on this github issue will be updated when the story is started.

46bit commented 4 years ago

Hi Bruce. Thanks for the review. I'm still going to fix up this PR but we've been quite busy and haven't had the chance yet.

bruce-ricard commented 4 years ago

Hi @46bit . Just a friendly ping since it's been almost a month since our last message. Are you still planning on updating the PR?

In particular, was our response wrt the "non client credentials" topic clear to you?

@peterhaochen47 and I

46bit commented 4 years ago

@bruce-ricard Hi. We've been trying to run Vault instead of CredHub, and that hasn't left much time for this. I see where you're coming from with the restructure, but since it doesn't affect functionality it's not something I can make time for. If you're up for refactoring as you have in mind then feel free; otherwise I suppose close this and someone else can finish it if they need it.

jknostman3 commented 4 years ago

@46bit Thanks for the feedback. Based on your suggestion we will close this PR for now. Let us know if you have any questions.