Fix: fix writing logs to file (#4588)

Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com>
(cherry picked from commit d4b3bbf049)

Co-authored-by: FogDong <dongtianxin.tx@alibaba-inc.com>

cmd/core/main.go

@@ -19,6 +19,7 @@ package main
 import (
 	"context"
 	"errors"
+	goflag "flag"
 	"fmt"
 	"io"
 	"net/http"
@@ -138,6 +139,7 @@ func main() {
 	flag.DurationVar(&clusterMetricsInterval, "cluster-metrics-interval", 15*time.Second, "The interval that ClusterMetricsMgr will collect metrics from clusters, default value is 15 seconds.")
 	flag.BoolVar(&controllerArgs.EnableCompatibility, "enable-asi-compatibility", false, "enable compatibility for asi")
 	flag.BoolVar(&controllerArgs.IgnoreAppWithoutControllerRequirement, "ignore-app-without-controller-version", false, "If true, application controller will not process the app without 'app.oam.dev/controller-version-require' annotation")
+	flag.BoolVar(&controllerArgs.IgnoreDefinitionWithoutControllerRequirement, "ignore-definition-without-controller-version", false, "If true, trait/component/workflowstep definition controller will not process the definition without 'definition.oam.dev/controller-version-require' annotation")
 	standardcontroller.AddOptimizeFlags()
 	standardcontroller.AddAdmissionFlags()
 	flag.IntVar(&resourcekeeper.MaxDispatchConcurrent, "max-dispatch-concurrent", 10, "Set the max dispatch concurrent number, default is 10")
@@ -146,9 +148,10 @@ func main() {
 	flag.IntVar(&custom.MaxWorkflowStepErrorRetryTimes, "max-workflow-step-error-retry-times", 10, "Set the max workflow step error retry times, default is 10")
 	utilfeature.DefaultMutableFeatureGate.AddFlag(flag.CommandLine)
 
-	flag.Parse()
 	// setup logging
 	klog.InitFlags(nil)
+	flag.CommandLine.AddGoFlagSet(goflag.CommandLine)
+	flag.Parse()
 	if logDebug {
 		_ = flag.Set("v", strconv.Itoa(int(commonconfig.LogDebug)))
 	}

pkg/appfile/parser.go

@@ -365,6 +365,9 @@ func (p *Parser) parsePoliciesFromRevision(ctx context.Context, af *Appfile) (er
 		return err
 	}
 	for _, policy := range af.Policies {
+		if policy.Properties == nil && policy.Type != v1alpha1.DebugPolicyType {
+			return fmt.Errorf("policy %s named %s must not have empty properties", policy.Type, policy.Name)
+		}
 		switch policy.Type {
 		case v1alpha1.GarbageCollectPolicyType:
 		case v1alpha1.ApplyOncePolicyType:
@@ -390,6 +393,9 @@ func (p *Parser) parsePolicies(ctx context.Context, af *Appfile) (err error) {
 		return err
 	}
 	for _, policy := range af.Policies {
+		if policy.Properties == nil && policy.Type != v1alpha1.DebugPolicyType {
+			return fmt.Errorf("policy %s named %s must not have empty properties", policy.Type, policy.Name)
+		}
 		switch policy.Type {
 		case v1alpha1.GarbageCollectPolicyType:
 		case v1alpha1.ApplyOncePolicyType:

pkg/appfile/parser_test.go

@@ -243,6 +243,20 @@ spec:
         image: "busybox"
 `
 
+const appfileYamlEmptyPolicy = `
+apiVersion: core.oam.dev/v1beta1
+kind: Application
+metadata:
+  name: application-sample
+  namespace: default
+spec:
+  components: []
+  policies:
+    - type: garbage-collect
+      name: somename
+      properties:
+`
+
 var _ = Describe("Test application parser", func() {
 	It("Test we can parse an application to an appFile", func() {
 		o := v1beta1.Application{}
@@ -282,6 +296,14 @@ var _ = Describe("Test application parser", func() {
 		Expect(err).ShouldNot(HaveOccurred())
 		_, err = NewApplicationParser(&tclient, dm, pd).GenerateAppFile(context.TODO(), &notfound)
 		Expect(err).Should(HaveOccurred())
+
+		By("app with empty policy")
+		emptyPolicy := v1beta1.Application{}
+		err = yaml.Unmarshal([]byte(appfileYamlEmptyPolicy), &emptyPolicy)
+		Expect(err).ShouldNot(HaveOccurred())
+		_, err = NewApplicationParser(&tclient, dm, pd).GenerateAppFile(context.TODO(), &emptyPolicy)
+		Expect(err).Should(HaveOccurred())
+		Expect(err.Error()).Should(ContainSubstring("have empty properties"))
 	})
 })
 

pkg/controller/core.oam.dev/oamruntime_controller.go

@@ -86,4 +86,7 @@ type Args struct {
 
 	// IgnoreAppWithoutControllerRequirement indicates that application controller will not process the app without 'app.oam.dev/controller-version-require' annotation.
 	IgnoreAppWithoutControllerRequirement bool
+
+	// IgnoreDefinitionWithoutControllerRequirement indicates that trait/component/workflowstep definition controller will not process the definition without 'definition.oam.dev/controller-version-require' annotation.
+	IgnoreDefinitionWithoutControllerRequirement bool
 }

pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go

@@ -136,7 +136,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	if annotations := app.GetAnnotations(); annotations == nil || annotations[oam.AnnotationKubeVelaVersion] == "" {
 		metav1.SetMetaDataAnnotation(&app.ObjectMeta, oam.AnnotationKubeVelaVersion, version.VelaVersion)
 	}
-	logCtx.AddTag("publish_version", app.GetAnnotations()[oam.AnnotationKubeVelaVersion])
+	logCtx.AddTag("publish_version", app.GetAnnotations()[oam.AnnotationPublishVersion])
 
 	appParser := appfile.NewApplicationParser(r.Client, r.dm, r.pd)
 	handler, err := NewAppHandler(logCtx, r, app, appParser)

pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller.go

@@ -43,17 +43,24 @@ import (
 	"github.com/oam-dev/kubevela/pkg/oam"
 	"github.com/oam-dev/kubevela/pkg/oam/discoverymapper"
 	"github.com/oam-dev/kubevela/pkg/oam/util"
+	"github.com/oam-dev/kubevela/version"
 )
 
 // Reconciler reconciles a ComponentDefinition object
 type Reconciler struct {
 	client.Client
-	dm                   discoverymapper.DiscoveryMapper
-	pd                   *packages.PackageDiscover
-	Scheme               *runtime.Scheme
-	record               event.Recorder
+	dm     discoverymapper.DiscoveryMapper
+	pd     *packages.PackageDiscover
+	Scheme *runtime.Scheme
+	record event.Recorder
+	options
+}
+
+type options struct {
 	defRevLimit          int
 	concurrentReconciles int
+	ignoreDefNoCtrlReq   bool
+	controllerVersion    string
 }
 
 // Reconcile is the main logic for ComponentDefinition controller
@@ -68,6 +75,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	if !r.matchControllerRequirement(&componentDefinition) {
+		klog.InfoS("skip componentDefinition: not match the controller requirement of componentDefinition", "componentDefinition", klog.KObj(&componentDefinition))
+		return ctrl.Result{}, nil
+	}
+
 	// refresh package discover when componentDefinition is registered
 	if componentDefinition.Spec.Workload.Type != types.AutoDetectWorkloadDefinition {
 		err := utils.RefreshPackageDiscover(ctx, r.Client, r.dm, r.pd, &componentDefinition)
@@ -187,12 +199,32 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 // Setup adds a controller that reconciles ComponentDefinition.
 func Setup(mgr ctrl.Manager, args oamctrl.Args) error {
 	r := Reconciler{
-		Client:               mgr.GetClient(),
-		Scheme:               mgr.GetScheme(),
-		dm:                   args.DiscoveryMapper,
-		pd:                   args.PackageDiscover,
-		defRevLimit:          args.DefRevisionLimit,
-		concurrentReconciles: args.ConcurrentReconciles,
+		Client:  mgr.GetClient(),
+		Scheme:  mgr.GetScheme(),
+		dm:      args.DiscoveryMapper,
+		pd:      args.PackageDiscover,
+		options: parseOptions(args),
 	}
 	return r.SetupWithManager(mgr)
 }
+
+func parseOptions(args oamctrl.Args) options {
+	return options{
+		defRevLimit:          args.DefRevisionLimit,
+		concurrentReconciles: args.ConcurrentReconciles,
+		ignoreDefNoCtrlReq:   args.IgnoreDefinitionWithoutControllerRequirement,
+		controllerVersion:    version.VelaVersion,
+	}
+}
+
+func (r *Reconciler) matchControllerRequirement(componentDefinition *v1beta1.ComponentDefinition) bool {
+	if componentDefinition.Annotations != nil {
+		if requireVersion, ok := componentDefinition.Annotations[oam.AnnotationControllerRequirement]; ok {
+			return requireVersion == r.controllerVersion
+		}
+	}
+	if r.ignoreDefNoCtrlReq {
+		return false
+	}
+	return true
+}

pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_suit_test.go

@@ -90,11 +90,13 @@ var _ = BeforeSuite(func(done Done) {
 	Expect(err).ToNot(HaveOccurred())
 
 	r = Reconciler{
-		Client:      mgr.GetClient(),
-		Scheme:      mgr.GetScheme(),
-		dm:          dm,
-		pd:          pd,
-		defRevLimit: defRevisionLimit,
+		Client: mgr.GetClient(),
+		Scheme: mgr.GetScheme(),
+		dm:     dm,
+		pd:     pd,
+		options: options{
+			defRevLimit: defRevisionLimit,
+		},
 	}
 	Expect(r.SetupWithManager(mgr)).ToNot(HaveOccurred())
 	var ctx context.Context

pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitdefinition_controller.go

@@ -42,17 +42,24 @@ import (
 	"github.com/oam-dev/kubevela/pkg/oam"
 	"github.com/oam-dev/kubevela/pkg/oam/discoverymapper"
 	"github.com/oam-dev/kubevela/pkg/oam/util"
+	"github.com/oam-dev/kubevela/version"
 )
 
 // Reconciler reconciles a TraitDefinition object
 type Reconciler struct {
 	client.Client
-	dm                   discoverymapper.DiscoveryMapper
-	pd                   *packages.PackageDiscover
-	Scheme               *runtime.Scheme
-	record               event.Recorder
+	dm     discoverymapper.DiscoveryMapper
+	pd     *packages.PackageDiscover
+	Scheme *runtime.Scheme
+	record event.Recorder
+	options
+}
+
+type options struct {
 	defRevLimit          int
 	concurrentReconciles int
+	ignoreDefNoCtrlReq   bool
+	controllerVersion    string
 }
 
 // Reconcile is the main logic for TraitDefinition controller
@@ -67,6 +74,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	if !r.matchControllerRequirement(&traitdefinition) {
+		klog.InfoS("skip traitDefinition: not match the controller requirement of traitDefinition", "traitDefinition", klog.KObj(&traitdefinition))
+		return ctrl.Result{}, nil
+	}
+
 	// this is a placeholder for finalizer here in the future
 	if traitdefinition.DeletionTimestamp != nil {
 		klog.InfoS("The TraitDefinition is being deleted", "traitDefinition", klog.KRef(req.Namespace, req.Name))
@@ -193,12 +205,32 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 // Setup adds a controller that reconciles TraitDefinition.
 func Setup(mgr ctrl.Manager, args oamctrl.Args) error {
 	r := Reconciler{
-		Client:               mgr.GetClient(),
-		Scheme:               mgr.GetScheme(),
-		dm:                   args.DiscoveryMapper,
-		pd:                   args.PackageDiscover,
-		defRevLimit:          args.DefRevisionLimit,
-		concurrentReconciles: args.ConcurrentReconciles,
+		Client:  mgr.GetClient(),
+		Scheme:  mgr.GetScheme(),
+		dm:      args.DiscoveryMapper,
+		pd:      args.PackageDiscover,
+		options: parseOptions(args),
 	}
 	return r.SetupWithManager(mgr)
 }
+
+func parseOptions(args oamctrl.Args) options {
+	return options{
+		defRevLimit:          args.DefRevisionLimit,
+		concurrentReconciles: args.ConcurrentReconciles,
+		ignoreDefNoCtrlReq:   args.IgnoreDefinitionWithoutControllerRequirement,
+		controllerVersion:    version.VelaVersion,
+	}
+}
+
+func (r *Reconciler) matchControllerRequirement(traitDefinition *v1beta1.TraitDefinition) bool {
+	if traitDefinition.Annotations != nil {
+		if requireVersion, ok := traitDefinition.Annotations[oam.AnnotationControllerRequirement]; ok {
+			return requireVersion == r.controllerVersion
+		}
+	}
+	if r.ignoreDefNoCtrlReq {
+		return false
+	}
+	return true
+}

pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitdefinition_suite_test.go

@@ -90,11 +90,13 @@ var _ = BeforeSuite(func(done Done) {
 	Expect(err).ToNot(HaveOccurred())
 
 	r = Reconciler{
-		Client:      mgr.GetClient(),
-		Scheme:      mgr.GetScheme(),
-		dm:          dm,
-		pd:          pd,
-		defRevLimit: defRevisionLimit,
+		Client: mgr.GetClient(),
+		Scheme: mgr.GetScheme(),
+		dm:     dm,
+		pd:     pd,
+		options: options{
+			defRevLimit: defRevisionLimit,
+		},
 	}
 	Expect(r.SetupWithManager(mgr)).ToNot(HaveOccurred())
 	var ctx context.Context

pkg/controller/core.oam.dev/v1alpha2/core/workflow/workflowstepdefinition/suite_test.go

@@ -90,11 +90,13 @@ var _ = BeforeSuite(func(done Done) {
 	Expect(err).ToNot(HaveOccurred())
 
 	r = Reconciler{
-		Client:      mgr.GetClient(),
-		Scheme:      mgr.GetScheme(),
-		dm:          dm,
-		pd:          pd,
-		defRevLimit: defRevisionLimit,
+		Client: mgr.GetClient(),
+		Scheme: mgr.GetScheme(),
+		dm:     dm,
+		pd:     pd,
+		options: options{
+			defRevLimit: defRevisionLimit,
+		},
 	}
 	Expect(r.SetupWithManager(mgr)).ToNot(HaveOccurred())
 	var ctx context.Context

pkg/controller/core.oam.dev/v1alpha2/core/workflow/workflowstepdefinition/workflowstepdefinition_controller.go

@@ -42,17 +42,24 @@ import (
 	"github.com/oam-dev/kubevela/pkg/oam"
 	"github.com/oam-dev/kubevela/pkg/oam/discoverymapper"
 	"github.com/oam-dev/kubevela/pkg/oam/util"
+	"github.com/oam-dev/kubevela/version"
 )
 
 // Reconciler reconciles a WorkflowStepDefinition object
 type Reconciler struct {
 	client.Client
-	dm                   discoverymapper.DiscoveryMapper
-	pd                   *packages.PackageDiscover
-	Scheme               *runtime.Scheme
-	record               event.Recorder
+	dm     discoverymapper.DiscoveryMapper
+	pd     *packages.PackageDiscover
+	Scheme *runtime.Scheme
+	record event.Recorder
+	options
+}
+
+type options struct {
 	defRevLimit          int
 	concurrentReconciles int
+	ignoreDefNoCtrlReq   bool
+	controllerVersion    string
 }
 
 // Reconcile is the main logic for WorkflowStepDefinition controller
@@ -68,6 +75,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	if !r.matchControllerRequirement(&wfstepdefinition) {
+		klog.InfoS("skip workflowStepDefinition: not match the controller requirement of workflowStepDefinition", "workflowStepDefinition", klog.KObj(&wfstepdefinition))
+		return ctrl.Result{}, nil
+	}
+
 	// this is a placeholder for finalizer here in the future
 	if wfstepdefinition.DeletionTimestamp != nil {
 		return ctrl.Result{}, nil
@@ -192,11 +204,32 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 // Setup adds a controller that reconciles WorkflowStepDefinition.
 func Setup(mgr ctrl.Manager, args oamctrl.Args) error {
 	r := Reconciler{
-		Client:      mgr.GetClient(),
-		Scheme:      mgr.GetScheme(),
-		dm:          args.DiscoveryMapper,
-		pd:          args.PackageDiscover,
-		defRevLimit: args.DefRevisionLimit,
+		Client:  mgr.GetClient(),
+		Scheme:  mgr.GetScheme(),
+		dm:      args.DiscoveryMapper,
+		pd:      args.PackageDiscover,
+		options: parseOptions(args),
 	}
 	return r.SetupWithManager(mgr)
 }
+
+func parseOptions(args oamctrl.Args) options {
+	return options{
+		defRevLimit:          args.DefRevisionLimit,
+		concurrentReconciles: args.ConcurrentReconciles,
+		ignoreDefNoCtrlReq:   args.IgnoreDefinitionWithoutControllerRequirement,
+		controllerVersion:    version.VelaVersion,
+	}
+}
+
+func (r *Reconciler) matchControllerRequirement(wfstepdefinition *v1beta1.WorkflowStepDefinition) bool {
+	if wfstepdefinition.Annotations != nil {
+		if requireVersion, ok := wfstepdefinition.Annotations[oam.AnnotationControllerRequirement]; ok {
+			return requireVersion == r.controllerVersion
+		}
+	}
+	if r.ignoreDefNoCtrlReq {
+		return false
+	}
+	return true
+}

pkg/oam/labels.go

@@ -201,7 +201,7 @@ const (
 	// AnnotationWorkloadName indicates the managed workload's name by trait
 	AnnotationWorkloadName = "trait.oam.dev/workload-name"
 
-	// AnnotationControllerRequirement indicates the controller version that can process the application.
+	// AnnotationControllerRequirement indicates the controller version that can process the application/definition.
 	AnnotationControllerRequirement = "app.oam.dev/controller-version-require"
 
 	// AnnotationApplicationServiceAccountName indicates the name of the ServiceAccount to use to apply Components and run Workflow.

pkg/policy/common_test.go

@@ -81,3 +81,14 @@ func TestParseApplyOncePolicy(t *testing.T) {
 	r.NoError(err)
 	r.Equal(policySpec, spec)
 }
+
+func TestParsePolicy(t *testing.T) {
+	r := require.New(t)
+	// Test skipping empty policy
+	app := &v1beta1.Application{Spec: v1beta1.ApplicationSpec{
+		Policies: []v1beta1.AppPolicy{{Type: "example", Name: "s", Properties: nil}},
+	}}
+	exists, err := parsePolicy(app, "example", nil)
+	r.False(exists, "empty policy should not be included")
+	r.NoError(err)
+}

pkg/policy/envbinding/utils.go

@@ -34,7 +34,7 @@ const (
 // GetEnvBindingPolicy extract env-binding policy with given policy name, if policy name is empty, the first env-binding policy will be used
 func GetEnvBindingPolicy(app *v1beta1.Application, policyName string) (*v1alpha1.EnvBindingSpec, error) {
 	for _, policy := range app.Spec.Policies {
-		if (policy.Name == policyName || policyName == "") && policy.Type == v1alpha1.EnvBindingPolicyType {
+		if (policy.Name == policyName || policyName == "") && policy.Type == v1alpha1.EnvBindingPolicyType && policy.Properties != nil {
 			envBindingSpec := &v1alpha1.EnvBindingSpec{}
 			err := json.Unmarshal(policy.Properties.Raw, envBindingSpec)
 			return envBindingSpec, err

pkg/policy/override.go

@@ -19,6 +19,7 @@ package policy
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 
 	"github.com/pkg/errors"
 	errors2 "k8s.io/apimachinery/pkg/api/errors"
@@ -32,6 +33,9 @@ import (
 
 // ParseOverridePolicyRelatedDefinitions get definitions inside override policy
 func ParseOverridePolicyRelatedDefinitions(ctx context.Context, cli client.Client, app *v1beta1.Application, policy v1beta1.AppPolicy) (compDefs []*v1beta1.ComponentDefinition, traitDefs []*v1beta1.TraitDefinition, err error) {
+	if policy.Properties == nil {
+		return compDefs, traitDefs, fmt.Errorf("override policy %s must not have empty properties", policy.Name)
+	}
 	spec := &v1alpha1.OverridePolicySpec{}
 	if err = json.Unmarshal(policy.Properties.Raw, spec); err != nil {
 		return nil, nil, errors.Wrapf(err, "invalid override policy spec")

pkg/policy/override_test.go

@@ -62,6 +62,12 @@ func TestParseOverridePolicyRelatedDefinitions(t *testing.T) {
 			Policy: v1beta1.AppPolicy{Properties: &runtime.RawExtension{Raw: []byte(`{"components":[{"type":"comp","traits":[{"type":"trait-404"}]}]}`)}},
 			Error:  "failed to get trait definition",
 		},
+		"empty-policy": {
+			Policy:        v1beta1.AppPolicy{Properties: nil},
+			ComponentDefs: nil,
+			TraitDefs:     nil,
+			Error:         "have empty properties",
+		},
 	}
 	for name, tt := range testCases {
 		t.Run(name, func(t *testing.T) {

pkg/policy/topology.go

@@ -18,6 +18,7 @@ package policy
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/pkg/errors"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
@@ -65,6 +66,9 @@ func GetPlacementsFromTopologyPolicies(ctx context.Context, cli client.Client, a
 	hasTopologyPolicy := false
 	for _, policy := range policies {
 		if policy.Type == v1alpha1.TopologyPolicyType {
+			if policy.Properties == nil {
+				return nil, fmt.Errorf("topology policy %s must not have empty properties", policy.Name)
+			}
 			hasTopologyPolicy = true
 			topologySpec := &v1alpha1.TopologyPolicySpec{}
 			if err := utils.StrictUnmarshal(policy.Properties.Raw, topologySpec); err != nil {

pkg/policy/topology_test.go

@@ -146,6 +146,10 @@ func TestGetClusterLabelSelectorInTopology(t *testing.T) {
 			Inputs:  []v1beta1.AppPolicy{},
 			Outputs: []v1alpha1.PlacementDecision{{Cluster: "local", Namespace: ""}},
 		},
+		"empty-topology-policy": {
+			Inputs: []v1beta1.AppPolicy{{Type: "topology", Name: "some-name", Properties: nil}},
+			Error:  "have empty properties",
+		},
 	}
 	for name, tt := range testCases {
 		t.Run(name, func(t *testing.T) {

pkg/resourcekeeper/gc.go

@@ -437,7 +437,7 @@ func (h *gcHandler) GarbageCollectLegacyResourceTrackers(ctx context.Context) er
 		}
 	}
 	for _, policy := range h.app.Spec.Policies {
-		if policy.Type == v1alpha1.EnvBindingPolicyType {
+		if policy.Type == v1alpha1.EnvBindingPolicyType && policy.Properties != nil {
 			spec := &v1alpha1.EnvBindingSpec{}
 			if err = json.Unmarshal(policy.Properties.Raw, &spec); err == nil {
 				for _, env := range spec.Envs {

pkg/webhook/core.oam.dev/v1alpha2/application/validating_handler.go

@@ -96,7 +96,9 @@ func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) a
 	switch req.Operation {
 	case admissionv1.Create:
 		if allErrs := h.ValidateCreate(ctx, app); len(allErrs) > 0 {
-			return admission.Errored(http.StatusUnprocessableEntity, mergeErrors(allErrs))
+			// http.StatusUnprocessableEntity will NOT report any error descriptions
+			// to the client, use generic http.StatusBadRequest instead.
+			return admission.Errored(http.StatusBadRequest, mergeErrors(allErrs))
 		}
 	case admissionv1.Update:
 		oldApp := &v1beta1.Application{}
@@ -105,7 +107,7 @@ func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) a
 		}
 		if app.ObjectMeta.DeletionTimestamp.IsZero() {
 			if allErrs := h.ValidateUpdate(ctx, app, oldApp); len(allErrs) > 0 {
-				return admission.Errored(http.StatusUnprocessableEntity, mergeErrors(allErrs))
+				return admission.Errored(http.StatusBadRequest, mergeErrors(allErrs))
 			}
 		}
 	default:

pkg/webhook/core.oam.dev/v1alpha2/application/validating_handler_test.go

@@ -373,4 +373,21 @@ var _ = Describe("Test Application Validator", func() {
 		resp = handler.Handle(ctx, req)
 		Expect(resp.Allowed).Should(BeFalse())
 	})
+
+	It("Test Application with empty policy", func() {
+		req := admission.Request{
+			AdmissionRequest: admissionv1.AdmissionRequest{
+				Operation: admissionv1.Create,
+				Resource:  metav1.GroupVersionResource{Group: "core.oam.dev", Version: "v1beta1", Resource: "applications"},
+				Object: runtime.RawExtension{
+					Raw: []byte(`
+{"kind":"Application","metadata":{"name":"app-with-empty-policy-webhook-test", "namespace":"default"},
+"spec":{"components":[],"policies":[{"name":"2345","type":"garbage-collect","properties":null}]}}
+`),
+				},
+			},
+		}
+		resp := handler.Handle(ctx, req)
+		Expect(resp.Allowed).Should(BeFalse())
+	})
 })

pkg/webhook/core.oam.dev/v1alpha2/applicationconfiguration/validating_handler.go

@@ -118,13 +118,14 @@ func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) a
 		if err := h.Decoder.DecodeRaw(req.AdmissionRequest.OldObject, oldApp); err != nil {
 			return admission.Errored(http.StatusBadRequest, err)
 		}
-
 		if allErrs := h.ValidateUpdate(ctx, app, oldApp); len(allErrs) > 0 {
-			return admission.Errored(http.StatusUnprocessableEntity, allErrs.ToAggregate())
+			// http.StatusUnprocessableEntity will NOT report any error descriptions
+			// to the client, use generic http.StatusBadRequest instead.
+			return admission.Errored(http.StatusBadRequest, allErrs.ToAggregate())
 		}
 	case admissionv1.Create:
 		if allErrs := h.ValidateCreate(ctx, app); len(allErrs) > 0 {
-			return admission.Errored(http.StatusUnprocessableEntity, allErrs.ToAggregate())
+			return admission.Errored(http.StatusBadRequest, allErrs.ToAggregate())
 		}
 	default:
 		// Do nothing for CONNECT

pkg/workflow/providers/kube/handle.go

@@ -134,7 +134,7 @@ func (h *provider) ApplyInParallel(ctx wfContext.Context, v *value.Value, act ty
 	deployCtx := multicluster.ContextWithClusterName(context.Background(), cluster)
 	deployCtx = auth.ContextWithUserInfo(deployCtx, h.app)
 	if err = h.apply(deployCtx, cluster, common.WorkflowResourceCreator, workloads...); err != nil {
-		return v.FillObject(err, "err")
+		return err
 	}
 	return nil
 }

pkg/workflow/providers/multicluster/deploy.go

@@ -126,6 +126,9 @@ func overrideConfiguration(policies []v1beta1.AppPolicy, components []common.App
 	var err error
 	for _, policy := range policies {
 		if policy.Type == v1alpha1.OverridePolicyType {
+			if policy.Properties == nil {
+				return nil, fmt.Errorf("override policy %s must not have empty properties", policy.Name)
+			}
 			overrideSpec := &v1alpha1.OverridePolicySpec{}
 			if err := utils.StrictUnmarshal(policy.Properties.Raw, overrideSpec); err != nil {
 				return nil, errors.Wrapf(err, "failed to parse override policy %s", policy.Name)

pkg/workflow/providers/multicluster/deploy_test.go

@@ -48,6 +48,14 @@ func TestOverrideConfiguration(t *testing.T) {
 			}},
 			Error: "failed to parse override policy",
 		},
+		"empty-policy": {
+			Policies: []v1beta1.AppPolicy{{
+				Name:       "override-policy",
+				Type:       "override",
+				Properties: nil,
+			}},
+			Error: "empty properties",
+		},
 		"normal": {
 			Policies: []v1beta1.AppPolicy{{
 				Name:       "override-policy",

pkg/workflow/workflow.go

@@ -403,14 +403,14 @@ func (w *workflow) setMetadataToContext(wfCtx wfContext.Context) error {
 	return wfCtx.SetVar(metadata, wfTypes.ContextKeyMetadata)
 }
 
-func (e *engine) getBackoffTimes(stepID string) (success bool, backoffTimes int) {
+func (e *engine) getBackoffTimes(stepID string) int {
 	if v, ok := e.wfCtx.GetValueInMemory(wfTypes.ContextPrefixBackoffTimes, stepID); ok {
 		times, ok := v.(int)
 		if ok {
-			return true, times
+			return times
 		}
 	}
-	return false, 0
+	return -1
 }
 
 func (e *engine) getBackoffWaitTime() int {
@@ -418,17 +418,19 @@ func (e *engine) getBackoffWaitTime() int {
 	minTimes := 15
 	found := false
 	for _, step := range e.status.Steps {
-		success, backoffTimes := e.getBackoffTimes(step.ID)
-		if success && backoffTimes < minTimes {
-			minTimes = backoffTimes
+		if backoffTimes := e.getBackoffTimes(step.ID); backoffTimes > 0 {
 			found = true
+			if backoffTimes < minTimes {
+				minTimes = backoffTimes
+			}
 		}
 		if step.SubStepsStatus != nil {
 			for _, subStep := range step.SubStepsStatus {
-				success, backoffTimes := e.getBackoffTimes(subStep.ID)
-				if success && backoffTimes < minTimes {
-					minTimes = backoffTimes
+				if backoffTimes := e.getBackoffTimes(subStep.ID); backoffTimes > 0 {
 					found = true
+					if backoffTimes < minTimes {
+						minTimes = backoffTimes
+					}
 				}
 			}
 		}

pkg/workflow/workflow_test.go

@@ -912,10 +912,12 @@ var _ = Describe("Test Workflow", func() {
 			Expect(interval).Should(BeEquivalentTo(int(0.05 * math.Pow(2, float64(i+5)))))
 		}
 
-		_, err = wf.ExecuteSteps(ctx, revision, runners)
-		Expect(err).ToNot(HaveOccurred())
-		interval = e.getBackoffWaitTime()
-		Expect(interval).Should(BeEquivalentTo(MaxWorkflowWaitBackoffTime))
+		for i := 0; i < 10; i++ {
+			_, err = wf.ExecuteSteps(ctx, revision, runners)
+			Expect(err).ToNot(HaveOccurred())
+			interval = e.getBackoffWaitTime()
+			Expect(interval).Should(BeEquivalentTo(MaxWorkflowWaitBackoffTime))
+		}
 
 		By("Test get backoff time after clean")
 		wfContext.CleanupMemoryStore(app.Name, app.Namespace)