Skip to content

feat(acls): Implement compute acls #987

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jmoney
Copy link
Contributor

@jmoney jmoney commented Apr 28, 2025

accidentally removed the whole template, sorry.

Tests pass and this is the same state as #972 just could not reopen that one

@jmoney jmoney requested a review from a team as a code owner April 28, 2025 19:29
@jmoney jmoney requested a review from philippschulte April 28, 2025 19:29
@jmoney jmoney changed the title Jsm.fastly acls feat(acls): Implement compute acls Apr 28, 2025
@jmoney
Copy link
Contributor Author

jmoney commented May 1, 2025

hey @philippschulte. you have time for a read?

Copy link
Member

@philippschulte philippschulte left a comment

Choose a reason for hiding this comment

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

Tests pass and this is the same state as #972 just could not reopen that one

I was waiting for you to complete this PR. This is not the same state as #972. The Compute ACL entries implementation is missing.

Before you add the missing resource please address all of my comments first. Please don't force push. Thank you!

@@ -4,6 +4,7 @@

### ENHANCEMENTS:

- feat(acl): add support for Compute@Edge ACLs
Copy link
Member

Choose a reason for hiding this comment

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

Please replace all instances of Compute@Edge with Compute in the entire pr. Please also add the link to the PR.

resource "fastly_compute_acl" "example" {
name = "example_acl"
force_destroy = true
}
Copy link
Member

Choose a reason for hiding this comment

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


[1]: https://developer.fastly.com/reference/api/acls/

{{ .SchemaMarkdown | trimspace }}
Copy link
Member

Choose a reason for hiding this comment

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

@@ -0,0 +1,775 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this file.

@@ -0,0 +1 @@
terraform import fastly_acl.example 7d991f5f-7c40-4c8c-a0c1-6ea5e45e4bcf/entries
Copy link
Member

Choose a reason for hiding this comment

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

The filename and resource name doesn't look right.

resource "fastly_compute_acl" "example" {
name = "example_acl"
force_destroy = true
}
Copy link
Member

Choose a reason for hiding this comment

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

To make it complete please add:

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

```
```

## Argument Reference
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this since the schema gets generated at the bottom.

* `name` - (Required) A unique name to identify the ACL.
* `force_destroy` - (Optional) Allow the ACL to be deleted even if it contains entries. Defaults to `false`.

## Attributes Reference
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this since the schema gets generated at the bottom.

$ terraform import fastly_compute_acl.example 7d991f5f-7c40-4c8c-a0c1-6ea5e45e4bcf
```

Where `7d991f5f-7c40-4c8c-a0c1-6ea5e45e4bcf` is the ACL ID.
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this line.

resource "fastly_compute_acl" "example" {
name = "example_acl"
force_destroy = true
}
Copy link
Member

Choose a reason for hiding this comment

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

Please add the following to make the example complete:

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

@jmoney
Copy link
Contributor Author

jmoney commented May 1, 2025

gah, opened the wrong PR. let me find the right branch

@jmoney jmoney closed this May 1, 2025
@philippschulte philippschulte reopened this May 1, 2025
@philippschulte
Copy link
Member

philippschulte commented May 1, 2025

@jmoney please use this PR because it has 40 comments for the data and compute ACL resource already! Just add the missing resource. Thank you!

@jmoney
Copy link
Contributor Author

jmoney commented May 1, 2025

can do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants