Skip to content

Commit

Permalink
page_rules: Swap to completely replacing rules (#338)
Browse files Browse the repository at this point in the history
* page_rules: Swap to completely replacing rules

For a long time, this provider relied on sending page rules using
cloudflare-go's `ChangePageRule` which was documentated to allow inline
updates using a PATCH request. Until recently, this wasn't the case and
it actually mimicked the PUT alternative (`UpdatePageRule`) where it
attempted to replace the entire rule. This was fixed[1] and while you
could now send inline updates to actions, there wasn't a way to remove
them once they were there. To address this, I've swapped back to
`UpdatePageRule` (the PUT request) which will resolve the aforementioned
issue with removing rules but it does re-introduce the caveat of not
being able to manage actions that require a Solutions Engineer. This
means, you can't manage them using this endpoint as you won't have
permission to apply them. This extends to page rule changes like
re-ordering or updating other actions which are in the same rule. If you
hit this use case, you should can move the cache key into Cloudflare
Workers.

This commit backs out most of the changes from c1799f4 as they are no
longer required for differentiating the actions to send.

[1]: https://github.com/terraform-providers/terraform-provider-cloudflare/pull/176#issuecomment-452906541

* Check actions for existence and change status

Updates the `transformToCloudflarePageRuleAction` functionality to
perform checks using `HasChange` to determine whether or not the value
should be sent or `nil`.
  • Loading branch information
jacobbednarz authored and patryk committed May 10, 2019
1 parent dca5645 commit 14128d0
Showing 1 changed file with 27 additions and 26 deletions.
53 changes: 27 additions & 26 deletions cloudflare/resource_cloudflare_page_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,8 @@ func resourceCloudflarePageRuleCreate(d *schema.ResourceData, meta interface{})
log.Printf("[DEBUG] Actions found in config: %#v", actions)
for _, action := range actions {
for id, value := range action.(map[string]interface{}) {
newPageRuleAction, err := transformToCloudflarePageRuleAction(id, value, false)

newPageRuleAction, err := transformToCloudflarePageRuleAction(id, value, d)
if err != nil {
return err
} else if newPageRuleAction.Value == nil || newPageRuleAction.Value == "" {
Expand Down Expand Up @@ -463,25 +464,24 @@ func resourceCloudflarePageRuleUpdate(d *schema.ResourceData, meta interface{})
}
}

old, new := d.GetChange("actions")

oldActions := old.([]interface{})[0].(map[string]interface{})
newActions := new.([]interface{})[0].(map[string]interface{})

newPageRuleActions := make([]cloudflare.PageRuleAction, 0, len(newActions))

for id, value := range newActions {
hasChanged := id != "forwarding_url" && id != "minify" && oldActions[id] != value
newPageRuleAction, err := transformToCloudflarePageRuleAction(id, value, hasChanged)
if err != nil {
return err
} else if newPageRuleAction.Value == nil {
continue
if v, ok := d.GetOk("actions"); ok {
actions := v.([]interface{})
newPageRuleActions := make([]cloudflare.PageRuleAction, 0, len(actions))

for _, action := range actions {
for id, value := range action.(map[string]interface{}) {
newPageRuleAction, err := transformToCloudflarePageRuleAction(id, value, d)
if err != nil {
return err
} else if newPageRuleAction.Value == nil {
continue
}
newPageRuleActions = append(newPageRuleActions, newPageRuleAction)
}
}
newPageRuleActions = append(newPageRuleActions, newPageRuleAction)
}

updatePageRule.Actions = newPageRuleActions
updatePageRule.Actions = newPageRuleActions
}

if priority, ok := d.GetOk("priority"); ok {
updatePageRule.Priority = priority.(int)
Expand All @@ -493,9 +493,7 @@ func resourceCloudflarePageRuleUpdate(d *schema.ResourceData, meta interface{})

log.Printf("[DEBUG] Cloudflare Page Rule update configuration: %#v", updatePageRule)

// contrary to docs, change page rule actually does a full replace
// this part of the api needs some work, so it may change in future
if err := client.ChangePageRule(zoneID, d.Id(), updatePageRule); err != nil {
if err := client.UpdatePageRule(zoneID, d.Id(), updatePageRule); err != nil {
return fmt.Errorf("Failed to update Cloudflare Page Rule: %s", err)
}

Expand Down Expand Up @@ -591,10 +589,12 @@ func transformFromCloudflarePageRuleAction(pageRuleAction *cloudflare.PageRuleAc
return
}

func transformToCloudflarePageRuleAction(id string, value interface{}, changed bool) (pageRuleAction cloudflare.PageRuleAction, err error) {
func transformToCloudflarePageRuleAction(id string, value interface{}, d *schema.ResourceData) (pageRuleAction cloudflare.PageRuleAction, err error) {

pageRuleAction.ID = id

changed := d.HasChange(fmt.Sprintf("actions.0.%s", id))

if strValue, ok := value.(string); ok {
if strValue == "" && !changed {
pageRuleAction.Value = nil
Expand All @@ -616,11 +616,12 @@ func transformToCloudflarePageRuleAction(id string, value interface{}, changed b
}
}
} else if intValue, ok := value.(int); ok {
if (id == "edge_cache_ttl" && intValue == 0) || !changed {
// This happens when not set by the user
pageRuleAction.Value = nil
} else {
if id == "browser_cache_ttl" && changed {
pageRuleAction.Value = intValue
} else if id == "edge_cache_ttl" && intValue > 0 && changed {
pageRuleAction.Value = intValue
} else {
pageRuleAction.Value = nil
}
} else if id == "forwarding_url" {
forwardActionSchema := value.([]interface{})
Expand Down

0 comments on commit 14128d0

Please sign in to comment.