Adding support for encryption with customer-managed keys to Azure Search #5858
Conversation
|
Can one of the admins verify this patch? |
|
Since I couldn't find how to set the option myself, please use "squash" commits when completing the PR (once it gets approval). Thanks |
src/SDKs/Search/Management/Search.Management.Tests/Utilities/SearchServiceFixture.cs
Show resolved
Hide resolved
src/SDKs/Search/Management/Search.Management.Tests/Search.Management.Tests.csproj
Outdated
Show resolved
Hide resolved
src/SDKs/Search/DataPlane/Search.Tests/Tests/EncryptionWithCMK.cs
Outdated
Show resolved
Hide resolved
src/SDKs/Search/DataPlane/Search.Tests/Tests/EncryptionWithCMK.cs
Outdated
Show resolved
Hide resolved
src/SDKs/Search/DataPlane/Search.Tests/Tests/EncryptionWithCMK.cs
Outdated
Show resolved
Hide resolved
src/SDKs/Search/DataPlane/Search.Tests/Tests/EncryptionWithCMK.cs
Outdated
Show resolved
Hide resolved
| { | ||
| SearchServiceClient searchClient = Data.GetSearchServiceClient(); | ||
|
|
||
| Index encryptedIndex = CreateEncryptedTestIndex(new EncryptionKey() |
There was a problem hiding this comment.
I wanted to call this out as an example of test code done right. Your instinct in production code might be to factor these details out into a method, but in a test like this having the details be explicit clearly communicates the intended usage of the API. This test communicates exactly what it needs to.
|
@brjohnstmsft Hi Bruce, can you please take a look at the latest iteration when you have some time? Thanks |
|
@shmed Please pull from latest master and update the PR to fix the Travis failures |
|
@shmed Will review on Wednesday. |
src/SDKs/Search/DataPlane/Search.Tests/Utilities/EncryptionFixture.cs
Outdated
Show resolved
Hide resolved
src/SDKs/Search/DataPlane/Search.Tests/Utilities/EncryptionFixture.cs
Outdated
Show resolved
Hide resolved
src/SDKs/Search/DataPlane/Search.Tests/Utilities/EncryptionFixture.cs
Outdated
Show resolved
Hide resolved
src/SDKs/Search/DataPlane/Search.Tests/Utilities/EncryptionFixture.cs
Outdated
Show resolved
Hide resolved
src/SDKs/Search/DataPlane/Search.Tests/Utilities/EncryptionFixture.cs
Outdated
Show resolved
Hide resolved
|
@dsgouda Any idea why this "default" check is stuck at "expected"? It stayed this way for hours today, and closing and re-opening the PR didn't seem to help. |
|
Taking a look |
|
@brjohnstmsft We have some unexpectedly high load and all 4 Jenkins workers are busy, I am guessing this will be picked up once one of them is free. |
|
@dsgouda Thanks |
|
@dsgouda This still appears to be stuck and is blocking our deliverables for //build. Can you please investigate? |
|
Apologies, taking a look again,. |
This adds the generated code for the changes in API specs from this commit -> Azure/azure-rest-api-specs@6720121
which was reviewed in this PR -> Azure/azure-rest-api-specs#5567
Adds the Azure Search models for EncryptionKey and AzureActiveDirectoryApplicationCredentials, as well as adds the encryptionKey property to the Index and SynonymMap models.
This PR also includes serialization and deserialization test for the following scenario