pkg/sca/sca.go: Implement getLdSoConfDLibPaths
#1850
Conversation
pkg/sca/sca.go
Outdated
| return err | ||
| } | ||
|
|
||
| if !fi.Mode().IsRegular() { |
There was a problem hiding this comment.
should we support symlinks to?
There was a problem hiding this comment.
Hah, it's awesome that you thought about that :-).
I spent some time implementing support for symlinks, but unfortunately I could not get it to work the way I wanted. The problem is that if you have a symlink inside /etc/ld.so.conf.d pointing to a file outside of it, I could not figure out how to obtain the full path of said file using the filesystem functions that are at my disposal. I'm sure it's possible, but I'm still learning as I go (pun intended)...
sergiodj
force-pushed
the
add-ld-so-conf-d-support
branch
3 times, most recently
from
ab035fc to
44693f0
Compare
sergiodj
force-pushed
the
add-ld-so-conf-d-support
branch
from
44693f0 to
a0b7ae1
Compare
|
Hi @jonjohnsonjr! I'd be delighted if you could offer your review here as well. Thanks! |
This commit implements the machinery necessary for SCA to consider files under /etc/ld.so.conf.d/. The idea is that, if there is a configuration for ld.so.conf.d being installed by the package on hand, then the libraries shipped by the package should be listed as "provides" if they're installed inside the paths listed in the ld.so.conf.d configuration file. Signed-off-by: Sergio Durigan Junior <sergiodj@chainguard.dev>
sergiodj
force-pushed
the
add-ld-so-conf-d-support
branch
from
a0b7ae1 to
faecd8e
Compare
Thanks, @dannf! I pushed a small change to fix the failure. |
This commit implements a simple test to make sure that the new function is properly parsing the configuration file inside /etc/ld.so.conf.d/. A small modification had to be made to the (th *testHandle) RelativeNames() function in order to make TestDocSca pass, because (th *testHandle) FilesystemForRelative() isn't implemented. Signed-off-by: Sergio Durigan Junior <sergiodj@chainguard.dev>
sergiodj
force-pushed
the
add-ld-so-conf-d-support
branch
from
faecd8e to
d040dd1
Compare
| @@ -804,5 +894,9 @@ func Analyze(ctx context.Context, hdl SCAHandle, generated *config.Dependencies) | |||
| generated.Provides = nil | |||
| } | |||
|
|
|||
| if oldLibDirs != nil { | |||
| libDirs = oldLibDirs | |||
There was a problem hiding this comment.
This frightens me a bit -- instead of mutating the global here do you mind plumbing this around everywhere?
There was a problem hiding this comment.
For sure. But let me take the opportunity and ask a broader question.
I thought about moving libDirs inside the SCABuildInterface, because this really should be a per-package variable. However, SCABuildInterface only represents one single entity in the package (either the sole package, or one of its subpackages if they exist). I couldn't find a way to say "hey, this applies to the package and its subpackages".
The reason I wanted to do it is because, as you can see now, getLdSoConfDLibPaths is being invoked for the package and its subpackages, and it iterates over all files from them, every time. This is suboptimal.
Something else I thought about is that we could have a global flag to signal whether getLdSoConfDLibPaths was already invoked. But then I have another question: is it guaranteed that we will be dealing with only one package (and its subpackages)?
Sorry for the barrage of questions and thoughts; this code is new to me and I'm trying to make sense of how to best implement what I need here. Thanks!
This PR implements the machinery necessary for SCA to consider files under
/etc/ld.so.conf.d/.The idea is that, if there is a configuration for
ld.so.conf.dbeing installed by the package on hand, then the libraries shipped by the package should be listed asprovidesif they're installed inside the paths listed in theld.so.conf.dconfiguration file.This commit also implements a simple test to make sure that the new function is properly parsing the configuration file inside
/etc/ld.so.conf.d/.Notes:
SCA Changes
Notes:
As of this writing no packages are actually using
ld.so.conf.don Wolfi OS. I built/nvidia-cudnn-9(from extra-packages) and it works as expected:Relates: https://github.com/chainguard-dev/internal-dev/issues/7609