From 3e97126a5f926473a82258c456804d258a238270 Mon Sep 17 00:00:00 2001 From: Alex Kalenyuk Date: Thu, 9 Mar 2023 13:04:46 +0200 Subject: [PATCH] Move pkg/clone/auth to API lib and introduce single dv.Authorize() This way other projects don't have to vendor in CDI, the API lib will be enough. While it's not a huge deal at this point, each existing dependency is an entry door for pulling in more stuff in the future by accident. Signed-off-by: Alex Kalenyuk --- pkg/apiserver/webhooks/BUILD.bazel | 2 - pkg/apiserver/webhooks/datavolume-mutate.go | 153 ++++-------------- pkg/clone/BUILD.bazel | 14 -- .../containerized-data-importer-api/go.mod | 3 +- .../containerized-data-importer-api/go.sum | 2 + .../pkg/apis/core/v1beta1/BUILD.bazel | 7 + .../pkg/apis/core/v1beta1/authorize.go | 98 +++++++++++ .../pkg/apis/core/v1beta1/authorize_utils.go | 146 ++++++++++++++--- tests/BUILD.bazel | 1 - tests/webhook_test.go | 10 +- 10 files changed, 262 insertions(+), 174 deletions(-) delete mode 100644 pkg/clone/BUILD.bazel create mode 100644 staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/authorize.go rename pkg/clone/auth.go => staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/authorize_utils.go (64%) diff --git a/pkg/apiserver/webhooks/BUILD.bazel b/pkg/apiserver/webhooks/BUILD.bazel index 0990896c0c..46decbac09 100644 --- a/pkg/apiserver/webhooks/BUILD.bazel +++ b/pkg/apiserver/webhooks/BUILD.bazel @@ -15,14 +15,12 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/client/clientset/versioned:go_default_library", - "//pkg/clone:go_default_library", "//pkg/common:go_default_library", "//pkg/controller/common:go_default_library", "//pkg/token:go_default_library", "//staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library", "//vendor/github.com/appscode/jsonpatch:go_default_library", "//vendor/github.com/gorhill/cronexpr:go_default_library", - "//vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1:go_default_library", "//vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/clientset/versioned:go_default_library", "//vendor/k8s.io/api/admission/v1:go_default_library", "//vendor/k8s.io/api/admissionregistration/v1:go_default_library", diff --git a/pkg/apiserver/webhooks/datavolume-mutate.go b/pkg/apiserver/webhooks/datavolume-mutate.go index 703dd3eb74..5bfb7d5407 100644 --- a/pkg/apiserver/webhooks/datavolume-mutate.go +++ b/pkg/apiserver/webhooks/datavolume-mutate.go @@ -23,10 +23,9 @@ import ( "context" "encoding/json" - snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" admissionv1 "k8s.io/api/admission/v1" authv1 "k8s.io/api/authorization/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8sfield "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/kubernetes" @@ -34,7 +33,6 @@ import ( cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" cdiclient "kubevirt.io/containerized-data-importer/pkg/client/clientset/versioned" - "kubevirt.io/containerized-data-importer/pkg/clone" "kubevirt.io/containerized-data-importer/pkg/common" cc "kubevirt.io/containerized-data-importer/pkg/controller/common" "kubevirt.io/containerized-data-importer/pkg/token" @@ -44,45 +42,31 @@ type dataVolumeMutatingWebhook struct { k8sClient kubernetes.Interface cdiClient cdiclient.Interface tokenGenerator token.Generator - proxy clone.SubjectAccessReviewsProxy + proxy cdiv1.SubjectAccessReviewsProxy } type sarProxy struct { client kubernetes.Interface } -type cloneType int - -const ( - noClone cloneType = iota - pvcClone - snapshotClone -) +func (p *sarProxy) Create(sar *authv1.SubjectAccessReview) (*authv1.SubjectAccessReview, error) { + return p.client.AuthorizationV1().SubjectAccessReviews().Create(context.TODO(), sar, metav1.CreateOptions{}) +} -type cloneSourceHandler struct { - cloneType cloneType - tokenResource metav1.GroupVersionResource - cloneAuthFunc clone.UserCloneAuthFunc - sourceName string - sourceNamespace string +type nsProxy struct { + client kubernetes.Interface } -var ( - tokenResourcePvc = metav1.GroupVersionResource{ - Group: "", - Version: "v1", - Resource: "persistentvolumeclaims", - } +func (p *nsProxy) Get(name string) (*corev1.Namespace, error) { + return p.client.CoreV1().Namespaces().Get(context.TODO(), name, metav1.GetOptions{}) +} - tokenResourceSnapshot = metav1.GroupVersionResource{ - Group: snapshotv1.GroupName, - Version: snapshotv1.SchemeGroupVersion.Version, - Resource: "volumesnapshots", - } -) +type dsProxy struct { + client cdiclient.Interface +} -func (p *sarProxy) Create(sar *authv1.SubjectAccessReview) (*authv1.SubjectAccessReview, error) { - return p.client.AuthorizationV1().SubjectAccessReviews().Create(context.TODO(), sar, metav1.CreateOptions{}) +func (p *dsProxy) Get(namespace, name string) (*cdiv1.DataSource, error) { + return p.client.CdiV1beta1().DataSources(namespace).Get(context.TODO(), name, metav1.GetOptions{}) } func (wh *dataVolumeMutatingWebhook) Admit(ar admissionv1.AdmissionReview) *admissionv1.AdmissionResponse { @@ -120,10 +104,6 @@ func (wh *dataVolumeMutatingWebhook) Admit(ar admissionv1.AdmissionReview) *admi } } - _, prePopulated := dataVolume.Annotations[cc.AnnPrePopulated] - _, checkStaticVolume := dataVolume.Annotations[cc.AnnCheckStaticVolume] - noTokenOkay := prePopulated || checkStaticVolume - targetNamespace, targetName := dataVolume.Namespace, dataVolume.Name if targetNamespace == "" { targetNamespace = ar.Request.Namespace @@ -133,52 +113,15 @@ func (wh *dataVolumeMutatingWebhook) Admit(ar admissionv1.AdmissionReview) *admi targetName = ar.Request.Name } - cloneSourceHandler, err := newCloneSourceHandler(dataVolume, wh.cdiClient) + ok, reason, response, err := modifiedDataVolume.Authorize(ar.Request.Namespace, ar.Request.Name, &sarProxy{client: wh.k8sClient}, &nsProxy{client: wh.k8sClient}, &dsProxy{client: wh.cdiClient}, ar.Request.UserInfo) if err != nil { - if k8serrors.IsNotFound(err) && noTokenOkay { - // no token needed, likely since no datasource - klog.V(3).Infof("DataVolume %s/%s is pre/static populated, not adding token, no datasource", targetNamespace, targetName) + if err == cdiv1.ErrNoTokenOkay { return toPatchResponse(dataVolume, modifiedDataVolume) } return toAdmissionResponseError(err) } - if cloneSourceHandler.cloneType == noClone { - klog.V(3).Infof("DataVolume %s/%s not cloning", targetNamespace, targetName) - return toPatchResponse(dataVolume, modifiedDataVolume) - } - - // only add token at create time - if ar.Request.Operation != admissionv1.Create { - return toPatchResponse(dataVolume, modifiedDataVolume) - } - - sourceName, sourceNamespace := cloneSourceHandler.sourceName, cloneSourceHandler.sourceNamespace - if sourceNamespace == "" { - sourceNamespace = targetNamespace - } - - _, err = wh.k8sClient.CoreV1().Namespaces().Get(context.TODO(), sourceNamespace, metav1.GetOptions{}) - if err != nil { - if k8serrors.IsNotFound(err) && noTokenOkay { - // no token needed, likely since no source namespace - klog.V(3).Infof("DataVolume %s/%s is pre/static populated, not adding token, no source namespace", targetNamespace, targetName) - return toPatchResponse(dataVolume, modifiedDataVolume) - } - return toAdmissionResponseError(err) - } - - ok, reason, err := cloneSourceHandler.cloneAuthFunc(wh.proxy, sourceNamespace, sourceName, targetNamespace, ar.Request.UserInfo) - if err != nil { - return toAdmissionResponseError(err) - } - if !ok { - if noTokenOkay { - klog.V(3).Infof("DataVolume %s/%s is pre/static populated, not adding token, auth failed", targetNamespace, targetName) - return toPatchResponse(dataVolume, modifiedDataVolume) - } - causes := []metav1.StatusCause{ { Type: metav1.CauseTypeFieldValueInvalid, @@ -189,11 +132,21 @@ func (wh *dataVolumeMutatingWebhook) Admit(ar admissionv1.AdmissionReview) *admi return toRejectedAdmissionResponse(causes) } + // only add token at create time + if ar.Request.Operation != admissionv1.Create { + return toPatchResponse(dataVolume, modifiedDataVolume) + } + + sourceName, sourceNamespace := response.SourceName, response.SourceNamespace + if sourceNamespace == "" { + sourceNamespace = targetNamespace + } + tokenData := &token.Payload{ Operation: token.OperationClone, Name: sourceName, Namespace: sourceNamespace, - Resource: cloneSourceHandler.tokenResource, + Resource: response.TokenResource, Params: map[string]string{ "targetNamespace": targetNamespace, "targetName": targetName, @@ -214,53 +167,3 @@ func (wh *dataVolumeMutatingWebhook) Admit(ar admissionv1.AdmissionReview) *admi return toPatchResponse(dataVolume, modifiedDataVolume) } - -func newCloneSourceHandler(dataVolume *cdiv1.DataVolume, cdiClient cdiclient.Interface) (*cloneSourceHandler, error) { - var pvcSource *cdiv1.DataVolumeSourcePVC - var snapshotSource *cdiv1.DataVolumeSourceSnapshot - - if dataVolume.Spec.Source != nil { - if dataVolume.Spec.Source.PVC != nil { - pvcSource = dataVolume.Spec.Source.PVC - } else if dataVolume.Spec.Source.Snapshot != nil { - snapshotSource = dataVolume.Spec.Source.Snapshot - } - } else if dataVolume.Spec.SourceRef != nil && dataVolume.Spec.SourceRef.Kind == cdiv1.DataVolumeDataSource { - ns := dataVolume.Namespace - if dataVolume.Spec.SourceRef.Namespace != nil && *dataVolume.Spec.SourceRef.Namespace != "" { - ns = *dataVolume.Spec.SourceRef.Namespace - } - dataSource, err := cdiClient.CdiV1beta1().DataSources(ns).Get(context.TODO(), dataVolume.Spec.SourceRef.Name, metav1.GetOptions{}) - if err != nil { - return nil, err - } - if dataSource.Spec.Source.PVC != nil { - pvcSource = dataSource.Spec.Source.PVC - } else if dataSource.Spec.Source.Snapshot != nil { - snapshotSource = dataSource.Spec.Source.Snapshot - } - } - - switch { - case pvcSource != nil: - return &cloneSourceHandler{ - cloneType: pvcClone, - tokenResource: tokenResourcePvc, - cloneAuthFunc: clone.CanUserClonePVC, - sourceName: pvcSource.Name, - sourceNamespace: pvcSource.Namespace, - }, nil - case snapshotSource != nil: - return &cloneSourceHandler{ - cloneType: snapshotClone, - tokenResource: tokenResourceSnapshot, - cloneAuthFunc: clone.CanUserCloneSnapshot, - sourceName: snapshotSource.Name, - sourceNamespace: snapshotSource.Namespace, - }, nil - default: - return &cloneSourceHandler{ - cloneType: noClone, - }, nil - } -} diff --git a/pkg/clone/BUILD.bazel b/pkg/clone/BUILD.bazel deleted file mode 100644 index d4ab127cd2..0000000000 --- a/pkg/clone/BUILD.bazel +++ /dev/null @@ -1,14 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") - -go_library( - name = "go_default_library", - srcs = ["auth.go"], - importpath = "kubevirt.io/containerized-data-importer/pkg/clone", - visibility = ["//visibility:public"], - deps = [ - "//staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library", - "//vendor/k8s.io/api/authentication/v1:go_default_library", - "//vendor/k8s.io/api/authorization/v1:go_default_library", - "//vendor/k8s.io/klog/v2:go_default_library", - ], -) diff --git a/staging/src/kubevirt.io/containerized-data-importer-api/go.mod b/staging/src/kubevirt.io/containerized-data-importer-api/go.mod index 4014ac3968..394ff37f60 100644 --- a/staging/src/kubevirt.io/containerized-data-importer-api/go.mod +++ b/staging/src/kubevirt.io/containerized-data-importer-api/go.mod @@ -3,9 +3,11 @@ module kubevirt.io/containerized-data-importer-api go 1.19 require ( + github.com/kubernetes-csi/external-snapshotter/client/v6 v6.0.1 github.com/openshift/api v0.0.0-20211217221424-8779abfbd571 k8s.io/api v0.23.5 k8s.io/apimachinery v0.23.5 + k8s.io/klog/v2 v2.40.1 kubevirt.io/controller-lifecycle-operator-sdk/api v0.0.0-20220329064328-f3cc58c6ed90 ) @@ -22,7 +24,6 @@ require ( golang.org/x/text v0.7.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - k8s.io/klog/v2 v2.40.1 // indirect k8s.io/utils v0.0.0-20211116205334-6203023598ed // indirect sigs.k8s.io/json v0.0.0-20211020170558-c049b76a60c6 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect diff --git a/staging/src/kubevirt.io/containerized-data-importer-api/go.sum b/staging/src/kubevirt.io/containerized-data-importer-api/go.sum index b1473055fd..36e2f048cb 100644 --- a/staging/src/kubevirt.io/containerized-data-importer-api/go.sum +++ b/staging/src/kubevirt.io/containerized-data-importer-api/go.sum @@ -92,6 +92,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/kubernetes-csi/external-snapshotter/client/v6 v6.0.1 h1:OqBS3UAo3eGWplYXoMLaWnx/7Zj5Ogh0VO/FuVOL+/o= +github.com/kubernetes-csi/external-snapshotter/client/v6 v6.0.1/go.mod h1:tnHiLn3P10N95fjn7O40QH5ovN0EFGAxqdTpUMrX6bU= github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= github.com/mailru/easyjson v0.7.6/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= diff --git a/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/BUILD.bazel b/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/BUILD.bazel index cfedd463f5..adc6380934 100644 --- a/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/BUILD.bazel +++ b/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/BUILD.bazel @@ -3,6 +3,8 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", srcs = [ + "authorize.go", + "authorize_utils.go", "doc.go", "register.go", "types.go", @@ -15,11 +17,16 @@ go_library( visibility = ["//visibility:public"], deps = [ "//staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core:go_default_library", + "//vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1:go_default_library", "//vendor/github.com/openshift/api/config/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/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//vendor/k8s.io/klog/v2:go_default_library", "//vendor/kubevirt.io/controller-lifecycle-operator-sdk/api:go_default_library", ], ) diff --git a/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/authorize.go b/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/authorize.go new file mode 100644 index 0000000000..20980e0366 --- /dev/null +++ b/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/authorize.go @@ -0,0 +1,98 @@ +/* +Copyright 2020 The CDI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 + +import ( + "errors" + + authentication "k8s.io/api/authentication/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/klog/v2" + + "kubevirt.io/containerized-data-importer-api/pkg/apis/core" +) + +const ( + // AnnPrePopulated is a PVC annotation telling the datavolume controller that the PVC is already populated + AnnPrePopulated = core.GroupName + "/storage.prePopulated" + // AnnCheckStaticVolume checks if a statically allocated PV exists before creating the target PVC. + // If so, PVC is still created but population is skipped + AnnCheckStaticVolume = core.GroupName + "/storage.checkStaticVolume" +) + +var ErrNoTokenOkay = errors.New("proceeding without token is okay under the circumstances") + +// Authorize indicates if the creating context is authorized to create the data volume +// For sources other than clone (import/upload/etc), this is a no-op +func (dv *DataVolume) Authorize(requestNamespace, requestName string, sarClient SubjectAccessReviewsProxy, nsClient NamespaceProxy, dsClient DataSourceProxy, userInfo authentication.UserInfo) (bool, string, cloneSourceHandler, error) { + _, prePopulated := dv.Annotations[AnnPrePopulated] + _, checkStaticVolume := dv.Annotations[AnnCheckStaticVolume] + noTokenOkay := prePopulated || checkStaticVolume + + targetNamespace, targetName := dv.Namespace, dv.Name + if targetNamespace == "" { + targetNamespace = requestNamespace + } + + if targetName == "" { + targetName = requestName + } + + cloneSourceHandler, err := newCloneSourceHandler(dv, dsClient) + if err != nil { + if k8serrors.IsNotFound(err) && noTokenOkay { + // no token needed, likely since no datasource + klog.V(3).Infof("DataVolume %s/%s is pre/static populated, not adding token, no datasource", targetNamespace, targetName) + return true, "", cloneSourceHandler, ErrNoTokenOkay + } + return false, "", cloneSourceHandler, err + } + + if cloneSourceHandler.CloneType == noClone { + klog.V(3).Infof("DataVolume %s/%s not cloning", targetNamespace, targetName) + return true, "", cloneSourceHandler, ErrNoTokenOkay + } + + sourceName, sourceNamespace := cloneSourceHandler.SourceName, cloneSourceHandler.SourceNamespace + if sourceNamespace == "" { + sourceNamespace = targetNamespace + } + + _, err = nsClient.Get(sourceNamespace) + if err != nil { + if k8serrors.IsNotFound(err) && noTokenOkay { + // no token needed, likely since no source namespace + klog.V(3).Infof("DataVolume %s/%s is pre/static populated, not adding token, no source namespace", targetNamespace, targetName) + return true, "", cloneSourceHandler, ErrNoTokenOkay + } + return false, "", cloneSourceHandler, err + } + + ok, reason, err := cloneSourceHandler.CloneAuthFunc(sarClient, sourceNamespace, sourceName, targetNamespace, userInfo) + if err != nil { + return false, reason, cloneSourceHandler, err + } + + if !ok { + if noTokenOkay { + klog.V(3).Infof("DataVolume %s/%s is pre/static populated, not adding token, auth failed", targetNamespace, targetName) + return true, "", cloneSourceHandler, ErrNoTokenOkay + } + } + + return ok, reason, cloneSourceHandler, err +} diff --git a/pkg/clone/auth.go b/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/authorize_utils.go similarity index 64% rename from pkg/clone/auth.go rename to staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/authorize_utils.go index 740d47e643..313555d314 100644 --- a/pkg/clone/auth.go +++ b/staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/authorize_utils.go @@ -1,47 +1,141 @@ /* - * This file is part of the CDI project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * Copyright 2019 Red Hat, Inc. - * - */ - -package clone +Copyright 2020 The CDI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 import ( "fmt" + snapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" authentication "k8s.io/api/authentication/v1" authorization "k8s.io/api/authorization/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog/v2" +) + +func newCloneSourceHandler(dataVolume *DataVolume, dsClient DataSourceProxy) (cloneSourceHandler, error) { + var pvcSource *DataVolumeSourcePVC + var snapshotSource *DataVolumeSourceSnapshot + + if dataVolume.Spec.Source != nil { + if dataVolume.Spec.Source.PVC != nil { + pvcSource = dataVolume.Spec.Source.PVC + } else if dataVolume.Spec.Source.Snapshot != nil { + snapshotSource = dataVolume.Spec.Source.Snapshot + } + } else if dataVolume.Spec.SourceRef != nil && dataVolume.Spec.SourceRef.Kind == DataVolumeDataSource { + ns := dataVolume.Namespace + if dataVolume.Spec.SourceRef.Namespace != nil && *dataVolume.Spec.SourceRef.Namespace != "" { + ns = *dataVolume.Spec.SourceRef.Namespace + } + dataSource, err := dsClient.Get(ns, dataVolume.Spec.SourceRef.Name) + if err != nil { + return cloneSourceHandler{}, err + } + if dataSource.Spec.Source.PVC != nil { + pvcSource = dataSource.Spec.Source.PVC + } else if dataSource.Spec.Source.Snapshot != nil { + snapshotSource = dataSource.Spec.Source.Snapshot + } + } + + switch { + case pvcSource != nil: + return cloneSourceHandler{ + CloneType: pvcClone, + TokenResource: tokenResourcePvc, + CloneAuthFunc: CanUserClonePVC, + SourceName: pvcSource.Name, + SourceNamespace: pvcSource.Namespace, + }, nil + case snapshotSource != nil: + return cloneSourceHandler{ + CloneType: snapshotClone, + TokenResource: tokenResourceSnapshot, + CloneAuthFunc: CanUserCloneSnapshot, + SourceName: snapshotSource.Name, + SourceNamespace: snapshotSource.Namespace, + }, nil + default: + return cloneSourceHandler{ + CloneType: noClone, + }, nil + } +} - cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" +var ( + tokenResourcePvc = metav1.GroupVersionResource{ + Group: "", + Version: "v1", + Resource: "persistentvolumeclaims", + } + + tokenResourceSnapshot = metav1.GroupVersionResource{ + Group: snapshotv1.GroupName, + Version: snapshotv1.SchemeGroupVersion.Version, + Resource: "volumesnapshots", + } +) + +type cloneType int + +const ( + noClone cloneType = iota + pvcClone + snapshotClone ) +// +k8s:openapi-gen=false +type cloneSourceHandler struct { + CloneType cloneType + TokenResource metav1.GroupVersionResource + CloneAuthFunc UserCloneAuthFunc + SourceName string + SourceNamespace string +} + // SubjectAccessReviewsProxy proxies calls to work with SubjectAccessReviews type SubjectAccessReviewsProxy interface { Create(*authorization.SubjectAccessReview) (*authorization.SubjectAccessReview, error) } +// DataSourceProxy proxies calls to work with DataSources +type DataSourceProxy interface { + Get(string, string) (*DataSource, error) +} + +// NamespaceProxy proxies calls to work with Namespaces +type NamespaceProxy interface { + Get(string) (*corev1.Namespace, error) +} + +// AuthorizationHelperProxy proxies calls to APIs used for DV authorization +type AuthorizationHelperProxy interface { + CreateSar(*authorization.SubjectAccessReview) (*authorization.SubjectAccessReview, error) + GetNamespace(string) (*corev1.Namespace, error) + GetDataSource(string, string) (*DataSource, error) +} + // UserCloneAuthFunc represents a user clone auth func type UserCloneAuthFunc func(client SubjectAccessReviewsProxy, sourceNamespace, pvcName, targetNamespace string, userInfo authentication.UserInfo) (bool, string, error) // ServiceAccountCloneAuthFunc represents a serviceaccount clone auth func type ServiceAccountCloneAuthFunc func(client SubjectAccessReviewsProxy, pvcNamespace, pvcName, saNamespace, saName string) (bool, string, error) -type getResourceAttributesFunc func(namespace, name string) []authorization.ResourceAttributes - // CanUserClonePVC checks if a user has "appropriate" permission to clone from the given PVC func CanUserClonePVC(client SubjectAccessReviewsProxy, sourceNamespace, pvcName, targetNamespace string, userInfo authentication.UserInfo) (bool, string, error) { @@ -211,9 +305,9 @@ func getResourceAttributesPvc(namespace, name string) []authorization.ResourceAt { Namespace: namespace, Verb: "create", - Group: cdiv1.SchemeGroupVersion.Group, + Group: SchemeGroupVersion.Group, Resource: "datavolumes", - Subresource: cdiv1.DataVolumeCloneSourceSubresource, + Subresource: DataVolumeCloneSourceSubresource, Name: name, }, { @@ -229,9 +323,9 @@ func getExplicitResourceAttributeSnapshot(namespace, name string) authorization. return authorization.ResourceAttributes{ Namespace: namespace, Verb: "create", - Group: cdiv1.SchemeGroupVersion.Group, + Group: SchemeGroupVersion.Group, Resource: "datavolumes", - Subresource: cdiv1.DataVolumeCloneSourceSubresource, + Subresource: DataVolumeCloneSourceSubresource, Name: name, } } diff --git a/tests/BUILD.bazel b/tests/BUILD.bazel index 69f52468eb..0ed3fe1617 100644 --- a/tests/BUILD.bazel +++ b/tests/BUILD.bazel @@ -49,7 +49,6 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/client/clientset/versioned:go_default_library", - "//pkg/clone:go_default_library", "//pkg/common:go_default_library", "//pkg/controller:go_default_library", "//pkg/controller/common:go_default_library", diff --git a/tests/webhook_test.go b/tests/webhook_test.go index 3ed447987a..b0dbb9e1ee 100644 --- a/tests/webhook_test.go +++ b/tests/webhook_test.go @@ -17,7 +17,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" - "kubevirt.io/containerized-data-importer/pkg/clone" + cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1" "kubevirt.io/containerized-data-importer/tests/framework" "kubevirt.io/containerized-data-importer/tests/utils" ) @@ -235,7 +235,7 @@ var _ = Describe("Clone Auth Webhook tests", func() { Expect(err).To(HaveOccurred()) // let's do manual check as well - allowed, reason, err := clone.CanServiceAccountClonePVC(&sarProxy{client: f.K8sClient}, + allowed, reason, err := cdiv1.CanServiceAccountClonePVC(&sarProxy{client: f.K8sClient}, srcPVCDef.Namespace, srcPVCDef.Name, targetNamespace.Name, @@ -260,7 +260,7 @@ var _ = Describe("Clone Auth Webhook tests", func() { }, 60*time.Second, 2*time.Second).ShouldNot(HaveOccurred()) // let's do another manual check as well - allowed, reason, err = clone.CanServiceAccountClonePVC(&sarProxy{client: f.K8sClient}, + allowed, reason, err = cdiv1.CanServiceAccountClonePVC(&sarProxy{client: f.K8sClient}, srcPVCDef.Namespace, srcPVCDef.Name, targetNamespace.Name, @@ -319,7 +319,7 @@ var _ = Describe("Clone Auth Webhook tests", func() { Expect(err).To(HaveOccurred()) // let's do manual check as well - allowed, reason, err := clone.CanServiceAccountCloneSnapshot(&sarProxy{client: f.K8sClient}, + allowed, reason, err := cdiv1.CanServiceAccountCloneSnapshot(&sarProxy{client: f.K8sClient}, srcPVCDef.Namespace, srcPVCDef.Name, targetNamespace.Name, @@ -354,7 +354,7 @@ var _ = Describe("Clone Auth Webhook tests", func() { }, 60*time.Second, 2*time.Second).ShouldNot(HaveOccurred()) // let's do another manual check as well - allowed, reason, err = clone.CanServiceAccountCloneSnapshot(&sarProxy{client: f.K8sClient}, + allowed, reason, err = cdiv1.CanServiceAccountCloneSnapshot(&sarProxy{client: f.K8sClient}, srcPVCDef.Namespace, srcPVCDef.Name, targetNamespace.Name,