OCPCLOUD-3384, OCPCLOUD-3365: Implement admission action Warn and ensure validations not active when not configured#492
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRefactors validators to accept CompatibilityRequirement objects, adds owner-reference checks and enablement guards for webhooks, implements action-driven Warn/Deny behavior for validation and pruning, expands warning-mode tests, and adds CRD/test helpers plus a REST WarningHandler utility. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
@JoelSpeed: This pull request references OCPCLOUD-3384 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 task to target the "4.22.0" version, but no target version was set. This pull request references OCPCLOUD-3365 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 task to target the "4.22.0" version, but no target version was set. 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. |
JoelSpeed
force-pushed
the
compatibility-admit-warn
branch
from
18f2100 to
4478e45
Compare
|
@JoelSpeed: This pull request references OCPCLOUD-3384 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 task to target the "4.22.0" version, but no target version was set. This pull request references OCPCLOUD-3365 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 task to target the "4.22.0" version, but no target version was set. 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
removed
the
do-not-merge/work-in-progress
|
@JoelSpeed: This pull request references OCPCLOUD-3384 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 task to target the "4.22.0" version, but no target version was set. This pull request references OCPCLOUD-3365 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 task to target the "4.22.0" version, but no target version was set. 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/controllers/crdcompatibility/reconcile.go (1)
232-249:⚠️ Potential issue | 🔴 CriticalPrevent deletion of non-owned
ValidatingWebhookConfigurationobjects.At Line 247, deletion is unconditional once the object is found. Combined with Line 202-205 (disable path), this can remove a same-named webhook not controlled by this
CompatibilityRequirement.🔒 Proposed fix
func (r *reconcileState) removeObjectValidationWebhook(ctx context.Context, obj *apiextensionsv1alpha1.CompatibilityRequirement) error { webhookConfig := &admissionregistrationv1.ValidatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ Name: obj.Name, }, } @@ if err := r.client.Get(ctx, types.NamespacedName{Name: webhookConfig.Name}, webhookConfig); err != nil { if apierrors.IsNotFound(err) { return nil } return fmt.Errorf("failed to get ValidatingWebhookConfiguration %s: %w", webhookConfig.Name, err) } + + // If we don't own the webhook config, we should not be deleting it. + if !metav1.IsControlledBy(webhookConfig, obj) { + return nil + } if err := r.client.Delete(ctx, webhookConfig); err != nil { return fmt.Errorf("failed to delete ValidatingWebhookConfiguration %s: %w", webhookConfig.Name, err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/crdcompatibility/reconcile.go` around lines 232 - 249, The deletion is unconditional in removeObjectValidationWebhook which can remove a same-named ValidatingWebhookConfiguration not owned by this CompatibilityRequirement; before calling r.client.Delete check ownership (use metav1.IsControlledBy or controllerutil.ContainsReference/IsOwned helper) to ensure webhookConfig is owned/controlled by the obj (CompatibilityRequirement) and only delete if true—if not owned, skip deletion and return nil (or a clear no-op) to avoid deleting resources owned by others.
🧹 Nitpick comments (2)
go.mod (1)
19-29: Alignrequireversions withreplacepins to reduce dependency drift.The
replacedirectives pin Kubernetes modules to v0.35.2/v1.35.2, but severalrequireentries lag behind:
- Line 55:
k8s.io/apiextensions-apiserver v0.34.1→ should bev0.35.2- Line 61:
k8s.io/kubernetes v1.34.1→ should bev1.35.2- Line 351:
k8s.io/controller-manager v0.32.1(indirect) → should bev0.35.2- Line 354:
k8s.io/kubelet v0.34.1(indirect) → should bev0.35.2This drift may cause future
go mod tidychurn and makes the intended dependency versions less clear.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 19 - 29, Update the go.mod require entries so they match the pinned versions in the replace directives: change k8s.io/apiextensions-apiserver to v0.35.2, k8s.io/kubernetes to v1.35.2, and ensure indirect requires for k8s.io/controller-manager and k8s.io/kubelet are v0.35.2; run go get <module>@<version> or edit the require block to the listed versions and then run go mod tidy to verify there is no drift between require and replace pins.pkg/controllers/crdcompatibility/reconcile_test.go (1)
381-382: Use the typed NotFound matcher here.Matching
"not found"couples the test to error text and can pass for the wrong failure.test.BeK8SNotFound()is already used in the deletion specs above and keeps these assertions tied to the actual API status.♻️ Suggested cleanup
- Expect(kWithCtx(ctx).Get(noObjectSchemaValidatingWebhook)()).To(MatchError(ContainSubstring("not found"))) - Expect(kWithCtx(ctx).Get(noObjectSchemaMutatingWebhook)()).To(MatchError(ContainSubstring("not found"))) + Expect(kWithCtx(ctx).Get(noObjectSchemaValidatingWebhook)()).To(test.BeK8SNotFound()) + Expect(kWithCtx(ctx).Get(noObjectSchemaMutatingWebhook)()).To(test.BeK8SNotFound()) … - Eventually(kWithCtx(ctx).Get(noObjectSchemaValidatingWebhook)).WithContext(ctx).Should(MatchError(ContainSubstring("not found"))) - Eventually(kWithCtx(ctx).Get(noObjectSchemaMutatingWebhook)).WithContext(ctx).Should(MatchError(ContainSubstring("not found"))) + Eventually(kWithCtx(ctx).Get(noObjectSchemaValidatingWebhook)).WithContext(ctx).Should(test.BeK8SNotFound()) + Eventually(kWithCtx(ctx).Get(noObjectSchemaMutatingWebhook)).WithContext(ctx).Should(test.BeK8SNotFound())Also applies to: 399-400
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/crdcompatibility/reconcile_test.go` around lines 381 - 382, Replace the fragile string-based error assertions with the typed NotFound matcher: change the two expectations using MatchError(ContainSubstring("not found")) for kWithCtx(ctx).Get(noObjectSchemaValidatingWebhook)() and kWithCtx(ctx).Get(noObjectSchemaMutatingWebhook)() to use test.BeK8SNotFound() instead; also make the same replacement for the identical assertions noted elsewhere in the file (the other occurrence around the deletion specs) so the tests assert Kubernetes NotFound status rather than matching error text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/controllers/crdcompatibility/objectpruning/webhook.go`:
- Around line 119-125: The handler currently only returns early when
ObjectSchemaValidation.Action == CRDAdmitActionWarn, but when
ObjectSchemaValidation is nil the zero-value falls through and causes pruning;
update the early-exit logic in the webhook handler (around
compatibilityRequirement.Spec.ObjectSchemaValidation.Action and the call to
v.getStructuralSchema) to treat unset/cleared validation as disabled by
returning nil unless the action is explicitly
apiextensionsv1alpha1.CRDAdmitActionDeny — i.e., check if
Spec.ObjectSchemaValidation is nil or its Action != CRDAdmitActionDeny and
return nil to keep the admission inert during reconciliation windows.
In `@pkg/controllers/crdcompatibility/objectvalidation/handle_test.go`:
- Around line 853-860: The BeforeEach mutates the package-global cl and relies
on the implicit global scheme; instead create an explicit scheme and avoid
changing global state: build a fresh scheme (e.g. runtime.NewScheme()) and
explicitly register the needed API types (the same apiextensionsv1alpha1 types
registered in pkg/test/envtest.go) and pass that scheme into client.New via a
copied rest.Config (warningConfig), then create a local client variable (e.g.
testCl) from client.New rather than assigning to the package-global cl; if other
helpers require the global, save the oldCl before creating testCl and restore
oldCl in AfterEach (and ensure any client cleanup runs even on panic) so tests
remain isolated.
In `@pkg/controllers/crdcompatibility/reconcile.go`:
- Around line 314-317: The isObjectValidationWebhookEnabled function currently
treats a non-nil MatchConditions slice as enabled even if it's empty; change the
check from osl.MatchConditions != nil to len(osv.MatchConditions) > 0 so an
explicitly empty list is considered not configured, i.e., update the return
expression in isObjectValidationWebhookEnabled to use len(osv.MatchConditions) >
0 (keeping the existing checks for osv.Action and the namespace/object selector
helpers).
---
Outside diff comments:
In `@pkg/controllers/crdcompatibility/reconcile.go`:
- Around line 232-249: The deletion is unconditional in
removeObjectValidationWebhook which can remove a same-named
ValidatingWebhookConfiguration not owned by this CompatibilityRequirement;
before calling r.client.Delete check ownership (use metav1.IsControlledBy or
controllerutil.ContainsReference/IsOwned helper) to ensure webhookConfig is
owned/controlled by the obj (CompatibilityRequirement) and only delete if
true—if not owned, skip deletion and return nil (or a clear no-op) to avoid
deleting resources owned by others.
---
Nitpick comments:
In `@go.mod`:
- Around line 19-29: Update the go.mod require entries so they match the pinned
versions in the replace directives: change k8s.io/apiextensions-apiserver to
v0.35.2, k8s.io/kubernetes to v1.35.2, and ensure indirect requires for
k8s.io/controller-manager and k8s.io/kubelet are v0.35.2; run go get
<module>@<version> or edit the require block to the listed versions and then run
go mod tidy to verify there is no drift between require and replace pins.
In `@pkg/controllers/crdcompatibility/reconcile_test.go`:
- Around line 381-382: Replace the fragile string-based error assertions with
the typed NotFound matcher: change the two expectations using
MatchError(ContainSubstring("not found")) for
kWithCtx(ctx).Get(noObjectSchemaValidatingWebhook)() and
kWithCtx(ctx).Get(noObjectSchemaMutatingWebhook)() to use test.BeK8SNotFound()
instead; also make the same replacement for the identical assertions noted
elsewhere in the file (the other occurrence around the deletion specs) so the
tests assert Kubernetes NotFound status rather than matching error text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 01a51f56-b065-4956-b2f5-83eb8e268ea7
⛔ Files ignored due to path filters (285)
e2e/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workmanifests-gen/go.sumis excluded by!**/*.sumvendor/github.com/gogo/protobuf/sortkeys/sortkeys.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/.ci-operator.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/AGENTS.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/Dockerfile.ocpis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apiextensions/v1alpha1/types_compatibilityrequirement.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/apps/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/authorization/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/build/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/cloudnetwork/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_networks.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.crd-manifests/0000_10_openshift-controller-manager_01_builds.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_pki.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/features.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/image/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/network/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/network/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/networkoperator/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/networkoperator/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/oauth/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/oauth/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/openapi/generated_openapi/zz_generated.openapi.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_dns_00_dnses.crd.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/project/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/project/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/quota/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/quota/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/route/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/route/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/samples/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/samples/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/security/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/security/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/template/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/template/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/user/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/openshift/api/user/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/chain.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/constantdelay.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/cron.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/logger.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/option.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/parser.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/robfig/cron/v3/spec.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/retry_interceptor.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/admission/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1beta1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1beta1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1beta1/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admission/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/admissionregistration/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/admissionregistration/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1alpha1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1alpha1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1alpha1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1alpha1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/admissionregistration/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1beta1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1beta1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/admissionregistration/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/apidiscovery/v2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apidiscovery/v2/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apidiscovery/v2/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apidiscovery/v2/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/apidiscovery/v2beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apidiscovery/v2beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apidiscovery/v2beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apidiscovery/v2beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/apiserverinternal/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apiserverinternal/v1alpha1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apiserverinternal/v1alpha1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apiserverinternal/v1alpha1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/apps/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/apps/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta1/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/apps/v1beta2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta2/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta2/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta2/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta2/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta2/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/apps/v1beta2/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/authentication/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/authentication/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/authentication/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/authentication/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/authentication/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/authentication/v1alpha1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/authentication/v1alpha1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/authentication/v1alpha1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/authentication/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/authentication/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/authentication/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/authentication/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/authorization/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/authorization/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/authorization/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/authorization/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/authorization/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/authorization/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/authorization/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/authorization/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/autoscaling/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/autoscaling/v2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/autoscaling/v2beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/autoscaling/v2beta2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2beta2/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2beta2/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/autoscaling/v2beta2/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/batch/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/batch/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/batch/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/batch/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/batch/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/batch/v1/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/batch/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/batch/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/batch/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/batch/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/batch/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/certificates/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/certificates/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1alpha1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1alpha1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1alpha1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1alpha1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1alpha1/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/certificates/v1alpha1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/certificates/v1alpha1/zz_generated.prerelease-lifecycle.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/certificates/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1beta1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1beta1/register.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1beta1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1beta1/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/certificates/v1beta1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/certificates/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/certificates/v1beta1/zz_generated.prerelease-lifecycle.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/coordination/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/coordination/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/coordination/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/coordination/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/coordination/v1alpha2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/coordination/v1alpha2/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/coordination/v1alpha2/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/coordination/v1alpha2/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/coordination/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/coordination/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/coordination/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/coordination/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/core/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/core/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/core/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/core/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/core/v1/toleration.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/core/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/core/v1/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/core/v1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/core/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/discovery/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/discovery/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1beta1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1beta1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1beta1/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/discovery/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/events/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/events/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/events/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/events/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/events/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/events/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/events/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/events/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/extensions/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/extensions/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/extensions/v1beta1/generated.protois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/extensions/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/extensions/v1beta1/types.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/extensions/v1beta1/types_swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/extensions/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/extensions/v1beta1/zz_generated.validations.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/flowcontrol/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/flowcontrol/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/flowcontrol/v1beta2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1beta2/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1beta2/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1beta2/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/flowcontrol/v1beta3/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1beta3/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1beta3/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/flowcontrol/v1beta3/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/imagepolicy/v1alpha1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/imagepolicy/v1alpha1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/imagepolicy/v1alpha1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/imagepolicy/v1alpha1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/networking/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/networking/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/networking/v1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/networking/v1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/networking/v1beta1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/networking/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/networking/v1beta1/generated.protomessage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/k8s.io/api/networking/v1beta1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/k8s.io/api/node/v1/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/api/node/v1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**
📒 Files selected for processing (15)
e2e/go.modgo.modhack/tools/go.modmanifests-gen/go.modpkg/controllers/crdcompatibility/objectpruning/handle_test.gopkg/controllers/crdcompatibility/objectpruning/validator_unit_test.gopkg/controllers/crdcompatibility/objectpruning/webhook.gopkg/controllers/crdcompatibility/objectvalidation/handle_test.gopkg/controllers/crdcompatibility/objectvalidation/suite_test.gopkg/controllers/crdcompatibility/objectvalidation/validator_unit_test.gopkg/controllers/crdcompatibility/objectvalidation/webhook.gopkg/controllers/crdcompatibility/reconcile.gopkg/controllers/crdcompatibility/reconcile_test.gopkg/test/crdbuilder.gopkg/test/warninghandler.go
|
@JoelSpeed: This pull request references OCPCLOUD-3384 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 task to target the "4.22.0" version, but no target version was set. This pull request references OCPCLOUD-3365 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 task to target the "4.22.0" version, but no target version was set. 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. |
| if err := r.client.Get(ctx, types.NamespacedName{Name: webhookConfig.Name}, existingWebhookConfig); err != nil && !apierrors.IsNotFound(err) { | ||
| return fmt.Errorf("failed to get ValidatingWebhookConfiguration %s: %w", webhookConfig.Name, err) | ||
| } else if err == nil && !metav1.IsControlledBy(existingWebhookConfig, obj) { | ||
| return fmt.Errorf("%w: %s", errWebhookConfigNotControlledByCompatibilityRequirement, webhookConfig.Name) |
| retrievedObj.SetName(inputObject.GetName()) | ||
| retrievedObj.SetNamespace(inputObject.GetNamespace()) | ||
|
|
||
| Eventually(kWithCtx(ctx).Get(retrievedObj)).WithContext(ctx).Should(Succeed()) |
There was a problem hiding this comment.
I thought we were doing timeouts too, now?
There was a problem hiding this comment.
I haven't swept through to make that update yet, this is fine when the tests pass, but messy when the tests fail, planning to do a sweep that's mechanical afterwards once I work out a good pattern
| scenario.CompatibilityRequirement.Spec.ObjectSchemaValidation.Action = apiextensionsv1alpha1.CRDAdmitActionWarn | ||
| })).WithContext(ctx).Should(Succeed()) | ||
|
|
||
| By("Updating the object again, should not be pruned") |
There was a problem hiding this comment.
I feel like you need to wait here until the CR has a success condition with observedGeneration >= the return from your Update?
There was a problem hiding this comment.
I don't think there's a need to. The webhook loads the CR from the cache on every request and checks whether the action is set to warn or deny. We aren't reliant on the controller to make any changes to reflect that change
openshift-merge-robot
added
the
needs-rebase
JoelSpeed
force-pushed
the
compatibility-admit-warn
branch
from
fa03d1b to
c54005f
Compare
openshift-merge-robot
removed
the
needs-rebase
|
@JoelSpeed: This pull request references OCPCLOUD-3384 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 task to target the "4.22.0" version, but no target version was set. This pull request references OCPCLOUD-3365 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 task to target the "4.22.0" version, but no target version was set. 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. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/controllers/crdcompatibility/objectvalidation/webhook.go`:
- Around line 162-189: In ValidateUpdate (webhook.go) add the same short-circuit
that ValidateCreate uses when object admission is no longer enabled: after
loading the CompatibilityRequirement and before calling
getValidationStrategy/ValidateUpdate, detect the removed/empty admit action
(compatibilityRequirement.Spec.ObjectSchemaValidation.Action == "" or equivalent
"admission disabled" condition) and return nil, nil (or return empty warnings,
nil) to ignore stale webhook calls instead of falling through to the default
branch that errors; update any references to
strategy.ValidateUpdate/WarningsOnUpdate to only run when admission is enabled.
In `@pkg/controllers/crdcompatibility/reconcile.go`:
- Around line 209-214: The check that returns an error when an existing
ValidatingWebhookConfiguration/MutatingWebhookConfiguration
(existingWebhookConfig) is not owned by the CompatibilityRequirement (obj)
should instead attempt a migration/adoption: after r.client.Get finds an
existing resource and metav1.IsControlledBy(existingWebhookConfig, obj) is
false, try to adopt it by setting the CompatibilityRequirement as controller
(use controllerutil.SetControllerReference or construct an OwnerReference) and
patch/update the existingWebhookConfig via r.client.Patch or r.client.Update,
handling conflicts/retries; only return
errWebhookConfigNotControlledByCompatibilityRequirement if the patch/update
fails or owner adoption is rejected. Apply the same adoption logic to the
analogous check for MutatingWebhookConfiguration around the other block
referenced (lines 263-268).
- Around line 202-204: The current disabled/unconfigured path calls
removeObjectValidationWebhook which can delete cluster-scoped
ValidatingWebhookConfigurations that this controller does not own; update the
logic so we only remove webhooks that this controller created/owns: modify
removeObjectValidationWebhook to fetch the ValidatingWebhookConfiguration and
verify ownership (e.g., ownerReference matching the compatibility CR's UID or a
controller-specific label/annotation set when creating the webhook) before
deleting, and adjust the reconcile branch around
isObjectValidationWebhookEnabled/compatibilityCRD to skip deletion when there is
no compatibility CR present; use the symbols isObjectValidationWebhookEnabled,
removeObjectValidationWebhook, and compatibilityCRD to locate and change the
code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 0067868a-73bb-491f-98b3-a31f570b610c
📒 Files selected for processing (12)
pkg/controllers/crdcompatibility/objectpruning/handle_test.gopkg/controllers/crdcompatibility/objectpruning/suite_test.gopkg/controllers/crdcompatibility/objectpruning/validator_unit_test.gopkg/controllers/crdcompatibility/objectpruning/webhook.gopkg/controllers/crdcompatibility/objectvalidation/handle_test.gopkg/controllers/crdcompatibility/objectvalidation/suite_test.gopkg/controllers/crdcompatibility/objectvalidation/validator_unit_test.gopkg/controllers/crdcompatibility/objectvalidation/webhook.gopkg/controllers/crdcompatibility/reconcile.gopkg/controllers/crdcompatibility/reconcile_test.gopkg/test/crdbuilder.gopkg/test/warninghandler.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/test/crdbuilder.go
| existingWebhookConfig := &admissionregistrationv1.ValidatingWebhookConfiguration{} | ||
| if err := r.client.Get(ctx, types.NamespacedName{Name: webhookConfig.Name}, existingWebhookConfig); err != nil && !apierrors.IsNotFound(err) { | ||
| return fmt.Errorf("failed to get ValidatingWebhookConfiguration %s: %w", webhookConfig.Name, err) | ||
| } else if err == nil && !metav1.IsControlledBy(existingWebhookConfig, obj) { | ||
| return fmt.Errorf("%w: %s", errWebhookConfigNotControlledByCompatibilityRequirement, webhookConfig.Name) | ||
| } |
There was a problem hiding this comment.
Add a migration path for existing webhook configs before enforcing ownership.
These checks only accept configs that already have a controller ref to the CompatibilityRequirement. Because this PR also starts adding that owner ref to generated configs, any same-name VWC/MWC already present during upgrade will terminal-error here instead of being adopted or patched, which can wedge reconciliation until manual cleanup.
Also applies to: 263-268
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/controllers/crdcompatibility/reconcile.go` around lines 209 - 214, The
check that returns an error when an existing
ValidatingWebhookConfiguration/MutatingWebhookConfiguration
(existingWebhookConfig) is not owned by the CompatibilityRequirement (obj)
should instead attempt a migration/adoption: after r.client.Get finds an
existing resource and metav1.IsControlledBy(existingWebhookConfig, obj) is
false, try to adopt it by setting the CompatibilityRequirement as controller
(use controllerutil.SetControllerReference or construct an OwnerReference) and
patch/update the existingWebhookConfig via r.client.Patch or r.client.Update,
handling conflicts/retries; only return
errWebhookConfigNotControlledByCompatibilityRequirement if the patch/update
fails or owner adoption is rejected. Apply the same adoption logic to the
analogous check for MutatingWebhookConfiguration around the other block
referenced (lines 263-268).
There was a problem hiding this comment.
This isn't required, as the feature has not been released.
There was a problem hiding this comment.
@mdbooth, understood — since this feature hasn't shipped yet, there are no pre-existing webhook configurations without owner refs that could be encountered during upgrade. The concern doesn't apply here.
JoelSpeed
force-pushed
the
compatibility-admit-warn
branch
from
c54005f to
0a75757
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
pkg/controllers/crdcompatibility/reconcile.go (1)
209-214:⚠️ Potential issue | 🟠 MajorAdd upgrade adoption path before hard-failing on webhook ownership.
Line 212 and Line 271 still terminal-error on same-name webhook configs lacking owner refs. That can block reconciliation after upgrade from legacy configs until manual intervention.
Suggested direction
@@ - } else if err == nil && !metav1.IsControlledBy(existingWebhookConfig, obj) { - return fmt.Errorf("%w: %s", errWebhookConfigNotControlledByCompatibilityRequirement, webhookConfig.Name) + } else if err == nil && !metav1.IsControlledBy(existingWebhookConfig, obj) { + // Legacy migration path: adopt only unowned configs; keep failing for foreign-owned configs. + if len(existingWebhookConfig.OwnerReferences) > 0 { + return fmt.Errorf("%w: %s", errWebhookConfigNotControlledByCompatibilityRequirement, webhookConfig.Name) + } + existingWebhookConfig.OwnerReferences = []metav1.OwnerReference{ + *metav1.NewControllerRef(obj, apiextensionsv1alpha1.GroupVersion.WithKind("CompatibilityRequirement")), + } + if err := r.client.Update(ctx, existingWebhookConfig); err != nil { + return fmt.Errorf("failed to adopt ValidatingWebhookConfiguration %s: %w", webhookConfig.Name, err) + } } @@ - } else if err == nil && !metav1.IsControlledBy(existingWebhookConfig, obj) { - return fmt.Errorf("%w: %s", errWebhookConfigNotControlledByCompatibilityRequirement, webhookConfig.Name) + } else if err == nil && !metav1.IsControlledBy(existingWebhookConfig, obj) { + if len(existingWebhookConfig.OwnerReferences) > 0 { + return fmt.Errorf("%w: %s", errWebhookConfigNotControlledByCompatibilityRequirement, webhookConfig.Name) + } + existingWebhookConfig.OwnerReferences = []metav1.OwnerReference{ + *metav1.NewControllerRef(obj, apiextensionsv1alpha1.GroupVersion.WithKind("CompatibilityRequirement")), + } + if err := r.client.Update(ctx, existingWebhookConfig); err != nil { + return fmt.Errorf("failed to adopt MutatingWebhookConfiguration %s: %w", webhookConfig.Name, err) + } }Also applies to: 268-273
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/crdcompatibility/reconcile.go` around lines 209 - 214, The current logic in reconcile.go returns a hard error when an existing ValidatingWebhookConfiguration with the same name is found but not controlled by the CompatibilityRequirement (check uses metav1.IsControlledBy on existingWebhookConfig and returns errWebhookConfigNotControlledByCompatibilityRequirement); instead implement an upgrade/adoption path: if r.client.Get finds an existing webhook and metav1.IsControlledBy(...) is false, attempt to adopt it by adding the CompatibilityRequirement as an owner reference to existingWebhookConfig and updating it via r.client.Update (or Patch) while handling conflicts/retries, and only return the ownership error if the adopt/update fails; apply the same change to the equivalent block around lines 268-273 that checks MutatingWebhookConfiguration ownership.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/controllers/crdcompatibility/reconcile.go`:
- Around line 209-214: The current logic in reconcile.go returns a hard error
when an existing ValidatingWebhookConfiguration with the same name is found but
not controlled by the CompatibilityRequirement (check uses metav1.IsControlledBy
on existingWebhookConfig and returns
errWebhookConfigNotControlledByCompatibilityRequirement); instead implement an
upgrade/adoption path: if r.client.Get finds an existing webhook and
metav1.IsControlledBy(...) is false, attempt to adopt it by adding the
CompatibilityRequirement as an owner reference to existingWebhookConfig and
updating it via r.client.Update (or Patch) while handling conflicts/retries, and
only return the ownership error if the adopt/update fails; apply the same change
to the equivalent block around lines 268-273 that checks
MutatingWebhookConfiguration ownership.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 1edcbf9a-908b-4ccf-91ea-1fabffeb280c
📒 Files selected for processing (12)
pkg/controllers/crdcompatibility/objectpruning/handle_test.gopkg/controllers/crdcompatibility/objectpruning/suite_test.gopkg/controllers/crdcompatibility/objectpruning/validator_unit_test.gopkg/controllers/crdcompatibility/objectpruning/webhook.gopkg/controllers/crdcompatibility/objectvalidation/handle_test.gopkg/controllers/crdcompatibility/objectvalidation/suite_test.gopkg/controllers/crdcompatibility/objectvalidation/validator_unit_test.gopkg/controllers/crdcompatibility/objectvalidation/webhook.gopkg/controllers/crdcompatibility/reconcile.gopkg/controllers/crdcompatibility/reconcile_test.gopkg/test/crdbuilder.gopkg/test/warninghandler.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/controllers/crdcompatibility/objectpruning/suite_test.go
- pkg/controllers/crdcompatibility/objectpruning/webhook.go
|
/verified by @sunzhaohua2 |
openshift-ci-robot
added
the
verified
|
@JoelSpeed: 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. |
JoelSpeed
force-pushed
the
compatibility-admit-warn
branch
from
0a75757 to
f7ea34d
Compare
openshift-ci-robot
removed
the
verified
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/controllers/crdcompatibility/reconcile.go (1)
275-275: Stale comment should be removed or relocated.The comment "If we don't own the webhook config, we should not be overwriting it." appears after the ownership check (lines 271-273), making it misleading since the check already happened above.
🔧 Suggested fix
- // If we don't own the webhook config, we should not be overwriting it. if _, _, err := resourceapply.ApplyMutatingWebhookConfigurationImproved(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/crdcompatibility/reconcile.go` at line 275, The comment "If we don't own the webhook config, we should not be overwriting it." is stale because the ownership check is already performed just above it; remove or relocate that comment in pkg/controllers/crdcompatibility/reconcile.go (inside the reconcile/Reconcile function) so it no longer appears after the ownership check, or update it to reflect the actual behavior (e.g., "ownership was checked above; skipping overwrite if not owned"). Ensure the comment string is either deleted or moved/rewritten to precede the ownership check or accurately describe the post-check path.pkg/controllers/crdcompatibility/objectpruning/webhook.go (1)
303-310: Consider extracting duplicate helper functions to a shared package.
isObjectValidationWebhookEnabledandlabelSelectorIsEmptyare duplicated in:
pkg/controllers/crdcompatibility/reconcile.gopkg/controllers/crdcompatibility/objectpruning/webhook.gopkg/controllers/crdcompatibility/objectvalidation/webhook.goThese could be consolidated into a shared utility package to reduce duplication.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/crdcompatibility/objectpruning/webhook.go` around lines 303 - 310, The isObjectValidationWebhookEnabled and labelSelectorIsEmpty helpers are duplicated; extract them into a shared utility (e.g., a new package like crdcompat/util or crdcompat/helpers), move the implementations for labelSelectorIsEmpty and isObjectValidationWebhookEnabled there, update callers in reconcile.go, objectpruning/webhook.go, and objectvalidation/webhook.go to import that package and call util.LabelSelectorIsEmpty and util.IsObjectValidationWebhookEnabled (or similar exported names), and remove the local duplicate implementations so compilation imports/uses the single shared functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/controllers/crdcompatibility/objectvalidation/handle_test.go`:
- Around line 908-911: The Eventually call wrapping kWithCtx(ctx).Update(...)
for liveCRD lacks the .WithContext(ctx) invocation used elsewhere, which can
prevent proper timeout/cancellation; update that call to use
Eventually(...).WithContext(ctx).Should(Succeed()) so the test uses the provided
ctx for timeout control—specifically modify the Eventually invocation that wraps
kWithCtx(ctx).Update(liveCRD, func() { ... }) to append .WithContext(ctx) before
.Should(Succeed()).
---
Nitpick comments:
In `@pkg/controllers/crdcompatibility/objectpruning/webhook.go`:
- Around line 303-310: The isObjectValidationWebhookEnabled and
labelSelectorIsEmpty helpers are duplicated; extract them into a shared utility
(e.g., a new package like crdcompat/util or crdcompat/helpers), move the
implementations for labelSelectorIsEmpty and isObjectValidationWebhookEnabled
there, update callers in reconcile.go, objectpruning/webhook.go, and
objectvalidation/webhook.go to import that package and call
util.LabelSelectorIsEmpty and util.IsObjectValidationWebhookEnabled (or similar
exported names), and remove the local duplicate implementations so compilation
imports/uses the single shared functions.
In `@pkg/controllers/crdcompatibility/reconcile.go`:
- Line 275: The comment "If we don't own the webhook config, we should not be
overwriting it." is stale because the ownership check is already performed just
above it; remove or relocate that comment in
pkg/controllers/crdcompatibility/reconcile.go (inside the reconcile/Reconcile
function) so it no longer appears after the ownership check, or update it to
reflect the actual behavior (e.g., "ownership was checked above; skipping
overwrite if not owned"). Ensure the comment string is either deleted or
moved/rewritten to precede the ownership check or accurately describe the
post-check path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: b448f091-3ee1-4869-9304-188afc92371e
📒 Files selected for processing (12)
pkg/controllers/crdcompatibility/objectpruning/handle_test.gopkg/controllers/crdcompatibility/objectpruning/suite_test.gopkg/controllers/crdcompatibility/objectpruning/validator_unit_test.gopkg/controllers/crdcompatibility/objectpruning/webhook.gopkg/controllers/crdcompatibility/objectvalidation/handle_test.gopkg/controllers/crdcompatibility/objectvalidation/suite_test.gopkg/controllers/crdcompatibility/objectvalidation/validator_unit_test.gopkg/controllers/crdcompatibility/objectvalidation/webhook.gopkg/controllers/crdcompatibility/reconcile.gopkg/controllers/crdcompatibility/reconcile_test.gopkg/test/crdbuilder.gopkg/test/warninghandler.go
✅ Files skipped from review due to trivial changes (2)
- pkg/test/crdbuilder.go
- pkg/test/warninghandler.go
🚧 Files skipped from review as they are similar to previous changes (5)
- pkg/controllers/crdcompatibility/objectvalidation/validator_unit_test.go
- pkg/controllers/crdcompatibility/objectvalidation/suite_test.go
- pkg/controllers/crdcompatibility/objectpruning/suite_test.go
- pkg/controllers/crdcompatibility/objectpruning/validator_unit_test.go
- pkg/controllers/crdcompatibility/reconcile_test.go
mdbooth
left a comment
mdbooth
left a comment
There was a problem hiding this comment.
A few comments inline. Nothing I'd block on this week.
I would appreciate the requested change to re-use duplicate test code. I find this kind of highly duplicated test very difficult to 'see' when trying to read the test. You can't transfer context from test to test because you're scanning to see if any lines are different.
We're also missing test coverage for:
- Setting owner references on webhooks
- Ownership conflict on webhooks
- Non-cleanup of non-owned webhooks
| } | ||
|
|
||
| By("Checking that the webhook configurations are not present") | ||
| Expect(kWithCtx(ctx).Get(noObjectSchemaValidatingWebhook)()).To(MatchError(ContainSubstring("not found"))) |
There was a problem hiding this comment.
nit: Discovered this recently and like it.
| Expect(kWithCtx(ctx).Get(noObjectSchemaValidatingWebhook)()).To(MatchError(ContainSubstring("not found"))) | |
| Expect(kWithCtx(ctx).Get(noObjectSchemaValidatingWebhook)()).To(Satisfy(apierrors.IsNotFound)) |
There was a problem hiding this comment.
What's the output when it does not satisfy?
There was a problem hiding this comment.
Expect(kWithCtx(ctx).Get(noObjectSchemaValidatingWebhook)()).NotTo(Satisfy(apierrors.IsNotFound), "not found")Gives:
[FAILED] not found
Expected
<*errors.StatusError | 0x2676b5f6b9a0>: validatingwebhookconfigurations.admissionregistration.k8s.io "test-requirement-bqhtq" not found
{
ErrStatus: {
TypeMeta: {Kind: "", APIVersion: ""},
ListMeta: {
SelfLink: "",
ResourceVersion: "",
Continue: "",
RemainingItemCount: nil,
},
Status: "Failure",
Message: "validatingwebhookconfigurations.admissionregistration.k8s.io \"test-requirement-bqhtq\" not found",
Reason: "NotFound", Details: { Name: "test-requirement-bqhtq", Group: "admissionregistration.k8s.io", Kind: "validatingwebhookconfigurations", UID: "", Causes: nil, RetryAfterSeconds: 0, }, Code: 404, }, } to not satisfy predicate <func(error) bool>: 0xa62c00 In [It] at: /home/mbooth/src/openshift/cluster-capi-operator/pkg/controllers/crdcompatibility/reconcile_test.go:402 @ 03/25/26 19:08:01.169
So... you have to remember to include an error message because the address of the predicate isn't that revealing. I guess there's not much in it.
I won't make this change this in your PR.
| requirementWithoutObjectSchemaValidation.Spec.ObjectSchemaValidation = requirement.Spec.ObjectSchemaValidation | ||
| })).WithContext(ctx).Should(Succeed()) | ||
|
|
||
| By("Checking that the webhook configurations are now present") |
There was a problem hiding this comment.
nit: My preference would be to wait on Admitted=true with the correct observedGeneration, then assert without Eventually that the webhooks are not present.
There was a problem hiding this comment.
Do we have an observed generation to observe for this test?
There was a problem hiding this comment.
We just updated the CR, so whatever we got back from that.
| return fmt.Errorf("failed to delete ValidatingWebhookConfiguration %s: %w", webhookConfig.Name, err) | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
nit: I don't think we're adding Finalizers so this is probably ok in practice. However, in general I'd prefer to hold here until the object is gone.
There was a problem hiding this comment.
Does that not add unnecessary delay when you're reconciling multiple objects? If the object still exists it will come back through the queue and then be re-delete-attempted later?
There was a problem hiding this comment.
It does delay, but it's a correct delay: Admitted is not set until the managed object is gone first. In practice the delay will be extremely short, almost instantaneous. Also we'll be reconciled again anyway.
| By("Attempting to update the object, should prune the object again", func() { | ||
| inputObject.Object["spec"] = scenario.InputObject["spec"] | ||
| Expect(cl.Update(ctx, inputObject)).To(Succeed()) | ||
| }) |
There was a problem hiding this comment.
Yeah probably a good idea to avoid mutation, good catch
| invalidObj := test.NewTestObject(gvk). | ||
| WithNamespace(namespace). | ||
| WithField("requiredField", "value"). | ||
| WithNestedField("spec.replicas", int64(150)). // Above maximum of 100 | ||
| Build() | ||
|
|
There was a problem hiding this comment.
Test object doesn't match the test description.
There was a problem hiding this comment.
Test description is wrong, the test case needs to test numeric bounds because the required/not required distinction isn't testable here
| }, defaultNodeTimeout) | ||
| }) | ||
|
|
||
| Context("when ObjectSchemaValidation Action is Warn for status subresource", func() { |
There was a problem hiding this comment.
nit: These tests are almost identical. I think they would be clearer as a table.
There was a problem hiding this comment.
I generally like tables, so sure, lets try
Fixes 2 discovered issues: * Ownership conflicts should be terminal * Conflict detection was missing on validation webhook delete path
mdbooth
force-pushed
the
compatibility-admit-warn
branch
from
cc66378 to
e93dc78
Compare
|
/approve |
|
Pipeline controller notification The |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mdbooth 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 |
openshift-ci
Bot
added
the
approved
|
/verified by CI |
openshift-ci-robot
added
the
verified
|
@mdbooth: 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. |
|
/pipeline required |
|
Scheduling required tests: Scheduling tests matching the |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
/override pull-ci-openshift-cluster-capi-operator-main-e2e-openstack-ovn-techpreview |
|
@mdbooth: Overrode contexts on behalf of mdbooth: ci/prow/e2e-openstack-ovn-techpreview 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 kubernetes-sigs/prow repository. |
|
/test e2e-aws-ovn |
|
/override pull-ci-openshift-cluster-capi-operator-main-e2e-openstack-ovn-techpreview |
|
@mdbooth: Overrode contexts on behalf of mdbooth: ci/prow/e2e-openstack-ovn-techpreview 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 kubernetes-sigs/prow repository. |
|
Looks unrelated, but the same unrelated thing twice. /test e2e-aws-ovn |
|
@JoelSpeed: The following test failed, say
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-merge-bot
Bot
merged commit 8f1b726
into
openshift:main
4 participants
This ensures that users configuring admission to warn do not have pruning or rejected writes for object admission
Also ensures that VWC/MWCs are removed when object admission is not configured