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

Incorrect hosts attribute type for cloudflare_certificate_pack #3287

Open
3 tasks done
tothdavid opened this issue May 6, 2024 · 6 comments
Open
3 tasks done

Incorrect hosts attribute type for cloudflare_certificate_pack #3287

tothdavid opened this issue May 6, 2024 · 6 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/debug-log-attached Indicates an issue or PR has a complete Terraform debug log.

Comments

@tothdavid
Copy link

tothdavid commented May 6, 2024

Confirmation

  • This is a bug with an existing resource and is not a feature request or enhancement. Feature requests should be submitted with Cloudflare Support or your account team.
  • I have searched the issue tracker and my issue isn't already found.
  • I have replicated my issue using the latest version of the provider and it is still present.

Terraform and Cloudflare provider version

Terraform v1.8.2
(Although it has nothing to do with the issue)

Affected resource(s)

cloudflare_certificate_pack

Terraform configuration files

resource "cloudflare_certificate_pack" "certificate" {
  zone_id                = data.cloudflare_zone.zone.id
  type                   = "advanced"
  hosts                  = ["b.example.com", "a.example.com", "c.example.com"]
  validation_method      = "txt"
  validity_days          = 90
  certificate_authority  = "lets_encrypt"
  cloudflare_branding    = false
  wait_for_active_status = true
}

Link to debug output

https://gist.github.com/tothdavid/d8e276439e7520f42b2ca41a615b2361

Panic output

No response

Expected output

In the created certificate pack the order of hostnames should be retained. This can be checked on cloudflare' API:
https://api.cloudflare.com/client/v4/zones/{zone_id}/ssl/certificate_packs/{certificate_pack_id} (https://developers.cloudflare.com/api/operations/certificate-packs-get-certificate-pack)

Actual output

The order of hostnames in the created certificate pack is not defined.

Steps to reproduce

See "Additional factoids"

Additional factoids

The type of the hosts attribute for the cloudflare_certificate_pack resource is wrong:
On the Cloudflare's API it is defined as an array[strings] (see: https://developers.cloudflare.com/api/operations/certificate-packs-order-advanced-certificate-manager-certificate-pack#request-body), on the other hand this resource defines it as set(string) (see: https://registry.terraform.io/providers/cloudflare/cloudflare/latest/docs/resources/certificate_pack#hosts )
This will make the order of the hostnames passed to the Cloudflare's API undefined, so the user has no control over that. This is however important, because Cloudflare will use the first entry for the CN of the certificate.

Currently the only workaround is to create the Advanced certificate pack manually, although the UI will also sort the hostnames in a certain order after the pack is created, but for creating the certificate the order of the provided hostnames are respected.

References

No response

@tothdavid tothdavid 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 May 6, 2024
Copy link
Contributor

github-actions bot commented May 6, 2024

Community Note

Voting for Prioritization

  • Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

  • If you are interested in working on this issue, please leave a comment.
  • If this would be your first contribution, please review the contribution guide.

Copy link
Contributor

github-actions bot commented May 6, 2024

Terraform debug log detected ✅

@github-actions github-actions bot added triage/needs-information Indicates an issue needs more information in order to work on it. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 6, 2024
@tothdavid
Copy link
Author

tothdavid commented May 6, 2024

Terraform debug log detected ✅

@github-actions github-actions bot added triage/debug-log-attached Indicates an issue or PR has a complete Terraform debug log. and removed triage/needs-information Indicates an issue needs more information in order to work on it. labels May 6, 2024
@tothdavid
Copy link
Author

So, the provided log file is a dummy one, since the issue does not need diagnosis, any logs would be irrelevant.

@tyrken
Copy link

tyrken commented Sep 4, 2024

Looks like it was made to be a Set due to an issue with changing host order in API when the cert goes from Pending to Active: #799

@tothdavid
Copy link
Author

That explains a lot. I am not very familiar with the provider implementation (or the framework/interfaces used for implementing the providers), but is it possible to have the type as a string array, but still ignore the ordering when comparing the content of the state to the API response?
If not, then probably this is an issue that would need to be fixed in cloudflare API itself first, so that the order of the hosts would not be changed (although that might not happen very soon I am afraid...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/debug-log-attached Indicates an issue or PR has a complete Terraform debug log.
Projects
None yet
Development

No branches or pull requests

2 participants