Skip to content

Change address lookup behaviour if VAULT_SRV_LOOKUP is set. #14192

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gounselor
Copy link

@gounselor gounselor commented Feb 22, 2022

I think the behaviour of VAULT_SRV_LOOKUP is strange:
It should not expect http (what about a prod https setup?)
And actually the VAULT_ADDR should not contain a scheme in this case.

I made a minimal change to make it work for us.
We are using SRV records like _vault._tcp.example.domain which return
a list of hosts with the port appended.

The impure thing is that it overrides the scheme to be https, because i expect to use a SRV record approach in prod only which should https.

I am not 100% sure if this is the correct way, maybe we should discuss how to work with SRV records.

EDIT: fixed typos

It should not expect http (what about a prod https setup?)
Room for discussion.
@hashicorp-cla
Copy link

hashicorp-cla commented Feb 22, 2022

CLA assistant check
All committers have signed the CLA.

@vercel vercel bot temporarily deployed to Preview – vault-storybook February 22, 2022 13:01 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook February 22, 2022 13:04 Inactive
@heatherezell
Copy link
Contributor

Hello @gounselor - is this in relationship to an open issue? We ask that PRs be linked to an existing issue, so discussions about the desired solution can be hashed out there. Thanks!

@heatherezell heatherezell added the core Issues and Pull-Requests specific to Vault Core label Feb 22, 2022
@gounselor
Copy link
Author

gounselor commented Feb 23, 2022 via email

@heatherezell
Copy link
Contributor

Hi, no, it’s not but i could open one if this is required. Best regards, Michael

On 22. Feb 2022, at 23:05, Heather Simon @.***> wrote: Hello @gounselor - is this in relationship to an open issue? We ask that PRs be linked to an existing issue, so discussions about the desired solution can be hashed out there. Thanks! — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.

Please do - as I mentioned, we prefer to encourage discussion of the problem as well as its solution, to make sure that all the angles are covered. Thanks! :)

@heatherezell heatherezell linked an issue Feb 23, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Issues and Pull-Requests specific to Vault Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

strange net.LookupSRV behaviour
3 participants