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

Regression after recent commit, Segmentation fault #621

Closed
ZOrfeas opened this issue Sep 14, 2023 · 11 comments · Fixed by #723
Closed

Regression after recent commit, Segmentation fault #621

ZOrfeas opened this issue Sep 14, 2023 · 11 comments · Fixed by #723
Labels
bug Something isn't working

Comments

@ZOrfeas
Copy link

ZOrfeas commented Sep 14, 2023

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 ZOrfeas added the bug Something isn't working label Sep 14, 2023
@ZOrfeas ZOrfeas changed the title Segmentation fault after d5a8f94 commit Regression after recent commit, Segmentation fault Sep 14, 2023
@ZOrfeas
Copy link
Author

ZOrfeas commented Oct 30, 2023

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?

@ImFlog
Copy link

ImFlog commented Dec 14, 2023

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
Copy link

ImFlog commented Dec 19, 2023

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 argoproj/argo-cd#8516 (when the failing line was introduced). I think it might be related to the cred store.
Still looking ...

@ImFlog
Copy link

ImFlog commented Dec 21, 2023

I found the issue, it's here :

return repo.GetGitCreds(nil), nil

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 ?

@Ulrar
Copy link

Ulrar commented May 17, 2024

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.

@CygnusHyoga
Copy link

CygnusHyoga commented May 17, 2024

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

@christian-schlichtherle

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

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.

@ZOrfeas
Copy link
Author

ZOrfeas commented May 18, 2024

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

@christian-schlichtherle
Copy link

christian-schlichtherle commented May 19, 2024

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.

@akram
Copy link

akram commented May 22, 2024

I found the issue, it's here :

return repo.GetGitCreds(nil), nil

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
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants