Skip to content

Conversation

@kfaseela
Copy link
Member

RFC: auto-sni and auto-san support

Previous activity can be seen in #1773.

Signed-off-by: Faseela K faseela.k@est.tech

@istio-testing istio-testing added do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 11, 2022
@kfaseela kfaseela marked this pull request as draft March 11, 2022 20:06
@istio-testing
Copy link
Collaborator

@kfaseela: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
release-notes_api f84fd56 link false /test release-notes_api

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.

Copy link
Member

@nrjpoddar nrjpoddar left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I think we need to make sure UX is correct here as there are 2-3 fields all related to the same area and we need to understand how they all work together.

I also see that the RFC never got approved.

@kfaseela
Copy link
Member Author

Thanks for the PR. I think we need to make sure UX is correct here as there are 2-3 fields all related to the same area and we need to understand how they all work together.

I also see that the RFC never got approved.

@nrjpoddar Thanks for the review. Have added points in the RFC to clarify the behavior when auto-sni and sni fields come together. I presented the RFC only in the last working group meeting, so might need some more time to refine all usecases, and get the RFC approved, however the group was positive about the proposal. There was even a point whether we should be doing this without the need for an additional API change, and make this the default behavior. The discussion is still in progress. I thought it would be easier to comment on the code-review, so thought of pushing the changes.

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Mar 16, 2022
Signed-off-by: Faseela K <faseela.k@est.tech>
@istio-testing istio-testing added needs-rebase Indicates a PR needs to be rebased before being merged and removed needs-rebase Indicates a PR needs to be rebased before being merged labels Mar 22, 2022
@istio-testing
Copy link
Collaborator

@kfaseela: PR needs rebase.

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.

@kfaseela
Copy link
Member Author

As per the latest discussions in the networking working group, an API change is not needed for supporting the same. The details are updated in the RFC, and closing this PR. Will be working on the agreed upon approach

@kfaseela kfaseela closed this Mar 25, 2022
@kfaseela kfaseela deleted the auto-sni-san branch August 5, 2022 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. needs-rebase Indicates a PR needs to be rebased before being merged size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants