Skip to content
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

app: Fix for MacOS PATH when run under Finder #1908

Merged
merged 1 commit into from
May 29, 2024
Merged

Conversation

illume
Copy link
Contributor

@illume illume commented Apr 15, 2024

On at least MacOS, GUI apps do not take on the shell PATH.
However they are needed for kubectl auth plugins, and perhaps other binaries like minikube

Fixes #1885

How to test

  • How to test:
    • change main.ts pathInfoDebug = true
    • make app-macos
    • Run the macos app from the Finder (do not launch it from the terminal)
    • See alert message with "previousPath" and "newPath" should match the same path as the terminal/shell.

image

@illume
Copy link
Contributor Author

illume commented Apr 15, 2024

The frontend failure is unrelated to this PR, and is fixed in #1910

@illume illume added bug Something isn't working app AKS Related to Azure Kubernetes Engine labels Apr 15, 2024
@illume illume force-pushed the fix-path-macos branch 2 times, most recently from f44f426 to 1640422 Compare April 15, 2024 14:49
@illume illume added this to the v0.24.0 milestone Apr 15, 2024
app/electron/main.ts Outdated Show resolved Hide resolved
@illume illume marked this pull request as draft April 23, 2024 12:09
@illume illume added the mac Issues related to Mac OS label May 11, 2024
@illume illume marked this pull request as ready for review May 11, 2024 12:36
@illume illume requested review from joaquimrocha and a team May 13, 2024 07:38
@illume
Copy link
Contributor Author

illume commented May 20, 2024

@ashu8912 to recap where we are up to, and suggestions on next steps...

  • the azure/kubelogin isn't working in headlamp (in Headlamp started by terminal, or in Headlamp started by finder). So this is not a good thing to test that this PR is working.
    - [ ] aws-iam-authenticator hasn't been tested yet (@ashu8912 can you do this one?)
  • I will outline some steps to test more clearly with the minikube/az binaries as well (which might be on a terminal path but not in the PATH for the user running the app from finder) @illume to do
  • there should also be a check to see if the app is NOT being run by the terminal, and in that case there is no need to modify the PATH. @illume to do
  • Add a logging of the PATH variable, which would allow us to check that it is equivalent to the terminal one easily

@illume illume marked this pull request as draft May 20, 2024 08:21
@illume
Copy link
Contributor Author

illume commented May 27, 2024

Hi @ashu8912 I made changes to the PR.

Also outlined some much simpler testing steps, to just check the PATH from the terminal is being added.

@illume illume changed the title app: Fix for MacOS auth plugins app: Fix for MacOS PATH when run under Finder May 27, 2024
@illume illume marked this pull request as ready for review May 27, 2024 05:34
On at least MacOS, GUI apps do not take on the shell paths.
However they are needed for kubectl auth plugins,
and other binaries like minikube.

Fixes #1885

Signed-off-by: René Dudfield <[email protected]>
@illume
Copy link
Contributor Author

illume commented May 29, 2024

@joaquimrocha I've updated the test instructions and added a screen shot showing it works to the PR description.

@joaquimrocha joaquimrocha merged commit 730b800 into main May 29, 2024
15 checks passed
@joaquimrocha joaquimrocha deleted the fix-path-macos branch May 29, 2024 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AKS Related to Azure Kubernetes Engine app bug Something isn't working mac Issues related to Mac OS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

external auth (exec) not working (azure/kubelogin and aws-iam-authenticator)
2 participants