Merge pull request #426 from tthvo/CORS-4209

CORS-4209: ensure NodeIPFamilies field is kept as-is after transforming

Author: openshift-merge-bot[bot]

pkg/cloud/aws/aws_config_transformer.go

@@ -66,16 +66,6 @@ func marshalAWSConfig(cfg *awsconfig.CloudConfig) (string, error) {
 		}
 	}
 
-	for _, section := range file.Sections() {
-		for key, value := range section.KeysHash() {
-			// Ignore anything that is the zero value for its type.
-			// Everything appears as a string in the INI file, so 0 and false are also considered zero values.
-			if value == "" {
-				section.DeleteKey(key)
-			}
-		}
-	}
-
 	// Ensure service override sections are last and ordered numerically.
 	// 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.
@@ -85,7 +75,8 @@ func marshalAWSConfig(cfg *awsconfig.CloudConfig) (string, error) {
 	})
 
 	// Create a new INI file with sections in sorted order
-	sortedFile := ini.Empty()
+	// Configure iniv1 to allow shadow fields to enable multiple entries of NodeIPFamilies.
+	sortedFile := ini.Empty(ini.LoadOptions{AllowShadows: true})
 	for _, section := range sections {
 		newSection, err := sortedFile.NewSection(section.Name())
 		if err != nil {
@@ -96,6 +87,35 @@ func marshalAWSConfig(cfg *awsconfig.CloudConfig) (string, error) {
 		}
 	}
 
+	// In dual-stack environment, the CCM expects NodeIPFamilies to be in the format:
+	//
+	// NodeIPFamilies=ipv4
+	// NodeIPFamilies=ipv6
+	//
+	// However, iniv1 is serializing go slices as comma-separated list, for example:
+	//
+	// NodeIPFamilies=ipv4,ipv6
+	//
+	// Below logic ensures the original NodeIPFamilies field is kept as-is after transforming.
+	nodeIPKey := sortedFile.Section("Global").Key("NodeIPFamilies")
+	for i, ipFamily := range cfg.Global.NodeIPFamilies {
+		if i == 0 {
+			nodeIPKey.SetValue(ipFamily)
+		} else if err := nodeIPKey.AddShadow(ipFamily); err != nil {
+			return "", fmt.Errorf("failed to set NodeIPFamilies: %w", err)
+		}
+	}
+
+	for _, section := range sortedFile.Sections() {
+		for key, value := range section.KeysHash() {
+			// Ignore anything that is the zero value for its type.
+			// Everything appears as a string in the INI file, so 0 and false are also considered zero values.
+			if value == "" {
+				section.DeleteKey(key)
+			}
+		}
+	}
+
 	buf := &bytes.Buffer{}
 
 	if _, err := sortedFile.WriteTo(buf); err != nil {

pkg/cloud/aws/aws_config_transformer_test.go

@@ -183,6 +183,34 @@ ClusterServiceSharedLoadBalancerHealthProbePort = 0
 `,
 			features: mockDisabledFeatureGates,
 		},
+		{
+			name: "with NodeIPFamilies with ipv4 first",
+			source: `[Global]
+NodeIPFamilies = ipv4
+NodeIPFamilies = ipv6
+			`,
+			expected: `[Global]
+DisableSecurityGroupIngress                     = false
+NodeIPFamilies                                  = ipv4
+NodeIPFamilies                                  = ipv6
+ClusterServiceLoadBalancerHealthProbeMode       = Shared
+ClusterServiceSharedLoadBalancerHealthProbePort = 0
+`,
+		},
+		{
+			name: "with NodeIPFamilies with ipv6 first",
+			source: `[Global]
+NodeIPFamilies = ipv6
+NodeIPFamilies = ipv4
+			`,
+			expected: `[Global]
+DisableSecurityGroupIngress                     = false
+NodeIPFamilies                                  = ipv6
+NodeIPFamilies                                  = ipv4
+ClusterServiceLoadBalancerHealthProbeMode       = Shared
+ClusterServiceSharedLoadBalancerHealthProbePort = 0
+`,
+		},
 	}
 
 	for _, tc := range testCases {