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

certificate_id an optional argument of azurerm_container_app resource #24117

Closed
wants to merge 4 commits into from

Conversation

harshavmb
Copy link
Contributor

@harshavmb harshavmb commented Dec 5, 2023

This fixes #24110

Current azurerm_container_app resource has ingress block which has custom_domain which mandates certificate_id to be a required field even when certificate_binding_type set to Disabled by default. This contradicts with the documentation here for Azure managed certificates.

This PR aims to set certificate_id as Optional & only required when certificate_binding_type set to SniEnabled.

…port of non-existing resources"

This reverts commit bed17a2.
Copy link

This PR is being labeled as "stale" because it has not been updated for 30 or more days.

If this PR is still valid, please remove the "stale" label. If this PR is blocked, please add it to the "Blocked" milestone.

If you need some help completing this PR, please leave a comment letting us know. Thank you!

@github-actions github-actions bot added the stale label Jan 15, 2024
@caiola
Copy link

caiola commented Feb 6, 2024

I waiting for this feature to be merged to fix my terraform code ( related with issue #24110 ).

@github-actions github-actions bot removed the stale label Feb 6, 2024
@jk-f5
Copy link

jk-f5 commented Feb 9, 2024

I'd also like to see this merged asap.

@jpmicrosoft
Copy link
Contributor

@tombuildsstuff Can this move forward?

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

LGTM 🌵

@katbyte
Copy link
Collaborator

katbyte commented Feb 28, 2024

noticed #24421 might replace this functionality so going to leave this till that PR is reviewed

@jackofallops
Copy link
Member

Hi @harshavmb - Thanks for this PR. This block has been deprecated due to a circular reference problem and replaced with a new resource azurerm_container_app_custom_domain. We're investigating the support of the Azure Managed Certificates in that resource at the moment, so I'm going to close this PR for now. Please follow #25356 for progress.

Thanks

Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

On azurerm_container_app adding custom domain, the field certificate_id is required and should not be
6 participants