Skip to content

NO-JIRA: Fix continous reconciliation of VAPs due to server-side defaulting#581

Open
mdbooth wants to merge 1 commit into
openshift:mainfrom
openshift-cloud-team:vap-defaulting
Open

NO-JIRA: Fix continous reconciliation of VAPs due to server-side defaulting#581
mdbooth wants to merge 1 commit into
openshift:mainfrom
openshift-cloud-team:vap-defaulting

Conversation

@mdbooth

@mdbooth mdbooth commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

matchConstraints has several fields which have server-side defaults. If omitted, these trigger a continuous reconciliation of all managed objects because the spec doesn't match the manifest. There is unfortunately no good way to detect server-side defaulted values, so the easiest solution is to specify them explicitly.

This is a non-functional change. All the added values are just the defaults.

Summary by CodeRabbit

  • Configuration Updates
    • Improved consistency and standardized behavior for cluster resource validation policies across creation and update operations, ensuring more uniform policy application across namespaces and resources.

matchConstraints has several fields which have server-side defaults.
If omitted, these trigger a continuous reconciliation of all managed
objects because the spec doesn't match the manifest. There is
unfortunately no good way to detect server-side defaulted values, so the
easiest solution is to specify them explicitly.

This is a non-functional change. All the added values are just the
defaults.
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 5, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@mdbooth: This pull request explicitly references no jira issue.

Details

In response to this:

matchConstraints has several fields which have server-side defaults. If omitted, these trigger a continuous reconciliation of all managed objects because the spec doesn't match the manifest. There is unfortunately no good way to detect server-side defaulted values, so the easiest solution is to specify them explicitly.

This is a non-functional change. All the added values are just the defaults.

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.

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 4d160b06-582f-4caf-b0a7-3b3ec4b4a614

📥 Commits

Reviewing files that changed from the base of the PR and between 05c113e and e6dae93.

📒 Files selected for processing (15)
  • admission-policies/aws/unsupported-aws-spec-fields.yaml
  • admission-policies/default/authoritative-api-transition-requires-capi-infrastructure-ready.yaml
  • admission-policies/default/cluster-api-machine-set-vap.yaml
  • admission-policies/default/cluster-api-machine-vap.yaml
  • admission-policies/default/machine-api-machine-set-vap.yaml
  • admission-policies/default/machine-api-machine-vap.yaml
  • admission-policies/default/only-create-mapi-machine-if-authoritative-api-capi.yaml
  • admission-policies/default/prevent-authoritative-mapi-machineset-create-when-capi-exists.yaml
  • admission-policies/default/prevent-capi-fields-unsupported-by-mapi.yaml
  • admission-policies/default/prevent-migration-when-machine-updating.yaml
  • admission-policies/default/provide-warning-when-not-synchronized.yaml
  • admission-policies/default/validate-capi-machine-creation.yaml
  • admission-policies/default/validate-capi-machine-set-creation.yaml
  • capi-operator-manifests/aws/manifests.yaml
  • capi-operator-manifests/default/manifests.yaml

Walkthrough

Standardize admission policy matching behavior across 14 Kubernetes ValidatingAdmissionPolicy and ValidatingAdmissionPolicyBinding resources by adding explicit matchPolicy: Equivalent fields, normalizing namespace and object selector scoping, and adding scope: '*' to resource rules for consistent cluster-wide and namespace-level enforcement.

Changes

Admission policy matching standardization

Layer / File(s) Summary
AWS admission policy standardization
admission-policies/aws/unsupported-aws-spec-fields.yaml
Add matchPolicy: Equivalent to binding and policy; normalize to namespaceSelector: {} and objectSelector: {} in policy constraints while keeping resource rules scoped to awsmachines and awsmachinetemplates.
Default admission policies standardization
admission-policies/default/authoritative-api-transition-requires-capi-infrastructure-ready.yaml, cluster-api-machine-set-vap.yaml, cluster-api-machine-vap.yaml, machine-api-machine-set-vap.yaml, machine-api-machine-vap.yaml, only-create-mapi-machine-if-authoritative-api-capi.yaml, prevent-authoritative-mapi-machineset-create-when-capi-exists.yaml, prevent-capi-fields-unsupported-by-mapi.yaml, prevent-migration-when-machine-updating.yaml, provide-warning-when-not-synchronized.yaml, validate-capi-machine-creation.yaml, validate-capi-machine-set-creation.yaml
Add matchPolicy: Equivalent to 11 policy bindings and policies; set explicit objectSelector: {} in bindings; normalize policy constraints to namespaceSelector: {} and objectSelector: {}; add scope: '*' to resourceRules where needed to enforce across cluster and namespace scopes.
AWS operator manifests standardization
capi-operator-manifests/aws/manifests.yaml
Mirror AWS policy standardization within operator manifests: add matchPolicy: Equivalent, adjust binding selectors with matchExpressions for namespace narrowing and empty objectSelector, and add scope: '*' to validation resourceRules.
Default operator manifests standardization
capi-operator-manifests/default/manifests.yaml
Mirror default policies standardization within bundled operator manifests for 8 policies (machine-api, cluster-api machines/machinesets, prevent-capi-fields, only-create-mapi, prevent-migration, provide-warning, validate-capi, authoritative-api-transition): add matchPolicy: Equivalent across bindings and policies, normalize selectors to explicit configurations, and add scope: '*' to resourceRules.

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing continuous reconciliation of ValidatingAdmissionPolicies caused by server-side defaulting on matchConstraints fields.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed All newly added Ginkgo test names in this PR use stable, deterministic strings without dynamic content like pod names with suffixes, timestamps, UUIDs, or variable interpolation.
Test Structure And Quality ✅ Passed PR contains only YAML manifest files for Kubernetes admission policies; no Ginkgo test code was added or modified, making this check inapplicable.
Microshift Test Compatibility ✅ Passed This PR contains no new Ginkgo e2e tests. All changes are YAML admission policy and manifest configuration files only. The MicroShift compatibility check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed New e2e tests are protected via skipIfNoWorkerCAPIMachines() and make no multi-node assumptions like pod scheduling, anti-affinity, or node drain.
Topology-Aware Scheduling Compatibility ✅ Passed PR modifies only ValidatingAdmissionPolicy YAML files; no scheduling constraints, Deployments, or operator code are introduced.
Ote Binary Stdout Contract ✅ Passed Only code change is in BeforeEach() block within test file, not process-level code. No stdout writes detected. All other PR changes are YAML manifests and Makefile.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This PR contains only YAML configuration file changes (19 files, 0 Go test files). No new Ginkgo e2e tests are added, so the IPv6/disconnected network test compatibility check is not applicable.
No-Weak-Crypto ✅ Passed PR contains only YAML admission policy configuration changes, Makefile updates, and test refactoring. No weak cryptographic usage, custom crypto implementations, or secret comparison logic detected.
Container-Privileges ✅ Passed No container or Pod specifications present in PR changes. All modified files are ValidatingAdmissionPolicy/ValidatingAdmissionPolicyBinding YAML manifests with no privilege-related configurations.
No-Sensitive-Data-In-Logs ✅ Passed PR modifies only YAML admission policy configs to add explicit defaults. No new logging introduced; existing messages contain no sensitive data.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci openshift-ci Bot requested review from damdo and racheljpg June 5, 2026 15:02

@damdo damdo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve
/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 10, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-capi-disconnected-techpreview
/test e2e-aws-capi-techpreview
/test e2e-aws-capi-techpreview-post-install
/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

@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damdo

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 10, 2026
@damdo

damdo commented Jun 10, 2026

Copy link
Copy Markdown
Member

/retest

@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

@mdbooth: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-openstack-ovn-techpreview e6dae93 link false /test e2e-openstack-ovn-techpreview
ci/prow/e2e-azure-ovn-techpreview e6dae93 link false /test e2e-azure-ovn-techpreview
ci/prow/e2e-aws-ovn e6dae93 link true /test e2e-aws-ovn
ci/prow/e2e-vsphere-capi-techpreview e6dae93 link true /test e2e-vsphere-capi-techpreview

Full PR test history. Your PR dashboard.

Details

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants