Replace netaddr gem with stdlib IPAddr

The netaddr gem is effectively unmaintained (no releases despite open
requests). Security group rule validation only needs parsing, family
detection, and range-endpoint ordering, all of which IPAddr covers.

Behaviour is preserved:
- leading-zero IPv4 octets are normalised before parsing (IPAddr rejects
  them, netaddr accepted them);
- range endpoints must be plain addresses, so a prefix is rejected there.

Add a rule_validator spec covering the v2 validate_destination path,
which previously had no dedicated coverage.

Author: johha

Gemfile

@@ -12,7 +12,6 @@ gem 'httpclient'
 gem 'json-diff'
 gem 'json-schema'
 gem 'mime-types', '~> 3.7'
-gem 'netaddr', '>= 2.0.4'
 gem 'newrelic_rpm'
 gem 'nokogiri', '>=1.10.5'
 gem 'oj'

Gemfile.lock

@@ -158,7 +158,6 @@ GEM
     mutex_m (0.3.0)
     mysql2 (0.5.7)
       bigdecimal
-    netaddr (2.0.6)
     newrelic_rpm (10.6.0)
       logger
     nio4r (2.7.5)
@@ -459,7 +458,6 @@ DEPENDENCIES
   mime-types (~> 3.7)
   mock_redis
   mysql2 (~> 0.5.7)
-  netaddr (>= 2.0.4)
   newrelic_rpm
   nokogiri (>= 1.10.5)
   oj

app/messages/validators/security_group_rule_validator.rb

@@ -153,30 +153,33 @@ def validate_destination(destination, protocol, allowed_ip_version, record, inde
     if address_list.length == 1
       parsed_ip = CloudController::RuleValidator.parse_ip(address_list.first)
       add_rule_error(error_message, record, index) unless parsed_ip
-      add_rule_error("for protocol \"#{protocol}\" you cannot use IPv#{parsed_ip.version} addresses", record, index) \
+      add_rule_error("for protocol \"#{protocol}\" you cannot use IPv#{ip_version(parsed_ip)} addresses", record, index) \
         unless valid_ip_version?(allowed_ip_version, parsed_ip)
     elsif address_list.length == 2
       ips = CloudController::RuleValidator.parse_ip(address_list)
 
       return add_rule_error('destination IP address range is invalid', record, index) unless ips
 
-      sorted_ips = if ips.first.is_a?(NetAddr::IPv4)
-                     NetAddr.sort_IPv4(ips)
-                   else
-                     NetAddr.sort_IPv6(ips)
-                   end
+      sorted_ips = ips.sort
 
       reversed_range_error = 'beginning of IP address range is numerically greater than the end of its range (range endpoints are inverted)'
       add_rule_error(reversed_range_error, record, index) unless ips.first == sorted_ips.first
-      add_rule_error("for protocol \"#{protocol}\" you cannot use IPv#{ips.first.version} addresses", record, index) \
+      add_rule_error("for protocol \"#{protocol}\" you cannot use IPv#{ip_version(ips.first)} addresses", record, index) \
         unless valid_ip_version?(allowed_ip_version, sorted_ips.first)
     else
       add_rule_error(error_message, record, index)
     end
   end
 
+  def ip_version(parsed_ip)
+    return 4 if parsed_ip.ipv4?
+    return 6 if parsed_ip.ipv6?
+
+    raise ArgumentError.new("unsupported IP family: #{parsed_ip}")
+  end
+
   def valid_ip_version?(allowed_ip_version, parsed_ip)
-    parsed_ip.nil? || allowed_ip_version.nil? || parsed_ip.version == allowed_ip_version
+    parsed_ip.nil? || allowed_ip_version.nil? || ip_version(parsed_ip) == allowed_ip_version
   end
 
   def add_rule_error(message, record, index)

app/models/runtime/security_group.rb

@@ -1,5 +1,3 @@
-require 'netaddr'
-
 module VCAP::CloudController
   class SecurityGroup < Sequel::Model
     SECURITY_GROUP_NAME_REGEX = /\A[[:alnum:][:punct:][:print:]]+\Z/

lib/cloud_controller/rule_validator.rb

@@ -1,3 +1,5 @@
+require 'ipaddr'
+
 module CloudController
   class RuleValidator
     class_attribute :required_fields, :optional_fields
@@ -53,11 +55,7 @@ def self.validate_destination(destination)
         ips = parse_ip(address_list)
         return false if ips.nil?
 
-        sorted_ips = if ips.first.is_a?(NetAddr::IPv4)
-                       NetAddr.sort_IPv4(ips)
-                     else
-                       NetAddr.sort_IPv6(ips)
-                     end
+        sorted_ips = ips.sort
 
         return true if ips.first == sorted_ips.first
       end
@@ -122,26 +120,46 @@ def self.no_leading_zeros(destination)
 
     private_class_method def self.parse_ipv4(val)
       if val.is_a?(Array)
-        val.map do |ip|
-          NetAddr::IPv4.parse(ip)
-        end
+        val.map { |ip| parse_address(ip, :ipv4?) }
       else
-        NetAddr::IPv4Net.parse(val)
+        parse_cidr(val, :ipv4?)
       end
-    rescue NetAddr::ValidationError
+    rescue IPAddr::Error
       nil
     end
 
     private_class_method def self.parse_ipv6(val)
       if val.is_a?(Array)
-        val.map do |ip|
-          NetAddr::IPv6.parse(ip)
-        end
+        val.map { |ip| parse_address(ip, :ipv6?) }
       else
-        NetAddr::IPv6Net.parse(val)
+        parse_cidr(val, :ipv6?)
       end
-    rescue NetAddr::ValidationError
+    rescue IPAddr::Error
       nil
     end
+
+    # A range endpoint must be a plain address, so a prefix is not allowed here.
+    private_class_method def self.parse_address(val, family)
+      raise IPAddr::InvalidAddressError if val.include?('/')
+
+      parse_cidr(val, family)
+    end
+
+    private_class_method def self.parse_cidr(val, family)
+      ip = IPAddr.new(drop_leading_zeros(val))
+      raise IPAddr::InvalidAddressError unless ip.public_send(family)
+
+      ip
+    end
+
+    # IPAddr rejects zero-padded IPv4 octets (e.g. '010.0.0.53'); normalise them to plain decimals.
+    private_class_method def self.drop_leading_zeros(val)
+      return val unless val.include?('.')
+
+      addr, prefix = val.split('/', 2)
+      # For every octet, delete its leading zeros when a digit follows (e.g. '03.005.10.02' -> '3.5.10.2', but a lone '0' stays); non-numeric octets are left for IPAddr to reject.
+      addr = addr.split('.').map { |octet| octet.sub(/\A0+(?=\d)/, '') }.join('.')
+      prefix ? "#{addr}/#{prefix}" : addr
+    end
   end
 end

spec/unit/lib/cloud_controller/rule_validator_spec.rb

@@ -0,0 +1,86 @@
+require 'spec_helper'
+
+module CloudController
+  RSpec.describe RuleValidator do
+    describe '.validate_destination' do
+      subject { described_class.validate_destination(destination) }
+
+      before do
+        TestConfig.override(
+          enable_ipv6: true,
+          security_groups: { enable_comma_delimited_destinations: false }
+        )
+      end
+
+      context 'with a single valid IPv4 address' do
+        let(:destination) { '192.168.10.2' }
+
+        it { is_expected.to be true }
+      end
+
+      context 'with a valid CIDR' do
+        let(:destination) { '10.0.0.0/8' }
+
+        it { is_expected.to be true }
+      end
+
+      context 'with a valid ascending range' do
+        let(:destination) { '192.168.10.2-192.168.15.254' }
+
+        it { is_expected.to be true }
+      end
+
+      context 'with an inverted range' do
+        let(:destination) { '200.0.0.0-150.0.0.0' }
+
+        it { is_expected.to be false }
+      end
+
+      context 'with a range whose second endpoint is a CIDR' do
+        let(:destination) { '1.1.1.1-2.2.2.2/30' }
+
+        it { is_expected.to be false }
+      end
+
+      context 'with a malformed address' do
+        let(:destination) { '999.999.999.999' }
+
+        it { is_expected.to be false }
+      end
+
+      # This path deliberately does not run the no_leading_zeros check (unlike the v3 message
+      # validator), so zero-padded octets are accepted and normalised, matching prior behaviour.
+      context 'with leading zeros' do
+        context 'in the first octet' do
+          let(:destination) { '010.0.0.53' }
+
+          it { is_expected.to be true }
+        end
+
+        context 'in every octet' do
+          let(:destination) { '03.005.010.02' }
+
+          it { is_expected.to be true }
+        end
+
+        context 'in a CIDR' do
+          let(:destination) { '010.000.000.000/24' }
+
+          it { is_expected.to be true }
+        end
+      end
+
+      context 'with a valid IPv6 address' do
+        let(:destination) { '2001:db8::1' }
+
+        it { is_expected.to be true }
+      end
+
+      context 'with an inverted IPv6 range' do
+        let(:destination) { '2001:db8::ff-2001:db8::1' }
+
+        it { is_expected.to be false }
+      end
+    end
+  end
+end