Skip to content

Commit 077d180

Browse files
committed
fix(control-plane-operator): validate cloud config hash in ignition server
- Add cloudConfigHash parameter to IgnitionProvider.GetPayload interface (appended after osStream to preserve existing parameter positions) - Gate ignition payload generation on cloud config hash match in LocalIgnitionProvider, returning an error when the ConfigMap is stale - Invalidate cached payloads when cloud config hash changes by storing the hash in CacheValue and comparing on reconciliation - Report CloudConfigPending reason on token secret when hash validation fails so NodePool controller can observe convergence state Signed-off-by: Todd Wolff <twolff@redhat.com> Commit-Message-Assisted-by: Claude (via Claude Code)
1 parent 946d0f9 commit 077d180

6 files changed

Lines changed: 292 additions & 31 deletions

File tree

ignition-server/cmd/run_local_ignitionprovider.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func (o *RunLocalIgnitionProviderOptions) Run(ctx context.Context) error {
112112
}
113113

114114
osStream := string(token.Data[controllers.TokenSecretOSStreamKey])
115-
payload, err := p.GetPayload(ctx, o.Image, config.String(), "", "", "", osStream)
115+
payload, err := p.GetPayload(ctx, o.Image, config.String(), "", "", "", osStream, "")
116116
if err != nil {
117117
return err
118118
}

ignition-server/controllers/cache.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ type ExpiringCache struct {
3232
}
3333

3434
type CacheValue struct {
35-
Payload []byte
36-
SecretName string
35+
Payload []byte
36+
SecretName string
37+
CloudConfigHash string
3738
}
3839

3940
type entry struct {

ignition-server/controllers/local_ignitionprovider.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,10 @@ func (p *LocalIgnitionProvider) extractImageReferences(ctx context.Context, rele
300300
return releaseImage, nil
301301
}
302302

303-
// For Azure and OpenStack, extract the cloud provider config file as MCO input
304-
func (p *LocalIgnitionProvider) writeCloudProviderConfig(ctx context.Context, mcoDir string) error {
303+
// For Azure and OpenStack, extract the cloud provider config file as MCO input.
304+
// When cloudConfigHash is non-empty, the ConfigMap content is verified against the hash
305+
// before writing to ensure the ignition server does not serve stale cloud config.
306+
func (p *LocalIgnitionProvider) writeCloudProviderConfig(ctx context.Context, mcoDir, cloudConfigHash string) error {
305307
if p.CloudProvider != hyperv1.AzurePlatform && p.CloudProvider != hyperv1.OpenStackPlatform {
306308
return nil
307309
}
@@ -316,6 +318,13 @@ func (p *LocalIgnitionProvider) writeCloudProviderConfig(ctx context.Context, mc
316318
return fmt.Errorf("failed to get cloud provider configmap: %w", err)
317319
}
318320
}
321+
if cloudConfigHash != "" {
322+
actualHash := util.HashConfigMapData(cloudConfigMap.Data)
323+
if actualHash != cloudConfigHash {
324+
return fmt.Errorf("cloud config %s/%s hash mismatch (expected %s, got %s), waiting for update",
325+
cloudConfigMap.Namespace, cloudConfigMap.Name, cloudConfigHash, actualHash)
326+
}
327+
}
319328
cloudConfYaml, err := yaml.Marshal(cloudConfigMap)
320329
if err != nil {
321330
return fmt.Errorf("failed to marshal cloud config: %w", err)
@@ -607,7 +616,7 @@ func (p *LocalIgnitionProvider) runMCSAndFetchPayload(ctx context.Context, dirs
607616
return payload, err
608617
}
609618

610-
func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, customConfig, pullSecretHash, additionalTrustBundleHash, hcConfigurationHash, osStream string) ([]byte, error) {
619+
func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, customConfig, pullSecretHash, additionalTrustBundleHash, hcConfigurationHash, osStream, cloudConfigHash string) ([]byte, error) {
611620
p.lock.Lock()
612621
defer p.lock.Unlock()
613622

@@ -682,7 +691,7 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, cu
682691
return nil, fmt.Errorf("failed to extract image-references from image: %w", err)
683692
}
684693

685-
if err := p.writeCloudProviderConfig(ctx, dirs.mcoDir); err != nil {
694+
if err := p.writeCloudProviderConfig(ctx, dirs.mcoDir, cloudConfigHash); err != nil {
686695
return nil, err
687696
}
688697

ignition-server/controllers/local_ignitionprovider_test.go

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
1818
"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/imageprovider"
19+
"github.com/openshift/hypershift/support/util"
1920

2021
corev1 "k8s.io/api/core/v1"
2122
"k8s.io/apimachinery/pkg/api/meta"
@@ -1105,11 +1106,12 @@ func TestWriteCloudProviderConfig(t *testing.T) {
11051106
t.Parallel()
11061107

11071108
tests := []struct {
1108-
name string
1109-
cloudProvider hyperv1.PlatformType
1110-
objects []client.Object
1111-
expectFile bool
1112-
expectError string
1109+
name string
1110+
cloudProvider hyperv1.PlatformType
1111+
objects []client.Object
1112+
cloudConfigHash string
1113+
expectFile bool
1114+
expectError string
11131115
}{
11141116
{
11151117
name: "When cloud provider is AWS, it should return nil without writing any file",
@@ -1149,6 +1151,30 @@ func TestWriteCloudProviderConfig(t *testing.T) {
11491151
objects: []client.Object{},
11501152
expectError: "failed to get cloud provider configmap",
11511153
},
1154+
{
1155+
name: "When cloud provider is Azure and hash matches, it should write cloud config file",
1156+
cloudProvider: hyperv1.AzurePlatform,
1157+
objects: []client.Object{
1158+
&corev1.ConfigMap{
1159+
ObjectMeta: metav1.ObjectMeta{Name: "azure-cloud-config", Namespace: "test-ns"},
1160+
Data: map[string]string{"cloud.conf": "azure-config-data"},
1161+
},
1162+
},
1163+
cloudConfigHash: util.HashConfigMapData(map[string]string{"cloud.conf": "azure-config-data"}),
1164+
expectFile: true,
1165+
},
1166+
{
1167+
name: "When cloud provider is Azure and hash does not match, it should return an error",
1168+
cloudProvider: hyperv1.AzurePlatform,
1169+
objects: []client.Object{
1170+
&corev1.ConfigMap{
1171+
ObjectMeta: metav1.ObjectMeta{Name: "azure-cloud-config", Namespace: "test-ns"},
1172+
Data: map[string]string{"cloud.conf": "azure-config-data"},
1173+
},
1174+
},
1175+
cloudConfigHash: "wrong-hash",
1176+
expectError: "hash mismatch",
1177+
},
11521178
}
11531179

11541180
for _, tt := range tests {
@@ -1165,7 +1191,7 @@ func TestWriteCloudProviderConfig(t *testing.T) {
11651191
CloudProvider: tt.cloudProvider,
11661192
}
11671193

1168-
err := provider.writeCloudProviderConfig(t.Context(), mcoDir)
1194+
err := provider.writeCloudProviderConfig(t.Context(), mcoDir, tt.cloudConfigHash)
11691195
if tt.expectError != "" {
11701196
g.Expect(err).To(HaveOccurred())
11711197
g.Expect(err.Error()).To(ContainSubstring(tt.expectError))

ignition-server/controllers/tokensecret_controller.go

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ const (
3535
TokenSecretPullSecretHashKey = "pull-secret-hash"
3636
TokenSecretHCConfigurationHashKey = "hc-configuration-hash"
3737
TokenSecretAdditionalTrustBundleHashKey = "additional-trust-bundle-hash"
38+
TokenSecretCloudConfigHashKey = "cloud-config-hash"
3839
InvalidConfigReason = "InvalidConfig"
40+
CloudConfigPendingReason = "CloudConfigPending"
3941
TokenSecretReasonKey = "reason"
4042
// TokenSecretOSStreamKey is intentionally duplicated from nodepool/token.go
4143
// to avoid a dependency from ignition-server → hypershift-operator.
@@ -88,7 +90,7 @@ func NewPayloadStore() *ExpiringCache {
8890
type IgnitionProvider interface {
8991
// GetPayload returns the ignition payload content for
9092
// the provided release image and a config string containing 0..N MachineConfig yaml definitions.
91-
GetPayload(ctx context.Context, payloadImage, config, pullSecretHash, additionalTrustBundleHash, hcConfigurationHash, osStream string) ([]byte, error)
93+
GetPayload(ctx context.Context, payloadImage, config, pullSecretHash, additionalTrustBundleHash, hcConfigurationHash, osStream, cloudConfigHash string) ([]byte, error)
9294
}
9395

9496
// TokenSecretReconciler watches token Secrets
@@ -231,25 +233,35 @@ func (r *TokenSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request)
231233
}
232234

233235
token := string(tokenSecret.Data[TokenSecretTokenKey])
236+
cloudConfigHash := string(tokenSecret.Data[TokenSecretCloudConfigHashKey])
234237
if value, ok := r.PayloadStore.Get(token); ok {
235-
log.Info("Payload found in cache")
236-
237-
if tokenNeedRotation(timeLived) {
238-
log.Info("Rotating token ID")
239-
if err := r.rotateToken(ctx, tokenSecret, value, now); err != nil {
240-
return ctrl.Result{}, err
238+
if value.CloudConfigHash != cloudConfigHash {
239+
log.Info("Cloud config hash changed, invalidating cached payload")
240+
r.PayloadStore.Delete(token)
241+
} else {
242+
log.Info("Payload found in cache")
243+
244+
if tokenNeedRotation(timeLived) {
245+
log.Info("Rotating token ID")
246+
if err := r.rotateToken(ctx, tokenSecret, value, now); err != nil {
247+
return ctrl.Result{}, err
248+
}
249+
TokenRotationTotal.Inc()
241250
}
242-
TokenRotationTotal.Inc()
251+
return ctrl.Result{RequeueAfter: ttl/2 - durationDeref(timeLived)}, nil
243252
}
244-
return ctrl.Result{RequeueAfter: ttl/2 - durationDeref(timeLived)}, nil
245253
}
246254

247255
// If something else rotated the token (e.g. running in HA), we fall back to set the cache value from the old one.
248256
oldToken, ok := tokenSecret.Data[TokenSecretOldTokenKey]
249257
if ok {
250258
if value, ok := r.PayloadStore.Get(string(oldToken)); ok {
251-
r.PayloadStore.Set(token, value)
252-
return ctrl.Result{RequeueAfter: ttl/2 - durationDeref(timeLived)}, nil
259+
if value.CloudConfigHash == cloudConfigHash {
260+
r.PayloadStore.Set(token, value)
261+
return ctrl.Result{RequeueAfter: ttl/2 - durationDeref(timeLived)}, nil
262+
}
263+
log.Info("Cloud config hash changed, invalidating old token cached payload")
264+
r.PayloadStore.Delete(string(oldToken))
253265
}
254266
}
255267

@@ -279,7 +291,7 @@ func (r *TokenSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request)
279291
osStream := string(tokenSecret.Data[TokenSecretOSStreamKey])
280292
payload, err := func() ([]byte, error) {
281293
start := time.Now()
282-
payload, err := r.IgnitionProvider.GetPayload(ctx, releaseImage, config.String(), pullSecretHash, additionalTrustBundleHash, hcConfigurationHash, osStream)
294+
payload, err := r.IgnitionProvider.GetPayload(ctx, releaseImage, config.String(), pullSecretHash, additionalTrustBundleHash, hcConfigurationHash, osStream, cloudConfigHash)
283295
if err != nil {
284296
return nil, fmt.Errorf("error getting ignition payload: %w", err)
285297
}
@@ -292,12 +304,16 @@ func (r *TokenSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request)
292304
// This patch could flood the API server, so we should only do it when the reason or message is different from the current one.
293305
// More info here: https://issues.redhat.com/browse/OCPBUGS-42320.
294306
errWithFullMsg := fmt.Errorf("failed to generate payload: %w", err)
295-
if hasSameReasonAndMessage(tokenSecret, InvalidConfigReason, errWithFullMsg) {
307+
reason := InvalidConfigReason
308+
if strings.Contains(err.Error(), "hash mismatch") {
309+
reason = CloudConfigPendingReason
310+
}
311+
if hasSameReasonAndMessage(tokenSecret, reason, errWithFullMsg) {
296312
return ctrl.Result{}, errWithFullMsg
297313
}
298314

299315
patch := tokenSecret.DeepCopy()
300-
patch.Data[TokenSecretReasonKey] = []byte(InvalidConfigReason)
316+
patch.Data[TokenSecretReasonKey] = []byte(reason)
301317
patch.Data[TokenSecretMessageKey] = []byte(errWithFullMsg.Error())
302318
if err := r.Client.Patch(ctx, patch, client.MergeFrom(tokenSecret)); err != nil {
303319
return ctrl.Result{}, fmt.Errorf("failed to patch tokenSecret with payload content: %w", err)
@@ -307,12 +323,13 @@ func (r *TokenSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request)
307323
}
308324

309325
log.Info("IgnitionProvider generated payload")
310-
r.PayloadStore.Set(token, CacheValue{Payload: payload, SecretName: tokenSecret.Name})
326+
cacheValue := CacheValue{Payload: payload, SecretName: tokenSecret.Name, CloudConfigHash: cloudConfigHash}
327+
r.PayloadStore.Set(token, cacheValue)
311328
oldToken, ok = tokenSecret.Data[TokenSecretOldTokenKey]
312329
if ok {
313330
// If we got here and there's an old token e.g. ignition server pod was restarted, then we set it as well
314331
// So Machines that were given that token right before the restart can succeed.
315-
r.PayloadStore.Set(string(oldToken), CacheValue{Payload: payload, SecretName: tokenSecret.Name})
332+
r.PayloadStore.Set(string(oldToken), cacheValue)
316333
}
317334

318335
patch := tokenSecret.DeepCopy()

0 commit comments

Comments
 (0)