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 an field to avoid interpreting the hosts on dr and vs #2231

Closed
wants to merge 3 commits into from

Conversation

Patrick0308
Copy link

@Patrick0308 Patrick0308 commented Feb 7, 2022

See istio/istio#36864

Adding an option on virtual service's spec. if the field is true, istio will be not resolve short name to FQDN. Denstation rule is the same.

@istio-testing istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 7, 2022
@istio-testing
Copy link
Collaborator

Hi @Patrick0308. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@Patrick0308 Patrick0308 changed the title add an field is_fully_host on dr and vs add an field to avoid interpreting the hosts on dr and vs Feb 7, 2022
@ericvn
Copy link
Contributor

ericvn commented Feb 7, 2022

/ok-to-test

@istio-testing istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. needs-rebase Indicates a PR needs to be rebased before being merged and removed needs-ok-to-test needs-rebase Indicates a PR needs to be rebased before being merged labels Feb 7, 2022
@Patrick0308
Copy link
Author

Patrick0308 commented Feb 10, 2022

@ericvn @howardjohn @hzxuzhonghu. Hey guys, I want to know what's your opinion about this.

@istio-testing
Copy link
Collaborator

@Patrick0308: 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 1953b3e 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.

@louiscryan
Copy link
Contributor

What is the purpose of this change? I get that host interpretation can be complex - see DNS - but do we have a reason not to follow the DNS spec for host strings?

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 11, 2022
@istio-testing
Copy link
Collaborator

@Patrick0308: 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.

@Patrick0308
Copy link
Author

@louiscryan Pls see istio/istio#36864. When the service host is the k8s service host, the short name interpreted to FQDN. It's ok. However, if the host is for service entry, the interpretation is unwarranted.

@howardjohn howardjohn removed the needs-rebase Indicates a PR needs to be rebased before being merged label May 15, 2024
@istio-policy-bot
Copy link

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2022-02-11. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.

@istio-policy-bot istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants