[Backport release-1.5] Fix: empty health policy do not check object existence (#4522)

* Fix: empty health policy do not check object existence

Signed-off-by: Somefive <yd219913@alibaba-inc.com>
(cherry picked from commit c759603094)

* Fix: fix health check error

Signed-off-by: Somefive <yd219913@alibaba-inc.com>
(cherry picked from commit d46e49f9ef)

* Fix: add test

Signed-off-by: Somefive <yd219913@alibaba-inc.com>
(cherry picked from commit c29ded0765)

Co-authored-by: Somefive <yd219913@alibaba-inc.com>

pkg/appfile/appfile.go

@@ -109,7 +109,7 @@ func (wl *Workload) EvalStatus(ctx process.Context, cli client.Client, accessor
 // EvalHealth eval workload health check
 func (wl *Workload) EvalHealth(ctx process.Context, client client.Client, accessor util.NamespaceAccessor) (bool, error) {
 	// if health of template is not set or standard workload is managed by trait always return true
-	if wl.FullTemplate.Health == "" || wl.SkipApplyWorkload {
+	if wl.SkipApplyWorkload {
 		return true, nil
 	}
 	return wl.engine.HealthCheck(ctx, client, accessor, wl.FullTemplate.Health)
@@ -152,9 +152,6 @@ func (trait *Trait) EvalStatus(ctx process.Context, cli client.Client, accessor
 
 // EvalHealth eval trait health check
 func (trait *Trait) EvalHealth(ctx process.Context, client client.Client, accessor util.NamespaceAccessor) (bool, error) {
-	if trait.FullTemplate.Health == "" {
-		return true, nil
-	}
 	return trait.engine.HealthCheck(ctx, client, accessor, trait.HealthCheckPolicy)
 }
 

pkg/cue/definition/template.go

@@ -186,7 +186,7 @@ func (wd *workloadDef) getTemplateContext(ctx process.Context, cli client.Reader
 			return nil, err
 		}
 		// AuxiliaryWorkload will have a unique label("trait.oam.dev/resource"="name of outputs") in per component/app level
-		object, err := getResourceFromObj(ctx.GetCtx(), traitRef, cli, accessor.For(componentWorkload), util.MergeMapOverrideWithDst(map[string]string{
+		object, err := getResourceFromObj(ctx.GetCtx(), traitRef, cli, accessor.For(traitRef), util.MergeMapOverrideWithDst(map[string]string{
 			oam.TraitTypeLabel: AuxiliaryWorkload,
 		}, commonLabels), assist.Name)
 		if err != nil {
@@ -202,9 +202,6 @@ func (wd *workloadDef) getTemplateContext(ctx process.Context, cli client.Reader
 
 // HealthCheck address health check for workload
 func (wd *workloadDef) HealthCheck(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, healthPolicyTemplate string) (bool, error) {
-	if healthPolicyTemplate == "" {
-		return true, nil
-	}
 	templateContext, err := wd.getTemplateContext(ctx, cli, accessor)
 	if err != nil {
 		return false, errors.WithMessage(err, "get template context")
@@ -213,6 +210,9 @@ func (wd *workloadDef) HealthCheck(ctx process.Context, cli client.Client, acces
 }
 
 func checkHealth(templateContext map[string]interface{}, healthPolicyTemplate string) (bool, error) {
+	if healthPolicyTemplate == "" {
+		return true, nil
+	}
 	bt, err := json.Marshal(templateContext)
 	if err != nil {
 		return false, errors.WithMessage(err, "json marshal template context")
@@ -233,9 +233,6 @@ func checkHealth(templateContext map[string]interface{}, healthPolicyTemplate st
 
 // Status get workload status by customStatusTemplate
 func (wd *workloadDef) Status(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, customStatusTemplate string, parameter interface{}) (string, error) {
-	if customStatusTemplate == "" {
-		return "", nil
-	}
 	templateContext, err := wd.getTemplateContext(ctx, cli, accessor)
 	if err != nil {
 		return "", errors.WithMessage(err, "get template context")
@@ -244,6 +241,9 @@ func (wd *workloadDef) Status(ctx process.Context, cli client.Client, accessor u
 }
 
 func getStatusMessage(pd *packages.PackageDiscover, templateContext map[string]interface{}, customStatusTemplate string, parameter interface{}) (string, error) {
+	if customStatusTemplate == "" {
+		return "", nil
+	}
 	bi := build.NewContext().NewInstance("", nil)
 	var ctxBuff string
 	var paramBuff = "parameter: {}\n"
@@ -455,9 +455,6 @@ func (td *traitDef) getTemplateContext(ctx process.Context, cli client.Reader, a
 
 // Status get trait status by customStatusTemplate
 func (td *traitDef) Status(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, customStatusTemplate string, parameter interface{}) (string, error) {
-	if customStatusTemplate == "" {
-		return "", nil
-	}
 	templateContext, err := td.getTemplateContext(ctx, cli, accessor)
 	if err != nil {
 		return "", errors.WithMessage(err, "get template context")
@@ -467,9 +464,6 @@ func (td *traitDef) Status(ctx process.Context, cli client.Client, accessor util
 
 // HealthCheck address health check for trait
 func (td *traitDef) HealthCheck(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, healthPolicyTemplate string) (bool, error) {
-	if healthPolicyTemplate == "" {
-		return true, nil
-	}
 	templateContext, err := td.getTemplateContext(ctx, cli, accessor)
 	if err != nil {
 		return false, errors.WithMessage(err, "get template context")

test/e2e-multicluster-test/multicluster_test.go

@@ -570,5 +570,25 @@ var _ = Describe("Test multicluster scenario", func() {
 				g.Expect(k8sClient.Get(hubCtx, types.NamespacedName{Namespace: namespace, Name: "non-shared-app3"}, &corev1.ConfigMap{})).Should(Satisfy(kerrors.IsNotFound))
 			}, 10*time.Second).Should(Succeed())
 		})
+
+		It("Test applications with bad resource", func() {
+			bs, err := ioutil.ReadFile("./testdata/app/app-bad-resource.yaml")
+			Expect(err).Should(Succeed())
+			appYaml := strings.ReplaceAll(string(bs), "TEST_NAMESPACE", testNamespace)
+			app := &v1beta1.Application{}
+			Expect(yaml.Unmarshal([]byte(appYaml), app)).Should(Succeed())
+			ctx := context.Background()
+			Expect(k8sClient.Create(ctx, app)).Should(Succeed())
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(hubCtx, client.ObjectKeyFromObject(app), app)).Should(Succeed())
+				g.Expect(app.Status.Phase).Should(Equal(common.ApplicationRunningWorkflow))
+				g.Expect(len(app.Status.Workflow.Steps) > 0).Should(BeTrue())
+				g.Expect(app.Status.Workflow.Steps[0].Message).Should(ContainSubstring("is invalid"))
+			}, 20*time.Second).Should(Succeed())
+			Expect(k8sClient.Delete(ctx, app)).Should(Succeed())
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(hubCtx, client.ObjectKeyFromObject(app), app)).Should(Satisfy(kerrors.IsNotFound))
+			}, 10*time.Second).Should(Succeed())
+		})
 	})
 })

test/e2e-multicluster-test/testdata/app/app-bad-resource.yaml

@@ -0,0 +1,22 @@
+apiVersion: core.oam.dev/v1beta1
+kind: Application
+metadata:
+  name: bad-resource
+  namespace: TEST_NAMESPACE
+spec:
+  components:
+    - name: bad-resource
+      properties:
+        objects:
+        - apiVersion: apiregistration.k8s.io/v1beta1
+          kind: APIService
+          metadata:
+            name: test-bad-resource
+          spec:
+            group: bad
+      type: k8s-objects
+  policies:
+    - name: topology
+      type: topology
+      properties:
+        clusters: ["local"]