Skip to content

Commit 2447b74

Browse files
committed
fix: improve Homer compatibility with type inference and YAML key mapping
- Fix smartInferType to parse "0" and "1" as integers, not booleans - Add float detection for values like danger_value: 95.5 - Expand getYAMLKey to handle 20+ camelCase Homer field names - Fix isItemHidden to handle integer truthiness (0=false, non-zero=true) - Apply getYAMLKey to service parameters (was only applied to items) - Remove DEBUG statements from production code - Add LocalCluster constant to fix lint warning - Update tests to reflect correct integer parsing behavior
1 parent b5c1736 commit 2447b74

2 files changed

Lines changed: 89 additions & 41 deletions

File tree

pkg/homer/annotation_processing_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,29 +21,30 @@ import (
2121
)
2222

2323
// TestBooleanValueParsing tests comprehensive boolean value parsing
24+
// Note: "1" and "0" are intentionally NOT boolean - they are parsed as integers
25+
// This is correct because Homer uses these values for fields like apiVersion, timeout, etc.
26+
// JavaScript's truthiness will handle integer values correctly when accessing boolean fields.
2427
func TestBooleanValueParsing(t *testing.T) {
2528
tests := []struct {
2629
input string
2730
expected bool
2831
}{
29-
// True values
32+
// True values (explicit boolean strings)
3033
{"true", true},
3134
{"TRUE", true},
3235
{"True", true},
33-
{"1", true},
3436
{"yes", true},
3537
{"YES", true},
3638
{"on", true},
3739
{"ON", true},
3840
{" true ", true}, // test trimming
3941

40-
// False values
42+
// False values (explicit boolean strings)
4143
{"false", false},
4244
{"FALSE", false},
43-
{"0", false},
4445
{"no", false},
4546
{"off", false},
46-
{"invalid", false},
47+
{"invalid", false}, // non-boolean strings are not parsed as bool
4748
{"", false},
4849
{" false ", false}, // test trimming
4950
}

pkg/homer/config.go

Lines changed: 83 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ const (
3939
DefaultNamespace = "default"
4040
GenericType = "Generic"
4141
CRDSource = "crd"
42+
LocalCluster = "local"
4243
NameField = "name"
4344
URLField = "url"
4445
WarningValueField = "warning_value"
@@ -387,13 +388,6 @@ func createConfigMapWithHTTPRoutesAndHealth(
387388

388389
cleanupHomerConfig(config)
389390

390-
fmt.Fprintf(os.Stderr, "DEBUG: createConfigMapWithHTTPRoutesAndHealth called with %d HTTPRoutes\n", len(httproutes))
391-
for i, httproute := range httproutes {
392-
if clusterAnnot, ok := httproute.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok {
393-
fmt.Fprintf(os.Stderr, "DEBUG: HTTPRoute[%d] %s has cluster annotation: %s, labels: %v\n", i, httproute.ObjectMeta.Name, clusterAnnot, httproute.ObjectMeta.Labels)
394-
}
395-
}
396-
397391
for _, ingress := range ingresses.Items {
398392
UpdateHomerConfigIngress(config, ingress, domainFilters)
399393
}
@@ -1133,15 +1127,11 @@ func createIngressItems(ingress networkingv1.Ingress, domainFilters []string) []
11331127

11341128
// Append cluster name suffix from label AFTER processing annotations
11351129
// so that it takes precedence over any name annotations
1136-
if clusterName, ok := ingress.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != "local" {
1137-
fmt.Fprintf(os.Stderr, "DEBUG: Ingress %s from cluster %s, labels: %v\n", ingress.ObjectMeta.Name, clusterName, ingress.ObjectMeta.Labels)
1130+
if clusterName, ok := ingress.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != LocalCluster {
11381131
if suffix, hasSuffix := ingress.ObjectMeta.Labels["cluster-name-suffix"]; hasSuffix && suffix != "" {
11391132
if currentName, hasName := item.Parameters["name"]; hasName && currentName != "" {
1140-
fmt.Fprintf(os.Stderr, "DEBUG: Appending suffix %q to name %q\n", suffix, currentName)
11411133
setItemParameter(&item, "name", currentName+suffix)
11421134
}
1143-
} else {
1144-
fmt.Fprintf(os.Stderr, "DEBUG: No cluster-name-suffix label found for %s\n", ingress.ObjectMeta.Name)
11451135
}
11461136
}
11471137

@@ -1197,14 +1187,14 @@ func createIngressItem(ingress networkingv1.Ingress, host string, validRuleCount
11971187
// Set metadata for conflict detection
11981188
// For remote clusters, include cluster name in Source to make it unique
11991189
item.Source = ingress.ObjectMeta.Name
1200-
if clusterName, ok := ingress.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != "local" {
1190+
if clusterName, ok := ingress.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != LocalCluster {
12011191
item.Source = ingress.ObjectMeta.Name + "@" + clusterName
12021192
}
12031193
item.Namespace = ingress.ObjectMeta.Namespace
12041194
item.LastUpdate = ingress.ObjectMeta.CreationTimestamp.Time.Format("2006-01-02T15:04:05Z")
12051195

12061196
// Auto-tag with cluster name if cluster-tagstyle label is set
1207-
if clusterName, ok := ingress.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != "local" {
1197+
if clusterName, ok := ingress.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != LocalCluster {
12081198
// Only add tag if cluster-tagstyle is explicitly set
12091199
if tagStyle, hasStyle := ingress.ObjectMeta.Labels["cluster-tagstyle"]; hasStyle && tagStyle != "" {
12101200
setItemParameter(&item, "tag", clusterName)
@@ -1308,7 +1298,7 @@ func updateHomerConfigWithHTTPRoutes(
13081298
// Set metadata for conflict detection
13091299
// For remote clusters, include cluster name in Source to make it unique
13101300
item.Source = httproute.ObjectMeta.Name
1311-
if clusterName, ok := httproute.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != "local" {
1301+
if clusterName, ok := httproute.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != LocalCluster {
13121302
item.Source = httproute.ObjectMeta.Name + "@" + clusterName
13131303
}
13141304
item.Namespace = httproute.ObjectMeta.Namespace
@@ -1318,15 +1308,11 @@ func updateHomerConfigWithHTTPRoutes(
13181308

13191309
// Append cluster name suffix from label AFTER processing annotations
13201310
// so that it takes precedence over any name annotations
1321-
if clusterName, ok := httproute.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != "local" {
1322-
fmt.Fprintf(os.Stderr, "DEBUG: HTTPRoute %s from cluster %s, labels: %v\n", httproute.ObjectMeta.Name, clusterName, httproute.ObjectMeta.Labels)
1311+
if clusterName, ok := httproute.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != LocalCluster {
13231312
if suffix, hasSuffix := httproute.ObjectMeta.Labels["cluster-name-suffix"]; hasSuffix && suffix != "" {
13241313
if currentName, hasName := item.Parameters["name"]; hasName && currentName != "" {
1325-
fmt.Fprintf(os.Stderr, "DEBUG: Appending suffix %q to name %q\n", suffix, currentName)
13261314
setItemParameter(&item, "name", currentName+suffix)
13271315
}
1328-
} else {
1329-
fmt.Fprintf(os.Stderr, "DEBUG: No cluster-name-suffix label found for %s\n", httproute.ObjectMeta.Name)
13301316
}
13311317
}
13321318

@@ -1365,7 +1351,7 @@ func createHTTPRouteItem(httproute *gatewayv1.HTTPRoute, hostname, protocol stri
13651351
}
13661352

13671353
// Auto-tag with cluster name if cluster-tagstyle label is set
1368-
if clusterName, ok := httproute.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != "local" {
1354+
if clusterName, ok := httproute.ObjectMeta.Annotations["homer.rajsingh.info/cluster"]; ok && clusterName != "" && clusterName != LocalCluster {
13691355
// Only add tag if cluster-tagstyle is explicitly set
13701356
if tagStyle, hasStyle := httproute.ObjectMeta.Labels["cluster-tagstyle"]; hasStyle && tagStyle != "" {
13711357
setItemParameter(&item, "tag", clusterName)
@@ -1615,20 +1601,29 @@ func processDynamicParameter(item *Item, fieldName, value string, validationLeve
16151601
func smartInferType(value string) interface{} {
16161602
value = strings.TrimSpace(value)
16171603

1618-
// Boolean detection
1604+
// Integer detection FIRST - prevents "0" and "1" from being converted to booleans
1605+
// This is important for fields like updateInterval, timeout, apiVersion, etc.
1606+
if i, err := strconv.Atoi(value); err == nil {
1607+
return i
1608+
}
1609+
1610+
// Float detection - for values like danger_value: 95.5
1611+
// Only if it contains a decimal point to avoid converting integers
1612+
if strings.Contains(value, ".") {
1613+
if f, err := strconv.ParseFloat(value, 64); err == nil {
1614+
return f
1615+
}
1616+
}
1617+
1618+
// Boolean detection - only for explicit boolean strings
16191619
lower := strings.ToLower(value)
1620-
if lower == "true" || lower == "1" || lower == "yes" || lower == "on" {
1620+
if lower == "true" || lower == "yes" || lower == "on" {
16211621
return true
16221622
}
1623-
if lower == "false" || lower == "0" || lower == "no" || lower == "off" {
1623+
if lower == "false" || lower == "no" || lower == "off" {
16241624
return false
16251625
}
16261626

1627-
// Integer detection
1628-
if i, err := strconv.Atoi(value); err == nil {
1629-
return i
1630-
}
1631-
16321627
return value
16331628
}
16341629

@@ -1665,13 +1660,18 @@ func isItemHidden(item *Item) bool {
16651660

16661661
// Check for hide parameter
16671662
if hideValue, exists := item.Parameters["hide"]; exists {
1668-
// Use smart type inference to handle boolean values
1663+
// Use smart type inference to handle boolean and integer values
16691664
hideInterface := smartInferType(hideValue)
1670-
if hideBool, ok := hideInterface.(bool); ok {
1671-
return hideBool
1665+
switch v := hideInterface.(type) {
1666+
case bool:
1667+
return v
1668+
case int:
1669+
// Treat 0 as false, non-zero as true (JavaScript truthiness)
1670+
return v != 0
1671+
default:
1672+
// For strings, treat non-empty as true
1673+
return hideValue != ""
16721674
}
1673-
// If not a boolean, treat non-empty string as true
1674-
return hideValue != ""
16751675
}
16761676

16771677
return false
@@ -2428,10 +2428,11 @@ func flattenServicesForYAML(services []Service) []map[string]interface{} {
24282428
for _, service := range services {
24292429
serviceMap := make(map[string]interface{})
24302430

2431-
// Add parameters with smart type inference
2431+
// Add parameters with smart type inference and YAML key conversion
24322432
if service.Parameters != nil {
24332433
for key, value := range service.Parameters {
2434-
serviceMap[key] = smartInferType(value)
2434+
yamlKey := getYAMLKey(key)
2435+
serviceMap[yamlKey] = smartInferType(value)
24352436
}
24362437
}
24372438

@@ -2491,14 +2492,60 @@ func flattenItemsForYAML(items []Item) []map[string]interface{} {
24912492
}
24922493

24932494
// getYAMLKey converts parameter keys to proper YAML field names
2495+
// Homer uses camelCase for JavaScript property access, so we need to ensure
2496+
// annotation keys (which may be lowercase) are converted to proper camelCase
24942497
func getYAMLKey(key string) string {
24952498
switch strings.ToLower(key) {
2499+
// Service/Item parameters from Homer service components
24962500
case "legacyapi":
24972501
return "legacyApi"
24982502
case "librarytype":
24992503
return "libraryType"
25002504
case "usecredentials":
25012505
return "useCredentials"
2506+
case "apiversion":
2507+
return "apiVersion"
2508+
case "checkinterval":
2509+
return "checkInterval"
2510+
case "updateinterval":
2511+
return "updateInterval"
2512+
case "refreshinterval":
2513+
return "refreshInterval"
2514+
case "successcodes":
2515+
return "successCodes"
2516+
case "rateinterval":
2517+
return "rateInterval"
2518+
case "torrentinterval":
2519+
return "torrentInterval"
2520+
case "downloadinterval":
2521+
return "downloadInterval"
2522+
case "hideaverages":
2523+
return "hideaverages" // Homer uses lowercase
2524+
case "api_token":
2525+
return "api_token" // Homer uses underscore
2526+
case "warning_value":
2527+
return "warning_value" // Homer uses underscore
2528+
case "danger_value":
2529+
return "danger_value" // Homer uses underscore
2530+
case "hide_decimals":
2531+
return "hide_decimals" // Homer uses underscore
2532+
case "small_font_on_small_screens":
2533+
return "small_font_on_small_screens" // Homer uses underscore
2534+
case "small_font_on_desktop":
2535+
return "small_font_on_desktop" // Homer uses underscore
2536+
2537+
// Global config fields
2538+
case "documenttitle":
2539+
return "documentTitle"
2540+
case "colortheme":
2541+
return "colorTheme"
2542+
case "connectivitycheck":
2543+
return "connectivityCheck"
2544+
case "externalconfig":
2545+
return "externalConfig"
2546+
case "tagstyle":
2547+
return "tagstyle" // Homer uses lowercase
2548+
25022549
default:
25032550
return key
25042551
}

0 commit comments

Comments
 (0)