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

fix: allow resource settings with empty values. #42

Merged
merged 1 commit into from
Jun 11, 2024
Merged

fix: allow resource settings with empty values. #42

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
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ ignoreImages: ["ghcr.io/foo/bar:1.23", "myimage", "otherimages:v1"]

Users can skip the optional parts of the configuration, but an empty configuration is not
allowed. Thus, at least one of the configurations, `cpu`, or `memory` should
be defined including all their sub fields. All CPU and memory configuration
be defined. In other words, users can only remove or keep the values empty for a single
resource configuration. But not for both. All CPU and memory configuration
should be expressed using the [quantity
definitions](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/)
of Kubernetes.
Expand Down
37 changes: 37 additions & 0 deletions e2e.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,40 @@
[ $(expr "$output" : '.*allowed":false') -ne 0 ]
[ $(expr "$output" : '.*patch.*') -eq 0 ]
}

@test "reject containers exceeding the expected CPU range ignoring memory values due missing values" {
run kwctl run annotated-policy.wasm -r test_data/pod_exceeding_range.json \
--settings-json '{"cpu": {"maxLimit": "1m", "defaultRequest" : "1m", "defaultLimit" : "1m"}, "memory" : {"ignoreValues": false}, "ignoreImages": ["image:latest"]}'

[ "$status" -eq 0 ]
[ $(expr "$output" : '.*allowed":false') -ne 0 ]
[ $(expr "$output" : '.*cpu limit.*exceeds the max allowed value.*') -ne 0 ]
[ $(expr "$output" : '.*patch.*') -eq 0 ]
}

@test "allow containers exceeding the expected CPU range when the resource should be ignore due missing values" {
run kwctl run annotated-policy.wasm -r test_data/pod_exceeding_cpu_range.json \
--settings-json '{"cpu": {"ignoreValues":false}, "memory" : {"maxLimit": "1Gi", "defaultRequest" : "1Gi", "defaultLimit" : "1Gi", "ignoreValues":false}, "ignoreImages": ["image:latest"]}'

[ "$status" -eq 0 ]
[ $(expr "$output" : '.*allowed":true') -ne 0 ]
[ $(expr "$output" : '.*patch.*') -eq 0 ]
}

@test "allow containers exceeding the expected memory range when the resource should be ignore due missing values" {
run kwctl run annotated-policy.wasm -r test_data/pod_exceeding_memory_range.json \
--settings-json '{"cpu": {"maxLimit": "1m", "defaultRequest" : "1m", "defaultLimit" : "1m", "ignoreVaues": false}, "memory" : {"ignoreValues":false}, "ignoreImages": ["image:latest"]}'

[ "$status" -eq 0 ]
[ $(expr "$output" : '.*allowed":true') -ne 0 ]
[ $(expr "$output" : '.*patch.*') -eq 0 ]
}

@test "invalid settings when both resources have empty values" {
run kwctl run annotated-policy.wasm -r test_data/pod_within_range.json \
--settings-json '{"cpu": {"ignoreVaues": false}, "memory" : {"ignoreValues":false}, "ignoreImages": ["image:latest"]}'

[ "$status" -ne 0 ]
[ $(expr "$output" : '.*invalid cpu settings.*') -ne 0 ]
[ $(expr "$output" : '.*invalid memory settings.*') -ne 0 ]
}
22 changes: 18 additions & 4 deletions settings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,24 @@ type Settings struct {
IgnoreImages []string `json:"ignoreImages,omitempty"`
}

type AllValuesAreZeroError struct{}

func (e AllValuesAreZeroError) Error() string {
return "all the quantities must be defined"
}

func (s *Settings) shouldIgnoreCpuValues() bool {
return s.Cpu != nil && s.Cpu.IgnoreValues
return s.Cpu != nil && (s.Cpu.IgnoreValues || (!s.Cpu.IgnoreValues && s.Cpu.allValuesAreZero()))
}

func (s *Settings) shouldIgnoreMemoryValues() bool {
return s.Memory != nil && s.Memory.IgnoreValues
return s.Memory != nil && (s.Memory.IgnoreValues || (!s.Memory.IgnoreValues && s.Memory.allValuesAreZero()))
}

func (r *ResourceConfiguration) valid() error {

if r.MaxLimit.IsZero() && r.DefaultLimit.IsZero() && r.DefaultRequest.IsZero() && !r.IgnoreValues {
return fmt.Errorf("all the quantities must be defined")
if r.allValuesAreZero() && !r.IgnoreValues {
return AllValuesAreZeroError{}
}

if r.MaxLimit.Cmp(r.DefaultLimit) < 0 ||
Expand All @@ -45,6 +51,10 @@ func (r *ResourceConfiguration) valid() error {
return nil
}

func (r *ResourceConfiguration) allValuesAreZero() bool {
return r.MaxLimit.IsZero() && r.DefaultLimit.IsZero() && r.DefaultRequest.IsZero()
}

func (s *Settings) Valid() error {
if s.Cpu == nil && s.Memory == nil {
return fmt.Errorf("no settings provided. At least one resource limit or request must be verified")
Expand All @@ -63,6 +73,10 @@ func (s *Settings) Valid() error {
}
}
if cpuError != nil || memoryError != nil {
// user want to validate only one type of resource. The other one should be ignored
if (cpuError == nil && errors.Is(memoryError, AllValuesAreZeroError{})) || (memoryError == nil && errors.Is(cpuError, AllValuesAreZeroError{})) {
return nil
}
return errors.Join(cpuError, memoryError)
}
return nil
Expand Down
7 changes: 5 additions & 2 deletions settings_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,11 @@ func TestParsingSettings(t *testing.T) {
{"valid settings with cpu field only", []byte(`{"cpu": {"maxLimit": "1m", "defaultRequest": "1m", "defaultLimit": "1m"}}`), ""},
{"valid settings with memory fields only", []byte(`{"memory":{ "defaultLimit": "200M", "defaultRequest": "100M", "maxLimit": "500M"}}`), ""},
{"no suffix", []byte(`{"cpu": {"maxLimit": "3", "defaultLimit": "2", "defaultRequest": "1"}, "memory": {"maxLimit": "3", "defaultLimit": "2", "defaultRequest": "1"}, "ignoreImages": []}`), ""},
{"invalid cpu settings", []byte(`{"cpu": {"maxLimit": "2m", "defaultRequest": "3m", "defaultLimit": "4m"}, "memory":{ "defaultLimit": "2G", "defaultRequest": "1G", "maxLimit": "3G"}, "ignoreImages": ["image:latest"]}`), "invalid cpu settings"},
{"invalid memory settings", []byte(`{"cpu": {"maxLimit": "2m", "defaultRequest": "1m", "defaultLimit": "1m"}, "memory":{ "defaultLimit": "2G", "defaultRequest": "3G", "maxLimit": "1G"}, "ignoreImages": ["image:latest"]}`), "invalid memory settings"},
{"invalid cpu settings", []byte(`{"cpu": {"maxLimit": "2m", "defaultRequest": "3m", "defaultLimit": "4m"}, "memory":{ "defaultLimit": "2G", "defaultRequest": "1G", "maxLimit": "3G"}, "ignoreImages": ["image:latest"]}`), "default values cannot be greater than the max limit"},
{"invalid memory settings", []byte(`{"cpu": {"maxLimit": "2m", "defaultRequest": "1m", "defaultLimit": "1m"}, "memory":{ "defaultLimit": "2G", "defaultRequest": "3G", "maxLimit": "1G"}, "ignoreImages": ["image:latest"]}`), "default values cannot be greater than the max limit"},
{"valid settings with empty memory settings", []byte(`{"cpu": {"maxLimit": "1m", "defaultRequest": "1m", "defaultLimit": "1m"}, "memory":{"ignoreValues": false}, "ignoreImages": ["image:latest"]}`), ""},
{"valid settings with empty cpu settings", []byte(`{"cpu": {"ignoreValues": false}, "memory":{ "defaultLimit": "200M", "defaultRequest": "100M", "maxLimit": "500M", "ignoreValues": false}, "ignoreImages": ["image:latest"]}`), ""},
{"invalid settings with empty cpu and memory settings", []byte(`{"cpu": {"ignoreValues": false}, "memory":{"ignoreValues": false}, "ignoreImages": ["image:latest"]}`), "invalid cpu settings\nall the quantities must be defined\ninvalid memory settings\nall the quantities must be defined"},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
Expand Down
93 changes: 93 additions & 0 deletions test_data/pod_exceeding_cpu_range.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
{
"uid": "1299d386-525b-4032-98ae-1949f69f9cfc",
"kind": {
"group": "",
"version": "v1",
"kind": "Pod"
},
"resource": {
"group": "",
"version": "v1",
"resource": "pods"
},
"requestKind": {
"group": "",
"version": "v1",
"kind": "Pod"
},
"requestResource": {
"group": "",
"version": "v1",
"resource": "pods"
},
"name": "nginx",
"namespace": "default",
"operation": "CREATE",
"userInfo": {
"username": "kubernetes-admin",
"groups": [
"system:masters",
"system:authenticated"
]
},
"object": {
"kind": "Pod",
"apiVersion": "v1",
"metadata": {
"name": "nginx",
"namespace": "default",
"uid": "04dc7a5e-e1f1-4e34-8d65-2c9337a43e64",
"labels": {
"env": "test"
},
"annotations": {
"kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"kind\":\"Pod\",\"metadata\":{\"annotations\":{},\"labels\":{\"env\":\"test\"},\"name\":\"nginx\",\"namespace\":\"default\"},\"spec\":{\"containers\":[{\"image\":\"nginx\",\"imagePullPolicy\":\"IfNotPresent\",\"name\":\"nginx\"}],\"tolerations\":[{\"effect\":\"NoSchedule\",\"key\":\"example-key\",\"operator\":\"Exists\"}]}}\n"
}
},
"spec": {
"containers": [
{
"name": "pause",
"image": "registry.k8s.io/pause",
"resources": {
"limits": {
"cpu": "3m",
"memory": "1Gi"
},
"requests": {
"cpu": "3m",
"memory": "1Gi"
}
}
},
{
"name": "mycontainer",
"image": "image:latest",
"resources": {
"limits": {
"cpu": "2m",
"memory": "1Gi"
},
"requests": {
"cpu": "2m",
"memory": "1Gi"
}
}
}
],
"restartPolicy": "Always",
"terminationGracePeriodSeconds": 30,
"dnsPolicy": "ClusterFirst",
"serviceAccountName": "default",
"serviceAccount": "default",
"securityContext": {},
"schedulerName": "default-scheduler"
}
},
"oldObject": null,
"dryRun": false,
"options": {
"kind": "CreateOptions",
"apiVersion": "meta.k8s.io/v1"
}
}
93 changes: 93 additions & 0 deletions test_data/pod_exceeding_memory_range.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
{
"uid": "1299d386-525b-4032-98ae-1949f69f9cfc",
"kind": {
"group": "",
"version": "v1",
"kind": "Pod"
},
"resource": {
"group": "",
"version": "v1",
"resource": "pods"
},
"requestKind": {
"group": "",
"version": "v1",
"kind": "Pod"
},
"requestResource": {
"group": "",
"version": "v1",
"resource": "pods"
},
"name": "nginx",
"namespace": "default",
"operation": "CREATE",
"userInfo": {
"username": "kubernetes-admin",
"groups": [
"system:masters",
"system:authenticated"
]
},
"object": {
"kind": "Pod",
"apiVersion": "v1",
"metadata": {
"name": "nginx",
"namespace": "default",
"uid": "04dc7a5e-e1f1-4e34-8d65-2c9337a43e64",
"labels": {
"env": "test"
},
"annotations": {
"kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"kind\":\"Pod\",\"metadata\":{\"annotations\":{},\"labels\":{\"env\":\"test\"},\"name\":\"nginx\",\"namespace\":\"default\"},\"spec\":{\"containers\":[{\"image\":\"nginx\",\"imagePullPolicy\":\"IfNotPresent\",\"name\":\"nginx\"}],\"tolerations\":[{\"effect\":\"NoSchedule\",\"key\":\"example-key\",\"operator\":\"Exists\"}]}}\n"
}
},
"spec": {
"containers": [
{
"name": "pause",
"image": "registry.k8s.io/pause",
"resources": {
"limits": {
"cpu": "1m",
"memory": "3Gi"
},
"requests": {
"cpu": "1m",
"memory": "3Gi"
}
}
},
{
"name": "mycontainer",
"image": "image:latest",
"resources": {
"limits": {
"cpu": "1m",
"memory": "2Gi"
},
"requests": {
"cpu": "1m",
"memory": "2Gi"
}
}
}
],
"restartPolicy": "Always",
"terminationGracePeriodSeconds": 30,
"dnsPolicy": "ClusterFirst",
"serviceAccountName": "default",
"serviceAccount": "default",
"securityContext": {},
"schedulerName": "default-scheduler"
}
},
"oldObject": null,
"dryRun": false,
"options": {
"kind": "CreateOptions",
"apiVersion": "meta.k8s.io/v1"
}
}
Loading