Skip to content

Conversation

@blueww
Copy link
Member

@blueww blueww commented Feb 18, 2016

  • Add cmdlet to generate SAS token against storage account
    • New-AzureStorageAccountSASToken
  • Add IPAddressOrRange/Protocol support in cmdlets to generate SAS token against blob, container, file, share, table, queue
    • New-AzureStorageBlobSASToken
    • New-AzureStorageContainerSASToken
    • New-AzureStorageFileSASToken
    • New-AzureStorageShareSASToken
    • New-AzureStorageQueueSASToken
    • New-AzureStorageTableSASToken
      *Change the Help xml file for the cmdlets update. (The help text is pending Tamra review as he is OOF, might have another PR for Help Text change from Tamra later)
      *The Help xml has many changes as it's re-generated from the PSHelp Edit Tool. You can find all Help text change (and conclusion of each change) from:
      HelpChange.docx.

@azurecla
Copy link

Hi @blueww, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!


It looks like you're working at Microsoft (weiwei). If you're full-time, we DON'T require a contribution license agreement.



If you are a vendor, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

private string accessPolicyIdentifier;

[Parameter(HelpMessage = "Permissions for a container. Permissions can be any not-empty subset of \"rwdl\".",
ParameterSetName = SasPermissionParameterSet)]
Copy link
Member

Choose a reason for hiding this comment

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

indentation

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@markcowl
Copy link
Member

@blueww I'm excited to see this feature. It looks like a little bit of rethinking input parmater types is necessary to make the cmdlets usable - the user should not have to separately create a complex .Net type in order to use the cmdlet.

Also, need to see some test coverage and I assume the help update covers the new cmdlets.

@blueww
Copy link
Member Author

blueww commented Feb 19, 2016

Hi Mark,

Thanks for the review and comments!

I have replied all your comments. Summarize as below:

  1. For the parameter types “SharedAccessAccountServices” , “SharedAccessAccountResourceTypes” , “SharedAccessProtocol”, they are all Enum, and Powershell support Enum input very well. User can just press tab to get the enum value, don't need to create by themselves. Such as following parameters works well:
    New-AzureStorageAccountSASToken -Service Blob,File,Table,Queue -ResourceType Container,Object,Service -Permission rw -Protocol HttpsOrHttp
  2. You have a comments to change “ExpiryTime” to “ExpirationTime”. As "ExpiryTime" is standard name in azure rest API, and release long ago in cmdlets to create SAS token, we don't plan to change that.
  3. I have fixed the indentation issue raised in your comments in the PR.
  4. We have already complete the test implementation of the new features, and 100% passed. See the attached report.
  5. Yes, the help file has already been changed to include the new support features.

Please let me know if you have other questions or comments!
Thanks!

<SpecificVersion>False</SpecificVersion>
<HintPath>..\..\..\packages\Newtonsoft.Json.6.0.8\lib\net45\Newtonsoft.Json.dll</HintPath>
</Reference>
<Reference Include="System.Spatial, Version=5.6.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you have changes here?

hovsepm pushed a commit that referenced this pull request Feb 23, 2016
Add new Account SAS Token cmdlet, and add IPacl/Protocal support to B/T/Q/F new SAS cmdlets
@hovsepm hovsepm merged commit f088168 into Azure:dev Feb 23, 2016
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.

5 participants