From 5d35e156764b10a666d3943668dd65f97875c58c Mon Sep 17 00:00:00 2001 From: Oscar Hermoso Date: Sat, 12 Oct 2024 09:40:52 +0800 Subject: [PATCH] =?UTF-8?q?=E2=AD=95=20retry=20on=20failed=20create=20of?= =?UTF-8?q?=20server,=20load=20balancer,=20and=20SSH=20key?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .env.example | 6 ++ internal/binarylane/client.go | 49 ++++++++++++++++ internal/provider/load_balancer_resource.go | 56 ++++++++++++------ internal/provider/provider.go | 64 +++++---------------- internal/provider/server_resource.go | 47 ++++++++++++--- internal/provider/ssh_key_resource.go | 56 +++++++++++++----- 6 files changed, 188 insertions(+), 90 deletions(-) create mode 100644 internal/binarylane/client.go diff --git a/.env.example b/.env.example index bb3aaa8..eead8e0 100644 --- a/.env.example +++ b/.env.example @@ -1,2 +1,8 @@ TF_ACC=1 BINARYLANE_API_TOKEN= + +# Uncomment to enable debug logging (will dump request bodies) +# TF_LOG=DEBUG + +# Uncomment to disable the Go cache when testing, useful for flaky tests +# GOCACHE=off diff --git a/internal/binarylane/client.go b/internal/binarylane/client.go new file mode 100644 index 0000000..0c60855 --- /dev/null +++ b/internal/binarylane/client.go @@ -0,0 +1,49 @@ +package binarylane + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httputil" + + "github.com/deepmap/oapi-codegen/pkg/securityprovider" + "github.com/hashicorp/terraform-plugin-log/tflog" +) + +func NewClientWithAuth(endpoint string, token string) (*ClientWithResponses, error) { + if token == "" { + return nil, errors.New("missing or empty value for the Binary Lane API " + + "token. Set the `api_token` value in the configuration or use the " + + "BINARYLANE_API_TOKEN environment variable. If either is already set, " + + "ensure the value is not empty") + } + + auth, err := securityprovider.NewSecurityProviderBearerToken(token) + if err != nil { + return nil, fmt.Errorf("failed to create API client with supplied API token: %w", err) + } + + if endpoint == "" { + endpoint = "https://api.binarylane.com.au/v2" + } + + client, err := NewClientWithResponses( + endpoint, + WithRequestEditorFn(func(ctx context.Context, req *http.Request) error { + dump, err := httputil.DumpRequestOut(req, true) + if err != nil { + return err + } + tflog.Debug(ctx, fmt.Sprintf("%q\n", dump)) + return nil + }), + WithRequestEditorFn(auth.Intercept), // include auth AFTER the request logger + ) + + if err != nil { + return nil, fmt.Errorf("failed to create Binary Lane API client: %w", err) + } + + return client, nil +} diff --git a/internal/provider/load_balancer_resource.go b/internal/provider/load_balancer_resource.go index 61056bd..1898617 100644 --- a/internal/provider/load_balancer_resource.go +++ b/internal/provider/load_balancer_resource.go @@ -128,27 +128,49 @@ func (r *loadBalancerResource) Create(ctx context.Context, req resource.CreateRe tflog.Info(ctx, fmt.Sprintf("Creating Load Balancer: name=%s", data.Name.ValueString())) - lbResp, err := r.bc.client.PostLoadBalancersWithResponse(ctx, body) - if err != nil { - tflog.Info(ctx, fmt.Sprintf("Attempted to create new load balancer: request=%+v", body)) - resp.Diagnostics.AddError( - fmt.Sprintf("Error sending request to create load balancer: name=%s", data.Name.ValueString()), - err.Error(), - ) - return + const maxRetries = 3 + var lbResp binarylane.PostLoadBalancersResponse + +retryLoop: + for i := 0; i < maxRetries; i++ { + lbResp, err := r.bc.client.PostLoadBalancersWithResponse(ctx, body) + if err != nil { + tflog.Info(ctx, fmt.Sprintf("Attempted to create new load balancer: request=%+v", body)) + resp.Diagnostics.AddError( + fmt.Sprintf("Error sending request to create load balancer: name=%s", data.Name.ValueString()), + err.Error(), + ) + return + } + + switch lbResp.StatusCode() { + + case http.StatusOK: + break retryLoop + + case http.StatusInternalServerError: + if i < maxRetries-1 { + tflog.Warn(ctx, "Received 500 creating load balancer, retrying...") + time.Sleep(time.Second * 5) + continue + } + + default: + tflog.Info(ctx, fmt.Sprintf("Attempted to create new load balancer: request=%+v", body)) + resp.Diagnostics.AddError( + "Unexpected HTTP status code creating load balancer", + fmt.Sprintf("Received %s creating new load balancer: name=%s. Details: %s", lbResp.Status(), data.Name.ValueString(), lbResp.Body), + ) + return + } } + // Check if retries exceeded if lbResp.StatusCode() != http.StatusOK { - tflog.Info(ctx, fmt.Sprintf("Attempted to create new load balancer: request=%+v", body)) resp.Diagnostics.AddError( - "Unexpected HTTP status code creating load balancer", - fmt.Sprintf("Received %s creating new load balancer: name=%s. Details: %s", lbResp.Status(), data.Name.ValueString(), lbResp.Body)) - return - } - - diags = SetLoadBalancerModelState(ctx, &data.LoadBalancerModel, lbResp.JSON200.LoadBalancer) - resp.Diagnostics.Append(diags...) - if diags.HasError() { + "Failed to create load balancer after retries", + fmt.Sprintf("Final status code: %d", lbResp.StatusCode()), + ) return } diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 45e9f91..3ea8da4 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -5,9 +5,7 @@ import ( "os" "terraform-provider-binarylane/internal/binarylane" - "github.com/deepmap/oapi-codegen/pkg/securityprovider" "github.com/hashicorp/terraform-plugin-framework/datasource" - "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/hashicorp/terraform-plugin-framework/provider/schema" "github.com/hashicorp/terraform-plugin-framework/resource" @@ -67,60 +65,24 @@ func (p *binarylaneProvider) Configure(ctx context.Context, req provider.Configu return } - if config.Endpoint.IsUnknown() { - resp.Diagnostics.AddAttributeError( - path.Root("api_endpoint"), - "Unknown Binary Lane API endpoint", - "The provider cannot create the Binary Lane API client as there is an unknown configuration value for the "+ - "Binary Lane API endpoint. Either target apply the source of the value first, set the value statically in "+ - "the configuration, or use the BINARYLANE_API_ENDPOINT environment variable.", - ) - } - if config.Token.IsUnknown() { - resp.Diagnostics.AddAttributeError( - path.Root("api_token"), - "Unknown Binary Lane token", - "The provider cannot create the Binary Lane API client as there is an unknown configuration value for the "+ - "Binary Lane API token. Either target apply the source of the value first, set the value statically in the "+ - "configuration, or use the BINARYLANE_API_TOKEN environment variable.", - ) - } - if resp.Diagnostics.HasError() { - return - } - - // Default values to environment variables, but override - // with Terraform configuration value if set. - endpoint := os.Getenv("BINARYLANE_API_ENDPOINT") - token := os.Getenv("BINARYLANE_API_TOKEN") - if !config.Endpoint.IsNull() { - endpoint = config.Endpoint.ValueString() - } - if !config.Token.IsNull() { - token = config.Token.ValueString() - } + endpoint := config.Endpoint.ValueString() if endpoint == "" { - endpoint = "https://api.binarylane.com.au/v2" + endpoint = os.Getenv("BINARYLANE_API_ENDPOINT") + if endpoint == "" { + endpoint = "https://api.binarylane.com.au/v2" + } } + + token := config.Token.ValueString() if token == "" { - resp.Diagnostics.AddAttributeError( - path.Root("api_token"), - "Missing Binary Lane API token", - "The provider cannot create the Binary Lane API client as there is a missing or empty value for the Binary "+ - "Lane API token. Set the token value in the configuration or use the BINARYLANE_API_TOKEN environment "+ - "variable. If either is already set, ensure the value is not empty.", - ) - } - if resp.Diagnostics.HasError() { - return - } - auth, err := securityprovider.NewSecurityProviderBearerToken(token) - if err != nil { - resp.Diagnostics.AddError("Failed to create security provider with supplied token", err.Error()) - return + token = os.Getenv("BINARYLANE_API_TOKEN") } - client, err := binarylane.NewClientWithResponses(endpoint, binarylane.WithRequestEditorFn(auth.Intercept)) + client, err := binarylane.NewClientWithAuth( + endpoint, + token, + ) + if err != nil { resp.Diagnostics.AddError("Failed to create Binary Lane API client", err.Error()) return diff --git a/internal/provider/server_resource.go b/internal/provider/server_resource.go index 310f1bc..c125068 100644 --- a/internal/provider/server_resource.go +++ b/internal/provider/server_resource.go @@ -255,21 +255,50 @@ func (r *serverResource) Create(ctx context.Context, req resource.CreateRequest, data.Password = types.StringNull() } else { body.Password = data.Password.ValueStringPointer() + ctx = tflog.MaskMessageStrings(ctx, data.Password.String()) } - serverResp, err := r.bc.client.PostServersWithResponse(ctx, body) - if err != nil { - resp.Diagnostics.AddError( - fmt.Sprintf("Error creating server: name=%s", data.Name.ValueString()), - err.Error(), - ) - return + const maxRetries = 3 + var serverResp binarylane.PostServersResponse + +retryLoop: + for i := 0; i < maxRetries; i++ { + serverResp, err := r.bc.client.PostServersWithResponse(ctx, body) + if err != nil { + resp.Diagnostics.AddError( + fmt.Sprintf("Error creating server: name=%s", data.Name.ValueString()), + err.Error(), + ) + return + } + + switch serverResp.StatusCode() { + + case http.StatusOK: + break retryLoop + + case http.StatusInternalServerError: + if i < maxRetries-1 { + tflog.Warn(ctx, "Received 500 creating server, retrying...") + time.Sleep(time.Second * 5) + continue + } + + default: + resp.Diagnostics.AddError( + "Unexpected HTTP status code creating server", + fmt.Sprintf("Received %s creating new server: name=%s. Details: %s", serverResp.Status(), data.Name.ValueString(), serverResp.Body), + ) + return + } } + // Check if retries exceeded if serverResp.StatusCode() != http.StatusOK { resp.Diagnostics.AddError( - "Unexpected HTTP status code creating server", - fmt.Sprintf("Received %s creating new server: name=%s. Details: %s", serverResp.Status(), data.Name.ValueString(), serverResp.Body)) + "Failed to create server after retries", + fmt.Sprintf("Final status code: %d", serverResp.StatusCode()), + ) return } diff --git a/internal/provider/ssh_key_resource.go b/internal/provider/ssh_key_resource.go index 9e87735..5030dad 100644 --- a/internal/provider/ssh_key_resource.go +++ b/internal/provider/ssh_key_resource.go @@ -7,6 +7,7 @@ import ( "strconv" "terraform-provider-binarylane/internal/binarylane" "terraform-provider-binarylane/internal/resources" + "time" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" @@ -15,6 +16,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-log/tflog" ) // Ensure the implementation satisfies the expected interfaces. @@ -103,22 +105,50 @@ func (r *sshKeyResource) Create(ctx context.Context, req resource.CreateRequest, } // Create API call logic - sshResp, err := r.bc.client.PostAccountKeysWithResponse(ctx, binarylane.SshKeyRequest{ - Name: data.Name.ValueString(), - Default: data.Default.ValueBoolPointer(), - PublicKey: data.PublicKey.ValueString(), - }) - if err != nil { - resp.Diagnostics.AddError( - fmt.Sprintf("Error creating SSH Key: name=%s", data.Name.ValueString()), - err.Error(), - ) - return + const maxRetries = 3 + var sshResp binarylane.PostAccountKeysResponse + +retryLoop: + for i := 0; i < maxRetries; i++ { + sshResp, err := r.bc.client.PostAccountKeysWithResponse(ctx, binarylane.SshKeyRequest{ + Name: data.Name.ValueString(), + Default: data.Default.ValueBoolPointer(), + PublicKey: data.PublicKey.ValueString(), + }) + if err != nil { + resp.Diagnostics.AddError( + fmt.Sprintf("Error creating SSH Key: name=%s", data.Name.ValueString()), + err.Error(), + ) + return + } + + switch sshResp.StatusCode() { + + case http.StatusOK: + break retryLoop + + case http.StatusInternalServerError: + if i < maxRetries-1 { + tflog.Warn(ctx, "Received 500 creating SSH key, retrying...") + time.Sleep(time.Second * 5) + continue + } + + default: + resp.Diagnostics.AddError( + "Unexpected HTTP status code creating SSH key", + fmt.Sprintf("Received %s creating SSH key: name=%s. Details: %s", sshResp.Status(), data.Name.ValueString(), sshResp.Body), + ) + return + } } + + // Check if retries exceeded if sshResp.StatusCode() != http.StatusOK { resp.Diagnostics.AddError( - "Unexpected HTTP status code creating new SSH key", - fmt.Sprintf("Received %s creating new SSH key: name=%s. Details: %s", sshResp.Status(), data.Name.ValueString(), sshResp.Body), + "Failed to create SSH key after retries", + fmt.Sprintf("Final status code: %d", sshResp.StatusCode()), ) return }