-
Notifications
You must be signed in to change notification settings - Fork 113
Detect TLS cert secret changes in NodeSet reconciler #1990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -596,6 +596,16 @@ func checkDeployment(ctx context.Context, helper *helper.Helper, | |
| continue | ||
| } | ||
|
|
||
| hasCertSecretsChanged, err := checkCertSecretsChanged(ctx, helper, instance, deployment.Status.SecretHashes) | ||
|
|
||
| if err != nil { | ||
| return isNodeSetDeploymentReady, isNodeSetDeploymentRunning, isNodeSetDeploymentFailed, failedDeploymentName, err | ||
| } | ||
|
|
||
| if hasCertSecretsChanged { | ||
| continue | ||
| } | ||
|
|
||
| isNodeSetDeploymentReady = true | ||
| for k, v := range deployment.Status.ConfigMapHashes { | ||
| instance.Status.ConfigMapHashes[k] = v | ||
|
|
@@ -853,6 +863,24 @@ func (r *OpenStackDataPlaneNodeSetReconciler) secretWatcherFn( | |
| ctx context.Context, obj client.Object, | ||
| ) []reconcile.Request { | ||
| Log := r.GetLogger(ctx) | ||
|
|
||
| // Check if this is a cert secret (has both NodeSet and Service labels). | ||
| // Cert secrets created by EnsureTLSCerts carry these labels but are not | ||
| // listed in AnsibleVarsFrom, so the field-index lookup below would miss them. | ||
| labels := obj.GetLabels() | ||
| if nodeSetName, ok := labels[deployment.NodeSetLabel]; ok { | ||
| if _, hasSvcLabel := labels[deployment.ServiceLabel]; hasSvcLabel { | ||
| Log.Info(fmt.Sprintf("reconcile loop for openstackdataplanenodeset %s triggered by cert secret %s", | ||
| nodeSetName, obj.GetName())) | ||
| return []reconcile.Request{{ | ||
| NamespacedName: types.NamespacedName{ | ||
| Namespace: obj.GetNamespace(), | ||
| Name: nodeSetName, | ||
| }, | ||
| }} | ||
| } | ||
| } | ||
|
|
||
| nodeSets := &dataplanev1.OpenStackDataPlaneNodeSetList{} | ||
| kind := strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind) | ||
| selector := "spec.ansibleVarsFrom.ansible.configMaps" | ||
|
|
@@ -986,3 +1014,27 @@ func checkAnsibleVarsFromChanged( | |
|
|
||
| return false, nil | ||
| } | ||
|
|
||
| // checkCertSecretsChanged computes current hashes for TLS cert secrets | ||
| // belonging to the NodeSet and compares them with deployed hashes. | ||
| // Returns true if any cert secret content has changed, false otherwise. | ||
| func checkCertSecretsChanged( | ||
| ctx context.Context, | ||
| helper *helper.Helper, | ||
| instance *dataplanev1.OpenStackDataPlaneNodeSet, | ||
| deployedSecretHashes map[string]string, | ||
| ) (bool, error) { | ||
| currentCertHashes, err := deployment.GetCertSecretHashes(ctx, helper, instance.Namespace, instance.Name) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| for name, currentHash := range currentCertHashes { | ||
| if deployedHash, exists := deployedSecretHashes[name]; exists && deployedHash != currentHash { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a review with claude has pointed out:
|
||
| helper.GetLogger().Info("Cert secret content changed", "secret", name) | ||
| return true, nil | ||
| } | ||
| } | ||
|
|
||
| return false, nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
claude is recommending to also check the existence of the ServiceKeyLabel here in order to be more precise.