ARO-24037: gate cloud config on hash to prevent serving stale content

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"
@@ -405,3 +407,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

@@ -3,6 +3,7 @@ package nodepool
 import (
 	"bytes"
 	"compress/gzip"
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -12,6 +13,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"
@@ -24,6 +26,7 @@ import (
 
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 
 	"github.com/google/go-cmp/cmp"
 )
@@ -1991,3 +1994,144 @@ 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()},
+		},
+		{
+			name:     "When platform is OpenStack and cloud config exists it should return a hash",
+			platform: hyperv1.OpenStackPlatform,
+			objects: []crclient.Object{func() crclient.Object {
+				cm := cpomanifests.OpenStackProviderConfig(controlPlaneNamespace)
+				cm.Data = map[string]string{"cloud.conf": `[Global]\nauth-url=https://openstack.example.com`}
+				return cm
+			}()},
+		},
+		{
+			name:        "When platform is OpenStack and cloud config is missing it should return empty",
+			platform:    hyperv1.OpenStackPlatform,
+			objects:     []crclient.Object{},
+			expectEmpty: true,
+		},
+	}
+
+	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))
+	})
+
+	t.Run("When a non-NotFound error occurs it should return the error", func(t *testing.T) {
+		g := NewWithT(t)
+		injectedErr := errors.New("connection refused")
+		fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).WithInterceptorFuncs(interceptor.Funcs{
+			Get: func(_ context.Context, _ crclient.WithWatch, _ crclient.ObjectKey, _ crclient.Object, _ ...crclient.GetOption) error {
+				return injectedErr
+			},
+		}).Build()
+
+		cg := &ConfigGenerator{
+			Client: fakeClient,
+			hostedCluster: &hyperv1.HostedCluster{
+				Spec: hyperv1.HostedClusterSpec{
+					Platform: hyperv1.PlatformSpec{
+						Type: hyperv1.AzurePlatform,
+					},
+				},
+			},
+			controlplaneNamespace: controlPlaneNamespace,
+			rolloutConfig:         &rolloutConfig{},
+		}
+
+		_, err := cg.GetCloudConfigHash(t.Context())
+		g.Expect(err).To(HaveOccurred())
+		g.Expect(err.Error()).To(ContainSubstring("connection refused"))
+	})
+}

hypershift-operator/controllers/nodepool/nodepool_controller.go

@@ -51,6 +51,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/blang/semver"
@@ -143,6 +144,10 @@ func (r *NodePoolReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(enqueueParentNodePool), builder.WithPredicates(supportutil.PredicatesForHostedClusterAnnotationScoping(mgr.GetClient()))).
 		// We want to reconcile when the ConfigMaps referenced by the spec.config and also the core ones change.
 		Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.enqueueNodePoolsForConfig), builder.WithPredicates(supportutil.PredicatesForHostedClusterAnnotationScoping(mgr.GetClient()))).
+		// We want to reconcile when cloud provider config ConfigMaps change in the control plane namespace.
+		Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.enqueueNodePoolsForCloudConfig), builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool {
+			return obj.GetName() == "azure-cloud-config" || obj.GetName() == "openstack-cloud-config"
+		}))).
 		WithOptions(controller.Options{
 			RateLimiter:             workqueue.NewTypedItemExponentialFailureRateLimiter[reconcile.Request](1*time.Second, 10*time.Second),
 			MaxConcurrentReconciles: 10,
@@ -959,6 +964,33 @@ func (r *NodePoolReconciler) enqueueNodePoolsForConfig(ctx context.Context, obj
 	return result
 }
 
+func (r *NodePoolReconciler) enqueueNodePoolsForCloudConfig(ctx context.Context, obj client.Object) []reconcile.Request {
+	hcpList := &hyperv1.HostedControlPlaneList{}
+	if err := r.List(ctx, hcpList, client.InNamespace(obj.GetNamespace())); err != nil || len(hcpList.Items) == 0 {
+		return nil
+	}
+	hcName, ok := hcpList.Items[0].Annotations[k8sutil.HostedClusterAnnotation]
+	if !ok {
+		return nil
+	}
+	hc := supportutil.ParseNamespacedName(hcName)
+
+	nodePoolList := &hyperv1.NodePoolList{}
+	if err := r.List(ctx, nodePoolList, client.InNamespace(hc.Namespace)); err != nil {
+		return nil
+	}
+
+	var result []reconcile.Request
+	for i := range nodePoolList.Items {
+		if nodePoolList.Items[i].Spec.ClusterName == hc.Name {
+			result = append(result, reconcile.Request{
+				NamespacedName: client.ObjectKeyFromObject(&nodePoolList.Items[i]),
+			})
+		}
+	}
+	return result
+}
+
 // getNodePoolNamespace returns the namespaced name of a NodePool, given the NodePools name
 // and the control plane namespace name for the hosted cluster that this NodePool is a part of.
 func (r *NodePoolReconciler) getNodePoolNamespacedName(nodePoolName string, controlPlaneNamespace string) (types.NamespacedName, error) {

hypershift-operator/controllers/nodepool/nodepool_controller_test.go

@@ -3747,3 +3747,128 @@ func TestNodePoolReconciler_reconcile(t *testing.T) {
 		})
 	}
 }
+
+func TestEnqueueNodePoolsForCloudConfig(t *testing.T) {
+	t.Parallel()
+	hcNamespace := "clusters"
+	hcName := "my-cluster"
+	cpNamespace := "clusters-my-cluster"
+
+	hcp := &hyperv1.HostedControlPlane{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      hcName,
+			Namespace: cpNamespace,
+			Annotations: map[string]string{
+				k8sutil.HostedClusterAnnotation: hcNamespace + "/" + hcName,
+			},
+		},
+	}
+
+	matchingNodePool := &hyperv1.NodePool{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "np-1",
+			Namespace: hcNamespace,
+		},
+		Spec: hyperv1.NodePoolSpec{
+			ClusterName: hcName,
+		},
+	}
+
+	unrelatedNodePool := &hyperv1.NodePool{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "np-other",
+			Namespace: hcNamespace,
+		},
+		Spec: hyperv1.NodePoolSpec{
+			ClusterName: "other-cluster",
+		},
+	}
+
+	testCases := []struct {
+		name     string
+		cm       *corev1.ConfigMap
+		objects  []client.Object
+		expected []reconcile.Request
+	}{
+		{
+			name: "When azure-cloud-config changes, it should enqueue matching NodePools",
+			cm: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "azure-cloud-config",
+					Namespace: cpNamespace,
+				},
+			},
+			objects: []client.Object{hcp, matchingNodePool, unrelatedNodePool},
+			expected: []reconcile.Request{
+				{NamespacedName: types.NamespacedName{Name: "np-1", Namespace: hcNamespace}},
+			},
+		},
+		{
+			name: "When openstack-cloud-config changes, it should enqueue matching NodePools",
+			cm: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "openstack-cloud-config",
+					Namespace: cpNamespace,
+				},
+			},
+			objects: []client.Object{hcp, matchingNodePool},
+			expected: []reconcile.Request{
+				{NamespacedName: types.NamespacedName{Name: "np-1", Namespace: hcNamespace}},
+			},
+		},
+		{
+			name: "When no HostedControlPlane exists in the namespace, it should return nil",
+			cm: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "azure-cloud-config",
+					Namespace: cpNamespace,
+				},
+			},
+			objects:  []client.Object{matchingNodePool},
+			expected: nil,
+		},
+		{
+			name: "When HostedControlPlane has no cluster annotation, it should return nil",
+			cm: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "azure-cloud-config",
+					Namespace: cpNamespace,
+				},
+			},
+			objects: []client.Object{
+				&hyperv1.HostedControlPlane{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      hcName,
+						Namespace: cpNamespace,
+					},
+				},
+				matchingNodePool,
+			},
+			expected: nil,
+		},
+		{
+			name: "When no NodePools match the HostedCluster, it should return empty",
+			cm: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "azure-cloud-config",
+					Namespace: cpNamespace,
+				},
+			},
+			objects:  []client.Object{hcp, unrelatedNodePool},
+			expected: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			g := NewWithT(t)
+
+			c := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects(tc.objects...).Build()
+			r := &NodePoolReconciler{Client: c}
+
+			result := r.enqueueNodePoolsForCloudConfig(context.Background(), tc.cm)
+			g.Expect(result).To(Equal(tc.expected))
+		})
+	}
+}