autorotate DP secrets if data has changed

.secrets.baseline

@@ -304,14 +304,14 @@
         "filename": "internal/dinosaur/pkg/presenters/managedcentral.go",
         "hashed_secret": "f4ac636d63edfd5477df8f25e4f4794c73e91d51",
         "is_verified": false,
-        "line_number": 208
+        "line_number": 209
       },
       {
         "type": "Secret Keyword",
         "filename": "internal/dinosaur/pkg/presenters/managedcentral.go",
         "hashed_secret": "e26735ec1cbf8ad15cb7d1eea4893035f61297aa",
         "is_verified": false,
-        "line_number": 214
+        "line_number": 215
       }
     ],
     "internal/dinosaur/pkg/services/dinosaurservice_moq.go": [
@@ -473,5 +473,5 @@
       }
     ]
   },
-  "generated_at": "2024-06-17T20:09:11Z"
+  "generated_at": "2024-07-02T18:42:50Z"
 }

fleetshard/pkg/central/reconciler/reconciler.go

@@ -4,6 +4,7 @@ package reconciler
 import (
 	"bytes"
 	"context"
+	"crypto/sha256"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -824,14 +825,18 @@ func (r *CentralReconciler) collectReconciliationStatus(ctx context.Context, rem
 	}
 
 	// Only report secrets if Central is ready, to ensure we're not trying to get secrets before they are created.
-	// Only report secrets if not all secrets are already stored to ensure we don't overwrite initial secrets with corrupted secrets
-	// from the cluster state.
-	if isRemoteCentralReady(remoteCentral) && !r.areSecretsStored(remoteCentral.Metadata.SecretsStored) {
-		secrets, err := r.collectSecretsEncrypted(ctx, remoteCentral)
+	if isRemoteCentralReady(remoteCentral) {
+		secrets, secretSum, err := r.collectSecretsEncrypted(ctx, remoteCentral)
 		if err != nil {
 			return nil, err
 		}
-		status.Secrets = secrets // pragma: allowlist secret
+
+		// Only report secrets if data hash differs to make sure we don't produce huge amount of data
+		// if no update is required on the fleet-manager DB
+		if secretSum != remoteCentral.Metadata.SecretDataSum { // pragma: allowlist secret
+			status.Secrets = secrets         // pragma: allowlist secret
+			status.SecretDataSum = secretSum // pragma: allowlist secret
+		}
 	}
 
 	return status, nil
@@ -875,18 +880,18 @@ func (r *CentralReconciler) collectSecrets(ctx context.Context, remoteCentral *p
 	return secrets, nil
 }
 
-func (r *CentralReconciler) collectSecretsEncrypted(ctx context.Context, remoteCentral *private.ManagedCentral) (map[string]string, error) {
+func (r *CentralReconciler) collectSecretsEncrypted(ctx context.Context, remoteCentral *private.ManagedCentral) (map[string]string, string, error) {
 	secrets, err := r.collectSecrets(ctx, remoteCentral)
 	if err != nil {
-		return nil, err
+		return nil, "", err
 	}
 
-	encryptedSecrets, err := r.encryptSecrets(secrets)
+	encryptedSecrets, secretDataSha, err := r.encryptSecrets(secrets)
 	if err != nil {
-		return nil, fmt.Errorf("encrypting secrets for namespace: %s: %w", remoteCentral.Metadata.Namespace, err)
+		return nil, "", fmt.Errorf("encrypting secrets for namespace: %s: %w", remoteCentral.Metadata.Namespace, err)
 	}
 
-	return encryptedSecrets, nil
+	return encryptedSecrets, secretDataSha, nil
 }
 
 func (r *CentralReconciler) decryptSecrets(secrets map[string]string) (map[string]*corev1.Secret, error) {
@@ -914,24 +919,34 @@ func (r *CentralReconciler) decryptSecrets(secrets map[string]string) (map[strin
 	return decryptedSecrets, nil
 }
 
-func (r *CentralReconciler) encryptSecrets(secrets map[string]*corev1.Secret) (map[string]string, error) {
+// encryptSecrets return the encrypted secrets and a sha256 sum of secret data to check if secrets
+// need update later on
+func (r *CentralReconciler) encryptSecrets(secrets map[string]*corev1.Secret) (map[string]string, string, error) {
 	encryptedSecrets := map[string]string{}
 
+	allSecretData := []byte{}
 	for key, secret := range secrets { // pragma: allowlist secret
 		secretBytes, err := json.Marshal(secret)
 		if err != nil {
-			return nil, fmt.Errorf("error marshaling secret for encryption: %s: %w", key, err)
+			return nil, "", fmt.Errorf("error marshaling secret for encryption: %s: %w", key, err)
+		}
+
+		for _, data := range secret.Data {
+			allSecretData = append(allSecretData, data...)
 		}
 
 		encryptedBytes, err := r.secretCipher.Encrypt(secretBytes)
 		if err != nil {
-			return nil, fmt.Errorf("encrypting secret: %s: %w", key, err)
+			return nil, "", fmt.Errorf("encrypting secret: %s: %w", key, err)
 		}
 
 		encryptedSecrets[key] = base64.StdEncoding.EncodeToString(encryptedBytes)
 	}
 
-	return encryptedSecrets, nil
+	secretSum := sha256.Sum256(allSecretData)
+	secretShaStr := base64.StdEncoding.EncodeToString(secretSum[:])
+
+	return encryptedSecrets, secretShaStr, nil
 
 }
 

internal/dinosaur/pkg/api/dbapi/central_request_types.go

@@ -74,9 +74,13 @@ type CentralRequest struct {
 	// We store this in the database to ensure that old centrals whose namespace contained "owner-<central-id>" information will continue to work.
 
 	// Secrets stores the encrypted secrets reported for a central tenant
-	Secrets          api.JSON `json:"secrets"`
-	Namespace        string   `json:"namespace"`
-	RoutesCreationID string   `json:"routes_creation_id"`
+	Secrets api.JSON `json:"secrets"`
+	// SecretDataSum is the b64 encoded hash of plain text data of the stored secrets.
+	// It used used for equality checks of secrets in the dataplane cluster with the secrets stored in DB
+	SecretDataSum string `json:"secrets_data_sum"`
+
+	Namespace        string `json:"namespace"`
+	RoutesCreationID string `json:"routes_creation_id"`
 	// DeletionTimestamp stores the timestamp of the DELETE api call for the resource.
 	DeletionTimestamp sql.NullTime `json:"deletionTimestamp"`
 

internal/dinosaur/pkg/api/dbapi/data_plane_central_status.go

@@ -11,6 +11,7 @@ type DataPlaneCentralStatus struct {
 	// Going to ignore the rest of fields (like capacity and versions) for now, until when they are needed
 	Routes                 []DataPlaneCentralRoute
 	Secrets                map[string]string
+	SecretDataSum          string
 	CentralVersion         string
 	CentralOperatorVersion string
 }

internal/dinosaur/pkg/api/private/api/openapi.yaml

@@ -382,6 +382,9 @@ components:
             type: string
           description: Map of Secrets created for a Central
           type: object
+        secretDataSum:
+          description: Hash of plain text secret data used for equality check
+          type: string
       type: object
     DataPlaneCentralStatusUpdateRequest:
       additionalProperties:
@@ -444,6 +447,8 @@ components:
           additionalProperties:
             type: string
           type: object
+        secretDataSum:
+          type: string
         expired-at:
           format: date-time
           nullable: true

internal/dinosaur/pkg/presenters/data_plane_dinosaur_status.go

@@ -34,7 +34,8 @@ func ConvertDataPlaneDinosaurStatus(status map[string]private.DataPlaneCentralSt
 			CentralClusterID: k,
 			Conditions:       c,
 			Routes:           routes,
-			Secrets:          v.Secrets, // pragma: allowlist secret
+			Secrets:          v.Secrets,       // pragma: allowlist secret
+			SecretDataSum:    v.SecretDataSum, // pragma: allowlist secret
 		})
 	}
 

internal/dinosaur/pkg/presenters/managedcentral.go

@@ -120,6 +120,7 @@ func (c *ManagedCentralPresenter) presentManagedCentral(gitopsConfig gitops.Conf
 			},
 			Internal:      from.Internal,
 			SecretsStored: getSecretNames(from), // pragma: allowlist secret
+			SecretDataSum: from.SecretDataSum,   // pragma: allowlist secret
 			ExpiredAt:     dbapi.NullTimeToTimePtr(from.ExpiredAt),
 		},
 		Spec: private.ManagedCentralAllOfSpec{

internal/dinosaur/pkg/services/data_plane_dinosaur.go

@@ -280,11 +280,18 @@ func (d *dataPlaneCentralService) addSecretsToRequest(centralRequest *dbapi.Cent
 		logger.Logger.V(10).Infof("skip persisting secrets for Central %s, report is empty or nil", centralRequest.ID)
 		return nil
 	}
+
+	if centralStatus.SecretDataSum == "" {
+		// TODO: change this to send a bad request later, once we are sure no FS version without SecretDataSum feature is running
+		logger.Logger.V(10).Warningf("persisting secret but no secret data sum. this might be a request of a outdated fleetshard version")
+	}
+
 	logger.Logger.Infof("store secret information for central %s", centralRequest.ID)
 
 	if err := centralRequest.SetSecrets(centralStatus.Secrets); err != nil {
 		return serviceError.NewWithCause(serviceError.ErrorGeneral, err, "failed to set secrets for central %s", centralRequest.ID)
 	}
+	centralRequest.SecretDataSum = centralStatus.SecretDataSum // pragma: allowlist secret
 
 	return nil
 }

internal/dinosaur/pkg/services/dinosaur.go

@@ -183,11 +183,11 @@ func (k *dinosaurService) RotateCentralRHSSOClient(ctx context.Context, centralR
 
 func (k *dinosaurService) ResetCentralSecretBackup(ctx context.Context, centralRequest *dbapi.CentralRequest) *errors.ServiceError {
 	centralRequest.Secrets = nil // pragma: allowlist secret
-
+	centralRequest.SecretDataSum = ""
 	logStateChange("reset secrets", centralRequest.ID, nil)
 
 	dbConn := k.connectionFactory.New()
-	if err := dbConn.Model(centralRequest).Select("secrets").Updates(centralRequest).Error; err != nil {
+	if err := dbConn.Model(centralRequest).Select("secrets", "secrets_data_hash").Updates(centralRequest).Error; err != nil {
 		return errors.NewWithCause(errors.ErrorGeneral, err, "Unable to reset secrets for central request")
 	}
 

openapi/fleet-manager-private.yaml

@@ -269,6 +269,8 @@ components:
                   type: object
                   additionalProperties:
                     type: string
+                secretDataSum:
+                  type: string
                 expired-at:
                   type: string
                   format: date-time
@@ -459,6 +461,9 @@ components:
           type: object
           additionalProperties:
             type: string
+        secretDataSum:
+          description: "Hash of plain text secret data used for equality check"
+          type: string
 
       example:
         $ref: "#/components/examples/DataPlaneCentralStatusRequestExample"