-
Notifications
You must be signed in to change notification settings - Fork 563
Fix secrets adapters failing fetch on Windows #1350
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
base: main
Are you sure you want to change the base?
Conversation
…ks to system calls, still failing tests
…ed to check tests
…nse, could use bool instead, but having a symbol leaves stub open to other command formats in the future
Hi @akumajoe - can you deploy with Windows with these changes? I think there are lots of things in Kamal that assume a Unix-like environment. You'd probably need to run deployments under the WSL in which case these changes wouldn't be needed I think. |
Hey, @djmb! Sorry, haven't been keeping tabs on this, but yes last I messed with this these changes were necessary to use kamal for deployment with Bitwarden as my secrets manager. Issue originated purely from using the backtick shorthand to call system commands as opposed to calling the system method, which is OS agnostic as far as I can tell. I'm also aware that RoR prefers Mac/Linux in general; I just have a Windows machine and prefer not using WSL if I don't need to. This was the only thing truly blocking my deployment for my test apps, so figured I'd just put a PR out there. I'm sure I can fix whatever conflicts have cropped up since my last merge down, though yeah, if y'all aren't comfortable merging this in for the sake of accommodating Windows, I can remove this PR. |
Hey @akumajoe - no I've no objection to changing this, I was just surprised that it is the only thing that's stopping it working from Windows, but I guess things assuming Linux are all on the application host side! |
Wanted to implement secret fetching as described in this video: https://youtu.be/sPUk9-1WVXI?si=uqoqUZx-IHrp_fxD&t=526
However, on my Windows environment, this call failed due to the nullification of stderr via
{command} 2> /dev/null
whenever the CLI was checked. Was able to fix by using the system method instead. This should work on any OS, and retain the same behavior as before, though I haven't checked on Mac or Linux.(
true
andfalse
were also issues in testing, so i replaced withexit 0
andexit 1
)