Discover EKS: allow custom labels for Kube Server

[marcoandredinis]

This PR allows the UI to send extra labels for setting up the EKS cluster.

Related #46976

Demo

$ curl 'https://lenix.marcoandredinis.com/v1/webapi/sites/lenix.marcoandredinis.com/integrations/aws-oidc/teleportdev/enrolleksclusters' \
--data-raw '{"region":"eu-west-2","enableAppDiscovery":true,"clusterNames":["MarcoEKSCustomLabels01"], "extraLabels":[{"name":"marco-label-key", "value":"marco-label-value"}]}'

[marcoandredinis]

@doggydogworld @timothyb89 Can you please take a look when you get a chance?

[r0mant]

lgtm with a question/suggestion

[r0mant]

I assume "extraLabels" are basically user-provided labels and "static labels" are labels from EKS tags, correct? In that case, should user-defined labels take precedence, i.e. should we swap the order of these two copies?

[marcoandredinis]

I assume "extraLabels" are basically user-provided labels and "static labels" are labels from EKS tags, correct?

Yes 👍

In that case, should user-defined labels take precedence, i.e. should we swap the order of these two copies?

My assumption was that we want static/cloud labels to always be applied after the custom ones, preventing any override of values such as the AWS Account ID/Region.

I looked into how Teleport Server + ServerInfo labels and it seems that its order is:

• Cloud Labels
• ServerInfo Labels
• Static Labels
  

  teleport/lib/srv/regular/sshserver.go

  Lines 1026 to 1040 in 4846ef4

  	func (s *Server) getStaticLabels() map[string]string {
  	labels := make(map[string]string, len(s.labels))
  	if s.cloudLabels != nil {
  	maps.Copy(labels, s.cloudLabels.Get())
  	}
  	// Let labels sent over ics override labels from instance metadata.
  	if s.inventoryHandle != nil {
  	maps.Copy(labels, s.inventoryHandle.GetUpstreamLabels(proto.LabelUpdateKind_SSHServerCloudLabels))
  	}
  	// Let static labels override any other labels.
  	maps.Copy(labels, s.labels)
  	return labels
  	}

Static labels end up being the last to be applied.

Having said that, I'm ok changing the order 👍

[doggydogworld]

Is it necessary to use UUID generation for testing?

[marcoandredinis]

We can use whatever value here 👍
Is it bad to use it? I was trying to use a real value here instead of a static one.

[doggydogworld]

Can't we use maps.Clone instead? Since maps.Copy is used I think it might make it more consistent.

[marcoandredinis]

req.ExtraLabels format is not a map.
It's a []ui.Label where ui.Label is a struct with Name, Value.

So, I don't think we can use maps.Clone here

[doggydogworld]

Just some nits otherwise lgtm.

[public-teleport-github-review-bot]

@marcoandredinis See the table below for backport results.

Branch	Result
branch/v16	Failed
branch/v17	Create PR