Skip to content
This repository has been archived by the owner on Jan 25, 2023. It is now read-only.

addresses https://github.com/hashicorp/terraform-aws-vault/issues/238 #239

Merged
merged 6 commits into from
Apr 8, 2021

Conversation

dchernivetsky
Copy link
Contributor

No description provided.

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 30, 2021

CLA assistant check
All committers have signed the CLA.

@dchernivetsky dchernivetsky changed the title addresses #238 addresses https://github.com/hashicorp/terraform-aws-vault/issues/238 Mar 30, 2021
@dchernivetsky
Copy link
Contributor Author

#238

Copy link
Collaborator

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

NIT: please run terraform fmt on the code.

modules/vault-elb/main.tf Outdated Show resolved Hide resolved
modules/vault-elb/variables.tf Outdated Show resolved Hide resolved
modules/vault-elb/main.tf Outdated Show resolved Hide resolved
modules/vault-elb/variables.tf Outdated Show resolved Hide resolved
@dchernivetsky
Copy link
Contributor Author

@brikis98 corrected the findings.

modules/vault-elb/main.tf Outdated Show resolved Hide resolved
modules/vault-elb/main.tf Outdated Show resolved Hide resolved
modules/vault-elb/variables.tf Outdated Show resolved Hide resolved
modules/vault-elb/variables.tf Outdated Show resolved Hide resolved
@dchernivetsky
Copy link
Contributor Author

corrected all @brikis98

Copy link
Collaborator

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! I'll kick off tests now.

@brikis98
Copy link
Collaborator

brikis98 commented Apr 8, 2021

Tests passed! Merging now.

@brikis98 brikis98 merged commit 2a67b66 into hashicorp:master Apr 8, 2021
@brikis98
Copy link
Collaborator

brikis98 commented Apr 8, 2021

@dchernivetsky
Copy link
Contributor Author

@brikis98 on second thought. I just tried to use 0.14.3 It doesnt appear that approach taken in the PR is valid. Specifically with ["once"] snippet.

Error: Invalid function argument

  on .terraform/modules/vault_cluster_elb/modules/vault-elb/main.tf line 32, in resource "aws_elb" "vault":
  32:       enabled       = lookup(access_logs.value, "enabled", lookup(access_logs.value, "bucket", null))
    |----------------
    | access_logs.value is "once"

Invalid value for "inputMap" parameter: lookup() requires a map as the first
argument.

@brikis98
Copy link
Collaborator

Ah, yea, it should be var.access_logs instead of access_logs.value.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants