Simplify DataImportCron ServiceAccount authorization

Add ServiceAccountName to DataImportCron spec, replacing CreatedBy
which was added in #3946.

In case of DataImportCron with PVC source, the controller checks the
ServiceAccount is authorized to clone the source PVC.

Signed-off-by: Arnon Gilboa <[email protected]>

Author: arnongilboa

api/openapi-spec/swagger.json

@@ -4377,10 +4377,6 @@
      "managedDataSource"
     ],
     "properties": {
-     "createdBy": {
-      "description": "CreatedBy is the JSON-marshaled UserInfo of the user who created this DataImportCron. This field is set by the mutating webhook and cannot be set by users.",
-      "type": "string"
-     },
      "garbageCollect": {
       "description": "GarbageCollect specifies whether old PVCs should be cleaned up after a new PVC is imported. Options are currently \"Outdated\" and \"Never\", defaults to \"Outdated\".",
       "type": "string"
@@ -4404,6 +4400,10 @@
       "type": "string",
       "default": ""
      },
+     "serviceAccountName": {
+      "description": "ServiceAccountName is the name of the ServiceAccount for creating DataVolumes.",
+      "type": "string"
+     },
      "template": {
       "description": "Template specifies template for the DVs to be created",
       "default": {},

pkg/apis/core/v1beta1/openapi_generated.go

@@ -79,8 +79,6 @@ const (
 
 	dataImportCronValidatePath = "/dataimportcron-validate"
 
-	dataImportCronMutatePath = "/dataimportcron-mutate"
-
 	populatorValidatePath = "/populator-validate"
 
 	healthzPath = "/healthz"
@@ -216,11 +214,6 @@ func NewCdiAPIServer(bindAddress string,
 		return nil, errors.Errorf("failed to create DataImportCron validating webhook: %s", err)
 	}
 
-	err = app.createDataImportCronMutatingWebhook()
-	if err != nil {
-		return nil, errors.Errorf("failed to create DataImportCron mutating webhook: %s", err)
-	}
-
 	err = app.createPopulatorValidatingWebhook()
 	if err != nil {
 		return nil, errors.Errorf("failed to create Populator validating webhook: %s", err)
@@ -563,11 +556,6 @@ func (app *cdiAPIApp) createDataImportCronValidatingWebhook() error {
 	return nil
 }
 
-func (app *cdiAPIApp) createDataImportCronMutatingWebhook() error {
-	app.container.ServeMux.Handle(dataImportCronMutatePath, webhooks.NewDataImportCronMutatingWebhook())
-	return nil
-}
-
 func (app *cdiAPIApp) createPopulatorValidatingWebhook() error {
 	app.container.ServeMux.Handle(populatorValidatePath, webhooks.NewPopulatorValidatingWebhook(app.client, app.cdiClient))
 	return nil

pkg/apiserver/apiserver.go

@@ -4,7 +4,6 @@ go_library(
     name = "go_default_library",
     srcs = [
         "cdi-validate.go",
-        "dataimportcron-mutate.go",
         "dataimportcron-validate.go",
         "datavolume-mutate.go",
         "datavolume-validate.go",
@@ -30,7 +29,6 @@ go_library(
         "//vendor/github.com/robfig/cron/v3:go_default_library",
         "//vendor/k8s.io/api/admission/v1:go_default_library",
         "//vendor/k8s.io/api/admissionregistration/v1:go_default_library",
-        "//vendor/k8s.io/api/authentication/v1:go_default_library",
         "//vendor/k8s.io/api/authorization/v1:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
@@ -53,7 +51,6 @@ go_test(
     name = "go_default_test",
     srcs = [
         "cdi-validate_test.go",
-        "dataimportcron-mutate_test.go",
         "dataimportcron-validate_test.go",
         "datavolume-mutate_test.go",
         "datavolume-validate_test.go",
@@ -73,7 +70,6 @@ go_test(
         "//vendor/github.com/onsi/ginkgo/v2:go_default_library",
         "//vendor/github.com/onsi/gomega:go_default_library",
         "//vendor/k8s.io/api/admission/v1:go_default_library",
-        "//vendor/k8s.io/api/authentication/v1:go_default_library",
         "//vendor/k8s.io/api/authorization/v1:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/api/storage/v1:go_default_library",

pkg/apiserver/webhooks/BUILD.bazel

@@ -161,25 +161,5 @@ func (wh *dataImportCronValidatingWebhook) validateDataImportCronSpec(request *a
 		return causes
 	}
 
-	if request.Operation == admissionv1.Create {
-		userInfoStr, err := getUserInfoString(&request.UserInfo)
-		if err != nil {
-			causes = append(causes, metav1.StatusCause{
-				Type:    metav1.CauseTypeInternal,
-				Message: fmt.Sprintf("Cannot marshal UserInfo to string: %v", err),
-				Field:   field.Child("CreatedBy").String(),
-			})
-			return causes
-		}
-		if spec.CreatedBy != nil && *userInfoStr != *spec.CreatedBy {
-			causes = append(causes, metav1.StatusCause{
-				Type:    metav1.CauseTypeForbidden,
-				Message: "CreatedBy field is set automatically and cannot be specified by users",
-				Field:   field.Child("CreatedBy").String(),
-			})
-			return causes
-		}
-	}
-
 	return causes
 }

pkg/apiserver/webhooks/dataimportcron-mutate.go

@@ -148,14 +148,6 @@ var _ = Describe("Validating Webhook", func() {
 			resp := validateDataImportCronCreate(cron)
 			Expect(resp.Allowed).To(BeFalse())
 		})
-		It("should reject DataImportCron with CreatedBy field set by user on create", func() {
-			cron := newDataImportCron(cdiv1.DataVolumeSourceRegistry{URL: &testRegistryURL})
-			createdBy := `{"username":"hacker"}`
-			cron.Spec.CreatedBy = &createdBy
-			resp := validateDataImportCronCreate(cron)
-			Expect(resp.Allowed).To(BeFalse())
-			Expect(resp.Result.Message).To(ContainSubstring("CreatedBy field is set automatically"))
-		})
 		It("should reject invalid DataImportCron spec update", func() {
 			newCron := newDataImportCron(cdiv1.DataVolumeSourceRegistry{URL: &testRegistryURL})
 			newBytes, _ := json.Marshal(&newCron)

pkg/apiserver/webhooks/dataimportcron-mutate_test.go

@@ -91,11 +91,6 @@ func NewDataImportCronValidatingWebhook(k8sClient kubernetes.Interface, cdiClien
 	return newAdmissionHandler(&dataImportCronValidatingWebhook{dataVolumeValidatingWebhook{k8sClient: k8sClient, cdiClient: cdiClient}})
 }
 
-// NewDataImportCronMutatingWebhook creates a new DataImportCron mutating webhook
-func NewDataImportCronMutatingWebhook() http.Handler {
-	return newAdmissionHandler(&dataImportCronMutatingWebhook{})
-}
-
 // NewPopulatorValidatingWebhook creates a new DataVolumeValidation webhook
 func NewPopulatorValidatingWebhook(k8sClient kubernetes.Interface, cdiClient cdiclient.Interface) http.Handler {
 	return newAdmissionHandler(&populatorValidatingWebhook{dataVolumeValidatingWebhook{k8sClient: k8sClient, cdiClient: cdiClient}})

pkg/apiserver/webhooks/dataimportcron-validate.go

@@ -40,7 +40,6 @@ go_library(
         "//vendor/github.com/pkg/errors:go_default_library",
         "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
         "//vendor/github.com/robfig/cron/v3:go_default_library",
-        "//vendor/k8s.io/api/authentication/v1:go_default_library",
         "//vendor/k8s.io/api/authorization/v1:go_default_library",
         "//vendor/k8s.io/api/batch/v1:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",