From 863fa0b9c114920a4a3dd8d30f004961f185276d Mon Sep 17 00:00:00 2001 From: Matthew Wong Date: Thu, 29 Sep 2022 15:16:11 -0700 Subject: [PATCH] Revert "EFS-CSI pod impersonation implementation" --- .../templates/csidriver.yaml | 7 - .../templates/node-serviceaccount.yaml | 30 ---- charts/aws-efs-csi-driver/values.yaml | 1 - cmd/main.go | 1 - .../base/controller-serviceaccount.yaml | 2 +- deploy/kubernetes/base/csidriver.yaml | 6 - .../kubernetes/base/node-serviceaccount.yaml | 28 ---- .../pod_iam_impersonation/README.md | 83 ----------- .../pod_iam_impersonation/specs/example.yaml | 54 ------- pkg/driver/driver.go | 16 --- pkg/driver/node.go | 133 ------------------ pkg/driver/node_test.go | 58 -------- 12 files changed, 1 insertion(+), 418 deletions(-) delete mode 100644 examples/kubernetes/pod_iam_impersonation/README.md delete mode 100644 examples/kubernetes/pod_iam_impersonation/specs/example.yaml diff --git a/charts/aws-efs-csi-driver/templates/csidriver.yaml b/charts/aws-efs-csi-driver/templates/csidriver.yaml index 3f5c8c316..e6b4d4196 100644 --- a/charts/aws-efs-csi-driver/templates/csidriver.yaml +++ b/charts/aws-efs-csi-driver/templates/csidriver.yaml @@ -8,10 +8,3 @@ metadata: "helm.sh/resource-policy": keep spec: attachRequired: false - {{- if .Values.podIAMAuthorization }} - podInfoOnMount: true - tokenRequests: - - audience: "sts.amazonaws.com" - expirationSeconds: 3600 - requiresRepublish: true - {{- end}} diff --git a/charts/aws-efs-csi-driver/templates/node-serviceaccount.yaml b/charts/aws-efs-csi-driver/templates/node-serviceaccount.yaml index 917995f45..9fd3c7a0f 100644 --- a/charts/aws-efs-csi-driver/templates/node-serviceaccount.yaml +++ b/charts/aws-efs-csi-driver/templates/node-serviceaccount.yaml @@ -10,33 +10,3 @@ metadata: {{- toYaml . | nindent 4 }} {{- end }} {{- end }} - -{{- if .Values.podIAMAuthorization }} ---- -kind: ClusterRole -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: efs-csi-external-provisioner-role-node - labels: - app.kubernetes.io/name: {{ include "aws-efs-csi-driver.name" . }} -rules: - - apiGroups: [""] - resources: ["serviceaccounts"] - verbs: ["get"] ---- -# Source: aws-efs-csi-driver/templates/node-serviceaccount.yaml -kind: ClusterRoleBinding -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: efs-csi-node-binding - labels: - app.kubernetes.io/name: aws-efs-csi-driver -subjects: - - kind: ServiceAccount - name: {{ .Values.node.serviceAccount.name }} - namespace: {{ .Release.Namespace }} -roleRef: - kind: ClusterRole - name: efs-csi-external-provisioner-role-node - apiGroup: rbac.authorization.k8s.io -{{- end }} \ No newline at end of file diff --git a/charts/aws-efs-csi-driver/values.yaml b/charts/aws-efs-csi-driver/values.yaml index c5e0620ba..46d4afb8c 100644 --- a/charts/aws-efs-csi-driver/values.yaml +++ b/charts/aws-efs-csi-driver/values.yaml @@ -8,7 +8,6 @@ fullnameOverride: "" replicaCount: 2 useFIPS: false -podIAMAuthorization: false image: repository: amazon/aws-efs-csi-driver diff --git a/cmd/main.go b/cmd/main.go index 920dcd749..1e4b23a6c 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -60,7 +60,6 @@ func main() { if err != nil { klog.Fatalln(err) } - drv := driver.NewDriver(*endpoint, etcAmazonEfs, *efsUtilsStaticFilesPath, *tags, *volMetricsOptIn, *volMetricsRefreshPeriod, *volMetricsFsRateLimit, *deleteAccessPointRootDir) if err := drv.Run(); err != nil { klog.Fatalln(err) diff --git a/deploy/kubernetes/base/controller-serviceaccount.yaml b/deploy/kubernetes/base/controller-serviceaccount.yaml index 581a18031..655c0268c 100644 --- a/deploy/kubernetes/base/controller-serviceaccount.yaml +++ b/deploy/kubernetes/base/controller-serviceaccount.yaml @@ -50,7 +50,7 @@ metadata: subjects: - kind: ServiceAccount name: efs-csi-controller-sa - namespace: kube-system + namespace: default roleRef: kind: ClusterRole name: efs-csi-external-provisioner-role diff --git a/deploy/kubernetes/base/csidriver.yaml b/deploy/kubernetes/base/csidriver.yaml index 3ad186327..852021d8e 100644 --- a/deploy/kubernetes/base/csidriver.yaml +++ b/deploy/kubernetes/base/csidriver.yaml @@ -10,9 +10,3 @@ metadata: "helm.sh/resource-policy": keep spec: attachRequired: false - podInfoOnMount: true - tokenRequests: - - audience: "sts.amazonaws.com" - expirationSeconds: 3600 - requiresRepublish: true - diff --git a/deploy/kubernetes/base/node-serviceaccount.yaml b/deploy/kubernetes/base/node-serviceaccount.yaml index 022f300a8..9840fb6ad 100644 --- a/deploy/kubernetes/base/node-serviceaccount.yaml +++ b/deploy/kubernetes/base/node-serviceaccount.yaml @@ -6,31 +6,3 @@ metadata: name: efs-csi-node-sa labels: app.kubernetes.io/name: aws-efs-csi-driver ---- -kind: ClusterRole -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: efs-csi-external-provisioner-role-node - labels: - app.kubernetes.io/name: aws-efs-csi-driver -rules: - - apiGroups: [""] - resources: ["serviceaccounts"] - verbs: ["get"] ---- -# Source: aws-efs-csi-driver/templates/node-serviceaccount.yaml -kind: ClusterRoleBinding -apiVersion: rbac.authorization.k8s.io/v1 -metadata: - name: efs-csi-node-binding - labels: - app.kubernetes.io/name: aws-efs-csi-driver -subjects: - - kind: ServiceAccount - name: efs-csi-node-sa - namespace: kube-system -roleRef: - kind: ClusterRole - name: efs-csi-external-provisioner-role-node - apiGroup: rbac.authorization.k8s.io - diff --git a/examples/kubernetes/pod_iam_impersonation/README.md b/examples/kubernetes/pod_iam_impersonation/README.md deleted file mode 100644 index d032add7b..000000000 --- a/examples/kubernetes/pod_iam_impersonation/README.md +++ /dev/null @@ -1,83 +0,0 @@ -## CSI Driver POD IMPERSONATION - -This example shows how you can use the pod impersonation feature available -in the CSI Driver to enforce access control when mounting EFS Access point protected with EFS resource policy. -This feature is for EKS only and leverages IAM roles for service accounts (IRSA), your EKS cluster must be enabled for IRSA. -You can follow this [link](https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html) on how to setup and use IRSA. - -**Note:** This feature only works when installed with Helm and `podIAMAuthorization` is set to `true`. - -### IMPORTANT -This feature will not work if you are upgrading the Helm chart. It MUST be a clean install as the CSI Driver object is immutable. -The field `podIAMAuthorization` in the HELM chart is set to keep backward compatibility in case you need to upgrade/patch an already existing -deployment of the EFS CSI Driver or do not want to use the Pod impersonation feature. - -## Example - -This example assumes you have an access point set up and is protected with a resource policy. -If you would like to learn how to configure IAM policies for access points you can follow this [link](https://docs.aws.amazon.com/efs/latest/ug/efs-access-points.html#access-points-iam-policy). - -### Create the Persistent Volume Spec - -``` -kind: PersistentVolume -metadata: - name: efs-pv1 -spec: - capacity: - storage: 5Gi - volumeMode: Filesystem - accessModes: - - ReadWriteMany - persistentVolumeReclaimPolicy: Retain - storageClassName: efs-sc - csi: - driver: efs.csi.aws.com - volumeHandle: [FileSystemId]::[AccessPointId] - volumeAttributes: - podIAMAuthorization: "true" ---- -apiVersion: v1 -kind: PersistentVolumeClaim -metadata: - name: efs-claim1 -spec: - accessModes: - - ReadWriteMany - storageClassName: efs-sc - resources: - requests: - storage: 5Gi -``` - -Here it is important to set the property `podIAMAuthorization` in `volumeAttribute` to `true`. -This indicates to the EFS CSI Driver when mounting the Access Point to use -the role annotated to the service account associated with the `pod`. - -### Deploy an Application - -You can use the sample application in `examples/kubernetes/pod_iam_impersonation/specs/example.yaml` -to create a Persistent Volume (PV), Persistent Volume Claim (PVC) and a sample application. - -**Note:** The pod defined in the example has a service account associated with it. -In the example the service account is `efs-app-sa`. The service account needs to be create and annotated with a role -that is allowed in the EFS resource policy associated with Access Pointed referenced above in the PV. - -Once you have created the service account you can apply the following command: - -```sh ->> kubectl apply -f examples/kubernetes/pod_iam_impersonation/specs/example.yaml -``` - -### Check EFS filesystem is used -After the objects are created, verify the pod is running: - -```sh ->> kubectl get pods -``` - -You can also verify that data is being written into the EFS file system. - -```sh ->> kubectl exec -ti efs-app-1 -- tail -f /data-dir1/out.txt -``` diff --git a/examples/kubernetes/pod_iam_impersonation/specs/example.yaml b/examples/kubernetes/pod_iam_impersonation/specs/example.yaml deleted file mode 100644 index 5a2f99fe8..000000000 --- a/examples/kubernetes/pod_iam_impersonation/specs/example.yaml +++ /dev/null @@ -1,54 +0,0 @@ -kind: StorageClass -apiVersion: storage.k8s.io/v1 -metadata: - name: efs-sc -provisioner: efs.csi.aws.com ---- -apiVersion: v1 -kind: PersistentVolume -metadata: - name: efs-pv1 -spec: - capacity: - storage: 5Gi - volumeMode: Filesystem - accessModes: - - ReadWriteMany - persistentVolumeReclaimPolicy: Retain - storageClassName: efs-sc - csi: - driver: efs.csi.aws.com - volumeHandle: fs-e8a95a42::fsap-068c22f0246419f75 - volumeAttributes: - podIAMAuthorization: "true" ---- -apiVersion: v1 -kind: PersistentVolumeClaim -metadata: - name: efs-claim1 -spec: - accessModes: - - ReadWriteMany - storageClassName: efs-sc - resources: - requests: - storage: 5Gi ---- -apiVersion: v1 -kind: Pod -metadata: - name: efs-app-1 -spec: - serviceAccountName: efs-app-sa - containers: - - name: app - image: centos - command: ["/bin/sh"] - args: ["-c", "while true; do echo $(date -u) >> /data-dir1/out.txt; sleep 5; done"] - volumeMounts: - - name: efs-volume-1 - mountPath: /data-dir1 - volumes: - - name: efs-volume-1 - persistentVolumeClaim: - claimName: efs-claim1 \ No newline at end of file diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index 354bdf50a..a78d8eb17 100644 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -23,9 +23,6 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi" "google.golang.org/grpc" - "k8s.io/client-go/kubernetes" - k8sv1 "k8s.io/client-go/kubernetes/typed/core/v1" - "k8s.io/client-go/rest" "k8s.io/klog" "github.com/kubernetes-sigs/aws-efs-csi-driver/pkg/cloud" @@ -51,7 +48,6 @@ type Driver struct { gidAllocator GidAllocator deleteAccessPointRootDir bool tags map[string]string - k8sClient k8sv1.CoreV1Interface } func NewDriver(endpoint, efsUtilsCfgPath, efsUtilsStaticFilesPath, tags string, volMetricsOptIn bool, volMetricsRefreshPeriod float64, volMetricsFsRateLimit int, deleteAccessPointRootDir bool) *Driver { @@ -62,17 +58,6 @@ func NewDriver(endpoint, efsUtilsCfgPath, efsUtilsStaticFilesPath, tags string, nodeCaps := SetNodeCapOptInFeatures(volMetricsOptIn) watchdog := newExecWatchdog(efsUtilsCfgPath, efsUtilsStaticFilesPath, "amazon-efs-mount-watchdog") - - cfg, err := rest.InClusterConfig() - if err != nil { - klog.Fatal(err) - } - - clientset, err := kubernetes.NewForConfig(cfg) - if err != nil { - klog.Fatal(err) - } - return &Driver{ endpoint: endpoint, nodeID: cloud.GetMetadata().GetInstanceID(), @@ -87,7 +72,6 @@ func NewDriver(endpoint, efsUtilsCfgPath, efsUtilsStaticFilesPath, tags string, gidAllocator: NewGidAllocator(), deleteAccessPointRootDir: deleteAccessPointRootDir, tags: parseTagsFromStr(strings.TrimSpace(tags)), - k8sClient: clientset.CoreV1(), } } diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 900726f82..cd73422f5 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -18,7 +18,6 @@ package driver import ( "context" - "encoding/json" "fmt" "os" "path" @@ -29,24 +28,9 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - k8sv1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/klog" ) -const ( - jwtTokenFilePath = "/home/token" - roleAnnotationKey = "eks.amazonaws.com/role-arn" - docURL = "https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html" -) - -type podAuth struct { - nameSpace, serviceAccount string - k8sClient k8sv1.CoreV1Interface - ctx context.Context - jwtToken string -} - var ( volumeCapAccessModes = []csi.VolumeCapability_AccessMode_Mode{ csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, @@ -64,16 +48,6 @@ func (d *Driver) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVolu return nil, status.Error(codes.Unimplemented, "") } -func newPodAuth(ctx context.Context, nameSpace, serviceAccount string, k8sClient k8sv1.CoreV1Interface, JWT string) (podauth *podAuth) { - return &podAuth{ - nameSpace: nameSpace, - serviceAccount: serviceAccount, - k8sClient: k8sClient, - ctx: ctx, - jwtToken: JWT, - } -} - func (d *Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) { klog.V(4).Infof("NodePublishVolume: called with args %+v", req) mountOptions := []string{} @@ -99,38 +73,9 @@ func (d *Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolu // TODO when CreateVolume is implemented, it must use the same key names subpath := "/" encryptInTransit := true - iamAuth := false volContext := req.GetVolumeContext() - for k, v := range volContext { switch strings.ToLower(k) { - case "csi.storage.k8s.io/pod.name", "csi.storage.k8s.io/pod.namespace", "csi.storage.k8s.io/pod.uid", "csi.storage.k8s.io/serviceaccount.name", "csi.storage.k8s.io/ephemeral": - continue - - case "csi.storage.k8s.io/serviceaccount.tokens": - //TODO get the token in a cleaner way, not like this. - result := make(map[string]interface{}) - err := json.Unmarshal([]byte(volContext["csi.storage.k8s.io/serviceAccount.tokens"]), &result) - if err != nil { - return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Invalid JSON string for tokens : %s, error %v", v, err)) - } - - //Get JWT token with aud as sts.amazonaws.com - tokenInfo := fmt.Sprintf("%v", result["sts.amazonaws.com"].(map[string]interface{})["token"]) - - // Set the podauth object then call set pod credentials (ROLE-ARN and Toke file) that are called through efs-utils - podauth := newPodAuth(ctx, volContext["csi.storage.k8s.io/pod.namespace"], volContext["csi.storage.k8s.io/serviceAccount.name"], d.k8sClient, tokenInfo) - err = podauth.setPodCredentials() - if err != nil { - return nil, status.Error(codes.Internal, fmt.Sprintf("Unable to set pod iam credentials %v", err)) - } - - case "podiamauthorization": - var err error - iamAuth, err = strconv.ParseBool(v) - if err != nil { - return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("Volume context property %q must be a boolean value: %v", k, err)) - } //Deprecated case "path": klog.Warning("Use of path under volumeAttributes is deprecated. This field will be removed in future release") @@ -175,11 +120,6 @@ func (d *Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolu mountOptions = append(mountOptions, fmt.Sprintf("accesspoint=%s", apid), "tls") } - //Setting the mount option to iam for efs-utils to mount with IAM - if iamAuth { - mountOptions = append(mountOptions, "iam") - } - if encryptInTransit { // The TLS option may have been added above if apid was set // TODO: mountOptions should be a set to avoid all this hasOption checking @@ -241,7 +181,6 @@ func (d *Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolu } klog.V(5).Infof("NodePublishVolume: mounting %s at %s with options %v", source, target, mountOptions) - if err := d.mounter.Mount(source, target, "efs", mountOptions); err != nil { os.Remove(target) return nil, status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) @@ -486,78 +425,6 @@ func parseVolumeId(volumeId string) (fsid, subpath, apid string, err error) { return } -func (p podAuth) setPodCredentials() (e error) { - - err := writeToken(p) - if err != nil { - return err - } - - err = setRoleArn(p) - if err != nil { - return err - } - - return nil -} - -func writeToken(p podAuth) (e error) { - destFile, err := os.Create(jwtTokenFilePath) - if err != nil { - klog.Errorf("Unable to create file at path %s", jwtTokenFilePath) - return err - } - - n, err := destFile.Write([]byte(p.jwtToken)) - if err != nil { - klog.Errorf("Unable to write JWT token at %s", jwtTokenFilePath) - return err - } - klog.Infof("Wrote JWT bytes of length %d", n) - - err = destFile.Chmod(0400) - if err != nil { - klog.Errorf("Unable to change the file mode of %s", jwtTokenFilePath) - return err - } - - err = destFile.Close() - if err != nil { - klog.Errorf("Error while closing file of %s", jwtTokenFilePath) - return err - } - - err = os.Setenv("AWS_WEB_IDENTITY_TOKEN_FILE", jwtTokenFilePath) - if err != nil { - klog.Errorf("Unable to set environment variable %s", err) - return err - } - - return nil -} - -func setRoleArn(p podAuth) (e error) { - response, err := p.k8sClient.ServiceAccounts(p.nameSpace).Get(p.ctx, p.serviceAccount, metav1.GetOptions{}) - if err != nil { - klog.Errorf("Unable to get the service account description %s", err) - return err - } - roleArn := response.Annotations[roleAnnotationKey] - - if len(roleArn) <= 0 { - klog.Errorf("Need IAM role for service account %s (namespace: %s) - %s", p.serviceAccount, p.nameSpace, docURL) - return fmt.Errorf("An IAM role must be associated with service account %s (namespace: %s)", p.serviceAccount, p.nameSpace) - } - - err = os.Setenv("AWS_ROLE_ARN", roleArn) - if err != nil { - klog.Errorf("Unable to set environment variable %s", err) - return err - } - - return nil -} - // Check and avoid adding duplicate mount options func hasOption(options []string, opt string) bool { for _, o := range options { diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index 12923ab07..20c55cd90 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -172,26 +172,6 @@ func TestNodePublishVolume(t *testing.T) { mountArgs: []interface{}{volumeId + ":/", targetPath, "efs", []string{"tls"}}, mountSuccess: true, }, - { - name: "success: normal with iam mount options", - req: &csi.NodePublishVolumeRequest{ - VolumeId: volumeId, - VolumeCapability: &csi.VolumeCapability{ - AccessType: &csi.VolumeCapability_Mount{ - Mount: &csi.VolumeCapability_MountVolume{ - MountFlags: []string{"iam"}, - }, - }, - AccessMode: &csi.VolumeCapability_AccessMode{ - Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, - }, - }, - TargetPath: targetPath, - }, - expectMakeDir: true, - mountArgs: []interface{}{volumeId + ":/", targetPath, "efs", []string{"tls", "iam"}}, - mountSuccess: true, - }, { // TODO: Validate deprecation warning name: "success: normal with path in volume context", @@ -323,30 +303,6 @@ func TestNodePublishVolume(t *testing.T) { mountArgs: []interface{}{volumeId + ":/", targetPath, "efs", []string{}}, mountSuccess: true, }, - { - name: "success: normal with podIAMAuthorization true volume context", - req: &csi.NodePublishVolumeRequest{ - VolumeId: volumeId + "::" + accessPointID, - VolumeCapability: stdVolCap, - TargetPath: targetPath, - VolumeContext: map[string]string{"podIAMAuthorization": "true"}, - }, - expectMakeDir: true, - mountArgs: []interface{}{volumeId + ":/", targetPath, "efs", []string{"accesspoint=" + accessPointID, "tls", "iam"}}, - mountSuccess: true, - }, - { - name: "success: normal with podIAMAuthorization true volume context", - req: &csi.NodePublishVolumeRequest{ - VolumeId: volumeId + "::" + accessPointID, - VolumeCapability: stdVolCap, - TargetPath: targetPath, - VolumeContext: map[string]string{"podIAMAuthorization": "false"}, - }, - expectMakeDir: true, - mountArgs: []interface{}{volumeId + ":/", targetPath, "efs", []string{"accesspoint=" + accessPointID, "tls"}}, - mountSuccess: true, - }, { name: "success: normal with volume context populated from dynamic provisioning", req: &csi.NodePublishVolumeRequest{ @@ -612,20 +568,6 @@ func TestNodePublishVolume(t *testing.T) { message: "Found tls in mountOptions but encryptInTransit is false", }, }, - { - name: "fail: normal with podIAMAuthorization true volume context", - req: &csi.NodePublishVolumeRequest{ - VolumeId: volumeId + "::" + accessPointID, - VolumeCapability: stdVolCap, - TargetPath: targetPath, - VolumeContext: map[string]string{"podIAMAuthorization": "aaa"}, - }, - expectMakeDir: false, - expectError: errtyp{ - code: "InvalidArgument", - message: "Volume context property \"podIAMAuthorization\" must be a boolean value: strconv.ParseBool: parsing \"aaa\": invalid syntax", - }, - }, { name: "fail: encryptInTransit invalid boolean value volume context", req: &csi.NodePublishVolumeRequest{