From af6e643746a72d9e4998a16d07d30304b9940bcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 12 Mar 2025 20:59:35 +0100 Subject: [PATCH 1/3] fix: skip nil strategies in wait.ForAll --- wait/all.go | 7 +++++++ wait/all_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/wait/all.go b/wait/all.go index fb7eb4e5f3..d372d87c80 100644 --- a/wait/all.go +++ b/wait/all.go @@ -62,6 +62,13 @@ func (ms *MultiStrategy) WaitUntilReady(ctx context.Context, target StrategyTarg } for _, strategy := range ms.Strategies { + if strategy == nil { + // A module could be appending strategies after part of the container initialization, + // and use wait.ForAll on a not initialized strategy. + // In this case, we just skip the nil strategy. + continue + } + strategyCtx := ctx // Set default Timeout when strategy implements StrategyTimeout diff --git a/wait/all_test.go b/wait/all_test.go index f49d0cbd66..c69e672249 100644 --- a/wait/all_test.go +++ b/wait/all_test.go @@ -124,3 +124,27 @@ func TestMultiStrategy_WaitUntilReady(t *testing.T) { }) } } + +func TestMultiStrategy_handleNils(t *testing.T) { + t.Run("nil-strategy", func(t *testing.T) { + strategy := ForAll(nil) + err := strategy.WaitUntilReady(context.Background(), NopStrategyTarget{}) + require.NoError(t, err) + }) + + t.Run("nil-strategy-in-the-middle", func(t *testing.T) { + strategy := ForAll(nil, ForLog("docker")) + err := strategy.WaitUntilReady(context.Background(), NopStrategyTarget{ + ReaderCloser: io.NopCloser(bytes.NewReader([]byte("docker"))), + }) + require.NoError(t, err) + }) + + t.Run("nil-strategy-last", func(t *testing.T) { + strategy := ForAll(ForLog("docker"), nil) + err := strategy.WaitUntilReady(context.Background(), NopStrategyTarget{ + ReaderCloser: io.NopCloser(bytes.NewReader([]byte("docker"))), + }) + require.NoError(t, err) + }) +} From da13fce536c09dd0b698e804f4ac0cda3f97ee3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Fri, 28 Mar 2025 11:54:50 +0100 Subject: [PATCH 2/3] fix: handle nil strategies correctly --- wait/all.go | 3 ++- wait/all_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/wait/all.go b/wait/all.go index d372d87c80..9bf4cbe8b2 100644 --- a/wait/all.go +++ b/wait/all.go @@ -3,6 +3,7 @@ package wait import ( "context" "errors" + "reflect" "time" ) @@ -62,7 +63,7 @@ func (ms *MultiStrategy) WaitUntilReady(ctx context.Context, target StrategyTarg } for _, strategy := range ms.Strategies { - if strategy == nil { + if strategy == nil || reflect.ValueOf(strategy).IsNil() { // A module could be appending strategies after part of the container initialization, // and use wait.ForAll on a not initialized strategy. // In this case, we just skip the nil strategy. diff --git a/wait/all_test.go b/wait/all_test.go index c69e672249..9daf9a151f 100644 --- a/wait/all_test.go +++ b/wait/all_test.go @@ -147,4 +147,34 @@ func TestMultiStrategy_handleNils(t *testing.T) { }) require.NoError(t, err) }) + + t.Run("nil-type-implements-strategy", func(t *testing.T) { + var nilStrategy Strategy = nil + + strategy := ForAll(ForLog("docker"), nilStrategy) + err := strategy.WaitUntilReady(context.Background(), NopStrategyTarget{ + ReaderCloser: io.NopCloser(bytes.NewReader([]byte("docker"))), + }) + require.NoError(t, err) + }) + + t.Run("nil-concrete-value-implements-strategy", func(t *testing.T) { + // Create a nil pointer to a type that implements Strategy + var nilPointerStrategy *nilWaitStrategy + // When we assign it to the interface, the type information is preserved + // but the concrete value is nil + var strategyInterface Strategy = nilPointerStrategy + + strategy := ForAll(ForLog("docker"), strategyInterface) + err := strategy.WaitUntilReady(context.Background(), NopStrategyTarget{ + ReaderCloser: io.NopCloser(bytes.NewReader([]byte("docker"))), + }) + require.NoError(t, err) + }) +} + +type nilWaitStrategy struct{} + +func (s *nilWaitStrategy) WaitUntilReady(ctx context.Context, target StrategyTarget) error { + return nil } From 586315d2bba0e929c8e253df14fe7cd3e0be9e21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Fri, 28 Mar 2025 11:58:38 +0100 Subject: [PATCH 3/3] fix: lint --- wait/all_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wait/all_test.go b/wait/all_test.go index 9daf9a151f..4a8d019f24 100644 --- a/wait/all_test.go +++ b/wait/all_test.go @@ -149,7 +149,7 @@ func TestMultiStrategy_handleNils(t *testing.T) { }) t.Run("nil-type-implements-strategy", func(t *testing.T) { - var nilStrategy Strategy = nil + var nilStrategy Strategy strategy := ForAll(ForLog("docker"), nilStrategy) err := strategy.WaitUntilReady(context.Background(), NopStrategyTarget{ @@ -175,6 +175,6 @@ func TestMultiStrategy_handleNils(t *testing.T) { type nilWaitStrategy struct{} -func (s *nilWaitStrategy) WaitUntilReady(ctx context.Context, target StrategyTarget) error { +func (s *nilWaitStrategy) WaitUntilReady(_ context.Context, _ StrategyTarget) error { return nil }