-
Notifications
You must be signed in to change notification settings - Fork 652
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
add-ons/azure_devops_v1: add service endpoints for Azure container registry #283
add-ons/azure_devops_v1: add service endpoints for Azure container registry #283
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this pattern from another PR: #264 would be a better solution as it will reduce the maintenance to remove additional variable every time we have a new target endpoint (GitHub, ACR etc.)
Hi! This looks good to me and actually covers off a similar use case I'm currently working through (#290). If I'm reading @iriahk89's comment correctly you just need to replace your variable |
8a3adfd
to
baae587
Compare
@iriahk89 I have updated the resource according to the given pattern |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Introduces a new component
service_endpoints_acr
to add a Docker Registry service connection to Azure DevOps from an Azure Container registry created in a previous landing zone.It uses the
azuredevops_serviceendpoint_dockerregistry
resource rather than theazuredevops_serviceendpoint_azurecr
resource, as the latter requires to be able to create a service principal for connecting to the ACR on demand, which seems not feasible to me.The component requires an existing
azure_container_registries
withadmin_enabled
.Does this introduce a breaking change
Testing