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

provider/vsphere: Add insecure option for 'insecure' SSL requests #3933

Closed
wants to merge 1 commit into from

Conversation

tkak
Copy link
Contributor

@tkak tkak commented Nov 16, 2015

This fixes #3521.

@@ -29,6 +29,13 @@ func Provider() terraform.ResourceProvider {
DefaultFunc: schema.EnvDefaultFunc("VSPHERE_VCENTER", nil),
Description: "The vCenter Server name for vSphere API operations.",
},

"insecure": &schema.Schema{
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should consider naming this allow_unverified_ssl such that it consistent with the Rundeck provider, and in order that it describes more clearly exactly which form of insecurity is invited?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 lets use allow_unverified_ssl as the parameter name here.

@jen20
Copy link
Contributor

jen20 commented Nov 16, 2015

Hi @tkak! Thanks for opening up this PR. I've left some comments inline; could you review them and let me know your thoughts?

@chrislovecnm
Copy link
Contributor

@tkak you have a couple of moments to tweak the name??

@tkak
Copy link
Contributor Author

tkak commented Nov 20, 2015

@jen20 Thanks for the review! For the reason of insecure, I referenced the usage of VMware vSphere go library.
https://github.com/vmware/govmomi/blob/8f2d79fd740b69c4f64d10e8ac0b8c6b30247c0d/examples/datastores/main.go#L70.

And insecure parameter is used at the OpenStack provider.

@jen20
Copy link
Contributor

jen20 commented Nov 25, 2015

Hi @tkak - In this case I still think allow_unverified_ssl is better than insecure (and that we should actually update the OpenStack provider as well and deprecate insecure).

@jtopjian
Copy link
Contributor

(I have gmail filters for "openstack" so I saw this conversation 😄 )

I'm mixed on the rename. On one hand, I totally agree that allow_unverified_ssl is more intuitive than insecure, but on the other hand, insecure is used widely outside of Terraform.

Edit for clarification: insecure has almost become a standard option for any type of SSL bypass. For example, man wget and man curl and search for insecure.

@chrislovecnm
Copy link
Contributor

@jen20 @jtopjian @phinze - who can make a call? I would like to just get this in 😆

@tkak
Copy link
Contributor Author

tkak commented Dec 2, 2015

@jen20 I've fixed insecure parameter to allow_unverified_ssl 😄 . Because I think intuitiveness is more important than precedents for TF configuration file.

@phinze
Copy link
Contributor

phinze commented Dec 2, 2015

This looks good - just needs a rebase and we can get it merged. 👍

@phinze
Copy link
Contributor

phinze commented Dec 2, 2015

I too prefer the more intuitive name over respecting precedent, provided we are doing what we can to remain consistent within Terraform.

@tkak
Copy link
Contributor Author

tkak commented Dec 3, 2015

@phinze I tried to rebase. 👍

@phinze phinze closed this in 5985220 Dec 4, 2015
omeid pushed a commit to omeid/terraform that referenced this pull request Mar 30, 2018
…nal-regex-pattern-set

New Resource: aws_wafregional_regex_pattern_set
@ghost
Copy link

ghost commented Apr 29, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vsphere: turn on SSL verification and expose configuration option to disable
6 participants