panic: nil pointer dereference at fn.go:181 when enableDeletionSequencing=true and a "before" resource is missing from observedComposed

[gabgaudreau]

Hit a deadlock with enableDeletionSequencing: true.

We have a composition where one branch of the resource graph is optional. Roughly:

providerconfig-* → appinsights-* → pushsecret-* ─┐
                                                 ├→ gitfile-*
identity-* ──────────────────────────────────────┘

The providerconfig/appinsights/pushsecret chain is only emitted when an optional spec field is set — gitfile is always emitted. Sequencer rules:

rules:
  - sequence: [identity-.*, gitfile-.*]
  - sequence: [providerconfig-.*, appinsights-.*, pushsecret-.*, gitfile-.*]

This works fine on initial create. But if you flip the optional field on later, here's what happens:

1. gitfile-foo is already in observedComposed from earlier reconciles.
2. The upstream function adds providerconfig-foo, appinsights-foo, pushsecret-foo to desiredComposed for the first time. None of them are in observed yet — Crossplane hasn't applied them.
3. Sequencer hits its deletion-sequencing branch, looks at the observed gitfile-foo, walks the keys list (built from desiredComposed), and panics:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x15a85fb]

main.(*Function).RunFunction(...)
    /fn/fn.go:181 +0x1c1b

Last log line before the crash:

DEBUG fn/fn.go:180  Generate Usage of  {"k:": "providerconfig-foo", "by c:": "gitfile-foo"}

The bad spot is here in v0.5.0:

if in.EnableDeletionSequencing {
    for c, o := range observedComposed {
        if currentRegex.MatchString(string(c)) && !isUsage(o, in.UsageVersion) {
            for _, k := range keys {
                f.log.Debug("Generate Usage of ", "k:", k, "by c:", c)
                usage := GenerateUsage(&observedComposed[k].Resource.Unstructured, // ← here
                                       &o.Resource.Unstructured, ...)

keys comes from desiredComposed, but observedComposed[k] is looked up without checking if it's there. Missing key → zero-value ObservedComposed{Resource: nil} → nil deref on .Resource.Unstructured.

The function crashing means Crossplane can't compute desired state, so the new resources never get applied, so providerconfig-foo never enters observed state, so the next reconcile crashes in exactly the same place. The XR is wedged. The only ways out we found:

• set enableDeletionSequencing: false on the composition, or
• delete the composite and re-create it with the optional field set from the start, so the new branch and gitfile-foo enter observed state in the same reconcile.

Suggested fix:

             if in.EnableDeletionSequencing {
                 for c, o := range observedComposed {
                     if currentRegex.MatchString(string(c)) && !isUsage(o, in.UsageVersion) {
                         for _, k := range keys {
+                            of, ok := observedComposed[k]
+                            if !ok {
+                                f.log.Debug("skipping usage; before-resource not yet observed",
+                                    "k:", k, "by c:", c)
+                                continue
+                            }
                             f.log.Debug("Generate Usage of ", "k:", k, "by c:", c)
-                            usage := GenerateUsage(&observedComposed[k].Resource.Unstructured,
-                                &o.Resource.Unstructured,
+                            usage := GenerateUsage(&of.Resource.Unstructured,
+                                &o.Resource.Unstructured,
                                 in.ReplayDeletion, in.UsageVersion)

Skipping seems safe, a Usage protecting a not-yet-observed resource has nothing to protect this round, and the next reconcile (once it's observed) will generate it normally.

Versions:

• function-sequencer v0.5.0
• function-sdk-go v0.5.0

Curious to hear if you think we're handling our use case correctly, or if we actually landed in a real issue. Also open to suggestions if there are other ways to handle this.

[bobh66]

I think the fix in #138 might apply here as well - there is no reason for the function to be doing deletion sequencing (creating Usages) for any resources besides the "n-1" resource. But it probably still makes sense to double check that the resource is actually in observedComposed before attempting to use it.

[gabgaudreau]

I figure we can close this issue now? I mislinked the issue in my PR, I had # 139 with a space, so it didn't link.