From d0d7be9dc2700fcac6826dd543eb4c0626b794dd Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Thu, 10 Apr 2025 15:39:36 +0200 Subject: [PATCH 1/4] Check `consultopo` loaded >0 credentials Signed-off-by: Tim Vaillancourt --- go/vt/topo/consultopo/server.go | 5 ++ go/vt/topo/consultopo/server_flaky_test.go | 60 ++++++++++++++++------ 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/go/vt/topo/consultopo/server.go b/go/vt/topo/consultopo/server.go index 0a059b12f65..afa3e67e8b8 100644 --- a/go/vt/topo/consultopo/server.go +++ b/go/vt/topo/consultopo/server.go @@ -31,6 +31,7 @@ import ( "github.com/spf13/pflag" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/servenv" "vitess.io/vitess/go/vt/topo" "vitess.io/vitess/go/vt/vterrors" @@ -104,6 +105,10 @@ func getClientCreds() (creds map[string]*ClientAuthCred, err error) { err = vterrors.Wrapf(err, "Error parsing consul_auth_static_file") return creds, err } + if len(creds) == 0 { + err = vterrors.New(vtrpc.Code_FAILED_PRECONDITION, "Found no credentials in consul_auth_static_file") + return creds, err + } return creds, nil } diff --git a/go/vt/topo/consultopo/server_flaky_test.go b/go/vt/topo/consultopo/server_flaky_test.go index a987336dd01..d6177f68ec2 100644 --- a/go/vt/topo/consultopo/server_flaky_test.go +++ b/go/vt/topo/consultopo/server_flaky_test.go @@ -297,25 +297,53 @@ func TestConsulTopoWithAuthFailure(t *testing.T) { consulAuthClientStaticFile = tmpFile.Name() - jsonConfig := "{\"global\":{\"acl_token\":\"badtoken\"}}" - if err := os.WriteFile(tmpFile.Name(), []byte(jsonConfig), 0600); err != nil { - t.Fatalf("couldn't write temp file: %v", err) - } + // check valid, empty json causes error + { + jsonConfig := "{}" + if err := os.WriteFile(tmpFile.Name(), []byte(jsonConfig), 0600); err != nil { + t.Fatalf("couldn't write temp file: %v", err) + } - // Create the server on the new root. - ts, err := topo.OpenServer("consul", serverAddr, path.Join("globalRoot", topo.GlobalCell)) - if err != nil { - t.Fatalf("OpenServer() failed: %v", err) + // Create the server on the new root. + ts, err := topo.OpenServer("consul", serverAddr, path.Join("globalRoot", topo.GlobalCell)) + if err != nil { + t.Fatalf("OpenServer() failed: %v", err) + } + + // Attempt to Create the CellInfo. + err = ts.CreateCellInfo(context.Background(), test.LocalCellName, &topodatapb.CellInfo{ + ServerAddress: serverAddr, + Root: path.Join("globalRoot", test.LocalCellName), + }) + + want := "Failed request: ACL not found" + if err == nil || err.Error() != want { + t.Errorf("Expected CreateCellInfo to fail: got %v, want %s", err, want) + } } - // Attempt to Create the CellInfo. - err = ts.CreateCellInfo(context.Background(), test.LocalCellName, &topodatapb.CellInfo{ - ServerAddress: serverAddr, - Root: path.Join("globalRoot", test.LocalCellName), - }) + // check bad token causes error + { + jsonConfig := "{\"global\":{\"acl_token\":\"badtoken\"}}" + if err := os.WriteFile(tmpFile.Name(), []byte(jsonConfig), 0600); err != nil { + t.Fatalf("couldn't write temp file: %v", err) + } - want := "Failed request: ACL not found" - if err == nil || err.Error() != want { - t.Errorf("Expected CreateCellInfo to fail: got %v, want %s", err, want) + // Create the server on the new root. + ts, err := topo.OpenServer("consul", serverAddr, path.Join("globalRoot", topo.GlobalCell)) + if err != nil { + t.Fatalf("OpenServer() failed: %v", err) + } + + // Attempt to Create the CellInfo. + err = ts.CreateCellInfo(context.Background(), test.LocalCellName, &topodatapb.CellInfo{ + ServerAddress: serverAddr, + Root: path.Join("globalRoot", test.LocalCellName), + }) + + want := "Failed request: ACL not found" + if err == nil || err.Error() != want { + t.Errorf("Expected CreateCellInfo to fail: got %v, want %s", err, want) + } } } From e3206736c0bc3954f22cec8635ef479bec9909cc Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Thu, 10 Apr 2025 17:01:56 +0200 Subject: [PATCH 2/4] fix test Signed-off-by: Tim Vaillancourt --- go/vt/topo/consultopo/server_flaky_test.go | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/go/vt/topo/consultopo/server_flaky_test.go b/go/vt/topo/consultopo/server_flaky_test.go index d6177f68ec2..f89a2851aa0 100644 --- a/go/vt/topo/consultopo/server_flaky_test.go +++ b/go/vt/topo/consultopo/server_flaky_test.go @@ -26,11 +26,10 @@ import ( "testing" "time" - "vitess.io/vitess/go/vt/log" - "github.com/hashicorp/consul/api" "vitess.io/vitess/go/testfiles" + "vitess.io/vitess/go/vt/log" "vitess.io/vitess/go/vt/topo" "vitess.io/vitess/go/vt/topo/test" @@ -306,19 +305,8 @@ func TestConsulTopoWithAuthFailure(t *testing.T) { // Create the server on the new root. ts, err := topo.OpenServer("consul", serverAddr, path.Join("globalRoot", topo.GlobalCell)) - if err != nil { - t.Fatalf("OpenServer() failed: %v", err) - } - - // Attempt to Create the CellInfo. - err = ts.CreateCellInfo(context.Background(), test.LocalCellName, &topodatapb.CellInfo{ - ServerAddress: serverAddr, - Root: path.Join("globalRoot", test.LocalCellName), - }) - - want := "Failed request: ACL not found" - if err == nil || err.Error() != want { - t.Errorf("Expected CreateCellInfo to fail: got %v, want %s", err, want) + if err == nil { + t.Fatal("Expected OpenServer() to return an error due to bad config, got nil") } } From d7a2bc20a469cd0c4323f20bfe66c220fa32791c Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Thu, 10 Apr 2025 17:19:52 +0200 Subject: [PATCH 3/4] fix test, again Signed-off-by: Tim Vaillancourt --- go/vt/topo/consultopo/server_flaky_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/topo/consultopo/server_flaky_test.go b/go/vt/topo/consultopo/server_flaky_test.go index f89a2851aa0..3a3a6ad3205 100644 --- a/go/vt/topo/consultopo/server_flaky_test.go +++ b/go/vt/topo/consultopo/server_flaky_test.go @@ -304,7 +304,7 @@ func TestConsulTopoWithAuthFailure(t *testing.T) { } // Create the server on the new root. - ts, err := topo.OpenServer("consul", serverAddr, path.Join("globalRoot", topo.GlobalCell)) + _, err := topo.OpenServer("consul", serverAddr, path.Join("globalRoot", topo.GlobalCell)) if err == nil { t.Fatal("Expected OpenServer() to return an error due to bad config, got nil") } From 751d4dd4a0a3dd8e82bc9d0f2168ec951ff718db Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 11 Apr 2025 00:29:56 +0200 Subject: [PATCH 4/4] force CI to run again Signed-off-by: Tim Vaillancourt