fix(ignition): gate cloud config on hash to prevent serving stale content

The ignition server reads the cloud config ConfigMap (azure-cloud-config,
openstack-cloud-config) live during payload generation. If the ConfigMap
is updated by the CPO after the nodepool controller creates the token but
before the ignition server reads it, stale content could be served.

Store a hash of the cloud config ConfigMap in the token secret (computed
by the nodepool controller), and verify it matches the live ConfigMap in
the ignition server before writing cloud.conf. On mismatch, return an
error to trigger retry, matching the existing pattern for pullSecretHash
and additionalTrustBundleHash.

Signed-off-by: Todd Wolff <twolff@redhat.com>

Author: twolff-gh

hypershift-operator/controllers/nodepool/config.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
+	cpomanifests "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests"
 	"github.com/openshift/hypershift/support/api"
 	"github.com/openshift/hypershift/support/backwardcompat"
 	"github.com/openshift/hypershift/support/capabilities"
@@ -24,6 +25,7 @@ import (
 	"github.com/openshift/api/operator/v1alpha1"
 
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	serializer "k8s.io/apimachinery/pkg/runtime/serializer/json"
@@ -354,3 +356,26 @@ func globalConfigString(hcluster *hyperv1.HostedCluster) (string, error) {
 	// Some fields in the ClusterConfiguration have changes that are not backwards compatible with older versions of the CPO.
 	return backwardcompat.GetBackwardCompatibleConfigString(rawConfig), nil
 }
+
+// GetCloudConfigHash returns a hash of the platform-specific cloud config ConfigMap content.
+// For platforms without a cloud config ConfigMap (e.g. AWS), it returns an empty string.
+func (cg *ConfigGenerator) GetCloudConfigHash(ctx context.Context) (string, error) {
+	var cm *corev1.ConfigMap
+	switch cg.hostedCluster.Spec.Platform.Type {
+	case hyperv1.AzurePlatform:
+		cm = cpomanifests.AzureProviderConfig(cg.controlplaneNamespace)
+	case hyperv1.OpenStackPlatform:
+		cm = cpomanifests.OpenStackProviderConfig(cg.controlplaneNamespace)
+	default:
+		return "", nil
+	}
+
+	if err := cg.Get(ctx, client.ObjectKeyFromObject(cm), cm); err != nil {
+		if apierrors.IsNotFound(err) {
+			return "", nil
+		}
+		return "", fmt.Errorf("failed to get cloud config ConfigMap %s/%s: %w", cm.Namespace, cm.Name, err)
+	}
+
+	return supportutil.HashConfigMapData(cm.Data), nil
+}

hypershift-operator/controllers/nodepool/config_test.go

@@ -12,6 +12,7 @@ import (
 
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
 	"github.com/openshift/hypershift/api/util/ipnet"
+	cpomanifests "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests"
 	api "github.com/openshift/hypershift/support/api"
 	"github.com/openshift/hypershift/support/releaseinfo"
 	supportutil "github.com/openshift/hypershift/support/util"
@@ -1812,3 +1813,102 @@ func TestGlobalConfigString(t *testing.T) {
 		})
 	}
 }
+
+func TestCloudConfigHash(t *testing.T) {
+	controlPlaneNamespace := "test-cp"
+	azureCloudConfig := cpomanifests.AzureProviderConfig(controlPlaneNamespace)
+	azureCloudConfig.Data = map[string]string{
+		"cloud.conf": `{"tenantId":"t1","subscriptionId":"s1"}`,
+	}
+
+	azureCloudConfigDifferent := cpomanifests.AzureProviderConfig(controlPlaneNamespace)
+	azureCloudConfigDifferent.Data = map[string]string{
+		"cloud.conf": `{"tenantId":"t1","subscriptionId":"s1","userAssignedIdentityID":"new-id"}`,
+	}
+
+	testCases := []struct {
+		name        string
+		platform    hyperv1.PlatformType
+		objects     []crclient.Object
+		expectEmpty bool
+	}{
+		{
+			name:     "When platform is Azure and cloud config exists it should return a hash",
+			platform: hyperv1.AzurePlatform,
+			objects:  []crclient.Object{azureCloudConfig.DeepCopy()},
+		},
+		{
+			name:        "When platform is Azure and cloud config is missing it should return empty",
+			platform:    hyperv1.AzurePlatform,
+			objects:     []crclient.Object{},
+			expectEmpty: true,
+		},
+		{
+			name:        "When platform is AWS it should return empty",
+			platform:    hyperv1.AWSPlatform,
+			objects:     []crclient.Object{},
+			expectEmpty: true,
+		},
+		{
+			name:     "When cloud config content changes it should produce a different hash",
+			platform: hyperv1.AzurePlatform,
+			objects:  []crclient.Object{azureCloudConfigDifferent.DeepCopy()},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
+			fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(tc.objects...).Build()
+
+			cg := &ConfigGenerator{
+				Client: fakeClient,
+				hostedCluster: &hyperv1.HostedCluster{
+					Spec: hyperv1.HostedClusterSpec{
+						Platform: hyperv1.PlatformSpec{
+							Type: tc.platform,
+						},
+					},
+				},
+				controlplaneNamespace: controlPlaneNamespace,
+				rolloutConfig:         &rolloutConfig{},
+			}
+
+			hash, err := cg.GetCloudConfigHash(t.Context())
+			g.Expect(err).ToNot(HaveOccurred())
+
+			if tc.expectEmpty {
+				g.Expect(hash).To(BeEmpty())
+			} else {
+				g.Expect(hash).ToNot(BeEmpty())
+			}
+		})
+	}
+
+	t.Run("When content differs the hashes should differ", func(t *testing.T) {
+		g := NewWithT(t)
+
+		getHash := func(obj crclient.Object) string {
+			fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(obj).Build()
+			cg := &ConfigGenerator{
+				Client: fakeClient,
+				hostedCluster: &hyperv1.HostedCluster{
+					Spec: hyperv1.HostedClusterSpec{
+						Platform: hyperv1.PlatformSpec{
+							Type: hyperv1.AzurePlatform,
+						},
+					},
+				},
+				controlplaneNamespace: controlPlaneNamespace,
+				rolloutConfig:         &rolloutConfig{},
+			}
+			hash, err := cg.GetCloudConfigHash(t.Context())
+			g.Expect(err).ToNot(HaveOccurred())
+			return hash
+		}
+
+		hash1 := getHash(azureCloudConfig.DeepCopy())
+		hash2 := getHash(azureCloudConfigDifferent.DeepCopy())
+		g.Expect(hash1).ToNot(Equal(hash2))
+	})
+}

hypershift-operator/controllers/nodepool/token.go

@@ -41,6 +41,7 @@ const (
 	TokenSecretPullSecretHashKey         = "pull-secret-hash"
 	TokenSecretHCConfigurationHashKey    = "hc-configuration-hash"
 	TokenSecretAdditionalTrustBundleKey  = "additional-trust-bundle-hash"
+	TokenSecretCloudConfigHashKey        = "cloud-config-hash"
 	TokenSecretConfigKey                 = "config"
 	TokenSecretAnnotation                = "hypershift.openshift.io/ignition-config"
 	TokenSecretIgnitionReachedAnnotation = "hypershift.openshift.io/ignition-reached"
@@ -61,6 +62,7 @@ type Token struct {
 	pullSecretHash            []byte
 	additionalTrustBundleHash []byte
 	globalConfigHash          []byte
+	cloudConfigHash           []byte
 	userData                  *userData
 }
 
@@ -113,13 +115,19 @@ func NewToken(ctx context.Context, configGenerator *ConfigGenerator, cpoCapabili
 		return nil, fmt.Errorf("failed to hash HostedCluster configuration: %w", err)
 	}
 
+	cloudConfigHash, err := configGenerator.GetCloudConfigHash(ctx)
+	if err != nil {
+		return nil, err
+	}
+
 	token := &Token{
 		CreateOrUpdateProvider:    upsert.New(false),
 		ConfigGenerator:           configGenerator,
 		cpoCapabilities:           cpoCapabilities,
 		pullSecretHash:            []byte(supportutil.HashSimple(pullSecretBytes)),
 		additionalTrustBundleHash: []byte(supportutil.HashSimple(additionalTrustBundle)),
 		globalConfigHash:          []byte(hcConfigurationHash),
+		cloudConfigHash:           []byte(cloudConfigHash),
 	}
 
 	// User data input.
@@ -354,11 +362,13 @@ func (t *Token) reconcileTokenSecret(tokenSecret *corev1.Secret) error {
 		tokenSecret.Data[TokenSecretPullSecretHashKey] = t.pullSecretHash
 		tokenSecret.Data[TokenSecretAdditionalTrustBundleKey] = t.additionalTrustBundleHash
 		tokenSecret.Data[TokenSecretHCConfigurationHashKey] = t.globalConfigHash
+		tokenSecret.Data[TokenSecretCloudConfigHashKey] = t.cloudConfigHash
 	}
 	// TODO (alberto): Only apply this on creation and change the hash generation to only use triggering upgrade fields.
 	// We let this change to happen inplace now as the tokenSecret and the mcs config use the whole spec.Config for the comparing hash.
 	// Otherwise if something which does not trigger a new token generation from spec.Config changes, like .IDP, both hashes would mismatch forever.
 	tokenSecret.Data[TokenSecretHCConfigurationHashKey] = t.globalConfigHash
+	tokenSecret.Data[TokenSecretCloudConfigHashKey] = t.cloudConfigHash
 
 	return nil
 }

ignition-server/cmd/run_local_ignitionprovider.go

@@ -111,7 +111,7 @@ func (o *RunLocalIgnitionProviderOptions) Run(ctx context.Context) error {
 		FeatureGateManifest:   o.FeatureGateManifest,
 	}
 
-	payload, err := p.GetPayload(ctx, o.Image, config.String(), "", "", "")
+	payload, err := p.GetPayload(ctx, o.Image, config.String(), "", "", "", "")
 	if err != nil {
 		return err
 	}

ignition-server/controllers/cache.go

@@ -32,8 +32,9 @@ type ExpiringCache struct {
 }
 
 type CacheValue struct {
-	Payload    []byte
-	SecretName string
+	Payload         []byte
+	SecretName      string
+	CloudConfigHash string
 }
 
 type entry struct {

ignition-server/controllers/local_ignitionprovider.go

@@ -300,8 +300,10 @@ func (p *LocalIgnitionProvider) extractImageReferences(ctx context.Context, rele
 	return releaseImage, nil
 }
 
-// For Azure and OpenStack, extract the cloud provider config file as MCO input
-func (p *LocalIgnitionProvider) writeCloudProviderConfig(ctx context.Context, mcoDir string) error {
+// For Azure and OpenStack, extract the cloud provider config file as MCO input.
+// When cloudConfigHash is non-empty, the ConfigMap content is verified against the hash
+// before writing to ensure the ignition server does not serve stale cloud config.
+func (p *LocalIgnitionProvider) writeCloudProviderConfig(ctx context.Context, mcoDir, cloudConfigHash string) error {
 	if p.CloudProvider != hyperv1.AzurePlatform && p.CloudProvider != hyperv1.OpenStackPlatform {
 		return nil
 	}
@@ -316,6 +318,9 @@ func (p *LocalIgnitionProvider) writeCloudProviderConfig(ctx context.Context, mc
 			return fmt.Errorf("failed to get cloud provider configmap: %w", err)
 		}
 	}
+	if cloudConfigHash != "" && util.HashConfigMapData(cloudConfigMap.Data) != cloudConfigHash {
+		return fmt.Errorf("cloud config does not match hash, waiting for update")
+	}
 	cloudConfYaml, err := yaml.Marshal(cloudConfigMap)
 	if err != nil {
 		return fmt.Errorf("failed to marshal cloud config: %w", err)
@@ -607,7 +612,7 @@ func (p *LocalIgnitionProvider) runMCSAndFetchPayload(ctx context.Context, dirs
 	return payload, err
 }
 
-func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, customConfig, pullSecretHash, additionalTrustBundleHash, hcConfigurationHash string) ([]byte, error) {
+func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, customConfig, pullSecretHash, additionalTrustBundleHash, hcConfigurationHash, cloudConfigHash string) ([]byte, error) {
 	p.lock.Lock()
 	defer p.lock.Unlock()
 
@@ -687,7 +692,7 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, cu
 		return nil, fmt.Errorf("failed to extract image-references from image: %w", err)
 	}
 
-	if err := p.writeCloudProviderConfig(ctx, dirs.mcoDir); err != nil {
+	if err := p.writeCloudProviderConfig(ctx, dirs.mcoDir, cloudConfigHash); err != nil {
 		return nil, err
 	}
 

ignition-server/controllers/local_ignitionprovider_test.go

@@ -16,6 +16,7 @@ import (
 
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
 	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/imageprovider"
+	"github.com/openshift/hypershift/support/util"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -1104,11 +1105,12 @@ func TestWriteCloudProviderConfig(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		name          string
-		cloudProvider hyperv1.PlatformType
-		objects       []client.Object
-		expectFile    bool
-		expectError   string
+		name            string
+		cloudProvider   hyperv1.PlatformType
+		objects         []client.Object
+		cloudConfigHash string
+		expectFile      bool
+		expectError     string
 	}{
 		{
 			name:          "When cloud provider is AWS, it should return nil without writing any file",
@@ -1148,6 +1150,30 @@ func TestWriteCloudProviderConfig(t *testing.T) {
 			objects:       []client.Object{},
 			expectError:   "failed to get cloud provider configmap",
 		},
+		{
+			name:          "When cloud provider is Azure and hash matches, it should write cloud config file",
+			cloudProvider: hyperv1.AzurePlatform,
+			objects: []client.Object{
+				&corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{Name: "azure-cloud-config", Namespace: "test-ns"},
+					Data:       map[string]string{"cloud.conf": "azure-config-data"},
+				},
+			},
+			cloudConfigHash: util.HashConfigMapData(map[string]string{"cloud.conf": "azure-config-data"}),
+			expectFile:      true,
+		},
+		{
+			name:          "When cloud provider is Azure and hash does not match, it should return an error",
+			cloudProvider: hyperv1.AzurePlatform,
+			objects: []client.Object{
+				&corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{Name: "azure-cloud-config", Namespace: "test-ns"},
+					Data:       map[string]string{"cloud.conf": "azure-config-data"},
+				},
+			},
+			cloudConfigHash: "wrong-hash",
+			expectError:     "cloud config does not match hash, waiting for update",
+		},
 	}
 
 	for _, tt := range tests {
@@ -1164,7 +1190,7 @@ func TestWriteCloudProviderConfig(t *testing.T) {
 				CloudProvider: tt.cloudProvider,
 			}
 
-			err := provider.writeCloudProviderConfig(t.Context(), mcoDir)
+			err := provider.writeCloudProviderConfig(t.Context(), mcoDir, tt.cloudConfigHash)
 			if tt.expectError != "" {
 				g.Expect(err).To(HaveOccurred())
 				g.Expect(err.Error()).To(ContainSubstring(tt.expectError))