diff --git a/go.mod b/go.mod index 8517b8ba5..0495968ea 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( k8s.io/apiextensions-apiserver v0.34.1 k8s.io/apimachinery v0.34.1 k8s.io/client-go v0.34.1 - k8s.io/cloud-provider-aws v1.34.1 + k8s.io/cloud-provider-aws v1.34.1-0.20250912204608-8a0025b4efb1 k8s.io/cloud-provider-vsphere v1.34.0 k8s.io/component-base v0.34.1 k8s.io/controller-manager v0.34.0 diff --git a/go.sum b/go.sum index cd4f7a619..ba83423d4 100644 --- a/go.sum +++ b/go.sum @@ -753,8 +753,8 @@ k8s.io/apiserver v0.34.1 h1:U3JBGdgANK3dfFcyknWde1G6X1F4bg7PXuvlqt8lITA= k8s.io/apiserver v0.34.1/go.mod h1:eOOc9nrVqlBI1AFCvVzsob0OxtPZUCPiUJL45JOTBG0= k8s.io/client-go v0.34.1 h1:ZUPJKgXsnKwVwmKKdPfw4tB58+7/Ik3CrjOEhsiZ7mY= k8s.io/client-go v0.34.1/go.mod h1:kA8v0FP+tk6sZA0yKLRG67LWjqufAoSHA2xVGKw9Of8= -k8s.io/cloud-provider-aws v1.34.1 h1:IbVH3Yg5QUrB6Uz0x/pZIP6GcmUB58FbZXPFUzfki6c= -k8s.io/cloud-provider-aws v1.34.1/go.mod h1:a8p1e6RHviJmZ/ZJK9S26CpZ07uv/jCZa93opvKSDA8= +k8s.io/cloud-provider-aws v1.34.1-0.20250912204608-8a0025b4efb1 h1:yXrYREaxoMix9I+xfgZxwgvRhr7vs0iZJGrvELXYpik= +k8s.io/cloud-provider-aws v1.34.1-0.20250912204608-8a0025b4efb1/go.mod h1:a8p1e6RHviJmZ/ZJK9S26CpZ07uv/jCZa93opvKSDA8= k8s.io/cloud-provider-vsphere v1.34.0 h1:XVn67iOmwld6pQI6DGCxwiA515VsHofIOUIwaVN8VLo= k8s.io/cloud-provider-vsphere v1.34.0/go.mod h1:W+o/i/LjkiXU3yNt8fXcoY97zroeOUfCk0vWtXfUJAQ= k8s.io/component-base v0.34.1 h1:v7xFgG+ONhytZNFpIz5/kecwD+sUhVE6HU7qQUiRM4A= diff --git a/pkg/cloud/aws/aws_config_transformer.go b/pkg/cloud/aws/aws_config_transformer.go index 92f96805b..60201aaa9 100644 --- a/pkg/cloud/aws/aws_config_transformer.go +++ b/pkg/cloud/aws/aws_config_transformer.go @@ -11,6 +11,7 @@ import ( "gopkg.in/ini.v1" awsconfig "k8s.io/cloud-provider-aws/pkg/providers/v1/config" + "k8s.io/klog/v2" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" ) @@ -28,7 +29,7 @@ func CloudConfigTransformer(source string, infra *configv1.Infrastructure, netwo return "", fmt.Errorf("failed to read the cloud.conf: %w", err) } - setOpenShiftDefaults(cfg) + setOpenShiftDefaults(cfg, features) return marshalAWSConfig(cfg) } @@ -76,24 +77,65 @@ func marshalAWSConfig(cfg *awsconfig.CloudConfig) (string, error) { } // Ensure service override sections are last and ordered numerically. - sort.Slice(file.Sections(), func(i, j int) bool { - return file.Sections()[i].Name() < file.Sections()[j].Name() + // We need to create a new file with sorted sections because file.Sections() + // returns a new slice each time, so sorting it doesn't modify the original. + sections := file.Sections() + sort.Slice(sections, func(i, j int) bool { + return sections[i].Name() < sections[j].Name() }) + // Create a new INI file with sections in sorted order + sortedFile := ini.Empty() + for _, section := range sections { + newSection, err := sortedFile.NewSection(section.Name()) + if err != nil { + return "", fmt.Errorf("failed to create section: %w", err) + } + for _, key := range section.Keys() { + newSection.Key(key.Name()).SetValue(key.Value()) + } + } + buf := &bytes.Buffer{} - if _, err := file.WriteTo(buf); err != nil { + if _, err := sortedFile.WriteTo(buf); err != nil { return "", fmt.Errorf("failed to write INI file: %w", err) } return buf.String(), nil } -func setOpenShiftDefaults(cfg *awsconfig.CloudConfig) { +// isFeatureGateEnabled safely checks if a feature gate is enabled without panicking +// if the feature is not registered. Returns false if features is nil or if the +// feature is not in the known features list. +func isFeatureGateEnabled(features featuregates.FeatureGate, featureName string) bool { + // features.Enabled returns panic if the feature is not registered in FeatureGates, + // this functions prevents the panic by returning false if the feature is not registered in FeatureGates. + if features == nil || len(featureName) == 0 { + return false + } + for _, known := range features.KnownFeatures() { + if string(known) == featureName { + return features.Enabled(known) + } + } + return false +} + +func setOpenShiftDefaults(cfg *awsconfig.CloudConfig, features featuregates.FeatureGate) { if cfg.Global.ClusterServiceLoadBalancerHealthProbeMode == "" { // OpenShift uses Shared mode by default. // This attaches the health check for Cluster scope services to the "kube-proxy" // health check endpoint served by OVN. cfg.Global.ClusterServiceLoadBalancerHealthProbeMode = "Shared" } + if isFeatureGateEnabled(features, "AWSServiceLBNetworkSecurityGroup") { + if cfg.Global.NLBSecurityGroupMode != awsconfig.NLBSecurityGroupModeManaged { + // When the feature gate AWSServiceLBNetworkSecurityGroup is enabled, + // OpenShift configures the AWS CCM to manage security groups for + // Network Load Balancer (NLB) Services. + klog.Infof("Enforcing cloud provider AWS configuration NLBSecurityGroupMode to Managed") + cfg.Global.NLBSecurityGroupMode = awsconfig.NLBSecurityGroupModeManaged + } + } } diff --git a/pkg/cloud/aws/aws_config_transformer_test.go b/pkg/cloud/aws/aws_config_transformer_test.go index 34c5116d8..edb1a1b5b 100644 --- a/pkg/cloud/aws/aws_config_transformer_test.go +++ b/pkg/cloud/aws/aws_config_transformer_test.go @@ -5,14 +5,81 @@ import ( . "github.com/onsi/gomega" + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" ) +var mockEmptyFeatureGates = featuregates.NewFeatureGate([]configv1.FeatureGateName{}, []configv1.FeatureGateName{}) +var mockEnabledFeatureGates = featuregates.NewFeatureGate([]configv1.FeatureGateName{"AWSServiceLBNetworkSecurityGroup"}, []configv1.FeatureGateName{}) +var mockDisabledFeatureGates = featuregates.NewFeatureGate([]configv1.FeatureGateName{}, []configv1.FeatureGateName{"AWSServiceLBNetworkSecurityGroup"}) + +func TestIsFeatureGateEnabled(t *testing.T) { + testCases := []struct { + name string + features featuregates.FeatureGate + featureName string + expected bool + }{ + { + name: "returns false when features is nil", + features: nil, + featureName: "AWSServiceLBNetworkSecurityGroup", + expected: false, + }, + { + name: "returns false when feature name is empty string", + features: mockEnabledFeatureGates, + featureName: "", + expected: false, + }, + { + name: "returns false when feature is not registered (empty feature gate)", + features: mockEmptyFeatureGates, + featureName: "AWSServiceLBNetworkSecurityGroup", + expected: false, + }, + { + name: "returns true when feature is enabled", + features: mockEnabledFeatureGates, + featureName: "AWSServiceLBNetworkSecurityGroup", + expected: true, + }, + { + name: "returns false when feature is explicitly disabled", + features: mockDisabledFeatureGates, + featureName: "AWSServiceLBNetworkSecurityGroup", + expected: false, + }, + { + name: "returns false when feature is not in known features list", + features: featuregates.NewFeatureGate([]configv1.FeatureGateName{"SomeOtherFeature"}, []configv1.FeatureGateName{}), + featureName: "AWSServiceLBNetworkSecurityGroup", + expected: false, + }, + { + name: "returns false for unknown feature with disabled features registered", + features: featuregates.NewFeatureGate([]configv1.FeatureGateName{}, []configv1.FeatureGateName{"SomeOtherFeature"}), + featureName: "AWSServiceLBNetworkSecurityGroup", + expected: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + result := isFeatureGateEnabled(tc.features, tc.featureName) + g.Expect(result).To(Equal(tc.expected), "Expected isFeatureGateEnabled to return %v for feature '%s'", tc.expected, tc.featureName) + }) + } +} + func TestCloudConfigTransformer(t *testing.T) { testCases := []struct { name string source string expected string + features featuregates.FeatureGate }{ { name: "default source", @@ -23,6 +90,7 @@ DisableSecurityGroupIngress = false ClusterServiceLoadBalancerHealthProbeMode = Shared ClusterServiceSharedLoadBalancerHealthProbePort = 0 `, + features: mockEmptyFeatureGates, }, { name: "completely empty source", @@ -32,6 +100,7 @@ DisableSecurityGroupIngress = false ClusterServiceLoadBalancerHealthProbeMode = Shared ClusterServiceSharedLoadBalancerHealthProbePort = 0 `, + features: mockEmptyFeatureGates, }, { name: "with existing configuration", @@ -45,6 +114,7 @@ DisableSecurityGroupIngress = true ClusterServiceLoadBalancerHealthProbeMode = Shared ClusterServiceSharedLoadBalancerHealthProbePort = 0 `, // Ordered based on the order of fields in the AWS CloudConfig struct. + features: mockEmptyFeatureGates, }, { name: "with existing configuration and overrides", @@ -82,6 +152,36 @@ Region = us-west-1 URL = https://s3.foo.bar SigningRegion = signing_region `, // Ordered based on the order of fields in the AWS CloudConfig struct. + features: mockEmptyFeatureGates, + }, + { + name: "with AWSServiceLBNetworkSecurityGroup feature gate enabled", + source: `[Global] +DisableSecurityGroupIngress = true +Zone = Foo +`, + expected: `[Global] +Zone = Foo +DisableSecurityGroupIngress = true +ClusterServiceLoadBalancerHealthProbeMode = Shared +ClusterServiceSharedLoadBalancerHealthProbePort = 0 +NLBSecurityGroupMode = Managed +`, + features: mockEnabledFeatureGates, + }, + { + name: "with AWSServiceLBNetworkSecurityGroup feature gate disabled", + source: `[Global] +DisableSecurityGroupIngress = true +Zone = Foo +`, + expected: `[Global] +Zone = Foo +DisableSecurityGroupIngress = true +ClusterServiceLoadBalancerHealthProbeMode = Shared +ClusterServiceSharedLoadBalancerHealthProbePort = 0 +`, + features: mockDisabledFeatureGates, }, } @@ -89,7 +189,7 @@ SigningRegion = signing_region t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - gotConfig, err := CloudConfigTransformer(tc.source, nil, nil, featuregates.NewFeatureGate(nil, nil)) // No Infra or Network are required for the current functionality. + gotConfig, err := CloudConfigTransformer(tc.source, nil, nil, tc.features) // No Infra or Network are required for the current functionality. g.Expect(err).ToNot(HaveOccurred()) g.Expect(gotConfig).To(Equal(tc.expected)) diff --git a/pkg/controllers/cloud_config_sync_controller_test.go b/pkg/controllers/cloud_config_sync_controller_test.go index 575318dd2..513e944e3 100644 --- a/pkg/controllers/cloud_config_sync_controller_test.go +++ b/pkg/controllers/cloud_config_sync_controller_test.go @@ -206,7 +206,7 @@ var _ = Describe("Cloud config sync controller", func() { ManagedNamespace: targetNamespaceName, }, Scheme: scheme.Scheme, - FeatureGateAccess: featuregates.NewHardcodedFeatureGateAccessForTesting(nil, nil, nil, nil), + FeatureGateAccess: featuregates.NewHardcodedFeatureGateAccessForTesting(nil, []configv1.FeatureGateName{"AWSServiceLBNetworkSecurityGroup"}, nil, nil), } Expect(reconciler.SetupWithManager(mgr)).To(Succeed()) @@ -408,7 +408,7 @@ var _ = Describe("Cloud config sync reconciler", func() { ManagedNamespace: targetNamespaceName, }, Scheme: scheme.Scheme, - FeatureGateAccess: featuregates.NewHardcodedFeatureGateAccessForTesting(nil, nil, nil, nil), + FeatureGateAccess: featuregates.NewHardcodedFeatureGateAccessForTesting(nil, []configv1.FeatureGateName{"AWSServiceLBNetworkSecurityGroup"}, nil, nil), } networkResource := makeNetworkResource() diff --git a/vendor/k8s.io/cloud-provider-aws/pkg/providers/v1/config/config.go b/vendor/k8s.io/cloud-provider-aws/pkg/providers/v1/config/config.go index 6dd65a208..557a125a1 100644 --- a/vendor/k8s.io/cloud-provider-aws/pkg/providers/v1/config/config.go +++ b/vendor/k8s.io/cloud-provider-aws/pkg/providers/v1/config/config.go @@ -25,6 +25,12 @@ const ( // ClusterServiceLoadBalancerHealthProbeModeServiceNodePort is the service node port health probe mode for cluster service load balancer. ClusterServiceLoadBalancerHealthProbeModeServiceNodePort = "ServiceNodePort" + + // NLBSecurityGroupModeManaged indicates the controller should automatically create and manage + // security groups for Network Load Balancer (NLB) services. When this mode is enabled, + // the controller creates a dedicated security group for each NLB service and configures + // ingress rules based on the service's port mappings and source ranges. + NLBSecurityGroupModeManaged = "Managed" ) // CloudConfig wraps the settings for the AWS cloud provider. @@ -99,6 +105,10 @@ type CloudConfig struct { // // WARNING: Updating the default behavior and corresponding unit tests would be a much safer option. SupportedTopologyInstanceTypePattern string `json:"supportedTopologyInstanceTypePattern,omitempty" yaml:"supportedTopologyInstanceTypePattern,omitempty"` + + // NLBSecurityGroupMode determines if the controller manages, creates, and attaches the security group when a service of type LoadBalancer (NLB) is created. + // Supported value is `Managed`. + NLBSecurityGroupMode string `json:"nlbSecurityGroupMode,omitempty" yaml:"nlbSecurityGroupMode,omitempty"` } // [ServiceOverride "1"] // Service = s3 @@ -209,6 +219,22 @@ func (cfg *CloudConfig) GetCustomEC2Resolver() ec2.EndpointResolverV2 { } } +// IsNLBSecurityGroupModeManaged checks if the NLBSecurityGroupMode is set to "Managed", +// returning an error if the mode is invalid. +// +// Returns: +// - bool: true if the mode is "Managed", otherwise false +// - error: nil if the mode is "Managed" or empty, otherwise an error is returned +func (cfg *CloudConfig) IsNLBSecurityGroupModeManaged() (bool, error) { + if len(cfg.Global.NLBSecurityGroupMode) == 0 { + return false, nil + } + if cfg.Global.NLBSecurityGroupMode == NLBSecurityGroupModeManaged { + return true, nil + } + return false, fmt.Errorf("invalid NLB security group mode: %q. Expected: %q", cfg.Global.NLBSecurityGroupMode, NLBSecurityGroupModeManaged) +} + // EC2Resolver overrides the endpoint for an AWS SDK Go V2 EC2 Client, // using the provided CloudConfig to determine if an override // is appropriate. diff --git a/vendor/modules.txt b/vendor/modules.txt index 348e81b95..6dcfb3545 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2097,7 +2097,7 @@ k8s.io/client-go/util/homedir k8s.io/client-go/util/keyutil k8s.io/client-go/util/retry k8s.io/client-go/util/workqueue -# k8s.io/cloud-provider-aws v1.34.1 +# k8s.io/cloud-provider-aws v1.34.1-0.20250912204608-8a0025b4efb1 ## explicit; go 1.24.4 k8s.io/cloud-provider-aws/pkg/providers/v1/config # k8s.io/cloud-provider-vsphere v1.34.0