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

Chore: improve integration tests output and reduce noise #693

Merged
merged 5 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 23 additions & 18 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,21 @@ This document intends to give guidelines for developing new resources for the en

## Project Structure

* client - Contains code for using the [env0 API](https://developer.env0.com/docs/api) + unit tests.
* env0 - Contains code that implements the env0 Terraform "resources" and "data" + acceptance tests.
* docs - Contains auto-generated md files derived from the "resources" and "data" go files. Deployed as part of the terraform provider documentation.
* tests - Contains integration tests (executed by harness.go).
* examples - Contains examples files for resources and data. Deployed as part of the documentation. Deployed as part of the terraform provider documentation.
- client - Contains code for using the [env0 API](https://developer.env0.com/docs/api) + unit tests.
- env0 - Contains code that implements the env0 Terraform "resources" and "data" + acceptance tests.
- docs - Contains auto-generated md files derived from the "resources" and "data" go files. Deployed as part of the terraform provider documentation.
- tests - Contains integration tests (executed by harness.go).
- examples - Contains examples files for resources and data. Deployed as part of the documentation. Deployed as part of the terraform provider documentation.

## Client

The first step to adding a new resource is implementing the client API calls under the folder client.

Review the API documentation. And pay attention to the following details:
* URL Path (path parameters) and HTTP method (GET, POST, PUT, PATCH, etc...).
* Request Body (pay special attention to required fields).
* Response Body.

- URL Path (path parameters) and HTTP method (GET, POST, PUT, PATCH, etc...).
- Request Body (pay special attention to required fields).
- Response Body.

The env0 website uses the API as well. Therefore, if examples are required. The easiest way to understand the API is to make the calls from the GUI itself. Use Chrome developer tools and review the relevant API requests and responses.

Expand All @@ -36,9 +37,10 @@ After the client API calls are implemented, the next step is implementing the re
Create a new file under the env0 directory. Use existing implementations as templates for implementing the new resource. Check [resource_module.go](./env0/resource_module.go) for reference.

Start by defining the schema. Use the API documentation to identify:
* what fields are required vs. optional.
* what fields require custom validators (see existing [validators](./env0/validators.go))
* what fields force a new resource if modified.

- what fields are required vs. optional.
- what fields require custom validators (see existing [validators](./env0/validators.go))
- what fields force a new resource if modified.

For all resources: CreateContext, ReadContext and DeleteContext are required.
Most resources also implement UpdateContext and Importer (for imports).
Expand All @@ -59,8 +61,9 @@ The writeResourceData function receives a golang struct and a Terraform configur
Check [resource_module.go](./env0/resource_module.go) that uses the utilities vs [resource_environment.go](./env0/resource_environment.go) that does not.

Pay attention to the following caveats:
* The golang fields are in CamalCase, while the terraform fields are in snake_case. They must match. E.g., ProjectName (golang) == project_name (Terraform). To override the default CamalCase to snake_case conversion you may use the tag `tfschema`. To ignore a field set the `tfschema` tag value to `-`.
* In some cases if a field's value is empty (string: "", boolean: false, int: 0) it may be desired to ignore it. To apply this behavior add `omitempty` to the tfschema tag.

- The golang fields are in CamalCase, while the terraform fields are in snake_case. They must match. E.g., ProjectName (golang) == project_name (Terraform). To override the default CamalCase to snake_case conversion you may use the tag `tfschema`. To ignore a field set the `tfschema` tag value to `-`.
- In some cases if a field's value is empty (string: "", boolean: false, int: 0) it may be desired to ignore it. To apply this behavior add `omitempty` to the tfschema tag.

#### writeResourceDataSlice

Expand All @@ -82,7 +85,7 @@ apiClient := meta.(client.ApiClientInterface)
module, err := apiClient.Module(d.Id())
if err != nil {
return ResourceGetFailure("module", d, err)
return ResourceGetFailure(ctx, "module", d, err)
}
.
Expand All @@ -91,7 +94,7 @@ if err != nil {
func ResourceGetFailure(resourceName string, d *schema.ResourceData, err error) diag.Diagnostics {
if frerr, ok := err.(*http.FailedResponseError); ok && frerr.NotFound() {
log.Printf("[WARN] Drift Detected: Terraform will remove %s from state", d.Id())
tflog.Warn(ctx, "Drift Detected: Terraform will remove id from state", map[string]interface{}{"id": d.Id()})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is the only change...
the rest vscode plugin formatting the readme file.

d.SetId("")
return nil
}
Expand All @@ -110,6 +113,7 @@ Check [data_gcp_credentials.go](./env0/data_gcp_credentials.go) for an example.
Finally create acceptance tests for data. Check [data_gcp_credentials_test.go](./env0/data_gcp_credentials_test.go) for an example.

**Note** In most cases, data is calculated by name or id. Names may not be unique. If searching by name and more than one resource is returned, it's considered an error.

```
func getGcpCredentialsByName(name interface{}, meta interface{}) (client.Credentials, diag.Diagnostics) {
apiClient := meta.(client.ApiClientInterface)
Expand Down Expand Up @@ -141,9 +145,10 @@ If applicable, create an integration test for the new resource and data.
The folder tests/integration contains a list of folders. One folder for each resource.

Each numbered resource folder contains the following files:
* conf.tf - The provider configuration. In most cases, this doesn't change.
* main.tf - The terraform instructions to run.
* expected_outputs.json - The Terraform outputs (using the `output` Terraform functionality).

- conf.tf - The provider configuration. In most cases, this doesn't change.
- main.tf - The terraform instructions to run.
- expected_outputs.json - The Terraform outputs (using the `output` Terraform functionality).

Note: if there is no expected output, set expected_outputs.json contents to `{}`.

Expand Down
12 changes: 6 additions & 6 deletions env0/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import (
"context"
"errors"
"fmt"
"log"

"github.com/env0/terraform-provider-env0/client"
"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)
Expand Down Expand Up @@ -75,13 +75,13 @@ func getCredentialsById(id string, prefixList []string, meta interface{}) (clien
return credentials, nil
}

func getCredentials(id string, prefixList []string, meta interface{}) (client.Credentials, error) {
func getCredentials(ctx context.Context, id string, prefixList []string, meta interface{}) (client.Credentials, error) {
_, err := uuid.Parse(id)
if err == nil {
log.Println("[INFO] Resolving credentials by id: ", id)
tflog.Info(ctx, "Resolving credentials by id", map[string]interface{}{"id": id})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Everything is now using tflog.
fmt.Printf has been deprecated.

return getCredentialsById(id, prefixList, meta)
} else {
log.Println("[INFO] Resolving credentials by name: ", id)
tflog.Info(ctx, "Resolving credentials by name", map[string]interface{}{"name": id})
return getCredentialsByName(id, prefixList, meta)
}
}
Expand All @@ -103,7 +103,7 @@ func resourceCredentialsRead(cloudType CloudType) schema.ReadContextFunc {

credentials, err := apiClient.CloudCredentials(d.Id())
if err != nil {
return ResourceGetFailure(string(cloudType)+" credentials", d, err)
return ResourceGetFailure(ctx, string(cloudType)+" credentials", d, err)
}

if err := writeResourceData(&credentials, d); err != nil {
Expand All @@ -116,7 +116,7 @@ func resourceCredentialsRead(cloudType CloudType) schema.ReadContextFunc {

func resourceCredentialsImport(cloudType CloudType) schema.StateContextFunc {
return func(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
credentials, err := getCredentials(d.Id(), credentialsTypeToPrefixList[cloudType], meta)
credentials, err := getCredentials(ctx, d.Id(), credentialsTypeToPrefixList[cloudType], meta)
if err != nil {
if _, ok := err.(*client.NotFoundError); ok {
return nil, fmt.Errorf(string(cloudType)+" credentials resource with id %v not found", d.Id())
Expand Down
2 changes: 1 addition & 1 deletion env0/data_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func dataEnvironmentRead(ctx context.Context, d *schema.ResourceData, meta inter
}
}

setEnvironmentSchema(d, environment, client.ConfigurationChanges{})
setEnvironmentSchema(ctx, d, environment, client.ConfigurationChanges{})

templateId := environment.LatestDeploymentLog.BlueprintId

Expand Down
7 changes: 4 additions & 3 deletions env0/errors.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package env0

import (
"log"
"context"

"github.com/env0/terraform-provider-env0/client"
"github.com/env0/terraform-provider-env0/client/http"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)
Expand All @@ -21,9 +22,9 @@ func driftDetected(err error) bool {
return false
}

func ResourceGetFailure(resourceName string, d *schema.ResourceData, err error) diag.Diagnostics {
func ResourceGetFailure(ctx context.Context, resourceName string, d *schema.ResourceData, err error) diag.Diagnostics {
if driftDetected(err) {
log.Printf("[WARN] Drift Detected: Terraform will remove %s from state", d.Id())
tflog.Warn(ctx, "Drift Detected: Terraform will remove id from state", map[string]interface{}{"id": d.Id()})
d.SetId("")
return nil
}
Expand Down
21 changes: 16 additions & 5 deletions env0/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ package env0

import (
"context"
"log"
"os"
"time"

"github.com/env0/terraform-provider-env0/client"
"github.com/env0/terraform-provider-env0/client/http"
"github.com/go-resty/resty/v2"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/plugin"
Expand Down Expand Up @@ -155,26 +155,37 @@ func configureProvider(version string, p *schema.Provider) schema.ConfigureConte
isIntegrationTest = true
}

restyClient.
subCtx := tflog.NewSubsystem(ctx, "env0_api_client")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

using a subcontext you can filter api_client logs by grep-ing "env0_api_client"


restyClient = restyClient.
SetRetryCount(5).
SetRetryWaitTime(time.Second).
SetRetryMaxWaitTime(time.Second * 5).
OnBeforeRequest(func(c *resty.Client, r *resty.Request) error {
tflog.SubsystemInfo(subCtx, "env0_api_client", "Sending request", map[string]interface{}{"method": r.Method, "url": r.URL})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

some more logs.

return nil
}).
OnAfterResponse(func(c *resty.Client, r *resty.Response) error {
tflog.SubsystemInfo(subCtx, "env0_api_client", "Received respose", map[string]interface{}{"method": r.Request.Method, "url": r.Request.URL, "status": r.Status()})
return nil
}).
AddRetryAfterErrorCondition().
Copy link
Collaborator Author

@TomerHeber TomerHeber Aug 15, 2023

Choose a reason for hiding this comment

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

This may have been needed to retry 4xx and 5xx.
Not sure... but worth checking it out if it helps (documentation isn't very clear).

Copy link
Collaborator Author

@TomerHeber TomerHeber Aug 15, 2023

Choose a reason for hiding this comment

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

looking in the code... it's just:

	c.AddRetryCondition(func(response *Response, err error) bool {
		return response.IsError()
	})
	return c
}

but we'll try...

AddRetryCondition(func(r *resty.Response, err error) bool {
if r == nil {
// No response. Possiblly a networking issue (E.g. DNS lookup failure).
log.Printf("[WARN] No response, retrying request: %s %s", r.Request.Method, r.Request.URL)
tflog.SubsystemWarn(subCtx, "env0_api_client", "No response, retrying request", map[string]interface{}{"method": r.Request.Method, "url": r.Request.URL})
return true
}

// When running integration tests 404 may occur due to "database eventual consistency".
// Retry when there's a 5xx error. Otherwise do not retry.
if r.StatusCode() >= 500 || (isIntegrationTest && r.StatusCode() == 404) {
log.Printf("[WARN] Received %d status code, retrying request: %s %s", r.StatusCode(), r.Request.Method, r.Request.URL)
tflog.SubsystemWarn(subCtx, "env0_api_client", "Received a failed or not found response, retrying request", map[string]interface{}{"method": r.Request.Method, "url": r.Request.URL, "status code": r.StatusCode()})
return true
}

if r.StatusCode() == 200 && isIntegrationTest && r.String() == "[]" {
log.Printf("[WARN] Received an empty list for an integration test, retrying request: %s %s", r.Request.Method, r.Request.URL)
tflog.SubsystemWarn(subCtx, "env0_api_client", "Received an empty list , retrying request", map[string]interface{}{"method": r.Request.Method, "url": r.Request.URL})
return true
}

Expand Down
4 changes: 2 additions & 2 deletions env0/resource_agent_project_assignment.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ package env0
import (
"context"
"fmt"
"log"
"strings"
"sync"

"github.com/env0/terraform-provider-env0/client"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)
Expand Down Expand Up @@ -112,7 +112,7 @@ func resourceAgentProjectAssignmentRead(ctx context.Context, d *schema.ResourceD
}

if assignment == nil {
log.Printf("[WARN] Drift Detected: Terraform will remove %s from state", d.Id())
tflog.Warn(ctx, "Drift Detected: Terraform will remove id from state", map[string]interface{}{"id": d.Id()})
d.SetId("")
return nil
}
Expand Down
12 changes: 6 additions & 6 deletions env0/resource_api_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ package env0
import (
"context"
"fmt"
"log"

"github.com/env0/terraform-provider-env0/client"
"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)
Expand Down Expand Up @@ -88,7 +88,7 @@ func resourceApiKeyRead(ctx context.Context, d *schema.ResourceData, meta interf
return diag.Errorf("could not get api key: %v", err)
}
if apiKey == nil {
log.Printf("[WARN] Drift Detected: Terraform will remove %s from state", d.Id())
tflog.Warn(ctx, "Drift Detected: Terraform will remove id from state", map[string]interface{}{"id": d.Id()})
d.SetId("")
return nil
}
Expand Down Expand Up @@ -153,19 +153,19 @@ func getApiKeyByName(name string, meta interface{}) (*client.ApiKey, error) {
return &foundApiKeys[0], nil
}

func getApiKey(id string, meta interface{}) (*client.ApiKey, error) {
func getApiKey(ctx context.Context, id string, meta interface{}) (*client.ApiKey, error) {
_, err := uuid.Parse(id)
if err == nil {
log.Println("[INFO] Resolving api key by id: ", id)
tflog.Info(ctx, "Resolving api key by id", map[string]interface{}{"id": id})
return getApiKeyById(id, meta)
} else {
log.Println("[INFO] Resolving api key by name: ", id)
tflog.Info(ctx, "Resolving api key by name", map[string]interface{}{"name": id})
return getApiKeyByName(id, meta)
}
}

func resourceApiKeyImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
apiKey, err := getApiKey(d.Id(), meta)
apiKey, err := getApiKey(ctx, d.Id(), meta)
if err != nil {
return nil, err
}
Expand Down
14 changes: 7 additions & 7 deletions env0/resource_approval_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ package env0
import (
"context"
"fmt"
"log"

"github.com/env0/terraform-provider-env0/client"
"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)
Expand Down Expand Up @@ -49,11 +49,11 @@ func resourceApprovalPolicyRead(ctx context.Context, d *schema.ResourceData, met

approvalPolicy, err := apiClient.Template(d.Id())
if err != nil {
return ResourceGetFailure("approval policy", d, err)
return ResourceGetFailure(ctx, "approval policy", d, err)
}

if approvalPolicy.IsDeleted && !d.IsNewResource() {
log.Printf("[WARN] Drift Detected: Terraform will remove %s from state", d.Id())
tflog.Warn(ctx, "Drift Detected: Terraform will remove id from state", map[string]interface{}{"id": d.Id()})
d.SetId("")
return nil
}
Expand Down Expand Up @@ -112,9 +112,9 @@ func getApprovalPolicyByName(name string, meta interface{}) (*client.ApprovalPol
return &approvalPolicies[0], nil
}

func getApprovalPolicy(id string, meta interface{}) (interface{}, error) {
func getApprovalPolicy(ctx context.Context, id string, meta interface{}) (interface{}, error) {
if _, err := uuid.Parse(id); err == nil {
log.Println("[INFO] Resolving approval policy by id: ", id)
tflog.Info(ctx, "Resolving approval policy by id", map[string]interface{}{"id": id})

template, err := meta.(client.ApiClientInterface).Template(id)
if err != nil {
Expand All @@ -127,14 +127,14 @@ func getApprovalPolicy(id string, meta interface{}) (interface{}, error) {

return &template, nil
} else {
log.Println("[INFO] Resolving approval policy by name: ", id)
tflog.Info(ctx, "Resolving approval policy by name", map[string]interface{}{"name": id})

return getApprovalPolicyByName(id, meta)
}
}

func resourceApprovalPolicyImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
approvalPolicy, err := getApprovalPolicy(d.Id(), meta)
approvalPolicy, err := getApprovalPolicy(ctx, d.Id(), meta)
if err != nil {
return nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions env0/resource_approval_policy_assignment.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ package env0
import (
"context"
"fmt"
"log"

"github.com/env0/terraform-provider-env0/client"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)
Expand Down Expand Up @@ -84,7 +84,7 @@ func resourceApprovalPolicyAssignmentRead(ctx context.Context, d *schema.Resourc

approvalPolicyByScopeArr, err := apiClient.ApprovalPolicyByScope(scope, scopeId)
if err != nil {
return ResourceGetFailure("approval policy assignment", d, err)
return ResourceGetFailure(ctx, "approval policy assignment", d, err)
}

found := false
Expand All @@ -96,7 +96,7 @@ func resourceApprovalPolicyAssignmentRead(ctx context.Context, d *schema.Resourc
}

if !found {
log.Printf("[WARN] Drift Detected: Terraform will remove %s from state", d.Id())
tflog.Warn(ctx, "Drift Detected: Terraform will remove id from state", map[string]interface{}{"id": d.Id()})
d.SetId("")
return nil
}
Expand Down
Loading