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

Regression in version v2.11.0: cloudflare_access_policy Okta 'Error: missing expected [' #807

Closed
groodt opened this issue Sep 28, 2020 · 13 comments
Labels
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

@groodt
Copy link

groodt commented Sep 28, 2020

Terraform Version

v0.12.26

Affected Resource(s)

  • cloudflare_access_policy

Terraform Configuration Files

Working in v2.10.1

resource "cloudflare_access_policy" "foo" {
  application_id = cloudflare_access_application.foo.id
  zone_id = data.terraform_remote_state.zones.outputs.zone_id
  name = "Full Access"
  decision = "allow"
  precedence = 1

  include {
    okta {
      name = "Okta Group Name"
      identity_provider_id = local.identity_provider_id
    }
  }
}

v2.11.0 requires a change to a list

resource "cloudflare_access_policy" "foo" {
  application_id = cloudflare_access_application.foo.id
  zone_id = data.terraform_remote_state.zones.outputs.zone_id
  name = "Full Access"
  decision = "allow"
  precedence = 1

  include {
    okta {
      name = ["Okta Group Name"]
      identity_provider_id = local.identity_provider_id
    }
  }
}

However, this fails with an error.

Debug Output

N/A

Panic Output

N/A

Expected Behavior

The plan should run successfully.

Actual Behavior

An error occurs:

'Error: missing expected ['

Steps to Reproduce

  1. terraform plan

Important Factoids

N/A

References

#739

@groodt
Copy link
Author

groodt commented Sep 28, 2020

cc @jacobbednarz

@jacobbednarz
Copy link
Member

Thank you @groodt 🙇‍♂️ I appreciate it.

@jacobbednarz
Copy link
Member

Using the following setup, I wasn't able to replicate the issue.

variable "cloudflare_email" {}
variable "cloudflare_api_token" {}
variable "cloudflare_account_id" {}
variable "cloudflare_domain" {}
variable "okta_client_id" {}
variable "okta_secret" {}
variable "okta_account_url" {}

provider "cloudflare" {
  email      = var.cloudflare_email
  api_token  = var.cloudflare_api_token
  account_id = var.cloudflare_account_id
}

resource "cloudflare_access_identity_provider" "example" {
  account_id = var.cloudflare_account_id
  name       = "okta test"
  type       = "okta"

  config {
    client_id     = var.okta_client_id
    client_secret = var.okta_secret
    okta_account  = var.okta_account_url
  }
}

resource "cloudflare_access_application" "example" {
  account_id                = var.cloudflare_account_id
  name                      = "example application"
  domain                    = "example.${var.cloudflare_domain}"
  session_duration          = "24h"
  auto_redirect_to_identity = false
}

resource "cloudflare_access_policy" "example" {
  application_id = cloudflare_access_application.example.id
  account_id     = var.cloudflare_account_id
  name           = "all access"
  decision       = "allow"
  precedence     = 1

  include {
    okta {
      name                 = ["Everyone"]
      identity_provider_id = cloudflare_access_identity_provider.example.id
    }
  }
}

Is there anything from the above that I'm missing?

@jacobbednarz
Copy link
Member

Ah, the moment after I sent this I had a 💡 moment. Are you trying to update existing resources? Or are they new?

@groodt
Copy link
Author

groodt commented Sep 28, 2020

Update in my case.

@jacobbednarz
Copy link
Member

🆒 that's great to know. Is it possible for you to recreate the policy? I'll have a look at a schema migrator might look like and add it as a check for the next schema changes.

@groodt
Copy link
Author

groodt commented Sep 28, 2020

Do you mean to test it out? What we have done in our repo is simply to pin to v2.10.1 for the time-being.

Recreating them all would be a pain since we have quite a few, but could be done I think. Even so, there should probably be a better error message instructing people what to do if the only way to recover is to destroy and recreate (incurring downtime).

@jacobbednarz
Copy link
Member

I was more asking whether it's possible for you to delete and create or whether there are too many moving parts to make that viable (i.e. cross team, security, etc)

Definitely agree the upgrade path should be smoother here; it was an oversight when the types were changed which I'll take a look at improving but also wanted to try and unblock the current users.

You could do this without downtime by doing a two phase swap. Create the new policy (with the correct syntax), attaching it to the existing application and then after it's applied, remove the old policy. Alternatively, if you can take 5-10s of interruption, just delete the resource and recreate it in place.

@groodt
Copy link
Author

groodt commented Sep 29, 2020

Got it.

Yes, those workarounds are reasonable and hopefully unblock any others stumbling on this issue in future.

@jacobbednarz jacobbednarz added 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. labels Oct 20, 2020
@analytically
Copy link

Still broken in 2.14.0.

@jacobbednarz
Copy link
Member

Closing this one off as the fix now is pretty complex and not showing a large impact to end users. In future, we'll take note of this one and try to provide a better upgrade path instead of this breakage.

If you are stumbling upon this and having issues, please add a mention here and try the work arounds first. Should someone want to file a PR for this, I'm happy to review it.

@richstokes
Copy link

richstokes commented Mar 9, 2021

Running into this going from 2.10.1 -> 2.18.0. It's unclear what we need to do to fix this since the error message doesn't reference specific resources. Any suggestions?

@fllaca
Copy link

fllaca commented Sep 20, 2021

This error has just happened to me with last 3.0.0 for a cloudflare_record resource, downgrading to 2.27.0 made TF plans pass again.

boekkooi-lengoo pushed a commit to boekkooi-lengoo/terraform-provider-cloudflare that referenced this issue Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

No branches or pull requests

5 participants