Always write migration entry to trigger secondary clusters to wake up

 - Some PR feedback and handle a case in which the primary cluster does
   not have a CA bundle within storage but somehow a secondary does.

builtin/logical/pki/cert_util.go

@@ -89,11 +89,22 @@ func getFormat(data *framework.FieldData) string {
 	return format
 }
 
-// Fetches the CA info.
+// fetchCAInfo will fetch the CA info, will return an error if no ca info exists.
 func fetchCAInfo(ctx context.Context, b *backend, req *logical.Request, issuerRef string) (*certutil.CAInfoBundle, error) {
 	bundle, err := fetchCertBundle(ctx, b, req.Storage, issuerRef)
 	if err != nil {
-		return nil, errutil.InternalError{Err: err.Error()}
+		switch err.(type) {
+		case errutil.UserError:
+			return nil, err
+		case errutil.InternalError:
+			return nil, err
+		default:
+			return nil, errutil.InternalError{Err: fmt.Sprintf("error fetching CA info: %v", err)}
+		}
+	}
+
+	if bundle == nil {
+		return nil, errutil.UserError{Err: "no CA information is present"}
 	}
 
 	parsedBundle, err := parseCABundle(ctx, b, req, bundle)
@@ -130,6 +141,7 @@ func fetchCAInfo(ctx context.Context, b *backend, req *logical.Request, issuerRe
 // performed or load the bundle from the new key/issuer storage. Any function that needs a bundle
 // should load it using this method to maintain compatibility on secondary nodes for which their
 // primary's have not upgraded yet.
+// NOTE: This function can return a nil, nil response.
 func fetchCertBundle(ctx context.Context, b *backend, s logical.Storage, issuerRef string) (*certutil.CertBundle, error) {
 	if b.useLegacyBundleCaStorage() {
 		// We have not completed the migration so attempt to load the bundle from the legacy location
@@ -139,7 +151,7 @@ func fetchCertBundle(ctx context.Context, b *backend, s logical.Storage, issuerR
 	id, err := resolveIssuerReference(ctx, s, issuerRef)
 	if err != nil {
 		// Usually a bad label from the user or misconfigured default.
-		return nil, err
+		return nil, errutil.UserError{Err: err.Error()}
 	}
 
 	return fetchCertBundleByIssuerId(ctx, s, id, true)

builtin/logical/pki/path_intermediate.go

@@ -48,7 +48,7 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req
 	var err error
 
 	if b.useLegacyBundleCaStorage() {
-		return logical.ErrorResponse("Can not create intermediary until migration has completed"), nil
+		return logical.ErrorResponse("Can not create intermediate until migration has completed"), nil
 	}
 
 	exported, format, role, errorResp := b.getGenerationParams(ctx, data, req.MountPoint)

builtin/logical/pki/storage_migrations.go

@@ -34,18 +34,15 @@ func getMigrationInfo(ctx context.Context, s logical.Storage) (migrationInfo, er
 		isRequired:       false,
 		legacyBundle:     nil,
 		legacyBundleHash: "",
+		migrationLog:     nil,
 	}
-	var err error
 
+	var err error
 	migrationInfo.legacyBundle, err = getLegacyCertBundle(ctx, s)
 	if err != nil {
 		return migrationInfo, err
 	}
 
-	if migrationInfo.legacyBundle == nil {
-		return migrationInfo, nil
-	}
-
 	migrationInfo.migrationLog, err = getLegacyBundleMigrationLog(ctx, s)
 	if err != nil {
 		return migrationInfo, err
@@ -56,15 +53,14 @@ func getMigrationInfo(ctx context.Context, s logical.Storage) (migrationInfo, er
 		return migrationInfo, err
 	}
 
-	if migrationInfo.migrationLog != nil {
-		// At this point we have already migrated something previously.
-		if migrationInfo.migrationLog.Hash == migrationInfo.legacyBundleHash &&
-			migrationInfo.migrationLog.MigrationVersion == latestMigrationVersion {
-			return migrationInfo, nil
-		}
+	// Even if there isn't anything to migrate, we always want to write out the log entry
+	// as that will trigger the secondary clusters to toggle/wake up
+	if (migrationInfo.migrationLog == nil) ||
+		(migrationInfo.migrationLog.Hash != migrationInfo.legacyBundleHash) ||
+		(migrationInfo.migrationLog.MigrationVersion != latestMigrationVersion) {
+		migrationInfo.isRequired = true
 	}
 
-	migrationInfo.isRequired = true
 	return migrationInfo, nil
 }
 
@@ -76,26 +72,25 @@ func migrateStorage(ctx context.Context, req *logical.InitializationRequest, log
 	}
 
 	if !migrationInfo.isRequired {
-		if migrationInfo.legacyBundle == nil {
-			// No legacy certs to migrate, we are done...
-			logger.Debug("No legacy certs found, no migration required.")
-		}
-		if migrationInfo.migrationLog != nil {
-			// The hashes are the same, no need to try and re-import...
-			logger.Debug("existing migration hash found and matched legacy bundle, skipping migration.")
-		}
+		// No migration was deemed to be required.
+		logger.Debug("existing migration found and was considered valid, skipping migration.")
 		return nil
 	}
 
 	logger.Info("performing PKI migration to new keys/issuers layout")
-
-	anIssuer, aKey, err := writeCaBundle(ctx, s, migrationInfo.legacyBundle, "current", "current")
-	if err != nil {
-		return err
+	if migrationInfo.legacyBundle != nil {
+		anIssuer, aKey, err := writeCaBundle(ctx, s, migrationInfo.legacyBundle, "current", "current")
+		if err != nil {
+			return err
+		}
+		logger.Debug("Migration generated the following ids and set them as defaults",
+			"issuer id", anIssuer.ID, "key id", aKey.ID)
+	} else {
+		logger.Debug("No legacy CA certs found, no migration required.")
 	}
-	logger.Debug("Migration generated the following ids and set them as defaults",
-		"issuer id", anIssuer.ID, "key id", aKey.ID)
 
+	// We always want to write out this log entry as the secondary clusters leverage this path to wake up
+	// if they were upgraded prior to the primary cluster's migration occurred.
 	err = setLegacyBundleMigrationLog(ctx, s, &legacyBundleMigrationLog{
 		Hash:             migrationInfo.legacyBundleHash,
 		Created:          time.Now(),
@@ -104,21 +99,25 @@ func migrateStorage(ctx context.Context, req *logical.InitializationRequest, log
 	if err != nil {
 		return err
 	}
+
 	logger.Info("successfully completed migration to new keys/issuers layout")
 	return nil
 }
 
 func computeHashOfLegacyBundle(bundle *certutil.CertBundle) (string, error) {
-	// We only hash the main certificate and the certs within the CAChain,
-	// assuming that any sort of change that occurred would have influenced one of those two fields.
 	hasher := sha256.New()
-	if _, err := hasher.Write([]byte(bundle.Certificate)); err != nil {
-		return "", err
-	}
-	for _, cert := range bundle.CAChain {
-		if _, err := hasher.Write([]byte(cert)); err != nil {
+	// Generate an empty hash if the bundle does not exist.
+	if bundle != nil {
+		// We only hash the main certificate and the certs within the CAChain,
+		// assuming that any sort of change that occurred would have influenced one of those two fields.
+		if _, err := hasher.Write([]byte(bundle.Certificate)); err != nil {
 			return "", err
 		}
+		for _, cert := range bundle.CAChain {
+			if _, err := hasher.Write([]byte(cert)); err != nil {
+				return "", err
+			}
+		}
 	}
 	return hex.EncodeToString(hasher.Sum(nil)), nil
 }

builtin/logical/pki/storage_migrations_test.go

@@ -11,11 +11,16 @@ import (
 )
 
 func Test_migrateStorageEmptyStorage(t *testing.T) {
+	startTime := time.Now()
 	ctx := context.Background()
 	b, s := createBackendWithStorage(t)
-	request := &logical.InitializationRequest{Storage: s}
 
-	err := migrateStorage(ctx, request, b.Logger())
+	// Reset the version the helper above set to 1.
+	b.pkiStorageVersion.Store(0)
+	require.True(t, b.useLegacyBundleCaStorage(), "pre migration we should have been told to use legacy storage.")
+
+	request := &logical.InitializationRequest{Storage: s}
+	err := b.initialize(ctx, request)
 	require.NoError(t, err)
 
 	issuerIds, err := listIssuers(ctx, s)
@@ -28,13 +33,35 @@ func Test_migrateStorageEmptyStorage(t *testing.T) {
 
 	logEntry, err := getLegacyBundleMigrationLog(ctx, s)
 	require.NoError(t, err)
-	require.Nil(t, logEntry)
+	require.NotNil(t, logEntry)
+	require.Equal(t, latestMigrationVersion, logEntry.MigrationVersion)
+	require.True(t, len(strings.TrimSpace(logEntry.Hash)) > 0,
+		"Hash value (%s) should not have been empty", logEntry.Hash)
+	require.True(t, startTime.Before(logEntry.Created),
+		"created log entry time (%v) was before our start time(%v)?", logEntry.Created, startTime)
+
+	require.False(t, b.useLegacyBundleCaStorage(), "post migration we are still told to use legacy storage")
+
+	// Make sure we can re-run the migration without issues
+	request = &logical.InitializationRequest{Storage: s}
+	err = b.initialize(ctx, request)
+	require.NoError(t, err)
+	logEntry2, err := getLegacyBundleMigrationLog(ctx, s)
+	require.NoError(t, err)
+	require.NotNil(t, logEntry2)
+
+	// Make sure the hash and created times have not changed.
+	require.Equal(t, logEntry.Created, logEntry2.Created)
+	require.Equal(t, logEntry.Hash, logEntry2.Hash)
 }
 
 func Test_migrateStorageSimpleBundle(t *testing.T) {
 	startTime := time.Now()
 	ctx := context.Background()
 	b, s := createBackendWithStorage(t)
+	// Reset the version the helper above set to 1.
+	b.pkiStorageVersion.Store(0)
+	require.True(t, b.useLegacyBundleCaStorage(), "pre migration we should have been told to use legacy storage.")
 
 	bundle := genCertBundle(t, b)
 	json, err := logical.StorageEntryJSON(legacyCertBundlePath, bundle)
@@ -43,8 +70,8 @@ func Test_migrateStorageSimpleBundle(t *testing.T) {
 	require.NoError(t, err)
 
 	request := &logical.InitializationRequest{Storage: s}
-
-	err = migrateStorage(ctx, request, b.Logger())
+	err = b.initialize(ctx, request)
+	require.NoError(t, err)
 	require.NoError(t, err)
 
 	issuerIds, err := listIssuers(ctx, s)
@@ -107,4 +134,6 @@ func Test_migrateStorageSimpleBundle(t *testing.T) {
 
 	require.Equal(t, logEntry.Created, logEntry2.Created)
 	require.Equal(t, logEntry.Hash, logEntry2.Hash)
+
+	require.False(t, b.useLegacyBundleCaStorage(), "post migration we are still told to use legacy storage")
 }