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

common: fix code scanning alerts #6111

Merged
merged 2 commits into from
Sep 23, 2024
Merged

common: fix code scanning alerts #6111

merged 2 commits into from
Sep 23, 2024

Conversation

osalyk
Copy link
Contributor

@osalyk osalyk commented Sep 13, 2024

This change is Reviewable

@osalyk osalyk added sprint goal This pull request is part of the ongoing sprint no changelog Add to skip the changelog check on your pull request labels Sep 13, 2024
Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 10 of 10 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @osalyk)


utils/docker/images/Dockerfile.centos-7 line 10 at r1 (raw file):


# Pull base image
FROM centos:7@sha256:8652b9f0cb4c0599575e5a003f5906876e10c1ceb2ab9fe1786712dac14a50cf

I'm not sure what the requirements are but if we want to narrow down what will be used by adding a hash I think adding the exact repository is worth a while as well, isn't it?

Suggestion:

quay.io/centos/centos:7@sha256:8652b9f0cb4c0599575e5a003f5906876e10c1ceb2ab9fe1786712dac14a50cf

utils/docker/images/Dockerfile.debian-11 line 10 at r1 (raw file):


# Pull base image
FROM debian:11@sha256:34662f5448c46bb16e56edc0a8224cc0c0fb0a6bc4713061857761e8684ff388

.

Suggestion:

docker.io/library/debian:11@sha256:34662f5448c46bb16e56edc0a8224cc0c0fb0a6bc4713061857761e8684ff388

utils/docker/images/Dockerfile.fedora-31 line 10 at r1 (raw file):


# Pull base image
FROM fedora:31@sha256:a7a37f74ff864eec55891b64ad360d07020827486e30a92ea81d16459645b26a

.

Suggestion:

registry.fedoraproject.org/fedora:31@sha256:a7a37f74ff864eec55891b64ad360d07020827486e30a92ea81d16459645b26a

utils/docker/images/Dockerfile.fedora-37 line 10 at r1 (raw file):


# Pull base image
FROM fedora:37@sha256:4105b568d464732d3ea6a3ce9a0095f334fa7aa86fdd1129f288d2687801d87d

Suggestion:

registry.fedoraproject.org/fedora:37@sha256:4105b568d464732d3ea6a3ce9a0095f334fa7aa86fdd1129f288d2687801d87d

utils/docker/images/Dockerfile.opensuse-leap-15 line 10 at r1 (raw file):


# Pull base image
FROM opensuse/leap:15@sha256:2a8c959e6b77beb76401943d01735a4fe55f2deb9eb5cba8dc659f354bdb2a23

Suggestion:

registry.opensuse.org/opensuse/leap:15@sha256:2a8c959e6b77beb76401943d01735a4fe55f2deb9eb5cba8dc659f354bdb2a23

utils/docker/images/Dockerfile.rockylinux-8 line 10 at r1 (raw file):


# Pull base image
FROM rockylinux/rockylinux:8@sha256:fcc573d8a467898553374348812a0c93d900161b7b97cf71c76476db5e41c7b3

Suggestion:

docker.io/rockylinux/rockylinux:8@sha256:fcc573d8a467898553374348812a0c93d900161b7b97cf71c76476db5e41c7b3

utils/docker/images/Dockerfile.rockylinux-9 line 10 at r1 (raw file):


# Pull base image
FROM rockylinux/rockylinux:9@sha256:3c8f8ff398c0125ad595e3cfbc9e592e9c184e278e29ae3b8d70fbbd147e5989

Suggestion:

docker.io/rockylinux/rockylinux:9@sha256:3c8f8ff398c0125ad595e3cfbc9e592e9c184e278e29ae3b8d70fbbd147e5989

utils/docker/images/Dockerfile.ubuntu-22.04 line 10 at r1 (raw file):


# Pull base image
FROM ubuntu:22.04@sha256:adbb90115a21969d2fe6fa7f9af4253e16d45f8d4c1e930182610c4731962658

docker pull... worked for me. So you might like to paste in my sha256 instead. @grom72 you may care to check this one especially.

Suggestion:

docker.io/library/ubuntu:22.04@sha256:53a843653cbcd9e10be207e951d907dc2481d9c222de57d24cfcac32e5165188

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

I propose to remove obsolete Fedora and Centos Docker files and there is no line of code/test/CI that uses them.

Fedora is officially removed from validation in the release 2.0.0

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @osalyk)

Copy link
Contributor Author

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

Done.

Reviewable status: 2 of 10 files reviewed, 1 unresolved discussion (waiting on @grom72 and @janekmi)


utils/docker/images/Dockerfile.ubuntu-22.04 line 10 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

docker pull... worked for me. So you might like to paste in my sha256 instead. @grom72 you may care to check this one especially.

Done.

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 8 files at r2.
Reviewable status: 6 of 10 files reviewed, all discussions resolved (waiting on @osalyk)

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 10 files at r1, 3 of 8 files at r2, 1 of 5 files at r4, 4 of 4 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @osalyk)

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 5 files at r4, 4 of 4 files at r5, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @osalyk)

a discussion (no related file):
It would be nice to remove unsupported images from our Docker images repository as well.
Just to make sure we won't use a stale one by mistake.



utils/docker/images/Dockerfile.centos-stream line 1 at r5 (raw file):

# SPDX-License-Identifier: BSD-3-Clause

We don't have this one in the docker_rebuild.yml as well. I suggest removing it.


utils/docker/images/Dockerfile.ubuntu-22.04 line 10 at r5 (raw file):


# Pull base image
FROM docker.io/library/ubuntu:22.04@sha256:adbb90115a21969d2fe6fa7f9af4253e16d45f8d4c1e930182610c4731962658

Is this one updated daily or what? 😆
For me now it is "97271d29cb7956f0908cfb1449610a2cd9cb46b004ac8af25f0255663eb364ba".

Copy link
Contributor Author

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 10 files at r1, 3 of 8 files at r2, 1 of 5 files at r4, 4 of 4 files at r5.
Reviewable status: 9 of 10 files reviewed, 3 unresolved discussions (waiting on @grom72 and @janekmi)

a discussion (no related file):

Previously, janekmi (Jan Michalski) wrote…

It would be nice to remove unsupported images from our Docker images repository as well.
Just to make sure we won't use a stale one by mistake.

Done.



utils/docker/images/Dockerfile.centos-stream line 1 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

We don't have this one in the docker_rebuild.yml as well. I suggest removing it.

Done.


utils/docker/images/Dockerfile.ubuntu-22.04 line 10 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Is this one updated daily or what? 😆
For me now it is "97271d29cb7956f0908cfb1449610a2cd9cb46b004ac8af25f0255663eb364ba".

I don't know, I took the hash from the suggestion.

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @osalyk)


utils/docker/images/Dockerfile.ubuntu-22.04 line 10 at r5 (raw file):

Previously, osalyk (Oksana Sałyk) wrote…

I don't know, I took the hash from the suggestion.

I guess you made a mistake. The suggestion was: "53a843653cbcd9e10be207e951d907dc2481d9c222de57d24cfcac32e5165188".

Let's settle on using "97271d29cb7956f0908cfb1449610a2cd9cb46b004ac8af25f0255663eb364ba" for now.

Copy link
Contributor Author

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi)


utils/docker/images/Dockerfile.ubuntu-22.04 line 10 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I guess you made a mistake. The suggestion was: "53a843653cbcd9e10be207e951d907dc2481d9c222de57d24cfcac32e5165188".

Let's settle on using "97271d29cb7956f0908cfb1449610a2cd9cb46b004ac8af25f0255663eb364ba" for now.

I used the suggestion from https://github.com/pmem/pmdk/security/code-scanning/69 . I would stay with it because it is the only hash that works.

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @osalyk)

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @osalyk)

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @osalyk)

@janekmi janekmi merged commit 6948a4b into pmem:master Sep 23, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog Add to skip the changelog check on your pull request sprint goal This pull request is part of the ongoing sprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants