crossplane / crossplane/upjet

Defining a field in both `forProvider` and `initProvider` may cause an infinite update loop

Open
#299 0 comments 0 reactions 0 assignees View on GitHub
bug reconciler v2
Dominant language
Go
Stars
481
Forks
131
Avg merge
2d 1h
Merged PRs (30d)
11

Description

### What happened?

I discovered this bug while working on supporting `initProvider`-related behavior in [no-fork external client](https://github.com/crossplane/upjet/pull/294). An infinite update loop occurs when the following conditions hold at the same time:
1. A field is defined in both `initProvider` and `forProvider`,
2. The field corresponds to a Terraform `schema.TypeSet`, [such as `managed_policy_arns`](https://github.com/hashicorp/terraform-provider-aws/blob/0dbed883d0d6c895eadf1404c36f8173dcdaf635/internal/service/iam/role.go#L121),
3. The field in `initProvider` has more elements than corresponding one in `forProvider`.

Provider reports “Cannot observe external resource” and an event is published with message: “cannot run refresh: refresh failed: Invalid index: Elements of a set are identified only by their value and don't have any separate index or key to select with, so it's only possible to perform operations across all elements of the set.”

The problem stems from the fact that [Upjet puts](https://github.com/crossplane/upjet/blob/main/pkg/terraform/files.go#L85) `initProvider`-exclusive fields into Terraform's `ignore_changes` lifecycle meta argument. Even though not explicitly specified, [Terraform documentation](https://developer.hashicorp.com/terraform/language/meta-arguments/lifecycle#ignore_changes) suggests that `ignore_changes` doesn't support sets:

> Map and list elements can be referenced using index notation, like `tags["Name"]` and `list[0]` respectively.

Currently, we use index notation, like `set[0]`, for sets as well.

I can't think of an easy resolution. We cannot ignore changes in the whole set, because doing so would prevent the values defined in `forProvider` to take effect. We can let `forProvider` overwrite `initProvider`, in which case we should decide whether we do so for lists and maps as well.

### How can we reproduce it?

We will create an IAM Role to reproduce the issue. Because we will use [`managed_policy_arns`](https://github.com/hashicorp/terraform-provider-aws/blob/0dbed883d0d6c895eadf1404c36f8173dcdaf635/internal/service/iam/role.go#L121) that refers to IAM Policy resources, we will begin with creating IAM Policies.

1. Create two IAM Policies by applying the following configuration:
```yaml
apiVersion: iam.aws.upbound.io/v1beta1
kind: Policy
metadata:
annotations:
meta.upbound.io/example-id: iam/v1beta1/policy
labels:
testing.upbound.io/example-name: policy
name: test-upjet-bug-iam-policy-1
spec:
forProvider:
policy: |
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "VisualEditor0",
"Effect": "Allow",
"Action": "elastic-inference:Connect",
"Resource": "*"
}
]
}
---
apiVersion: iam.aws.upbound.io/v1beta1
kind: Policy
metadata:
annotations:
meta.upbound.io/example-id: iam/v1beta1/policy
labels:
testing.upbound.io/example-name: policy
name: test-upjet-bug-iam-policy-2
spec:
forProvider:
policy: |
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "VisualEditor0",
"Effect": "Allow",
"Action": "elastic-inference:Connect",
"Resource": "*"
}
]
}
```

4. After the policies are created, get their ARN:
```sh
$ kubectl get policy.iam test-upjet-bug-iam-policy-1 -o=jsonpath='{.status.atProvider.arn}'
arn:aws:iam:::policy/test-upjet-bug-iam-policy-1

$ kubectl get policy.iam test-upjet-bug-iam-policy-2 -o=jsonpath='{.status.atProvider.arn}'
arn:aws:iam:::policy/test-upjet-bug-iam-policy-2
```

5. Finally, create the IAM role, as follows, using the ARNs you got in the previous step. Note that it's important for `initProvider.managedPolicyArns` to have more elements than `forProvider.managedPolicyArns`, to be able to reproduce the bug.
```yaml
apiVersion: iam.aws.upbound.io/v1beta1
kind: Role
metadata:
labels:
testing.upbound.io/example-name: realtimelogconfig
name: test-upjet-bug-iam-role
spec:
initProvider:
managedPolicyArns:
- "arn:aws:iam:::policy/test-upjet-bug-iam-policy-1"
- "arn:aws:iam:::policy/test-upjet-bug-iam-policy-2"
forProvider:
managedPolicyArns:
- "arn:aws:iam:::policy/test-upjet-bug-iam-policy-1"
assumeRolePolicy: |
{
"Version": "2012-10-17",
"Statement": [
{
"Action": "sts:AssumeRole",
"Principal": {
"Service": "cloudfront.amazonaws.com"
},
"Effect": "Allow"
}
]
}
```

6. Check out provider logs to see the update loop.
7. Check out events to see:
```
NAMESPACE LAST SEEN TYPE REASON OBJECT MESSAGE
default 94s Warning CannotObserveExternalResource role/test-upjet-bug-iam-role cannot run refresh: refresh failed: Invalid index: Elements of a set are identified only by their value and don't have any separate index or key to select with, so it's only possible to perform operations across all elements of the set.
```

### Related Issues

#298 (duplicate of https://github.com/upbound/provider-aws/issues/946), #295

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.