Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing maps webhook, fix inconsistencies #6489

Merged
merged 4 commits into from
Mar 6, 2023

Conversation

pebrc
Copy link
Collaborator

@pebrc pebrc commented Mar 6, 2023

Fixes #6151
Fixes #6152

I also removed config/webhook/manifests.yaml I am not sure what purpose it still served. It seems to me that it is unused. Looks like the static manifest is generated by controller-gen I am not sure why we need that.

@pebrc pebrc added >bug Something isn't working v2.7.0 labels Mar 6, 2023
@pebrc pebrc changed the title Add missing maps webhook, fix inconsistencies, remove static wh manifest Add missing maps webhook, fix inconsistencies Mar 6, 2023
@pebrc pebrc requested a review from thbkrkr March 6, 2023 08:30
Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

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

Looks like the static manifest is generated by controller-gen I am not sure why we need that.

I'm not sure either, maybe to keep the reference version and be able to compare with our template. We started managing the definition of webhooks in templates almost from the start (#2126).

If we follow this logic, we should also update the kubebuilder:webhook marker here:

// +kubebuilder:webhook:path=/validate-ems-k8s-elastic-co-v1alpha1-mapsservers,mutating=false,failurePolicy=ignore,groups=maps.k8s.elastic.co,resources=mapsservers,verbs=create;update,versions=v1alpha1,name=elastic-ems-validation-v1alpha1.k8s.elastic.co,sideEffects=None,admissionReviewVersions=v1alpha1,matchPolicy=Exact
and regenerate.

Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working v2.7.0
Projects
None yet
2 participants