refactor: use argocd-git-ask-pass to pass git credentials to git/kustomize #8516

alexmt · 2022-02-15T22:21:47Z

Signed-off-by: Alexander Matyushentsev AMatyushentsev@gmail.com

In order to provide git credentials to Kustomize Argo CD sets GIT_ASKPASS=git-ask-pass.sh env variable so that git clone call git-ask-pass.sh for credentials. The git-ask-pass.sh needs to get credentials somewhere so we also set GIT_USERNAME and GIT_PASSWORD env variables. The problem is that GIT_USERNAME and GIT_PASSWORD also available to kustomize and user can leverage kustomize config map generator to just print env variables.

Fix:

I think we agreed that it is not safe to have any secret in env variable or file, because one way or another almost all config tools provide a way to read file/env variable. But we can assume config management tool does not allow user to run any custom binary. So proposing following solution:

introduce argocd-git-ask-pass binary and new, localhost-only endpoint in repo-server
during manifests generation repo server generates uuid, associate it with git credentials and fork/exec config management tool with GIT_ASKPASS='argocd-git-ask-pass '
git executes argocd-git-ask-pass , then argocd-git-ask-pass calls send uuid to new repo-server endpoint and receives associated credentials
once manifest generation is completed repo-server forget about uuid and associated credentials

codecov · 2022-02-15T22:53:25Z

Codecov Report

Merging #8516 (1b2a4cf) into master (a84e993) will increase coverage by 0.07%.
The diff coverage is 26.08%.

@@            Coverage Diff             @@
##           master    #8516      +/-   ##
==========================================
+ Coverage   42.32%   42.40%   +0.07%     
==========================================
  Files         176      177       +1     
  Lines       22822    22936     +114     
==========================================
+ Hits         9660     9726      +66     
- Misses      11784    11824      +40     
- Partials     1378     1386       +8

Impacted Files	Coverage Δ
cmd/argocd/commands/app.go	`0.53% <0.00%> (ø)`
pkg/apis/application/v1alpha1/repository_types.go	`60.52% <0.00%> (ø)`
util/git/creds.go	`9.80% <4.34%> (-0.20%)`	⬇️
reposerver/askpass/server.go	`35.29% <35.29%> (ø)`
reposerver/repository/repository.go	`58.66% <57.14%> (-0.18%)`	⬇️
util/git/client.go	`44.74% <100.00%> (ø)`
controller/state.go	`73.90% <0.00%> (-0.54%)`	⬇️
pkg/apis/application/v1alpha1/types.go	`55.08% <0.00%> (ø)`
util/settings/settings.go	`47.91% <0.00%> (+0.69%)`	⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a84e993...1b2a4cf. Read the comment docs.

reposerver/askpass/server.go

util/git/client.go

reposerver/repository/repository.go

crenshaw-dev

Oops, didn't mean to approve yet. :-)

alexec

belt-and-braces - unset the env when forking kustomize process

util/git/creds.go

…omize Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

…ucture Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

cmd/argocd-git-ask-pass/commands/argocd_git_ask_pass.go

util/git/creds.go

crenshaw-dev

Added optional nitpicks, but LGTM!

cmd/argocd-git-ask-pass/commands/argocd_git_ask_pass.go

util/git/creds.go

reposerver/askpass/server_test.go

util/git/creds.go

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

jannfis

LGTM, thanks @alexmt

jessesuen

Took a close look and it looks solid. Great work!

…omize (argoproj#8516) refactor: use argocd-git-ask-pass to pass git credentials to git/kustomize (argoproj#8516) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

…omize (#8516) refactor: use argocd-git-ask-pass to pass git credentials to git/kustomize (#8516) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

patoarvizu · 2022-03-17T20:40:09Z

Is this something that should be available to sidecar plugins too?

I have a sidecar plugin for Tanka, that uses jsonnet-bundler to pull dependencies from private repos. I have the ConfigManagementPlugin configured to have init run jb install, which is how jsonnet-bundler pulls its dependencies, and it calls git under the covers. I assume that because this calls to git are unknown to/unmanaged by Argo, the configured credentials weren't available, but up until 2.2.5, I was able to have a workaround this issue by mounting a git-ask-pass.sh via a ConfigMap, and setting GIT_USERNAME and GIT_PASSWORD as environment variables on the sidecar container. Once I tried to upgrade to 2.3.1, this stopped working.

I understand that this feature is done to prevent potential credential hijacking, but I don't quite understand what about the new feature broke my workaround, and whether I can now leverage this functionality in my custom sidecar, or if I have to find an alternative.

(I can open a new issue to track this if you prefer.)

crenshaw-dev · 2022-03-17T20:46:06Z

@patoarvizu can you open an issue? I'll have to go back and look at the PR to see what was missed. But in theory it shouldn't be too difficult to let the sidecar use the credentials service.

patoarvizu · 2022-03-17T20:56:19Z

@crenshaw-dev Will do!

…omize (argoproj#8516) refactor: use argocd-git-ask-pass to pass git credentials to git/kustomize (argoproj#8516) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> Signed-off-by: wojtekidd <wojtek.cichon@protonmail.com>

alexmt added this to the v2.3 milestone Feb 15, 2022

alexmt requested review from jannfis, crenshaw-dev, jessesuen and leoluz and removed request for jannfis February 15, 2022 22:21

alexmt changed the title ~~feat: support disabling manifest generation using config management tools~~ refactor: use argocd-git-ask-pass to pass git credentials to git/kustomize Feb 15, 2022

alexmt force-pushed the git-ask-pass branch 2 times, most recently from 1cb6ee4 to 4b7c97a Compare February 16, 2022 17:52

crenshaw-dev approved these changes Feb 16, 2022

View reviewed changes

reposerver/askpass/server.go Outdated Show resolved Hide resolved

reposerver/askpass/server.go Show resolved Hide resolved

util/git/client.go Show resolved Hide resolved

reposerver/repository/repository.go Outdated Show resolved Hide resolved

crenshaw-dev requested changes Feb 16, 2022

View reviewed changes

alexmt force-pushed the git-ask-pass branch from 4b7c97a to 02c9e19 Compare February 16, 2022 18:21

alexec reviewed Feb 16, 2022

View reviewed changes

jessesuen reviewed Feb 17, 2022

View reviewed changes

util/git/creds.go Outdated Show resolved Hide resolved

refactor: use argocd-git-ask-pass to pass git credentials to git/kust…

323b086

…omize Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

alexmt force-pushed the git-ask-pass branch 2 times, most recently from 52926c8 to 1a0a509 Compare February 17, 2022 05:20

Apply reviewer notes: move git.CredsStore to the credentials data str…

62c1ebd

…ucture Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

alexmt force-pushed the git-ask-pass branch from 1a0a509 to 62c1ebd Compare February 17, 2022 05:54

terrytangyuan reviewed Feb 17, 2022

View reviewed changes

cmd/argocd-git-ask-pass/commands/argocd_git_ask_pass.go Outdated Show resolved Hide resolved

util/git/creds.go Outdated Show resolved Hide resolved

crenshaw-dev approved these changes Feb 18, 2022

View reviewed changes

cmd/argocd-git-ask-pass/commands/argocd_git_ask_pass.go Outdated Show resolved Hide resolved

util/git/creds.go Outdated Show resolved Hide resolved

reposerver/askpass/server_test.go Outdated Show resolved Hide resolved

util/git/creds.go Outdated Show resolved Hide resolved

apply reviewer notes: move repeated strings into constants

1b2a4cf

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

alexmt force-pushed the git-ask-pass branch from 294ce33 to 1b2a4cf Compare February 18, 2022 22:39

crenshaw-dev approved these changes Feb 22, 2022

View reviewed changes

leoluz approved these changes Feb 22, 2022

View reviewed changes

jannfis approved these changes Feb 22, 2022

View reviewed changes

jessesuen approved these changes Feb 22, 2022

View reviewed changes

alexmt merged commit 764b7a6 into argoproj:master Feb 23, 2022

alexmt deleted the git-ask-pass branch February 23, 2022 01:57

jgwest mentioned this pull request Mar 8, 2022

chore: Update ApplicationSet to latest Argo CD v2.3 dependencies argoproj/applicationset#494

Merged

alexmt mentioned this pull request Mar 8, 2022

fix: downgrade Argo CD dependency to exclude argoproj/argocd#ca2e3041 argoproj/applicationset#533

Merged

patoarvizu mentioned this pull request Mar 18, 2022

Using git credentials in sidecar plugin #8820

Closed

3 tasks

jmcshane mentioned this pull request Aug 10, 2023

ARGOCD_GIT_ASKPASS_NONCE value removed from git creds store before being used by CMP Plugin for git/kustomize #14998

Closed

3 tasks

ImFlog mentioned this pull request Dec 19, 2023

Regression after recent commit, Segmentation fault argoproj-labs/argocd-image-updater#621

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: use argocd-git-ask-pass to pass git credentials to git/kustomize #8516

refactor: use argocd-git-ask-pass to pass git credentials to git/kustomize #8516

alexmt commented Feb 15, 2022

codecov bot commented Feb 15, 2022 •

edited

Loading

crenshaw-dev left a comment

alexec left a comment

crenshaw-dev left a comment

jannfis left a comment

jessesuen left a comment

patoarvizu commented Mar 17, 2022

crenshaw-dev commented Mar 17, 2022

patoarvizu commented Mar 17, 2022

refactor: use argocd-git-ask-pass to pass git credentials to git/kustomize #8516

refactor: use argocd-git-ask-pass to pass git credentials to git/kustomize #8516

Conversation

alexmt commented Feb 15, 2022

codecov bot commented Feb 15, 2022 • edited Loading

Codecov Report

crenshaw-dev left a comment

Choose a reason for hiding this comment

alexec left a comment

Choose a reason for hiding this comment

crenshaw-dev left a comment

Choose a reason for hiding this comment

jannfis left a comment

Choose a reason for hiding this comment

jessesuen left a comment

Choose a reason for hiding this comment

patoarvizu commented Mar 17, 2022

crenshaw-dev commented Mar 17, 2022

patoarvizu commented Mar 17, 2022

codecov bot commented Feb 15, 2022 •

edited

Loading