Merge pull request #117020 from cji/cji-seccomplocalhost

Fix seccomp localhost error handling

pkg/kubelet/kuberuntime/helpers.go

@@ -212,32 +212,36 @@ func toKubeRuntimeStatus(status *runtimeapi.RuntimeStatus) *kubecontainer.Runtim
 	return &kubecontainer.RuntimeStatus{Conditions: conditions}
 }
 
-func fieldProfile(scmp *v1.SeccompProfile, profileRootPath string, fallbackToRuntimeDefault bool) string {
+func fieldProfile(scmp *v1.SeccompProfile, profileRootPath string, fallbackToRuntimeDefault bool) (string, error) {
 	if scmp == nil {
 		if fallbackToRuntimeDefault {
-			return v1.SeccompProfileRuntimeDefault
+			return v1.SeccompProfileRuntimeDefault, nil
 		}
-		return ""
+		return "", nil
 	}
 	if scmp.Type == v1.SeccompProfileTypeRuntimeDefault {
-		return v1.SeccompProfileRuntimeDefault
+		return v1.SeccompProfileRuntimeDefault, nil
 	}
-	if scmp.Type == v1.SeccompProfileTypeLocalhost && scmp.LocalhostProfile != nil && len(*scmp.LocalhostProfile) > 0 {
+	if scmp.Type == v1.SeccompProfileTypeLocalhost {
+		if scmp.LocalhostProfile != nil && len(*scmp.LocalhostProfile) > 0 {
 			fname := filepath.Join(profileRootPath, *scmp.LocalhostProfile)
-		return v1.SeccompLocalhostProfileNamePrefix + fname
+			return v1.SeccompLocalhostProfileNamePrefix + fname, nil
+		} else {
+			return "", fmt.Errorf("localhostProfile must be set if seccompProfile type is Localhost.")
+		}
 	}
 	if scmp.Type == v1.SeccompProfileTypeUnconfined {
-		return v1.SeccompProfileNameUnconfined
+		return v1.SeccompProfileNameUnconfined, nil
 	}
 
 	if fallbackToRuntimeDefault {
-		return v1.SeccompProfileRuntimeDefault
+		return v1.SeccompProfileRuntimeDefault, nil
 	}
-	return ""
+	return "", nil
 }
 
 func (m *kubeGenericRuntimeManager) getSeccompProfilePath(annotations map[string]string, containerName string,
-	podSecContext *v1.PodSecurityContext, containerSecContext *v1.SecurityContext, fallbackToRuntimeDefault bool) string {
+	podSecContext *v1.PodSecurityContext, containerSecContext *v1.SecurityContext, fallbackToRuntimeDefault bool) (string, error) {
 	// container fields are applied first
 	if containerSecContext != nil && containerSecContext.SeccompProfile != nil {
 		return fieldProfile(containerSecContext.SeccompProfile, m.seccompProfileRoot, fallbackToRuntimeDefault)
@@ -249,42 +253,46 @@ func (m *kubeGenericRuntimeManager) getSeccompProfilePath(annotations map[string
 	}
 
 	if fallbackToRuntimeDefault {
-		return v1.SeccompProfileRuntimeDefault
+		return v1.SeccompProfileRuntimeDefault, nil
 	}
 
-	return ""
+	return "", nil
 }
 
-func fieldSeccompProfile(scmp *v1.SeccompProfile, profileRootPath string, fallbackToRuntimeDefault bool) *runtimeapi.SecurityProfile {
+func fieldSeccompProfile(scmp *v1.SeccompProfile, profileRootPath string, fallbackToRuntimeDefault bool) (*runtimeapi.SecurityProfile, error) {
 	if scmp == nil {
 		if fallbackToRuntimeDefault {
 			return &runtimeapi.SecurityProfile{
 				ProfileType: runtimeapi.SecurityProfile_RuntimeDefault,
-			}
+			}, nil
 		}
 		return &runtimeapi.SecurityProfile{
 			ProfileType: runtimeapi.SecurityProfile_Unconfined,
-		}
+		}, nil
 	}
 	if scmp.Type == v1.SeccompProfileTypeRuntimeDefault {
 		return &runtimeapi.SecurityProfile{
 			ProfileType: runtimeapi.SecurityProfile_RuntimeDefault,
+		}, nil
 	}
-	}
+	if scmp.Type == v1.SeccompProfileTypeLocalhost {
-	if scmp.Type == v1.SeccompProfileTypeLocalhost && scmp.LocalhostProfile != nil && len(*scmp.LocalhostProfile) > 0 {
+		if scmp.LocalhostProfile != nil && len(*scmp.LocalhostProfile) > 0 {
 			fname := filepath.Join(profileRootPath, *scmp.LocalhostProfile)
 			return &runtimeapi.SecurityProfile{
 				ProfileType:  runtimeapi.SecurityProfile_Localhost,
 				LocalhostRef: fname,
+			}, nil
+		} else {
+			return nil, fmt.Errorf("localhostProfile must be set if seccompProfile type is Localhost.")
 		}
 	}
 	return &runtimeapi.SecurityProfile{
 		ProfileType: runtimeapi.SecurityProfile_Unconfined,
-	}
+	}, nil
 }
 
 func (m *kubeGenericRuntimeManager) getSeccompProfile(annotations map[string]string, containerName string,
-	podSecContext *v1.PodSecurityContext, containerSecContext *v1.SecurityContext, fallbackToRuntimeDefault bool) *runtimeapi.SecurityProfile {
+	podSecContext *v1.PodSecurityContext, containerSecContext *v1.SecurityContext, fallbackToRuntimeDefault bool) (*runtimeapi.SecurityProfile, error) {
 	// container fields are applied first
 	if containerSecContext != nil && containerSecContext.SeccompProfile != nil {
 		return fieldSeccompProfile(containerSecContext.SeccompProfile, m.seccompProfileRoot, fallbackToRuntimeDefault)
@@ -298,10 +306,10 @@ func (m *kubeGenericRuntimeManager) getSeccompProfile(annotations map[string]str
 	if fallbackToRuntimeDefault {
 		return &runtimeapi.SecurityProfile{
 			ProfileType: runtimeapi.SecurityProfile_RuntimeDefault,
-		}
+		}, nil
 	}
 
 	return &runtimeapi.SecurityProfile{
 		ProfileType: runtimeapi.SecurityProfile_Unconfined,
-	}
+	}, nil
 }

pkg/kubelet/kuberuntime/helpers_linux_test.go

@@ -224,17 +224,18 @@ func TestFieldProfile(t *testing.T) {
 		scmpProfile     *v1.SeccompProfile
 		rootPath        string
 		expectedProfile string
+		expectedError   string
 	}{
 		{
 			description:     "no seccompProfile should return empty",
 			expectedProfile: "",
 		},
 		{
-			description: "type localhost without profile should return empty",
+			description: "type localhost without profile should return error",
 			scmpProfile: &v1.SeccompProfile{
 				Type: v1.SeccompProfileTypeLocalhost,
 			},
-			expectedProfile: "",
+			expectedError: "localhostProfile must be set if seccompProfile type is Localhost.",
 		},
 		{
 			description: "unknown type should return empty",
@@ -269,9 +270,14 @@ func TestFieldProfile(t *testing.T) {
 	}
 
 	for i, test := range tests {
-		seccompProfile := fieldProfile(test.scmpProfile, test.rootPath, false)
+		seccompProfile, err := fieldProfile(test.scmpProfile, test.rootPath, false)
+		if test.expectedError != "" {
+			assert.EqualError(t, err, test.expectedError, "TestCase[%d]: %s", i, test.description)
+		} else {
+			assert.NoError(t, err, "TestCase[%d]: %s", i, test.description)
 			assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description)
 		}
+	}
 }
 
 func TestFieldProfileDefaultSeccomp(t *testing.T) {
@@ -280,17 +286,18 @@ func TestFieldProfileDefaultSeccomp(t *testing.T) {
 		scmpProfile     *v1.SeccompProfile
 		rootPath        string
 		expectedProfile string
+		expectedError   string
 	}{
 		{
 			description:     "no seccompProfile should return runtime/default",
 			expectedProfile: v1.SeccompProfileRuntimeDefault,
 		},
 		{
-			description: "type localhost without profile should return runtime/default",
+			description: "type localhost without profile should return error",
 			scmpProfile: &v1.SeccompProfile{
 				Type: v1.SeccompProfileTypeLocalhost,
 			},
-			expectedProfile: v1.SeccompProfileRuntimeDefault,
+			expectedError: "localhostProfile must be set if seccompProfile type is Localhost.",
 		},
 		{
 			description: "unknown type should return runtime/default",
@@ -325,9 +332,14 @@ func TestFieldProfileDefaultSeccomp(t *testing.T) {
 	}
 
 	for i, test := range tests {
-		seccompProfile := fieldProfile(test.scmpProfile, test.rootPath, true)
+		seccompProfile, err := fieldProfile(test.scmpProfile, test.rootPath, true)
+		if test.expectedError != "" {
+			assert.EqualError(t, err, test.expectedError, "TestCase[%d]: %s", i, test.description)
+		} else {
+			assert.NoError(t, err, "TestCase[%d]: %s", i, test.description)
 			assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description)
 		}
+	}
 }
 
 func TestGetSeccompProfilePath(t *testing.T) {
@@ -341,6 +353,7 @@ func TestGetSeccompProfilePath(t *testing.T) {
 		containerSc     *v1.SecurityContext
 		containerName   string
 		expectedProfile string
+		expectedError   string
 	}{
 		{
 			description:     "no seccomp should return empty",
@@ -377,14 +390,14 @@ func TestGetSeccompProfilePath(t *testing.T) {
 			expectedProfile: seccompLocalhostPath("filename"),
 		},
 		{
-			description:     "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns empty",
+			description:   "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error",
 			podSc:         &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}},
-			expectedProfile: "",
+			expectedError: "localhostProfile must be set if seccompProfile type is Localhost.",
 		},
 		{
-			description:     "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns empty",
+			description:   "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error",
 			containerSc:   &v1.SecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}},
-			expectedProfile: "",
+			expectedError: "localhostProfile must be set if seccompProfile type is Localhost.",
 		},
 		{
 			description:     "container seccomp profile set to SeccompProfileTypeLocalhost returns 'localhost/' + LocalhostProfile",
@@ -400,9 +413,14 @@ func TestGetSeccompProfilePath(t *testing.T) {
 	}
 
 	for i, test := range tests {
-		seccompProfile := m.getSeccompProfilePath(test.annotation, test.containerName, test.podSc, test.containerSc, false)
+		seccompProfile, err := m.getSeccompProfilePath(test.annotation, test.containerName, test.podSc, test.containerSc, false)
+		if test.expectedError != "" {
+			assert.EqualError(t, err, test.expectedError, "TestCase[%d]: %s", i, test.description)
+		} else {
+			assert.NoError(t, err, "TestCase[%d]: %s", i, test.description)
 			assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description)
 		}
+	}
 }
 
 func TestGetSeccompProfilePathDefaultSeccomp(t *testing.T) {
@@ -416,6 +434,7 @@ func TestGetSeccompProfilePathDefaultSeccomp(t *testing.T) {
 		containerSc     *v1.SecurityContext
 		containerName   string
 		expectedProfile string
+		expectedError   string
 	}{
 		{
 			description:     "no seccomp should return runtime/default",
@@ -452,14 +471,14 @@ func TestGetSeccompProfilePathDefaultSeccomp(t *testing.T) {
 			expectedProfile: seccompLocalhostPath("filename"),
 		},
 		{
-			description:     "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns runtime/default",
+			description:   "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error",
 			podSc:         &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}},
-			expectedProfile: v1.SeccompProfileRuntimeDefault,
+			expectedError: "localhostProfile must be set if seccompProfile type is Localhost.",
 		},
 		{
-			description:     "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns runtime/default",
+			description:   "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error",
 			containerSc:   &v1.SecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}},
-			expectedProfile: v1.SeccompProfileRuntimeDefault,
+			expectedError: "localhostProfile must be set if seccompProfile type is Localhost.",
 		},
 		{
 			description:     "container seccomp profile set to SeccompProfileTypeLocalhost returns 'localhost/' + LocalhostProfile",
@@ -475,9 +494,14 @@ func TestGetSeccompProfilePathDefaultSeccomp(t *testing.T) {
 	}
 
 	for i, test := range tests {
-		seccompProfile := m.getSeccompProfilePath(test.annotation, test.containerName, test.podSc, test.containerSc, true)
+		seccompProfile, err := m.getSeccompProfilePath(test.annotation, test.containerName, test.podSc, test.containerSc, true)
+		if test.expectedError != "" {
+			assert.EqualError(t, err, test.expectedError, "TestCase[%d]: %s", i, test.description)
+		} else {
+			assert.NoError(t, err, "TestCase[%d]: %s", i, test.description)
 			assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description)
 		}
+	}
 }
 
 func TestGetSeccompProfile(t *testing.T) {
@@ -499,6 +523,7 @@ func TestGetSeccompProfile(t *testing.T) {
 		containerSc     *v1.SecurityContext
 		containerName   string
 		expectedProfile *runtimeapi.SecurityProfile
+		expectedError   string
 	}{
 		{
 			description:     "no seccomp should return unconfined",
@@ -533,14 +558,14 @@ func TestGetSeccompProfile(t *testing.T) {
 			},
 		},
 		{
-			description:     "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns unconfined",
+			description:   "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error",
 			podSc:         &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}},
-			expectedProfile: unconfinedProfile,
+			expectedError: "localhostProfile must be set if seccompProfile type is Localhost.",
 		},
 		{
-			description:     "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns unconfined",
+			description:   "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error",
 			containerSc:   &v1.SecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}},
-			expectedProfile: unconfinedProfile,
+			expectedError: "localhostProfile must be set if seccompProfile type is Localhost.",
 		},
 		{
 			description: "container seccomp profile set to SeccompProfileTypeLocalhost returns 'localhost/' + LocalhostProfile",
@@ -569,9 +594,14 @@ func TestGetSeccompProfile(t *testing.T) {
 	}
 
 	for i, test := range tests {
-		seccompProfile := m.getSeccompProfile(test.annotation, test.containerName, test.podSc, test.containerSc, false)
+		seccompProfile, err := m.getSeccompProfile(test.annotation, test.containerName, test.podSc, test.containerSc, false)
+		if test.expectedError != "" {
+			assert.EqualError(t, err, test.expectedError, "TestCase[%d]: %s", i, test.description)
+		} else {
+			assert.NoError(t, err, "TestCase[%d]: %s", i, test.description)
 			assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description)
 		}
+	}
 }
 
 func TestGetSeccompProfileDefaultSeccomp(t *testing.T) {
@@ -593,6 +623,7 @@ func TestGetSeccompProfileDefaultSeccomp(t *testing.T) {
 		containerSc     *v1.SecurityContext
 		containerName   string
 		expectedProfile *runtimeapi.SecurityProfile
+		expectedError   string
 	}{
 		{
 			description:     "no seccomp should return RuntimeDefault",
@@ -627,14 +658,14 @@ func TestGetSeccompProfileDefaultSeccomp(t *testing.T) {
 			},
 		},
 		{
-			description:     "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns unconfined",
+			description:   "pod seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error",
 			podSc:         &v1.PodSecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}},
-			expectedProfile: unconfinedProfile,
+			expectedError: "localhostProfile must be set if seccompProfile type is Localhost.",
 		},
 		{
-			description:     "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns unconfined",
+			description:   "container seccomp profile set to SeccompProfileTypeLocalhost with empty LocalhostProfile returns error",
 			containerSc:   &v1.SecurityContext{SeccompProfile: &v1.SeccompProfile{Type: v1.SeccompProfileTypeLocalhost}},
-			expectedProfile: unconfinedProfile,
+			expectedError: "localhostProfile must be set if seccompProfile type is Localhost.",
 		},
 		{
 			description: "container seccomp profile set to SeccompProfileTypeLocalhost returns 'localhost/' + LocalhostProfile",
@@ -663,9 +694,14 @@ func TestGetSeccompProfileDefaultSeccomp(t *testing.T) {
 	}
 
 	for i, test := range tests {
-		seccompProfile := m.getSeccompProfile(test.annotation, test.containerName, test.podSc, test.containerSc, true)
+		seccompProfile, err := m.getSeccompProfile(test.annotation, test.containerName, test.podSc, test.containerSc, true)
+		if test.expectedError != "" {
+			assert.EqualError(t, err, test.expectedError, "TestCase[%d]: %s", i, test.description)
+		} else {
+			assert.NoError(t, err, "TestCase[%d]: %s", i, test.description)
 			assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]: %s", i, test.description)
 		}
+	}
 }
 
 func getLocal(v string) *string {

pkg/kubelet/kuberuntime/security_context.go

@@ -37,9 +37,16 @@ func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Po
 
 	// TODO: Deprecated, remove after we switch to Seccomp field
 	// set SeccompProfilePath.
-	synthesized.SeccompProfilePath = m.getSeccompProfilePath(pod.Annotations, container.Name, pod.Spec.SecurityContext, container.SecurityContext, m.seccompDefault)
+	var err error
+	synthesized.SeccompProfilePath, err = m.getSeccompProfilePath(pod.Annotations, container.Name, pod.Spec.SecurityContext, container.SecurityContext, m.seccompDefault)
+	if err != nil {
+		return nil, err
+	}
 
-	synthesized.Seccomp = m.getSeccompProfile(pod.Annotations, container.Name, pod.Spec.SecurityContext, container.SecurityContext, m.seccompDefault)
+	synthesized.Seccomp, err = m.getSeccompProfile(pod.Annotations, container.Name, pod.Spec.SecurityContext, container.SecurityContext, m.seccompDefault)
+	if err != nil {
+		return nil, err
+	}
 
 	// set ApparmorProfile.
 	synthesized.ApparmorProfile = apparmor.GetProfileNameFromPodAnnotations(pod.Annotations, container.Name)