Skip to content

Commit 27f9199

Browse files
u-kaiu-kai
authored
refactor(aws): abstract provider-specific boolean parsing (#6078)
* refactor(endpoint): add GetBoolProviderSpecificProperty method with comprehensive tests Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> * refactor(endpoint): add GetBoolProviderSpecificProperty method and update consumers Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> * fix(lint): remove named returns in GetBoolProviderSpecificProperty Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --------- Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com>
1 parent 2ea3727 commit 27f9199

File tree

4 files changed

+116
-6
lines changed

4 files changed

+116
-6
lines changed

‎endpoint/endpoint.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,22 @@ func (e *Endpoint) GetProviderSpecificProperty(key string) (string, bool) {
311311
return "", false
312312
}
313313

314+
// GetBoolProperty returns a boolean provider-specific property value.
315+
func (e *Endpoint) GetBoolProviderSpecificProperty(key string) (bool, bool) {
316+
prop, ok := e.GetProviderSpecificProperty(key)
317+
if !ok {
318+
return false, false
319+
}
320+
switch prop {
321+
case "true":
322+
return true, true
323+
case "false":
324+
return false, true
325+
default:
326+
return false, true
327+
}
328+
}
329+
314330
// SetProviderSpecificProperty sets the value of a ProviderSpecificProperty.
315331
func (e *Endpoint) SetProviderSpecificProperty(key string, value string) {
316332
for i, providerSpecific := range e.ProviderSpecific {

‎endpoint/endpoint_test.go‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,3 +1072,97 @@ func TestNewEndpointWithTTLPreservesDotsInTXTRecords(t *testing.T) {
10721072
require.NotNil(t, cnameEndpoint, "CNAME endpoint should be created")
10731073
assert.Equal(t, "target.example.com", cnameEndpoint.Targets[0], "CNAME record should have trailing dot trimmed")
10741074
}
1075+
1076+
func TestGetBoolProviderSpecificProperty(t *testing.T) {
1077+
tests := []struct {
1078+
name string
1079+
endpoint Endpoint
1080+
key string
1081+
expectedValue bool
1082+
expectedExists bool
1083+
}{
1084+
{
1085+
name: "key does not exist",
1086+
endpoint: Endpoint{},
1087+
key: "nonexistent",
1088+
expectedValue: false,
1089+
expectedExists: false,
1090+
},
1091+
{
1092+
name: "key exists with true value",
1093+
endpoint: Endpoint{
1094+
ProviderSpecific: []ProviderSpecificProperty{
1095+
{Name: "enabled", Value: "true"},
1096+
},
1097+
},
1098+
key: "enabled",
1099+
expectedValue: true,
1100+
expectedExists: true,
1101+
},
1102+
{
1103+
name: "key exists with false value",
1104+
endpoint: Endpoint{
1105+
ProviderSpecific: []ProviderSpecificProperty{
1106+
{Name: "disabled", Value: "false"},
1107+
},
1108+
},
1109+
key: "disabled",
1110+
expectedValue: false,
1111+
expectedExists: true,
1112+
},
1113+
{
1114+
name: "key exists with invalid boolean value",
1115+
endpoint: Endpoint{
1116+
ProviderSpecific: []ProviderSpecificProperty{
1117+
{Name: "invalid", Value: "maybe"},
1118+
},
1119+
},
1120+
key: "invalid",
1121+
expectedValue: false,
1122+
expectedExists: true,
1123+
},
1124+
{
1125+
name: "key exists with empty value",
1126+
endpoint: Endpoint{
1127+
ProviderSpecific: []ProviderSpecificProperty{
1128+
{Name: "empty", Value: ""},
1129+
},
1130+
},
1131+
key: "empty",
1132+
expectedValue: false,
1133+
expectedExists: true,
1134+
},
1135+
{
1136+
name: "key exists with numeric value",
1137+
endpoint: Endpoint{
1138+
ProviderSpecific: []ProviderSpecificProperty{
1139+
{Name: "numeric", Value: "1"},
1140+
},
1141+
},
1142+
key: "numeric",
1143+
expectedValue: false,
1144+
expectedExists: true,
1145+
},
1146+
{
1147+
name: "multiple properties, find correct one",
1148+
endpoint: Endpoint{
1149+
ProviderSpecific: []ProviderSpecificProperty{
1150+
{Name: "first", Value: "invalid"},
1151+
{Name: "second", Value: "true"},
1152+
{Name: "third", Value: "false"},
1153+
},
1154+
},
1155+
key: "second",
1156+
expectedValue: true,
1157+
expectedExists: true,
1158+
},
1159+
}
1160+
1161+
for _, tt := range tests {
1162+
t.Run(tt.name, func(t *testing.T) {
1163+
value, exists := tt.endpoint.GetBoolProviderSpecificProperty(tt.key)
1164+
assert.Equal(t, tt.expectedValue, value)
1165+
assert.Equal(t, tt.expectedExists, exists)
1166+
})
1167+
}
1168+
}

‎provider/aws/aws.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -922,8 +922,8 @@ func (p *AWSProvider) newChange(action route53types.ChangeAction, ep *endpoint.E
922922
change.ResourceRecordSet.Type = route53types.RRType(ep.RecordType)
923923
if targetHostedZone := isAWSAlias(ep); targetHostedZone != "" {
924924
evalTargetHealth := p.evaluateTargetHealth
925-
if prop, ok := ep.GetProviderSpecificProperty(providerSpecificEvaluateTargetHealth); ok {
926-
evalTargetHealth = prop == "true"
925+
if prop, exists := ep.GetBoolProviderSpecificProperty(providerSpecificEvaluateTargetHealth); exists {
926+
evalTargetHealth = prop
927927
}
928928
change.ResourceRecordSet.AliasTarget = &route53types.AliasTarget{
929929
DNSName: aws.String(ep.Targets[0]),
@@ -1345,8 +1345,8 @@ func useAlias(ep *endpoint.Endpoint, preferCNAME bool) bool {
13451345
// isAWSAlias determines if a given endpoint is supposed to create an AWS Alias record
13461346
// and (if so) returns the target hosted zone ID
13471347
func isAWSAlias(ep *endpoint.Endpoint) string {
1348-
isAlias, exists := ep.GetProviderSpecificProperty(providerSpecificAlias)
1349-
if exists && isAlias == "true" && slices.Contains([]string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA}, ep.RecordType) && len(ep.Targets) > 0 {
1348+
isAlias, _ := ep.GetBoolProviderSpecificProperty(providerSpecificAlias)
1349+
if isAlias && slices.Contains([]string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA}, ep.RecordType) && len(ep.Targets) > 0 {
13501350
// alias records can only point to canonical hosted zones (e.g. to ELBs) or other records in the same zone
13511351

13521352
if hostedZoneID, ok := ep.GetProviderSpecificProperty(providerSpecificTargetHostedZone); ok {

‎registry/txt/registry.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error
244244
}
245245

246246
// AWS Alias records have "new" format encoded as type "cname"
247-
if isAlias, found := ep.GetProviderSpecificProperty("alias"); found && isAlias == "true" && ep.RecordType == endpoint.RecordTypeA {
247+
if isAlias, found := ep.GetBoolProviderSpecificProperty("alias"); found && isAlias && ep.RecordType == endpoint.RecordTypeA {
248248
key.RecordType = endpoint.RecordTypeCNAME
249249
}
250250

@@ -299,7 +299,7 @@ func (im *TXTRegistry) generateTXTRecordWithFilter(r *endpoint.Endpoint, filter
299299
// Always create new format record
300300
recordType := r.RecordType
301301
// AWS Alias records are encoded as type "cname"
302-
if isAlias, found := r.GetProviderSpecificProperty("alias"); found && isAlias == "true" && recordType == endpoint.RecordTypeA {
302+
if isAlias, found := r.GetBoolProviderSpecificProperty("alias"); found && isAlias && recordType == endpoint.RecordTypeA {
303303
recordType = endpoint.RecordTypeCNAME
304304
}
305305

0 commit comments

Comments
 (0)