Deprecate boolean associatePublicIp and add AssociateIpStrategy enum for IP assignment control (#1148)

* Deprecate boolean `associatePublicIp` and add `AssociateIpStrategy` enum for IP assignment control
JENKINS-75002

Currently, public IP assignment behavior is inconsistent:

- `associatePublicIp=true`:
  - Creates a `NetworkInterface`
  - Sets `associatePublicIpAddress(true)`
  - Configures subnet and security groups on the interface

- `associatePublicIp=false`:
  - **On-demand**:
    - Does *not* use a `NetworkInterface`
    - Configures subnet/security groups at the RunInstance level
    - Public IP assignment depends on the subnet's `MapPublicIpOnLaunch` setting (inherited)
  - **Spot**:
    - Uses a `NetworkInterface`
    - Sets `associatePublicIpAddress(false)`, enforcing *no* public IP (does *not* inherit)

This prevents reliable enforcement of "no public IP" via AWS Service Control Policies.
The current logic follows [PR 447](#447), which reverted [JENKINS-58578](https://issues.jenkins.io/browse/JENKINS-58578), losing the ability of `associatePublicIp=false` to always disable public IP assignment.

- **Always create** the primary `NetworkInterface`.
- Replace the boolean `associatePublicIp` with the `AssociateIpStrategy` enum:
    - `PUBLIC_IP`: Always assign a public IP (`associatePublicIpAddress=true`)
    - `PRIVATE_IP`: Never assign a public IP (`associatePublicIpAddress=false`)
    - `SUBNET`: Inherit the subnet's `MapPublicIpOnLaunch` (leave unset)
    - `DEFAULT`: Matches current behavior: `PRIVATE_IP` for spot, `SUBNET` for on-demand

The `DEFAULT` option maintains backward compatibility and is used as the UI default.

* permission check in AssociateIPStrategy requests

* fix javadoc links

* Use label 'Associate IP Strategy' instead of 'Associate IP'

Co-authored-by: Jérôme Pochat <[email protected]>

---------

Co-authored-by: Jérôme Pochat <[email protected]>

Author: apuig

README.md

@@ -315,7 +315,7 @@ def sshPortToConnectWith = '22'
 // store parameters
 def slaveTemplateUsEast1Parameters = [
   ami:                           'ami-AAAAAAAA',
-  associatePublicIp:             false,
+  associateIpStrategy:           AssociateIpStrategy.valueOf('PRIVATE_IP'),
   spotConfig:                    null,
   connectBySSHProcess:           false,
   connectUsingPublicIp:          false,
@@ -445,7 +445,7 @@ SlaveTemplate slaveTemplateUsEast1 = new SlaveTemplate(
   slaveTemplateUsEast1Parameters.deleteRootOnTermination,
   slaveTemplateUsEast1Parameters.useEphemeralDevices,
   slaveTemplateUsEast1Parameters.launchTimeoutStr,
-  slaveTemplateUsEast1Parameters.associatePublicIp,
+  slaveTemplateUsEast1Parameters.associateIpStrategy,
   slaveTemplateUsEast1Parameters.customDeviceMapping,
   slaveTemplateUsEast1Parameters.connectBySSHProcess,
   slaveTemplateUsEast1Parameters.monitoring,

src/main/java/hudson/plugins/ec2/AssociateIPStrategy.java

@@ -0,0 +1,34 @@
+package hudson.plugins.ec2;
+
+/**
+ *
+ * Strategy for associating a public IPv4 address with the instance’s primary network interface at launch.
+ *
+ * @see <a href="https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_InstanceNetworkInterfaceSpecification.html">AWS Network Interface Specification</a>
+ * @see <a href="https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_Subnet.html">AWS Subnet API</a>
+ * */
+public enum AssociateIPStrategy {
+    SUBNET("Inherit from Subnet"),
+    PUBLIC_IP("Public IP"),
+    PRIVATE_IP("Private IP"),
+    DEFAULT("Default");
+
+    private final String displayText;
+
+    AssociateIPStrategy(String displayText) {
+        this.displayText = displayText;
+    }
+
+    public String getDisplayText() {
+        return this.displayText;
+    }
+
+    /**
+     * For backwards compatibility.
+     * @param associatePublicIp whether or not to use a public ip to establish a connection.
+     * @return an {@link AssociateIPStrategy} based on provided parameters that keeps {@code associatePublicIp} behavior.
+     */
+    public static AssociateIPStrategy backwardsCompatible(boolean associatePublicIp) {
+        return associatePublicIp ? PUBLIC_IP : DEFAULT;
+    }
+}

src/main/java/hudson/plugins/ec2/SlaveTemplate.java

@@ -217,7 +217,10 @@ public class SlaveTemplate implements Describable<SlaveTemplate> {
 
     public HostKeyVerificationStrategyEnum hostKeyVerificationStrategy;
 
-    public final boolean associatePublicIp;
+    public AssociateIPStrategy associateIPStrategy;
+
+    @Deprecated
+    public transient boolean associatePublicIp;
 
     protected transient EC2Cloud parent;
 
@@ -325,7 +328,7 @@ public SlaveTemplate(
             boolean deleteRootOnTermination,
             boolean useEphemeralDevices,
             String launchTimeoutStr,
-            boolean associatePublicIp,
+            AssociateIPStrategy associateIPStrategy,
             String customDeviceMapping,
             boolean connectBySSHProcess,
             boolean monitoring,
@@ -382,7 +385,6 @@ public SlaveTemplate(
         this.subnetId = subnetId;
         this.tags = tags;
         this.idleTerminationMinutes = idleTerminationMinutes;
-        this.associatePublicIp = associatePublicIp;
         this.connectionStrategy = connectionStrategy == null ? ConnectionStrategy.PRIVATE_IP : connectionStrategy;
         this.useDedicatedTenancy = tenancy == Tenancy.Dedicated;
         this.connectBySSHProcess = connectBySSHProcess;
@@ -431,9 +433,106 @@ public SlaveTemplate(
         this.metadataHopsLimit =
                 metadataHopsLimit != null ? metadataHopsLimit : EC2AbstractSlave.DEFAULT_METADATA_HOPS_LIMIT;
         this.enclaveEnabled = enclaveEnabled != null ? enclaveEnabled : EC2AbstractSlave.DEFAULT_ENCLAVE_ENABLED;
+        this.associateIPStrategy = associateIPStrategy != null ? associateIPStrategy : AssociateIPStrategy.DEFAULT;
+
         readResolve(); // initialize
     }
 
+    @Deprecated
+    public SlaveTemplate(
+            String ami,
+            String zone,
+            SpotConfiguration spotConfig,
+            String securityGroups,
+            String remoteFS,
+            String type,
+            boolean ebsOptimized,
+            String labelString,
+            Node.Mode mode,
+            String description,
+            String initScript,
+            String tmpDir,
+            String userData,
+            String numExecutors,
+            String remoteAdmin,
+            AMITypeData amiType,
+            String javaPath,
+            String jvmopts,
+            boolean stopOnTerminate,
+            String subnetId,
+            List<EC2Tag> tags,
+            String idleTerminationMinutes,
+            int minimumNumberOfInstances,
+            int minimumNumberOfSpareInstances,
+            String instanceCapStr,
+            String iamInstanceProfile,
+            boolean deleteRootOnTermination,
+            boolean useEphemeralDevices,
+            String launchTimeoutStr,
+            boolean associatePublicIp,
+            String customDeviceMapping,
+            boolean connectBySSHProcess,
+            boolean monitoring,
+            boolean t2Unlimited,
+            ConnectionStrategy connectionStrategy,
+            int maxTotalUses,
+            List<? extends NodeProperty<?>> nodeProperties,
+            HostKeyVerificationStrategyEnum hostKeyVerificationStrategy,
+            Tenancy tenancy,
+            EbsEncryptRootVolume ebsEncryptRootVolume,
+            Boolean metadataEndpointEnabled,
+            Boolean metadataTokensRequired,
+            Integer metadataHopsLimit,
+            Boolean metadataSupported,
+            Boolean enclaveEnabled) {
+        this(
+                ami,
+                zone,
+                spotConfig,
+                securityGroups,
+                remoteFS,
+                InstanceType.fromValue(type.toString()).toString(),
+                ebsOptimized,
+                labelString,
+                mode,
+                description,
+                initScript,
+                tmpDir,
+                userData,
+                numExecutors,
+                remoteAdmin,
+                amiType,
+                javaPath,
+                jvmopts,
+                stopOnTerminate,
+                subnetId,
+                tags,
+                idleTerminationMinutes,
+                minimumNumberOfInstances,
+                minimumNumberOfSpareInstances,
+                instanceCapStr,
+                iamInstanceProfile,
+                deleteRootOnTermination,
+                useEphemeralDevices,
+                launchTimeoutStr,
+                AssociateIPStrategy.backwardsCompatible(associatePublicIp),
+                customDeviceMapping,
+                connectBySSHProcess,
+                monitoring,
+                t2Unlimited,
+                connectionStrategy,
+                maxTotalUses,
+                nodeProperties,
+                hostKeyVerificationStrategy,
+                tenancy,
+                ebsEncryptRootVolume,
+                metadataEndpointEnabled,
+                metadataTokensRequired,
+                metadataHopsLimit,
+                metadataSupported,
+                enclaveEnabled);
+    }
+
     @Deprecated
     public SlaveTemplate(
             String ami,
@@ -1640,24 +1739,29 @@ public String getCurrentSubnetId() {
         return currentSubnetId;
     }
 
+    @Deprecated
     public boolean getAssociatePublicIp() {
-        return associatePublicIp;
+        return AssociateIPStrategy.PUBLIC_IP == associateIPStrategy;
+    }
+
+    public AssociateIPStrategy getAssociateIPStrategy() {
+        return associateIPStrategy;
     }
 
     @Deprecated
     @DataBoundSetter
     public void setConnectUsingPublicIp(boolean connectUsingPublicIp) {
         this.connectUsingPublicIp = connectUsingPublicIp;
         this.connectionStrategy = ConnectionStrategy.backwardsCompatible(
-                this.usePrivateDnsName, this.connectUsingPublicIp, this.associatePublicIp);
+                this.usePrivateDnsName, this.connectUsingPublicIp, getAssociatePublicIp());
     }
 
     @Deprecated
     @DataBoundSetter
     public void setUsePrivateDnsName(boolean usePrivateDnsName) {
         this.usePrivateDnsName = usePrivateDnsName;
         this.connectionStrategy = ConnectionStrategy.backwardsCompatible(
-                this.usePrivateDnsName, this.connectUsingPublicIp, this.associatePublicIp);
+                this.usePrivateDnsName, this.connectUsingPublicIp, getAssociatePublicIp());
     }
 
     @Deprecated
@@ -2011,11 +2115,7 @@ HashMap<RunInstancesRequest, List<Filter>> makeRunInstancesRequestAndFilters(
 
         InstanceNetworkInterfaceSpecification.Builder netBuilder = InstanceNetworkInterfaceSpecification.builder();
         if (StringUtils.isNotBlank(subnetId)) {
-            if (getAssociatePublicIp()) {
-                netBuilder.subnetId(subnetId);
-            } else {
-                riRequestBuilder.subnetId(subnetId);
-            }
+            netBuilder.subnetId(subnetId);
 
             diFilters.add(Filter.builder().name("subnet-id").values(subnetId).build());
 
@@ -2026,11 +2126,7 @@ HashMap<RunInstancesRequest, List<Filter>> makeRunInstancesRequestAndFilters(
                 List<String> groupIds = getEc2SecurityGroups(ec2);
 
                 if (!groupIds.isEmpty()) {
-                    if (getAssociatePublicIp()) {
-                        netBuilder.groups(groupIds);
-                    } else {
-                        riRequestBuilder.securityGroupIds(groupIds);
-                    }
+                    netBuilder.groups(groupIds);
 
                     diFilters.add(Filter.builder()
                             .name("instance.group-id")
@@ -2042,11 +2138,8 @@ HashMap<RunInstancesRequest, List<Filter>> makeRunInstancesRequestAndFilters(
             List<String> groupIds = getSecurityGroupsBy("group-name", securityGroupSet, ec2).securityGroups().stream()
                     .map(SecurityGroup::groupId)
                     .collect(Collectors.toList());
-            if (getAssociatePublicIp()) {
-                netBuilder.groups(groupIds);
-            } else {
-                riRequestBuilder.securityGroups(securityGroupSet);
-            }
+            netBuilder.groups(groupIds);
+
             if (!groupIds.isEmpty()) {
                 diFilters.add(Filter.builder()
                         .name("instance.group-id")
@@ -2055,13 +2148,21 @@ HashMap<RunInstancesRequest, List<Filter>> makeRunInstancesRequestAndFilters(
             }
         }
 
-        netBuilder.associatePublicIpAddress(getAssociatePublicIp());
-        netBuilder.deviceIndex(0);
-
-        if (getAssociatePublicIp()) {
-            riRequestBuilder.networkInterfaces(netBuilder.build());
+        switch (getAssociateIPStrategy()) {
+            case PUBLIC_IP:
+                netBuilder.associatePublicIpAddress(true);
+                break;
+            case PRIVATE_IP:
+                netBuilder.associatePublicIpAddress(false);
+                break;
+            case SUBNET:
+            case DEFAULT:
+                break;
         }
 
+        netBuilder.deviceIndex(0);
+        riRequestBuilder.networkInterfaces(netBuilder.build());
+
         HashSet<Tag> instTags = buildTags(EC2Cloud.EC2_SLAVE_TYPE_DEMAND);
         for (Tag tag : instTags) {
             diFilters.add(Filter.builder()
@@ -2505,7 +2606,18 @@ private List<EC2AbstractSlave> provisionSpot(Image image, int number, EnumSet<Pr
             launchSpecificationBuilder.keyName(keyPair.getKeyPairInfo().keyName());
             launchSpecificationBuilder.instanceType(type);
 
-            netBuilder.associatePublicIpAddress(getAssociatePublicIp());
+            switch (getAssociateIPStrategy()) {
+                case PUBLIC_IP:
+                    netBuilder.associatePublicIpAddress(true);
+                    break;
+                case PRIVATE_IP:
+                case DEFAULT:
+                    netBuilder.associatePublicIpAddress(false);
+                    break;
+                case SUBNET:
+                    break;
+            }
+
             netBuilder.deviceIndex(0);
             launchSpecificationBuilder.networkInterfaces(netBuilder.build());
 
@@ -2889,10 +3001,14 @@ protected Object readResolve() {
             type = InstanceTypeCompat.of(type).toString();
         }
 
+        if (associateIPStrategy == null) {
+            associateIPStrategy = AssociateIPStrategy.backwardsCompatible(associatePublicIp);
+        }
+
         // 1.43 new parameters
         if (connectionStrategy == null) {
-            connectionStrategy =
-                    ConnectionStrategy.backwardsCompatible(usePrivateDnsName, connectUsingPublicIp, associatePublicIp);
+            connectionStrategy = ConnectionStrategy.backwardsCompatible(
+                    usePrivateDnsName, connectUsingPublicIp, AssociateIPStrategy.PUBLIC_IP == associateIPStrategy);
         }
 
         if (maxTotalUses == 0) {
@@ -3384,6 +3500,34 @@ public FormValidation doCheckConnectionStrategy(@QueryParameter String connectio
                     .orElse(FormValidation.error("Could not find selected connection strategy"));
         }
 
+        @POST
+        public ListBoxModel doFillAssociateIPStrategyItems(@QueryParameter String associateIPStrategy) {
+            checkPermission(EC2Cloud.PROVISION);
+            return Stream.of(AssociateIPStrategy.values())
+                    .map(v -> {
+                        if (v.name().equals(associateIPStrategy)) {
+                            return new ListBoxModel.Option(v.getDisplayText(), v.name(), true);
+                        } else {
+                            return new ListBoxModel.Option(v.getDisplayText(), v.name(), false);
+                        }
+                    })
+                    .collect(Collectors.toCollection(ListBoxModel::new));
+        }
+
+        @POST
+        public FormValidation doCheckAssociateIPStrategy(@QueryParameter String associateIPStrategy) {
+            checkPermission(EC2Cloud.PROVISION);
+            return Stream.of(AssociateIPStrategy.values())
+                    .filter(v -> v.name().equals(associateIPStrategy))
+                    .findFirst()
+                    .map(s -> FormValidation.ok())
+                    .orElse(FormValidation.error("Could not find selected associate IP strategy"));
+        }
+
+        public String getDefaultAssociateIPStrategy() {
+            return AssociateIPStrategy.DEFAULT.name();
+        }
+
         public String getDefaultHostKeyVerificationStrategy() {
             // new templates default to the most secure strategy
             return HostKeyVerificationStrategyEnum.CHECK_NEW_HARD.name();

src/main/resources/hudson/plugins/ec2/SlaveTemplate/config.jelly

@@ -205,8 +205,8 @@ THE SOFTWARE.
       <f:textbox />
     </f:entry>
 
-    <f:entry title="${%Associate Public IP}" field="associatePublicIp">
-      <f:checkbox />
+    <f:entry title="${%Associate IP Strategy}" field="associateIPStrategy">
+      <f:select default="${descriptor.getDefaultAssociateIPStrategy()}"/>
     </f:entry>
 
     <f:entry title="${%Tenancy}" field="tenancy">

src/main/resources/hudson/plugins/ec2/SlaveTemplate/help-associateIPStrategy.html

@@ -0,0 +1,10 @@
+<div>
+	Strategy for associating a public IPv4 address with the instance’s primary network interface at launch.
+	<ul>
+		<li><strong>Public IP</strong>: Explicitly request a public IPv4 address.</li>
+		<li><strong>Private IP</strong>: Explicitly prevent association of a public IPv4 address.</li>
+		<li><strong>Inherit from Subnet</strong>: Choose this to delegate the decision to subnet configuration. 
+ 		Inherit the subnet’s behavior, allowing the subnet’s 'MapPublicIpOnLaunch' setting to decide whether a public IPv4 address is associated.</li>
+		<li><strong>Default</strong>: Existing behavior is preserved: Spot instances use 'Private IP' (explicitly false), while on‑demand instances use 'Inherit from Subnet' (omit the field and use subnet setting).</li>
+	</ul>
+</div>