fix empty rolloutBatch will panic whole controller bug

pkg/controller/common/rollout/workloads/common.go

@@ -29,6 +29,10 @@ import (
 // verifyBatchesWithRollout verifies that the the sum of all the batch replicas is valid given the total replica
 // each batch replica can be absolute or a percentage
 func verifyBatchesWithRollout(rolloutSpec *v1alpha1.RolloutPlan, totalReplicas int32) error {
+	// If rolloutBatches length equal to zero will cause index out of bounds panic, guarantee don't crash whole vela controller
+	if len(rolloutSpec.RolloutBatches) == 0 {
+		return fmt.Errorf("the rolloutPlan must have batches")
+	}
 	// if not set, the sum of all the batch sizes minus the last batch cannot be more than the totalReplicas
 	totalRollout := 0
 	for i := 0; i < len(rolloutSpec.RolloutBatches)-1; i++ {
@@ -57,6 +61,10 @@ func verifyBatchesWithRollout(rolloutSpec *v1alpha1.RolloutPlan, totalReplicas i
 
 // verifyBatchesWithScale verifies that executing batches finally reach the target size starting from original size
 func verifyBatchesWithScale(rolloutSpec *v1alpha1.RolloutPlan, originalSize, targetSize int) error {
+	// If rolloutBatches length equal to zero will cause index out of bounds panic, guarantee don't crash whole vela controller
+	if len(rolloutSpec.RolloutBatches) == 0 {
+		return fmt.Errorf("the rolloutPlan must have batches")
+	}
 	totalRollout := originalSize
 	for i := 0; i < len(rolloutSpec.RolloutBatches)-1; i++ {
 		rb := rolloutSpec.RolloutBatches[i]

pkg/controller/common/rollout/workloads/common_test.go

@@ -19,6 +19,8 @@ package workloads
 import (
 	"testing"
 
+	"k8s.io/utils/pointer"
+
 	"k8s.io/apimachinery/pkg/util/intstr"
 
 	"github.com/oam-dev/kubevela/apis/standard.oam.dev/v1alpha1"
@@ -217,6 +219,15 @@ func TestVerifyBatchesWithRolloutRelaxed(t *testing.T) {
 	}
 }
 
+func TestVerifyEmptyRolloutBatches(t *testing.T) {
+	plan := &v1alpha1.RolloutPlan{
+		TargetSize: pointer.Int32Ptr(2),
+	}
+	if err := verifyBatchesWithRollout(plan, 3); err == nil {
+		t.Errorf("verifyBatchesWithRollout() = %v, want error", nil)
+	}
+}
+
 func TestVerifyBatchesWithRolloutNumeric(t *testing.T) {
 	// test hard number
 	if err := verifyBatchesWithRollout(rolloutNumericSpec, 6); err == nil {
@@ -372,6 +383,11 @@ func Test_VerifyBatchesWithScaleFailCases(t *testing.T) {
 			originalSize: 16,
 			targetSize:   10,
 		},
+		"empty rollingBatches": {
+			rolloutSpec:  &v1alpha1.RolloutPlan{},
+			targetSize:   3,
+			originalSize: 2,
+		},
 	}
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {

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

@@ -121,6 +121,26 @@ var _ = Describe("Test Application Validator", func() {
 "metadata":{"name":"application-sample","annotations":{"app.oam.dev/rollout-template":"false"}},
 "spec":{"components":[{"type":"worker","properties":{"cmd":["sleep","1000"],"image":"busybox"},
 "traits":[{"type":"scaler","properties":{"replicas":10}}]}]}}
+`),
+				},
+			},
+		}
+		resp := handler.Handle(ctx, req)
+		Expect(resp.Allowed).Should(BeFalse())
+	})
+
+	It("Test Application Validator rolloutPlan [error]", func() {
+		req := admission.Request{
+			AdmissionRequest: admissionv1beta1.AdmissionRequest{
+				Operation: admissionv1beta1.Create,
+				Resource:  metav1.GroupVersionResource{Group: "core.oam.dev", Version: "v1alpha2", Resource: "applications"},
+				Object: runtime.RawExtension{
+					Raw: []byte(`
+{"kind":"Application","metadata":{"name":"test-rolling","annotations":null},
+"spec":{"components":[{"name":"metrics-provider","type":"worker",
+"properties":{"cmd":["./podinfo","stress-cpu=3.0"],
+"image":"stefanprodan/podinfo:4.0.6","port":8080}}],
+"rolloutPlan":{"rolloutStrategy":"IncreaseFirst","targetSize":3}}}
 `),
 				},
 			},

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

@@ -24,6 +24,7 @@ import (
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
 	"github.com/oam-dev/kubevela/pkg/appfile"
 	"github.com/oam-dev/kubevela/pkg/oam"
+	"github.com/oam-dev/kubevela/pkg/webhook/common/rollout"
 )
 
 // ValidateCreate validates the Application on creation
@@ -44,6 +45,9 @@ func (h *ValidatingHandler) ValidateCreate(ctx context.Context, app *v1beta1.App
 	if v := app.GetAnnotations()[oam.AnnotationAppRollout]; len(v) != 0 && v != "true" {
 		componentErrs = append(componentErrs, field.Invalid(field.NewPath("annotation:app.oam.dev/rollout-template"), app, "the annotation value of rollout-template must be true"))
 	}
+	if app.Spec.RolloutPlan != nil {
+		componentErrs = append(componentErrs, rollout.ValidateCreate(h.Client, app.Spec.RolloutPlan, field.NewPath("rolloutPlan"))...)
+	}
 	return componentErrs
 }
 

test/e2e-test/application_test.go

@@ -165,4 +165,12 @@ var _ = Describe("Application Normal tests", func() {
 		newApp.Namespace = namespaceName
 		Expect(k8sClient.Create(ctx, &newApp)).ShouldNot(BeNil())
 	})
+
+	It("Test app have empty rollingBatches rolloutPlan", func() {
+		By("Apply an application")
+		var newApp v1beta1.Application
+		Expect(common.ReadYamlToObject("testdata/app/app6.yaml", &newApp)).Should(BeNil())
+		newApp.Namespace = namespaceName
+		Expect(k8sClient.Create(ctx, &newApp)).ShouldNot(BeNil())
+	})
 })

test/e2e-test/testdata/app/app6.yaml

@@ -0,0 +1,17 @@
+apiVersion: core.oam.dev/v1beta1
+kind: Application
+metadata:
+  name: test-rolling
+spec:
+  components:
+    - name: metrics-provider
+      type: worker
+      properties:
+        cmd:
+          - ./podinfo
+          - stress-cpu=3.0
+        image: stefanprodan/podinfo:4.0.6
+        port: 8080
+  rolloutPlan:
+    rolloutStrategy: "IncreaseFirst"
+    targetSize: 3