Skip to content

[gke] skip hard error when client misses permissions for some projects#51344

Merged
tigrato merged 2 commits intomasterfrom
tigrato/fix-gke-perm-errors
Jan 22, 2025
Merged

[gke] skip hard error when client misses permissions for some projects#51344
tigrato merged 2 commits intomasterfrom
tigrato/fix-gke-perm-errors

Conversation

@tigrato
Copy link
Copy Markdown
Contributor

@tigrato tigrato commented Jan 22, 2025

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

Changelog: Fixed a bug in GKE auto-discovery where the process failed to discover any clusters if the identity lacked permissions for one or more detected GCP project IDs.

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 <tiago.silva@goteleport.com>
Comment on lines +117 to +122
if trace.IsAccessDenied(err) {
a.Logger.WarnContext(ctx, "Access denied to list GKE clusters", "project_id", projectID, "location", a.Location)
return nil, nil
} else if err != nil {
return nil, trace.Wrap(err)
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this is a nicer way of doing this

Suggested change
if trace.IsAccessDenied(err) {
a.Logger.WarnContext(ctx, "Access denied to list GKE clusters", "project_id", projectID, "location", a.Location)
return nil, nil
} else if err != nil {
return nil, trace.Wrap(err)
}
if err != nil {
if trace.IsAccessDenied(err) {
a.Logger.WarnContext(ctx, "Access denied to list GKE clusters", "project_id", projectID, "location", a.Location)
return nil, nil
}
return nil, trace.Wrap(err)
}

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from eriktate January 22, 2025 17:35
@tigrato tigrato enabled auto-merge January 22, 2025 18:22
@tigrato tigrato added this pull request to the merge queue Jan 22, 2025
Merged via the queue into master with commit 32076b7 Jan 22, 2025
@tigrato tigrato deleted the tigrato/fix-gke-perm-errors branch January 22, 2025 19:49
@public-teleport-github-review-bot
Copy link
Copy Markdown

@tigrato See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Failed
branch/v17 Failed

tigrato added a commit that referenced this pull request Jan 23, 2025
#51344)

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 <tiago.silva@goteleport.com>
tigrato added a commit that referenced this pull request Jan 23, 2025
#51344)

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 <tiago.silva@goteleport.com>
tigrato added a commit that referenced this pull request Jan 23, 2025
#51344)

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 <tiago.silva@goteleport.com>
github-merge-queue Bot pushed a commit that referenced this pull request Jan 23, 2025
#51344) (#51400)

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 <tiago.silva@goteleport.com>
github-merge-queue Bot pushed a commit that referenced this pull request Jan 23, 2025
#51344) (#51399)

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 <tiago.silva@goteleport.com>
github-merge-queue Bot pushed a commit that referenced this pull request Jan 23, 2025
#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 <tiago.silva@goteleport.com>
carloscastrojumo pushed a commit to carloscastrojumo/teleport that referenced this pull request Feb 19, 2025
gravitational#51344)

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 gravitational#48101

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gracefully fail and continue discovery if GKE project is missing required wildcard project_ids permissions

3 participants