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

Private repo credentials deleted from disk before plugin is run #7995

Closed
3 tasks done
davemann-mgl opened this issue Dec 20, 2021 · 4 comments · Fixed by #8389 or #15107
Closed
3 tasks done

Private repo credentials deleted from disk before plugin is run #7995

davemann-mgl opened this issue Dec 20, 2021 · 4 comments · Fixed by #8389 or #15107
Assignees
Labels
bug Something isn't working component:cmp Config Management Plugin related issues

Comments

@davemann-mgl
Copy link

davemann-mgl commented Dec 20, 2021

If you are trying to resolve an environment-specific issue or have a one-off question about the edge case that does not require a feature then please consider asking a question in argocd slack channel.

Checklist:

  • I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
  • I've included steps to reproduce the bug.
  • I've pasted the output of argocd version.

Describe the bug

When using ArgoCD 2.2.0 with an existing plugin (Not using the new plugin server), the environment contains a GIT_SSH_COMMAND which includes an ssh identity on shared memory containing the private key for the detected repository, placed there by ArgoCD. In the latest changes for the CMP server, the file is deleted before it can be read due to the pluginEnv's being constructed in a seperate getPluginEnvs method now which itself does a defer cleanup of the SSH key before the plugin itself has run.

https://github.com/argoproj/argo-cd/blame/6d8ff61b16163e960126f2205166513d924a64a9/reposerver/repository/repository.go#L1142

To Reproduce

Create an application with a config management plugin that internally uses git and is pointed at a private repository.
Attempt to refresh the application

Expected behavior

The identity file contained in the GIT_SSH_COMMAND is not deleted before the plugin is run

Screenshots

If applicable, add screenshots to help explain your problem.
N/A

Version

2.2.0

Logs

Warning: Identity file /dev/shm/254152362 not accessible: No such file or directory.
@davemann-mgl davemann-mgl added the bug Something isn't working label Dec 20, 2021
@davemann-mgl
Copy link
Author

https://github.com/argoproj/argo-cd/blame/6d8ff61b16163e960126f2205166513d924a64a9/reposerver/repository/repository.go#L1167 on this commit message you can see it talks on the change being made to support kustomize remote bases.

We use these, but via a plugin, so that we can pass environment variables down to kustomize (as this is now supported in Kustomize - https://github.com/kubernetes-sigs/kustomize/blob/master/api/kv/kv.go#L163, potentially the default kustomize plugin can gain an env field?)

@davemann-mgl davemann-mgl changed the title Kustomize remote bases broken in latest version Private repo credentials deleted from disk before plugin is run Dec 20, 2021
@dhild
Copy link
Contributor

dhild commented Feb 5, 2022

I ran into this same issue. I may have a simple fix in code; but I am just setting up a test environment for the first time so this may take awhile to validate.

dhild added a commit to dhild/argo-cd that referenced this issue Feb 5, 2022
The io.Closer returned from the call to creds.Environ() removes the
underlying file. The desired behavior here is to clean up the
credentials files only after the plugin code has been run, which now
happens one level up in the call stack.

Fixes argoproj#7995

Signed-off-by: D. Ryan Hild <rhild@starbucks.com>
crenshaw-dev added a commit that referenced this issue Apr 8, 2022
The io.Closer returned from the call to creds.Environ() removes the
underlying file. The desired behavior here is to clean up the
credentials files only after the plugin code has been run, which now
happens one level up in the call stack.

Fixes #7995

Signed-off-by: D. Ryan Hild <rhild@starbucks.com>

Co-authored-by: Michael Crenshaw <michael@crenshaw.dev>
ashutosh16 pushed a commit to ashutosh16/argo-cd that referenced this issue Apr 9, 2022
The io.Closer returned from the call to creds.Environ() removes the
underlying file. The desired behavior here is to clean up the
credentials files only after the plugin code has been run, which now
happens one level up in the call stack.

Fixes argoproj#7995

Signed-off-by: D. Ryan Hild <rhild@starbucks.com>

Co-authored-by: Michael Crenshaw <michael@crenshaw.dev>
Signed-off-by: asingh51 <Ashutosh_Singh@intuit.com>
wojtekidd pushed a commit to wojtekidd/argo-cd that referenced this issue Apr 25, 2022
The io.Closer returned from the call to creds.Environ() removes the
underlying file. The desired behavior here is to clean up the
credentials files only after the plugin code has been run, which now
happens one level up in the call stack.

Fixes argoproj#7995

Signed-off-by: D. Ryan Hild <rhild@starbucks.com>

Co-authored-by: Michael Crenshaw <michael@crenshaw.dev>
Signed-off-by: wojtekidd <wojtek.cichon@protonmail.com>
@eupestov
Copy link

I'd suggest to re-open the issue as the fix was reverted in #9105

@crenshaw-dev crenshaw-dev reopened this Jun 2, 2022
@crenshaw-dev crenshaw-dev self-assigned this Jun 2, 2022
@crenshaw-dev crenshaw-dev added the component:cmp Config Management Plugin related issues label Jun 2, 2022
@crenshaw-dev crenshaw-dev added this to the v2.5 milestone Jun 2, 2022
@crenshaw-dev crenshaw-dev modified the milestones: v2.5, v2.6 Sep 15, 2022
@wtam2018 wtam2018 moved this to Backlog in Argo CD Roadmap Dec 12, 2022
@wtam2018 wtam2018 removed this from the v2.6 milestone Dec 12, 2022
@jmcshane
Copy link
Contributor

I have a PR going that should resolve both this issue and #8820. Would love a review: #15107

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component:cmp Config Management Plugin related issues
Projects
Status: Backlog
6 participants