Skip to content

Changes for Management.Sql to allow customers to add TDE keys and set TDE protector for managed instances #4738

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

Merged
merged 5 commits into from
Sep 5, 2018

Conversation

nivimsft
Copy link
Contributor

@nivimsft nivimsft commented Sep 4, 2018

Description

Changes for Management.Sql to allow customers to add TDE keys and set TDE protector for managed instances

Rest api spec PR


This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

SDK Generation Guidelines

  • If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
  • The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code.
  • The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

- Bumping client version
- Adding test session records
-Removing unrelated messages in csproj
@@ -53,12 +55,13 @@ public void TestCreateUpdateDropServerKey()
var keyList = sqlClient.ServerKeys.ListByServer(resourceGroup.Name, server.Name);
Assert.Equal(2, keyList.Count());

// Delete key
sqlClient.ServerKeys.Delete(resourceGroup.Name, server.Name, serverKeyName);
//TODO: Temporarily disabling this since delete operation is affected by a production bug.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the production bug? We should avoid publishing an SDK that has known issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Jared,

The bug is in management service where the post condition checks are failing even on operation success. This issue seems to affect multiple APIs in prod. Management service team is currently investigating the issue.
It is important to note that the failure occurs in an existing functionality (sqlClient.ServerKeys.Delete() method which was released last year) and is not caused by this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to managed instance keys.

- reverted the version to 1.20.0
- revised the release notes
Copy link
Contributor

@dsgouda dsgouda left a comment

Choose a reason for hiding this comment

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

Left a couple of comments.

@@ -56,6 +56,8 @@ internal static partial class SdkInfo
new Tuple<string, string, string>("Sql", "Jobs", "2017-03-01-preview"),
new Tuple<string, string, string>("Sql", "LongTermRetentionBackups", "2017-03-01-preview"),
new Tuple<string, string, string>("Sql", "ManagedDatabases", "2017-03-01-preview"),
new Tuple<string, string, string>("Sql", "ManagedInstanceEncryptionProtectors", "2017-10-01-preview"),
new Tuple<string, string, string>("Sql", "ManagedInstanceKeys", "2017-10-01-preview"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please run msbuild build.proj /t:Build /p:Scope=SDKs\SqlManagement and check in any changes generated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not see anything else being generated. Is there anything specific that you are looking for?

Copy link
Contributor

Choose a reason for hiding this comment

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

If there's nothing generated, we are good.

Copy link
Contributor

@dsgouda dsgouda left a comment

Choose a reason for hiding this comment

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

LGTM

@dsgouda dsgouda merged commit d8dd2e2 into Azure:psSdkJson6 Sep 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants