diff --git a/pkg/driver/controller_modify_volume.go b/pkg/driver/controller_modify_volume.go index cbfceaa7c2..09b8e1d4ed 100644 --- a/pkg/driver/controller_modify_volume.go +++ b/pkg/driver/controller_modify_volume.go @@ -200,6 +200,8 @@ func parseModifyVolumeParameters(params map[string]string) (*modifyVolumeRequest options.modifyTagsOptions.TagsToAdd[st[0]] = st[1] } else if strings.HasPrefix(key, ModificationDeleteTag) { options.modifyTagsOptions.TagsToDelete = append(options.modifyTagsOptions.TagsToDelete, value) + } else { + return nil, status.Errorf(codes.InvalidArgument, "Invalid mutable parameter key: %s", key) } } } diff --git a/pkg/driver/controller_modify_volume_test.go b/pkg/driver/controller_modify_volume_test.go index 5e5eea68da..67b7748d87 100644 --- a/pkg/driver/controller_modify_volume_test.go +++ b/pkg/driver/controller_modify_volume_test.go @@ -37,6 +37,7 @@ const ( tagSpecificationWithNoEqual = "key1" validTagDeletion = "key2" invalidTagSpecification = "aws:test=TEST" + invalidParameter = "invalid_parameter" ) func TestParseModifyVolumeParameters(t *testing.T) { @@ -125,6 +126,13 @@ func TestParseModifyVolumeParameters(t *testing.T) { }, expectError: true, }, + { + name: "invalid VAC parameter", + params: map[string]string{ + invalidParameter: "20", + }, + expectError: true, + }, } for _, tc := range testCases { diff --git a/tests/e2e/modify_volume.go b/tests/e2e/modify_volume.go index b9fc348a01..1c0382174b 100644 --- a/tests/e2e/modify_volume.go +++ b/tests/e2e/modify_volume.go @@ -37,33 +37,33 @@ var ( modifyVolumeTests = map[string]testsuites.ModifyVolumeTest{ "with a new iops annotation": { CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters, - ModifyVolumeAnnotations: map[string]string{ - testsuites.AnnotationIops: "4000", + ModifyVolumeParameters: map[string]string{ + testsuites.Iops: "4000", }, ShouldResizeVolume: false, ShouldTestInvalidModificationRecovery: false, }, "with a new io2 volumeType annotation": { CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters, - ModifyVolumeAnnotations: map[string]string{ - testsuites.AnnotationVolumeType: awscloud.VolumeTypeIO2, - testsuites.AnnotationIops: testsuites.DefaultIopsIoVolumes, // As of aws-ebs-csi-driver v1.25.0, parameter iops must be re-specified when modifying volumeType io2 volumes. + ModifyVolumeParameters: map[string]string{ + testsuites.VolumeType: awscloud.VolumeTypeIO2, + testsuites.Iops: testsuites.DefaultIopsIoVolumes, // As of aws-ebs-csi-driver v1.25.0, parameter iops must be re-specified when modifying volumeType io2 volumes. }, ShouldResizeVolume: false, ShouldTestInvalidModificationRecovery: false, }, "with a new throughput annotation": { CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters, - ModifyVolumeAnnotations: map[string]string{ - testsuites.AnnotationThroughput: "150", + ModifyVolumeParameters: map[string]string{ + testsuites.Throughput: "150", }, ShouldResizeVolume: false, ShouldTestInvalidModificationRecovery: false, }, "with a new tag annotation": { CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters, - ModifyVolumeAnnotations: map[string]string{ - testsuites.AnnotationsTagSpec: "key1=test1", + ModifyVolumeParameters: map[string]string{ + testsuites.TagSpec: "key1=test1", }, ShouldResizeVolume: false, ShouldTestInvalidModificationRecovery: false, @@ -71,19 +71,19 @@ var ( }, "with new throughput, and iops annotations": { CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters, - ModifyVolumeAnnotations: map[string]string{ - testsuites.AnnotationIops: "4000", - testsuites.AnnotationThroughput: "150", + ModifyVolumeParameters: map[string]string{ + testsuites.Iops: "4000", + testsuites.Throughput: "150", }, ShouldResizeVolume: false, ShouldTestInvalidModificationRecovery: false, }, "with new throughput, iops, and tag annotations": { CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters, - ModifyVolumeAnnotations: map[string]string{ - testsuites.AnnotationIops: "4000", - testsuites.AnnotationThroughput: "150", - testsuites.AnnotationsTagSpec: "key2=test2", + ModifyVolumeParameters: map[string]string{ + testsuites.Iops: "4000", + testsuites.Throughput: "150", + testsuites.TagSpec: "key2=test2", }, ShouldResizeVolume: false, ShouldTestInvalidModificationRecovery: false, @@ -91,18 +91,18 @@ var ( }, "with a larger size and new throughput and iops annotations": { CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters, - ModifyVolumeAnnotations: map[string]string{ - testsuites.AnnotationIops: "4000", - testsuites.AnnotationThroughput: "150", + ModifyVolumeParameters: map[string]string{ + testsuites.Iops: "4000", + testsuites.Throughput: "150", }, ShouldResizeVolume: true, ShouldTestInvalidModificationRecovery: false, }, "with a larger size and new throughput and iops annotations after providing an invalid annotation": { CreateVolumeParameters: defaultModifyVolumeTestGp3CreateVolumeParameters, - ModifyVolumeAnnotations: map[string]string{ - testsuites.AnnotationIops: "4000", - testsuites.AnnotationThroughput: "150", + ModifyVolumeParameters: map[string]string{ + testsuites.Iops: "4000", + testsuites.Throughput: "150", }, ShouldResizeVolume: true, ShouldTestInvalidModificationRecovery: true, @@ -113,10 +113,10 @@ var ( ebscsidriver.FSTypeKey: ebscsidriver.FSTypeExt4, ebscsidriver.IopsKey: testsuites.DefaultIopsIoVolumes, }, - ModifyVolumeAnnotations: map[string]string{ - testsuites.AnnotationVolumeType: awscloud.VolumeTypeGP3, - testsuites.AnnotationIops: "4000", - testsuites.AnnotationThroughput: "150", + ModifyVolumeParameters: map[string]string{ + testsuites.VolumeType: awscloud.VolumeTypeGP3, + testsuites.Iops: "4000", + testsuites.Throughput: "150", }, ShouldResizeVolume: true, ShouldTestInvalidModificationRecovery: false, diff --git a/tests/e2e/testsuites/e2e_utils.go b/tests/e2e/testsuites/e2e_utils.go index e69d7d4b1d..31129eb624 100644 --- a/tests/e2e/testsuites/e2e_utils.go +++ b/tests/e2e/testsuites/e2e_utils.go @@ -39,11 +39,11 @@ const ( DefaultResizeTimout = 1 * time.Minute DefaultK8sApiPollingInterval = 5 * time.Second - AnnotationIops = "ebs.csi.aws.com/iops" - AnnotationThroughput = "ebs.csi.aws.com/throughput" - AnnotationVolumeType = "ebs.csi.aws.com/volumeType" - AnnotationsTagSpec = "ebs.csi.aws.com/tagSpecification" - AnnotationTagDel = "ebs.csi.aws.com/tagDeletion" + Iops = "iops" + Throughput = "throughput" + VolumeType = "type" + TagSpec = "tagSpecification" + TagDel = "tagDeletion" ) var DefaultGeneratedVolumeMount = VolumeMountDetails{ @@ -173,3 +173,11 @@ func CreateVolumeDetails(createVolumeParameters map[string]string, volumeSize st return &volume } + +func PrefixAnnotations(prefix string, parameters map[string]string) map[string]string { + result := make(map[string]string) + for key, value := range parameters { + result[prefix+key] = value + } + return result +} diff --git a/tests/e2e/testsuites/modify_volume_tester.go b/tests/e2e/testsuites/modify_volume_tester.go index d8d4015a5f..751570445a 100644 --- a/tests/e2e/testsuites/modify_volume_tester.go +++ b/tests/e2e/testsuites/modify_volume_tester.go @@ -31,7 +31,7 @@ import ( // ModifyVolumeTest will provision pod with attached volume, and test that modifying its pvc will modify the associated pv. type ModifyVolumeTest struct { CreateVolumeParameters map[string]string - ModifyVolumeAnnotations map[string]string + ModifyVolumeParameters map[string]string ShouldResizeVolume bool ShouldTestInvalidModificationRecovery bool ExternalResizerOnly bool @@ -39,7 +39,7 @@ type ModifyVolumeTest struct { var ( invalidAnnotations = map[string]string{ - AnnotationIops: "1", + Iops: "1", } volumeSize = "10Gi" // Different from driver.MinimumSizeForVolumeType to simplify iops, throughput, volumeType modification ) @@ -57,6 +57,8 @@ func (modifyVolumeTest *ModifyVolumeTest) Run(c clientset.Interface, ns *v1.Name testVolume, _ := volumeDetails.SetupDynamicPersistentVolumeClaim(c, ns, ebsDriver) defer testVolume.Cleanup() + parametersWithPrefix := PrefixAnnotations("ebs.csi.aws.com/", modifyVolumeTest.ModifyVolumeParameters) + By("deploying pod continuously writing to volume") formatOptionMountPod := createPodWithVolume(c, ns, PodCmdContinuousWrite(DefaultMountPath), testVolume, volumeDetails) defer formatOptionMountPod.Cleanup() @@ -70,7 +72,7 @@ func (modifyVolumeTest *ModifyVolumeTest) Run(c clientset.Interface, ns *v1.Name By("modifying the pvc") modifyingPvc, _ := c.CoreV1().PersistentVolumeClaims(ns.Name).Get(context.TODO(), testVolume.persistentVolumeClaim.Name, metav1.GetOptions{}) if testType == VolumeModifierForK8s { - AnnotatePvc(modifyingPvc, modifyVolumeTest.ModifyVolumeAnnotations) + AnnotatePvc(modifyingPvc, parametersWithPrefix) } else if testType == ExternalResizer { vac, err := c.StorageV1alpha1().VolumeAttributesClasses().Create(context.Background(), &v1alpha1.VolumeAttributesClass{ ObjectMeta: metav1.ObjectMeta{ @@ -78,7 +80,7 @@ func (modifyVolumeTest *ModifyVolumeTest) Run(c clientset.Interface, ns *v1.Name Namespace: ns.Name, }, DriverName: "ebs.csi.aws.com", - Parameters: modifyVolumeTest.ModifyVolumeAnnotations, + Parameters: modifyVolumeTest.ModifyVolumeParameters, }, metav1.CreateOptions{}) framework.ExpectNoError(err) @@ -100,7 +102,7 @@ func (modifyVolumeTest *ModifyVolumeTest) Run(c clientset.Interface, ns *v1.Name By("wait for and confirm pv modification") if testType == VolumeModifierForK8s { - err = WaitForPvToModify(c, ns, testVolume.persistentVolume.Name, modifyVolumeTest.ModifyVolumeAnnotations, DefaultModificationTimeout, DefaultK8sApiPollingInterval) + err = WaitForPvToModify(c, ns, testVolume.persistentVolume.Name, parametersWithPrefix, DefaultModificationTimeout, DefaultK8sApiPollingInterval) } else if testType == ExternalResizer { err = WaitForVacToApplyToPv(c, ns, testVolume.persistentVolume.Name, *modifyingPvc.Spec.VolumeAttributesClassName, DefaultModificationTimeout, DefaultK8sApiPollingInterval) }