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

Support redirect lists #1700

Merged

Conversation

orium
Copy link
Member

@orium orium commented Jun 15, 2022

This is a draft because we are waiting for a cloudflare-go release.

Closes #1342

go.mod Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

This project handles dependency version bumps (including upstream changes from cloudflare-go) independently of the standard PR process using automation. This allows the dependency upgrades to land without causing merge conflicts in multiple branches and handled in a consistent way. The exception to this is security related dependency upgrades but they should be co-ordinated with the maintainer team privately.

Please remove the changes to the go.mod or go.sum files from this PR in order to proceed with review and merging.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 15, 2022

changelog detected ✅

@orium orium force-pushed the dsousa/FLPROD-397-redirect-list-support branch from f8edb50 to 438e821 Compare June 15, 2022 13:51
@jacobbednarz jacobbednarz added the workflow/pending-upstream-library Indicates an issue or PR requires changes from an upstream library. label Jun 20, 2022
@cloudflare cloudflare deleted a comment from github-actions bot Jun 22, 2022
@orium orium force-pushed the dsousa/FLPROD-397-redirect-list-support branch from 4690a53 to cd7c11c Compare June 22, 2022 09:09
@orium orium marked this pull request as ready for review June 22, 2022 09:25
@jacobbednarz
Copy link
Member

acceptance tests are all green

TF_ACC=1 go test $(go list ./...) -v -run "^TestAccCloudflareList_" -count 1 -parallel 1 -timeout 120m -parallel 1
?       github.com/cloudflare/terraform-provider-cloudflare     [no test files]
=== RUN   TestAccCloudflareList_Exists
--- PASS: TestAccCloudflareList_Exists (9.30s)
=== RUN   TestAccCloudflareList_UpdateDescription
--- PASS: TestAccCloudflareList_UpdateDescription (16.59s)
=== RUN   TestAccCloudflareList_Update
--- PASS: TestAccCloudflareList_Update (36.95s)
PASS
ok      github.com/cloudflare/terraform-provider-cloudflare/internal/provider   63.258s
?       github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/changelog-check   [no test files]
?       github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/maintainer-only-file-check        [no test files]
?       github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/tf-log-check      [no test files]
?       github.com/cloudflare/terraform-provider-cloudflare/version     [no test files]

@jacobbednarz jacobbednarz merged commit efb815e into cloudflare:master Jun 23, 2022
@github-actions github-actions bot added this to the v3.18.0 milestone Jun 23, 2022
@jacobbednarz
Copy link
Member

thanks @orium!

github-actions bot pushed a commit that referenced this pull request Jun 23, 2022
@orium
Copy link
Member Author

orium commented Jun 23, 2022

Thanks 😆

@KevinM2k
Copy link

Any ideas when this version will be publicly available?

@orium
Copy link
Member Author

orium commented Jun 28, 2022

@KevinM2k There's likely going to be a new release tomorrow.

@github-actions
Copy link
Contributor

This functionality has been released in v3.18.0 of the Terraform Cloudflare Provider.

Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@KevinM2k
Copy link

Hey thanks for releasing so quickly...

This enables me to create the redirect lists, however how do we actually create the bulk redirects that use the list that we've created?

Thanks

@KevinM2k
Copy link

KevinM2k commented Jun 29, 2022

Also once the list has been created, ive gone in to make a change to it, the plan works but it fails on apply with the following error:

\│ Error: error creating List Items: invalid value for redirect at position 0: invalid redirect status code (10047), invalid value for redirect at position 1: invalid redirect status code (10047), invalid value for redirect at position 2: invalid redirect status code (10047), invalid value for redirect at position 3: invalid redirect status code (10047), invalid value for redirect at position 4: invalid redirect status code (10047)

Update: Reason for this one is that I didn't specify the status code when creating the list, it gets set to 0, which is actually an invalid state, so this should be defaulted to 301 or made a required metadata field

@orium
Copy link
Member Author

orium commented Jun 29, 2022

Reason for this one is that I didn't specify the status code when creating the list, it gets set to 0, which is actually an invalid state, so this should be defaulted to 301 or made a required metadata field.

Thanks for reporting. I'll open a PR with a fix today.

@orium
Copy link
Member Author

orium commented Jun 29, 2022

@KevinM2k I'm not able to reproduce it. Can you post a tf file that reproduces the problem?

@KevinM2k
Copy link

KevinM2k commented Jun 29, 2022

resource "cloudflare_list" "redirects" {
  account_id  = var.cloudflare_account_id
  name        =  redirects"
  description = "Legacy Redirecs"
  kind        = "redirect"

  dynamic "item" {
    for_each = local.redirects
    content {
      value {
        redirect {
          source_url           = item.key
          target_url           = item.value.target_url
        }
      }
    }
  }
}

Then modify one of the entires, and you should get the problem. The fix for this was to simply add the status_code as 301.

I found that the changes it was making in the plan was:

status_code = 0 > status_code = 301

With regards to the actual creation of the bulk list, the docs in TF just doesn't mention how to use the redirect kind and the list_from option isn't available (is in api) - hence issue #1342 shouldnt be closed just yet.

@orium
Copy link
Member Author

orium commented Jul 13, 2022

Hi @KevinM2k. This should be fixed in the new release. Let me know if you have any issues.

@KevinM2k
Copy link

KevinM2k commented Jul 13, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
workflow/pending-upstream-library Indicates an issue or PR requires changes from an upstream library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Bulk Redirects feature
3 participants