Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: pdb-min-available when Replica number is controlled via HPA #688

Merged
merged 15 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading