nil pointer dereference in Autoscaling attachment

[stevendborrelli]

What happened?

When creating an autoscaling attachment with a minimal forProvider, the provider will crash with a nil pointer dereference:

xb", "gvk": "autoscaling.aws.upbound.io/v1beta1, Kind=Attachment"}
2024-01-03T23:10:02Z    DEBUG   provider-aws    Async create starting...    {"trackerUID": "36fcfc55-92e7-4562-bf33-a491a42e67ab", "resourceName": "dms-k8s-ddm-97cnf-9x4xb", "tfID": ""}
2024-01-03T23:10:02Z    DEBUG   provider-aws    Creating the external resource  {"uid": "36fcfc55-92e7-4562-bf33-a491a42e67ab", "name": "dms-k8s-ddm-97cnf-9x4xb", "gvk": "autoscaling.aws.upbound.io/v1beta1, Kind=Attachment"}
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x78 pc=0x16dc16a]

goroutine 4598 [running]:
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).Apply(0xc0011120e0, {0x12dd8228, 0xc00bafdb90}, 0x0, 0x0, {0x10f83600, 0xc00c820000})
    github.com/hashicorp/terraform-plugin-sdk/v2@v2.26.1/helper/schema/resource.go:779 +0x14a
github.com/crossplane/upjet/pkg/controller.(*noForkExternal).Create(0xc006921110, {0x12dd8228, 0xc00bafdb90}, {0x12e2ecd0?, 0xc00ba66a00})
    github.com/crossplane/upjet@v1.1.0-rc.0.0.20231227120826-4cb45f9104ac/pkg/controller/external_nofork.go:574 +0xe4
github.com/crossplane/upjet/pkg/controller.(*noForkAsyncExternal).Create.func1()
    github.com/crossplane/upjet@v1.1.0-rc.0.0.20231227120826-4cb45f9104ac/pkg/controller/external_async_nofork.go:137 +0x145
created by github.com/crossplane/upjet/pkg/controller.(*noForkAsyncExternal).Create in goroutine 3638
    github.com/crossplane/upjet@v1.1.0-rc.0.0.20231227120826-4cb45f9104ac/pkg/controller/external_async_nofork.go:133 +0x168

How can we reproduce it?

Use the following attachment file, which is accepted by the API server:

apiVersion: autoscaling.aws.upbound.io/v1beta1
kind: Attachment
metadata:
  name: dms-k8s-ddm-97cnf-9x4xb
spec:
  deletionPolicy: Delete
  forProvider:
    region: us-west-2
  initProvider: {}
  managementPolicies:
  - '*'
  providerConfigRef:
    name: default

What environment did it happen in?

• Crossplane Version: 1.14.5
• Provider Version: v0.47.1
• Kubernetes Version: 1.29
• Kubernetes Distribution: Kind

[sergenyalcin]

Thank you for reporting this, @stevendborrelli. There are two main points on this issue:

1. As you reported, this minimal[1] example manifest causes this type of issue. Because, for some resources, we calculate the instanceDiff as nil if we do not pass anything in the resource schema. And it causes this issue. What does we do not pass anything in the resource schema mean? This means that if there is not any required field in our CRD schema, if we do not inject any tags to the resource (because this resource does not have a tags field in its schema), and if we also do not inject any identifier field (like name) then we observe this issue: No required field in CRD schema AND No injected identifier field AND No injected non-identifier field. This fix will handle this type of problem.

2. It seems that there is a required field in the resource schema. Because of this, is a reference field, it is not a required one in our CRD. In other words, this manifest is not valid in terms of functionality. However, the important thing is that since the provider panics, we fail before reaching this input validation phase. Therefore, fixing this issue you have caught is critical. The real problem will become visible with the above fix: InvalidParameter: 1 validation error(s) found. minimum field size of 1, AttachLoadBalancerTargetGroupsInput.AutoScalingGroupName.

[ulucinar]

Thank you @sergenyalcin for the analysis & the fix. Here's a related issue to generate the CRD validation rules for the cross-resource reference fields.

[github-actions[bot]]

This provider repo does not have enough maintainers to address every issue. Since there has been no activity in the last 90 days it is now marked as stale. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

[github-actions[bot]]

This issue is being closed since there has been no activity for 14 days since marking it as stale. If you still need help, feel free to comment or reopen the issue!