From ec54b69f8f0b4aac2a04710afa40728e9076ca47 Mon Sep 17 00:00:00 2001 From: Charlie OConor Date: Mon, 27 Nov 2023 13:03:38 -0600 Subject: [PATCH] Review comments --- docs/generated/checks.md | 2 +- docs/generated/templates.md | 2 +- e2etests/bats-tests.sh | 10 ++- pkg/builtinchecks/yamls/liveness-port.yaml | 2 +- pkg/templates/livenessport/template.go | 34 ++++---- pkg/templates/livenessport/template_test.go | 86 ++++++++++++++++++++- tests/checks/invalid-target-ports.yaml | 2 +- tests/checks/liveness-port.yml | 9 ++- 8 files changed, 121 insertions(+), 26 deletions(-) diff --git a/docs/generated/checks.md b/docs/generated/checks.md index e5b1e3dc1..43f30dccb 100644 --- a/docs/generated/checks.md +++ b/docs/generated/checks.md @@ -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. diff --git a/docs/generated/templates.md b/docs/generated/templates.md index 422656c33..a37d16689 100644 --- a/docs/generated/templates.md +++ b/docs/generated/templates.md @@ -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 diff --git a/e2etests/bats-tests.sh b/e2etests/bats-tests.sh index 9905acd63..af8667180 100755 --- a/e2etests/bats-tests.sh +++ b/e2etests/bats-tests.sh @@ -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" { diff --git a/pkg/builtinchecks/yamls/liveness-port.yaml b/pkg/builtinchecks/yamls/liveness-port.yaml index 8e1e0cda2..53089ba0e 100644 --- a/pkg/builtinchecks/yamls/liveness-port.yaml +++ b/pkg/builtinchecks/yamls/liveness-port.yaml @@ -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. diff --git a/pkg/templates/livenessport/template.go b/pkg/templates/livenessport/template.go index 1e5ed074e..f5f9e80f6 100644 --- a/pkg/templates/livenessport/template.go +++ b/pkg/templates/livenessport/template.go @@ -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}, }, @@ -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 diff --git a/pkg/templates/livenessport/template_test.go b/pkg/templates/livenessport/template_test.go index 08755a987..05b4b1001 100644 --- a/pkg/templates/livenessport/template_test.go +++ b/pkg/templates/livenessport/template_test.go @@ -39,6 +39,17 @@ 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{ @@ -46,6 +57,7 @@ func (s *MissingLivenessPort) TestDeploymentWith() { { Name: "http", ContainerPort: 8080, + Protocol: v1.ProtocolTCP, }, }, LivenessProbe: &v1.Probe{ @@ -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{ @@ -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"}, }, }, }, @@ -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"}, }, }, }, diff --git a/tests/checks/invalid-target-ports.yaml b/tests/checks/invalid-target-ports.yaml index a553e7ba7..775f9fa9e 100644 --- a/tests/checks/invalid-target-ports.yaml +++ b/tests/checks/invalid-target-ports.yaml @@ -1,4 +1,4 @@ -napiVersion: v1 +apiVersion: v1 kind: Service metadata: namespace: test diff --git a/tests/checks/liveness-port.yml b/tests/checks/liveness-port.yml index 8b48cc7bf..f8340cd1a 100644 --- a/tests/checks/liveness-port.yml +++ b/tests/checks/liveness-port.yml @@ -8,7 +8,6 @@ spec: ports: - containerPort: 8080 name: http - protocol: TCP livenessProbe: httpGet: path: "/" @@ -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