OCPBUGS-31666: route: Fix insecureEdgeTerminationPolicy default by Miciah · Pull Request #1845 · openshift/api

Miciah · 2024-04-02T23:56:33Z

Fix the godoc for the spec.tls.insecureEdgeTerminationPolicy field to say that "None", not "Allow", is the default value.

Also, add a more explicit statement about the default value.

Finally, fix the formatting so that the list displays properly in the output of oc explain. Before, the output looked like this:

% oc explain routes.spec.tls.insecureEdgeTerminationPolicy
KIND:     Route
VERSION:  route.openshift.io/v1

FIELD:    insecureEdgeTerminationPolicy <string>

DESCRIPTION:
     insecureEdgeTerminationPolicy indicates the desired behavior for insecure
     connections to a route. While each router may make its own decisions on
     which ports to expose, this is normally port 80.

     * Allow - traffic is sent to the server on the insecure port
     (edge/reencrypt terminations only) (default). * None - no traffic is
     allowed on the insecure port. * Redirect - clients are redirected to the
     secure port.

After this change, the output looks like this:

% oc explain routes.spec.tls.insecureEdgeTerminationPolicy
KIND:     Route
VERSION:  route.openshift.io/v1

FIELD:    insecureEdgeTerminationPolicy <string>

DESCRIPTION:
     insecureEdgeTerminationPolicy indicates the desired behavior for insecure
     connections to a route. While each router may make its own decisions on
     which ports to expose, this is normally port 80. If a route does not
     specify insecureEdgeTerminationPolicy, then the default behavior is "None".
     * Allow - traffic is sent to the server on the insecure port
     (edge/reencrypt terminations only).
     * None - no traffic is allowed on the insecure port (default).
     * Redirect - clients are redirected to the secure port.

openshift-ci-robot · 2024-04-02T23:56:40Z

@Miciah: This pull request references Jira Issue OCPBUGS-31666, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.16.0) matches configured target version for branch (4.16.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lihongan

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Fix the godoc for the spec.tls.insecureEdgeTerminationPolicy field to say that "None", not "Allow", is the default value.

Also, add a more explicit statement about the default value.

Finally, fix the formatting so that the list displays properly in the output of oc explain. Before, the output looked like this:

% oc explain routes.spec.tls.insecureEdgeTerminationPolicy
KIND: Route
VERSION: route.openshift.io/v1

FIELD: insecureEdgeTerminationPolicy

DESCRIPTION:
insecureEdgeTerminationPolicy indicates the desired behavior for insecure
connections to a route. While each router may make its own decisions on
which ports to expose, this is normally port 80.
    * Allow - traffic is sent to the server on the insecure port
    (edge/reencrypt terminations only) (default). * None - no traffic is
    allowed on the insecure port. * Redirect - clients are redirected to the
    secure port.
After this change, the output looks like this:

% oc explain routes.spec.tls.insecureEdgeTerminationPolicy --api-version=crdroute.openshift.io/v1
KIND: Route
VERSION: crdroute.openshift.io/v1

FIELD: insecureEdgeTerminationPolicy

DESCRIPTION:
insecureEdgeTerminationPolicy indicates the desired behavior for insecure
connections to a route. While each router may make its own decisions on
which ports to expose, this is normally port 80. If a route does not
specify insecureEdgeTerminationPolicy, then the default behavior is "None".
* Allow - traffic is sent to the server on the insecure port
(edge/reencrypt terminations only).
* None - no traffic is allowed on the insecure port (default).
* Redirect - clients are redirected to the secure port.

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 · 2024-04-02T23:56:44Z

Hello @Miciah! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

openshift-ci-robot · 2024-04-02T23:57:00Z

@Miciah: This pull request references Jira Issue OCPBUGS-31666, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.16.0) matches configured target version for branch (4.16.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lihongan

Details

In response to this:

Fix the godoc for the spec.tls.insecureEdgeTerminationPolicy field to say that "None", not "Allow", is the default value.

Also, add a more explicit statement about the default value.

Finally, fix the formatting so that the list displays properly in the output of oc explain. Before, the output looked like this:

% oc explain routes.spec.tls.insecureEdgeTerminationPolicy
KIND: Route
VERSION: route.openshift.io/v1

FIELD: insecureEdgeTerminationPolicy

DESCRIPTION:
insecureEdgeTerminationPolicy indicates the desired behavior for insecure
connections to a route. While each router may make its own decisions on
which ports to expose, this is normally port 80.
    * Allow - traffic is sent to the server on the insecure port
    (edge/reencrypt terminations only) (default). * None - no traffic is
    allowed on the insecure port. * Redirect - clients are redirected to the
    secure port.
After this change, the output looks like this:

% oc explain routes.spec.tls.insecureEdgeTerminationPolicy --api-version=route.openshift.io/v1
KIND: Route
VERSION: crdroute.openshift.io/v1

FIELD: insecureEdgeTerminationPolicy

DESCRIPTION:
insecureEdgeTerminationPolicy indicates the desired behavior for insecure
connections to a route. While each router may make its own decisions on
which ports to expose, this is normally port 80. If a route does not
specify insecureEdgeTerminationPolicy, then the default behavior is "None".
* Allow - traffic is sent to the server on the insecure port
(edge/reencrypt terminations only).
* None - no traffic is allowed on the insecure port (default).
* Redirect - clients are redirected to the secure port.

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.

frobware · 2024-04-03T15:22:36Z

/lgtm

ShudiLi · 2024-04-08T06:41:51Z

It was strange after this change, the output looked as same as before

created a cluster by the bot
launch OCPBUGS-31666: route: Fix insecureEdgeTerminationPolicy default #1845 gcp

% oc get clusterversion
NAME VERSION AVAILABLE PROGRESSING SINCE STATUS
version 4.16.0-0.ci.test-2024-04-08-055444-ci-ln-11w2cxk-latest True False 11m Cluster version is 4.16.0-0.ci.test-2024-04-08-055444-ci-ln-11w2cxk-latest

the default is Allow
% oc explain routes.spec.tls.insecureEdgeTerminationPolicy
GROUP: route.openshift.io
KIND: Route
VERSION: v1

FIELD: insecureEdgeTerminationPolicy

DESCRIPTION:
insecureEdgeTerminationPolicy indicates the desired behavior for insecure
connections to a route. While each router may make its own decisions on
which ports to expose, this is normally port 80.

* Allow - traffic is sent to the server on the insecure port (edge/reencrypt
terminations only) (default). * None - no traffic is allowed on the insecure
port. * Redirect - clients are redirected to the secure port.

%

deads2k · 2024-04-11T20:37:08Z

Fix the godoc for the spec.tls.insecureEdgeTerminationPolicy field to say that "None", not "Allow", is the default value.

I'm taking this to mean that the default isn't changing, we're just fixing the doc. If I understand correctly, please release the hold at your discretion.

/approve
/hold

Miciah · 2024-04-24T14:52:30Z

I'm taking this to mean that the default isn't changing, we're just fixing the doc. If I understand correctly, please release the hold at your discretion.

That is correct. Thanks!

/hold cancel

openshift-ci-robot · 2024-04-24T15:54:08Z

/retest-required

Remaining retests: 0 against base HEAD 29a704b and 2 for PR HEAD 8a46cb2 in total

openshift-ci-robot · 2024-04-24T21:41:30Z

/retest-required

Remaining retests: 0 against base HEAD e1deb89 and 1 for PR HEAD 8a46cb2 in total

Fix the godoc for the spec.tls.insecureEdgeTerminationPolicy field to say that "None", not "Allow", is the default value. Also, add a more explicit statement about the default value. Finally, fix the formatting so that the list displays properly in the output of oc explain. Before, the output looked like this: % oc explain routes.spec.tls.insecureEdgeTerminationPolicy KIND: Route VERSION: route.openshift.io/v1 FIELD: insecureEdgeTerminationPolicy <string> DESCRIPTION: insecureEdgeTerminationPolicy indicates the desired behavior for insecure connections to a route. While each router may make its own decisions on which ports to expose, this is normally port 80. * Allow - traffic is sent to the server on the insecure port (edge/reencrypt terminations only) (default). * None - no traffic is allowed on the insecure port. * Redirect - clients are redirected to the secure port. After this commit, the output looks like this: % oc explain routes.spec.tls.insecureEdgeTerminationPolicy KIND: Route VERSION: route.openshift.io/v1 FIELD: insecureEdgeTerminationPolicy <string> DESCRIPTION: insecureEdgeTerminationPolicy indicates the desired behavior for insecure connections to a route. While each router may make its own decisions on which ports to expose, this is normally port 80. If a route does not specify insecureEdgeTerminationPolicy, then the default behavior is "None". * Allow - traffic is sent to the server on the insecure port (edge/reencrypt terminations only). * None - no traffic is allowed on the insecure port (default). * Redirect - clients are redirected to the secure port. This commit fixes OCPBUGS-31666. https://issues.redhat.com/browse/OCPBUGS-31666 * route/v1/types.go: Update. * openapi/generated_openapi/zz_generated.openapi.go: * openapi/openapi.json: * route/v1/generated.proto: * route/v1/zz_generated.crd-manifests/routes-CustomNoUpgrade.crd.yaml: * route/v1/zz_generated.crd-manifests/routes-Default.crd.yaml: * route/v1/zz_generated.crd-manifests/routes-DevPreviewNoUpgrade.crd.yaml: * route/v1/zz_generated.crd-manifests/routes-TechPreviewNoUpgrade.crd.yaml: * route/v1/zz_generated.featuregated-crd-manifests/routes.route.openshift.io/AAA_ungated.yaml: * route/v1/zz_generated.featuregated-crd-manifests/routes.route.openshift.io/ExternalRouteCertificate.yaml: * route/v1/zz_generated.swagger_doc_generated.go: Regenerate.

Miciah · 2024-04-24T22:14:25Z

https://github.com/openshift/api/compare/8a46cb2399761db925d8987c308b344423c09df6..b9885cc0c986b493f6ea5f4b6fc2583fe7182cd1 rebases and updates route/v1/zz_generated.crd-manifests/routes-DevPreviewNoUpgrade.crd.yaml, which was added in #1825.

Miciah · 2024-04-24T22:41:37Z

@frobware, would you mind retagging the PR?

frobware · 2024-04-25T07:51:22Z

@frobware, would you mind retagging the PR?

/lgtm

openshift-ci · 2024-04-25T07:52:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, frobware, Miciah

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [deads2k]

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

openshift-ci · 2024-04-25T08:13:28Z

@Miciah: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot · 2024-04-25T08:15:50Z

@Miciah: Jira Issue OCPBUGS-31666: All pull requests linked via external trackers have merged:

openshift/api#1845

Jira Issue OCPBUGS-31666 has been moved to the MODIFIED state.

Details

In response to this:

Fix the godoc for the spec.tls.insecureEdgeTerminationPolicy field to say that "None", not "Allow", is the default value.

Also, add a more explicit statement about the default value.

Finally, fix the formatting so that the list displays properly in the output of oc explain. Before, the output looked like this:

% oc explain routes.spec.tls.insecureEdgeTerminationPolicy
KIND: Route
VERSION: route.openshift.io/v1

FIELD: insecureEdgeTerminationPolicy

DESCRIPTION:
insecureEdgeTerminationPolicy indicates the desired behavior for insecure
connections to a route. While each router may make its own decisions on
which ports to expose, this is normally port 80.
    * Allow - traffic is sent to the server on the insecure port
    (edge/reencrypt terminations only) (default). * None - no traffic is
    allowed on the insecure port. * Redirect - clients are redirected to the
    secure port.
After this change, the output looks like this:

% oc explain routes.spec.tls.insecureEdgeTerminationPolicy
KIND: Route
VERSION: route.openshift.io/v1

FIELD: insecureEdgeTerminationPolicy

DESCRIPTION:
insecureEdgeTerminationPolicy indicates the desired behavior for insecure
connections to a route. While each router may make its own decisions on
which ports to expose, this is normally port 80. If a route does not
specify insecureEdgeTerminationPolicy, then the default behavior is "None".
* Allow - traffic is sent to the server on the insecure port
(edge/reencrypt terminations only).
* None - no traffic is allowed on the insecure port (default).
* Redirect - clients are redirected to the secure port.

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-bot · 2024-04-25T11:27:28Z

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-cluster-config-api-container-v4.16.0-202404250649.p0.g8203151.assembly.stream.el9 for distgit ose-cluster-config-api.
All builds following this will include this PR.

Miciah · 2024-04-30T21:06:17Z

openshift/openshift-apiserver#433 bumps openshift/api in openshift-apiserver in order to incorporate the API godoc update from this PR.

openshift-merge-robot · 2024-05-09T13:22:47Z

Fix included in accepted release 4.16.0-0.nightly-2024-05-08-222442

openshift-ci bot requested a review from lihongan April 2, 2024 23:56

openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 2, 2024

openshift-ci bot requested review from JoelSpeed and knobunc April 2, 2024 23:57

Miciah force-pushed the OCPBUGS-31666-route-fix-insecureEdgeTerminationPolicy-default branch from 1c23023 to 8a46cb2 Compare April 2, 2024 23:57

openshift-ci bot assigned frobware Apr 3, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 3, 2024

openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 11, 2024

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 24, 2024

Miciah force-pushed the OCPBUGS-31666-route-fix-insecureEdgeTerminationPolicy-default branch from 8a46cb2 to b9885cc Compare April 24, 2024 22:13

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 24, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 25, 2024

openshift-merge-bot bot merged commit 8203151 into openshift:master Apr 25, 2024

Miciah mentioned this pull request May 2, 2024

OCPBUGS-31666: Bump openshift/api for Route API godoc fix openshift/openshift-apiserver#433

Merged

Conversation

Miciah commented Apr 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Apr 2, 2024

Uh oh!

openshift-ci bot commented Apr 2, 2024

Uh oh!

openshift-ci-robot commented Apr 2, 2024

Uh oh!

frobware commented Apr 3, 2024

Uh oh!

ShudiLi commented Apr 8, 2024

Uh oh!

deads2k commented Apr 11, 2024

Uh oh!

Miciah commented Apr 24, 2024

Uh oh!

openshift-ci-robot commented Apr 24, 2024

Uh oh!

openshift-ci-robot commented Apr 24, 2024

Uh oh!

Miciah commented Apr 24, 2024

Uh oh!

Miciah commented Apr 24, 2024

Uh oh!

frobware commented Apr 25, 2024

Uh oh!

openshift-ci bot commented Apr 25, 2024

Uh oh!

openshift-ci bot commented Apr 25, 2024

Uh oh!

openshift-ci-robot commented Apr 25, 2024

Uh oh!

openshift-bot commented Apr 25, 2024

Uh oh!

Miciah commented Apr 30, 2024

Uh oh!

openshift-merge-robot commented May 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Miciah commented Apr 2, 2024 •

edited

Loading