From b4e72814be6b8771aa887d03a64727b6aa6f6d06 Mon Sep 17 00:00:00 2001 From: Tao Li Date: Wed, 22 Jan 2025 15:52:10 +0800 Subject: [PATCH 1/4] feat: fork to omit '--credentials' command for azure-blob-storage credentials Signed-off-by: Tao Li --- pkg/command/commandbuilder.go | 20 ++++++++++++++++++++ pkg/command/commandbuilder_test.go | 22 ++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/pkg/command/commandbuilder.go b/pkg/command/commandbuilder.go index 7b1452b0..5ab4b144 100644 --- a/pkg/command/commandbuilder.go +++ b/pkg/command/commandbuilder.go @@ -115,6 +115,10 @@ func appendCloudProviderOptions( break } + if checkUseDefaultAzureCredentials(ctx) { + break + } + if !capabilities.HasAzureManagedIdentity { err := fmt.Errorf( "barman >= 2.18 is required to use azureInheritFromAzureAD, current: %v", @@ -143,3 +147,19 @@ func appendCloudProviderOptions( return options, nil } + +type contextKey string + +const useDefaultAzureCredentials contextKey = "useDefaultAzureCredentials" + +func checkUseDefaultAzureCredentials(ctx context.Context) bool { + v := ctx.Value(useDefaultAzureCredentials) + if v == nil { + return false + } + result, ok := v.(bool) + if !ok { + return false + } + return result +} diff --git a/pkg/command/commandbuilder_test.go b/pkg/command/commandbuilder_test.go index 00d661c0..a1037e1f 100644 --- a/pkg/command/commandbuilder_test.go +++ b/pkg/command/commandbuilder_test.go @@ -17,6 +17,7 @@ limitations under the License. package command import ( + "context" "strings" barmanApi "github.com/cloudnative-pg/barman-cloud/pkg/api" @@ -57,3 +58,24 @@ var _ = Describe("barmanCloudWalRestoreOptions", func() { )) }) }) + +var _ = Describe("checkUseDefaultAzureCredentials", func() { + It("checkUseDefaultAzureCredentials should be false by default", func(ctx SpecContext) { + Expect(checkUseDefaultAzureCredentials(ctx)).To(BeFalse()) + }) + + It("checkUseDefaultAzureCredentials should be false if ctx contains invalid value", func(ctx SpecContext) { + newCtx := context.WithValue(ctx, useDefaultAzureCredentials, "invalidValue") + Expect(checkUseDefaultAzureCredentials(newCtx)).To(BeFalse()) + }) + + It("checkUseDefaultAzureCredentials should be false if ctx contains false value", func(ctx SpecContext) { + newCtx := context.WithValue(ctx, useDefaultAzureCredentials, false) + Expect(checkUseDefaultAzureCredentials(newCtx)).To(BeFalse()) + }) + + It("checkUseDefaultAzureCredentials should be true only if ctx contains true value", func(ctx SpecContext) { + newCtx := context.WithValue(ctx, useDefaultAzureCredentials, true) + Expect(checkUseDefaultAzureCredentials(newCtx)).To(BeTrue()) + }) +}) From b4ad0224c78cd51dc80681aad835a2d5c0f230ef Mon Sep 17 00:00:00 2001 From: Tao Li Date: Fri, 24 Jan 2025 11:14:18 +0800 Subject: [PATCH 2/4] chore: review ensure the unique of the key Signed-off-by: Tao Li --- pkg/command/commandbuilder.go | 12 ++++++++++-- pkg/command/commandbuilder_test.go | 8 ++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/pkg/command/commandbuilder.go b/pkg/command/commandbuilder.go index 5ab4b144..d0c63ac3 100644 --- a/pkg/command/commandbuilder.go +++ b/pkg/command/commandbuilder.go @@ -115,7 +115,7 @@ func appendCloudProviderOptions( break } - if checkUseDefaultAzureCredentials(ctx) { + if CheckUseDefaultAzureCredentials(ctx) { break } @@ -150,9 +150,12 @@ func appendCloudProviderOptions( type contextKey string +// useDefaultAzureCredentials context key holding the flag if to use DefaultAzureCredentials +// for azure-blob-storage const useDefaultAzureCredentials contextKey = "useDefaultAzureCredentials" -func checkUseDefaultAzureCredentials(ctx context.Context) bool { +// CheckUseDefaultAzureCredentials return true if useDefaultAzureCredentials is set +func CheckUseDefaultAzureCredentials(ctx context.Context) bool { v := ctx.Value(useDefaultAzureCredentials) if v == nil { return false @@ -163,3 +166,8 @@ func checkUseDefaultAzureCredentials(ctx context.Context) bool { } return result } + +// NewContextWithUseDefaultAzureCredentials create a context with useDefaultAzureCredentials set +func NewContextWithUseDefaultAzureCredentials(ctx context.Context) context.Context { + return context.WithValue(ctx, useDefaultAzureCredentials, true) +} diff --git a/pkg/command/commandbuilder_test.go b/pkg/command/commandbuilder_test.go index a1037e1f..cf15fce0 100644 --- a/pkg/command/commandbuilder_test.go +++ b/pkg/command/commandbuilder_test.go @@ -61,21 +61,21 @@ var _ = Describe("barmanCloudWalRestoreOptions", func() { var _ = Describe("checkUseDefaultAzureCredentials", func() { It("checkUseDefaultAzureCredentials should be false by default", func(ctx SpecContext) { - Expect(checkUseDefaultAzureCredentials(ctx)).To(BeFalse()) + Expect(CheckUseDefaultAzureCredentials(ctx)).To(BeFalse()) }) It("checkUseDefaultAzureCredentials should be false if ctx contains invalid value", func(ctx SpecContext) { newCtx := context.WithValue(ctx, useDefaultAzureCredentials, "invalidValue") - Expect(checkUseDefaultAzureCredentials(newCtx)).To(BeFalse()) + Expect(CheckUseDefaultAzureCredentials(newCtx)).To(BeFalse()) }) It("checkUseDefaultAzureCredentials should be false if ctx contains false value", func(ctx SpecContext) { newCtx := context.WithValue(ctx, useDefaultAzureCredentials, false) - Expect(checkUseDefaultAzureCredentials(newCtx)).To(BeFalse()) + Expect(CheckUseDefaultAzureCredentials(newCtx)).To(BeFalse()) }) It("checkUseDefaultAzureCredentials should be true only if ctx contains true value", func(ctx SpecContext) { newCtx := context.WithValue(ctx, useDefaultAzureCredentials, true) - Expect(checkUseDefaultAzureCredentials(newCtx)).To(BeTrue()) + Expect(CheckUseDefaultAzureCredentials(newCtx)).To(BeTrue()) }) }) From 24eaa72dec822b353ba07ad896db55d7091722ac Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Fri, 24 Jan 2025 16:36:43 +0100 Subject: [PATCH 3/4] chore: review Signed-off-by: Armando Ruocco --- pkg/command/commandbuilder.go | 19 +++++++++---------- pkg/command/commandbuilder_test.go | 18 +++++++++--------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/pkg/command/commandbuilder.go b/pkg/command/commandbuilder.go index d0c63ac3..a3c29ca4 100644 --- a/pkg/command/commandbuilder.go +++ b/pkg/command/commandbuilder.go @@ -111,11 +111,11 @@ func appendCloudProviderOptions( "--cloud-provider", "azure-blob-storage") - if !credentials.Azure.InheritFromAzureAD { + if useDefaultAzureCredentials(ctx) { break } - if CheckUseDefaultAzureCredentials(ctx) { + if !credentials.Azure.InheritFromAzureAD { break } @@ -150,12 +150,10 @@ func appendCloudProviderOptions( type contextKey string -// useDefaultAzureCredentials context key holding the flag if to use DefaultAzureCredentials -// for azure-blob-storage -const useDefaultAzureCredentials contextKey = "useDefaultAzureCredentials" +// contextKeyUseDefaultAzureCredentials contains a bool indicating if the default azure credentials should be used +const contextKeyUseDefaultAzureCredentials contextKey = "useDefaultAzureCredentials" -// CheckUseDefaultAzureCredentials return true if useDefaultAzureCredentials is set -func CheckUseDefaultAzureCredentials(ctx context.Context) bool { +func useDefaultAzureCredentials(ctx context.Context) bool { v := ctx.Value(useDefaultAzureCredentials) if v == nil { return false @@ -167,7 +165,8 @@ func CheckUseDefaultAzureCredentials(ctx context.Context) bool { return result } -// NewContextWithUseDefaultAzureCredentials create a context with useDefaultAzureCredentials set -func NewContextWithUseDefaultAzureCredentials(ctx context.Context) context.Context { - return context.WithValue(ctx, useDefaultAzureCredentials, true) +// ContextWithDefaultAzureCredentials create a context that contains the contextKeyUseDefaultAzureCredentials flag. +// When set to true barman-cloud will use the default Azure credentials. +func ContextWithDefaultAzureCredentials(ctx context.Context, enabled bool) context.Context { + return context.WithValue(ctx, contextKeyUseDefaultAzureCredentials, enabled) } diff --git a/pkg/command/commandbuilder_test.go b/pkg/command/commandbuilder_test.go index cf15fce0..a2da8b04 100644 --- a/pkg/command/commandbuilder_test.go +++ b/pkg/command/commandbuilder_test.go @@ -59,23 +59,23 @@ var _ = Describe("barmanCloudWalRestoreOptions", func() { }) }) -var _ = Describe("checkUseDefaultAzureCredentials", func() { - It("checkUseDefaultAzureCredentials should be false by default", func(ctx SpecContext) { - Expect(CheckUseDefaultAzureCredentials(ctx)).To(BeFalse()) +var _ = Describe("useDefaultAzureCredentials", func() { + It("should be false by default", func(ctx SpecContext) { + Expect(useDefaultAzureCredentials(ctx)).To(BeFalse()) }) - It("checkUseDefaultAzureCredentials should be false if ctx contains invalid value", func(ctx SpecContext) { + It("should be false if ctx contains an invalid value", func(ctx SpecContext) { newCtx := context.WithValue(ctx, useDefaultAzureCredentials, "invalidValue") - Expect(CheckUseDefaultAzureCredentials(newCtx)).To(BeFalse()) + Expect(useDefaultAzureCredentials(newCtx)).To(BeFalse()) }) - It("checkUseDefaultAzureCredentials should be false if ctx contains false value", func(ctx SpecContext) { + It("should be false if ctx contains false value", func(ctx SpecContext) { newCtx := context.WithValue(ctx, useDefaultAzureCredentials, false) - Expect(CheckUseDefaultAzureCredentials(newCtx)).To(BeFalse()) + Expect(useDefaultAzureCredentials(newCtx)).To(BeFalse()) }) - It("checkUseDefaultAzureCredentials should be true only if ctx contains true value", func(ctx SpecContext) { + It("should be true only if ctx contains true value", func(ctx SpecContext) { newCtx := context.WithValue(ctx, useDefaultAzureCredentials, true) - Expect(CheckUseDefaultAzureCredentials(newCtx)).To(BeTrue()) + Expect(useDefaultAzureCredentials(newCtx)).To(BeTrue()) }) }) From 4e9ab101ddecbbff4e9d7a904e74331a605429e9 Mon Sep 17 00:00:00 2001 From: Tao Li Date: Wed, 5 Feb 2025 15:08:28 +0800 Subject: [PATCH 4/4] chore: review correct Signed-off-by: Tao Li --- pkg/command/commandbuilder.go | 2 +- pkg/command/commandbuilder_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/command/commandbuilder.go b/pkg/command/commandbuilder.go index a3c29ca4..ea02c12e 100644 --- a/pkg/command/commandbuilder.go +++ b/pkg/command/commandbuilder.go @@ -154,7 +154,7 @@ type contextKey string const contextKeyUseDefaultAzureCredentials contextKey = "useDefaultAzureCredentials" func useDefaultAzureCredentials(ctx context.Context) bool { - v := ctx.Value(useDefaultAzureCredentials) + v := ctx.Value(contextKeyUseDefaultAzureCredentials) if v == nil { return false } diff --git a/pkg/command/commandbuilder_test.go b/pkg/command/commandbuilder_test.go index a2da8b04..9f7e16e4 100644 --- a/pkg/command/commandbuilder_test.go +++ b/pkg/command/commandbuilder_test.go @@ -65,17 +65,17 @@ var _ = Describe("useDefaultAzureCredentials", func() { }) It("should be false if ctx contains an invalid value", func(ctx SpecContext) { - newCtx := context.WithValue(ctx, useDefaultAzureCredentials, "invalidValue") + newCtx := context.WithValue(ctx, contextKeyUseDefaultAzureCredentials, "invalidValue") Expect(useDefaultAzureCredentials(newCtx)).To(BeFalse()) }) It("should be false if ctx contains false value", func(ctx SpecContext) { - newCtx := context.WithValue(ctx, useDefaultAzureCredentials, false) + newCtx := context.WithValue(ctx, contextKeyUseDefaultAzureCredentials, false) Expect(useDefaultAzureCredentials(newCtx)).To(BeFalse()) }) It("should be true only if ctx contains true value", func(ctx SpecContext) { - newCtx := context.WithValue(ctx, useDefaultAzureCredentials, true) + newCtx := context.WithValue(ctx, contextKeyUseDefaultAzureCredentials, true) Expect(useDefaultAzureCredentials(newCtx)).To(BeTrue()) }) })