diff --git a/go.mod b/go.mod index 1d7dddf98..e80edb20d 100644 --- a/go.mod +++ b/go.mod @@ -15,13 +15,14 @@ replace ( ) require ( + github.com/fluxcd/cli-utils v0.36.0-flux.1 github.com/fluxcd/helm-controller/api v0.36.2 github.com/fluxcd/pkg/apis/acl v0.1.0 github.com/fluxcd/pkg/apis/event v0.6.0 github.com/fluxcd/pkg/apis/kustomize v1.2.0 github.com/fluxcd/pkg/apis/meta v1.2.0 github.com/fluxcd/pkg/runtime v0.43.0 - github.com/fluxcd/pkg/ssa v0.34.0 + github.com/fluxcd/pkg/ssa v0.35.0 github.com/fluxcd/pkg/testserver v0.5.0 github.com/fluxcd/source-controller/api v1.1.2 github.com/go-logr/logr v1.3.0 @@ -77,7 +78,6 @@ require ( github.com/evanphx/json-patch/v5 v5.7.0 // indirect github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect github.com/fatih/color v1.13.0 // indirect - github.com/fluxcd/cli-utils v0.36.0-flux.1 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/go-errors/errors v1.4.2 // indirect github.com/go-gorp/gorp/v3 v3.1.0 // indirect diff --git a/go.sum b/go.sum index 5604a3a98..658dff04a 100644 --- a/go.sum +++ b/go.sum @@ -100,8 +100,8 @@ github.com/fluxcd/pkg/apis/meta v1.2.0 h1:O766PzGAdMdQKybSflGL8oV0+GgCNIkdsxfalR github.com/fluxcd/pkg/apis/meta v1.2.0/go.mod h1:fU/Az9AoVyIxC0oI4ihG0NVMNnvrcCzdEym3wxjIQsc= github.com/fluxcd/pkg/runtime v0.43.0 h1:dU4cWct5VTpddGzJUU80zxNl80jbbVEN5Y5rbt4YUnw= github.com/fluxcd/pkg/runtime v0.43.0/go.mod h1:RuqJ9VEXELjzgurK2+UXBBgVN1vS0hZ7CYVG2xBAEVM= -github.com/fluxcd/pkg/ssa v0.34.0 h1:hpMo0D7G3faieRYH39e9YD8Jl+aC2hTgUep8ojG5+LE= -github.com/fluxcd/pkg/ssa v0.34.0/go.mod h1:rhVh0EtYVUOznKXlz6E7JOSgdc8xWbIwA4L5HVtJRLA= +github.com/fluxcd/pkg/ssa v0.35.0 h1:8T3WY4P9SQWApa2hq1rU1u2WE8oqP3MMTsAiEWwhmfo= +github.com/fluxcd/pkg/ssa v0.35.0/go.mod h1:rhVh0EtYVUOznKXlz6E7JOSgdc8xWbIwA4L5HVtJRLA= github.com/fluxcd/pkg/testserver v0.5.0 h1:n/Iskk0tXNt2AgIgjz9qeFK/VhEXGfqeazABXZmO2Es= github.com/fluxcd/pkg/testserver v0.5.0/go.mod h1:/p4st6d0uPLy8wXydeF/kDJgxUYO9u2NqySuXb9S+Fo= github.com/fluxcd/source-controller/api v1.1.2 h1:FfKDKVWnopo+Q2pOAxgHEjrtr4MP41L8aapR4mqBhBk= diff --git a/internal/action/diff.go b/internal/action/diff.go index f20dac538..fb330eca9 100644 --- a/internal/action/diff.go +++ b/internal/action/diff.go @@ -18,20 +18,27 @@ package action import ( "context" + "encoding/json" + "errors" "fmt" + "sort" "strings" helmaction "helm.sh/helm/v3/pkg/action" helmrelease "helm.sh/helm/v3/pkg/release" - "k8s.io/apimachinery/pkg/util/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + apierrutil "k8s.io/apimachinery/pkg/util/errors" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + "github.com/fluxcd/cli-utils/pkg/object" "github.com/fluxcd/pkg/ssa" "github.com/fluxcd/pkg/ssa/jsondiff" v2 "github.com/fluxcd/helm-controller/api/v2beta2" + "github.com/fluxcd/helm-controller/internal/diff" ) // Diff returns a jsondiff.DiffSet of the changes between the state of the @@ -61,6 +68,11 @@ func Diff(ctx context.Context, config *helmaction.Configuration, rls *helmreleas errs []error ) for _, obj := range objects { + // Set the Helm metadata on the object which is normally set by Helm + // during object creation. + setHelmMetadata(obj, rls) + + // Set the namespace of the object if it is not set. if obj.GetNamespace() == "" { // Manifest does not contain the namespace of the release. // Figure out if the object is namespaced if the namespace is not @@ -86,7 +98,6 @@ func Diff(ctx context.Context, config *helmaction.Configuration, rls *helmreleas diffOpts := []jsondiff.ListOption{ jsondiff.FieldOwner(fieldOwner), jsondiff.ExclusionSelector{v2.DriftDetectionMetadataKey: v2.DriftDetectionDisabledValue}, - jsondiff.MaskSecrets(true), jsondiff.Rationalize(true), jsondiff.Graceful(true), } @@ -119,5 +130,149 @@ func Diff(ctx context.Context, config *helmaction.Configuration, rls *helmreleas if err != nil { errs = append(errs, err) } - return set, errors.Reduce(errors.Flatten(errors.NewAggregate(errs))) + return set, apierrutil.Reduce(apierrutil.Flatten(apierrutil.NewAggregate(errs))) +} + +// ApplyDiff applies the changes described in the provided jsondiff.DiffSet to +// the Kubernetes cluster. +func ApplyDiff(ctx context.Context, config *helmaction.Configuration, diffSet jsondiff.DiffSet, fieldOwner string) (*ssa.ChangeSet, error) { + cfg, err := config.RESTClientGetter.ToRESTConfig() + if err != nil { + return nil, err + } + c, err := client.New(cfg, client.Options{}) + if err != nil { + return nil, err + } + + var toCreate, toPatch sortableDiffs + for _, d := range diffSet { + switch d.Type { + case jsondiff.DiffTypeCreate: + toCreate = append(toCreate, d) + case jsondiff.DiffTypeUpdate: + toPatch = append(toPatch, d) + } + } + + var ( + changeSet = ssa.NewChangeSet() + errs []error + ) + + sort.Sort(toCreate) + for _, d := range toCreate { + obj := d.DesiredObject.DeepCopyObject().(client.Object) + if err := c.Create(ctx, obj, client.FieldOwner(fieldOwner)); err != nil { + errs = append(errs, fmt.Errorf("%s creation failure: %w", diff.ResourceName(obj), err)) + continue + } + changeSet.Add(objectToChangeSetEntry(obj, ssa.CreatedAction)) + } + + sort.Sort(toPatch) + for _, d := range toPatch { + data, err := json.Marshal(d.Patch) + if err != nil { + errs = append(errs, fmt.Errorf("%s patch failure: %w", diff.ResourceName(d.DesiredObject), err)) + continue + } + + obj := d.DesiredObject.DeepCopyObject().(client.Object) + patch := client.RawPatch(types.JSONPatchType, data) + if err := c.Patch(ctx, obj, patch, client.FieldOwner(fieldOwner)); err != nil { + if obj.GetObjectKind().GroupVersionKind().Kind == "Secret" { + err = maskSensitiveErrData(err) + } + errs = append(errs, fmt.Errorf("%s patch failure: %w", diff.ResourceName(obj), err)) + continue + } + changeSet.Add(objectToChangeSetEntry(obj, ssa.ConfiguredAction)) + } + + return changeSet, apierrutil.NewAggregate(errs) +} + +const ( + appManagedByLabel = "app.kubernetes.io/managed-by" + appManagedByHelm = "Helm" + helmReleaseNameAnnotation = "meta.helm.sh/release-name" + helmReleaseNamespaceAnnotation = "meta.helm.sh/release-namespace" +) + +// setHelmMetadata sets the metadata on the given object to indicate that it is +// managed by Helm. This is safe to do, because we apply it to objects that +// originate from the Helm release itself. +// xref: https://github.com/helm/helm/blob/v3.13.2/pkg/action/validate.go +// xref: https://github.com/helm/helm/blob/v3.13.2/pkg/action/rollback.go#L186-L191 +func setHelmMetadata(obj client.Object, rls *helmrelease.Release) { + labels := obj.GetLabels() + if labels == nil { + labels = make(map[string]string, 1) + } + labels[appManagedByLabel] = appManagedByHelm + obj.SetLabels(labels) + + annotations := obj.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string, 2) + } + annotations[helmReleaseNameAnnotation] = rls.Name + annotations[helmReleaseNamespaceAnnotation] = rls.Namespace + obj.SetAnnotations(annotations) +} + +// objectToChangeSetEntry returns a ssa.ChangeSetEntry for the given object and +// action. +func objectToChangeSetEntry(obj client.Object, action ssa.Action) ssa.ChangeSetEntry { + return ssa.ChangeSetEntry{ + ObjMetadata: object.ObjMetadata{ + GroupKind: obj.GetObjectKind().GroupVersionKind().GroupKind(), + Name: obj.GetName(), + Namespace: obj.GetNamespace(), + }, + GroupVersion: obj.GetObjectKind().GroupVersionKind().Version, + Subject: diff.ResourceName(obj), + Action: action, + } +} + +// maskSensitiveErrData masks potentially sensitive data from the error message +// returned by the Kubernetes API server. +// This avoids leaking any sensitive data in logs or other output when a patch +// operation fails. +func maskSensitiveErrData(err error) error { + if apierrors.IsInvalid(err) { + // The last part of the error message is the reason for the error. + if i := strings.LastIndex(err.Error(), `:`); i != -1 { + err = errors.New(strings.TrimSpace(err.Error()[i+1:])) + } + } + return err +} + +// sortableDiffs is a sortable slice of jsondiff.Diffs. +type sortableDiffs []*jsondiff.Diff + +// Len returns the length of the slice. +func (s sortableDiffs) Len() int { return len(s) } + +// Swap swaps the elements with indexes i and j. +func (s sortableDiffs) Swap(i, j int) { s[i], s[j] = s[j], s[i] } + +// Less returns true if the element with index i should sort before the element +// with index j. +// The elements are sorted by GroupKind, Namespace and Name. +func (s sortableDiffs) Less(i, j int) bool { + iDiff, jDiff := s[i], s[j] + + if !ssa.Equals(iDiff.GroupVersionKind().GroupKind(), jDiff.GroupVersionKind().GroupKind()) { + return ssa.IsLessThan(iDiff.GroupVersionKind().GroupKind(), jDiff.GroupVersionKind().GroupKind()) + } + + if iDiff.GetNamespace() != jDiff.GetNamespace() { + return iDiff.GetNamespace() < jDiff.GetNamespace() + } + + return iDiff.GetName() < jDiff.GetName() } diff --git a/internal/action/diff_test.go b/internal/action/diff_test.go index e3d71e3cb..493ce7fb5 100644 --- a/internal/action/diff_test.go +++ b/internal/action/diff_test.go @@ -18,6 +18,7 @@ package action import ( "context" + "encoding/base64" "fmt" "strings" "testing" @@ -25,13 +26,16 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + . "github.com/onsi/gomega" extjsondiff "github.com/wI2L/jsondiff" helmaction "helm.sh/helm/v3/pkg/action" helmrelease "helm.sh/helm/v3/pkg/release" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + apierrutil "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/rand" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -46,8 +50,8 @@ import ( func TestDiff(t *testing.T) { // Normally, we would create e.g. a `suite_test.go` file with a `TestMain` - // function. But because this is the only test in this package which needs - // a test cluster, we create it here instead. + // function. As this is one of the few tests in this package which needs a + // test cluster, we create it here instead. config, cleanup := newTestCluster(t) t.Cleanup(func() { t.Log("Stopping the test environment") @@ -72,7 +76,7 @@ func TestDiff(t *testing.T) { manifest string ignoreRules []v2.IgnoreRule mutateCluster func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, error) - want func(namepace string) jsondiff.DiffSet + want func(namespace string, desired, cluster []*unstructured.Unstructured) jsondiff.DiffSet wantErr bool }{ { @@ -118,16 +122,12 @@ data: } return clusterObjs, nil }, - want: func(namespace string) jsondiff.DiffSet { + want: func(namespace string, desired, cluster []*unstructured.Unstructured) jsondiff.DiffSet { return jsondiff.DiffSet{ { - Type: jsondiff.DiffTypeUpdate, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "ConfigMap", - }, - Namespace: namespace, - Name: "changed", + Type: jsondiff.DiffTypeUpdate, + DesiredObject: namespacedUnstructured(desired[0], namespace), + ClusterObject: cluster[0], Patch: extjsondiff.Patch{ { Type: extjsondiff.OperationReplace, @@ -138,22 +138,13 @@ data: }, }, { - Type: jsondiff.DiffTypeCreate, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "ConfigMap", - }, - Namespace: namespace, - Name: "deleted", + Type: jsondiff.DiffTypeCreate, + DesiredObject: namespacedUnstructured(desired[1], namespace), }, { - Type: jsondiff.DiffTypeNone, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "ConfigMap", - }, - Namespace: namespace, - Name: "unchanged", + Type: jsondiff.DiffTypeNone, + DesiredObject: namespacedUnstructured(desired[2], namespace), + ClusterObject: cluster[1], }, } }, @@ -185,53 +176,11 @@ data: } return clusterObjs, nil }, - want: func(namespace string) jsondiff.DiffSet { - return jsondiff.DiffSet{ - { - Type: jsondiff.DiffTypeExclude, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "ConfigMap", - }, - Namespace: namespace, - Name: "disabled", - }, - } - }, - }, - { - name: "manifest with disabled annotation", - manifest: fmt.Sprintf(`--- -apiVersion: v1 -kind: ConfigMap -metadata: - name: disabled - labels: - %[1]s: %[2]s -data: - key: value`, v2.DriftDetectionMetadataKey, v2.DriftDetectionDisabledValue), - mutateCluster: func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, error) { - var clusterObjs []*unstructured.Unstructured - for _, obj := range objs { - obj := obj.DeepCopy() - obj.SetNamespace(namespace) - if err := unstructured.SetNestedField(obj.Object, "changed", "data", "key"); err != nil { - return nil, fmt.Errorf("failed to set nested field: %w", err) - } - clusterObjs = append(clusterObjs, obj) - } - return clusterObjs, nil - }, - want: func(namespace string) jsondiff.DiffSet { + want: func(namespace string, desired, cluster []*unstructured.Unstructured) jsondiff.DiffSet { return jsondiff.DiffSet{ { - Type: jsondiff.DiffTypeExclude, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "ConfigMap", - }, - Namespace: namespace, - Name: "disabled", + Type: jsondiff.DiffTypeExclude, + DesiredObject: namespacedUnstructured(desired[0], namespace), }, } }, @@ -299,51 +248,34 @@ data: {Target: &kustomize.Selector{Name: "partially-ignored"}, Paths: []string{"/data/key"}}, {Paths: []string{"/data/globalKey"}}, }, - want: func(namespace string) jsondiff.DiffSet { + want: func(namespace string, desired, cluster []*unstructured.Unstructured) jsondiff.DiffSet { return jsondiff.DiffSet{ { - Type: jsondiff.DiffTypeExclude, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "ConfigMap", - }, - Namespace: namespace, - Name: "fully-ignored", + Type: jsondiff.DiffTypeExclude, + DesiredObject: namespacedUnstructured(desired[0], namespace), }, { - Type: jsondiff.DiffTypeUpdate, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "Secret", - }, - Namespace: namespace, - Name: "partially-ignored", + Type: jsondiff.DiffTypeUpdate, + DesiredObject: namespacedUnstructured(desired[1], namespace), + ClusterObject: cluster[1], Patch: extjsondiff.Patch{ { Type: extjsondiff.OperationReplace, Path: "/data/otherKey", - OldValue: "*** (before)", - Value: "*** (after)", + OldValue: base64.StdEncoding.EncodeToString([]byte("changed")), + Value: base64.StdEncoding.EncodeToString([]byte("otherValue")), }, }, }, { - Type: jsondiff.DiffTypeNone, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "Secret", - }, - Namespace: namespace, - Name: "globally-ignored", + Type: jsondiff.DiffTypeNone, + DesiredObject: namespacedUnstructured(desired[2], namespace), + ClusterObject: cluster[2], }, { - Type: jsondiff.DiffTypeUpdate, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "ConfigMap", - }, - Namespace: namespace, - Name: "not-ignored", + Type: jsondiff.DiffTypeUpdate, + DesiredObject: namespacedUnstructured(desired[3], namespace), + ClusterObject: cluster[3], Patch: extjsondiff.Patch{ { Type: extjsondiff.OperationReplace, @@ -394,66 +326,62 @@ data: } return clusterObjs, nil }, - want: func(namespace string) jsondiff.DiffSet { + want: func(namespace string, desired, cluster []*unstructured.Unstructured) jsondiff.DiffSet { return jsondiff.DiffSet{ { - Type: jsondiff.DiffTypeNone, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "ConfigMap", - }, - Namespace: namespace, - Name: "without-namespace", + Type: jsondiff.DiffTypeNone, + DesiredObject: namespacedUnstructured(desired[0], namespace), + ClusterObject: cluster[1], }, { - Type: jsondiff.DiffTypeNone, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "ConfigMap", - }, - Namespace: "diff-fixed-ns", - Name: "with-namespace", + Type: jsondiff.DiffTypeNone, + DesiredObject: namespacedUnstructured(desired[1], desired[1].GetNamespace()), + ClusterObject: cluster[2], }, } }, }, { - name: "masks Secret data", + name: "configures Helm metadata", manifest: `--- apiVersion: v1 -kind: Secret +kind: ConfigMap metadata: - name: secret -stringData: + name: without-helm-metadata +data: key: value`, mutateCluster: func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, error) { var clusterObjs []*unstructured.Unstructured for _, obj := range objs { obj := obj.DeepCopy() - obj.SetNamespace(namespace) - if err := unstructured.SetNestedField(obj.Object, "changed", "stringData", "key"); err != nil { - return nil, fmt.Errorf("failed to set nested field: %w", err) + if obj.GetNamespace() == "" { + obj.SetNamespace(namespace) } + obj.SetAnnotations(nil) + obj.SetLabels(nil) clusterObjs = append(clusterObjs, obj) } return clusterObjs, nil }, - want: func(namespace string) jsondiff.DiffSet { + want: func(namespace string, desired, cluster []*unstructured.Unstructured) jsondiff.DiffSet { return jsondiff.DiffSet{ { - Type: jsondiff.DiffTypeUpdate, - GroupVersionKind: schema.GroupVersionKind{ - Version: "v1", - Kind: "Secret", - }, - Namespace: namespace, - Name: "secret", + Type: jsondiff.DiffTypeUpdate, + DesiredObject: namespacedUnstructured(desired[0], namespace), + ClusterObject: cluster[0], Patch: extjsondiff.Patch{ { - Type: extjsondiff.OperationReplace, - Path: "/data/key", - OldValue: "*** (before)", - Value: "*** (after)", + Type: extjsondiff.OperationAdd, + Path: "/metadata", + Value: map[string]interface{}{ + "labels": map[string]interface{}{ + appManagedByLabel: appManagedByHelm, + }, + "annotations": map[string]interface{}{ + helmReleaseNameAnnotation: "configures Helm metadata", + helmReleaseNamespaceAnnotation: namespace, + }, + }, }, }, }, @@ -476,10 +404,15 @@ stringData: } }) + rls := &helmrelease.Release{Name: tt.name, Namespace: ns.Name, Manifest: tt.manifest} + objs, err := ssa.ReadObjects(strings.NewReader(tt.manifest)) if err != nil { t.Fatalf("Failed to read release objects: %v", err) } + for _, obj := range objs { + setHelmMetadata(obj, rls) + } clusterObjs := objs if tt.mutateCluster != nil { @@ -505,8 +438,6 @@ stringData: } } - rls := &helmrelease.Release{Namespace: ns.Name, Manifest: tt.manifest} - got, err := Diff(ctx, &helmaction.Configuration{RESTClientGetter: getter}, rls, testOwner, tt.ignoreRules...) if (err != nil) != tt.wantErr { t.Errorf("Diff() error = %v, wantErr %v", err, tt.wantErr) @@ -515,7 +446,7 @@ stringData: var want jsondiff.DiffSet if tt.want != nil { - want = tt.want(ns.Name) + want = tt.want(ns.Name, objs, clusterObjs) } if diff := cmp.Diff(want, got, cmpopts.IgnoreUnexported(extjsondiff.Operation{})); diff != "" { t.Errorf("Diff() mismatch (-want +got):\n%s", diff) @@ -524,6 +455,341 @@ stringData: } } +func TestApplyDiff(t *testing.T) { + // Normally, we would create e.g. a `suite_test.go` file with a `TestMain` + // function. As this is one of the few tests in this package which needs a + // test cluster, we create it here instead. + config, cleanup := newTestCluster(t) + t.Cleanup(func() { + t.Log("Stopping the test environment") + if err := cleanup(); err != nil { + t.Logf("Failed to stop the test environment: %v", err) + } + }) + + // Construct a REST client getter for Helm's action configuration. + getter := kube.NewMemoryRESTClientGetter(config) + + // Construct a client for to be able to mutate the cluster. + c, err := client.New(config, client.Options{}) + if err != nil { + t.Fatalf("Failed to create client for test environment: %v", err) + } + + const testOwner = "helm-controller" + + tests := []struct { + name string + diffSet func(namespace string) jsondiff.DiffSet + expect func(g *GomegaWithT, namespace string, got *ssa.ChangeSet, err error) + }{ + { + name: "creates and updates resources", + diffSet: func(namespace string) jsondiff.DiffSet { + return jsondiff.DiffSet{ + { + Type: jsondiff.DiffTypeCreate, + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "test-secret", + "namespace": namespace, + }, + "stringData": map[string]interface{}{ + "key": "value", + }, + }, + }, + }, + { + Type: jsondiff.DiffTypeUpdate, + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm", + "namespace": namespace, + }, + "data": map[string]interface{}{ + "key": "value", + }, + }, + }, + ClusterObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm", + "namespace": namespace, + }, + "data": map[string]interface{}{ + "key": "changed", + }, + }, + }, + Patch: extjsondiff.Patch{ + { + Type: extjsondiff.OperationReplace, + Path: "/data/key", + Value: "value", + }, + }, + }, + } + }, + expect: func(g *GomegaWithT, namespace string, got *ssa.ChangeSet, err error) { + g.THelper() + + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(got).NotTo(BeNil()) + g.Expect(got.Entries).To(HaveLen(2)) + + g.Expect(got.Entries[0].Subject).To(Equal("Secret/" + namespace + "/test-secret")) + g.Expect(got.Entries[0].Action).To(Equal(ssa.CreatedAction)) + g.Expect(c.Get(context.TODO(), types.NamespacedName{ + Namespace: namespace, + Name: "test-secret", + }, &corev1.Secret{})).To(Succeed()) + + g.Expect(got.Entries[1].Subject).To(Equal("ConfigMap/" + namespace + "/test-cm")) + g.Expect(got.Entries[1].Action).To(Equal(ssa.ConfiguredAction)) + cm := &corev1.ConfigMap{} + g.Expect(c.Get(context.TODO(), types.NamespacedName{ + Namespace: namespace, + Name: "test-cm", + }, cm)).To(Succeed()) + g.Expect(cm.Data).To(HaveKeyWithValue("key", "value")) + }, + }, + { + name: "continues on error", + diffSet: func(namespace string) jsondiff.DiffSet { + return jsondiff.DiffSet{ + { + Type: jsondiff.DiffTypeCreate, + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "invalid-test-secret", + "namespace": namespace, + }, + "data": map[string]interface{}{ + // Illegal base64 encoded data. + "key": "secret value", + }, + }, + }, + }, + { + Type: jsondiff.DiffTypeCreate, + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm", + "namespace": namespace, + }, + }, + }, + }, + { + Type: jsondiff.DiffTypeUpdate, + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "invalid-test-secret-update", + "namespace": namespace, + }, + "data": map[string]interface{}{ + // Illegal base64 encoded data. + "key": "secret value2", + }, + }, + }, + ClusterObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "invalid-test-secret-update", + "namespace": namespace, + }, + "stringData": map[string]interface{}{ + "key": "value", + }, + }, + }, + Patch: extjsondiff.Patch{ + { + Type: extjsondiff.OperationReplace, + Path: "/data/key", + // Illegal base64 encoded data. + Value: "value", + }, + }, + }, + { + Type: jsondiff.DiffTypeUpdate, + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm-2", + "namespace": namespace, + }, + "data": map[string]interface{}{ + "key": "value", + }, + }, + }, + ClusterObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm-2", + "namespace": namespace, + }, + "data": map[string]interface{}{ + "key": "changed", + }, + }, + }, + Patch: extjsondiff.Patch{ + { + Type: extjsondiff.OperationReplace, + Path: "/data/key", + Value: "value", + }, + }, + }, + } + }, + expect: func(g *GomegaWithT, namespace string, got *ssa.ChangeSet, err error) { + g.THelper() + + g.Expect(err).To(HaveOccurred()) + g.Expect(err.(apierrutil.Aggregate).Errors()).To(HaveLen(2)) + g.Expect(err.Error()).To(ContainSubstring("invalid-test-secret creation failure")) + g.Expect(err.Error()).To(ContainSubstring("invalid-test-secret-update patch failure")) + + // Verify that the error message does not contain the secret data. + g.Expect(err.Error()).ToNot(ContainSubstring("secret value")) + g.Expect(err.Error()).ToNot(ContainSubstring("secret value2")) + + g.Expect(got).NotTo(BeNil()) + g.Expect(got.Entries).To(HaveLen(2)) + + g.Expect(got.Entries[0].Subject).To(Equal("ConfigMap/" + namespace + "/test-cm")) + g.Expect(got.Entries[0].Action).To(Equal(ssa.CreatedAction)) + g.Expect(c.Get(context.TODO(), types.NamespacedName{ + Namespace: namespace, + Name: "test-cm", + }, &corev1.ConfigMap{})).To(Succeed()) + + g.Expect(got.Entries[1].Subject).To(Equal("ConfigMap/" + namespace + "/test-cm-2")) + g.Expect(got.Entries[1].Action).To(Equal(ssa.ConfiguredAction)) + + cm2 := &corev1.ConfigMap{} + g.Expect(c.Get(context.TODO(), types.NamespacedName{ + Namespace: namespace, + Name: "test-cm-2", + }, cm2)).To(Succeed()) + g.Expect(cm2.Data).To(HaveKeyWithValue("key", "value")) + }, + }, + { + name: "creates namespace before dependent resources", + diffSet: func(namespace string) jsondiff.DiffSet { + otherNS := generateName("test-ns") + + return jsondiff.DiffSet{ + { + Type: jsondiff.DiffTypeCreate, + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "test-cm", + "namespace": otherNS, + }, + }, + }, + }, + { + Type: jsondiff.DiffTypeCreate, + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Namespace", + "metadata": map[string]interface{}{ + "name": otherNS, + }, + }, + }, + }, + } + }, + expect: func(g *GomegaWithT, namespace string, got *ssa.ChangeSet, err error) { + g.THelper() + + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(got).NotTo(BeNil()) + g.Expect(got.Entries).To(HaveLen(2)) + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + t.Cleanup(cancel) + + ns, err := generateNamespace(ctx, c, "diff-action") + if err != nil { + t.Fatalf("Failed to generate namespace: %v", err) + } + t.Cleanup(func() { + if err := c.Delete(context.Background(), ns); client.IgnoreNotFound(err) != nil { + t.Logf("Failed to delete generated namespace: %v", err) + } + }) + + diff := tt.diffSet(ns.Name) + + for _, d := range diff { + if d.ClusterObject != nil { + if err := c.Create(ctx, d.ClusterObject, client.FieldOwner(testOwner)); err != nil { + t.Fatalf("Failed to create cluster object: %v", err) + } + t.Cleanup(func() { + if err := c.Delete(ctx, d.ClusterObject); client.IgnoreNotFound(err) != nil { + t.Logf("Failed to delete cluster object: %v", err) + } + }) + } + } + + got, err := ApplyDiff(context.Background(), &helmaction.Configuration{RESTClientGetter: getter}, diff, testOwner) + tt.expect(g, ns.Name, got, err) + }) + } +} + // newTestCluster creates a new test cluster and returns a rest.Config and a // function to stop the test cluster. func newTestCluster(t *testing.T) (*rest.Config, func() error) { @@ -552,3 +818,15 @@ func generateNamespace(ctx context.Context, c client.Client, generateName string } return ns, nil } + +// generateName generates a name with the given name and a random suffix. +func generateName(name string) string { + return fmt.Sprintf("%s-%s", name, rand.String(5)) +} + +func namespacedUnstructured(obj *unstructured.Unstructured, namespace string) *unstructured.Unstructured { + obj = obj.DeepCopy() + obj.SetNamespace(namespace) + _ = ssa.NormalizeUnstructured(obj) + return obj +} diff --git a/internal/diff/summarize.go b/internal/diff/summarize.go index c553120c7..9b1d80d9b 100644 --- a/internal/diff/summarize.go +++ b/internal/diff/summarize.go @@ -21,6 +21,7 @@ import ( "strings" extjsondiff "github.com/wI2L/jsondiff" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/fluxcd/pkg/ssa/jsondiff" ) @@ -49,10 +50,10 @@ var DefaultDiffTypes = []jsondiff.DiffType{ // // For example: // -// Deployment/default/hello-world: changed (1 added, 1 changed, 1 removed) -// Deployment/default/hello-world2: removed -// Deployment/default/hello-world3: excluded -// Deployment/default/hello-world4: unchanged +// Deployment/default/hello-world changed (1 added, 1 changed, 1 removed) +// Deployment/default/hello-world2 removed +// Deployment/default/hello-world3 excluded +// Deployment/default/hello-world4 unchanged func SummarizeDiffSet(set jsondiff.DiffSet, include ...jsondiff.DiffType) string { if include == nil { include = DefaultDiffTypes @@ -66,16 +67,16 @@ func SummarizeDiffSet(set jsondiff.DiffSet, include ...jsondiff.DiffType) string switch diff.Type { case jsondiff.DiffTypeNone: - writeResourceName(diff, &summary) + writeResourceName(diff.DesiredObject, &summary) summary.WriteString(" unchanged\n") case jsondiff.DiffTypeCreate: - writeResourceName(diff, &summary) + writeResourceName(diff.DesiredObject, &summary) summary.WriteString(" removed\n") case jsondiff.DiffTypeExclude: - writeResourceName(diff, &summary) + writeResourceName(diff.DesiredObject, &summary) summary.WriteString(" excluded\n") case jsondiff.DiffTypeUpdate: - writeResourceName(diff, &summary) + writeResourceName(diff.DesiredObject, &summary) added, changed, removed := summarizeUpdate(diff) summary.WriteString(fmt.Sprintf(" changed (%d additions, %d changes, %d removals)\n", added, changed, removed)) } @@ -127,14 +128,25 @@ func SummarizeDiffSetBrief(set jsondiff.DiffSet, include ...jsondiff.DiffType) s return strings.TrimSuffix(summary.String(), ", ") } +// ResourceName returns the resource name in the format `kind/namespace/name`. +func ResourceName(obj client.Object) string { + var summary strings.Builder + writeResourceName(obj, &summary) + return summary.String() +} + +const resourceSeparator = "/" + // writeResourceName writes the resource name in the format // `kind/namespace/name` to the given strings.Builder. -func writeResourceName(diff *jsondiff.Diff, summary *strings.Builder) { - summary.WriteString(diff.GroupVersionKind.Kind) - summary.WriteString("/") - summary.WriteString(diff.Namespace) - summary.WriteString("/") - summary.WriteString(diff.Name) +func writeResourceName(obj client.Object, summary *strings.Builder) { + summary.WriteString(obj.GetObjectKind().GroupVersionKind().Kind) + summary.WriteString(resourceSeparator) + if ns := obj.GetNamespace(); ns != "" { + summary.WriteString(ns) + summary.WriteString(resourceSeparator) + } + summary.WriteString(obj.GetName()) } // SummarizeUpdate returns the number of added, changed and removed fields diff --git a/internal/diff/summarize_test.go b/internal/diff/summarize_test.go index 8b9024379..b67206e8a 100644 --- a/internal/diff/summarize_test.go +++ b/internal/diff/summarize_test.go @@ -20,7 +20,8 @@ import ( "testing" extjsondiff "github.com/wI2L/jsondiff" - "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/fluxcd/pkg/ssa/jsondiff" ) @@ -28,36 +29,52 @@ import ( func TestSummarizeDiffSet(t *testing.T) { diffSet := jsondiff.DiffSet{ &jsondiff.Diff{ - GroupVersionKind: schema.GroupVersionKind{ - Kind: "ConfigMap", + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "config", + "namespace": "namespace-1", + }, + }, }, - Namespace: "namespace-1", - Name: "config", - Type: jsondiff.DiffTypeNone, + Type: jsondiff.DiffTypeNone, }, &jsondiff.Diff{ - GroupVersionKind: schema.GroupVersionKind{ - Kind: "Secret", + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "naughty", + "namespace": "namespace-x", + }, + }, }, - Namespace: "namespace-x", - Name: "naughty", - Type: jsondiff.DiffTypeCreate, + Type: jsondiff.DiffTypeCreate, }, &jsondiff.Diff{ - GroupVersionKind: schema.GroupVersionKind{ - Kind: "StatefulSet", + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "StatefulSet", + "metadata": map[string]interface{}{ + "name": "hello-world", + "namespace": "default", + }, + }, }, - Namespace: "default", - Name: "hello-world", - Type: jsondiff.DiffTypeExclude, + Type: jsondiff.DiffTypeExclude, }, &jsondiff.Diff{ - GroupVersionKind: schema.GroupVersionKind{ - Kind: "Deployment", + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "touched-me", + "namespace": "tenant-y", + }, + }, }, - Namespace: "tenant-y", - Name: "touched-me", - Type: jsondiff.DiffTypeUpdate, + Type: jsondiff.DiffTypeUpdate, Patch: extjsondiff.Patch{ {Type: extjsondiff.OperationAdd}, {Type: extjsondiff.OperationReplace}, @@ -188,3 +205,44 @@ func TestSummarizeDiffSetBrief(t *testing.T) { }) } } + +func TestResourceName(t *testing.T) { + tests := []struct { + name string + resource client.Object + want string + }{ + { + name: "with namespace", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "touched-me", + "namespace": "tenant-y", + }, + }, + }, + want: "Deployment/tenant-y/touched-me", + }, + { + name: "without namespace", + resource: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "ClusterIssuer", + "metadata": map[string]interface{}{ + "name": "letsencrypt", + }, + }, + }, + want: "ClusterIssuer/letsencrypt", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ResourceName(tt.resource); got != tt.want { + t.Errorf("ResourceName() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/reconcile/atomic_release.go b/internal/reconcile/atomic_release.go index 3949cc0b2..87190f6ad 100644 --- a/internal/reconcile/atomic_release.go +++ b/internal/reconcile/atomic_release.go @@ -23,6 +23,7 @@ import ( "strings" "time" + "helm.sh/helm/v3/pkg/kube" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -320,8 +321,14 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state case ReleaseStatusDrifted: log.Info(msgWithReason("detected changes in cluster state", diff.SummarizeDiffSetBrief(state.Diff))) for _, change := range state.Diff { - if change.Type == jsondiff.DiffTypeCreate || change.Type == jsondiff.DiffTypeUpdate { - log.V(logger.DebugLevel).Info("observed change in cluster state", "diff", change) + switch change.Type { + case jsondiff.DiffTypeCreate: + log.V(logger.DebugLevel).Info("resource deleted", + "resource", diff.ResourceName(change.DesiredObject)) + case jsondiff.DiffTypeUpdate: + log.V(logger.DebugLevel).Info("resource modified", + "resource", diff.ResourceName(change.DesiredObject), + "patch", jsondiff.MaskSecretPatchData(change.Patch)) } } @@ -331,7 +338,7 @@ func (r *AtomicRelease) actionForState(ctx context.Context, req *Request, state ) if req.Object.GetDriftDetection().GetMode() == v2.DriftDetectionEnabled { - return NewUpgrade(r.configFactory, r.eventRecorder), nil + return NewCorrectClusterDrift(r.configFactory, r.eventRecorder, state.Diff, kube.ManagedFieldsManager), nil } return nil, nil diff --git a/internal/reconcile/atomic_release_test.go b/internal/reconcile/atomic_release_test.go index a6cbcde1b..d932fb65f 100644 --- a/internal/reconcile/atomic_release_test.go +++ b/internal/reconcile/atomic_release_test.go @@ -31,7 +31,7 @@ import ( helmdriver "helm.sh/helm/v3/pkg/storage/driver" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/tools/record" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -1062,17 +1062,20 @@ func TestAtomicRelease_actionForState(t *testing.T) { want: &Upgrade{}, }, { - name: "drifted release triggers upgrade if enabled", + name: "drifted release triggers correction if enabled", state: ReleaseState{Status: ReleaseStatusDrifted, Diff: jsondiff.DiffSet{ { Type: jsondiff.DiffTypeCreate, - GroupVersionKind: schema.GroupVersionKind{ - Group: "apps", - Kind: "Deployment", - Version: "v1", + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "mock", + "namespace": "something", + }, + }, }, - Name: "mock", - Namespace: "something", }, }}, spec: func(spec *v2.HelmReleaseSpec) { @@ -1091,7 +1094,7 @@ func TestAtomicRelease_actionForState(t *testing.T) { }, } }, - want: &Upgrade{}, + want: &CorrectClusterDrift{}, wantEvent: &corev1.Event{ Reason: "DriftDetected", Type: corev1.EventTypeWarning, @@ -1123,13 +1126,32 @@ func TestAtomicRelease_actionForState(t *testing.T) { state: ReleaseState{Status: ReleaseStatusDrifted, Diff: jsondiff.DiffSet{ { Type: jsondiff.DiffTypeUpdate, - GroupVersionKind: schema.GroupVersionKind{ - Group: "apps", - Kind: "Deployment", - Version: "v1", + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "mock", + "namespace": "something", + }, + "spec": map[string]interface{}{ + "replicas": 2, + }, + }, + }, + ClusterObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "mock", + "namespace": "something", + }, + "spec": map[string]interface{}{ + "replicas": 1, + }, + }, }, - Name: "mock", - Namespace: "something", Patch: extjsondiff.Patch{ { Type: extjsondiff.OperationReplace, diff --git a/internal/reconcile/correct_cluster_drift.go b/internal/reconcile/correct_cluster_drift.go new file mode 100644 index 000000000..bbd6f7a31 --- /dev/null +++ b/internal/reconcile/correct_cluster_drift.go @@ -0,0 +1,117 @@ +/* +Copyright 2023 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconcile + +import ( + "context" + "strings" + + corev1 "k8s.io/api/core/v1" + apierrutil "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/client-go/tools/record" + + "github.com/fluxcd/pkg/ssa" + "github.com/fluxcd/pkg/ssa/jsondiff" + + v2 "github.com/fluxcd/helm-controller/api/v2beta2" + "github.com/fluxcd/helm-controller/internal/action" +) + +// CorrectClusterDrift is a reconciler that attempts to correct the cluster state +// of a Helm release. It does so by applying the Helm release's desired state +// to the cluster based on a jsondiff.DiffSet. +// +// The reconciler will only attempt to correct the cluster state if the Helm +// release has drift detection enabled and the jsondiff.DiffSet is not empty. +// +// The reconciler will emit a Kubernetes event upon completion indicating +// whether the cluster state was successfully corrected or not. +type CorrectClusterDrift struct { + configFactory *action.ConfigFactory + eventRecorder record.EventRecorder + diff jsondiff.DiffSet + fieldManager string +} + +func NewCorrectClusterDrift(configFactory *action.ConfigFactory, recorder record.EventRecorder, diff jsondiff.DiffSet, fieldManager string) *CorrectClusterDrift { + return &CorrectClusterDrift{ + configFactory: configFactory, + eventRecorder: recorder, + diff: diff, + fieldManager: fieldManager, + } +} + +func (r *CorrectClusterDrift) Reconcile(ctx context.Context, req *Request) error { + if req.Object.GetDriftDetection().GetMode() != v2.DriftDetectionEnabled || len(r.diff) == 0 { + return nil + } + + ctx, cancel := context.WithTimeout(ctx, req.Object.GetTimeout().Duration) + defer cancel() + + changeSet, err := action.ApplyDiff(ctx, r.configFactory.Build(nil), r.diff, r.fieldManager) + r.report(req.Object, changeSet, err) + return nil +} + +func (r *CorrectClusterDrift) report(obj *v2.HelmRelease, changeSet *ssa.ChangeSet, err error) { + cur := obj.Status.History.Latest() + + switch { + case err != nil: + var sb strings.Builder + sb.WriteString("Failed to ") + if changeSet != nil && len(changeSet.Entries) > 0 { + sb.WriteString("partially ") + } + sb.WriteString("correct cluster state of release ") + sb.WriteString(cur.FullReleaseName()) + sb.WriteString(":\n") + if agErr, ok := err.(apierrutil.Aggregate); ok { + for i := range agErr.Errors() { + if i > 0 { + sb.WriteString("\n") + } + sb.WriteString(agErr.Errors()[i].Error()) + } + } else { + sb.WriteString(err.Error()) + } + + if changeSet != nil && len(changeSet.Entries) > 0 { + sb.WriteString("\n\n") + sb.WriteString("Successful corrections:\n") + sb.WriteString(changeSet.String()) + } + + r.eventRecorder.AnnotatedEventf(obj, eventMeta(cur.ChartVersion, cur.ConfigDigest), corev1.EventTypeWarning, + "DriftCorrectionFailed", sb.String()) + case changeSet != nil && len(changeSet.Entries) > 0: + r.eventRecorder.AnnotatedEventf(obj, eventMeta(cur.ChartVersion, cur.ConfigDigest), corev1.EventTypeNormal, + "DriftCorrected", "Cluster state of release %s has been corrected:\n%s", + obj.Status.History.Latest().FullReleaseName(), changeSet.String()) + } +} + +func (r *CorrectClusterDrift) Name() string { + return "correct cluster drift" +} + +func (r *CorrectClusterDrift) Type() ReconcilerType { + return ReconcilerTypeDriftCorrection +} diff --git a/internal/reconcile/correct_cluster_drift_test.go b/internal/reconcile/correct_cluster_drift_test.go new file mode 100644 index 000000000..83c14a071 --- /dev/null +++ b/internal/reconcile/correct_cluster_drift_test.go @@ -0,0 +1,314 @@ +/* +Copyright 2023 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconcile + +import ( + "context" + "errors" + "testing" + + . "github.com/onsi/gomega" + extjsondiff "github.com/wI2L/jsondiff" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + apierrutil "k8s.io/apimachinery/pkg/util/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/fluxcd/pkg/ssa" + "github.com/fluxcd/pkg/ssa/jsondiff" + + v2 "github.com/fluxcd/helm-controller/api/v2beta2" + "github.com/fluxcd/helm-controller/internal/action" + "github.com/fluxcd/helm-controller/internal/testutil" +) + +func TestCorrectClusterDrift_Reconcile(t *testing.T) { + mockStatus := v2.HelmReleaseStatus{ + History: v2.Snapshots{ + { + Version: 2, + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + }, + }, + } + + tests := []struct { + name string + obj *v2.HelmRelease + diff func(namespace string) jsondiff.DiffSet + wantEvent bool + }{ + { + name: "corrects cluster drift", + obj: &v2.HelmRelease{ + Spec: v2.HelmReleaseSpec{ + DriftDetection: &v2.DriftDetection{ + Mode: v2.DriftDetectionEnabled, + }, + }, + Status: *mockStatus.DeepCopy(), + }, + diff: func(namespace string) jsondiff.DiffSet { + return jsondiff.DiffSet{ + { + Type: jsondiff.DiffTypeCreate, + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "secret", + "namespace": namespace, + }, + }, + }, + }, + { + Type: jsondiff.DiffTypeUpdate, + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "configmap", + "namespace": namespace, + }, + "data": map[string]interface{}{ + "key": "value", + }, + }, + }, + ClusterObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "configmap", + "namespace": namespace, + }, + }, + }, + Patch: extjsondiff.Patch{ + { + Type: extjsondiff.OperationAdd, + Path: "/data", + Value: map[string]interface{}{ + "key": "value", + }, + }, + }, + }, + } + }, + wantEvent: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + namedNS, err := testEnv.CreateNamespace(context.TODO(), mockReleaseNamespace) + g.Expect(err).NotTo(HaveOccurred()) + t.Cleanup(func() { + _ = testEnv.Delete(context.TODO(), namedNS) + }) + + diff := tt.diff(namedNS.Name) + for _, diff := range diff { + if diff.ClusterObject != nil { + obj := diff.ClusterObject.DeepCopyObject() + g.Expect(testEnv.Create(context.TODO(), obj.(client.Object))).To(Succeed()) + } + } + + getter, err := RESTClientGetterFromManager(testEnv.Manager, namedNS.Name) + g.Expect(err).ToNot(HaveOccurred()) + + cfg, err := action.NewConfigFactory(getter, action.WithStorage(action.DefaultStorageDriver, namedNS.Name)) + g.Expect(err).ToNot(HaveOccurred()) + + recorder := testutil.NewFakeRecorder(10, false) + + r := NewCorrectClusterDrift(cfg, recorder, tt.diff(namedNS.Name), testFieldManager) + g.Expect(r.Reconcile(context.TODO(), &Request{ + Object: tt.obj, + })).ToNot(HaveOccurred()) + + if tt.wantEvent { + g.Expect(recorder.GetEvents()).To(HaveLen(1)) + } else { + g.Expect(recorder.GetEvents()).To(BeEmpty()) + } + }) + } +} + +func TestCorrectClusterDrift_report(t *testing.T) { + mockObj := &v2.HelmRelease{ + Status: v2.HelmReleaseStatus{ + History: v2.Snapshots{ + { + Version: 3, + Name: mockReleaseName, + Namespace: mockReleaseNamespace, + }, + }, + }, + } + + tests := []struct { + name string + obj *v2.HelmRelease + changeSet *ssa.ChangeSet + err error + wantEvent []corev1.Event + }{ + { + name: "with multiple changes", + obj: mockObj.DeepCopy(), + changeSet: &ssa.ChangeSet{ + Entries: []ssa.ChangeSetEntry{ + { + Subject: "Secret/namespace/name", + Action: ssa.CreatedAction, + }, + { + Subject: "Deployment/namespace/name", + Action: ssa.ConfiguredAction, + }, + }, + }, + wantEvent: []corev1.Event{ + { + Type: corev1.EventTypeNormal, + Reason: "DriftCorrected", + Message: `Cluster state of release mock-ns/mock-release.v3 has been corrected: +Secret/namespace/name created +Deployment/namespace/name configured`, + }, + }, + }, + { + name: "with multiple changes and errors", + obj: mockObj.DeepCopy(), + changeSet: &ssa.ChangeSet{ + Entries: []ssa.ChangeSetEntry{ + { + Subject: "Secret/namespace/name", + Action: ssa.CreatedAction, + }, + { + Subject: "Deployment/namespace/name", + Action: ssa.ConfiguredAction, + }, + { + Subject: "ConfigMap/namespace/name", + Action: ssa.ConfiguredAction, + }, + }, + }, + err: apierrutil.NewAggregate([]error{ + errors.New("error 1"), + errors.New("error 2"), + }), + wantEvent: []corev1.Event{ + { + Type: corev1.EventTypeWarning, + Reason: "DriftCorrectionFailed", + Message: `Failed to partially correct cluster state of release mock-ns/mock-release.v3: +error 1 +error 2 + +Successful corrections: +Secret/namespace/name created +Deployment/namespace/name configured +ConfigMap/namespace/name configured`, + }, + }, + }, + { + name: "with multiple errors", + obj: mockObj.DeepCopy(), + err: apierrutil.NewAggregate([]error{ + errors.New("error 1"), + errors.New("error 2"), + }), + wantEvent: []corev1.Event{ + { + Type: corev1.EventTypeWarning, + Reason: "DriftCorrectionFailed", + Message: `Failed to correct cluster state of release mock-ns/mock-release.v3: +error 1 +error 2`, + }, + }, + }, + { + name: "with single change", + obj: mockObj.DeepCopy(), + changeSet: &ssa.ChangeSet{ + Entries: []ssa.ChangeSetEntry{ + { + Subject: "Secret/namespace/name", + Action: ssa.CreatedAction, + }, + }, + }, + wantEvent: []corev1.Event{ + { + Type: corev1.EventTypeNormal, + Reason: "DriftCorrected", + Message: `Cluster state of release mock-ns/mock-release.v3 has been corrected: +Secret/namespace/name created`, + }, + }, + }, + { + name: "with single error", + obj: mockObj.DeepCopy(), + err: errors.New("error 1"), + wantEvent: []corev1.Event{ + { + Type: corev1.EventTypeWarning, + Reason: "DriftCorrectionFailed", + Message: `Failed to correct cluster state of release mock-ns/mock-release.v3: +error 1`, + }, + }, + }, + { + name: "empty change set", + obj: mockObj.DeepCopy(), + wantEvent: []corev1.Event{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + recorder := testutil.NewFakeRecorder(10, false) + r := &CorrectClusterDrift{ + eventRecorder: recorder, + } + + r.report(tt.obj, tt.changeSet, tt.err) + g.Expect(recorder.GetEvents()).To(ConsistOf(tt.wantEvent)) + }) + } +} diff --git a/internal/reconcile/reconcile.go b/internal/reconcile/reconcile.go index 2565e57fc..1cd9a0999 100644 --- a/internal/reconcile/reconcile.go +++ b/internal/reconcile/reconcile.go @@ -38,6 +38,9 @@ const ( // release in a stale pending state. It differs from ReconcilerTypeRemediate // in that it does not produce a new Helm release. ReconcilerTypeUnlock ReconcilerType = "unlock" + // ReconcilerTypeDriftCorrection is an ActionReconciler which corrects + // Helm releases which have drifted from the cluster state. + ReconcilerTypeDriftCorrection ReconcilerType = "drift correction" ) // ReconcilerType is a string which identifies the type of ActionReconciler. diff --git a/internal/reconcile/release_test.go b/internal/reconcile/release_test.go index 038c94309..117820323 100644 --- a/internal/reconcile/release_test.go +++ b/internal/reconcile/release_test.go @@ -17,7 +17,6 @@ limitations under the License. package reconcile import ( - "reflect" "testing" "github.com/go-logr/logr" @@ -36,23 +35,6 @@ const ( mockReleaseNamespace = "mock-ns" ) -func Test_observedReleases_sortedVersions(t *testing.T) { - tests := []struct { - name string - r observedReleases - wantVersions []int - }{ - // TODO: Add test cases. - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if gotVersions := tt.r.sortedVersions(); !reflect.DeepEqual(gotVersions, tt.wantVersions) { - t.Errorf("sortedVersions() = %v, want %v", gotVersions, tt.wantVersions) - } - }) - } -} - func Test_summarize(t *testing.T) { tests := []struct { name string diff --git a/internal/reconcile/state_test.go b/internal/reconcile/state_test.go index 62154f684..abc25ef3e 100644 --- a/internal/reconcile/state_test.go +++ b/internal/reconcile/state_test.go @@ -27,7 +27,7 @@ import ( helmrelease "helm.sh/helm/v3/pkg/release" helmstorage "helm.sh/helm/v3/pkg/storage" helmdriver "helm.sh/helm/v3/pkg/storage/driver" - "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/fluxcd/pkg/ssa" "github.com/fluxcd/pkg/ssa/jsondiff" @@ -517,12 +517,24 @@ func TestDetermineReleaseState_DriftDetection(t *testing.T) { Diff: jsondiff.DiffSet{ { Type: jsondiff.DiffTypeCreate, - GroupVersionKind: schema.GroupVersionKind{ - Kind: "Secret", - Version: "v1", + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "fixture", + "namespace": namespace, + "creationTimestamp": nil, + "labels": map[string]interface{}{ + "app.kubernetes.io/managed-by": "Helm", + }, + "annotations": map[string]interface{}{ + "meta.helm.sh/release-name": mockReleaseName, + "meta.helm.sh/release-namespace": namespace, + }, + }, + }, }, - Namespace: namespace, - Name: "fixture", }, }, } @@ -545,12 +557,24 @@ func TestDetermineReleaseState_DriftDetection(t *testing.T) { Diff: jsondiff.DiffSet{ { Type: jsondiff.DiffTypeCreate, - GroupVersionKind: schema.GroupVersionKind{ - Kind: "Secret", - Version: "v1", + DesiredObject: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "fixture", + "namespace": namespace, + "creationTimestamp": nil, + "labels": map[string]interface{}{ + "app.kubernetes.io/managed-by": "Helm", + }, + "annotations": map[string]interface{}{ + "meta.helm.sh/release-name": mockReleaseName, + "meta.helm.sh/release-namespace": namespace, + }, + }, + }, }, - Namespace: namespace, - Name: "fixture", }, }, } @@ -601,6 +625,13 @@ func TestDetermineReleaseState_DriftDetection(t *testing.T) { for _, obj := range objs { g.Expect(ssa.NormalizeUnstructured(obj)).To(Succeed()) obj.SetNamespace(releaseNamespace) + obj.SetLabels(map[string]string{ + "app.kubernetes.io/managed-by": "Helm", + }) + obj.SetAnnotations(map[string]string{ + "meta.helm.sh/release-name": rls.Name, + "meta.helm.sh/release-namespace": rls.Namespace, + }) g.Expect(testEnv.Create(context.Background(), obj)).To(Succeed()) } }