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 wrapper function for go-sockaddr templating #1087

Closed

Conversation

thevilledev
Copy link
Contributor

This PR bundles the many templating functionalities offered by hashicorp/go-sockaddr to consul-template. These template functionalities are especially useful in cases where consul-template is run upon system startup, for example by cloud-init, to render interface IP addresses to configuration files.

Also adds vendoring for go-sockaddr, which might make this PR look like a bit daunting (1,1M additions).

Documentation updated as well.

@pearkes
Copy link
Contributor

pearkes commented Jun 8, 2018

I think there may be a mistake in the vendoring here -- I don't think all of these are dependencies required by go-sockaddr. Can you review how you did the vendoring and double check you're only pulling in the necessary deps?

@pearkes
Copy link
Contributor

pearkes commented Jun 8, 2018

Thanks for the contribution!

@thevilledev thevilledev force-pushed the feature/go-sockaddr-support branch from 412ef07 to 4b2d7e8 Compare June 14, 2018 11:53
@thevilledev
Copy link
Contributor Author

Thanks @pearkes for the heads up. I just did dep ensure -add github.com/hashicorp/go-sockaddr/template while doing this and it appears to add a bunch of other non-related dependencies as well. If you clone consul-template and just run dep ensure you'll see that it fetches a bunch of stuff that it thinks are missing from vendoring. Is this expected?

Anyway, I fixed the vendoring in this PR to just include github.com/hashicorp/go-sockaddr. Only other package it depends on is github.com/hashicorp/errwrap but it's already included in the project vendoring.

Let me know if there's anything else we need to do before merging.

@aaglenn
Copy link

aaglenn commented Jul 23, 2018

will this get merged before the next release? I have an admittedly odd use case, specifically...but I would really, really enjoy having this in consul-template.

@eikenb
Copy link
Contributor

eikenb commented Jun 19, 2019

Hey @vtorhonen, since you submitted your pull request we've implemented a policy of requiring a signed CLA before merging. Would you please consider signing the CLA? Thanks.

@thevilledev thevilledev force-pushed the feature/go-sockaddr-support branch from 4b2d7e8 to 5870f91 Compare August 10, 2019 12:32
@hashicorp-cla
Copy link

hashicorp-cla commented Aug 10, 2019

CLA assistant check
All committers have signed the CLA.

@thevilledev thevilledev force-pushed the feature/go-sockaddr-support branch from 5870f91 to ff2466d Compare August 10, 2019 12:37
@thevilledev
Copy link
Contributor Author

@eikenb CLA signed and branch rebased. Sorry for the wait. :)

@eikenb
Copy link
Contributor

eikenb commented Aug 14, 2019

@vtorhonen No problem. Thanks for doing that.

I'm making one more bug sweep right now, but after I plan on reviewing all outstanding PRs to see what I can merge. I'll get to this then (think a couple weeks).

Thanks.

@eikenb
Copy link
Contributor

eikenb commented Aug 21, 2019

Hey @vtorhonen, reviewing all the PRs now and would like to ask if you could add a simple test in funcs_test.go for this. It won't be a blocker as this is such a simple function, but I've been asking all the other new functions for one and really would like to have better coverage of the functions directly (vs. going through the template rendering).

Thanks.

@eikenb eikenb added this to the 0.22.0 - Community PR focused milestone Aug 28, 2019
@eikenb eikenb closed this in 2734e51 Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants