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 Port configuration to ServiceReference of Admission webhooks, CRD Conversion Webhooks, AuditSink Webhooks and kube-aggregator #74855

Merged
merged 7 commits into from Apr 8, 2019

Conversation

@mbohlool
Copy link
Member

commented Mar 2, 2019

This is a work item for Admission Webhooks GA. To be consistent in the feature set of all webhooks and service references, the port configuration added to the rest of the features including kube-aggregator. All of these services share some level of code and it make sense to support this in all of them.

This however need to be documented for each service separately.

- Added port configuration to Admission webhook configuration service reference.
- Added port configuration to AuditSink webhook configuration service reference.
- Added port configuration to CRD Conversion webhook configuration service reference.
- Added port configuration to kube-aggregator service reference.

@liggitt @sttts @caesarxuchao

// SetDefaults_ServiceReference sets defaults for AuditSync Webhook's ServiceReference
func SetDefaults_ServiceReference(obj *ServiceReference) {
if obj.Port == nil {
obj.Port = utilpointer.Int32Ptr(493)

This comment has been minimized.

Copy link
@liggitt

liggitt Mar 2, 2019

Member

443

@fejta-bot

This comment has been minimized.

Copy link

commented Mar 3, 2019

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

@mbohlool mbohlool force-pushed the mbohlool:sam branch from dad12e7 to 923325c Mar 3, 2019

@liggitt liggitt added the api-review label Mar 4, 2019

@liggitt liggitt added this to Changes requested in API Reviews Mar 4, 2019

@liggitt liggitt self-assigned this Mar 4, 2019

@liggitt liggitt added this to the v1.14 milestone Mar 4, 2019

@mbohlool mbohlool force-pushed the mbohlool:sam branch from 923325c to 8814c8a Mar 4, 2019

@@ -302,4 +300,10 @@ type ServiceReference struct {
// this service.
// +optional
Path *string

// If specified, the port on the service that hosting webhook.
// Default to 443 for backward compatibility.

This comment has been minimized.

Copy link
@liggitt

liggitt Mar 4, 2019

Member

drop the default comment from the internal versions, defaults only apply to external versions

This comment has been minimized.

Copy link
@mbohlool

mbohlool Mar 4, 2019

Author Member

done

@mbohlool

This comment has been minimized.

Copy link
Member Author

commented Mar 4, 2019

/retest

@liggitt liggitt removed this from Changes requested in API Reviews Mar 29, 2019

@mbohlool mbohlool force-pushed the mbohlool:sam branch from e589310 to daf51a2 Apr 8, 2019

Mehdy Bohlool added some commits Mar 1, 2019

Mehdy Bohlool
Add port to ServiceReference of Admission Webhooks, ConversionWebhook…
…s and AuditSync with defaulter and validator
Mehdy Bohlool
Mehdy Bohlool
Mehdy Bohlool

@mbohlool mbohlool force-pushed the mbohlool:sam branch from daf51a2 to e6f5f4f Apr 8, 2019

@mbohlool

This comment has been minimized.

Copy link
Member Author

commented Apr 8, 2019

@liggitt PTAL

@mbohlool

This comment has been minimized.

Copy link
Member Author

commented Apr 8, 2019

/hold cancel

@liggitt

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

/lgtm
/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, mbohlool

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

@k8s-ci-robot k8s-ci-robot merged commit eb65eac into kubernetes:master Apr 8, 2019

16 of 17 checks passed

pull-kubernetes-kubemark-e2e-gce-big Job triggered.
Details
cla/linuxfoundation mbohlool authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-godeps Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Job succeeded.
Details
tide In merge pool.
Details
@ashleyschuett

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

Is this going to be cherry picked back to any 1.13 or 1.14 releases? If it's not currently planned to be, can it be?

@liggitt

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

no. new features/capabilities are not backported, just bug/security fixes

@caesarxuchao caesarxuchao referenced this pull request Apr 12, 2019

Open

Admission webhook #492

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.