Skip to content

[KeyVault] - Added immutable flag to key release policy#20090

Merged
maorleger merged 5 commits intoAzure:mainfrom
maorleger:immutable-release-policy
Jan 27, 2022
Merged

[KeyVault] - Added immutable flag to key release policy#20090
maorleger merged 5 commits intoAzure:mainfrom
maorleger:immutable-release-policy

Conversation

@maorleger
Copy link
Copy Markdown
Member

Packages impacted by this PR

@azure/keyvault-keys

Issues associated with this PR

Resolves #19857

Describe the problem that is addressed by this PR

Now that this flag has been added we can regenerate code and add support for
immutable key release policies. Once a policy is marked immutable it can no
longer be modified.

Are there test cases added in this PR? (If not, why?)

Yes, I debated whether I should also add a test case for "create a key, then
update the release policy to be immutable, then try to change it" but it
wouldn't drive any code so I skipped it.

Do note that since MHSM does not currently support this flag I was unable to add
a test for it for MHSM, but I will create an issue to add that in later.

Checklists

  • Added impacted package name to the issue description
  • Does this PR needs any fixes in the SDK Generator?** (If so, create an Issue in the Autorest/typescript repository and link it here)
  • Added a changelog (if necessary)

};

/** Known values of {@link EncryptionAlgorithm} that the service accepts. */
export enum KnownEncryptionAlgorithms {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh yeah, I forgot I did this. At some point the swagger removed all these comments and I got tired of adding them back in every time I regenerate!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you prevent the generated enum from ending up in source then? Or is the idea with minimization is that apps should use webpack or something to remove unused code?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We already GA'd with this enum in-place, it was just re-exported directly from generated code

DeletionRecoveryLevel,
KnownDeletionRecoveryLevel,
KeyVaultClientGetKeysOptionalParams,
GetKeysOptionalParams,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These changes just absorb the corresponding name changes in the TS codegen so we can use the latest and greatest

@azure-sdk
Copy link
Copy Markdown
Collaborator

API changes have been detected in @azure/keyvault-keys. You can review API changes here

API changes

+     immutable?: boolean;
- export declare enum KnownJsonWebKeyEncryptionAlgorithm 
+ export declare enum KnownEncryptionAlgorithms 

@maorleger
Copy link
Copy Markdown
Member Author

maorleger commented Jan 27, 2022

API changes have been detected in @azure/keyvault-keys. You can review API changes here

API changes

- export declare enum KnownJsonWebKeyEncryptionAlgorithm 
+ export declare enum KnownEncryptionAlgorithms 

This is a bug in APIView / api-extractor and not a real change. The problem is that it misrepresents something that is either imported with a rename (import {x as y}) or exported under a different name (export { x as y }) - I forget if it's just the former or both cases 😄

If you look at our published docs or pull the last beta / GA you can see that it is clearly named KnownEncryptionAlgorithms and now APIView will reflect it

@maorleger maorleger requested a review from heaths January 27, 2022 16:27
};

/** Known values of {@link EncryptionAlgorithm} that the service accepts. */
export enum KnownEncryptionAlgorithms {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you prevent the generated enum from ending up in source then? Or is the idea with minimization is that apps should use webpack or something to remove unused code?

Copy link
Copy Markdown
Contributor

@sadasant sadasant left a comment

Choose a reason for hiding this comment

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

Oh so clean 👏

Copy link
Copy Markdown
Member

@xirzec xirzec left a comment

Choose a reason for hiding this comment

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

API changes look good

@maorleger maorleger merged commit 9ef90e4 into Azure:main Jan 27, 2022
@maorleger maorleger deleted the immutable-release-policy branch January 27, 2022 21:01
sadasant pushed a commit to sadasant/azure-sdk-for-js that referenced this pull request Jan 28, 2022
### Packages impacted by this PR
@azure/keyvault-keys

### Issues associated with this PR
Resolves Azure#19857

### Describe the problem that is addressed by this PR
Now that this flag has been added we can regenerate code and add support for
immutable key release policies. Once a policy is marked immutable it can no
longer be modified.

### Are there test cases added in this PR? _(If not, why?)_
Yes, I debated whether I should also add a test case for "create a key, then
update the release policy to be immutable, then try to change it" but it
wouldn't drive any code so I skipped it.

Do note that since MHSM does not currently support this flag I was unable to add
a test for it for MHSM, but I will create an issue to add that in later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regenerate, make sure immutable property is added

5 participants