Update azure priv fetch #2012

prestist · 2025-02-05T19:21:35Z

This is a followup to the PR #1923

Add fallback logic, and add documentation.

cc: #2008

internal/resource/url.go

jlebon · 2025-02-07T14:32:41Z

internal/resource/url.go

@@ -153,6 +153,11 @@ func (f *Fetcher) FetchToBuffer(u url.URL, opts FetchOptions) ([]byte, error) {
 	case "http", "https":
 		if strings.HasSuffix(u.Host, ".blob.core.windows.net") {


Really, I think we should gate this behaviour on actually being on Azure. Otherwise it will be guaranteed to fail everywhere else. E.g. we can set an option on the fetcher only when on Azure.

ie, look at the platfrom.id? that makes sense I will add that.

or is it more reasonable to have the azure provider call directly the fetch method for azure blob, taking the responsibility of fall back on to http fetch?

The provider is passed to Ignition via the --platform flag which then determines the Provider to use. So you could use provider.Name() == "azure" in the code higher up to tell if you're on Azure.

But actually, looking at this now. We should probably just follow the same structure that we use for AWS. The main code calls NewFetcher() , which gives a chance for the provider to give a fetcher with more information in it. The AWS provider uses this to inject session credentials if found. We should do the same here for Azure.

A major optimization BTW of setting a fetcher option up front is that we don't have to re-obtain credentials on each blob fetch.

Ok, yeah that makes sense, not bad; I will go ahead and follow that pattern. Thank you for the info!

Alright, we are mirroring aws more, and the session is stored in the fetcher object. should be good? lmk

ignore the history sadly. (I tried to go through and make changes purposefully and stumbled on code duplication and missing spots, so looking through some of the force pushes is painful, sorry about that).

internal/resource/url.go

docs/operator-notes.md

prestist · 2025-02-12T17:15:40Z

Ok, I updated the commit with all changes minus the re-work one; I am working on the re-work but wanted to make sure I did not regress on the addressed comments; Expect a followup push.

jlebon · 2025-02-13T03:08:33Z

internal/providers/azure/azure.go

+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)


Is this always expected to succeed even if the user doesn't e.g. explicitly attach credentials to the VM?

No I dont think so, it goes through and attempts a chain of authentication types. After it goes through each one, if they all return errors, it then appends those errors and returns them with a nil cred.

Thats why if we dont get an error its going to include a credential. Which is great! but obviously the crediential might not have access to the object that is trying to be fetched. (i.e why we still fall back to http fetch even if a session is there)

what am I missing?

I'm not sure if this answers my question. Regardless of whether the credentials have access to a specific blob or not, is it always the case that there is a set of credentials that the VM will be able to obtain via the call here? Or does even that require an additional setup?

Ah sorry. To be clear no, credentials will not be returned if they don't exist.

More detail:
As an admin you would have to configure the environment to have a managed identity. The function checks a chain of locations to see if credentials exist, but if none are present you will not get a credential returned. (from my understanding).

Their documentation is confusing, mostly because of the number of points credentials could exist.

In that case, instead of hard erroring, we should just log the error here and not bubble it up.

internal/resource/url.go

jlebon · 2025-02-13T04:09:04Z

internal/resource/url.go

+		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 obtain file using azclient with az credentials %v", err)
+				f.Logger.Info("falling back to HTTP fetch")
+			}
+		}
+		if !isAzureBlob || err != nil {
+			err = f.fetchFromHTTP(u, dest, opts)
 		}
-		return f.fetchFromHTTP(u, dest, opts)
+		return err


Since in this case we can directly return from the case blocks, we could simplify the logic here I think. But OK too to structure it the same way as above.

I would prefer them to follow a similar flow so its clearer at a glance. Obviously not strongly apposed, but was my initial inclination to keep them the same.

jlebon · 2025-02-13T04:14:33Z

docs/operator-notes.md

+Ignition supports fetching resources from Azure Blob Storage. The URL format for Azure Blob Storage is `https://<storageAccount>.blob.core.windows.net/<container>/<fileName>`. Ignition will recognize this format and attempt to authenticate using the [default Azure credential](https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity#DefaultAzureCredential) to fetch the resource via the [Azure Blob Storage API](https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/storage/azblob#section-readme).
+
+If no credentials are found, Ignition will fall back to doing a normal HTTP fetch, which may be sufficient if the blob is public. For private blobs, ensure the environment has credentials with the necessary permissions to access the storage account and storage blob. One approach is to configure a managed identity with contributor access to the storage account and assign it to the VM during creation.


I think this needs to be reworked a bit given how the code now works.

Maybe something like

When Ignition is run in Azure, it will authenticate using the default Azure credential. These credentials are then used to fetch resources from Azure Blob Storage. The URL format ...

If fetching fails, Ignition will fall back to doing a normal HTTP fetch, ...

fixes:coreos#2011

prestist mentioned this pull request Feb 5, 2025

operator-notes: add section for azure blob access #2010

Closed

prestist requested review from jlebon and travier February 5, 2025 19:25

prestist force-pushed the update-azure-priv-fetch branch from b46a576 to e312ee8 Compare February 5, 2025 19:52

prestist mentioned this pull request Feb 6, 2025

Support fetching from protected assets with Azure Storage #2011

Open

jlebon reviewed Feb 7, 2025

View reviewed changes

prestist force-pushed the update-azure-priv-fetch branch from e312ee8 to 435a35d Compare February 12, 2025 17:14

prestist force-pushed the update-azure-priv-fetch branch 3 times, most recently from 5baea62 to 6a8da1a Compare February 12, 2025 18:41

jlebon reviewed Feb 13, 2025

View reviewed changes

prestist added 2 commits February 13, 2025 11:15

url: update azure blob fetching to fallback on HTTP fetch on failure

6256606

fixes:coreos#2011

operator-notes: add section for azure blob access

f826c55

fixes:coreos#2011

prestist force-pushed the update-azure-priv-fetch branch from 6a8da1a to f826c55 Compare February 13, 2025 16:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update azure priv fetch #2012

Update azure priv fetch #2012

prestist commented Feb 5, 2025 •

edited

Loading

jlebon Feb 7, 2025

prestist Feb 11, 2025

prestist Feb 11, 2025 •

edited

Loading

jlebon Feb 11, 2025

prestist Feb 11, 2025

prestist Feb 12, 2025

prestist commented Feb 12, 2025

jlebon Feb 13, 2025

prestist Feb 13, 2025

jlebon Feb 14, 2025

prestist Feb 14, 2025

jlebon Feb 14, 2025

jlebon Feb 13, 2025

prestist Feb 13, 2025

jlebon Feb 13, 2025

		@@ -153,6 +153,11 @@ func (f *Fetcher) FetchToBuffer(u url.URL, opts FetchOptions) ([]byte, error) {
		case "http", "https":
		if strings.HasSuffix(u.Host, ".blob.core.windows.net") {

		Ignition supports fetching resources from Azure Blob Storage. The URL format for Azure Blob Storage is `https://<storageAccount>.blob.core.windows.net/<container>/<fileName>`. Ignition will recognize this format and attempt to authenticate using the [default Azure credential](https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity#DefaultAzureCredential) to fetch the resource via the [Azure Blob Storage API](https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/storage/azblob#section-readme).

		If no credentials are found, Ignition will fall back to doing a normal HTTP fetch, which may be sufficient if the blob is public. For private blobs, ensure the environment has credentials with the necessary permissions to access the storage account and storage blob. One approach is to configure a managed identity with contributor access to the storage account and assign it to the VM during creation.

Update azure priv fetch #2012

Are you sure you want to change the base?

Update azure priv fetch #2012

Conversation

prestist commented Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prestist Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prestist commented Feb 12, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prestist commented Feb 5, 2025 •

edited

Loading

prestist Feb 11, 2025 •

edited

Loading