OCPCLOUD-3005,OCPCLOUD-3186,OCPCLOUD-3367: Implement validation webhook for custom resource admission#461
Open
JoelSpeed wants to merge 11 commits intoopenshift:mainfrom
Open
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
Contributor
|
Skipping CI for Draft Pull Request. |
|
@JoelSpeed: This pull request references OCPCLOUD-3005 which is a valid 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
|
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:
📝 WalkthroughWalkthroughAdds a webhook-based object validator for CompatibilityRequirements, wires it into manager startup, manages ValidatingWebhookConfiguration lifecycle in the reconciler, implements per-(requirement,version,generation) strategy caching/pruning and admission handling, and includes unit and end-to-end tests plus small test fixture updates. Changes
sequenceDiagram
participant APIServer as API Server
participant Manager as Manager
participant Validator as Validation Webhook
participant K8sClient as Kubernetes API
participant Strategy as Validation Strategy
APIServer->>Manager: Admission request (CREATE/UPDATE/DELETE)
Manager->>Validator: Forward request to Handle
Validator->>Validator: Extract CompatibilityRequirement name from path/context
Validator->>K8sClient: Get CompatibilityRequirement (schema YAML)
K8sClient-->>Validator: Return CompatibilityRequirement
Validator->>Strategy: Lookup or build per-(requirement,version,generation) strategy
alt Cache miss
Strategy->>Strategy: Parse CRD schema, build validators
Strategy-->>Validator: Return new strategy (cached)
else Cache hit
Strategy-->>Validator: Return cached strategy
end
Validator->>Strategy: Validate object (and old object for UPDATE)
alt Validation succeeds
Strategy-->>Validator: Optional warnings
Validator-->>APIServer: Admission Allowed (with warnings)
else Validation fails
Strategy-->>Validator: Validation errors
Validator-->>APIServer: Admission Denied (status error)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
JoelSpeed
changed the title
|
@JoelSpeed: This pull request references OCPCLOUD-3005 which is a valid jira issue. This pull request references OCPCLOUD-3186 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. This pull request references OCPCLOUD-3367 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. |
JoelSpeed
force-pushed
the
compatibility-object-admission
branch
from
08d1969 to
d564e4e
Compare
This was referenced Feb 16, 2026
openshift-merge-robot
added
the
needs-rebase
JoelSpeed
force-pushed
the
compatibility-object-admission
branch
from
d564e4e to
2daa19b
Compare
openshift-merge-robot
removed
the
needs-rebase
JoelSpeed
force-pushed
the
compatibility-object-admission
branch
2 times, most recently
from
07f96d2 to
958e566
Compare
|
@JoelSpeed: This pull request references OCPCLOUD-3005 which is a valid jira issue. This pull request references OCPCLOUD-3186 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. This pull request references OCPCLOUD-3367 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-3005 which is a valid jira issue. This pull request references OCPCLOUD-3186 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. This pull request references OCPCLOUD-3367 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: 7
🧹 Nitpick comments (1)
pkg/controllers/crdcompatibility/reconcile_test.go (1)
258-280: Assert the selector and match condition fields you configure in setup.Lines 228-245 configure
NamespaceSelector,ObjectSelector, andMatchConditions, but the assertion block does not verify them. This leaves a key behavior untested.🤖 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 258 - 280, The test currently asserts many webhook fields but omits the NamespaceSelector, ObjectSelector, and MatchConditions configured in setup; update the assertion on webhookConfig (the Eventually(kWithCtx(ctx).Object(webhookConfig)) block) to include HaveField checks for "NamespaceSelector", "ObjectSelector", and "MatchConditions" inside the Webhooks ConsistOf SatisfyAll so they verify the values you set in setup (compare against the selector/condition objects you created there — reference the same variables used in setup when asserting, and add them alongside the existing HaveField checks for Name/ClientConfig/Rules).
🤖 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 482-489: The test uses compatibilityRequirement.Name (creating
testPath) without initializing compatibilityRequirement, causing a hidden
cross-context dependency; fix by declaring and initializing a local test
object/name in this spec (e.g., set a local variable like reqName := "test-name"
or construct a local compatibilityRequirement with Name set) and use that local
value when building testPath and asserting against
compatibilityRequrementFromContext; update the lines that reference
compatibilityRequirement.Name to use the newly initialized local variable or
object so the test is self-contained and does not rely on outer contexts.
In `@pkg/controllers/crdcompatibility/objectvalidation/handle.go`:
- Around line 39-48: The function compatibilityRequrementFromContext currently
panics if the context key compatibilityRequirementContextKey{} is missing or the
value is not a string; change it to return (string, error) instead of panicking
(e.g., return "", fmt.Errorf("missing or invalid CompatibilityRequirement in
context: %T", v)) and update all callers of compatibilityRequrementFromContext
to handle the error and convert it into a proper admission failure response (no
panics) in the webhook request handling path.
In `@pkg/controllers/crdcompatibility/objectvalidation/webhook.go`:
- Around line 103-135: The current ValidateCreate and ValidateUpdate always
convert validation errors to apierrors.NewInvalid (deny); update both functions
to honor the requirement's action by checking the compatibility
requirement/action returned by getValidationStrategy (or the strategy object,
e.g., strategy.Action or requirement.Action) and: if the action is "Warn" return
the collected warnings and nil even when errs is non-empty, and if the action is
"Deny" continue returning apierrors.NewInvalid as now; keep using
strategy.Validate/ValidateUpdate and strategy.WarningsOnCreate/WarningsOnUpdate,
but branch on the action before converting errs into an admission error.
In `@pkg/controllers/crdcompatibility/reconcile_test.go`:
- Around line 226-249: Replace the dependency on the parent-level testCRDClean
by creating and using a locally-scoped CRD inside this BeforeAll: call the
appropriate helper to construct a fresh CRD (instead of referencing
testCRDClean) and pass that to test.GenerateTestCompatibilityRequirement so
requirement is initialized from a local variable; update references in this
BeforeAll to use that local CRD (e.g., localCRD / newCRD) before calling
createTestObject and leave all other fields (ObjectSchemaValidation, selectors,
MatchConditions) unchanged.
In `@pkg/controllers/crdcompatibility/reconcile.go`:
- Around line 230-286: The validatingWebhookConfigurationFor function builds a
ValidatingWebhookConfiguration but omits copying object-scoped selectors and
match conditions; update validatingWebhookConfigurationFor to set the
ValidatingWebhookConfiguration.Webhooks[*].NamespaceSelector,
Webhooks[*].ObjectSelector, and Webhooks[*].MatchConditions from the
CompatibilityRequirement.Spec.ObjectSchemaValidation (or the appropriate field
on obj that holds NamespaceSelector, ObjectSelector, and MatchConditions).
Locate where vwc.Webhooks[0] is constructed and after creating the webhook
assign vwc.Webhooks[0].NamespaceSelector =
obj.Spec.ObjectSchemaValidation.NamespaceSelector,
vwc.Webhooks[0].ObjectSelector = obj.Spec.ObjectSchemaValidation.ObjectSelector,
and vwc.Webhooks[0].MatchConditions =
obj.Spec.ObjectSchemaValidation.MatchConditions (ensuring types match and
handling nil safely) so namespace/object-scoped admission behavior is preserved.
- Around line 192-193: The code calls validatingWebhookConfigurationFor(obj,
r.compatibilityCRD) without ensuring r.compatibilityCRD is non-nil, which can
cause a panic; before building webhookConfig, add a nil-check for
r.compatibilityCRD (e.g., if r.compatibilityCRD == nil) and handle the condition
by returning an appropriate error or reconciling requeue, then only call
validatingWebhookConfigurationFor and proceed with the existing r.client.Get
using webhookConfig. Ensure you update the surrounding control flow so
webhookConfig is only constructed when r.compatibilityCRD is present.
- Around line 187-202: The reconcile in ensureObjectValidationWebhook is wrong:
stop returning early when isObjectValidationWebhookEnabled(obj) is false and
instead ensure previously-created resources are removed (call r.client.Get with
an empty apiextensionsv1alpha1.ValidatingWebhookConfiguration and
r.client.Delete if found); when reconciling the desired config use
validatingWebhookConfigurationFor(obj, r.compatibilityCRD) only as the desired
object and do not pass that same instance into r.client.Get (use a fresh empty
instance for Get/IsNotFound checks), and on create/update call r.client.Create
or r.client.Update with the desired webhookConfig as built.
---
Nitpick comments:
In `@pkg/controllers/crdcompatibility/reconcile_test.go`:
- Around line 258-280: The test currently asserts many webhook fields but omits
the NamespaceSelector, ObjectSelector, and MatchConditions configured in setup;
update the assertion on webhookConfig (the
Eventually(kWithCtx(ctx).Object(webhookConfig)) block) to include HaveField
checks for "NamespaceSelector", "ObjectSelector", and "MatchConditions" inside
the Webhooks ConsistOf SatisfyAll so they verify the values you set in setup
(compare against the selector/condition objects you created there — reference
the same variables used in setup when asserting, and add them alongside the
existing HaveField checks for Name/ClientConfig/Rules).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (290)
go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sumvendor/github.com/coreos/go-semver/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-semver/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-semver/semver/semver.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-semver/semver/sort.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/NOTICEis excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/daemon/sdnotify.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/daemon/watchdog.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal_unix.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/coreos/go-systemd/v22/journal/journal_windows.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/gogoproto/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/gogoproto/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/gogoproto/gogo.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/gogoproto/gogo.pb.goldenis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/gogoproto/gogo.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/gogoproto/helper.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/protoc-gen-gogo/descriptor/Makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/protoc-gen-gogo/descriptor/descriptor.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/protoc-gen-gogo/descriptor/descriptor.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/protoc-gen-gogo/descriptor/descriptor_gostring.gen.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/gogo/protobuf/protoc-gen-gogo/descriptor/helper.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/AUTHORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/CONTRIBUTORSis excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/buffer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/defaults.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/deprecated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/discard.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/extensions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/properties.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/proto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/registry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/text_decode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/text_encode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/wire.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/golang/protobuf/proto/wrappers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/.gitignoreis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/.travis.ymlis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/README.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/client_metrics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/client_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/makefileis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/metric_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/server.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/server_metrics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/server_reporter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/go-grpc-prometheus/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/BUILD.bazelis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/annotations_protoopaque.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/buf.gen.yamlis excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2.protois excluded by!**/vendor/**,!vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2_protoopaque.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/authpb/auth.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/authpb/auth.protois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/etcdserverpb/etcdserver.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/etcdserverpb/etcdserver.protois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/etcdserverpb/raft_internal.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/etcdserverpb/raft_internal.protois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/etcdserverpb/raft_internal_stringer.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/etcdserverpb/rpc.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/etcdserverpb/rpc.protois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/membershippb/membership.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/membershippb/membership.protois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/mvccpb/kv.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/mvccpb/kv.protois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/v3rpc/rpctypes/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/v3rpc/rpctypes/error.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/v3rpc/rpctypes/md.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/v3rpc/rpctypes/metadatafields.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/versionpb/version.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/api/v3/versionpb/version.protois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/dir_unix.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/dir_windows.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/filereader.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/fileutil.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/lock.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/lock_flock.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/lock_linux.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/lock_plan9.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/lock_solaris.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/lock_unix.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/lock_windows.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/preallocate.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/preallocate_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/preallocate_unix.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/preallocate_unsupported.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/purge.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/read_dir.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/sync.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/sync_darwin.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/fileutil/sync_linux.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/logutil/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/logutil/log_format.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/logutil/log_level.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/logutil/zap.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/logutil/zap_journal.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/systemd/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/systemd/journal.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/tlsutil/cipher_suites.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/tlsutil/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/tlsutil/tlsutil.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/tlsutil/versions.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/keepalive_listener.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/keepalive_listener_openbsd.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/keepalive_listener_unix.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/limit_listen.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/listener.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/listener_opts.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/listener_tls.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/sockopt.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/sockopt_solaris.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/sockopt_unix.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/sockopt_wasm.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/sockopt_windows.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/timeout_conn.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/timeout_dialer.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/timeout_listener.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/timeout_transport.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/tls.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/transport.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/transport/unix_listener.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/types/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/types/id.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/types/set.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/types/slice.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/types/urls.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/types/urlsmap.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/pkg/v3/verify/verify.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/auth.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/client.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/cluster.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/compact_op.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/compare.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/config.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/credentials/credentials.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/ctx.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/internal/endpoint/endpoint.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/internal/resolver/resolver.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/kubernetes/client.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/kubernetes/interface.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/kv.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/lease.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/logger.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/maintenance.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/op.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/options.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/retry.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/retry_interceptor.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/sort.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/txn.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/utils.gois excluded by!**/vendor/**,!vendor/**vendor/go.etcd.io/etcd/client/v3/watch.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/config.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/interceptorinfo.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/internal/parse.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/metadata_supplier.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.30.0/MIGRATION.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.30.0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.30.0/attribute_group.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.30.0/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.30.0/exception.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.30.0/metric.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.30.0/schema.gois excluded by!**/vendor/**,!vendor/**vendor/go.uber.org/zap/zapgrpc/zapgrpc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/cryptobyte/asn1.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/cryptobyte/asn1/asn1.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/cryptobyte/builder.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/crypto/cryptobyte/string.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/context/context.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/annotations/annotations.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/annotations/client.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/annotations/field_behavior.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/annotations/field_info.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/annotations/http.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/annotations/resource.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/annotations/routing.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/launch_stage.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/resolver/manual/manual.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install/install.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/metrics.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/nop_converter.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_discovery.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_discovery_controller.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/helpers.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype/validation.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/apiextensions/interface.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/apiextensions/v1/customresourcedefinition.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/apiextensions/v1/interface.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/apiextensions/v1beta1/customresourcedefinition.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/apiextensions/v1beta1/interface.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/factory.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/generic.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/internalinterfaces/factory_interfaces.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1/customresourcedefinition.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1/expansion_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1beta1/customresourcedefinition.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1beta1/expansion_generated.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/apiapproval/apiapproval_controller.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/establish/establishing_controller.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/finalizer/crd_finalizer.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/nonstructuralschema/nonstructuralschema_controller.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/merge.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/openapi/controller.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/openapi/metrics.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/openapi/v2/conversion.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/openapiv3/controller.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/openapiv3/metrics.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/openapiv3/util.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/controller/status/naming_controller.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/crdserverscheme/unstructured.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/generated/openapi/doc.gois excluded by!**/generated/**,!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/generated/openapi/zz_generated.openapi.gois excluded by!**/generated/**,!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/registry/customresource/status_strategy.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor/tableconvertor.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/api/meta/table/table.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/apis/meta/internalversion/validation/validation.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/apis/meta/v1beta1/validation/validation.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/duration/duration.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/waitgroup/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/waitgroup/ratelimited_waitgroup.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apimachinery/pkg/util/waitgroup/waitgroup.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/configuration/configuration_manager.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle/admission.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/accessor.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/compilation.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/dispatcher.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/errors.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/metrics/errors.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/metrics/metrics.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/plugin.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/reinvocationcontext.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/errors/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/errors/statuserror.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/reinvocationcontext.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/request/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/apis/apidiscovery/v2/conversion.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/apis/apidiscovery/v2/doc.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/apis/apidiscovery/v2/register.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap/default.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/endpoints/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/endpoints/deprecation/deprecation.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/endpoints/discovery/OWNERSis excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/endpoints/discovery/addresses.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/etag.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/fake.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/handler.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/metrics.gois excluded by!**/vendor/**,!vendor/**vendor/k8s.io/apiserver/pkg/endpoints/discovery/aggregated/negotiation.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (10)
cmd/crd-compatibility-checker/main.gogo.modpkg/controllers/crdcompatibility/objectvalidation/handle.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.go
pkg/controllers/crdcompatibility/objectvalidation/handle_test.go
Outdated
Show resolved
Hide resolved
JoelSpeed
force-pushed
the
compatibility-object-admission
branch
from
958e566 to
8252156
Compare
openshift-merge-robot
added
the
needs-rebase
|
@JoelSpeed: This pull request references OCPCLOUD-3005 which is a valid jira issue. This pull request references OCPCLOUD-3186 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. This pull request references OCPCLOUD-3367 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: 2
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)
170-185:⚠️ Potential issue | 🟠 MajorFinalizer removal before webhook cleanup risks orphaning the webhook config.
errors.Joinexecutes bothclearFinalizerandremoveObjectValidationWebhookregardless of individual outcomes. IfclearFinalizersucceeds butremoveObjectValidationWebhookfails, the finalizer is already gone and the object will be garbage-collected — the controller won't get another reconcile to retry webhook cleanup.Consider removing the webhook before clearing the finalizer:
Proposed fix
func (r *reconcileState) reconcileDelete(ctx context.Context, obj *apiextensionsv1alpha1.CompatibilityRequirement) (ctrl.Result, error) { logger := logf.FromContext(ctx) logger.Info("Reconciling CompatibilityRequirement deletion") - err := errors.Join( - clearFinalizer(ctx, r.client, obj), - r.removeObjectValidationWebhook(ctx, obj), - ) + if err := r.removeObjectValidationWebhook(ctx, obj); err != nil { + return ctrl.Result{}, err + } + + if err := clearFinalizer(ctx, r.client, obj); err != nil { + return ctrl.Result{}, err + } - if err != nil { - return ctrl.Result{}, err - } return ctrl.Result{}, nil }🤖 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 170 - 185, The deletion currently uses errors.Join to run clearFinalizer and removeObjectValidationWebhook in parallel which can remove the finalizer even if webhook cleanup fails; change reconcileDelete to call removeObjectValidationWebhook(ctx, obj) first and if it returns an error return that error (so the finalizer remains), and only after successful webhook removal call clearFinalizer(ctx, r.client, obj) and return any error from that; stop using errors.Join so the finalizer is only cleared after removeObjectValidationWebhook succeeds.
♻️ Duplicate comments (6)
pkg/controllers/crdcompatibility/reconcile.go (3)
156-161:⚠️ Potential issue | 🔴 Critical
errors.Joinchains sequential dependencies — later functions run even if earlier ones failed.
parseCompatibilityCRD,fetchCurrentCRD,checkCompatibilityRequirement, andensureObjectValidationWebhookare called viaerrors.Join, but they have data dependencies (r.compatibilityCRDset byparseCompatibilityCRDis used byensureObjectValidationWebhook). IfparseCompatibilityCRDfails,r.compatibilityCRDstays nil andensureObjectValidationWebhookwill panic invalidatingWebhookConfigurationFor.The nil-guard suggested in the past review comment on
r.compatibilityCRDwould help, but the root issue is usingerrors.Joinfor sequential-dependent steps.🤖 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 156 - 161, Replace the use of errors.Join (which runs all calls unconditionally) with a sequential error-check pattern so dependent steps run only if prior steps succeed: call r.parseCompatibilityCRD(obj) first and return on error, then call r.fetchCurrentCRD(ctx, logger) and return on error, then r.checkCompatibilityRequirement() and return on error, then r.ensureObjectValidationWebhook(ctx, obj) and return on error; also keep or add a nil-guard around r.compatibilityCRD (used by validatingWebhookConfigurationFor) to avoid panics if it ever becomes nil.
243-282:⚠️ Potential issue | 🟠 MajorWebhook config does not propagate
NamespaceSelector,ObjectSelector, orMatchConditionsfrom the spec.
validatingWebhookConfigurationForbuilds the webhook but never sets these fields fromobj.Spec.ObjectSchemaValidation. Users configuring namespace/object-scoped admission or match conditions will find them silently ignored.Proposed fix (add to the webhook definition)
Webhooks: []admissionregistrationv1.ValidatingWebhook{ { + NamespaceSelector: &obj.Spec.ObjectSchemaValidation.NamespaceSelector, + ObjectSelector: &obj.Spec.ObjectSchemaValidation.ObjectSelector, + MatchConditions: obj.Spec.ObjectSchemaValidation.MatchConditions, AdmissionReviewVersions: []string{"v1"},🤖 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 243 - 282, The ValidatingWebhookConfiguration built in validatingWebhookConfigurationFor is missing propagation of namespace/object selectors and match conditions from the CR's spec; update the ValidatingWebhook (inside validatingWebhookConfigurationFor) to set NamespaceSelector, ObjectSelector and MatchConditions using values from obj.Spec.ObjectSchemaValidation (copying those fields into the admissionregistrationv1.ValidatingWebhook fields), ensuring proper type conversion if needed so NamespaceSelector and ObjectSelector become *metav1.LabelSelector and MatchConditions map to admissionregistrationv1.MatchCondition slice on the webhook; keep the field names (NamespaceSelector, ObjectSelector, MatchConditions) consistent with admissionregistrationv1 API and read from obj.Spec.ObjectSchemaValidation to preserve user intent.
187-210:⚠️ Potential issue | 🔴 Critical
Getoverwrites the desired webhook config, makingUpdatea no-op.
webhookConfigis built with the desired state on line 192, butr.client.Geton line 193 overwrites it with the server's current state. The subsequentUpdateon line 205 then writes back the server state unchanged. Also, whenisObjectValidationWebhookEnabledreturnstrue(i.e., no webhook config needed), any previously-created webhook is left behind.Proposed fix
func (r *reconcileState) ensureObjectValidationWebhook(ctx context.Context, obj *apiextensionsv1alpha1.CompatibilityRequirement) error { - if isObjectValidationWebhookEnabled(obj) { - return nil + if !isObjectValidationWebhookNeeded(obj) { + return r.removeObjectValidationWebhook(ctx, obj) } + if r.compatibilityCRD == nil { + return fmt.Errorf("cannot reconcile object validation webhook: compatibility CRD is nil") + } - webhookConfig := validatingWebhookConfigurationFor(obj, r.compatibilityCRD) - if err := r.client.Get(ctx, types.NamespacedName{Name: webhookConfig.Name}, webhookConfig); err != nil { + desired := validatingWebhookConfigurationFor(obj, r.compatibilityCRD) + current := &admissionregistrationv1.ValidatingWebhookConfiguration{} + if err := r.client.Get(ctx, types.NamespacedName{Name: desired.Name}, current); err != nil { if apierrors.IsNotFound(err) { - if err := r.client.Create(ctx, webhookConfig); err != nil { - return fmt.Errorf("failed to create ValidatingWebhookConfiguration %s: %w", webhookConfig.Name, err) + if err := r.client.Create(ctx, desired); err != nil { + return fmt.Errorf("failed to create ValidatingWebhookConfiguration %s: %w", desired.Name, err) } return nil } - return fmt.Errorf("failed to get ValidatingWebhookConfiguration %s: %w", webhookConfig.Name, err) + return fmt.Errorf("failed to get ValidatingWebhookConfiguration %s: %w", desired.Name, err) } - if err := r.client.Update(ctx, webhookConfig); err != nil { - return fmt.Errorf("failed to update ValidatingWebhookConfiguration %s: %w", webhookConfig.Name, err) + current.Annotations = desired.Annotations + current.Webhooks = desired.Webhooks + if err := r.client.Update(ctx, current); err != nil { + return fmt.Errorf("failed to update ValidatingWebhookConfiguration %s: %w", desired.Name, err) } return nil }🤖 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 187 - 210, The ensureObjectValidationWebhook function currently builds a desired webhookConfig then calls r.client.Get into that same object (overwriting the desired state) and later Update becomes a no-op; also when isObjectValidationWebhookEnabled(obj) returns true the controller skips creating but never deletes an existing webhook. Fix by (1) if isObjectValidationWebhookEnabled(obj) is true, attempt to fetch the existing ValidatingWebhookConfiguration into a separate variable (e.g., existing := &admissionv1.ValidatingWebhookConfiguration{}) and Delete it if found; (2) when creating/updating, keep your desired webhook (webhookConfig) separate from the fetched one: call r.client.Get(ctx, NamespacedName{Name: webhookConfig.Name}, fetched) and if NotFound call r.client.Create(ctx, webhookConfig); otherwise copy fetched.ResourceVersion into webhookConfig (or merge only the ResourceVersion) before calling r.client.Update(ctx, webhookConfig) so Update applies the desired content rather than re-saving the fetched object.pkg/controllers/crdcompatibility/reconcile_test.go (1)
222-255:⚠️ Potential issue | 🔴 Critical
BeforeAllruns before the parentBeforeEach, sotestCRDCleanis nil here.In Ginkgo v2, a nested Ordered context's
BeforeAll(line 226) executes before the parent'sBeforeEach(line 52), sotestCRDCleanreferenced on line 227 has not been initialized yet. This will cause a nil pointer dereference whenGenerateTestCompatibilityRequirementtries to marshal it.Proposed fix: use a locally-scoped CRD
BeforeAll(func(ctx context.Context) { - requirement = test.GenerateTestCompatibilityRequirement(testCRDClean) + localCRD := test.GenerateTestCRD() + requirement = test.GenerateTestCompatibilityRequirement(localCRD)Then also update any assertions that reference
testCRDClean(lines 270-273) to uselocalCRD.🤖 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 222 - 255, BeforeAll in this nested Ordered context runs before the parent's BeforeEach, so testCRDClean is nil when calling GenerateTestCompatibilityRequirement; fix by declaring and initializing a locally-scoped CRD (e.g., localCRD) inside this Context and pass that to GenerateTestCompatibilityRequirement instead of testCRDClean; also update any later assertions or comparisons that reference testCRDClean (the checks against the created CRD/requirement) to use localCRD so the test uses the locally-initialized CRD rather than the uninitialized package variable; keep the rest of the setup (requirement.Spec.ObjectSchemaValidation, createTestObject, webhookConfig) unchanged.pkg/controllers/crdcompatibility/objectvalidation/handle_test.go (1)
477-491:⚠️ Potential issue | 🟠 MajorCross-context dependency:
compatibilityRequirement.Namerelies on prior test execution.
compatibilityRequirement(line 482) is only initialized in the "when schemas match exactly"BeforeEach(line 129). ThisDescribeblock depends on that prior context having run and set the variable. WithContinueOnFailure, if the earlier context fails, this test would use stale or nil state.Proposed fix: use a local test name
- testPath := fmt.Sprintf("%s%s", WebhookPrefix, compatibilityRequirement.Name) + requirementName := "test-requirement" + testPath := fmt.Sprintf("%s%s", WebhookPrefix, requirementName) req := &http.Request{} req.URL = &url.URL{Path: testPath} ctxWithName := compatibilityRequrementIntoContext(ctx, req) extractedName := compatibilityRequrementFromContext(ctxWithName) - Expect(extractedName).To(Equal(compatibilityRequirement.Name)) + Expect(extractedName).To(Equal(requirementName))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/crdcompatibility/objectvalidation/handle_test.go` around lines 477 - 491, This test depends on the outer variable compatibilityRequirement.Name set in a different BeforeEach; instead create and use a local name to avoid cross-context dependency: declare a local const or var (e.g., testName := "local-cr-name") inside the It block, build testPath with that local name, pass req through compatibilityRequrementIntoContext and assert compatibilityRequrementFromContext returns testName (instead of referencing compatibilityRequirement.Name); update references to the local name in the testPath and Expect call while keeping compatibilityRequrementIntoContext and compatibilityRequrementFromContext as the functions under test.pkg/controllers/crdcompatibility/objectvalidation/webhook.go (1)
102-134:⚠️ Potential issue | 🟠 MajorValidation action (warn vs deny) is not honored — violations always deny admission.
ValidateCreateandValidateUpdatealways convert validation errors toapierrors.NewInvalid, which denies the request. TheObjectSchemaValidation.Actionfield (which can beWarnorDeny) is never consulted. Objects that should only trigger warnings underWarnmode will be incorrectly rejected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/crdcompatibility/objectvalidation/webhook.go` around lines 102 - 134, ValidateCreate and ValidateUpdate always convert validation errors to apierrors.NewInvalid and thus deny requests; change them to consult the validation action (e.g., ObjectSchemaValidation.Action) from the strategy returned by getValidationStrategy and only produce an apierrors.NewInvalid when the action is Deny. Concretely: after calling strategy.Validate(...) / strategy.ValidateUpdate(...), if errs is non-empty check the strategy's action field (or the ObjectSchemaValidation.Action exposed by the strategy) and if action == Warn return the warnings and nil, otherwise (action == Deny) return warnings and apierrors.NewInvalid(obj.GroupVersionKind().GroupKind(), obj.GetName(), errs).
🧹 Nitpick comments (4)
pkg/controllers/crdcompatibility/objectvalidation/validator_unit_test.go (1)
83-95:Equalchecks deep equality, not pointer identity — doesn't prove caching.The comment says "Should be the exact same object (cached)" but
Equal(line 94) usesreflect.DeepEqual. A freshly constructed but identical strategy would also pass. UseBeIdenticalToto verify the cached instance is reused:- Expect(strategy1).To(Equal(strategy2)) + Expect(strategy1).To(BeIdenticalTo(strategy2))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/crdcompatibility/objectvalidation/validator_unit_test.go` around lines 83 - 95, The test "should use cached strategy on subsequent calls" currently asserts pointer equality using Expect(strategy1).To(Equal(strategy2)), which uses deep equality; change the assertion to check identity instead by using Expect(strategy1).To(BeIdenticalTo(strategy2)) (or the equivalent pointer-identity matcher) so getValidationStrategy's caching is actually verified for the same instance; locate the assertions around variables strategy1/strategy2 in the It block and update the matcher accordingly.pkg/controllers/crdcompatibility/objectvalidation/webhook.go (2)
141-164: Missing double-checked locking — cache miss can cause redundant strategy creation under concurrency.After
getValidationStrategyFromCachereturns a miss (line 147), another goroutine may have already populated the cache by the time the write lock is acquired (line 152). The code should re-check the cache after acquiring the write lock to avoid redundantcreateVersionedStrategycalls:Proposed fix
v.validationStrategyCacheLock.Lock() defer v.validationStrategyCacheLock.Unlock() + // Re-check cache after acquiring write lock (double-checked locking) + cacheKey := getValidationStrategyCacheKey(compatibilityRequirement, version) + if cached, ok := v.validationStrategyCache[cacheKey]; ok { + return cached, nil + } + strategy, err := v.createVersionedStrategy(ctx, compatibilityRequirementName, version)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/crdcompatibility/objectvalidation/webhook.go` around lines 141 - 164, getValidationStrategy has a race where after getValidationStrategyFromCache returns a miss another goroutine may populate the cache before you acquire validationStrategyCacheLock, causing redundant createVersionedStrategy calls; fix by applying double-checked locking: after acquiring validationStrategyCacheLock in getValidationStrategy call getValidationStrategyFromCache again and return the found strategy if present, otherwise proceed to call createVersionedStrategy, then call storeValidationStrategyInCache and pruneOldValidationStrategies as before (refer to functions getValidationStrategyFromCache, createVersionedStrategy, storeValidationStrategyInCache, pruneOldValidationStrategies and the validationStrategyCacheLock).
206-211: RedundantGetof CompatibilityRequirement — already fetched by caller.
getValidationStrategy(line 143) fetches the CompatibilityRequirement, thencreateVersionedStrategy(line 208-211) fetches it again. Consider passing the already-fetched object to avoid the extra API call:Proposed fix
-func (v *validator) createVersionedStrategy(ctx context.Context, compatibilityRequirementName string, version string) (rest.RESTCreateUpdateStrategy, error) { - // Get the CompatibilityRequirement - compatibilityRequirement := &apiextensionsv1alpha1.CompatibilityRequirement{} - if err := v.client.Get(ctx, client.ObjectKey{Name: compatibilityRequirementName}, compatibilityRequirement); err != nil { - return nil, fmt.Errorf("failed to get CompatibilityRequirement %q: %w", compatibilityRequirementName, err) - } +func (v *validator) createVersionedStrategy(ctx context.Context, compatibilityRequirement *apiextensionsv1alpha1.CompatibilityRequirement, version string) (rest.RESTCreateUpdateStrategy, error) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/controllers/crdcompatibility/objectvalidation/webhook.go` around lines 206 - 211, The code redundantly fetches the CompatibilityRequirement inside createVersionedStrategy even though getValidationStrategy already retrieved it; update createVersionedStrategy to accept a pre-fetched *apiextensionsv1alpha1.CompatibilityRequirement parameter (instead of name) and remove the client.Get call, then adjust callers (e.g., getValidationStrategy) to pass the existing compatibilityRequirement when invoking createVersionedStrategy; ensure function signature and all call sites are updated consistently to avoid the extra API call.pkg/controllers/crdcompatibility/reconcile.go (1)
234-237: Misleading function name:isObjectValidationWebhookEnabledreturnstruewhen nothing is configured.The function returns
truewhenAction == "",MatchConditions == nil, and both selectors are empty — meaning the webhook is not needed. This is the opposite of what "enabled" suggests. Consider renaming toisObjectValidationWebhookDisabledor inverting toisObjectValidationWebhookNeeded.🤖 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 234 - 237, The function isObjectValidationWebhookEnabled currently returns true when no object schema validation is configured (osv.Action == "" && osv.MatchConditions == nil && labelSelectorIsEmpty(osv.NamespaceSelector) && labelSelectorIsEmpty(osv.ObjectSelector)), which is the opposite of its name; either rename it to isObjectValidationWebhookDisabled or invert its boolean logic and rename to isObjectValidationWebhookNeeded, update the return expression accordingly (negate the current condition or remove the negation if renaming to Disabled), and update all references to this function throughout the codebase to use the new name or inverted semantics (ensure any callers relying on the old truthiness are updated to maintain behavior).
🤖 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 315-333: Update the misleading inline comment near the test case
that reads "This should succeed with looser validation": change it to state that
the creation is expected to fail because the base CRD's own OpenAPI schema
(baseCRD.Spec) still marks requiredField as required, so cl.Create(ctx,
objMissingFormerlyRequired) should return an invalid error; locate the comment
around the test It("should not allow objects without required fields through API
server") and the objMissingFormerlyRequired variable to make this clarification
referencing the CRD schema rather than the webhook.
In `@pkg/controllers/crdcompatibility/objectvalidation/webhook.go`:
- Around line 279-283: The error message in the scale conversion block is
incorrect: when converting subresources.Scale to the internal type (using
Convert_v1_CustomResourceSubresourceScale_To_apiextensions_CustomResourceSubresourceScale)
update the fmt.Errorf call to reference the "scale subresource" instead of
"status subresource" (the relevant symbols are subresources.Scale, scaleSpec,
and
Convert_v1_CustomResourceSubresourceScale_To_apiextensions_CustomResourceSubresourceScale
in the webhook.go code).
---
Outside diff comments:
In `@pkg/controllers/crdcompatibility/reconcile.go`:
- Around line 170-185: The deletion currently uses errors.Join to run
clearFinalizer and removeObjectValidationWebhook in parallel which can remove
the finalizer even if webhook cleanup fails; change reconcileDelete to call
removeObjectValidationWebhook(ctx, obj) first and if it returns an error return
that error (so the finalizer remains), and only after successful webhook removal
call clearFinalizer(ctx, r.client, obj) and return any error from that; stop
using errors.Join so the finalizer is only cleared after
removeObjectValidationWebhook succeeds.
---
Duplicate comments:
In `@pkg/controllers/crdcompatibility/objectvalidation/handle_test.go`:
- Around line 477-491: This test depends on the outer variable
compatibilityRequirement.Name set in a different BeforeEach; instead create and
use a local name to avoid cross-context dependency: declare a local const or var
(e.g., testName := "local-cr-name") inside the It block, build testPath with
that local name, pass req through compatibilityRequrementIntoContext and assert
compatibilityRequrementFromContext returns testName (instead of referencing
compatibilityRequirement.Name); update references to the local name in the
testPath and Expect call while keeping compatibilityRequrementIntoContext and
compatibilityRequrementFromContext as the functions under test.
In `@pkg/controllers/crdcompatibility/objectvalidation/webhook.go`:
- Around line 102-134: ValidateCreate and ValidateUpdate always convert
validation errors to apierrors.NewInvalid and thus deny requests; change them to
consult the validation action (e.g., ObjectSchemaValidation.Action) from the
strategy returned by getValidationStrategy and only produce an
apierrors.NewInvalid when the action is Deny. Concretely: after calling
strategy.Validate(...) / strategy.ValidateUpdate(...), if errs is non-empty
check the strategy's action field (or the ObjectSchemaValidation.Action exposed
by the strategy) and if action == Warn return the warnings and nil, otherwise
(action == Deny) return warnings and
apierrors.NewInvalid(obj.GroupVersionKind().GroupKind(), obj.GetName(), errs).
In `@pkg/controllers/crdcompatibility/reconcile_test.go`:
- Around line 222-255: BeforeAll in this nested Ordered context runs before the
parent's BeforeEach, so testCRDClean is nil when calling
GenerateTestCompatibilityRequirement; fix by declaring and initializing a
locally-scoped CRD (e.g., localCRD) inside this Context and pass that to
GenerateTestCompatibilityRequirement instead of testCRDClean; also update any
later assertions or comparisons that reference testCRDClean (the checks against
the created CRD/requirement) to use localCRD so the test uses the
locally-initialized CRD rather than the uninitialized package variable; keep the
rest of the setup (requirement.Spec.ObjectSchemaValidation, createTestObject,
webhookConfig) unchanged.
In `@pkg/controllers/crdcompatibility/reconcile.go`:
- Around line 156-161: Replace the use of errors.Join (which runs all calls
unconditionally) with a sequential error-check pattern so dependent steps run
only if prior steps succeed: call r.parseCompatibilityCRD(obj) first and return
on error, then call r.fetchCurrentCRD(ctx, logger) and return on error, then
r.checkCompatibilityRequirement() and return on error, then
r.ensureObjectValidationWebhook(ctx, obj) and return on error; also keep or add
a nil-guard around r.compatibilityCRD (used by
validatingWebhookConfigurationFor) to avoid panics if it ever becomes nil.
- Around line 243-282: The ValidatingWebhookConfiguration built in
validatingWebhookConfigurationFor is missing propagation of namespace/object
selectors and match conditions from the CR's spec; update the ValidatingWebhook
(inside validatingWebhookConfigurationFor) to set NamespaceSelector,
ObjectSelector and MatchConditions using values from
obj.Spec.ObjectSchemaValidation (copying those fields into the
admissionregistrationv1.ValidatingWebhook fields), ensuring proper type
conversion if needed so NamespaceSelector and ObjectSelector become
*metav1.LabelSelector and MatchConditions map to
admissionregistrationv1.MatchCondition slice on the webhook; keep the field
names (NamespaceSelector, ObjectSelector, MatchConditions) consistent with
admissionregistrationv1 API and read from obj.Spec.ObjectSchemaValidation to
preserve user intent.
- Around line 187-210: The ensureObjectValidationWebhook function currently
builds a desired webhookConfig then calls r.client.Get into that same object
(overwriting the desired state) and later Update becomes a no-op; also when
isObjectValidationWebhookEnabled(obj) returns true the controller skips creating
but never deletes an existing webhook. Fix by (1) if
isObjectValidationWebhookEnabled(obj) is true, attempt to fetch the existing
ValidatingWebhookConfiguration into a separate variable (e.g., existing :=
&admissionv1.ValidatingWebhookConfiguration{}) and Delete it if found; (2) when
creating/updating, keep your desired webhook (webhookConfig) separate from the
fetched one: call r.client.Get(ctx, NamespacedName{Name: webhookConfig.Name},
fetched) and if NotFound call r.client.Create(ctx, webhookConfig); otherwise
copy fetched.ResourceVersion into webhookConfig (or merge only the
ResourceVersion) before calling r.client.Update(ctx, webhookConfig) so Update
applies the desired content rather than re-saving the fetched object.
---
Nitpick comments:
In `@pkg/controllers/crdcompatibility/objectvalidation/validator_unit_test.go`:
- Around line 83-95: The test "should use cached strategy on subsequent calls"
currently asserts pointer equality using Expect(strategy1).To(Equal(strategy2)),
which uses deep equality; change the assertion to check identity instead by
using Expect(strategy1).To(BeIdenticalTo(strategy2)) (or the equivalent
pointer-identity matcher) so getValidationStrategy's caching is actually
verified for the same instance; locate the assertions around variables
strategy1/strategy2 in the It block and update the matcher accordingly.
In `@pkg/controllers/crdcompatibility/objectvalidation/webhook.go`:
- Around line 141-164: getValidationStrategy has a race where after
getValidationStrategyFromCache returns a miss another goroutine may populate the
cache before you acquire validationStrategyCacheLock, causing redundant
createVersionedStrategy calls; fix by applying double-checked locking: after
acquiring validationStrategyCacheLock in getValidationStrategy call
getValidationStrategyFromCache again and return the found strategy if present,
otherwise proceed to call createVersionedStrategy, then call
storeValidationStrategyInCache and pruneOldValidationStrategies as before (refer
to functions getValidationStrategyFromCache, createVersionedStrategy,
storeValidationStrategyInCache, pruneOldValidationStrategies and the
validationStrategyCacheLock).
- Around line 206-211: The code redundantly fetches the CompatibilityRequirement
inside createVersionedStrategy even though getValidationStrategy already
retrieved it; update createVersionedStrategy to accept a pre-fetched
*apiextensionsv1alpha1.CompatibilityRequirement parameter (instead of name) and
remove the client.Get call, then adjust callers (e.g., getValidationStrategy) to
pass the existing compatibilityRequirement when invoking
createVersionedStrategy; ensure function signature and all call sites are
updated consistently to avoid the extra API call.
In `@pkg/controllers/crdcompatibility/reconcile.go`:
- Around line 234-237: The function isObjectValidationWebhookEnabled currently
returns true when no object schema validation is configured (osv.Action == "" &&
osv.MatchConditions == nil && labelSelectorIsEmpty(osv.NamespaceSelector) &&
labelSelectorIsEmpty(osv.ObjectSelector)), which is the opposite of its name;
either rename it to isObjectValidationWebhookDisabled or invert its boolean
logic and rename to isObjectValidationWebhookNeeded, update the return
expression accordingly (negate the current condition or remove the negation if
renaming to Disabled), and update all references to this function throughout the
codebase to use the new name or inverted semantics (ensure any callers relying
on the old truthiness are updated to maintain behavior).
pkg/controllers/crdcompatibility/objectvalidation/handle_test.go
Outdated
Show resolved
Hide resolved
JoelSpeed
force-pushed
the
compatibility-object-admission
branch
from
8252156 to
8969e3e
Compare
openshift-merge-robot
removed
the
needs-rebase
|
@JoelSpeed: This pull request references OCPCLOUD-3005 which is a valid jira issue. This pull request references OCPCLOUD-3186 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. This pull request references OCPCLOUD-3367 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. |
JoelSpeed
force-pushed
the
compatibility-object-admission
branch
2 times, most recently
from
f5afc62 to
2c43d65
Compare
Contributor
|
/test unit |
| cl client.Client | ||
| ) | ||
|
|
||
| var defaultNodeTimeout = NodeTimeout(10 * time.Second) |
Contributor
There was a problem hiding this comment.
Given slow / laggy CI I might bump this to 30s/1m or split on things we expect to maybe slow vs immediately return
Contributor
|
broadly LGTM on changes, a few nits but would be happy to merge.. No idea what's going on with units though :( |
JoelSpeed
force-pushed
the
compatibility-object-admission
branch
from
2c43d65 to
9a38bff
Compare
|
Scheduling tests matching the |
Contributor
|
[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
Contributor
Author
|
/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. |
Contributor
|
@JoelSpeed: The following tests 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This implements the object admission validation webhook and ValidatingWebhookConfiguration reconciliation. This will allow folks who want to implement object schema validations to restrict the resources in a particular namespace based on the compatibility requirements CRD
Summary by CodeRabbit
New Features
Tests