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: create datadome_endpoint resource #31

Open
wants to merge 7 commits into
base: feat/INT-4354/endpoint-management
Choose a base branch
from

Conversation

lriberon
Copy link
Contributor

@lriberon lriberon commented Jan 2, 2025

Description

  • Add a new resource datadome_endpoint to the Terraform Provider with its Schema (empty CRUD functions for now)
  • Add a new ClientEndpoint for endpoint management
  • Add an endpoint.md documentation about the datadome_endpoint resource and its fields
  • Rename Client to ClientCustomRule in order to create a new client for endpoints
  • Update the Read function of the ClientCustomRule in order to pass the ID of the custom rule we want to fetch
  • Update timeout value for Acceptance test (was 120 minutes, I set it to 1 minute)
  • Update the HttpRequest structure to do not be specific to the CustomRules only
  • Update the providerConfigure function to provide 2 clients: ClientCustomRule and ClientEndpoint

How to test?

The documentation

  1. Go on this website: https://registry.terraform.io/tools/doc-preview
  2. Copy the content of the endpoint.md file
  3. Paste the content to the website to have a preview of the rendered documentation

You should have a preview of the documentation

Acceptance Tests

The aim of this test is to validate that the custom_rule management feature was not impacted by the changes (i.e. renaming, different metadata).

make testacc

@lriberon lriberon self-assigned this Jan 2, 2025
@lriberon lriberon requested a review from a team as a code owner January 2, 2025 17:07
Read(ctx context.Context) ([]T, error)
type API[T any, I comparable] interface {
Create(ctx context.Context, params T) (*I, error)
Read(ctx context.Context, id I) (*T, error)
Copy link

@laurodd laurodd Jan 9, 2025

Choose a reason for hiding this comment

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

question: Now we are returning only one element?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we do.
We were returning a list of elements due to the design of the custom_rules (and also trusted proxies): it does not have a route to return a resource by its ID.

The standard way (i.e. recommended by Hashicorp) is to call a route that retrieve a resource by its ID.
So I have changed the signature of the Read method to only return a single element, and I have moved the logic to find a custom_rule by its ID in the Read method (the logic moved from this file to this one)

@@ -116,7 +123,7 @@ func (c *Client) Create(ctx context.Context, params CustomRule) (*int, error) {
req, err := http.NewRequestWithContext(
ctx,
"POST",
fmt.Sprintf("%s/custom-rules", c.HostURL),
c.HostURL,
Copy link

Choose a reason for hiding this comment

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

kudos: way cleaner!

return fmt.Errorf("status: %d, body: %s", res.StatusCode, body)
}

log.Printf("[DEBUG] %s\n", body)
Copy link

Choose a reason for hiding this comment

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

question: should we keep this print here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that the logs are not that useful.
I think that we should get rid of those logs and add terraform logs instead in a new ticket.
WDYT?

return nil, err
}

log.Printf("[DEBUG] %+v\n", params)
Copy link

Choose a reason for hiding this comment

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

question: same doubt here

docs/resources/endpoint.md Outdated Show resolved Hide resolved
@lriberon lriberon requested a review from laurodd January 9, 2025 12:53
datadome-client-go/client_custom_rule.go Outdated Show resolved Hide resolved
datadome-client-go/client_custom_rule.go Outdated Show resolved Hide resolved
datadome-client-go/client_endpoint.go Outdated Show resolved Hide resolved
@lriberon lriberon requested a review from Lauredg January 9, 2025 14:59
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.

3 participants