From e98a86b3d63e250c33571a44df61821fb864aab4 Mon Sep 17 00:00:00 2001 From: Rens Groothuijsen Date: Fri, 13 Dec 2024 10:29:21 +0100 Subject: [PATCH 1/2] fix(azureblob): Return error if Azure returns no service principal token (#13195) This adds an extra nil check to verify the Azure blob storage client has received a service principal token before trying to do anything with said token. This has led to crashes if `use_federated_token is enabled` and an error message is returned instead of a token. --- .../chunk/client/azure/blob_storage_client.go | 4 ++++ .../client/azure/blob_storage_client_test.go | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/pkg/storage/chunk/client/azure/blob_storage_client.go b/pkg/storage/chunk/client/azure/blob_storage_client.go index 86b59bc6f420b..b4025e4089311 100644 --- a/pkg/storage/chunk/client/azure/blob_storage_client.go +++ b/pkg/storage/chunk/client/azure/blob_storage_client.go @@ -474,6 +474,10 @@ func (b *BlobStorage) getServicePrincipalToken(authFunctions authFunctions) (*ad if b.cfg.UseFederatedToken { token, err := b.servicePrincipalTokenFromFederatedToken(resource, authFunctions.NewOAuthConfigFunc, authFunctions.NewServicePrincipalTokenFromFederatedTokenFunc) + if err != nil { + return nil, err + } + var customRefreshFunc adal.TokenRefresh = func(_ context.Context, resource string) (*adal.Token, error) { newToken, err := b.servicePrincipalTokenFromFederatedToken(resource, authFunctions.NewOAuthConfigFunc, authFunctions.NewServicePrincipalTokenFromFederatedTokenFunc) if err != nil { diff --git a/pkg/storage/chunk/client/azure/blob_storage_client_test.go b/pkg/storage/chunk/client/azure/blob_storage_client_test.go index 8a3362d4a7c7d..2fefb6c795bf9 100644 --- a/pkg/storage/chunk/client/azure/blob_storage_client_test.go +++ b/pkg/storage/chunk/client/azure/blob_storage_client_test.go @@ -10,6 +10,8 @@ import ( "testing" "time" + "github.com/pkg/errors" + "github.com/Azure/go-autorest/autorest/adal" "github.com/Azure/go-autorest/autorest/azure" "github.com/grafana/dskit/flagext" @@ -80,6 +82,28 @@ func (suite *FederatedTokenTestSuite) TestGetServicePrincipalToken() { require.True(suite.T(), suite.mockedServicePrincipalToken == token, "should return the mocked object") } +func (suite *FederatedTokenTestSuite) Test_HandleNoServicePrincipalToken() { + newOAuthConfigFunc := func(activeDirectoryEndpoint, tenantID string) (*adal.OAuthConfig, error) { + require.Equal(suite.T(), azure.PublicCloud.ActiveDirectoryEndpoint, activeDirectoryEndpoint) + require.Equal(suite.T(), "myTenantId", tenantID) + + _, err := adal.NewOAuthConfig(activeDirectoryEndpoint, tenantID) + require.NoError(suite.T(), err) + + return suite.mockOAuthConfig, nil + } + + servicePrincipalTokenFromFederatedTokenFunc := func(_ adal.OAuthConfig, _ string, _ string, _ string, _ ...adal.TokenRefreshCallback) (*adal.ServicePrincipalToken, error) { + return nil, errors.New("No token") + } + + token, err := suite.config.getServicePrincipalToken(authFunctions{newOAuthConfigFunc, servicePrincipalTokenFromFederatedTokenFunc}) + + require.Error(suite.T(), err) + require.EqualError(suite.T(), err, "No token") + require.True(suite.T(), token == nil, "should return error if no token was retrieved") +} + func Test_Hedging(t *testing.T) { for _, tc := range []struct { name string From db2b6dbfed67d051bf15458945c29daca771897e Mon Sep 17 00:00:00 2001 From: Christian Haudum Date: Fri, 13 Dec 2024 11:36:50 +0100 Subject: [PATCH 2/2] fix(ci): Fix test compile error (#15404) Signed-off-by: Christian Haudum --- go.mod | 2 +- pkg/distributor/distributor_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 85ed5b458171c..5aca3328513b9 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/grafana/loki/v3 go 1.23.0 -toolchain go1.23.4 +toolchain go1.23.1 require ( cloud.google.com/go/bigtable v1.33.0 diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go index b438780547ca0..310aa52df7708 100644 --- a/pkg/distributor/distributor_test.go +++ b/pkg/distributor/distributor_test.go @@ -648,7 +648,7 @@ func Test_DiscardEmptyStreamsAfterValidation(t *testing.T) { limits, ingester := setup() distributors, _ := prepare(t, 1, 5, limits, func(_ string) (ring_client.PoolClient, error) { return ingester, nil }) - _, err := distributors[0].Push(ctx, makeWriteRequestWithLabels(1, 1, []string{})) + _, err := distributors[0].Push(ctx, makeWriteRequestWithLabels(1, 1, []string{}, false, false, false)) require.Equal(t, err, httpgrpc.Errorf(http.StatusUnprocessableEntity, validation.MissingStreamsErrorMsg)) topVal := ingester.Peek() require.Nil(t, topVal)