Skip to content

Commit

Permalink
fix: pdb-min-available when Replica number is controlled via HPA (#688)
Browse files Browse the repository at this point in the history
  • Loading branch information
jpedrobf authored Jan 23, 2024
1 parent e0225f4 commit e68f7d1
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 5 deletions.
4 changes: 4 additions & 0 deletions e2etests/bats-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -638,8 +638,12 @@ get_value_from() {


message1=$(get_value_from "${lines[0]}" '.Reports[0].Object.K8sObject.GroupVersionKind.Kind + ": " + .Reports[0].Diagnostic.Message')
message2=$(get_value_from "${lines[0]}" '.Reports[1].Object.K8sObject.GroupVersionKind.Kind + ": " + .Reports[1].Diagnostic.Message')
count=$(get_value_from "${lines[0]}" '.Reports | length')

[[ "${message1}" == "PodDisruptionBudget: The current number of replicas for deployment foo is equal to or lower than the minimum number of replicas specified by its PDB." ]]
[[ "${message2}" == "PodDisruptionBudget: The current number of replicas for deployment foo2 is equal to or lower than the minimum number of replicas specified by its PDB." ]]
[[ "${count}" == "2" ]]
}

@test "privilege-escalation-container" {
Expand Down
16 changes: 16 additions & 0 deletions pkg/extract/hpa_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,19 @@ func checkReplicas(minReplicas *int32) (int32, bool) {
// If numReplicas is a `nil` pointer, then it defaults to 1.
return 1, true
}

// HPAScaleTargetRefName extracts Spec.ScaleTargetRef.Name
func HPAScaleTargetRefName(obj k8sutil.Object) (string, bool) {
switch hpa := obj.(type) {
case *autoscalingV2Beta1.HorizontalPodAutoscaler:
return hpa.Spec.ScaleTargetRef.Name, true
case *autoscalingV2Beta2.HorizontalPodAutoscaler:
return hpa.Spec.ScaleTargetRef.Name, true
case *autoscalingV2.HorizontalPodAutoscaler:
return hpa.Spec.ScaleTargetRef.Name, true
case *autoscalingV1.HorizontalPodAutoscaler:
return hpa.Spec.ScaleTargetRef.Name, true
default:
return "", false
}
}
49 changes: 44 additions & 5 deletions pkg/templates/pdbminavailable/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ func minAvailableCheck(lintCtx lintcontext.LintContext, object lintcontext.Objec
}
}

// Evaluate Deploymet Likes in the lintContext to see if they have MinAvailable set too low
// Builds an HPA map for that namespace in case there is no replicas set on deployment
hpa := getHorizontalPodAutoscalers(lintCtx, pdb.Namespace)

// Evaluate Deployment Likes in the lintContext to see if they have MinAvailable set too low
deploymentLikes, err := getDeploymentLikeObjects(lintCtx, labelSelector, pdb.Namespace)
if err != nil {
return []diagnostic.Diagnostic{
Expand All @@ -103,6 +106,10 @@ func minAvailableCheck(lintCtx lintcontext.LintContext, object lintcontext.Objec
for _, dl := range deploymentLikes {
pdbMinAvailable := value
replicas, _ := extract.Replicas(dl)
if int(replicas) == 1 {
// if replicas number not set on deployment, use HPA MinReplicas
replicas = transformReplicaIntoMinReplicas(dl, hpa, replicas)
}
if isPercent {
// Calulate the actual value of the MinAvailable with respect to the Replica count if a percentage is set
pdbMinAvailable = int(math.Ceil(float64(replicas) * (float64(value) / float64(100))))
Expand Down Expand Up @@ -137,10 +144,6 @@ func getDeploymentLikeObjects(lintCtx lintcontext.LintContext, labelSelector lab
if !exists {
continue
}
// If there are no Replicas set on the Deployment Like, it's not possible to compare to a PDB
if _, ok := extract.Replicas(obj.K8sObject); !ok {
continue
}

objLabelSelector, err := metaV1.LabelSelectorAsSelector(selectors)
if err != nil {
Expand Down Expand Up @@ -177,3 +180,39 @@ func getIntOrPercentValueSafelyFromString(intOrStr string) (int, bool, error) {
}
return v, true, nil
}

// Function to get the list of HPA's provided
func getHorizontalPodAutoscalers(lintCtx lintcontext.LintContext, namespace string) map[string]k8sutil.Object {

m := make(map[string]k8sutil.Object, len(lintCtx.Objects()))

for _, obj := range lintCtx.Objects() {
// Ensure that only HPA objects are processed
if obj.GetK8sObjectName().GroupVersionKind.Kind != objectkinds.HorizontalPodAutoscaler {
continue
}

// Ensure that only HPAs are in the same namespaces as the PDB
if obj.GetK8sObjectName().Namespace != namespace {
continue
}
// validate object with HPA versions using the HPAScaleTargetRefName extractor package function and add to map
hpaSpecScaleTargetRefName, ok := extract.HPAScaleTargetRefName(obj.K8sObject)
if !ok {
continue
}
m[hpaSpecScaleTargetRefName] = obj.K8sObject
}

return m
}

// Function to transform the replica count into the minReplicas count if the deployment has a HPA with a minReplicas set
func transformReplicaIntoMinReplicas(deployment k8sutil.Object, hpaMap map[string]k8sutil.Object, replicas int32) int32 {
hpa := hpaMap[deployment.GetName()]
hpaMinReplicas, ok := extract.HPAMinReplicas(hpa)
if !ok {
return replicas
}
return hpaMinReplicas
}
42 changes: 42 additions & 0 deletions pkg/templates/pdbminavailable/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"golang.stackrox.io/kube-linter/pkg/lintcontext/mocks"
"golang.stackrox.io/kube-linter/pkg/templates"
appsV1 "k8s.io/api/apps/v1"
autoscalingV2 "k8s.io/api/autoscaling/v2"
v1 "k8s.io/api/policy/v1"
metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
Expand Down Expand Up @@ -177,3 +178,44 @@ func (p *PDBTestSuite) TestPDBMinAvailableOneHundredPercent() {
},
})
}

// test that the check run with a deployment that has no replicas and a HPA that has a minReplicas
func (p *PDBTestSuite) TestPDBWithMinAvailableHPA() {
p.ctx.AddMockDeployment(p.T(), "test-deploy")
p.ctx.ModifyDeployment(p.T(), "test-deploy", func(deployment *appsV1.Deployment) {
deployment.Namespace = "test"
deployment.Spec.Replicas = nil
deployment.Spec.Selector = &metaV1.LabelSelector{}
deployment.Spec.Selector.MatchLabels = map[string]string{"foo": "bar"}
})
p.ctx.AddMockHorizontalPodAutoscaler(p.T(), "test-hpa", "v2")
p.ctx.ModifyHorizontalPodAutoscalerV2(p.T(), "test-hpa", func(hpa *autoscalingV2.HorizontalPodAutoscaler) {
hpa.Namespace = "test"
hpa.Spec.ScaleTargetRef = autoscalingV2.CrossVersionObjectReference{
Kind: "Deployment",
Name: "test-deploy",
APIVersion: "apps/v1",
}
hpa.Spec.MinReplicas = nil
})
p.ctx.AddMockPodDisruptionBudget(p.T(), "test-pdb")
p.ctx.ModifyPodDisruptionBudget(p.T(), "test-pdb", func(pdb *v1.PodDisruptionBudget) {
pdb.Namespace = "test"
pdb.Spec.Selector = &metaV1.LabelSelector{}
pdb.Spec.Selector.MatchLabels = map[string]string{"foo": "bar"}
pdb.Spec.MinAvailable = &intstr.IntOrString{StrVal: "50%", Type: intstr.String}
})

p.Validate(p.ctx, []templates.TestCase{
{
Param: params.Params{},
Diagnostics: map[string][]diagnostic.Diagnostic{
"test-pdb": {
{Message: "The current number of replicas for deployment test-deploy is equal to or lower than the minimum number of replicas specified by its PDB."},
},
},
ExpectInstantiationError: false,
},
})

}
41 changes: 41 additions & 0 deletions tests/checks/pdb-min-available.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,44 @@ spec:
selector:
matchLabels:
name: cloud-ingress-operator
---
apiVersion: policy/v1
kind: PodDisruptionBudget
metadata:
name: baz2
namespace: bar2
spec:
minAvailable: 1
selector:
matchLabels:
name: cloud-ingress-operator2
---
apiVersion: autoscaling/v2beta1
kind: HorizontalPodAutoscaler
metadata:
name: app2
namespace: bar2
spec:
minReplicas: 1
maxReplicas: 100
scaleTargetRef:
apiVersion: apps/v1
kind: Deployment
name: foo2
namespace: bar2
targetCPUUtilizationPercentage: 85
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: foo2
namespace: bar2
spec:
replicas: 1
selector:
matchLabels:
name: cloud-ingress-operator2
template:
metadata:
labels:
name: cloud-ingress-operator2

0 comments on commit e68f7d1

Please sign in to comment.