Skip to content

Commit

Permalink
[gke] skip hard error when client misses permissions for some projects (
Browse files Browse the repository at this point in the history
#51344) (#51401)

This PR fixes a behavior problem that results in GKE discovery to
completely fail when the client misses permissions in some of the
discovered projects.

If the client misses list permissions for projectID 1 but has the
required permissions for projectID 2, the discovery service should
continue with the projectID 2 discovery and skip the failing one.

Fixes #48101

Signed-off-by: Tiago Silva <[email protected]>
  • Loading branch information
tigrato authored Jan 23, 2025
1 parent f55a669 commit 6fb0031
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 4 deletions.
4 changes: 2 additions & 2 deletions lib/cloud/gcp/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func (g *gkeClient) ListClusters(ctx context.Context, projectID string, location
},
)
if err != nil {
return nil, trace.Wrap(err)
return nil, trace.Wrap(convertAPIError(err))
}
var clusters []GKECluster
for _, cluster := range res.Clusters {
Expand Down Expand Up @@ -226,7 +226,7 @@ func (g *gkeClient) GetClusterRestConfig(ctx context.Context, cfg ClusterDetails
},
)
if err != nil {
return nil, time.Time{}, trace.Wrap(err)
return nil, time.Time{}, trace.Wrap(convertAPIError(err))
}

// Generate a SA Authentication Token.
Expand Down
9 changes: 9 additions & 0 deletions lib/srv/discovery/fetchers/gke.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,15 @@ func (a *gkeFetcher) getGKEClusters(ctx context.Context, projectID string) (type
var clusters types.KubeClusters

gkeClusters, err := a.GKEClient.ListClusters(ctx, projectID, a.Location)
if trace.IsAccessDenied(err) {
a.Log.WithFields(logrus.Fields{
"project_id": projectID,
"location": a.Location,
}).Warn("Access denied to list GKE clusters")
return nil, nil
} else if err != nil {
return nil, trace.Wrap(err)
}
for _, gkeCluster := range gkeClusters {
cluster, err := a.getMatchingKubeCluster(gkeCluster)
// trace.CompareFailed is returned if the cluster did not match the matcher filtering labels
Expand Down
26 changes: 24 additions & 2 deletions lib/srv/discovery/fetchers/gke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"testing"

containerpb "cloud.google.com/go/container/apiv1/containerpb"
"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"

Expand All @@ -36,6 +37,7 @@ func TestGKEFetcher(t *testing.T) {
location string
filterLabels types.Labels
projectID string
errs map[string]error
}
tests := []struct {
name string
Expand Down Expand Up @@ -110,11 +112,26 @@ func TestGKEFetcher(t *testing.T) {
},
want: gkeClustersToResources(t, gkeMockClusters...),
},
{
name: "list everything but one project misses permissions",
args: args{
location: "uswest2",
filterLabels: types.Labels{
"env": []string{"prod", "stg"},
},
projectID: "*",
errs: map[string]error{
"p2": trace.AccessDenied("no access"),
},
},
want: gkeClustersToResources(t, gkeMockClusters[:4]...),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := newPopulatedGCPMock(tt.args.errs)
cfg := GKEFetcherConfig{
GKEClient: newPopulatedGCPMock(),
GKEClient: client,
ProjectClient: newPopulatedGCPProjectsMock(),
FilterLabels: tt.args.filterLabels,
Location: tt.args.location,
Expand All @@ -134,9 +151,13 @@ func TestGKEFetcher(t *testing.T) {
type mockGKEAPI struct {
gcp.GKEClient
clusters []gcp.GKECluster
errs map[string]error
}

func (m *mockGKEAPI) ListClusters(ctx context.Context, projectID string, location string) ([]gcp.GKECluster, error) {
if err, ok := m.errs[projectID]; ok {
return nil, err
}
var clusters []gcp.GKECluster
for _, cluster := range m.clusters {
if cluster.ProjectID != projectID {
Expand All @@ -148,9 +169,10 @@ func (m *mockGKEAPI) ListClusters(ctx context.Context, projectID string, locatio
return clusters, nil
}

func newPopulatedGCPMock() *mockGKEAPI {
func newPopulatedGCPMock(errs map[string]error) *mockGKEAPI {
return &mockGKEAPI{
clusters: gkeMockClusters,
errs: errs,
}
}

Expand Down

0 comments on commit 6fb0031

Please sign in to comment.