Pass module.this.context to cloudposse/lb-s3-bucket/aws #86

jwstric2 · 2021-04-13T03:09:58Z

what

Context, as inherited from the user defined context, is passed and used in the access_logs

why

Bug found where the s3 bucket was not using our defined context label_order label_order = ["namespace", "environment", "name", "attributes"]. The bucket being created was still using our defined stage of prod. In our use case, if the context is passed in, label_order will be respected by the lb-s3-bucket.
This change, however, could have repercussions if the the label orders lets say does not have attributes in it, which could try and create buckets that could collide in naming (depending on user setup and resources).
We are accepting if this PR is rejected that it's possible to create your own bucket and pass it in; which is an acceptable solution

jwstric2 · 2021-04-13T10:43:17Z

/test all

Gowiem · 2021-04-13T15:08:42Z

/test all

Gowiem · 2021-04-13T15:10:57Z

main.tf

@@ -57,6 +57,7 @@ module "access_logs" {
  noncurrent_version_transition_days = var.noncurrent_version_transition_days
  standard_transition_days           = var.standard_transition_days
  force_destroy                      = var.alb_access_logs_s3_bucket_force_destroy
+  context                            = module.this.context


@jwstric2 since you added the context argument, can you remove the name, namespace, stage, environment, delimiter, and tags? context will replace all those values so they're not needed. Obviously, let's keep attributes.

will do, I forgot to ask if those should be removed when opening up this PR.

…iter, and tags that are part of the given context

jwstric2 · 2021-04-22T14:16:54Z

/test all

jwstric2 · 2021-04-22T14:44:13Z

I am unable to view the Infrastructure as Code analysis failure. If I should have access to view then I'm def willing to fix my authorization issues on my side and address the issue.

nitrocode · 2021-04-22T14:47:57Z

/test all

jwstric2 · 2021-04-22T18:24:52Z

docker run -it -v `pwd`:/tf bridgecrew/checkov -d /tf


       _               _              
   ___| |__   ___  ___| | _______   __
  / __| '_ \ / _ \/ __| |/ / _ \ \ / /
 | (__| | | |  __/ (__|   < (_) \ V / 
  \___|_| |_|\___|\___|_|\_\___/ \_/  
                                      
By bridgecrew.io | version: 2.0.66 
Update available 2.0.66 -> 2.0.67
Run pip3 install -U checkov to update 


terraform scan results:

Passed checks: 14, Failed checks: 5, Skipped checks: 0

Check: CKV_AWS_41: "Ensure no hard coded AWS access key and secret key exists in provider"
	PASSED for resource: aws.default
	File: /examples/complete/main.tf:1-3
	Guide: https://docs.bridgecrew.io/docs/bc_aws_secrets_5

Check: CKV_AWS_23: "Ensure every security groups rule has a description"
	PASSED for resource: module.alb.aws_security_group.default
	File: /main.tf:1-7
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_31

Check: CKV_AWS_24: "Ensure no security groups allow ingress from 0.0.0.0:0 to port 22"
	PASSED for resource: module.alb.aws_security_group.default
	File: /main.tf:1-7
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_1-port-security

Check: CKV_AWS_25: "Ensure no security groups allow ingress from 0.0.0.0:0 to port 3389"
	PASSED for resource: module.alb.aws_security_group.default
	File: /main.tf:1-7
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_2

Check: CKV_AWS_23: "Ensure every security groups rule has a description"
	PASSED for resource: module.alb.aws_security_group_rule.egress
	File: /main.tf:9-17
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_31

Check: CKV_AWS_23: "Ensure every security groups rule has a description"
	PASSED for resource: module.alb.aws_security_group_rule.http_ingress
	File: /main.tf:19-28
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_31

Check: CKV_AWS_24: "Ensure no security groups allow ingress from 0.0.0.0:0 to port 22"
	PASSED for resource: module.alb.aws_security_group_rule.http_ingress
	File: /main.tf:19-28
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_1-port-security

Check: CKV_AWS_25: "Ensure no security groups allow ingress from 0.0.0.0:0 to port 3389"
	PASSED for resource: module.alb.aws_security_group_rule.http_ingress
	File: /main.tf:19-28
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_2

Check: CKV_AWS_23: "Ensure every security groups rule has a description"
	PASSED for resource: module.alb.aws_security_group_rule.https_ingress
	File: /main.tf:30-39
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_31

Check: CKV_AWS_24: "Ensure no security groups allow ingress from 0.0.0.0:0 to port 22"
	PASSED for resource: module.alb.aws_security_group_rule.https_ingress
	File: /main.tf:30-39
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_1-port-security

Check: CKV_AWS_25: "Ensure no security groups allow ingress from 0.0.0.0:0 to port 3389"
	PASSED for resource: module.alb.aws_security_group_rule.https_ingress
	File: /main.tf:30-39
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_2

Check: CKV_AWS_2: "Ensure ALB protocol is HTTPS"
	PASSED for resource: module.alb.aws_lb_listener.http_redirect
	File: /main.tf:150-166
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_29

Check: CKV_AWS_103: "Ensure that load balancer is using TLS 1.2"
	PASSED for resource: module.alb.aws_lb_listener.http_redirect
	File: /main.tf:150-166
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/bc_aws_general_43

Check: CKV_AWS_2: "Ensure ALB protocol is HTTPS"
	PASSED for resource: module.alb.aws_lb_listener.https
	File: /main.tf:168-190
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_29

Check: CKV_AWS_131: "Ensure that ALB drops HTTP headers"
	FAILED for resource: module.alb.aws_lb.default
	File: /main.tf:57-81
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/ensure-that-alb-drops-http-headers

		57 | resource "aws_lb" "default" {
		58 |   count              = module.this.enabled ? 1 : 0
		59 |   name               = module.this.id
		60 |   tags               = module.this.tags
		61 |   internal           = var.internal
		62 |   load_balancer_type = "application"
		63 | 
		64 |   security_groups = compact(
		65 |     concat(var.security_group_ids, [join("", aws_security_group.default.*.id)]),
		66 |   )
		67 | 
		68 |   subnets                          = var.subnet_ids
		69 |   enable_cross_zone_load_balancing = var.cross_zone_load_balancing_enabled
		70 |   enable_http2                     = var.http2_enabled
		71 |   idle_timeout                     = var.idle_timeout
		72 |   ip_address_type                  = var.ip_address_type
		73 |   enable_deletion_protection       = var.deletion_protection_enabled
		74 |   drop_invalid_header_fields       = var.drop_invalid_header_fields
		75 | 
		76 |   access_logs {
		77 |     bucket  = try(element(compact([var.access_logs_s3_bucket_id, module.access_logs.bucket_id]), 0), "")
		78 |     prefix  = var.access_logs_prefix
		79 |     enabled = var.access_logs_enabled
		80 |   }
		81 | }


Check: CKV_AWS_91: "Ensure the ELBv2 (Application/Network) has access logging enabled"
	FAILED for resource: module.alb.aws_lb.default
	File: /main.tf:57-81
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/bc_aws_logging_22

		57 | resource "aws_lb" "default" {
		58 |   count              = module.this.enabled ? 1 : 0
		59 |   name               = module.this.id
		60 |   tags               = module.this.tags
		61 |   internal           = var.internal
		62 |   load_balancer_type = "application"
		63 | 
		64 |   security_groups = compact(
		65 |     concat(var.security_group_ids, [join("", aws_security_group.default.*.id)]),
		66 |   )
		67 | 
		68 |   subnets                          = var.subnet_ids
		69 |   enable_cross_zone_load_balancing = var.cross_zone_load_balancing_enabled
		70 |   enable_http2                     = var.http2_enabled
		71 |   idle_timeout                     = var.idle_timeout
		72 |   ip_address_type                  = var.ip_address_type
		73 |   enable_deletion_protection       = var.deletion_protection_enabled
		74 |   drop_invalid_header_fields       = var.drop_invalid_header_fields
		75 | 
		76 |   access_logs {
		77 |     bucket  = try(element(compact([var.access_logs_s3_bucket_id, module.access_logs.bucket_id]), 0), "")
		78 |     prefix  = var.access_logs_prefix
		79 |     enabled = var.access_logs_enabled
		80 |   }
		81 | }


Check: CKV_AWS_2: "Ensure ALB protocol is HTTPS"
	FAILED for resource: module.alb.aws_lb_listener.http_forward
	File: /main.tf:129-148
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/networking_29

		129 | resource "aws_lb_listener" "http_forward" {
		130 |   count             = var.http_enabled && var.http_redirect != true ? 1 : 0
		131 |   load_balancer_arn = join("", aws_lb.default.*.arn)
		132 |   port              = var.http_port
		133 |   protocol          = "HTTP"
		134 | 
		135 |   default_action {
		136 |     target_group_arn = var.listener_http_fixed_response != null ? null : join("", aws_lb_target_group.default.*.arn)
		137 |     type             = var.listener_http_fixed_response != null ? "fixed-response" : "forward"
		138 | 
		139 |     dynamic "fixed_response" {
		140 |       for_each = var.listener_http_fixed_response != null ? [var.listener_http_fixed_response] : []
		141 |       content {
		142 |         content_type = fixed_response.value["content_type"]
		143 |         message_body = fixed_response.value["message_body"]
		144 |         status_code  = fixed_response.value["status_code"]
		145 |       }
		146 |     }
		147 |   }
		148 | }


Check: CKV_AWS_103: "Ensure that load balancer is using TLS 1.2"
	FAILED for resource: module.alb.aws_lb_listener.http_forward
	File: /main.tf:129-148
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/bc_aws_general_43

		129 | resource "aws_lb_listener" "http_forward" {
		130 |   count             = var.http_enabled && var.http_redirect != true ? 1 : 0
		131 |   load_balancer_arn = join("", aws_lb.default.*.arn)
		132 |   port              = var.http_port
		133 |   protocol          = "HTTP"
		134 | 
		135 |   default_action {
		136 |     target_group_arn = var.listener_http_fixed_response != null ? null : join("", aws_lb_target_group.default.*.arn)
		137 |     type             = var.listener_http_fixed_response != null ? "fixed-response" : "forward"
		138 | 
		139 |     dynamic "fixed_response" {
		140 |       for_each = var.listener_http_fixed_response != null ? [var.listener_http_fixed_response] : []
		141 |       content {
		142 |         content_type = fixed_response.value["content_type"]
		143 |         message_body = fixed_response.value["message_body"]
		144 |         status_code  = fixed_response.value["status_code"]
		145 |       }
		146 |     }
		147 |   }
		148 | }


Check: CKV_AWS_103: "Ensure that load balancer is using TLS 1.2"
	FAILED for resource: module.alb.aws_lb_listener.https
	File: /main.tf:168-190
	Calling File: /examples/complete/main.tf:24-50
	Guide: https://docs.bridgecrew.io/docs/bc_aws_general_43

		168 | resource "aws_lb_listener" "https" {
		169 |   count             = module.this.enabled && var.https_enabled ? 1 : 0
		170 |   load_balancer_arn = join("", aws_lb.default.*.arn)
		171 | 
		172 |   port            = var.https_port
		173 |   protocol        = "HTTPS"
		174 |   ssl_policy      = var.https_ssl_policy
		175 |   certificate_arn = var.certificate_arn
		176 | 
		177 |   default_action {
		178 |     target_group_arn = var.listener_https_fixed_response != null ? null : join("", aws_lb_target_group.default.*.arn)
		179 |     type             = var.listener_https_fixed_response != null ? "fixed-response" : "forward"
		180 | 
		181 |     dynamic "fixed_response" {
		182 |       for_each = var.listener_https_fixed_response != null ? [var.listener_https_fixed_response] : []
		183 |       content {
		184 |         content_type = fixed_response.value["content_type"]
		185 |         message_body = fixed_response.value["message_body"]
		186 |         status_code  = fixed_response.value["status_code"]
		187 |       }
		188 |     }
		189 |   }
		190 | }

I'm not sure if these are new checks but don't seem to be coming from an area this PR touches.

jwstric2 · 2021-04-22T20:19:49Z

@nitrocode or @Gowiem, working with a gentleman on my team who suggested these be added as skips after talking to respective members through the slack channel, #bridgecrew:skip=BC_... which seems to be what the general consensus is after seeing further skips in other repos. I'm working on these now and will submit shortly.

jwstric2 · 2021-04-22T21:06:36Z

Some of these skips seem like they could be fixed with changes to the default vars; like the default ssl policy. Be happy if the team wants to track these in a separate issue/PR to evaluate

jamengual · 2021-04-23T17:09:29Z

@nitrocode or @Gowiem, working with a gentleman on my team who suggested these be added as skips after talking to respective members through the slack channel, #bridgecrew:skip=BC_... which seems to be what the general consensus is after seeing further skips in other repos. I'm working on these now and will submit shortly.

that is correct, depending on the case we so skip for bridcrew rules but is case by case.

jamengual · 2021-04-23T17:11:58Z

/test all

Pass module.this.context to cloudposse/lb-s3-bucket/aws

56e31c5

jwstric2 requested review from a team as code owners April 13, 2021 03:09

jwstric2 requested review from dotCipher and nitrocode and removed request for a team April 13, 2021 03:09

Gowiem reviewed Apr 13, 2021

View reviewed changes

jwstric2 and others added 2 commits April 22, 2021 09:58

Remove the existing values name, namespace, stage, environment, delim…

13fcbbb

…iter, and tags that are part of the given context

Merge branch 'master' into bugfix/context_s3_access_bucket

1b748ef

Add skips to new bridgecrew checks

c265932

Merge branch 'master' into bugfix/context_s3_access_bucket

9edb000

jamengual approved these changes Apr 23, 2021

View reviewed changes

jamengual merged commit af9ea1c into cloudposse:master Apr 23, 2021

jwstric2 mentioned this pull request Apr 23, 2021

Remove use of deprecated null_data_source cloudposse/terraform-aws-ec2-instance#94

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass module.this.context to cloudposse/lb-s3-bucket/aws #86

Pass module.this.context to cloudposse/lb-s3-bucket/aws #86

jwstric2 commented Apr 13, 2021 •

edited by Nuru

jwstric2 commented Apr 13, 2021

Gowiem commented Apr 13, 2021

Gowiem Apr 13, 2021

jwstric2 Apr 14, 2021

jwstric2 commented Apr 22, 2021

jwstric2 commented Apr 22, 2021

nitrocode commented Apr 22, 2021

jwstric2 commented Apr 22, 2021

jwstric2 commented Apr 22, 2021

jwstric2 commented Apr 22, 2021

jamengual commented Apr 23, 2021

jamengual commented Apr 23, 2021

Pass module.this.context to cloudposse/lb-s3-bucket/aws #86

Pass module.this.context to cloudposse/lb-s3-bucket/aws #86

Conversation

jwstric2 commented Apr 13, 2021 • edited by Nuru

what

why

jwstric2 commented Apr 13, 2021

Gowiem commented Apr 13, 2021

Gowiem Apr 13, 2021

Choose a reason for hiding this comment

jwstric2 Apr 14, 2021

Choose a reason for hiding this comment

jwstric2 commented Apr 22, 2021

jwstric2 commented Apr 22, 2021

nitrocode commented Apr 22, 2021

jwstric2 commented Apr 22, 2021

jwstric2 commented Apr 22, 2021

jwstric2 commented Apr 22, 2021

jamengual commented Apr 23, 2021

jamengual commented Apr 23, 2021

jwstric2 commented Apr 13, 2021 •

edited by Nuru