Fix flaky TestGetObjectWithOutdatedDisks (#15687)

On occasion this test fails: ``` 2022-09-12T17:22:44.6562737Z === RUN TestGetObjectWithOutdatedDisks 2022-09-12T17:22:44.6563751Z erasure-object_test.go:1214: Test 2: Expected data to have md5sum = `c946b71bb69c07daf25470742c967e7c`, found `7d16d23f07072af1a809707ba101ae07` 2 ``` Theory: Both objects are written with the same timestamp due to lower timer resolution on Windows. This results in secondary resolution, which is deterministic, but random. Solution: Instead of hacking in a wait we request the specific version we want. Should still keep the test relevant. Bonus: Remote action dependency for vulncheck
2024-12-24 06:05:55 -05:00 · 2022-09-14 17:17:39 +02:00 · 2022-09-14 17:17:39 +02:00 · 5c61c3ccdc
commit 5c61c3ccdc
parent fb8d512f58
4 changed files with 33 additions and 10 deletions
--- a/.github/workflows/vulncheck.yml
+++ b/.github/workflows/vulncheck.yml
@ -2,8 +2,10 @@ name: VulnCheck
 on:
  pull_request:
    branches:
-    - master
-
+      - master
+  push:
+    branches:
+      - master
 jobs:
  vulncheck:
    name: Analysis
@ -11,9 +13,14 @@ jobs:
    steps:
      - name: Check out code into the Go module directory
        uses: actions/checkout@v3
-      - uses: actions/setup-go@v3
+      - name: Set up Go
+        uses: actions/setup-go@v3
        with:
          go-version: 1.19.x
          check-latest: true
-      - name: Check for vulnerabilities
-        uses: kmulvey/govulncheck-action@v1.0.0
+      - name: Get official govulncheck
+        run: go install golang.org/x/vuln/cmd/govulncheck@latest
+        shell: bash
+      - name: Run govulncheck
+        run: govulncheck ./...
+        shell: bash
--- a/cmd/admin-handlers-users_test.go
+++ b/cmd/admin-handlers-users_test.go
@ -34,7 +34,7 @@ import (
 	"time"

 	"github.com/minio/madmin-go"
-	minio "github.com/minio/minio-go/v7"
+	"github.com/minio/minio-go/v7"
 	"github.com/minio/minio-go/v7/pkg/credentials"
 	cr "github.com/minio/minio-go/v7/pkg/credentials"
 	"github.com/minio/minio-go/v7/pkg/s3utils"
@ -1141,6 +1141,7 @@ func (s *TestSuiteIAM) TestAccMgmtPlugin(c *check) {
 }

 func (c *check) mustCreateIAMUser(ctx context.Context, admClnt *madmin.AdminClient) madmin.Credentials {
+	c.Helper()
 	randUser := mustGetUUID()
 	randPass := mustGetUUID()
 	err := admClnt.AddUser(ctx, randUser, randPass)
@ -1154,6 +1155,7 @@ func (c *check) mustCreateIAMUser(ctx context.Context, admClnt *madmin.AdminClie
 }

 func (c *check) mustGetIAMUserInfo(ctx context.Context, admClnt *madmin.AdminClient, accessKey string) madmin.UserInfo {
+	c.Helper()
 	ui, err := admClnt.GetUserInfo(ctx, accessKey)
 	if err != nil {
 		c.Fatalf("should be able to get user info: %v", err)
@ -1162,6 +1164,7 @@ func (c *check) mustGetIAMUserInfo(ctx context.Context, admClnt *madmin.AdminCli
 }

 func (c *check) mustNotCreateIAMUser(ctx context.Context, admClnt *madmin.AdminClient) {
+	c.Helper()
 	randUser := mustGetUUID()
 	randPass := mustGetUUID()
 	err := admClnt.AddUser(ctx, randUser, randPass)
@ -1171,6 +1174,7 @@ func (c *check) mustNotCreateIAMUser(ctx context.Context, admClnt *madmin.AdminC
 }

 func (c *check) mustCreateSvcAccount(ctx context.Context, tgtUser string, admClnt *madmin.AdminClient) madmin.Credentials {
+	c.Helper()
 	cr, err := admClnt.AddServiceAccount(ctx, madmin.AddServiceAccountReq{
 		TargetUser: tgtUser,
 	})
@ -1181,6 +1185,7 @@ func (c *check) mustCreateSvcAccount(ctx context.Context, tgtUser string, admCln
 }

 func (c *check) mustNotCreateSvcAccount(ctx context.Context, tgtUser string, admClnt *madmin.AdminClient) {
+	c.Helper()
 	_, err := admClnt.AddServiceAccount(ctx, madmin.AddServiceAccountReq{
 		TargetUser: tgtUser,
 	})
@ -1190,6 +1195,7 @@ func (c *check) mustNotCreateSvcAccount(ctx context.Context, tgtUser string, adm
 }

 func (c *check) mustNotListObjects(ctx context.Context, client *minio.Client, bucket string) {
+	c.Helper()
 	res := client.ListObjects(ctx, bucket, minio.ListObjectsOptions{})
 	v, ok := <-res
 	if !ok || v.Err == nil {
@ -1198,6 +1204,7 @@ func (c *check) mustNotListObjects(ctx context.Context, client *minio.Client, bu
 }

 func (c *check) mustListObjects(ctx context.Context, client *minio.Client, bucket string) {
+	c.Helper()
 	res := client.ListObjects(ctx, bucket, minio.ListObjectsOptions{})
 	v, ok := <-res
 	if ok && v.Err != nil {
@ -1206,6 +1213,7 @@ func (c *check) mustListObjects(ctx context.Context, client *minio.Client, bucke
 }

 func (c *check) mustListBuckets(ctx context.Context, client *minio.Client) {
+	c.Helper()
 	_, err := client.ListBuckets(ctx)
 	if err != nil {
 		c.Fatalf("user was unable to list buckets: %v", err)
@ -1213,6 +1221,7 @@ func (c *check) mustListBuckets(ctx context.Context, client *minio.Client) {
 }

 func (c *check) mustNotUpload(ctx context.Context, client *minio.Client, bucket string) {
+	c.Helper()
 	_, err := client.PutObject(ctx, bucket, "some-object", bytes.NewBuffer([]byte("stuff")), 5, minio.PutObjectOptions{})
 	if e, ok := err.(minio.ErrorResponse); ok {
 		if e.Code == "AccessDenied" {
@ -1228,6 +1237,7 @@ func (c *check) assertSvcAccS3Access(ctx context.Context, s *TestSuiteIAM, cr ma
 }

 func (c *check) assertSvcAccAppearsInListing(ctx context.Context, madmClient *madmin.AdminClient, parentAK, svcAK string) {
+	c.Helper()
 	listResp, err := madmClient.ListServiceAccounts(ctx, parentAK)
 	if err != nil {
 		c.Fatalf("unable to list svc accounts: %v", err)
@ -1253,6 +1263,7 @@ func (c *check) assertSvcAccInfoQueryable(ctx context.Context, madmClient *madmi
 // bucket. It creates a session policy that restricts listing on the bucket and
 // then enables it again in a session policy update call.
 func (c *check) assertSvcAccSessionPolicyUpdate(ctx context.Context, s *TestSuiteIAM, madmClient *madmin.AdminClient, accessKey, bucket string) {
+	c.Helper()
 	svcAK, svcSK := mustGenerateCredentials(c)

 	// This policy does not allow listing objects.
@ -1308,6 +1319,7 @@ func (c *check) assertSvcAccSessionPolicyUpdate(ctx context.Context, s *TestSuit
 }

 func (c *check) assertSvcAccSecretKeyAndStatusUpdate(ctx context.Context, s *TestSuiteIAM, madmClient *madmin.AdminClient, accessKey, bucket string) {
+	c.Helper()
 	svcAK, svcSK := mustGenerateCredentials(c)
 	cr, err := madmClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{
 		TargetUser: accessKey,
@ -1344,6 +1356,7 @@ func (c *check) assertSvcAccSecretKeyAndStatusUpdate(ctx context.Context, s *Tes
 }

 func (c *check) assertSvcAccDeletion(ctx context.Context, s *TestSuiteIAM, madmClient *madmin.AdminClient, accessKey, bucket string) {
+	c.Helper()
 	svcAK, svcSK := mustGenerateCredentials(c)
 	cr, err := madmClient.AddServiceAccount(ctx, madmin.AddServiceAccountReq{
 		TargetUser: accessKey,
@ -1364,6 +1377,7 @@ func (c *check) assertSvcAccDeletion(ctx context.Context, s *TestSuiteIAM, madmC
 }

 func mustGenerateCredentials(c *check) (string, string) {
+	c.Helper()
 	ak, sk, err := auth.GenerateCredentials()
 	if err != nil {
 		c.Fatalf("unable to generate credentials: %v", err)
--- a/cmd/data-update-tracker_test.go
+++ b/cmd/data-update-tracker_test.go
@ -200,11 +200,13 @@ func TestDataUpdateTracker(t *testing.T) {
 	}

 	ctx, cancel = context.WithCancel(context.Background())
-	defer cancel()
-
 	// Reload...
 	dut = newDataUpdateTracker()
 	dut.start(ctx, tmpDir)
+	defer func() {
+		cancel()
+		<-dut.saveExited
+	}()

 	if dut.current() != 2 {
 		t.Fatal("current idx after load not preserved. want 2, got:", dut.current())
--- a/cmd/erasure-object_test.go
+++ b/cmd/erasure-object_test.go
@ -1183,7 +1183,7 @@ func TestGetObjectWithOutdatedDisks(t *testing.T) {
 			return disks
 		}
 		sets.erasureDisksMu.Unlock()
-		_, err = z.PutObject(ctx, testCase.bucket, testCase.object, mustGetPutObjReader(t, bytes.NewReader(testCase.content), int64(len(testCase.content)), "", ""),
+		got, err := z.PutObject(ctx, testCase.bucket, testCase.object, mustGetPutObjReader(t, bytes.NewReader(testCase.content), int64(len(testCase.content)), "", ""),
 			ObjectOptions{Versioned: testCase.versioned})
 		if err != nil {
 			t.Fatalf("Test %d: Failed to upload the final object: %v", i+1, err)
@ -1193,7 +1193,7 @@ func TestGetObjectWithOutdatedDisks(t *testing.T) {
 		sets.erasureDisksMu.Lock()
 		xl.getDisks = func() []StorageAPI { return origErasureDisks }
 		sets.erasureDisksMu.Unlock()
-		gr, err := z.GetObjectNInfo(ctx, testCase.bucket, testCase.object, nil, nil, readLock, ObjectOptions{})
+		gr, err := z.GetObjectNInfo(ctx, testCase.bucket, testCase.object, nil, nil, readLock, ObjectOptions{VersionID: got.VersionID})
 		if err != nil {
 			t.Fatalf("Expected GetObject to succeed, but failed with %v", err)
 		}