-
Notifications
You must be signed in to change notification settings - Fork 399
oscap-docker: remove atomic, fix environment variable configuration #1931
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
Conversation
The last release of atomic was 5 years ago. The project is officially deprecated. No linux distributions still package atomic.
Getting the client api from the client reuses the client configuration. This fixes an inconsistency where environment variable configuration is used for the client but not for the client api. This change therefore enables environment variable configuration (ex DOCKER_HOST) to work.
OscapDockerScan will throw an error if docker isn't running; there's no need to create a separate docker client in a different way just to do this ping test.
Starting images to scan them has downsides, including that the image must be able to be started and stay running, which isn't always easy. For example, not all images contain `bash` or a shell at all. By instead using the approach taken by `oscap-podman` and extracting the image then scanning it (instead of starting the container and scanning the resulting mount), the container never needs to be run. This approach allows `oscap-docker` to scan any image (including those without `bash`).
@jan-cerny @evgenyz and anyone else, Can you please take a look? The 4 failing checks are unrelated to this PR as far as I can tell. Thank you in advance! |
@candrews Thank you very much for your PR! I agree that we should remove the dependency on Atomic, so I'm happy about your effort. I was trying to test this PR. In RHEL and Fedora the Docker has been replaced by
Can you take a look? Have you encountered a similar problem? Are you on Ubuntu or on a different system? Which images and content do you recommend to use or are using personally? Thanks for any help. |
I did some research, and "invalid reference format" usually means that the container name requested doesn't meet docker's container name requirements. It seems docker (but not podman!) limits container names to 30 characters, but this generated name is 47 characters long. I've updated this PR with a commit to use the docker generated container id instead, eliminating the need to generate a name at all. Can you please try your test again?
I'm currently using Fedora 37. I've actually been testing this I need
Here are some tests that I've performed successfully:
You're welcome, I'm glad to have the opportunity work with you :-) |
I have discovered that the traceback that I posted above also happens with the current upstream maint-1.3 branch, that means the traceback isn't caused by the contents of this PR. I also discovered that it doesn't happen when I try to scan a Fedora image on this Ubuntu host. Also, I was successful to scan the
Great, but the same traceback still occurs. But as it isn't caused by the contents of this PR we can try to fix that later in a separate PR.
Thanks for the hint. It's an interesting use-case, normally I would recommend using So I tried this PR on a Fedora 37 machine as well. I have started the podman service which provides the API compatibility server and tried out the
I would recommend testing the changes also on these systems, or eg. on Ubuntu where the real docker is present, because the So I think that this PR is actually OK, it doesn't introduce any new problems, but we should track the problems found (the traceback with ubuntu 20.04, the all rules notapplicable, etc.) and fix them later. |
I ran
I'm aware and I'm also doing testing with real docker (and not just podman) as well. I'm actually testing on Alpine too, and have been reporting and working with them to fix issues with its |
What further can I do to facilitate the merging of this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The last release of atomic was 5 years ago.
The project is officially deprecated.
No linux distributions still package atomic.
Getting the client api from the client reuses the client configuration.
This fixes an inconsistency where environment variable configuration is
used for the client but not for the client api.
This change therefore enables environment variable configuration (ex
DOCKER_HOST) to work.
OscapDockerScan will throw an error if docker isn't running;
there's no need to create a separate docker client in a different way
just to do this ping test.
Fixes #1585