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

Terraform import/plan crashes after 2.18.0 upgrade #935

Closed
ouranos opened this issue Feb 4, 2021 · 7 comments · Fixed by #940
Closed

Terraform import/plan crashes after 2.18.0 upgrade #935

ouranos opened this issue Feb 4, 2021 · 7 comments · Fixed by #940
Labels
kind/crash Categorizes issue or PR as related to a crash caused by the provider. kind/regression Categorizes issue or PR as related to a regression from a prior release. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@ouranos
Copy link
Contributor

ouranos commented Feb 4, 2021

Terraform version

Terraform v0.14.5
+ provider registry.terraform.io/cloudflare/cloudflare v2.18.0

Affected resource(s)

  • cloudflare_access_group

Terraform configuration files

resource "cloudflare_access_group" "test_warp" {
  account_id = var.cloudflare_account_id
  name       = "Test WARP"

  include {
    email_domain = ["example.com"]
  }
}

Debug output

Panic output

https://gist.github.com/ouranos/4a778cb21dc80a35cca973b051b547e1

Expected behavior

The group is expected to be imported

Actual behavior

Terraform crashed

Steps to reproduce

  1. Create an Access group with terraform apply
  2. From the Cloudflare UI, edit the group to add "Require: Google Groups = [email protected]"
  3. Run terraform plan
  4. Terraform crash

Important factoids

The Cloudflare API reports this additional condition:

   "require":[
      {
         "gsuite":{
            "id":"redacted",
            "email":"[email protected]",
            "identity_provider_id":"<redacted_uid>"
         }
      }
   ]

Also note that if trying to require Gateway connections by adding "Require: Warp = Warp", terraform plan won't detect this configuration change. The API report it properly:

   "require":[
      {
         "warp":{
            "integration_uid":"<redacted_uid>",
            "integration_name":"Warp"
         }
      }
   ]

References

Community note

  • Please vote on this issue by adding a 👍 reaction
    to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull
    request, please leave a comment
@ouranos ouranos added kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 4, 2021
@jacobbednarz
Copy link
Member

Just to confirm here, you're applying changes, updating via the UI and then attempting to apply again and you're getting a crash?

@ouranos
Copy link
Contributor Author

ouranos commented Feb 4, 2021

Just to confirm here, you're applying changes, updating via the UI and then attempting to apply again and you're getting a crash?

Yes, sorry just edited to add a bit more details as I pressed submit too early.

I was trying to figure out how to set up some conditions that were not in the documentation. So I applied the initial change then manually edited it via the UI and was expecting terraform plan to show me the diff but instead:

  • With a GSuite group requirement, it crashes (with 2.17.0 it wasn't showing a difference)
  • With a WARP requirement, it doesn't show any difference

Here's the kind of group I'm trying to setup via Terraform:
image

@jacobbednarz
Copy link
Member

jacobbednarz commented Feb 4, 2021

The API (and here Terraform) doesn't yet support WARP attributes so that isn't going to be managed at the current time. We don't generally add support until the teams add public API support for it and commit to an interface they will continue to support. I can chase up this internally for a timeline.

Editing combined Access policies in the UI actually gives them a new ID which Terraform won't be able to manage. It doesn't look like that is the issue here but just FYI in case that pops up.

As for Gsuite, I'll have to spin up a full integration test for that as we don't current have a good way to test it automatically. In the meantime, you should manage your policies either via the UI or Terraform; not both.

@ouranos
Copy link
Contributor Author

ouranos commented Feb 4, 2021

The API (and here Terraform) doesn't yet support WARP attributes so that isn't going to be managed at the current time. We don't generally add support until the teams add public API support for it and commit to an interface they will continue to support. I can chase up this internally for a timeline.

Thanks for pointing that out. I saw the API was showing it but if it's not publicly supported it makes sense.

Editing combined Access policies in the UI actually gives them a new ID which Terraform won't be able to manage. It doesn't look like that is the issue here but just FYI in case that pops up.

Ok, good to know.

As for Gsuite, I'll have to spin up a full integration test for that as we don't current have a good way to test it automatically. In the meantime, you should manage your policies either via the UI or Terraform; not both.

Ok, I'll stick with the UI for now.

Trying to create a new group, the following still causes a crash when trying to apply (the group is created correctly though):

resource "cloudflare_access_group" "test_warp" {
  account_id = var.cloudflare_account_id
  name       = "Test WARP"

  include {
    email_domain = ["example.com"]
  }

  require {
    gsuite {
      email                = ["[email protected]"]
      identity_provider_id = cloudflare_access_identity_provider.gsuite_oauth.id 
    }
  }
}

jacobbednarz added a commit that referenced this issue Feb 7, 2021
With the fixes and refactor in v2.18.0, I copied and pasted the wrong
attribute for the GSuite configuration, resulting in a crash. This
updates the attribute being accessed to be the correct one and prevents
the crash.

Fixes #935
@jacobbednarz
Copy link
Member

I managed to reproduce this one and made the fix in #940. Turns out I copy/pasta the attribute name wrong which results in trying to access a non-existent map key.

Thanks for raising this one @ouranos, we appreciate it!

@jacobbednarz jacobbednarz added kind/crash Categorizes issue or PR as related to a crash caused by the provider. kind/regression Categorizes issue or PR as related to a regression from a prior release. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 7, 2021
@warwick-mitchell1
Copy link

Hi, is there an ETA on when 2.19.0 will be released? We're currently suffering from this bug but can't revert to an older version as using new resource cloudflare_argo_tunnel

@jacobbednarz
Copy link
Member

no ETA at the moment; likely in the next week or so though once the 0.14 cloudflare-go breaking changes are incorporated.

if you need the changes immediately, you can build the provider locally following https://github.com/cloudflare/terraform-provider-cloudflare#building-the-provider

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/crash Categorizes issue or PR as related to a crash caused by the provider. kind/regression Categorizes issue or PR as related to a regression from a prior release. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants