Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[buddy] Discovery service panics on GKE clusters without labels #30643

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

tigrato
Copy link
Contributor

@tigrato tigrato commented Aug 17, 2023

Fixes #30619.

For a GKE cluster without labels like

$ gcloud container clusters list --format='json(status,resourceLabels)'
[
  {
    "status": "RUNNING"
  }
]

, Teleport with discovery_service enabled for GKE

discovery_service:
  enabled: "yes"
  discovery_group: "main"
  gcp:
    - types: ["gke"]
      locations: ["*"]
      project_ids: ["some-project-id-123456789"]
      tags:
        "*" : "*"

panics with the following output in its log

panic: assignment to entry in nil map
goroutine 363 [running]:
github.com/gravitational/teleport/lib/services.labelsFromGCPKubeCluster(...)
        github.com/gravitational/teleport/lib/services/kubernetes.go:260
github.com/gravitational/teleport/lib/services.NewKubeClusterFromGCPGKE({{0xc0024572b8, 0x4}, {0x0, 0x0}, {0xc002457560, 0xc}, {0xc00159a198, 0x17}, 0x2, 0x0})
        github.com/gravitational/teleport/lib/services/kubernetes.go:235 +0x1dc
github.com/gravitational/teleport/lib/srv/discovery/fetchers.(*gkeFetcher).getMatchingKubeCluster(0xc001580190, {{0xc0024572b8, 0x4}, {0x0, 0x0}, {0xc002457560, 0xc}, {0xc00159a198, 0x17}, 0x2, ...})
        github.com/gravitational/teleport/lib/srv/discovery/fetchers/gke.go:137 +0x58
github.com/gravitational/teleport/lib/srv/discovery/fetchers.(*gkeFetcher).getGKEClusters(0xc001580190, {0x98466c0?, 0xc0015806e0?})
        github.com/gravitational/teleport/lib/srv/discovery/fetchers/gke.go:96 +0x145
github.com/gravitational/teleport/lib/srv/discovery/fetchers.(*gkeFetcher).Get(0x0?, {0x98466c0?, 0xc0015806e0?})
        github.com/gravitational/teleport/lib/srv/discovery/fetchers/gke.go:82 +0x3b
github.com/gravitational/teleport/lib/srv/discovery/common.(*Watcher).fetchAndSend.func1()
        github.com/gravitational/teleport/lib/srv/discovery/common/watcher.go:129 +0x70
golang.org/x/sync/errgroup.(*Group).Go.func1()
        golang.org/x/sync@v0.3.0/errgroup/errgroup.go:75 +0x64
created by golang.org/x/sync/errgroup.(*Group).Go
        golang.org/x/sync@v0.3.0/errgroup/errgroup.go:72 +0xa5

The reason for that is because

  1. the GCP client sets Labels equal to nil for a GKE cluster without labels.
  2. the Clone() function in exp/maps returns nil when a nil is
    passed as an argument; see [0].

Fixed that by creating an empty labels map and then populating it using Copy().

Co-authored-by: @tigrato

[0] https://go-review.googlesource.com/c/exp/+/417274

buddies: #30618

Fix Teleport's `discovery_service` panic for a GKE cluster without labels.

Co-authored-by: Tiago Silva <tfsantossilva93@gmail.com>
Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
@tigrato tigrato added this pull request to the merge queue Aug 17, 2023
Merged via the queue into master with commit e869ebd Aug 17, 2023
23 checks passed
@tigrato tigrato deleted the tigrato/pr-buddy-30618 branch August 17, 2023 21:34
@public-teleport-github-review-bot

@tigrato See the table below for backport results.

Branch Result
branch/v11 Failed
branch/v12 Failed
branch/v13 Failed

tigrato added a commit that referenced this pull request Aug 17, 2023
Fix Teleport's `discovery_service` panic for a GKE cluster without labels.

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Co-authored-by: Valeriy Khromov <valery.khromov@gmail.com>
tigrato added a commit that referenced this pull request Aug 17, 2023
Fix Teleport's `discovery_service` panic for a GKE cluster without labels.

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Co-authored-by: Valeriy Khromov <valery.khromov@gmail.com>
tigrato added a commit that referenced this pull request Aug 17, 2023
Fix Teleport's `discovery_service` panic for a GKE cluster without labels.

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Co-authored-by: Valeriy Khromov <valery.khromov@gmail.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 17, 2023
)

Fix Teleport's `discovery_service` panic for a GKE cluster without labels.

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Co-authored-by: Valeriy Khromov <valery.khromov@gmail.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 17, 2023
)

Fix Teleport's `discovery_service` panic for a GKE cluster without labels.

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Co-authored-by: Valeriy Khromov <valery.khromov@gmail.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 18, 2023
)

Fix Teleport's `discovery_service` panic for a GKE cluster without labels.

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
Co-authored-by: Valeriy Khromov <valery.khromov@gmail.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.

Discovery service panics on GKE clusters without labels
4 participants