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

[Bug]: Creation of compute Instances (and others) fails with SetNew only operates on computed keys #472

Closed
1 task done
JonathanO opened this issue Mar 11, 2024 · 3 comments · Fixed by #490
Closed
1 task done
Labels
bug Something isn't working is:triaged

Comments

@JonathanO
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Affected Resource(s)

  • compute.gcp.upbound.io/v1beta1 - Instance
  • cloudscheduler.gcp.upbound.io/v1beta1 - Job

Resource MRs required to reproduce the bug

examples/compute/instance.yaml
examples/cloudscheduler/job.yaml

Steps to Reproduce

Apply examples/compute/instance.yaml (or examples/cloudscheduler/job.yaml)

What happened?

External resource was not created, reconciliation fails with an error from terraform:
observe failed: cannot compute the instance diff: failed to get *terraform.InstanceDiff: SetNew only operates on computed keys - zone is not one
(For Job it's ... - region is not one)

Relevant Error Output Snippet

No response

Crossplane Version

1.15.0

Provider Version

1.0.0

Kubernetes Version

No response

Kubernetes Distribution

No response

Additional Info

The failing calls to SetNew appear to have been introduced in GoogleCloudPlatform/magic-modules#8569

@JonathanO JonathanO added bug Something isn't working needs:triage labels Mar 11, 2024
@turkenf
Copy link
Collaborator

turkenf commented Mar 11, 2024

Hi @JonathanO,

Thank you for bringing up this issue, I can reproduce the issue with the provided information.

  • For Instance.compute.gcp.upbound.io/v1beta1:
  - lastTransitionTime: "2024-03-11T21:07:55Z"
    message: 'observe failed: cannot compute the instance diff: failed to get *terraform.InstanceDiff:
      SetNew only operates on computed keys - zone is not one'
    reason: ReconcileError
    status: "False"
    type: Synced
  • For Job.cloudscheduler.gcp.upbound.io/v1beta1:
  - lastTransitionTime: "2024-03-11T21:16:22Z"
    message: 'observe failed: cannot compute the instance diff: failed to get *terraform.InstanceDiff:
      SetNew only operates on computed keys - region is not one'
    reason: ReconcileError
    status: "False"
    type: Synced

logs:

2024-03-12T00:22:30+03:00	DEBUG	provider-gcp	Cannot observe external resource	{"controller": "managed/cloudscheduler.gcp.upbound.io/v1beta1, kind=job", "request": {"name":"job"}, "uid": "c35370b0-4ef1-41b6-8b00-647d803b8e13", "version": "3463", "external-name": "job", "error": "cannot compute the instance diff: failed to get *terraform.InstanceDiff: SetNew only operates on computed keys - region is not one", "errorVerbose": "SetNew only operates on computed keys - region is not one\nfailed to get *terraform.InstanceDiff\ngithub.com/crossplane/upjet/pkg/controller.(*terraformPluginSDKExternal).getResourceDataDiff\n\tgithub.com/crossplane/[email protected]/pkg/controller/external_tfpluginsdk.go:427\ngithub.com/crossplane/upjet/pkg/controller.(*terraformPluginSDKExternal).Observe\n\tgithub.com/crossplane/[email protected]/pkg/controller/external_tfpluginsdk.go:501\ngithub.com/crossplane/upjet/pkg/controller.(*terraformPluginSDKAsyncExternal).Observe\n\tgithub.com/crossplane/[email protected]/pkg/controller/external_async_tfpluginsdk.go:126\ngithub.com/crossplane/crossplane-runtime/pkg/reconciler/managed.(*Reconciler).Reconcile\n\tgithub.com/crossplane/[email protected]/pkg/reconciler/managed/reconciler.go:903\ngithub.com/crossplane/crossplane-runtime/pkg/ratelimiter.(*Reconciler).Reconcile\n\tgithub.com/crossplane/[email protected]/pkg/ratelimiter/reconciler.go:54\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:119\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:316\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227\nruntime.goexit\n\truntime/asm_arm64.s:1197\ncannot compute the instance diff\ngithub.com/crossplane/upjet/pkg/controller.(*terraformPluginSDKExternal).Observe\n\tgithub.com/crossplane/[email protected]/pkg/controller/external_tfpluginsdk.go:503\ngithub.com/crossplane/upjet/pkg/controller.(*terraformPluginSDKAsyncExternal).Observe\n\tgithub.com/crossplane/[email protected]/pkg/controller/external_async_tfpluginsdk.go:126\ngithub.com/crossplane/crossplane-runtime/pkg/reconciler/managed.(*Reconciler).Reconcile\n\tgithub.com/crossplane/[email protected]/pkg/reconciler/managed/reconciler.go:903\ngithub.com/crossplane/crossplane-runtime/pkg/ratelimiter.(*Reconciler).Reconcile\n\tgithub.com/crossplane/[email protected]/pkg/ratelimiter/reconciler.go:54\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:119\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:316\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227\nruntime.goexit\n\truntime/asm_arm64.s:1197"}
2024-03-12T00:22:30+03:00	DEBUG	events	cannot compute the instance diff: failed to get *terraform.InstanceDiff: SetNew only operates on computed keys - region is not one	{"type": "Warning", "object": {"kind":"Job","name":"job","uid":"c35370b0-4ef1-41b6-8b00-647d803b8e13","apiVersion":"cloudscheduler.gcp.upbound.io/v1beta1","resourceVersion":"3463"}, "reason": "CannotObserveExternalResource"}

@selfuryon
Copy link

For already created resources it's the same:

Events:                                                                                                                                                                                                          
  Type     Reason                         Age                 From                                                   Message                                                                                     
  ----     ------                         ----                ----                                                   -------                                                                                     
  Warning  CannotObserveExternalResource  27s (x36 over 30m)  managed/compute.gcp.upbound.io/v1beta1, kind=instance  cannot compute the instance diff: failed to get *terraform.InstanceDiff: SetNew only operate
s on computed keys - zone is not one    

I faced with the problem on these resources:

  • instance.compute.gcp.upbound.io
  • instancegroup.compute.gcp.upbound.io

@ulucinar
Copy link
Collaborator

ulucinar commented Mar 19, 2024

We have investigated the root cause of this issue with @sergenyalcin. upjet's config.MarkAsRequired works by manipulating the native Terraform schema of a resource as follows:

func MarkAsRequired(sch *schema.Resource, fieldpaths ...string) {
	for _, fieldpath := range fieldpaths {
		if s := GetSchema(sch, fieldpath); s != nil {
			s.Computed = false
			s.Optional = false
		}
	}
}

Setting schema.Schema.Computed and schema.Schema.Optional forces the corresponding field in the generated CRD to be a required field because upjet's code generation pipelines check these two properties when deciding whether a field in the generated CRD should be a required field.

It turns out that interfering with the native schema has adverse affects here. The GCP project and zone defaulting functions introduced as CustomizeDiff functions for the Terraform resource make calls to the schema.ResourceDiff.SetNew, which in turn asserts the schema of the attribute being set so that it's a computed value.

However, MarkAsRequired's modification on the native schema breaks this check and results in the error message reported in this issue. These defaulting CustomizeDiff functions were not being used in the previous Terraform provider version we consumed (v4.77.0) before the v1.0.0 release bumped it to v5.19.0. So, these bugs silently creeped in with the v1.0.0 release, and are especially dangerous.

We had better get rid of all the Terraform schema modifications as they can potentially cause harmful interference with the Terraform runtime like this issue demonstrates. We have plans to remove the Terraform plugin SDK from upjet's code generation pipelines, which will enforce this. Till then, we must be very careful (and avoid as much as possible) modifying the Terraform schema for managed resource configuration.

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