pass: changed the way for checking if password-store is initalized by eyJhb · Pull Request #124 · docker/docker-credential-helpers

eyJhb · 2018-09-22T10:53:02Z

Changed the way that docker-credential-helpers check if password-store is initialized.
The current way inserts and removes a password, each time it is called, to make sure that pass is initialized.

This creates a lot of commits if the password-store is made as a git repository (which is quite awful).
The following is how many commits has been made, over a span of ~2 months...

$ git log | grep 'docker-pass-initialized-check' | wc -l
1430

I have changed the way to just do a pass ls, as it will fail if pass is not initialized.
If it should fail, it will fail at a read of pass or insert of pass anyways, and therefore this solution is much more elegant.

GordonTheTurtle · 2018-09-22T10:53:04Z

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "master" git@github.com:eyJhb/docker-credential-helpers.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

Signed-off-by: eyjhbb@gmail.com <eyjhbb@gmail.com>

eyJhb · 2018-09-23T07:12:20Z

This also fixes #120 #92, since those lines has been removed.

daltonmatos · 2018-09-24T13:11:07Z

+	// We just run a `pass ls`, if it fails then pass is not initialized.
+	_, err := p.runPassHelper("", "ls")
 	if err != nil {
 		return fmt.Errorf("error initializing pass: %v", err)


Hello @eyJhb,

What do you think about changing the error message to something on these lines?

password-store not initialized: %v.

Since we are not trying to intialize the password-store, this message could confuse users.

Good point! Kept it as pass instead of password-store, hope that is OK :)

Sure! No problem. 🎉

Thanks for the fix!

No problem! Just hope that it will get approved and merged at some point, but looking at the other PRs, I am not sure it will happen anytime soon.

…lized Signed-off-by: eyjhbb@gmail.com <eyjhbb@gmail.com>

eyJhb · 2018-09-24T15:34:33Z

Just a little ping, @vdemeester @n4ss if you get the time to review and merge this.
Would really like some feedback, and it seems like this can be a somewhat inactive repo at times. :)

vdemeester

SGTM 🧢

eyJhb · 2018-09-25T07:42:56Z

Great! Do not hesitate if you have any questions or I need to do anything, so we can get this merged and close the two PRs and the one issue :)

javabrett · 2019-04-17T06:12:55Z

I've been seeing my pass db for Docker regularly corrupted by insertion and non-deletion of an incomplete docker-pass-initialized-check key, so it would be positive to see this change released promptly from master.

javabrett · 2019-04-17T06:19:50Z

OK here's another essential tip if you find your pass repo constantly corrupted by incomplete docker-pass-initialized-check:

export GPG_TTY=$(tty)

... so that gpg properly prompts for key passphrase, assuming you have one.

full diff: docker/docker-credential-helpers@5241b46...8a9f93a includes: - docker/docker-credential-helpers#29 C.free(unsafe.Pointer(err)) -> C.g_error_free(err) - docker/docker-credential-helpers#124 pass: changed the way for checking if password-store is initalized - addresses docker/docker-credential-helpers#133 docker-credential-pass commits about 10 times every time I run a docker command - docker/docker-credential-helpers#143 Fix docker-credential-osxkeychain list behaviour in case of missing entry in keychain - docker/docker-credential-helpers#139 make docker-credential-wincred work like docker-credential-osxkeychain Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

full diff: docker/docker-credential-helpers@5241b46...8a9f93a includes: - docker/docker-credential-helpers#29 C.free(unsafe.Pointer(err)) -> C.g_error_free(err) - docker/docker-credential-helpers#124 pass: changed the way for checking if password-store is initalized - addresses docker/docker-credential-helpers#133 docker-credential-pass commits about 10 times every time I run a docker command - docker/docker-credential-helpers#143 Fix docker-credential-osxkeychain list behaviour in case of missing entry in keychain - docker/docker-credential-helpers#139 make docker-credential-wincred work like docker-credential-osxkeychain Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: f6a4c76fbb6d594f4874dea5059e82eadfae867a Component: cli

full diff: docker/docker-credential-helpers@5241b46...8a9f93a includes: - docker/docker-credential-helpers#29 C.free(unsafe.Pointer(err)) -> C.g_error_free(err) - docker/docker-credential-helpers#124 pass: changed the way for checking if password-store is initalized - addresses docker/docker-credential-helpers#133 docker-credential-pass commits about 10 times every time I run a docker command - docker/docker-credential-helpers#143 Fix docker-credential-osxkeychain list behaviour in case of missing entry in keychain - docker/docker-credential-helpers#139 make docker-credential-wincred work like docker-credential-osxkeychain Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit f6a4c76) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

full diff: docker/docker-credential-helpers@5241b46...8a9f93a includes: - docker/docker-credential-helpers#29 C.free(unsafe.Pointer(err)) -> C.g_error_free(err) - docker/docker-credential-helpers#124 pass: changed the way for checking if password-store is initalized - addresses docker/docker-credential-helpers#133 docker-credential-pass commits about 10 times every time I run a docker command - docker/docker-credential-helpers#143 Fix docker-credential-osxkeychain list behaviour in case of missing entry in keychain - docker/docker-credential-helpers#139 make docker-credential-wincred work like docker-credential-osxkeychain Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit f6a4c76fbb6d594f4874dea5059e82eadfae867a) Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: 24dcc56123634e048dc0c7d8fa2eeff65b97a33a Component: cli

full diff: docker/docker-credential-helpers@5241b46...8a9f93a includes: - docker/docker-credential-helpers#29 C.free(unsafe.Pointer(err)) -> C.g_error_free(err) - docker/docker-credential-helpers#124 pass: changed the way for checking if password-store is initalized - addresses docker/docker-credential-helpers#133 docker-credential-pass commits about 10 times every time I run a docker command - docker/docker-credential-helpers#143 Fix docker-credential-osxkeychain list behaviour in case of missing entry in keychain - docker/docker-credential-helpers#139 make docker-credential-wincred work like docker-credential-osxkeychain Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit f6a4c76fbb6d594f4874dea5059e82eadfae867a) Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: 11b15544c503d91d8ea3ba782d30d2070a24d84e Component: cli

GordonTheTurtle added the dco/no label Sep 22, 2018

pass: changed the way for checking if password-store is initalized

d6c1f13

Signed-off-by: eyjhbb@gmail.com <eyjhbb@gmail.com>

eyJhb force-pushed the master branch from e13d52b to d6c1f13 Compare September 22, 2018 10:54

GordonTheTurtle removed the dco/no label Sep 22, 2018

This was referenced Sep 23, 2018

docker-credential-pass unusable with PGP Smartcard (with touch enabled) #122

Closed

docker-credential-pass does not work at all #118

Open

daltonmatos reviewed Sep 24, 2018

View reviewed changes

pass: changed error message returned, if password-store is not initia…

9c18f03

…lized Signed-off-by: eyjhbb@gmail.com <eyjhbb@gmail.com>

vdemeester approved these changes Sep 24, 2018

View reviewed changes

nassycheezy approved these changes Sep 25, 2018

View reviewed changes

vdemeester merged commit 123ba1b into docker:master Sep 25, 2018

This was referenced Sep 25, 2018

correct spelling mistake #92

Closed

Fix typo and spelling error #120

Closed

joelnb mentioned this pull request Jan 14, 2019

docker-credential-pass commits about 10 times every time I run a docker command #133

Closed

thaJeztah mentioned this pull request Jun 11, 2019

bump docker-credential-helpers v0.6.2 docker/cli#1935

Merged

This was referenced Jul 4, 2019

[19.03 backport] bump docker-credential-helpers v0.6.3 docker/cli#1983

Merged

[18.09 backport] bump docker-credential-helpers v0.6.3 docker/cli#1984

Merged

Navigation Menu

Uh oh!

pass: changed the way for checking if password-store is initalized - #124

pass: changed the way for checking if password-store is initalized#124
vdemeester merged 2 commits into
docker:masterfrom
eyJhb:master

eyJhb commented Sep 22, 2018

Uh oh!

GordonTheTurtle commented Sep 22, 2018

Uh oh!

eyJhb commented Sep 23, 2018

Uh oh!

daltonmatos Sep 24, 2018

Uh oh!

eyJhb Sep 24, 2018

Uh oh!

daltonmatos Sep 24, 2018

Uh oh!

eyJhb Sep 24, 2018

Uh oh!

eyJhb commented Sep 24, 2018

Uh oh!

vdemeester left a comment

Uh oh!

eyJhb commented Sep 25, 2018

Uh oh!

javabrett commented Apr 17, 2019

Uh oh!

javabrett commented Apr 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

eyJhb commented Sep 22, 2018

Uh oh!

GordonTheTurtle commented Sep 22, 2018

Uh oh!

eyJhb commented Sep 23, 2018

Uh oh!

daltonmatos Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

eyJhb Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

daltonmatos Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

eyJhb Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

eyJhb commented Sep 24, 2018

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

eyJhb commented Sep 25, 2018

Uh oh!

javabrett commented Apr 17, 2019

Uh oh!

javabrett commented Apr 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants