From 485cb60add06a0938b73128f55cb2d02cbf24ada Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Fri, 19 May 2023 11:12:42 -0400 Subject: [PATCH 01/18] feat: Implement Server-Side Diffs Signed-off-by: Leonardo Luz Almeida --- pkg/cache/cluster.go | 6 ++++ pkg/diff/diff.go | 51 ++++++++++++++++++++++++++++++++++ pkg/diff/diff_options.go | 22 +++++++++++++++ pkg/utils/kube/resource_ops.go | 17 ++++++++++-- 4 files changed, 93 insertions(+), 3 deletions(-) diff --git a/pkg/cache/cluster.go b/pkg/cache/cluster.go index 02ca18911..a54a0c8e8 100644 --- a/pkg/cache/cluster.go +++ b/pkg/cache/cluster.go @@ -132,6 +132,8 @@ type ClusterCache interface { OnResourceUpdated(handler OnResourceUpdatedHandler) Unsubscribe // OnEvent register event handler that is executed every time when new K8S event received OnEvent(handler OnEventHandler) Unsubscribe + + GetKubectl() kube.Kubectl } type WeightedSemaphore interface { @@ -319,6 +321,10 @@ func (c *clusterCache) GetGVKParser() *managedfields.GvkParser { return c.gvkParser } +func (c *clusterCache) GetKubectl() kube.Kubectl { + return c.kubectl +} + func (c *clusterCache) appendAPIResource(info kube.APIResourceInfo) { exists := false for i := range c.apiResources { diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 39087784b..775b12d01 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -6,6 +6,7 @@ package diff import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -20,6 +21,7 @@ import ( "k8s.io/apimachinery/pkg/util/managedfields" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/client-go/kubernetes/scheme" + cmdutil "k8s.io/kubectl/pkg/cmd/util" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" "sigs.k8s.io/structured-merge-diff/v4/merge" "sigs.k8s.io/structured-merge-diff/v4/typed" @@ -83,6 +85,14 @@ func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, Normalize(live, opts...) } + if o.serverSideDiff { + r, err := ServerSideDiff(config, live, o.manager, o.kubeApplier, opts...) + if err != nil { + return nil, fmt.Errorf("error calculating server side diff: %w", err) + } + return r, nil + } + // TODO The two variables bellow are necessary because there is a cyclic // dependency with the kube package that blocks the usage of constants // from common package. common package needs to be refactored and exclude @@ -120,6 +130,47 @@ func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, return TwoWayDiff(config, live) } +func ServerSideDiff(config, live *unstructured.Unstructured, manager string, kubeApplier KubeApplier, opts ...Option) (*DiffResult, error) { + if live != nil && config != nil { + return serverSideDiff(config, live, manager, kubeApplier, opts...) + } + return handleResourceCreateOrDeleteDiff(config, live) +} + +func serverSideDiff(config, live *unstructured.Unstructured, manager string, kubeApplier KubeApplier, opts ...Option) (*DiffResult, error) { + predictedLiveStr, err := kubeApplier.ApplyResource(context.Background(), config, cmdutil.DryRunServer, false, false, true, manager) + if err != nil { + return nil, fmt.Errorf("error running server side apply in dryrun mode: %w", err) + } + predictedLive, err := jsonStrToUnstructured(predictedLiveStr) + if err != nil { + return nil, fmt.Errorf("error converting json string to unstructured: %w", err) + } + + Normalize(predictedLive, opts...) + + predictedLiveBytes, err := json.Marshal(predictedLive) + if err != nil { + return nil, fmt.Errorf("error marshaling predicted live resource: %w", err) + } + + unstructured.RemoveNestedField(live.Object, "metadata", "managedFields") + liveBytes, err := json.Marshal(live) + if err != nil { + return nil, fmt.Errorf("error marshaling live resource: %w", err) + } + return buildDiffResult(predictedLiveBytes, liveBytes), nil +} + +func jsonStrToUnstructured(jsonString string) (*unstructured.Unstructured, error) { + res := make(map[string]interface{}) + err := json.Unmarshal([]byte(jsonString), &res) + if err != nil { + return nil, fmt.Errorf("unmarshal error: %s", err) + } + return &unstructured.Unstructured{Object: res}, nil +} + // StructuredMergeDiff will calculate the diff using the structured-merge-diff // k8s library (https://github.com/kubernetes-sigs/structured-merge-diff). func StructuredMergeDiff(config, live *unstructured.Unstructured, gvkParser *managedfields.GvkParser, manager string) (*DiffResult, error) { diff --git a/pkg/diff/diff_options.go b/pkg/diff/diff_options.go index 64cfdf414..e9c486061 100644 --- a/pkg/diff/diff_options.go +++ b/pkg/diff/diff_options.go @@ -1,9 +1,13 @@ package diff import ( + "context" + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/managedfields" "k8s.io/klog/v2/klogr" + cmdutil "k8s.io/kubectl/pkg/cmd/util" ) type Option func(*options) @@ -17,6 +21,8 @@ type options struct { structuredMergeDiff bool gvkParser *managedfields.GvkParser manager string + serverSideDiff bool + kubeApplier KubeApplier } func applyOptions(opts []Option) options { @@ -31,6 +37,10 @@ func applyOptions(opts []Option) options { return o } +type KubeApplier interface { + ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) +} + func IgnoreAggregatedRoles(ignore bool) Option { return func(o *options) { o.ignoreAggregatedRoles = ignore @@ -66,3 +76,15 @@ func WithManager(manager string) Option { o.manager = manager } } + +func WithServerSideDiff(ssd bool) Option { + return func(o *options) { + o.serverSideDiff = ssd + } +} + +func WithKubeApplier(ka KubeApplier) Option { + return func(o *options) { + o.kubeApplier = ka + } +} diff --git a/pkg/utils/kube/resource_ops.go b/pkg/utils/kube/resource_ops.go index 34ece2cd5..a033a7f2c 100644 --- a/pkg/utils/kube/resource_ops.go +++ b/pkg/utils/kube/resource_ops.go @@ -292,9 +292,16 @@ func (k *kubectlResourceOperations) newApplyOptions(ioStreams genericclioptions. return nil, err } case cmdutil.DryRunServer: - err = o.PrintFlags.Complete("%s (server dry run)") - if err != nil { - return nil, err + // TODO (SSD): This logic should be refactored. PrintFlags should be + // configurable by the caller. + if serverSideApply { + o.PrintFlags.JSONYamlPrintFlags.ShowManagedFields = false + return o.PrintFlags.JSONYamlPrintFlags.ToPrinter("json") + } else { + err = o.PrintFlags.Complete("%s (server dry run)") + if err != nil { + return nil, err + } } } return o.PrintFlags.ToPrinter() @@ -312,6 +319,10 @@ func (k *kubectlResourceOperations) newApplyOptions(ioStreams genericclioptions. return o, nil } +func toPointer(str string) *string { + return &str +} + func (k *kubectlResourceOperations) newCreateOptions(config *rest.Config, ioStreams genericclioptions.IOStreams, fileName string, dryRunStrategy cmdutil.DryRunStrategy) (*create.CreateOptions, error) { o := create.NewCreateOptions(ioStreams) From e1754b3bfc5ff152d1e603b4a7078d2c5b7de346 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Wed, 15 Nov 2023 10:00:44 -0500 Subject: [PATCH 02/18] trigger build Signed-off-by: Leonardo Luz Almeida From 622eeff4399818fd192fd7903eb27e9373e9214f Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Wed, 15 Nov 2023 10:13:47 -0500 Subject: [PATCH 03/18] chore: remove unused function Signed-off-by: Leonardo Luz Almeida --- pkg/utils/kube/resource_ops.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/utils/kube/resource_ops.go b/pkg/utils/kube/resource_ops.go index a033a7f2c..ded42f179 100644 --- a/pkg/utils/kube/resource_ops.go +++ b/pkg/utils/kube/resource_ops.go @@ -319,10 +319,6 @@ func (k *kubectlResourceOperations) newApplyOptions(ioStreams genericclioptions. return o, nil } -func toPointer(str string) *string { - return &str -} - func (k *kubectlResourceOperations) newCreateOptions(config *rest.Config, ioStreams genericclioptions.IOStreams, fileName string, dryRunStrategy cmdutil.DryRunStrategy) (*create.CreateOptions, error) { o := create.NewCreateOptions(ioStreams) From ebe332e329b5ca266ee979b66c46305bff844da1 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Fri, 1 Dec 2023 16:32:00 -0500 Subject: [PATCH 04/18] make HasAnnotationOption more generic Signed-off-by: Leonardo Luz Almeida --- pkg/sync/resource/annotations.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pkg/sync/resource/annotations.go b/pkg/sync/resource/annotations.go index e62297eb3..73a879ce7 100644 --- a/pkg/sync/resource/annotations.go +++ b/pkg/sync/resource/annotations.go @@ -2,11 +2,18 @@ package resource import ( "strings" - - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -func GetAnnotationCSVs(obj *unstructured.Unstructured, key string) []string { +// AnnotationGetter defines the operations required to inspect if a resource +// has annotations +type AnnotationGetter interface { + GetAnnotations() map[string]string +} + +// GetAnnotationCSVs will return the value of the annotation identified by +// the given key. If the annotation has comma separated values, the returned +// list will contain all deduped values. +func GetAnnotationCSVs(obj AnnotationGetter, key string) []string { // may for de-duping valuesToBool := make(map[string]bool) for _, item := range strings.Split(obj.GetAnnotations()[key], ",") { @@ -22,7 +29,9 @@ func GetAnnotationCSVs(obj *unstructured.Unstructured, key string) []string { return values } -func HasAnnotationOption(obj *unstructured.Unstructured, key, val string) bool { +// HasAnnotationOption will return if the given obj has an annotation defined +// as the given key and has in its values, the ocurrence of val. +func HasAnnotationOption(obj AnnotationGetter, key, val string) bool { for _, item := range GetAnnotationCSVs(obj, key) { if item == val { return true From bcd1567fd355bf41c8082e6cc82878c64eea2bbf Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Mon, 4 Dec 2023 10:59:43 -0500 Subject: [PATCH 05/18] add server-side-diff printer option Signed-off-by: Leonardo Luz Almeida --- pkg/diff/diff.go | 2 +- pkg/diff/diff_options.go | 2 +- pkg/sync/sync_context.go | 2 +- .../kube/kubetest/mock_resource_operations.go | 2 +- pkg/utils/kube/resource_ops.go | 14 ++++++-------- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 775b12d01..07c679f41 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -138,7 +138,7 @@ func ServerSideDiff(config, live *unstructured.Unstructured, manager string, kub } func serverSideDiff(config, live *unstructured.Unstructured, manager string, kubeApplier KubeApplier, opts ...Option) (*DiffResult, error) { - predictedLiveStr, err := kubeApplier.ApplyResource(context.Background(), config, cmdutil.DryRunServer, false, false, true, manager) + predictedLiveStr, err := kubeApplier.ApplyResource(context.Background(), config, cmdutil.DryRunServer, false, false, true, manager, true) if err != nil { return nil, fmt.Errorf("error running server side apply in dryrun mode: %w", err) } diff --git a/pkg/diff/diff_options.go b/pkg/diff/diff_options.go index e9c486061..6767337c0 100644 --- a/pkg/diff/diff_options.go +++ b/pkg/diff/diff_options.go @@ -38,7 +38,7 @@ func applyOptions(opts []Option) options { } type KubeApplier interface { - ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) + ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string, serverSideDiff bool) (string, error) } func IgnoreAggregatedRoles(ignore bool) Option { diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index 6561a9e10..c2ed67691 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -923,7 +923,7 @@ func (sc *syncContext) applyObject(t *syncTask, dryRun, force, validate bool) (c shouldReplace := sc.replace || resourceutil.HasAnnotationOption(t.targetObj, common.AnnotationSyncOptions, common.SyncOptionReplace) applyFn := func(dryRunStrategy cmdutil.DryRunStrategy) (string, error) { if !shouldReplace { - return sc.resourceOps.ApplyResource(context.TODO(), t.targetObj, dryRunStrategy, force, validate, serverSideApply, sc.serverSideApplyManager) + return sc.resourceOps.ApplyResource(context.TODO(), t.targetObj, dryRunStrategy, force, validate, serverSideApply, sc.serverSideApplyManager, false) } if t.liveObj == nil { return sc.resourceOps.CreateResource(context.TODO(), t.targetObj, dryRunStrategy, validate) diff --git a/pkg/utils/kube/kubetest/mock_resource_operations.go b/pkg/utils/kube/kubetest/mock_resource_operations.go index 31ea6a0e4..f56d460bf 100644 --- a/pkg/utils/kube/kubetest/mock_resource_operations.go +++ b/pkg/utils/kube/kubetest/mock_resource_operations.go @@ -91,7 +91,7 @@ func (r *MockResourceOps) GetLastResourceCommand(key kube.ResourceKey) string { return r.lastCommandPerResource[key] } -func (r *MockResourceOps) ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) { +func (r *MockResourceOps) ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string, serverSideDiff bool) (string, error) { r.SetLastValidate(validate) r.SetLastServerSideApply(serverSideApply) r.SetLastServerSideApplyManager(manager) diff --git a/pkg/utils/kube/resource_ops.go b/pkg/utils/kube/resource_ops.go index ded42f179..4920283f1 100644 --- a/pkg/utils/kube/resource_ops.go +++ b/pkg/utils/kube/resource_ops.go @@ -38,7 +38,7 @@ import ( // ResourceOperations provides methods to manage k8s resources type ResourceOperations interface { - ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) + ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string, serverSideDiff bool) (string, error) ReplaceResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force bool) (string, error) CreateResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, validate bool) (string, error) UpdateResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy) (*unstructured.Unstructured, error) @@ -224,7 +224,7 @@ func (k *kubectlResourceOperations) UpdateResource(ctx context.Context, obj *uns } // ApplyResource performs an apply of a unstructured resource -func (k *kubectlResourceOperations) ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) { +func (k *kubectlResourceOperations) ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string, serverSideDiff bool) (string, error) { span := k.tracer.StartSpan("ApplyResource") span.SetBaggageItem("kind", obj.GetKind()) span.SetBaggageItem("name", obj.GetName()) @@ -237,7 +237,7 @@ func (k *kubectlResourceOperations) ApplyResource(ctx context.Context, obj *unst } defer cleanup() - applyOpts, err := k.newApplyOptions(ioStreams, obj, fileName, validate, force, serverSideApply, dryRunStrategy, manager) + applyOpts, err := k.newApplyOptions(ioStreams, obj, fileName, validate, force, serverSideApply, dryRunStrategy, manager, serverSideDiff) if err != nil { return err } @@ -245,7 +245,7 @@ func (k *kubectlResourceOperations) ApplyResource(ctx context.Context, obj *unst }) } -func (k *kubectlResourceOperations) newApplyOptions(ioStreams genericclioptions.IOStreams, obj *unstructured.Unstructured, fileName string, validate bool, force, serverSideApply bool, dryRunStrategy cmdutil.DryRunStrategy, manager string) (*apply.ApplyOptions, error) { +func (k *kubectlResourceOperations) newApplyOptions(ioStreams genericclioptions.IOStreams, obj *unstructured.Unstructured, fileName string, validate bool, force, serverSideApply bool, dryRunStrategy cmdutil.DryRunStrategy, manager string, serverSideDiff bool) (*apply.ApplyOptions, error) { flags := apply.NewApplyFlags(k.fact, ioStreams) o := &apply.ApplyOptions{ IOStreams: ioStreams, @@ -292,10 +292,8 @@ func (k *kubectlResourceOperations) newApplyOptions(ioStreams genericclioptions. return nil, err } case cmdutil.DryRunServer: - // TODO (SSD): This logic should be refactored. PrintFlags should be - // configurable by the caller. - if serverSideApply { - o.PrintFlags.JSONYamlPrintFlags.ShowManagedFields = false + if serverSideDiff { + o.PrintFlags.JSONYamlPrintFlags.ShowManagedFields = true return o.PrintFlags.JSONYamlPrintFlags.ToPrinter("json") } else { err = o.PrintFlags.Complete("%s (server dry run)") From 19f82e637fe0f9c3554c9f48b058e285e3d36874 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Tue, 5 Dec 2023 11:05:13 -0500 Subject: [PATCH 06/18] remove managedFields during server-side-diff Signed-off-by: Leonardo Luz Almeida --- pkg/diff/diff.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 07c679f41..cc635d144 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -148,6 +148,7 @@ func serverSideDiff(config, live *unstructured.Unstructured, manager string, kub } Normalize(predictedLive, opts...) + unstructured.RemoveNestedField(predictedLive.Object, "metadata", "managedFields") predictedLiveBytes, err := json.Marshal(predictedLive) if err != nil { From 833d0787470086108ef8c2004a51fd1eadee4565 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Tue, 5 Dec 2023 15:47:12 -0500 Subject: [PATCH 07/18] add ignore mutation webhook logic Signed-off-by: Leonardo Luz Almeida --- pkg/diff/diff.go | 55 ++++++++++++++++++++++++++++++++++++---- pkg/diff/diff_options.go | 8 ++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index cc635d144..4e3c4fe9b 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -86,7 +86,7 @@ func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, } if o.serverSideDiff { - r, err := ServerSideDiff(config, live, o.manager, o.kubeApplier, opts...) + r, err := ServerSideDiff(config, live, opts...) if err != nil { return nil, fmt.Errorf("error calculating server side diff: %w", err) } @@ -130,15 +130,19 @@ func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, return TwoWayDiff(config, live) } -func ServerSideDiff(config, live *unstructured.Unstructured, manager string, kubeApplier KubeApplier, opts ...Option) (*DiffResult, error) { +func ServerSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, error) { if live != nil && config != nil { - return serverSideDiff(config, live, manager, kubeApplier, opts...) + return serverSideDiff(config, live, opts...) } return handleResourceCreateOrDeleteDiff(config, live) } -func serverSideDiff(config, live *unstructured.Unstructured, manager string, kubeApplier KubeApplier, opts ...Option) (*DiffResult, error) { - predictedLiveStr, err := kubeApplier.ApplyResource(context.Background(), config, cmdutil.DryRunServer, false, false, true, manager, true) +func serverSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, error) { + o := applyOptions(opts) + if o.kubeApplier == nil { + return nil, fmt.Errorf("serverSideDiff error: kubeApplier is null") + } + predictedLiveStr, err := o.kubeApplier.ApplyResource(context.Background(), config, cmdutil.DryRunServer, false, false, true, o.manager, true) if err != nil { return nil, fmt.Errorf("error running server side apply in dryrun mode: %w", err) } @@ -147,6 +151,13 @@ func serverSideDiff(config, live *unstructured.Unstructured, manager string, kub return nil, fmt.Errorf("error converting json string to unstructured: %w", err) } + if o.ignoreMutationWebhook { + predictedLive, err = removeWebhookMutation(predictedLive, config, live, o.gvkParser, o.manager) + if err != nil { + return nil, fmt.Errorf("error removing webhook mutations: %w", err) + } + } + Normalize(predictedLive, opts...) unstructured.RemoveNestedField(predictedLive.Object, "metadata", "managedFields") @@ -163,6 +174,40 @@ func serverSideDiff(config, live *unstructured.Unstructured, manager string, kub return buildDiffResult(predictedLiveBytes, liveBytes), nil } +func removeWebhookMutation(predictedLive, config, live *unstructured.Unstructured, gvkParser *managedfields.GvkParser, manager string) (*unstructured.Unstructured, error) { + gvk := predictedLive.GetObjectKind().GroupVersionKind() + pt := gvkParser.Type(gvk) + typedPreditedLive, err := pt.FromUnstructured(predictedLive.Object) + if err != nil { + return nil, fmt.Errorf("error creating typedPreditedLive: %s", err) + } + typedConfig, err := pt.FromUnstructured(config.Object) + if err != nil { + return nil, fmt.Errorf("error creating typedConfig: %s", err) + } + typedLive, err := pt.FromUnstructured(live.Object) + if err != nil { + return nil, fmt.Errorf("error creating typedLive: %s", err) + } + + comparison, err := typedPreditedLive.Compare(typedLive) + if err != nil { + return nil, fmt.Errorf("error comparing typedPreditedLive with typedLive: %s", err) + } + + configfs, _ := typedConfig.ToFieldSet() + + webhookAdded := comparison.Added.Difference(configfs) + typedPreditedLive = typedPreditedLive.RemoveItems(webhookAdded) + + plu := typedPreditedLive.AsValue().Unstructured() + pl, ok := plu.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("error converting live typedValue: expected map got %T", plu) + } + return &unstructured.Unstructured{Object: pl}, nil +} + func jsonStrToUnstructured(jsonString string) (*unstructured.Unstructured, error) { res := make(map[string]interface{}) err := json.Unmarshal([]byte(jsonString), &res) diff --git a/pkg/diff/diff_options.go b/pkg/diff/diff_options.go index 6767337c0..b4178f297 100644 --- a/pkg/diff/diff_options.go +++ b/pkg/diff/diff_options.go @@ -23,11 +23,13 @@ type options struct { manager string serverSideDiff bool kubeApplier KubeApplier + ignoreMutationWebhook bool } func applyOptions(opts []Option) options { o := options{ ignoreAggregatedRoles: false, + ignoreMutationWebhook: true, normalizer: GetNoopNormalizer(), log: klogr.New(), } @@ -83,6 +85,12 @@ func WithServerSideDiff(ssd bool) Option { } } +func WithIgnoreMutationWebhook(mw bool) Option { + return func(o *options) { + o.ignoreMutationWebhook = mw + } +} + func WithKubeApplier(ka KubeApplier) Option { return func(o *options) { o.kubeApplier = ka From 47e1e5b8e73fb4edddf7f5d71e12b888da796382 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Tue, 5 Dec 2023 16:29:35 -0500 Subject: [PATCH 08/18] fix configSet Signed-off-by: Leonardo Luz Almeida --- pkg/diff/diff.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 4e3c4fe9b..c9c52450d 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -25,6 +25,7 @@ import ( "sigs.k8s.io/structured-merge-diff/v4/fieldpath" "sigs.k8s.io/structured-merge-diff/v4/merge" "sigs.k8s.io/structured-merge-diff/v4/typed" + "sigs.k8s.io/structured-merge-diff/v4/value" "github.com/argoproj/gitops-engine/internal/kubernetes_vendor/pkg/api/v1/endpoints" "github.com/argoproj/gitops-engine/pkg/diff/internal/fieldmanager" @@ -181,10 +182,7 @@ func removeWebhookMutation(predictedLive, config, live *unstructured.Unstructure if err != nil { return nil, fmt.Errorf("error creating typedPreditedLive: %s", err) } - typedConfig, err := pt.FromUnstructured(config.Object) - if err != nil { - return nil, fmt.Errorf("error creating typedConfig: %s", err) - } + typedLive, err := pt.FromUnstructured(live.Object) if err != nil { return nil, fmt.Errorf("error creating typedLive: %s", err) @@ -195,9 +193,10 @@ func removeWebhookMutation(predictedLive, config, live *unstructured.Unstructure return nil, fmt.Errorf("error comparing typedPreditedLive with typedLive: %s", err) } - configfs, _ := typedConfig.ToFieldSet() + configValue := value.NewValueInterface(config.Object) + configSet := fieldpath.SetFromValue(configValue) - webhookAdded := comparison.Added.Difference(configfs) + webhookAdded := comparison.Added.Difference(configSet) typedPreditedLive = typedPreditedLive.RemoveItems(webhookAdded) plu := typedPreditedLive.AsValue().Unstructured() From f8c0ba75eac656214756b27526d83fce0844297a Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Tue, 5 Dec 2023 16:43:22 -0500 Subject: [PATCH 09/18] Fix comparison Signed-off-by: Leonardo Luz Almeida --- pkg/diff/diff.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index c9c52450d..adb98353a 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -188,7 +188,7 @@ func removeWebhookMutation(predictedLive, config, live *unstructured.Unstructure return nil, fmt.Errorf("error creating typedLive: %s", err) } - comparison, err := typedPreditedLive.Compare(typedLive) + comparison, err := typedLive.Compare(typedPreditedLive) if err != nil { return nil, fmt.Errorf("error comparing typedPreditedLive with typedLive: %s", err) } From e7b9c99dfbe8e8eaae5823c23a6289c2edc843f3 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Tue, 5 Dec 2023 17:39:52 -0500 Subject: [PATCH 10/18] merge typedconfig in typedpredictedlive Signed-off-by: Leonardo Luz Almeida --- pkg/diff/diff.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index adb98353a..020e455d9 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -199,6 +199,12 @@ func removeWebhookMutation(predictedLive, config, live *unstructured.Unstructure webhookAdded := comparison.Added.Difference(configSet) typedPreditedLive = typedPreditedLive.RemoveItems(webhookAdded) + typedConfig := typed.AsTypedUnvalidated(configValue, typedLive.Schema(), typedLive.TypeRef()) + typedPreditedLive, err = typedPreditedLive.Merge(typedConfig) + if err != nil { + return nil, fmt.Errorf("error merging typedPreditedLive with typedConfig: %s", err) + } + plu := typedPreditedLive.AsValue().Unstructured() pl, ok := plu.(map[string]interface{}) if !ok { From 1cca5368485852610b566f7f947b51af425bfcf0 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Wed, 6 Dec 2023 17:15:30 -0500 Subject: [PATCH 11/18] handle webhook diff conflicts Signed-off-by: Leonardo Luz Almeida --- pkg/diff/diff.go | 88 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 9 deletions(-) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 020e455d9..48aa70dd6 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -11,6 +11,7 @@ import ( "errors" "fmt" "reflect" + "strings" jsonpatch "github.com/evanphx/json-patch" corev1 "k8s.io/api/core/v1" @@ -89,6 +90,9 @@ func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, if o.serverSideDiff { r, err := ServerSideDiff(config, live, opts...) if err != nil { + if _, ok := err.(WebhookConflictErrors); ok { + return r, err + } return nil, fmt.Errorf("error calculating server side diff: %w", err) } return r, nil @@ -152,10 +156,15 @@ func serverSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*D return nil, fmt.Errorf("error converting json string to unstructured: %w", err) } + var webhookConflictErros error if o.ignoreMutationWebhook { predictedLive, err = removeWebhookMutation(predictedLive, config, live, o.gvkParser, o.manager) if err != nil { - return nil, fmt.Errorf("error removing webhook mutations: %w", err) + if e, ok := err.(WebhookConflictErrors); ok { + webhookConflictErros = e + } else { + return nil, fmt.Errorf("error removing webhook mutations: %w", err) + } } } @@ -172,9 +181,14 @@ func serverSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*D if err != nil { return nil, fmt.Errorf("error marshaling live resource: %w", err) } - return buildDiffResult(predictedLiveBytes, liveBytes), nil + return buildDiffResult(predictedLiveBytes, liveBytes), webhookConflictErros } +// removeWebhookMutation will compare the predictedLive with live and config to +// identify changes in the predictedLive state done by mutation webhooks. Changes +// that are not part of the desired state (config) are reverted with the state +// from live. Changes on fields defined in the desired state are considered as +// conflict and a WebhookConflictErrors is also returned in this case. func removeWebhookMutation(predictedLive, config, live *unstructured.Unstructured, gvkParser *managedfields.GvkParser, manager string) (*unstructured.Unstructured, error) { gvk := predictedLive.GetObjectKind().GroupVersionKind() pt := gvkParser.Type(gvk) @@ -196,13 +210,52 @@ func removeWebhookMutation(predictedLive, config, live *unstructured.Unstructure configValue := value.NewValueInterface(config.Object) configSet := fieldpath.SetFromValue(configValue) - webhookAdded := comparison.Added.Difference(configSet) - typedPreditedLive = typedPreditedLive.RemoveItems(webhookAdded) + if comparison.Added != nil { + // check if the added fields are not part of the desired state + webhookAdded := comparison.Added.Difference(configSet) + if !webhookAdded.Empty() { + // if not part of desired state, added fields are removed from the + // predictedLive + typedPreditedLive = typedPreditedLive.RemoveItems(webhookAdded) + } + } + + errs := []error{} + if comparison.Modified != nil { + // check if the modified fields are not part of the desired state + webhookModified := comparison.Modified.Difference(configSet) + if !webhookModified.Empty() { + // if not part of desired state, modified fields are reverted with + // the current state from live + liveModValues := typedLive.ExtractItems(webhookModified) + typedPreditedLive, err = typedPreditedLive.Merge(liveModValues) + if err != nil { + return nil, fmt.Errorf("error merging webhookModified in predictedLive: %s", err) + } - typedConfig := typed.AsTypedUnvalidated(configValue, typedLive.Schema(), typedLive.TypeRef()) - typedPreditedLive, err = typedPreditedLive.Merge(typedConfig) - if err != nil { - return nil, fmt.Errorf("error merging typedPreditedLive with typedConfig: %s", err) + // check if the modified fields are part of the desired state + webhookModifiedWithConflict := comparison.Modified.Intersection(configSet) + if !webhookModifiedWithConflict.Empty() { + errs = append(errs, fmt.Errorf("%s with name %s has fields modified", predictedLive.GetKind(), predictedLive.GetName())) + } + } + } + + if comparison.Removed != nil { + // check if the removed fields are not part of the desired state + webhookRemoved := comparison.Removed.Difference(configSet) + if !webhookRemoved.Empty() { + // if not part of desired state, removed fields are added back with + // the current state from live + liveRmValues := typedLive.ExtractItems(webhookRemoved) + typedPreditedLive.Merge(liveRmValues) + + // check if the removed fields are part of the desired state + webhookRemovedWithConflict := comparison.Removed.Intersection(configSet) + if !webhookRemovedWithConflict.Empty() { + errs = append(errs, fmt.Errorf("%s with name %s has fields removed", predictedLive.GetKind(), predictedLive.GetName())) + } + } } plu := typedPreditedLive.AsValue().Unstructured() @@ -210,7 +263,24 @@ func removeWebhookMutation(predictedLive, config, live *unstructured.Unstructure if !ok { return nil, fmt.Errorf("error converting live typedValue: expected map got %T", plu) } - return &unstructured.Unstructured{Object: pl}, nil + var errors error + if len(errs) > 0 { + errors = WebhookConflictErrors{errors: errs} + } + return &unstructured.Unstructured{Object: pl}, errors +} + +type WebhookConflictErrors struct { + errors []error +} + +func (e WebhookConflictErrors) Error() string { + var sb strings.Builder + sb.WriteString("Webhook mutations found in the desired state") + for _, err := range e.errors { + sb.WriteString(fmt.Sprintf(": %s", err.Error())) + } + return sb.String() } func jsonStrToUnstructured(jsonString string) (*unstructured.Unstructured, error) { From 158a833350fcf3d55b4814cc12cfd4420317ebd8 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Thu, 7 Dec 2023 14:58:44 -0500 Subject: [PATCH 12/18] Fix webhook normalization logic Signed-off-by: Leonardo Luz Almeida --- go.mod | 2 +- go.sum | 4 +- pkg/diff/diff.go | 114 +++++++++++++++-------------------------------- 3 files changed, 39 insertions(+), 81 deletions(-) diff --git a/go.mod b/go.mod index 9e4b02627..0cd2d2759 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f k8s.io/kubectl v0.26.4 k8s.io/kubernetes v1.26.4 - sigs.k8s.io/structured-merge-diff/v4 v4.2.3 + sigs.k8s.io/structured-merge-diff/v4 v4.4.1 sigs.k8s.io/yaml v1.3.0 ) diff --git a/go.sum b/go.sum index eff7f90e9..572406000 100644 --- a/go.sum +++ b/go.sum @@ -670,7 +670,7 @@ sigs.k8s.io/kustomize/api v0.12.1 h1:7YM7gW3kYBwtKvoY216ZzY+8hM+lV53LUayghNRJ0vM sigs.k8s.io/kustomize/api v0.12.1/go.mod h1:y3JUhimkZkR6sbLNwfJHxvo1TCLwuwm14sCYnkH6S1s= sigs.k8s.io/kustomize/kyaml v0.13.9 h1:Qz53EAaFFANyNgyOEJbT/yoIHygK40/ZcvU3rgry2Tk= sigs.k8s.io/kustomize/kyaml v0.13.9/go.mod h1:QsRbD0/KcU+wdk0/L0fIp2KLnohkVzs6fQ85/nOXac4= -sigs.k8s.io/structured-merge-diff/v4 v4.2.3 h1:PRbqxJClWWYMNV1dhaG4NsibJbArud9kFxnAMREiWFE= -sigs.k8s.io/structured-merge-diff/v4 v4.2.3/go.mod h1:qjx8mGObPmV2aSZepjQjbmb2ihdVs8cGKBraizNC69E= +sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4= +sigs.k8s.io/structured-merge-diff/v4 v4.4.1/go.mod h1:N8hJocpFajUSSeSJ9bOZ77VzejKZaXsTtZo4/u7Io08= sigs.k8s.io/yaml v1.3.0 h1:a2VclLzOGrwOHDiV8EfBGhvjHvP46CtW5j6POvhYGGo= sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8= diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 48aa70dd6..6ef50e7e8 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -11,7 +11,6 @@ import ( "errors" "fmt" "reflect" - "strings" jsonpatch "github.com/evanphx/json-patch" corev1 "k8s.io/api/core/v1" @@ -26,7 +25,6 @@ import ( "sigs.k8s.io/structured-merge-diff/v4/fieldpath" "sigs.k8s.io/structured-merge-diff/v4/merge" "sigs.k8s.io/structured-merge-diff/v4/typed" - "sigs.k8s.io/structured-merge-diff/v4/value" "github.com/argoproj/gitops-engine/internal/kubernetes_vendor/pkg/api/v1/endpoints" "github.com/argoproj/gitops-engine/pkg/diff/internal/fieldmanager" @@ -90,9 +88,6 @@ func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, if o.serverSideDiff { r, err := ServerSideDiff(config, live, opts...) if err != nil { - if _, ok := err.(WebhookConflictErrors); ok { - return r, err - } return nil, fmt.Errorf("error calculating server side diff: %w", err) } return r, nil @@ -156,15 +151,10 @@ func serverSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*D return nil, fmt.Errorf("error converting json string to unstructured: %w", err) } - var webhookConflictErros error if o.ignoreMutationWebhook { - predictedLive, err = removeWebhookMutation(predictedLive, config, live, o.gvkParser, o.manager) + predictedLive, err = removeWebhookMutation(predictedLive, live, o.gvkParser, o.manager) if err != nil { - if e, ok := err.(WebhookConflictErrors); ok { - webhookConflictErros = e - } else { - return nil, fmt.Errorf("error removing webhook mutations: %w", err) - } + return nil, fmt.Errorf("error removing non config mutations: %w", err) } } @@ -181,15 +171,14 @@ func serverSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*D if err != nil { return nil, fmt.Errorf("error marshaling live resource: %w", err) } - return buildDiffResult(predictedLiveBytes, liveBytes), webhookConflictErros + return buildDiffResult(predictedLiveBytes, liveBytes), nil } -// removeWebhookMutation will compare the predictedLive with live and config to -// identify changes in the predictedLive state done by mutation webhooks. Changes -// that are not part of the desired state (config) are reverted with the state -// from live. Changes on fields defined in the desired state are considered as -// conflict and a WebhookConflictErrors is also returned in this case. -func removeWebhookMutation(predictedLive, config, live *unstructured.Unstructured, gvkParser *managedfields.GvkParser, manager string) (*unstructured.Unstructured, error) { +// removeWebhookMutation will compare the predictedLive with live to identify +// changes done by mutation webhooks. Webhook mutations are identified by finding +// changes in predictedLive fields not associated with any manager. All fields +// under this condition will be reverted with their state from live. +func removeWebhookMutation(predictedLive, live *unstructured.Unstructured, gvkParser *managedfields.GvkParser, manager string) (*unstructured.Unstructured, error) { gvk := predictedLive.GetObjectKind().GroupVersionKind() pt := gvkParser.Type(gvk) typedPreditedLive, err := pt.FromUnstructured(predictedLive.Object) @@ -207,54 +196,40 @@ func removeWebhookMutation(predictedLive, config, live *unstructured.Unstructure return nil, fmt.Errorf("error comparing typedPreditedLive with typedLive: %s", err) } - configValue := value.NewValueInterface(config.Object) - configSet := fieldpath.SetFromValue(configValue) - - if comparison.Added != nil { - // check if the added fields are not part of the desired state - webhookAdded := comparison.Added.Difference(configSet) - if !webhookAdded.Empty() { - // if not part of desired state, added fields are removed from the - // predictedLive - typedPreditedLive = typedPreditedLive.RemoveItems(webhookAdded) + for _, mfEntry := range predictedLive.GetManagedFields() { + mfs := &fieldpath.Set{} + err := mfs.FromJSON(bytes.NewReader(mfEntry.FieldsV1.Raw)) + if err != nil { + return nil, fmt.Errorf("error building managedFields set: %s", err) + } + if comparison.Added != nil && !comparison.Added.Empty() { + comparison.Added = comparison.Added.Difference(mfs) + } + if comparison.Modified != nil && !comparison.Modified.Empty() { + comparison.Modified = comparison.Modified.Difference(mfs) + } + if comparison.Removed != nil && !comparison.Removed.Empty() { + comparison.Removed = comparison.Removed.Difference(mfs) } } - errs := []error{} - if comparison.Modified != nil { - // check if the modified fields are not part of the desired state - webhookModified := comparison.Modified.Difference(configSet) - if !webhookModified.Empty() { - // if not part of desired state, modified fields are reverted with - // the current state from live - liveModValues := typedLive.ExtractItems(webhookModified) - typedPreditedLive, err = typedPreditedLive.Merge(liveModValues) - if err != nil { - return nil, fmt.Errorf("error merging webhookModified in predictedLive: %s", err) - } + if comparison.Added != nil && !comparison.Added.Empty() { + typedPreditedLive = typedPreditedLive.RemoveItems(comparison.Added) + } - // check if the modified fields are part of the desired state - webhookModifiedWithConflict := comparison.Modified.Intersection(configSet) - if !webhookModifiedWithConflict.Empty() { - errs = append(errs, fmt.Errorf("%s with name %s has fields modified", predictedLive.GetKind(), predictedLive.GetName())) - } + if comparison.Modified != nil && !comparison.Modified.Empty() { + liveModValues := typedLive.ExtractItems(comparison.Modified) + typedPreditedLive, err = typedPreditedLive.Merge(liveModValues) + if err != nil { + return nil, fmt.Errorf("error merging liveModValues in typedPreditedLive: %s", err) } } - if comparison.Removed != nil { - // check if the removed fields are not part of the desired state - webhookRemoved := comparison.Removed.Difference(configSet) - if !webhookRemoved.Empty() { - // if not part of desired state, removed fields are added back with - // the current state from live - liveRmValues := typedLive.ExtractItems(webhookRemoved) - typedPreditedLive.Merge(liveRmValues) - - // check if the removed fields are part of the desired state - webhookRemovedWithConflict := comparison.Removed.Intersection(configSet) - if !webhookRemovedWithConflict.Empty() { - errs = append(errs, fmt.Errorf("%s with name %s has fields removed", predictedLive.GetKind(), predictedLive.GetName())) - } + if comparison.Removed != nil && !comparison.Removed.Empty() { + liveRmValues := typedLive.ExtractItems(comparison.Removed) + typedPreditedLive, err = typedPreditedLive.Merge(liveRmValues) + if err != nil { + return nil, fmt.Errorf("error merging liveRmValues in typedPreditedLive: %s", err) } } @@ -263,24 +238,7 @@ func removeWebhookMutation(predictedLive, config, live *unstructured.Unstructure if !ok { return nil, fmt.Errorf("error converting live typedValue: expected map got %T", plu) } - var errors error - if len(errs) > 0 { - errors = WebhookConflictErrors{errors: errs} - } - return &unstructured.Unstructured{Object: pl}, errors -} - -type WebhookConflictErrors struct { - errors []error -} - -func (e WebhookConflictErrors) Error() string { - var sb strings.Builder - sb.WriteString("Webhook mutations found in the desired state") - for _, err := range e.errors { - sb.WriteString(fmt.Sprintf(": %s", err.Error())) - } - return sb.String() + return &unstructured.Unstructured{Object: pl}, nil } func jsonStrToUnstructured(jsonString string) (*unstructured.Unstructured, error) { From 25f19df754a0a9faf52d15ed85d938c698da5ff2 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Fri, 8 Dec 2023 11:37:27 -0500 Subject: [PATCH 13/18] address review comments 1/2 Signed-off-by: Leonardo Luz Almeida --- pkg/diff/diff.go | 53 +++++++++++++++++++++++++++++----------- pkg/diff/diff_options.go | 24 +++++++++++++++--- 2 files changed, 60 insertions(+), 17 deletions(-) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 6ef50e7e8..eecb3f6aa 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -130,6 +130,10 @@ func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, return TwoWayDiff(config, live) } +// ServerSideDiff will execute a k8s server-side apply in dry-run mode with the +// given config. The result will be compared with given live resource to determine +// diff. If config or live are nil it means resource creation or deletion. In this +// no call will be made to kube-api and a simple diff will be returned. func ServerSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, error) { if live != nil && config != nil { return serverSideDiff(config, live, opts...) @@ -137,6 +141,10 @@ func ServerSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*D return handleResourceCreateOrDeleteDiff(config, live) } +// ServerSideDiff will execute a k8s server-side apply in dry-run mode with the +// given config. The result will be compared with given live resource to determine +// diff. Modifications done by mutation webhooks are removed from the diff by default. +// This behaviour can be customized with Option.WithIgnoreMutationWebhook. func serverSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, error) { o := applyOptions(opts) if o.kubeApplier == nil { @@ -176,64 +184,81 @@ func serverSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*D // removeWebhookMutation will compare the predictedLive with live to identify // changes done by mutation webhooks. Webhook mutations are identified by finding -// changes in predictedLive fields not associated with any manager. All fields -// under this condition will be reverted with their state from live. +// changes in predictedLive fields not associated with any manager in the +// managedFields. All fields under this condition will be reverted with their state +// from live. If the given predictedLive does not have the managedFields, an error +// will be returned. func removeWebhookMutation(predictedLive, live *unstructured.Unstructured, gvkParser *managedfields.GvkParser, manager string) (*unstructured.Unstructured, error) { + plManagedFields := predictedLive.GetManagedFields() + if plManagedFields == nil || len(plManagedFields) == 0 { + return nil, fmt.Errorf("predictedLive for resource %s/%s must have the managedFields", predictedLive.GetKind(), predictedLive.GetName()) + } gvk := predictedLive.GetObjectKind().GroupVersionKind() pt := gvkParser.Type(gvk) - typedPreditedLive, err := pt.FromUnstructured(predictedLive.Object) + typedPredictedLive, err := pt.FromUnstructured(predictedLive.Object) if err != nil { - return nil, fmt.Errorf("error creating typedPreditedLive: %s", err) + return nil, fmt.Errorf("error converting predicted live state from unstructured to %s: %w", gvk, err) } typedLive, err := pt.FromUnstructured(live.Object) if err != nil { - return nil, fmt.Errorf("error creating typedLive: %s", err) + return nil, fmt.Errorf("error converting live state from unstructured to %s: %w", gvk, err) } - comparison, err := typedLive.Compare(typedPreditedLive) + // Compare the predicted live with the live resource + comparison, err := typedLive.Compare(typedPredictedLive) if err != nil { - return nil, fmt.Errorf("error comparing typedPreditedLive with typedLive: %s", err) + return nil, fmt.Errorf("error comparing predicted resource to live resource: %w", err) } - for _, mfEntry := range predictedLive.GetManagedFields() { + // Loop over all existing managers in predicted live resource to identify + // fields mutated (in predicted live) not owned by any manager. + for _, mfEntry := range plManagedFields { mfs := &fieldpath.Set{} err := mfs.FromJSON(bytes.NewReader(mfEntry.FieldsV1.Raw)) if err != nil { return nil, fmt.Errorf("error building managedFields set: %s", err) } if comparison.Added != nil && !comparison.Added.Empty() { + // exclude the added fields owned by this manager from the comparison comparison.Added = comparison.Added.Difference(mfs) } if comparison.Modified != nil && !comparison.Modified.Empty() { + // exclude the modified fields owned by this manager from the comparison comparison.Modified = comparison.Modified.Difference(mfs) } if comparison.Removed != nil && !comparison.Removed.Empty() { + // exclude the removed fields owned by this manager from the comparison comparison.Removed = comparison.Removed.Difference(mfs) } } + // At this point, comparison holds all mutations that aren't owned by any + // of the existing managers. if comparison.Added != nil && !comparison.Added.Empty() { - typedPreditedLive = typedPreditedLive.RemoveItems(comparison.Added) + // remove added fields that aren't owned by any manager + typedPredictedLive = typedPredictedLive.RemoveItems(comparison.Added) } if comparison.Modified != nil && !comparison.Modified.Empty() { liveModValues := typedLive.ExtractItems(comparison.Modified) - typedPreditedLive, err = typedPreditedLive.Merge(liveModValues) + // revert modified fields not owned by any manager + typedPredictedLive, err = typedPredictedLive.Merge(liveModValues) if err != nil { - return nil, fmt.Errorf("error merging liveModValues in typedPreditedLive: %s", err) + return nil, fmt.Errorf("error reverting webhook modified fields in predicted live resource: %s", err) } } if comparison.Removed != nil && !comparison.Removed.Empty() { liveRmValues := typedLive.ExtractItems(comparison.Removed) - typedPreditedLive, err = typedPreditedLive.Merge(liveRmValues) + // revert removed fields not owned by any manager + typedPredictedLive, err = typedPredictedLive.Merge(liveRmValues) if err != nil { - return nil, fmt.Errorf("error merging liveRmValues in typedPreditedLive: %s", err) + return nil, fmt.Errorf("error reverting webhook removed fields in predicted live resource: %s", err) } } - plu := typedPreditedLive.AsValue().Unstructured() + plu := typedPredictedLive.AsValue().Unstructured() pl, ok := plu.(map[string]interface{}) if !ok { return nil, fmt.Errorf("error converting live typedValue: expected map got %T", plu) diff --git a/pkg/diff/diff_options.go b/pkg/diff/diff_options.go index b4178f297..bdcdbd541 100644 --- a/pkg/diff/diff_options.go +++ b/pkg/diff/diff_options.go @@ -22,7 +22,7 @@ type options struct { gvkParser *managedfields.GvkParser manager string serverSideDiff bool - kubeApplier KubeApplier + serverSideDryRunner ServerSideDryRunner ignoreMutationWebhook bool } @@ -43,6 +43,24 @@ type KubeApplier interface { ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string, serverSideDiff bool) (string, error) } +type ServerSideDryRunner interface { + ServerSideApplyDryRun(ctx context.Context, obj *unstructured.Unstructured, manager string) (string, error) +} + +type K8sServerSideDryRunner struct { + dryrunApplier KubeApplier +} + +func NewK8sServerSideDryRunner(kubeApplier KubeApplier) *K8sServerSideDryRunner { + return &K8sServerSideDryRunner{ + dryrunApplier: kubeApplier, + } +} + +func (kdr *K8sServerSideDryRunner) ServerSideApplyDryRun(ctx context.Context, obj *unstructured.Unstructured, manager string) (string, error) { + return kdr.dryrunApplier.ApplyResource(context.Background(), obj, cmdutil.DryRunServer, false, false, true, manager, true) +} + func IgnoreAggregatedRoles(ignore bool) Option { return func(o *options) { o.ignoreAggregatedRoles = ignore @@ -91,8 +109,8 @@ func WithIgnoreMutationWebhook(mw bool) Option { } } -func WithKubeApplier(ka KubeApplier) Option { +func WithServerSideDryRunner(ssadr ServerSideDryRunner) Option { return func(o *options) { - o.kubeApplier = ka + o.serverSideDryRunner = ssadr } } From 46668737fe5bec49b06b8f12298e3fccb4e90787 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Fri, 8 Dec 2023 15:34:02 -0500 Subject: [PATCH 14/18] address review comments 2/2 Signed-off-by: Leonardo Luz Almeida --- pkg/diff/diff.go | 25 +++++++++++++++++++------ pkg/diff/diff_options.go | 14 +++++++++++--- pkg/utils/kube/resource_ops.go | 10 ++++++++-- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index eecb3f6aa..f9183c08a 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -21,7 +21,6 @@ import ( "k8s.io/apimachinery/pkg/util/managedfields" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/client-go/kubernetes/scheme" - cmdutil "k8s.io/kubectl/pkg/cmd/util" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" "sigs.k8s.io/structured-merge-diff/v4/merge" "sigs.k8s.io/structured-merge-diff/v4/typed" @@ -136,9 +135,23 @@ func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, // no call will be made to kube-api and a simple diff will be returned. func ServerSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, error) { if live != nil && config != nil { - return serverSideDiff(config, live, opts...) + result, err := serverSideDiff(config, live, opts...) + if err != nil { + return nil, fmt.Errorf("serverSideDiff error: %w", err) + } + return result, nil + } + // Currently, during resource creation a shallow diff (non ServerSide apply + // based) will be returned. The reasons are: + // - Saves 1 additional call to KubeAPI + // - Much lighter/faster diff + // - This is the existing behaviour users are already used to + // - No direct benefit to the user + result, err := handleResourceCreateOrDeleteDiff(config, live) + if err != nil { + return nil, fmt.Errorf("error handling resource creation or deletion: %w", err) } - return handleResourceCreateOrDeleteDiff(config, live) + return result, nil } // ServerSideDiff will execute a k8s server-side apply in dry-run mode with the @@ -147,10 +160,10 @@ func ServerSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*D // This behaviour can be customized with Option.WithIgnoreMutationWebhook. func serverSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, error) { o := applyOptions(opts) - if o.kubeApplier == nil { - return nil, fmt.Errorf("serverSideDiff error: kubeApplier is null") + if o.serverSideDryRunner == nil { + return nil, fmt.Errorf("serverSideDryRunner is null") } - predictedLiveStr, err := o.kubeApplier.ApplyResource(context.Background(), config, cmdutil.DryRunServer, false, false, true, o.manager, true) + predictedLiveStr, err := o.serverSideDryRunner.Run(context.Background(), config, o.manager) if err != nil { return nil, fmt.Errorf("error running server side apply in dryrun mode: %w", err) } diff --git a/pkg/diff/diff_options.go b/pkg/diff/diff_options.go index bdcdbd541..41628b0ea 100644 --- a/pkg/diff/diff_options.go +++ b/pkg/diff/diff_options.go @@ -43,22 +43,30 @@ type KubeApplier interface { ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string, serverSideDiff bool) (string, error) } +// ServerSideDryRunner defines the contract to run a server-side apply in +// dryrun mode. type ServerSideDryRunner interface { - ServerSideApplyDryRun(ctx context.Context, obj *unstructured.Unstructured, manager string) (string, error) + Run(ctx context.Context, obj *unstructured.Unstructured, manager string) (string, error) } +// K8sServerSideDryRunner is the Kubernetes implementation of ServerSideDryRunner. type K8sServerSideDryRunner struct { dryrunApplier KubeApplier } +// NewK8sServerSideDryRunner will instantiate a new K8sServerSideDryRunner with +// the given kubeApplier. func NewK8sServerSideDryRunner(kubeApplier KubeApplier) *K8sServerSideDryRunner { return &K8sServerSideDryRunner{ dryrunApplier: kubeApplier, } } -func (kdr *K8sServerSideDryRunner) ServerSideApplyDryRun(ctx context.Context, obj *unstructured.Unstructured, manager string) (string, error) { - return kdr.dryrunApplier.ApplyResource(context.Background(), obj, cmdutil.DryRunServer, false, false, true, manager, true) +// ServerSideApplyDryRun will invoke a kubernetes server-side apply with the given +// obj and the given manager in dryrun mode. Will return the predicted live state +// json as string. +func (kdr *K8sServerSideDryRunner) Run(ctx context.Context, obj *unstructured.Unstructured, manager string) (string, error) { + return kdr.dryrunApplier.ApplyResource(ctx, obj, cmdutil.DryRunServer, false, false, true, manager, true) } func IgnoreAggregatedRoles(ignore bool) Option { diff --git a/pkg/utils/kube/resource_ops.go b/pkg/utils/kube/resource_ops.go index 4920283f1..554a6875b 100644 --- a/pkg/utils/kube/resource_ops.go +++ b/pkg/utils/kube/resource_ops.go @@ -293,12 +293,18 @@ func (k *kubectlResourceOperations) newApplyOptions(ioStreams genericclioptions. } case cmdutil.DryRunServer: if serverSideDiff { + // managedFields are required by server-side diff to identify + // changes made by mutation webhooks. o.PrintFlags.JSONYamlPrintFlags.ShowManagedFields = true - return o.PrintFlags.JSONYamlPrintFlags.ToPrinter("json") + p, err := o.PrintFlags.JSONYamlPrintFlags.ToPrinter("json") + if err != nil { + return nil, fmt.Errorf("error configuring server-side diff printer: %w", err) + } + return p, nil } else { err = o.PrintFlags.Complete("%s (server dry run)") if err != nil { - return nil, err + return nil, fmt.Errorf("error configuring server dryrun printer: %w", err) } } } From 4aa27cbd2cb64d1d13a79084739b197c2944a882 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Fri, 8 Dec 2023 16:38:38 -0500 Subject: [PATCH 15/18] fix lint Signed-off-by: Leonardo Luz Almeida --- pkg/diff/diff.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index f9183c08a..fd3427886 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -203,7 +203,7 @@ func serverSideDiff(config, live *unstructured.Unstructured, opts ...Option) (*D // will be returned. func removeWebhookMutation(predictedLive, live *unstructured.Unstructured, gvkParser *managedfields.GvkParser, manager string) (*unstructured.Unstructured, error) { plManagedFields := predictedLive.GetManagedFields() - if plManagedFields == nil || len(plManagedFields) == 0 { + if len(plManagedFields) == 0 { return nil, fmt.Errorf("predictedLive for resource %s/%s must have the managedFields", predictedLive.GetKind(), predictedLive.GetName()) } gvk := predictedLive.GetObjectKind().GroupVersionKind() From c1e3999317ff36dd62ef29bcd368af6eda61d6ba Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Fri, 8 Dec 2023 17:05:25 -0500 Subject: [PATCH 16/18] remove kubectl getter from cluster-cache Signed-off-by: Leonardo Luz Almeida --- pkg/cache/cluster.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/cache/cluster.go b/pkg/cache/cluster.go index a54a0c8e8..02ca18911 100644 --- a/pkg/cache/cluster.go +++ b/pkg/cache/cluster.go @@ -132,8 +132,6 @@ type ClusterCache interface { OnResourceUpdated(handler OnResourceUpdatedHandler) Unsubscribe // OnEvent register event handler that is executed every time when new K8S event received OnEvent(handler OnEventHandler) Unsubscribe - - GetKubectl() kube.Kubectl } type WeightedSemaphore interface { @@ -321,10 +319,6 @@ func (c *clusterCache) GetGVKParser() *managedfields.GvkParser { return c.gvkParser } -func (c *clusterCache) GetKubectl() kube.Kubectl { - return c.kubectl -} - func (c *clusterCache) appendAPIResource(info kube.APIResourceInfo) { exists := false for i := range c.apiResources { From 68b5014a5d636733f49bf2ef3ae3e680c0ad9df1 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Wed, 13 Dec 2023 15:36:02 -0500 Subject: [PATCH 17/18] fix query param verifier instantiation Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- pkg/utils/kube/resource_ops.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/utils/kube/resource_ops.go b/pkg/utils/kube/resource_ops.go index 554a6875b..7faa037b2 100644 --- a/pkg/utils/kube/resource_ops.go +++ b/pkg/utils/kube/resource_ops.go @@ -267,7 +267,7 @@ func (k *kubectlResourceOperations) newApplyOptions(ioStreams genericclioptions. return nil, err } o.OpenAPISchema = k.openAPISchema - o.DryRunVerifier = resource.NewQueryParamVerifier(dynamicClient, k.fact.OpenAPIGetter(), resource.QueryParamFieldValidation) + o.DryRunVerifier = resource.NewQueryParamVerifier(dynamicClient, k.fact.OpenAPIGetter(), resource.QueryParamDryRun) o.FieldValidationVerifier = resource.NewQueryParamVerifier(dynamicClient, k.fact.OpenAPIGetter(), resource.QueryParamFieldValidation) validateDirective := metav1.FieldValidationIgnore if validate { @@ -337,7 +337,7 @@ func (k *kubectlResourceOperations) newCreateOptions(config *rest.Config, ioStre return nil, err } - o.DryRunVerifier = resource.NewQueryParamVerifier(dynamicClient, k.fact.OpenAPIGetter(), resource.QueryParamFieldValidation) + o.DryRunVerifier = resource.NewQueryParamVerifier(dynamicClient, k.fact.OpenAPIGetter(), resource.QueryParamDryRun) o.FieldValidationVerifier = resource.NewQueryParamVerifier(dynamicClient, k.fact.OpenAPIGetter(), resource.QueryParamFieldValidation) switch dryRunStrategy { @@ -384,7 +384,7 @@ func (k *kubectlResourceOperations) newReplaceOptions(config *rest.Config, f cmd return nil, err } - o.DryRunVerifier = resource.NewQueryParamVerifier(dynamicClient, k.fact.OpenAPIGetter(), resource.QueryParamFieldValidation) + o.DryRunVerifier = resource.NewQueryParamVerifier(dynamicClient, k.fact.OpenAPIGetter(), resource.QueryParamDryRun) o.FieldValidationVerifier = resource.NewQueryParamVerifier(dynamicClient, k.fact.OpenAPIGetter(), resource.QueryParamFieldValidation) o.Builder = func() *resource.Builder { return f.NewBuilder() From 8742000282c47ea620b05836cac800114874b717 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Sun, 17 Dec 2023 20:03:30 -0500 Subject: [PATCH 18/18] Add server-side-diff unit tests Signed-off-by: Leonardo Luz Almeida --- pkg/diff/diff_test.go | 73 ++++++++++++++++++ pkg/diff/mocks/ServerSideDryRunner.go | 58 +++++++++++++++ pkg/diff/testdata/data.go | 9 +++ pkg/diff/testdata/ssd-service-config.yaml | 16 ++++ pkg/diff/testdata/ssd-service-live.yaml | 55 ++++++++++++++ .../testdata/ssd-service-predicted-live.json | 74 +++++++++++++++++++ 6 files changed, 285 insertions(+) create mode 100644 pkg/diff/mocks/ServerSideDryRunner.go create mode 100644 pkg/diff/testdata/ssd-service-config.yaml create mode 100644 pkg/diff/testdata/ssd-service-live.yaml create mode 100644 pkg/diff/testdata/ssd-service-predicted-live.json diff --git a/pkg/diff/diff_test.go b/pkg/diff/diff_test.go index ec8b82038..d2e3736ea 100644 --- a/pkg/diff/diff_test.go +++ b/pkg/diff/diff_test.go @@ -1,6 +1,7 @@ package diff import ( + "context" "encoding/json" "fmt" "os" @@ -9,9 +10,11 @@ import ( "strings" "testing" + "github.com/argoproj/gitops-engine/pkg/diff/mocks" "github.com/argoproj/gitops-engine/pkg/diff/testdata" openapi_v2 "github.com/google/gnostic/openapiv2" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" appsv1 "k8s.io/api/apps/v1" @@ -887,6 +890,76 @@ func TestStructuredMergeDiff(t *testing.T) { }) } +func TestServerSideDiff(t *testing.T) { + buildOpts := func(predictedLive string) []Option { + gvkParser := buildGVKParser(t) + manager := "argocd-controller" + dryRunner := mocks.NewServerSideDryRunner(t) + + dryRunner.On("Run", mock.Anything, mock.AnythingOfType("*unstructured.Unstructured"), manager). + Return(func(ctx context.Context, obj *unstructured.Unstructured, manager string) (string, error) { + return predictedLive, nil + }) + opts := []Option{ + WithGVKParser(gvkParser), + WithManager(manager), + WithServerSideDryRunner(dryRunner), + } + + return opts + } + + t.Run("will ignore modifications done by mutation webhook by default", func(t *testing.T) { + // given + t.Parallel() + liveState := StrToUnstructured(testdata.ServiceLiveYAMLSSD) + desiredState := StrToUnstructured(testdata.ServiceConfigYAMLSSD) + opts := buildOpts(testdata.ServicePredictedLiveJSONSSD) + + // when + result, err := serverSideDiff(desiredState, liveState, opts...) + + // then + require.NoError(t, err) + assert.NotNil(t, result) + assert.True(t, result.Modified) + predictedSVC := YamlToSvc(t, result.PredictedLive) + liveSVC := YamlToSvc(t, result.NormalizedLive) + require.NotNil(t, predictedSVC.Spec.InternalTrafficPolicy) + require.NotNil(t, liveSVC.Spec.InternalTrafficPolicy) + assert.Equal(t, "Cluster", string(*predictedSVC.Spec.InternalTrafficPolicy)) + assert.Equal(t, "Cluster", string(*liveSVC.Spec.InternalTrafficPolicy)) + assert.Empty(t, predictedSVC.Annotations[AnnotationLastAppliedConfig]) + assert.Empty(t, liveSVC.Annotations[AnnotationLastAppliedConfig]) + assert.Empty(t, predictedSVC.Labels["event"]) + }) + t.Run("will include mutation webhook modifications", func(t *testing.T) { + // given + t.Parallel() + liveState := StrToUnstructured(testdata.ServiceLiveYAMLSSD) + desiredState := StrToUnstructured(testdata.ServiceConfigYAMLSSD) + opts := buildOpts(testdata.ServicePredictedLiveJSONSSD) + opts = append(opts, WithIgnoreMutationWebhook(false)) + + // when + result, err := serverSideDiff(desiredState, liveState, opts...) + + // then + require.NoError(t, err) + assert.NotNil(t, result) + assert.True(t, result.Modified) + predictedSVC := YamlToSvc(t, result.PredictedLive) + liveSVC := YamlToSvc(t, result.NormalizedLive) + require.NotNil(t, predictedSVC.Spec.InternalTrafficPolicy) + require.NotNil(t, liveSVC.Spec.InternalTrafficPolicy) + assert.Equal(t, "Cluster", string(*predictedSVC.Spec.InternalTrafficPolicy)) + assert.Equal(t, "Cluster", string(*liveSVC.Spec.InternalTrafficPolicy)) + assert.Empty(t, predictedSVC.Annotations[AnnotationLastAppliedConfig]) + assert.Empty(t, liveSVC.Annotations[AnnotationLastAppliedConfig]) + assert.NotEmpty(t, predictedSVC.Labels["event"]) + }) +} + func createSecret(data map[string]string) *unstructured.Unstructured { secret := corev1.Secret{TypeMeta: metav1.TypeMeta{Kind: "Secret"}} if data != nil { diff --git a/pkg/diff/mocks/ServerSideDryRunner.go b/pkg/diff/mocks/ServerSideDryRunner.go new file mode 100644 index 000000000..321ecf5fc --- /dev/null +++ b/pkg/diff/mocks/ServerSideDryRunner.go @@ -0,0 +1,58 @@ +// Code generated by mockery v2.38.0. DO NOT EDIT. + +package mocks + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" + + unstructured "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +// ServerSideDryRunner is an autogenerated mock type for the ServerSideDryRunner type +type ServerSideDryRunner struct { + mock.Mock +} + +// Run provides a mock function with given fields: ctx, obj, manager +func (_m *ServerSideDryRunner) Run(ctx context.Context, obj *unstructured.Unstructured, manager string) (string, error) { + ret := _m.Called(ctx, obj, manager) + + if len(ret) == 0 { + panic("no return value specified for Run") + } + + var r0 string + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *unstructured.Unstructured, string) (string, error)); ok { + return rf(ctx, obj, manager) + } + if rf, ok := ret.Get(0).(func(context.Context, *unstructured.Unstructured, string) string); ok { + r0 = rf(ctx, obj, manager) + } else { + r0 = ret.Get(0).(string) + } + + if rf, ok := ret.Get(1).(func(context.Context, *unstructured.Unstructured, string) error); ok { + r1 = rf(ctx, obj, manager) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewServerSideDryRunner creates a new instance of ServerSideDryRunner. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewServerSideDryRunner(t interface { + mock.TestingT + Cleanup(func()) +}) *ServerSideDryRunner { + mock := &ServerSideDryRunner{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/diff/testdata/data.go b/pkg/diff/testdata/data.go index cc88d04bb..047f3190b 100644 --- a/pkg/diff/testdata/data.go +++ b/pkg/diff/testdata/data.go @@ -32,4 +32,13 @@ var ( // //go:embed openapiv2.bin OpenAPIV2Doc []byte + + //go:embed ssd-service-config.yaml + ServiceConfigYAMLSSD string + + //go:embed ssd-service-live.yaml + ServiceLiveYAMLSSD string + + //go:embed ssd-service-predicted-live.json + ServicePredictedLiveJSONSSD string ) diff --git a/pkg/diff/testdata/ssd-service-config.yaml b/pkg/diff/testdata/ssd-service-config.yaml new file mode 100644 index 000000000..ca0678ffc --- /dev/null +++ b/pkg/diff/testdata/ssd-service-config.yaml @@ -0,0 +1,16 @@ +apiVersion: v1 +kind: Service +metadata: + labels: + app.kubernetes.io/instance: httpbin + name: httpbin-svc + namespace: httpbin +spec: + ports: + - name: http-port + port: 7777 + targetPort: 80 + - name: test + port: 333 + selector: + app: httpbin diff --git a/pkg/diff/testdata/ssd-service-live.yaml b/pkg/diff/testdata/ssd-service-live.yaml new file mode 100644 index 000000000..98d01a709 --- /dev/null +++ b/pkg/diff/testdata/ssd-service-live.yaml @@ -0,0 +1,55 @@ +apiVersion: v1 +kind: Service +metadata: + creationTimestamp: '2023-12-18T00:34:22Z' + labels: + app.kubernetes.io/instance: httpbin + managedFields: + - apiVersion: v1 + fieldsType: FieldsV1 + fieldsV1: + 'f:metadata': + 'f:labels': + 'f:app.kubernetes.io/instance': {} + 'f:spec': + 'f:ports': + 'k:{"port":333,"protocol":"TCP"}': + .: {} + 'f:name': {} + 'f:port': {} + 'k:{"port":7777,"protocol":"TCP"}': + .: {} + 'f:name': {} + 'f:port': {} + 'f:targetPort': {} + 'f:selector': {} + manager: argocd-controller + operation: Apply + time: '2023-12-18T00:34:22Z' + name: httpbin-svc + namespace: httpbin + resourceVersion: '2836' + uid: 0e898e6f-c275-476d-9b4f-5e96072cc129 +spec: + clusterIP: 10.43.223.115 + clusterIPs: + - 10.43.223.115 + internalTrafficPolicy: Cluster + ipFamilies: + - IPv4 + ipFamilyPolicy: SingleStack + ports: + - name: http-port + port: 7777 + protocol: TCP + targetPort: 80 + - name: test + port: 333 + protocol: TCP + targetPort: 333 + selector: + app: httpbin + sessionAffinity: None + type: ClusterIP +status: + loadBalancer: {} diff --git a/pkg/diff/testdata/ssd-service-predicted-live.json b/pkg/diff/testdata/ssd-service-predicted-live.json new file mode 100644 index 000000000..ca1323f31 --- /dev/null +++ b/pkg/diff/testdata/ssd-service-predicted-live.json @@ -0,0 +1,74 @@ +{ + "apiVersion": "v1", + "kind": "Service", + "metadata": { + "creationTimestamp": "2023-12-18T00:34:22Z", + "labels": { + "event": "FROM-MUTATION-WEBHOOK" + }, + "managedFields": [ + { + "apiVersion": "v1", + "fieldsType": "FieldsV1", + "fieldsV1": { + "f:spec": { + "f:ports": { + "k:{\"port\":333,\"protocol\":\"TCP\"}": { + ".": {}, + "f:name": {}, + "f:port": {} + }, + "k:{\"port\":7777,\"protocol\":\"TCP\"}": { + ".": {}, + "f:name": {}, + "f:port": {}, + "f:targetPort": {} + } + }, + "f:selector": {} + } + }, + "manager": "argocd-controller", + "operation": "Apply", + "time": "2023-12-18T00:38:28Z" + } + ], + "name": "httpbin-svc", + "namespace": "httpbin", + "resourceVersion": "2836", + "uid": "0e898e6f-c275-476d-9b4f-5e96072cc129" + }, + "spec": { + "clusterIP": "10.43.223.115", + "clusterIPs": [ + "10.43.223.115" + ], + "internalTrafficPolicy": "Cluster", + "ipFamilies": [ + "IPv4" + ], + "ipFamilyPolicy": "SingleStack", + "ports": [ + { + "name": "http-port", + "port": 7777, + "protocol": "TCP", + "targetPort": 80 + }, + { + "name": "test", + "port": 333, + "protocol": "TCP", + "targetPort": 333 + } + ], + "selector": { + "app": "httpbin" + }, + "sessionAffinity": "None", + "type": "ClusterIP" + }, + "status": { + "loadBalancer": {} + } +}