Skip to content

Commit

Permalink
Add deprecation notice for zone, take 2 (#452)
Browse files Browse the repository at this point in the history 
We have been working to perform some breaking changes in #227 and the
first attempt at this in #421 neglected to allow people to set `zone_id`
and action the deprecation notice 🤦

This second attempt has the same goal of deprecating `zone` in favour of
explicit `zone_id` however this time I've updated both schema properties
to be optional and added some validation in the various `Create` methods
to ensure that we get at least one of them during the swap over period.
That code can be removed as soon we've cut a breaking change release.

Closes #439
  • Loading branch information
@jacobbednarz @patryk
jacobbednarz authored and patryk committed Aug 22, 2019
1 parent 1b53e57 commit d1b586b
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 61 deletions.
32 changes: 23 additions & 9 deletions cloudflare/resource_cloudflare_load_balancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,16 @@ func resourceCloudflareLoadBalancer() *schema.Resource {
SchemaVersion: 0,
Schema: map[string]*schema.Schema{
"zone": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
// Deprecated: "`zone` is deprecated in favour of explicit `zone_id` and will be removed in the next major release",
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Deprecated: "`zone` is deprecated in favour of explicit `zone_id` and will be removed in the next major release",
},

"zone_id": {
Type: schema.TypeString,
Computed: true,
Optional: true,
ForceNew: true,
},

"name": {
Expand Down Expand Up @@ -172,6 +173,16 @@ var localPoolElems = map[string]*schema.Resource{
func resourceCloudflareLoadBalancerCreate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*cloudflare.API)

zoneName := d.Get("zone").(string)
zoneID := d.Get("zone_id").(string)

// While we are deprecating `zone`, we need to perform the validation
// inside the `Create` to ensure we get at least one of the expected
// values.
if zoneName == "" && zoneID == "" {
return fmt.Errorf("either zone name or ID must be provided")
}

enabled := d.Get("enabled").(bool)
newLoadBalancer := cloudflare.LoadBalancer{
Name: d.Get("name").(string),
Expand Down Expand Up @@ -208,11 +219,14 @@ func resourceCloudflareLoadBalancerCreate(d *schema.ResourceData, meta interface
newLoadBalancer.PopPools = expandedPopPools
}

zoneName := d.Get("zone").(string)
zoneID, err := client.ZoneIDByName(zoneName)
if err != nil {
return fmt.Errorf("error finding zone %q: %s", zoneName, err)
if zoneID == "" {
var err error
zoneID, err = client.ZoneIDByName(zoneName)
if err != nil {
return fmt.Errorf("error finding zone %q: %s", zoneName, err)
}
}

d.Set("zone_id", zoneID)

log.Printf("[INFO] Creating Cloudflare Load Balancer from struct: %+v", newLoadBalancer)
Expand Down
28 changes: 20 additions & 8 deletions cloudflare/resource_cloudflare_page_rule.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ func resourceCloudflarePageRule() *schema.Resource {
SchemaVersion: 0,
Schema: map[string]*schema.Schema{
"zone": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
// Deprecated: "`zone` is deprecated in favour of explicit `zone_id` and will be removed in the next major release",
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Deprecated: "`zone` is deprecated in favour of explicit `zone_id` and will be removed in the next major release",
},

"zone_id": {
Type: schema.TypeString,
Computed: true,
Optional: true,
ForceNew: true,
},

"target": {
Expand Down Expand Up @@ -330,6 +331,14 @@ func suppressEquivalentURLs(k, old, new string, d *schema.ResourceData) bool {
func resourceCloudflarePageRuleCreate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*cloudflare.API)
zone := d.Get("zone").(string)
zoneID := d.Get("zone_id").(string)

// While we are deprecating `zone`, we need to perform the validation
// inside the `Create` to ensure we get at least one of the expected
// values.
if zone == "" && zoneID == "" {
return fmt.Errorf("either zone name or ID must be provided")
}

newPageRuleTargets := []cloudflare.PageRuleTarget{
{
Expand Down Expand Up @@ -372,9 +381,12 @@ func resourceCloudflarePageRuleCreate(d *schema.ResourceData, meta interface{})
Status: d.Get("status").(string),
}

zoneID, err := client.ZoneIDByName(zone)
if err != nil {
return fmt.Errorf("Error finding zone %q: %s", zone, err)
if zoneID == "" {
var err error
zoneID, err = client.ZoneIDByName(zone)
if err != nil {
return fmt.Errorf("Error finding zone %q: %s", zone, err)
}
}

d.Set("zone_id", zoneID)
Expand Down
38 changes: 26 additions & 12 deletions cloudflare/resource_cloudflare_rate_limit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ func resourceCloudflareRateLimit() *schema.Resource {
SchemaVersion: 0,
Schema: map[string]*schema.Schema{
"zone": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
// Deprecated: "`zone` is deprecated in favour of explicit `zone_id` and will be removed in the next major release",
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Deprecated: "`zone` is deprecated in favour of explicit `zone_id` and will be removed in the next major release",
},

"zone_id": {
Type: schema.TypeString,
Computed: true,
Optional: true,
ForceNew: true,
},

"threshold": {
Expand Down Expand Up @@ -199,6 +200,16 @@ func resourceCloudflareRateLimit() *schema.Resource {
func resourceCloudflareRateLimitCreate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*cloudflare.API)

zoneName := d.Get("zone").(string)
zoneID := d.Get("zone_id").(string)

// While we are deprecating `zone`, we need to perform the validation
// inside the `Create` to ensure we get at least one of the expected
// values.
if zoneName == "" && zoneID == "" {
return fmt.Errorf("either zone name or ID must be provided")
}

rateLimitAction, err := expandRateLimitAction(d)
if err != nil {
return errors.Wrap(err, "error expanding rate limit action")
Expand Down Expand Up @@ -236,15 +247,20 @@ func resourceCloudflareRateLimitCreate(d *schema.ResourceData, meta interface{})
}
newRateLimit.Action = newRateLimitAction

zoneName := d.Get("zone").(string)
zoneId, err := client.ZoneIDByName(zoneName)
if err != nil {
return fmt.Errorf("error finding zone %q: %s", zoneName, err)
if zoneID == "" {
var err error
zoneID, err = client.ZoneIDByName(zoneName)
if err != nil {
return fmt.Errorf("error finding zone %q: %s", zoneName, err)
}
}

// assume ids are immutable, not going to look it up from the api again
d.Set("zone_id", zoneID)

log.Printf("[DEBUG] Creating Cloudflare Rate Limit from struct: %+v", newRateLimit)

r, err := client.CreateRateLimit(zoneId, newRateLimit)
r, err := client.CreateRateLimit(zoneID, newRateLimit)
if err != nil {
return errors.Wrap(err, "error creating rate limit for zone")
}
Expand All @@ -254,8 +270,6 @@ func resourceCloudflareRateLimitCreate(d *schema.ResourceData, meta interface{})
}

d.SetId(r.ID)
// assume ids are immutable, not going to look it up from the api again
d.Set("zone_id", zoneId)

log.Printf("[INFO] Cloudflare Rate Limit ID: %s", d.Id())

Expand Down
25 changes: 18 additions & 7 deletions cloudflare/resource_cloudflare_waf_rule.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ func resourceCloudflareWAFRule() *schema.Resource {
},

"zone": {
Type: schema.TypeString,
Required: true,
// Deprecated: "`zone` is deprecated in favour of explicit `zone_id` and will be removed in the next major release",
Type: schema.TypeString,
Optional: true,
Deprecated: "`zone` is deprecated in favour of explicit `zone_id` and will be removed in the next major release",
},

"zone_id": {
Type: schema.TypeString,
Computed: true,
Optional: true,
},

"package_id": {
Expand Down Expand Up @@ -73,11 +73,22 @@ func resourceCloudflareWAFRuleCreate(d *schema.ResourceData, meta interface{}) e
client := meta.(*cloudflare.API)
ruleID := d.Get("rule_id").(string)
zone := d.Get("zone").(string)
zoneID := d.Get("zone_id").(string)
mode := d.Get("mode").(string)

zoneID, err := client.ZoneIDByName(zone)
if err != nil {
return err
// While we are deprecating `zone`, we need to perform the validation
// inside the `Create` to ensure we get at least one of the expected
// values.
if zone == "" && zoneID == "" {
return fmt.Errorf("either zone name or ID must be provided")
}

if zoneID == "" {
var err error
zoneID, err = client.ZoneIDByName(zone)
if err != nil {
return err
}
}

packs, err := client.ListWAFPackages(zoneID)
Expand Down
63 changes: 38 additions & 25 deletions cloudflare/resource_cloudflare_worker_route.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,16 @@ func resourceCloudflareWorkerRoute() *schema.Resource {

Schema: map[string]*schema.Schema{
"zone": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
// Deprecated: "`zone` is deprecated in favour of explicit `zone_id` and will be removed in the next major release",
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Deprecated: "`zone` is deprecated in favour of explicit `zone_id` and will be removed in the next major release",
},

"zone_id": {
Type: schema.TypeString,
Computed: true,
Optional: true,
ForceNew: true,
},

"multi_script": {
Expand Down Expand Up @@ -77,18 +78,30 @@ func getRouteFromResource(d *schema.ResourceData) cloudflare.WorkerRoute {
func resourceCloudflareWorkerRouteCreate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*cloudflare.API)
route := getRouteFromResource(d)

zoneName := d.Get("zone").(string)
zoneId, err := client.ZoneIDByName(zoneName)
if err != nil {
return fmt.Errorf("error finding zone %q: %s", zoneName, err)
zoneID := d.Get("zone_id").(string)

// While we are deprecating `zone`, we need to perform the validation
// inside the `Create` to ensure we get at least one of the expected
// values.
if zoneName == "" && zoneID == "" {
return fmt.Errorf("either zone name or ID must be provided")
}
d.Set("zone_id", zoneId)

if zoneID == "" {
var err error
zoneID, err = client.ZoneIDByName(zoneName)
if err != nil {
return fmt.Errorf("error finding zone %q: %s", zoneName, err)
}
}

d.Set("zone_id", zoneID)
d.Set("multi_script", route.Script != "")

log.Printf("[INFO] Creating Cloudflare Worker Route from struct: %+v", route)

r, err := client.CreateWorkerRoute(zoneId, route)
r, err := client.CreateWorkerRoute(zoneID, route)
if err != nil {
return errors.Wrap(err, "error creating worker route")
}
Expand All @@ -106,20 +119,20 @@ func resourceCloudflareWorkerRouteCreate(d *schema.ResourceData, meta interface{

func resourceCloudflareWorkerRouteRead(d *schema.ResourceData, meta interface{}) error {
client := meta.(*cloudflare.API)
zoneId := d.Get("zone_id").(string)
routeId := d.Id()
zoneID := d.Get("zone_id").(string)
routeID := d.Id()

// There isn't a dedicated endpoint for retrieving a specific route, so we
// list all routes and find the target route by comparing IDs
resp, err := client.ListWorkerRoutes(zoneId)
resp, err := client.ListWorkerRoutes(zoneID)

if err != nil {
return errors.Wrap(err, "error reading worker routes")
}

var route cloudflare.WorkerRoute
for _, r := range resp.Routes {
if r.ID == routeId {
if r.ID == routeID {
route = r
break
}
Expand All @@ -145,12 +158,12 @@ func resourceCloudflareWorkerRouteRead(d *schema.ResourceData, meta interface{})

func resourceCloudflareWorkerRouteUpdate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*cloudflare.API)
zoneId := d.Get("zone_id").(string)
zoneID := d.Get("zone_id").(string)
route := getRouteFromResource(d)

log.Printf("[INFO] Updating Cloudflare Worker Route from struct: %+v", route)

_, err := client.UpdateWorkerRoute(zoneId, route.ID, route)
_, err := client.UpdateWorkerRoute(zoneID, route.ID, route)
if err != nil {
return errors.Wrap(err, "error updating worker route")
}
Expand All @@ -161,12 +174,12 @@ func resourceCloudflareWorkerRouteUpdate(d *schema.ResourceData, meta interface{

func resourceCloudflareWorkerRouteDelete(d *schema.ResourceData, meta interface{}) error {
client := meta.(*cloudflare.API)
zoneId := d.Get("zone_id").(string)
zoneID := d.Get("zone_id").(string)
route := getRouteFromResource(d)

log.Printf("[INFO] Deleting Cloudflare Worker Route from zone %+v with id: %+v", zoneId, route.ID)
log.Printf("[INFO] Deleting Cloudflare Worker Route from zone %+v with id: %+v", zoneID, route.ID)

_, err := client.DeleteWorkerRoute(zoneId, route.ID)
_, err := client.DeleteWorkerRoute(zoneID, route.ID)
if err != nil {
return errors.Wrap(err, "error deleting worker route")
}
Expand All @@ -181,19 +194,19 @@ func resourceCloudflareWorkerRouteImport(d *schema.ResourceData, meta interface{
// split the id so we can lookup
idAttr := strings.SplitN(d.Id(), "/", 2)
var zoneName string
var routeId string
var routeID string
if len(idAttr) == 2 {
zoneName = idAttr[0]
routeId = idAttr[1]
routeID = idAttr[1]
} else {
return nil, fmt.Errorf("invalid id (\"%s\") specified, should be in format \"zoneName/routeId\"", d.Id())
return nil, fmt.Errorf("invalid id (\"%s\") specified, should be in format \"zoneName/routeID\"", d.Id())
}

zoneID, err := client.ZoneIDByName(zoneName)
routes, err := client.ListWorkerRoutes(zoneID)

for _, r := range routes.Routes {
if r.ID == routeId && client.OrganizationID != "" {
if r.ID == routeID && client.OrganizationID != "" {
isEnterpriseWorker = true
}
}
Expand All @@ -205,7 +218,7 @@ func resourceCloudflareWorkerRouteImport(d *schema.ResourceData, meta interface{
d.Set("zone", zoneName)
d.Set("zone_id", zoneID)
d.Set("multi_script", isEnterpriseWorker)
d.SetId(routeId)
d.SetId(routeID)

return []*schema.ResourceData{d}, nil
}

0 comments on commit d1b586b

Please sign in to comment.