azurerm_linux_function_app - fix linuxFxVersion for docker runtime

[xiaxyi]

fix #16483
fix #18236

1.The app cannot be started if linuxFxVersion has "https://" or "http://" included like DOCKER|https://registry.hub.docker.com/test/dotnet/samples:aspnetapp
2. we need to validate the format of the registry url to align with official guidance:https://docs.microsoft.com/en-us/troubleshoot/azure/app-service/faqs-app-service-linux#what-is-the-format-for-the-private-registry-server-url-

[jackofallops]

Hi @xiaxyi - Thanks for this PR.
I think the approach used here is the wrong way around, and that the docs you linked are in fact misleading. (The private registry URL setting linked does not, afaik, have anything to do with the fx string).
Since we cannot retrieve the URL scheme back from the service in any way, we should in fact be informing users not to supply it in the registry_url property. Adding validation to prevent it would also be advisable. The value having a missing scheme will lead to a permanent diff for the configuration.

Thanks again.

[jackofallops]

The prefix filtering introduced below makes this redundant? Also, since this value when used should not have the http[s] prefix, we shouldn't be enforcing its presence here. Also there's no way to infer the correct prefix on read, since the data isn't contained in the response, so we'd end up with a diff here.

[xiaxyi]

Thanks @jackofallops for the comments. I will remove the http(s) validation.

[xiaxyi]

Thanks @jackofallops for the review, I removed the http(s) validation. feel free to let me know if the code LGTY. Thanks!

[jackofallops]

Hi @xiaxyi
Thanks for the changes so far. Apologies if I was unclear. We should have validation ensuring the users don't include the HTTP/HTTPS schemes as we cannot read it back from the service so cannot ensure we don't have a diff from the user's config. The docs should also reflect that the scheme should be omitted since this will prevent the container being pulled?

Also - I don't believe this can be confirmed to resolve issue 18236 at this time, the conversation there is inconclusive?

[xiaxyi]

Hi @xiaxyi Thanks for the changes so far. Apologies if I was unclear. We should have validation ensuring the users don't include the HTTP/HTTPS schemes as we cannot read it back from the service so cannot ensure we don't have a diff from the user's config. The docs should also reflect that the scheme should be omitted since this will prevent the container being pulled?

Also - I don't believe this can be confirmed to resolve issue 18236 at this time, the conversation there is inconclusive?

Thanks for the feedbacks, I think we should leave the http(s) options to the users to align with the api, as you can tell, I trimmed the prefix when setting the property back.

[jackofallops]

Thanks for the feedbacks, I think we should leave the http(s) options to the users to align with the api, as you can tell, I trimmed the prefix when setting the property back.

Unfortunately the problem here is that the user config may have the scheme prefix in it, but the response won't so we cannot know to put the prefix in or not. Performing a remove of the prefix here is redundant, given we've already trimmed it from the request data in the Create it's guaranteed not to be there on the Read. Instead we should reconcile the data read with what Terraform is expecting in the config, however, since that data is lost this is not possible. The only way to ensure this is deterministic is to prevent the use at all.

[xiaxyi]

Thanks for the comment @jackofallops ~

The reason why I make the http(s) prefix allowed is due to the key DOCKER_REGISTRY_SERVER_URL in app_setting block. As you can tell, it's referring to the registry_url in docker property in application_stack. Also, the official documentation does mentioned that the user needs to include the full URL including the http(s) prefix, otherwise, we are not sure if there will be some issues to pull the image from the private endpoints such as azure container registry.

The other reason is as what the user mentioned, we are allowing the prefix in http(s) in linux web app as well.

It's not harm to left the prefix to user, as we are not including it in linuxFxVersion (it gets trimmed), just leaving it to the registry_url property.

[jackofallops]

Hi @xiaxyi - Apologies for the delay in circling back here.
I've pushed a couple changes to your branch to speed things along, I think this will be good to merge now after we get the tests run.

Thanks again!

[jackofallops]

Tests are looking good:

[github-actions]

This functionality has been released in v3.42.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

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.