From 48f01b5965055614bb0d423c46a2162c7af14b0d Mon Sep 17 00:00:00 2001 From: Alexandre Gaudreault Date: Tue, 16 Dec 2025 12:32:05 -0500 Subject: [PATCH] test(engine): refactor engine tests to ignore dry-run results (#25674) Signed-off-by: Alexandre Gaudreault --- gitops-engine/pkg/sync/sync_context_test.go | 54 +++--------- .../kube/kubetest/mock_resource_operations.go | 27 ++++-- .../pkg/utils/testing/api_resources.go | 87 +++++++++++++++++++ gitops-engine/pkg/utils/testing/testdata.go | 4 +- util/metrics/kubectl/util_test.go | 4 +- 5 files changed, 121 insertions(+), 55 deletions(-) create mode 100644 gitops-engine/pkg/utils/testing/api_resources.go diff --git a/gitops-engine/pkg/sync/sync_context_test.go b/gitops-engine/pkg/sync/sync_context_test.go index 59f4c52ebd..8f4b5dee88 100644 --- a/gitops-engine/pkg/sync/sync_context_test.go +++ b/gitops-engine/pkg/sync/sync_context_test.go @@ -38,31 +38,13 @@ import ( testingutils "github.com/argoproj/gitops-engine/pkg/utils/testing" ) -var standardVerbs = metav1.Verbs{"create", "delete", "deletecollection", "get", "list", "patch", "update", "watch"} - func newTestSyncCtx(getResourceFunc *func(ctx context.Context, config *rest.Config, gvk schema.GroupVersionKind, name string, namespace string) (*unstructured.Unstructured, error), opts ...SyncOpt) *syncContext { - fakeDisco := &fakedisco.FakeDiscovery{Fake: &testcore.Fake{}} - fakeDisco.Resources = append(make([]*metav1.APIResourceList, 0), - &metav1.APIResourceList{ - GroupVersion: "v1", - APIResources: []metav1.APIResource{ - {Name: "pods", Kind: "Pod", Group: "", Version: "v1", Namespaced: true, Verbs: standardVerbs}, - {Name: "services", Kind: "Service", Group: "", Version: "v1", Namespaced: true, Verbs: standardVerbs}, - {Name: "namespaces", Kind: "Namespace", Group: "", Version: "v1", Namespaced: false, Verbs: standardVerbs}, - }, - }, - &metav1.APIResourceList{ - GroupVersion: "apps/v1", - APIResources: []metav1.APIResource{ - {Name: "deployments", Kind: "Deployment", Group: "apps", Version: "v1", Namespaced: true, Verbs: standardVerbs}, - }, - }) sc := syncContext{ config: &rest.Config{}, rawConfig: &rest.Config{}, namespace: testingutils.FakeArgoCDNamespace, revision: "FooBarBaz", - disco: fakeDisco, + disco: &fakedisco.FakeDiscovery{Fake: &testcore.Fake{Resources: testingutils.StaticAPIResources}}, log: textlogger.NewLogger(textlogger.NewConfig()).WithValues("application", "fake-app"), resources: map[kube.ResourceKey]reconciledResource{}, syncRes: map[string]synccommon.ResourceSyncResult{}, @@ -145,6 +127,7 @@ func TestSyncNamespaceCreatedBeforeDryRunWithFailure(t *testing.T) { return true, nil }), func(ctx *syncContext) { resourceOps := ctx.resourceOps.(*kubetest.MockResourceOps) + resourceOps.ExecuteForDryRun = true resourceOps.Commands = map[string]kubetest.KubectlOutput{} resourceOps.Commands[pod.GetName()] = kubetest.KubectlOutput{ Output: "should not be returned", @@ -230,27 +213,19 @@ func TestSyncCustomResources(t *testing.T) { t.Run(tt.name, func(t *testing.T) { knownCustomResourceTypes := []metav1.APIResource{} if tt.fields.crdAlreadyPresent { - knownCustomResourceTypes = append(knownCustomResourceTypes, metav1.APIResource{Kind: "TestCrd", Group: "argoproj.io", Version: "v1", Namespaced: true, Verbs: standardVerbs}) + knownCustomResourceTypes = append(knownCustomResourceTypes, metav1.APIResource{Kind: "TestCrd", Group: "test.io", Version: "v1", Namespaced: true, Verbs: testingutils.CommonVerbs}) } syncCtx := newTestSyncCtx(nil) fakeDisco := syncCtx.disco.(*fakedisco.FakeDiscovery) - fakeDisco.Resources = []*metav1.APIResourceList{ - { - GroupVersion: "argoproj.io/v1", - APIResources: knownCustomResourceTypes, - }, - { - GroupVersion: "apiextensions.k8s.io/v1beta1", - APIResources: []metav1.APIResource{ - {Kind: "CustomResourceDefinition", Group: "apiextensions.k8s.io", Version: "v1beta1", Namespaced: true, Verbs: standardVerbs}, - }, - }, - } + fakeDisco.Resources = append(fakeDisco.Resources, &metav1.APIResourceList{ + GroupVersion: "test.io/v1", + APIResources: knownCustomResourceTypes, + }) cr := testingutils.Unstructured(` { - "apiVersion": "argoproj.io/v1", + "apiVersion": "test.io/v1", "kind": "TestCrd", "metadata": { "name": "my-resource" @@ -533,15 +508,6 @@ func TestSync_ApplyOutOfSyncOnly_ClusterResources(t *testing.T) { syncCtx := newTestSyncCtx(nil, WithResourceModificationChecker(true, diffResultListClusterResource())) syncCtx.applyOutOfSyncOnly = true - fakeDisco := syncCtx.disco.(*fakedisco.FakeDiscovery) - fakeDisco.Resources = []*metav1.APIResourceList{ - { - GroupVersion: "v1", - APIResources: []metav1.APIResource{ - {Kind: "Namespace", Group: "", Version: "v1", Namespaced: false, Verbs: standardVerbs}, - }, - }, - } t.Run("cluster resource with target ns having namespace filled", func(t *testing.T) { syncCtx.resources = groupResources(ReconciliationResult{ @@ -1741,11 +1707,11 @@ func Test_syncContext_hasCRDOfGroupKind(t *testing.T) { assert.True(t, (&syncContext{resources: groupResources(ReconciliationResult{ Live: []*unstructured.Unstructured{nil}, Target: []*unstructured.Unstructured{testingutils.NewCRD()}, - })}).hasCRDOfGroupKind("argoproj.io", "TestCrd")) + })}).hasCRDOfGroupKind("test.io", "TestCrd")) // hook assert.False(t, (&syncContext{hooks: []*unstructured.Unstructured{testingutils.NewCRD()}}).hasCRDOfGroupKind("", "")) - assert.True(t, (&syncContext{hooks: []*unstructured.Unstructured{testingutils.NewCRD()}}).hasCRDOfGroupKind("argoproj.io", "TestCrd")) + assert.True(t, (&syncContext{hooks: []*unstructured.Unstructured{testingutils.NewCRD()}}).hasCRDOfGroupKind("test.io", "TestCrd")) } func Test_setRunningPhase_healthyState(t *testing.T) { diff --git a/gitops-engine/pkg/utils/kube/kubetest/mock_resource_operations.go b/gitops-engine/pkg/utils/kube/kubetest/mock_resource_operations.go index 8f42428218..ae93c7a957 100644 --- a/gitops-engine/pkg/utils/kube/kubetest/mock_resource_operations.go +++ b/gitops-engine/pkg/utils/kube/kubetest/mock_resource_operations.go @@ -15,9 +15,10 @@ import ( ) type MockResourceOps struct { - Commands map[string]KubectlOutput - Events chan watch.Event - DynamicClient dynamic.Interface + ExecuteForDryRun bool + Commands map[string]KubectlOutput + Events chan watch.Event + DynamicClient dynamic.Interface lastCommandPerResource map[kube.ResourceKey]string lastValidate bool @@ -106,7 +107,10 @@ func (r *MockResourceOps) GetLastResourceCommand(key kube.ResourceKey) string { return r.lastCommandPerResource[key] } -func (r *MockResourceOps) ApplyResource(_ context.Context, obj *unstructured.Unstructured, _ cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) { +func (r *MockResourceOps) ApplyResource(_ context.Context, obj *unstructured.Unstructured, dryRun cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) { + if dryRun != cmdutil.DryRunNone && !r.ExecuteForDryRun { + return "", nil + } r.SetLastValidate(validate) r.SetLastServerSideApply(serverSideApply) r.SetLastServerSideApplyManager(manager) @@ -120,7 +124,10 @@ func (r *MockResourceOps) ApplyResource(_ context.Context, obj *unstructured.Uns return command.Output, command.Err } -func (r *MockResourceOps) ReplaceResource(_ context.Context, obj *unstructured.Unstructured, _ cmdutil.DryRunStrategy, force bool) (string, error) { +func (r *MockResourceOps) ReplaceResource(_ context.Context, obj *unstructured.Unstructured, dryRun cmdutil.DryRunStrategy, force bool) (string, error) { + if dryRun != cmdutil.DryRunNone && !r.ExecuteForDryRun { + return "", nil + } r.SetLastForce(force) command, ok := r.Commands[obj.GetName()] r.SetLastResourceCommand(kube.GetResourceKey(obj), "replace") @@ -131,7 +138,10 @@ func (r *MockResourceOps) ReplaceResource(_ context.Context, obj *unstructured.U return command.Output, command.Err } -func (r *MockResourceOps) UpdateResource(_ context.Context, obj *unstructured.Unstructured, _ cmdutil.DryRunStrategy) (*unstructured.Unstructured, error) { +func (r *MockResourceOps) UpdateResource(_ context.Context, obj *unstructured.Unstructured, dryRun cmdutil.DryRunStrategy) (*unstructured.Unstructured, error) { + if dryRun != cmdutil.DryRunNone && !r.ExecuteForDryRun { + return obj, nil + } r.SetLastResourceCommand(kube.GetResourceKey(obj), "update") command, ok := r.Commands[obj.GetName()] if !ok { @@ -140,7 +150,10 @@ func (r *MockResourceOps) UpdateResource(_ context.Context, obj *unstructured.Un return obj, command.Err } -func (r *MockResourceOps) CreateResource(_ context.Context, obj *unstructured.Unstructured, _ cmdutil.DryRunStrategy, _ bool) (string, error) { +func (r *MockResourceOps) CreateResource(_ context.Context, obj *unstructured.Unstructured, dryRun cmdutil.DryRunStrategy, _ bool) (string, error) { + if dryRun != cmdutil.DryRunNone && !r.ExecuteForDryRun { + return "", nil + } r.SetLastResourceCommand(kube.GetResourceKey(obj), "create") command, ok := r.Commands[obj.GetName()] if !ok { diff --git a/gitops-engine/pkg/utils/testing/api_resources.go b/gitops-engine/pkg/utils/testing/api_resources.go new file mode 100644 index 0000000000..86c14abcab --- /dev/null +++ b/gitops-engine/pkg/utils/testing/api_resources.go @@ -0,0 +1,87 @@ +package testing + +import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + +var ( + CommonVerbs = metav1.Verbs{"create", "get", "list", "watch", "update", "patch", "delete", "deletecollection"} + subresourceVerbs = metav1.Verbs{"get", "update", "patch"} +) + +// StaticAPIResources defines the common Kubernetes API resources that are usually returned by a DiscoveryClient +var StaticAPIResources = []*metav1.APIResourceList{ + { + GroupVersion: "v1", + APIResources: []metav1.APIResource{ + {Name: "pods", SingularName: "pod", Namespaced: true, Kind: "Pod", Verbs: CommonVerbs}, + {Name: "pods/status", SingularName: "", Namespaced: true, Kind: "Pod", Verbs: subresourceVerbs}, + {Name: "pods/log", SingularName: "", Namespaced: true, Kind: "Pod", Verbs: metav1.Verbs{"get"}}, + {Name: "pods/exec", SingularName: "", Namespaced: true, Kind: "Pod", Verbs: metav1.Verbs{"create"}}, + {Name: "services", SingularName: "service", Namespaced: true, Kind: "Service", Verbs: CommonVerbs}, + {Name: "services/status", SingularName: "", Namespaced: true, Kind: "Service", Verbs: subresourceVerbs}, + {Name: "configmaps", SingularName: "configmap", Namespaced: true, Kind: "ConfigMap", Verbs: CommonVerbs}, + {Name: "secrets", SingularName: "secret", Namespaced: true, Kind: "Secret", Verbs: CommonVerbs}, + {Name: "namespaces", SingularName: "namespace", Namespaced: false, Kind: "Namespace", Verbs: CommonVerbs}, + {Name: "namespaces/status", SingularName: "", Namespaced: false, Kind: "Namespace", Verbs: subresourceVerbs}, + {Name: "nodes", SingularName: "node", Namespaced: false, Kind: "Node", Verbs: metav1.Verbs{"get", "list", "watch"}}, + {Name: "persistentvolumes", SingularName: "persistentvolume", Namespaced: false, Kind: "PersistentVolume", Verbs: CommonVerbs}, + {Name: "persistentvolumeclaims", SingularName: "persistentvolumeclaim", Namespaced: true, Kind: "PersistentVolumeClaim", Verbs: CommonVerbs}, + {Name: "persistentvolumeclaims/status", SingularName: "", Namespaced: true, Kind: "PersistentVolumeClaim", Verbs: subresourceVerbs}, + {Name: "events", SingularName: "event", Namespaced: true, Kind: "Event", Verbs: metav1.Verbs{"create", "get", "list", "watch"}}, + {Name: "serviceaccounts", SingularName: "serviceaccount", Namespaced: true, Kind: "ServiceAccount", Verbs: CommonVerbs}, + }, + }, + { + GroupVersion: "apps/v1", + APIResources: []metav1.APIResource{ + {Name: "deployments", SingularName: "deployment", Namespaced: true, Kind: "Deployment", Verbs: CommonVerbs}, + {Name: "deployments/status", SingularName: "", Namespaced: true, Kind: "Deployment", Verbs: subresourceVerbs}, + {Name: "deployments/scale", SingularName: "", Namespaced: true, Kind: "Scale", Verbs: subresourceVerbs}, + {Name: "statefulsets", SingularName: "statefulset", Namespaced: true, Kind: "StatefulSet", Verbs: CommonVerbs}, + {Name: "statefulsets/status", SingularName: "", Namespaced: true, Kind: "StatefulSet", Verbs: subresourceVerbs}, + {Name: "statefulsets/scale", SingularName: "", Namespaced: true, Kind: "Scale", Verbs: subresourceVerbs}, + {Name: "daemonsets", SingularName: "daemonset", Namespaced: true, Kind: "DaemonSet", Verbs: CommonVerbs}, + {Name: "daemonsets/status", SingularName: "", Namespaced: true, Kind: "DaemonSet", Verbs: subresourceVerbs}, + {Name: "replicasets", SingularName: "replicaset", Namespaced: true, Kind: "ReplicaSet", Verbs: CommonVerbs}, + {Name: "replicasets/status", SingularName: "", Namespaced: true, Kind: "ReplicaSet", Verbs: subresourceVerbs}, + }, + }, + { + GroupVersion: "batch/v1", + APIResources: []metav1.APIResource{ + {Name: "jobs", SingularName: "job", Namespaced: true, Kind: "Job", Verbs: CommonVerbs}, + {Name: "jobs/status", SingularName: "", Namespaced: true, Kind: "Job", Verbs: subresourceVerbs}, + {Name: "cronjobs", SingularName: "cronjob", Namespaced: true, Kind: "CronJob", Verbs: CommonVerbs}, + {Name: "cronjobs/status", SingularName: "", Namespaced: true, Kind: "CronJob", Verbs: subresourceVerbs}, + }, + }, + { + GroupVersion: "rbac.authorization.k8s.io/v1", + APIResources: []metav1.APIResource{ + {Name: "roles", SingularName: "role", Namespaced: true, Kind: "Role", Verbs: CommonVerbs}, + {Name: "rolebindings", SingularName: "rolebinding", Namespaced: true, Kind: "RoleBinding", Verbs: CommonVerbs}, + {Name: "clusterroles", SingularName: "clusterrole", Namespaced: false, Kind: "ClusterRole", Verbs: CommonVerbs}, + {Name: "clusterrolebindings", SingularName: "clusterrolebinding", Namespaced: false, Kind: "ClusterRoleBinding", Verbs: CommonVerbs}, + }, + }, + { + GroupVersion: "networking.k8s.io/v1", + APIResources: []metav1.APIResource{ + {Name: "ingresses", SingularName: "ingress", Namespaced: true, Kind: "Ingress", Verbs: CommonVerbs}, + {Name: "ingresses/status", SingularName: "", Namespaced: true, Kind: "Ingress", Verbs: subresourceVerbs}, + {Name: "networkpolicies", SingularName: "networkpolicy", Namespaced: true, Kind: "NetworkPolicy", Verbs: CommonVerbs}, + }, + }, + { + GroupVersion: "policy/v1", + APIResources: []metav1.APIResource{ + {Name: "poddisruptionbudgets", SingularName: "poddisruptionbudget", Namespaced: true, Kind: "PodDisruptionBudget", Verbs: CommonVerbs}, + {Name: "poddisruptionbudgets/status", SingularName: "", Namespaced: true, Kind: "PodDisruptionBudget", Verbs: subresourceVerbs}, + }, + }, + { + GroupVersion: "apiextensions.k8s.io/v1", + APIResources: []metav1.APIResource{ + {Kind: "CustomResourceDefinition", Group: "apiextensions.k8s.io", Version: "v1", Namespaced: false, Verbs: CommonVerbs}, + }, + }, +} diff --git a/gitops-engine/pkg/utils/testing/testdata.go b/gitops-engine/pkg/utils/testing/testdata.go index ad446fdfb8..5ea16e8cd1 100644 --- a/gitops-engine/pkg/utils/testing/testdata.go +++ b/gitops-engine/pkg/utils/testing/testdata.go @@ -77,12 +77,12 @@ func NewService() *unstructured.Unstructured { } func NewCRD() *unstructured.Unstructured { - return Unstructured(`apiVersion: apiextensions.k8s.io/v1beta1 + return Unstructured(`apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: name: testcrds.argoproj.io spec: - group: argoproj.io + group: test.io version: v1 scope: Namespaced names: diff --git a/util/metrics/kubectl/util_test.go b/util/metrics/kubectl/util_test.go index d0d8fabb98..d69232785b 100644 --- a/util/metrics/kubectl/util_test.go +++ b/util/metrics/kubectl/util_test.go @@ -91,13 +91,13 @@ func Test_resolveK8sRequestVerb(t *testing.T) { { testName: "CRD List", method: "GET", - url: "https://127.0.0.1/apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions", + url: "https://127.0.0.1/apis/apiextensions.k8s.io/v1/customresourcedefinitions", expected: "List", }, { testName: "CRD Get", method: "GET", - url: "https://127.0.0.1/apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/dummies.argoproj.io", + url: "https://127.0.0.1/apis/apiextensions.k8s.io/v1/customresourcedefinitions/dummies.argoproj.io", expected: "Get", }, {