Possibility to override role path (Terraform)

[ldcorentin]

Hello guys!

I add a new variable role_path_prefix so we can override the path in every roles that the module creates.

Thanks for the review 🙏

[alexcasalboni]

Hi @ldcorentin 👋 Thanks for contributing to Lambda Power Tuning 🙏

I think this is a useful addition.

Because it's a full override, maybe should we rename the variable from role_path_prefix to role_path_override? Something like this:

locals {
  role_path = format("/%s/", var.role_path_override != null ? var.role_path_override : var.lambda_function_prefix)
}

resource "aws_iam_role" "executor_role" {
  ...
  path = local.role_path
}

Alternatively, we could make it an actual prefix. Something like this:

locals {
  role_path = format("/%s%s/", var.role_path_prefix != null ? "${var.role_path_prefix}/" : "", var.lambda_function_prefix)
}

resource "aws_iam_role" "executor_role" {
  ...
  path = local.role_path
}

Which one works better for you? Do you need full control over the role path or are you ok with just enforcing a prefix?

[ldcorentin]

Hey, sorry I was off.

I do need to have the control over the role path because I have restriction and some roles need to have an empty path.

This is why I did :

var.role_path_prefix != "" ? var.role_path_prefix : "/${var.lambda_function_prefix}/"

[alexcasalboni]

Thanks for the update @ldcorentin 🙏

Only a couple of minor changes required, then I'll be happy to merge 🎉

[ldcorentin]

Thanks for the review @alexcasalboni 🙏

[alexcasalboni]

some roles need to have an empty path

@ldcorentin did you test this implementation with an empty path? Or by empty you mean "/"?

[ldcorentin]

@alexcasalboni By empty, I meant "/", sorry for the confusion I introduced

[alexcasalboni]

Happy to merge this! 🎉 🙏