Draft: Enable WithIgnoreMutationWebhook

[KalenWessel]

This PR is WIP.

By setting WithIgnoreMutationWebhook in the generateDiff function we can trigger mutating webhooks. This also requires ServerSide diffing to be enabled.

This is in preparation of being able to have kubechecks determine if a resource is going to fail a kyverno policy.

[github-actions]

Temporary image deleted.

[zapier-sre-bot]

Mergecat's Review

Click to read mergecats review!

😼 Mergecat review of go.mod

@@ -60,7 +60,8 @@ require (
 	k8s.io/api v0.31.3
 	k8s.io/apiextensions-apiserver v0.31.2
 	k8s.io/apimachinery v0.31.3
-	k8s.io/client-go v0.31.3
+	k8s.io/client-go v1.5.2
+	k8s.io/klog/v2 v2.130.1
 	sigs.k8s.io/controller-runtime v0.19.3
 	sigs.k8s.io/yaml v1.4.0
 )
@@ -295,7 +296,6 @@ require (
 	k8s.io/cli-runtime v0.31.3 // indirect
 	k8s.io/component-base v0.31.3 // indirect
 	k8s.io/component-helpers v0.31.3 // indirect
-	k8s.io/klog/v2 v2.130.1 // indirect
 	k8s.io/kube-aggregator v0.31.2 // indirect
 	k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
 	k8s.io/kubectl v0.31.2 // indirect

Feedback & Suggestions:

1. Version Downgrade Alert: The version of k8s.io/client-go has been downgraded from v0.31.3 to v1.5.2. This seems unusual as the versioning scheme suggests a major version change. Ensure this downgrade is intentional and verify compatibility with other dependencies. 🧐

2. Dependency Management: The addition of k8s.io/klog/v2 as a direct dependency while removing it from the indirect dependencies list is fine if it's being used directly in your code. However, ensure that this change is necessary and that it doesn't introduce any conflicts with other dependencies. 🔍

3. Indirect Dependency Removal: The removal of k8s.io/klog/v2 from the indirect dependencies list is appropriate if it's now being used directly. Just double-check that this change aligns with your project's needs. ✅

---

😼 Mergecat review of pkg/checks/diff/diff.go

@@ -4,10 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io"
-	"strings"
-	"time"
-
 	cmdutil "github.com/argoproj/argo-cd/v2/cmd/util"
 	"github.com/argoproj/argo-cd/v2/controller"
 	"github.com/argoproj/argo-cd/v2/pkg/apiclient/application"
@@ -20,12 +16,18 @@ import (
 	"github.com/argoproj/gitops-engine/pkg/sync/hook"
 	"github.com/argoproj/gitops-engine/pkg/sync/ignore"
 	"github.com/argoproj/gitops-engine/pkg/utils/kube"
+	"github.com/argoproj/gitops-engine/pkg/utils/tracing"
 	"github.com/ghodss/yaml"
 	"github.com/go-logr/zerologr"
 	"github.com/pmezard/go-difflib/difflib"
 	"github.com/rs/zerolog/log"
+	"io"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/rest"
+	"k8s.io/klog/v2/textlogger"
+	"strings"
+	"time"
 
 	"github.com/zapier/kubechecks/pkg/checks"
 	"github.com/zapier/kubechecks/pkg/msg"
@@ -201,11 +203,33 @@ func generateDiff(ctx context.Context, request checks.Request, argoSettings *set
 	ignoreNormalizerOpts := normalizers.IgnoreNormalizerOpts{
 		JQExecutionTimeout: 1 * time.Second,
 	}
+	kubeCtl := &kube.KubectlCmd{
+		Tracer: tracing.NopTracer{},
+		Log:    textlogger.NewLogger(textlogger.NewConfig()),
+	}
+	config, err := rest.InClusterConfig()
+	if err != nil {
+		return diff.DiffResult{}, err
+	}
+	apiRes, _, err := kubeCtl.LoadOpenAPISchema(config)
+	if err != nil {
+		return diff.DiffResult{}, err
+	}
+	resources, _, err := kubeCtl.ManageResources(config, apiRes)
+	if err != nil {
+		return diff.DiffResult{}, err
+	}
+	dryRunner := diff.NewK8sServerSideDryRunner(resources)
+
 	diffConfig, err := argodiff.NewDiffConfigBuilder().
 		WithLogger(zerologr.New(&log.Logger)).
 		WithDiffSettings(request.App.Spec.IgnoreDifferences, overrides, ignoreAggregatedRoles, ignoreNormalizerOpts).
 		WithTracking(argoSettings.AppLabelKey, argoSettings.TrackingMethod).
 		WithNoCache().
+		WithIgnoreMutationWebhook(false).
+		WithServerSideDiff(true).
+		WithServerSideDryRunner(dryRunner).
+		WithManager("application/apply-patch").
 		Build()
 	if err != nil {
 		telemetry.SetError(span, err, "Build Diff")

Feedback & Suggestions:

1. Error Handling: The new code introduces several points where errors are returned if certain operations fail (e.g., InClusterConfig, LoadOpenAPISchema, ManageResources). Consider adding more context to these errors to make debugging easier. For example, wrap the errors with additional messages using fmt.Errorf("context: %w", err).

2. Performance Consideration: The addition of server-side diffing and dry-run capabilities is a great enhancement for accuracy and efficiency. However, ensure that these operations do not introduce significant latency, especially in large-scale environments. Consider adding metrics to monitor the performance impact.

3. Security: When dealing with Kubernetes configurations and API resources, ensure that the necessary permissions are in place and that sensitive data is not inadvertently exposed. Review the permissions required for InClusterConfig and ManageResources to ensure they align with the principle of least privilege.

4. Logging: The introduction of textlogger is a good step for structured logging. Ensure that sensitive information is not logged, and consider using log levels appropriately to avoid cluttering logs with debug information in production environments.

5. Code Clarity: The new imports and logic related to Kubernetes API interactions increase the complexity of the generateDiff function. Consider refactoring this logic into a separate function to improve readability and maintainability.

Overall, the changes introduce valuable functionality, but attention to error handling, performance, and security will ensure robustness. 🛡️✨

---

---

Dependency Review

Click to read mergecats review!

No suggestions found