argocd-image-updater icon indicating copy to clipboard operation
argocd-image-updater copied to clipboard

Regression after recent commit, Segmentation fault

Open ZOrfeas opened this issue 2 years ago • 11 comments

Describe the bug Segmentation fault due to (semi-)recent commit. On release v0.12.2 argocd-image-updater functions properly. But the current version in the master branch segfaults. The commit to blame is most probably d5a8f94 , this is the first one to break from the ones between v0.12.2 and now.

To Reproduce Not extremely straightforward, but currently using a very simple setup, with a helm chart for argocd-image-updater.

Chart config:
  image:
    repository: "<some-registry>/<something>/argocd-image-updater"
    pullPolicy: Always
    tag: "v0.12.2-patched"
  config:
    gitCommitTemplate: |
      build: automatic update of {{ .AppName }} [skip ci]

      {{ range .AppChanges -}}
      updates image {{ .Image }}:
        from '{{ .OldTag }}'
        to   '{{ .NewTag }}'
      {{ end -}}
    registries:
      - name: <Some registry name>
        api_url: https://<some-registry-api-url>
        prefix: <some-prefix>
        credentials: <redacted>
        default: true
  extraArgs:
    - --interval
    - 30s
And the relevant argo-Application annotations
    argocd-image-updater.argoproj.io/image-list: web-server=<some-registry>/<something>/<some-image>
    argocd-image-updater.argoproj.io/web-server.helm.image-name: image.repository
    argocd-image-updater.argoproj.io/web-server.update-strategy: latest # can also use "newest-build" if using manually built latest version of argocd-image-updater
    argocd-image-updater.argoproj.io/write-back-method: git

Expected behavior To not segfault :)

Additional context Encountered while trying to add a feature required for my team

Version First commit to break seems to be d5a8f94

Logs

Crash logs
time="2023-09-14T13:25:56Z" level=info msg=Trace args="[rm -rf /tmp/git-<redacted>3928236953]" dir= operation_name="exec rm" time_ms=1.767175
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0xc3eb4d]

goroutine 660 [running]:
github.com/argoproj/argo-cd/v2/util/git.HTTPSCreds.Environ({{0xc0004f5078, 0x7}, {0xc000b6b2e0, 0x1a}, 0x0, {0x0, 0x0}, {0x0, 0x0}, {0x0, ...}, ...})
	/go/pkg/mod/github.com/argoproj/argo-cd/[email protected]/util/git/creds.go:218 +0xb6d
github.com/argoproj-labs/argocd-image-updater/ext/git.(*nativeGitClient).runCredentialedCmd(0xc0001fc2a0, {0x244e214?, 0xc000a79e90?}, {0xc00099b2d8?, 0x6?, 0x27c9478?})
	/src/argocd-image-updater/ext/git/client.go:595 +0x8b
github.com/argoproj-labs/argocd-image-updater/ext/git.(*nativeGitClient).Fetch(0xc0001fc2a0, {0x0?, 0x0?})
	/src/argocd-image-updater/ext/git/client.go:332 +0x236
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.commitChangesGit(0xc0004a6c00, 0xc0002fa320, {0xc0007cc720, 0x1, 0x1}, 0x25852f0)
	/src/argocd-image-updater/pkg/argocd/git.go:159 +0x365
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.commitChanges(0xc0004b6fd0?, 0xc00088b050?, {0xc0007cc720?, 0xc00099b690?, 0x1be27fa?})
	/src/argocd-image-updater/pkg/argocd/update.go:563 +0x5b
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.commitChangesLocked(0xc0004a6c00?, 0xc0002fa320?, 0x34?, {0xc0007cc720?, 0x1?, 0x1?})
	/src/argocd-image-updater/pkg/argocd/update.go:543 +0x10c
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.UpdateApplication(0xc00099bb88, 0x19?)
	/src/argocd-image-updater/pkg/argocd/update.go:331 +0x1f2d
main.runImageUpdater.func1({_, _}, {{{{0xc0004f4910, 0xb}, {0xc0009fcf30, 0x14}}, {{0xc0004f4990, 0xa}, {0x0, 0x0}, ...}, ...}, ...})
	/src/argocd-image-updater/cmd/run.go:313 +0x287
created by main.runImageUpdater
	/src/argocd-image-updater/cmd/run.go:299 +0xa1e

ZOrfeas avatar Sep 14 '23 13:09 ZOrfeas

I know this is not an error on an active release... but seems a bit important to ignore...

I don't want to be that guy.. but is this project actively maintained?

ZOrfeas avatar Oct 30 '23 16:10 ZOrfeas

I can confirm that this issue is still happening. I was interested on the latests version of argo-cd image updater (namely #636). If possible I will try to find some time to pin the issue (and hopefully fix it).

ImFlog avatar Dec 14 '23 15:12 ImFlog

After some investigation I think the issue is related to the version bump of github.com/argoproj/argo-cd/v2 v2.2.7 to version v2.7.7 I think the related MR is https://github.com/argoproj/argo-cd/pull/8516 (when the failing line was introduced). I think it might be related to the cred store. Still looking ...

ImFlog avatar Dec 19 '23 10:12 ImFlog

I found the issue, it's here : https://github.com/argoproj-labs/argocd-image-updater/blob/7d93c7ab1521e0928764eb8c7bb5b27656b051a2/pkg/argocd/gitcreds.go#L46 Starting with the MR 8516 linked above, we need a CredsStore to fetch credentials. If we pass nil we segfault because it's not what's excepted. I tried to pass a repo.GetGitCreds(git.NoopCredsStore{}) but I end up with the following stack trace:

│ time="2023-12-21T16:06:19Z" level=error msg="`git fetch origin --tags --force` failed exit status 128: error: cannot run argocd: No such file or directory\nfatal: could not read Username for 'https://gitlab.com': terminal prompts disabled" execID=cffc2                  │

Which is a bit better but still not working. I should mention that my credentials are effectively present in the created Git client (not an issue with my credentials per se).

To be honest I am not really sure what is supposed to be passed in the credsStore, @alexmt, @jannfis I saw that you participated in the MR that introduced this change (https://github.com/argoproj/argo-cd/pull/8516). Any help on the subject ?

ImFlog avatar Dec 21 '23 16:12 ImFlog

Updated to the new 0.13 when it came out a couple of days ago, and haven't seen any updates since. Just noticed that it's because of this bug, the container just crashes as soon as an update is found.

Ulrar avatar May 17 '24 15:05 Ulrar

It's happening to me too in the new version v0.13. With old version v0.12.2 works all OK. Any help? Thanks.

CygnusHyoga avatar May 17 '24 17:05 CygnusHyoga

Same here. What's strange: I'm operating two clusters (for dev and prod) and in the dev cluster it's running. Both clusters have an equivalent configuration, i.e. the config differs mostly in secrets (and scalability ofc). So why it's running fine in my dev cluster, but in my prod cluster it's bailing out?!

christian-schlichtherle avatar May 18 '24 09:05 christian-schlichtherle

Here's the stack trace:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x7761d4]
goroutine 889 [running]:
github.com/argoproj/argo-cd/v2/util/git.HTTPSCreds.Environ({{0x40012b6d18, 0x8}, {0x4000f76db0, 0x28}, 0x0, {0x0, 0x0}, {0x0, 0x0}, {0x0, ...}, ...})
	/go/pkg/mod/github.com/argoproj/argo-cd/[email protected]/util/git/creds.go:218 +0x904
github.com/argoproj-labs/argocd-image-updater/ext/git.(*nativeGitClient).runCredentialedCmd(0x40002ce1c0, {0x2086fed?, 0x1cc6d60?}, {0x4000cbf270?, 0x208a662?, 0x6?})
	/src/argocd-image-updater/ext/git/client.go:595 +0x68
github.com/argoproj-labs/argocd-image-updater/ext/git.(*nativeGitClient).Fetch(0x40002ce1c0, {0x0?, 0x400128a8c0?})
	/src/argocd-image-updater/ext/git/client.go:332 +0x180
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.commitChangesGit(0x4000c8a800, 0x4000fac630, {0x4000e04408, 0x1, 0x1}, 0x21d1088)
	/src/argocd-image-updater/pkg/argocd/git.go:159 +0x29c
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.commitChanges(0x4000d3e0c0?, 0x4000e91fb0?, {0x4000e04408?, 0x4000124b80?, 0x4000cbf648?})
	/src/argocd-image-updater/pkg/argocd/update.go:627 +0x58
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.commitChangesLocked(0x400147b100?, 0x4000fac630?, 0x34?, {0x4000e04408?, 0x1?, 0x1?})
	/src/argocd-image-updater/pkg/argocd/update.go:607 +0x118
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.UpdateApplication(0x4000cbfb50, 0x19?)
	/src/argocd-image-updater/pkg/argocd/update.go:333 +0x1a28
main.runImageUpdater.func1({_, _}, {{{{0x4000e96900, 0xb}, {0x4000f831b8, 0x14}}, {{0x4000e96980, 0xa}, {0x0, 0x0}, ...}, ...}, ...})
	/src/argocd-image-updater/cmd/run.go:313 +0x1b4
created by main.runImageUpdater
	/src/argocd-image-updater/cmd/run.go:299 +0x890

Looking at the source code, it seems like c.store is the null reference. It seems like it's not setup when the client is created.

christian-schlichtherle avatar May 18 '24 18:05 christian-schlichtherle

I'll take this recent activity on this issue to mention that the delay in handling this (even after my very detailed report), or even getting any feedback was one of the main reasons for our team choosing to not go for this tool.

Seeing as there even was a new release without even addressing this at all I feel justified in our decision.

Sorry for minor venting

ZOrfeas avatar May 18 '24 18:05 ZOrfeas

I did an experiment and changed my configuration from HTTPSCreds to GitHubAppCreds, but the behavior is the same:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x77797c]
goroutine 893 [running]:
github.com/argoproj/argo-cd/v2/util/git.GitHubAppCreds.Environ({0xdc1d3, 0x308d063, {0x4000da2e00, 0x68f}, {0x0, 0x0}, {0x400085c0c0, 0x2e}, {0x0, 0x0}, ...})
	/go/pkg/mod/github.com/argoproj/argo-cd/[email protected]/util/git/creds.go:388 +0x7cc
github.com/argoproj-labs/argocd-image-updater/ext/git.(*nativeGitClient).runCredentialedCmd(0x400053c2a0, {0x2086fed?, 0x1cc6d60?}, {0x4000873270?, 0x208a662?, 0x6?})
	/src/argocd-image-updater/ext/git/client.go:595 +0x68
github.com/argoproj-labs/argocd-image-updater/ext/git.(*nativeGitClient).Fetch(0x400053c2a0, {0x0?, 0x4000c94600?})
	/src/argocd-image-updater/ext/git/client.go:332 +0x180
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.commitChangesGit(0x4000201000, 0x4001228840, {0x4001120de0, 0x1, 0x1}, 0x21d1088)
	/src/argocd-image-updater/pkg/argocd/git.go:159 +0x29c
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.commitChanges(0x400043b190?, 0x400085c0c0?, {0x4001120de0?, 0x400053bdc0?, 0x4000873648?})
	/src/argocd-image-updater/pkg/argocd/update.go:627 +0x58
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.commitChangesLocked(0x400094f7c0?, 0x4001228840?, 0x34?, {0x4001120de0?, 0x1?, 0x1?})
	/src/argocd-image-updater/pkg/argocd/update.go:607 +0x118
github.com/argoproj-labs/argocd-image-updater/pkg/argocd.UpdateApplication(0x40006e5b50, 0x19?)
	/src/argocd-image-updater/pkg/argocd/update.go:333 +0x1a28
main.runImageUpdater.func1({_, _}, {{{{0x4000d94460, 0xb}, {0x4000307a10, 0x14}}, {{0x4000d944f0, 0xa}, {0x0, 0x0}, ...}, ...}, ...})
	/src/argocd-image-updater/cmd/run.go:313 +0x1b4
created by main.runImageUpdater
	/src/argocd-image-updater/cmd/run.go:299 +0x890

When you check the first stack frame you can see that it now bails out in line 388 instead of line 218 in the exact same file util/git/creds.go. The lines have in common that they try to store a nonce in c.store, where c is an implementation of (HTTPS|GitHubApp)Creds. However, that store reference is null and so the app bails out with a SIGSEGV.

christian-schlichtherle avatar May 19 '24 08:05 christian-schlichtherle

I found the issue, it's here :

https://github.com/argoproj-labs/argocd-image-updater/blob/7d93c7ab1521e0928764eb8c7bb5b27656b051a2/pkg/argocd/gitcreds.go#L46

Starting with the MR 8516 linked above, we need a CredsStore to fetch credentials. If we pass nil we segfault because it's not what's excepted. I tried to pass a repo.GetGitCreds(git.NoopCredsStore{}) but I end up with the following stack trace:

│ time="2023-12-21T16:06:19Z" level=error msg="`git fetch origin --tags --force` failed exit status 128: error: cannot run argocd: No such file or directory\nfatal: could not read Username for 'https://gitlab.com': terminal prompts disabled" execID=cffc2                  │

Which is a bit better but still not working. I should mention that my credentials are effectively present in the created Git client (not an issue with my credentials per se).

To be honest I am not really sure what is supposed to be passed in the credsStore, @alexmt, @jannfis I saw that you participated in the MR that introduced this change (argoproj/argo-cd#8516). Any help on the subject ?

@ImFlog I am trying this:

store := &memoryCredsStore{creds: make(map[string]cred)}

with memoryCredsStore being:

type cred struct {
	username string
	password string
}

type memoryCredsStore struct {
	creds map[string]cred
}

akram avatar May 22 '24 20:05 akram