From ec57d8a4d0b1260dd207b2a3076db535676a24f3 Mon Sep 17 00:00:00 2001 From: jadarsie Date: Mon, 18 Nov 2019 00:09:36 -0800 Subject: [PATCH 1/5] Support Azure Stack dynamic environments --- .../azure/azure_credentials.go | 2 +- .../azure/auth/azure_auth.go | 48 ++++++++++++++++--- .../legacy-cloud-providers/azure/azure.go | 2 +- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/pkg/credentialprovider/azure/azure_credentials.go b/pkg/credentialprovider/azure/azure_credentials.go index 3dd3c84db5d..f59a4602ec0 100644 --- a/pkg/credentialprovider/azure/azure_credentials.go +++ b/pkg/credentialprovider/azure/azure_credentials.go @@ -144,7 +144,7 @@ func (a *acrProvider) loadConfig(rdr io.Reader) error { klog.Errorf("Failed to load azure credential file: %v", err) } - a.environment, err = auth.ParseAzureEnvironment(a.config.Cloud) + a.environment, err = auth.ParseAzureEnvironment(a.config.Cloud, a.config.cloudFQDN, a.config.IdentitySystem) if err != nil { return err } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go b/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go index 37e55373f1e..26eabc22f13 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go @@ -30,11 +30,14 @@ import ( "k8s.io/klog" ) +const ( + // ADFSIdentitySystem is the override value for tenantID on Azure Stack clouds. + ADFSIdentitySystem = "adfs" +) + var ( // ErrorNoAuth indicates that no credentials are provided. ErrorNoAuth = fmt.Errorf("no credentials provided for Azure cloud provider") - // ADFSIdentitySystem indicates value of tenantId for ADFS on Azure Stack. - ADFSIdentitySystem = "ADFS" ) // AzureAuthConfig holds auth related part of cloud config @@ -59,15 +62,19 @@ type AzureAuthConfig struct { UserAssignedIdentityID string `json:"userAssignedIdentityID,omitempty" yaml:"userAssignedIdentityID,omitempty"` // The ID of the Azure Subscription that the cluster is deployed in SubscriptionID string `json:"subscriptionId,omitempty" yaml:"subscriptionId,omitempty"` - // Identity system value for the deployment. This gets populate for Azure Stack case. - IdentitySystem string `json:"identitySystem,omitempty" yaml:"identitySystem,omitempty"` + // IdentitySystem indicates the identity provider. Relevant only to hybrid clouds (Azure Stack). + // Allowed values are 'azure_ad' (default), 'adfs'. + IdentitySystem string `json:"identitySystem" yaml:"identitySystem"` + // CloudFQDN represents the hybrid cloud's fully qualified domain name: {location}.{domain} + // If set, cloud provider will generate its autorest.Environment instead of using one of the pre-defined ones. + CloudFQDN string `json:"cloudFQDN" yaml:"cloudFQDN"` } // GetServicePrincipalToken creates a new service principal token based on the configuration func GetServicePrincipalToken(config *AzureAuthConfig, env *azure.Environment) (*adal.ServicePrincipalToken, error) { var tenantID string if strings.EqualFold(config.IdentitySystem, ADFSIdentitySystem) { - tenantID = "adfs" + tenantID = ADFSIdentitySystem } else { tenantID = config.TenantID } @@ -126,10 +133,18 @@ func GetServicePrincipalToken(config *AzureAuthConfig, env *azure.Environment) ( } // ParseAzureEnvironment returns azure environment by name -func ParseAzureEnvironment(cloudName string) (*azure.Environment, error) { +func ParseAzureEnvironment(cloudName, cloudFQDN, identitySystem string) (*azure.Environment, error) { var env azure.Environment var err error - if cloudName == "" { + if cloudFQDN != "" { + resourceManagerEndpoint := fmt.Sprintf("https://management.%s/", cloudFQDN) + nameOverride := azure.OverrideProperty{Key: azure.EnvironmentName, Value: cloudName} + klog.V(4).Infof("Loading environment from resource manager endpoint: %s", resourceManagerEndpoint) + env, err = azure.EnvironmentFromURL(resourceManagerEndpoint, nameOverride) + if err == nil && strings.EqualFold(cloudName, "AzureStackCloud") { + azureStackOverrides(env, cloudFQDN, identitySystem) + } + } else if cloudName == "" { env = azure.PublicCloud } else { env, err = azure.EnvironmentFromName(cloudName) @@ -151,3 +166,22 @@ func decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.Private return certificate, rsaPrivateKey, nil } + +func azureStackOverrides(env azure.Environment, cloudFQDN, identitySystem string) azure.Environment { + // if AzureStack, make sure the generated environment matches what AKSe currently generates + env.ManagementPortalURL = fmt.Sprintf("https://portal.%s/", cloudFQDN) + // TODO: figure out why AKSe does this + // why is autorest not setting ServiceManagementEndpoint? + env.ServiceManagementEndpoint = env.TokenAudience + // TODO: figure out why AKSe does this + // ResourceManagerVMDNSSuffix is not referenced in k/k + split := strings.Split(cloudFQDN, ".") + domain := strings.Join(split[1:], ".") + env.ResourceManagerVMDNSSuffix = fmt.Sprintf("cloudapp.%s", domain) + // NOTE: autorest sets KeyVaultEndpoint while AKSe does not + if strings.EqualFold(identitySystem, ADFSIdentitySystem) { + env.ActiveDirectoryEndpoint = strings.TrimSuffix(env.ActiveDirectoryEndpoint, "/") + env.ActiveDirectoryEndpoint = strings.TrimSuffix(env.ActiveDirectoryEndpoint, "adfs") + } + return env +} diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go index 63be11c2199..ae7da436c15 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go @@ -325,7 +325,7 @@ func (az *Cloud) InitializeCloudFromConfig(config *Config, fromSecret bool) erro } } - env, err := auth.ParseAzureEnvironment(config.Cloud) + env, err := auth.ParseAzureEnvironment(config.Cloud, config.CloudFQDN, config.IdentitySystem) if err != nil { return err } From 55828d059dcdf9f0fb90b320b982d71b0162ae9e Mon Sep 17 00:00:00 2001 From: jadarsie Date: Thu, 21 Nov 2019 10:18:34 -0800 Subject: [PATCH 2/5] misc fixes --- pkg/credentialprovider/azure/azure_credentials.go | 2 +- .../legacy-cloud-providers/azure/auth/azure_auth.go | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/credentialprovider/azure/azure_credentials.go b/pkg/credentialprovider/azure/azure_credentials.go index f59a4602ec0..08f293ff882 100644 --- a/pkg/credentialprovider/azure/azure_credentials.go +++ b/pkg/credentialprovider/azure/azure_credentials.go @@ -144,7 +144,7 @@ func (a *acrProvider) loadConfig(rdr io.Reader) error { klog.Errorf("Failed to load azure credential file: %v", err) } - a.environment, err = auth.ParseAzureEnvironment(a.config.Cloud, a.config.cloudFQDN, a.config.IdentitySystem) + a.environment, err = auth.ParseAzureEnvironment(a.config.Cloud, a.config.CloudFQDN, a.config.IdentitySystem) if err != nil { return err } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go b/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go index 26eabc22f13..22dd24b78d2 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go @@ -64,10 +64,10 @@ type AzureAuthConfig struct { SubscriptionID string `json:"subscriptionId,omitempty" yaml:"subscriptionId,omitempty"` // IdentitySystem indicates the identity provider. Relevant only to hybrid clouds (Azure Stack). // Allowed values are 'azure_ad' (default), 'adfs'. - IdentitySystem string `json:"identitySystem" yaml:"identitySystem"` + IdentitySystem string `json:"identitySystem,omitempty" yaml:"identitySystem,omitempty"` // CloudFQDN represents the hybrid cloud's fully qualified domain name: {location}.{domain} // If set, cloud provider will generate its autorest.Environment instead of using one of the pre-defined ones. - CloudFQDN string `json:"cloudFQDN" yaml:"cloudFQDN"` + CloudFQDN string `json:"cloudFQDN,omitempty" yaml:"cloudFQDN,omitempty"` } // GetServicePrincipalToken creates a new service principal token based on the configuration @@ -142,11 +142,13 @@ func ParseAzureEnvironment(cloudName, cloudFQDN, identitySystem string) (*azure. klog.V(4).Infof("Loading environment from resource manager endpoint: %s", resourceManagerEndpoint) env, err = azure.EnvironmentFromURL(resourceManagerEndpoint, nameOverride) if err == nil && strings.EqualFold(cloudName, "AzureStackCloud") { - azureStackOverrides(env, cloudFQDN, identitySystem) + azureStackOverrides(&env, cloudFQDN, identitySystem) } } else if cloudName == "" { + klog.V(4).Info("Using public cloud environment") env = azure.PublicCloud } else { + klog.V(4).Infof("Using %s environment", cloudName) env, err = azure.EnvironmentFromName(cloudName) } return &env, err @@ -167,7 +169,7 @@ func decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.Private return certificate, rsaPrivateKey, nil } -func azureStackOverrides(env azure.Environment, cloudFQDN, identitySystem string) azure.Environment { +func azureStackOverrides(env *azure.Environment, cloudFQDN, identitySystem string) { // if AzureStack, make sure the generated environment matches what AKSe currently generates env.ManagementPortalURL = fmt.Sprintf("https://portal.%s/", cloudFQDN) // TODO: figure out why AKSe does this @@ -183,5 +185,4 @@ func azureStackOverrides(env azure.Environment, cloudFQDN, identitySystem string env.ActiveDirectoryEndpoint = strings.TrimSuffix(env.ActiveDirectoryEndpoint, "/") env.ActiveDirectoryEndpoint = strings.TrimSuffix(env.ActiveDirectoryEndpoint, "adfs") } - return env } From 11b0e9af25139f74c17e4a85b3caccb8451fd040 Mon Sep 17 00:00:00 2001 From: jadarsie Date: Thu, 21 Nov 2019 10:36:04 -0800 Subject: [PATCH 3/5] better comments --- .../legacy-cloud-providers/azure/auth/azure_auth.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go b/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go index 22dd24b78d2..85301801bde 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go @@ -132,7 +132,9 @@ func GetServicePrincipalToken(config *AzureAuthConfig, env *azure.Environment) ( return nil, ErrorNoAuth } -// ParseAzureEnvironment returns azure environment by name +// ParseAzureEnvironment returns the azure environment. +// If 'cloudFQDN' is set, environment is computed by quering the cloud's resource manager endpoint. +// Otherwise, a pre-defined Environment is looked up by name. func ParseAzureEnvironment(cloudName, cloudFQDN, identitySystem string) (*azure.Environment, error) { var env azure.Environment var err error @@ -169,14 +171,14 @@ func decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.Private return certificate, rsaPrivateKey, nil } +// azureStackOverrides ensures that the Environment matches what AKSe currently generates for Azure Stack func azureStackOverrides(env *azure.Environment, cloudFQDN, identitySystem string) { - // if AzureStack, make sure the generated environment matches what AKSe currently generates env.ManagementPortalURL = fmt.Sprintf("https://portal.%s/", cloudFQDN) // TODO: figure out why AKSe does this // why is autorest not setting ServiceManagementEndpoint? env.ServiceManagementEndpoint = env.TokenAudience // TODO: figure out why AKSe does this - // ResourceManagerVMDNSSuffix is not referenced in k/k + // May not be required, ResourceManagerVMDNSSuffix is not used by k/k split := strings.Split(cloudFQDN, ".") domain := strings.Join(split[1:], ".") env.ResourceManagerVMDNSSuffix = fmt.Sprintf("cloudapp.%s", domain) From 3322ff9551e93584f0033bfbf69d71f72c87de2a Mon Sep 17 00:00:00 2001 From: jadarsie Date: Thu, 21 Nov 2019 19:14:47 -0800 Subject: [PATCH 4/5] generalize solution --- .../azure/azure_credentials.go | 2 +- .../azure/auth/azure_auth.go | 36 +++++++++---------- .../legacy-cloud-providers/azure/azure.go | 2 +- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/pkg/credentialprovider/azure/azure_credentials.go b/pkg/credentialprovider/azure/azure_credentials.go index 08f293ff882..1cf5f908d18 100644 --- a/pkg/credentialprovider/azure/azure_credentials.go +++ b/pkg/credentialprovider/azure/azure_credentials.go @@ -144,7 +144,7 @@ func (a *acrProvider) loadConfig(rdr io.Reader) error { klog.Errorf("Failed to load azure credential file: %v", err) } - a.environment, err = auth.ParseAzureEnvironment(a.config.Cloud, a.config.CloudFQDN, a.config.IdentitySystem) + a.environment, err = auth.ParseAzureEnvironment(a.config.Cloud, a.config.ResourceManagerEndpoint, a.config.IdentitySystem) if err != nil { return err } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go b/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go index 85301801bde..1771b731d2f 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go @@ -65,9 +65,9 @@ type AzureAuthConfig struct { // IdentitySystem indicates the identity provider. Relevant only to hybrid clouds (Azure Stack). // Allowed values are 'azure_ad' (default), 'adfs'. IdentitySystem string `json:"identitySystem,omitempty" yaml:"identitySystem,omitempty"` - // CloudFQDN represents the hybrid cloud's fully qualified domain name: {location}.{domain} - // If set, cloud provider will generate its autorest.Environment instead of using one of the pre-defined ones. - CloudFQDN string `json:"cloudFQDN,omitempty" yaml:"cloudFQDN,omitempty"` + // ResourceManagerEndpoint is the cloud's resource manager endpoint. If set, cloud provider queries this endpoint + // in order to generate an autorest.Environment instance instead of using one of the pre-defined Environments. + ResourceManagerEndpoint string `json:"resourceManagerEndpoint,omitempty" yaml:"resourceManagerEndpoint,omitempty"` } // GetServicePrincipalToken creates a new service principal token based on the configuration @@ -133,18 +133,17 @@ func GetServicePrincipalToken(config *AzureAuthConfig, env *azure.Environment) ( } // ParseAzureEnvironment returns the azure environment. -// If 'cloudFQDN' is set, environment is computed by quering the cloud's resource manager endpoint. +// If 'resourceManagerEndpoint' is set, the environment is computed by quering the cloud's resource manager endpoint. // Otherwise, a pre-defined Environment is looked up by name. -func ParseAzureEnvironment(cloudName, cloudFQDN, identitySystem string) (*azure.Environment, error) { +func ParseAzureEnvironment(cloudName, resourceManagerEndpoint, identitySystem string) (*azure.Environment, error) { var env azure.Environment var err error - if cloudFQDN != "" { - resourceManagerEndpoint := fmt.Sprintf("https://management.%s/", cloudFQDN) - nameOverride := azure.OverrideProperty{Key: azure.EnvironmentName, Value: cloudName} + if resourceManagerEndpoint != "" { klog.V(4).Infof("Loading environment from resource manager endpoint: %s", resourceManagerEndpoint) + nameOverride := azure.OverrideProperty{Key: azure.EnvironmentName, Value: cloudName} env, err = azure.EnvironmentFromURL(resourceManagerEndpoint, nameOverride) - if err == nil && strings.EqualFold(cloudName, "AzureStackCloud") { - azureStackOverrides(&env, cloudFQDN, identitySystem) + if err == nil { + azureStackOverrides(&env, resourceManagerEndpoint, identitySystem) } } else if cloudName == "" { klog.V(4).Info("Using public cloud environment") @@ -172,19 +171,16 @@ func decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.Private } // azureStackOverrides ensures that the Environment matches what AKSe currently generates for Azure Stack -func azureStackOverrides(env *azure.Environment, cloudFQDN, identitySystem string) { - env.ManagementPortalURL = fmt.Sprintf("https://portal.%s/", cloudFQDN) - // TODO: figure out why AKSe does this - // why is autorest not setting ServiceManagementEndpoint? +func azureStackOverrides(env *azure.Environment, resourceManagerEndpoint, identitySystem string) { + env.ManagementPortalURL = strings.Replace(resourceManagerEndpoint, "https://management.", "https://portal.", -1) + // TODO: figure out why AKSe does this, why is autorest not setting ServiceManagementEndpoint? env.ServiceManagementEndpoint = env.TokenAudience - // TODO: figure out why AKSe does this - // May not be required, ResourceManagerVMDNSSuffix is not used by k/k - split := strings.Split(cloudFQDN, ".") - domain := strings.Join(split[1:], ".") - env.ResourceManagerVMDNSSuffix = fmt.Sprintf("cloudapp.%s", domain) - // NOTE: autorest sets KeyVaultEndpoint while AKSe does not + // TODO: figure out why AKSe does this, may not be required, ResourceManagerVMDNSSuffix is not referenced anywhere + env.ResourceManagerVMDNSSuffix = strings.Replace(resourceManagerEndpoint, "https://management.", "cloudapp.", -1) + env.ResourceManagerVMDNSSuffix = strings.TrimSuffix(env.ResourceManagerVMDNSSuffix, "/") if strings.EqualFold(identitySystem, ADFSIdentitySystem) { env.ActiveDirectoryEndpoint = strings.TrimSuffix(env.ActiveDirectoryEndpoint, "/") env.ActiveDirectoryEndpoint = strings.TrimSuffix(env.ActiveDirectoryEndpoint, "adfs") } + // NOTE: autorest sets KeyVaultEndpoint while AKSe does not } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go index ae7da436c15..3abb91eaf32 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go @@ -325,7 +325,7 @@ func (az *Cloud) InitializeCloudFromConfig(config *Config, fromSecret bool) erro } } - env, err := auth.ParseAzureEnvironment(config.Cloud, config.CloudFQDN, config.IdentitySystem) + env, err := auth.ParseAzureEnvironment(config.Cloud, config.ResourceManagerEndpoint, config.IdentitySystem) if err != nil { return err } From f96dda6fcfd17c38ef308b549c27335e42528041 Mon Sep 17 00:00:00 2001 From: jadarsie Date: Sat, 23 Nov 2019 11:17:12 -0800 Subject: [PATCH 5/5] removed comments referencing akse --- .../src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go b/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go index 1771b731d2f..7332a1240d5 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/auth/azure_auth.go @@ -173,14 +173,11 @@ func decodePkcs12(pkcs []byte, password string) (*x509.Certificate, *rsa.Private // azureStackOverrides ensures that the Environment matches what AKSe currently generates for Azure Stack func azureStackOverrides(env *azure.Environment, resourceManagerEndpoint, identitySystem string) { env.ManagementPortalURL = strings.Replace(resourceManagerEndpoint, "https://management.", "https://portal.", -1) - // TODO: figure out why AKSe does this, why is autorest not setting ServiceManagementEndpoint? env.ServiceManagementEndpoint = env.TokenAudience - // TODO: figure out why AKSe does this, may not be required, ResourceManagerVMDNSSuffix is not referenced anywhere env.ResourceManagerVMDNSSuffix = strings.Replace(resourceManagerEndpoint, "https://management.", "cloudapp.", -1) env.ResourceManagerVMDNSSuffix = strings.TrimSuffix(env.ResourceManagerVMDNSSuffix, "/") if strings.EqualFold(identitySystem, ADFSIdentitySystem) { env.ActiveDirectoryEndpoint = strings.TrimSuffix(env.ActiveDirectoryEndpoint, "/") env.ActiveDirectoryEndpoint = strings.TrimSuffix(env.ActiveDirectoryEndpoint, "adfs") } - // NOTE: autorest sets KeyVaultEndpoint while AKSe does not }