From 7797080dc495e32a104b38f0fd8eaa2bd2d6029d Mon Sep 17 00:00:00 2001 From: Janelle Law Date: Fri, 19 Sep 2025 10:33:58 -0700 Subject: [PATCH 1/4] Move input resource creation to helper func ref: https://issues.redhat.com/browse/ACM-22932 No logic changed here. Refactor before adding a new dry run cli flag Signed-off-by: Janelle Law --- pkg/dryrun/dryrun.go | 87 +++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 38 deletions(-) diff --git a/pkg/dryrun/dryrun.go b/pkg/dryrun/dryrun.go index 58ef48a9..27b52419 100644 --- a/pkg/dryrun/dryrun.go +++ b/pkg/dryrun/dryrun.go @@ -74,44 +74,9 @@ func (d *DryRunner) dryRun(cmd *cobra.Command, args []string) error { return fmt.Errorf("unable to setup the dryrun reconciler: %w", err) } - // Apply the user's resources to the fake cluster - for _, obj := range inputObjects { - gvk := obj.GroupVersionKind() - - scopedGVR, err := rec.DynamicWatcher.GVKToGVR(gvk) - if err != nil { - if errors.Is(err, depclient.ErrNoVersionedResource) { - return fmt.Errorf("%w for kind %v: if this is a custom resource, it may need an "+ - "entry in the mappings file", err, gvk.Kind) - } - - return fmt.Errorf("unable to apply an input resource: %w", err) - } - - var resInt dynamic.ResourceInterface - - if scopedGVR.Namespaced { - if obj.GetNamespace() == "" { - obj.SetNamespace("default") - } - - resInt = rec.TargetK8sDynamicClient.Resource(scopedGVR.GroupVersionResource).Namespace(obj.GetNamespace()) - } else { - resInt = rec.TargetK8sDynamicClient.Resource(scopedGVR.GroupVersionResource) - } - - sanitizeForCreation(obj) - - if _, err := resInt.Create(ctx, obj, metav1.CreateOptions{}); err != nil && - !k8serrors.IsAlreadyExists(err) { - return fmt.Errorf("unable to apply an input resource: %w", err) - } - - // Manually convert resources from the dynamic client to the runtime client - err = rec.Client.Create(ctx, obj) - if err != nil && !k8serrors.IsAlreadyExists(err) { - return err - } + err = d.applyInputResources(ctx, rec, inputObjects) + if err != nil { + return fmt.Errorf("unable to apply input resources: %w", err) } cfgPolicyNN := types.NamespacedName{ @@ -367,6 +332,52 @@ func (d *DryRunner) readInputResources(cmd *cobra.Command, args []string) ( return rawInputs, nil } +func (d *DryRunner) applyInputResources( + ctx context.Context, rec *ctrl.ConfigurationPolicyReconciler, inputObjects []*unstructured.Unstructured, +) error { + // Apply the user's resources to the fake cluster + for _, obj := range inputObjects { + gvk := obj.GroupVersionKind() + + scopedGVR, err := rec.DynamicWatcher.GVKToGVR(gvk) + if err != nil { + if errors.Is(err, depclient.ErrNoVersionedResource) { + return fmt.Errorf("%w for kind %v: if this is a custom resource, it may need an "+ + "entry in the mappings file", err, gvk.Kind) + } + + return fmt.Errorf("unable to apply an input resource: %w", err) + } + + var resInt dynamic.ResourceInterface + + if scopedGVR.Namespaced { + if obj.GetNamespace() == "" { + obj.SetNamespace("default") + } + + resInt = rec.TargetK8sDynamicClient.Resource(scopedGVR.GroupVersionResource).Namespace(obj.GetNamespace()) + } else { + resInt = rec.TargetK8sDynamicClient.Resource(scopedGVR.GroupVersionResource) + } + + sanitizeForCreation(obj) + + if _, err := resInt.Create(ctx, obj, metav1.CreateOptions{}); err != nil && + !k8serrors.IsAlreadyExists(err) { + return fmt.Errorf("unable to apply an input resource: %w", err) + } + + // Manually convert resources from the dynamic client to the runtime client + err = rec.Client.Create(ctx, obj) + if err != nil && !k8serrors.IsAlreadyExists(err) { + return err + } + } + + return nil +} + // setupLogs configures klog and the controller-runtime logger to send logs to the // path defined in the configuration. If that option is empty, logs will be discarded. func (d *DryRunner) setupLogs() error { From d2b358c656dc5335c33d071a89760ac567ab732b Mon Sep 17 00:00:00 2001 From: Janelle Law Date: Fri, 19 Sep 2025 10:38:03 -0700 Subject: [PATCH 2/4] Add dryrun CLI flag to read cluster resources ref: https://issues.redhat.com/browse/ACM-22932 Instead of providing input yaml files to the dryrun command args, set the --from-cluster flag to read cluster resources via default kubeconfig. As before, the policies are patched with remediationAction: Inform, preventing modifications to the cluster. Signed-off-by: Janelle Law --- pkg/dryrun/cmd.go | 13 ++++ pkg/dryrun/dryrun.go | 147 ++++++++++++++++++++++++++++++------------- 2 files changed, 117 insertions(+), 43 deletions(-) diff --git a/pkg/dryrun/cmd.go b/pkg/dryrun/cmd.go index 2d41a1b0..c357c707 100644 --- a/pkg/dryrun/cmd.go +++ b/pkg/dryrun/cmd.go @@ -21,6 +21,7 @@ type DryRunner struct { logPath string noColors bool fullDiffs bool + fromCluster bool } var ErrNonCompliant = errors.New("policy is NonCompliant") @@ -105,6 +106,18 @@ func (d *DryRunner) GetCmd() *cobra.Command { "the DRYRUN_MAPPINGS_FILE environment variable.", ) + fromCluster := os.Getenv("DRYRUN_FROM_CLUSTER") == "true" // false if not set + + cmd.Flags().BoolVar( + &d.fromCluster, + "from-cluster", + fromCluster, + "Read the current state of resources from the currently configured Kubernetes cluster instead of "+ + "from input files. Uses the default kubeconfig or KUBECONFIG environment variable. "+ + "Any input files representing the cluster state are ignored. "+ + "Can also be set via the DRYRUN_FROM_CLUSTER environment variable.", + ) + cmd.AddCommand(&cobra.Command{ Use: "generate", Short: "Generate an API Mappings file", diff --git a/pkg/dryrun/dryrun.go b/pkg/dryrun/dryrun.go index 27b52419..59172948 100644 --- a/pkg/dryrun/dryrun.go +++ b/pkg/dryrun/dryrun.go @@ -30,8 +30,10 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" dynfake "k8s.io/client-go/dynamic/fake" + "k8s.io/client-go/kubernetes" clientsetfake "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/record" klog "k8s.io/klog/v2" parentpolicyv1 "open-cluster-management.io/governance-policy-propagator/api/v1" @@ -57,11 +59,6 @@ func (d *DryRunner) dryRun(cmd *cobra.Command, args []string) error { return fmt.Errorf("unable to read input policy: %w", err) } - inputObjects, err := d.readInputResources(cmd, args) - if err != nil { - return fmt.Errorf("unable to read input resources: %w", err) - } - if err := d.setupLogs(); err != nil { return fmt.Errorf("unable to setup the logging configuration: %w", err) } @@ -74,9 +71,16 @@ func (d *DryRunner) dryRun(cmd *cobra.Command, args []string) error { return fmt.Errorf("unable to setup the dryrun reconciler: %w", err) } - err = d.applyInputResources(ctx, rec, inputObjects) - if err != nil { - return fmt.Errorf("unable to apply input resources: %w", err) + if !d.fromCluster { + inputObjects, err := d.readInputResources(cmd, args) + if err != nil { + return fmt.Errorf("unable to read input resources: %w", err) + } + + err = d.applyInputResources(ctx, rec, inputObjects) + if err != nil { + return fmt.Errorf("unable to apply input resources: %w", err) + } } cfgPolicyNN := types.NamespacedName{ @@ -332,10 +336,12 @@ func (d *DryRunner) readInputResources(cmd *cobra.Command, args []string) ( return rawInputs, nil } +// applyInputResources applies the user's resources to the fake cluster func (d *DryRunner) applyInputResources( - ctx context.Context, rec *ctrl.ConfigurationPolicyReconciler, inputObjects []*unstructured.Unstructured, + ctx context.Context, + rec *ctrl.ConfigurationPolicyReconciler, + inputObjects []*unstructured.Unstructured, ) error { - // Apply the user's resources to the fake cluster for _, obj := range inputObjects { gvk := obj.GroupVersionKind() @@ -346,7 +352,7 @@ func (d *DryRunner) applyInputResources( "entry in the mappings file", err, gvk.Kind) } - return fmt.Errorf("unable to apply an input resource: %w", err) + return err } var resInt dynamic.ResourceInterface @@ -365,7 +371,7 @@ func (d *DryRunner) applyInputResources( if _, err := resInt.Create(ctx, obj, metav1.CreateOptions{}); err != nil && !k8serrors.IsAlreadyExists(err) { - return fmt.Errorf("unable to apply an input resource: %w", err) + return err } // Manually convert resources from the dynamic client to the runtime client @@ -428,10 +434,34 @@ func (d *DryRunner) setupReconciler( return nil, err } - dynamicClient := dynfake.NewSimpleDynamicClient(scheme.Scheme) - clientset := clientsetfake.NewSimpleClientset() - watcherReconciler, _ := depclient.NewControllerRuntimeSource() + runtimeClient := clientfake.NewClientBuilder(). + WithScheme(scheme.Scheme). + WithObjects(configPolCRD, cfgPolicy). + WithStatusSubresource(cfgPolicy). + Build() + nsSelUpdatesChan := make(chan event.GenericEvent, 20) + var clientset kubernetes.Interface + var dynamicClient dynamic.Interface + var nsSelReconciler common.NamespaceSelectorReconciler + + if d.fromCluster { + var nsSelClient client.Client + var err error + + clientset, dynamicClient, nsSelClient, err = setupClusterClients() + if err != nil { + return nil, err + } + + nsSelReconciler = common.NewNamespaceSelectorReconciler(nsSelClient, nsSelUpdatesChan) + } else { + dynamicClient = dynfake.NewSimpleDynamicClient(scheme.Scheme) + clientset = clientsetfake.NewSimpleClientset() + nsSelReconciler = common.NewNamespaceSelectorReconciler(runtimeClient, nsSelUpdatesChan) + } + + watcherReconciler, _ := depclient.NewControllerRuntimeSource() dynamicWatcher := depclient.NewWithClients( dynamicClient, clientset.Discovery(), @@ -446,14 +476,28 @@ func (d *DryRunner) setupReconciler( } }() - runtimeClient := clientfake.NewClientBuilder(). - WithScheme(scheme.Scheme). - WithObjects(configPolCRD, cfgPolicy). - WithStatusSubresource(cfgPolicy). - Build() + rec := ctrl.ConfigurationPolicyReconciler{ + Client: runtimeClient, + DecryptionConcurrency: 1, + DynamicWatcher: dynamicWatcher, + Scheme: scheme.Scheme, + Recorder: record.NewFakeRecorder(8), + InstanceName: "policy-cli", + TargetK8sClient: clientset, + TargetK8sDynamicClient: dynamicClient, + SelectorReconciler: &nsSelReconciler, + EnableMetrics: false, + UninstallMode: false, + EvalBackoffSeconds: 5, + FullDiffs: d.fullDiffs, + } - nsSelUpdatesChan := make(chan event.GenericEvent, 20) - nsSelReconciler := common.NewNamespaceSelectorReconciler(runtimeClient, nsSelUpdatesChan) + // wait for dynamic watcher to have started + <-rec.DynamicWatcher.Started() + + if d.fromCluster { + return &rec, nil + } defaultNs := &unstructured.Unstructured{ Object: map[string]interface{}{ @@ -478,21 +522,7 @@ func (d *DryRunner) setupReconciler( return nil, err } - rec := ctrl.ConfigurationPolicyReconciler{ - Client: runtimeClient, - DecryptionConcurrency: 1, - DynamicWatcher: dynamicWatcher, - Scheme: scheme.Scheme, - Recorder: record.NewFakeRecorder(8), - InstanceName: "policy-cli", - TargetK8sClient: clientset, - TargetK8sDynamicClient: dynamicClient, - SelectorReconciler: &nsSelReconciler, - EnableMetrics: false, - UninstallMode: false, - EvalBackoffSeconds: 5, - FullDiffs: d.fullDiffs, - } + fakeClientset := clientset.(*clientsetfake.Clientset) if d.mappingsPath != "" { mFile, err := os.ReadFile(d.mappingsPath) @@ -505,19 +535,16 @@ func (d *DryRunner) setupReconciler( return nil, err } - clientset.Resources = mappings.ResourceLists(apiMappings) + fakeClientset.Resources = mappings.ResourceLists(apiMappings) } else { - clientset.Resources, err = mappings.DefaultResourceLists() + fakeClientset.Resources, err = mappings.DefaultResourceLists() if err != nil { return nil, err } } // Add open-cluster-management policy CRD - addSupportedResources(clientset) - - // wait for dynamic watcher to have started - <-rec.DynamicWatcher.Started() + addSupportedResources(fakeClientset) return &rec, nil } @@ -633,6 +660,40 @@ func sanitizeForCreation(obj *unstructured.Unstructured) { delete(obj.Object["metadata"].(map[string]interface{}), "uid") } +func setupClusterClients() (kubernetes.Interface, dynamic.Interface, client.Client, error) { + loadingRules := clientcmd.NewDefaultClientConfigLoadingRules() + clientConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig( + loadingRules, &clientcmd.ConfigOverrides{}, + ) + + kubeConfig, err := clientConfig.ClientConfig() + if err != nil { + return nil, nil, nil, err + } + + clientset, err := kubernetes.NewForConfig(kubeConfig) + if err != nil { + return nil, nil, nil, err + } + + dynamicClient, err := dynamic.NewForConfig(kubeConfig) + if err != nil { + return nil, nil, nil, err + } + + readOnlyMode := true // Prevent modifications to the cluster + + runtimeClient, err := client.New(kubeConfig, client.Options{ + Scheme: scheme.Scheme, + DryRun: &readOnlyMode, + }) + if err != nil { + return nil, nil, nil, err + } + + return clientset, dynamicClient, runtimeClient, nil +} + func addSupportedResources(clientset *clientsetfake.Clientset) { clientset.Resources = append(clientset.Resources, &metav1.APIResourceList{ GroupVersion: parentpolicyv1.GroupVersion.String(), From 048b588b28ef8db9ca5251791a2e488290c44b4f Mon Sep 17 00:00:00 2001 From: Janelle Law Date: Mon, 15 Sep 2025 10:20:43 -0700 Subject: [PATCH 3/4] Add dryrun CLI e2e tests ref: https://issues.redhat.com/browse/ACM-22932 Tests the dryrun CLI can read resources from a cluster with --from-cluster flag. Reuses the dryrun unit test cases, applies the resources to the test kind cluster, then runs the e2e tests. Sometimes the policy diff from the cluster returns extra context lines surrounding the relevant +/- changes. When this diff does not match the expected output.txt from the unit tests, the e2e test will compare the output with the output_from_cluster.txt file. Assisted by Cursor. Signed-off-by: Janelle Law --- .../output_from_cluster.txt | 15 + .../object_namespaced/output_from_cluster.txt | 16 + .../object_namespaced/policy.yaml | 2 +- .../object_pod/output_from_cluster.txt | 15 + .../output_from_cluster.txt | 17 + .../output_from_cluster.txt | 36 ++ .../objectname_nsselector/policy.yaml | 2 +- .../desired_status_from_cluster.yaml | 62 ++++ .../diff/truncated/output_from_cluster.txt | 64 ++++ .../output_from_cluster.txt | 41 +++ test/e2e/case46_dryrun_cli_test.go | 333 ++++++++++++++++++ 11 files changed, 601 insertions(+), 2 deletions(-) create mode 100644 test/dryrun/context_vars/object_cluster_scoped/output_from_cluster.txt create mode 100644 test/dryrun/context_vars/object_namespaced/output_from_cluster.txt create mode 100644 test/dryrun/context_vars/object_pod/output_from_cluster.txt create mode 100644 test/dryrun/context_vars/object_pod_default_func/output_from_cluster.txt create mode 100644 test/dryrun/context_vars/objectname_nsselector/output_from_cluster.txt create mode 100644 test/dryrun/diff/truncated/desired_status_from_cluster.yaml create mode 100644 test/dryrun/diff/truncated/output_from_cluster.txt create mode 100644 test/dryrun/no_name/with_object_selector/musthave_mixed_noncompliant/output_from_cluster.txt create mode 100644 test/e2e/case46_dryrun_cli_test.go diff --git a/test/dryrun/context_vars/object_cluster_scoped/output_from_cluster.txt b/test/dryrun/context_vars/object_cluster_scoped/output_from_cluster.txt new file mode 100644 index 00000000..8dbf13d5 --- /dev/null +++ b/test/dryrun/context_vars/object_cluster_scoped/output_from_cluster.txt @@ -0,0 +1,15 @@ +# Diffs: +v1 Namespace mega-mart: +--- mega-mart : existing ++++ mega-mart : updated +@@ -5,10 +5,12 @@ + city: durham + labels: + box: big ++ name: mega-mart ++ new-label: durham + name: mega-mart + spec: + finalizers: +# Compliance messages: +NonCompliant; violation - namespaces [mega-mart] found but not as specified diff --git a/test/dryrun/context_vars/object_namespaced/output_from_cluster.txt b/test/dryrun/context_vars/object_namespaced/output_from_cluster.txt new file mode 100644 index 00000000..c24882d1 --- /dev/null +++ b/test/dryrun/context_vars/object_namespaced/output_from_cluster.txt @@ -0,0 +1,16 @@ +# Diffs: +v1 ConfigMap mega-mart/inventory: +--- mega-mart/inventory : existing ++++ mega-mart/inventory : updated +@@ -2,10 +2,12 @@ + data: + inventory.yaml: 'appliance: toaster' + kind: ConfigMap + metadata: ++ labels: ++ new-label: toaster + name: inventory + namespace: mega-mart + +# Compliance messages: +NonCompliant; violation - configmaps [inventory] found but not as specified in namespace mega-mart diff --git a/test/dryrun/context_vars/object_namespaced/policy.yaml b/test/dryrun/context_vars/object_namespaced/policy.yaml index c1b403e6..e323101a 100644 --- a/test/dryrun/context_vars/object_namespaced/policy.yaml +++ b/test/dryrun/context_vars/object_namespaced/policy.yaml @@ -19,4 +19,4 @@ spec: name: '{{ .ObjectName }}' namespace: '{{ .ObjectNamespace }}' labels: - new-label: '{{ (fromYAML (index .Object.data "inventory.yaml")).appliance }}' + new-label: '{{ ne (printf "%s" .ObjectName) "inventory" | skipObject }}{{ (fromYAML (index .Object.data "inventory.yaml")).appliance }}' diff --git a/test/dryrun/context_vars/object_pod/output_from_cluster.txt b/test/dryrun/context_vars/object_pod/output_from_cluster.txt new file mode 100644 index 00000000..363ae900 --- /dev/null +++ b/test/dryrun/context_vars/object_pod/output_from_cluster.txt @@ -0,0 +1,15 @@ +# Diffs: +v1 Pod default/nginx-pod: +--- default/nginx-pod : existing ++++ default/nginx-pod : updated +@@ -1,9 +1,11 @@ + apiVersion: v1 + kind: Pod + metadata: ++ labels: ++ image: nginx:1.7.9 + name: nginx-pod + namespace: default + spec: +# Compliance messages: +NonCompliant; violation - pods [nginx-pod] found but not as specified in namespace default diff --git a/test/dryrun/context_vars/object_pod_default_func/output_from_cluster.txt b/test/dryrun/context_vars/object_pod_default_func/output_from_cluster.txt new file mode 100644 index 00000000..a70641c1 --- /dev/null +++ b/test/dryrun/context_vars/object_pod_default_func/output_from_cluster.txt @@ -0,0 +1,17 @@ +# Diffs: +v1 Pod dangler/nginx-pod: + +v1 Pod default/nginx-pod: +--- default/nginx-pod : existing ++++ default/nginx-pod : updated +@@ -1,9 +1,11 @@ + apiVersion: v1 + kind: Pod + metadata: ++ labels: ++ image: nginx:1.7.9 + name: nginx-pod + namespace: default + spec: +# Compliance messages: +NonCompliant; violation - pods [nginx-pod] not found in namespace dangler; pods [nginx-pod] found but not as specified in namespace default diff --git a/test/dryrun/context_vars/objectname_nsselector/output_from_cluster.txt b/test/dryrun/context_vars/objectname_nsselector/output_from_cluster.txt new file mode 100644 index 00000000..21a4efc8 --- /dev/null +++ b/test/dryrun/context_vars/objectname_nsselector/output_from_cluster.txt @@ -0,0 +1,36 @@ +# Diffs: +v1 ConfigMap mega-mart/inventory: +--- mega-mart/inventory : existing ++++ mega-mart/inventory : updated +@@ -1,6 +1,8 @@ + apiVersion: v1 ++data: ++ hocus: pocus + kind: ConfigMap + metadata: + name: inventory + namespace: mega-mart +v1 ConfigMap mega-mart-2/inventory: +--- mega-mart-2/inventory : existing ++++ mega-mart-2/inventory : updated +@@ -1,7 +1,8 @@ + apiVersion: v1 + data: ++ hocus: pocus + things: original-stuff + kind: ConfigMap + metadata: + name: inventory +v1 ConfigMap mega-mart-2/inventory-2: +--- mega-mart-2/inventory-2 : existing ++++ mega-mart-2/inventory-2 : updated +@@ -1,7 +1,8 @@ + apiVersion: v1 + data: ++ hocus: pocus + things: stuff + kind: ConfigMap + metadata: + name: inventory-2 +# Compliance messages: +NonCompliant; violation - configmaps [inventory-2] found but not as specified in namespace mega-mart-2; configmaps [inventory] found but not as specified in namespaces: mega-mart, mega-mart-2 diff --git a/test/dryrun/context_vars/objectname_nsselector/policy.yaml b/test/dryrun/context_vars/objectname_nsselector/policy.yaml index 3c014dcf..0163c408 100644 --- a/test/dryrun/context_vars/objectname_nsselector/policy.yaml +++ b/test/dryrun/context_vars/objectname_nsselector/policy.yaml @@ -15,4 +15,4 @@ spec: kind: ConfigMap metadata: name: '{{ not (hasPrefix "inv" .ObjectName) | skipObject }}' - data: '{{ set .Object.data "hocus" "pocus" | toJSON | toLiteral }}' + data: '{{ set (.Object.data | default (dict)) "hocus" "pocus" | toJSON | toLiteral }}' diff --git a/test/dryrun/diff/truncated/desired_status_from_cluster.yaml b/test/dryrun/diff/truncated/desired_status_from_cluster.yaml new file mode 100644 index 00000000..5143709f --- /dev/null +++ b/test/dryrun/diff/truncated/desired_status_from_cluster.yaml @@ -0,0 +1,62 @@ +compliant: NonCompliant +relatedObjects: +- compliant: NonCompliant + object: + apiVersion: v1 + kind: Namespace + properties: + diff: |- + # Truncated: showing 50/68 diff lines: + --- default : existing + +++ default : updated + @@ -1,8 +1,64 @@ + apiVersion: v1 + kind: Namespace + metadata: + + annotations: + + message1: message + + message2: message + + message3: message + + message4: message + + message5: message + + message6: message + + message7: message + + message8: message + + message9: message + + message10: message + + message11: message + + message12: message + + message13: message + + message14: message + + message15: message + + message16: message + + message17: message + + message18: message + + message19: message + + message20: message + + message21: message + + message22: message + + message23: message + + message24: message + + message25: message + + message26: message + + message27: message + + message28: message + + message29: message + + message30: message + + message31: message + + message32: message + + message33: message + + message34: message + + message35: message + + message36: message + + message37: message + + message38: message + + message39: message + + message40: message + + message41: message + + message42: message + + message43: message + + message44: message + + message45: message + + message46: message diff --git a/test/dryrun/diff/truncated/output_from_cluster.txt b/test/dryrun/diff/truncated/output_from_cluster.txt new file mode 100644 index 00000000..30e0b151 --- /dev/null +++ b/test/dryrun/diff/truncated/output_from_cluster.txt @@ -0,0 +1,64 @@ +# Status compare: +.compliant: 'NonCompliant' does match 'NonCompliant' +.relatedObjects[0] matches +.relatedObjects matches + Expected status matches the actual status + +# Diffs: +v1 Namespace default: +# Truncated: showing 50/68 diff lines: +--- default : existing ++++ default : updated +@@ -1,8 +1,64 @@ + apiVersion: v1 + kind: Namespace + metadata: ++ annotations: ++ message1: message ++ message2: message ++ message3: message ++ message4: message ++ message5: message ++ message6: message ++ message7: message ++ message8: message ++ message9: message ++ message10: message ++ message11: message ++ message12: message ++ message13: message ++ message14: message ++ message15: message ++ message16: message ++ message17: message ++ message18: message ++ message19: message ++ message20: message ++ message21: message ++ message22: message ++ message23: message ++ message24: message ++ message25: message ++ message26: message ++ message27: message ++ message28: message ++ message29: message ++ message30: message ++ message31: message ++ message32: message ++ message33: message ++ message34: message ++ message35: message ++ message36: message ++ message37: message ++ message38: message ++ message39: message ++ message40: message ++ message41: message ++ message42: message ++ message43: message ++ message44: message ++ message45: message ++ message46: message +# Compliance messages: +NonCompliant; violation - namespaces [default] found but not as specified diff --git a/test/dryrun/no_name/with_object_selector/musthave_mixed_noncompliant/output_from_cluster.txt b/test/dryrun/no_name/with_object_selector/musthave_mixed_noncompliant/output_from_cluster.txt new file mode 100644 index 00000000..06aa24b8 --- /dev/null +++ b/test/dryrun/no_name/with_object_selector/musthave_mixed_noncompliant/output_from_cluster.txt @@ -0,0 +1,41 @@ +# Status compare: +.compliant: 'NonCompliant' does match 'NonCompliant' +.relatedObjects[0] matches +.relatedObjects[1] matches +.relatedObjects[2] matches +.relatedObjects matches + Expected status matches the actual status + +# Diffs: +networking.k8s.io/v1 Ingress default/good-ingress: + +networking.k8s.io/v1 Ingress default/wrong-1-ingress: +--- default/wrong-1-ingress : existing ++++ default/wrong-1-ingress : updated +@@ -7,11 +7,11 @@ + name: wrong-1-ingress + namespace: default + spec: +- ingressClassName: wrong-name ++ ingressClassName: test + rules: + - http: + paths: + - backend: + service: +networking.k8s.io/v1 Ingress default/wrong-2-ingress: +--- default/wrong-2-ingress : existing ++++ default/wrong-2-ingress : updated +@@ -7,11 +7,11 @@ + name: wrong-2-ingress + namespace: default + spec: +- ingressClassName: wrong-name ++ ingressClassName: test + rules: + - http: + paths: + - backend: + service: +# Compliance messages: +NonCompliant; violation - ingresses [wrong-1-ingress, wrong-2-ingress] found but not as specified in namespace default diff --git a/test/e2e/case46_dryrun_cli_test.go b/test/e2e/case46_dryrun_cli_test.go new file mode 100644 index 00000000..a49ccbfd --- /dev/null +++ b/test/e2e/case46_dryrun_cli_test.go @@ -0,0 +1,333 @@ +// Copyright (c) 2025 Red Hat, Inc. +// Copyright Contributors to the Open Cluster Management project + +package e2e + +import ( + "bytes" + "fmt" + "os" + "path/filepath" + "slices" + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "gopkg.in/yaml.v3" + + "open-cluster-management.io/config-policy-controller/pkg/dryrun" + "open-cluster-management.io/config-policy-controller/test/utils" +) + +const ( + case46TestDataPath = "../dryrun/" + policyYAML = "policy.yaml" + resourceYamlPrefix = "input" + namespaceYamlPrefix = "input_ns" + errOutputFile = "error.txt" + defaultOutputFile = "output.txt" + outputFromClusterFile = "output_from_cluster.txt" + desiredStatusFile = "desired_status.yaml" + desiredStatusFromClusterFile = "desired_status_from_cluster.yaml" +) + +// persistentNamespaces are namespaces that should not be deleted after this test case +// because they persist across test suites or are system namespaces +var persistentNamespaces = []string{ + "managed", + "default", +} + +var untrackedMetadata = []string{ + "resourceVersion:", + "generatedName:", + "creationTimestamp:", + "deletionTimestamp:", + "selfLink:", + "uid:", + "metadata.kubernetes.io/metadata.name:", + "kubernetes.io/metadata.name:", +} + +var _ = Describe("Testing dryrun CLI", Ordered, func() { + var testNamespaces []string + + BeforeAll(func() { + // Setup test namespaces before creating any resources + testNamespaces, err := findInputNamespaces(case46TestDataPath) + Expect(err).ToNot(HaveOccurred()) + + for _, nsFilePath := range testNamespaces { + utils.Kubectl("apply", "-f", nsFilePath) + } + }) + + AfterAll(func() { + for _, nsFilePath := range testNamespaces { + if shouldDelete, err := containsManagedNamespace(nsFilePath); shouldDelete && err == nil { + utils.KubectlDelete("-f", nsFilePath, "--wait") + } + } + }) + + testScenarios := []struct { + description string + path string + }{ + {"Should handle context variables", "context_vars"}, + {"Should handle diffs", "diff"}, + {"Should handle pod annotations", "kind_field"}, + {"Should handle missing fields", "missing"}, + {"Should handle multiple objects", "multiple"}, + {"Should handle no name objects", "no_name"}, + {"Should handle object selector scenarios", "no_name/with_object_selector"}, + {"Should handle cluster-scoped no name", "no_name_clusterscope"}, + {"Should handle no namespace", "no_ns"}, + {"Should handle namespace selector", "ns_selector"}, + {"Should handle object template raw", "obj_tmpl_raw"}, + } + + for _, scenario := range testScenarios { + It(scenario.description, func() { + testDryRunScenarios(scenario.path) + }) + } +}) + +// testDryRunScenarios runs all test scenarios in a given test category +func testDryRunScenarios(testCategory string) { + testDirs, err := findTestDirectories(filepath.Join(case46TestDataPath, testCategory)) + Expect(err).ToNot(HaveOccurred()) + + for _, testDir := range testDirs { + By("Testing " + testCategory + "/" + testDir) + testDryrunCommand(testCategory, testDir) + } +} + +// testDryrunCommand runs a dryrun test scenario +func testDryrunCommand(testCategory, testDir string) { + testPath := filepath.Join(case46TestDataPath, testCategory, testDir) + inputResourcePaths, desiredStatusPath, outputPath := findTestFiles(testPath) + + for _, resource := range inputResourcePaths { + By("Applying input file " + resource) + utils.Kubectl("apply", "-f", resource) + } + + defer func() { + for _, resource := range inputResourcePaths { + By("Deleting input file " + resource) + utils.KubectlDelete("-f", resource, "--wait") + } + }() + + expectedBytes, err := os.ReadFile(outputPath) + Expect(err).ToNot(HaveOccurred()) + + expectedOutput := string(expectedBytes) + shouldFail := strings.HasSuffix(outputPath, errOutputFile) + + if shouldFail { + // Match dry run error whitespace formatting for error files + expectedOutput = strings.TrimSpace(strings.ReplaceAll(expectedOutput, "\n", " ")) + } + + By("Running dryrun command") + Eventually(func() string { + var output bytes.Buffer + cmd := (&dryrun.DryRunner{}).GetCmd() + cmd.SetOut(&output) + + args := []string{"--from-cluster", "--policy", filepath.Join(testPath, policyYAML), "--no-colors"} + if desiredStatusPath != "" { + args = append(args, "--desired-status", desiredStatusPath) + } + + cmd.SetArgs(args) + err := cmd.Execute() + actualOutput := output.String() + + if shouldFail { + Expect(err).To(HaveOccurred()) + + return fmt.Sprintf("Error: %v", err.Error()) + } else if err != nil { + Expect(err).To(MatchError(dryrun.ErrNonCompliant)) + + return normalizeDiffOutput(actualOutput) + } + + return actualOutput + }, 5, 1).Should(Equal(expectedOutput)) +} + +// findTestFiles finds all test files in the given test path +func findTestFiles(testPath string) (inputResourcePaths []string, desiredStatusPath string, outputPath string) { + files, err := os.ReadDir(testPath) + if err != nil { + return inputResourcePaths, desiredStatusPath, outputPath + } + + for _, file := range files { + name := file.Name() + fullPath := filepath.Join(testPath, name) + + // Handle input YAML files (skip namespace files) + if strings.HasPrefix(name, "input") && strings.HasSuffix(name, ".yaml") { + if !strings.HasPrefix(name, namespaceYamlPrefix) { + inputResourcePaths = append(inputResourcePaths, fullPath) + } + + continue + } + + // Handle output files (prefer cluster files when present) + switch name { + case errOutputFile, outputFromClusterFile: + outputPath = fullPath + case defaultOutputFile: + if outputPath == "" { + outputPath = fullPath + } + case desiredStatusFromClusterFile: + desiredStatusPath = fullPath + case desiredStatusFile: + if desiredStatusPath == "" { + desiredStatusPath = fullPath + } + } + } + + return inputResourcePaths, desiredStatusPath, outputPath +} + +// findInputNamespaces finds all namespace YAML files in the given root path +func findInputNamespaces(rootPath string) (nsFiles []string, err error) { + err = filepath.WalkDir(rootPath, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + + if !d.IsDir() && strings.HasPrefix(d.Name(), namespaceYamlPrefix) && strings.HasSuffix(d.Name(), ".yaml") { + nsFiles = append(nsFiles, path) + } + + return nil + }) + if err != nil { + return nil, err + } + + return nsFiles, nil +} + +// normalizeDiffOutput removes metadata fields that should not be +// compared with diff output +func normalizeDiffOutput(dryrunOutput string) string { + lines := strings.Split(dryrunOutput, "\n") + var result []string + + for _, line := range lines { + content := strings.TrimSpace(line) + shouldKeep := true + + for _, field := range untrackedMetadata { + if strings.Contains(content, field) { + shouldKeep = false + + break + } + } + + if shouldKeep { + result = append(result, line) + } + } + + return strings.Join(result, "\n") +} + +// containsManagedNamespace checks if a namespace YAML file contains any managed namespaces +func containsManagedNamespace(nsFilePath string) (bool, error) { + content, err := os.ReadFile(nsFilePath) + if err != nil { + return false, err + } + + for _, doc := range strings.Split(string(content), "---") { + if doc = strings.TrimSpace(doc); doc == "" { + continue + } + + var obj map[string]any + if yaml.Unmarshal([]byte(doc), &obj) != nil { + continue + } + + if obj["kind"] != "Namespace" { + continue + } + + metadata, ok := obj["metadata"].(map[string]any) + if !ok { + continue + } + + name, ok := metadata["name"].(string) + if ok && slices.Contains(persistentNamespaces, name) { + return true, nil + } + } + + return false, nil +} + +// findTestDirectories returns test directory names that contain required test files +func findTestDirectories(testPath string) ([]string, error) { + entries, err := os.ReadDir(testPath) + if err != nil { + return nil, err + } + + var testDirs []string + + for _, entry := range entries { + if entry.IsDir() { + if isTestDir, _ := isTestDirectory(filepath.Join(testPath, entry.Name())); isTestDir { + testDirs = append(testDirs, entry.Name()) + } + } + } + + return testDirs, nil +} + +// isTestDirectory checks if a directory contains policy.yaml and an output file +func isTestDirectory(dirPath string) (bool, error) { + entries, err := os.ReadDir(dirPath) + if err != nil { + return false, err + } + + hasPolicyYaml, hasOutputFile := false, false + + for _, entry := range entries { + if !entry.IsDir() { + name := entry.Name() + if name == policyYAML { + hasPolicyYaml = true + } + + if name == defaultOutputFile || name == outputFromClusterFile || name == errOutputFile { + hasOutputFile = true + } + + if hasPolicyYaml && hasOutputFile { + return true, nil // Early return when both found + } + } + } + + return hasPolicyYaml && hasOutputFile, nil +} From 6db1305dc795bb81351aea334226523e8622be88 Mon Sep 17 00:00:00 2001 From: Janelle Law Date: Fri, 3 Oct 2025 11:05:13 -0700 Subject: [PATCH 4/4] Remove ordering on e2e tests Tests dryrun scenarios in any order. Refactored namespace creation/deletion before and after each test Entry in DescribeTable. Eventually() will retry the dryrun command to wait for any duplicate resources pending deletion from previous test cases. ref: https://issues.redhat.com/browse/ACM-22932 Assisted by Cursor. Signed-off-by: Janelle Law --- .../{input.yaml => input_ns.yaml} | 0 .../context_vars/object_namespaced/input.yaml | 7 - .../object_namespaced/input_ns.yaml | 8 + .../object_pod_default_func/input.yaml | 5 - .../object_pod_default_func/input_ns.yaml | 6 + .../object_pod_nsselector/input.yaml | 5 - .../object_pod_nsselector/input_ns.yaml | 6 + .../object_templated_ns/input.yaml | 7 - .../object_templated_ns/input_ns.yaml | 8 + .../object_unnamed_objdef/input.yaml | 7 - .../object_unnamed_objdef/input_ns.yaml | 8 + .../objectname_nsselector/input.yaml | 15 - .../objectname_nsselector/input_ns.yaml | 16 + .../{input.yaml => input_ns.yaml} | 0 .../objectns_templated_empty/input.yaml | 10 - .../objectns_templated_empty/input_ns.yaml | 11 + .../input.yaml | 10 - .../input_ns.yaml | 11 + .../missing/missing_kind/input_ns_1.yaml | 17 + .../missing/missing_kind_name/input_ns_1.yaml | 17 + test/e2e/case46_dryrun_cli_test.go | 410 +++++++++--------- 21 files changed, 308 insertions(+), 276 deletions(-) rename test/dryrun/context_vars/object_cluster_scoped/{input.yaml => input_ns.yaml} (100%) create mode 100644 test/dryrun/context_vars/object_namespaced/input_ns.yaml create mode 100644 test/dryrun/context_vars/object_pod_default_func/input_ns.yaml create mode 100644 test/dryrun/context_vars/object_pod_nsselector/input_ns.yaml create mode 100644 test/dryrun/context_vars/object_templated_ns/input_ns.yaml create mode 100644 test/dryrun/context_vars/object_unnamed_objdef/input_ns.yaml create mode 100644 test/dryrun/context_vars/objectname_nsselector/input_ns.yaml rename test/dryrun/context_vars/objectns_cluster_scoped/{input.yaml => input_ns.yaml} (100%) create mode 100644 test/dryrun/context_vars/objectns_templated_empty/input_ns.yaml create mode 100644 test/dryrun/context_vars/objectns_templated_no_nsselector/input_ns.yaml create mode 100644 test/dryrun/missing/missing_kind/input_ns_1.yaml create mode 100644 test/dryrun/missing/missing_kind_name/input_ns_1.yaml diff --git a/test/dryrun/context_vars/object_cluster_scoped/input.yaml b/test/dryrun/context_vars/object_cluster_scoped/input_ns.yaml similarity index 100% rename from test/dryrun/context_vars/object_cluster_scoped/input.yaml rename to test/dryrun/context_vars/object_cluster_scoped/input_ns.yaml diff --git a/test/dryrun/context_vars/object_namespaced/input.yaml b/test/dryrun/context_vars/object_namespaced/input.yaml index 51fff337..eb5ff5aa 100644 --- a/test/dryrun/context_vars/object_namespaced/input.yaml +++ b/test/dryrun/context_vars/object_namespaced/input.yaml @@ -1,12 +1,5 @@ --- apiVersion: v1 -kind: Namespace -metadata: - name: mega-mart - labels: - box: big ---- -apiVersion: v1 kind: ConfigMap metadata: name: inventory diff --git a/test/dryrun/context_vars/object_namespaced/input_ns.yaml b/test/dryrun/context_vars/object_namespaced/input_ns.yaml new file mode 100644 index 00000000..8fa457d0 --- /dev/null +++ b/test/dryrun/context_vars/object_namespaced/input_ns.yaml @@ -0,0 +1,8 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: mega-mart + labels: + box: big + diff --git a/test/dryrun/context_vars/object_pod_default_func/input.yaml b/test/dryrun/context_vars/object_pod_default_func/input.yaml index ed526672..3f7c15cb 100644 --- a/test/dryrun/context_vars/object_pod_default_func/input.yaml +++ b/test/dryrun/context_vars/object_pod_default_func/input.yaml @@ -1,10 +1,5 @@ --- apiVersion: v1 -kind: Namespace -metadata: - name: dangler ---- -apiVersion: v1 kind: Pod metadata: name: nginx-pod diff --git a/test/dryrun/context_vars/object_pod_default_func/input_ns.yaml b/test/dryrun/context_vars/object_pod_default_func/input_ns.yaml new file mode 100644 index 00000000..2d77ff4d --- /dev/null +++ b/test/dryrun/context_vars/object_pod_default_func/input_ns.yaml @@ -0,0 +1,6 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: dangler + diff --git a/test/dryrun/context_vars/object_pod_nsselector/input.yaml b/test/dryrun/context_vars/object_pod_nsselector/input.yaml index ed526672..3f7c15cb 100644 --- a/test/dryrun/context_vars/object_pod_nsselector/input.yaml +++ b/test/dryrun/context_vars/object_pod_nsselector/input.yaml @@ -1,10 +1,5 @@ --- apiVersion: v1 -kind: Namespace -metadata: - name: dangler ---- -apiVersion: v1 kind: Pod metadata: name: nginx-pod diff --git a/test/dryrun/context_vars/object_pod_nsselector/input_ns.yaml b/test/dryrun/context_vars/object_pod_nsselector/input_ns.yaml new file mode 100644 index 00000000..2d77ff4d --- /dev/null +++ b/test/dryrun/context_vars/object_pod_nsselector/input_ns.yaml @@ -0,0 +1,6 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: dangler + diff --git a/test/dryrun/context_vars/object_templated_ns/input.yaml b/test/dryrun/context_vars/object_templated_ns/input.yaml index 51fff337..eb5ff5aa 100644 --- a/test/dryrun/context_vars/object_templated_ns/input.yaml +++ b/test/dryrun/context_vars/object_templated_ns/input.yaml @@ -1,12 +1,5 @@ --- apiVersion: v1 -kind: Namespace -metadata: - name: mega-mart - labels: - box: big ---- -apiVersion: v1 kind: ConfigMap metadata: name: inventory diff --git a/test/dryrun/context_vars/object_templated_ns/input_ns.yaml b/test/dryrun/context_vars/object_templated_ns/input_ns.yaml new file mode 100644 index 00000000..8fa457d0 --- /dev/null +++ b/test/dryrun/context_vars/object_templated_ns/input_ns.yaml @@ -0,0 +1,8 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: mega-mart + labels: + box: big + diff --git a/test/dryrun/context_vars/object_unnamed_objdef/input.yaml b/test/dryrun/context_vars/object_unnamed_objdef/input.yaml index 51fff337..eb5ff5aa 100644 --- a/test/dryrun/context_vars/object_unnamed_objdef/input.yaml +++ b/test/dryrun/context_vars/object_unnamed_objdef/input.yaml @@ -1,12 +1,5 @@ --- apiVersion: v1 -kind: Namespace -metadata: - name: mega-mart - labels: - box: big ---- -apiVersion: v1 kind: ConfigMap metadata: name: inventory diff --git a/test/dryrun/context_vars/object_unnamed_objdef/input_ns.yaml b/test/dryrun/context_vars/object_unnamed_objdef/input_ns.yaml new file mode 100644 index 00000000..8fa457d0 --- /dev/null +++ b/test/dryrun/context_vars/object_unnamed_objdef/input_ns.yaml @@ -0,0 +1,8 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: mega-mart + labels: + box: big + diff --git a/test/dryrun/context_vars/objectname_nsselector/input.yaml b/test/dryrun/context_vars/objectname_nsselector/input.yaml index 969ec42f..68c001c4 100644 --- a/test/dryrun/context_vars/objectname_nsselector/input.yaml +++ b/test/dryrun/context_vars/objectname_nsselector/input.yaml @@ -1,20 +1,5 @@ --- apiVersion: v1 -kind: Namespace -metadata: - name: mega-mart ---- -apiVersion: v1 -kind: Namespace -metadata: - name: mega-mart-2 ---- -apiVersion: v1 -kind: Namespace -metadata: - name: mega-mart-3 ---- -apiVersion: v1 kind: ConfigMap metadata: name: inventory diff --git a/test/dryrun/context_vars/objectname_nsselector/input_ns.yaml b/test/dryrun/context_vars/objectname_nsselector/input_ns.yaml new file mode 100644 index 00000000..c22db20a --- /dev/null +++ b/test/dryrun/context_vars/objectname_nsselector/input_ns.yaml @@ -0,0 +1,16 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: mega-mart +--- +apiVersion: v1 +kind: Namespace +metadata: + name: mega-mart-2 +--- +apiVersion: v1 +kind: Namespace +metadata: + name: mega-mart-3 + diff --git a/test/dryrun/context_vars/objectns_cluster_scoped/input.yaml b/test/dryrun/context_vars/objectns_cluster_scoped/input_ns.yaml similarity index 100% rename from test/dryrun/context_vars/objectns_cluster_scoped/input.yaml rename to test/dryrun/context_vars/objectns_cluster_scoped/input_ns.yaml diff --git a/test/dryrun/context_vars/objectns_templated_empty/input.yaml b/test/dryrun/context_vars/objectns_templated_empty/input.yaml index 0ed619bc..f7364fc4 100644 --- a/test/dryrun/context_vars/objectns_templated_empty/input.yaml +++ b/test/dryrun/context_vars/objectns_templated_empty/input.yaml @@ -1,15 +1,5 @@ --- apiVersion: v1 -kind: Namespace -metadata: - name: my-namespace ---- -apiVersion: v1 -kind: Namespace -metadata: - name: my-other-namespace ---- -apiVersion: v1 kind: ConfigMap metadata: name: templated-ns-configmap diff --git a/test/dryrun/context_vars/objectns_templated_empty/input_ns.yaml b/test/dryrun/context_vars/objectns_templated_empty/input_ns.yaml new file mode 100644 index 00000000..a0c9268d --- /dev/null +++ b/test/dryrun/context_vars/objectns_templated_empty/input_ns.yaml @@ -0,0 +1,11 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: my-namespace +--- +apiVersion: v1 +kind: Namespace +metadata: + name: my-other-namespace + diff --git a/test/dryrun/context_vars/objectns_templated_no_nsselector/input.yaml b/test/dryrun/context_vars/objectns_templated_no_nsselector/input.yaml index 0ed619bc..f7364fc4 100644 --- a/test/dryrun/context_vars/objectns_templated_no_nsselector/input.yaml +++ b/test/dryrun/context_vars/objectns_templated_no_nsselector/input.yaml @@ -1,15 +1,5 @@ --- apiVersion: v1 -kind: Namespace -metadata: - name: my-namespace ---- -apiVersion: v1 -kind: Namespace -metadata: - name: my-other-namespace ---- -apiVersion: v1 kind: ConfigMap metadata: name: templated-ns-configmap diff --git a/test/dryrun/context_vars/objectns_templated_no_nsselector/input_ns.yaml b/test/dryrun/context_vars/objectns_templated_no_nsselector/input_ns.yaml new file mode 100644 index 00000000..a0c9268d --- /dev/null +++ b/test/dryrun/context_vars/objectns_templated_no_nsselector/input_ns.yaml @@ -0,0 +1,11 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: my-namespace +--- +apiVersion: v1 +kind: Namespace +metadata: + name: my-other-namespace + diff --git a/test/dryrun/missing/missing_kind/input_ns_1.yaml b/test/dryrun/missing/missing_kind/input_ns_1.yaml new file mode 100644 index 00000000..62db25cf --- /dev/null +++ b/test/dryrun/missing/missing_kind/input_ns_1.yaml @@ -0,0 +1,17 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: n1 +spec: {} +--- +apiVersion: v1 +kind: Namespace +metadata: + name: n2 +spec: {} +--- +apiVersion: v1 +kind: Namespace +metadata: + name: n3 +spec: {} diff --git a/test/dryrun/missing/missing_kind_name/input_ns_1.yaml b/test/dryrun/missing/missing_kind_name/input_ns_1.yaml new file mode 100644 index 00000000..62db25cf --- /dev/null +++ b/test/dryrun/missing/missing_kind_name/input_ns_1.yaml @@ -0,0 +1,17 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: n1 +spec: {} +--- +apiVersion: v1 +kind: Namespace +metadata: + name: n2 +spec: {} +--- +apiVersion: v1 +kind: Namespace +metadata: + name: n3 +spec: {} diff --git a/test/e2e/case46_dryrun_cli_test.go b/test/e2e/case46_dryrun_cli_test.go index a49ccbfd..61155ae1 100644 --- a/test/e2e/case46_dryrun_cli_test.go +++ b/test/e2e/case46_dryrun_cli_test.go @@ -5,6 +5,7 @@ package e2e import ( "bytes" + "errors" "fmt" "os" "path/filepath" @@ -19,213 +20,249 @@ import ( "open-cluster-management.io/config-policy-controller/test/utils" ) -const ( - case46TestDataPath = "../dryrun/" - policyYAML = "policy.yaml" - resourceYamlPrefix = "input" - namespaceYamlPrefix = "input_ns" - errOutputFile = "error.txt" - defaultOutputFile = "output.txt" - outputFromClusterFile = "output_from_cluster.txt" - desiredStatusFile = "desired_status.yaml" - desiredStatusFromClusterFile = "desired_status_from_cluster.yaml" -) - -// persistentNamespaces are namespaces that should not be deleted after this test case -// because they persist across test suites or are system namespaces -var persistentNamespaces = []string{ - "managed", - "default", +type dryrunTestFiles struct { + testPath string + inputNamespaces []string + inputResources []string + desiredStatusPath string + outputPath string } -var untrackedMetadata = []string{ - "resourceVersion:", - "generatedName:", - "creationTimestamp:", - "deletionTimestamp:", - "selfLink:", - "uid:", - "metadata.kubernetes.io/metadata.name:", - "kubernetes.io/metadata.name:", -} +var _ = Describe("Testing dryrun CLI", Serial, func() { + const case46TestDataPath = "../dryrun/" -var _ = Describe("Testing dryrun CLI", Ordered, func() { - var testNamespaces []string + testFilesCache, err := findDryrunTestFiles(case46TestDataPath) + Expect(err).ToNot(HaveOccurred()) - BeforeAll(func() { - // Setup test namespaces before creating any resources - testNamespaces, err := findInputNamespaces(case46TestDataPath) - Expect(err).ToNot(HaveOccurred()) + describeTableArgs := []any{func(testPath string) { + files := testFilesCache[testPath] - for _, nsFilePath := range testNamespaces { - utils.Kubectl("apply", "-f", nsFilePath) - } - }) + DeferCleanup(func() { + for _, resource := range files.inputResources { + By("Deleting input file " + resource) + utils.KubectlDelete("-f", resource, "--wait") + } - AfterAll(func() { - for _, nsFilePath := range testNamespaces { - if shouldDelete, err := containsManagedNamespace(nsFilePath); shouldDelete && err == nil { + for _, nsFilePath := range files.inputNamespaces { + isManagedNs, err := containsManagedNamespace(nsFilePath) + if isManagedNs || err != nil { + continue + } + + By("Deleting namespace from " + nsFilePath) utils.KubectlDelete("-f", nsFilePath, "--wait") } - } - }) + }) - testScenarios := []struct { - description string - path string - }{ - {"Should handle context variables", "context_vars"}, - {"Should handle diffs", "diff"}, - {"Should handle pod annotations", "kind_field"}, - {"Should handle missing fields", "missing"}, - {"Should handle multiple objects", "multiple"}, - {"Should handle no name objects", "no_name"}, - {"Should handle object selector scenarios", "no_name/with_object_selector"}, - {"Should handle cluster-scoped no name", "no_name_clusterscope"}, - {"Should handle no namespace", "no_ns"}, - {"Should handle namespace selector", "ns_selector"}, - {"Should handle object template raw", "obj_tmpl_raw"}, - } + Eventually(func(g Gomega) { + for _, nsFilePath := range files.inputNamespaces { + isManagedNs, err := containsManagedNamespace(nsFilePath) + if isManagedNs || err != nil { + continue + } - for _, scenario := range testScenarios { - It(scenario.description, func() { - testDryRunScenarios(scenario.path) - }) + By("Creating namespace from " + nsFilePath) + utils.Kubectl("apply", "-f", nsFilePath) + } + + for _, resource := range files.inputResources { + By("Applying input file " + resource) + utils.Kubectl("apply", "-f", resource) + } + + verifyDryrunOutput(g, files) + }, defaultTimeoutSeconds, 1).Should(Succeed()) + }} + + // Generate Entry items dynamically for each test + for testPath := range testFilesCache { + relPath := strings.TrimPrefix(testPath, case46TestDataPath) + relPath = strings.TrimPrefix(relPath, "/") + describeTableArgs = append(describeTableArgs, Entry("Should handle "+relPath, testPath)) } + + DescribeTable("When reading cluster resources with dryrun CLI", describeTableArgs...) }) -// testDryRunScenarios runs all test scenarios in a given test category -func testDryRunScenarios(testCategory string) { - testDirs, err := findTestDirectories(filepath.Join(case46TestDataPath, testCategory)) - Expect(err).ToNot(HaveOccurred()) +// findDryrunTestFiles discovers all test directories and their files in a single traversal +func findDryrunTestFiles(rootPath string) (map[string]dryrunTestFiles, error) { + const ( + policyYAML = "policy.yaml" + errOutputFile = "error.txt" + namespaceYamlPrefix = "input_ns" + defaultOutputFile = "output.txt" + outputFromClusterFile = "output_from_cluster.txt" + desiredStatusFile = "desired_status.yaml" + desiredStatusFromClusterFile = "desired_status_from_cluster.yaml" + ) + + result := make(map[string]dryrunTestFiles) + + err := filepath.WalkDir(rootPath, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } - for _, testDir := range testDirs { - By("Testing " + testCategory + "/" + testDir) - testDryrunCommand(testCategory, testDir) - } -} + // Skip if policy file not found + if d.IsDir() || d.Name() != policyYAML { + return nil + } -// testDryrunCommand runs a dryrun test scenario -func testDryrunCommand(testCategory, testDir string) { - testPath := filepath.Join(case46TestDataPath, testCategory, testDir) - inputResourcePaths, desiredStatusPath, outputPath := findTestFiles(testPath) + testDir := filepath.Dir(path) - for _, resource := range inputResourcePaths { - By("Applying input file " + resource) - utils.Kubectl("apply", "-f", resource) - } + files, err := os.ReadDir(testDir) + if err != nil { + return err + } - defer func() { - for _, resource := range inputResourcePaths { - By("Deleting input file " + resource) - utils.KubectlDelete("-f", resource, "--wait") + var inputNamespaces []string + var inputResources []string + var desiredStatusPath string + var outputPath string + + // Categorize all files in the test directory + for _, file := range files { + name := file.Name() + fullPath := filepath.Join(testDir, name) + + // Handle input YAML files + if strings.HasPrefix(name, "input") && strings.HasSuffix(name, ".yaml") { + if strings.HasPrefix(name, namespaceYamlPrefix) { + inputNamespaces = append(inputNamespaces, fullPath) + } else { + inputResources = append(inputResources, fullPath) + } + + continue + } + + // Handle output files (prefer cluster files when present) + switch name { + case errOutputFile, outputFromClusterFile: + outputPath = fullPath + case defaultOutputFile: + if outputPath == "" { + outputPath = fullPath + } + case desiredStatusFromClusterFile: + desiredStatusPath = fullPath + case desiredStatusFile: + if desiredStatusPath == "" { + desiredStatusPath = fullPath + } + } } - }() - expectedBytes, err := os.ReadFile(outputPath) - Expect(err).ToNot(HaveOccurred()) + result[testDir] = dryrunTestFiles{ + testPath: testDir, + inputNamespaces: inputNamespaces, + inputResources: inputResources, + desiredStatusPath: desiredStatusPath, + outputPath: outputPath, + } + + return nil + }) + if err != nil { + return nil, err + } + + return result, nil +} + +// verifyDryrunOutput executes the dryrun command and compares actual vs expected output +func verifyDryrunOutput(g Gomega, files dryrunTestFiles) { + GinkgoHelper() + + const ( + policyYAML = "policy.yaml" + errOutputFile = "error.txt" + ) + + expectedBytes, err := os.ReadFile(files.outputPath) + g.Expect(err).ToNot(HaveOccurred()) expectedOutput := string(expectedBytes) - shouldFail := strings.HasSuffix(outputPath, errOutputFile) + wantedErr := filepath.Base(files.outputPath) == errOutputFile - if shouldFail { + if wantedErr { // Match dry run error whitespace formatting for error files expectedOutput = strings.TrimSpace(strings.ReplaceAll(expectedOutput, "\n", " ")) } By("Running dryrun command") - Eventually(func() string { - var output bytes.Buffer - cmd := (&dryrun.DryRunner{}).GetCmd() - cmd.SetOut(&output) - - args := []string{"--from-cluster", "--policy", filepath.Join(testPath, policyYAML), "--no-colors"} - if desiredStatusPath != "" { - args = append(args, "--desired-status", desiredStatusPath) - } + var output bytes.Buffer - cmd.SetArgs(args) - err := cmd.Execute() - actualOutput := output.String() + cmd := (&dryrun.DryRunner{}).GetCmd() + cmd.SetOut(&output) - if shouldFail { - Expect(err).To(HaveOccurred()) - - return fmt.Sprintf("Error: %v", err.Error()) - } else if err != nil { - Expect(err).To(MatchError(dryrun.ErrNonCompliant)) - - return normalizeDiffOutput(actualOutput) - } - - return actualOutput - }, 5, 1).Should(Equal(expectedOutput)) -} - -// findTestFiles finds all test files in the given test path -func findTestFiles(testPath string) (inputResourcePaths []string, desiredStatusPath string, outputPath string) { - files, err := os.ReadDir(testPath) - if err != nil { - return inputResourcePaths, desiredStatusPath, outputPath + args := []string{"--from-cluster", "--policy", filepath.Join(files.testPath, policyYAML), "--no-colors"} + if files.desiredStatusPath != "" { + args = append(args, "--desired-status", files.desiredStatusPath) } - for _, file := range files { - name := file.Name() - fullPath := filepath.Join(testPath, name) + cmd.SetArgs(args) + err = cmd.Execute() + actualOutput := output.String() - // Handle input YAML files (skip namespace files) - if strings.HasPrefix(name, "input") && strings.HasSuffix(name, ".yaml") { - if !strings.HasPrefix(name, namespaceYamlPrefix) { - inputResourcePaths = append(inputResourcePaths, fullPath) - } + if wantedErr { + g.Expect(err).To(HaveOccurred()) - continue - } + actualOutput = fmt.Sprintf("Error: %v", err.Error()) + } else if err != nil { + g.Expect(err).To(MatchError(dryrun.ErrNonCompliant)) - // Handle output files (prefer cluster files when present) - switch name { - case errOutputFile, outputFromClusterFile: - outputPath = fullPath - case defaultOutputFile: - if outputPath == "" { - outputPath = fullPath - } - case desiredStatusFromClusterFile: - desiredStatusPath = fullPath - case desiredStatusFile: - if desiredStatusPath == "" { - desiredStatusPath = fullPath - } - } + actualOutput = normalizeDiffOutput(actualOutput) } - return inputResourcePaths, desiredStatusPath, outputPath + g.Expect(actualOutput).To(Equal(expectedOutput)) } -// findInputNamespaces finds all namespace YAML files in the given root path -func findInputNamespaces(rootPath string) (nsFiles []string, err error) { - err = filepath.WalkDir(rootPath, func(path string, d os.DirEntry, err error) error { - if err != nil { - return err - } +// containsManagedNamespace checks if a namespace YAML file contains any managed namespaces +func containsManagedNamespace(nsFilePath string) (bool, error) { + // managedNamespaces are namespaces that should not be deleted after this test case + // because they persist across test suites or are system namespaces + managedNamespaces := []string{ + "managed", + "default", + } + + hasPersistent := false + err := listNamespacesInFile(nsFilePath, func(name string) error { + if slices.Contains(managedNamespaces, name) { + hasPersistent = true - if !d.IsDir() && strings.HasPrefix(d.Name(), namespaceYamlPrefix) && strings.HasSuffix(d.Name(), ".yaml") { - nsFiles = append(nsFiles, path) + return errors.New("found persistent namespace") } return nil }) - if err != nil { - return nil, err + + // Ignore the early exit error + if err != nil && hasPersistent { + return true, nil } - return nsFiles, nil + return hasPersistent, err } // normalizeDiffOutput removes metadata fields that should not be // compared with diff output func normalizeDiffOutput(dryrunOutput string) string { + untrackedMetadata := []string{ + "resourceVersion:", + "generatedName:", + "creationTimestamp:", + "deletionTimestamp:", + "selfLink:", + "uid:", + "metadata.kubernetes.io/metadata.name:", + "kubernetes.io/metadata.name:", + "deletionGracePeriodSeconds:", + } + lines := strings.Split(dryrunOutput, "\n") + var result []string for _, line := range lines { @@ -248,11 +285,11 @@ func normalizeDiffOutput(dryrunOutput string) string { return strings.Join(result, "\n") } -// containsManagedNamespace checks if a namespace YAML file contains any managed namespaces -func containsManagedNamespace(nsFilePath string) (bool, error) { +// listNamespacesInFile reads a YAML file and calls the provided function for each namespace found +func listNamespacesInFile(nsFilePath string, fn func(name string) error) error { content, err := os.ReadFile(nsFilePath) if err != nil { - return false, err + return err } for _, doc := range strings.Split(string(content), "---") { @@ -275,59 +312,12 @@ func containsManagedNamespace(nsFilePath string) (bool, error) { } name, ok := metadata["name"].(string) - if ok && slices.Contains(persistentNamespaces, name) { - return true, nil - } - } - - return false, nil -} - -// findTestDirectories returns test directory names that contain required test files -func findTestDirectories(testPath string) ([]string, error) { - entries, err := os.ReadDir(testPath) - if err != nil { - return nil, err - } - - var testDirs []string - - for _, entry := range entries { - if entry.IsDir() { - if isTestDir, _ := isTestDirectory(filepath.Join(testPath, entry.Name())); isTestDir { - testDirs = append(testDirs, entry.Name()) - } - } - } - - return testDirs, nil -} - -// isTestDirectory checks if a directory contains policy.yaml and an output file -func isTestDirectory(dirPath string) (bool, error) { - entries, err := os.ReadDir(dirPath) - if err != nil { - return false, err - } - - hasPolicyYaml, hasOutputFile := false, false - - for _, entry := range entries { - if !entry.IsDir() { - name := entry.Name() - if name == policyYAML { - hasPolicyYaml = true - } - - if name == defaultOutputFile || name == outputFromClusterFile || name == errOutputFile { - hasOutputFile = true - } - - if hasPolicyYaml && hasOutputFile { - return true, nil // Early return when both found + if ok { + if err := fn(name); err != nil { + return err } } } - return hasPolicyYaml && hasOutputFile, nil + return nil }