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

[release-3.11] Bug 1884422: Backport router hostindex fixes #25891

Merged
merged 2 commits into from Aug 12, 2021

Conversation

Miciah
Copy link
Contributor

@Miciah Miciah commented Feb 11, 2021

Backport openshift/router#57 and openshift/router#59.

hostindex: Passthrough displaces path-based TLS

Ensure that a passthrough route displaces any path-based TLS routes with the same host, because passthrough is incompatible with path-based routing.

  • pkg/router/controller/hostindex/activation.go (hasExistingMatch): Return true if both routes are TLS and the existing route is a passthrough route.
  • pkg/router/controller/hostindex/hostindex_test.go (Test_hostIndex): Verify that a passthrough route displaces path-based TLS routes with the same host, but does not displace non-TLS routes.

hostindex: Path-based TLS displaces passthrough

Just as a passthrough route displaces any path-based TLS routes with the same host, a path-based TLS route displaces any passthrough route with the same host.

  • pkg/router/controller/hostindex/activation.go (hasExistingMatch): Return true if both routes are TLS and either route is passthrough.
  • pkg/router/controller/hostindex/hostindex_test.go (Test_hostIndex): Verify that a path-based TLS route displaces passthrough routes with the same host.

Ensure that a passthrough route displaces any path-based TLS routes with
the same host, because passthrough is incompatible with path-based routing.

This commit fixes bug 1691190.

https://bugzilla.redhat.com/show_bug.cgi?id=1691190

* pkg/router/controller/hostindex/activation.go (hasExistingMatch): Return
true if both routes are TLS and the existing route is a passthrough route.
* pkg/router/controller/hostindex/hostindex_test.go (Test_hostIndex):
Verify that a passthrough route displaces path-based TLS routes with the
same host, but does not displace non-TLS routes.
Just as a passthrough route displaces any path-based TLS routes with the
same host, a path-based TLS route displaces any passthrough route with the
same host.

Follow-up to commit 65e784fe491ef02c4db0346348e6ac6192d68e30.

* pkg/router/controller/hostindex/activation.go (hasExistingMatch): Return
true if both routes are TLS and *either* route is passthrough.
* pkg/router/controller/hostindex/hostindex_test.go (Test_hostIndex):
Verify that a path-based TLS route displaces passthrough routes with the
same host.
@openshift-ci-robot openshift-ci-robot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Feb 11, 2021
@openshift-ci-robot
Copy link

@Miciah: This pull request references Bugzilla bug 1884422, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (3.11.z) matches configured target release for branch (3.11.z)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

[release-3.11] Bug 1884422: Backport router hostindex fixes

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.

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Miciah

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 11, 2021
@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 13, 2021
@openshift-bot
Copy link
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 12, 2021
@openshift-bot
Copy link
Contributor

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 12, 2021

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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.

@openshift-ci openshift-ci bot closed this Jul 12, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 12, 2021

@Miciah: This pull request references Bugzilla bug 1884422. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state.

In response to this:

[release-3.11] Bug 1884422: Backport router hostindex fixes

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.

@Miciah
Copy link
Contributor Author

Miciah commented Jul 31, 2021

/reopen
/remove-lifecycle rotten

@openshift-ci openshift-ci bot reopened this Jul 31, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 31, 2021

@Miciah: Reopened this PR.

In response to this:

/reopen
/remove-lifecycle rotten

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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 31, 2021

@Miciah: An error was encountered searching for bug 1884422 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details.

Full error message. response code 502 not 200

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

In response to this:

[release-3.11] Bug 1884422: Backport router hostindex fixes

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.

@openshift-ci openshift-ci bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jul 31, 2021
@frobware
Copy link
Contributor

frobware commented Aug 2, 2021

/retest

@frobware
Copy link
Contributor

frobware commented Aug 2, 2021

Not sure if this the cause for both test failures:

error: shallow cloning repository "https://github.com/sclorg/nodejs-ex" to "/tmp/gen052590313" failed: fatal: unable to access 'https://github.com/sclorg/nodejs-ex/': Failed connect to github.com:443; Connection timed out

/retest

@frobware
Copy link
Contributor

frobware commented Aug 3, 2021

/retest

@Miciah
Copy link
Contributor Author

Miciah commented Aug 3, 2021

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 3, 2021

@Miciah: This pull request references Bugzilla bug 1884422, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (3.11.z) matches configured target release for branch (3.11.z)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @quarterpin

In response to this:

/bugzilla refresh

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.

@openshift-ci openshift-ci bot requested a review from quarterpin August 3, 2021 16:19
@Miciah
Copy link
Contributor Author

Miciah commented Aug 3, 2021

/test unit

@candita
Copy link
Contributor

candita commented Aug 12, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 12, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: candita, Miciah

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

@openshift-ci openshift-ci bot merged commit 107467e into openshift:release-3.11 Aug 12, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 12, 2021

@Miciah: All pull requests linked via external trackers have merged:

Bugzilla bug 1884422 has been moved to the MODIFIED state.

In response to this:

[release-3.11] Bug 1884422: Backport router hostindex fixes

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants