DNM/SPLAT-2587: aws/ccm configuration

AGENTS.md

@@ -73,6 +73,27 @@ bin/hypershift install --development # Install in development mode
 bin/hypershift-operator run          # Run operator locally
 ```
 
+## Local Development Environment
+
+For detailed instructions on local development workflows including:
+- Building custom operator images
+- Installing HyperShift with custom images
+- Creating and managing hosted clusters
+- Running E2E tests
+- Iteration workflows
+
+See the comprehensive guides in:
+- **[HACKING.md](./HACKING.md)** - General development how-to guides
+- **[.claude/skills/dev/](./.claude/skills/dev/)** - Step-by-step development skills for specific tasks:
+  - `build-ho-image` - Building HyperShift operator images
+  - `build-cpo-image` - Building control plane operator images
+  - `install-ho-aws` - Installing HyperShift on AWS
+  - `create-hc-aws` - Creating hosted clusters
+  - `e2e-run-aws` - Running and iterating on E2E tests
+  - `destroy-hc-aws` - Cleaning up hosted clusters
+
+These skills include environment setup, prerequisites, troubleshooting, and iteration workflows.
+
 ## Testing Strategy
 
 ### Unit Tests

cmd/infra/aws/iam.go

@@ -933,20 +933,54 @@ func (o *CreateIAMOptions) CreateOIDCResources(ctx context.Context, iamClient aw
 		ingressRoleName := output.Roles.IngressARN[strings.LastIndex(output.Roles.IngressARN, "/")+1:]
 
 		// Cloud Controller Manager's (CCM) managed policy needs to be updated on ROSA to allow new permissions downstream controllers to work.
-		// The permissions are:
-		// - elasticloadbalancing:DescribeTargetGroupAttributes
-		// - elasticloadbalancing:ModifyTargetGroupAttributes
+		// The permissions are required for NLB with managed security groups feature.
 		//
+		// References for hairpinning support:
 		// https://issues.redhat.com/browse/OCPBUGS-65885
-		//
-		// This inline policy must be removed when the following issue is resolved:
 		// https://issues.redhat.com/browse/SREP-2895
 		// https://redhat-internal.slack.com/archives/C03SZLX3A10/p1765396356482459
+		//
+		// References for managed security group support:
+		// https://github.com/kubernetes/cloud-provider-aws/pull/1158
+		// https://issues.redhat.com/browse/SREP-3643
+		// https://issues.redhat.com/browse/OCPSTRAT-1553
 		ccmPolicyStatement := `{
 				"Effect": "Allow",
 				"Action": [
+					"elasticloadbalancing:CreateLoadBalancer",
+					"elasticloadbalancing:DeleteLoadBalancer",
+					"elasticloadbalancing:DescribeLoadBalancers",
+					"elasticloadbalancing:ModifyLoadBalancerAttributes",
+					"elasticloadbalancing:CreateTargetGroup",
+					"elasticloadbalancing:DeleteTargetGroup",
+					"elasticloadbalancing:DescribeTargetGroups",
 					"elasticloadbalancing:DescribeTargetGroupAttributes",
-					"elasticloadbalancing:ModifyTargetGroupAttributes"
+					"elasticloadbalancing:ModifyTargetGroupAttributes",
+					"elasticloadbalancing:DescribeTargetHealth",
+					"elasticloadbalancing:RegisterTargets",
+					"elasticloadbalancing:DeregisterTargets",
+					"elasticloadbalancing:CreateListener",
+					"elasticloadbalancing:DeleteListener",
+					"elasticloadbalancing:DescribeListeners",
+					"elasticloadbalancing:SetSecurityGroups",
+					"elasticloadbalancing:DescribeTags",
+					"elasticloadbalancing:AddTags",
+					"elasticloadbalancing:RemoveTags"
+				],
+				"Resource": "*"
+			},
+			{
+				"Effect": "Allow",
+				"Action": [
+					"ec2:CreateSecurityGroup",
+					"ec2:DeleteSecurityGroup",
+					"ec2:DescribeSecurityGroups",
+					"ec2:AuthorizeSecurityGroupIngress",
+					"ec2:RevokeSecurityGroupIngress",
+					"ec2:DescribeSubnets",
+					"ec2:DescribeVpcs",
+					"ec2:DescribeInstances",
+					"ec2:CreateTags"
 				],
 				"Resource": "*"
 			}`

control-plane-operator/controllers/hostedcontrolplane/v2/cloud_controller_manager/aws/config.go

@@ -4,7 +4,9 @@ import (
 	"fmt"
 	"strconv"
 
+	configv1 "github.com/openshift/api/config/v1"
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
+	"github.com/openshift/hypershift/control-plane-operator/featuregates"
 	component "github.com/openshift/hypershift/support/controlplane-component"
 
 	corev1 "k8s.io/api/core/v1"
@@ -60,6 +62,19 @@ func adaptConfig(cpContext component.WorkloadContext, cm *corev1.ConfigMap) erro
 		baseConfig += fmt.Sprintf("\nClusterServiceSharedLoadBalancerHealthProbePort = %s", portStr)
 	}
 
+	// Add NLBSecurityGroupMode when the AWSServiceLBNetworkSecurityGroup feature gate is enabled for this cluster.
+	// Check the feature gate based on the cluster's configured feature gate spec, not the global operator feature set.
+	var featureGateSpec *configv1.FeatureGateSpec
+	if cpContext.HCP.Spec.Configuration != nil {
+		featureGateSpec = cpContext.HCP.Spec.Configuration.FeatureGate
+	}
+
+	isEnabled := featuregates.IsFeatureEnabledInFeatureGateSpec(featureGateSpec, featuregates.AWSServiceLBNetworkSecurityGroup)
+	fmt.Printf("DEBUG: AWSServiceLBNetworkSecurityGroup enabled=%v, featureGateSpec=%+v\n", isEnabled, featureGateSpec)
+	if isEnabled {
+		baseConfig += "\nNLBSecurityGroupMode = Managed"
+	}
+
 	cm.Data[configKey] = baseConfig
 	return nil
 }

control-plane-operator/controllers/hostedcontrolplane/v2/cloud_controller_manager/aws/config_test.go

@@ -6,11 +6,14 @@ import (
 
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
 	assets "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/v2/assets"
+	"github.com/openshift/hypershift/control-plane-operator/featuregates"
 	"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/api"
 	component "github.com/openshift/hypershift/support/controlplane-component"
 	"github.com/openshift/hypershift/support/testutil"
 	"github.com/openshift/hypershift/support/util"
 
+	configv1 "github.com/openshift/api/config/v1"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
@@ -101,3 +104,95 @@ func TestConfigErrorStates(t *testing.T) {
 		})
 	}
 }
+
+func TestConfigWithCustomAnnotations(t *testing.T) {
+	tests := []struct {
+		name              string
+		hcp               *hyperv1.HostedControlPlane
+		configContains    string
+		configNotContains string
+	}{
+		{
+			name: "When Default feature set is set it should not have custom configuration",
+			hcp: func() *hyperv1.HostedControlPlane {
+				hcp := newTestHCP(map[string]string{})
+				hcp.Spec.Configuration = &hyperv1.ClusterConfiguration{
+					FeatureGate: &configv1.FeatureGateSpec{
+						FeatureGateSelection: configv1.FeatureGateSelection{
+							FeatureSet: configv1.Default,
+						},
+					},
+				}
+				return hcp
+			}(),
+			configContains:    "",
+			configNotContains: "NLBSecurityGroupMode",
+		},
+		{
+			name: "With TechPreviewNoUpgrade feature set it should have NLBSecurityGroupMode set to Managed in the config",
+			hcp: func() *hyperv1.HostedControlPlane {
+				hcp := newTestHCP(map[string]string{})
+				hcp.Spec.Configuration = &hyperv1.ClusterConfiguration{
+					FeatureGate: &configv1.FeatureGateSpec{
+						FeatureGateSelection: configv1.FeatureGateSelection{
+							FeatureSet: configv1.TechPreviewNoUpgrade,
+						},
+					},
+				}
+				return hcp
+			}(),
+			configContains: "NLBSecurityGroupMode = Managed",
+		},
+		{
+			name: "With CustomNoUpgrade feature set it should have NLBSecurityGroupMode set to Managed in the config when the feature gate is enabled",
+			hcp: func() *hyperv1.HostedControlPlane {
+				hcp := newTestHCP(map[string]string{})
+				hcp.Spec.Configuration = &hyperv1.ClusterConfiguration{
+					FeatureGate: &configv1.FeatureGateSpec{
+						FeatureGateSelection: configv1.FeatureGateSelection{
+							FeatureSet: configv1.CustomNoUpgrade,
+							CustomNoUpgrade: &configv1.CustomFeatureGates{
+								Enabled: []configv1.FeatureGateName{
+									"AWSServiceLBNetworkSecurityGroup",
+								},
+							},
+						},
+					},
+				}
+				return hcp
+			}(),
+			configContains: "NLBSecurityGroupMode = Managed",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Ensure global feature gate is set to Default to verify that adaptConfig
+			// reads the feature set from HCP.Spec.Configuration, not the global gate.
+			// This proves the fix for the issue where the global gate was incorrectly
+			// used instead of the per-cluster configuration.
+			featuregates.ConfigureFeatureSet(string(configv1.Default))
+
+			cm := &corev1.ConfigMap{}
+			_, _, err := assets.LoadManifestInto(ComponentName, "config.yaml", cm)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			cpContext := component.WorkloadContext{
+				HCP: tt.hcp,
+			}
+			err = adaptConfig(cpContext, cm)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			// Verify expected content is present (if specified)
+			if tt.configContains != "" && !strings.Contains(cm.Data[configKey], tt.configContains) {
+				t.Fatalf("expected config to contain %q, got: %s", tt.configContains, cm.Data[configKey])
+			}
+			// Verify unexpected content is absent (if specified)
+			if tt.configNotContains != "" && strings.Contains(cm.Data[configKey], tt.configNotContains) {
+				t.Fatalf("expected config to NOT contain %q, got: %s", tt.configNotContains, cm.Data[configKey])
+			}
+		})
+	}
+}

control-plane-operator/featuregates/featuregates.go

@@ -12,6 +12,7 @@ import (
 const (
 	ExternalOIDCWithUIDAndExtraClaimMappings featuregate.Feature = "ExternalOIDCWithUIDAndExtraClaimMappings"
 	ExternalOIDCWithUpstreamParity           featuregate.Feature = "ExternalOIDCWithUpstreamParity"
+	AWSServiceLBNetworkSecurityGroup         featuregate.Feature = "AWSServiceLBNetworkSecurityGroup"
 )
 
 // Initialize new features here
@@ -20,12 +21,15 @@ var (
 
 	externalOIDCWithUIDAndExtraClaimMappingsFeature = featuregates.NewFeature(ExternalOIDCWithUIDAndExtraClaimMappings, featuregates.WithEnableForFeatureSets(configv1.TechPreviewNoUpgrade, configv1.Default))
 	externalOIDCWithUpstreamParityFeature           = featuregates.NewFeature(ExternalOIDCWithUpstreamParity, featuregates.WithEnableForFeatureSets(configv1.TechPreviewNoUpgrade))
+	awsServiceLBNetworkSecurityGroupFeature         = featuregates.NewFeature(AWSServiceLBNetworkSecurityGroup, featuregates.WithEnableForFeatureSets(configv1.TechPreviewNoUpgrade))
 )
 
 func init() {
 	// Add featuregates here
 	allFeatures.AddFeature(externalOIDCWithUIDAndExtraClaimMappingsFeature)
 	allFeatures.AddFeature(externalOIDCWithUpstreamParityFeature)
+	allFeatures.AddFeature(awsServiceLBNetworkSecurityGroupFeature)
+
 	// Default to configuring the Default featureset
 	ConfigureFeatureSet(string(configv1.Default))
 }
@@ -55,3 +59,57 @@ func ConfigureFeatureSet(featureSet string) {
 
 	globalGate = featureGate
 }
+
+// IsFeatureEnabledInFeatureGateSpec checks if a feature gate is enabled based on a FeatureGateSpec.
+// This allows checking feature gates for a given configuration without changing the global gate.
+// It handles both fixed feature sets (Default, TechPreviewNoUpgrade, DevPreviewNoUpgrade) and
+// custom feature sets (CustomNoUpgrade) where features are explicitly enabled/disabled.
+// If the feature gate spec is nil, it falls back to checking the global gate.
+func IsFeatureEnabledInFeatureGateSpec(featureGateSpec *configv1.FeatureGateSpec, feature featuregate.Feature) bool {
+	if featureGateSpec == nil {
+		// No feature gate configuration, fall back to global gate
+		return globalGate.Enabled(feature)
+	}
+
+	featureSet := featureGateSpec.FeatureSet
+
+	// Handle CustomNoUpgrade feature sets by checking the explicit enabled/disabled lists
+	if featureSet == configv1.CustomNoUpgrade {
+		if featureGateSpec.CustomNoUpgrade == nil {
+			// CustomNoUpgrade without custom configuration, fall back to global gate
+			return globalGate.Enabled(feature)
+		}
+
+		featureName := configv1.FeatureGateName(feature)
+
+		// Check if explicitly disabled
+		for _, disabled := range featureGateSpec.CustomNoUpgrade.Disabled {
+			if disabled == featureName {
+				return false
+			}
+		}
+
+		// Check if explicitly enabled
+		for _, enabled := range featureGateSpec.CustomNoUpgrade.Enabled {
+			if enabled == featureName {
+				return true
+			}
+		}
+
+		// Not in either list, fall back to the feature's default for Default feature set
+		featureGate, err := allFeatures.FeatureGatesForFeatureSet(configv1.Default)
+		if err != nil {
+			return globalGate.Enabled(feature)
+		}
+		return featureGate.Enabled(feature)
+	}
+
+	// Handle fixed feature sets (Default, TechPreviewNoUpgrade, DevPreviewNoUpgrade)
+	featureGate, err := allFeatures.FeatureGatesForFeatureSet(featureSet)
+	if err != nil {
+		// If there's an error (e.g., unknown feature set), fall back to checking the global gate
+		return globalGate.Enabled(feature)
+	}
+
+	return featureGate.Enabled(feature)
+}