-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Users/ajya/nationalcloudclientsidechanges #3551
Changes from 7 commits
75ff28b
c0ea485
c948cb9
23cbd25
4162125
2ae2c5e
a484fcf
047f93c
ff1f403
107a0ee
a5cea63
28c263d
41b7a4e
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 |
---|---|---|
|
@@ -5,19 +5,20 @@ import * as httpClient from 'vso-node-api/HttpClient'; | |
import * as restClient from 'vso-node-api/RestClient'; | ||
|
||
var httpObj = new httpClient.HttpCallbackClient(tl.getVariable("AZURE_HTTP_USER_AGENT")); | ||
var authUrl = 'https://login.windows.net/'; | ||
var defaultAuthUrl = 'https://login.windows.net/'; | ||
var azureApiVersion = '2016-09-01'; | ||
|
||
function getAccessToken(SPN, endpointUrl: string): Q.Promise<string> { | ||
function getAccessToken(endpoint, endpointUrl: string): Q.Promise<string> { | ||
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. having the second parameter as enpointurl is confusing. There is already endpoint taken as input which has endpoint.url. I presume this is the resource for which you are requesting token. So should we just call it resource? |
||
|
||
var deferred = Q.defer<string>(); | ||
var authorityUrl = authUrl + SPN.tenantID + "/oauth2/token/"; | ||
var envAuthUrl = (endpoint.envAuthUrl) ? endpoint.envAuthUrl : defaultAuthUrl; | ||
var authorityUrl = envAuthUrl + endpoint.tenantID + "/oauth2/token/"; | ||
|
||
var post_data = querystring.stringify({ | ||
resource: endpointUrl, | ||
client_id: SPN.servicePrincipalClientID, | ||
client_id: endpoint.servicePrincipalClientID, | ||
grant_type: "client_credentials", | ||
client_secret: SPN.servicePrincipalKey | ||
client_secret: endpoint.servicePrincipalKey | ||
}); | ||
|
||
var requestHeader = { | ||
|
@@ -40,11 +41,11 @@ function getAccessToken(SPN, endpointUrl: string): Q.Promise<string> { | |
return deferred.promise; | ||
} | ||
|
||
export async function getNetworkInterfacesInRG(SPN, endpointUrl: string, resourceGroupName: string) { | ||
export async function getNetworkInterfacesInRG(endpoint, endpointUrl: string, resourceGroupName: string) { | ||
|
||
var deferred = Q.defer<any>(); | ||
var restUrl = "https://management.azure.com/subscriptions/" + SPN.subscriptionId + "/resourceGroups/" + resourceGroupName + "/providers/Microsoft.Network/networkInterfaces?api-version=" + azureApiVersion; | ||
var accessToken = await getAccessToken(SPN, endpointUrl); | ||
var restUrl = endpoint.url + "subscriptions/" + endpoint.subscriptionId + "/resourceGroups/" + resourceGroupName + "/providers/Microsoft.Network/networkInterfaces?api-version=" + azureApiVersion; | ||
var accessToken = await getAccessToken(endpoint, endpointUrl); | ||
|
||
var requestHeader = { | ||
Authorization: 'Bearer ' + accessToken | ||
|
@@ -66,11 +67,11 @@ export async function getNetworkInterfacesInRG(SPN, endpointUrl: string, resourc | |
return deferred.promise; | ||
} | ||
|
||
export async function getLoadBalancer(SPN, endpointUrl: string, name: string, resourceGroupName: string) { | ||
export async function getLoadBalancer(endpoint, endpointUrl: string, name: string, resourceGroupName: string) { | ||
|
||
var deferred = Q.defer<any>(); | ||
var restUrl = "https://management.azure.com/subscriptions/" + SPN.subscriptionId + "/resourceGroups/" + resourceGroupName + "/providers/Microsoft.Network/loadBalancers/" + name + "?api-version=" + azureApiVersion; | ||
var accessToken = await getAccessToken(SPN, endpointUrl); | ||
var restUrl = endpoint.url + "subscriptions/" + endpoint.subscriptionId + "/resourceGroups/" + resourceGroupName + "/providers/Microsoft.Network/loadBalancers/" + name + "?api-version=" + azureApiVersion; | ||
var accessToken = await getAccessToken(endpoint, endpointUrl); | ||
|
||
var requestHeader = { | ||
authorization: 'Bearer ' + accessToken | ||
|
@@ -92,10 +93,10 @@ export async function getLoadBalancer(SPN, endpointUrl: string, name: string, re | |
return deferred.promise; | ||
} | ||
|
||
export async function getNetworkInterface(SPN, endpointUrl, name: string, resourceGroupName: string) { | ||
export async function getNetworkInterface(endpoint, endpointUrl, name: string, resourceGroupName: string) { | ||
var deferred = Q.defer<any>(); | ||
var restUrl = "https://management.azure.com/subscriptions/" + SPN.subscriptionId + "/resourceGroups/" + resourceGroupName + "/providers/Microsoft.Network/networkInterfaces/" + name + "?api-version=" + azureApiVersion; | ||
var accessToken = await getAccessToken(SPN, endpointUrl); | ||
var restUrl = endpoint.url + "subscriptions/" + endpoint.subscriptionId + "/resourceGroups/" + resourceGroupName + "/providers/Microsoft.Network/networkInterfaces/" + name + "?api-version=" + azureApiVersion; | ||
var accessToken = await getAccessToken(endpoint, endpointUrl); | ||
|
||
var requestHeader = { | ||
authorization: 'Bearer ' + accessToken | ||
|
@@ -136,11 +137,11 @@ async function checkProvisioningState(url: string, accessToken: string) { | |
return deferred.promise; | ||
} | ||
|
||
export async function setNetworkInterface(SPN, endpointUrl: string, nic, resourceGroupName: string){ | ||
export async function setNetworkInterface(endpoint, endpointUrl: string, nic, resourceGroupName: string){ | ||
|
||
var deferred = Q.defer(); | ||
var restUrl = "https://management.azure.com/subscriptions/" + SPN.subscriptionId + "/resourceGroups/" + resourceGroupName + "/providers/Microsoft.Network/networkInterfaces/" + nic.name + "?api-version=" + azureApiVersion; | ||
var accessToken = await getAccessToken(SPN, endpointUrl); | ||
var restUrl = endpoint.url + "subscriptions/" + endpoint.subscriptionId + "/resourceGroups/" + resourceGroupName + "/providers/Microsoft.Network/networkInterfaces/" + nic.name + "?api-version=" + azureApiVersion; | ||
var accessToken = await getAccessToken(endpoint, endpointUrl); | ||
var requestHeader = { | ||
"Content-Type": "application/json; charset=utf-8", | ||
"Authorization": 'Bearer ' + accessToken | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -321,6 +321,8 @@ | |
"ClientIdCannotBeEmpty": "clientId must be a non empty string.", | ||
"DomainCannotBeEmpty": "domain must be a non empty string.", | ||
"SecretCannotBeEmpty": "secret must be a non empty string.", | ||
"armUrlCannotBeEmpty": "arm Url must be a non empty string.", | ||
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. Is the plan only to add support in latest task version for ARG? I am fine with that call, but want to confirm with PMs. |
||
"authorityUrlCannotBeEmpty": "authority must be a non empty string.", | ||
"CouldNotFetchAccessTokenforAzureStatusCode": "Could not fetch access token for azure. Status code: %s, status message: %s", | ||
"LoadBalancerNameCannotBeNull": "'loadBalancerName cannot be null or undefined and it must be of type string.'", | ||
"NetworkInterfaceNameCannotBeNull": "networkInterfaceName cannot be null or undefined and it must be of type string.", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,11 @@ function Initialize-AzureSubscription { | |
|
||
#Set UserAgent for Azure Calls | ||
Set-UserAgent | ||
|
||
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 you ensure Azure PowerShell task is covered as part of your testing? 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. Yes I have tested Azure Powershell Task |
||
$environmentName = "AzureCloud" | ||
if( $Endpoint.Data.Environment ) { | ||
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. this does not validate if it contains an empty string with space. Please check if you want to trim it before it gets used |
||
$environmentName = $Endpoint.Data.Environment | ||
} | ||
|
||
if ($Endpoint.Auth.Scheme -eq 'Certificate') { | ||
# Certificate is only supported for the Azure module. | ||
|
@@ -57,11 +62,6 @@ function Initialize-AzureSubscription { | |
$additional['CurrentStorageAccountName'] = $StorageAccount | ||
} | ||
|
||
$environmentName = "AzureCloud" | ||
if( $Endpoint.Data.Environment ) { | ||
$environmentName = $Endpoint.Data.Environment | ||
} | ||
|
||
# Set the subscription. | ||
Write-Host "##[command]Set-AzureSubscription -SubscriptionName $($Endpoint.Data.SubscriptionName) -SubscriptionId $($Endpoint.Data.SubscriptionId) -Certificate ******** -Environment $environmentName $(Format-Splat $additional)" | ||
Set-AzureSubscription -SubscriptionName $Endpoint.Data.SubscriptionName -SubscriptionId $Endpoint.Data.SubscriptionId -Certificate $certificate -Environment $environmentName @additional | ||
|
@@ -127,7 +127,7 @@ function Initialize-AzureSubscription { | |
# Else, this is AzureRM. | ||
try { | ||
Write-Host "##[command]Add-AzureRMAccount -ServicePrincipal -Tenant $($Endpoint.Auth.Parameters.TenantId) -Credential $psCredential" | ||
$null = Add-AzureRMAccount -ServicePrincipal -Tenant $Endpoint.Auth.Parameters.TenantId -Credential $psCredential | ||
$null = Add-AzureRMAccount -ServicePrincipal -Tenant $Endpoint.Auth.Parameters.TenantId -Credential $psCredential -EnvironmentName $environmentName | ||
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. better to log $environmentName if it is within security rules |
||
} catch { | ||
# Provide an additional, custom, credentials-related error message. | ||
Write-VstsTaskError -Message $_.Exception.Message | ||
|
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.
Did this API version work fine in other Cloud environment types. Do you see any issue with Azure deployments causing issues?