diff --git a/.secrets.baseline b/.secrets.baseline index 692288f829..cf809fecf2 100644 --- a/.secrets.baseline +++ b/.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" } diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index 7bc256bc2b..add1c5aed1 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/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 } diff --git a/internal/dinosaur/pkg/api/dbapi/central_request_types.go b/internal/dinosaur/pkg/api/dbapi/central_request_types.go index d05a5eb709..96209f71ba 100644 --- a/internal/dinosaur/pkg/api/dbapi/central_request_types.go +++ b/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-" 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"` diff --git a/internal/dinosaur/pkg/api/dbapi/data_plane_central_status.go b/internal/dinosaur/pkg/api/dbapi/data_plane_central_status.go index 6e7b6b4b5f..fcc5640125 100644 --- a/internal/dinosaur/pkg/api/dbapi/data_plane_central_status.go +++ b/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 } diff --git a/internal/dinosaur/pkg/api/private/api/openapi.yaml b/internal/dinosaur/pkg/api/private/api/openapi.yaml index 87301baf16..eb8d34af5b 100644 --- a/internal/dinosaur/pkg/api/private/api/openapi.yaml +++ b/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 diff --git a/internal/dinosaur/pkg/api/private/model_data_plane_central_status.go b/internal/dinosaur/pkg/api/private/model_data_plane_central_status.go index f8519590f8..553f079325 100644 --- a/internal/dinosaur/pkg/api/private/model_data_plane_central_status.go +++ b/internal/dinosaur/pkg/api/private/model_data_plane_central_status.go @@ -18,4 +18,6 @@ type DataPlaneCentralStatus struct { Routes []DataPlaneCentralStatusRoutes `json:"routes,omitempty"` // Map of Secrets created for a Central Secrets map[string]string `json:"secrets,omitempty"` + // Hash of plain text secret data used for equality check + SecretDataSum string `json:"secretDataSum,omitempty"` } diff --git a/internal/dinosaur/pkg/api/private/model_managed_central_all_of_metadata.go b/internal/dinosaur/pkg/api/private/model_managed_central_all_of_metadata.go index 8abc1c1245..a581580f8c 100644 --- a/internal/dinosaur/pkg/api/private/model_managed_central_all_of_metadata.go +++ b/internal/dinosaur/pkg/api/private/model_managed_central_all_of_metadata.go @@ -23,5 +23,6 @@ type ManagedCentralAllOfMetadata struct { DeletionTimestamp string `json:"deletionTimestamp,omitempty"` SecretsStored []string `json:"secretsStored,omitempty"` Secrets map[string]string `json:"secrets,omitempty"` + SecretDataSum string `json:"secretDataSum,omitempty"` ExpiredAt *time.Time `json:"expired-at,omitempty"` } diff --git a/internal/dinosaur/pkg/presenters/data_plane_dinosaur_status.go b/internal/dinosaur/pkg/presenters/data_plane_dinosaur_status.go index e1b0789f5d..30008b2629 100644 --- a/internal/dinosaur/pkg/presenters/data_plane_dinosaur_status.go +++ b/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 }) } diff --git a/internal/dinosaur/pkg/presenters/managedcentral.go b/internal/dinosaur/pkg/presenters/managedcentral.go index 8561a0cab1..e7e76b45bc 100644 --- a/internal/dinosaur/pkg/presenters/managedcentral.go +++ b/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{ diff --git a/internal/dinosaur/pkg/services/data_plane_dinosaur.go b/internal/dinosaur/pkg/services/data_plane_dinosaur.go index 58288a69b5..505934189c 100644 --- a/internal/dinosaur/pkg/services/data_plane_dinosaur.go +++ b/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 } diff --git a/internal/dinosaur/pkg/services/dinosaur.go b/internal/dinosaur/pkg/services/dinosaur.go index 533f3e5fd3..f494ce4b2c 100644 --- a/internal/dinosaur/pkg/services/dinosaur.go +++ b/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") } diff --git a/openapi/fleet-manager-private.yaml b/openapi/fleet-manager-private.yaml index 38f686b2c8..20bac9c1d4 100644 --- a/openapi/fleet-manager-private.yaml +++ b/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"