From 1c876faeab410590fb340309171128e1de054939 Mon Sep 17 00:00:00 2001 From: Rael Garcia Date: Thu, 21 May 2026 17:32:06 +0200 Subject: [PATCH] config/types: support slice indexing in GetByPath Until now, Configuration.GetByPath only walked map[string]any nodes; a slice value was an opaque leaf that could only be retrieved as a whole. This made it impossible to author a config reference of the form `imageRegistryPolicy.extraAllowedRegistries.0` to address a single element of a list-valued configuration, even though list values are otherwise first-class in our configuration shape (and the sdp-pipelines dunder rendering pipeline needs them to be). This change teaches GetByPath that a key applied to a []any node is a non-negative integer index into that slice. Existing behaviour for map navigation is unchanged. The error wording in the fall-through is adjusted ("nested map or slice") to reflect the new capability. Errors: - MissingKeyError continues to be returned for missing map keys. - A new IndexOutOfRangeError is returned for negative or out-of-bounds indices so callers can distinguish that failure mode without string matching. - A non-numeric key applied to a slice returns a plain wrapped error. Tests cover top-level slice indexing, deep navigation through slice-of-map and slice-of-slice, the realistic image-registry-policy shape, and the three error paths (missing key, out-of-range index, non-numeric slice key). The existing TestGetByPath error-string expectation is updated to match the new wording. Refs: AROSLSRE-913 --- config/types/configuration.go | 46 ++++++++- config/types/configuration_test.go | 146 +++++++++++++++++++++++++++++ config/types_test.go | 2 +- 3 files changed, 189 insertions(+), 5 deletions(-) diff --git a/config/types/configuration.go b/config/types/configuration.go index 7fe30032..342fd4a2 100644 --- a/config/types/configuration.go +++ b/config/types/configuration.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "regexp" + "strconv" "strings" "k8s.io/apimachinery/pkg/util/sets" @@ -26,19 +27,56 @@ func (e MissingKeyError) Error() string { return fmt.Sprintf("configuration%s: key %s not found", e.Path, e.Key) } +// IndexOutOfRangeError is returned by GetByPath when a numeric key is used +// to index into a slice but falls outside the bounds of that slice. +type IndexOutOfRangeError struct { + Path string + Index int + Length int +} + +func (e IndexOutOfRangeError) Error() string { + return fmt.Sprintf("configuration%s: index %d out of range [0, %d)", e.Path, e.Index, e.Length) +} + +// GetByPath navigates the configuration tree following the dot-separated keys +// in path and returns the value at that location. +// +// Keys are interpreted in two ways depending on the type of the current node: +// - For map nodes (map[string]any), the key is used as a literal map key. +// - For slice nodes ([]any), the key must be a non-negative integer literal +// that is used as the index into the slice (e.g. "containers.0.image"). +// +// Errors: +// - MissingKeyError when a map key does not exist. +// - IndexOutOfRangeError when a slice index is outside the slice bounds. +// - A plain error when a slice is reached but the next key is not a valid +// non-negative integer, or when the current node is neither a map nor a +// slice but more keys remain. func (v Configuration) GetByPath(path string) (any, error) { keys := strings.Split(path, ".") var current any = map[string]any(v) var currentPath string for _, key := range keys { - if m, ok := current.(map[string]any); ok { - current, ok = m[key] + switch typed := current.(type) { + case map[string]any: + next, ok := typed[key] if !ok { return nil, &MissingKeyError{Path: currentPath, Key: key} } - } else { - return nil, fmt.Errorf("configuration%s: expected nested map, found %T; cannot index with %s", currentPath, current, key) + current = next + case []any: + idx, err := strconv.Atoi(key) + if err != nil { + return nil, fmt.Errorf("configuration%s: expected non-negative integer to index slice, got %q", currentPath, key) + } + if idx < 0 || idx >= len(typed) { + return nil, &IndexOutOfRangeError{Path: currentPath, Index: idx, Length: len(typed)} + } + current = typed[idx] + default: + return nil, fmt.Errorf("configuration%s: expected nested map or slice, found %T; cannot index with %s", currentPath, current, key) } currentPath += "[" + key + "]" } diff --git a/config/types/configuration_test.go b/config/types/configuration_test.go index d914635d..79da1609 100644 --- a/config/types/configuration_test.go +++ b/config/types/configuration_test.go @@ -2,6 +2,7 @@ package types import ( "path/filepath" + "reflect" "testing" ) @@ -114,3 +115,148 @@ func TestResolveSchemaPath(t *testing.T) { }) } } + +func TestGetByPathSlices(t *testing.T) { + cfg := Configuration{ + "scalar": "hello", + "nested": map[string]any{ + "inner": "world", + "number": 42, + }, + "items": []any{ + "first", + "second", + "third", + }, + "objects": []any{ + map[string]any{ + "name": "alpha", + "value": 1, + }, + map[string]any{ + "name": "beta", + "value": 2, + }, + }, + "matrix": []any{ + []any{"a", "b"}, + []any{"c", "d"}, + }, + "acr": map[string]any{ + "extraAllowedRegistries": []any{ + "registry.connect.redhat.com", + "registry.redhat.io", + "quay.io/redhat-user-workloads", + }, + }, + } + + tests := []struct { + name string + path string + want any + wantErr bool + // errIs lets us assert a particular sentinel error type. + // Leave nil to skip the check. + errIs func(error) bool + }{ + { + name: "top-level scalar", + path: "scalar", + want: "hello", + }, + { + name: "nested map key", + path: "nested.inner", + want: "world", + }, + { + name: "slice index into top-level slice", + path: "items.0", + want: "first", + }, + { + name: "slice index, last element", + path: "items.2", + want: "third", + }, + { + name: "slice index into slice of maps then map key", + path: "objects.1.name", + want: "beta", + }, + { + name: "nested slice of slices", + path: "matrix.1.0", + want: "c", + }, + { + name: "realistic: map -> slice index (image-registry-policy shape)", + path: "acr.extraAllowedRegistries.0", + want: "registry.connect.redhat.com", + }, + { + name: "missing top-level key", + path: "doesNotExist", + wantErr: true, + errIs: func(err error) bool { + _, ok := err.(*MissingKeyError) + return ok + }, + }, + { + name: "slice index out of range", + path: "items.5", + wantErr: true, + errIs: func(err error) bool { + _, ok := err.(*IndexOutOfRangeError) + return ok + }, + }, + { + name: "negative slice index", + path: "items.-1", + wantErr: true, + errIs: func(err error) bool { + _, ok := err.(*IndexOutOfRangeError) + return ok + }, + }, + { + name: "non-numeric key applied to slice", + path: "items.first", + wantErr: true, + }, + { + name: "navigating into a scalar", + path: "scalar.x", + wantErr: true, + }, + { + name: "trailing slice returned whole", + path: "items", + want: []any{"first", "second", "third"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := cfg.GetByPath(tt.path) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error for path %q, got nil (value=%v)", tt.path, got) + } + if tt.errIs != nil && !tt.errIs(err) { + t.Fatalf("error type mismatch for path %q: got %T (%v)", tt.path, err, err) + } + return + } + if err != nil { + t.Fatalf("unexpected error for path %q: %v", tt.path, err) + } + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("path %q: got %#v, want %#v", tt.path, got, tt.want) + } + }) + } +} diff --git a/config/types_test.go b/config/types_test.go index d54de324..8f9c3a65 100644 --- a/config/types_test.go +++ b/config/types_test.go @@ -64,7 +64,7 @@ func TestGetByPath(t *testing.T) { }, }, path: "parent.key.nested", - err: "configuration[parent][key]: expected nested map, found string; cannot index with nested", + err: "configuration[parent][key]: expected nested map or slice, found string; cannot index with nested", }, } for _, tt := range tests {