Promotion: invalid memory address or nil pointer dereference caused controller crash

[jessesuen]

Tried to use bookkeeper promotion, not sure if the spec is correct, but it resulted in a controller panic.

apiVersion: kargo.akuity.io/v1alpha1
kind: Stage
metadata:
  creationTimestamp: "2023-08-25T06:04:19Z"
  generation: 7
  name: dev
  namespace: kargo-guestbook
  resourceVersion: "127234591"
  uid: 32626a1f-c00f-468a-8e76-d1a0d83e7920
spec:
  promotionMechanisms:
    argoCDAppUpdates:
    - appName: guestbook-dev
      appNamespace: argocd
    gitRepoUpdates:
    - bookkeeper: {}
      readBranch: ""
      repoURL: https://github.com/jessesuen/guestbook-deploy.git
      writeBranch: env/dev
  subscriptions:
    repos:
      git:
      - branch: main
        repoURL: https://github.com/jessesuen/guestbook-deploy.git
      images:
      - gitRepoURL: https://github.com/jessesuen/guestbook.git
        repoURL: ghcr.io/jessesuen/guestbook
        updateStrategy: SemVer

Promotion:

apiVersion: kargo.akuity.io/v1alpha1
kind: Promotion
metadata:
  creationTimestamp: "2023-09-06T20:35:31Z"
  generation: 1
  name: dev.01h9p3a3qwkfajkay4j0zfgq9n.6075e1a
  namespace: kargo-guestbook
  ownerReferences:
  - apiVersion: kargo.akuity.io/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: Stage
    name: dev
    uid: 32626a1f-c00f-468a-8e76-d1a0d83e7920
  resourceVersion: "127228619"
  uid: e74751e1-4360-4930-88d8-ec3c9c632d29
spec:
  freight: 6075e1ab632d6ed194b6ecda069998c46702b6ea
  stage: dev
status:
  phase: Pending

Logs

time="2023-09-06T20:37:34Z" level=info msg="Starting Kargo Controller" commit=50e24a3071582e43de758b90818f0adde99a4634 version=devel+50e24a3.dirty
time="2023-09-06T20:37:45Z" level=info msg="kustomize build /tmp/4076788782/repo/env/dev" dir= execID=4f5dd
time="2023-09-06T20:37:45Z" level=info msg=Trace args="[kustomize build /tmp/4076788782/repo/env/dev]" dir= operation_name="exec kustomize" time_ms=117.601822
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1d331f0]

goroutine 484 [running]:
github.com/akuity/bookkeeper.(*service).RenderManifests(0xc0000bca60, {0x279be88, 0xc0008941b0}, {{0xc0007687b0, 0x24}, {0xc000d5edc0, 0x31}, {{0x0, 0x0}, {0xc0003a31e0, ...}, ...}, ...})
        /go/pkg/mod/github.com/akuity/[email protected]/service.go:160 +0xa90
github.com/akuity/kargo/internal/controller/promotion.(*bookkeeperMechanism).doSingleUpdate(_, {_, _}, {_, _}, {{0xc000d5edc0, 0x31}, {0x0, 0x0}, {0xc0002ad1e0, ...}, ...}, ...)
        /kargo/internal/controller/promotion/bookkeeper.go:153 +0x586
github.com/akuity/kargo/internal/controller/promotion.(*bookkeeperMechanism).Promote(_, {_, _}, _, {{0xc000d66720, 0x28}, 0xc00088e3a8, {0x0, 0x0}, {0x0, ...}, ...})
        /kargo/internal/controller/promotion/bookkeeper.go:90 +0x6a3
github.com/akuity/kargo/internal/controller/promotion.(*compositeMechanism).Promote(_, {_, _}, _, {{0xc000d66720, 0x28}, 0xc00088e390, {0x0, 0x0}, {0x0, ...}, ...})
        /kargo/internal/controller/promotion/composite.go:57 +0x2ae
github.com/akuity/kargo/internal/controller/promotion.(*compositeMechanism).Promote(_, {_, _}, _, {{0xc000d66720, 0x28}, 0xc00088e378, {0x0, 0x0}, {0x0, ...}, ...})
        /kargo/internal/controller/promotion/composite.go:57 +0x2ae
github.com/akuity/kargo/internal/controller/promotions.(*reconciler).promote(0xc00023c370, {0x279be88, 0xc0008941b0}, {0xc0009dece9, 0x3}, {0xc0009decd0, 0xf}, {0xc0009d2270, 0x28})
        /kargo/internal/controller/promotions/promotions.go:413 +0x610
github.com/akuity/kargo/internal/controller/promotions.(*reconciler).serializedSync(0xc00023c370, {0x279be88, 0xc000c78b40}, 0x0?)
        /kargo/internal/controller/promotions/promotions.go:329 +0x5fd
created by github.com/akuity/kargo/internal/controller/promotions.(*reconciler).Reconcile.func1 in goroutine 483
        /kargo/internal/controller/promotions/promotions.go:139 +0x129

There are two issues:

1. The nil pointer dereference itself
2. Even when we do have a nil pointer dereference, the controller itself should not crash.

[jessesuen]

Bookkeeper code:

https://github.com/akuity/bookkeeper/blob/main/service.go#L156-L160

The panic is because oldTargetBranchMetadata is nil.

[krancour]

Yup... known issue: akuity/kargo-render#166

[krancour]

I would surmise that env/dev already contained stuff that was not already Bookkeeper-managed.

[jessesuen]

I would surmise that env/dev already contained stuff that was not already Bookkeeper-managed.

That is correct. I just changed the Stage spec to use bookkeeper instead of kustomize and was using different write branch from main.

[krancour]

I will be sure to get akuity/kargo-render#166 fixed tomorrow.

[jessesuen]

I will be sure to get akuity/kargo-render#166 fixed tomorrow.

Fix is not urgent. I'm not blocked since I know the workaround (delete the branch). It'll benefit others, however, who try to switch promotion strategies. I suggest the error we return in bookkeeper should hint at the corrective course of action (delete the branch).

[krancour]

An alternative that doesn't lose the branch's existing history is to touch .bookkeeper/metadata.yaml