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 available at ghcr.io/zapier/kubechecks:0.0.0-pr319.

[zapier-sre-bot]

Mergecat's Review

Click to read mergecats review!

😼 Mergecat review of go.mod

@@ -61,7 +61,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
 )
@@ -294,7 +295,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. Ensure that this change is intentional and that the older version is compatible with the rest of your dependencies. Downgrading can introduce compatibility issues or missing features. 🕵️‍♂️

2. Dependency Management: The k8s.io/klog/v2 dependency has been moved from an indirect to a direct requirement. This is fine if you are directly using klog in your code. However, if it's not directly used, consider keeping it as an indirect dependency to avoid unnecessary clutter in your go.mod. 📦

3. Consistency Check: Ensure that the changes made to the require section are reflected in the replace section if necessary. This helps maintain consistency across your dependency management. 🔄

---

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

@@ -19,15 +19,18 @@ import (
 	"github.com/argoproj/gitops-engine/pkg/diff"
 	"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"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/rest"
+	"k8s.io/klog/v2/textlogger"
 
 	"github.com/zapier/kubechecks/pkg/checks"
+	"github.com/zapier/kubechecks/pkg/gitops-engine/pkg/utils/kube"
 	"github.com/zapier/kubechecks/pkg/msg"
 	"github.com/zapier/kubechecks/telemetry"
 )
@@ -201,11 +204,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 directly. Consider adding more context to these errors using fmt.Errorf or a similar approach to make debugging easier. For example, when rest.InClusterConfig() fails, it would be helpful to know that the error occurred while trying to get the in-cluster config.

2. Logging: The new code uses textlogger for logging. Ensure that this logger is properly configured to capture logs at the desired level. Additionally, consider using structured logging to provide more context in logs, which can be beneficial for debugging.

3. Performance: The introduction of server-side diff and dry-runner is a good enhancement for performance. However, ensure that these operations are not too resource-intensive, especially in large clusters. Monitor the performance impact after deployment.

4. Security: When using rest.InClusterConfig(), ensure that the application has the necessary permissions to access the Kubernetes API. Misconfigured permissions can lead to security vulnerabilities.

5. Code Clarity: The kubeCtl initialization and subsequent API calls are a bit dense. Consider breaking them into smaller, well-named functions to improve readability and maintainability.

6. Dependency Management: The import path for kube has changed. Ensure that this change is intentional and that the new path is correct and accessible in all environments where this code will run.

---

---

Dependency Review

Click to read mergecats review!

No suggestions found