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.

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

Update debian support #1096

Merged
merged 2 commits into from
Mar 25, 2024
Merged

Update debian support #1096

merged 2 commits into from
Mar 25, 2024

Conversation

AyanSinhaMahapatra
Copy link
Contributor

Also updates SCTK to latest develop and aboutcode-org/scancode-toolkit#3682
So it has changes from aboutcode-org/scancode-toolkit#3620 and regenerated test files.

@tdruez
Copy link
Contributor

tdruez commented Mar 4, 2024

@AyanSinhaMahapatra What is the status of a proper release for SCTK?

@AyanSinhaMahapatra
Copy link
Contributor Author

@tdruez we're hoping for a release this week 🤞
Opened this PR as a part of aboutcode-org/purldb#245 as I'm currently testing all the updates in SCTK, SCIO and purldb together, but I will update this PR to a proper released version of SCTK before we merge this.

Signed-off-by: Ayan Sinha Mahapatra <[email protected]>
Copy link
Contributor Author

@AyanSinhaMahapatra AyanSinhaMahapatra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready for review now. I've rebased this on top of the merged SCTK v32.1.0 now in main.

@@ -265,6 +265,8 @@ def scan_rootfs_for_system_packages(project, rootfs):
installed_packages = rootfs.get_installed_packages(package_getter)
for index, (purl, package) in enumerate(installed_packages):
logger.info(f"Creating package #{index}: {purl}")
if package.namespace != distro_id:
Copy link
Contributor Author

@AyanSinhaMahapatra AyanSinhaMahapatra Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is related to: #899

In SCTK we are now getting the namespace populated from clues present in package data with this commit and if we don't have any clues we are assigning a defualt value to the namespace which is debian for debian packages.

In SCIO we get distro data from os-release which overrides these values, as they are always correct and takes care of occasional false-positives and default debian cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the logic to only optionally update the namespace when we have multiple namespaces detected among all the packages, as this is when we want to override.
@pombredanne ping also, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the distroless case at 486d69e#diff-9fc8547b0afef7ac993fcd0ca883098de99e6b77d53c245e2b87bdf1dbd2ed84L123? Is this correct to make the namespace debian?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per https://github.com/GoogleContainerTools/distroless

see https://github.com/GoogleContainerTools/distroless/blob/f55b2b343481f52ef3bde34c8a2f3631a40a36c1/debian_packages.yaml#L75 ... the namespace is debian in all cases.... some deb11 some deb12. Eventually the way to go will be to match these.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case we are good, thanks!

"version_codename": null,
"version_id": "9",
"pretty_name": "Debian GNU/Linux 9 (stretch)",
"name": "Ubuntu",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I've updated the os-release file in the basic-rootfs test case to the ubuntu distro file, as the packages detected below seem to be ubuntu packages, and the old example seemed to be wrong. Otherwise the namespaces being overridden from this os-release file seemed to be wrong.

@AyanSinhaMahapatra AyanSinhaMahapatra marked this pull request as ready for review March 25, 2024 10:27
Signed-off-by: Ayan Sinha Mahapatra <[email protected]>
Copy link
Contributor

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tdruez tdruez merged commit fb6476d into main Mar 25, 2024
7 checks passed
@tdruez tdruez deleted the update-debian-support branch March 25, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add missing namespace to debian packages
3 participants