-
Notifications
You must be signed in to change notification settings - Fork 5
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
Do we keep the Container App module or archive it? #131
Comments
@equinor/terraform-baseline @BouvetMorten any thoughts or comments regarding this. |
Can you explain what the reasoning is for the rule of one resource? But I am a bit conflicted in that maybe what I'm asking for are some examples. |
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 ? |
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. |
Right now the module does not do anything drastically different from the defaults. |
Is it safe to assume then that we agrre to just archive the container app repo ? @equinor/terraform-baseline |
Once this issue has been closed, we could repurpose this module to create a Container App environment and a diagnostic setting: hashicorp/terraform-provider-azurerm#25426 Alternatively, use the AzAPI provider to create the Container App environment instead. |
i will go ahead and archive the container app module as well as close this issue. |
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. |
Ah i get it 😆 then i will reopen this issue and the container app aswell |
There has been no activity on this issue for 60 days. |
This issue is still under discussion. |
@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. |
We could make it like the Proof of Concept in the Key Vault module, where the main module uses both an Container App Environment + Container App. Then create two submodules. One for the container app environment and one for the container app. So if you only need one of each you can just create the simple main module, but if you need multiple container apps in one container app environment or multiple environments with different container apps going to different environments you can use the submodules? |
I've started on a POC for this suggestion. I'll link the PR here when its ready. |
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.
The text was updated successfully, but these errors were encountered: