equinor / terraform-baseline

Best practices for creating reusable Terraform modules using the Azure provider
https://equinor.github.io/terraform-baseline/
MIT License
10 stars 5 forks source link

Do we keep the Container App module or archive it? #131

Open kov117 opened 7 months ago

kov117 commented 7 months ago

It was previously discussed outside of Github, that this repo only makes one resource and our standard is that a repo should make more than one resource., so by default the option was to simply delete or archive. However since we do not have any recording of that discussion, then it makes sense to have an issue where we can either further discuss it or confirm wether or not to we delete/archieve this repo.

kov117 commented 7 months ago

@equinor/terraform-baseline @BouvetMorten any thoughts or comments regarding this.

BouvetMorten commented 7 months ago

Can you explain what the reasoning is for the rule of one resource? I would argue that it gives value to have a module with some examples of how to set up the resource. There's a lot of attributes that you can set, and a multiple ways to set it. You can set default values according to best practices even if it's only one resource. And terraform documentation can be a bit lacking.

But I am a bit conflicted in that maybe what I'm asking for are some examples. And maybe there are better ways of doing that than having a module?

kov117 commented 7 months ago

i see what you are saying and i do agree, what did you have in mind regarding better ways of having examples ? as to why the rule, i can honestly say i cannot remember or find information as to why it is again we have that rule 😅 maybe someone from the team @equinor/terraform-baseline could help and remind me again why that is ?

hknutsen commented 7 months ago

If the module simplifies the creating of the resource drastically, then it can make sense.

For example, a Key Vault module that only creates a Key Vault module would only make sense if it configures the Key Vault in a way that is drastically different from the defaults, for example disable public access by default, enable RBAC instead of access policies etc.

If the module is a simple wrapper around the resource that adds no real value, then using the resource directly instead is easier.

Vaguely inspired by this documentation.

BouvetMorten commented 7 months ago

Right now the module does not do anything drastically different from the defaults. Only value it brings at the moment is that it's an example of how to dynamically create multiple containers in the container app.

kov117 commented 6 months ago

Is it safe to assume then that we agrre to just archive the container app repo ? @equinor/terraform-baseline

hknutsen commented 5 months ago

Once this issue has been closed, we could repurpose this module to create a Container App environment and a diagnostic setting: https://github.com/hashicorp/terraform-provider-azurerm/issues/25426

Alternatively, use the AzAPI provider to create the Container App environment instead.

kov117 commented 5 months ago

i will go ahead and archive the container app module as well as close this issue.

hknutsen commented 5 months ago

I see how my last comment was unclear 😅

I meant that the possibility of repurposing this module to create a Container App environment and diagnostic setting instead of a simple Container App is a reason to keep this issue open and continue discussing.

kov117 commented 5 months ago

Ah i get it 😆 then i will reopen this issue and the container app aswell

github-actions[bot] commented 2 months ago

There has been no activity on this issue for 60 days. stale label will be added. If no additional activity occurs, the issue will be closed in 14 days.

helenakallekleiv commented 1 month ago

This issue is still under discussion.

helenakallekleiv commented 2 weeks ago

@equinor/terraform-baseline-maintainers Investigate if we could use container app environment and diagn. setting as main module, and repurpose the container app resource as a submodule.