ministryofjustice / modernisation-platform

A place for the core work of the Modernisation Platform • This repository is defined and managed in Terraform
https://user-guide.modernisation-platform.service.justice.gov.uk
MIT License
680 stars 290 forks source link

Refactor use of `app-ecr-repo` module #5367

Open dms1981 opened 10 months ago

dms1981 commented 10 months ago

User Story

As a Modernisation Platform Engineer I want to refactor how the app-ecr-repo is supplied with values So that my code is less verbose

Value / Purpose

At present we add ECR repositories in core-shared-services through repeating module calls.

You can see our documentation here.

We could refactor this code to use a single module call that we supply with a map of values through a for_each expression. If done correctly, this could be done non-destructively and reduce the quantity of code in the file.

EG:

locals {
  ecr_repositories = {
    example-app = {
      push_principals = [ "arn:aws:iam::${local.environment_management.account_ids["example-development"]}:role/modernisation-platform-oidc-cicd" ]
      pull_principals = [ local.environment_management.account_ids["data-platform-development" ]
      permitted_lambdas = [ "arn:aws:lambda:eu-west-2:${local.environment_management.account_ids["dexample-development"]}:function:example_lambda" ]
    }
    customer-app-1 = { ... }
    customer-app-2 = { ... }
  }
}     

module "data_platform_update_metadata_ecr_repo" {
  source = "../../modules/app-ecr-repo"
  for_each = local.ecr_repositories
  app_name = each.key
  push_principals = each.value.push_principals
  pull_principals = each.value.pull_principals
  enable_retrieval_policy_for_lambdas = each.value.permitted_lambdas
  tags_common = merge(
    local.tags,
    { "Name" = each.key }
  )
}

This work would need to be handled cautiously, as there's a high risk of destroying customer ECR repositories, and probably won't result in significant savings in code quantity, but would make for cleaner code and be a good learning experience.

Useful Contacts

No response

Additional Information

No response

Proposal / Unknowns

No response

Definition of Done

SimonPPledger commented 10 months ago

Not clear what the benefit of this is

mayur-dev1 commented 10 months ago

@SimonPPledger Isn't the bigger explanation given above about the cleaner code and reduction in the verbosity self explanatory?

ewastempel commented 5 months ago

Thanks for your PR proposal @mayur-dev1. However, this ticket still needs refining.