-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
New Resource: azurerm_service_fabric_cluster
#4
New Resource: azurerm_service_fabric_cluster
#4
Conversation
Tests pass: ``` $ acctests azurerm TestAccAzureRMServiceFabricCluster_ === RUN TestAccAzureRMServiceFabricCluster_basic --- PASS: TestAccAzureRMServiceFabricCluster_basic (65.66s) === RUN TestAccAzureRMServiceFabricCluster_addOnFeatures --- PASS: TestAccAzureRMServiceFabricCluster_addOnFeatures (54.47s) === RUN TestAccAzureRMServiceFabricCluster_certificate --- PASS: TestAccAzureRMServiceFabricCluster_certificate (68.11s) === RUN TestAccAzureRMServiceFabricCluster_clientCertificateThumbprint --- PASS: TestAccAzureRMServiceFabricCluster_clientCertificateThumbprint (59.19s) === RUN TestAccAzureRMServiceFabricCluster_diagnosticsConfig --- PASS: TestAccAzureRMServiceFabricCluster_diagnosticsConfig (82.73s) === RUN TestAccAzureRMServiceFabricCluster_fabricSettings --- PASS: TestAccAzureRMServiceFabricCluster_fabricSettings (61.09s) === RUN TestAccAzureRMServiceFabricCluster_fabricSettingsRemove --- PASS: TestAccAzureRMServiceFabricCluster_fabricSettingsRemove (76.76s) === RUN TestAccAzureRMServiceFabricCluster_nodeTypeCustomPorts --- PASS: TestAccAzureRMServiceFabricCluster_nodeTypeCustomPorts (67.23s) === RUN TestAccAzureRMServiceFabricCluster_nodeTypesMultiple --- PASS: TestAccAzureRMServiceFabricCluster_nodeTypesMultiple (65.39s) === RUN TestAccAzureRMServiceFabricCluster_nodeTypesUpdate --- PASS: TestAccAzureRMServiceFabricCluster_nodeTypesUpdate (65.42s) === RUN TestAccAzureRMServiceFabricCluster_tags --- PASS: TestAccAzureRMServiceFabricCluster_tags (62.97s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 729.355s ```
c495150
to
9e68cd6
Compare
hi @lstolyarov Thanks for this PR :) Firstly, apologies for the delay with this PR - it's been sat at the bottom of the PR Review list for far too long (which is totally my fault!). In order to see how we could move forward with this, I spent a little time yesterday looking into what this would entail. I've amended your original commit (although your name's still on the commit) to remove the old SDK and the Whilst I appreciate this PR has been sitting here awkwardly for far too long - I hope you don't mind that I've made these changes and pushed them to your fork. This now LGTM (although I'm going to ask @katbyte to review it) - and I believe we should be able to get this shipped into the next release of the AzureRM Provider. Thanks! |
azurerm_service_fabric_cluster
Hey @tombuildsstuff, no problems at all. Nice to see this in terraform! |
Thank you!!! |
@pixelicous only the node count of a cluster can be changed via TF, due to limitations within the API - please see the docs for more info 😄 |
@tombuildsstuff Only that? Damn, that makes this very unusable because one of the major things you need to continuously change in the cluster is pushing rolled certificaes. We provide certificates to the nodes so that the applications and their processes can use it and authenticate with keyvault, today we are doing it with ARM template, isn't it working with API as well?? I read the documentation for this object on terraform docs, it seems the "properties" block is missing, this also is a problem because you cannot use the RBAC block and this forces users to use client certificates which is not a best practice, the RBAC block is created using Microsoft's AADTOOL and looks like this:
Details can be found here under "Set up Azure Active Directory for client authentication" |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
Fixes #541