Add option to use containerd snapshotter to generate SBOMs #1290
Conversation
| {{- if .Values.datadog.sbom.containerImage.uncompressedLayersSupport -}} | ||
| {{- $capabilities := dict "capabilities" (dict "add" (list "SYS_ADMIN")) }} | ||
| {{ include "generate-security-context" (dict "securityContext" (merge $capabilities .Values.agents.containers.agent.securityContext) "targetSystem" "foobar" "seccomp" "" "kubeversion" .Capabilities.KubeVersion.Version) | indent 2 }} | ||
| {{- else }} |
There was a problem hiding this comment.
we already have a function generate-security-context that tune the securityContext. Can we add this logic to the function too?
There was a problem hiding this comment.
Hi @lebauce
I made some comments. Also could you add or CI example in https://github.com/DataDog/helm-charts/tree/main/charts/datadog/ci
to make sure that the securityContext capability merge function works as expected 🙇
Also I think if this new option in enabled and the customer is also using the appArmor option we should add the specific appArmor annotation to the agent container.
lebauce
force-pushed
the
lebauce/csm-vm-container-image-mount
branch
from
e892634 to
b9729af
Compare
|
@clamoriniere I have little knowledge regarding apparmor. Could you please provide the annotations that should be added ? |
lebauce
force-pushed
the
lebauce/csm-vm-container-image-mount
branch
2 times, most recently
from
099d643 to
e91e7c1
Compare
Sorry, I just saw your message. I think you found it. |
clamoriniere
added
chart/datadog
lebauce
force-pushed
the
lebauce/csm-vm-container-image-mount
branch
from
e91e7c1 to
3744e19
Compare
| # This should be set to true when using EKS, GKE or if containerd is configured to | ||
| # discard uncompressed layers. | ||
| # This feature will cause the SYS_ADMIN capability to be added to the Agent container. | ||
| uncompressedLayersSupport: false |
There was a problem hiding this comment.
I feel like the option name does not exactly represent what it does. Using mount has different facets (more performant, more permissions, etc.) and perhaps in the future we'll have another way to support uncompressed layers while still offering the mount option.
There was a problem hiding this comment.
This is intended: the idea of this flag is more to make the feature work in the case of discarded layers. This will allow us to more to a better solution in the future. Naming it mountImages would make the migration more complex as we wouldn't know if the feature was specifically request or if it's because it was used as a workaround.
Does it make sense ?
There was a problem hiding this comment.
It makes sense to have a dedicated uncompressedLayersSupport that activates useMount. What does not make sense is that useMount does not exist.
I know we did not advertise this option before, but I believe it should have its own existence and we could put in the docstring that uncompressedLayersSupport implies useMount, with the proper explanation on useMount
There was a problem hiding this comment.
lets continue this discussion about the useMount option or equivalent for another PR, and merge this one. so we can provide an easy configuration for users already
clamoriniere
force-pushed
the
lebauce/csm-vm-container-image-mount
branch
from
aae454f to
3ab20a4
Compare
What this PR does / why we need it:
This PR set up container image scanning using the containerd snapshotter.
To do so, it bind mounts the containerd folder and adds the required
capability to mount container images. This is intended to be used when
facing the
discard_uncompress_layersissue with containerd.Which issue this PR fixes
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged)Special notes for your reviewer:
Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]
.github/helm-docs.sh)CHANGELOG.mdhas been updatedREADME.mdmake update-test-baselines)