Skip to content

Commit

Permalink
validate localities on agent configs and registration endpoints (#17712)
Browse files Browse the repository at this point in the history
  • Loading branch information
erichaberkorn authored Jun 15, 2023
1 parent 37bd0e1 commit 0994ccf
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 0 deletions.
4 changes: 4 additions & 0 deletions agent/config/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1290,6 +1290,10 @@ func (b *builder) validate(rt RuntimeConfig) error {
"1 and 63 bytes.", rt.NodeName)
}

if err := rt.StructLocality().Validate(); err != nil {
return fmt.Errorf("locality is invalid: %s", err)
}

if ipaddr.IsAny(rt.AdvertiseAddrLAN.IP) {
return fmt.Errorf("Advertise address cannot be 0.0.0.0, :: or [::]")
}
Expand Down
7 changes: 7 additions & 0 deletions agent/config/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1036,6 +1036,13 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
},
},
})
run(t, testCase{
desc: "locality invalid",
args: []string{`-data-dir=` + dataDir},
json: []string{`{"locality": {"zone": "us-west-1a"}}`},
hcl: []string{`locality { zone = "us-west-1a" }`},
expectedErr: "locality is invalid: zone cannot be set without region",
})
run(t, testCase{
desc: "client addr and ports == 0",
args: []string{`-data-dir=` + dataDir},
Expand Down
16 changes: 16 additions & 0 deletions agent/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1480,6 +1480,10 @@ func (s *NodeService) IsGateway() bool {
func (s *NodeService) Validate() error {
var result error

if err := s.Locality.Validate(); err != nil {
result = multierror.Append(result, err)
}

if s.Kind == ServiceKindConnectProxy {
if s.Port == 0 && s.SocketPath == "" {
result = multierror.Append(result, fmt.Errorf("Port or SocketPath must be set for a %s", s.Kind))
Expand Down Expand Up @@ -3111,3 +3115,15 @@ func (l *Locality) GetRegion() string {
}
return l.Region
}

func (l *Locality) Validate() error {
if l == nil {
return nil
}

if l.Region == "" && l.Zone != "" {
return fmt.Errorf("zone cannot be set without region")
}

return nil
}
44 changes: 44 additions & 0 deletions agent/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,43 @@ func TestStructs_ServiceNode_Conversions(t *testing.T) {
}
}

func TestStructs_Locality_Validate(t *testing.T) {
type testCase struct {
locality *Locality
err string
}
cases := map[string]testCase{
"nil": {
nil,
"",
},
"region only": {
&Locality{Region: "us-west-1"},
"",
},
"region and zone": {
&Locality{Region: "us-west-1", Zone: "us-west-1a"},
"",
},
"zone only": {
&Locality{Zone: "us-west-1a"},
"zone cannot be set without region",
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
err := tc.locality.Validate()
if tc.err == "" {
require.NoError(t, err)
} else {
require.Error(t, err)
require.Contains(t, err.Error(), tc.err)
}
})
}
}

func TestStructs_NodeService_ValidateMeshGateway(t *testing.T) {
type testCase struct {
Modify func(*NodeService)
Expand Down Expand Up @@ -1152,6 +1189,13 @@ func TestStructs_NodeService_ValidateConnectProxy(t *testing.T) {
},
"",
},
{
"connect-proxy: invalid locality",
func(x *NodeService) {
x.Locality = &Locality{Zone: "bad"}
},
"zone cannot be set without region",
},
}

for _, tc := range cases {
Expand Down

0 comments on commit 0994ccf

Please sign in to comment.