From f4ebbfc7a38ad83e0dc637d467e4f48be95e6296 Mon Sep 17 00:00:00 2001 From: Robbie Zhang Date: Tue, 13 Feb 2018 19:07:27 -0800 Subject: [PATCH] [Azure] Optimize VK Setup in ACS/AKS (#85) * Read ACS Credentials for Azure Authentication Supprt a new environment variable: ACS_CREDENTIAL_LOCATION Expect the value to be the ACS credential filepath, which is the /etc/kubernetes/azure.json file generated on the ACS nodes. If the ACS_CREDENTIAL_LOCATION is specified and loaded, create the Azure Authentication class from its values. If the AZURE_AUTHENTICATION_LOCATION is specified and loaded, its values will overwrite the value above. Refactor the ACI provider and ACI client to be able to override the SPN by environment variable --- charts/virtual-kubelet-for-aks-0.1.3.tgz | Bin 0 -> 1627 bytes charts/virtual-kubelet-for-aks/Chart.yaml | 8 ++ .../templates/NOTES.txt | 5 + .../templates/_helpers.tpl | 16 +++ .../templates/deployment.yaml | 57 ++++++++ .../templates/secrets.yaml | 9 ++ charts/virtual-kubelet-for-aks/values.yaml | 16 +++ providers/azure/aci.go | 61 ++++++++- providers/azure/acsCredential.go | 38 ++++++ providers/azure/acsCredential_test.go | 129 ++++++++++++++++++ providers/azure/client/aci/client.go | 10 +- providers/azure/client/aci/client_test.go | 37 ++--- providers/azure/client/auth.go | 57 ++++---- .../azure/client/resourcegroups/client.go | 10 +- .../client/resourcegroups/client_test.go | 32 ++--- 15 files changed, 399 insertions(+), 86 deletions(-) create mode 100644 charts/virtual-kubelet-for-aks-0.1.3.tgz create mode 100644 charts/virtual-kubelet-for-aks/Chart.yaml create mode 100644 charts/virtual-kubelet-for-aks/templates/NOTES.txt create mode 100644 charts/virtual-kubelet-for-aks/templates/_helpers.tpl create mode 100644 charts/virtual-kubelet-for-aks/templates/deployment.yaml create mode 100644 charts/virtual-kubelet-for-aks/templates/secrets.yaml create mode 100644 charts/virtual-kubelet-for-aks/values.yaml create mode 100644 providers/azure/acsCredential.go create mode 100644 providers/azure/acsCredential_test.go diff --git a/charts/virtual-kubelet-for-aks-0.1.3.tgz b/charts/virtual-kubelet-for-aks-0.1.3.tgz new file mode 100644 index 0000000000000000000000000000000000000000..599598f8f3fd7ed09ce39013c0af29403fd1ce6a GIT binary patch literal 1627 zcmV-h2Bi5PiwG0|00000|0w_~VMtOiV@ORlOnEsqVl!4SWK%V1T2nbTPgYhoO;>Dc zVQyr3R8em|NM&qo0PIw|fH0P*WBdHz8&KA9*FlcFP zb3=(*lJZ8E^>;5w`5{}*I*%gh0iJ&>jmRHgGe6FZXH=-1Fy}5GAw%Vixp2sxY_z9D zD7PR9`|xV;Et0G5?SW>_z!SigboEW#(jKX8=8VXsw z3^XCF-gY);GO1;4ni$#|5Ylj1~$nu-EwJ`V2^Xg zG>8o@%;)xd@8h3U_WS>EiY!ARU6s8s6xiecP0u^t^#7LEY_$&l{}%G_(D)T*H1R>A zf-z;NiVV*ay)DrcefYI8pIf?>72IbejX_P(1n$7c=GkNmuC-|AK#XIWBG{#KI%_S- znF6;=EVZM9`?(A^VM<{e=+F(`P27i*)tO~Rb#M!lFm=k z-lWe3>~ps9y>0Ly?DmBg%rn(%iOo+kOVd;W{jmljo$S|M#wagb5lu; zYdzL^b9MYh>byDetJ~j#>tHav>4k&UM*vf#@xg9-trO3#^}1;HhwU)947!8P#r5#I z*S;8Zdfhre165S3o{>npUnEcKu)p7i!Eg|CFS>(a=duo0St+J7cO${CE_D;IfBRd% z9d>T?6rT*cStWQJynWpXo-x~H&0;TLKWK+R?MVJ6c;Z)-01_jC>I#bx$D3cC;zpb8 z@38P8GBfNO*c}Jo8HPc>cN?~Y;is^7d$WJpTeX#)D>ty8cAC(`!zzrizoAsQj!U-j zCzPi)`~dbU_}6n$@37bj`g!e(m%~nNW9vt6!p9xQ zZuB=qO?{mwt6aJnk1K#P&hiBR!ShtfIwM2^V@wq#Ojd9c-MYlIcde?<5hRk%8tWNQ zD}GpGMm$M~zAyf5Z;$(*HJyuz)GZt*6MRNv6!u53jo0aiN@YO5E^KP?g~w>T;^T(7 zZI88Kcgunxe21?&SEjIFz_{Fkxu$7;U+-L&3@hDDZ+*e2Eda9y^G5`UXdTE@A}ct{ zEh*2rc315>;F3kEhE!_0Ph^lihd$-9Jn_l^nXv$40C%grs8KG}+z#&@z#QaNvOoSW zP3$#)1?{W?k1>s^VDEi` Z2RX<=4)QYjZvX%Q|NnmfsWAX1005PfAvXX3 literal 0 HcmV?d00001 diff --git a/charts/virtual-kubelet-for-aks/Chart.yaml b/charts/virtual-kubelet-for-aks/Chart.yaml new file mode 100644 index 000000000..c92909014 --- /dev/null +++ b/charts/virtual-kubelet-for-aks/Chart.yaml @@ -0,0 +1,8 @@ +name: virtual-kubelet-for-aks +version: 0.1.3 +description: a Helm chart to install virtual kubelet in an AKS or ACS cluster. +sources: + - https://github.com/virtual-kubelet/virtual-kubelet +maintainers: + - name: Robbie Zhang + email: junjiez@microsoft.com diff --git a/charts/virtual-kubelet-for-aks/templates/NOTES.txt b/charts/virtual-kubelet-for-aks/templates/NOTES.txt new file mode 100644 index 000000000..8a27ac8e2 --- /dev/null +++ b/charts/virtual-kubelet-for-aks/templates/NOTES.txt @@ -0,0 +1,5 @@ +The virtual kubelet is getting deployed on your cluster. + +To verify that virtual kubelet has started, run: + + kubectl --namespace={{ .Release.Namespace }} get pods -l "app={{ template "fullname" . }}" \ No newline at end of file diff --git a/charts/virtual-kubelet-for-aks/templates/_helpers.tpl b/charts/virtual-kubelet-for-aks/templates/_helpers.tpl new file mode 100644 index 000000000..c199f18f0 --- /dev/null +++ b/charts/virtual-kubelet-for-aks/templates/_helpers.tpl @@ -0,0 +1,16 @@ +{{/* vim: set filetype=mustache: */}} +{{/* +Expand the name of the chart. +*/}} +{{- define "name" -}} +{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}} +{{- end -}} + +{{/* +Create a default fully qualified app name. +We truncate at 24 chars because some Kubernetes name fields are limited to this (by the DNS naming spec). +*/}} +{{- define "fullname" -}} +{{- $name := default .Chart.Name .Values.nameOverride -}} +{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}} +{{- end -}} diff --git a/charts/virtual-kubelet-for-aks/templates/deployment.yaml b/charts/virtual-kubelet-for-aks/templates/deployment.yaml new file mode 100644 index 000000000..ac0fc23ba --- /dev/null +++ b/charts/virtual-kubelet-for-aks/templates/deployment.yaml @@ -0,0 +1,57 @@ +apiVersion: extensions/v1beta1 +kind: Deployment +metadata: + name: {{ template "fullname" . }} +spec: + replicas: 1 + template: + metadata: + labels: + app: {{ template "fullname" . }} + spec: + containers: + - name: {{ template "fullname" . }} + image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}" + imagePullPolicy: {{ .Values.image.pullPolicy }} + env: + - name: KUBELET_PORT + value: "10250" + - name: ACS_CREDENTIAL_LOCATION + value: /etc/virtual-kubelet/acs.json + - name: AZURE_TENANT_ID + value: {{ .Values.env.azureTenantId }} + - name: AZURE_SUBSCRIPTION_ID + value: {{ .Values.env.azureSubscriptionId }} + - name: AZURE_CLIENT_ID + value: {{ .Values.env.azureClientId }} + - name: AZURE_CLIENT_SECRET + valueFrom: + secretKeyRef: + name: {{ template "fullname" . }} + key: clientSecret + - name: ACI_RESOURCE_GROUP + value: {{ .Values.env.aciResourceGroup }} + - name: ACI_REGION + value: {{ default "westus" .Values.env.aciRegion }} + - name: APISERVER_CERT_LOCATION + value: /etc/virtual-kubelet/cert.pem + - name: APISERVER_KEY_LOCATION + value: /etc/virtual-kubelet/key.pem + - name: VKUBELET_POD_IP + valueFrom: + fieldRef: + fieldPath: status.podIP + volumeMounts: + - name: credentials + mountPath: "/etc/virtual-kubelet" + - name: acs-credential + mountPath: "/etc/virtual-kubelet/acs.json" + command: ["virtual-kubelet"] + args: ["--provider", "azure", "--namespace", "default", "--nodename", {{ default "virtual-kubelet" .Values.env.nodeName | quote }} , "--os", {{ default "Linux" .Values.env.nodeOsType | quote }}, "--taint", {{ default "azure.com/aci" .Values.env.nodeTaint | quote }}] + volumes: + - name: credentials + secret: + secretName: {{ template "fullname" . }} + - name: acs-credential + hostPath: + path: /etc/kubernetes/azure.json diff --git a/charts/virtual-kubelet-for-aks/templates/secrets.yaml b/charts/virtual-kubelet-for-aks/templates/secrets.yaml new file mode 100644 index 000000000..46252b3e0 --- /dev/null +++ b/charts/virtual-kubelet-for-aks/templates/secrets.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Secret +metadata: + name: {{ template "fullname" . }} +type: Opaque +data: + cert.pem: {{ (default "TUlTU0lORw==" .Values.env.apiserverCert) | quote }} + key.pem: {{ (default "TUlTU0lORw==" .Values.env.apiserverKey) | quote }} + clientSecret: {{ default "" .Values.env.azureClientKey | b64enc | quote }} diff --git a/charts/virtual-kubelet-for-aks/values.yaml b/charts/virtual-kubelet-for-aks/values.yaml new file mode 100644 index 000000000..f24dfa86c --- /dev/null +++ b/charts/virtual-kubelet-for-aks/values.yaml @@ -0,0 +1,16 @@ +image: + repository: microsoft/virtual-kubelet + tag: 0.2-beta-6 + pullPolicy: Always +env: + azureClientId: + azureClientKey: + azureTenantId: + azureSubscriptionId: + aciResourceGroup: + aciRegion: + nodeName: + nodeTaint: + nodeOsType: + apiserverCert: + apiserverKey: diff --git a/providers/azure/aci.go b/providers/azure/aci.go index d0a5bb999..31e9ef0d9 100644 --- a/providers/azure/aci.go +++ b/providers/azure/aci.go @@ -14,6 +14,7 @@ import ( "time" "github.com/virtual-kubelet/virtual-kubelet/manager" + client "github.com/virtual-kubelet/virtual-kubelet/providers/azure/client" "github.com/virtual-kubelet/virtual-kubelet/providers/azure/client/aci" "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" @@ -58,11 +59,6 @@ func NewACIProvider(config string, rm *manager.ResourceManager, nodeName, operat p.resourceManager = rm - p.aciClient, err = aci.NewClient() - if err != nil { - return nil, err - } - if config != "" { f, err := os.Open(config) if err != nil { @@ -75,6 +71,61 @@ func NewACIProvider(config string, rm *manager.ResourceManager, nodeName, operat } } + var azAuth *client.Authentication + + if authFilepath := os.Getenv("AZURE_AUTH_LOCATION"); authFilepath != "" { + auth, err := client.NewAuthenticationFromFile(authFilepath) + if err != nil { + return nil, err + } + + azAuth = auth + } + + if acsFilepath := os.Getenv("ACS_CREDENTIAL_LOCATION"); acsFilepath != "" { + acsCredential, err := NewAcsCredential(acsFilepath) + if err != nil { + return nil, err + } + + if acsCredential != nil { + if acsCredential.Cloud != client.PublicCloud.Name { + return nil, fmt.Errorf("ACI only supports Public Azure. '%v' is not supported", acsCredential.Cloud) + } + + azAuth = client.NewAuthentication( + acsCredential.Cloud, + acsCredential.ClientID, + acsCredential.ClientSecret, + acsCredential.SubscriptionID, + acsCredential.TenantID) + + p.resourceGroup = acsCredential.ResourceGroup + p.region = acsCredential.Region + } + } + + if clientID := os.Getenv("AZURE_CLIENT_ID"); clientID != "" { + azAuth.ClientID = clientID + } + + if clientSecret := os.Getenv("AZURE_CLIENT_SECRET"); clientSecret != "" { + azAuth.ClientSecret = clientSecret + } + + if tenantID := os.Getenv("AZURE_TENANT_ID"); tenantID != "" { + azAuth.TenantID = tenantID + } + + if subscriptionID := os.Getenv("AZURE_SUBSCRIPTION_ID"); subscriptionID != "" { + azAuth.SubscriptionID = subscriptionID + } + + p.aciClient, err = aci.NewClient(azAuth) + if err != nil { + return nil, err + } + if rg := os.Getenv("ACI_RESOURCE_GROUP"); rg != "" { p.resourceGroup = rg } diff --git a/providers/azure/acsCredential.go b/providers/azure/acsCredential.go new file mode 100644 index 000000000..417bbbeca --- /dev/null +++ b/providers/azure/acsCredential.go @@ -0,0 +1,38 @@ +package azure + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "log" +) + +// AcsCredential represents the credential file for ACS +type AcsCredential struct { + Cloud string `json:"cloud"` + TenantID string `json:"tenantId"` + SubscriptionID string `json:"subscriptionId"` + ClientID string `json:"aadClientId"` + ClientSecret string `json:"aadClientSecret"` + ResourceGroup string `json:"resourceGroup"` + Region string `json:"location"` +} + +// NewAcsCredential returns an AcsCredential struct from file path +func NewAcsCredential(filepath string) (*AcsCredential, error) { + log.Printf("Reading ACS credential file %q", filepath) + + b, err := ioutil.ReadFile(filepath) + if err != nil { + return nil, fmt.Errorf("Reading ACS credential file %q failed: %v", filepath, err) + } + + // Unmarshal the authentication file. + var cred AcsCredential + if err := json.Unmarshal(b, &cred); err != nil { + return nil, err + } + + log.Printf("Load ACS credential file %q successfully", filepath) + return &cred, nil +} \ No newline at end of file diff --git a/providers/azure/acsCredential_test.go b/providers/azure/acsCredential_test.go new file mode 100644 index 000000000..f08c4f5a6 --- /dev/null +++ b/providers/azure/acsCredential_test.go @@ -0,0 +1,129 @@ +package azure + +import ( + "io/ioutil" + "os" + "testing" +) + +const cred = ` +{ + "cloud":"AzurePublicCloud", + "tenantId": "72f988bf-86f1-41af-91ab-2d7cd011db47", + "subscriptionId": "11122233-4444-5555-6666-777888999000", + "aadClientId": "123", + "aadClientSecret": "456", + "resourceGroup": "vk-test-rg", + "location": "westcentralus", + "subnetName": "k8s-subnet", + "securityGroupName": "k8s-master-nsg", + "vnetName": "k8s-vnet", + "routeTableName": "k8s-master-routetable", + "primaryAvailabilitySetName": "agentpool1-availabilitySet", + "cloudProviderBackoff": true, + "cloudProviderBackoffRetries": 6, + "cloudProviderBackoffExponent": 1.5, + "cloudProviderBackoffDuration": 5, + "cloudProviderBackoffJitter": 1, + "cloudProviderRatelimit": true, + "cloudProviderRateLimitQPS": 3, + "cloudProviderRateLimitBucket": 10 +}` + +func TestAcsCred(t *testing.T) { + file, err := ioutil.TempFile("", "acs_test") + if err != nil { + t.Error(err) + } + + defer os.Remove(file.Name()) + + if _, err := file.Write([]byte(cred)); err != nil { + t.Error(err) + } + + cred, err := NewAcsCredential(file.Name()) + if err != nil { + t.Error(err) + } + wanted := "AzurePublicCloud" + if cred.Cloud != wanted { + t.Errorf("Wanted %s, got %s.", wanted, cred.Cloud) + } + + wanted = "72f988bf-86f1-41af-91ab-2d7cd011db47" + if cred.TenantID != wanted { + t.Errorf("Wanted %s, got %s.", wanted, cred.TenantID) + } + + wanted = "11122233-4444-5555-6666-777888999000" + if cred.SubscriptionID != wanted { + t.Errorf("Wanted %s, got %s.", wanted, cred.SubscriptionID) + } + wanted = "123" + if cred.ClientID != wanted { + t.Errorf("Wanted %s, got %s.", wanted, cred.ClientID) + } + + wanted = "456" + if cred.ClientSecret != wanted { + t.Errorf("Wanted %s, got %s.", wanted, cred.ClientSecret) + } + + wanted = "vk-test-rg" + if cred.ResourceGroup != wanted { + t.Errorf("Wanted %s, got %s.", wanted, cred.ResourceGroup) + } + + wanted = "westcentralus" + if cred.Region != wanted { + t.Errorf("Wanted %s, got %s.", wanted, cred.Region) + } +} + +func TestAcsCredFileNotFound(t *testing.T) { + file, err := ioutil.TempFile("", "acs_test") + if err != nil { + t.Error(err) + } + + fileName := file.Name() + + if err := file.Close(); err != nil { + t.Error(err) + } + + os.Remove(fileName) + + if _, err := NewAcsCredential(fileName); err == nil { + t.Fatal("expected to fail with bad json") + } +} + +const credBad = ` +{ + "cloud":"AzurePublicCloud", + "tenantId": "72f988bf-86f1-41af-91ab-2d7cd011db47", + "subscriptionId": "11122233-4444-5555-6666-777888999000", + "aadClientId": "123", + "aadClientSecret": "456", + "resourceGroup": "vk-test-rg", + "location": "westcentralus", + "subnetName": "k8s-subnet",` + +func TestAcsCredBadJson(t *testing.T) { + file, err := ioutil.TempFile("", "acs_test") + if err != nil { + t.Error(err) + } + + defer os.Remove(file.Name()) + + if _, err := file.Write([]byte(credBad)); err != nil { + t.Error(err) + } + + if _, err := NewAcsCredential(file.Name()); err == nil { + t.Fatal("expected to fail with bad json") + } +} diff --git a/providers/azure/client/aci/client.go b/providers/azure/client/aci/client.go index 2e192bbd3..e760ed30a 100644 --- a/providers/azure/client/aci/client.go +++ b/providers/azure/client/aci/client.go @@ -29,16 +29,14 @@ type Client struct { } // NewClient creates a new Azure Container Instances client. -func NewClient() (*Client, error) { - // Get authentication credentials from file. - auth, err := azure.NewAuthenticationFromFile() - if err != nil { - return nil, fmt.Errorf("Creating azure authentication from file failed: %v", err) +func NewClient(auth *azure.Authentication) (*Client, error) { + if auth == nil { + return nil, fmt.Errorf("Authentication is not supplied for the Azure client") } client, err := azure.NewClient(auth, BaseURI, userAgent) if err != nil { - return nil, fmt.Errorf("Creating azure client failed: %v", err) + return nil, fmt.Errorf("Creating Azure client failed: %v", err) } return &Client{hc: client.HTTPClient, auth: auth}, nil diff --git a/providers/azure/client/aci/client_test.go b/providers/azure/client/aci/client_test.go index 88aa3f0c0..dc51efd0d 100644 --- a/providers/azure/client/aci/client_test.go +++ b/providers/azure/client/aci/client_test.go @@ -3,11 +3,10 @@ package aci import ( "log" "os" - "path/filepath" - "runtime" "strings" "testing" + azure "github.com/virtual-kubelet/virtual-kubelet/providers/azure/client" "github.com/virtual-kubelet/virtual-kubelet/providers/azure/client/resourcegroups" "github.com/google/uuid" ) @@ -20,23 +19,6 @@ var ( ) func init() { - // Check if the AZURE_AUTH_LOCATION variable is already set. - // If it is not set, set it to the root of this project in a credentials.json file. - if os.Getenv("AZURE_AUTH_LOCATION") == "" { - // Check if the credentials.json file exists in the root of this project. - _, filename, _, _ := runtime.Caller(0) - dir := filepath.Dir(filename) - file := filepath.Join(dir, "../../../../credentials.json") - - // Check if the file exists. - if _, err := os.Stat(file); os.IsNotExist(err) { - log.Fatalf("Either set AZURE_AUTH_LOCATION or add a credentials.json file to the root of this project.") - } - - // Set the environment variable for the authentication file. - os.Setenv("AZURE_AUTH_LOCATION", file) - } - // Create a resource group name with uuid. uid := uuid.New() resourceGroup += "-" + uid.String()[0:6] @@ -45,8 +27,13 @@ func init() { // The TestMain function creates a resource group for testing // and deletes in when it's done. func TestMain(m *testing.M) { + auth, err := azure.NewAuthenticationFromFile("../../../../credentials.json") + if err != nil { + log.Fatalf("Failed to load Azure authentication file: %v", err) + } + // Check if the resource group exists and create it if not. - rgCli, err := resourcegroups.NewClient() + rgCli, err := resourcegroups.NewClient(auth) if err != nil { log.Fatalf("creating new resourcegroups client failed: %v", err) } @@ -84,11 +71,17 @@ func TestMain(m *testing.M) { } func TestNewClient(t *testing.T) { - var err error - client, err = NewClient() + auth, err := azure.NewAuthenticationFromFile("../../../../credentials.json") + if err != nil { + log.Fatalf("Failed to load Azure authentication file: %v", err) + } + + c, err := NewClient(auth) if err != nil { t.Fatal(err) } + + client = c } func TestCreateContainerGroupFails(t *testing.T) { diff --git a/providers/azure/client/auth.go b/providers/azure/client/auth.go index 66a440d69..a5348d8f8 100644 --- a/providers/azure/client/auth.go +++ b/providers/azure/client/auth.go @@ -6,19 +6,11 @@ import ( "encoding/json" "fmt" "io/ioutil" - "os" "unicode/utf16" "github.com/dimchansky/utfbom" ) -const ( - // AuthenticationFilepathName defines the name of the environment variable - // containing the path to the file to be used to populate Authentication - // for Azure. - AuthenticationFilepathName = "AZURE_AUTH_LOCATION" -) - // Authentication represents the authentication file for Azure. type Authentication struct { ClientID string `json:"clientId,omitempty"` @@ -35,32 +27,49 @@ type Authentication struct { // NewAuthentication returns an authentication struct from user provided // credentials. -func NewAuthentication(clientID, clientSecret, subscriptionID, tenantID string) *Authentication { +func NewAuthentication(azureCloud, clientID, clientSecret, subscriptionID, tenantID string) *Authentication { + environment := PublicCloud + + switch azureCloud { + case PublicCloud.Name: + environment = PublicCloud + break; + case USGovernmentCloud.Name: + environment = USGovernmentCloud + break; + case ChinaCloud.Name: + environment = ChinaCloud + break; + case GermanCloud.Name: + environment = GermanCloud + break; + } + return &Authentication{ - ClientID: clientID, - ClientSecret: clientSecret, - SubscriptionID: subscriptionID, - TenantID: tenantID, + ClientID: clientID, + ClientSecret: clientSecret, + SubscriptionID: subscriptionID, + TenantID: tenantID, + ActiveDirectoryEndpoint: environment.ActiveDirectoryEndpoint, + ResourceManagerEndpoint: environment.ResourceManagerEndpoint, + GraphResourceID: environment.GraphEndpoint, + SQLManagementEndpoint: environment.SQLDatabaseDNSSuffix, + GalleryEndpoint: environment.GalleryEndpoint, + ManagementEndpoint: environment.ServiceManagementEndpoint, } } -// NewAuthenticationFromFile returns an authentication struct from file located -// at AZURE_AUTH_LOCATION. -func NewAuthenticationFromFile() (*Authentication, error) { - file := os.Getenv(AuthenticationFilepathName) - if file == "" { - return nil, fmt.Errorf("Authentication file not found, environment variable %s is not set", AuthenticationFilepathName) - } - - b, err := ioutil.ReadFile(file) +// NewAuthenticationFromFile returns an authentication struct from file path +func NewAuthenticationFromFile(filepath string) (*Authentication, error) { + b, err := ioutil.ReadFile(filepath) if err != nil { - return nil, fmt.Errorf("Reading authentication file %q failed: %v", file, err) + return nil, fmt.Errorf("Reading authentication file %q failed: %v", filepath, err) } // Authentication file might be encoded. decoded, err := decode(b) if err != nil { - return nil, fmt.Errorf("Decoding authentication file %q failed: %v", file, err) + return nil, fmt.Errorf("Decoding authentication file %q failed: %v", filepath, err) } // Unmarshal the authentication file. diff --git a/providers/azure/client/resourcegroups/client.go b/providers/azure/client/resourcegroups/client.go index d4dd15aba..a4f626954 100644 --- a/providers/azure/client/resourcegroups/client.go +++ b/providers/azure/client/resourcegroups/client.go @@ -26,16 +26,14 @@ type Client struct { } // NewClient creates a new Azure resource groups client. -func NewClient() (*Client, error) { - // Get authentication credentials from file. - auth, err := azure.NewAuthenticationFromFile() - if err != nil { - return nil, fmt.Errorf("Creating azure authentication from file failed: %v", err) +func NewClient(auth *azure.Authentication) (*Client, error) { + if auth == nil { + return nil, fmt.Errorf("Authentication is not supplied for the Azure client") } client, err := azure.NewClient(auth, BaseURI, userAgent) if err != nil { - return nil, fmt.Errorf("Creating azure client failed: %v", err) + return nil, fmt.Errorf("Creating Azure client failed: %v", err) } return &Client{hc: client.HTTPClient, auth: auth}, nil diff --git a/providers/azure/client/resourcegroups/client_test.go b/providers/azure/client/resourcegroups/client_test.go index 499dff983..6235572cd 100644 --- a/providers/azure/client/resourcegroups/client_test.go +++ b/providers/azure/client/resourcegroups/client_test.go @@ -1,13 +1,10 @@ package resourcegroups import ( - "log" - "os" - "path/filepath" - "runtime" "testing" "github.com/google/uuid" + azure "github.com/virtual-kubelet/virtual-kubelet/providers/azure/client" ) var ( @@ -17,34 +14,23 @@ var ( ) func init() { - // Check if the AZURE_AUTH_LOCATION variable is already set. - // If it is not set, set it to the root of this project in a credentials.json file. - if os.Getenv("AZURE_AUTH_LOCATION") == "" { - // Check if the credentials.json file exists in the root of this project. - _, filename, _, _ := runtime.Caller(0) - dir := filepath.Dir(filename) - file := filepath.Join(dir, "../../../../credentials.json") - - // Check if the file exists. - if _, err := os.Stat(file); os.IsNotExist(err) { - log.Fatalf("Either set AZURE_AUTH_LOCATION or add a credentials.json file to the root of this project.") - } - - // Set the environment variable for the authentication file. - os.Setenv("AZURE_AUTH_LOCATION", file) - } - // Create a resource group name with uuid. uid := uuid.New() resourceGroup += "-" + uid.String()[0:6] } func TestNewClient(t *testing.T) { - var err error - client, err = NewClient() + auth, err := azure.NewAuthenticationFromFile("../../../../credentials.json") + if err != nil { + t.Fatalf("Failed to load Azure authentication file: %v", err) + } + + c, err := NewClient(auth) if err != nil { t.Fatal(err) } + + client = c } func TestResourceGroupDoesNotExist(t *testing.T) {