Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Replace more usages of go-autorest #4775

Merged
merged 1 commit into from
Apr 25, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions azure/errors.go
Original file line number Diff line number Diff line change
@@ -24,23 +24,13 @@ import (
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/go-autorest/autorest"
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
)

// ResourceNotFound parses an error to check if its status code is Not Found (404).
func ResourceNotFound(err error) bool {
return hasStatusCode(err, http.StatusNotFound)
}

// hasStatusCode returns true if an error is a DetailedError or ResponseError with a matching status code.
func hasStatusCode(err error, statusCode int) bool {
derr := autorest.DetailedError{} // azure-sdk-for-go v1
if errors.As(err, &derr) {
return derr.StatusCode == statusCode
}
var rerr *azcore.ResponseError // azure-sdk-for-go v2
return errors.As(err, &rerr) && rerr.StatusCode == statusCode
var rerr *azcore.ResponseError
return errors.As(err, &rerr) && rerr.StatusCode == http.StatusNotFound
}

// VMDeletedError is returned when a virtual machine is deleted outside of capz.
11 changes: 0 additions & 11 deletions azure/errors_test.go
Original file line number Diff line number Diff line change
@@ -23,7 +23,6 @@ import (
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/go-autorest/autorest"
"github.com/pkg/errors"
)

@@ -77,16 +76,6 @@ func TestResourceNotFound(t *testing.T) {
err error
success bool
}{
{
name: "Not Found detailed error",
err: autorest.DetailedError{StatusCode: http.StatusNotFound},
success: true,
},
{
name: "Conflict detailed error",
err: autorest.DetailedError{StatusCode: http.StatusConflict},
success: false,
},
{
name: "Not Found response error",
err: &azcore.ResponseError{StatusCode: http.StatusNotFound},
47 changes: 22 additions & 25 deletions azure/scope/clients.go
Original file line number Diff line number Diff line change
@@ -25,7 +25,6 @@
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/go-autorest/autorest"
azureautorest "github.com/Azure/go-autorest/autorest/azure"
"github.com/Azure/go-autorest/autorest/azure/auth"
)
@@ -34,7 +33,6 @@
type AzureClients struct {
auth.EnvironmentSettings

Authorizer autorest.Authorizer
TokenCredential azcore.TokenCredential
ResourceManagerEndpoint string
ResourceManagerVMDNSSuffix string
@@ -47,23 +45,23 @@

// TenantID returns the Azure tenant id the controller runs in.
func (c *AzureClients) TenantID() string {
return c.Values[auth.TenantID]
return c.Values["AZURE_TENANT_ID"]

Check warning on line 48 in azure/scope/clients.go

Codecov / codecov/patch

azure/scope/clients.go#L48

Added line #L48 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like I should technically point out that constants for these would be good, but since the SDK doesn't even expose these that I can find, I'm not gonna push to do that here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's frustrating that there aren't any constants for these in the SDK. We could define them locally, but I didn't see a good common package to house them at first glance.

}

// ClientID returns the Azure client id from the controller environment.
func (c *AzureClients) ClientID() string {
return c.Values[auth.ClientID]
return c.Values["AZURE_CLIENT_ID"]

Check warning on line 53 in azure/scope/clients.go

Codecov / codecov/patch

azure/scope/clients.go#L53

Added line #L53 was not covered by tests
}

// ClientSecret returns the Azure client secret from the controller environment.
func (c *AzureClients) ClientSecret() string {
return c.Values[auth.ClientSecret]
return c.Values["AZURE_CLIENT_SECRET"]

Check warning on line 58 in azure/scope/clients.go

Codecov / codecov/patch

azure/scope/clients.go#L58

Added line #L58 was not covered by tests
}

// SubscriptionID returns the Azure subscription id of the cluster,
// either specified or from the environment.
func (c *AzureClients) SubscriptionID() string {
return c.Values[auth.SubscriptionID]
return c.Values["AZURE_SUBSCRIPTION_ID"]
}

// Token returns the Azure token credential of the cluster used for SDKv2 services.
@@ -99,47 +97,46 @@
c.EnvironmentSettings = settings
c.ResourceManagerEndpoint = settings.Environment.ResourceManagerEndpoint
c.ResourceManagerVMDNSSuffix = settings.Environment.ResourceManagerVMDNSSuffix
c.Values[auth.SubscriptionID] = strings.TrimSuffix(subscriptionID, "\n")
c.Values[auth.TenantID] = strings.TrimSuffix(credentialsProvider.GetTenantID(), "\n")
c.Values[auth.ClientID] = strings.TrimSuffix(credentialsProvider.GetClientID(), "\n")
c.Values["AZURE_SUBSCRIPTION_ID"] = strings.TrimSuffix(subscriptionID, "\n")
c.Values["AZURE_TENANT_ID"] = strings.TrimSuffix(credentialsProvider.GetTenantID(), "\n")
c.Values["AZURE_CLIENT_ID"] = strings.TrimSuffix(credentialsProvider.GetClientID(), "\n")

clientSecret, err := credentialsProvider.GetClientSecret(ctx)
if err != nil {
return err
}
c.Values[auth.ClientSecret] = strings.TrimSuffix(clientSecret, "\n")
c.Values["AZURE_CLIENT_SECRET"] = strings.TrimSuffix(clientSecret, "\n")

tokenCredential, err := credentialsProvider.GetTokenCredential(ctx, c.ResourceManagerEndpoint, c.Environment.ActiveDirectoryEndpoint, c.Environment.TokenAudience)
if err != nil {
return err
}
c.TokenCredential = tokenCredential
c.Authorizer, err = credentialsProvider.GetAuthorizer(ctx, tokenCredential, c.Environment.TokenAudience)
return err
}

func (c *AzureClients) getSettingsFromEnvironment(environmentName string) (s auth.EnvironmentSettings, err error) {
s = auth.EnvironmentSettings{
Values: map[string]string{},
}
s.Values[auth.EnvironmentName] = environmentName
setValue(s, auth.SubscriptionID)
setValue(s, auth.TenantID)
setValue(s, auth.AuxiliaryTenantIDs)
setValue(s, auth.ClientID)
setValue(s, auth.ClientSecret)
setValue(s, auth.CertificatePath)
setValue(s, auth.CertificatePassword)
setValue(s, auth.Username)
setValue(s, auth.Password)
setValue(s, auth.Resource)
if v := s.Values[auth.EnvironmentName]; v == "" {
s.Values["AZURE_ENVIRONMENT"] = environmentName
setValue(s, "AZURE_SUBSCRIPTION_ID")
setValue(s, "AZURE_TENANT_ID")
setValue(s, "AZURE_AUXILIARY_TENANT_IDS")
setValue(s, "AZURE_CLIENT_ID")
setValue(s, "AZURE_CLIENT_SECRET")
setValue(s, "AZURE_CERTIFICATE_PATH")
setValue(s, "AZURE_CERTIFICATE_PASSWORD")
setValue(s, "AZURE_USERNAME")
setValue(s, "AZURE_PASSWORD")
setValue(s, "AZURE_AD_RESOURCE")
if v := s.Values["AZURE_ENVIRONMENT"]; v == "" {
s.Environment = azureautorest.PublicCloud
} else {
s.Environment, err = azureautorest.EnvironmentFromName(v)
}
if s.Values[auth.Resource] == "" {
s.Values[auth.Resource] = s.Environment.ResourceManagerEndpoint
if s.Values["AZURE_AD_RESOURCE"] == "" {
s.Values["AZURE_AD_RESOURCE"] = s.Environment.ResourceManagerEndpoint
}
return
}
26 changes: 0 additions & 26 deletions azure/scope/identity.go
Original file line number Diff line number Diff line change
@@ -19,13 +19,10 @@ package scope
import (
"context"
"reflect"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/go-autorest/autorest"
"github.com/jongio/azidext/go/azidext"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -40,7 +37,6 @@ const AzureSecretKey = "clientSecret"

// CredentialsProvider defines the behavior for azure identity based credential providers.
type CredentialsProvider interface {
GetAuthorizer(ctx context.Context, tokenCredential azcore.TokenCredential, tokenAudience string) (autorest.Authorizer, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

My dumb grep also shows this which isn't strictly related I don't think, but is also unused AFAICT:

// GetAuthorizer returns an autorest.Authorizer-compatible object from MSAL.
func GetAuthorizer(settings auth.EnvironmentSettings) (autorest.Authorizer, error) {

Copy link
Contributor

Choose a reason for hiding this comment

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

also this?

func getCloudConfig(environment azureautorest.Environment) cloud.Configuration {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your dumb grep is smarter than I was--thanks! Love to remove dead code.

GetClientID() string
GetClientSecret(ctx context.Context) (string, error)
GetTenantID() string
@@ -95,11 +91,6 @@ func NewAzureClusterCredentialsProvider(ctx context.Context, kubeClient client.C
}, nil
}

// GetAuthorizer returns an Azure authorizer based on the provided azure identity. It delegates to AzureCredentialsProvider with AzureCluster metadata.
func (p *AzureClusterCredentialsProvider) GetAuthorizer(ctx context.Context, tokenCredential azcore.TokenCredential, tokenAudience string) (autorest.Authorizer, error) {
return p.AzureCredentialsProvider.GetAuthorizer(ctx, tokenCredential, tokenAudience)
}

// GetTokenCredential returns an Azure TokenCredential based on the provided azure identity.
func (p *AzureClusterCredentialsProvider) GetTokenCredential(ctx context.Context, resourceManagerEndpoint, activeDirectoryEndpoint, tokenAudience string) (azcore.TokenCredential, error) {
return p.AzureCredentialsProvider.GetTokenCredential(ctx, resourceManagerEndpoint, activeDirectoryEndpoint, tokenAudience, p.AzureCluster.ObjectMeta)
@@ -132,11 +123,6 @@ func NewManagedControlPlaneCredentialsProvider(ctx context.Context, kubeClient c
}, nil
}

// GetAuthorizer returns an Azure authorizer based on the provided azure identity. It delegates to AzureCredentialsProvider with AzureManagedControlPlane metadata.
func (p *ManagedControlPlaneCredentialsProvider) GetAuthorizer(ctx context.Context, tokenCredential azcore.TokenCredential, tokenAudience string) (autorest.Authorizer, error) {
return p.AzureCredentialsProvider.GetAuthorizer(ctx, tokenCredential, tokenAudience)
}

// GetTokenCredential returns an Azure TokenCredential based on the provided azure identity.
func (p *ManagedControlPlaneCredentialsProvider) GetTokenCredential(ctx context.Context, resourceManagerEndpoint, activeDirectoryEndpoint, tokenAudience string) (azcore.TokenCredential, error) {
return p.AzureCredentialsProvider.GetTokenCredential(ctx, resourceManagerEndpoint, activeDirectoryEndpoint, tokenAudience, p.AzureManagedControlPlane.ObjectMeta)
@@ -212,18 +198,6 @@ func (p *AzureCredentialsProvider) GetTokenCredential(ctx context.Context, resou
return cred, nil
}

// GetAuthorizer returns an Azure authorizer based on the provided azure identity, cluster metadata, and tokenCredential.
func (p *AzureCredentialsProvider) GetAuthorizer(ctx context.Context, cred azcore.TokenCredential, tokenAudience string) (autorest.Authorizer, error) {
// We must use TokenAudience for StackCloud, otherwise we get an
// AADSTS500011 error from the API
scope := tokenAudience
if !strings.HasSuffix(scope, "/.default") {
scope += "/.default"
}
authorizer := azidext.NewTokenCredentialAdapter(cred, []string{scope})
return authorizer, nil
}

// GetClientID returns the Client ID associated with the AzureCredentialsProvider's Identity.
func (p *AzureCredentialsProvider) GetClientID() string {
return p.Identity.Spec.ClientID
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -25,7 +25,6 @@ require (
github.com/google/uuid v1.6.0
github.com/hashicorp/go-retryablehttp v0.7.5
github.com/hashicorp/golang-lru v1.0.2
github.com/jongio/azidext/go/azidext v0.5.0
github.com/onsi/ginkgo/v2 v2.17.1
github.com/onsi/gomega v1.33.0
github.com/pkg/errors v0.9.1
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
@@ -323,12 +323,8 @@ github.com/jackc/pgx/v5 v5.5.3/go.mod h1:ez9gk+OAat140fv9ErkZDYFWmXLfV+++K0uAOiw
github.com/jackc/puddle/v2 v2.2.1 h1:RhxXJtFG022u4ibrCSMSiu5aOq1i77R3OHKNJj77OAk=
github.com/jackc/puddle/v2 v2.2.1/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4=
github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
github.com/joho/godotenv v1.3.0 h1:Zjp+RcGpHhGlrMbJzXTrZZPrWj+1vfm90La1wgB6Bhc=
github.com/joho/godotenv v1.3.0/go.mod h1:7hK45KPybAkOC6peb+G5yklZfMxEjkZhHbwpqxOKXbg=
github.com/jonboulle/clockwork v0.2.2 h1:UOGuzwb1PwsrDAObMuhUnj0p5ULPj8V/xJ7Kx9qUBdQ=
github.com/jonboulle/clockwork v0.2.2/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8=
github.com/jongio/azidext/go/azidext v0.5.0 h1:uPInXD4NZ3J0k79FPwIA0YXknFn+WcqZqSgs3/jPgvQ=
github.com/jongio/azidext/go/azidext v0.5.0/go.mod h1:TVRX/hJhzbsCKaOIzicH6a8IvOH0hpjWk/JwZZgtXeU=
github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
63 changes: 0 additions & 63 deletions util/azure/azure.go
Original file line number Diff line number Diff line change
@@ -18,18 +18,10 @@ package azure

import (
"context"
"os"
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/Azure/go-autorest/autorest"
azureautorest "github.com/Azure/go-autorest/autorest/azure"
"github.com/Azure/go-autorest/autorest/azure/auth"
"github.com/jongio/azidext/go/azidext"
"github.com/pkg/errors"
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -49,61 +41,6 @@ func IsAzureSystemNodeLabelKey(labelKey string) bool {
return strings.HasPrefix(labelKey, AzureSystemNodeLabelPrefix)
}

func getCloudConfig(environment azureautorest.Environment) cloud.Configuration {
var config cloud.Configuration
switch environment.Name {
case "AzureStackCloud":
config = cloud.Configuration{
ActiveDirectoryAuthorityHost: environment.ActiveDirectoryEndpoint,
Services: map[cloud.ServiceName]cloud.ServiceConfiguration{
cloud.ResourceManager: {
Audience: environment.TokenAudience,
Endpoint: environment.ResourceManagerEndpoint,
},
},
}
case "AzureChinaCloud":
config = cloud.AzureChina
case "AzureUSGovernmentCloud":
config = cloud.AzureGovernment
default:
config = cloud.AzurePublic
}
return config
}

// GetAuthorizer returns an autorest.Authorizer-compatible object from MSAL.
func GetAuthorizer(settings auth.EnvironmentSettings) (autorest.Authorizer, error) {
// azidentity uses different envvars for certificate authentication:
// azidentity: AZURE_CLIENT_CERTIFICATE_{PATH,PASSWORD}
// autorest: AZURE_CERTIFICATE_{PATH,PASSWORD}
// Let's set them according to the envvars used by autorest, in case they are present
_, azidSet := os.LookupEnv("AZURE_CLIENT_CERTIFICATE_PATH")
path, autorestSet := os.LookupEnv("AZURE_CERTIFICATE_PATH")
if !azidSet && autorestSet {
os.Setenv("AZURE_CLIENT_CERTIFICATE_PATH", path)
os.Setenv("AZURE_CLIENT_CERTIFICATE_PASSWORD", os.Getenv("AZURE_CERTIFICATE_PASSWORD"))
}

options := azidentity.DefaultAzureCredentialOptions{
ClientOptions: azcore.ClientOptions{
Cloud: getCloudConfig(settings.Environment),
},
}
cred, err := azidentity.NewDefaultAzureCredential(&options)
if err != nil {
return nil, err
}

// We must use TokenAudience for StackCloud, otherwise we get an
// AADSTS500011 error from the API
scope := settings.Environment.TokenAudience
if !strings.HasSuffix(scope, "/.default") {
scope += "/.default"
}
return azidext.NewTokenCredentialAdapter(cred, []string{scope}), nil
}

// FindParentMachinePool finds the parent MachinePool for the AzureMachinePool.
func FindParentMachinePool(ampName string, cli client.Client) (*expv1.MachinePool, error) {
ctx := context.Background()