unable to set AssociatePublicIpAddress for network interface to false

[tuananh]

In corporate context, we often have a SCP (service control policy) to block AssociatePublicIpAddress when set to true by default.

However, in machine-api-provider-aws, we have this code path

machine-api-provider-aws/pkg/actuators/machine/instances.go

Lines 364 to 389 in 318ae2f

	var networkInterfaces = []*ec2.InstanceNetworkInterfaceSpecification{
	{
	DeviceIndex: aws.Int64(machineProviderConfig.DeviceIndex),
	SubnetId: subnetID,
	Groups: securityGroupsIDs,
	},
	}
	// Public IP assignment is different in Wavelength Zones.
	// AvailabilityZone and LocalZone uses InternetGateway.
	// WavelengthZone uses Carrier Gateway.
	if aws.BoolValue(machineProviderConfig.PublicIP) {
	zoneName, err := getAvalabilityZoneFromSubnetID(*subnetID, awsClient)
	if err != nil {
	return nil, mapierrors.InvalidMachineConfiguration("error discoverying zone type: %v", err)
	}
	zoneType, err := getAvalabilityZoneTypeFromZoneName(zoneName, awsClient)
	if err != nil {
	return nil, mapierrors.InvalidMachineConfiguration("error discoverying zone type: %v", err)
	}
	if zoneType == "wavelength-zone" {
	networkInterfaces[0].AssociateCarrierIpAddress = machineProviderConfig.PublicIP
	} else {
	networkInterfaces[0].AssociatePublicIpAddress = machineProviderConfig.PublicIP
	}
	}

where as when we create new network interface, we did not explicity set AssociatePublicIpAddress. This field, when omited, AWS will use the default value which is true as documented here

Together with the if aws.BoolValue(machineProviderConfig.PublicIP) { only check, render us unable to set this value to false even if we set machineProviderConfig.PublicIP to false.

The fix for this could be

• Default value for network interface changes to AssociatePublicIpAddress to false.
• Or Add a else clause to handle false path.

What do you think?

[tuananh]

@theobarberbany any advice on how we should proceed? I can open a PR afterward.

[tuananh]

we tested with this minimal patch (default to false) and it works well for us

diff --git a/pkg/actuators/machine/instances.go b/pkg/actuators/machine/instances.go
index 69a47dc5..6211a523 100644
--- a/pkg/actuators/machine/instances.go
+++ b/pkg/actuators/machine/instances.go
@@ -365,6 +365,7 @@ func launchInstance(machine *machinev1beta1.Machine, machineProviderConfig *mach
                {
                        DeviceIndex: aws.Int64(machineProviderConfig.DeviceIndex),
                        SubnetId:    subnetID,
+                       AssociatePublicIpAddress: aws.Bool(false),
                        Groups:      securityGroupsIDs,
                },
        }