Inherit environment variables, volumes, and volume mounts from parent in cascading scans#538
Inherit environment variables, volumes, and volume mounts from parent in cascading scans#538J12934 merged 20 commits intosecureCodeBox:mainfrom EndPositive:cascading-scan-spec-merge-inherit
Conversation
…cading rule Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
…cascading rule Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
|
Hi, sorry for the late feedback, has been a busy week :) I think this makes sense and this is a valid usecase, but I don't think this should be enabled, but should be opted in by a setting on the scan, just like the inherit Labels and inherit Annotations settings: https://github.com/secureCodeBox/secureCodeBox/blob/main/operator/apis/execution/v1/scan_types.go#L19 apiVersion: "execution.securecodebox.io/v1"
kind: Scan
metadata:
name: "nmap-scanme.nmap.org"
spec:
scanType: "nmap"
parameters:
- scanme.nmap.org
env:
- name: TEST_ENV
valueFrom:
secretKeyRef:
key: secret-name
name: zap-customer-credentials
cascades:
inheritLabels: false
inheritAnnotations: true
inheritVolumes: true
inheritEnv: true
matchLabels:
securecodebox.io/intensive: lightThe |
Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
|
Done! I used cascades.inheritVolumes for both the volumes and volumeMounts, is that okay? Or do you prefer them separate? |
J12934
left a comment
J12934
left a comment
There was a problem hiding this comment.
Hi this already looks great 👍
Found some small issues while going over the changes and some things which confused me.
hooks/cascading-scans/hook.ts
Outdated
| let cascadingScans: Array<ExtendedScanSpec> = []; | ||
| const cascadingRuleChain = getScanChain(parentScan); | ||
|
|
||
| parentScan = purgeCascadedRuleFromScan(parentScan, cascadedRule); |
There was a problem hiding this comment.
This took me a minute to understand why this is needed.
I think this should at least have a comment explaining why this is required or be moved closer to the point where the env vars & volumes are merged to make it clear that this is done to avoid duplicates there.
There was a problem hiding this comment.
I've added some comments. Feel free to add more or move it where you think it fits better.
hooks/cascading-scans/hook.test.js
Outdated
|
|
||
| const cascadedScan = cascadedScans[0] | ||
|
|
||
| expect(cascadedScans).toMatchInlineSnapshot(` |
There was a problem hiding this comment.
I think we should skip these assertions on the cascadedScans in these new tests, as these are basically a intermediate result for the purpose of this test, as the actual relevant logic on really happens in getCascadingScanDefinition. I was confused for a while looking at this and wondering why there is only on env var in the cascadedScans var.
That in itself might be something we should refactor in the future, unifying getCascadingScans and getCascadingScanDefinition and run all assertions on the actual scan specs. But this doesn't have to happen in this PR.
There was a problem hiding this comment.
I've updated the getCascadingScans function to return the actual scan definition. Also the tests have been updated, so please take another look.
Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
…date the tests accordingly. Also replaces intermediate checks on the scan definition by only the test's specific assertion (e.g. random name test). Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
…method Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
Signed-off-by: Jop Zitman <jop-zitman@hotmail.com>
Signed-off-by: Jannik Hollenbach <jannik.hollenbach@iteratec.com>
Signed-off-by: Jannik Hollenbach <jannik.hollenbach@iteratec.com>
rfelber
changed the title
3 participants
Description
This PR ensures that child scans inherit environment variables, volumes, and volume mounts (hereafter named as 'spec') from their parent scan (499c077, 0f224fa). The spec from the applied cascading rule is merged with those from the parent scan (cascading rule spec takes precedence 3391d74). Finally, to ensure that the spec from the cascading rule is only applied to the currently matched scan (and not its children), the child scan purges the cascading rule spec from the parent scan when inheriting them (b7b1856).
This behavior is useful in cases where you want to set the
http_proxyvariable for cascaded scans or when you want to mount CA certificates into cascaded scans.I also considered retrieving the spec from the root scan, but currently there's no way (to my knowledge) to accurately determine the root scan in the cascading scan hook).
Checklist
npm testruns for the whole project.