Skip to content

Commit 877f83d

Browse files
malexmavemalexmave
committed
Respect affinity and tolerations from helm values
This commit makes the operator respect affinity and tolerations that are specified in ScanSpec and ParseDefinitions. The default values are used if the scan does override it, and otherwise discarded (not merged). Default values are not inherited like explicitly specified values are, and you need to make sure to specify affinity and tolerations for both scanner and parser (they are not copied over, either). Hooks will get the same treatment in the next commit. Signed-off-by: Max Maass <max.maass@iteratec.com>
1 parent a840ed6 commit 877f83d

File tree

9 files changed

+1410
-33
lines changed

9 files changed

+1410
-33
lines changed

‎hooks/cascading-scans/hook/hook.test.js‎

Lines changed: 80 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ test("Should create subsequent scans for open HTTPS ports (NMAP findings)", () =
103103
],
104104
},
105105
"spec": Object {
106-
"affinity": Object {},
106+
"affinity": undefined,
107107
"cascades": Object {},
108108
"env": Array [],
109109
"hookSelector": Object {},
@@ -113,7 +113,7 @@ test("Should create subsequent scans for open HTTPS ports (NMAP findings)", () =
113113
"foobar.com:443",
114114
],
115115
"scanType": "sslyze",
116-
"tolerations": Array [],
116+
"tolerations": undefined,
117117
"volumeMounts": Array [],
118118
"volumes": Array [],
119119
},
@@ -243,7 +243,7 @@ test("Should not crash when the annotations are not set", () => {
243243
],
244244
},
245245
"spec": Object {
246-
"affinity": Object {},
246+
"affinity": undefined,
247247
"cascades": Object {},
248248
"env": Array [],
249249
"hookSelector": Object {},
@@ -253,7 +253,7 @@ test("Should not crash when the annotations are not set", () => {
253253
"foobar.com:443",
254254
],
255255
"scanType": "sslyze",
256-
"tolerations": Array [],
256+
"tolerations": undefined,
257257
"volumeMounts": Array [],
258258
"volumes": Array [],
259259
},
@@ -377,7 +377,7 @@ test("Should allow wildcards in cascadingRules", () => {
377377
],
378378
},
379379
"spec": Object {
380-
"affinity": Object {},
380+
"affinity": undefined,
381381
"cascades": Object {},
382382
"env": Array [],
383383
"hookSelector": Object {},
@@ -387,7 +387,7 @@ test("Should allow wildcards in cascadingRules", () => {
387387
"foobar.com:8443",
388388
],
389389
"scanType": "sslyze",
390-
"tolerations": Array [],
390+
"tolerations": undefined,
391391
"volumeMounts": Array [],
392392
"volumes": Array [],
393393
},
@@ -1129,7 +1129,7 @@ test("Templating should apply to environment variables", () => {
11291129
],
11301130
},
11311131
"spec": Object {
1132-
"affinity": Object {},
1132+
"affinity": undefined,
11331133
"cascades": Object {},
11341134
"env": Array [
11351135
Object {
@@ -1144,7 +1144,7 @@ test("Templating should apply to environment variables", () => {
11441144
"foobar.com:443",
11451145
],
11461146
"scanType": "sslyze",
1147-
"tolerations": Array [],
1147+
"tolerations": undefined,
11481148
"volumeMounts": Array [
11491149
Object {
11501150
"mountPath": "/test",
@@ -1252,7 +1252,7 @@ test("Templating should apply to initContainer commands", () => {
12521252
],
12531253
},
12541254
"spec": Object {
1255-
"affinity": Object {},
1255+
"affinity": undefined,
12561256
"cascades": Object {},
12571257
"env": Array [],
12581258
"hookSelector": Object {},
@@ -1279,7 +1279,7 @@ test("Templating should apply to initContainer commands", () => {
12791279
"foobar.com:443",
12801280
],
12811281
"scanType": "sslyze",
1282-
"tolerations": Array [],
1282+
"tolerations": undefined,
12831283
"volumeMounts": Array [
12841284
Object {
12851285
"mountPath": "/test",
@@ -1388,7 +1388,7 @@ test("Templating should apply to initContainer environment variables", () => {
13881388
],
13891389
},
13901390
"spec": Object {
1391-
"affinity": Object {},
1391+
"affinity": undefined,
13921392
"cascades": Object {},
13931393
"env": Array [],
13941394
"hookSelector": Object {},
@@ -1418,7 +1418,7 @@ test("Templating should apply to initContainer environment variables", () => {
14181418
"foobar.com:443",
14191419
],
14201420
"scanType": "sslyze",
1421-
"tolerations": Array [],
1421+
"tolerations": undefined,
14221422
"volumeMounts": Array [
14231423
Object {
14241424
"mountPath": "/test",
@@ -1526,7 +1526,7 @@ test("Templating should not break special encoding (http://...) when using tripl
15261526
],
15271527
},
15281528
"spec": Object {
1529-
"affinity": Object {},
1529+
"affinity": undefined,
15301530
"cascades": Object {},
15311531
"env": Array [],
15321532
"hookSelector": Object {},
@@ -1553,7 +1553,7 @@ test("Templating should not break special encoding (http://...) when using tripl
15531553
"https://github.com/secureCodeBox/secureCodeBox",
15541554
],
15551555
"scanType": "sslyze",
1556-
"tolerations": Array [],
1556+
"tolerations": undefined,
15571557
"volumeMounts": Array [
15581558
Object {
15591559
"mountPath": "/test",
@@ -1850,9 +1850,9 @@ test("should not copy tolerations and affinity into cascaded scan if label disab
18501850

18511851
const cascadedScan = cascadedScans[0];
18521852

1853-
expect(cascadedScan.spec.affinity).toMatchInlineSnapshot(`Object {}`);
1853+
expect(cascadedScan.spec.affinity).toMatchInlineSnapshot(`undefined`);
18541854

1855-
expect(cascadedScan.spec.tolerations).toMatchInlineSnapshot(`Array []`);
1855+
expect(cascadedScan.spec.tolerations).toMatchInlineSnapshot(`undefined`);
18561856
});
18571857

18581858
test("should merge tolerations and replace affinity in cascaded scan if cascading spec sets new ones", () => {
@@ -1976,6 +1976,70 @@ test("should merge tolerations and replace affinity in cascaded scan if cascadin
19761976
`);
19771977
});
19781978

1979+
test("should not set affinity or tolerations to undefined if they are defined to be an empty map / list in upstream scan", () => {
1980+
const findings = [
1981+
{
1982+
name: "Port 443 is open",
1983+
category: "Open Port",
1984+
attributes: {
1985+
state: "open",
1986+
hostname: "foobar.com",
1987+
port: 443,
1988+
service: "https"
1989+
}
1990+
}
1991+
];
1992+
1993+
parentScan.spec.affinity = {}
1994+
1995+
parentScan.spec.tolerations = []
1996+
1997+
const cascadedScans = getCascadingScans(
1998+
parentScan,
1999+
findings,
2000+
sslyzeCascadingRules
2001+
);
2002+
2003+
const cascadedScan = cascadedScans[0];
2004+
2005+
// New values will completely replace the old values, not be merged
2006+
expect(cascadedScan.spec.affinity).toMatchInlineSnapshot(`Object {}`);
2007+
2008+
expect(cascadedScan.spec.tolerations).toMatchInlineSnapshot(`Array []`);
2009+
});
2010+
2011+
test("Should not set affinity or tolerations to undefined if they are defined to be an empty map / list in cascading ScanSpec", () => {
2012+
const findings = [
2013+
{
2014+
name: "Port 443 is open",
2015+
category: "Open Port",
2016+
attributes: {
2017+
state: "open",
2018+
hostname: "foobar.com",
2019+
port: 443,
2020+
service: "https"
2021+
}
2022+
}
2023+
];
2024+
2025+
sslyzeCascadingRules[0].spec.scanSpec.tolerations = [];
2026+
2027+
sslyzeCascadingRules[0].spec.scanSpec.affinity = {};
2028+
2029+
const cascadedScans = getCascadingScans(
2030+
parentScan,
2031+
findings,
2032+
sslyzeCascadingRules
2033+
);
2034+
2035+
const cascadedScan = cascadedScans[0];
2036+
2037+
// New values will completely replace the old values, not be merged
2038+
expect(cascadedScan.spec.affinity).toMatchInlineSnapshot(`Object {}`);
2039+
2040+
expect(cascadedScan.spec.tolerations).toMatchInlineSnapshot(`Array []`);
2041+
});
2042+
19792043
test("should only use tolerations and affinity of cascaded scan if inheritance is disabled", () => {
19802044
parentScan.spec.cascades.inheritAffinity = false;
19812045
parentScan.spec.cascades.inheritTolerations = false;

‎hooks/cascading-scans/hook/hook.ts‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,28 @@ function mergeCascadingRuleWithScan(
162162
cascadingRule: CascadingRule
163163
) {
164164
const { scanAnnotations, scanLabels } = cascadingRule.spec;
165-
let { env = [], volumes = [], volumeMounts = [], initContainers = [], hookSelector = {}, affinity = {}, tolerations = [] } = cascadingRule.spec.scanSpec;
165+
let { env = [], volumes = [], volumeMounts = [], initContainers = [], hookSelector = {}, affinity, tolerations } = cascadingRule.spec.scanSpec;
166166
let { inheritAnnotations, inheritLabels, inheritEnv, inheritVolumes, inheritInitContainers, inheritHookSelector, inheritAffinity = true, inheritTolerations = true} = scan.spec.cascades;
167167

168+
// We have to use a slightly complicated logic for inheriting / setting the tolerations and affinity to work around some
169+
// limitations in the operator. The goal is to avoid setting anything to an empty list [] or empty map {} if the keys are actually
170+
// missing in the specification, as this will lead to issues in the operator when pulling in default values from the templates of the
171+
// scanners. So, we are taking a bit more care to make sure that the value stays undefined (and thus nil in Go) unless someone explicitly
172+
// specified an empty list or map.
173+
let selectedTolerations = undefined;
174+
if (tolerations !== undefined) {
175+
selectedTolerations = mergeInheritedArray(scan.spec.tolerations, tolerations, inheritTolerations);
176+
} else if (inheritTolerations) {
177+
selectedTolerations = scan.spec.tolerations;
178+
}
179+
180+
let selectedAffinity = undefined;
181+
if (affinity !== undefined) {
182+
selectedAffinity = affinity
183+
} else if (inheritAffinity) {
184+
selectedAffinity = scan.spec.affinity;
185+
}
186+
168187
return {
169188
annotations: mergeInheritedMap(scan.metadata.annotations, scanAnnotations, inheritAnnotations),
170189
labels: mergeInheritedMap(scan.metadata.labels, scanLabels, inheritLabels),
@@ -173,9 +192,8 @@ function mergeCascadingRuleWithScan(
173192
volumeMounts: mergeInheritedArray(scan.spec.volumeMounts, volumeMounts, inheritVolumes),
174193
initContainers: mergeInheritedArray(scan.spec.initContainers, initContainers, inheritInitContainers),
175194
hookSelector: mergeInheritedSelector(scan.spec.hookSelector, hookSelector, inheritHookSelector),
176-
// Affinity and tolerations are always inherited
177-
affinity: mergeInheritedMap(scan.spec.affinity, affinity, inheritAffinity),
178-
tolerations: mergeInheritedArray(scan.spec.tolerations, tolerations, inheritTolerations),
195+
affinity: selectedAffinity,
196+
tolerations: selectedTolerations
179197
}
180198
}
181199

‎hooks/cascading-scans/hook/scan-helpers.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ export function mergeInheritedMap(parentProps, ruleProps, inherit: boolean = tru
8282
if (!inherit) {
8383
parentProps = {};
8484
}
85+
if (ruleProps === undefined) {
86+
return parentProps;
87+
}
8588
return {
8689
...parentProps,
8790
...ruleProps // ruleProps overwrites any duplicate keys from parentProps

‎operator/apis/execution/v1/parsedefinition_types.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ type ParseDefinitionSpec struct {
3434
Volumes []corev1.Volume `json:"volumes,omitempty"`
3535
// VolumeMounts allows to specify volume mounts for the parser container.
3636
VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"`
37+
38+
// Affinity allows to specify a node affinity, to control on which nodes you want a parser to run. See: https://kubernetes.io/docs/tasks/configure-pod-container/assign-pods-nodes-using-node-affinity/
39+
Affinity *corev1.Affinity `json:"affinity,omitempty"`
40+
// Tolerations are a different way to control on which nodes your parser is executed. See https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/
41+
Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
3742
}
3843

3944
// ParseDefinitionStatus defines the observed state of ParseDefinition

‎operator/apis/execution/v1/zz_generated.deepcopy.go‎

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)