github/kata-containers
1
0
Fork 1
mirror of https://github.com/kata-containers/kata-containers.git synced 2025-08-30 23:37:45 +00:00
Code Issues Projects Releases Wiki Activity

runtime-rs: remove validation of shim v2 -address value

Browse Source 
It appears that under the shim v2 protocol, a shim has no use of its own
for the -address value, it just passes it back to container runtime's
(mostly containerd or cri-o) event-publishing binary.  Since the -address
value only flows through the shim, being passed to the shim by a container
runtime and then essentially passed back by shim to the container runtime,
it seems inappropriate for a shim to validate the value that is fully
owned and only used by the container runtime.

This commit removes such validation from runtime-rs.  Doing so, it solves
(part of) an interoperability problem between runtime-rs and cri-o.  cri-o
seems to intentionally choose not to implement the event-publishing part
of the shim v2 protocol and thus it has no value it could pass to
runtime-rs for -address.  As a result, it sends an empty string which has
been failing the excessive validation performed by runtime-rs so far.

Signed-off-by: Pavel Mores <pmores@redhat.com>
This commit is contained in:
Pavel Mores 2024-02-01 11:30:05 +01:00
parent 2332552c8f
commit f0256fded5
2 changed files with 3 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
src/runtime-rs/crates/shim/src/args.rs
View File

@ -4,7 +4,7 @@
// SPDX-License-Identifier: Apache-2.0
//
use std::{os::unix::fs::FileTypeExt, path::PathBuf};
use std::path::PathBuf;
use anyhow::{anyhow, Context, Result};
use kata_sys_util::validate;
@ -38,11 +38,7 @@ impl Args {
/// The id, namespace, address and publish_binary are mandatory for START, RUN and DELETE.
/// And bundle is mandatory for DELETE.
pub fn validate(&mut self, should_check_bundle: bool) -> Result<()> {
if self.id.is_empty()
|| self.namespace.is_empty()
|| self.address.is_empty()
|| self.publish_binary.is_empty()
{
if self.id.is_empty() || self.namespace.is_empty() || self.publish_binary.is_empty() {
return Err(anyhow!(Error::ArgumentIsEmpty(format!(
"id: {} namespace: {} address: {} publish_binary: {}",
&self.id, &self.namespace, &self.address, &self.publish_binary
@ -52,21 +48,6 @@ impl Args {
validate::verify_id(&self.id).context("verify container id")?;
validate::verify_id(&self.namespace).context("verify namespace")?;
// Ensure `address` is a valid path.
let path = PathBuf::from(self.address.clone())
.canonicalize()
.context(Error::InvalidPath(self.address.clone()))?;
let md = path
.metadata()
.context(Error::FileGetMetadata(format!("{:?}", path)))?;
if !md.file_type().is_socket() {
return Err(Error::InvalidArgument).context("address is not socket");
}
self.address = path
.to_str()
.map(|v| v.to_owned())
.ok_or(Error::InvalidArgument)?;
// Ensure `bundle` is a valid path.
if should_check_bundle {
if self.bundle.is_empty() {
@ -182,10 +163,7 @@ mod tests {
arg.clone()
},
should_check_bundle: false,
result: Err(anyhow!(Error::ArgumentIsEmpty(format!(
"id: {} namespace: {} address: {} publish_binary: {}",
&arg.id, &arg.namespace, &arg.address, &arg.publish_binary
)))),
result: Ok(()),
},
TestData {
arg: {
@ -276,22 +254,6 @@ mod tests {
should_check_bundle: true,
result: Ok(()),
},
TestData {
arg: {
arg.address = default_address.clone() + "/..";
arg.clone()
},
should_check_bundle: true,
result: Err(anyhow!(Error::InvalidPath(arg.address.clone()))),
},
TestData {
arg: {
arg.address = default_address.clone() + "/..";
arg.clone()
},
should_check_bundle: true,
result: Err(anyhow!(Error::InvalidPath(arg.address.clone()))),
},
TestData {
arg: {
arg.address = default_address;

1
src/runtime-rs/crates/shim/src/shim_start.rs
View File

@ -61,7 +61,6 @@ impl ShimExecutor {
fn new_command(&self) -> Result<std::process::Command> {
if self.args.id.is_empty()
|| self.args.namespace.is_empty()
|| self.args.address.is_empty()
|| self.args.publish_binary.is_empty()
{
return Err(anyhow!("invalid param"));