fix(controller): handle comma-separated hook annotations for PreDelete/PostDelete hooks (cherry-pick #26420 for 3.3) (#26586)

Signed-off-by: linghaoSu <linghao.su@daocloud.io>
Co-authored-by: Linghao Su <linghao.su@daocloud.io>

controller/hook.go

@@ -3,6 +3,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"slices"
 	"strings"
 
 	"github.com/argoproj/gitops-engine/pkg/health"
@@ -43,8 +44,12 @@ func isHookOfType(obj *unstructured.Unstructured, hookType HookType) bool {
 	}
 
 	for k, v := range hookTypeAnnotations[hookType] {
-		if val, ok := obj.GetAnnotations()[k]; ok && val == v {
-			return true
+		if val, ok := obj.GetAnnotations()[k]; ok {
+			if slices.ContainsFunc(strings.Split(val, ","), func(item string) bool {
+				return strings.TrimSpace(item) == v
+			}) {
+				return true
+			}
 		}
 	}
 	return false

controller/hook_test.go

@@ -127,6 +127,16 @@ func TestIsPreDeleteHook(t *testing.T) {
 			annot:    map[string]string{"argocd.argoproj.io/hook": "PostDelete"},
 			expected: false,
 		},
+		{
+			name:     "Helm PreDelete & PreDelete hook",
+			annot:    map[string]string{"helm.sh/hook": "pre-delete,post-delete"},
+			expected: true,
+		},
+		{
+			name:     "ArgoCD PostDelete & PreDelete hook",
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PostDelete,PreDelete"},
+			expected: true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -160,6 +170,16 @@ func TestIsPostDeleteHook(t *testing.T) {
 			annot:    map[string]string{"argocd.argoproj.io/hook": "PreDelete"},
 			expected: false,
 		},
+		{
+			name:     "ArgoCD PostDelete & PreDelete hook",
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PostDelete,PreDelete"},
+			expected: true,
+		},
+		{
+			name:     "Helm PostDelete & PreDelete hook",
+			annot:    map[string]string{"helm.sh/hook": "post-delete,pre-delete"},
+			expected: true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -171,3 +191,38 @@ func TestIsPostDeleteHook(t *testing.T) {
 		})
 	}
 }
+
+func TestMultiHookOfType(t *testing.T) {
+	tests := []struct {
+		name     string
+		hookType []HookType
+		annot    map[string]string
+		expected bool
+	}{
+		{
+			name:     "helm PreDelete &  PostDelete hook",
+			hookType: []HookType{PreDeleteHookType, PostDeleteHookType},
+			annot:    map[string]string{"helm.sh/hook": "pre-delete,post-delete"},
+			expected: true,
+		},
+
+		{
+			name:     "ArgoCD PreDelete &  PostDelete hook",
+			hookType: []HookType{PreDeleteHookType, PostDeleteHookType},
+			annot:    map[string]string{"argocd.argoproj.io/hook": "PreDelete,PostDelete"},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			obj := &unstructured.Unstructured{}
+			obj.SetAnnotations(tt.annot)
+
+			for _, hookType := range tt.hookType {
+				result := isHookOfType(obj, hookType)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}

hack/update-manifests.sh

@@ -45,6 +45,10 @@ fi
 # if the tag has not been declared, and we are on a release branch, use the VERSION file.
 if [ "$IMAGE_TAG" = "" ]; then
   branch=$(git rev-parse --abbrev-ref HEAD)
+  # In GitHub Actions PRs, HEAD is detached; use GITHUB_BASE_REF (the target branch) instead
+  if [ "$branch" = "HEAD" ] && [ -n "${GITHUB_BASE_REF:-}" ]; then
+    branch="$GITHUB_BASE_REF"
+  fi
   if [[ $branch = release-* ]]; then
     pwd
     IMAGE_TAG=v$(cat "$SRCROOT/VERSION")

ui/src/app/applications/components/resource-icon.test.tsx

@@ -0,0 +1,137 @@
+import * as React from 'react';
+import * as renderer from 'react-test-renderer';
+import {ResourceIcon} from './resource-icon';
+
+// Mock the resourceIcons and resourceCustomizations
+jest.mock('./resources', () => ({
+    resourceIcons: new Map([
+        ['Ingress', 'ing'],
+        ['ConfigMap', 'cm'],
+        ['Deployment', 'deploy'],
+        ['Service', 'svc']
+    ])
+}));
+
+jest.mock('./resource-customizations', () => ({
+    resourceIconGroups: {
+        '*.crossplane.io': true,
+        '*.fluxcd.io': true,
+        'cert-manager.io': true
+    }
+}));
+
+describe('ResourceIcon', () => {
+    describe('kind-based icons (no group)', () => {
+        it('should show kind-based icon for ConfigMap without group', () => {
+            const testRenderer = renderer.create(<ResourceIcon group='' kind='ConfigMap' />);
+            const testInstance = testRenderer.root;
+            const imgs = testInstance.findAllByType('img');
+            expect(imgs.length).toBeGreaterThan(0);
+            expect(imgs[0].props.src).toBe('assets/images/resources/cm.svg');
+        });
+
+        it('should show kind-based icon for Deployment without group', () => {
+            const testRenderer = renderer.create(<ResourceIcon group='' kind='Deployment' />);
+            const testInstance = testRenderer.root;
+            const imgs = testInstance.findAllByType('img');
+            expect(imgs.length).toBeGreaterThan(0);
+            expect(imgs[0].props.src).toBe('assets/images/resources/deploy.svg');
+        });
+    });
+
+    describe('group-based icons (with matching group)', () => {
+        it('should show group-based icon for exact group match', () => {
+            const testRenderer = renderer.create(<ResourceIcon group='cert-manager.io' kind='Certificate' />);
+            const testInstance = testRenderer.root;
+            const imgs = testInstance.findAllByType('img');
+            expect(imgs.length).toBeGreaterThan(0);
+            expect(imgs[0].props.src).toBe('assets/images/resources/cert-manager.io/icon.svg');
+        });
+
+        it('should show group-based icon for wildcard group match (crossplane)', () => {
+            const testRenderer = renderer.create(<ResourceIcon group='pkg.crossplane.io' kind='Provider' />);
+            const testInstance = testRenderer.root;
+            const imgs = testInstance.findAllByType('img');
+            expect(imgs.length).toBeGreaterThan(0);
+            // Wildcard '*' should be replaced with '_' in the path
+            expect(imgs[0].props.src).toBe('assets/images/resources/_.crossplane.io/icon.svg');
+
+            const complexTestRenderer = renderer.create(<ResourceIcon group='identify.provider.crossplane.io' kind='Provider' />);
+            const complexTestInstance = complexTestRenderer.root;
+            const complexImgs = complexTestInstance.findAllByType('img');
+            expect(complexImgs.length).toBeGreaterThan(0);
+            // Wildcard '*' should be replaced with '_' in the path
+            expect(complexImgs[0].props.src).toBe('assets/images/resources/_.crossplane.io/icon.svg');
+        });
+
+        it('should show group-based icon for wildcard group match (fluxcd)', () => {
+            const testRenderer = renderer.create(<ResourceIcon group='source.fluxcd.io' kind='GitRepository' />);
+            const testInstance = testRenderer.root;
+            const imgs = testInstance.findAllByType('img');
+            expect(imgs.length).toBeGreaterThan(0);
+            expect(imgs[0].props.src).toBe('assets/images/resources/_.fluxcd.io/icon.svg');
+        });
+    });
+
+    describe('fallback to kind-based icons (with non-matching group) - THIS IS THE BUG FIX', () => {
+        it('should fallback to kind-based icon for Ingress with networking.k8s.io group', () => {
+            // This is the main bug fix test case
+            // Ingress has group 'networking.k8s.io' which is NOT in resourceCustomizations
+            // But Ingress IS in resourceIcons, so it should still show the icon
+            const testRenderer = renderer.create(<ResourceIcon group='networking.k8s.io' kind='Ingress' />);
+            const testInstance = testRenderer.root;
+            const imgs = testInstance.findAllByType('img');
+            expect(imgs.length).toBeGreaterThan(0);
+            expect(imgs[0].props.src).toBe('assets/images/resources/ing.svg');
+        });
+
+        it('should fallback to kind-based icon for Service with core group', () => {
+            const testRenderer = renderer.create(<ResourceIcon group='' kind='Service' />);
+            const testInstance = testRenderer.root;
+            const imgs = testInstance.findAllByType('img');
+            expect(imgs.length).toBeGreaterThan(0);
+            expect(imgs[0].props.src).toBe('assets/images/resources/svc.svg');
+        });
+    });
+
+    describe('fallback to initials (no matching group or kind)', () => {
+        it('should show initials for unknown resource with unknown group', () => {
+            const testRenderer = renderer.create(<ResourceIcon group='unknown.example.io' kind='UnknownResource' />);
+            const testInstance = testRenderer.root;
+            const imgs = testInstance.findAllByType('img');
+            expect(imgs.length).toBe(0);
+            // Should show initials "UR" (uppercase letters from UnknownResource)
+            const spans = testInstance.findAllByType('span');
+            const textSpan = spans.find(s => s.children.includes('UR'));
+            expect(textSpan).toBeTruthy();
+        });
+
+        it('should show initials for MyCustomKind', () => {
+            const testRenderer = renderer.create(<ResourceIcon group='' kind='MyCustomKind' />);
+            const testInstance = testRenderer.root;
+            const imgs = testInstance.findAllByType('img');
+            expect(imgs.length).toBe(0);
+            // Should show initials "MCK"
+            const spans = testInstance.findAllByType('span');
+            const textSpan = spans.find(s => s.children.includes('MCK'));
+            expect(textSpan).toBeTruthy();
+        });
+    });
+
+    describe('special cases', () => {
+        it('should show node icon for kind=node', () => {
+            const testRenderer = renderer.create(<ResourceIcon group='' kind='node' />);
+            const testInstance = testRenderer.root;
+            const imgs = testInstance.findAllByType('img');
+            expect(imgs.length).toBeGreaterThan(0);
+            expect(imgs[0].props.src).toBe('assets/images/infrastructure_components/node.svg');
+        });
+
+        it('should show application icon for kind=Application', () => {
+            const testRenderer = renderer.create(<ResourceIcon group='' kind='Application' />);
+            const testInstance = testRenderer.root;
+            const icons = testInstance.findAll(node => node.type === 'i' && typeof node.props.className === 'string' && node.props.className.includes('argo-icon-application'));
+            expect(icons.length).toBeGreaterThan(0);
+        });
+    });
+});

ui/src/app/applications/components/resource-icon.tsx

@@ -10,17 +10,18 @@ export const ResourceIcon = ({group, kind, customStyle}: {group: string; kind: s
     if (kind === 'Application') {
         return <i title={kind} className={`icon argo-icon-application`} style={customStyle} />;
     }
-    if (!group) {
-        const i = resourceIcons.get(kind);
-        if (i !== undefined) {
-            return <img src={'assets/images/resources/' + i + '.svg'} alt={kind} style={{padding: '2px', width: '40px', height: '32px', ...customStyle}} />;
-        }
-    } else {
+    // First, check for group-based custom icons
+    if (group) {
         const matchedGroup = matchGroupToResource(group);
         if (matchedGroup) {
             return <img src={`assets/images/resources/${matchedGroup}/icon.svg`} alt={kind} style={{paddingBottom: '2px', width: '40px', height: '32px', ...customStyle}} />;
         }
     }
+    // Fallback to kind-based icons (works for both empty group and non-matching groups)
+    const i = resourceIcons.get(kind);
+    if (i !== undefined) {
+        return <img src={'assets/images/resources/' + i + '.svg'} alt={kind} style={{padding: '2px', width: '40px', height: '32px', ...customStyle}} />;
+    }
     const initials = kind.replace(/[a-z]/g, '');
     const n = initials.length;
     const style: React.CSSProperties = {