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 (33)
📒 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 | ✅ 11✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 40.68% 40.69%
=======================================
Files 755 755
Lines 93368 93373 +5
=======================================
+ Hits 37985 37994 +9
+ Misses 52649 52646 -3
+ Partials 2734 2733 -1 see 15 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 |
enxebre
force-pushed
the
enxebre/fix-azure-private-topology-cel
branch
from
59e049c to
9cafe87
Compare
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 |
|
/label tide/merge-method-squash |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… 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>
enxebre
force-pushed
the
enxebre/fix-azure-private-topology-cel
branch
from
ca317d6 to
8716def
Compare
openshift-ci-robot
removed
the
verified
openshift-ci
Bot
removed
lgtm
|
@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. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/test e2e-aks |
|
/lgtm yolo |
|
/lgtm |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
/pipeline required |
|
Scheduling tests matching the |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
/retest |
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. |
|
Now I have all the evidence needed. Let me produce the final report: Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThis is a pre-existing flaky test unrelated to PR #8490. The PR exclusively changes Azure CEL validation rules in Root CauseThe root cause is a transient infrastructure issue causing one
Recommendations
Evidence
|
|
@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. |
7 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