Add encryption to EBS dynamic provisioner #30880
Conversation
k8s-github-robot
added
kind/design
markturansky
force-pushed
the
add_encryption
branch
from
f127355 to
b9eeaf1
Compare
k8s-github-robot
added
the
needs-rebase
markturansky
force-pushed
the
add_encryption
branch
from
b9eeaf1 to
326e4c4
Compare
k8s-github-robot
added
size/S
markturansky
force-pushed
the
add_encryption
branch
from
326e4c4 to
9a2645a
Compare
jsafrane
added
sig/storage
| @@ -64,6 +64,8 @@ parameters: | |||
| * `type`: `io1`, `gp2`, `sc1`, `st1`. See AWS docs for details. Default: `gp2`. | |||
| * `zone`: AWS zone. If not specified, a random zone in the same region as controller-manager will be chosen. | |||
| * `iopsPerGB`: only for `io1` volumes. I/O operations per second per GiB. AWS volume plugin multiplies this with size of requested volume to compute IOPS of the volume and caps it at 20 000 IOPS (maximum supported by AWS, see AWS docs). | |||
| * `encrypted`: denotes whether the EBS volume should be encrypted or not. Valid values are `true` or `false`. | |||
| * `kmsKeyId`: optional. The full Amazon Resource Name of the key to use when encrypting the volume. If none is supplied but `encrypted` is true, a key is generated by AWS. See AWS docs for valid ARN value. | |||
There was a problem hiding this comment.
since you need to fix this PR due to compliation error, can you also add an example of ARN: arn:aws:kms:us-east-1:012345678910:key/abcd1234-a123-456a-a12b-a123b4cd56ef as below? It would help users in searching for the right format.
|
@k8s-bot test this issue: #IGNORE |
|
weird, now it compiles without any code change |
|
LGTM to catch 1.4 |
jsafrane
added
the
lgtm
|
@jsafrane thanks! I had fixed the compilation error and pushed anew, but the build wasn't triggered until you did it. |
| @@ -1531,6 +1535,12 @@ func (c *Cloud) CreateDisk(volumeOptions *VolumeOptions) (string, error) { | |||
| volSize := int64(volumeOptions.CapacityGB) | |||
| request.Size = &volSize | |||
| request.VolumeType = &createType | |||
| request.Encrypted = &volumeOptions.Encrypted | |||
| request.KmsKeyId = &volumeOptions.KmsKeyId | |||
There was a problem hiding this comment.
I tend to prefer the aws.String(s) help function now that it is available (and Bool, Int64 etc friends). But just my personal preference.
Have we tested this without a KmsKeyId? Not all AWS APIs like being passed "" (which is why we have to do the whole crazy pointer dance in the first place!)
|
LGTM also (with a few style nits that do not need to be fixed at this stage) I presume e2e will pick up if there is a problem if KmsKeyId is not specified - we'll certainly find it in testing! |
|
GCE e2e build/test passed for commit 9a2645a. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit 9a2645a. |
|
Automatic merge from submit-queue |
Resolves #30792
Adds encryption to the EBS cloud provider and provisioner.
Follow up to #29006 (all commits but the one in this PR will drop out).
@kubernetes/sig-storage
This change is