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

Errors if log_prefix, certificate_arn not set #2

Closed
brandonjbjelland opened this issue Sep 27, 2017 · 6 comments
Closed

Errors if log_prefix, certificate_arn not set #2

brandonjbjelland opened this issue Sep 27, 2017 · 6 comments
Assignees

Comments

@brandonjbjelland
Copy link
Contributor

From @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
Copy link
Contributor Author

brandonjbjelland commented Sep 27, 2017

The fix here is two fold:

  1. default to HTTP rather than HTTPS, making the barrier to entry lower (no cert required).
  2. try to build locals such that if any variables necessary for when certificate_arn, log_prefix, log_bucket, or alb_protocol is HTTPS, if those variables are missing, they are automagically populated with something valid.

@brandonjbjelland
Copy link
Contributor Author

The second point above is only somewhat possible as I can't be sure what will be valid for a cert ARN in any given account. Moving to explicit logging variables helps alleviate the confusion I think.

The module is now defaulting to HTTP.

brandonjbjelland added a commit that referenced this issue Nov 17, 2017
…_in_tests

Simplifying tests (region) and defaulting to HTTP. Resolves #2
@egarbi
Copy link
Contributor

egarbi commented Dec 20, 2017

@brandoconnor regarding the second point it would be useful put somewhere in docs that even when you are building an internal alb you need to pass an empty certificate_arn variable

@brandonjbjelland
Copy link
Contributor Author

@egarbi hmm I think that even brings up a larger point. Why is certificate_arn a required variable when some people just want to use HTTP? I'll work through a better approach in the coming release. Thanks for the call out!

@brandonjbjelland
Copy link
Contributor Author

This should be resolved now.

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants