unit test: fix oidc periodicfunc flaky test (#15320)

* unit test: fix oidc periodicfunc flaky test

* update cycle 1 for two test cases

vault/identity_store_oidc_test.go

@@ -1019,165 +1019,164 @@ func TestOIDC_SignIDToken_NilSigningKey(t *testing.T) {
 	expectStrings(t, []string{err.Error()}, expectedStrings)
 }
 
+func testNamedKey(name string) *namedKey {
+	return &namedKey{
+		name:            name,
+		Algorithm:       "RS256",
+		VerificationTTL: 1 * time.Second,
+		RotationPeriod:  2 * time.Second,
+		KeyRing:         nil,
+		SigningKey:      nil,
+		NextSigningKey:  nil,
+		NextRotation:    time.Now(),
+	}
+}
+
 // TestOIDC_PeriodicFunc tests timing logic for running key
 // rotations and expiration actions.
 func TestOIDC_PeriodicFunc(t *testing.T) {
-	// Prepare a storage to run through periodicFunc
+	type testCase struct {
-	c, _, _ := TestCoreUnsealed(t)
+		cycle         int
-	ctx := namespace.RootContext(nil)
+		numKeys       int
-
+		numPublicKeys int
-	cyclePeriod := 2 * time.Second
+	}
-
 	testSets := []struct {
 		namedKey          *namedKey
 		expectedKeyCount  int
 		setSigningKey     bool
 		setNextSigningKey bool
-		cycles            int
+		testCases         []testCase
 	}{
 		{
-			namedKey: &namedKey{
+			namedKey:          testNamedKey("test-key"),
-				name:            "test-key",
-				Algorithm:       "RS256",
-				VerificationTTL: 1 * cyclePeriod,
-				RotationPeriod:  1 * cyclePeriod,
-				KeyRing:         nil,
-				SigningKey:      nil,
-				NextSigningKey:  nil,
-				NextRotation:    time.Now(),
-			},
-			expectedKeyCount:  3,
 			setSigningKey:     true,
 			setNextSigningKey: true,
-			cycles:            4,
+			testCases: []testCase{
+				{1, 2, 2},
+				{2, 2, 4},
+				{3, 2, 4},
+				{4, 2, 4},
+			},
 		},
 		{
 			// don't set SigningKey to ensure its non-existence can be handled
-			namedKey: &namedKey{
+			namedKey:          testNamedKey("test-key-nil-signing-key"),
-				name:            "test-key-nil-signing-key",
-				Algorithm:       "RS256",
-				VerificationTTL: 1 * cyclePeriod,
-				RotationPeriod:  1 * cyclePeriod,
-				KeyRing:         nil,
-				SigningKey:      nil,
-				NextSigningKey:  nil,
-				NextRotation:    time.Now(),
-			},
-			expectedKeyCount:  2,
 			setSigningKey:     false,
 			setNextSigningKey: true,
-			cycles:            2,
+			testCases: []testCase{
+				{1, 1, 2},
+				{2, 2, 4},
+			},
 		},
 		{
 			// don't set NextSigningKey to ensure its non-existence can be handled
-			namedKey: &namedKey{
+			namedKey:          testNamedKey("test-key-nil-next-signing-key"),
-				name:            "test-key-nil-next-signing-key",
-				Algorithm:       "RS256",
-				VerificationTTL: 1 * cyclePeriod,
-				RotationPeriod:  1 * cyclePeriod,
-				KeyRing:         nil,
-				SigningKey:      nil,
-				NextSigningKey:  nil,
-				NextRotation:    time.Now(),
-			},
-			expectedKeyCount:  2,
 			setSigningKey:     true,
 			setNextSigningKey: false,
-			cycles:            2,
+			testCases: []testCase{
+				{1, 1, 2},
+				{2, 2, 4},
+			},
 		},
 		{
 			// don't set keys to ensure non-existence can be handled
-			namedKey: &namedKey{
+			namedKey:          testNamedKey("test-key-nil-signing-and-next-signing-key"),
-				name:            "test-key-nil-signing-and-next-signing-key",
-				Algorithm:       "RS256",
-				VerificationTTL: 1 * cyclePeriod,
-				RotationPeriod:  1 * cyclePeriod,
-				KeyRing:         nil,
-				SigningKey:      nil,
-				NextSigningKey:  nil,
-				NextRotation:    time.Now(),
-			},
-			expectedKeyCount:  2,
 			setSigningKey:     false,
 			setNextSigningKey: false,
-			cycles:            2,
+			testCases: []testCase{
+				{1, 0, 2},
+				{2, 2, 4},
+			},
 		},
 	}
 
 	for _, testSet := range testSets {
-		storage := c.router.MatchingStorageByAPIPath(ctx, "identity/oidc")
+		testSet := testSet
-		if testSet.setSigningKey {
+		t.Run(testSet.namedKey.name, func(t *testing.T) {
-			if err := testSet.namedKey.generateAndSetKey(ctx, hclog.NewNullLogger(), storage); err != nil {
+			t.Parallel()
-				t.Fatalf("failed to set signing key")
-			}
-		}
-		if testSet.setNextSigningKey {
-			if err := testSet.namedKey.generateAndSetNextKey(ctx, hclog.NewNullLogger(), storage); err != nil {
-				t.Fatalf("failed to set next signing key")
-			}
-		}
-		// Store namedKey
-		entry, _ := logical.StorageEntryJSON(namedKeyConfigPath+testSet.namedKey.name, testSet.namedKey)
-		if err := storage.Put(ctx, entry); err != nil {
-			t.Fatalf("writing to in mem storage failed")
-		}
 
-		currentCycle := 0
+			// Prepare a storage to run through periodicFunc
-		lastCycle := testSet.cycles - 1
+			c, _, _ := TestCoreUnsealed(t)
-		namedKeySamples := make([]*logical.StorageEntry, testSet.cycles)
+			ctx := namespace.RootContext(nil)
-		publicKeysSamples := make([][]string, testSet.cycles)
+			storage := c.router.MatchingStorageByAPIPath(ctx, "identity/oidc")
 
-		i := 0
+			if testSet.setSigningKey {
-		for currentCycle <= lastCycle {
+				if err := testSet.namedKey.generateAndSetKey(ctx, hclog.NewNullLogger(), storage); err != nil {
-			c.identityStore.oidcPeriodicFunc(ctx)
+					t.Fatalf("failed to set signing key")
-			if currentCycle == i {
+				}
-				namedKeyEntry, _ := storage.Get(ctx, namedKeyConfigPath+testSet.namedKey.name)
-				publicKeysEntry, _ := storage.List(ctx, publicKeysConfigPath)
-				namedKeySamples[i] = namedKeyEntry
-				publicKeysSamples[i] = publicKeysEntry
-				i = i + 1
 			}
-			currentCycle = currentCycle + 1
+			if testSet.setNextSigningKey {
+				if err := testSet.namedKey.generateAndSetNextKey(ctx, hclog.NewNullLogger(), storage); err != nil {
+					t.Fatalf("failed to set next signing key")
+				}
+			}
+			testSet.namedKey.NextRotation = time.Now().Add(testSet.namedKey.RotationPeriod)
 
-			// sleep until we are in the next cycle - where a next run will happen
+			// Store namedKey
-			v, _, _ := c.identityStore.oidcCache.Get(noNamespace, "nextRun")
+			entry, _ := logical.StorageEntryJSON(namedKeyConfigPath+testSet.namedKey.name, testSet.namedKey)
-			nextRun := v.(time.Time)
+			if err := storage.Put(ctx, entry); err != nil {
-			now := time.Now()
+				t.Fatalf("writing to in mem storage failed")
-			diff := nextRun.Sub(now)
-			if now.Before(nextRun) {
-				time.Sleep(diff)
 			}
-		}
 
-		// measure collected samples
+			currentCycle := 1
-		for i := 0; i < testSet.cycles; i++ {
+			numCases := len(testSet.testCases)
-			cycle := i + 1
+			lastCycle := testSet.testCases[numCases-1].cycle
-			namedKeySamples[i].DecodeJSON(&testSet.namedKey)
+			namedKeySamples := make([]*logical.StorageEntry, numCases)
-			actualKeyRingLen := len(testSet.namedKey.KeyRing)
+			publicKeysSamples := make([][]string, numCases)
-			if actualKeyRingLen < testSet.expectedKeyCount {
-				t.Errorf(
-					"For key: %s at cycle: %d expected namedKey's KeyRing to be at least of length %d but was: %d",
-					testSet.namedKey.name,
-					cycle,
-					testSet.expectedKeyCount,
-					actualKeyRingLen,
-				)
-			}
-			actualPubKeysLen := len(publicKeysSamples[i])
-			if actualPubKeysLen < testSet.expectedKeyCount {
-				t.Errorf(
-					"For key: %s at cycle: %d expected public keys to be at least of length %d but was: %d",
-					testSet.namedKey.name,
-					cycle,
-					testSet.expectedKeyCount,
-					actualPubKeysLen,
-				)
-			}
-		}
 
-		if err := storage.Delete(ctx, namedKeyConfigPath+testSet.namedKey.name); err != nil {
+			i := 0
-			t.Fatalf("deleting from in mem storage failed")
+			for currentCycle <= lastCycle {
-		}
+				c.identityStore.oidcPeriodicFunc(ctx)
+				if currentCycle == testSet.testCases[i].cycle {
+					namedKeyEntry, _ := storage.Get(ctx, namedKeyConfigPath+testSet.namedKey.name)
+					publicKeysEntry, _ := storage.List(ctx, publicKeysConfigPath)
+					namedKeySamples[i] = namedKeyEntry
+					publicKeysSamples[i] = publicKeysEntry
+					i = i + 1
+				}
+				currentCycle = currentCycle + 1
+
+				// sleep until we are in the next cycle - where a next run will happen
+				v, _, _ := c.identityStore.oidcCache.Get(noNamespace, "nextRun")
+				nextRun := v.(time.Time)
+				now := time.Now()
+				diff := nextRun.Sub(now)
+				if now.Before(nextRun) {
+					time.Sleep(diff)
+				}
+			}
+
+			// measure collected samples
+			for i := range testSet.testCases {
+				expectedKeyCount := testSet.testCases[i].numKeys
+				namedKeySamples[i].DecodeJSON(&testSet.namedKey)
+				actualKeyRingLen := len(testSet.namedKey.KeyRing)
+				if actualKeyRingLen < expectedKeyCount {
+					t.Errorf(
+						"For key: %s at cycle: %d expected namedKey's KeyRing to be at least of length %d but was: %d",
+						testSet.namedKey.name,
+						testSet.testCases[i].cycle,
+						expectedKeyCount,
+						actualKeyRingLen,
+					)
+				}
+				expectedPublicKeyCount := testSet.testCases[i].numPublicKeys
+				actualPubKeysLen := len(publicKeysSamples[i])
+				if actualPubKeysLen < expectedPublicKeyCount {
+					t.Errorf(
+						"For key: %s at cycle: %d expected public keys to be at least of length %d but was: %d",
+						testSet.namedKey.name,
+						testSet.testCases[i].cycle,
+						expectedPublicKeyCount,
+						actualPubKeysLen,
+					)
+				}
+			}
+
+			if err := storage.Delete(ctx, namedKeyConfigPath+testSet.namedKey.name); err != nil {
+				t.Fatalf("deleting from in mem storage failed")
+			}
+		})
 	}
 }