Skip to content

Commit 0199af5

Browse files
vicentinileonardovicentinileonardo
authored
feat: added support for nested and complex identifiers (#51)
* feat: added support for nested fields in identifiers and additionalStatusFields * feat: added complex identifiers management * fix: inverted default match behaviour for identifiers * test: added additional unit tests for findby * test: added additional tests for populateStatusFields
1 parent dc9753b commit 0199af5

File tree

12 files changed

+918
-170
lines changed

12 files changed

+918
-170
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,3 +191,4 @@ The following environment variables can be configured in the rest-dynamic-contro
191191
| REST_CONTROLLER_MIN_ERROR_RETRY_INTERVAL | The minimum interval between retries when an error occurs. This should be less than max-error-retry-interval. | `1s` |
192192
| REST_CONTROLLER_MAX_ERROR_RETRIES | How many times to retry the processing of a resource when an error occurs before giving up and dropping the resource. | `5` |
193193
| REST_CONTROLLER_METRICS_SERVER_PORT | The port where the metrics server will be listening. If not set, the metrics server is disabled. | |
194+
| REST_CONTROLLER_IDENTIFIER_MATCH_POLICY | Policy to match identifier fields when checking if a remote resource exists. Possible values are "AND" (all fields must match) and "OR" (at least one field must match). | `OR` |

‎internal/controllers/helpers.go‎

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package restResources
33
import (
44
"fmt"
55
"math"
6+
"strings"
67

78
"github.com/krateoplatformops/rest-dynamic-controller/internal/tools/comparison"
89
getter "github.com/krateoplatformops/rest-dynamic-controller/internal/tools/definitiongetter"
@@ -35,8 +36,9 @@ func isCRUpdated(mg *unstructured.Unstructured, rm map[string]interface{}) (comp
3536
return comparison.CompareExisting(m, rm)
3637
}
3738

38-
// populateStatusFields populates the status fields in the mg object with the values from the body
39-
// It checks both the `identifiers` and `additionalStatusFields` defined in the resource
39+
// populateStatusFields populates the status fields in the mg object with the values from the response body of the API call.
40+
// It supports dot notation for nested fields and performs necessary type conversions.
41+
// It uses the identifiers and additionalStatusFields from the clientInfo to determine which fields to populate.
4042
func populateStatusFields(clientInfo *getter.Info, mg *unstructured.Unstructured, body map[string]interface{}) error {
4143
// Handle nil inputs
4244
if mg == nil {
@@ -49,34 +51,40 @@ func populateStatusFields(clientInfo *getter.Info, mg *unstructured.Unstructured
4951
return nil // Nothing to populate, but not an error
5052
}
5153

54+
// Combine identifiers and additionalStatusFields into one list.
55+
allFields := append(clientInfo.Resource.Identifiers, clientInfo.Resource.AdditionalStatusFields...)
5256
// Early return if no fields to populate
53-
if len(clientInfo.Resource.Identifiers) == 0 && len(clientInfo.Resource.AdditionalStatusFields) == 0 {
57+
if len(allFields) == 0 {
5458
return nil
5559
}
5660

57-
// Create a set of all fields we need to look for
58-
fieldsToPopulate := make(map[string]struct{})
61+
for _, fieldName := range allFields {
62+
//log.Printf("Managing field: %s", fieldName)
63+
// Split the field name by '.' to handle nested paths.
64+
path := strings.Split(fieldName, ".")
65+
//log.Printf("Field path split: %v", path)
5966

60-
// Add identifiers to the set
61-
for _, identifier := range clientInfo.Resource.Identifiers {
62-
fieldsToPopulate[identifier] = struct{}{}
63-
}
64-
65-
// Add additionalStatusFields to the set
66-
for _, additionalField := range clientInfo.Resource.AdditionalStatusFields {
67-
fieldsToPopulate[additionalField] = struct{}{}
68-
}
67+
// Extract the raw value from the response body without copying.
68+
value, found, err := unstructured.NestedFieldNoCopy(body, path...)
69+
if err != nil || !found {
70+
// An error here means the path was invalid or not found.
71+
// We can safely continue to the next field.
72+
//log.Printf("Field '%s' not found in response body or error occurred: %v", fieldName, err)
73+
continue
74+
}
75+
//log.Printf("Extracted value for field '%s': %v", fieldName, value)
6976

70-
// Single pass through the body map
71-
for k, v := range body {
72-
if _, exists := fieldsToPopulate[k]; exists {
73-
// Convert the value to a format that unstructured can handle
74-
convertedValue := deepCopyJSONValue(v)
77+
// Perform deep copy and type conversions (e.g., float64 to int64).
78+
convertedValue := deepCopyJSONValue(value)
79+
//log.Printf("Converted value for field '%s': %v", fieldName, convertedValue)
7580

76-
if err := unstructured.SetNestedField(mg.Object, convertedValue, "status", k); err != nil {
77-
return fmt.Errorf("setting nested field '%s' in status: %w", k, err)
78-
}
81+
// The destination path in the status should mirror the source path.
82+
statusPath := append([]string{"status"}, path...)
83+
//log.Printf("Setting value for field '%s' at status path: %v", fieldName, statusPath)
84+
if err := unstructured.SetNestedField(mg.Object, convertedValue, statusPath...); err != nil {
85+
return fmt.Errorf("setting nested field '%s' in status: %w", fieldName, err)
7986
}
87+
//log.Printf("Successfully set field '%s' with value: %v at path: %v", fieldName, convertedValue, statusPath)
8088
}
8189

8290
return nil

‎internal/controllers/helpers_test.go‎

Lines changed: 228 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,232 @@ func TestPopulateStatusFields(t *testing.T) {
531531
},
532532
},
533533
},
534+
{
535+
name: "nested identifiers and additional status fields",
536+
clientInfo: &getter.Info{
537+
Resource: getter.Resource{
538+
Identifiers: []string{"metadata.uid"},
539+
AdditionalStatusFields: []string{"spec.host", "status.phase"},
540+
},
541+
},
542+
mg: &unstructured.Unstructured{
543+
Object: map[string]interface{}{},
544+
},
545+
body: map[string]interface{}{
546+
"metadata": map[string]interface{}{
547+
"uid": "xyz-123",
548+
},
549+
"spec": map[string]interface{}{
550+
"host": "example.com",
551+
},
552+
"status": map[string]interface{}{
553+
"phase": "Running",
554+
},
555+
},
556+
wantErr: false,
557+
expected: map[string]interface{}{
558+
"status": map[string]interface{}{
559+
"metadata": map[string]interface{}{
560+
"uid": "xyz-123",
561+
},
562+
"spec": map[string]interface{}{
563+
"host": "example.com",
564+
},
565+
"status": map[string]interface{}{
566+
"phase": "Running",
567+
},
568+
},
569+
},
570+
},
571+
{
572+
name: "mixed top-level and nested fields",
573+
clientInfo: &getter.Info{
574+
Resource: getter.Resource{
575+
Identifiers: []string{"id"},
576+
AdditionalStatusFields: []string{"metadata.name"},
577+
},
578+
},
579+
mg: &unstructured.Unstructured{
580+
Object: map[string]interface{}{},
581+
},
582+
body: map[string]interface{}{
583+
"id": "abc-456",
584+
"metadata": map[string]interface{}{
585+
"name": "my-resource",
586+
},
587+
},
588+
wantErr: false,
589+
expected: map[string]interface{}{
590+
"status": map[string]interface{}{
591+
"id": "abc-456",
592+
"metadata": map[string]interface{}{
593+
"name": "my-resource",
594+
},
595+
},
596+
},
597+
},
598+
{
599+
name: "nested field not found in body",
600+
clientInfo: &getter.Info{
601+
Resource: getter.Resource{
602+
AdditionalStatusFields: []string{"spec.nonexistent"},
603+
},
604+
},
605+
mg: &unstructured.Unstructured{
606+
Object: map[string]interface{}{},
607+
},
608+
body: map[string]interface{}{
609+
"spec": map[string]interface{}{
610+
"host": "example.com",
611+
},
612+
},
613+
wantErr: false,
614+
expected: map[string]interface{}{},
615+
},
616+
{
617+
name: "complex nested object",
618+
clientInfo: &getter.Info{
619+
Resource: getter.Resource{
620+
AdditionalStatusFields: []string{"data.config.spec"},
621+
},
622+
},
623+
mg: &unstructured.Unstructured{
624+
Object: map[string]interface{}{},
625+
},
626+
body: map[string]interface{}{
627+
"data": map[string]interface{}{
628+
"config": map[string]interface{}{
629+
"spec": map[string]interface{}{
630+
"key": "value",
631+
"num": float64(123),
632+
},
633+
},
634+
},
635+
},
636+
wantErr: false,
637+
expected: map[string]interface{}{
638+
"status": map[string]interface{}{
639+
"data": map[string]interface{}{
640+
"config": map[string]interface{}{
641+
"spec": map[string]interface{}{
642+
"key": "value",
643+
"num": int64(123),
644+
},
645+
},
646+
},
647+
},
648+
},
649+
},
650+
{
651+
name: "slice of strings",
652+
clientInfo: &getter.Info{
653+
Resource: getter.Resource{
654+
AdditionalStatusFields: []string{"spec.tags"},
655+
},
656+
},
657+
mg: &unstructured.Unstructured{
658+
Object: map[string]interface{}{},
659+
},
660+
body: map[string]interface{}{
661+
"spec": map[string]interface{}{
662+
"tags": []interface{}{"tag1", "tag2"},
663+
},
664+
},
665+
wantErr: false,
666+
expected: map[string]interface{}{
667+
"status": map[string]interface{}{
668+
"spec": map[string]interface{}{
669+
"tags": []interface{}{"tag1", "tag2"},
670+
},
671+
},
672+
},
673+
},
674+
{
675+
name: "slice of objects with mixed numeric types",
676+
clientInfo: &getter.Info{
677+
Resource: getter.Resource{
678+
AdditionalStatusFields: []string{"spec.ports"},
679+
},
680+
},
681+
mg: &unstructured.Unstructured{
682+
Object: map[string]interface{}{},
683+
},
684+
body: map[string]interface{}{
685+
"spec": map[string]interface{}{
686+
"ports": []interface{}{
687+
map[string]interface{}{"name": "http", "port": 80},
688+
map[string]interface{}{"name": "https", "port": float32(443)},
689+
},
690+
},
691+
},
692+
wantErr: false,
693+
expected: map[string]interface{}{
694+
"status": map[string]interface{}{
695+
"spec": map[string]interface{}{
696+
"ports": []interface{}{
697+
map[string]interface{}{"name": "http", "port": int64(80)},
698+
map[string]interface{}{"name": "https", "port": int64(443)},
699+
},
700+
},
701+
},
702+
},
703+
},
704+
{
705+
name: "object with nil value",
706+
clientInfo: &getter.Info{
707+
Resource: getter.Resource{
708+
AdditionalStatusFields: []string{"config"},
709+
},
710+
},
711+
mg: &unstructured.Unstructured{
712+
Object: map[string]interface{}{},
713+
},
714+
body: map[string]interface{}{
715+
"config": map[string]interface{}{
716+
"settingA": "valueA",
717+
"settingB": nil,
718+
},
719+
},
720+
wantErr: false,
721+
expected: map[string]interface{}{
722+
"status": map[string]interface{}{
723+
"config": map[string]interface{}{
724+
"settingA": "valueA",
725+
"settingB": nil,
726+
},
727+
},
728+
},
729+
},
730+
{
731+
name: "slice of objects with mixed numeric types",
732+
clientInfo: &getter.Info{
733+
Resource: getter.Resource{
734+
AdditionalStatusFields: []string{"spec.ports"},
735+
},
736+
},
737+
mg: &unstructured.Unstructured{
738+
Object: map[string]interface{}{},
739+
},
740+
body: map[string]interface{}{
741+
"spec": map[string]interface{}{
742+
"ports": []interface{}{
743+
map[string]interface{}{"name": "http", "port": 80},
744+
map[string]interface{}{"name": "https", "port": float32(443)},
745+
},
746+
},
747+
},
748+
wantErr: false,
749+
expected: map[string]interface{}{
750+
"status": map[string]interface{}{
751+
"spec": map[string]interface{}{
752+
"ports": []interface{}{
753+
map[string]interface{}{"name": "http", "port": int64(80)},
754+
map[string]interface{}{"name": "https", "port": int64(443)},
755+
},
756+
},
757+
},
758+
},
759+
},
534760
}
535761

536762
for _, tt := range tests {
@@ -542,52 +768,8 @@ func TestPopulateStatusFields(t *testing.T) {
542768
}
543769

544770
if !tt.wantErr {
545-
// Validate the results
546-
if len(tt.expected) == 0 {
547-
// No status should be created or should be empty
548-
status, exists, _ := unstructured.NestedMap(tt.mg.Object, "status")
549-
if exists && len(status) > 0 {
550-
// Check if there were pre-existing status fields ("existing" field in status)
551-
hasPreExisting := false
552-
for _, test := range tests {
553-
if test.name == tt.name {
554-
if statusObj, ok := test.mg.Object["status"]; ok {
555-
if statusMap, ok := statusObj.(map[string]interface{}); ok && len(statusMap) > 0 {
556-
hasPreExisting = true
557-
break
558-
}
559-
}
560-
}
561-
}
562-
if !hasPreExisting {
563-
t.Errorf("populateStatusFields() unexpected status field created: %v while expected is empty", status)
564-
}
565-
}
566-
} else {
567-
// Validate expected status fields
568-
status, exists, _ := unstructured.NestedMap(tt.mg.Object, "status")
569-
if !exists {
570-
t.Errorf("populateStatusFields() status field not created while length of expected is %d", len(tt.expected))
571-
return
572-
}
573-
574-
expectedStatus := tt.expected["status"].(map[string]interface{})
575-
576-
// Check that all expected fields are present with correct values
577-
for k, expectedVal := range expectedStatus {
578-
if actualVal, ok := status[k]; !ok {
579-
t.Errorf("populateStatusFields() status.%s not found", k)
580-
} else if actualVal != expectedVal {
581-
t.Errorf("populateStatusFields() status.%s = %v, want %v", k, actualVal, expectedVal)
582-
}
583-
}
584-
585-
// For tests with existing status, ensure they're preserved
586-
if tt.name == "existing status fields should be preserved" {
587-
if status["existing"] != "existingValue" {
588-
t.Errorf("populateStatusFields() existing status field not preserved")
589-
}
590-
}
771+
if diff := cmp.Diff(tt.expected, tt.mg.Object); diff != "" {
772+
t.Errorf("populateStatusFields() mismatch (-want +got):\n%s", diff)
591773
}
592774
}
593775
})

0 commit comments

Comments
 (0)