From 9b7173c4d11ba5c64024c6f9af23c17b855a82bf Mon Sep 17 00:00:00 2001 From: Tim <32556895+Avarei@users.noreply.github.com> Date: Wed, 10 Jul 2024 20:46:52 +0200 Subject: [PATCH 1/4] add test for empty labels Signed-off-by: Tim <32556895+Avarei@users.noreply.github.com> --- fn_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/fn_test.go b/fn_test.go index dea92d8..aba8949 100644 --- a/fn_test.go +++ b/fn_test.go @@ -119,6 +119,15 @@ func TestRunFunction(t *testing.T) { } ] } + }, + { + "type": "Selector", + "kind": "EnvironmentConfig", + "apiVersion": "apiextensions.crossplane.io/v1alpha1", + "into": "obj-5", + "selector": { + "matchLabels": [] + } } ] } @@ -170,6 +179,15 @@ func TestRunFunction(t *testing.T) { }, }, }, + "obj-5": { + ApiVersion: "apiextensions.crossplane.io/v1alpha1", + Kind: "EnvironmentConfig", + Match: &fnv1beta1.ResourceSelector_MatchLabels{ + MatchLabels: &fnv1beta1.MatchLabels{ + Labels: map[string]string{}, + }, + }, + }, }, }, }, From 3b696a53c2641a18854263b88e0d232788e6a016 Mon Sep 17 00:00:00 2001 From: Tim <32556895+Avarei@users.noreply.github.com> Date: Wed, 10 Jul 2024 21:09:41 +0200 Subject: [PATCH 2/4] allow empty matchLabels Signed-off-by: Tim <32556895+Avarei@users.noreply.github.com> --- fn.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fn.go b/fn.go index edd11d1..c5e7038 100644 --- a/fn.go +++ b/fn.go @@ -130,6 +130,7 @@ func buildRequirements(in *v1beta1.Input, xr *resource.Composite) (*fnv1beta1.Re }, } case v1beta1.ResourceSourceTypeSelector: + containsMissingOptionalField := false matchLabels := map[string]string{} for _, selector := range extraResource.Selector.MatchLabels { switch selector.GetType() { @@ -142,12 +143,13 @@ func buildRequirements(in *v1beta1.Input, xr *resource.Composite) (*fnv1beta1.Re if !selector.FromFieldPathIsOptional() { return nil, errors.Wrapf(err, "cannot get value from field path %q", *selector.ValueFromFieldPath) } + containsMissingOptionalField = true continue } matchLabels[selector.Key] = value } } - if len(matchLabels) == 0 { + if len(matchLabels) == 0 && containsMissingOptionalField { continue } extraResources[extraResName] = &fnv1beta1.ResourceSelector{ From 11d7d4045d4e5e4b5765c3bc78e81494e3939431 Mon Sep 17 00:00:00 2001 From: Tim <32556895+Avarei@users.noreply.github.com> Date: Sat, 31 Aug 2024 00:04:10 +0200 Subject: [PATCH 3/4] break out typeSelectorRequirement building to reduce cyclomatic complexity for golangci-lint Signed-off-by: Tim <32556895+Avarei@users.noreply.github.com> --- fn.go | 64 ++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/fn.go b/fn.go index c5e7038..ee5d2b1 100644 --- a/fn.go +++ b/fn.go @@ -130,40 +130,50 @@ func buildRequirements(in *v1beta1.Input, xr *resource.Composite) (*fnv1beta1.Re }, } case v1beta1.ResourceSourceTypeSelector: - containsMissingOptionalField := false - matchLabels := map[string]string{} - for _, selector := range extraResource.Selector.MatchLabels { - switch selector.GetType() { - case v1beta1.ResourceSourceSelectorLabelMatcherTypeValue: - // TODO validate value not to be nil - matchLabels[selector.Key] = *selector.Value - case v1beta1.ResourceSourceSelectorLabelMatcherTypeFromCompositeFieldPath: - value, err := fieldpath.Pave(xr.Resource.Object).GetString(*selector.ValueFromFieldPath) - if err != nil { - if !selector.FromFieldPathIsOptional() { - return nil, errors.Wrapf(err, "cannot get value from field path %q", *selector.ValueFromFieldPath) - } - containsMissingOptionalField = true - continue - } - matchLabels[selector.Key] = value - } - } - if len(matchLabels) == 0 && containsMissingOptionalField { - continue + out, err := buildTypeSelectorRequirement(xr, extraResource) + if err != nil { + return nil, err } - extraResources[extraResName] = &fnv1beta1.ResourceSelector{ - ApiVersion: extraResource.APIVersion, - Kind: extraResource.Kind, - Match: &fnv1beta1.ResourceSelector_MatchLabels{ - MatchLabels: &fnv1beta1.MatchLabels{Labels: matchLabels}, - }, + if out != nil { + extraResources[extraResName] = out } } } return &fnv1beta1.Requirements{ExtraResources: extraResources}, nil } +func buildTypeSelectorRequirement(xr *resource.Composite, extraResource v1beta1.ResourceSource) (*fnv1beta1.ResourceSelector, error) { + containsMissingOptionalField := false + matchLabels := map[string]string{} + for _, selector := range extraResource.Selector.MatchLabels { + switch selector.GetType() { + case v1beta1.ResourceSourceSelectorLabelMatcherTypeValue: + // TODO validate value not to be nil + matchLabels[selector.Key] = *selector.Value + case v1beta1.ResourceSourceSelectorLabelMatcherTypeFromCompositeFieldPath: + value, err := fieldpath.Pave(xr.Resource.Object).GetString(*selector.ValueFromFieldPath) + if err != nil { + if !selector.FromFieldPathIsOptional() { + return nil, errors.Wrapf(err, "cannot get value from field path %q", *selector.ValueFromFieldPath) + } + containsMissingOptionalField = true + continue + } + matchLabels[selector.Key] = value + } + } + if len(matchLabels) == 0 && containsMissingOptionalField { + return nil, nil + } + return &fnv1beta1.ResourceSelector{ + ApiVersion: extraResource.APIVersion, + Kind: extraResource.Kind, + Match: &fnv1beta1.ResourceSelector_MatchLabels{ + MatchLabels: &fnv1beta1.MatchLabels{Labels: matchLabels}, + }, + }, nil +} + // Verify Min/Max and sort extra resources by field path within a single kind. func verifyAndSortExtras(in *v1beta1.Input, extraResources map[string][]resource.Extra, //nolint:gocyclo // TODO(reedjosh): refactor ) (cleanedExtras map[string][]unstructured.Unstructured, err error) { From 05b9b69a3c9f8ea150249574eacfaa3a5cdcb543 Mon Sep 17 00:00:00 2001 From: Tim <32556895+Avarei@users.noreply.github.com> Date: Sat, 31 Aug 2024 00:07:24 +0200 Subject: [PATCH 4/4] fix lint error 'G115: integer overflow conversion uint64 -> int' Signed-off-by: Tim <32556895+Avarei@users.noreply.github.com> --- fn.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fn.go b/fn.go index ee5d2b1..73dccd6 100644 --- a/fn.go +++ b/fn.go @@ -199,13 +199,13 @@ func verifyAndSortExtras(in *v1beta1.Input, extraResources map[string][]resource case v1beta1.ResourceSourceTypeSelector: selector := extraResource.Selector - if selector.MinMatch != nil && len(resources) < int(*selector.MinMatch) { + if selector.MinMatch != nil && uint64(len(resources)) < *selector.MinMatch { return nil, errors.Errorf("expected at least %d extra resources %q, got %d", *selector.MinMatch, extraResName, len(resources)) } if err := sortExtrasByFieldPath(resources, selector.GetSortByFieldPath()); err != nil { return nil, err } - if selector.MaxMatch != nil && len(resources) > int(*selector.MaxMatch) { + if selector.MaxMatch != nil && uint64(len(resources)) > *selector.MaxMatch { resources = resources[:*selector.MaxMatch] } for _, r := range resources {