From 595e5ae704d63a801029176c8fdedf0869c5257b Mon Sep 17 00:00:00 2001 From: Maxim Samoilov Date: Wed, 13 Dec 2023 15:58:18 +0400 Subject: [PATCH] Add support to limit applied policies in automation by specifying a selector Signed-off-by: Maxim Samoilov --- api/v1beta2/condition_types.go | 3 + api/v1beta2/imageupdateautomation_types.go | 5 ++ api/v1beta2/zz_generated.deepcopy.go | 5 ++ ...lkit.fluxcd.io_imageupdateautomations.yaml | 46 ++++++++++++++ docs/api/v1beta2/image-automation.md | 30 +++++++++ docs/spec/v1beta2/imageupdateautomations.md | 36 ++++++++++- .../imageupdateautomation_controller.go | 23 +++++-- internal/controller/update_test.go | 62 ++++++++++++++++++- 8 files changed, 204 insertions(+), 6 deletions(-) diff --git a/api/v1beta2/condition_types.go b/api/v1beta2/condition_types.go index 96bde947..910b706f 100644 --- a/api/v1beta2/condition_types.go +++ b/api/v1beta2/condition_types.go @@ -33,4 +33,7 @@ const ( // UpdateFailedReason represents a failure during source update. UpdateFailedReason string = "UpdateFailed" + + // InvalidPolicySelectorReason represents an invalid policy selector. + InvalidPolicySelectorReason string = "InvalidPolicySelector" ) diff --git a/api/v1beta2/imageupdateautomation_types.go b/api/v1beta2/imageupdateautomation_types.go index 70200b56..4eb31e7a 100644 --- a/api/v1beta2/imageupdateautomation_types.go +++ b/api/v1beta2/imageupdateautomation_types.go @@ -48,6 +48,11 @@ type ImageUpdateAutomationSpec struct { // +required Interval metav1.Duration `json:"interval"` + // PolicySelector allows to filter applied policies based on labels. + // By default includes all policies in namespace. + // +optional + PolicySelector *metav1.LabelSelector `json:"policySelector,omitempty"` + // Update gives the specification for how to update the files in // the repository. This can be left empty, to use the default // value. diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 9ebc16fa..2400f0a6 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -202,6 +202,11 @@ func (in *ImageUpdateAutomationSpec) DeepCopyInto(out *ImageUpdateAutomationSpec (*in).DeepCopyInto(*out) } out.Interval = in.Interval + if in.PolicySelector != nil { + in, out := &in.PolicySelector, &out.PolicySelector + *out = new(v1.LabelSelector) + (*in).DeepCopyInto(*out) + } if in.Update != nil { in, out := &in.Update, &out.Update *out = new(UpdateStrategy) diff --git a/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml b/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml index 12e6b781..d6e7c8ad 100644 --- a/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml +++ b/config/crd/bases/image.toolkit.fluxcd.io_imageupdateautomations.yaml @@ -499,6 +499,52 @@ spec: run should be attempted. pattern: ^([0-9]+(\.[0-9]+)?(ms|s|m|h))+$ type: string + policySelector: + description: |- + PolicySelector allows to filter applied policies based on labels. + By default includes all policies in namespace. + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. + The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the selector applies + to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic sourceRef: description: |- SourceRef refers to the resource giving access details diff --git a/docs/api/v1beta2/image-automation.md b/docs/api/v1beta2/image-automation.md index 3ef1ab9d..107ebb96 100644 --- a/docs/api/v1beta2/image-automation.md +++ b/docs/api/v1beta2/image-automation.md @@ -409,6 +409,21 @@ run should be attempted.

+policySelector
+ + +Kubernetes meta/v1.LabelSelector + + + + +(Optional) +

PolicySelector allows to filter applied policies based on labels. +By default includes all policies in namespace.

+ + + + update
@@ -517,6 +532,21 @@ run should be attempted.

+policySelector
+ + +Kubernetes meta/v1.LabelSelector + + + + +(Optional) +

PolicySelector allows to filter applied policies based on labels. +By default includes all policies in namespace.

+ + + + update
diff --git a/docs/spec/v1beta2/imageupdateautomations.md b/docs/spec/v1beta2/imageupdateautomations.md index 9d46d512..e21d6e35 100644 --- a/docs/spec/v1beta2/imageupdateautomations.md +++ b/docs/spec/v1beta2/imageupdateautomations.md @@ -535,6 +535,39 @@ the ImageUpdateAutomation, and changes to the resource or image policies or Git repository will not result in any update. When the field is set to `false` or removed, it will resume. +### PolicySelector + +`.spec.policySelector` is an optional field to limit policies that an +ImageUpdateAutomation takes into account. It supports the same selectors as +`Deployment.spec.selector` (`matchLabels` and `matchExpressions` fields). If +not specified, it defaults to `matchLabels: {}` which means all policies in +namespace. + +```yaml +--- +apiVersion: image.toolkit.fluxcd.io/v1beta2 +kind: ImageUpdateAutomation +metadata: + name: +spec: + policySelector: + matchLabels: + app.kubernetes.io/instance: my-app +--- +apiVersion: image.toolkit.fluxcd.io/v1beta2 +kind: ImageUpdateAutomation +metadata: + name: +spec: + policySelector: + matchExpressions: + - key: app.kubernetes.io/component + operator: In + values: + - my-component + - my-other-component +``` + ## Working with ImageUpdateAutomation ### Triggering a reconciliation @@ -901,11 +934,12 @@ completing. This can occur due to some of the following factors: - The source configuration is invalid for the current state of the source, for example, the specified branch does not exists in the remote source repository. - The remote source repository prevents push or creation of new push branch. +- The policy selector is invalid, for example, label is too long When this happens, the controller sets the `Ready` Condition status to `False` with the following reasons: -- `reason: AccessDenied` | `reason: InvalidSourceConfiguration` | `reason: GitOperationFailed` | `reason: UpdateFailed` +- `reason: AccessDenied` | `reason: InvalidSourceConfiguration` | `reason: GitOperationFailed` | `reason: UpdateFailed` | `reason: InvalidPolicySelector` While the ImageUpdateAutomation is in failing state, the controller will continue to attempt to update the source with an exponential backoff, until it diff --git a/internal/controller/imageupdateautomation_controller.go b/internal/controller/imageupdateautomation_controller.go index 0b22b56a..a5cf4905 100644 --- a/internal/controller/imageupdateautomation_controller.go +++ b/internal/controller/imageupdateautomation_controller.go @@ -26,6 +26,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" kerrors "k8s.io/apimachinery/pkg/util/errors" kuberecorder "k8s.io/client-go/tools/record" @@ -78,6 +79,8 @@ var imageUpdateAutomationNegativeConditions = []string{ meta.ReconcilingCondition, } +var errParsePolicySelector = errors.New("failed to parse policy selector") + // getPatchOptions composes patch options based on the given parameters. // It is used as the options used when patching an object. func getPatchOptions(ownedConditions []string, controllerName string) []patch.Option { @@ -303,9 +306,13 @@ func (r *ImageUpdateAutomationReconciler) reconcile(ctx context.Context, sp *pat } // List the policies and construct observed policies. - // TODO: Add support for filtering policies. - policies, err := getPolicies(ctx, r.Client, obj.Namespace) + policies, err := getPolicies(ctx, r.Client, obj.Namespace, obj.Spec.PolicySelector) if err != nil { + if errors.Is(err, errParsePolicySelector) { + conditions.MarkStalled(obj, imagev1.InvalidPolicySelectorReason, err.Error()) + result, retErr = ctrl.Result{}, nil + return + } result, retErr = ctrl.Result{}, err return } @@ -501,9 +508,17 @@ func (r *ImageUpdateAutomationReconciler) reconcileDelete(obj *imagev1.ImageUpda // getPolicies returns list of policies in the given namespace that have latest // image. -func getPolicies(ctx context.Context, kclient client.Client, namespace string) ([]imagev1_reflect.ImagePolicy, error) { +func getPolicies(ctx context.Context, kclient client.Client, namespace string, selector *metav1.LabelSelector) ([]imagev1_reflect.ImagePolicy, error) { + policySelector := labels.Everything() + var err error + if selector != nil { + if policySelector, err = metav1.LabelSelectorAsSelector(selector); err != nil { + return nil, fmt.Errorf("%w: %w", errParsePolicySelector, err) + } + } + var policies imagev1_reflect.ImagePolicyList - if err := kclient.List(ctx, &policies, &client.ListOptions{Namespace: namespace}); err != nil { + if err := kclient.List(ctx, &policies, &client.ListOptions{Namespace: namespace, LabelSelector: policySelector}); err != nil { return nil, fmt.Errorf("failed to list policies: %w", err) } diff --git a/internal/controller/update_test.go b/internal/controller/update_test.go index 81a6acfa..b335045d 100644 --- a/internal/controller/update_test.go +++ b/internal/controller/update_test.go @@ -38,6 +38,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -314,6 +315,47 @@ func TestImageUpdateAutomationReconciler_Reconcile(t *testing.T) { checker.WithT(g).CheckErr(ctx, obj) }) + t.Run("invalid policy selector results in stalled", func(t *testing.T) { + g := NewWithT(t) + + namespace, err := testEnv.CreateNamespace(ctx, "test-update") + g.Expect(err).ToNot(HaveOccurred()) + defer func() { g.Expect(testEnv.Delete(ctx, namespace)).To(Succeed()) }() + + obj := &imagev1.ImageUpdateAutomation{} + obj.Name = updateName + obj.Namespace = namespace.Name + obj.Spec = imagev1.ImageUpdateAutomationSpec{ + SourceRef: imagev1.CrossNamespaceSourceReference{ + Kind: "GitRepository", + Name: "foo", + }, + PolicySelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "label-too-long-" + strings.Repeat("0", validation.LabelValueMaxLength): "", + }, + }, + } + g.Expect(testEnv.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(deleteImageUpdateAutomation(ctx, testEnv, obj.Name, obj.Namespace)).To(Succeed()) + }() + + expectedConditions := []metav1.Condition{ + *conditions.TrueCondition(meta.StalledCondition, imagev1.InvalidPolicySelectorReason, "failed to parse policy selector"), + *conditions.FalseCondition(meta.ReadyCondition, imagev1.InvalidPolicySelectorReason, "failed to parse policy selector"), + } + g.Eventually(func(g Gomega) { + g.Expect(testEnv.Get(ctx, client.ObjectKeyFromObject(obj), obj)).To(Succeed()) + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(expectedConditions)) + }).Should(Succeed()) + + // Check if the object status is valid. + condns := &conditionscheck.Conditions{NegativePolarity: imageUpdateAutomationNegativeConditions} + checker := conditionscheck.NewChecker(testEnv.Client, condns) + checker.WithT(g).CheckErr(ctx, obj) + }) + t.Run("non-existing gitrepo results in failure", func(t *testing.T) { g := NewWithT(t) @@ -1434,11 +1476,13 @@ func Test_getPolicies(t *testing.T) { name string namespace string latestImage string + labels map[string]string } tests := []struct { name string listNamespace string + selector *metav1.LabelSelector policies []policyArgs wantPolicies []string }{ @@ -1453,6 +1497,21 @@ func Test_getPolicies(t *testing.T) { }, wantPolicies: []string{"p1", "p2"}, }, + { + name: "lists policies with label selector in same namespace", + listNamespace: testNS1, + selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "label": "one", + }, + }, + policies: []policyArgs{ + {name: "p1", namespace: testNS1, latestImage: "aaa:bbb", labels: map[string]string{"label": "one"}}, + {name: "p2", namespace: testNS1, latestImage: "ccc:ddd", labels: map[string]string{"label": "false"}}, + {name: "p3", namespace: testNS2, latestImage: "eee:fff", labels: map[string]string{"label": "one"}}, + }, + wantPolicies: []string{"p1"}, + }, { name: "no policies in empty namespace", listNamespace: testNS2, @@ -1475,13 +1534,14 @@ func Test_getPolicies(t *testing.T) { aPolicy.Status = imagev1_reflect.ImagePolicyStatus{ LatestImage: p.latestImage, } + aPolicy.Labels = p.labels testObjects = append(testObjects, aPolicy) } kClient := fakeclient.NewClientBuilder(). WithScheme(testEnv.GetScheme()). WithObjects(testObjects...).Build() - result, err := getPolicies(context.TODO(), kClient, tt.listNamespace) + result, err := getPolicies(context.TODO(), kClient, tt.listNamespace, tt.selector) g.Expect(err).ToNot(HaveOccurred()) // Extract policy name from the result and compare with the expected