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

Replace multiple ResourceMeta implementations with a single type #11886

Open
lobkovilya opened this issue Oct 28, 2024 · 0 comments
Open

Replace multiple ResourceMeta implementations with a single type #11886

lobkovilya opened this issue Oct 28, 2024 · 0 comments
Labels
kind/improvement Improvement on an existing feature triage/accepted The issue was reviewed and is complete enough to start working on it

Comments

@lobkovilya
Copy link
Contributor

lobkovilya commented Oct 28, 2024

Description

We have 9 different implementations of meta spread across various packages and 1 ResourceMeta interface in the core package. This has downsides:

  • introducing/changing fields in ResourceMeta requires changes in 9 files
  • common shared functions that work with ResourceMeta can't be implemented as methods and have to accept ResourceMeta interface, see GetDisplayName, IsReferenced, ResourceOrigin
  • we can't clone meta, instead we do hacks like this or this when we replace source meta with another ResourceMeta implementation

Single ResourceMeta type in core will be enough. ResourceMeta clients in various packages can create their own interfaces if needed.

type ResourceMeta struct {
	name             string
	version          string
	mesh             string
	creationTime     time.Time
	modificationTime time.Time
	labels           map[string]string

	nameExtensions map[string]string // kube specific
	annotations    map[string]string // kube specific
}

When it comes to KubernetesMetaAdapter, we don't have to store the entire v1.ObjectMeta object. We need to store only annotations and UID (UID can be encoded to meta.version like {.ResourceVersion}/{.UID}):

// kube meta -> core meta
func ResourceMeta(objectMeta *v1.ObjectMeta, mesh string) *core_model.ResourceMeta {
	var name string
	if objectMeta.Namespace == "" {
		name = objectMeta.Name
	} else {
		name = util_k8s.K8sNamespacedNameToCoreName(objectMeta.Name, objectMeta.Namespace)
	}
	labels := maps.Clone(objectMeta.GetLabels())
	if labels == nil {
		labels = map[string]string{}
	}
	if displayName, ok := objectMeta.GetAnnotations()[v1alpha1.DisplayName]; ok {
		labels[v1alpha1.DisplayName] = displayName
	} else {
		labels[v1alpha1.DisplayName] = objectMeta.GetName()
	}
	return (&core_model.ResourceMetaBuilder{}).
		WithName(name).
		WithMesh(mesh).
		WithVersion(strings.Join([]string{objectMeta.GetResourceVersion(), string(objectMeta.UID)}, "/")).
		WithCreationTime(objectMeta.GetCreationTimestamp().Time).
		WithModificationTime(objectMeta.GetCreationTimestamp().Time).
		WithLabels(labels).
		WithAnnotations(objectMeta.GetAnnotations()).
		WithNameExtensions(ResourceNameExtensions(objectMeta.Namespace, objectMeta.Name)).
		Build()
}

// core meta -> kube meta
func ObjectMetaAndMesh(meta *core_model.ResourceMeta) (*v1.ObjectMeta, string) {
	labels, annotations := SplitLabelsAndAnnotations(meta.GetLabels(), meta.GetAnnotations())
	versionAndUID := strings.Split(meta.GetVersion(), "/")
	return &v1.ObjectMeta{
		Name:              meta.GetNameExtensions()[core_model.K8sNameComponent],
		Namespace:         meta.GetNameExtensions()[core_model.K8sNamespaceComponent],
		Labels:            maps.Clone(labels),
		Annotations:       maps.Clone(annotations),
		CreationTimestamp: v1.Time{Time: meta.GetCreationTime()},
		ResourceVersion:   versionAndUID[0],
		UID:               types.UID(versionAndUID[1]),
	}, meta.GetMesh()
}
@lobkovilya lobkovilya added triage/pending This issue will be looked at on the next triage meeting kind/improvement Improvement on an existing feature labels Oct 28, 2024
@jakubdyszkiewicz jakubdyszkiewicz added triage/accepted The issue was reviewed and is complete enough to start working on it and removed triage/pending This issue will be looked at on the next triage meeting labels Oct 28, 2024
@github-actions github-actions bot added the triage/pending This issue will be looked at on the next triage meeting label Oct 28, 2024
@jakubdyszkiewicz jakubdyszkiewicz removed the triage/pending This issue will be looked at on the next triage meeting label Oct 28, 2024
michaelbeaumont added a commit that referenced this issue Nov 12, 2024
There were some places where labels on meta weren't not cloned when
duplicating the meta. Walked the code for odd usages of `GetLabels()`.

See also #11886

Signed-off-by: Charly Molter <[email protected]>
Signed-off-by: Mike Beaumont <[email protected]>
Co-authored-by: Mike Beaumont <[email protected]>
kumahq bot pushed a commit that referenced this issue Nov 12, 2024
There were some places where labels on meta weren't not cloned when
duplicating the meta. Walked the code for odd usages of `GetLabels()`.

See also #11886

Signed-off-by: Charly Molter <[email protected]>
Signed-off-by: Mike Beaumont <[email protected]>
Co-authored-by: Mike Beaumont <[email protected]>
kumahq bot pushed a commit that referenced this issue Nov 12, 2024
There were some places where labels on meta weren't not cloned when
duplicating the meta. Walked the code for odd usages of `GetLabels()`.

See also #11886

Signed-off-by: Charly Molter <[email protected]>
Signed-off-by: Mike Beaumont <[email protected]>
Co-authored-by: Mike Beaumont <[email protected]>
kumahq bot pushed a commit that referenced this issue Nov 12, 2024
There were some places where labels on meta weren't not cloned when
duplicating the meta. Walked the code for odd usages of `GetLabels()`.

See also #11886

Signed-off-by: Charly Molter <[email protected]>
Signed-off-by: Mike Beaumont <[email protected]>
Co-authored-by: Mike Beaumont <[email protected]>
kumahq bot pushed a commit that referenced this issue Nov 12, 2024
There were some places where labels on meta weren't not cloned when
duplicating the meta. Walked the code for odd usages of `GetLabels()`.

See also #11886

Signed-off-by: Charly Molter <[email protected]>
Signed-off-by: Mike Beaumont <[email protected]>
Co-authored-by: Mike Beaumont <[email protected]>
kumahq bot pushed a commit that referenced this issue Nov 12, 2024
There were some places where labels on meta weren't not cloned when
duplicating the meta. Walked the code for odd usages of `GetLabels()`.

See also #11886

Signed-off-by: Charly Molter <[email protected]>
Signed-off-by: Mike Beaumont <[email protected]>
Co-authored-by: Mike Beaumont <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/improvement Improvement on an existing feature triage/accepted The issue was reviewed and is complete enough to start working on it
Projects
None yet
Development

No branches or pull requests

2 participants