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.

Sign up for GitHub

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

fix(product_enablement): avoid accidentally disabling products on update #763

Merged
merged 1 commit into from
Oct 17, 2023
+60 −99
Merged

fix(product_enablement): avoid accidentally disabling products on update #763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions docs/resources/service_compute.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,12 +661,9 @@ Optional:
Optional:

- `fanout` (Boolean) Enable Fanout support
- `name` (String) Used by the provider to identify modified settings (changing this value will force the entire block to be deleted, then recreated)
- `websockets` (Boolean) Enable WebSockets support

Read-Only:

- `name` (String) Used internally by the provider to identify modified settings


<a id="nestedblock--resource_link"></a>
### Nested Schema for `resource_link`
Expand Down
5 changes: 1 addition & 4 deletions docs/resources/service_vcl.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1093,13 +1093,10 @@ Optional:
- `brotli_compression` (Boolean) Enable Brotli Compression support
- `domain_inspector` (Boolean) Enable Domain Inspector support
- `image_optimizer` (Boolean) Enable Image Optimizer support (requires at least one backend with a `shield` attribute)
- `name` (String) Used by the provider to identify modified settings (changing this value will force the entire block to be deleted, then recreated)
- `origin_inspector` (Boolean) Enable Origin Inspector support
- `websockets` (Boolean) Enable WebSockets support

Read-Only:

- `name` (String) Used internally by the provider to identify modified settings


<a id="nestedblock--rate_limiter"></a>
### Nested Schema for `rate_limiter`
Expand Down
143 changes: 57 additions & 86 deletions fastly/block_fastly_service_product_enablement.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,16 @@ func (h *ProductEnablementServiceAttributeHandler) GetSchema() *schema.Schema {
blockAttributes := map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Computed: true,
Description: "Used internally by the provider to identify modified settings",
Optional: true,
Default: "products",
Description: "Used by the provider to identify modified settings (changing this value will force the entire block to be deleted, then recreated)",
},
}

if h.GetServiceMetadata().serviceType == ServiceTypeCompute {
blockAttributes["fanout"] = &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Default: false,
Description: "Enable Fanout support",
}
}
Expand All @@ -54,25 +54,21 @@ func (h *ProductEnablementServiceAttributeHandler) GetSchema() *schema.Schema {
blockAttributes["brotli_compression"] = &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Default: false,
Description: "Enable Brotli Compression support",
}
blockAttributes["domain_inspector"] = &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Default: false,
Description: "Enable Domain Inspector support",
}
blockAttributes["image_optimizer"] = &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Default: false,
Description: "Enable Image Optimizer support (requires at least one backend with a `shield` attribute)",
}
blockAttributes["origin_inspector"] = &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Default: false,
Description: "Enable Origin Inspector support",
}
}
Expand All @@ -81,7 +77,6 @@ func (h *ProductEnablementServiceAttributeHandler) GetSchema() *schema.Schema {
blockAttributes["websockets"] = &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Default: false,
Description: "Enable WebSockets support",
}

Expand Down Expand Up @@ -214,11 +209,6 @@ func (h *ProductEnablementServiceAttributeHandler) Read(_ context.Context, d *sc
// The API returns a 400 if a product is not enabled.
// The API client returns an error if a non-2xx is returned from the API.

var (
enabled bool
err error
)

// NOTE: We must set name to a unique value for a plan diff to be calculated.
//
// This value can be static because (like with the 'package' block) there can
Expand All @@ -229,72 +219,54 @@ func (h *ProductEnablementServiceAttributeHandler) Read(_ context.Context, d *sc
// This is done so we can benefit from the 'modified' map data passed to the
// 'update' CRUD method.
result := map[string]any{
"name": "products",
"name": localState[0].(map[string]any)["name"].(string),
}

if h.GetServiceMetadata().serviceType == ServiceTypeCompute {
enabled = false
_, err = conn.GetProduct(&gofastly.ProductEnablementInput{
if _, err := conn.GetProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductFanout,
ServiceID: d.Id(),
})
if err == nil {
enabled = true
}); err == nil {
result["fanout"] = true
}
result["fanout"] = enabled
}

if h.GetServiceMetadata().serviceType == ServiceTypeVCL {
enabled = false
_, err = conn.GetProduct(&gofastly.ProductEnablementInput{
if _, err := conn.GetProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductBrotliCompression,
ServiceID: d.Id(),
})
if err == nil {
enabled = true
}); err == nil {
result["brotli_compression"] = true
}
result["brotli_compression"] = enabled

enabled = false
_, err = conn.GetProduct(&gofastly.ProductEnablementInput{
if _, err := conn.GetProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductDomainInspector,
ServiceID: d.Id(),
})
if err == nil {
enabled = true
}); err == nil {
result["domain_inspector"] = true
}
result["domain_inspector"] = enabled

enabled = false
_, err = conn.GetProduct(&gofastly.ProductEnablementInput{
if _, err := conn.GetProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductImageOptimizer,
ServiceID: d.Id(),
})
if err == nil {
enabled = true
}); err == nil {
result["image_optimizer"] = true
}
result["image_optimizer"] = enabled

enabled = false
_, err = conn.GetProduct(&gofastly.ProductEnablementInput{
if _, err := conn.GetProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductOriginInspector,
ServiceID: d.Id(),
})
if err == nil {
enabled = true
}); err == nil {
result["origin_inspector"] = true
}
result["origin_inspector"] = enabled
}

enabled = false
_, err = conn.GetProduct(&gofastly.ProductEnablementInput{
if _, err := conn.GetProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductWebSockets,
ServiceID: d.Id(),
})
if err == nil {
enabled = true
}); err == nil {
result["websockets"] = true
}
result["websockets"] = enabled

results := []map[string]any{result}

Expand All @@ -309,32 +281,19 @@ func (h *ProductEnablementServiceAttributeHandler) Read(_ context.Context, d *sc

// Update updates the resource.
//
// IMPORTANT: The Update method is never called due to an implementation bug.
//
// It's a side-effect of the SetDiff logic https://github.com/fastly/terraform-provider-fastly/blob/6e03cce3127a30db94c1cdfa8eb621d9a7231989/fastly/service_crud_attribute_definition.go#L89-L95
// The SetDiff logic uses `name` as a computed key (i.e. if there’s a change to
// `name`, then it means the resource has changed and needs to be recreated).
// IMPORTANT: We ignore errors related to entitlement when updating.
//
// The problem is we defined `name` as a Computed attribute, which means it will
// always be marked as being changed as it's reset to an empty string due to it
// being a Computed attribute where the value is known only AFTER an apply.
//
// Fixing this bug would mean needing to change `name` from being Computed to
// Optional and also setting a default to match the hardcoded value "products"
// that we used when it was Computed, and to configure the attribute to be
// ignored by the Terraform diff processing logic. That should in theory make it
// a non-breaking change.
//
// This isn't the end of the world, it just means there are a few more API calls
// made. We're also (as of Sept 2023) in the process of rewriting the Terraform
// provider and so it might be best to resolve this as part of the rewrite.
// This is to provide a non-breaking workaround for customers who used an older
// version of the Fastly Terraform provider. See details in the PR:
// https://github.com/fastly/terraform-provider-fastly/pull/763
func (h *ProductEnablementServiceAttributeHandler) Update(_ context.Context, d *schema.ResourceData, _, modified map[string]any, serviceVersion int, conn *gofastly.Client) error {
serviceID := d.Id()
log.Println("[DEBUG] Update Product Enablement")

if h.GetServiceMetadata().serviceType == ServiceTypeCompute {
if v, ok := modified["fanout"]; ok {
if v.(bool) {
log.Println("[DEBUG] fanout set")
log.Println("[DEBUG] fanout will be enabled")
_, err := conn.EnableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductFanout,
ServiceID: serviceID,
Expand All @@ -343,13 +302,15 @@ func (h *ProductEnablementServiceAttributeHandler) Update(_ context.Context, d *
return fmt.Errorf("failed to enable fanout: %w", err)
}
} else {
log.Println("[DEBUG] fanout not set")
log.Println("[DEBUG] fanout will be disabled")
err := conn.DisableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductFanout,
ServiceID: serviceID,
})
if err != nil {
return fmt.Errorf("failed to disable fanout: %w", err)
if e := h.checkAPIError(err); e != nil {
return e
}
}
}
}
Expand All @@ -358,7 +319,7 @@ func (h *ProductEnablementServiceAttributeHandler) Update(_ context.Context, d *
if h.GetServiceMetadata().serviceType == ServiceTypeVCL {
if v, ok := modified["brotli_compression"]; ok {
if v.(bool) {
log.Println("[DEBUG] brotli_compression set")
log.Println("[DEBUG] brotli_compression will be enabled")
_, err := conn.EnableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductBrotliCompression,
ServiceID: serviceID,
Expand All @@ -367,20 +328,22 @@ func (h *ProductEnablementServiceAttributeHandler) Update(_ context.Context, d *
return fmt.Errorf("failed to enable brotli_compression: %w", err)
}
} else {
log.Println("[DEBUG] brotli_compression not set")
log.Println("[DEBUG] brotli_compression will be disabled")
err := conn.DisableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductBrotliCompression,
ServiceID: serviceID,
})
if err != nil {
return fmt.Errorf("failed to disable brotli_compression: %w", err)
if e := h.checkAPIError(err); e != nil {
return e
}
}
}
}

if v, ok := modified["domain_inspector"]; ok {
if v.(bool) {
log.Println("[DEBUG] domain_inspector set")
log.Println("[DEBUG] domain_inspector will be enabled")
_, err := conn.EnableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductDomainInspector,
ServiceID: serviceID,
Expand All @@ -389,20 +352,22 @@ func (h *ProductEnablementServiceAttributeHandler) Update(_ context.Context, d *
return fmt.Errorf("failed to enable domain_inspector: %w", err)
}
} else {
log.Println("[DEBUG] domain_inspector not set")
log.Println("[DEBUG] domain_inspector will be disabled")
err := conn.DisableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductDomainInspector,
ServiceID: serviceID,
})
if err != nil {
return fmt.Errorf("failed to disable domain_inspector: %w", err)
if e := h.checkAPIError(err); e != nil {
return e
}
}
}
}

if v, ok := modified["image_optimizer"]; ok {
if v.(bool) {
log.Println("[DEBUG] image_optimizer set")
log.Println("[DEBUG] image_optimizer will be enabled")
_, err := conn.EnableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductImageOptimizer,
ServiceID: serviceID,
Expand All @@ -411,20 +376,22 @@ func (h *ProductEnablementServiceAttributeHandler) Update(_ context.Context, d *
return fmt.Errorf("failed to enable image_optimizer: %w", err)
}
} else {
log.Println("[DEBUG] image_optimizer not set")
log.Println("[DEBUG] image_optimizer will be disabled")
err := conn.DisableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductImageOptimizer,
ServiceID: serviceID,
})
if err != nil {
return fmt.Errorf("failed to disable image_optimizer: %w", err)
if e := h.checkAPIError(err); e != nil {
return e
}
}
}
}

if v, ok := modified["origin_inspector"]; ok {
if v.(bool) {
log.Println("[DEBUG] origin_inspector set")
log.Println("[DEBUG] origin_inspector will be enabled")
_, err := conn.EnableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductOriginInspector,
ServiceID: serviceID,
Expand All @@ -433,21 +400,23 @@ func (h *ProductEnablementServiceAttributeHandler) Update(_ context.Context, d *
return fmt.Errorf("failed to enable origin_inspector: %w", err)
}
} else {
log.Println("[DEBUG] origin_inspector not set")
log.Println("[DEBUG] origin_inspector will be disabled")
err := conn.DisableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductOriginInspector,
ServiceID: serviceID,
})
if err != nil {
return fmt.Errorf("failed to disable origin_inspector: %w", err)
if e := h.checkAPIError(err); e != nil {
return e
}
}
}
}
}

if v, ok := modified["websockets"]; ok {
if v.(bool) {
log.Println("[DEBUG] websockets set")
log.Println("[DEBUG] websockets will be enabled")
_, err := conn.EnableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductWebSockets,
ServiceID: serviceID,
Expand All @@ -456,13 +425,15 @@ func (h *ProductEnablementServiceAttributeHandler) Update(_ context.Context, d *
return fmt.Errorf("failed to enable websockets: %w", err)
}
} else {
log.Println("[DEBUG] websockets not set")
log.Println("[DEBUG] websockets will be disabled")
err := conn.DisableProduct(&gofastly.ProductEnablementInput{
ProductID: gofastly.ProductWebSockets,
ServiceID: serviceID,
})
if err != nil {
return fmt.Errorf("failed to disable websockets: %w", err)
if e := h.checkAPIError(err); e != nil {
return e
}
}
}
}
Expand Down
6 changes: 1 addition & 5 deletions tests/interface/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,7 @@ resource "fastly_service_vcl" "interface-test-project" {
}

product_enablement {
brotli_compression = false
domain_inspector = false
image_optimizer = false
origin_inspector = false
websockets = false
brotli_compression = true
}

rate_limiter {
Expand Down