Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

default kmsKeyID is not used when value is null #1267

Closed
amilanoski opened this issue Jun 8, 2022 · 6 comments
Closed

default kmsKeyID is not used when value is null #1267

amilanoski opened this issue Jun 8, 2022 · 6 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@amilanoski
Copy link

/kind bug

What happened?
default kmskeyid is not used even though CMK is set in EC2 Dashboard >EBS Encryption

What you expected to happen?

For aws-csi-driver to use the default CMK kms key when set in the aws console.

How to reproduce it (as minimally and precisely as possible)?
values.yaml

storageClasses:
# Add StorageClass resources like:
- name: ebs-sc
  # annotation metadata
  annotations:
    storageclass.kubernetes.io/is-default-class: "true"
  # label metadata
  # labels:
  #   my-label-is: supercool
  # defaults to WaitForFirstConsumer
  volumeBindingMode: WaitForFirstConsumer
  # defaults to Delete
  reclaimPolicy: Delete
  parameters:
    encrypted: "true"
  allowVolumeExpansion: true

Anything else we need to know?:
kms key is set to the aws generated kms key rather then the CMK we have created.

only work around is to set the kmskeyid to the arn we require. Either the docs need to be updated or there is a bug. I am not sure. Hoping for some guidance and answers. Thanks in advance.

Environment

  • Kubernetes version (use kubectl version):
  • Driver version: helm chart version: 2.6.3
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jun 8, 2022
@torredil
Copy link
Member

torredil commented Jun 8, 2022

@amilanoski This is actually abnormal behavior. This is not a bug and currently expected behavior. To clarify: Amazon EBS automatically creates a unique AWS managed key in each Region where you store AWS resources. This KMS key has the alias /aws/ebs. By default, Amazon EBS uses this KMS key for encryption. Alternatively, you can specify a symmetric customer managed encryption key that you created as the default KMS key for EBS encryption.

From the AWS API doc "If this parameter is not specified, your KMS key for Amazon EBS is used". I just tested this on my end and confirmed that the CSI driver indeed uses the default CMK kms key when set in the AWS console.

From previous project maintainers: #80
Screen Shot 2022-06-09 at 12 04 13 AM

As a first step, I would ensure you are creating the CMK in the right region.

@amilanoski
Copy link
Author

Thanks for confirming @torredil and updating the docs. I have tested some different paths with default EBS CMK configured in the console.

option 1 - values.yaml - encryption:true

storageClasses:
# Add StorageClass resources like:
- name: ebs-sc
  # annotation metadata
  annotations:
    storageclass.kubernetes.io/is-default-class: "true"
  # label metadata
  # labels:
  #   my-label-is: supercool
  # defaults to WaitForFirstConsumer
  volumeBindingMode: WaitForFirstConsumer
  # defaults to Delete
  reclaimPolicy: Delete
  parameters:
    encrypted: "true"
  allowVolumeExpansion: true

option 2 - values.yaml - no encryption set

storageClasses:
# Add StorageClass resources like:
- name: ebs-sc
  # annotation metadata
  annotations:
    storageclass.kubernetes.io/is-default-class: "true"
  # label metadata
  # labels:
  #   my-label-is: supercool
  # defaults to WaitForFirstConsumer
  volumeBindingMode: WaitForFirstConsumer
  # defaults to Delete
  reclaimPolicy: Delete
  # parameters:
  #   encrypted: "true"
  allowVolumeExpansion: true

option 3 - values.yaml - encryption set + kmskeyid

storageClasses:
# Add StorageClass resources like:
- name: ebs-sc
  # annotation metadata
  annotations:
    storageclass.kubernetes.io/is-default-class: "true"
  # label metadata
  # labels:
  #   my-label-is: supercool
  # defaults to WaitForFirstConsumer
  volumeBindingMode: WaitForFirstConsumer
  # defaults to Delete
  reclaimPolicy: Delete
  parameters:
    encrypted: "true"
    kmsKeyId: "arn:xxxxx"
  allowVolumeExpansion: true

Results:

  1. aws-csi-driver will encrypted the drive, but with aws managed kms key(which seems to be expected)
  2. aws-csi-driver will encrypted the drive and using the default EBS encryption key specified in the console.(the behaviour I was expecting)
  3. aws-csi-driver encrypted EBS volume with kmsKeyID as expected(expected)

Do you think docs should be updated, so that users who have a custom default EBS CMK set to not set parameters.encrypted or parameters.kmsKeyId ?

@ConnorJC3
Copy link
Contributor

@amilanoski could you provide a full test case for scenario 1? When testing, I get scenario 2's behavior (using the default EBS key) when specifying encrypted: "true". Are you sure the volume is in the region that your default key is in, because the default EBS encryption key is set per-region.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 27, 2022
@ConnorJC3
Copy link
Contributor

/close

I am still not able to reproduce the above issue. If anyone is able to reproduce the issue explained in scenario 1, please reach out with more information.

@k8s-ci-robot
Copy link
Contributor

@ConnorJC3: Closing this issue.

In response to this:

/close

I am still not able to reproduce the above issue. If anyone is able to reproduce the issue explained in scenario 1, please reach out with more information.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

5 participants