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.

Sign up for GitHub

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

Jump to bottom

Update webhook #529

Merged
merged 1 commit into from
May 22, 2024
Merged

Update webhook #529

merged 1 commit into from
May 22, 2024
+162 −51

Conversation

stuggi
Copy link
Contributor

@stuggi stuggi commented May 8, 2024

Updates webhook to be able to be call from the openstack-operator webhook on the GlanceSpecCore, like for other operators in [1].

Also adds check for valid service override endpoint type on create and update.

Depends-On: openstack-k8s-operators/lib-common#505

[1] https://github.com/openstack-k8s-operators/openstack-operator/blob/d2703d3a321c979dacaca95b5d4a634bf116e0db/apis/core/v1beta1/openstackcontrolplane_webhook.go#L181

@stuggi
Copy link
Contributor Author

stuggi commented May 8, 2024

/hold until after freeze

@openshift-ci openshift-ci bot requested review from dprince and konan-abhi May 8, 2024 11:57
@stuggi stuggi requested a review from fmount May 8, 2024 11:57
stuggi added a commit to stuggi/openstack-operator that referenced this pull request May 14, 2024
@stuggi
Add validation webhooks service operators
8f1bffd 
Adds calling service operator validations from within the
openstack-operator for:
- barbican
- neutron
- glance
- cinder
- heat
- manila
- swift
- octavia
- designate

Depends-On: openstack-k8s-operators/barbican-operator#115
Depends-On: openstack-k8s-operators/neutron-operator#347
Depends-On: openstack-k8s-operators/glance-operator#529
Depends-On: openstack-k8s-operators/cinder-operator#384
Depends-On: openstack-k8s-operators/heat-operator#361
Depends-On: openstack-k8s-operators/manila-operator#273
Depends-On: openstack-k8s-operators/swift-operator#218
Depends-On: openstack-k8s-operators/octavia-operator#301
Depends-On: openstack-k8s-operators/designate-operator#166
@stuggi
Update webhook
ac56675 
Updates webhook to be able to be call from the openstack-operator
webhook on the GlanceSpecCore, like for other operators in [1].

Also adds check for valid service override endpoint type on create
and update.

Depends-On: openstack-k8s-operators/lib-common#505

[1] https://github.com/openstack-k8s-operators/openstack-operator/blob/d2703d3a321c979dacaca95b5d4a634bf116e0db/apis/core/v1beta1/openstackcontrolplane_webhook.go#L181
fmount
fmount approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@fmount fmount left a comment

Choose a reason for hiding this comment

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

I didn't try this patch but from the code point of view seems correct.
I think we can improve the validation webhook tests but what we have looks ok.

api/v1beta1/glance_webhook.go

// ValidateCreate - Exported function wrapping non-exported validate functions,
// this function can be called externally to validate an ironic spec.
func (r *GlanceSpec) ValidateCreate(basePath *field.Path) field.ErrorList {
Copy link
Contributor

Choose a reason for hiding this comment

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

ok I was initially concerned about changing APISpec to APISpecCore but I see we can still test this with a standalone glance instance and this is basically a wrapper for GlanceSpecCore.ValidateCreate where the validation actually happens.

@openshift-ci openshift-ci bot added the lgtm label May 22, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented May 22, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fmount, stuggi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fmount fmount removed the lgtm label May 22, 2024
@fmount
Copy link
Contributor

fmount commented May 22, 2024 •
edited

Wait, I need to test it locally because I realized we don't have a test that proves we can't update the layout from single to split (or do the reverse). I'll try this locally and comment further.

@fmount
Copy link
Contributor

fmount commented May 22, 2024

ok I checked locally and looks safe, sorry for the delay, I knew it was ok in terms of code but I wanted to test it locally to make sure we preserve the current layout.

@fmount
Copy link
Contributor

fmount commented May 22, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label May 22, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit daeab50 into openstack-k8s-operators:main May 22, 2024
7 checks passed
stuggi added a commit to stuggi/openstack-operator that referenced this pull request May 23, 2024
@stuggi
Add validation webhooks service operators
fc8d0cd 
Adds calling service operator validations from within the
openstack-operator for:
- barbican
- neutron
- glance
- cinder
- heat
- manila
- swift
- octavia
- designate

Depends-On: openstack-k8s-operators/barbican-operator#115
Depends-On: openstack-k8s-operators/neutron-operator#347
Depends-On: openstack-k8s-operators/glance-operator#529
Depends-On: openstack-k8s-operators/cinder-operator#384
Depends-On: openstack-k8s-operators/heat-operator#361
Depends-On: openstack-k8s-operators/manila-operator#273
Depends-On: openstack-k8s-operators/swift-operator#218
Depends-On: openstack-k8s-operators/octavia-operator#301
Depends-On: openstack-k8s-operators/designate-operator#166
stuggi added a commit to stuggi/openstack-operator that referenced this pull request May 23, 2024
@stuggi
Add validation webhooks service operators
231c48b 
Adds calling service operator validations from within the
openstack-operator for:
- barbican
- neutron
- glance
- cinder
- heat
- manila
- swift
- octavia
- designate

Depends-On: openstack-k8s-operators/barbican-operator#115
Depends-On: openstack-k8s-operators/neutron-operator#347
Depends-On: openstack-k8s-operators/glance-operator#529
Depends-On: openstack-k8s-operators/cinder-operator#384
Depends-On: openstack-k8s-operators/heat-operator#361
Depends-On: openstack-k8s-operators/manila-operator#273
Depends-On: openstack-k8s-operators/swift-operator#218
Depends-On: openstack-k8s-operators/octavia-operator#301
Depends-On: openstack-k8s-operators/designate-operator#166
@stuggi stuggi mentioned this pull request May 23, 2024
Merged
stuggi added a commit to stuggi/openstack-operator that referenced this pull request May 24, 2024
@stuggi
Add validation webhooks service operators
e93f1c0 
Adds calling service operator validations from within the
openstack-operator for:
- keystone
- barbican
- neutron
- glance
- cinder
- heat
- manila
- swift
- octavia
- designate

Depends-On: openstack-k8s-operators/keystone-operator#414
Depends-On: openstack-k8s-operators/barbican-operator#115
Depends-On: openstack-k8s-operators/neutron-operator#347
Depends-On: openstack-k8s-operators/glance-operator#529
Depends-On: openstack-k8s-operators/cinder-operator#384
Depends-On: openstack-k8s-operators/heat-operator#361
Depends-On: openstack-k8s-operators/manila-operator#273
Depends-On: openstack-k8s-operators/swift-operator#218
Depends-On: openstack-k8s-operators/octavia-operator#301
Depends-On: openstack-k8s-operators/designate-operator#166
stuggi added a commit to stuggi/openstack-operator that referenced this pull request Jun 3, 2024
@stuggi
Add validation webhooks service operators
03d9852 
Adds calling service operator validations from within the
openstack-operator for:
- keystone
- barbican
- neutron
- glance
- cinder
- heat
- manila
- swift
- octavia
- designate

Depends-On: openstack-k8s-operators/keystone-operator#414
Depends-On: openstack-k8s-operators/barbican-operator#115
Depends-On: openstack-k8s-operators/neutron-operator#347
Depends-On: openstack-k8s-operators/glance-operator#529
Depends-On: openstack-k8s-operators/cinder-operator#384
Depends-On: openstack-k8s-operators/heat-operator#361
Depends-On: openstack-k8s-operators/manila-operator#273
Depends-On: openstack-k8s-operators/swift-operator#218
Depends-On: openstack-k8s-operators/octavia-operator#301
Depends-On: openstack-k8s-operators/designate-operator#166
stuggi added a commit to stuggi/openstack-operator that referenced this pull request Jun 3, 2024
@stuggi
Add validation webhooks service operators
c3916c9 
Adds calling service operator validations from within the
openstack-operator for:
- keystone
- barbican
- neutron
- glance
- cinder
- heat
- manila
- swift
- octavia
- designate

Depends-On: openstack-k8s-operators/keystone-operator#414
Depends-On: openstack-k8s-operators/barbican-operator#115
Depends-On: openstack-k8s-operators/neutron-operator#347
Depends-On: openstack-k8s-operators/glance-operator#529
Depends-On: openstack-k8s-operators/cinder-operator#384
Depends-On: openstack-k8s-operators/heat-operator#361
Depends-On: openstack-k8s-operators/manila-operator#273
Depends-On: openstack-k8s-operators/swift-operator#218
Depends-On: openstack-k8s-operators/octavia-operator#301
Depends-On: openstack-k8s-operators/designate-operator#166
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmount fmount fmount approved these changes

@dprince dprince Awaiting requested review from dprince

@konan-abhi konan-abhi Awaiting requested review from konan-abhi

Assignees

@fmount fmount

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@stuggi @fmount