diff --git a/pkg/diff/diff_options.go b/pkg/diff/diff_options.go index 069b2a0ed..b2d69bec3 100644 --- a/pkg/diff/diff_options.go +++ b/pkg/diff/diff_options.go @@ -40,7 +40,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, serverSideDiff bool) (string, error) + ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) } // ServerSideDryRunner defines the contract to run a server-side apply in @@ -66,7 +66,7 @@ func NewK8sServerSideDryRunner(kubeApplier KubeApplier) *K8sServerSideDryRunner // 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) + return kdr.dryrunApplier.ApplyResource(ctx, obj, cmdutil.DryRunServer, false, false, true, manager) } func IgnoreAggregatedRoles(ignore bool) Option { diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go index 35981ebaa..934aa248b 100644 --- a/pkg/sync/sync_context.go +++ b/pkg/sync/sync_context.go @@ -1002,7 +1002,7 @@ func (sc *syncContext) applyObject(t *syncTask, dryRun, validate bool) (common.R message, err = sc.resourceOps.CreateResource(context.TODO(), t.targetObj, dryRunStrategy, validate) } } else { - message, err = sc.resourceOps.ApplyResource(context.TODO(), t.targetObj, dryRunStrategy, force, validate, serverSideApply, sc.serverSideApplyManager, false) + message, err = sc.resourceOps.ApplyResource(context.TODO(), t.targetObj, dryRunStrategy, force, validate, serverSideApply, sc.serverSideApplyManager) } if err != nil { return common.ResultCodeSyncFailed, err.Error() diff --git a/pkg/utils/kube/ctl.go b/pkg/utils/kube/ctl.go index 3f494b389..a80c63ea5 100644 --- a/pkg/utils/kube/ctl.go +++ b/pkg/utils/kube/ctl.go @@ -19,6 +19,7 @@ import ( "k8s.io/kube-openapi/pkg/util/proto" "k8s.io/kubectl/pkg/util/openapi" + "github.com/argoproj/gitops-engine/pkg/diff" utils "github.com/argoproj/gitops-engine/pkg/utils/io" "github.com/argoproj/gitops-engine/pkg/utils/tracing" ) @@ -296,6 +297,31 @@ func (k *KubectlCmd) ManageResources(config *rest.Config, openAPISchema openapi. }, cleanup, nil } +func ManageServerSideDiffDryRuns(config *rest.Config, openAPISchema openapi.Resources, tracer tracing.Tracer, log logr.Logger, onKubectlRun OnKubectlRunFunc) (diff.KubeApplier, func(), error) { + f, err := os.CreateTemp(utils.TempDir, "") + if err != nil { + return nil, nil, fmt.Errorf("failed to generate temp file for kubeconfig: %v", err) + } + _ = f.Close() + err = WriteKubeConfig(config, "", f.Name()) + if err != nil { + utils.DeleteFile(f.Name()) + return nil, nil, fmt.Errorf("failed to write kubeconfig: %v", err) + } + fact := kubeCmdFactory(f.Name(), "", config) + cleanup := func() { + utils.DeleteFile(f.Name()) + } + return &kubectlServerSideDiffDryRunApplier{ + config: config, + fact: fact, + openAPISchema: openAPISchema, + tracer: tracer, + log: log, + onKubectlRun: onKubectlRun, + }, cleanup, nil +} + // ConvertToVersion converts an unstructured object into the specified group/version func (k *KubectlCmd) ConvertToVersion(obj *unstructured.Unstructured, group string, version string) (*unstructured.Unstructured, error) { span := k.Tracer.StartSpan("ConvertToVersion") diff --git a/pkg/utils/kube/kubetest/mock_resource_operations.go b/pkg/utils/kube/kubetest/mock_resource_operations.go index c6e30f4e4..2e55dd1f6 100644 --- a/pkg/utils/kube/kubetest/mock_resource_operations.go +++ b/pkg/utils/kube/kubetest/mock_resource_operations.go @@ -105,7 +105,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, serverSideDiff bool) (string, error) { +func (r *MockResourceOps) ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (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 a7a0c419d..da64f83c7 100644 --- a/pkg/utils/kube/resource_ops.go +++ b/pkg/utils/kube/resource_ops.go @@ -39,12 +39,13 @@ 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, serverSideDiff bool) (string, error) + ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (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) } +// This is a generic implementation for doing most kubectl operations. Implements the ResourceOperations interface. type kubectlResourceOperations struct { config *rest.Config log logr.Logger @@ -54,49 +55,72 @@ type kubectlResourceOperations struct { openAPISchema openapi.Resources } -type commandExecutor func(f cmdutil.Factory, ioStreams genericclioptions.IOStreams, fileName string) error +// This is an implementation specific for doing server-side diff dry runs. Implements the KubeApplier interface. +type kubectlServerSideDiffDryRunApplier struct { + config *rest.Config + log logr.Logger + tracer tracing.Tracer + onKubectlRun OnKubectlRunFunc + fact cmdutil.Factory + openAPISchema openapi.Resources +} -func (k *kubectlResourceOperations) runResourceCommand(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, serverSideDiff bool, executor commandExecutor) (string, error) { - manifestBytes, err := json.Marshal(obj) - if err != nil { - return "", err - } - manifestFile, err := os.CreateTemp(io.TempDir, "") - if err != nil { - return "", fmt.Errorf("Failed to generate temp file for manifest: %v", err) - } - defer io.DeleteFile(manifestFile.Name()) - if _, err = manifestFile.Write(manifestBytes); err != nil { - return "", fmt.Errorf("Failed to write manifest: %v", err) - } - if err = manifestFile.Close(); err != nil { - return "", fmt.Errorf("Failed to close manifest: %v", err) - } +type commandExecutor func(ioStreams genericclioptions.IOStreams, fileName string) error +func maybeLogManifest(manifestBytes []byte, log logr.Logger) error { // log manifest - if k.log.V(1).Enabled() { + if log.V(1).Enabled() { var obj unstructured.Unstructured err := json.Unmarshal(manifestBytes, &obj) if err != nil { - return "", err + return err } redacted, _, err := diff.HideSecretData(&obj, nil, nil) if err != nil { - return "", err + return err } redactedBytes, err := json.Marshal(redacted) if err != nil { - return "", err + return err } - k.log.V(1).Info(string(redactedBytes)) + log.V(1).Info(string(redactedBytes)) } + return nil +} + +func createManifestFile(obj *unstructured.Unstructured, log logr.Logger) (*os.File, error) { + manifestBytes, err := json.Marshal(obj) + if err != nil { + return nil, err + } + manifestFile, err := os.CreateTemp(io.TempDir, "") + if err != nil { + return nil, fmt.Errorf("Failed to generate temp file for manifest: %v", err) + } + if _, err = manifestFile.Write(manifestBytes); err != nil { + return nil, fmt.Errorf("Failed to write manifest: %v", err) + } + if err = manifestFile.Close(); err != nil { + return nil, fmt.Errorf("Failed to close manifest: %v", err) + } + + err = maybeLogManifest(manifestBytes, log) + if err != nil { + return nil, err + } + return manifestFile, nil +} + +func (k *kubectlResourceOperations) runResourceCommand(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, executor commandExecutor) (string, error) { + manifestFile, err := createManifestFile(obj, k.log) + if err != nil { + return "", err + } + defer io.DeleteFile(manifestFile.Name()) var out []string // rbac resouces are first applied with auth reconcile kubectl feature. - // serverSideDiff should avoid this step as the resources are not being actually - // applied but just running in dryrun mode. Also, kubectl auth reconcile doesn't - // currently support running dryrun in server mode. - if obj.GetAPIVersion() == "rbac.authorization.k8s.io/v1" && !serverSideDiff { + if obj.GetAPIVersion() == "rbac.authorization.k8s.io/v1" { outReconcile, err := k.rbacReconcile(ctx, obj, manifestFile.Name(), dryRunStrategy) if err != nil { return "", fmt.Errorf("error running rbacReconcile: %s", err) @@ -112,7 +136,7 @@ func (k *kubectlResourceOperations) runResourceCommand(ctx context.Context, obj Out: &bytes.Buffer{}, ErrOut: &bytes.Buffer{}, } - err = executor(k.fact, ioStreams, manifestFile.Name()) + err = executor(ioStreams, manifestFile.Name()) if err != nil { return "", errors.New(cleanKubectlOutput(err.Error())) } @@ -125,6 +149,40 @@ func (k *kubectlResourceOperations) runResourceCommand(ctx context.Context, obj return strings.Join(out, ". "), nil } +func (k *kubectlServerSideDiffDryRunApplier) runResourceCommand(obj *unstructured.Unstructured, executor commandExecutor) (string, error) { + manifestFile, err := createManifestFile(obj, k.log) + if err != nil { + return "", err + } + defer io.DeleteFile(manifestFile.Name()) + + stdoutBuf := &bytes.Buffer{} + stderrBuf := &bytes.Buffer{} + + // Run kubectl apply + ioStreams := genericclioptions.IOStreams{ + In: &bytes.Buffer{}, + Out: stdoutBuf, + ErrOut: stderrBuf, + } + err = executor(ioStreams, manifestFile.Name()) + if err != nil { + return "", errors.New(cleanKubectlOutput(err.Error())) + } + stdout := stdoutBuf.String() + stderr := stderrBuf.String() + + if stderr != "" && stdout == "" { + err := fmt.Errorf("Server-side dry run apply had non-empty stderr: %s", stderr) + k.log.Error(err, "server-side diff") + return "", err + } + if stderr != "" { + k.log.Info("Warning: Server-side dry run apply had non-empty stderr: %s", stderr) + } + return stdout, nil +} + // rbacReconcile will perform reconciliation for RBAC resources. It will run // the following command: // @@ -135,7 +193,7 @@ func (k *kubectlResourceOperations) runResourceCommand(ctx context.Context, obj // See: https://github.com/kubernetes/kubernetes/issues/66353 // `auth reconcile` will delete and recreate the resource if necessary func (k *kubectlResourceOperations) rbacReconcile(ctx context.Context, obj *unstructured.Unstructured, fileName string, dryRunStrategy cmdutil.DryRunStrategy) (string, error) { - cleanup, err := k.processKubectlRun("auth") + cleanup, err := processKubectlRun(k.onKubectlRun, "auth") if err != nil { return "", fmt.Errorf("error processing kubectl run auth: %w", err) } @@ -168,18 +226,18 @@ func (k *kubectlResourceOperations) ReplaceResource(ctx context.Context, obj *un span.SetBaggageItem("name", obj.GetName()) defer span.Finish() k.log.Info(fmt.Sprintf("Replacing resource %s/%s in cluster: %s, namespace: %s", obj.GetKind(), obj.GetName(), k.config.Host, obj.GetNamespace())) - return k.runResourceCommand(ctx, obj, dryRunStrategy, false, func(f cmdutil.Factory, ioStreams genericclioptions.IOStreams, fileName string) error { - cleanup, err := k.processKubectlRun("replace") + return k.runResourceCommand(ctx, obj, dryRunStrategy, func(ioStreams genericclioptions.IOStreams, fileName string) error { + cleanup, err := processKubectlRun(k.onKubectlRun, "replace") if err != nil { return err } defer cleanup() - replaceOptions, err := k.newReplaceOptions(k.config, f, ioStreams, fileName, obj.GetNamespace(), force, dryRunStrategy) + replaceOptions, err := k.newReplaceOptions(k.config, k.fact, ioStreams, fileName, obj.GetNamespace(), force, dryRunStrategy) if err != nil { return err } - return replaceOptions.Run(f) + return replaceOptions.Run(k.fact) }) } @@ -189,8 +247,8 @@ func (k *kubectlResourceOperations) CreateResource(ctx context.Context, obj *uns span.SetBaggageItem("kind", gvk.Kind) span.SetBaggageItem("name", obj.GetName()) defer span.Finish() - return k.runResourceCommand(ctx, obj, dryRunStrategy, false, func(f cmdutil.Factory, ioStreams genericclioptions.IOStreams, fileName string) error { - cleanup, err := k.processKubectlRun("create") + return k.runResourceCommand(ctx, obj, dryRunStrategy, func(ioStreams genericclioptions.IOStreams, fileName string) error { + cleanup, err := processKubectlRun(k.onKubectlRun, "create") if err != nil { return err } @@ -209,7 +267,7 @@ func (k *kubectlResourceOperations) CreateResource(ctx context.Context, obj *uns _ = command.Flags().Set("validate", "true") } - return createOptions.RunCreate(f, command) + return createOptions.RunCreate(k.fact, command) }) } @@ -243,7 +301,33 @@ 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, serverSideDiff bool) (string, error) { +func (k *kubectlServerSideDiffDryRunApplier) ApplyResource(_ context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) { + span := k.tracer.StartSpan("ApplyResource") + span.SetBaggageItem("kind", obj.GetKind()) + span.SetBaggageItem("name", obj.GetName()) + defer span.Finish() + k.log.WithValues( + "dry-run", [...]string{"none", "client", "server"}[dryRunStrategy], + "manager", manager, + "serverSideApply", serverSideApply).Info(fmt.Sprintf("Running server-side diff. Dry run applying resource %s/%s in cluster: %s, namespace: %s", obj.GetKind(), obj.GetName(), k.config.Host, obj.GetNamespace())) + + return k.runResourceCommand(obj, func(ioStreams genericclioptions.IOStreams, fileName string) error { + cleanup, err := processKubectlRun(k.onKubectlRun, "apply") + if err != nil { + return err + } + defer cleanup() + + applyOpts, err := k.newApplyOptions(ioStreams, obj, fileName, validate, force, serverSideApply, dryRunStrategy, manager) + if err != nil { + return err + } + return applyOpts.Run() + }) +} + +// 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) { span := k.tracer.StartSpan("ApplyResource") span.SetBaggageItem("kind", obj.GetKind()) span.SetBaggageItem("name", obj.GetName()) @@ -252,16 +336,16 @@ func (k *kubectlResourceOperations) ApplyResource(ctx context.Context, obj *unst "dry-run", [...]string{"none", "client", "server"}[dryRunStrategy], "manager", manager, "serverSideApply", serverSideApply, - "serverSideDiff", serverSideDiff).Info(fmt.Sprintf("Applying resource %s/%s in cluster: %s, namespace: %s", obj.GetKind(), obj.GetName(), k.config.Host, obj.GetNamespace())) + "serverSideDiff", true).Info(fmt.Sprintf("Applying resource %s/%s in cluster: %s, namespace: %s", obj.GetKind(), obj.GetName(), k.config.Host, obj.GetNamespace())) - return k.runResourceCommand(ctx, obj, dryRunStrategy, serverSideDiff, func(f cmdutil.Factory, ioStreams genericclioptions.IOStreams, fileName string) error { - cleanup, err := k.processKubectlRun("apply") + return k.runResourceCommand(ctx, obj, dryRunStrategy, func(ioStreams genericclioptions.IOStreams, fileName string) error { + cleanup, err := processKubectlRun(k.onKubectlRun, "apply") if err != nil { return err } defer cleanup() - applyOpts, err := k.newApplyOptions(ioStreams, obj, fileName, validate, force, serverSideApply, dryRunStrategy, manager, serverSideDiff) + applyOpts, err := k.newApplyOptions(ioStreams, obj, fileName, validate, force, serverSideApply, dryRunStrategy, manager) if err != nil { return err } @@ -269,7 +353,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, serverSideDiff bool) (*apply.ApplyOptions, error) { +func newApplyOptionsCommon(config *rest.Config, fact cmdutil.Factory, ioStreams genericclioptions.IOStreams, obj *unstructured.Unstructured, fileName string, validate bool, force, serverSideApply bool, dryRunStrategy cmdutil.DryRunStrategy, manager string) (*apply.ApplyOptions, error) { flags := apply.NewApplyFlags(ioStreams) o := &apply.ApplyOptions{ IOStreams: ioStreams, @@ -281,7 +365,7 @@ func (k *kubectlResourceOperations) newApplyOptions(ioStreams genericclioptions. OpenAPIPatch: true, ServerSideApply: serverSideApply, } - dynamicClient, err := dynamic.NewForConfig(k.config) + dynamicClient, err := dynamic.NewForConfig(config) if err != nil { return nil, err } @@ -290,19 +374,60 @@ func (k *kubectlResourceOperations) newApplyOptions(ioStreams genericclioptions. if err != nil { return nil, err } - o.OpenAPIGetter = k.fact + o.OpenAPIGetter = fact o.DryRunStrategy = dryRunStrategy o.FieldManager = manager validateDirective := metav1.FieldValidationIgnore if validate { validateDirective = metav1.FieldValidationStrict } - o.Validator, err = k.fact.Validator(validateDirective) + o.Validator, err = fact.Validator(validateDirective) if err != nil { return nil, err } - o.Builder = k.fact.NewBuilder() - o.Mapper, err = k.fact.ToRESTMapper() + o.Builder = fact.NewBuilder() + o.Mapper, err = fact.ToRESTMapper() + if err != nil { + return nil, err + } + + o.DeleteOptions.FilenameOptions.Filenames = []string{fileName} + o.Namespace = obj.GetNamespace() + o.DeleteOptions.ForceDeletion = force + o.DryRunStrategy = dryRunStrategy + if manager != "" { + o.FieldManager = manager + } + return o, nil +} + +func (k *kubectlServerSideDiffDryRunApplier) newApplyOptions(ioStreams genericclioptions.IOStreams, obj *unstructured.Unstructured, fileName string, validate bool, force, serverSideApply bool, dryRunStrategy cmdutil.DryRunStrategy, manager string) (*apply.ApplyOptions, error) { + o, err := newApplyOptionsCommon(k.config, k.fact, ioStreams, obj, fileName, validate, force, serverSideApply, dryRunStrategy, manager) + if err != nil { + return nil, err + } + + o.ToPrinter = func(operation string) (printers.ResourcePrinter, error) { + o.PrintFlags.NamePrintFlags.Operation = operation + if o.DryRunStrategy != cmdutil.DryRunServer { + return nil, fmt.Errorf("invalid dry run strategy passed to server-side diff dry run applier: %d, expected %d", o.DryRunStrategy, cmdutil.DryRunServer) + } + // managedFields are required by server-side diff to identify + // changes made by mutation webhooks. + o.PrintFlags.JSONYamlPrintFlags.ShowManagedFields = true + 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 + } + + o.ForceConflicts = true + return o, nil +} + +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) { + o, err := newApplyOptionsCommon(k.config, k.fact, ioStreams, obj, fileName, validate, force, serverSideApply, dryRunStrategy, manager) if err != nil { return nil, err } @@ -316,32 +441,15 @@ func (k *kubectlResourceOperations) newApplyOptions(ioStreams genericclioptions. return nil, err } case cmdutil.DryRunServer: - if serverSideDiff { - // managedFields are required by server-side diff to identify - // changes made by mutation webhooks. - o.PrintFlags.JSONYamlPrintFlags.ShowManagedFields = true - 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, fmt.Errorf("error configuring server dryrun printer: %w", err) - } + err = o.PrintFlags.Complete("%s (server dry run)") + if err != nil { + return nil, fmt.Errorf("error configuring server dryrun printer: %w", err) } } return o.PrintFlags.ToPrinter() } - o.DeleteOptions.FilenameOptions.Filenames = []string{fileName} - o.Namespace = obj.GetNamespace() - o.DeleteOptions.ForceDeletion = force - o.DryRunStrategy = dryRunStrategy - if manager != "" { - o.FieldManager = manager - } - if serverSideApply || serverSideDiff { + + if serverSideApply { o.ForceConflicts = true } return o, nil @@ -504,9 +612,9 @@ func (k *kubectlResourceOperations) authReconcile(ctx context.Context, obj *unst return strings.Join(out, ". "), nil } -func (k *kubectlResourceOperations) processKubectlRun(cmd string) (CleanupFunc, error) { - if k.onKubectlRun != nil { - return k.onKubectlRun(cmd) +func processKubectlRun(onKubectlRun OnKubectlRunFunc, cmd string) (CleanupFunc, error) { + if onKubectlRun != nil { + return onKubectlRun(cmd) } return func() {}, nil }