Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
charlesoconor committed Nov 27, 2023
1 parent a0ed4cd commit ec54b69
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 26 deletions.
2 changes: 1 addition & 1 deletion docs/generated/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ BlockList:
**Enabled by default**: Yes
**Description**: Indicates when containers have a liveness probe to a port the container didn't open.
**Description**: Indicates when containers have a liveness probe to a not exposed port.
**Remediation**: Check which ports you've opened and ensure they match what you have specified in the liveness probe.
Expand Down
2 changes: 1 addition & 1 deletion docs/generated/templates.md
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ KubeLinter supports the following templates:

**Key**: `liveness-http-port`

**Description**: Flag containers have a http liveness prop with a port they didn't open.
**Description**: Flag containers with an HTTP liveness probe to not exposed port.

**Supported Objects**: DeploymentLike

Expand Down
10 changes: 6 additions & 4 deletions e2etests/bats-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -507,12 +507,14 @@ 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')
message3=$(get_value_from "${lines[0]}" '.Reports[2].Object.K8sObject.GroupVersionKind.Kind + ": " + .Reports[2].Diagnostic.Message')
message4=$(get_value_from "${lines[0]}" '.Reports[3].Object.K8sObject.GroupVersionKind.Kind + ": " + .Reports[3].Diagnostic.Message')
count=$(get_value_from "${lines[0]}" '.Reports | length')

[[ "${message1}" == "Deployment: container \"fire-deployment-name\" does not have an open port http" ]]
[[ "${message2}" == "Deployment: container \"fire-deployment-int\" does not have an open port 8080" ]]
[[ "${message3}" == "StatefulSet: container \"fire-stateful-name\" does not have an open port healthcheck" ]]
[[ "${count}" == "3" ]]
[[ "${message1}" == "Deployment: container \"fire-deployment-name\" does not expose port http for the HTTPGet" ]]
[[ "${message2}" == "Deployment: container \"fire-deployment-int\" does not expose port 8080 for the HTTPGet" ]]
[[ "${message3}" == "Deployment: container \"fire-deployment-udp\" does not expose port udp for the TCPSocket" ]]
[[ "${message4}" == "StatefulSet: container \"fire-stateful-name\" does not expose port healthcheck for the HTTPGet" ]]
[[ "${count}" == "4" ]]
}

@test "no-node-affinity" {
Expand Down
2 changes: 1 addition & 1 deletion pkg/builtinchecks/yamls/liveness-port.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: "liveness-port"
description: "Indicates when containers have a liveness probe to a port the container didn't open."
description: "Indicates when containers have a liveness probe to a not exposed port."
remediation: >-
Check which ports you've opened and ensure they match what you have specified
in the liveness probe.
Expand Down
34 changes: 21 additions & 13 deletions pkg/templates/livenessport/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ import (

const templateKey = "liveness-http-port"

var sentinal = struct{}{}
var sentinel = struct{}{}

func init() {
templates.Register(check.Template{
HumanName: "Liveness Port Not Open",
Key: templateKey,
Description: "Flag containers have a http liveness prop with a port they didn't open.",
Description: "Flag containers with an HTTP liveness probe to not exposed port.",
SupportedObjectKinds: config.ObjectKindsDesc{
ObjectKinds: []string{objectkinds.DeploymentLike},
},
Expand All @@ -34,21 +34,29 @@ func init() {
return nil
}

httpProbe := container.LivenessProbe.ProbeHandler.HTTPGet
if httpProbe == nil {
return nil
}

ports := map[intstr.IntOrString]struct{}{}
for _, port := range container.Ports {
ports[intstr.FromInt(int(port.ContainerPort))] = sentinal
ports[intstr.FromString(port.Name)] = sentinal
if port.Protocol != "" && port.Protocol != v1.ProtocolTCP {
continue
}
ports[intstr.FromInt32(port.ContainerPort)] = sentinel
ports[intstr.FromString(port.Name)] = sentinel
}

if httpProbe := container.LivenessProbe.ProbeHandler.HTTPGet; httpProbe != nil {
if _, ok := ports[httpProbe.Port]; !ok {
return []diagnostic.Diagnostic{{
Message: fmt.Sprintf("container %q does not expose port %s for the HTTPGet", container.Name, httpProbe.Port.String()),
}}
}
}

if _, ok := ports[httpProbe.Port]; !ok {
return []diagnostic.Diagnostic{{
Message: fmt.Sprintf("container %q does not have an open port %s", container.Name, httpProbe.Port.String()),
}}
if tcpProbe := container.LivenessProbe.ProbeHandler.TCPSocket; tcpProbe != nil {
if _, ok := ports[tcpProbe.Port]; !ok {
return []diagnostic.Diagnostic{{
Message: fmt.Sprintf("container %q does not expose port %s for the TCPSocket", container.Name, tcpProbe.Port.String()),
}}
}
}
return nil
}), nil
Expand Down
86 changes: 82 additions & 4 deletions pkg/templates/livenessport/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,25 @@ func (s *MissingLivenessPort) TestDeploymentWith() {
container: v1.Container{},
expected: nil,
},
{
name: "NoLivenessProbeExecIgnored",
container: v1.Container{
LivenessProbe: &v1.Probe{
ProbeHandler: v1.ProbeHandler{
Exec: &v1.ExecAction{},
},
},
},
expected: nil,
},
{
name: "MatchinPortInt",
container: v1.Container{
Ports: []v1.ContainerPort{
{
Name: "http",
ContainerPort: 8080,
Protocol: v1.ProtocolTCP,
},
},
LivenessProbe: &v1.Probe{
Expand Down Expand Up @@ -77,6 +89,25 @@ func (s *MissingLivenessPort) TestDeploymentWith() {
},
expected: nil,
},
{
name: "MatchinPortStrTCPSSocket",
container: v1.Container{
Ports: []v1.ContainerPort{
{
Name: "http",
ContainerPort: 8080,
},
},
LivenessProbe: &v1.Probe{
ProbeHandler: v1.ProbeHandler{
TCPSocket: &v1.TCPSocketAction{
Port: intstr.FromString("http"),
},
},
},
},
expected: nil,
},
{
name: "MismaptchPort",
container: v1.Container{
Expand All @@ -97,8 +128,7 @@ func (s *MissingLivenessPort) TestDeploymentWith() {
},
expected: map[string][]diagnostic.Diagnostic{
targetName: {
// Ensure we only get one error for ENV_1
{Message: "container \"container\" does not have an open port 9999"},
{Message: "container \"container\" does not expose port 9999 for the HTTPGet"},
},
},
},
Expand All @@ -122,8 +152,56 @@ func (s *MissingLivenessPort) TestDeploymentWith() {
},
expected: map[string][]diagnostic.Diagnostic{
targetName: {
// Ensure we only get one error for ENV_1
{Message: "container \"container\" does not have an open port healthcheck"},
{Message: "container \"container\" does not expose port healthcheck for the HTTPGet"},
},
},
},
{
name: "MatchinPortUDP",
container: v1.Container{
Name: "container",
Ports: []v1.ContainerPort{
{
Name: "http",
ContainerPort: 8080,
Protocol: v1.ProtocolUDP,
},
},
LivenessProbe: &v1.Probe{
ProbeHandler: v1.ProbeHandler{
HTTPGet: &v1.HTTPGetAction{
Port: intstr.FromInt(8080),
},
},
},
},
expected: map[string][]diagnostic.Diagnostic{
targetName: {
{Message: "container \"container\" does not expose port 8080 for the HTTPGet"},
},
},
},
{
name: "MismaptchPortTCPSocket",
container: v1.Container{
Name: "container",
Ports: []v1.ContainerPort{
{
Name: "http",
ContainerPort: 8080,
},
},
LivenessProbe: &v1.Probe{
ProbeHandler: v1.ProbeHandler{
TCPSocket: &v1.TCPSocketAction{
Port: intstr.FromString("socket"),
},
},
},
},
expected: map[string][]diagnostic.Diagnostic{
targetName: {
{Message: "container \"container\" does not expose port socket for the TCPSocket"},
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion tests/checks/invalid-target-ports.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
napiVersion: v1
apiVersion: v1
kind: Service
metadata:
namespace: test
Expand Down
9 changes: 8 additions & 1 deletion tests/checks/liveness-port.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ spec:
ports:
- containerPort: 8080
name: http
protocol: TCP
livenessProbe:
httpGet:
path: "/"
Expand Down Expand Up @@ -36,6 +35,14 @@ spec:
httpGet:
path: "/"
port: 8080
- name: fire-deployment-udp
ports:
- containerPort: 9999
name: udp
protocol: UDP
livenessProbe:
tcpSocket:
port: udp
---
apiVersion: apps/v1
kind: StatefulSet
Expand Down

0 comments on commit ec54b69

Please sign in to comment.