Skip to content

Conversation

awgreene
Copy link
Member

@awgreene awgreene commented Oct 27, 2020

Problem: Operator Authors can create CSVs that define Webhooks with
containerPorts set outside the range of 0-65535, which prevents the
ValidatingWebhookConfiguration or MutatingWebhookConfiguration from
being created due to failed validation.

Solution: Update the CSV's CRD Validation to include the proper min and
max values for webhookDescription ContainerPort field.

Example OLM Failure Log:

time="2020-10-27T14:02:12Z" level=error msg="Webhooks: Error creating MutatingWebhookConfiguration: MutatingWebhookConfiguration.admissionregistration.k8s.io \"mopentelemetrycollector.kb.io-gtbpc\" is invalid: webhooks[0].clientConfig.service.port: Invalid value: 0: port is not valid: must be between 1 and 65535, inclusive"

@awgreene
Copy link
Member Author

awgreene commented Oct 27, 2020

OLM logs when a CSV with an invalid containerPort is created:

time="2020-10-27T14:02:12Z" level=error msg="Webhooks: Error creating MutatingWebhookConfiguration: MutatingWebhookConfiguration.admissionregistration.k8s.io \"mopentelemetrycollector.kb.io-gtbpc\" is invalid: webhooks[0].clientConfig.service.port: Invalid value: 0: port is not valid: must be between 1 and 65535, inclusive"

Copy link
Member

@dinhxuanvu dinhxuanvu left a comment

Choose a reason for hiding this comment

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

Looking good.

@awgreene awgreene force-pushed the webhook-port-values branch from 28edde2 to 0e214c9 Compare October 27, 2020 16:06
Copy link
Member

@dinhxuanvu dinhxuanvu left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 27, 2020
@awgreene awgreene force-pushed the webhook-port-values branch from 0e214c9 to 652cd68 Compare October 27, 2020 16:15
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 27, 2020
@dinhxuanvu
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 27, 2020
Problem: Operator Authors can create CSVs that define Webhooks with
containerPorts set outside the range of 0-65535, which prevents the
ValidatingWebhookConfiguration or MutatingWebhookConfiguration from
being created due to failed validation.

Solution: Update the CSV's CRD Validation to include the proper min and
max values for webhookDescription ContainerPort field.
@awgreene awgreene force-pushed the webhook-port-values branch from 652cd68 to 3b25e5d Compare October 27, 2020 16:29
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 27, 2020
Copy link
Member

@dinhxuanvu dinhxuanvu left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 27, 2020
@awgreene awgreene merged commit 69e6f9b into operator-framework:master Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants