Skip to content

Conversation

@JoeCSykes
Copy link
Contributor

No description provided.

@chrisbloe chrisbloe self-assigned this Aug 4, 2023
@Dusty-Meg
Copy link
Member

@JoeCSykes
Copy link
Contributor Author

Are we going to add the moved block in so that it does not break existing uses? https://developer.hashicorp.com/terraform/language/modules/develop/refactoring#moved-block-syntax https://developer.hashicorp.com/terraform/language/modules/develop/refactoring#removing-moved-blocks

I believe this is going to be in another PR. This PR just changes the resource name.

@robg-test
Copy link
Collaborator

The agreed approach was to bump up a major version to not have to deal with the moved block, as far as I'm aware.

@robg-test robg-test closed this Aug 4, 2023
@robg-test robg-test reopened this Aug 4, 2023
@chrisbloe chrisbloe force-pushed the update_vpc_resource_names branch from f8e3ae9 to 025dc07 Compare August 31, 2023 10:57
@JoeCSykes
Copy link
Contributor Author

This is my PR so I can't approve it, but I have checked and this looks good to me 👍

aws = {
source = "hashicorp/aws"
version = ">= 4.0"
version = ">= 5.14.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we document (in code, or in wiki) the reasoning/requirement behind minimum versions of providers? It will make it easier to understand cross-module provider requirements

description = "The email address of the owner."

validation {
condition = can(match("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$", var.owner))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
condition = can(match("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$", var.owner))
condition = can(regex("^.+@[^@]+\\.[^@]{2,}$", var.owner))

more permissive email validation (plus match doesn't exist)

resource "aws_cloudwatch_log_group" "vpc_flow_logs" {
count = var.enable_vpc_flow_logs ? 1 : 0

name = "${replace("AWS::Logs::LogGroup", "::", "-")}-${var.project_name}-${var.environment}-${local.aws_region_short}-vpc_flow_logs"
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason to use replace like this throughout? Also should be lower case. Surely we should just use something like log_group-...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants