Prompt for secrets if not provided on CLI #616

Merged
3wordchant merged 3 commits from calix/646 into main 2025-08-26 17:15:48 +00:00
Owner

Re #646

NB I added an integration test to check for the expected failure if the user provides --no-input, but I didn't add a test for the new interactive prompt behaviour because I couldn't find an existing example, and adding expect seemed to be a Big Change Probably Requiring Some Discussion.

Re #646 NB I added an integration test to check for the expected failure if the user provides `--no-input`, but I didn't add a test for the new interactive prompt behaviour because I couldn't find an existing example, and adding `expect` seemed to be a Big Change Probably Requiring Some Discussion.
3wordchant added 1 commit 2025-08-25 21:18:41 +00:00
Prompt for secrets if not provided on CLI
All checks were successful
continuous-integration/drone/push Build is passing
continuous-integration/drone/pr Build is passing
238647a987
Re #646
3wordchant requested review from decentral1se 2025-08-25 21:18:41 +00:00
decentral1se reviewed 2025-08-25 21:35:06 +00:00
decentral1se left a comment
Owner

Radical ☀️ I would also just do a bunch of manual testing for this and call it a day. I don't think we test interactive output anywhere in any part of our testing infra. So yeh, fuck it.

Radical ☀️ I would also just do a bunch of manual testing for this and call it a day. I don't think we test interactive output anywhere in any part of our testing infra. So yeh, fuck it.
@ -192,0 +196,4 @@
if len(args) > 3 {
data = args[3]
} else if internal.NoInput {
log.Fatal("Must provide <data> argument if --no-input is passed")
Owner

Gotta do the i18n.G(...) string wrapping translation dance 🙃

Also, convention for logging is usually no capitalisation / lower-case 🤷

Gotta do the `i18n.G(...)` string wrapping translation dance 🙃 Also, convention for logging is usually no capitalisation / lower-case 🤷
3wordchant marked this conversation as resolved
@ -213,0 +225,4 @@
if insertFromFile {
message = "Specify secret file"
}
prompt := &survey.Input{
Owner
[`survey.Password`](https://github.com/AlecAivazis/survey?tab=readme-ov-file#password)
3wordchant marked this conversation as resolved
Member

Nice! It will conflict with my changes though #614
I can update my PR to include your commit and make it work with the stdin reading

Nice! It will conflict with my changes though https://git.coopcloud.tech/toolshed/abra/pulls/614 I can update my PR to include your commit and make it work with the stdin reading
decentral1se added this to the Abra v0.11.x project 2025-08-25 21:39:02 +00:00
3wordchant added 1 commit 2025-08-25 22:12:27 +00:00
Implement PR feedback
All checks were successful
continuous-integration/drone/push Build is passing
7b7477062f
Author
Owner

Ty for the review @decentral1se ! Made those changes

Nice! It will conflict with my changes though #614

Argh sorry @p4u1, let me know if I can help.

Ty for the review @decentral1se ! Made those changes > Nice! It will conflict with my changes though #614 Argh sorry @p4u1, let me know if I can help.
decentral1se reviewed 2025-08-26 06:11:20 +00:00
@ -211,2 +220,4 @@
}
if data == "" && !internal.NoInput {
log.Debug("Secret data not provided on command-line, prompting")
Owner

i18n.G / capitalisaion 👁‍🗨

`i18n.G` / capitalisaion 👁‍🗨
3wordchant marked this conversation as resolved
@ -213,0 +224,4 @@
var prompt survey.Prompt
if !insertFromFile {
prompt = &survey.Password{
Message: i18n.G("Specify secret value"),
Owner

i18n.G / capitalisaion 👁‍🗨

`i18n.G` / capitalisaion 👁‍🗨
3wordchant marked this conversation as resolved
@ -213,0 +228,4 @@
}
} else {
prompt = &survey.Input{
Message: i18n.G("Specify secret file"),
Owner

i18n.G / capitalisaion 👁‍🗨

`i18n.G` / capitalisaion 👁‍🗨
3wordchant marked this conversation as resolved
Owner

We probably do need that fix suggested in #614 (comment) to wrap the i18n.G call inside the logging functions. If someone is keen to pick it up 🙏 Feel free to merge anyway and come back on it. Happy merge conflict resolving 🤸

We probably do need that fix suggested in https://git.coopcloud.tech/toolshed/abra/pulls/614#issuecomment-26246 to wrap the `i18n.G` call inside the logging functions. If someone is keen to pick it up 🙏 Feel free to merge anyway and come back on it. Happy merge conflict resolving 🤸
3wordchant added 1 commit 2025-08-26 17:15:15 +00:00
Moar PR feedback
All checks were successful
continuous-integration/drone/push Build is passing
7dd7f763f4
Author
Owner

OK additional tweaks made, mergin'

OK additional tweaks made, mergin'
3wordchant merged commit 7dd7f763f4 into main 2025-08-26 17:15:48 +00:00
3wordchant deleted branch calix/646 2025-08-26 17:15:49 +00:00
decentral1se moved this to Done in Abra v0.11.x on 2025-08-27 14:00:39 +00:00
decentral1se removed this from the Abra v0.11.x project 2025-08-30 10:14:56 +00:00
Sign in to join this conversation.
No description provided.