From 400c754e9a8214b79266cd4c4741781ecb4ae43f Mon Sep 17 00:00:00 2001 From: Charlie OConor Date: Mon, 27 Nov 2023 13:35:27 -0600 Subject: [PATCH 1/4] Create new check for liveness port Want to ensure the port is correctly set for liveness probe since it's easy to make a typo when people are doing copy pasta. This will let you know it's failing airier and not have to wait for a failed deployment. Want to do this also for startup and readiness checks. Assume it make sense to have them as other checks and was planning on doing that in a follow up. --- docs/generated/checks.md | 9 ++ docs/generated/templates.md | 9 ++ e2etests/bats-tests.sh | 23 ++- internal/defaultchecks/default_checks.go | 1 + pkg/builtinchecks/yamls/liveness-port.yaml | 9 ++ pkg/templates/all/all.go | 1 + .../internal/params/gen-params.go | 52 +++++++ .../livenessport/internal/params/params.go | 4 + pkg/templates/livenessport/template.go | 57 +++++++ pkg/templates/livenessport/template_test.go | 141 ++++++++++++++++++ pkg/templates/livenessprobe/template.go | 3 +- tests/checks/invalid-target-ports.yaml | 2 +- tests/checks/liveness-port.yml | 59 ++++++++ 13 files changed, 366 insertions(+), 4 deletions(-) create mode 100644 pkg/builtinchecks/yamls/liveness-port.yaml create mode 100644 pkg/templates/livenessport/internal/params/gen-params.go create mode 100644 pkg/templates/livenessport/internal/params/params.go create mode 100644 pkg/templates/livenessport/template.go create mode 100644 pkg/templates/livenessport/template_test.go create mode 100644 tests/checks/liveness-port.yml diff --git a/docs/generated/checks.md b/docs/generated/checks.md index 8b26682f6..e5b1e3dc1 100644 --- a/docs/generated/checks.md +++ b/docs/generated/checks.md @@ -295,6 +295,15 @@ BlockList: - ^[^:]*$ - (.*/[^:]+)$ ``` +## liveness-port + +**Enabled by default**: Yes + +**Description**: Indicates when containers have a liveness probe to a port the container didn't open. + +**Remediation**: Check which ports you've opened and ensure they match what you have specified in the liveness probe. + +**Template**: [liveness-http-port](templates.md#liveness-port-not-open) ## minimum-three-replicas **Enabled by default**: No diff --git a/docs/generated/templates.md b/docs/generated/templates.md index 569e0c059..422656c33 100644 --- a/docs/generated/templates.md +++ b/docs/generated/templates.md @@ -444,6 +444,15 @@ KubeLinter supports the following templates: type: array ``` +## Liveness Port Not Open + +**Key**: `liveness-http-port` + +**Description**: Flag containers have a http liveness prop with a port they didn't open. + +**Supported Objects**: DeploymentLike + + ## Liveness Probe Not Specified **Key**: `liveness-probe` diff --git a/e2etests/bats-tests.sh b/e2etests/bats-tests.sh index c9b03eba3..39b27ca6a 100755 --- a/e2etests/bats-tests.sh +++ b/e2etests/bats-tests.sh @@ -496,6 +496,25 @@ get_value_from() { [[ "${count}" == "2" ]] } +@test "liveness-port" { + tmp="tests/checks/liveness-port.yml" + cmd="${KUBE_LINTER_BIN} lint --include liveness-port --do-not-auto-add-defaults --format json ${tmp}" + run ${cmd} + + print_info "${status}" "${output}" "${cmd}" "${tmp}" + [ "$status" -eq 1 ] + + 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') + count=$(get_value_from "${lines[0]}" '.Reports | length') + + [[ "${message1}" == "Deployment: container \"fire-deployment-name\" does not have an open port 8080" ]] + [[ "${message2}" == "StatefulSet: container \"fire-deployment-int\" does not have an open port heath" ]] + [[ "${message3}" == "StatefulSet: container \"fire-stateful-name\" does not have an open port healthcheck" ]] + [[ "${count}" == "3" ]] +} + @test "no-node-affinity" { tmp="tests/checks/no-node-affinity.yml" cmd="${KUBE_LINTER_BIN} lint --include no-node-affinity --do-not-auto-add-defaults --format json ${tmp}" @@ -603,7 +622,7 @@ get_value_from() { run ${cmd} message1=$(get_value_from "${lines[0]}" '.Reports[0].Object.K8sObject.GroupVersionKind.Kind + ": " + .Reports[0].Diagnostic.Message') - + [[ "${message1}" == "PodDisruptionBudget: MaxUnavailable is set to 0" ]] } @@ -614,7 +633,7 @@ get_value_from() { cmd="${KUBE_LINTER_BIN} lint --include pdb-min-available --do-not-auto-add-defaults --format json ${tmp}" run ${cmd} - + message1=$(get_value_from "${lines[0]}" '.Reports[0].Object.K8sObject.GroupVersionKind.Kind + ": " + .Reports[0].Diagnostic.Message') [[ "${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." ]] diff --git a/internal/defaultchecks/default_checks.go b/internal/defaultchecks/default_checks.go index 513e7c978..fecc552f6 100644 --- a/internal/defaultchecks/default_checks.go +++ b/internal/defaultchecks/default_checks.go @@ -18,6 +18,7 @@ var ( "host-pid", "invalid-target-ports", "latest-tag", + "liveness-port", "mismatching-selector", "no-anti-affinity", "no-extensions-v1beta", diff --git a/pkg/builtinchecks/yamls/liveness-port.yaml b/pkg/builtinchecks/yamls/liveness-port.yaml new file mode 100644 index 000000000..8e1e0cda2 --- /dev/null +++ b/pkg/builtinchecks/yamls/liveness-port.yaml @@ -0,0 +1,9 @@ +name: "liveness-port" +description: "Indicates when containers have a liveness probe to a port the container didn't open." +remediation: >- + Check which ports you've opened and ensure they match what you have specified + in the liveness probe. +scope: + objectKinds: + - DeploymentLike +template: "liveness-http-port" diff --git a/pkg/templates/all/all.go b/pkg/templates/all/all.go index e29ed9639..e2192dac6 100644 --- a/pkg/templates/all/all.go +++ b/pkg/templates/all/all.go @@ -26,6 +26,7 @@ import ( _ "golang.stackrox.io/kube-linter/pkg/templates/hpareplicas" _ "golang.stackrox.io/kube-linter/pkg/templates/imagepullpolicy" _ "golang.stackrox.io/kube-linter/pkg/templates/latesttag" + _ "golang.stackrox.io/kube-linter/pkg/templates/livenessport" _ "golang.stackrox.io/kube-linter/pkg/templates/livenessprobe" _ "golang.stackrox.io/kube-linter/pkg/templates/memoryrequirements" _ "golang.stackrox.io/kube-linter/pkg/templates/mismatchingselector" diff --git a/pkg/templates/livenessport/internal/params/gen-params.go b/pkg/templates/livenessport/internal/params/gen-params.go new file mode 100644 index 000000000..21bb3aa5c --- /dev/null +++ b/pkg/templates/livenessport/internal/params/gen-params.go @@ -0,0 +1,52 @@ +// Code generated by kube-linter template codegen. DO NOT EDIT. +// +build !templatecodegen + +package params + +import ( + "fmt" + "strings" + + "github.com/pkg/errors" + "golang.stackrox.io/kube-linter/pkg/check" + "golang.stackrox.io/kube-linter/pkg/templates/util" +) + +var ( + // Use some imports in case they don't get used otherwise. + _ = util.MustParseParameterDesc + _ = fmt.Sprintf + + ParamDescs = []check.ParameterDesc{ + } +) + +func (p *Params) Validate() error { + var validationErrors []string + if len(validationErrors) > 0 { + return errors.Errorf("invalid parameters: %s", strings.Join(validationErrors, ", ")) + } + return nil +} + +// ParseAndValidate instantiates a Params object out of the passed map[string]interface{}, +// validates it, and returns it. +// The return type is interface{} to satisfy the type in the Template struct. +func ParseAndValidate(m map[string]interface{}) (interface{}, error) { + var p Params + if err := util.DecodeMapStructure(m, &p); err != nil { + return nil, err + } + if err := p.Validate(); err != nil { + return nil, err + } + return p, nil +} + +// WrapInstantiateFunc is a convenience wrapper that wraps an untyped instantiate function +// into a typed one. +func WrapInstantiateFunc(f func(p Params) (check.Func, error)) func (interface{}) (check.Func, error) { + return func(paramsInt interface{}) (check.Func, error) { + return f(paramsInt.(Params)) + } +} diff --git a/pkg/templates/livenessport/internal/params/params.go b/pkg/templates/livenessport/internal/params/params.go new file mode 100644 index 000000000..2bb6037f6 --- /dev/null +++ b/pkg/templates/livenessport/internal/params/params.go @@ -0,0 +1,4 @@ +package params + +// Params represents the params accepted by this template. +type Params struct{} diff --git a/pkg/templates/livenessport/template.go b/pkg/templates/livenessport/template.go new file mode 100644 index 000000000..1e5ed074e --- /dev/null +++ b/pkg/templates/livenessport/template.go @@ -0,0 +1,57 @@ +package livenessport + +import ( + "fmt" + + "golang.stackrox.io/kube-linter/pkg/check" + "golang.stackrox.io/kube-linter/pkg/config" + "golang.stackrox.io/kube-linter/pkg/diagnostic" + "golang.stackrox.io/kube-linter/pkg/objectkinds" + "golang.stackrox.io/kube-linter/pkg/templates" + "golang.stackrox.io/kube-linter/pkg/templates/livenessport/internal/params" + "golang.stackrox.io/kube-linter/pkg/templates/util" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +const templateKey = "liveness-http-port" + +var sentinal = 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.", + SupportedObjectKinds: config.ObjectKindsDesc{ + ObjectKinds: []string{objectkinds.DeploymentLike}, + }, + Parameters: params.ParamDescs, + ParseAndValidateParams: params.ParseAndValidate, + Instantiate: params.WrapInstantiateFunc(func(_ params.Params) (check.Func, error) { + return util.PerNonInitContainerCheck(func(container *v1.Container) []diagnostic.Diagnostic { + if container.LivenessProbe == nil { + 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 _, 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()), + }} + } + return nil + }), nil + }), + }) +} diff --git a/pkg/templates/livenessport/template_test.go b/pkg/templates/livenessport/template_test.go new file mode 100644 index 000000000..08755a987 --- /dev/null +++ b/pkg/templates/livenessport/template_test.go @@ -0,0 +1,141 @@ +package livenessport + +import ( + "testing" + + "github.com/stretchr/testify/suite" + "golang.stackrox.io/kube-linter/pkg/diagnostic" + "golang.stackrox.io/kube-linter/pkg/lintcontext/mocks" + "golang.stackrox.io/kube-linter/pkg/templates" + "golang.stackrox.io/kube-linter/pkg/templates/livenessport/internal/params" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +func TestMissingLivenessPort(t *testing.T) { + suite.Run(t, new(MissingLivenessPort)) +} + +type MissingLivenessPort struct { + templates.TemplateTestSuite + + ctx *mocks.MockLintContext +} + +func (s *MissingLivenessPort) SetupTest() { + s.Init(templateKey) + s.ctx = mocks.NewMockContext() +} + +func (s *MissingLivenessPort) TestDeploymentWith() { + const targetName = "deployment01" + testCases := []struct { + name string + container v1.Container + expected map[string][]diagnostic.Diagnostic + }{ + { + name: "NoLivenessProbe", + container: v1.Container{}, + expected: nil, + }, + { + name: "MatchinPortInt", + container: v1.Container{ + Ports: []v1.ContainerPort{ + { + Name: "http", + ContainerPort: 8080, + }, + }, + LivenessProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Port: intstr.FromInt(8080), + }, + }, + }, + }, + expected: nil, + }, + { + name: "MatchinPortStr", + container: v1.Container{ + Ports: []v1.ContainerPort{ + { + Name: "http", + ContainerPort: 8080, + }, + }, + LivenessProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Port: intstr.FromString("http"), + }, + }, + }, + }, + expected: nil, + }, + { + name: "MismaptchPort", + container: v1.Container{ + Name: "container", + Ports: []v1.ContainerPort{ + { + Name: "http", + ContainerPort: 8080, + }, + }, + LivenessProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Port: intstr.FromInt(9999), + }, + }, + }, + }, + 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"}, + }, + }, + }, + { + name: "MismaptchPort", + container: v1.Container{ + Name: "container", + Ports: []v1.ContainerPort{ + { + Name: "http", + ContainerPort: 8080, + }, + }, + LivenessProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Port: intstr.FromString("healthcheck"), + }, + }, + }, + }, + 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"}, + }, + }, + }, + } + for _, tc := range testCases { + s.Run(tc.name, func() { + s.ctx.AddMockDeployment(s.T(), targetName) + s.ctx.AddContainerToDeployment(s.T(), targetName, tc.container) + s.Validate(s.ctx, []templates.TestCase{{ + Param: params.Params{}, + Diagnostics: tc.expected, + }}) + }) + } +} diff --git a/pkg/templates/livenessprobe/template.go b/pkg/templates/livenessprobe/template.go index b0491668a..52474a9b9 100644 --- a/pkg/templates/livenessprobe/template.go +++ b/pkg/templates/livenessprobe/template.go @@ -21,7 +21,8 @@ func init() { SupportedObjectKinds: config.ObjectKindsDesc{ ObjectKinds: []string{objectkinds.DeploymentLike}, }, - Parameters: params.ParamDescs, + Parameters: params.ParamDescs, + ParseAndValidateParams: params.ParseAndValidate, Instantiate: params.WrapInstantiateFunc(func(_ params.Params) (check.Func, error) { return util.PerNonInitContainerCheck(func(container *v1.Container) []diagnostic.Diagnostic { diff --git a/tests/checks/invalid-target-ports.yaml b/tests/checks/invalid-target-ports.yaml index 775f9fa9e..a553e7ba7 100644 --- a/tests/checks/invalid-target-ports.yaml +++ b/tests/checks/invalid-target-ports.yaml @@ -1,4 +1,4 @@ -apiVersion: v1 +napiVersion: v1 kind: Service metadata: namespace: test diff --git a/tests/checks/liveness-port.yml b/tests/checks/liveness-port.yml new file mode 100644 index 000000000..8b48cc7bf --- /dev/null +++ b/tests/checks/liveness-port.yml @@ -0,0 +1,59 @@ +apiVersion: apps/v1 +kind: Deployment +spec: + template: + spec: + containers: + - name: dont-fire-deployment + ports: + - containerPort: 8080 + name: http + protocol: TCP + livenessProbe: + httpGet: + path: "/" + port: http + - name: dont-fire-deployment + ports: + - containerPort: 8080 + name: http + protocol: TCP + livenessProbe: + httpGet: + path: "/" + port: 8080 + - name: fire-deployment-name + livenessProbe: + httpGet: + path: "/" + port: http + - name: fire-deployment-int + ports: + - containerPort: 9999 + name: http + protocol: TCP + livenessProbe: + httpGet: + path: "/" + port: 8080 +--- +apiVersion: apps/v1 +kind: StatefulSet +spec: + template: + spec: + containers: + - name: dont-fire-stateful + ports: + - containerPort: 8080 + name: http + protocol: TCP + - name: fire-stateful-name + ports: + - containerPort: 9999 + name: http + protocol: TCP + livenessProbe: + httpGet: + path: "/" + port: healthcheck From 8446ded319708c3154dcbc7a5365c02488fbf3ff Mon Sep 17 00:00:00 2001 From: Charlie OConor Date: Fri, 17 Nov 2023 16:18:24 -0600 Subject: [PATCH 2/4] Fix e2e bats tests --- e2etests/bats-tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2etests/bats-tests.sh b/e2etests/bats-tests.sh index 39b27ca6a..9905acd63 100755 --- a/e2etests/bats-tests.sh +++ b/e2etests/bats-tests.sh @@ -509,8 +509,8 @@ get_value_from() { message3=$(get_value_from "${lines[0]}" '.Reports[2].Object.K8sObject.GroupVersionKind.Kind + ": " + .Reports[2].Diagnostic.Message') count=$(get_value_from "${lines[0]}" '.Reports | length') - [[ "${message1}" == "Deployment: container \"fire-deployment-name\" does not have an open port 8080" ]] - [[ "${message2}" == "StatefulSet: container \"fire-deployment-int\" does not have an open port heath" ]] + [[ "${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" ]] } From 97b2cd9d34cec14c8aa5e9cdbdc0d26bb5d1b576 Mon Sep 17 00:00:00 2001 From: Charlie OConor Date: Mon, 27 Nov 2023 13:03:38 -0600 Subject: [PATCH 3/4] 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 From 72124055e87db04dc2255bce55c119deb83edf82 Mon Sep 17 00:00:00 2001 From: Charlie OConor Date: Thu, 14 Dec 2023 20:08:51 -0600 Subject: [PATCH 4/4] sort bats test --- e2etests/bats-tests.sh | 43 +++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/e2etests/bats-tests.sh b/e2etests/bats-tests.sh index af8667180..832d08504 100755 --- a/e2etests/bats-tests.sh +++ b/e2etests/bats-tests.sh @@ -411,6 +411,28 @@ get_value_from() { [[ "${count}" == "2" ]] } +@test "liveness-port" { + tmp="tests/checks/liveness-port.yml" + cmd="${KUBE_LINTER_BIN} lint --include liveness-port --do-not-auto-add-defaults --format json ${tmp}" + run ${cmd} + + print_info "${status}" "${output}" "${cmd}" "${tmp}" + [ "$status" -eq 1 ] + + 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 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 "minimum-three-replicas" { tmp="tests/checks/minimum-three-replicas.yml" cmd="${KUBE_LINTER_BIN} lint --include minimum-three-replicas --do-not-auto-add-defaults --format json ${tmp}" @@ -496,27 +518,6 @@ get_value_from() { [[ "${count}" == "2" ]] } -@test "liveness-port" { - tmp="tests/checks/liveness-port.yml" - cmd="${KUBE_LINTER_BIN} lint --include liveness-port --do-not-auto-add-defaults --format json ${tmp}" - run ${cmd} - - print_info "${status}" "${output}" "${cmd}" "${tmp}" - [ "$status" -eq 1 ] - - 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 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" { tmp="tests/checks/no-node-affinity.yml" cmd="${KUBE_LINTER_BIN} lint --include no-node-affinity --do-not-auto-add-defaults --format json ${tmp}"