-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat!: Allow custom ECR, rename var.use_ecr_image, and cleanup #51
Conversation
💰 Infracost reportMonthly estimate generatedThis comment will be updated when code changes. |
cddef81
to
d85ec1e
Compare
d85ec1e
to
ce86705
Compare
@@ -2,6 +2,7 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved some sections around to make it easier to read
@@ -57,59 +58,22 @@ For example: | |||
|
|||
# How to use it | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved the auth steps into their own file
|
||
There are several ways to set up authentication to GitHub. | ||
Follow the guide [here](/docs/GITHUB-AUTH-SETUP.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before merging I probably need to make this a URI like the others. Or it wont work from the TF registry.
@@ -13,12 +13,6 @@ locals { | |||
|
|||
subnet_arns = [for subnet_id in var.subnet_ids : "arn:aws:ec2:${local.aws_region}:${local.aws_account_id}:subnet/${subnet_id}"] | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this closer to where the action happens
local.create_iam_role | ||
? aws_iam_role.this[0].arn | ||
: "arn:aws:iam::${local.aws_account_id}:role/${var.iam_role_name}" | ||
) | ||
|
||
artifacts { | ||
type = "NO_ARTIFACTS" | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tweaked the logic a bit
I'll also add a precommit hook for codespell for completeness |
a871c12
to
8113e44
Compare
Closes #48
Breaking change, as I decided to change one of the variables a bit to support more use cases.
The plan is to test this branch at the customer before merging it in.