WIP Replace oci_distribution usage with containerd_client and k8_cri #99
Conversation
Redent0r
force-pushed
the
saulparedes/use_containerd_client
branch
from
825358c to
4190aa4
Compare
Redent0r
force-pushed
the
saulparedes/use_containerd_client
branch
from
56d5743 to
46fc37f
Compare
Redent0r
changed the title
| use crate::policy; | ||
| use crate::verity; | ||
|
|
||
| use anyhow::{anyhow, Result}; | ||
| use docker_credential::{CredentialRetrievalError, DockerCredential}; | ||
| use containerd_client::services::v1::GetImageRequest; |
There was a problem hiding this comment.
Does the new code work if containerd is not installed on the current machine?
Does it work on Windows too?
src/tools/genpolicy/src/registry.rs
Outdated
| "Failed to pull container image manifest and config - error: {:#?}", | ||
| &e | ||
| ); | ||
| return Err(anyhow!("Failed to connect to containerd: {e:?}")); |
There was a problem hiding this comment.
Just {e} should be good enough - similar to {ep} from https://github.com/microsoft/kata-containers/blob/8c6b54be51e9c074faace8a28e62d4113dfd6080/src/agent/src/policy.rs#L183C45-L183C45
For bonus points, you can also use bail! instead of Err(anyhow). This should work here:
use anyhow::{bail, Result};
bail!("Failed to connect to containerd: {e}");
src/tools/genpolicy/src/registry.rs
Outdated
| .unwrap() | ||
| .connect_with_connector(service_fn(move |_: Uri| UnixStream::connect(socket.clone()))) | ||
| .await | ||
| .expect("Could not create client."); |
There was a problem hiding this comment.
Are we using expect or unwrap anywhere else in this code? If not, it might be better to use ? instead of expect.
| use tokio::{fs, io::AsyncWriteExt}; | ||
| use std::{io::Seek, io::Write, path::Path}; | ||
| use tokio::{fs}; | ||
| use k8s_cri::v1::image_service_client::ImageServiceClient; |
There was a problem hiding this comment.
Does the new code work without having K8s set-up for the current machine?
Clean up and refactor. Signed-off-by: Saul Paredes <saulparedes@microsoft.com>
Redent0r
force-pushed
the
saulparedes/use_containerd_client
branch
from
5353c89 to
96ce904
Compare
This change updates the way genpolicy manages the container image and its layers.
Before, we would pull manifest and config, then pull each image layer individually using oci_distribuition client.
Now, we pull the image (all its layers) and config using k8_cri, then pull manifest, then query each layer using containerd_client.
This change:
Notable changes:
docker.io/bprashanth/nginxhttps:1.0registry/subfolder/image_name:tag) as opposed to any shorthand version (egbprashanth/nginxhttps:1.0ordocker.io/bprashanth/nginxhttps) This requirement seems to come from k8_cri (crictl)layerskey), return response as jsonmanifestskey). So find manifest for linux and amd64. At this point, this has to be the correct manifest. At least all current tests pass doing this
Test run: https://dev.azure.com/mariner-org/mariner/_build/results?buildId=462783&view=ms.vss-test-web.build-test-results-tab
Item: https://microsoft.visualstudio.com/OS/_workitems/edit/44971145