diff --git a/lib/cloud/gcp/vm_test.go b/lib/cloud/gcp/vm_test.go index 77cc248f84ba7..8bfbecd08d994 100644 --- a/lib/cloud/gcp/vm_test.go +++ b/lib/cloud/gcp/vm_test.go @@ -73,7 +73,8 @@ func TestConvertAPIError(t *testing.T) { } 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") }) } diff --git a/lib/cloud/imds/gcp/imds.go b/lib/cloud/imds/gcp/imds.go index 0830ab3938a9c..3d80892e97b07 100644 --- a/lib/cloud/imds/gcp/imds.go +++ b/lib/cloud/imds/gcp/imds.go @@ -76,8 +76,22 @@ func NewInstanceMetadataClient(ctx context.Context) (*InstanceMetadataClient, er // IsAvailable checks if instance metadata is available. func (client *InstanceMetadataClient) IsAvailable(ctx context.Context) bool { - instanceData, err := client.getMetadata(ctx, "instance") - return err == nil && instanceData != "" + id, err := client.getNumericID(ctx) + return err == nil && id != 0 +} + +func (client *InstanceMetadataClient) getNumericID(ctx context.Context) (uint64, error) { + idStr, err := client.GetID(ctx) + if err != nil { + return 0, trace.Wrap(err) + } + id, err := strconv.ParseUint(idStr, 10, 64) + if err != nil { + // Shouldn't ever happen on GCP, but at least one other cloud provider (Yandex) + // implements this metadata API and doesn't have numeric IDs. + return 0, trace.Wrap(err) + } + return id, nil } // GetTags gets all of the GCP instance's labels (note: these are separate from @@ -96,11 +110,7 @@ func (client *InstanceMetadataClient) GetTags(ctx context.Context) (map[string]s if err != nil { return nil, trace.Wrap(err) } - idStr, err := client.GetID(ctx) - if err != nil { - return nil, trace.Wrap(err) - } - id, err := strconv.ParseUint(idStr, 10, 64) + id, err := client.getNumericID(ctx) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/cloud/imds/gcp/imds_test.go b/lib/cloud/imds/gcp/imds_test.go index 10ee2c1a2f5f6..c1bcee76db186 100644 --- a/lib/cloud/imds/gcp/imds_test.go +++ b/lib/cloud/imds/gcp/imds_test.go @@ -58,16 +58,44 @@ func (m *mockInstanceGetter) GetInstanceTags(ctx context.Context, req *gcp.Insta func TestIsInstanceMetadataAvailable(t *testing.T) { t.Parallel() - t.Run("not available", func(t *testing.T) { - client := &InstanceMetadataClient{ + tests := []struct { + name string + getMetadata metadataGetter + assert require.BoolAssertionFunc + }{ + { + name: "not available", getMetadata: func(ctx context.Context, path string) (string, error) { return "", trace.NotFound("") }, - } - require.False(t, client.IsAvailable(context.Background())) - }) + assert: require.False, + }, + { + name: "not on gcp", + getMetadata: func(ctx context.Context, path string) (string, error) { + return "non-numeric id", nil + }, + assert: require.False, + }, + { + name: "on mocked gcp", + getMetadata: func(ctx context.Context, path string) (string, error) { + return "12345678", nil + }, + assert: require.True, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := &InstanceMetadataClient{ + getMetadata: tc.getMetadata, + } + tc.assert(t, client.IsAvailable(context.Background())) + }) + } - t.Run("on gcp", func(t *testing.T) { + t.Run("on real gcp", func(t *testing.T) { if os.Getenv("TELEPORT_TEST_GCP") == "" { t.Skip("not on gcp") } diff --git a/lib/service/service.go b/lib/service/service.go index 69eaf1452a1a4..8076d92650822 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -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) } } @@ -985,7 +989,7 @@ 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{ @@ -993,7 +997,7 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) { Clock: cfg.Clock, }) if err != nil { - return nil, trace.Wrap(err) + cfg.Logger.ErrorContext(supervisor.ExitContext(), "Cloud labels will not be imported.", "error", err) } }