fix update strategy default handling (#1179)

* fix update strategy default handling

Signed-off-by: Zakhar Dvurechensky <72825626+Zakharden@users.noreply.github.com>

* test update strategy defaults

---------

Signed-off-by: Zakhar Dvurechensky <72825626+Zakharden@users.noreply.github.com>

Author: Zakharden

pkg/templates/updateconfig/template.go

@@ -17,6 +17,8 @@ import (
 	"golang.stackrox.io/kube-linter/pkg/templates"
 	"golang.stackrox.io/kube-linter/pkg/templates/updateconfig/internal/params"
 
+	ocsAppsv1 "github.com/openshift/api/apps/v1"
+	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
@@ -111,6 +113,21 @@ func needsRollingUpdateDefinition(p params.Params) bool {
 		p.MinSurge != "" || p.MaxSurge != "")
 }
 
+func defaultStrategyType(object lintcontext.Object) (string, bool) {
+	switch object.K8sObject.(type) {
+	case *appsv1.Deployment:
+		return string(appsv1.RollingUpdateDeploymentStrategyType), true
+	case *appsv1.DaemonSet:
+		return string(appsv1.RollingUpdateDaemonSetStrategyType), true
+	case *appsv1.StatefulSet:
+		return string(appsv1.RollingUpdateStatefulSetStrategyType), true
+	case *ocsAppsv1.DeploymentConfig:
+		return string(ocsAppsv1.DeploymentStrategyTypeRolling), true
+	default:
+		return "", false
+	}
+}
+
 func init() {
 	templates.Register(check.Template{
 		HumanName:   "Update configuration",
@@ -165,10 +182,16 @@ func init() {
 				if !strategy.TypeExists {
 					return nil
 				}
-				if !compiledRegex.MatchString(strategy.Type) {
+				strategyType := strategy.Type
+				if strategyType == "" {
+					if defaultType, ok := defaultStrategyType(object); ok {
+						strategyType = defaultType
+					}
+				}
+				if !compiledRegex.MatchString(strategyType) {
 					newD := diagnostic.Diagnostic{
 						Message: fmt.Sprintf("object has %s strategy type but must match regex %s",
-							stringutils.Ternary(strategy.Type != "", strategy.Type, "no"), p.StrategyTypeRegex)}
+							stringutils.Ternary(strategyType != "", strategyType, "no"), p.StrategyTypeRegex)}
 					diagnostics = append(diagnostics, newD)
 				}
 				if !strategy.RollingConfigExists {

pkg/templates/updateconfig/template_test.go

@@ -64,7 +64,8 @@ func (s *UpgradeConfigTestSuite) addReplicationControllerWithReplicas(name strin
 
 func (s *UpgradeConfigTestSuite) TestInvalidStrategyType() {
 	const (
-		noExplicitStrategy           = "no-explicit-strategy"
+		noExplicitDeployment         = "no-explicit-deployment-strategy"
+		noExplicitDeploymentConfig   = "no-explicit-deployment-config-strategy"
 		deploymentWithStrategy       = "deployment-strategy-recreate"
 		daemonSetWithStrategy        = "daemon-set-strategy-on-delete"
 		deploymentConfigWithStrategy = "deployment-config-strategy-recreate"
@@ -74,7 +75,8 @@ func (s *UpgradeConfigTestSuite) TestInvalidStrategyType() {
 		deploymentConfigStrategyType = ocsAppsv1.DeploymentStrategyTypeRecreate
 		strategyRegex                = "^(RollingUpdate|Rolling)$"
 	)
-	s.ctx.AddMockDeployment(s.T(), noExplicitStrategy)
+	s.ctx.AddMockDeployment(s.T(), noExplicitDeployment)
+	s.ctx.AddMockDeploymentConfig(s.T(), noExplicitDeploymentConfig)
 	s.addDeploymentWithStrategy(deploymentWithStrategy, appsv1.DeploymentStrategy{Type: deploymentStrategyType})
 	s.addDaemonSetWithStrategy(daemonSetWithStrategy, appsv1.DaemonSetUpdateStrategy{Type: daemonSetStrategyType})
 	s.addDeploymentConfigWithStrategy(deploymentConfigWithStrategy, ocsAppsv1.DeploymentStrategy{Type: deploymentConfigStrategyType})
@@ -89,9 +91,8 @@ func (s *UpgradeConfigTestSuite) TestInvalidStrategyType() {
 				StrategyTypeRegex: strategyRegex,
 			},
 			Diagnostics: map[string][]diagnostic.Diagnostic{
-				noExplicitStrategy: {
-					{Message: fmt.Sprintf("object has no strategy type but must match regex %s", strategyRegex)},
-				},
+				noExplicitDeployment:       {},
+				noExplicitDeploymentConfig: {},
 				deploymentWithStrategy: {
 					{Message: deploymentErrorMsg},
 				},
@@ -108,6 +109,63 @@ func (s *UpgradeConfigTestSuite) TestInvalidStrategyType() {
 	})
 }
 
+func (s *UpgradeConfigTestSuite) TestDefaultStrategyType() {
+	tests := map[string]struct {
+		addObject     func(name string)
+		expectedType  string
+		expectedFound bool
+	}{
+		"deployment": {
+			addObject: func(name string) {
+				s.ctx.AddMockDeployment(s.T(), name)
+			},
+			expectedType:  string(appsv1.RollingUpdateDeploymentStrategyType),
+			expectedFound: true,
+		},
+		"daemon set": {
+			addObject: func(name string) {
+				s.ctx.AddMockDaemonSet(s.T(), name)
+			},
+			expectedType:  string(appsv1.RollingUpdateDaemonSetStrategyType),
+			expectedFound: true,
+		},
+		"stateful set": {
+			addObject: func(name string) {
+				s.ctx.AddObject(name, &appsv1.StatefulSet{})
+			},
+			expectedType:  string(appsv1.RollingUpdateStatefulSetStrategyType),
+			expectedFound: true,
+		},
+		"deployment config": {
+			addObject: func(name string) {
+				s.ctx.AddMockDeploymentConfig(s.T(), name)
+			},
+			expectedType:  string(ocsAppsv1.DeploymentStrategyTypeRolling),
+			expectedFound: true,
+		},
+		"replication controller": {
+			addObject: func(name string) {
+				s.addReplicationControllerWithReplicas(name, 2)
+			},
+			expectedFound: false,
+		},
+	}
+
+	for name, test := range tests {
+		s.Run(name, func() {
+			s.SetupTest()
+			test.addObject(name)
+
+			objects := s.ctx.Objects()
+			s.Require().Len(objects, 1)
+			defaultType, found := defaultStrategyType(objects[0])
+
+			s.Equal(test.expectedFound, found)
+			s.Equal(test.expectedType, defaultType)
+		})
+	}
+}
+
 func (s *UpgradeConfigTestSuite) TestValidStrategyType() {
 	const (
 		deploymentWithStrategy       = "deployment-strategy-recreate"

tests/checks/no-rolling-update-strategy.yml

@@ -13,4 +13,27 @@ metadata:
   name: app2
 spec:
   strategy:
-    type: Other
+    type: Other
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: app3
+spec:
+  selector:
+    matchLabels:
+      app: app3
+  template:
+    metadata:
+      labels:
+        app: app3
+    spec:
+      containers:
+        - name: app
+          image: nginx
+---
+apiVersion: apps.openshift.io/v1
+kind: DeploymentConfig
+metadata:
+  name: app4
+spec: {}