Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to limit applied policies in automation by specifying a selector #619

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/v1beta2/condition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,7 @@ const (

// UpdateFailedReason represents a failure during source update.
UpdateFailedReason string = "UpdateFailed"

// InvalidPolicySelectorReason represents an invalid policy selector.
InvalidPolicySelectorReason string = "InvalidPolicySelector"
)
5 changes: 5 additions & 0 deletions api/v1beta2/imageupdateautomation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions api/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions docs/api/v1beta2/image-automation.md
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,21 @@ run should be attempted.</p>
</tr>
<tr>
<td>
<code>policySelector</code><br>
<em>
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#labelselector-v1-meta">
Kubernetes meta/v1.LabelSelector
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>PolicySelector allows to filter applied policies based on labels.
By default includes all policies in namespace.</p>
</td>
</tr>
<tr>
<td>
<code>update</code><br>
<em>
<a href="#image.toolkit.fluxcd.io/v1beta2.UpdateStrategy">
Expand Down Expand Up @@ -517,6 +532,21 @@ run should be attempted.</p>
</tr>
<tr>
<td>
<code>policySelector</code><br>
<em>
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#labelselector-v1-meta">
Kubernetes meta/v1.LabelSelector
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>PolicySelector allows to filter applied policies based on labels.
By default includes all policies in namespace.</p>
</td>
</tr>
<tr>
<td>
<code>update</code><br>
<em>
<a href="#image.toolkit.fluxcd.io/v1beta2.UpdateStrategy">
Expand Down
36 changes: 35 additions & 1 deletion docs/spec/v1beta2/imageupdateautomations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: <automation-name>
spec:
policySelector:
matchLabels:
app.kubernetes.io/instance: my-app
---
apiVersion: image.toolkit.fluxcd.io/v1beta2
kind: ImageUpdateAutomation
metadata:
name: <automation-name>
spec:
policySelector:
matchExpressions:
- key: app.kubernetes.io/component
operator: In
values:
- my-component
- my-other-component
```

## Working with ImageUpdateAutomation

### Triggering a reconciliation
Expand Down Expand Up @@ -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`
Nitive marked this conversation as resolved.
Show resolved Hide resolved

While the ImageUpdateAutomation is in failing state, the controller will
continue to attempt to update the source with an exponential backoff, until it
Expand Down
23 changes: 19 additions & 4 deletions internal/controller/imageupdateautomation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}

Expand Down
62 changes: 61 additions & 1 deletion internal/controller/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
}{
Expand All @@ -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,
Expand All @@ -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
Expand Down
Loading