Skip to content

Commit e604e71

Browse files
oliashishclaude
andcommitted
Detect TLS cert secret changes in NodeSet reconciler
Cert-manager renewals were invisible because cert secrets are owned by the Certificate CR, not the NodeSet. Add label-based detection in secretWatcherFn and hash comparison in checkDeployment to surface redeployment required signal after cert rotation. Jira: OSPRH-32671 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent d608ed5 commit e604e71

3 files changed

Lines changed: 184 additions & 0 deletions

File tree

internal/controller/dataplane/openstackdataplanenodeset_controller.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,16 @@ func checkDeployment(ctx context.Context, helper *helper.Helper,
596596
continue
597597
}
598598

599+
hasCertSecretsChanged, err := checkCertSecretsChanged(ctx, helper, instance, deployment.Status.SecretHashes)
600+
601+
if err != nil {
602+
return isNodeSetDeploymentReady, isNodeSetDeploymentRunning, isNodeSetDeploymentFailed, failedDeploymentName, err
603+
}
604+
605+
if hasCertSecretsChanged {
606+
continue
607+
}
608+
599609
isNodeSetDeploymentReady = true
600610
for k, v := range deployment.Status.ConfigMapHashes {
601611
instance.Status.ConfigMapHashes[k] = v
@@ -853,6 +863,24 @@ func (r *OpenStackDataPlaneNodeSetReconciler) secretWatcherFn(
853863
ctx context.Context, obj client.Object,
854864
) []reconcile.Request {
855865
Log := r.GetLogger(ctx)
866+
867+
// Check if this is a cert secret (has both NodeSet and Service labels).
868+
// Cert secrets created by EnsureTLSCerts carry these labels but are not
869+
// listed in AnsibleVarsFrom, so the field-index lookup below would miss them.
870+
labels := obj.GetLabels()
871+
if nodeSetName, ok := labels[deployment.NodeSetLabel]; ok {
872+
if _, hasSvcLabel := labels[deployment.ServiceLabel]; hasSvcLabel {
873+
Log.Info(fmt.Sprintf("reconcile loop for openstackdataplanenodeset %s triggered by cert secret %s",
874+
nodeSetName, obj.GetName()))
875+
return []reconcile.Request{{
876+
NamespacedName: types.NamespacedName{
877+
Namespace: obj.GetNamespace(),
878+
Name: nodeSetName,
879+
},
880+
}}
881+
}
882+
}
883+
856884
nodeSets := &dataplanev1.OpenStackDataPlaneNodeSetList{}
857885
kind := strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind)
858886
selector := "spec.ansibleVarsFrom.ansible.configMaps"
@@ -986,3 +1014,27 @@ func checkAnsibleVarsFromChanged(
9861014

9871015
return false, nil
9881016
}
1017+
1018+
// checkCertSecretsChanged computes current hashes for TLS cert secrets
1019+
// belonging to the NodeSet and compares them with deployed hashes.
1020+
// Returns true if any cert secret content has changed, false otherwise.
1021+
func checkCertSecretsChanged(
1022+
ctx context.Context,
1023+
helper *helper.Helper,
1024+
instance *dataplanev1.OpenStackDataPlaneNodeSet,
1025+
deployedSecretHashes map[string]string,
1026+
) (bool, error) {
1027+
currentCertHashes, err := deployment.GetCertSecretHashes(ctx, helper, instance.Namespace, instance.Name)
1028+
if err != nil {
1029+
return false, err
1030+
}
1031+
1032+
for name, currentHash := range currentCertHashes {
1033+
if deployedHash, exists := deployedSecretHashes[name]; exists && deployedHash != currentHash {
1034+
helper.GetLogger().Info("Cert secret content changed", "secret", name)
1035+
return true, nil
1036+
}
1037+
}
1038+
1039+
return false, nil
1040+
}

internal/dataplane/hashes.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,3 +139,36 @@ func ProcessAnsibleVarsFrom(
139139
}
140140
return nil
141141
}
142+
143+
// GetCertSecretHashes returns a map of secret-name to hash for all TLS cert
144+
// secrets belonging to the given NodeSet. Cert secrets are identified by having
145+
// both the NodeSetLabel and ServiceLabel labels set by EnsureTLSCerts.
146+
func GetCertSecretHashes(
147+
ctx context.Context,
148+
helper *helper.Helper,
149+
namespace string,
150+
nodeSetName string,
151+
) (map[string]string, error) {
152+
labelSelector := map[string]string{
153+
NodeSetLabel: nodeSetName,
154+
}
155+
secrets, err := secret.GetSecrets(ctx, helper, namespace, labelSelector)
156+
if err != nil {
157+
return nil, err
158+
}
159+
160+
certHashes := make(map[string]string)
161+
for i := range secrets.Items {
162+
sec := &secrets.Items[i]
163+
if _, hasSvcLabel := sec.Labels[ServiceLabel]; !hasSvcLabel {
164+
continue
165+
}
166+
hash, err := secret.Hash(sec)
167+
if err != nil {
168+
helper.GetLogger().Error(err, "Unable to hash cert Secret", "secret", sec.Name)
169+
return nil, err
170+
}
171+
certHashes[sec.Name] = hash
172+
}
173+
return certHashes, nil
174+
}

test/functional/dataplane/openstackdataplanenodeset_controller_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"
3232
"gopkg.in/yaml.v3"
3333
corev1 "k8s.io/api/core/v1"
34+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3435
"k8s.io/apimachinery/pkg/types"
3536
"k8s.io/utils/ptr"
3637
)
@@ -1806,6 +1807,104 @@ var _ = Describe("Dataplane NodeSet Test", func() {
18061807
})
18071808
})
18081809

1810+
When("Cert secret changes are detected after deployment", func() {
1811+
var certSecretName types.NamespacedName
1812+
var certTestServiceName types.NamespacedName
1813+
1814+
BeforeEach(func() {
1815+
certSecretName = types.NamespacedName{
1816+
Name: "cert-cert-test-service-default-edpm-compute-node-1",
1817+
Namespace: namespace,
1818+
}
1819+
certTestServiceName = types.NamespacedName{
1820+
Name: "cert-test-service",
1821+
Namespace: namespace,
1822+
}
1823+
1824+
nodeSetSpec := DefaultDataPlaneNodeSetSpec("edpm-compute")
1825+
nodeSetSpec["preProvisioned"] = true
1826+
nodeSetSpec["tlsEnabled"] = false
1827+
nodeSetSpec["services"] = []string{"cert-test-service"}
1828+
1829+
DeferCleanup(th.DeleteInstance, CreateDataPlaneServiceFromSpec(certTestServiceName, map[string]interface{}{
1830+
"playbook": "test",
1831+
"tlsCerts": map[string]interface{}{
1832+
"default": map[string]interface{}{
1833+
"contents": []string{"dnsnames"},
1834+
},
1835+
},
1836+
}))
1837+
1838+
// Create a cert secret with the labels that EnsureTLSCerts sets.
1839+
// Labels must match the service name and certKey so that
1840+
// GetDeploymentHashesForService picks them up.
1841+
certSecret := &corev1.Secret{
1842+
ObjectMeta: metav1.ObjectMeta{
1843+
Name: certSecretName.Name,
1844+
Namespace: certSecretName.Namespace,
1845+
Labels: map[string]string{
1846+
"osdpns": dataplaneNodeSetName.Name,
1847+
"osdp-service": certTestServiceName.Name,
1848+
"osdp-service-cert-key": "default",
1849+
"hostname": "edpm-compute-node-1",
1850+
},
1851+
},
1852+
Data: map[string][]byte{
1853+
"tls.crt": []byte("original-cert-data"),
1854+
"tls.key": []byte("original-key-data"),
1855+
"ca.crt": []byte("original-ca-data"),
1856+
},
1857+
}
1858+
Expect(th.K8sClient.Create(th.Ctx, certSecret)).To(Succeed())
1859+
DeferCleanup(th.K8sClient.Delete, th.Ctx, certSecret)
1860+
1861+
DeferCleanup(th.DeleteInstance, CreateNetConfig(dataplaneNetConfigName, DefaultNetConfigSpec()))
1862+
DeferCleanup(th.DeleteInstance, CreateDNSMasq(dnsMasqName, DefaultDNSMasqSpec()))
1863+
DeferCleanup(th.DeleteInstance, CreateDataplaneNodeSet(dataplaneNodeSetName, nodeSetSpec))
1864+
DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(dataplaneDeploymentName, DefaultDataPlaneDeploymentSpec()))
1865+
CreateSSHSecret(dataplaneSSHSecretName)
1866+
CreateCABundleSecret(caBundleSecretName)
1867+
SimulateDNSMasqComplete(dnsMasqName)
1868+
SimulateIPSetComplete(dataplaneNodeName)
1869+
SimulateDNSDataComplete(dataplaneNodeSetName)
1870+
})
1871+
1872+
It("Should detect cert renewal and mark NodeSet as needing redeployment", func() {
1873+
// Complete the deployment
1874+
Eventually(func(g Gomega) {
1875+
ansibleeeName := types.NamespacedName{
1876+
Name: "cert-test-service-" + dataplaneDeploymentName.Name + "-" + dataplaneNodeSetName.Name,
1877+
Namespace: namespace,
1878+
}
1879+
ansibleEE := GetAnsibleee(ansibleeeName)
1880+
ansibleEE.Status.Succeeded = 1
1881+
g.Expect(th.K8sClient.Status().Update(th.Ctx, ansibleEE)).To(Succeed())
1882+
}, th.Timeout, th.Interval).Should(Succeed())
1883+
1884+
// Wait for deployment to be ready and cert secret hash to be tracked
1885+
Eventually(func(g Gomega) {
1886+
instance := GetDataplaneNodeSet(dataplaneNodeSetName)
1887+
g.Expect(instance.Status.Conditions.IsTrue(condition.DeploymentReadyCondition)).To(BeTrue())
1888+
g.Expect(instance.Status.SecretHashes).Should(HaveKey(certSecretName.Name))
1889+
}, th.Timeout, th.Interval).Should(Succeed())
1890+
1891+
// Simulate cert-manager renewal: update the cert secret data
1892+
Eventually(func(g Gomega) {
1893+
secret := &corev1.Secret{}
1894+
g.Expect(th.K8sClient.Get(th.Ctx, certSecretName, secret)).To(Succeed())
1895+
secret.Data["tls.crt"] = []byte("renewed-cert-data")
1896+
secret.Data["tls.key"] = []byte("renewed-key-data")
1897+
g.Expect(th.K8sClient.Update(th.Ctx, secret)).To(Succeed())
1898+
}, th.Timeout, th.Interval).Should(Succeed())
1899+
1900+
// Verify DeploymentReadyCondition becomes False
1901+
Eventually(func(g Gomega) {
1902+
instance := GetDataplaneNodeSet(dataplaneNodeSetName)
1903+
g.Expect(instance.Status.Conditions.IsFalse(condition.DeploymentReadyCondition)).To(BeTrue())
1904+
}, th.Timeout, th.Interval).Should(Succeed())
1905+
})
1906+
})
1907+
18091908
When("Running deployments exist with completed deployment", func() {
18101909
BeforeEach(func() {
18111910
nodeSetSpec := DefaultDataPlaneNodeSetSpec("edpm-compute")

0 commit comments

Comments
 (0)