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.

Sign up for GitHub

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

Jump to bottom

New Resourece: azurerm_signalr_shared_private_link_resource #16187

Merged
merged 16 commits into from
Jul 29, 2022
Merged

New Resourece: azurerm_signalr_shared_private_link_resource #16187

merged 16 commits into from
Jul 29, 2022

Conversation

xiaxyi
Copy link
Contributor

@xiaxyi xiaxyi commented Mar 31, 2022

Comparing to the private link resource which supported by network, this can be considered as the outbound private link which is initiated by the corresponding service.

Acc tests:

--- PASS: TestAccSignalrSharedPrivateLinkResource_basicSites (569.88s)
--- PASS: TestAccSignalrSharedPrivateLinkResource_basic (647.43s)
--- PASS: TestAccSignalrSharedPrivateLinkResource_requiresImport (671.23s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/signalr       671.972s

catriona-m
catriona-m requested changes Apr 1, 2022
View reviewed changes
Copy link
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

Thanks @xiaxyi, this is looking good so far, just left a couple of comments inline. Also, looks like there may be a linting error that needs fixed up. Thanks!

website/docs/r/signalr_shared_private_link.html.markdown Outdated Show resolved Hide resolved
website/docs/r/signalr_shared_private_link.html.markdown Outdated Show resolved Hide resolved
website/docs/r/signalr_shared_private_link.html.markdown Outdated Show resolved Hide resolved
website/docs/r/signalr_shared_private_link.html.markdown Outdated Show resolved Hide resolved
internal/services/signalr/signalr_shared_private_link_resource.go Outdated Show resolved Hide resolved
@xiaxyi
Copy link
Contributor Author

xiaxyi commented Apr 1, 2022

Thanks @catriona-m , updated the code.

tombuildsstuff
tombuildsstuff reviewed Apr 11, 2022
View reviewed changes
internal/services/signalr/client/client.go Outdated
@@ -3,7 +3,7 @@ package client
import (
"github.com/Azure/azure-sdk-for-go/services/webpubsub/mgmt/2021-10-01/webpubsub"
"github.com/hashicorp/terraform-provider-azurerm/internal/common"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/signalr/sdk/2020-05-01/signalr"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/signalr/sdk/2021-10-01/signalr"
Copy link
Contributor

Choose a reason for hiding this comment

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

where's the upstream PR using this? I can only see 2020-05-01 upstream?

tombuildsstuff
tombuildsstuff reviewed Apr 11, 2022
View reviewed changes
internal/services/signalr/client/client.go Outdated
@@ -3,7 +3,7 @@ package client
import (
"github.com/Azure/azure-sdk-for-go/services/webpubsub/mgmt/2021-10-01/webpubsub"
"github.com/hashicorp/terraform-provider-azurerm/internal/common"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/signalr/sdk/2020-05-01/signalr"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/signalr/sdk/2021-10-01/signalr"
Copy link
Contributor

Choose a reason for hiding this comment

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

where's the upstream PR using this? I can only see 2020-05-01 upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tombuildsstuff are you suggesting to raise a PR to upgrade the SDK only?

Copy link
Collaborator

Choose a reason for hiding this comment

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

the service has now been moved over to go-azure-sdk, could we update the PR to reflect this?

@xiaxyi
Copy link
Contributor Author

xiaxyi commented Jul 20, 2022

@katbyte Sure, updated the SDK.

@katbyte katbyte changed the title Adding shared private link for Azure Signalr service New Resourece: azurerm_signalr_shared_private_link_resource Jul 29, 2022
katbyte
katbyte approved these changes Jul 29, 2022
View reviewed changes
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 katbyte merged commit 15cfe83 into hashicorp:main Jul 29, 2022
katbyte added a commit that referenced this pull request Jul 29, 2022
@katbyte
CHANGELOG #16187
1e720de
@github-actions github-actions bot added this to the v3.16.0 milestone Jul 29, 2022
@github-actions
Copy link

github-actions bot commented Aug 1, 2022

This functionality has been released in v3.16.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
Copy link

github-actions bot commented Sep 1, 2022

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 Sep 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tombuildsstuff tombuildsstuff tombuildsstuff left review comments

@catriona-m catriona-m catriona-m requested changes

@katbyte katbyte katbyte approved these changes

Assignees
No one assigned
Labels
documentation service/signalr size/XL
Projects
None yet
Milestone
v3.16.0
Development

Successfully merging this pull request may close these issues.
4 participants
@xiaxyi @tombuildsstuff @katbyte @catriona-m