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

Specify an explicit list of clusters with the cluster generator #13490

Closed
blakepettersson opened this issue May 8, 2023 · 1 comment
Closed
Labels
enhancement New feature or request

Comments

@blakepettersson
Copy link
Member

blakepettersson commented May 8, 2023

Summary

Add the ability to filter an explicit list of known clusters with the cluster generator, without needing to use labels.

Motivation

Currently we have the list generator, where we can specify a specific set of cluster urls and names

generators:
  - list:
      elements:
      - cluster: engineering-dev
        url: https://kubernetes.default.svc/

The downside of this is that with the list generator, there's no way to access the cluster metadata such as its labels, annotations etc. A workaround to use the cluster generator is to label each cluster secret with a specific identifier, but that is pretty hacky and fragile, and can lead to inconsistencies, since various teams might choose to implement this in a slightly different way.

Proposal

Extend the ClusterGenerator to allow for a list of specific cluster urls:

type ClusterGenerator struct {
	// URLs contains a set of cluster urls
	URLs []string `json:"urls,omitempty"`
}

If that list is non-empty, the generator will first filter the cluster secrets by those ids. If the generator also specifies a label selector, the filtered list will be further filtered by the labels (if any).

For this to be doable in an efficient manner, we need a way to be able to fetch cluster secrets by their urls. The ways to fetch a given object in Kubernetes is either through its name or by its labels. Since the cluster secret name is generated and not so straightforward to get from a given cluster URL, the proposed way to do so is through generating a hash from the cluster URL. The reason we need to use a hash instead of the full URL is simply because Kubernetes sets a limit of 63 characters for a label value.

This cluster URL hash does not exist for existing cluster secrets, so we need a way to migrate existing secrets without disruption, and we also need to be able to keep the hash in sync with the cluster URL in case it changes. As it turns out, on each invocation of GenerateParams we fetch all cluster secrets. Before going further, the list of retrieved secrets gets converted from a list of v1.Secret to a list of v1.Cluster.

It is during this conversion a conditional update would be performed:

if clusterSecret.Labels[common.LabelKeyUrlHash] != urlHash {
	clusterSecret.Labels[common.LabelKeyUrlHash] = urlHash
	_, err := clientset.CoreV1().Secrets(namespace).Update(ctx, clusterSecret, metav1.UpdateOptions{})
	if err != nil {
		return fmt.Errorf("unable to persist latest url hash to cluster secret '%s': %v", clusterSecret.Name, err)
	}
}

This would generally be a no-op, apart from the first time the hash is generated, or whenever a new cluster secret is created or updated.

Then in getSecretsByClusterName we would check for the presence of URLs and add an extra condition if there is at least one URL in the list:

if len(appSetGenerator.Clusters.URLs) > 0 {
  var urlHashes []string
  for _, url := range appSetGenerator.Clusters.URLs {
	  urlHashes = append(urlHashes, hash(url))
  }

  urlRequirement, err := labels.NewRequirement(common.LabelKeyUrlHash, selection.In, urlHashes)
  if err != nil {
	  return nil, fmt.Errorf("bad url requirements: %v", err)
  }

  secretSelector = secretSelector.Add(*urlRequirement)
}
@blakepettersson blakepettersson added the enhancement New feature or request label May 8, 2023
@blakepettersson blakepettersson changed the title Specify a specific list of clusters with the cluster generator Specify an explicit list of clusters with the cluster generator May 8, 2023
@blakepettersson
Copy link
Member Author

Superceded by #13715.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant