-
Notifications
You must be signed in to change notification settings - Fork 2
[ELI-688] - Adding Kinesis Data Stream as a buffer for Firehose #609
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
base: main
Are you sure you want to change the base?
Changes from all commits
e7dd4da
4eae536
49993b8
8fbec9e
1f23a9e
df722eb
3380d9c
fa0395c
4b7d1b1
b151610
5fa4468
0fc6e3e
959d744
c1211b0
cf1a135
89dd588
c2c46f8
c07dd85
28592fe
70b9494
c26a144
c14931e
c4670bf
dc1d560
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,11 @@ resource "aws_kinesis_firehose_delivery_stream" "eligibility_audit_firehose_deli | |
| name = "${terraform.workspace == "default" ? "" : "${terraform.workspace}-"}${var.project_name}-${var.environment}-${var.audit_firehose_delivery_stream_name}" | ||
| destination = "extended_s3" | ||
|
|
||
| kinesis_source_configuration { | ||
| kinesis_stream_arn = var.kinesis_source_stream_arn | ||
| role_arn = var.audit_firehose_role.arn | ||
| } | ||
|
|
||
| extended_s3_configuration { | ||
| role_arn = var.audit_firehose_role.arn | ||
| bucket_arn = var.s3_audit_bucket_arn | ||
|
|
@@ -14,19 +19,14 @@ resource "aws_kinesis_firehose_delivery_stream" "eligibility_audit_firehose_deli | |
|
|
||
| cloudwatch_logging_options { | ||
| enabled = true | ||
| log_group_name = var.kinesis_cloud_watch_log_group_name | ||
| log_stream_name = var.kinesis_cloud_watch_log_stream | ||
| log_group_name = var.firehose_cloud_watch_log_group_name | ||
| log_stream_name = var.firehose_cloud_watch_log_stream | ||
| } | ||
| } | ||
|
|
||
| server_side_encryption { | ||
| enabled = true | ||
| key_arn = aws_kms_key.firehose_cmk.arn | ||
| key_type = "CUSTOMER_MANAGED_CMK" | ||
| } | ||
|
|
||
| depends_on = [ | ||
| aws_kms_key.firehose_cmk, | ||
| var.kinesis_source_stream_arn, | ||
| var.audit_firehose_role | ||
| ] | ||
|
|
||
|
Comment on lines
28
to
32
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dont think this matters as my plan came back green AND this is not something new I'm doing its already done with the audit_firehose_role
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess my question is, does removing it cause Terraform to try do things out of order?
TOEL2 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ resource "aws_lambda_function" "eligibility_signposting_lambda" { | |
| PERSON_TABLE_NAME = var.eligibility_status_table_name, | ||
| RULES_BUCKET_NAME = var.eligibility_rules_bucket_name, | ||
| CONSUMER_MAPPING_BUCKET_NAME = var.eligibility_consumer_mappings_bucket_name, | ||
| KINESIS_AUDIT_STREAM_TO_S3 = var.kinesis_audit_stream_to_s3_name | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not your issue, but inconsistent use of commas in this block, so would be good to add them for the rest |
||
| KINESIS_AUDIT_STREAM = var.kinesis_audit_stream_name | ||
| ENV = var.environment | ||
| LOG_LEVEL = var.log_level | ||
| ENABLE_XRAY_PATCHING = var.enable_xray_patching | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| resource "aws_kms_key" "kinesis_data_stream_kms_key" { | ||
| description = "${terraform.workspace == "default" ? "" : "${terraform.workspace}-"} kinesis_data_stream_kms Master Key" | ||
| deletion_window_in_days = 14 | ||
| is_enabled = true | ||
| enable_key_rotation = true | ||
| } | ||
|
|
||
| resource "aws_kms_alias" "kinesis_data_stream_kms_key" { | ||
| name = "alias/${var.project_name}-${var.environment}-kinesis-audit-stream" | ||
| target_key_id = aws_kms_key.kinesis_data_stream_kms_key.key_id | ||
| } | ||
|
|
||
|
|
||
| data "aws_iam_policy_document" "kinesis_stream_kms_key_policy" { | ||
| #checkov:skip=CKV_AWS_111 Root user needs full KMS key management | ||
| #checkov:skip=CKV_AWS_109 Root user needs full KMS key management | ||
| #checkov:skip=CKV_AWS_356 Root user needs full KMS key management | ||
| statement { | ||
| sid = "EnableRootPermissions" | ||
| effect = "Allow" | ||
|
|
||
| principals { | ||
| type = "AWS" | ||
| identifiers = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"] | ||
| } | ||
|
|
||
| actions = ["kms:*"] | ||
| resources = ["*"] | ||
| } | ||
|
|
||
| statement { | ||
| sid = "AllowLambdaUseOfKey" | ||
| effect = "Allow" | ||
|
|
||
| principals { | ||
| type = "AWS" | ||
| identifiers = [aws_iam_role.eligibility_lambda_role.arn] | ||
| } | ||
|
|
||
| actions = [ | ||
| "kms:Encrypt", | ||
| "kms:Decrypt", | ||
| "kms:ReEncrypt*", | ||
| "kms:GenerateDataKey*", | ||
| "kms:DescribeKey" | ||
| ] | ||
|
|
||
| resources = ["*"] | ||
|
|
||
| condition { | ||
| test = "StringEquals" | ||
| variable = "kms:ViaService" | ||
| values = ["kinesis.${var.default_aws_region}.amazonaws.com"] | ||
| } | ||
| } | ||
|
|
||
| statement { | ||
| sid = "AllowFirehoseRoleUseOfKey" | ||
| effect = "Allow" | ||
|
|
||
| principals { | ||
| type = "AWS" | ||
| identifiers = [aws_iam_role.eligibility_audit_firehose_role.arn] | ||
| } | ||
|
|
||
| actions = [ | ||
| "kms:Decrypt", | ||
| "kms:GenerateDataKey*", | ||
| "kms:DescribeKey" | ||
| ] | ||
|
|
||
| resources = ["*"] | ||
|
|
||
| condition { | ||
| test = "StringEquals" | ||
| variable = "kms:ViaService" | ||
| values = ["firehose.eu-west-2.amazonaws.com"] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. values = ["firehose.${var.default_aws_region}.amazonaws.com"] ? |
||
| } | ||
| } | ||
| } | ||
|
||
|
|
||
| resource "aws_kms_key_policy" "kinesis_stream_kms_key_policy" { | ||
| key_id = aws_kms_key.kinesis_data_stream_kms_key.id | ||
| policy = data.aws_iam_policy_document.kinesis_stream_kms_key_policy.json | ||
| } | ||
|
|
||
| resource "aws_kinesis_stream" "kinesis_source_stream" { | ||
| name = "${var.project_name}-${var.environment}-kinesis-audit-stream" | ||
| retention_period = 24 | ||
|
|
||
| stream_mode_details { | ||
| stream_mode = "ON_DEMAND" # can discuss later | ||
| } | ||
|
|
||
| encryption_type = "KMS" | ||
| kms_key_id = aws_kms_key.kinesis_data_stream_kms_key.arn | ||
| } | ||
TOEL2 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,8 @@ module "eligibility_audit_firehose_delivery_stream" { | |
| stack_name = local.stack_name | ||
| workspace = local.workspace | ||
| tags = local.tags | ||
| kinesis_cloud_watch_log_group_name = aws_cloudwatch_log_group.firehose_audit.name | ||
| kinesis_cloud_watch_log_stream = aws_cloudwatch_log_stream.firehose_audit_stream.name | ||
| firehose_cloud_watch_log_group_name = aws_cloudwatch_log_group.firehose_audit.name | ||
| firehose_cloud_watch_log_stream = aws_cloudwatch_log_stream.firehose_audit_stream.name | ||
| eligibility_lambda_role_arn = aws_iam_role.eligibility_lambda_role.arn | ||
| kinesis_source_stream_arn = aws_kinesis_stream.kinesis_source_stream.arn | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably want alignment of ='s |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,6 +86,22 @@ data "aws_iam_policy_document" "permissions_boundary" { | |
| "firehose:StartDeliveryStreamEncryption", | ||
| "firehose:StopDeliveryStreamEncryption", | ||
|
|
||
| # Kinesis Stream - audit log streaming | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we might sensibly leave this as kinesis:* if we run into policy length issues |
||
| "kinesis:CreateStream", | ||
| "kinesis:DeleteStream", | ||
| "kinesis:DescribeStream", | ||
| "kinesis:ListStreams", | ||
| "kinesis:PutRecord", | ||
| "kinesis:PutRecords", | ||
| "kinesis:TagStream", | ||
| "kinesis:ListTagsForStream", | ||
| "kinesis:UntagStream", | ||
| "kinesis:GetShardIterator", | ||
| "kinesis:GetRecords", | ||
| "kinesis:ListShards", | ||
| "kinesis:SubscribeToShard", | ||
| "kinesis:DescribeStreamSummary", | ||
|
|
||
| # IAM - specific role and policy management | ||
| "iam:GetRole*", | ||
| "iam:GetPolicy*", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming this was removed intentionally e.g. kinesis data stream sources are incompatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If so, surprised Checkov didn't spot it. Might be worth a comment to call out we've removed it / it's not present for that reason, in this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the tf plan failed and flagged it