Skip to content

private link resources swagger#6866

Merged
jhendrixMSFT merged 1 commit intoAzure:masterfrom
beoberha:privatelinkresources
Aug 27, 2019
Merged

private link resources swagger#6866
jhendrixMSFT merged 1 commit intoAzure:masterfrom
beoberha:privatelinkresources

Conversation

@beoberha
Copy link
Contributor

@beoberha beoberha commented Aug 5, 2019

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

  • Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
  • adding/removing APIs.
  • adding/removing properties.
  • adding/removing API-version.
  • adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

  • If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
    Please follow the link to find more details on API review process.

@beoberha beoberha requested a review from jaredmoo as a code owner August 5, 2019 22:05
@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Aug 5, 2019

In Testing, Please Ignore

[Logs] (Generated from df33ca9, Iteration 3)

Warning .NET: test-repo-billy/azure-sdk-for-net [Logs] [Diff]
  • No packages generated.
Succeeded Python: test-repo-billy/azure-sdk-for-python [Logs] [Diff]
Failed Java: test-repo-billy/azure-sdk-for-java [Logs] [Diff]
Warning Go: test-repo-billy/azure-sdk-for-go [Logs] [Diff]
  • Warning preview/sql/mgmt/2015-05-01-preview [Logs]
  • Warning preview/sql/mgmt/2017-03-01-preview [Logs]
  • Warning preview/sql/mgmt/2017-10-01-preview [Logs]
  • Warning sql/mgmt/2014-04-01 [Logs]
Succeeded JavaScript: test-repo-billy/azure-sdk-for-js [Logs] [Diff]
Succeeded Ruby: test-repo-billy/azure-sdk-for-ruby [Logs] [Diff]

@AutorestCI
Copy link

AutorestCI commented Aug 5, 2019

Automation for azure-sdk-for-python

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-python#6919

@AutorestCI
Copy link

AutorestCI commented Aug 5, 2019

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@jhendrixMSFT jhendrixMSFT added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Aug 5, 2019
@jaredmoo
Copy link
Contributor

jaredmoo commented Aug 6, 2019

Looks good, waiting for Ben to give signal that it's ready to merge.

@beoberha beoberha changed the title private link resources swagger [DO NOT MERGE] private link resources swagger Aug 6, 2019
"$ref": "#/definitions/PrivateLinkResourceListResult"
}
},
"default": {
Copy link
Contributor

Choose a reason for hiding this comment

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

please start including the default response. @jaredmoo - any ETA when SQL can start including the default response where the schema complies with th ARM error contract schema?

"description": "Properties of a private link resource.",
"type": "object",
"properties": {
"groupId": {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this resource group id as indicated in the description or something else? from the example, it doesnt seem like this is the resource group id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GroupId is not related to Resource Groups in any way. GroupId represents different endpoints that a service can be accessed. Azure Networking's PrivateLink feature is being implemented by many services and during creation of PrivateLink you need to specify which "group" or endpoint you want to create the link against. For example, CosmosDB has an endpoint for MongoDB and an endpoint for Cassandra. Thus there are two "groups" that can be chosen. This API is a utility API that lists them.

SQL is a bit trivial because we only have one endpoint and thus one group.

"type": "string",
"readOnly": true
},
"requiredMembers": {
Copy link
Contributor

Choose a reason for hiding this comment

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

what are some examples of members?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Members are sub sections of groups. I am not completely positive how they work honestly because SQL only has one group and one member: "sqlServer"

}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Sql/servers/{serverName}/privateLinkResources/{groupName}": {
Copy link
Contributor

Choose a reason for hiding this comment

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

how is a private link resource created?

Copy link
Contributor

Choose a reason for hiding this comment

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

how is the groupName parameter in the URL different from the groupId parameter in the body?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this API doesn't map to any actual resource. This API is a utility to allow customers to be able to pass the proper "groupId" and "memberName" parameters to PUT PrivateEndpoint. Since groupId and memberName are different for each service, all implementing services have to implement this API. It is defined in section 6.4 in the NRP Spec.

Since SQL has only one groupId and member, we just return static values.

@ravbhatnagar
Copy link
Contributor

@beoberha - few comments, please take a look.

@ravbhatnagar
Copy link
Contributor

Signing off from ARM side.

@ravbhatnagar ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMChangesRequested WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Aug 12, 2019
@jhendrixMSFT
Copy link
Member

Looks good from SDK perspective, please let me know when this has been deployed and is ready for merge.

@jhendrixMSFT
Copy link
Member

@beoberha has this been deployed yet?

@beoberha beoberha changed the title [DO NOT MERGE] private link resources swagger private link resources swagger Aug 27, 2019
@beoberha
Copy link
Contributor Author

beoberha commented Aug 27, 2019

@jhendrixMSFT This is ready for merging now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants