OCPCLOUD-2640: Adds MachineSet VAP test suite

[theobarberbany]

• Adds test suite for MachineSet VAPs
• Adds missing tests for openshift-cluster-api-prevent-setting-of-capi-fields-unsupported-by-mapi
• Extracts some repeated logic to helper funcs, and updates code to use them.

Summary by CodeRabbit

• Tests
  • Added test utilities for validating admission policies, including a sentinel validation to block resources by label and helpers to scope policy bindings to target namespaces.
  • Added comprehensive E2E tests covering MachineSet validation enforcement, asserting forbidden-field errors and verifying allowed operations and cleanup.
  • Refactored existing tests to use centralized helpers for namespace and sentinel handling, improving maintainability and consistency.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[theobarberbany]

/test unit

[coderabbitai]

Walkthrough

Adds VAP test utilities (sentinel validation and namespace-updating helpers), a new MachineSet VAP E2E test exercising policy enforcement, refactors existing machine_sync tests to use the helpers, and promotes a module dependency from indirect to direct in go.mod.

Changes

Cohort / File(s)	Summary
VAP Test Utilities pkg/admissionpolicy/testutils/util.go	Adds SentinelValidationExpression constant (CEL expression), AddSentinelValidation(vap *admissionregistrationv1.ValidatingAdmissionPolicy) to append a sentinel Validation with a fixed message, and UpdateVAPBindingNamespaces(binding *admissionregistrationv1.ValidatingAdmissionPolicyBinding, paramNamespace, targetNamespace string) to set ParamRef.Namespace when present and update MatchResources.NamespaceSelector.
MachineSet VAP E2E Test pkg/controllers/machinesetsync/machineset_vap_test.go	Adds a comprehensive Go/Ginkgo E2E test that deploys VAP and binding, scopes binding namespaces, creates required CAPI/OpenShift resources, registers sentinel MachineSet resources, verifies allowed and forbidden create/update flows, and checks specific forbidden-field error messages.
Refactored Machine Sync Tests pkg/controllers/machinesync/machine_sync_controller_test.go	Replaces inline mutations with calls to admissiontestutils.AddSentinelValidation() and admissiontestutils.UpdateVAPBindingNamespaces() to centralize sentinel and namespace logic.
Module deps go.mod	Moves sigs.k8s.io/kube-storage-version-migrator v0.0.6-0.20230721195810-5c8923c5ff96 from indirect to a direct require.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Test Suite
    participant Helpers as testutils.Helpers
    participant VAP as ValidatingAdmissionPolicy
    participant Binding as ValidatingAdmissionPolicyBinding
    participant API as Kubernetes API

    Test->>Helpers: AddSentinelValidation(vap)
    activate Helpers
    Helpers->>VAP: Append Validation {expr: SentinelValidationExpression, message: fixed}
    Helpers-->>Test: done
    deactivate Helpers

    Test->>Helpers: UpdateVAPBindingNamespaces(binding, paramNs, targetNs)
    activate Helpers
    alt binding.ParamRef exists
        Helpers->>Binding: Set ParamRef.Namespace = paramNs
    end
    Helpers->>Binding: Update MatchResources.NamespaceSelector -> targetNs
    Helpers-->>Test: done
    deactivate Helpers

    Test->>API: Apply VAP and Binding
    Test->>API: Create/Update MachineSet (scenarios)
    API-->>Test: Admit or Reject (forbidden messages for .version/.readinessGates)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Pay attention to:
  • Exact semantics and correctness of the CEL SentinelValidationExpression.
  • Nil checks and mutation behavior in UpdateVAPBindingNamespaces (handling absent ParamRef, selectors).
  • Stability, teardown, and timing of the new E2E test (controller startup/shutdown, resource cleanup).
  • Error message matching in tests for forbidden-field checks.

Poem

🐰 I nibble code and nudge the rails,

Sentinel hops through testy trails.
Namespaces set with one small tweak,
MachineSets guarded, no fields sneak.
Hop—tests run green, I twitch my tail.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The pull request title 'ADDS MachineSet VAP test suite' directly and clearly aligns with the primary change in the changeset. The raw summary shows that the major addition is a comprehensive E2E test suite for MachineSet Validation Admission Policy (VAP) enforcement, and the title accurately reflects this core objective. The PR description confirms this is the main focus, stating 'Adds test suite for MachineSet VAPs.'

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between a395cf1 and f9c9684.

📒 Files selected for processing (3)

• pkg/admissionpolicy/testutils/util.go (2 hunks)
• pkg/controllers/machinesetsync/machineset_vap_test.go (1 hunks)
• pkg/controllers/machinesync/machine_sync_controller_test.go (9 hunks)

[nrb]

This is rather silly, but it looks like the vendor jobs are failing because sigs.k8s.io/kube-storage-version-migrator v0.0.6-0.20230721195810-5c8923c5ff96 in the go.mod has the // indirect comment removed.

Running make vendor, committing the go.mod, then running make verify-vendor locally fixed the failures for me.

[theobarberbany]

@nrb good catch! I saw this in vscode, and thought it was my local branch being borked. How did we get here?

Either way, thanks! :) I'll update

[chrischdi]

/lgtm

[openshift-ci-robot]

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-capi-techpreview
/test e2e-aws-ovn
/test e2e-aws-ovn-serial-1of2
/test e2e-aws-ovn-serial-2of2
/test e2e-aws-ovn-techpreview
/test e2e-aws-ovn-techpreview-upgrade
/test e2e-azure-capi-techpreview
/test e2e-azure-ovn-techpreview
/test e2e-azure-ovn-techpreview-upgrade
/test e2e-gcp-capi-techpreview
/test e2e-gcp-ovn-techpreview
/test e2e-metal3-capi-techpreview
/test e2e-openstack-capi-techpreview
/test e2e-openstack-ovn-techpreview
/test e2e-vsphere-capi-techpreview
/test regression-clusterinfra-aws-ipi-techpreview-capi

[sunzhaohua2]

/retest

[theobarberbany]

/retest

[theobarberbany]

/retest

[theobarberbany]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: theobarberbany

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [theobarberbany]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[theobarberbany]

/verified by e2es and unit tests

[openshift-ci-robot]

@theobarberbany: This PR has been marked as verified by e2es and unit tests.

In response to this:

/verified by e2es and unit tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@theobarberbany: This pull request references OCPCLOUD-2640 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

In response to this:

• Adds test suite for MachineSet VAPs
• Adds missing tests for openshift-cluster-api-prevent-setting-of-capi-fields-unsupported-by-mapi
• Extracts some repeated logic to helper funcs, and updates code to use them.

Summary by CodeRabbit

• Tests
• Added test utilities for validating admission policies, including a sentinel validation to block resources by label and helpers to scope policy bindings to target namespaces.
• Added comprehensive E2E tests covering MachineSet validation enforcement, asserting forbidden-field errors and verifying allowed operations and cleanup.
• Refactored existing tests to use centralized helpers for namespace and sentinel handling, improving maintainability and consistency.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

@theobarberbany: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.