Skip to content

Commit

Permalink
Handle validation errors for 204 endpoints
Browse files Browse the repository at this point in the history
When the successful response to a client request is 204, it is possible
that we'll parse an error response into successful (empty) result and
not detect that it is an error.

This change adds the validation error handler we've used in other tools
such as the catalog-importer so that we detect errors properly. This was
a report from a customer who saw that Terraform trying to delete a
catalog type resulted in a validation error from our servers but the
catalog type was never deleted (because the endpoint had 422'd).
  • Loading branch information
lawrencejones committed Oct 28, 2024
1 parent 2182f3a commit 88dcc84
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 50 deletions.
121 changes: 121 additions & 0 deletions internal/client/client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
package client

import (
"context"
"fmt"
"io"
"net/http"
"time"

"github.com/deepmap/oapi-codegen/pkg/securityprovider"
"github.com/hashicorp/go-retryablehttp"
"github.com/pkg/errors"
)

func New(ctx context.Context, apiKey, apiEndpoint, version string, opts ...ClientOption) (*ClientWithResponses, error) {
bearerTokenProvider, bearerTokenProviderErr := securityprovider.NewSecurityProviderBearerToken(apiKey)
if bearerTokenProviderErr != nil {
return nil, bearerTokenProviderErr
}

retryClient := retryablehttp.NewClient()
retryClient.RetryMax = maxRetries
retryClient.Backoff = attentiveBackoff

base := retryClient.StandardClient()

// The generated client won't turn validation errors into actual errors, so we do this
// inside of a generic middleware.
base.Transport = Wrap(base.Transport, func(req *http.Request, next http.RoundTripper) (*http.Response, error) {
resp, err := next.RoundTrip(req)
if err != nil {
return nil, err
}
if err == nil && resp.StatusCode > 299 {
data, err := io.ReadAll(resp.Body)
if err != nil {
return nil, fmt.Errorf("status %d: no response body", resp.StatusCode)
}

return nil, fmt.Errorf("status %d: %s", resp.StatusCode, string(data))
}

return resp, err
})

clientOpts := append([]ClientOption{
WithHTTPClient(base),
WithRequestEditorFn(bearerTokenProvider.Intercept),
// Add a user-agent so we can tell which version these requests came from.
WithRequestEditorFn(func(ctx context.Context, req *http.Request) error {
req.Header.Add("user-agent", fmt.Sprintf("terraform-provider-incident/%s", version))
return nil
}),
}, opts...)

client, err := NewClientWithResponses(apiEndpoint, clientOpts...)
if err != nil {
return nil, errors.Wrap(err, "creating client")
}

return client, nil
}

const (
maxRetries = 10
)

func attentiveBackoff(minDuration, maxDuration time.Duration, attemptNum int, resp *http.Response) time.Duration {
// Retry for rate limits and server errors.
if resp != nil && resp.StatusCode == http.StatusTooManyRequests {
// Check for a 'Retry-After' header.
retryAfter := resp.Header.Get("Retry-After")
if retryAfter != "" {
retryAfterDate, err := time.Parse(time.RFC1123, retryAfter)
if err != nil {
// If we can't parse the Retry-After, lets just wait for 10 seconds
return 10
}

timeToWait := time.Until(retryAfterDate)

if timeToWait < 1*time.Second {
// by default lets back off at least 1 second
return 1 * time.Second
}

return timeToWait
}

}
// otherwise use the default backoff
return retryablehttp.DefaultBackoff(minDuration, maxDuration, attemptNum, resp)
}

// WithReadOnly restricts the client to GET requests only, useful when creating a client
// for the purpose of dry-running.
func WithReadOnly() ClientOption {
return WithRequestEditorFn(func(ctx context.Context, req *http.Request) error {
if req.Method != http.MethodGet {
return fmt.Errorf("read-only client tried to make mutating request: %s %s", req.Method, req.URL.String())
}

return nil
})
}

// RoundTripperFunc wraps a function to implement the RoundTripper interface, allowing
// easy wrapping of existing round-trippers.
type RoundTripperFunc func(req *http.Request) (*http.Response, error)

func (f RoundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) {
return f(req)
}

// Wrap allows easy wrapping of an existing RoundTripper with a function that can
// optionally call the original, or do its own thing.
func Wrap(next http.RoundTripper, apply func(req *http.Request, next http.RoundTripper) (*http.Response, error)) http.RoundTripper {
return RoundTripperFunc(func(req *http.Request) (*http.Response, error) {
return apply(req, next)
})
}
53 changes: 3 additions & 50 deletions internal/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,16 @@ package provider

import (
"context"
"fmt"
"net/http"
"os"
"time"

_ "embed"

"github.com/deepmap/oapi-codegen/pkg/securityprovider"
"github.com/hashicorp/go-cleanhttp"
"github.com/hashicorp/go-retryablehttp"
"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/hashicorp/terraform-plugin-framework/provider"
"github.com/hashicorp/terraform-plugin-framework/provider/schema"
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/incident-io/terraform-provider-incident/internal/client"
"github.com/motemen/go-loghttp"
)

var _ provider.Provider = &IncidentProvider{}
Expand Down Expand Up @@ -101,57 +94,17 @@ func (p *IncidentProvider) Configure(ctx context.Context, req provider.Configure
apiKey = data.APIKey.ValueString()
}

bearerTokenProvider, bearerTokenProviderErr := securityprovider.NewSecurityProviderBearerToken(apiKey)
if bearerTokenProviderErr != nil {
panic(bearerTokenProviderErr)
}

base := retryablehttp.NewClient()
base.RetryMax = 10
base.Backoff = func(minimum, maximum time.Duration, attemptNum int, httpResp *http.Response) time.Duration {
// Retry for rate limits and server errors.
if httpResp != nil && httpResp.StatusCode == http.StatusTooManyRequests {
retryAfter := httpResp.Header.Get("Retry-After")
if retryAfter != "" {
retryAfterDate, err := time.Parse(time.RFC1123, retryAfter)
if err != nil {
// If we can't parse the Retry-After, lets just wait for 10 seconds
return 10
}

timeToWait := time.Until(retryAfterDate)

return timeToWait
}
}
// Fallback to the default backoff
return retryablehttp.DefaultBackoff(minimum, maximum, attemptNum, httpResp)
}

base.HTTPClient.Transport = &loghttp.Transport{
Transport: cleanhttp.DefaultTransport(),
}

client, err := client.NewClientWithResponses(
endpoint,
client.WithHTTPClient(base.StandardClient()),
client.WithRequestEditorFn(bearerTokenProvider.Intercept),
// Add a user-agent so we can tell which version these requests came from.
client.WithRequestEditorFn(func(ctx context.Context, req *http.Request) error {
req.Header.Add("user-agent", fmt.Sprintf("terraform-provider-incident/%s", p.version))
return nil
}),
)
c, err := client.New(ctx, apiKey, endpoint, p.version)
if err != nil {
panic(err)
}

resp.DataSourceData = &IncidentProviderData{
Client: client,
Client: c,
TerraformVersion: req.TerraformVersion,
}
resp.ResourceData = &IncidentProviderData{
Client: client,
Client: c,
TerraformVersion: req.TerraformVersion,
}
}
Expand Down

0 comments on commit 88dcc84

Please sign in to comment.