Skip to content

feat: Add KMS policy to Velero IAM policy for CMK KMS keys #578

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions examples/iam-role-for-service-accounts-eks/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -337,9 +337,10 @@ module "node_termination_handler_irsa_role" {
module "velero_irsa_role" {
source = "../../modules/iam-role-for-service-accounts-eks"

role_name = "velero"
attach_velero_policy = true
velero_s3_bucket_arns = ["arn:aws:s3:::velero-backups"]
role_name = "velero"
attach_velero_policy = true
velero_s3_bucket_arns = ["arn:aws:s3:::velero-backups"]
velero_s3_kms_key_arns = ["arn:aws:kms:eu-west-1:123456789012:key/abcd1234-12ab-34cd-56ef-1234567890ab"]

oidc_providers = {
ex = {
Expand Down
1 change: 1 addition & 0 deletions modules/iam-role-for-service-accounts-eks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ No modules.
| <a name="input_role_policy_arns"></a> [role\_policy\_arns](#input\_role\_policy\_arns) | ARNs of any policies to attach to the IAM role | `map(string)` | `{}` | no |
| <a name="input_tags"></a> [tags](#input\_tags) | A map of tags to add the the IAM role | `map(any)` | `{}` | no |
| <a name="input_velero_s3_bucket_arns"></a> [velero\_s3\_bucket\_arns](#input\_velero\_s3\_bucket\_arns) | List of S3 Bucket ARNs that Velero needs access to in order to backup and restore cluster resources | `list(string)` | <pre>[<br/> "*"<br/>]</pre> | no |
| <a name="input_velero_s3_kms_key_arns"></a> [velero\_s3\_kms\_key\_arns](#input\_velero\_s3\_kms\_key\_arns) | List of KMS Key ARNs that Velero needs access to in order to encrypt backups | `list(string)` | `[]` | no |
| <a name="input_vpc_cni_enable_cloudwatch_logs"></a> [vpc\_cni\_enable\_cloudwatch\_logs](#input\_vpc\_cni\_enable\_cloudwatch\_logs) | Determines whether to enable VPC CNI permission to create CloudWatch Log groups and publish network policy events | `bool` | `false` | no |
| <a name="input_vpc_cni_enable_ipv4"></a> [vpc\_cni\_enable\_ipv4](#input\_vpc\_cni\_enable\_ipv4) | Determines whether to enable IPv4 permissions for VPC CNI policy | `bool` | `false` | no |
| <a name="input_vpc_cni_enable_ipv6"></a> [vpc\_cni\_enable\_ipv6](#input\_vpc\_cni\_enable\_ipv6) | Determines whether to enable IPv6 permissions for VPC CNI policy | `bool` | `false` | no |
Expand Down
16 changes: 16 additions & 0 deletions modules/iam-role-for-service-accounts-eks/policies.tf
Original file line number Diff line number Diff line change
Expand Up @@ -1565,6 +1565,22 @@ data "aws_iam_policy_document" "velero" {
]
resources = var.velero_s3_bucket_arns
}

dynamic "statement" {
for_each = length(var.velero_s3_kms_key_arns) > 0 ? [1] : []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will work if you pass in computed values since those are unknown

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've updated to code that this should be computed values safe.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, toset() will not work either

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again, removed the toset, the idea was there to "ignore" duplicates in the case there were any in the arns.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this scenario, we can accept a list(string) of KMS key ARNs, but we should have a 2nd variable like velero_enable_kms that is a bool type to toggle on/off since we can't rely on length() or toset() due to computed values not being known


content {
sid = "KMSReadWrite"
actions = [
"kms:Encrypt",
"kms:Decrypt",
"kms:ReEncrypt*",
"kms:GenerateDataKey*",
"kms:DescribeKey"
]
resources = var.velero_s3_kms_key_arns
}
}
}

resource "aws_iam_policy" "velero" {
Expand Down
6 changes: 6 additions & 0 deletions modules/iam-role-for-service-accounts-eks/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ variable "velero_s3_bucket_arns" {
default = ["*"]
}

variable "velero_s3_kms_key_arns" {
description = "List of KMS Key ARNs that Velero needs access to in order to encrypt backups"
type = list(string)
default = []
}

# VPC CNI
variable "attach_vpc_cni_policy" {
description = "Determines whether to attach the VPC CNI IAM policy to the role"
Expand Down
1 change: 1 addition & 0 deletions wrappers/iam-role-for-service-accounts-eks/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ module "wrapper" {
role_policy_arns = try(each.value.role_policy_arns, var.defaults.role_policy_arns, {})
tags = try(each.value.tags, var.defaults.tags, {})
velero_s3_bucket_arns = try(each.value.velero_s3_bucket_arns, var.defaults.velero_s3_bucket_arns, ["*"])
velero_s3_kms_key_arns = try(each.value.velero_s3_kms_key_arns, var.defaults.velero_s3_kms_key_arns, [])
vpc_cni_enable_cloudwatch_logs = try(each.value.vpc_cni_enable_cloudwatch_logs, var.defaults.vpc_cni_enable_cloudwatch_logs, false)
vpc_cni_enable_ipv4 = try(each.value.vpc_cni_enable_ipv4, var.defaults.vpc_cni_enable_ipv4, false)
vpc_cni_enable_ipv6 = try(each.value.vpc_cni_enable_ipv6, var.defaults.vpc_cni_enable_ipv6, false)
Expand Down