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

fix(product_enablement): add additional error message filter #740

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

Integralist
Copy link
Collaborator

@Integralist Integralist commented Aug 30, 2023

Problem

Customer changed origin_inspector = true to origin_inspector = false, resulting in the following diff...

- product_enablement {
    - brotli_compression = false -> null
    - domain_inspector   = false -> null
    - image_optimizer    = false -> null
    - name               = "products" -> null
    - origin_inspector   = true -> null
    - websockets         = false -> null
  }
+ product_enablement {
    + brotli_compression = false
    + domain_inspector   = false
    + image_optimizer    = false
    + name               = (known after apply)
    + origin_inspector   = false
    + websockets         = false
  }

They were unable to apply this change because the Fastly API returned the following error (which caused Terraform to halt)...

Customer <REDACTED> has access to image_optimizer but cannot self-disable

This error was not being ignored by our error-checking logic.

Solution

It's fixed in this PR by adding "cannot self-disable" as an additional message filter.

Why are we ignoring API errors?

To understand why we're ignoring errors, we first need to understand what happened for this customer.

The product_enablement 'block' is defined as a TypeSet (a specific Terraform type). A TypeSet effectively means any change to the block (so in this case the change to the origin_inspector attribute value) will cause the entire block to be marked by Terraform as needing to be "deleted", then "created" (i.e. 'recreated' in Terraform parlance). This is why we see -> null after each attribute, it's Terraform's way of indicating that the block is a TypeSet that's going to be deleted then recreated.

Because of this config change, the product_enablement 'block' was being recreated, and thus the Terraform DELETE operation was actioned.

The DELETE operation will cause the Fastly Terraform provider to issue multiple API calls (one 'disable product' API call for each product). This happens even if the user hasn't turned on a product.

So in this case, the customer wasn't entitled to make the 'Disable ImageOptmiser' API call, hence the "400 Bad Request" response Customer <REDACTED> has access to image_optimizer but cannot self-disable.

Because Terraform has to issue multiple 'disable product' calls (because that's what should happen if the user ever 'deleted' the product_enablement block **), it means there is a possibility that a user will see an error from trying to disable a product and that error will stop Terraform from applying the remaining config changes and leave the user's state file incomplete.

** Even if Terraform made a GET request first (to check if the product was enabled) before trying to disable the product, the user might still see an error related to permissions and not being allowed to disable the product via the API.

This is why we have to check the error response to see if it's related to the user not being allowed to disable the product, and if that's the case, we ignore the error as we don't want that error (which is expected for certain users) to stop Terraform from processing the rest of the changes.

@Integralist Integralist merged commit 3f5dccd into main Aug 30, 2023
@Integralist Integralist deleted the integralist/fix-prodenablement-error-check branch August 30, 2023 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants