Conversation
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
This refactor is well-structured — factory errors are propagated correctly in get, list, and rm, and subcommands that don't need the store (version, help) correctly skip initialization.
One ordering issue was found in set.go where newStore() is called before argument validation, defeating the lazy-init goal for invalid inputs. Verification returned an inconclusive response; the finding is surfaced for author evaluation.
Note: No test covers a factory that returns an error, so the new error-propagation path in all four subcommands is untested.
Collaborator
Author
|
/review |
Root previously took a fully-initialized store.Store, which forced the caller to open the OS keychain before cobra had even dispatched the subcommand. On headless systems with no D-Bus session bus this hangs indefinitely (godbus autolaunches dbus-launch, which waits forever), so even `docker-pass version` and `--help` are unusable. Switch Root to take a StoreFactory — a `func() (store.Store, error)` — and invoke it from each subcommand's RunE. Subcommands that never touch the store (version, help, completion) no longer open the keychain. Signed-off-by: Johannes Großmann <grossmann.johannes@t-online.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root previously took a fully-initialized store.Store, which forced the caller to open the OS keychain before cobra had even dispatched the subcommand. On headless systems with no D-Bus session bus this hangs indefinitely (godbus autolaunches dbus-launch, which waits forever), so even
docker-pass versionand--helpare unusable.Switch Root to take a StoreFactory — a
func() (store.Store, error)— and invoke it from each subcommand's RunE. Subcommands that never touch the store (version, help, completion) no longer open the keychain.