fixup! url: update azure blob fetching to fallback on HTTP fetch on f…

…ailure
coreos · Feb 12, 2025 · 5baea62 · 5baea62
1 parent 380a4b5
commit 5baea62
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 26 deletions.
diff --git a/internal/providers/azure/azure.go b/internal/providers/azure/azure.go
@@ -25,6 +25,7 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
 	"github.com/coreos/ignition/v2/config/shared/errors"
 	"github.com/coreos/ignition/v2/config/v3_6_experimental/types"
 	execUtil "github.com/coreos/ignition/v2/internal/exec/util"
@@ -71,8 +72,9 @@ var (
 
 func init() {
 	platform.Register(platform.Provider{
-		Name:  "azure",
-		Fetch: fetchConfig,
+		Name:       "azure",
+		NewFetcher: newFetcher,
+		Fetch:      fetchConfig,
 	})
 }
 
@@ -81,6 +83,21 @@ func fetchConfig(f *resource.Fetcher) (types.Config, report.Report, error) {
 	return fetchFromAzureMetadata(f)
 }
 
+// newFetcher returns a fetcher that tries to authenticate with Azure's default credential chain.
+func newFetcher(l *log.Logger) (resource.Fetcher, error) {
+	// Read about NewDefaultAzureCredential https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity#DefaultAzureCredential
+	// DefaultAzureCredential is a default credential chain for applications deployed to azure.
+	session, err := azidentity.NewDefaultAzureCredential(nil)
+	if err != nil {
+		return resource.Fetcher{}, err
+	}
+
+	return resource.Fetcher{
+		Logger:    l,
+		AzSession: session,
+	}, nil
+}
+
 // fetchFromAzureMetadata first tries to fetch userData from IMDS then fallback on customData in case
 // of empty config.
 func fetchFromAzureMetadata(f *resource.Fetcher) (types.Config, report.Report, error) {

diff --git a/internal/resource/url.go b/internal/resource/url.go
@@ -100,6 +100,10 @@ type Fetcher struct {
 	// It is used when fetching resources from GCS.
 	GCSSession *storage.Client
 
+	// Azure credential to use when fetching resources from Azure Blob Storage.
+	// using DefaultAzureCredential()
+	AzSession *azidentity.DefaultAzureCredential
+
 	// Whether to only attempt fetches which can be performed offline. This
 	// currently only includes the "data" scheme. Other schemes will result in
 	// ErrNeedNet. In the future, we can improve on this by dropping this
@@ -151,14 +155,15 @@ func (f *Fetcher) FetchToBuffer(u url.URL, opts FetchOptions) ([]byte, error) {
 	dest := new(bytes.Buffer)
 	switch u.Scheme {
 	case "http", "https":
-		if strings.HasSuffix(u.Host, ".blob.core.windows.net") {
+		isAzureBlob := strings.HasSuffix(u.Host, ".blob.core.windows.net")
+		if f.AzSession != nil && isAzureBlob {
 			err = f.fetchFromAzureBlob(u, dest, opts)
 			if err != nil {
-				f.Logger.Info("could not authenticate using azure default credentials: %v", err)
-				f.Logger.Debug("falling back to fetchFromHTTP")
-				err = f.fetchFromHTTP(u, dest, opts)
+				f.Logger.Info("could not obtain file using azclient with az credentials %v", err)
+				f.Logger.Info("falling back to HTTP fetch")
 			}
-		} else {
+		}
+		if !isAzureBlob || err != nil {
 			err = f.fetchFromHTTP(u, dest, opts)
 		}
 	case "tftp":
@@ -218,19 +223,21 @@ func (f *Fetcher) Fetch(u url.URL, dest *os.File, opts FetchOptions) error {
 	if f.Offline && util.UrlNeedsNet(u) {
 		return ErrNeedNet
 	}
-
+	var err error
 	switch u.Scheme {
 	case "http", "https":
-		if strings.HasSuffix(u.Host, ".blob.core.windows.net") {
-			err := f.fetchFromAzureBlob(u, dest, opts)
+		isAzureBlob := strings.HasSuffix(u.Host, ".blob.core.windows.net")
+		if f.AzSession != nil && isAzureBlob {
+			err = f.fetchFromAzureBlob(u, dest, opts)
 			if err != nil {
-				f.Logger.Info("could not authenticate using azure default credentials: %v", err)
-				f.Logger.Debug("falling back to fetchFromHTTP")
-				err = f.fetchFromHTTP(u, dest, opts)
+				f.Logger.Info("could not obtain file using azclient with az credentials %v", err)
+				f.Logger.Info("falling back to HTTP fetch")
 			}
-			return err
 		}
-		return f.fetchFromHTTP(u, dest, opts)
+		if !isAzureBlob || err != nil {
+			err = f.fetchFromHTTP(u, dest, opts)
+		}
+		return err
 	case "tftp":
 		return f.fetchFromTFTP(u, dest, opts)
 	case "data":
@@ -590,16 +597,8 @@ func (f *Fetcher) parseAzureStorageUrl(u url.URL) (string, string, string, error
 }
 
 func (f *Fetcher) fetchFromAzureBlob(u url.URL, dest io.Writer, opts FetchOptions) error {
-	// Read about NewDefaultAzureCredential https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity#DefaultAzureCredential
-	// DefaultAzureCredential is a default credential chain for applications deployed to azure.
-	cred, err := azidentity.NewDefaultAzureCredential(nil)
-	if err != nil {
-		f.Logger.Debug("failed to obtain Azure credential: %v", err)
-		return fmt.Errorf("failed to obtain Azure credential: %w", err)
-	}
-
-	// Create a context with timeout
-	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	// Create a context
+	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
 	storageAccount, container, file, err := f.parseAzureStorageUrl(u)
@@ -608,7 +607,7 @@ func (f *Fetcher) fetchFromAzureBlob(u url.URL, dest io.Writer, opts FetchOption
 	}
 
 	// Create Azure Blob Storage client
-	storageClient, err := azblob.NewClient(storageAccount, cred, nil)
+	storageClient, err := azblob.NewClient(storageAccount, f.AzSession, nil)
 	if err != nil {
 		f.Logger.Debug("failed to create azblob client: %v", err)
 		return fmt.Errorf("failed to create azblob client: %w", err)