crossplane-contrib / provider-pagerduty

Apache License 2.0
13 stars 13 forks source link

fix: ensure we always have an ID #70

Open fernandezcuesta opened 3 weeks ago

fernandezcuesta commented 3 weeks ago

Description of your changes

id field in tfstate must be either filled in or omitted. This PR attempts to fix it by setting an intermediate value (PENDING) not to fall into the raised bug.

Fixes #66

I have:

How has this code been tested

Locally with valid credentials.

fernandezcuesta commented 3 weeks ago

Not sure if it is possible to omit the id from the tfstate by instructing upjet, but when we get the following we hit the bug as per #66:

The solution is either omit the "id" attribute or put any value on it (current workaround)

{
  "version": 4,
  "terraform_version": "1.5.5",
  "serial": 1,
  "lineage": "8a917ef2-2690-4360-9d29-0bf2a4dfe8b7",
  "outputs": null,
  "resources": [
    {
      "mode": "managed",
      "type": "pagerduty_team",
      "name": "test-team",
      "provider": "provider[\"registry.terraform.io/PagerDuty/pagerduty\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "description": "Test team",
            "id": "",   <------------------- THIS SEEMS TO BE THE OFFENDER
            "name": "Test Team"
          }
        }
      ]
    }
  ]
}
fernandezcuesta commented 3 weeks ago

Looks to me like an error in the upstream terraform provider, which doesn't like having an empty id. Didn't find the way to instruct upjet not to set it in the tfstate

fnicolelli-ls commented 3 weeks ago

Hello team, i've tested this locally and it is working, i can deploy pagerduty Teams without any issues.

fernandezcuesta commented 1 week ago

@haarchri regarding the "fix" here, what I did so far:

Do you know if there's any way to omit the id field in tfstate? If not, I guess the solution here