mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 17:11:30 +00:00
fix(gateway): resolve DescribeRegions endpoint from the gateway's advertised host (#1066)
* fix(gateway): resolve DescribeRegions endpoint from the gateway's advertised host DescribeRegions hardcoded https://localhost:9999 as the Region's endpoint regardless of where the gateway actually runs, so a workload doing endpoint discovery through this API was pointed at its own loopback instead of a reachable address. The endpoint is now resolved by the caller from GatewayConfig.RegistryHost/RegistryPort, the same advertised host:port ECR already uses (concrete AWSGW bind host, else AdvertiseIP), and passed into DescribeRegions rather than invented there. Falls back to localhost:9999 only when no concrete host is configured, which is also the only case where that value is actually correct. Closes mulga-0bqih. * style: trim DescribeRegions endpoint comments
This commit is contained in:
+20
-1
@@ -5,6 +5,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"net"
|
||||
"net/http"
|
||||
"strings"
|
||||
"uuid"
|
||||
@@ -143,6 +144,24 @@ func ec2HandlerWithReq[In any](handler func(ctx context.Context, input *In, gw *
|
||||
}
|
||||
}
|
||||
|
||||
// advertisedEndpoint returns this gateway's own dialable base URL, from the
|
||||
// same RegistryHost/RegistryPort that already advertise ECR. Falls back to
|
||||
// localhost only when no concrete host is configured.
|
||||
func (gw *GatewayConfig) advertisedEndpoint() string {
|
||||
host := gw.RegistryHost
|
||||
if host == "" {
|
||||
host = "localhost"
|
||||
}
|
||||
port := gw.RegistryPort
|
||||
if port == "" {
|
||||
port = "9999"
|
||||
}
|
||||
if port == "443" {
|
||||
return "https://" + host
|
||||
}
|
||||
return "https://" + net.JoinHostPort(host, port)
|
||||
}
|
||||
|
||||
var ec2Actions = map[string]ec2Action{
|
||||
"DescribeInstances": ec2Handler(func(ctx context.Context, input *ec2.DescribeInstancesInput, gw *GatewayConfig, accountID string) (any, error) {
|
||||
out, err := gateway_ec2_instance.DescribeInstancesChecked(ctx, input, gw.NATSConn, 0, gw.NodeIDs, accountID)
|
||||
@@ -293,7 +312,7 @@ var ec2Actions = map[string]ec2Action{
|
||||
return gateway_ec2_image.ResetImageAttribute(ctx, input, gw.NATSConn, accountID)
|
||||
}),
|
||||
"DescribeRegions": ec2Handler(func(ctx context.Context, input *ec2.DescribeRegionsInput, gw *GatewayConfig, accountID string) (any, error) {
|
||||
return gateway_ec2_zone.DescribeRegions(input, gw.Region)
|
||||
return gateway_ec2_zone.DescribeRegions(input, gw.Region, gw.advertisedEndpoint())
|
||||
}),
|
||||
"DescribeAvailabilityZones": ec2Handler(func(ctx context.Context, input *ec2.DescribeAvailabilityZonesInput, gw *GatewayConfig, accountID string) (any, error) {
|
||||
return gateway_ec2_zone.DescribeAvailabilityZones(input, gw.Region, gw.AZ)
|
||||
|
||||
@@ -26,11 +26,14 @@ func DescribeAvailabilityZones(input *ec2.DescribeAvailabilityZonesInput, region
|
||||
return output, nil
|
||||
}
|
||||
|
||||
func DescribeRegions(input *ec2.DescribeRegionsInput, region string) (output *ec2.DescribeRegionsOutput, err error) {
|
||||
// DescribeRegions reports the current Region and a dialable endpoint. The
|
||||
// caller resolves endpoint from the gateway's advertised host so a remote
|
||||
// workload is never pointed at its own loopback.
|
||||
func DescribeRegions(input *ec2.DescribeRegionsInput, region string, endpoint string) (output *ec2.DescribeRegionsOutput, err error) {
|
||||
output = &ec2.DescribeRegionsOutput{
|
||||
Regions: []*ec2.Region{
|
||||
{
|
||||
Endpoint: aws.String("https://localhost:9999"),
|
||||
Endpoint: aws.String(endpoint),
|
||||
RegionName: aws.String(region),
|
||||
OptInStatus: aws.String("opt-in-not-required"),
|
||||
},
|
||||
|
||||
@@ -51,30 +51,35 @@ func TestDescribeAvailabilityZones(t *testing.T) {
|
||||
|
||||
func TestDescribeRegions(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
region string
|
||||
name string
|
||||
region string
|
||||
endpoint string
|
||||
}{
|
||||
{
|
||||
name: "Sydney",
|
||||
region: "ap-southeast-2",
|
||||
name: "Sydney",
|
||||
region: "ap-southeast-2",
|
||||
endpoint: "https://10.0.0.5:9999",
|
||||
},
|
||||
{
|
||||
name: "US East",
|
||||
region: "us-east-1",
|
||||
name: "US East",
|
||||
region: "us-east-1",
|
||||
endpoint: "https://gw.example.com:9999",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
input := &ec2.DescribeRegionsInput{}
|
||||
output, err := DescribeRegions(input, tt.region)
|
||||
output, err := DescribeRegions(input, tt.region, tt.endpoint)
|
||||
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, output)
|
||||
require.Len(t, output.Regions, 1)
|
||||
|
||||
region := output.Regions[0]
|
||||
assert.Equal(t, "https://localhost:9999", *region.Endpoint)
|
||||
// The endpoint is resolved by the caller and must be echoed back
|
||||
// verbatim, never a literal this function invents itself.
|
||||
assert.Equal(t, tt.endpoint, *region.Endpoint)
|
||||
assert.Equal(t, tt.region, *region.RegionName)
|
||||
assert.Equal(t, "opt-in-not-required", *region.OptInStatus)
|
||||
})
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
package gateway
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestAdvertisedEndpoint_ResolvesFromRegistryHost asserts DescribeRegions'
|
||||
// endpoint tracks this gateway's own reachable host:port (the same
|
||||
// RegistryHost/RegistryPort precedence ecrRegistryHost already applies),
|
||||
// rather than a literal that never changes with deployment.
|
||||
func TestAdvertisedEndpoint_ResolvesFromRegistryHost(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
registryHost string
|
||||
port string
|
||||
want string
|
||||
}{
|
||||
{"no concrete host falls back to localhost:9999", "", "", "https://localhost:9999"},
|
||||
{"concrete host, default port", "10.0.0.5", "", "https://10.0.0.5:9999"},
|
||||
{"concrete host, explicit port", "10.0.0.5", "9999", "https://10.0.0.5:9999"},
|
||||
{"concrete host, 443 omits port", "gw.example.com", "443", "https://gw.example.com"},
|
||||
{"concrete host, custom port", "gw.example.com", "8443", "https://gw.example.com:8443"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
gw := &GatewayConfig{RegistryHost: tc.registryHost, RegistryPort: tc.port}
|
||||
assert.Equal(t, tc.want, gw.advertisedEndpoint())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestDescribeRegions_UsesAdvertisedEndpoint drives the real dispatch entry
|
||||
// EC2_Request uses and asserts the resolved gateway address reaches the wire
|
||||
// response. A caller on a concrete-host deployment must never be told to
|
||||
// dial its own loopback, which is the defect this guards against.
|
||||
func TestDescribeRegions_UsesAdvertisedEndpoint(t *testing.T) {
|
||||
h := ec2Actions["DescribeRegions"]
|
||||
input, err := h.parse(map[string]string{})
|
||||
require.NoError(t, err)
|
||||
|
||||
gw := &GatewayConfig{Region: "ap-southeast-2", RegistryHost: "10.0.0.5", RegistryPort: "9999"}
|
||||
xmlOutput, err := h.dispatch("DescribeRegions", input, gw, "acct-123", nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
body := string(xmlOutput)
|
||||
assert.Contains(t, body, "<regionEndpoint>https://10.0.0.5:9999</regionEndpoint>")
|
||||
assert.NotContains(t, body, "localhost", "must not advertise the gateway's own loopback")
|
||||
}
|
||||
Reference in New Issue
Block a user