Merge pull request #40265 from feiskyer/cri-verify

Automatic merge from submit-queue

CRI: verify responses from remote runtime

Closes #40264.

pkg/kubelet/remote/remote_image.go

@@ -17,10 +17,13 @@ limitations under the License.
 package remote
 
 import (
+	"errors"
+	"fmt"
 	"time"
 
 	"github.com/golang/glog"
 	"google.golang.org/grpc"
+
 	internalapi "k8s.io/kubernetes/pkg/kubelet/api"
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 )
@@ -75,6 +78,14 @@ func (r *RemoteImageService) ImageStatus(image *runtimeapi.ImageSpec) (*runtimea
 		return nil, err
 	}
 
+	if resp.Image != nil {
+		if resp.Image.Id == "" || resp.Image.Size_ == 0 {
+			errorMessage := fmt.Sprintf("Id or size of image %q is not set", image.Image)
+			glog.Errorf("ImageStatus failed: %s", errorMessage)
+			return nil, errors.New(errorMessage)
+		}
+	}
+
 	return resp.Image, nil
 }
 
@@ -92,6 +103,12 @@ func (r *RemoteImageService) PullImage(image *runtimeapi.ImageSpec, auth *runtim
 		return "", err
 	}
 
+	if resp.ImageRef == "" {
+		errorMessage := fmt.Sprintf("imageRef of image %q is not set", image.Image)
+		glog.Errorf("PullImage failed: %s", errorMessage)
+		return "", errors.New(errorMessage)
+	}
+
 	return resp.ImageRef, nil
 }
 

pkg/kubelet/remote/remote_runtime.go

@@ -17,12 +17,14 @@ limitations under the License.
 package remote
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 	"time"
 
 	"github.com/golang/glog"
 	"google.golang.org/grpc"
+
 	internalapi "k8s.io/kubernetes/pkg/kubelet/api"
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 	utilexec "k8s.io/kubernetes/pkg/util/exec"
@@ -62,6 +64,10 @@ func (r *RemoteRuntimeService) Version(apiVersion string) (*runtimeapi.VersionRe
 		return nil, err
 	}
 
+	if typedVersion.Version == "" || typedVersion.RuntimeName == "" || typedVersion.RuntimeApiVersion == "" || typedVersion.RuntimeVersion == "" {
+		return nil, fmt.Errorf("not all fields are set in VersionResponse (%q)", *typedVersion)
+	}
+
 	return typedVersion, err
 }
 
@@ -79,6 +85,12 @@ func (r *RemoteRuntimeService) RunPodSandbox(config *runtimeapi.PodSandboxConfig
 		return "", err
 	}
 
+	if resp.PodSandboxId == "" {
+		errorMessage := fmt.Sprintf("PodSandboxId is not set for sandbox %q", config.GetMetadata())
+		glog.Errorf("RunPodSandbox failed: %s", errorMessage)
+		return "", errors.New(errorMessage)
+	}
+
 	return resp.PodSandboxId, nil
 }
 
@@ -125,10 +137,15 @@ func (r *RemoteRuntimeService) PodSandboxStatus(podSandBoxID string) (*runtimeap
 		PodSandboxId: podSandBoxID,
 	})
 	if err != nil {
-		glog.Errorf("PodSandboxStatus %q from runtime service failed: %v", podSandBoxID, err)
 		return nil, err
 	}
 
+	if resp.Status != nil {
+		if err := verifySandboxStatus(resp.Status); err != nil {
+			return nil, err
+		}
+	}
+
 	return resp.Status, nil
 }
 
@@ -163,6 +180,12 @@ func (r *RemoteRuntimeService) CreateContainer(podSandBoxID string, config *runt
 		return "", err
 	}
 
+	if resp.ContainerId == "" {
+		errorMessage := fmt.Sprintf("ContainerId is not set for container %q", config.GetMetadata())
+		glog.Errorf("CreateContainer failed: %s", errorMessage)
+		return "", errors.New(errorMessage)
+	}
+
 	return resp.ContainerId, nil
 }
 
@@ -245,6 +268,13 @@ func (r *RemoteRuntimeService) ContainerStatus(containerID string) (*runtimeapi.
 		return nil, err
 	}
 
+	if resp.Status != nil {
+		if err := verifyContainerStatus(resp.Status); err != nil {
+			glog.Errorf("ContainerStatus of %q failed: %v", containerID, err)
+			return nil, err
+		}
+	}
+
 	return resp.Status, nil
 }
 
@@ -288,6 +318,12 @@ func (r *RemoteRuntimeService) Exec(req *runtimeapi.ExecRequest) (*runtimeapi.Ex
 		return nil, err
 	}
 
+	if resp.Url == "" {
+		errorMessage := "URL is not set"
+		glog.Errorf("Exec failed: %s", errorMessage)
+		return nil, errors.New(errorMessage)
+	}
+
 	return resp, nil
 }
 
@@ -302,6 +338,11 @@ func (r *RemoteRuntimeService) Attach(req *runtimeapi.AttachRequest) (*runtimeap
 		return nil, err
 	}
 
+	if resp.Url == "" {
+		errorMessage := "URL is not set"
+		glog.Errorf("Exec failed: %s", errorMessage)
+		return nil, errors.New(errorMessage)
+	}
 	return resp, nil
 }
 
@@ -316,6 +357,12 @@ func (r *RemoteRuntimeService) PortForward(req *runtimeapi.PortForwardRequest) (
 		return nil, err
 	}
 
+	if resp.Url == "" {
+		errorMessage := "URL is not set"
+		glog.Errorf("Exec failed: %s", errorMessage)
+		return nil, errors.New(errorMessage)
+	}
+
 	return resp, nil
 }
 
@@ -351,5 +398,11 @@ func (r *RemoteRuntimeService) Status() (*runtimeapi.RuntimeStatus, error) {
 		return nil, err
 	}
 
+	if resp.Status == nil || len(resp.Status.Conditions) < 2 {
+		errorMessage := "RuntimeReady or NetworkReady condition are not set"
+		glog.Errorf("Status failed: %s", errorMessage)
+		return nil, errors.New(errorMessage)
+	}
+
 	return resp.Status, nil
 }

pkg/kubelet/remote/utils.go

@@ -17,10 +17,13 @@ limitations under the License.
 package remote
 
 import (
+	"fmt"
 	"net"
 	"time"
 
 	"golang.org/x/net/context"
+
+	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 )
 
 // dial creates a net.Conn by unix socket addr.
@@ -32,3 +35,55 @@ func dial(addr string, timeout time.Duration) (net.Conn, error) {
 func getContextWithTimeout(timeout time.Duration) (context.Context, context.CancelFunc) {
 	return context.WithTimeout(context.Background(), timeout)
 }
+
+// verifySandboxStatus verified whether all required fields are set in PodSandboxStatus.
+func verifySandboxStatus(status *runtimeapi.PodSandboxStatus) error {
+	if status.Id == "" {
+		return fmt.Errorf("Id is not set")
+	}
+
+	if status.Metadata == nil {
+		return fmt.Errorf("Metadata is not set")
+	}
+
+	metadata := status.Metadata
+	if metadata.Name == "" || metadata.Namespace == "" || metadata.Uid == "" {
+		return fmt.Errorf("Name, Namespace or Uid is not in metadata %q", metadata)
+	}
+
+	if status.CreatedAt == 0 {
+		return fmt.Errorf("CreatedAt is not set")
+	}
+
+	return nil
+}
+
+// verifyContainerStatus verified whether all required fields are set in ContainerStatus.
+func verifyContainerStatus(status *runtimeapi.ContainerStatus) error {
+	if status.Id == "" {
+		return fmt.Errorf("Id is not set")
+	}
+
+	if status.Metadata == nil {
+		return fmt.Errorf("Metadata is not set")
+	}
+
+	metadata := status.Metadata
+	if metadata.Name == "" {
+		return fmt.Errorf("Name is not in metadata %q", metadata)
+	}
+
+	if status.CreatedAt == 0 {
+		return fmt.Errorf("CreatedAt is not set")
+	}
+
+	if status.Image == nil || status.Image.Image == "" {
+		return fmt.Errorf("Image is not set")
+	}
+
+	if status.ImageRef == "" {
+		return fmt.Errorf("ImageRef is not set")
+	}
+
+	return nil
+}