-
Notifications
You must be signed in to change notification settings - Fork 246
Credentials fix #1090
Credentials fix #1090
Changes from 5 commits
b1064fe
258ab1c
d110575
1634b0c
7f3f5cc
511f774
2f87e29
c6c093b
9a7b70e
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 |
|---|---|---|
|
|
@@ -63,6 +63,41 @@ Note: x64 Ruby for Windows is known to have some compatibility issues. | |
|
|
||
| # Getting Started with Azure Resource Manager Usage (Preview) | ||
|
|
||
| ## Prerequisite | ||
|
|
||
| In order to use the Azure SDK, you must supply the following values to the Azure SDK: | ||
|
|
||
| * Tenant Id | ||
| * Client Id | ||
| * Subscription Id | ||
| * Client Secret | ||
|
|
||
| You could pass the above values in the following ways: | ||
|
|
||
| ### Option 1 - Environment Variables | ||
| You can set the (above) values using the following environment variables: | ||
|
|
||
| * AZURE_TENANT_ID | ||
| * AZURE_CLIENT_ID | ||
| * AZURE_SUBSCRIPTION_ID | ||
| * AZURE_CLIENT_SECRET | ||
|
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. can we put simple instructions on how to set them in mac and windows? (I think we had this before)
Contributor
Author
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. Done |
||
|
|
||
| ### Option 2 - Options Hash | ||
| You can set the (above) values using the options hash: | ||
|
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. I think we need to give context on what the options hash is - for example "the options hash getting passed as parameter when constructing a client for the service/profile"
Contributor
Author
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. Done |
||
|
|
||
| ```ruby | ||
| options = { | ||
| tenant_id: 'YOUR TENANT ID', | ||
| client_id: 'YOUR CLIENT ID', | ||
| client_secret: 'YOUR CLIENT SECRET', | ||
| subscription_id: 'YOUR SUBSCRIPTION ID' | ||
| } | ||
| ``` | ||
|
|
||
| ### Option 3 - Combination of Environment Variables & Options Hash | ||
|
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. what's the recommendation for the case in which the user wants to create credentials themselves? either from TokenProvider or MSITokenProvider? if they were to pass them in the hash, they would still need to pass in the 4 values specified above? so if credentials are passed, there are 5 values required for the hash?
Contributor
Author
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. Done |
||
| You can set the (above) values using a combination of environment variables and options hash. The values mentioned in the options hash will take precedence over the environment variables. | ||
|
|
||
|
|
||
| ## Install the rubygem packages | ||
|
|
||
| You can install the azure rubygem packages directly. | ||
|
|
@@ -150,14 +185,10 @@ The following lines should be used to instantiate a profile client: | |
|
|
||
| ```ruby | ||
| # Provide credentials | ||
| provider = MsRestAzure::ApplicationTokenProvider.new( | ||
| ENV['AZURE_TENANT_ID'], | ||
| ENV['AZURE_CLIENT_ID'], | ||
| ENV['AZURE_CLIENT_SECRET']) | ||
| credentials = MsRest::TokenCredentials.new(provider) | ||
|
|
||
| options = { | ||
| credentials: credentials, | ||
| tenant_id: ENV['AZURE_TENANT_ID'], | ||
| client_id: ENV['AZURE_CLIENT_ID'], | ||
| client_secret: ENV['AZURE_CLIENT_SECRET'], | ||
| subscription_id: ENV['AZURE_SUBSCRIPTION_ID'] | ||
| } | ||
|
|
||
|
|
@@ -195,14 +226,10 @@ The following lines should be used to instantiate a profile client: | |
|
|
||
| ```ruby | ||
| # Provide credentials | ||
| provider = MsRestAzure::ApplicationTokenProvider.new( | ||
| ENV['AZURE_TENANT_ID'], | ||
| ENV['AZURE_CLIENT_ID'], | ||
| ENV['AZURE_CLIENT_SECRET']) | ||
| credentials = MsRest::TokenCredentials.new(provider) | ||
|
|
||
| options = { | ||
| credentials: credentials, | ||
| tenant_id: ENV['AZURE_TENANT_ID'], | ||
| client_id: ENV['AZURE_CLIENT_ID'], | ||
| client_secret: ENV['AZURE_CLIENT_SECRET'], | ||
| subscription_id: ENV['AZURE_SUBSCRIPTION_ID'] | ||
| } | ||
|
|
||
|
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. would our guidelines from before profile changes not work for initializing credentials, I believe we were doing: provider = MsRestAzure::ApplicationTokenProvider.new(
ENV['AZURE_TENANT_ID'],
ENV['AZURE_CLIENT_ID'],
ENV['AZURE_CLIENT_SECRET'])
credentials = MsRest::TokenCredentials.new(provider)
@client = Azure::ARM::Resources::ResourceManagementClient.new(credentials)
@client.subscription_id = @subscription_idIf so, should we leave them as they were, just updating the namespace for ResourceManagementClient? so we demonstrate the minimum change?
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. BTW, after this PR is done, we'd need to re-update the samples that were updated with the last release too.
Contributor
Author
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. The detailed explanation would be that we always want the user (of the SDK) to provide us with the tenant_id, client_id, client_secret and subscription_id. There is no escaping that. The credentials is just a derived value from these. The user may or may not provide it. But, when you think about it, why would a user want to provide a derived value when he/she is supplying the original values anyway? That is the reason, I am updating the examples in readme and removed the credentials. But, if a user wants to provide it, then he is free to do that.
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. So... Credentials are not a bad thing to provide. This abstraction allows any token provider to be used. |
||
|
|
@@ -226,8 +253,7 @@ purchase_plan_obj = Azure::Compute::Profiles::Latest::Mgmt::Models::PurchasePlan | |
|
|
||
| ## Usage of Individual gem using using specific api-version | ||
|
|
||
| In the previous section, we used the profile associated with individual gem. In the current section, we could use the | ||
| version directly. | ||
| In the previous section, we used the profile associated with individual gem. In the current section, we could use the version directly. | ||
|
|
||
| ### Install | ||
|
|
||
|
|
@@ -248,14 +274,9 @@ provider = MsRestAzure::ApplicationTokenProvider.new( | |
| ENV['AZURE_CLIENT_SECRET']) | ||
| credentials = MsRest::TokenCredentials.new(provider) | ||
|
|
||
| options = { | ||
| credentials: credentials, | ||
| subscription_id: ENV['AZURE_SUBSCRIPTION_ID'] | ||
| } | ||
|
|
||
| # Target client for 2016_03_30 version of Compute | ||
| compute_client = Azure::Compute::Mgmt::V2016_03_30::ComputeManagementClient.new(credentials) | ||
| compute_client.subscription_id = subscription_id | ||
| compute_client.subscription_id = ENV['AZURE_SUBSCRIPTION_ID'] | ||
| ``` | ||
|
|
||
| The compute client could be used to access operations and models: | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,86 @@ | ||||
| # encoding: utf-8 | ||||
| # Copyright (c) Microsoft Corporation. All rights reserved. | ||||
| # Licensed under the MIT License. See License.txt in the project root for license information. | ||||
|
|
||||
| module Azure::Common | ||||
| # The Azure::Common::Configurable module provides basic configuration for Azure activities. | ||||
| module Configurable | ||||
| # @return [String] Azure tenant id (also known as domain). | ||||
| attr_accessor :tenant_id | ||||
|
|
||||
| # @return [String] Azure client id. | ||||
| attr_accessor :client_id | ||||
|
|
||||
| # @return [String] Azure secret key. | ||||
| attr_accessor :client_secret | ||||
|
|
||||
| # @return [String] Azure subscription id. | ||||
| attr_accessor :subscription_id | ||||
|
|
||||
| # @return [MsRestAzure::ActiveDirectoryServiceSettings] Azure active directory service settings. | ||||
| attr_accessor :active_directory_settings | ||||
|
|
||||
| # @return [MsRest::ServiceClientCredentials] credentials to authorize HTTP requests made by the service client. | ||||
| attr_accessor :credentials | ||||
|
|
||||
| class << self | ||||
| # | ||||
| # List of configurable keys for {Azure::Common::Client}. | ||||
| # @return [Array] of option keys. | ||||
| # | ||||
| def keys | ||||
| @keys ||= [:tenant_id, :client_id, :client_secret, :subscription_id, :active_directory_settings] | ||||
| end | ||||
| end | ||||
|
|
||||
| # | ||||
| # Set configuration options using a block. | ||||
| # | ||||
| def configure | ||||
| yield self | ||||
| end | ||||
|
|
||||
| # | ||||
| # Resets the configurable options to provided options or defaults. | ||||
| # This will also creates MsRest::TokenCredentials to be used for subsequent Azure Resource Manager clients. | ||||
| # | ||||
| def reset!(options = {}) | ||||
| Azure::Common::Configurable.keys.each do |key| | ||||
| default_value = Azure::Common::Default.options[key] | ||||
| instance_variable_set(:"@#{key}", options.fetch(key, default_value)) | ||||
| end | ||||
|
|
||||
| fail ArgumentError, 'tenant_id is nil' if self.tenant_id.nil? | ||||
|
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. can we give an indication on how to fix it? set the env vars or pass it in options?
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. also maybe match the message from azure-sdk-for-ruby/runtime/ms_rest_azure/lib/ms_rest_azure/credentials/application_token_provider.rb Line 50 in ab9cdb6
|
||||
| fail ArgumentError, 'client_id is nil' if self.client_id.nil? | ||||
| fail ArgumentError, 'client_secret is nil' if self.client_secret.nil? | ||||
| fail ArgumentError, 'subscription_id is nil' if self.subscription_id.nil? | ||||
| fail ArgumentError, 'active_directory_settings is nil' if self.active_directory_settings.nil? | ||||
|
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. could this be nil at this point? this is not something we're asking the user to pass in, right?
Contributor
Author
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. Technically, yes. They will not be nil. I just wanted to be cautious that one of our users intentionally passing nil value for active_directory_settings in the options hash. |
||||
|
|
||||
| default_value = MsRest::TokenCredentials.new( | ||||
| MsRestAzure::ApplicationTokenProvider.new( | ||||
| self.tenant_id, self.client_id, self.client_secret, self.active_directory_settings)) | ||||
|
|
||||
| instance_variable_set(:"@credentials", options.fetch(:credentials, default_value)) | ||||
|
|
||||
| self | ||||
| end | ||||
|
|
||||
| def config | ||||
| self | ||||
| end | ||||
|
|
||||
| private | ||||
|
|
||||
| # | ||||
| # configures configurable options to default values | ||||
| # | ||||
| def setup_default_options | ||||
| opts = {} | ||||
| Azure::Common::Configurable.keys.map do |key| | ||||
| opts[key] = Azure::Common::Default.options[key] | ||||
| end | ||||
|
|
||||
| opts | ||||
| end | ||||
| end | ||||
| end | ||||
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.
I think would place this section after the "Install the rubygem packages" section, as that one provides context on authentication. Then we can talk about how to set this for usage in the packages.'