Errors if log_prefix, certificate_arn not set

[hasbro-ielo]

The documentation says that log_prefix, certificate_arn are optional. If I don't set them I get the following:

| => tf validate && tf plan
1 error(s) occurred:

* module root: 
    module alb: required variable "certificate_arn" not set
    module alb: required variable "log_prefix" not set

Here's how I'm using the module

module "alb" {
  source = "modules/terraform-community-modules/tf_aws_alb/alb"

  //alb_is_internal = ""
  //alb_name
  //alb_protocols
  alb_security_groups = "${module.alb_sg.security_group_id_web}"
  aws_region  = "${var.region}"
  aws_account_id = "${var.account_id}"
  //backend_port
  //backend_protocol
  //certificate_arn
  //cookie_duration
  //health_check_path
  log_bucket = "${var.alb_logs_bucket}"
  //log_prefix
  //principal_account_id
  subnets = "${var.alb_subnets}"
  vpc_id = "${module.vpc.vpc_id}"
  //tags
}

[brandonjbjelland]

Hey @hasbro-ielo , thanks for the issue submission. Breaking them down:

re: log_prefix - I'll need to do some further testing to see if terraform is able to get around something like this. It might be that if log_bucket is given, log_prefix also needs to be provided, even if it's a blank variable. There are some real limitations around what can be done within an attribute (like the access_logs block). It might just be an opportunity to clarify the docs on what is actually required.

Re: certificate_arn - this is the issue: the module expects as the default behavior to use HTTPS but that requires a cert. A better out of the box experience (which is more in line with the docs and easier to do) would be defaulting to HTTP. That's a super easy fix I can submit in the next week.

[hasbro-ielo]

Thank you very much for explaining the bit about log_prefix needing to be blank.

[brandonjbjelland]

Sorry for letting this languish. I'm transitioning workplaces at the moment and will have the time to focus on this next week.

[brandonjbjelland]

I think there's a fix here that could be derived from the new locals feature. I'll migrate this issue over to the new repo and plan to look at it shortly.