allow setting of Master Password via Environment Variable#57
allow setting of Master Password via Environment Variable#57dannyfast wants to merge 22 commits intocloudflare:mainfrom
Conversation
…ng one additional file storage concern, tests passed
| } | ||
|
|
||
| // Testing GOKEY_MASTER environment variable | ||
| os.Setenv("GOKEY_MASTER", "pass1") |
There was a problem hiding this comment.
I think this test is useless, because we're not exercising the code from the cmdline utility at all. This test just goes through the library functions, but env handling is done via cmdline tool.
I think we need a different test here, which invokes gokey tool twice:
- once specifying the password on the cmdline and
- once specifying the password in env variable
and compare the results
There was a problem hiding this comment.
If there is a pattern to run sub-process commands within a testing scope or run I suppose that would be one option. I am not aware of others.
There was a problem hiding this comment.
we can probably add commands directly to https://github.com/cloudflare/gokey/blob/main/.github/workflows/ci.yml (we would need to compile the cmdline tool first anyway)
There was a problem hiding this comment.
We still haven't resolved this
|
can you also, please, rebase your branch as I've fixed the tests for >= Go 1.19 in #59 ? |
…ng one additional file storage concern, tests passed
dannyfast
force-pushed
the
dannyfast-set-master-pw-via-env-var
branch
from
9954045 to
3b5ee8f
Compare
|
Rebased. |
|
Ready for review. Thank you for the suggestions and corrections. |
|
Corrected the go.mod. |
reverted chars to use upper/lower
ignatk
left a comment
ignatk
left a comment
There was a problem hiding this comment.
let's add a cmdline test testing the feature
|
@dannyfast Thanks a ton for taking this on, exactly the feature I needed 👍🏽 Is this already functional of your branch? |
|
Moved and updated the code in #72:
|
4 participants
Linking issue #56