Skip to content

Commit

Permalink
Handle *url.Error + improve logging
Browse files Browse the repository at this point in the history
  • Loading branch information
atburke committed Jun 4, 2024
1 parent c316482 commit 6ba6b9a
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 5 deletions.
16 changes: 16 additions & 0 deletions lib/cloud/gcp/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ import (
"fmt"
"io"
"net"
"net/url"
"regexp"
"slices"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -55,6 +58,10 @@ const sshUser = "teleport"
// sshDefaultTimeout is the default timeout for dialing an instance.
const sshDefaultTimeout = 10 * time.Second

// urlErrorCodePattern is a regex for attempting to extract an HTTP error
// code from a *url.Error.
var urlErrorCodePattern = regexp.MustCompile(`\b[2-5]\d{2}\b`)

// convertAPIError converts an error from the GCP API into a trace error.
func convertAPIError(err error) error {
var googleError *googleapi.Error
Expand All @@ -69,6 +76,15 @@ func convertAPIError(err error) error {
return trail.FromGRPC(apiError)
}
}
var urlError *url.Error
if errors.As(err, &urlError) {
if codeStr := urlErrorCodePattern.FindString(urlError.Error()); codeStr != "" {
code, err := strconv.Atoi(codeStr)
if err == nil {
return trace.ReadError(code, []byte(urlError.Error()))
}
}
}
return err
}

Expand Down
13 changes: 12 additions & 1 deletion lib/cloud/gcp/vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ package gcp
import (
"bytes"
"context"
"fmt"
"net"
"net/http"
"net/url"
"sync/atomic"
"testing"
"time"
Expand Down Expand Up @@ -70,10 +72,19 @@ func TestConvertAPIError(t *testing.T) {
name: "api grpc error",
err: fromError(t, status.Error(codes.PermissionDenied, "abcd1234")),
},
{
name: "url error",
err: &url.Error{
Op: http.MethodGet,
URL: "https://example.com",
Err: fmt.Errorf("wrong code 99, wrong code 2007, wrong code 678, right code 403, wrong code 401 abcd1234"),
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
require.True(t, trace.IsAccessDenied(convertAPIError(tc.err)))
err := convertAPIError(tc.err)
require.True(t, trace.IsAccessDenied(err), "unexpected error of type %T: %v", tc.err, err)
require.Contains(t, tc.err.Error(), "abcd1234")
})
}
Expand Down
12 changes: 8 additions & 4 deletions lib/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -967,8 +967,12 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) {
imClient := cfg.InstanceMetadataClient
if imClient == nil {
imClient, err = cloud.DiscoverInstanceMetadata(supervisor.ExitContext())
if err != nil && !trace.IsNotFound(err) {
return nil, trace.Wrap(err)
if err == nil {
cfg.Logger.InfoContext(supervisor.ExitContext(),
"Found an instance metadata service. Teleport will import labels from this cloud instance.",
"type", imClient.GetType())
} else if !trace.IsNotFound(err) {
cfg.Logger.ErrorContext(supervisor.ExitContext(), "Error looking for cloud instance metadata.", "error", err)
}
}

Expand All @@ -985,15 +989,15 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) {
cfg.Logger.InfoContext(supervisor.ExitContext(), "Found invalid hostname in cloud tag TeleportHostname.", "hostname", cloudHostname)
}
} else if !trace.IsNotFound(err) {
return nil, trace.Wrap(err)
cfg.Logger.ErrorContext(supervisor.ExitContext(), "Error looking for hostname tag.", "error", err)
}

cloudLabels, err = labels.NewCloudImporter(supervisor.ExitContext(), &labels.CloudConfig{
Client: imClient,
Clock: cfg.Clock,
})
if err != nil {
return nil, trace.Wrap(err)
cfg.Logger.ErrorContext(supervisor.ExitContext(), "Error creating cloud label importer.", "error", err)
}
}

Expand Down

0 comments on commit 6ba6b9a

Please sign in to comment.