-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
VMWare vCloud Director Support #3785
Conversation
|
||
func resourceVcdDNATUpdate(d *schema.ResourceData, meta interface{}) error { | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's no implementation for Update
it's better to just omit this function from the Resource definition. That will also trigger a check to ensure that all the fields are set to ForceNew
, which they should be when an Update is not available.
Wow, this is looking really great! Tests and docs and everything. 👍 I just completed a review with a bunch of inline comments. Let me know if you have any follow up questions on those. Great work so far! After we address the comments this should be good to go. 🚀 |
@phinze Do you need us to version the dependent github.com/hmrc/vmware-govcd library? I am not sure how these versions get into appropriate terraform/deps/v0-6-X.json files, I presume this automatically pulls in appropriate version info and code and vendors them at release time? |
@nickithewatt Yep - no need to explicitly pin the dependency. Our policy is to track master between releases and snapshot at release time, so we'll capture reproducible build information with each release. |
Hi Paul, Thank you for the comments, they were a great help. I believe these have all been addressed now but let me know if you think there's anything else I should change. |
Make maxRetryTimeout (in seconds) configurable
@phinze Sorry, added one final change. That should be everything now though. |
Looking good! Let's land this sucker. 🍭 |
Terraform vcd provider seems to work fine for me in my inital tests.. Thanks a lot! There's one issue/limitation I noticed though.. I'd like to be able to specify different "original port" and "translated port" for DNAT rules.. currently it's only possible to specify "port", so both the "original port" and "translated port" will be the same, which isn't optimal. So basicly I'd like to support for example multiple port forwards to multiple ssh jump boxes, like this; and so on. I guess I should check the sources to see if it's easy to modify and add support for that.. |
Ok, so in vmware-govcd edgegateway.go func AddNATMapping(nattype, externalIP, internalIP, port string) I can see this:
so "port" is used for both OriginalPort and TranslatedPort.. so the limitation seems to come from that level. |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
In regards to issue #3784 this PR addresses support for vCloud Director with supporting tests and documentation