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

feat(kv_store): support KV Store #691

Merged
merged 11 commits into from
May 31, 2023
Merged

feat(kv_store): support KV Store #691

merged 11 commits into from
May 31, 2023

Conversation

Integralist
Copy link
Collaborator

@Integralist Integralist commented May 2, 2023

This PR introduces a new fastly_kvstore resource and a resource_link block within the fastly_service_compute resource.

This PR does NOT introduce any additional 'resource' for managing the contents of the KV Store as each key value can potentially be up to 25MB in size and that's not feasible for Terraform to manage (refer to Configuration not data).

✅ The full integration test suite has been run locally and is passing.

The files of interest are...

  • docs/resources/kvstore.md
  • fastly/block_fastly_service_resource_link.go
  • fastly/provider.go
  • fastly/resource_fastly_kvstore.go

Example config...

# IMPORTANT: Deleting a KV Store requires first deleting its resource_link.
# This requires a two-step `terraform apply` as we can't guarantee deletion order.
# e.g. resource_link deletion within fastly_service_compute might not finish first.
resource "fastly_kvstore" "example" {
  name = "my_kv_store"
}

resource "fastly_service_compute" "example" {
  name = "my_compute_service"

  domain {
    name = "demo.example.com"
  }

  package {
    filename         = "package.tar.gz"
    source_code_hash = data.fastly_package_hash.example.hash
  }
  
  resource_link {
    name        = "my_resource_link"
    resource_id = fastly_kvstore.example.id
  }

  force_destroy = true
}

data "fastly_package_hash" "example" {
  filename = "package.tar.gz"
}

Notes

This PR went through multiple design changes, and we ended up having to have resource_link as a block inside the fastly_service_compute resource (rather than as a separate resource) because we needed it to have access to the service version and this isn't something that could just be passed in as a 'expression reference' as the service itself needs to clone its currently tracked version.

Yes, it is possible to clone a service from within a separate resource just by calling the relevant API endpoint, but we then can't update the Terraform state to reflect the newly tracked version. I'm sure there would be some kind of workaround where we track this information via the file-system or some other external data store, but we can't know if the user has set activate = false in their service config (hence we can't trust just looking at the API response for currently active service as the user might be working on a draft).

The reason a deletion is a two-step apply is because you can't delete a KV Store until you delete any resource links associated with it. Meaning if we tried to delete both the fastly_kvstore resource and the resource_link block then there would be an error if fastly_kvstore's DELETE operation was executed first (as it would find a still active resource link associated with it). The reason this is even more likely to happen is because in our example the resource_link block references the fastly_kvstore and so that builds an implicit dependency graph and so Terraform will naturally try to delete the fastly_kvstore first.

@Integralist Integralist added the enhancement New feature or request label May 2, 2023
@Integralist Integralist marked this pull request as ready for review May 3, 2023 10:28
Copy link

@dennismartensson dennismartensson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, but I have never built anything for terraform, so can't really review that part.

@Integralist Integralist marked this pull request as draft May 3, 2023 15:42
@Integralist Integralist force-pushed the integralist/kvstore branch from 3c5ab42 to 84e6bdf Compare May 24, 2023 10:10
@Integralist Integralist marked this pull request as ready for review May 30, 2023 09:50
@Integralist Integralist requested a review from razaj92 May 30, 2023 09:50
@Integralist Integralist merged commit 3488948 into main May 31, 2023
@Integralist Integralist deleted the integralist/kvstore branch May 31, 2023 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants