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

Removing external dependencies #14

Merged
merged 3 commits into from
Apr 14, 2020

Conversation

thisismana
Copy link
Member

Added new optional variables to the module:

  • code_build_role
  • code_pipeline_role
  • code_pipeline_artifact_bucket

If the user supplies those variables/resources those are used instead of creating these resources for each service.

module "service" {
  source = "git@github.com:stroeer/terraform-aws-ecs-fargate.git"
  ...
  create_deployment_pipeline = true
  artifact_bucket_name       = "codepipeline-bucket-${data.aws_caller_identity.current.account_id}-${data.aws_region.current.name}"
  code_build_role            = "codebuild_role"
  code_pipeline_role         = "codepipeline_role"

fixes #9
fixes #11

Matthias Naber added 2 commits April 8, 2020 18:46
Added new optional variables to the module:

- `code_build_role`
- `code_pipeline_role`
- `code_pipeline_artifact_bucket`

If the user supplies those variables/resources those are used instead of creating these resources for each service.

```terraform
module "service" {
  source = "git@github.com:stroeer/terraform-aws-ecs-fargate.git"
  ...
  create_deployment_pipeline = true
  artifact_bucket_name       = "codepipeline-bucket-${data.aws_caller_identity.current.account_id}-${data.aws_region.current.name}"
  code_build_role            = "codebuild_role"
  code_pipeline_role         = "codepipeline_role"
```

fixes stroeer#9
fixes stroeer#11
variables.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
main.tf Outdated
@@ -107,6 +107,9 @@ module "code_deploy" {
cluster_name = var.cluster_id
ecr_repository_name = module.ecr.name
service_name = var.service_name
code_build_role = var.code_build_role
Copy link
Member

Choose a reason for hiding this comment

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

sort lexicographically

"iam:PassRole"
]
resources = ["*"]
# resources = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:role/ecs/task-role/*"]
Copy link
Member

Choose a reason for hiding this comment

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

commented code...., just remove or can we target the role path specifically?

Copy link
Member Author

Choose a reason for hiding this comment

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

that is tailored to the task-def-role we are using (it is an external dependency that is pulled in by name and cannot be overriden)

modules/deployment/data.tf Outdated Show resolved Hide resolved
modules/deployment/data.tf Outdated Show resolved Hide resolved
@@ -0,0 +1,19 @@
module "s3_bucket" {
source = "terraform-aws-modules/s3-bucket/aws"
Copy link
Member

Choose a reason for hiding this comment

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

sort configuration lexicographically :)

@@ -0,0 +1,19 @@
module "s3_bucket" {
Copy link
Member

Choose a reason for hiding this comment

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

the acl is private by default, but are there, from a security perspective, any other values we should set (e.g block_public_acls and block_public_policy?

Copy link
Member Author

Choose a reason for hiding this comment

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

private means that only the owner can access those files. with the block_* you can additionally block put requests that would violate this acl.

but since this is only a "technical bucket for artifact storage", no user should ever actually interfere with its contents.

@moritzzimmer moritzzimmer merged commit c23675d into stroeer:master Apr 14, 2020
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.

deployment module relies on existing S3 bucket deployment module relies on existing IAM roles
2 participants