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

nil pointer dereference in Autoscaling attachment #1062

Closed
stevendborrelli opened this issue Jan 3, 2024 · 4 comments
Closed

nil pointer dereference in Autoscaling attachment #1062

stevendborrelli opened this issue Jan 3, 2024 · 4 comments
Labels
bug Something isn't working needs:triage stale

Comments

@stevendborrelli
Copy link
Contributor

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/[email protected]/helper/schema/resource.go:779 +0x14a
github.com/crossplane/upjet/pkg/controller.(*noForkExternal).Create(0xc006921110, {0x12dd8228, 0xc00bafdb90}, {0x12e2ecd0?, 0xc00ba66a00})
	github.com/crossplane/[email protected]/pkg/controller/external_nofork.go:574 +0xe4
github.com/crossplane/upjet/pkg/controller.(*noForkAsyncExternal).Create.func1()
	github.com/crossplane/[email protected]/pkg/controller/external_async_nofork.go:137 +0x145
created by github.com/crossplane/upjet/pkg/controller.(*noForkAsyncExternal).Create in goroutine 3638
	github.com/crossplane/[email protected]/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
Copy link
Collaborator

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

ulucinar commented Jan 9, 2024

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.

Copy link

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 github-actions bot added the stale label Apr 16, 2024
Copy link

github-actions bot commented May 8, 2024

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!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs:triage stale
Projects
None yet
Development

No branches or pull requests

3 participants