gitPratikSingh commented on code in PR #6926:
URL: https://github.com/apache/camel-k/pull/6926#discussion_r4495391796


##########
pkg/trait/gc.go:
##########
@@ -255,89 +235,238 @@ func canBeDeleted(it *v1.Integration, u 
unstructured.Unstructured) bool {
        return false
 }
 
-// getDeletableTypes returns the list of deletable types resources, inspecting 
the rules for which the operator SA is allowed in the
-// Integration namespace.
-func (t *gcTrait) getDeletableTypes(e *Environment) 
(map[schema.GroupVersionKind]struct{}, error) {
-       lock.Lock()
-       defer lock.Unlock()
-
-       // Return a fresh map even when returning cached collectables
+// deletableTypesFromRules builds the in-memory map of deletable types from the
+// rules granted to the operator service account. It is a pure function over 
its
+// inputs, so the mapping from RBAC rules to deletable types can be tested
+// without a cluster.
+//
+// A resource is deletable when some rule allows the "delete" verb (or "*") for
+// its API group (or "*") and its resource name (or "*"). Only resources 
present
+// in resourceTypes are considered: role rules carry no version or kind
+// information, and subresources cannot be deleted as a whole.
+func deletableTypesFromRules(rules []rbacv1.PolicyRule, resourceTypes 
map[schema.GroupResource]schema.GroupVersionKind) 
map[schema.GroupVersionKind]struct{} {
        GVKs := make(map[schema.GroupVersionKind]struct{})
 
-       // Rate limit to avoid Discovery and SelfSubjectRulesReview requests at 
every reconciliation.
-       if !rateLimiter.Allow() {
-               // Return the cached set of garbage collectable GVKs.
-               maps.Copy(GVKs, collectableGVKs)
+       for groupResource, GVK := range resourceTypes {
+               for _, rule := range rules {
+                       if !slices.Contains(rule.Verbs, "delete") && 
!slices.Contains(rule.Verbs, "*") {
+                               continue
+                       }
+                       if !slices.Contains(rule.APIGroups, 
groupResource.Group) && !slices.Contains(rule.APIGroups, "*") {
+                               continue
+                       }
+                       if !slices.Contains(rule.Resources, 
groupResource.Resource) && !slices.Contains(rule.Resources, "*") {
+                               continue
+                       }
+                       GVKs[GVK] = struct{}{}
 
-               return GVKs, nil
+                       break
+               }
        }
 
-       // We rely on the discovery API to retrieve all the resources GVK,
-       // that results in an unbounded set that can impact garbage collection 
latency when scaling up.
-       resources, err := 
t.Client.Discovery().ServerPreferredNamespacedResources()
-       // Swallow group discovery errors, e.g., Knative serving exposes
-       // an aggregated API for custom.metrics.k8s.io that requires special
-       // authentication scheme while discovering preferred resources.
-       if err != nil && !discovery.IsGroupDiscoveryFailedError(err) {
-               return nil, err
+       return GVKs
+}
+
+// schemeResourceTypes indexes the types registered in a scheme by the API
+// group and resource name they are served under, as those names appear in
+// role rules. List types and the internal option types registered for every
+// group version are skipped, as they cannot be deleted. When a scheme
+// registers several versions of the same resource, the most stable and
+// recent one wins, so the result does not depend on registration order.
+func schemeResourceTypes(s *runtime.Scheme) 
map[schema.GroupResource]schema.GroupVersionKind {
+       known := s.AllKnownTypes()
+       GVKs := make([]schema.GroupVersionKind, 0, len(known))
+       for GVK := range known {
+               if strings.HasSuffix(GVK.Kind, "List") || 
isInternalKind(GVK.Kind) {
+                       continue
+               }
+               GVKs = append(GVKs, GVK)
        }
+       // Sort for a deterministic result when two versions rank equally.
+       slices.SortFunc(GVKs, func(a, b schema.GroupVersionKind) int {
+               return strings.Compare(a.String(), b.String())
+       })
 
-       // We only take types that support the "delete" verb,
-       // to prevents from performing queries that we know are going to return 
"MethodNotAllowed".
-       APIResourceLists := 
discovery.FilteredBy(discovery.SupportsAllVerbs{Verbs: []string{"delete"}}, 
resources)
+       resourceTypes := make(map[schema.GroupResource]schema.GroupVersionKind, 
len(GVKs))
+       for _, GVK := range GVKs {
+               groupResource := schema.GroupResource{Group: GVK.Group, 
Resource: kindToResource(GVK.Kind)}
+               if current, ok := resourceTypes[groupResource]; !ok || 
preferredVersion(GVK, current) {
+                       resourceTypes[groupResource] = GVK
+               }
+       }
 
-       // Retrieve the permissions granted to the operator service account.
-       // We assume the operator has only to garbage collect the resources it 
has created.
-       ssrr := &authorization.SelfSubjectRulesReview{
-               Spec: authorization.SelfSubjectRulesReviewSpec{
-                       Namespace: e.Integration.Namespace,
-               },
+       return resourceTypes
+}
+
+// isInternalKind reports whether a kind is one of the internal types that
+// metav1.AddToGroupVersion registers for every group version in a scheme.
+// They are API machinery, not resources an operator can delete.
+func isInternalKind(kind string) bool {
+       switch kind {
+       case "CreateOptions", "DeleteOptions", "GetOptions", "ListOptions",
+               "PatchOptions", "UpdateOptions", "WatchEvent", "Status":
+               return true
        }
-       ssrr, err = 
e.Client.AuthorizationV1().SelfSubjectRulesReviews().Create(e.Ctx, ssrr, 
metav1.CreateOptions{})
+
+       return false
+}
+
+// kindToResource converts a kind to the resource name it is served under,
+// applying the usual Kubernetes pluralization rules: "Gateway" becomes
+// "gateways", "NetworkPolicy" becomes "networkpolicies" and "Ingress"
+// becomes "ingresses". "Endpoints" is the one irregular case among the
+// registered types: its resource is the kind itself, unmodified.
+func kindToResource(kind string) string {
+       if kind == "Endpoints" {
+               return "endpoints"
+       }
+       lower := strings.ToLower(kind)
+       switch {
+       case strings.HasSuffix(lower, "s"), strings.HasSuffix(lower, "x"),
+               strings.HasSuffix(lower, "z"), strings.HasSuffix(lower, "ch"),
+               strings.HasSuffix(lower, "sh"):
+               return lower + "es"
+       case strings.HasSuffix(lower, "y"):
+               if len(lower) > 1 && strings.ContainsRune("aeiou", 
rune(lower[len(lower)-2])) {
+                       return lower + "s"
+               }
+
+               return lower[:len(lower)-1] + "ies"
+       default:
+               return lower + "s"
+       }
+}
+
+// preferredVersion reports whether candidate should replace current as the
+// version of a type: a stable version beats a beta, which beats an alpha,
+// and within the same stability level the higher version number wins.
+func preferredVersion(candidate, current schema.GroupVersionKind) bool {

Review Comment:
   Done — the version preference is gone entirely. Every version of a type 
registered in the scheme now enters the deletable set, and garbage collection 
attempts each of them: a version the cluster does not serve fails the list with 
a no-match error and is skipped, the same way a type whose CRD is not installed 
already was.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to