Skip to content

Commit 5d5b676

Browse files
committed
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]>
1 parent d5ae91f commit 5d5b676

File tree

11 files changed

+298
-15
lines changed

11 files changed

+298
-15
lines changed

api/openapi-spec/swagger.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4400,6 +4400,10 @@
44004400
"type": "string",
44014401
"default": ""
44024402
},
4403+
"serviceAccountName": {
4404+
"description": "ServiceAccountName is the name of the ServiceAccount for creating DataVolumes.",
4405+
"type": "string"
4406+
},
44034407
"template": {
44044408
"description": "Template specifies template for the DVs to be created",
44054409
"default": {},

pkg/apis/core/v1beta1/openapi_generated.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/controller/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ go_library(
4040
"//vendor/github.com/pkg/errors:go_default_library",
4141
"//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
4242
"//vendor/github.com/robfig/cron/v3:go_default_library",
43+
"//vendor/k8s.io/api/authorization/v1:go_default_library",
4344
"//vendor/k8s.io/api/batch/v1:go_default_library",
4445
"//vendor/k8s.io/api/core/v1:go_default_library",
4546
"//vendor/k8s.io/api/networking/v1:go_default_library",

pkg/controller/dataimportcron-conditions.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,13 @@ import (
3030
)
3131

3232
const (
33-
noDigest = "NoDigest"
34-
noImport = "NoImport"
35-
outdated = "Outdated"
36-
scheduled = "ImportScheduled"
37-
inProgress = "ImportProgressing"
38-
upToDate = "UpToDate"
33+
noDigest = "NoDigest"
34+
noImport = "NoImport"
35+
notAuthorized = "NotAuthorized"
36+
outdated = "Outdated"
37+
scheduled = "ImportScheduled"
38+
inProgress = "ImportProgressing"
39+
upToDate = "UpToDate"
3940
)
4041

4142
func updateDataImportCronCondition(cron *cdiv1.DataImportCron, conditionType cdiv1.DataImportCronConditionType, status corev1.ConditionStatus, message, reason string) {

pkg/controller/dataimportcron-controller.go

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/pkg/errors"
3434
cronexpr "github.com/robfig/cron/v3"
3535

36+
authorizationv1 "k8s.io/api/authorization/v1"
3637
batchv1 "k8s.io/api/batch/v1"
3738
corev1 "k8s.io/api/core/v1"
3839
storagev1 "k8s.io/api/storage/v1"
@@ -889,15 +890,62 @@ func (r *DataImportCronReconciler) createImportDataVolume(ctx context.Context, d
889890
return nil
890891
}
891892
}
892-
893893
dv := r.newSourceDataVolume(dataImportCron, dvName)
894+
if allowed, err := r.authorizeCloneDataVolume(dataImportCron, dv); err != nil {
895+
return err
896+
} else if !allowed {
897+
updateDataImportCronCondition(dataImportCron, cdiv1.DataImportCronProgressing, corev1.ConditionFalse,
898+
"Not authorized to create DataVolume", notAuthorized)
899+
return nil
900+
}
894901
if err := r.client.Create(ctx, dv); err != nil && !k8serrors.IsAlreadyExists(err) {
895902
return err
896903
}
897904

898905
return nil
899906
}
900907

908+
func (r *DataImportCronReconciler) authorizeCloneDataVolume(dataImportCron *cdiv1.DataImportCron, dv *cdiv1.DataVolume) (bool, error) {
909+
if !isPvcSource(dataImportCron) {
910+
return true, nil
911+
}
912+
saName := dataImportCron.Spec.ServiceAccountName
913+
if saName == "" {
914+
saName = "default"
915+
}
916+
if resp, err := dv.AuthorizeSA(dv.Namespace, dv.Name, r, dataImportCron.Namespace, saName); err != nil {
917+
return false, err
918+
} else if !resp.Allowed {
919+
r.log.Info("Not authorized to create DataVolume", "cron", dataImportCron.Name, "reason", resp.Reason)
920+
return false, nil
921+
}
922+
923+
return true, nil
924+
}
925+
926+
func (r *DataImportCronReconciler) CreateSar(sar *authorizationv1.SubjectAccessReview) (*authorizationv1.SubjectAccessReview, error) {
927+
if err := r.client.Create(context.TODO(), sar); err != nil {
928+
return nil, err
929+
}
930+
return sar, nil
931+
}
932+
933+
func (r *DataImportCronReconciler) GetNamespace(name string) (*corev1.Namespace, error) {
934+
ns := &corev1.Namespace{}
935+
if err := r.client.Get(context.TODO(), types.NamespacedName{Name: name}, ns); err != nil {
936+
return nil, err
937+
}
938+
return ns, nil
939+
}
940+
941+
func (r *DataImportCronReconciler) GetDataSource(namespace, name string) (*cdiv1.DataSource, error) {
942+
das := &cdiv1.DataSource{}
943+
if err := r.client.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: name}, das); err != nil {
944+
return nil, err
945+
}
946+
return das, nil
947+
}
948+
901949
func (r *DataImportCronReconciler) handleStorageClassChange(ctx context.Context, dataImportCron *cdiv1.DataImportCron, desiredStorageClass string) error {
902950
digest, ok := dataImportCron.Annotations[AnnSourceDesiredDigest]
903951
if !ok {

pkg/controller/dataimportcron-controller_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -920,7 +920,8 @@ var _ = Describe("All DataImportCron Tests", func() {
920920
Namespace: pvc.Namespace,
921921
},
922922
}
923-
reconciler = createDataImportCronReconciler(cron, pvc)
923+
reconciler = createDataImportCronReconciler(cron, pvc,
924+
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}})
924925
_, err := reconciler.Reconcile(context.TODO(), cronReq)
925926
Expect(err).ToNot(HaveOccurred())
926927
})

pkg/operator/resources/cluster/controller.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,19 @@ func getControllerClusterPolicyRules() []rbacv1.PolicyRule {
117117
"delete",
118118
},
119119
},
120+
{
121+
APIGroups: []string{
122+
"",
123+
},
124+
Resources: []string{
125+
"namespaces",
126+
},
127+
Verbs: []string{
128+
"get",
129+
"list",
130+
"watch",
131+
},
132+
},
120133
{
121134
APIGroups: []string{
122135
"",
@@ -271,6 +284,17 @@ func getControllerClusterPolicyRules() []rbacv1.PolicyRule {
271284
"update",
272285
},
273286
},
287+
{
288+
APIGroups: []string{
289+
"authorization.k8s.io",
290+
},
291+
Resources: []string{
292+
"subjectaccessreviews",
293+
},
294+
Verbs: []string{
295+
"create",
296+
},
297+
},
274298
}
275299
}
276300

pkg/operator/resources/crds_generated.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,9 @@ type DataImportCronSpec struct {
596596
// RetentionPolicy specifies whether the created DataVolumes and DataSources are retained when their DataImportCron is deleted. Default is RatainAll.
597597
// +optional
598598
RetentionPolicy *DataImportCronRetentionPolicy `json:"retentionPolicy,omitempty"`
599+
// ServiceAccountName is the name of the ServiceAccount for creating DataVolumes.
600+
// +optional
601+
ServiceAccountName string `json:"serviceAccountName,omitempty"`
599602
}
600603

601604
// DataImportCronGarbageCollect represents the DataImportCron garbage collection mode

staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types_swagger_generated.go

Lines changed: 8 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)