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

Credhub set using version cli 2.0.0 to server 1.7 fails. #50

Closed
2 tasks
jmcdice opened this issue Aug 9, 2018 · 7 comments
Closed
2 tasks

Credhub set using version cli 2.0.0 to server 1.7 fails. #50

jmcdice opened this issue Aug 9, 2018 · 7 comments
Labels

Comments

@jmcdice
Copy link

jmcdice commented Aug 9, 2018

What version of the credhub server you are using?
1.7

What version of the credhub cli you are using?
2.0.0

If you were attempting to accomplish a task, what was it you were attempting to do?
credhub set

What did you expect to happen?
value saved

What was the actual behavior?
credhub cli said it saved my value but it did not.

# credhub set -n /concourse/gcpops_dev/testone -t value -v "test value 3"
id: 3a433603-b55a-42ad-840b-4b5e915e4bd0
name: /concourse/gcpops_dev/testone
type: value
value: <redacted>
version_created_at: "2018-08-08T21:12:58Z" 

Looks successful ^^ but it was not.

# credhub get -n /concourse/gcpops_dev/testone
id: 3a433603-b55a-42ad-840b-4b5e915e4bd0
name: /concourse/gcpops_dev/testone
type: value
value: testone 1
version_created_at: "2018-08-08T21:12:58Z" 

At the very least.. the credhub set should fail.

Bonus if the cli detects incompatible client/server combination, or just be backwards compatible.

Please confirm where necessary:

  • I have included a log output
  • My log includes an error message
  • [* ] I have included steps for reproduction

If you are a PCF customer with an Operation Manager (PCF Ops Manager) please direct your questions to support (https://support.pivotal.io/)

@cf-gitbot
Copy link

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

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

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

@athornton2012
Copy link
Contributor

Thanks @jmcdice for your submission. The story to fix this is currently in flight. You can follow along here: https://www.pivotaltracker.com/story/show/160565578

@aeijdenberg
Copy link
Contributor

Um, that story is focused on backward compatibility. The OP here describes a clear bug - credhub set with CLI version 2.0.0 against a 1.7.0 (or in our case 1.9.3) server claims success (exit code 0) but doesn't actually set what it claims to have set.

I'm not sure how that can be considered anything except a serious bug? Am I misunderstanding?

We've just burned an afternoon on this.

It's frustrating to find a 2 month old bug on it that seems stalled pending a broader issue. ie while backwards compatibility is a fine goal, a quick fix to fail fast would surely be far more appropriate than the current fail slow behaviour?

aeijdenberg added a commit to govau/nginx-tlsconfig-release that referenced this issue Oct 5, 2018
@athornton2012
Copy link
Contributor

@aeijdenberg sorry to hear you spent so much time on this issue! I totally agree that this is a bug. But our fix for it is to have our new CLI check the version of the server that you are targeting, and based on that send the correct request. The problem is that the new behavior of the 2.0.0 server removes the no-overwrite mode from credential setting and makes the default behavior to overwrite the existing credential if a set request is received. You are using the old server with the new cli. The new cli never sends the overwrite flag (like it did in the old CLI) so the old server is not updating your credentials. The new cli and the old server are not compatible with each other as documented in our CLI release notes. Once the above story that I linked is done, they will be compatible again.

@aeijdenberg
Copy link
Contributor

@athornton2012 - my issue is that CredHub is being used a database, and as it stands either the CLI or the server is claiming that an update command succeeds, that in fact fails. That's pretty much the worst possible failure mode for any type of database and it leads to incorrect data being stored.

The statement in the release notes is misleading. In nearly any other system a statement like:

The CredHub CLI 2.x.x is only compatible with CredHub Server versions 2.0.0 and greater.

Implies that an operation will fail if they are not compatible. It goes completely against the principle of least surprise that an "incompatible" operation can return a success code.

In our case, I had read that notice, and was pretty sure our server was v2.0.0, so we just deployed it, expecting our test environment would produce a failure if I was wrong and they were not compatible. Instead all our automation succeeded - yet data was not updated in our database.

We have a number of different systems using CredHub - and we have a number of automation systems that will use the CLI (and some the raw API) for setting values. That we now know this is silently failing means that we (and possibly many others) are quietly corrupting our credential databases.

ie I'm glad this is being fixed, but it's surprising this issue isn't receiving more priority or attention.

@chhhavi
Copy link
Contributor

chhhavi commented Oct 8, 2018

@aeijdenberg
I understand your frustrations. The story is now delivered, and is waiting on acceptance. Thanks for your patience.

@chhhavi
Copy link
Contributor

chhhavi commented Oct 9, 2018

New release of credhub-cli is out. You can download it here. Or using the homebrew tap. Closing this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants