NO-JIRA: Fix Azure private/topology CEL validation rules#8490
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@enxebre: This pull request explicitly references no jira issue. DetailsIn response to this:
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. |
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
do-not-merge/work-in-progress
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (34)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughKubebuilder XValidation rules were tightened for two Azure API types in 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
openshift-ci
Bot
added
area/api
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8490 +/- ##
==========================================
+ Coverage 39.85% 40.00% +0.14%
==========================================
Files 751 751
Lines 92556 92838 +282
==========================================
+ Hits 36888 37137 +249
- Misses 52994 53014 +20
- Partials 2674 2687 +13 see 27 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Scheduling tests matching the |
The existing CEL rule on AzurePlatformSpec allowed setting the private field even when topology was not set, because `!has(self.topology)` short-circuited to true. Fix the rule to correctly forbid private when topology is absent or Public. Also add a CEL rule on AzurePrivateSpec to require the privateLink struct when type is PrivateLink — previously only the negative constraint (forbid privateLink when type is not PrivateLink) existed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
enxebre
force-pushed
the
enxebre/fix-azure-private-topology-cel
branch
from
59e049c to
9cafe87
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
enxebre
force-pushed
the
enxebre/fix-azure-private-topology-cel
branch
from
9cafe87 to
723185b
Compare
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, enxebre The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| // | ||
| // +kubebuilder:validation:XValidation:rule="self.type != 'PrivateLink' ? !has(self.privateLink) : true",message="privateLink is forbidden when type is not PrivateLink" | ||
| // +kubebuilder:validation:XValidation:rule="self.type != 'PrivateLink' || has(self.privateLink)",message="privateLink is required when type is PrivateLink" | ||
| // +union |
There was a problem hiding this comment.
can't those 2 rules be combined?
// +kubebuilder:validation:XValidation:rule="(self.type == 'PrivateLink') == has(self.privateLink)",message="privateLink must be set if and only if type is PrivateLink"
There was a problem hiding this comment.
sgtm, @JoelSpeed is there any preference / convention for this? what's the impact on budget?
There was a problem hiding this comment.
The one we typically use for this is
// +kubebuilder:validation:XValidation:rule="self.type == 'PrivateLink' ? has(self.privateLink) : !has(self.privateLink)",message="privateLink is required when type is PrivateLink, and forbidden otherwise"
Test Resultse2e-aks
e2e-aws
|
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
… rule Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/label tide/merge-method-squash |
openshift-ci
Bot
added
the
tide/merge-method-squash
enxebre
removed
tide/merge-method-squash
|
/lgtm |
|
Scheduling tests matching the |
|
/lgtm |
|
/verified by e2e @enxebre |
openshift-ci-robot
added
the
verified
|
@enxebre: This PR has been marked as verified by DetailsIn response to this:
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. |
|
/retest |
|
@enxebre: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
openshift-ci
Bot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions 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. |
|
Now I have the complete picture. Here's the analysis: Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryNo CI tests actually failed — all Prow e2e and presubmit jobs passed successfully. The tide ERROR state is caused by a merge conflict between this PR's branch and Root CauseThe root cause is a git merge conflict between the PR branch and the Conflict origin: PR #8490 modifies
The Regarding the Konflux enterprise-contract failures: These are a separate, repo-wide issue — the same two checks ( Recommendations
Evidence
|
6 participants
Description
Fix two CEL validation gaps on Azure
AzurePlatformSpecandAzurePrivateSpec:privatewas settable withouttopology: The existing rule!has(self.topology) || (...)short-circuited totruewhentopologywas omitted, allowingprivateto be set withouttopology. Fixed tohas(self.topology) && (...) ? has(self.private) : !has(self.private)— now correctly forbidsprivatewhentopologyis absent orPublic.privateLinkstruct not required whentypeisPrivateLink: Only the negative constraint existed (forbidprivateLinkwhen type is not PrivateLink). Addedself.type != 'PrivateLink' || has(self.privateLink)to require the struct.Changes
api/hypershift/v1beta1/azure.go: Fix topology/private CEL rule, add privateLink requirement ruleprivateLinkstructTest plan
make test-envtest-ocp— all 630 specs passSummary by CodeRabbit