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.

Sign up for GitHub

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

Jump to bottom

feat(checks): remove checks for cpu and memory limits #712

Closed
wants to merge 1 commit into from
+17 −48
Closed

feat(checks): remove checks for cpu and memory limits #712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ source-code-archive:

.PHONY: test
test:
echo $(TAG)
go test ./...

.PHONY: e2e-test
Expand Down
12 changes: 2 additions & 10 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,11 @@ Running KubeLinter to Lint your YAML files only requires two steps in its most b

### Example

Consider the following sample pod specification file `pod.yaml`. This file has two production readiness issues and one security issue:
Consider the following sample pod specification file `pod.yaml`. This file has one security issue:

**Security Issue:**
1. The container in this pod is not running as a read only file system, which could allow it to write to the root filesystem.

**Production readiness:**
1. The container's CPU limits are not set, which could allow it to consume excessive CPU.
1. The container's memory limits are not set, which could allow it to consume excessive memory

```yaml
apiVersion: v1
kind: Pod
Expand Down Expand Up @@ -172,12 +168,8 @@ Consider the following sample pod specification file `pod.yaml`. This file has t

```
pod.yaml: (object: <no namespace>/security-context-demo /v1, Kind=Pod) container "sec-ctx-demo" does not have a read-only root file system (check: no-read-only-root-fs, remediation: Set readOnlyRootFilesystem to true in your container's securityContext.)

pod.yaml: (object: <no namespace>/security-context-demo /v1, Kind=Pod) container "sec-ctx-demo" has cpu limit 0 (check: unset-cpu-requirements, remediation: Set your container's CPU requests and limits depending on its requirements. See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ #requests-and-limits for more details.)

pod.yaml: (object: <no namespace>/security-context-demo /v1, Kind=Pod) container "sec-ctx-demo" has memory limit 0 (check: unset-memory-requirements, remediation: Set your container's memory requests and limits depending on its requirements. See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ #requests-and-limits for more details.)

Error: found 3 lint errors
Error: found 1 lint error
```
To learn more about using and configuring KubeLinter, visit the [documentation](./docs) page.

Expand Down
14 changes: 2 additions & 12 deletions docs/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,6 @@ COSIGN_EXPERIMENTAL=1 cosign verify $IMAGE_NAME
> - The container in this pod is not running as a read-only file system,
> allowing it to write to the root filesystem.
>
> **Production readiness issue**
> - The configuration doesn't specify the container's CPU limits,
> allowing it to consume excessive CPU.
> - The configuration doesn't specify the container's memory limits,
> allowing it to consume excessive memory.

1. To lint this file with KubeLinter, run the following command:
```bash
Expand All @@ -188,11 +183,7 @@ COSIGN_EXPERIMENTAL=1 cosign verify $IMAGE_NAME
```
pod.yaml: (object: <no namespace>/security-context-demo /v1, Kind=Pod) container "sec-ctx-demo" does not have a read-only root file system (check: no-read-only-root-fs, remediation: Set readOnlyRootFilesystem to true in your container's securityContext.)

pod.yaml: (object: <no namespace>/security-context-demo /v1, Kind=Pod) container "sec-ctx-demo" has cpu limit 0 (check: unset-cpu-requirements, remediation: Set your container's CPU requests and limits depending on its requirements. See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for more details.)

pod.yaml: (object: <no namespace>/security-context-demo /v1, Kind=Pod) container "sec-ctx-demo" has memory limit 0 (check: unset-memory-requirements, remediation: Set your container's memory requests and limits depending on its requirements. See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for more details.)

Error: found 3 lint errors
Error: found 1 lint error
```


Expand All @@ -216,11 +207,10 @@ chart:

helm-chart-sample/helm-chart-sample/templates/tests/test-connection.yaml: (object: <no namespace>/test-release-helm-chart-sample-test-connection /v1, Kind=Pod) container "wget" is not set to runAsNonRoot (check: run-as-non-root, remediation: Set runAsUser to a non-zero number, and runAsNonRoot to true, in your pod or container securityContext. See https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ for more details.)

helm-chart-sample/helm-chart-sample/templates/tests/test-connection.yaml: (object: <no namespace>/test-release-helm-chart-sample-test-connection /v1, Kind=Pod) container "wget" has cpu request 0 (check: unset-cpu-requirements, remediation: Set your container's CPU requests and limits depending on its requirements. See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for more details.)

...

Error: found 12 lint errors
Error: found 11 lint errors
```


Expand Down
8 changes: 4 additions & 4 deletions docs/generated/checks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,9 +627,9 @@ unsafeSysCtls:

**Enabled by default**: Yes

**Description**: Indicates when containers do not have CPU requests and limits set.
**Description**: Indicates when containers do not have CPU requests set.

**Remediation**: Set CPU requests and limits for your container based on its requirements. Refer to https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for details.
**Remediation**: Set CPU requests for your container based on its requirements. Refer to https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for details.

**Template**: [cpu-requirements](templates.md#cpu-requirements)

Expand All @@ -644,9 +644,9 @@ upperBoundMillis: 0

**Enabled by default**: Yes

**Description**: Indicates when containers do not have memory requests and limits set.
**Description**: Indicates when containers do not have memory requests set.

**Remediation**: Set memory requests and limits for your container based on its requirements. Refer to https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for details.
**Remediation**: Set memory requests for your container based on its requirements. Refer to https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for details.

**Template**: [memory-requirements](templates.md#memory-requirements)

Expand Down
16 changes: 4 additions & 12 deletions e2etests/bats-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,15 +902,11 @@ 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 \"app\" has cpu request 0" ]]
[[ "${message2}" == "Deployment: container \"app\" has cpu limit 0" ]]
[[ "${message3}" == "DeploymentConfig: container \"app\" has cpu request 0" ]]
[[ "${message4}" == "DeploymentConfig: container \"app\" has cpu limit 0" ]]
[[ "${count}" == "4" ]]
[[ "${message2}" == "DeploymentConfig: container \"app\" has cpu request 0" ]]
[[ "${count}" == "2" ]]
}

@test "unset-memory-requirements" {
Expand All @@ -923,15 +919,11 @@ 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 \"app\" has memory request 0" ]]
[[ "${message2}" == "Deployment: container \"app\" has memory limit 0" ]]
[[ "${message3}" == "DeploymentConfig: container \"app\" has memory request 0" ]]
[[ "${message4}" == "DeploymentConfig: container \"app\" has memory limit 0" ]]
[[ "${count}" == "4" ]]
[[ "${message2}" == "DeploymentConfig: container \"app\" has memory request 0" ]]
[[ "${count}" == "2" ]]
}

@test "use-namespace" {
Expand Down
4 changes: 2 additions & 2 deletions pkg/builtinchecks/yamls/unset-cpu-requirements.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
name: "unset-cpu-requirements"
description: "Indicates when containers do not have CPU requests and limits set."
description: "Indicates when containers do not have CPU requests set."
scope:
objectKinds:
- DeploymentLike
remediation: >-
Set CPU requests and limits for your container based on its requirements.
Set CPU requests for your container based on its requirements.
Refer to https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for details.
template: "cpu-requirements"
params:
Expand Down
4 changes: 2 additions & 2 deletions pkg/builtinchecks/yamls/unset-memory-requirements.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: "unset-memory-requirements"
description: "Indicates when containers do not have memory requests and limits set."
description: "Indicates when containers do not have memory requests set."
remediation: >-
Set memory requests and limits for your container based on its requirements.
Set memory requests for your container based on its requirements.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Set memory requests for your container based on its requirements.
Set memory limits for your container based on its requirements.

According to the design we need limits for mem and requests for CPU.

Refer to https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/#requests-and-limits for details.
scope:
objectKinds:
Expand Down
3 changes: 0 additions & 3 deletions pkg/templates/cpurequirements/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ func init() {
if p.RequirementsType == "request" || p.RequirementsType == "any" {
process(&results, container.Name, "request", container.Resources.Requests.Cpu(), p.LowerBoundMillis, p.UpperBoundMillis)
}
if p.RequirementsType == "limit" || p.RequirementsType == "any" {
process(&results, container.Name, "limit", container.Resources.Limits.Cpu(), p.LowerBoundMillis, p.UpperBoundMillis)
}
return results
}), nil
}),
Expand Down
3 changes: 0 additions & 3 deletions pkg/templates/memoryrequirements/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ func init() {
if p.RequirementsType == "request" || p.RequirementsType == "any" {
process(&results, container.Name, "request", container.Resources.Requests.Memory(), lowerBoundBytes, upperBoundBytes)
}
if p.RequirementsType == "limit" || p.RequirementsType == "any" {
process(&results, container.Name, "limit", container.Resources.Limits.Memory(), lowerBoundBytes, upperBoundBytes)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not touch templates and only change the check https://github.com/stackrox/kube-linter/pull/712/files#diff-98bbd8809c1b605e233765c4d90b25a932af5d2545dc495de58774f3f3a7c0b2R11

return results
}), nil
}),
Expand Down
Loading