-
Notifications
You must be signed in to change notification settings - Fork 97
NetworkACl update #447
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
NetworkACl update #447
Changes from all commits
8b01f9b
840bc67
6b777b9
e3cedf9
ffb3d8c
8ee4d5b
665b61d
5f55bb3
531c670
a4dca55
929fb8b
dcd32d0
b4cdc48
bec4326
e547233
b739852
05b1f15
91fd957
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -108,6 +108,14 @@ public interface Vault extends | |||||||||
| */ | ||||||||||
| @Beta(SinceVersion.V1_11_0) | ||||||||||
| CreateMode createMode(); | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Get the networkAcls value. | ||||||||||
| * | ||||||||||
| * @return the networkAcls value | ||||||||||
| */ | ||||||||||
| @Beta(SinceVersion.V1_11_0) | ||||||||||
| NetworkRuleSet networkAcls(); | ||||||||||
|
|
||||||||||
| /************************************************************** | ||||||||||
| * Fluent interfaces to provision a Vault | ||||||||||
|
|
@@ -179,6 +187,20 @@ interface WithAccessPolicy { | |||||||||
| @Method | ||||||||||
| AccessPolicy.DefinitionStages.Blank<WithCreate> defineAccessPolicy(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * A key vault definition allowing the networkAcl to be set. | ||||||||||
| */ | ||||||||||
| interface WithNetworkAcls { | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Set the networkAcls value. | ||||||||||
| * | ||||||||||
| * @param networkAcls the networkAcls value to set | ||||||||||
| * @return the next stage of key vault definition | ||||||||||
| */ | ||||||||||
| WithCreate withNetworkAcls(NetworkRuleSet networkAcls); | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tiffanyachen Please rename this to "withNetworkRuleSet()". Also in Java we try to avoid passing a whole complex object that needs to be built aside and instead we "decompose" them in place into a sequence of "with...()" methods corresponding to each member of the class that can be set. Think of it similar to setting the AccessPolicy.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems the key vault NetworkRuleSet is EXACTLY same as storage account network rule set. Customer will have consistent experience If we can ensure key vault nw ruleset experience is same as that storage provide. Storage: Line 17 in 6f62630
KeyVault: Line 17 in 6f62630
Take a look at this “stage” in storage account definition Line 446 in 6f62630
Line 752 in 6f62630
It seems you can copy over the helper, stages and implementations from storage. But double check the logic work for key vault as well. |
||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * A key vault definition allowing various configurations to be set. | ||||||||||
|
|
@@ -252,6 +274,7 @@ interface WithCreate extends | |||||||||
| Creatable<Vault>, | ||||||||||
| GroupableResource.DefinitionWithTags<WithCreate>, | ||||||||||
| DefinitionStages.WithSku, | ||||||||||
| DefinitionStages.WithNetworkAcls, | ||||||||||
| DefinitionStages.WithConfigurations, | ||||||||||
| DefinitionStages.WithAccessPolicy { | ||||||||||
| } | ||||||||||
|
|
@@ -298,6 +321,20 @@ interface WithAccessPolicy { | |||||||||
| AccessPolicy.Update updateAccessPolicy(String objectId); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * A key vault update allowing the networkAcl to be set. | ||||||||||
| */ | ||||||||||
| interface WithNetworkAcls { | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Set the networkAcls value. | ||||||||||
| * | ||||||||||
| * @param networkAcls the networkAcls value to set | ||||||||||
| * @return the next stage of key vault definition | ||||||||||
| */ | ||||||||||
| Update withNetworkAcls(NetworkRuleSet networkAcls); | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same feedback as above. |
||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * A key vault update allowing various configurations to be set. | ||||||||||
| */ | ||||||||||
|
|
@@ -369,6 +406,7 @@ interface Update extends | |||||||||
| GroupableResource.UpdateWithTags<Update>, | ||||||||||
| Appliable<Vault>, | ||||||||||
| UpdateStages.WithAccessPolicy, | ||||||||||
| UpdateStages.WithNetworkAcls, | ||||||||||
| UpdateStages.WithConfigurations { | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tiffanyachen In Fluent we try to avoid using acronyms especially when these are not needed. In this the method can be replaced with "networkRuleSet()". It's also consistent with a similar class found in Storage management library.