fix(member): reject non-IP address for members in F5-backed pools

A member with a hostname as its address causes the F5 AS3 declaration
driver to receive a value that violates the f5ip format constraint,
resulting in HTTP 422 on every declaration POST and blocking all F5
syncs globally.

Validate at the API layer (POST and PUT) that the address is a valid
IPv4 or IPv6 address when the member's pool is associated with an F5
domain. Non-F5-backed pools are unaffected.

Closes #1227

Author: ronchi-oss

internal/controller/member.go

@@ -7,11 +7,13 @@ package controller
 import (
 	"errors"
 	"fmt"
+	"net"
 	"strings"
 
 	"github.com/apex/log"
 
 	"github.com/go-openapi/runtime/middleware"
+	"github.com/go-openapi/strfmt"
 	"github.com/go-sql-driver/mysql"
 	"github.com/jackc/pgerrcode"
 	"github.com/jmoiron/sqlx"
@@ -90,6 +92,14 @@ func (c MemberController) PostMembers(params members.PostMembersParams) middlewa
 	member.PoolID = &pool.ID
 	member.ProjectID = &projectID
 
+	f5, err := poolHasF5Domain(c.db, pool.ID)
+	if err != nil {
+		panic(err)
+	}
+	if f5 && net.ParseIP(*member.Address) == nil {
+		return members.NewPostMembersBadRequest().WithPayload(utils.InvalidMemberAddressForF5)
+	}
+
 	// Set default values
 	if err := utils.SetModelDefaults(member); err != nil {
 		panic(err)
@@ -160,6 +170,16 @@ func (c MemberController) PutMembersMemberID(params members.PutMembersMemberIDPa
 		return members.NewPutMembersMemberIDBadRequest().WithPayload(utils.PoolIDImmutable)
 	}
 
+	if params.Member.Member.Address != nil {
+		f5, err := poolHasF5Domain(c.db, *member.PoolID)
+		if err != nil {
+			panic(err)
+		}
+		if f5 && net.ParseIP(*params.Member.Member.Address) == nil {
+			return members.NewPutMembersMemberIDBadRequest().WithPayload(utils.InvalidMemberAddressForF5)
+		}
+	}
+
 	params.Member.Member.ID = params.MemberID
 	if err := db.TxExecute(c.db, func(tx *sqlx.Tx) error {
 		sql := `
@@ -231,3 +251,16 @@ func PopulateMember(db *sqlx.DB, member *models.Member, fields []string) error {
 	}
 	return nil
 }
+
+func poolHasF5Domain(db *sqlx.DB, poolID strfmt.UUID) (bool, error) {
+	var count int
+	sql := db.Rebind(`
+		SELECT COUNT(d.id) FROM domain d
+		JOIN domain_pool_relation dpr ON d.id = dpr.domain_id
+		WHERE dpr.pool_id = ? AND d.provider = 'f5'
+	`)
+	if err := db.Get(&count, sql, poolID); err != nil {
+		return false, err
+	}
+	return count > 0, nil
+}

internal/controller/member_test.go

@@ -9,11 +9,35 @@ import (
 	"net/http/httptest"
 
 	"github.com/go-openapi/runtime"
+	"github.com/go-openapi/strfmt"
+	"github.com/go-openapi/swag/conv"
 	"github.com/stretchr/testify/assert"
 
+	"github.com/sapcc/andromeda/models"
+	"github.com/sapcc/andromeda/restapi/operations/domains"
 	"github.com/sapcc/andromeda/restapi/operations/members"
 )
 
+func (t *SuiteTest) createF5Domain() strfmt.UUID {
+	fqdn := strfmt.Hostname("f5test.com")
+	domain := domains.PostDomainsBody{
+		Domain: &models.Domain{
+			Fqdn:     &fqdn,
+			Name:     conv.Pointer("f5test"),
+			Provider: conv.Pointer("f5"),
+		},
+	}
+
+	res := t.c.Domains.PostDomains(domains.PostDomainsParams{Domain: domain})
+	rr := httptest.NewRecorder()
+	res.WriteResponse(rr, runtime.JSONProducer())
+	assert.Equal(t.T(), http.StatusCreated, rr.Code, rr.Body)
+
+	domainResponse := domains.PostDomainsCreatedBody{}
+	_ = domainResponse.UnmarshalBinary(rr.Body.Bytes())
+	return domainResponse.Domain.ID
+}
+
 func (t *SuiteTest) TestMembers() {
 	mc := t.c.Members
 	rr := httptest.NewRecorder()
@@ -76,3 +100,123 @@ func (t *SuiteTest) TestMembers() {
 		t.FailNow(err.Error())
 	}
 }
+
+func (t *SuiteTest) TestMembersF5AddressValidation() {
+	mc := t.c.Members
+	defer t.cleanupDomains()
+	defer t.cleanupPools()
+
+	// Pool with F5 domain attached
+	domainID := t.createF5Domain()
+	f5PoolID := t.createPool([]strfmt.UUID{domainID})
+
+	// Pool with Akamai domain attached
+	akamaiDomainID := t.createDomain()
+	akamaiPoolID := t.createPool([]strfmt.UUID{akamaiDomainID})
+
+	// Pool with no domain
+	plainPoolID := t.createPool(nil)
+
+	t.Run("F5 pool: valid IPv4 address is accepted", func() {
+		body := members.PostMembersBody{}
+		_ = body.UnmarshalBinary([]byte(`{ "member": { "address": "192.0.2.1", "port": 80 } }`))
+		body.Member.PoolID = &f5PoolID
+
+		res := mc.PostMembers(members.PostMembersParams{Member: body})
+		rr := httptest.NewRecorder()
+		res.WriteResponse(rr, runtime.JSONProducer())
+		assert.Equal(t.T(), http.StatusCreated, rr.Code, rr.Body)
+
+		// Cleanup member
+		resp := members.PostMembersCreatedBody{}
+		_ = resp.UnmarshalBinary(rr.Body.Bytes())
+		mc.DeleteMembersMemberID(members.DeleteMembersMemberIDParams{MemberID: resp.Member.ID})
+	})
+
+	t.Run("F5 pool: valid IPv6 address is accepted", func() {
+		body := members.PostMembersBody{}
+		_ = body.UnmarshalBinary([]byte(`{ "member": { "address": "::1", "port": 80 } }`))
+		body.Member.PoolID = &f5PoolID
+
+		res := mc.PostMembers(members.PostMembersParams{Member: body})
+		rr := httptest.NewRecorder()
+		res.WriteResponse(rr, runtime.JSONProducer())
+		assert.Equal(t.T(), http.StatusCreated, rr.Code, rr.Body)
+
+		resp := members.PostMembersCreatedBody{}
+		_ = resp.UnmarshalBinary(rr.Body.Bytes())
+		mc.DeleteMembersMemberID(members.DeleteMembersMemberIDParams{MemberID: resp.Member.ID})
+	})
+
+	t.Run("F5 pool: hostname address is rejected", func() {
+		body := members.PostMembersBody{}
+		_ = body.UnmarshalBinary([]byte(`{ "member": { "address": "my.server.example.com", "port": 80 } }`))
+		body.Member.PoolID = &f5PoolID
+
+		res := mc.PostMembers(members.PostMembersParams{Member: body})
+		rr := httptest.NewRecorder()
+		res.WriteResponse(rr, runtime.JSONProducer())
+		assert.Equal(t.T(), http.StatusBadRequest, rr.Code, rr.Body)
+	})
+
+	t.Run("Akamai pool: hostname address is allowed", func() {
+		body := members.PostMembersBody{}
+		_ = body.UnmarshalBinary([]byte(`{ "member": { "address": "my.server.example.com", "port": 80 } }`))
+		body.Member.PoolID = &akamaiPoolID
+
+		res := mc.PostMembers(members.PostMembersParams{Member: body})
+		rr := httptest.NewRecorder()
+		res.WriteResponse(rr, runtime.JSONProducer())
+		assert.Equal(t.T(), http.StatusCreated, rr.Code, rr.Body)
+
+		resp := members.PostMembersCreatedBody{}
+		_ = resp.UnmarshalBinary(rr.Body.Bytes())
+		mc.DeleteMembersMemberID(members.DeleteMembersMemberIDParams{MemberID: resp.Member.ID})
+	})
+
+	t.Run("Pool with no domain: hostname address is allowed", func() {
+		body := members.PostMembersBody{}
+		_ = body.UnmarshalBinary([]byte(`{ "member": { "address": "my.server.example.com", "port": 80 } }`))
+		body.Member.PoolID = &plainPoolID
+
+		res := mc.PostMembers(members.PostMembersParams{Member: body})
+		rr := httptest.NewRecorder()
+		res.WriteResponse(rr, runtime.JSONProducer())
+		assert.Equal(t.T(), http.StatusCreated, rr.Code, rr.Body)
+
+		resp := members.PostMembersCreatedBody{}
+		_ = resp.UnmarshalBinary(rr.Body.Bytes())
+		mc.DeleteMembersMemberID(members.DeleteMembersMemberIDParams{MemberID: resp.Member.ID})
+	})
+
+	t.Run("F5 pool: PUT with hostname address is rejected", func() {
+		// Create member with a valid IP first
+		createBody := members.PostMembersBody{}
+		_ = createBody.UnmarshalBinary([]byte(`{ "member": { "address": "10.0.0.1", "port": 80 } }`))
+		createBody.Member.PoolID = &f5PoolID
+
+		createRes := mc.PostMembers(members.PostMembersParams{Member: createBody})
+		createRR := httptest.NewRecorder()
+		createRes.WriteResponse(createRR, runtime.JSONProducer())
+		assert.Equal(t.T(), http.StatusCreated, createRR.Code, createRR.Body)
+
+		created := members.PostMembersCreatedBody{}
+		_ = created.UnmarshalBinary(createRR.Body.Bytes())
+		memberID := created.Member.ID
+
+		// Attempt to update address to a hostname
+		updateBody := members.PutMembersMemberIDBody{}
+		hostname := "bad.hostname.example.com"
+		updateBody.Member = &models.Member{Address: &hostname}
+
+		putRes := mc.PutMembersMemberID(members.PutMembersMemberIDParams{
+			MemberID: memberID,
+			Member:   updateBody,
+		})
+		putRR := httptest.NewRecorder()
+		putRes.WriteResponse(putRR, runtime.JSONProducer())
+		assert.Equal(t.T(), http.StatusBadRequest, putRR.Code, putRR.Body)
+
+		mc.DeleteMembersMemberID(members.DeleteMembersMemberIDParams{MemberID: memberID})
+	})
+}

internal/utils/errors.go

@@ -28,6 +28,7 @@ var (
 	MissingFQDN                  = &models.Error{Code: 400, Message: "invalid value for 'fqdn': 'fqdn' is required"}
 	MissingProvider              = &models.Error{Code: 400, Message: "invalid value for 'provider': 'provider' is required"}
 	MissingAddressOrPort         = &models.Error{Code: 400, Message: "invalid value for 'address' and 'port': 'address' and 'port' are required"}
+	InvalidMemberAddressForF5    = &models.Error{Code: 400, Message: "invalid value for 'address': must be a valid IPv4 or IPv6 address for pools associated with F5 domains"}
 	FQDNImmutable                = &models.Error{Code: 400, Message: "invalid value for 'fqdn': change of immutable attribute 'fqdn' not allowed"}
 	RestrictedDatacenterProvider = &models.Error{Code: 400, Message: "invalid value for 'provider': project-specific f5 datacenters are not supported; please use those with scope=public already available"}
 	MySQLForeignKeyViolation     = &mysql.MySQLError{Number: 1451}